Re: [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs

Re: [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs

[kernel test robot]

:::::: 
:::::: Manual check reason: "low confidence bisect report"
:::::: 

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20240829131657.1917-4-ilpo.jarvinen@linux.intel.com>
References: <20240829131657.1917-4-ilpo.jarvinen@linux.intel.com>
TO: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>

Hi Ilpo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on shuah-kselftest/next]
[also build test WARNING on shuah-kselftest/fixes linus/master v6.11-rc5 next-20240830]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ilpo-J-rvinen/selftests-resctrl-Generalize-non-contiguous-CAT-check/20240829-212248
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
patch link:    https://lore.kernel.org/r/20240829131657.1917-4-ilpo.jarvinen%40linux.intel.com
patch subject: [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240901/202409010931.Xl6TB9hc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202409010931.Xl6TB9hc-lkp@intel.com/

All warnings (new ones prefixed by >>):

   corrupt_xstate_header.c: In function 'xsave_enabled':
>> corrupt_xstate_header.c:29:20: warning: 'ecx' is used uninitialized [-Wuninitialized]
      29 |         return ecx & (1U << 27);
         |                ~~~~^~~~~~~~~~~~
   corrupt_xstate_header.c:24:32: note: 'ecx' was declared here
      24 |         unsigned int eax, ebx, ecx, edx;
         |                                ^~~
--
   amx.c: In function 'check_cpuid_xtiledata':
>> amx.c:123:19: warning: 'ebx' is used uninitialized [-Wuninitialized]
     123 |         xbuf_size = ebx;
         |         ~~~~~~~~~~^~~~~
   amx.c:111:23: note: 'ebx' was declared here
     111 |         uint32_t eax, ebx, ecx, edx;
         |                       ^~~
>> amx.c:131:13: warning: 'eax' is used uninitialized [-Wuninitialized]
     131 |         if (!eax || !ebx)
         |             ^~~~
   amx.c:111:18: note: 'eax' was declared here
     111 |         uint32_t eax, ebx, ecx, edx;
         |                  ^~~
--
   lam.c: In function 'cpu_has_lam':
>> lam.c:124:24: warning: 'cpuinfo' is used uninitialized [-Wuninitialized]
     124 |         return (cpuinfo[0] & (1 << 26));
         |                 ~~~~~~~^~~
   lam.c:120:22: note: 'cpuinfo' declared here
     120 |         unsigned int cpuinfo[4];
         |                      ^~~~~~~
   lam.c: In function 'cpu_has_la57':
   lam.c:134:24: warning: 'cpuinfo' is used uninitialized [-Wuninitialized]
     134 |         return (cpuinfo[2] & (1 << 16));
         |                 ~~~~~~~^~~
   lam.c:130:22: note: 'cpuinfo' declared here
     130 |         unsigned int cpuinfo[4];
         |                      ^~~~~~~


vim +/ecx +29 tools/testing/selftests/x86/corrupt_xstate_header.c

b7c11876d24bdd7 Andy Lutomirski 2021-06-08  21  
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  22  static inline int xsave_enabled(void)
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  23  {
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  24  	unsigned int eax, ebx, ecx, edx;
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  25  
170d1c23f2a3569 Reinette Chatre 2022-04-25  26  	__cpuid_count(0x1, 0x0, eax, ebx, ecx, edx);
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  27  
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  28  	/* Is CR4.OSXSAVE enabled ? */
b7c11876d24bdd7 Andy Lutomirski 2021-06-08 @29  	return ecx & (1U << 27);
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  30  }
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  31  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v3 0/3] selftests: Fix cpuid / vendor checking build issues

[Ilpo Järvinen]

This series first generalizes resctrl selftest non-contiguous CAT check
to not assume non-AMD vendor implies Intel. Second, it improves
kselftest common parts and resctrl selftest such that the use of
__cpuid_count() does not lead into a build failure (happens at least on
ARM).

While ARM does not currently support resctrl features, there's an
ongoing work to enable resctrl support also for it on the kernel side.
In any case, a common header such as kselftest.h should have a proper
fallback in place for what it provides, thus it seems justified to fix
this common level problem on the common level rather than e.g.
disabling build for resctrl selftest for archs lacking resctrl support.

I've dropped reviewed and tested by tags from the last patch due to
major changes into the makefile logic. So it would be helpful if
Muhammad could retest with this version.

v3:
- Remove "empty" wording
- Also cast input parameters to void
- Initialize ARCH from uname -m if not set (this might allow cleaning
  up some other makefiles but that is left as future work)

v2:
- Removed RFC from the last patch & added Fixes and tags
- Fixed the error message's line splits
- Noted down the reason for void casts in the stub

Ilpo Järvinen (3):
  selftests/resctrl: Generalize non-contiguous CAT check
  selftests/resctrl: Always initialize ecx to avoid build warnings
  kselftest: Provide __cpuid_count() stub on non-x86 archs

 tools/testing/selftests/kselftest.h        |  6 +++++
 tools/testing/selftests/lib.mk             |  6 +++++
 tools/testing/selftests/resctrl/cat_test.c | 28 +++++++++++++---------
 3 files changed, 29 insertions(+), 11 deletions(-)

-- 
2.39.2

[PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs

[Ilpo Järvinen]

Building resctrl selftest fails on ARM because it uses __cpuid_count()
that fails the build with error:

  CC       resctrl_tests
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:323:6:
../kselftest.h:74:9: error: impossible constraint in 'asm'
   74 |         __asm__ __volatile__ ("cpuid\n\t"       \
      |         ^~~~~~~
cat_test.c:301:17: note: in expansion of macro '__cpuid_count'
  301 |                 __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:303:17: note: in expansion of macro '__cpuid_count'
  303 |                 __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
      |                 ^~~~~~~~~~~~~

The resctrl selftest would run that code only on Intel CPUs but as is,
the code cannot be build at all.

Define HAVE_CPUID in lib.mk based on ARCH (x86 or x86_64). If ARCH is
not set, acquire it using uname -m.

Provide a stub for __cpuid_count() if HAVE_CPUID is not present to
allow build to succeed. The stub casts its arguments to void to avoid
causing "unused variable" or "set but not used" warnings.

Fixes: ae638551ab64 ("selftests/resctrl: Add non-contiguous CBMs CAT test")
Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
v3:
- Remove "empty" wording
- Also cast input parameters to void
- Initialize ARCH from uname -m if not set (this might allow cleaning
  up some other makefiles but that is left as future work)
v2:
- Removed RFC & added Fixes and Tested-by
- Fixed the error message's line splits
- Noted down the reason for void casts in the stub
---
 tools/testing/selftests/kselftest.h | 6 ++++++
 tools/testing/selftests/lib.mk      | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index b8967b6e29d5..9c4bfbf107f1 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -70,10 +70,16 @@
  * have __cpuid_count().
  */
 #ifndef __cpuid_count
+#ifdef HAVE_CPUID
 #define __cpuid_count(level, count, a, b, c, d)				\
 	__asm__ __volatile__ ("cpuid\n\t"				\
 			      : "=a" (a), "=b" (b), "=c" (c), "=d" (d)	\
 			      : "0" (level), "2" (count))
+#else
+#define __cpuid_count(level, count, a, b, c, d)	do {			\
+	(void)level; (void)count; (void)a; (void)b; (void)c; (void)d;	\
+} while (0)
+#endif
 #endif
 
 /* define kselftest exit codes */
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index d6edcfcb5be8..8e3069926153 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -23,6 +23,8 @@ CLANG_TARGET_FLAGS_x86_64       := x86_64-linux-gnu
 
 # Default to host architecture if ARCH is not explicitly given.
 ifeq ($(ARCH),)
+ARCH := $(shell uname -m 2>/dev/null || echo not)
+ARCH := $(shell echo $(ARCH) | sed -e s/i.86/x86/)
 CLANG_TARGET_FLAGS := $(shell $(CLANG) -print-target-triple)
 else
 CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(ARCH))
@@ -199,6 +201,10 @@ clean: $(if $(TEST_GEN_MODS_DIR),clean_mods_dir)
 # Build with _GNU_SOURCE by default
 CFLAGS += -D_GNU_SOURCE=
 
+ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
+CFLAGS += -DHAVE_CPUID=
+endif
+
 # Enables to extend CFLAGS and LDFLAGS from command line, e.g.
 # make USERCFLAGS=-Werror USERLDFLAGS=-static
 CFLAGS += $(USERCFLAGS)
-- 
2.39.2

Re: [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs

[kernel test robot]

Hi Ilpo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on shuah-kselftest/next]
[also build test WARNING on shuah-kselftest/fixes linus/master v6.11-rc5 next-20240830]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ilpo-J-rvinen/selftests-resctrl-Generalize-non-contiguous-CAT-check/20240829-212248
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
patch link:    https://lore.kernel.org/r/20240829131657.1917-4-ilpo.jarvinen%40linux.intel.com
patch subject: [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240901/202409010931.Xl6TB9hc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202409010931.Xl6TB9hc-lkp@intel.com/

All warnings (new ones prefixed by >>):

   corrupt_xstate_header.c: In function 'xsave_enabled':
>> corrupt_xstate_header.c:29:20: warning: 'ecx' is used uninitialized [-Wuninitialized]
      29 |         return ecx & (1U << 27);
         |                ~~~~^~~~~~~~~~~~
   corrupt_xstate_header.c:24:32: note: 'ecx' was declared here
      24 |         unsigned int eax, ebx, ecx, edx;
         |                                ^~~
--
   amx.c: In function 'check_cpuid_xtiledata':
>> amx.c:123:19: warning: 'ebx' is used uninitialized [-Wuninitialized]
     123 |         xbuf_size = ebx;
         |         ~~~~~~~~~~^~~~~
   amx.c:111:23: note: 'ebx' was declared here
     111 |         uint32_t eax, ebx, ecx, edx;
         |                       ^~~
>> amx.c:131:13: warning: 'eax' is used uninitialized [-Wuninitialized]
     131 |         if (!eax || !ebx)
         |             ^~~~
   amx.c:111:18: note: 'eax' was declared here
     111 |         uint32_t eax, ebx, ecx, edx;
         |                  ^~~
--
   lam.c: In function 'cpu_has_lam':
>> lam.c:124:24: warning: 'cpuinfo' is used uninitialized [-Wuninitialized]
     124 |         return (cpuinfo[0] & (1 << 26));
         |                 ~~~~~~~^~~
   lam.c:120:22: note: 'cpuinfo' declared here
     120 |         unsigned int cpuinfo[4];
         |                      ^~~~~~~
   lam.c: In function 'cpu_has_la57':
   lam.c:134:24: warning: 'cpuinfo' is used uninitialized [-Wuninitialized]
     134 |         return (cpuinfo[2] & (1 << 16));
         |                 ~~~~~~~^~~
   lam.c:130:22: note: 'cpuinfo' declared here
     130 |         unsigned int cpuinfo[4];
         |                      ^~~~~~~


vim +/ecx +29 tools/testing/selftests/x86/corrupt_xstate_header.c

b7c11876d24bdd7 Andy Lutomirski 2021-06-08  21  
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  22  static inline int xsave_enabled(void)
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  23  {
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  24  	unsigned int eax, ebx, ecx, edx;
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  25  
170d1c23f2a3569 Reinette Chatre 2022-04-25  26  	__cpuid_count(0x1, 0x0, eax, ebx, ecx, edx);
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  27  
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  28  	/* Is CR4.OSXSAVE enabled ? */
b7c11876d24bdd7 Andy Lutomirski 2021-06-08 @29  	return ecx & (1U << 27);
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  30  }
b7c11876d24bdd7 Andy Lutomirski 2021-06-08  31  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs

[Ilpo Järvinen]

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

On Thu, 29 Aug 2024, Ilpo Järvinen wrote:

> Building resctrl selftest fails on ARM because it uses __cpuid_count()
> that fails the build with error:
> 
>   CC       resctrl_tests
> 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:323:6:
> ../kselftest.h:74:9: error: impossible constraint in 'asm'
>    74 |         __asm__ __volatile__ ("cpuid\n\t"       \
>       |         ^~~~~~~
> cat_test.c:301:17: note: in expansion of macro '__cpuid_count'
>   301 |                 __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:303:17: note: in expansion of macro '__cpuid_count'
>   303 |                 __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
>       |                 ^~~~~~~~~~~~~
> 
> The resctrl selftest would run that code only on Intel CPUs but as is,
> the code cannot be build at all.
> 
> Define HAVE_CPUID in lib.mk based on ARCH (x86 or x86_64). If ARCH is
> not set, acquire it using uname -m.
> 
> Provide a stub for __cpuid_count() if HAVE_CPUID is not present to
> allow build to succeed. The stub casts its arguments to void to avoid
> causing "unused variable" or "set but not used" warnings.
> 
> Fixes: ae638551ab64 ("selftests/resctrl: Add non-contiguous CBMs CAT test")
> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> v3:
> - Remove "empty" wording
> - Also cast input parameters to void
> - Initialize ARCH from uname -m if not set (this might allow cleaning
>   up some other makefiles but that is left as future work)
> v2:
> - Removed RFC & added Fixes and Tested-by
> - Fixed the error message's line splits
> - Noted down the reason for void casts in the stub
> ---
>  tools/testing/selftests/kselftest.h | 6 ++++++
>  tools/testing/selftests/lib.mk      | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
> index b8967b6e29d5..9c4bfbf107f1 100644
> --- a/tools/testing/selftests/kselftest.h
> +++ b/tools/testing/selftests/kselftest.h
> @@ -70,10 +70,16 @@
>   * have __cpuid_count().
>   */
>  #ifndef __cpuid_count
> +#ifdef HAVE_CPUID
>  #define __cpuid_count(level, count, a, b, c, d)				\
>  	__asm__ __volatile__ ("cpuid\n\t"				\
>  			      : "=a" (a), "=b" (b), "=c" (c), "=d" (d)	\
>  			      : "0" (level), "2" (count))
> +#else
> +#define __cpuid_count(level, count, a, b, c, d)	do {			\
> +	(void)level; (void)count; (void)a; (void)b; (void)c; (void)d;	\
> +} while (0)
> +#endif
>  #endif
>  
>  /* define kselftest exit codes */
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index d6edcfcb5be8..8e3069926153 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -23,6 +23,8 @@ CLANG_TARGET_FLAGS_x86_64       := x86_64-linux-gnu
>  
>  # Default to host architecture if ARCH is not explicitly given.
>  ifeq ($(ARCH),)
> +ARCH := $(shell uname -m 2>/dev/null || echo not)
> +ARCH := $(shell echo $(ARCH) | sed -e s/i.86/x86/)
>  CLANG_TARGET_FLAGS := $(shell $(CLANG) -print-target-triple)
>  else
>  CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(ARCH))
> @@ -199,6 +201,10 @@ clean: $(if $(TEST_GEN_MODS_DIR),clean_mods_dir)
>  # Build with _GNU_SOURCE by default
>  CFLAGS += -D_GNU_SOURCE=
>  
> +ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
> +CFLAGS += -DHAVE_CPUID=
> +endif

Hpmf, scratch this. CFLAGS are overwritten by x86 selftest makefile so I 
need to reorder things there before making this change.

-- 
 i.