Re: [PATCH] x86/fpu: Fix the os panic issue caused by the XGETBV instruction

[Sean Christopherson <seanjc@google.com>]

On Wed, Jan 15, 2025, Chang S. Bae wrote:
> On 1/1/2025 11:54 PM, Tony W Wang-oc wrote:
> > From: Lyle Li <LyleLi@zhaoxin.com>
> > 
> > The callers of the xfeatures_in_use function must ensure that the
> > current processor has the X86_FEATURE_XGETBV1 feature. However, in some
> > places where xfeatures_in_use is called, there is no check to see if the
> > processor supports this feature, leading to the execution of the XGETBV
> > XCR1 instruction on processors that do not support this feature,
> > triggering a #GP exception, and ultimately causing an OS panic.
> 
> I doubt this is a real issue. An XFD implementation without XGETBV1 is
> considerably broken; every AMX system includes XGETBV1. Similarly, as far as
> I can see, PKU implementations also include XGETBV1. QEMU's CPU feature list
> [1] seems consistent with this.

QEMU's CPU models are not authoritative when it comes to architecture, they are
purely what CPUs hardware vendors have shipped.  The absense of a model with PKU
but not XGETBV1 simply reflects that neither Intel nor AMD have ever shipped such
a model.

> Maybe a wild clearcpuid use may clear off the XGETBV1 flag. Adding this
> dependency to the table would make the relationship explicit:
> 
> static const struct cpuid_dep cpuid_deps[] = {
> ...
> +       { X86_FEATURE_PKU,                      X86_FEATURE_XGETBV1   },
>         {}
>  };
> 
> Note that XFD is already listed as dependent on XGETBV1.
> 
> But I doubt the kernel needs to be resilient to deliberately misconfigured
> or crazy virtual machine setups.

I don't see anything in the SDM that suggests this is a misconfigured CPU.  Intel
might not have plans to ship such CPUs, but AFAICT it's not a violation of the
architecture as defined in the SDM.

The SDM even explicitly says that protection keys can exist and be used without
PKU state being supported in XSAVE at all, at which point assuming the existence
of XGETBV1 is rather nonsensical.

  XCR0[9] is associated with PKRU state (see Section 13.5.7). Software can use
  the XSAVE feature set to manage PKRU state only if XCR0[9] = 1. The value of
  XCR0[9] in no way determines whether software can use protection keys or execute
  other instructions that access PKRU state (these instructions can be executed even
  if XCR0[9] = 0).

  XCR0[9] is 0 coming out of RESET. As noted in Section 13.2, a processor allows
  software to set XCR0[9] if and only if CPUID.(EAX=0DH,ECX=0):EAX[9] = 1.