Re: [PATCH 7/9] x86/sev-es: Include XSS value in GHCB CPUID request

[John Allen]

On Thu, Oct 12, 2023 at 02:59:24PM +0200, Borislav Petkov wrote:
> On Tue, Oct 10, 2023 at 08:02:18PM +0000, John Allen wrote:
> > When a guest issues a cpuid instruction for Fn0000000D_x0B (CetUserOffset), the
> > hypervisor may intercept and access the guest XSS value. For SEV-ES, this is
> > encrypted and needs to be included in the GHCB to be visible to the hypervisor.
> > The rdmsr instruction needs to be called directly as the code may be used in
> > early boot in which case the rdmsr wrappers should be avoided as they are
> > incompatible with the decompression boot phase.
> > 
> > Signed-off-by: John Allen <john.allen@xxxxxxx>
> > ---
> >  arch/x86/kernel/sev-shared.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> > index 2eabccde94fb..e38a1d049bc1 100644
> > --- a/arch/x86/kernel/sev-shared.c
> > +++ b/arch/x86/kernel/sev-shared.c
> > @@ -890,6 +890,21 @@ static enum es_result vc_handle_cpuid(struct ghcb *ghcb,
> >  		/* xgetbv will cause #GP - use reset value for xcr0 */
> >  		ghcb_set_xcr0(ghcb, 1);
> >  
> > +	if (has_cpuflag(X86_FEATURE_SHSTK) && regs->ax == 0xd && regs->cx <= 1) {
> > +		unsigned long lo, hi;
> > +		u64 xss;
> > +
> > +		/*
> > +		 * Since vc_handle_cpuid may be used during early boot, the
> > +		 * rdmsr wrappers are incompatible and should not be used.
> > +		 * Invoke the instruction directly.
> > +		 */
> > +		asm volatile("rdmsr" : "=a" (lo), "=d" (hi)
> > +			     : "c" (MSR_IA32_XSS));
> 
> Does __rdmsr() work too?
> 
> I know it has exception handling but a SEV-ES guest should not fault
> when accessing MSR_IA32_XSS anyway, especially if it has shadow stack
> enabled. And if it does fault, your version would explode too but
> __rdmsr() would be at least less code. :)

I looked into using __rdmsr in an earlier revision of the patch, but
found that it causes a build warning:

ld: warning: orphan section `__ex_table' from `arch/x86/boot/compressed/sev.o' being placed in section `__ex_table'

This is due to the __ex_table section not being used during
decompression boot. Do you know of a way around this?

Thanks,
John