[PATCH] selftests:resctrl: Fix build failure on archs without __cpuid_count()

[PATCH] selftests:resctrl: Fix build failure on archs without __cpuid_count()

[Shuah Khan]

When resctrl is built on architectures without __cpuid_count()
support, build fails. resctrl uses __cpuid_count() defined in
kselftest.h.

Even though the problem is seen while building resctrl on aarch64,
this error can be seen on any platform that doesn't support CPUID.

CPUID is a x86/x86-64 feature and code paths with CPUID asm commands
will fail to build on all other architectures.

All others tests call __cpuid_count() do so from x86/x86_64 code paths
when _i386__ or __x86_64__ are defined. resctrl is an exception.

Fix the problem by defining __cpuid_count() only when __i386__ or
__x86_64__ are defined in kselftest.h and changing resctrl to call
__cpuid_count() only when __i386__ or __x86_64__ are defined.

In file included from resctrl.h:24,
                 from cat_test.c:11:
In function ‘arch_supports_noncont_cat’,
    inlined from ‘noncont_cat_run_test’ at cat_test.c:326:6:
../kselftest.h:74:9: error: impossible constraint in ‘asm’
   74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
      |         ^~~~~~~
cat_test.c:304:17: note: in expansion of macro ‘__cpuid_count’
  304 |                 __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
      |                 ^~~~~~~~~~~~~
../kselftest.h:74:9: error: impossible constraint in ‘asm’
   74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
      |         ^~~~~~~
cat_test.c:306:17: note: in expansion of macro ‘__cpuid_count’
  306 |                 __cpuid_count(0x10, 2, eax, ebx, ecx, edx);

Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Reported-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 tools/testing/selftests/kselftest.h        | 2 ++
 tools/testing/selftests/resctrl/cat_test.c | 6 ++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index b8967b6e29d5..e195ec156859 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -61,6 +61,7 @@
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 #endif
 
+#if defined(__i386__) || defined(__x86_64__) /* arch */
 /*
  * gcc cpuid.h provides __cpuid_count() since v4.4.
  * Clang/LLVM cpuid.h provides  __cpuid_count() since v3.4.0.
@@ -75,6 +76,7 @@
 			      : "=a" (a), "=b" (b), "=c" (c), "=d" (d)	\
 			      : "0" (level), "2" (count))
 #endif
+#endif /* end arch */
 
 /* define kselftest exit codes */
 #define KSFT_PASS  0
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 742782438ca3..ae3f0fa5390b 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -290,12 +290,12 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param
 
 static bool arch_supports_noncont_cat(const struct resctrl_test *test)
 {
-	unsigned int eax, ebx, ecx, edx;
-
 	/* AMD always supports non-contiguous CBM. */
 	if (get_vendor() == ARCH_AMD)
 		return true;
 
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+	unsigned int eax, ebx, ecx, edx;
 	/* Intel support for non-contiguous CBM needs to be discovered. */
 	if (!strcmp(test->resource, "L3"))
 		__cpuid_count(0x10, 1, eax, ebx, ecx, edx);
@@ -305,6 +305,8 @@ static bool arch_supports_noncont_cat(const struct resctrl_test *test)
 		return false;
 
 	return ((ecx >> 3) & 1);
+#endif /* end arch */
+	return false;
 }
 
 static int noncont_cat_run_test(const struct resctrl_test *test,
-- 
2.40.1

Re: [PATCH] selftests:resctrl: Fix build failure on archs without __cpuid_count()

[Reinette Chatre]

Hi Shuah,

Thank you very much for looking into this.

On 9/5/24 11:02 AM, Shuah Khan wrote:
> When resctrl is built on architectures without __cpuid_count()
> support, build fails. resctrl uses __cpuid_count() defined in
> kselftest.h.
> 
> Even though the problem is seen while building resctrl on aarch64,
> this error can be seen on any platform that doesn't support CPUID.
> 
> CPUID is a x86/x86-64 feature and code paths with CPUID asm commands
> will fail to build on all other architectures.
> 
> All others tests call __cpuid_count() do so from x86/x86_64 code paths
> when _i386__ or __x86_64__ are defined. resctrl is an exception.
> 
> Fix the problem by defining __cpuid_count() only when __i386__ or
> __x86_64__ are defined in kselftest.h and changing resctrl to call
> __cpuid_count() only when __i386__ or __x86_64__ are defined.
> 
> In file included from resctrl.h:24,
>                   from cat_test.c:11:
> In function ‘arch_supports_noncont_cat’,
>      inlined from ‘noncont_cat_run_test’ at cat_test.c:326:6:
> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>     74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>        |         ^~~~~~~
> cat_test.c:304:17: note: in expansion of macro ‘__cpuid_count’
>    304 |                 __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
>        |                 ^~~~~~~~~~~~~
> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>     74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>        |         ^~~~~~~
> cat_test.c:306:17: note: in expansion of macro ‘__cpuid_count’
>    306 |                 __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
> 

If needing to know where this fix is needed, there can be a:
Fixes: ae638551ab64 ("selftests/resctrl: Add non-contiguous CBMs CAT test")

> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

Perhaps:
Closes: https://lore.kernel.org/lkml/20240809071059.265914-1-usama.anjum@collabora.com/

> Reported-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> ---
>   tools/testing/selftests/kselftest.h        | 2 ++
>   tools/testing/selftests/resctrl/cat_test.c | 6 ++++--
>   2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
> index b8967b6e29d5..e195ec156859 100644
> --- a/tools/testing/selftests/kselftest.h
> +++ b/tools/testing/selftests/kselftest.h
> @@ -61,6 +61,7 @@
>   #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>   #endif
>   
> +#if defined(__i386__) || defined(__x86_64__) /* arch */
>   /*
>    * gcc cpuid.h provides __cpuid_count() since v4.4.
>    * Clang/LLVM cpuid.h provides  __cpuid_count() since v3.4.0.
> @@ -75,6 +76,7 @@
>   			      : "=a" (a), "=b" (b), "=c" (c), "=d" (d)	\
>   			      : "0" (level), "2" (count))
>   #endif
> +#endif /* end arch */
>   
>   /* define kselftest exit codes */
>   #define KSFT_PASS  0
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index 742782438ca3..ae3f0fa5390b 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -290,12 +290,12 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param
>   
>   static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>   {
> -	unsigned int eax, ebx, ecx, edx;
> -
>   	/* AMD always supports non-contiguous CBM. */
>   	if (get_vendor() == ARCH_AMD)
>   		return true;
>   
> +#if defined(__i386__) || defined(__x86_64__) /* arch */
> +	unsigned int eax, ebx, ecx, edx;
>   	/* Intel support for non-contiguous CBM needs to be discovered. */
>   	if (!strcmp(test->resource, "L3"))
>   		__cpuid_count(0x10, 1, eax, ebx, ecx, edx);
> @@ -305,6 +305,8 @@ static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>   		return false;
>   
>   	return ((ecx >> 3) & 1);
> +#endif /* end arch */
> +	return false;
>   }
>   
>   static int noncont_cat_run_test(const struct resctrl_test *test,

Thank you very much.

Acked-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette

Re: [PATCH] selftests:resctrl: Fix build failure on archs without __cpuid_count()

[Shuah Khan]

On 9/5/24 14:45, Reinette Chatre wrote:
> Hi Shuah,
> 
> Thank you very much for looking into this.
> 
> On 9/5/24 11:02 AM, Shuah Khan wrote:
>> When resctrl is built on architectures without __cpuid_count()
>> support, build fails. resctrl uses __cpuid_count() defined in
>> kselftest.h.
>>
>> Even though the problem is seen while building resctrl on aarch64,
>> this error can be seen on any platform that doesn't support CPUID.
>>
>> CPUID is a x86/x86-64 feature and code paths with CPUID asm commands
>> will fail to build on all other architectures.
>>
>> All others tests call __cpuid_count() do so from x86/x86_64 code paths
>> when _i386__ or __x86_64__ are defined. resctrl is an exception.
>>
>> Fix the problem by defining __cpuid_count() only when __i386__ or
>> __x86_64__ are defined in kselftest.h and changing resctrl to call
>> __cpuid_count() only when __i386__ or __x86_64__ are defined.
>>
>> In file included from resctrl.h:24,
>>                   from cat_test.c:11:
>> In function ‘arch_supports_noncont_cat’,
>>      inlined from ‘noncont_cat_run_test’ at cat_test.c:326:6:
>> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>>     74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>>        |         ^~~~~~~
>> cat_test.c:304:17: note: in expansion of macro ‘__cpuid_count’
>>    304 |                 __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
>>        |                 ^~~~~~~~~~~~~
>> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>>     74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>>        |         ^~~~~~~
>> cat_test.c:306:17: note: in expansion of macro ‘__cpuid_count’
>>    306 |                 __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
>>
> 
> If needing to know where this fix is needed, there can be a:
> Fixes: ae638551ab64 ("selftests/resctrl: Add non-contiguous CBMs CAT test")

Thanks - will add it when I apply the patch.

> 
>> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> 
> Perhaps:
> Closes: https://lore.kernel.org/lkml/20240809071059.265914-1-usama.anjum@collabora.com/

Thanks. I will add it when I apply the patch.

> 
>> Reported-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
>> ---
>>   tools/testing/selftests/kselftest.h        | 2 ++
>>   tools/testing/selftests/resctrl/cat_test.c | 6 ++++--
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
>> index b8967b6e29d5..e195ec156859 100644
>> --- a/tools/testing/selftests/kselftest.h
>> +++ b/tools/testing/selftests/kselftest.h
>> @@ -61,6 +61,7 @@
>>   #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>>   #endif
>> +#if defined(__i386__) || defined(__x86_64__) /* arch */
>>   /*
>>    * gcc cpuid.h provides __cpuid_count() since v4.4.
>>    * Clang/LLVM cpuid.h provides  __cpuid_count() since v3.4.0.
>> @@ -75,6 +76,7 @@
>>                     : "=a" (a), "=b" (b), "=c" (c), "=d" (d)    \
>>                     : "0" (level), "2" (count))
>>   #endif
>> +#endif /* end arch */
>>   /* define kselftest exit codes */
>>   #define KSFT_PASS  0
>> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
>> index 742782438ca3..ae3f0fa5390b 100644
>> --- a/tools/testing/selftests/resctrl/cat_test.c
>> +++ b/tools/testing/selftests/resctrl/cat_test.c
>> @@ -290,12 +290,12 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param
>>   static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>>   {
>> -    unsigned int eax, ebx, ecx, edx;
>> -
>>       /* AMD always supports non-contiguous CBM. */
>>       if (get_vendor() == ARCH_AMD)
>>           return true;
>> +#if defined(__i386__) || defined(__x86_64__) /* arch */
>> +    unsigned int eax, ebx, ecx, edx;
>>       /* Intel support for non-contiguous CBM needs to be discovered. */
>>       if (!strcmp(test->resource, "L3"))
>>           __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
>> @@ -305,6 +305,8 @@ static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>>           return false;
>>       return ((ecx >> 3) & 1);
>> +#endif /* end arch */
>> +    return false;
>>   }
>>   static int noncont_cat_run_test(const struct resctrl_test *test,
> 
> Thank you very much.
> 
> Acked-by: Reinette Chatre <reinette.chatre@intel.com>
> 

thanks,
-- Shuah

Re: [PATCH] selftests:resctrl: Fix build failure on archs without __cpuid_count()

[Muhammad Usama Anjum]

Hi Shuah,

Thank you for fixing it.

On 9/5/24 11:02 PM, Shuah Khan wrote:
> When resctrl is built on architectures without __cpuid_count()
> support, build fails. resctrl uses __cpuid_count() defined in
> kselftest.h.
> 
> Even though the problem is seen while building resctrl on aarch64,
> this error can be seen on any platform that doesn't support CPUID.
> 
> CPUID is a x86/x86-64 feature and code paths with CPUID asm commands
> will fail to build on all other architectures.
> 
> All others tests call __cpuid_count() do so from x86/x86_64 code paths
> when _i386__ or __x86_64__ are defined. resctrl is an exception.
> 
> Fix the problem by defining __cpuid_count() only when __i386__ or
> __x86_64__ are defined in kselftest.h and changing resctrl to call
> __cpuid_count() only when __i386__ or __x86_64__ are defined.
> 
> In file included from resctrl.h:24,
>                  from cat_test.c:11:
> In function ‘arch_supports_noncont_cat’,
>     inlined from ‘noncont_cat_run_test’ at cat_test.c:326:6:
> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>    74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>       |         ^~~~~~~
> cat_test.c:304:17: note: in expansion of macro ‘__cpuid_count’
>   304 |                 __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
>       |                 ^~~~~~~~~~~~~
> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>    74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>       |         ^~~~~~~
> cat_test.c:306:17: note: in expansion of macro ‘__cpuid_count’
>   306 |                 __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
> 
> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Reported-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
LGTM
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

...
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index 742782438ca3..ae3f0fa5390b 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -290,12 +290,12 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param
>  
>  static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>  {
> -	unsigned int eax, ebx, ecx, edx;
> -
>  	/* AMD always supports non-contiguous CBM. */
>  	if (get_vendor() == ARCH_AMD)
>  		return true;
>  
> +#if defined(__i386__) || defined(__x86_64__) /* arch */
> +	unsigned int eax, ebx, ecx, edx;
>  	/* Intel support for non-contiguous CBM needs to be discovered. */
>  	if (!strcmp(test->resource, "L3"))
>  		__cpuid_count(0x10, 1, eax, ebx, ecx, edx);
> @@ -305,6 +305,8 @@ static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>  		return false;
>  
>  	return ((ecx >> 3) & 1);
> +#endif /* end arch */
> +	return false;
nit: empty line before return

>  }
>  
>  static int noncont_cat_run_test(const struct resctrl_test *test,

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH] selftests:resctrl: Fix build failure on archs without __cpuid_count()

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4455 bytes --]

On Thu, 5 Sep 2024, Shuah Khan wrote:

> When resctrl is built on architectures without __cpuid_count()
> support, build fails. resctrl uses __cpuid_count() defined in
> kselftest.h.
> 
> Even though the problem is seen while building resctrl on aarch64,
> this error can be seen on any platform that doesn't support CPUID.
> 
> CPUID is a x86/x86-64 feature and code paths with CPUID asm commands
> will fail to build on all other architectures.
> 
> All others tests call __cpuid_count() do so from x86/x86_64 code paths
> when _i386__ or __x86_64__ are defined. resctrl is an exception.
> 
> Fix the problem by defining __cpuid_count() only when __i386__ or
> __x86_64__ are defined in kselftest.h and changing resctrl to call
> __cpuid_count() only when __i386__ or __x86_64__ are defined.
> 
> In file included from resctrl.h:24,
>                  from cat_test.c:11:
> In function ‘arch_supports_noncont_cat’,
>     inlined from ‘noncont_cat_run_test’ at cat_test.c:326:6:
> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>    74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>       |         ^~~~~~~
> cat_test.c:304:17: note: in expansion of macro ‘__cpuid_count’
>   304 |                 __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
>       |                 ^~~~~~~~~~~~~
> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>    74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>       |         ^~~~~~~
> cat_test.c:306:17: note: in expansion of macro ‘__cpuid_count’
>   306 |                 __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
> 
> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Reported-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>

When the small things from Muhammad and Reinette addressed, this seems 
okay.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Thanks for the solution.


I'm still left to wonder if the x86 selftest is supposed to clobber 
CFLAGS? It seems that problem is orthogonal to this cpuid/resctrl problem.
I mean this question from the perspective of coherency in the entire 
kselftest framework, lib.mk seems to want to adjust CFLAGS but those
changes will get clobbered in the case of x86 selftest.

-- 
 i.

> ---
>  tools/testing/selftests/kselftest.h        | 2 ++
>  tools/testing/selftests/resctrl/cat_test.c | 6 ++++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
> index b8967b6e29d5..e195ec156859 100644
> --- a/tools/testing/selftests/kselftest.h
> +++ b/tools/testing/selftests/kselftest.h
> @@ -61,6 +61,7 @@
>  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>  #endif
>  
> +#if defined(__i386__) || defined(__x86_64__) /* arch */
>  /*
>   * gcc cpuid.h provides __cpuid_count() since v4.4.
>   * Clang/LLVM cpuid.h provides  __cpuid_count() since v3.4.0.
> @@ -75,6 +76,7 @@
>  			      : "=a" (a), "=b" (b), "=c" (c), "=d" (d)	\
>  			      : "0" (level), "2" (count))
>  #endif
> +#endif /* end arch */
>  
>  /* define kselftest exit codes */
>  #define KSFT_PASS  0
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index 742782438ca3..ae3f0fa5390b 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -290,12 +290,12 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param
>  
>  static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>  {
> -	unsigned int eax, ebx, ecx, edx;
> -
>  	/* AMD always supports non-contiguous CBM. */
>  	if (get_vendor() == ARCH_AMD)
>  		return true;
>  
> +#if defined(__i386__) || defined(__x86_64__) /* arch */
> +	unsigned int eax, ebx, ecx, edx;
>  	/* Intel support for non-contiguous CBM needs to be discovered. */
>  	if (!strcmp(test->resource, "L3"))
>  		__cpuid_count(0x10, 1, eax, ebx, ecx, edx);
> @@ -305,6 +305,8 @@ static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>  		return false;
>  
>  	return ((ecx >> 3) & 1);
> +#endif /* end arch */
> +	return false;
>  }
>  
>  static int noncont_cat_run_test(const struct resctrl_test *test,
> 

Re: [PATCH] selftests:resctrl: Fix build failure on archs without __cpuid_count()

[Shuah Khan]

On 9/6/24 04:12, Ilpo Järvinen wrote:
> On Thu, 5 Sep 2024, Shuah Khan wrote:
> 
>> When resctrl is built on architectures without __cpuid_count()
>> support, build fails. resctrl uses __cpuid_count() defined in
>> kselftest.h.
>>
>> Even though the problem is seen while building resctrl on aarch64,
>> this error can be seen on any platform that doesn't support CPUID.
>>
>> CPUID is a x86/x86-64 feature and code paths with CPUID asm commands
>> will fail to build on all other architectures.
>>
>> All others tests call __cpuid_count() do so from x86/x86_64 code paths
>> when _i386__ or __x86_64__ are defined. resctrl is an exception.
>>
>> Fix the problem by defining __cpuid_count() only when __i386__ or
>> __x86_64__ are defined in kselftest.h and changing resctrl to call
>> __cpuid_count() only when __i386__ or __x86_64__ are defined.
>>
>> In file included from resctrl.h:24,
>>                   from cat_test.c:11:
>> In function ‘arch_supports_noncont_cat’,
>>      inlined from ‘noncont_cat_run_test’ at cat_test.c:326:6:
>> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>>     74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>>        |         ^~~~~~~
>> cat_test.c:304:17: note: in expansion of macro ‘__cpuid_count’
>>    304 |                 __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
>>        |                 ^~~~~~~~~~~~~
>> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>>     74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>>        |         ^~~~~~~
>> cat_test.c:306:17: note: in expansion of macro ‘__cpuid_count’
>>    306 |                 __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
>>
>> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> Reported-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> 
> When the small things from Muhammad and Reinette addressed, this seems
> okay.
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 

Will do. Thank you for the review.

> Thanks for the solution.
> 
> 
> I'm still left to wonder if the x86 selftest is supposed to clobber
> CFLAGS? It seems that problem is orthogonal to this cpuid/resctrl problem.
> I mean this question from the perspective of coherency in the entire
> kselftest framework, lib.mk seems to want to adjust CFLAGS but those
> changes will get clobbered in the case of x86 selftest.
> 

This isn't the case x86 clobbering the CFLAGS. This falls into the case
of x86 customizing the flags for the test and the ones set by the common
lib.mk might interfere with the flags it needs.

There isn't anything here to fix based on the history of this test and
lib.mk.

thanks,
-- Shuah

Re: [PATCH] selftests:resctrl: Fix build failure on archs without __cpuid_count()

[Shuah Khan]

On 9/6/24 01:35, Muhammad Usama Anjum wrote:
> Hi Shuah,
> 
> Thank you for fixing it.
> 
> On 9/5/24 11:02 PM, Shuah Khan wrote:
>> When resctrl is built on architectures without __cpuid_count()
>> support, build fails. resctrl uses __cpuid_count() defined in
>> kselftest.h.
>>
>> Even though the problem is seen while building resctrl on aarch64,
>> this error can be seen on any platform that doesn't support CPUID.
>>
>> CPUID is a x86/x86-64 feature and code paths with CPUID asm commands
>> will fail to build on all other architectures.
>>
>> All others tests call __cpuid_count() do so from x86/x86_64 code paths
>> when _i386__ or __x86_64__ are defined. resctrl is an exception.
>>
>> Fix the problem by defining __cpuid_count() only when __i386__ or
>> __x86_64__ are defined in kselftest.h and changing resctrl to call
>> __cpuid_count() only when __i386__ or __x86_64__ are defined.
>>
>> In file included from resctrl.h:24,
>>                   from cat_test.c:11:
>> In function ‘arch_supports_noncont_cat’,
>>      inlined from ‘noncont_cat_run_test’ at cat_test.c:326:6:
>> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>>     74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>>        |         ^~~~~~~
>> cat_test.c:304:17: note: in expansion of macro ‘__cpuid_count’
>>    304 |                 __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
>>        |                 ^~~~~~~~~~~~~
>> ../kselftest.h:74:9: error: impossible constraint in ‘asm’
>>     74 |         __asm__ __volatile__ ("cpuid\n\t"                               \
>>        |         ^~~~~~~
>> cat_test.c:306:17: note: in expansion of macro ‘__cpuid_count’
>>    306 |                 __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
>>
>> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> Reported-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> LGTM
> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

Thank you for the review and finding the problem to begin with.
Much appreciated.

> 
> ...
>> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
>> index 742782438ca3..ae3f0fa5390b 100644
>> --- a/tools/testing/selftests/resctrl/cat_test.c
>> +++ b/tools/testing/selftests/resctrl/cat_test.c
>> @@ -290,12 +290,12 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param
>>   
>>   static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>>   {
>> -	unsigned int eax, ebx, ecx, edx;
>> -
>>   	/* AMD always supports non-contiguous CBM. */
>>   	if (get_vendor() == ARCH_AMD)
>>   		return true;
>>   
>> +#if defined(__i386__) || defined(__x86_64__) /* arch */
>> +	unsigned int eax, ebx, ecx, edx;
>>   	/* Intel support for non-contiguous CBM needs to be discovered. */
>>   	if (!strcmp(test->resource, "L3"))
>>   		__cpuid_count(0x10, 1, eax, ebx, ecx, edx);
>> @@ -305,6 +305,8 @@ static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>>   		return false;
>>   
>>   	return ((ecx >> 3) & 1);
>> +#endif /* end arch */
>> +	return false;
> nit: empty line before return

Will do.

> 
>>   }
>>   
>>   static int noncont_cat_run_test(const struct resctrl_test *test,
> 

thanks,
-- Shuah