Re: [PATCH v2 1/6] KVM: x86: Clear all supported MPX xfeatures if they are not all set

[Mingwei Zhang <mizhang@google.com>]

On Tue, Jan 10, 2023, Chang S. Bae wrote:
> On 1/10/2023 6:49 AM, Aaron Lewis wrote:
> > 
> > When I run xcr0_cpuid_test it fails because
> > xstate_get_guest_group_perm() reports partial support on SPR.  It's
> > reporting 0x206e7 rather than the 0x6e7 I was hoping for.  That's why
> > I went down the road of sanitizing xcr0.  Though, if it's expected for
> > that to report something valid then sanitizing seems like the wrong
> > approach.  If xcr0 is invalid it should stay invalid, and it should
> > cause a test to fail.
> 
> FWIW, we have this [1]:
> 
> /* Features which are dynamically enabled for a process on request */
> #define XFEATURE_MASK_USER_DYNAMIC	XFEATURE_MASK_XTILE_DATA
> 
> IOW, TILE_CFG is not part of the dynamic state. Because this state is not
> XFD-supported, we can't enforce the state use. SDM has relevant text here
> [2]:
> 
> "LDTILECFG and TILERELEASE initialize the TILEDATA state component. An
> execution of either of these instructions does not generate #NM when
> XCR0[18] = IA32_XFD[18] = 1; instead, it initializes TILEDATA normally.
> (Note that STTILECFG does not use the TILEDATA state component. Thus, an
> execution of this instruction does
> not generate #NM when XCR0[18] = IA32_XFD[18] = 1.)"
> 
> > Looking at how xstate_get_guest_group_perm() comes through with
> > invalid bits I came across this commit:
> > 
> > 2308ee57d93d ("x86/fpu/amx: Enable the AMX feature in 64-bit mode")
> > 
> > -       /* [XFEATURE_XTILE_DATA] = XFEATURE_MASK_XTILE, */
> > +       [XFEATURE_XTILE_DATA] = XFEATURE_MASK_XTILE_DATA,
> > 
> > Seems like it should really be:
> > 
> > +       [XFEATURE_XTILE_DATA] = XFEATURE_MASK_XTILE,
> 
> Thus, the change was intentional as far as I can remember.
> 
> Thank,
> Chang
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/include/asm/fpu/xstate.h#n50
> [2] SDM Vol 1. 13.14 Extended Feature Disable (XFD), https://www.intel.com/content/www/us/en/developer/articles/technical/intel-sdm.html
> 

Hmm. Given that XFEATURE_XTILE_DATA is a dynamic feature, that means
XFEATURE_XTILE_CFG could be set without XFEATURE_XTILE_DATA. Shall we
also consider loose the constraints at __kvm_set_xcr() as well?

https://elixir.bootlin.com/linux/v6.1.4/source/arch/x86/kvm/x86.c#L1079