Messages in this thread | | | Date | Tue, 1 Aug 2023 16:31:57 -0500 | Subject | Re: [PATCH] x86/sev: Fix SNP CPUID requests to the hypervisor | From | Tom Lendacky <> |
| |
On 7/31/23 22:58, Dionna Amalie Glaze wrote: >> Fixes: ee0bfa08a345 ("x86/compressed/64: Add support for SEV-SNP CPUID table in #VC handlers") >> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > > Nice catch. > >> +static int __sev_cpuid_hv_ghcb(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf) >> +{ >> + u32 cr4 = native_read_cr4(); >> + int ret; >> + >> + ghcb_set_rax(ghcb, leaf->fn); >> + ghcb_set_rcx(ghcb, leaf->subfn); >> + >> + if (cr4 & X86_CR4_OSXSAVE) >> + /* Safe to read xcr0 */ >> + ghcb_set_xcr0(ghcb, xgetbv(XCR_XFEATURE_ENABLED_MASK)); >> + else >> + /* xgetbv will cause #GP - use reset value for xcr0 */ >> + ghcb_set_xcr0(ghcb, 1); >> + > > Everything looks good except I'm confused by this last comment. I > thought xgetbv would #UD if OSXSAVE isn't set in cr4. Is that what > happens after you set it to 1 as some kind of workaround?
Yes, after checking the APM, it would #UD. This code was copied from the existing CPUID #VC support. Not sure it requires a new version or if the maintainers could fix it up when applying.
Thanks, Tom
> > ....oh I think I do remember hitting this weird technicality when > setting up CPUID support. I think it'd be helpful to document a little > bit more why the ghcb's value is xcr0 is relevant at all when cr4's > OSXSAVE bit is 0 though. > >
| |