Re: [PATCH v2 1/2] KVM: selftests: Assert that XSAVE supports XTILE in amx_test

[Sean Christopherson <seanjc@xxxxxxxxxx>]

On Tue, Jan 03, 2023, Sean Christopherson wrote:
> On Fri, Dec 30, 2022, Aaron Lewis wrote:
> > The check in amx_test that ensures that XSAVE supports XTILE, doesn't
> > actually check anything.  It simply returns a bool which the test does
> > nothing with.
> > 
> > Assert that XSAVE supports XTILE.
> > 
> > Fixes: 5dc19f1c7dd3 ("KVM: selftests: Convert AMX test to use X86_PROPRETY_XXX")
> 
> Doh.
> 
> > Signed-off-by: Aaron Lewis <aaronlewis@xxxxxxxxxx>
> > ---
> >  tools/testing/selftests/kvm/x86_64/amx_test.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c
> > index bd72c6eb3b670..2f555f5c93e99 100644
> > --- a/tools/testing/selftests/kvm/x86_64/amx_test.c
> > +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c
> > @@ -119,9 +119,9 @@ static inline void check_cpuid_xsave(void)
> >  	GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE));
> >  }
> >  
> > -static bool check_xsave_supports_xtile(void)
> > +static inline void check_xsave_supports_xtile(void)
> 
> Don't explicitly tag local static functions as inline (ignore the existing code
> that sets a bad precedent), modern compilers don't need the hint to generate
> optimal code,
> 
> >  {
> > -	return __xgetbv(0) & XFEATURE_MASK_XTILE;
> > +	GUEST_ASSERT(__xgetbv(0) & XFEATURE_MASK_XTILE);
> 
> Any objection to moving the assertion into check_xtile_info() and dropping this
> one-line helper?

Actually, this code is silly, and arguably unnecessary. init_regs() explicitly
sets XCR0 to XFEATURE_MASK_XTILE.  If something goes awry, XSETBV should #GP.
If we want to be really paranoid and assert that KVM didn't silently fail XSETBV,
then the more logical place for the assertion is immediately after the XSETBV.

i.e.

  static void init_regs(void)
  {
	uint64_t cr4, xcr0;

	/* turn on CR4.OSXSAVE */
	cr4 = get_cr4();
	cr4 |= X86_CR4_OSXSAVE;
	set_cr4(cr4);

	xcr0 = __xgetbv(0);
	xcr0 |= XFEATURE_MASK_XTILE;
	__xsetbv(0x0, xcr0);

	GUEST_ASSERT((__xgetbv(0) & XFEATURE_MASK_XTILE) == XFEATURE_MASK_XTILE);
  }