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(-)
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,
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:
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)
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.
linux-kselftest-mirror@lists.linaro.org
-
Ilpo Järvinen