| Message ID | 20240314114502.466806-1-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] selftests: x86: skip the tests if prerequisites aren't fulfilled | expand |
On 3/14/2024 4:44 AM, Muhammad Usama Anjum wrote: > Skip instead of failing when prerequisite conditions aren't fulfilled, > such as invalid xstate values etc. This patch would make the tests show > as skip when run by: > make -C tools/testing/selftests/ TARGETS=x86 run_tests > > ... > # timeout set to 45 > # selftests: x86: amx_64 > # # xstate cpuid: invalid tile data size/offset: 0/0 > ok 42 selftests: x86: amx_64 # SKIP > # timeout set to 45 > # selftests: x86: lam_64 > # # Unsupported LAM feature! > ok 43 selftests: x86: lam_64 # SKIP > ... > > Cc: Chang S. Bae <chang.seok.bae@intel.com> > Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com> > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > Changes since v1: > - Use arch_prctl to check if amx is supported > --- > tools/testing/selftests/x86/amx.c | 27 ++++++++++----------------- > tools/testing/selftests/x86/lam.c | 2 +- > 2 files changed, 11 insertions(+), 18 deletions(-) > > diff --git a/tools/testing/selftests/x86/amx.c b/tools/testing/selftests/x86/amx.c > index d884fd69dd510..95aad6d8849be 100644 > --- a/tools/testing/selftests/x86/amx.c > +++ b/tools/testing/selftests/x86/amx.c > @@ -103,21 +103,6 @@ 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) > -{ > - uint32_t eax, ebx, ecx, edx; > - > - /* > - * CPUID.1:ECX.XSAVE[bit 26] enumerates general > - * support for the XSAVE feature set, including > - * 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"); > -} > > static uint32_t xbuf_size; > > @@ -350,6 +335,7 @@ enum expected_result { FAIL_EXPECTED, SUCCESS_EXPECTED }; > > /* arch_prctl() and sigaltstack() test */ > > +#define ARCH_GET_XCOMP_SUPP 0x1021 > #define ARCH_GET_XCOMP_PERM 0x1022 > #define ARCH_REQ_XCOMP_PERM 0x1023 > > @@ -928,8 +914,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; > + } > + Reviewed-by: Chang S. Bae <chang.seok.bae@intel.com> Thanks, Chang
On 3/14/24 05:44, Muhammad Usama Anjum wrote: > Skip instead of failing when prerequisite conditions aren't fulfilled, > such as invalid xstate values etc. This patch would make the tests show > as skip when run by: > make -C tools/testing/selftests/ TARGETS=x86 run_tests > > ... > # timeout set to 45 > # selftests: x86: amx_64 > # # xstate cpuid: invalid tile data size/offset: 0/0 > ok 42 selftests: x86: amx_64 # SKIP > # timeout set to 45 > # selftests: x86: lam_64 > # # Unsupported LAM feature! > ok 43 selftests: x86: lam_64 # SKIP > ... > > Cc: Chang S. Bae <chang.seok.bae@intel.com> > Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com> > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > Changes since v1: > - Use arch_prctl to check if amx is supported This should be mentioned in the changelog and also show that there are bo backwards compatibility issues. > --- > tools/testing/selftests/x86/amx.c | 27 ++++++++++----------------- > tools/testing/selftests/x86/lam.c | 2 +- > 2 files changed, 11 insertions(+), 18 deletions(-) > > diff --git a/tools/testing/selftests/x86/amx.c b/tools/testing/selftests/x86/amx.c > index d884fd69dd510..95aad6d8849be 100644 > --- a/tools/testing/selftests/x86/amx.c > +++ b/tools/testing/selftests/x86/amx.c > @@ -103,21 +103,6 @@ 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) > -{ > - uint32_t eax, ebx, ecx, edx; > - > - /* > - * CPUID.1:ECX.XSAVE[bit 26] enumerates general > - * support for the XSAVE feature set, including > - * 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"); > -} > Why doesn't the changelog mention the code removal? thanks, -- Shuah
On 3/26/2024 1:51 PM, Shuah Khan wrote: > > show that there are bo backwards compatibility issues In older kernels lacking AMX support [1], arch_prctl() returns EINVAL. With AMX support, the kernel will properly set 'features'. It is also worth noting that this simplification was previously acknowledged [2], albeit some time ago. Thanks, Chang [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=db8268df0983adc2bb1fb48c9e5f7bfbb5f617f3 [2] https://lore.kernel.org/lkml/de61ffdb-638a-ca84-31b5-55f6a8616597@linuxfoundation.org/
On 3/26/24 15:20, Chang S. Bae wrote: > On 3/26/2024 1:51 PM, Shuah Khan wrote: >> >> show that there are bo backwards compatibility issues > In older kernels lacking AMX support [1], arch_prctl() returns EINVAL. With AMX support, the kernel will properly set 'features'. > > It is also worth noting that this simplification was previously acknowledged [2], albeit some time ago. > > Thanks, > Chang > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=db8268df0983adc2bb1fb48c9e5f7bfbb5f617f3 > [2] https://lore.kernel.org/lkml/de61ffdb-638a-ca84-31b5-55f6a8616597@linuxfoundation.org/ > > Thanks. This can be included in the change log so it is clear. thanks, -- Shuah
diff --git a/tools/testing/selftests/x86/amx.c b/tools/testing/selftests/x86/amx.c index d884fd69dd510..95aad6d8849be 100644 --- a/tools/testing/selftests/x86/amx.c +++ b/tools/testing/selftests/x86/amx.c @@ -103,21 +103,6 @@ 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) -{ - uint32_t eax, ebx, ecx, edx; - - /* - * CPUID.1:ECX.XSAVE[bit 26] enumerates general - * support for the XSAVE feature set, including - * 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"); -} static uint32_t xbuf_size; @@ -350,6 +335,7 @@ enum expected_result { FAIL_EXPECTED, SUCCESS_EXPECTED }; /* arch_prctl() and sigaltstack() test */ +#define ARCH_GET_XCOMP_SUPP 0x1021 #define ARCH_GET_XCOMP_PERM 0x1022 #define ARCH_REQ_XCOMP_PERM 0x1023 @@ -928,8 +914,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; + } + check_cpuid_xtiledata(); init_stashed_xsave(); diff --git a/tools/testing/selftests/x86/lam.c b/tools/testing/selftests/x86/lam.c index 215b8150b7cca..c0f016f45ee17 100644 --- a/tools/testing/selftests/x86/lam.c +++ b/tools/testing/selftests/x86/lam.c @@ -1183,7 +1183,7 @@ int main(int argc, char **argv) if (!cpu_has_lam()) { ksft_print_msg("Unsupported LAM feature!\n"); - return -1; + return KSFT_SKIP; } while ((c = getopt(argc, argv, "ht:")) != -1) {