Re: [PATCH 07/11] x86/sev: Provide SVSM discovery support

[Tom Lendacky]

On 1/29/24 04:41, Jeremi Piotrowski wrote:
> On Fri, Jan 26, 2024 at 04:16:00PM -0600, Tom Lendacky wrote:
> > The SVSM specification documents an alternative method of discovery for
> > the SVSM using a reserved CPUID bit and a reserved MSR.
> >
> > For the CPUID support, the #VC handler of an SEV-SNP guest should modify
> > the returned value in the EAX register for the 0x8000001f CPUID function
> > by setting bit 28 when an SVSM is present.
>
> It seems awkward that the guest would have to set the bit in the CPUID response
> itself. Is there no way for the SVSM to fixup the CPUID page contents, or the
> CPUID instruction return?

It's possible for the SVSM to update the CPUID information provided to the 
guest, but it hasn't been specifically called out as a requirement in the 
SVSM spec. That is why snp_cpuid_postprocess() is used to ensure that the 
bit is set. But, yes, we could update the CPUID information in the CPUID 
page directly instead of the snp_cpuid_postprocess() change.

I'll look into that and see what it takes.

> Could you also add a definition for the feature to arch/x86/include/asm/cpufeatures.h.

Yes, I missed that, will add it.

Thanks,
Tom

> > For the MSR support, new reserved MSR 0xc001f000 has been defined. A #VC
> > should be generated when accessing this MSR. The #VC handler is expected
> > to ignore writes to this MSR and return the physical calling area address
> > (CAA) on reads of this MSR.
> >
> > Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> > ---
> >   arch/x86/include/asm/msr-index.h |  2 ++
> >   arch/x86/kernel/sev-shared.c     |  4 ++++
> >   arch/x86/kernel/sev.c            | 17 +++++++++++++++++
> >   3 files changed, 23 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> > index f1bd7b91b3c6..4746135cbe21 100644
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -622,6 +622,8 @@
> >   
> >   #define MSR_AMD64_VIRT_SPEC_CTRL	0xc001011f
> >   
> > +#define MSR_SVSM_CAA			0xc001f000
> > +
> >   /* AMD Collaborative Processor Performance Control MSRs */
> >   #define MSR_AMD_CPPC_CAP1		0xc00102b0
> >   #define MSR_AMD_CPPC_ENABLE		0xc00102b1
> > diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> > index f26e872bc5d0..9bd7d7e75b31 100644
> > --- a/arch/x86/kernel/sev-shared.c
> > +++ b/arch/x86/kernel/sev-shared.c
> > @@ -628,6 +628,10 @@ static int snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
> >   		/* node ID */
> >   		leaf->ecx = (leaf->ecx & GENMASK(31, 8)) | (leaf_hv.ecx & GENMASK(7, 0));
> >   		break;
> > +	case 0x8000001f:
> > +		if (vmpl)
> > +			leaf->eax |= BIT(28);
> > +		break;
> >   	default:
> >   		/* No fix-ups needed, use values as-is. */
> >   		break;