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

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

[Muhammad Usama Anjum]

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;
+	}
+
 	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) {
-- 
2.39.2

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

[Chang S. Bae]

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

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

[Shuah Khan]

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

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

[Chang S. Bae]

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/

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

[Shuah Khan]

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