Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

[Brijesh Singh]

Hi Boris and Paolo,

On 03/09/2017 10:29 AM, Borislav Petkov wrote:
> On Thu, Mar 09, 2017 at 05:13:33PM +0100, Paolo Bonzini wrote:
> > This is not how you check if running under a hypervisor; you should
> > check the HYPERVISOR bit, i.e. bit 31 of cpuid(1).ecx.  This in turn
> > tells you if leaf 0x40000000 is valid.
>
> Ah, good point, I already do that in the microcode loader :)
>
>         /*
>          * CPUID(1).ECX[31]: reserved for hypervisor use. This is still not
>          * completely accurate as xen pv guests don't see that CPUID bit set but
>          * that's good enough as they don't land on the BSP path anyway.
>          */
>         if (native_cpuid_ecx(1) & BIT(31))
>                 return *res;
>
> > That said, the main issue with this function is that it hardcodes the
> > behavior for KVM.  It is possible that another hypervisor defines its
> > 0x40000001 leaf in such a way that KVM_FEATURE_SEV has a different meaning.
> >
> > Instead, AMD should define a "well-known" bit in its own space (i.e.
> > 0x800000xx) that is only used by hypervisors that support SEV.  This is
> > similar to how Intel defined one bit in leaf 1 to say "is leaf
> > 0x40000000 valid".
> >
> > > +	if (eax > 0x40000000) {
> > > +		eax = 0x40000001;
> > > +		ecx = 0;
> > > +		native_cpuid(&eax, &ebx, &ecx, &edx);
> > > +		if (!(eax & BIT(KVM_FEATURE_SEV)))
> > > +			goto out;
> > > +
> > > +		eax = 0x8000001f;
> > > +		ecx = 0;
> > > +		native_cpuid(&eax, &ebx, &ecx, &edx);
> > > +		if (!(eax & 1))
>
> Right, so this is testing CPUID_0x8000001f_ECX(0)[0], SME. Why not
> simply set that bit for the guest too, in kvm?

CPUID_8000_001F[EAX] indicates whether the feature is supported.
CPUID_0x8000001F[EAX]:
 * Bit 0 - SME supported
 * Bit 1 - SEV supported
 * Bit 3 - SEV-ES supported

We can use MSR_K8_SYSCFG[MemEncryptionModeEnc] to check if memory encryption is enabled.
Currently, KVM returns zero when guest OS read MSR_K8_SYSCFG. I can update my patch sets
to set this bit for SEV enabled guests.

We could update this patch to use the below logic:

 * CPUID(0) - Check for AuthenticAMD
 * CPID(1) - Check if under hypervisor
 * CPUID(0x80000000) - Check for highest supported leaf
 * CPUID(0x8000001F).EAX - Check for SME and SEV support
 * rdmsr (MSR_K8_SYSCFG)[MemEncryptionModeEnc] - Check if SMEE is set


Paolo,

One question, do we need "AuthenticAMD" check when we are running under hypervisor ?
I was looking at qemu code and found that qemu exposes parameters to change the CPU
vendor id. The above check will fail if user changes the vendor id while launching
the SEV guest.

-Brijesh