Re: [RFC PATCH 0/2] target/i386: SEV: allow running SNP guests with "-cpu host"

[Michael Roth]

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
>