Some CPUID features may be provided by KVM for some guests, independent of
processor support, for example TSC deadline or TSC adjust.  They are not going
to be present in named models unless the vendor implements them in hardware,
but they will be present in "-cpu host".

If these bits are not supported by the confidential computing firmware,
however, the guest will fail to start, and indeed this is a problem when
you run SNP guests with "-cpu host".  This series fixes the issue.

However, I am marking this as RFC because it's not future proof.
If in the future AMD processors do provide any of these bits, this is
going to break (tsc_deadline and tsc_adjust are the most likely one).
Including the bits if they are present in host CPUID is not super safe
either, since the firmware might not be updated to follow suit.

Michael, any ideas?  Is there a way for the host to retrieve the supported
CPUID bits for SEV-SNP guests?

One possibility is to set up a fake guest---either in QEMU or when KVM
starts---to do a LAUNCH_UPDATE for the CPUID page, but even that is not
perfect.  For example, I got

 > function 0x7, index: 0x0 provided: edx: 0xbc000010, expected: edx: 0x00000000

even though the FSRM bit (0x10) is supported.  That might be just a
firmware bug however.

Paolo

Based-on: <20240627140628.1025317-1-pbonzini@redhat.com>

Paolo Bonzini (4):
  target/i386: add support for masking CPUID features in confidential
    guests
  target/i386/SEV: implement mask_cpuid_features

 target/i386/confidential-guest.h | 24 ++++++++++++++++++++++++
 target/i386/cpu.c                |  9 +++++++++
 target/i386/cpu.h                |  4 ++++
 target/i386/kvm/kvm.c            |  5 +++++
 target/i386/sev.c                | 33 +++++++++++++++++++++++++++++++++
 5 files changed, 75 insertions(+)

-- 
2.45.2

On Wed, Jul 03, 2024 at 01:01:32PM +0200, Paolo Bonzini wrote:
> Some CPUID features may be provided by KVM for some guests, independent of
> processor support, for example TSC deadline or TSC adjust.  They are not going
> to be present in named models unless the vendor implements them in hardware,
> but they will be present in "-cpu host".
> 
> If these bits are not supported by the confidential computing firmware,
> however, the guest will fail to start, and indeed this is a problem when
> you run SNP guests with "-cpu host".  This series fixes the issue.
> 
> However, I am marking this as RFC because it's not future proof.
> If in the future AMD processors do provide any of these bits, this is
> going to break (tsc_deadline and tsc_adjust are the most likely one).
> Including the bits if they are present in host CPUID is not super safe
> either, since the firmware might not be updated to follow suit.
> 
> Michael, any ideas?  Is there a way for the host to retrieve the supported
> CPUID bits for SEV-SNP guests?

If we want to support -cpu host, then I don't really see a way around
needing to maintain a filter of some sort sanitize what gets passed to
firmware. Generally, every new CPU model is likely to have some features
which might be a liability security-wise to allow in SNP guests, so the
CPUID validation is sort of a whitelist of curated features that make
sense for guests and can be enabled securely in the context of SNP.

Everything else would need to be filtered out, so we'd need to keep that
list constantly updated.

I think that may be possible, but do we have a strong use-case for
supporting -cpu host in conjunction with SNP guests that this would be
a worthwhile endeavor?

> 
> One possibility is to set up a fake guest---either in QEMU or when KVM
> starts---to do a LAUNCH_UPDATE for the CPUID page, but even that is not
> perfect.  For example, I got

Yah, the firmware-provided responses are more of a debug tool and not
something I think we can rely on to enumerate capabilities.

You could in theory take the ruleset in the PPR (Chapter 2, CPUID Policy
Enforcement), turn that into something programmatic, and apply that
against the host's CPUID values, but the policies are a bit more
specific in some cases, and the PPR is per-CPU-model so both the rules
and inputs can change from one host to the next.

So I don't see a great way to leverage that to make things easier here.
The manually-maintained filter you've proposed here seems more reliable
to me.

> 
>  > function 0x7, index: 0x0 provided: edx: 0xbc000010, expected: edx: 0x00000000
> 
> even though the FSRM bit (0x10) is supported.  That might be just a
> firmware bug however.

That's a possibility. It seems like 'BitMask' fields (as documented in
the PPR section "CPUID Policy Enforcement") generally return the AND of
what the host supports with what is passed in. I'll look into this a
bit more and raise a ticket with firmware folks if it is unexpected.

Thanks,

Mike

> 
> Paolo
> 
> Based-on: <20240627140628.1025317-1-pbonzini@redhat.com>
> 
> Paolo Bonzini (4):
>   target/i386: add support for masking CPUID features in confidential
>     guests
>   target/i386/SEV: implement mask_cpuid_features
> 
>  target/i386/confidential-guest.h | 24 ++++++++++++++++++++++++
>  target/i386/cpu.c                |  9 +++++++++
>  target/i386/cpu.h                |  4 ++++
>  target/i386/kvm/kvm.c            |  5 +++++
>  target/i386/sev.c                | 33 +++++++++++++++++++++++++++++++++
>  5 files changed, 75 insertions(+)
> 
> -- 
> 2.45.2
>

On Thu, Jul 4, 2024 at 2:26 AM Michael Roth <michael.roth@amd.com> wrote:
> > Michael, any ideas?  Is there a way for the host to retrieve the supported
> > CPUID bits for SEV-SNP guests?
>
> If we want to support -cpu host, then I don't really see a way around
> needing to maintain a filter of some sort sanitize what gets passed to
> firmware. Generally, every new CPU model is likely to have some features
> which might be a liability security-wise to allow in SNP guests, so the
> CPUID validation is sort of a whitelist of curated features that make
> sense for guests and can be enabled securely in the context of SNP.
>
> Everything else would need to be filtered out, so we'd need to keep that
> list constantly updated.

It would be per new model and right now there are only a handful of
bits that have to be blocked; so it wouldn't be particularly bad.

> I think that may be possible, but do we have a strong use-case for
> supporting -cpu host in conjunction with SNP guests that this would be
> a worthwhile endeavor?

It's a common way to launch a guest if you're not interested in
migration (which is obviously the case for SNP right now), so it's
more like "why not". :)

> > One possibility is to set up a fake guest---either in QEMU or when KVM
> > starts---to do a LAUNCH_UPDATE for the CPUID page, but even that is not
> > perfect.  For example, I got
>
> Yah, the firmware-provided responses are more of a debug tool and not
> something I think we can rely on to enumerate capabilities.
>
> You could in theory take the ruleset in the PPR (Chapter 2, CPUID Policy
> Enforcement), turn that into something programmatic, and apply that
> against the host's CPUID values, but the policies are a bit more
> specific in some cases, and the PPR is per-CPU-model so both the rules
> and inputs can change from one host to the next.

Yeah, and if you mix that with knowledge of what KVM can/cannot
virtualize that doesn't exist in the processor (which isn't that
much), then you end up with something a lot like patch 2

It would be nice if the policy enforcement were changed to allow the
TSC deadline timer and X2APIC bits (you probably don't want TSC
adjust, that's the right call; and virt SSBD is not accessible because
you use V_SPEC_CTRL instead). But then there would be no way to find
out if the change actually happened.

> So I don't see a great way to leverage that to make things easier here.
> The manually-maintained filter you've proposed here seems more reliable
> to me.

Yep, I think I'll include that patch as the maintainability doesn't seem bad.

Paolo