First, generalize resctrl selftest non-contiguous CAT check to not assume non-AMD vendor implies Intel. Second, improve 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).
The last patch might still require some work on which symbol the conditional in kselftest.h is implemented. I could not find any pre-existing one that could be used. Perhaps somebody who's more familiar with the kselftest build system has a better suggestion on which symbol the logic should be based at?
Ilpo Järvinen (3): selftests/resctrl: Generalize non-contiguous CAT check selftests/resctrl: Always initialize ecx to avoid build warnings [RFC] kselftest: Provide __cpuid_count() stub on non-x86 archs
tools/testing/selftests/kselftest.h | 6 +++++ tools/testing/selftests/lib.mk | 4 ++++ tools/testing/selftests/resctrl/cat_test.c | 28 +++++++++++++--------- 3 files changed, 27 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 --- 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,
On 8/13/24 3:45 PM, Ilpo Järvinen wrote:
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. */
- /* 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);elsereturn 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 --- 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:
On 8/13/24 3:45 PM, Ilpo Järvinen wrote:
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.
Provide an empty stub for __cpuid_count() if it is not supported to allow build to succeed.
Reported-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com --- tools/testing/selftests/kselftest.h | 6 ++++++ tools/testing/selftests/lib.mk | 4 ++++ 2 files changed, 10 insertions(+)
diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index b8967b6e29d5..71593add1b39 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)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..236db9b24037 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -199,6 +199,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 8/13/24 3:45 PM, 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.
Provide an empty stub for __cpuid_count() if it is not supported to allow build to succeed.
Reported-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com
Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com Tested-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/kselftest.h | 6 ++++++ tools/testing/selftests/lib.mk | 4 ++++ 2 files changed, 10 insertions(+)
diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index b8967b6e29d5..71593add1b39 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)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..236db9b24037 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -199,6 +199,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 8/13/24 04:45, Ilpo Järvinen wrote:
First, generalize resctrl selftest non-contiguous CAT check to not assume non-AMD vendor implies Intel. Second, improve 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).
The last patch might still require some work on which symbol the conditional in kselftest.h is implemented. I could not find any pre-existing one that could be used. Perhaps somebody who's more familiar with the kselftest build system has a better suggestion on which symbol the logic should be based at?
Ilpo Järvinen (3): selftests/resctrl: Generalize non-contiguous CAT check selftests/resctrl: Always initialize ecx to avoid build warnings [RFC] kselftest: Provide __cpuid_count() stub on non-x86 archs
tools/testing/selftests/kselftest.h | 6 +++++ tools/testing/selftests/lib.mk | 4 ++++ tools/testing/selftests/resctrl/cat_test.c | 28 +++++++++++++--------- 3 files changed, 27 insertions(+), 11 deletions(-)
These changes look good to me. Can you send the RFC patch without the RFC tag for me to pull in? I don't apply RFC patches.
Usama, does this fix the problem you are seeing?
Hi Reinette - do these look okay to you? Can you give me an ack if they do?
thanks, -- Shuah
On 8/21/24 11:30 AM, Shuah Khan wrote:
On 8/13/24 04:45, Ilpo Järvinen wrote:
First, generalize resctrl selftest non-contiguous CAT check to not assume non-AMD vendor implies Intel. Second, improve 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).
The last patch might still require some work on which symbol the conditional in kselftest.h is implemented. I could not find any pre-existing one that could be used. Perhaps somebody who's more familiar with the kselftest build system has a better suggestion on which symbol the logic should be based at?
Ilpo Järvinen (3): selftests/resctrl: Generalize non-contiguous CAT check selftests/resctrl: Always initialize ecx to avoid build warnings [RFC] kselftest: Provide __cpuid_count() stub on non-x86 archs
tools/testing/selftests/kselftest.h | 6 +++++ tools/testing/selftests/lib.mk | 4 ++++ tools/testing/selftests/resctrl/cat_test.c | 28 +++++++++++++--------- 3 files changed, 27 insertions(+), 11 deletions(-)
These changes look good to me. Can you send the RFC patch without the RFC tag for me to pull in? I don't apply RFC patches.
Usama, does this fix the problem you are seeing?
Yeah, build errors are resolved.
Hi Reinette - do these look okay to you? Can you give me an ack if they do?
thanks, -- Shuah
On Thu, 22 Aug 2024, Muhammad Usama Anjum wrote:
On 8/21/24 11:30 AM, Shuah Khan wrote:
On 8/13/24 04:45, Ilpo Järvinen wrote:
First, generalize resctrl selftest non-contiguous CAT check to not assume non-AMD vendor implies Intel. Second, improve 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).
The last patch might still require some work on which symbol the conditional in kselftest.h is implemented. I could not find any pre-existing one that could be used. Perhaps somebody who's more familiar with the kselftest build system has a better suggestion on which symbol the logic should be based at?
Ilpo Järvinen (3): selftests/resctrl: Generalize non-contiguous CAT check selftests/resctrl: Always initialize ecx to avoid build warnings [RFC] kselftest: Provide __cpuid_count() stub on non-x86 archs
tools/testing/selftests/kselftest.h | 6 +++++ tools/testing/selftests/lib.mk | 4 ++++ tools/testing/selftests/resctrl/cat_test.c | 28 +++++++++++++--------- 3 files changed, 27 insertions(+), 11 deletions(-)
These changes look good to me. Can you send the RFC patch without the RFC tag for me to pull in? I don't apply RFC patches.
Usama, does this fix the problem you are seeing?
Yeah, build errors are resolved.
Great, thanks for testing!
I'll send v2 with RFC removed soon.
Hi Reinette - do these look okay to you? Can you give me an ack if they do?
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
Ilpo Järvinen -
Muhammad Usama Anjum -
Shuah Khan