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

Linux Kernel Selftest development
 help / color / mirror / Atom feed

* [PATCH v3 0/3] selftests: Fix cpuid / vendor checking build issues
@ 2024-08-29 13:16 Ilpo Järvinen
  2024-08-29 13:16 ` [PATCH v3 1/3] selftests/resctrl: Generalize non-contiguous CAT check Ilpo Järvinen
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2024-08-29 13:16 UTC (permalink / raw)
  To: Muhammad Usama Anjum, Shuah Khan, Reinette Chatre,
	linux-kselftest
  Cc: linux-kernel, Shaopeng Tan, Fenghua Yu,
	Maciej Wieczór-Retman, 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/3] selftests/resctrl: Generalize non-contiguous CAT check
  2024-08-29 13:16 [PATCH v3 0/3] selftests: Fix cpuid / vendor checking build issues Ilpo Järvinen
@ 2024-08-29 13:16 ` Ilpo Järvinen
  2024-08-29 13:16 ` [PATCH v3 2/3] selftests/resctrl: Always initialize ecx to avoid build warnings Ilpo Järvinen
  2024-08-29 13:16 ` [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs Ilpo Järvinen
  2 siblings, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2024-08-29 13:16 UTC (permalink / raw)
  To: Muhammad Usama Anjum, Shuah Khan, Reinette Chatre,
	linux-kselftest
  Cc: linux-kernel, Shaopeng Tan, Fenghua Yu,
	Maciej Wieczór-Retman, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/3] selftests/resctrl: Always initialize ecx to avoid build warnings
  2024-08-29 13:16 [PATCH v3 0/3] selftests: Fix cpuid / vendor checking build issues Ilpo Järvinen
  2024-08-29 13:16 ` [PATCH v3 1/3] selftests/resctrl: Generalize non-contiguous CAT check Ilpo Järvinen
@ 2024-08-29 13:16 ` Ilpo Järvinen
  2024-08-29 13:16 ` [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs Ilpo Järvinen
  2 siblings, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2024-08-29 13:16 UTC (permalink / raw)
  To: Muhammad Usama Anjum, Shuah Khan, Reinette Chatre,
	linux-kselftest
  Cc: linux-kernel, Shaopeng Tan, Fenghua Yu,
	Maciej Wieczór-Retman, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs
  2024-08-29 13:16 [PATCH v3 0/3] selftests: Fix cpuid / vendor checking build issues Ilpo Järvinen
  2024-08-29 13:16 ` [PATCH v3 1/3] selftests/resctrl: Generalize non-contiguous CAT check Ilpo Järvinen
  2024-08-29 13:16 ` [PATCH v3 2/3] selftests/resctrl: Always initialize ecx to avoid build warnings Ilpo Järvinen
@ 2024-08-29 13:16 ` Ilpo Järvinen
  2024-09-03  9:57   ` Ilpo Järvinen
  2 siblings, 1 reply; 5+ messages in thread
From: Ilpo Järvinen @ 2024-08-29 13:16 UTC (permalink / raw)
  To: Muhammad Usama Anjum, Shuah Khan, Reinette Chatre,
	linux-kselftest
  Cc: linux-kernel, Shaopeng Tan, Fenghua Yu,
	Maciej Wieczór-Retman, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs
  2024-08-29 13:16 ` [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs Ilpo Järvinen
@ 2024-09-03  9:57   ` Ilpo Järvinen
  0 siblings, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2024-09-03  9:57 UTC (permalink / raw)
  To: Muhammad Usama Anjum, Shuah Khan
  Cc: Reinette Chatre, linux-kselftest, LKML, Shaopeng Tan, Fenghua Yu,
	Maciej Wieczór-Retman

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-09-03  9:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-29 13:16 [PATCH v3 0/3] selftests: Fix cpuid / vendor checking build issues Ilpo Järvinen
2024-08-29 13:16 ` [PATCH v3 1/3] selftests/resctrl: Generalize non-contiguous CAT check Ilpo Järvinen
2024-08-29 13:16 ` [PATCH v3 2/3] selftests/resctrl: Always initialize ecx to avoid build warnings Ilpo Järvinen
2024-08-29 13:16 ` [PATCH v3 3/3] kselftest: Provide __cpuid_count() stub on non-x86 archs Ilpo Järvinen
2024-09-03  9:57   ` Ilpo Järvinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox