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

[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 1/3] selftests/resctrl: Generalize non-contiguous CAT check

[Ilpo Järvinen]

arch_supports_noncont_cat() checks if vendor is ARCH_AMD and if that is
not true, ARCH_INTEL is assumed which might not be true either because
get_vendor() can also return zero if neither AMD nor Intel is detected.

Generalize the vendor check using switch/case logic and return false
for unknown vendors.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/resctrl/cat_test.c | 26 +++++++++++++---------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 742782438ca3..51a1cb6aac34 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -292,19 +292,25 @@ 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)
+	switch (get_vendor()) {
+	case ARCH_AMD:
+		/* AMD always supports non-contiguous CBM. */
 		return true;
 
-	/* Intel support for non-contiguous CBM needs to be discovered. */
-	if (!strcmp(test->resource, "L3"))
-		__cpuid_count(0x10, 1, eax, ebx, ecx, edx);
-	else if (!strcmp(test->resource, "L2"))
-		__cpuid_count(0x10, 2, eax, ebx, ecx, edx);
-	else
-		return false;
+	case ARCH_INTEL:
+		/* Intel support for non-contiguous CBM needs to be discovered. */
+		if (!strcmp(test->resource, "L3"))
+			__cpuid_count(0x10, 1, eax, ebx, ecx, edx);
+		else if (!strcmp(test->resource, "L2"))
+			__cpuid_count(0x10, 2, eax, ebx, ecx, edx);
+		else
+			return false;
+
+		return ((ecx >> 3) & 1);
 
-	return ((ecx >> 3) & 1);
+	default:
+		return false;
+	}
 }
 
 static int noncont_cat_run_test(const struct resctrl_test *test,
-- 
2.39.2

[PATCH v3 2/3] selftests/resctrl: Always initialize ecx to avoid build warnings

[Ilpo Järvinen]

To avoid warnings when __cpuid_count() is an empty stub, always
initialize ecx because it is used in the return statement.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/resctrl/cat_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 51a1cb6aac34..9882c5d19408 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -290,7 +290,7 @@ 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;
+	unsigned int eax, ebx, ecx = 0, edx;
 
 	switch (get_vendor()) {
 	case ARCH_AMD:
-- 
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

[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.