Re: [PATCH] selftests: x86: skip the tests if prerequisites aren't fulfilled

On 3/7/2024 10:37 AM, Muhammad Usama Anjum wrote:

diff --git a/tools/testing/selftests/x86/amx.c b/tools/testing/selftests/x86/amx.c index d884fd69dd510..5d1ca0bbaaae7 100644 --- a/tools/testing/selftests/x86/amx.c +++ b/tools/testing/selftests/x86/amx.c @@ -103,9 +103,10 @@ static void clearhandler(int sig) #define CPUID_LEAF1_ECX_XSAVE_MASK (1 << 26) #define CPUID_LEAF1_ECX_OSXSAVE_MASK (1 << 27) -static inline void check_cpuid_xsave(void) +static inline int check_cpuid_xsave(void) { uint32_t eax, ebx, ecx, edx;

• int ret = 0;

/* * CPUID.1:ECX.XSAVE[bit 26] enumerates general @@ -113,10 +114,16 @@ static inline void check_cpuid_xsave(void) * XGETBV. */ __cpuid_count(1, 0, eax, ebx, ecx, edx);

• if (!(ecx & CPUID_LEAF1_ECX_XSAVE_MASK))

• fatal_error("cpuid: no CPU xsave support");
  

• if (!(ecx & CPUID_LEAF1_ECX_OSXSAVE_MASK))

• fatal_error("cpuid: no OS xsave support");
  

• if (!(ecx & CPUID_LEAF1_ECX_XSAVE_MASK)) {

• ksft_print_msg("cpuid: no CPU xsave support\n");
  

• ret = -1;
  

• }
• if (!(ecx & CPUID_LEAF1_ECX_OSXSAVE_MASK)) {

• ksft_print_msg("cpuid: no OS xsave support\n");
  

• ret = -1;
  

• }
• return ret; }

I thought check_cpuid_xsave() can go away [1] by simplifying the availability check through arch_prctl():

+#define ARCH_GET_XCOMP_SUPP 0x1021 #define ARCH_GET_XCOMP_PERM 0x1022 #define ARCH_REQ_XCOMP_PERM 0x1023

@@ -928,8 +911,15 @@ static void test_ptrace(void)

int main(void) { - /* Check hardware availability at first */ - check_cpuid_xsave(); + unsigned long features; + long rc; + + rc = syscall(SYS_arch_prctl, ARCH_GET_XCOMP_SUPP, &features); + if (rc || (features & XFEATURE_MASK_XTILE) != XFEATURE_MASK_XTILE) { + ksft_print_msg("no AMX support\n"); + return KSFT_SKIP; + }

-static void check_cpuid_xtiledata(void) +static int check_cpuid_xtiledata(void) { uint32_t eax, ebx, ecx, edx; @@ -153,12 +160,16 @@ static void check_cpuid_xtiledata(void) * eax: XTILEDATA state component size * ebx: XTILEDATA state component offset in user buffer */

• if (!eax || !ebx)

• fatal_error("xstate cpuid: invalid tile data size/offset: %d/%d",
  

• 		eax, ebx);
  

• if (!eax || !ebx) {

• ksft_print_msg("xstate cpuid: invalid tile data size/offset: %d/%d\n",
  

• 	       eax, ebx);
  

• return -1;
  

• }

xtiledata.size = eax; xtiledata.xbuf_offset = ebx;

• return 0; }

I don't think it is okay to silently skip the test here. If the feature is available, the tile data size and offset should not be zero.

Thanks, Chang

[1] https://lore.kernel.org/lkml/327cde12-daea-84ba-4b24-64fe12e89dea@intel.com/