* [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure
@ 2021-11-08 2:14 Li Wang
2021-11-08 2:14 ` [LTP] [PATCH v3 2/3] testcase: make use of .supported_archs Li Wang
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Li Wang @ 2021-11-08 2:14 UTC (permalink / raw)
To: ltp
Testcases for specific arch should be limited on that only being supported
platform to run, we now involve a .supported_archs to achieve this feature
in LTP library. All you need to run a test on the expected arch is to set
the '.supported_archs' array in the 'struct tst_test' to choose the required
arch list. e.g.
.supported_archs = (const char *const []){"x86_64", "ppc64", NULL}
This helps move the TCONF info from code to tst_test metadata as well.
And, we also export a struct tst_arch to save the system architecture
for using in the whole test cases.
extern struct arch {
char name[16];
enum tst_arch_type type;
} tst_arch;
Signed-off-by: Li Wang <liwang@redhat.com>
---
doc/c-test-api.txt | 36 +++++++++++++++
include/tst_arch.h | 39 ++++++++++++++++
include/tst_test.h | 10 +++++
lib/tst_arch.c | 108 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 193 insertions(+)
create mode 100644 include/tst_arch.h
create mode 100644 lib/tst_arch.c
diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
index 3127018a4..5e32b7075 100644
--- a/doc/c-test-api.txt
+++ b/doc/c-test-api.txt
@@ -2261,6 +2261,42 @@ struct tst_test test = {
};
-------------------------------------------------------------------------------
+1.39 Testing on the specific architecture
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Testcases for specific arch should be limited on that only being supported
+platform to run, we now involve a .supported_archs to achieve this feature
+in LTP library. All you need to run a test on the expected arch is to set
+the '.supported_archs' array in the 'struct tst_test' to choose the required
+arch list. e.g.
+
+ .supported_archs = (const char *const []){"x86_64", "ppc64", NULL}
+
+This helps move the TCONF info from code to tst_test metadata as well.
+
+And, we also export a struct tst_arch to save the system architecture for
+using in the whole test cases.
+
+ extern struct arch {
+ char name[16];
+ enum tst_arch_type type;
+ } tst_arch;
+
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+
+static struct tst_test test = {
+ ...
+ .setup = setup,
+ .supported_archs = (const char *const []) {
+ "x86_64",
+ "ppc64",
+ "s390x",
+ NULL
+ },
+};
+-------------------------------------------------------------------------------
+
2. Common problems
------------------
diff --git a/include/tst_arch.h b/include/tst_arch.h
new file mode 100644
index 000000000..784c3093b
--- /dev/null
+++ b/include/tst_arch.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (c) 2021 Li Wang <liwang@redhat.com>
+ */
+
+#ifndef TST_ARCH_H__
+#define TST_ARCH_H__
+
+enum tst_arch_type {
+ TST_I386,
+ TST_X86,
+ TST_X86_64,
+ TST_IA64,
+ TST_PPC,
+ TST_PPC64,
+ TST_S390,
+ TST_S390X,
+ TST_ARM,
+ TST_AARCH64,
+ TST_SPARC,
+};
+
+/*
+ * This tst_arch is to save the system architecture for
+ * using in the whole testcase.
+ */
+extern struct arch {
+ char name[16];
+ enum tst_arch_type type;
+} tst_arch;
+
+/*
+ * Check if test platform is in the given arch list. If yes return 1,
+ * otherwise return 0.
+ *
+ * @archlist A NULL terminated array of architectures to support.
+ */
+int tst_is_on_arch(const char *const *archlist);
+
+#endif /* TST_ARCH_H__ */
diff --git a/include/tst_test.h b/include/tst_test.h
index 3dcb45de0..7611520ee 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -43,6 +43,7 @@
#include "tst_fips.h"
#include "tst_taint.h"
#include "tst_memutils.h"
+#include "tst_arch.h"
/*
* Reports testcase result.
@@ -139,6 +140,12 @@ struct tst_test {
const char *min_kver;
+ /*
+ * The supported_archs is a NULL terminated list of archs the test
+ * does support.
+ */
+ const char *const *supported_archs;
+
/* If set the test is compiled out */
const char *tconf_msg;
@@ -316,6 +323,9 @@ static struct tst_test test;
int main(int argc, char *argv[])
{
+ if (!tst_is_on_arch(test.supported_archs))
+ tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
+
tst_run_tcases(argc, argv, &test);
}
diff --git a/lib/tst_arch.c b/lib/tst_arch.c
new file mode 100644
index 000000000..ae5817075
--- /dev/null
+++ b/lib/tst_arch.c
@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (c) 2021 Li Wang <liwang@redhat.com>
+ */
+
+#include <string.h>
+#include <stdlib.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_arch.h"
+#include "tst_test.h"
+
+struct arch tst_arch;
+
+static const char *const arch_type_list[] = {
+ "i386",
+ "x86",
+ "x86_64",
+ "ia64",
+ "ppc",
+ "ppc64",
+ "s390",
+ "s390x",
+ "arm",
+ "aarch64",
+ "sparc",
+ NULL
+};
+
+static int is_matched_arch(const char *arch, const char *name)
+{
+ if (strcmp(arch, name) == 0)
+ return 1;
+
+ return 0;
+}
+
+static int is_valid_arch_name(const char *name)
+{
+ unsigned int i;
+
+ for (i = 0; arch_type_list[i]; i++) {
+ if (is_matched_arch(arch_type_list[i], name))
+ return 1;
+ }
+
+ return 0;
+}
+
+static void get_system_arch(void)
+{
+#if defined(__i386__)
+ strcpy(tst_arch.name, "i386");
+ tst_arch.type = TST_I386;
+#elif defined(__x86__)
+ strcpy(tst_arch.name, "x86");
+ tst_arch.type = TST_X86;
+#elif defined(__x86_64__)
+ strcpy(tst_arch.name, "x86_64");
+ tst_arch.type = TST_X86_64;
+#elif defined(__ia64__)
+ strcpy(tst_arch.name, "ia64");
+ tst_arch.type = TST_IA64;
+#elif defined(__powerpc__)
+ strcpy(tst_arch.name, "ppc");
+ tst_arch.type = TST_PPC;
+#elif defined(__powerpc64__)
+ strcpy(tst_arch.name, "ppc64");
+ tst_arch.type = TST_PPC64;
+#elif defined(__s390x__)
+ strcpy(tst_arch.name, "s390x");
+ tst_arch.type = TST_S390X;
+#elif defined(__s390__)
+ strcpy(tst_arch.name, "s390");
+ tst_arch.type = TST_S390;
+#elif defined(__arm__)
+ strcpy(tst_arch.name, "s390x");
+ tst_arch.type = TST_ARM;
+#elif defined(__aarch64__)
+ strcpy(tst_arch.name, "aarch64");
+ tst_arch.type = TST_AARCH64;
+#elif defined(__sparc__)
+ strcpy(tst_arch.name, "sparc");
+ tst_arch.type = TST_SPARC;
+#endif
+}
+
+int tst_is_on_arch(const char *const *archlist)
+{
+ unsigned int i;
+
+ get_system_arch();
+
+ if (archlist == NULL)
+ return 1;
+
+ for (i = 0; archlist[i]; i++) {
+ if (!is_valid_arch_name(archlist[i]))
+ tst_brk(TBROK, "%s is invalid arch, please reset!",
+ archlist[i]);
+ }
+
+ for (i = 0; archlist[i]; i++) {
+ if (is_matched_arch(tst_arch.name, archlist[i]))
+ return 1;
+ }
+
+ return 0;
+}
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 11+ messages in thread* [LTP] [PATCH v3 2/3] testcase: make use of .supported_archs 2021-11-08 2:14 [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure Li Wang @ 2021-11-08 2:14 ` Li Wang 2021-11-08 12:41 ` Cyril Hrubis 2021-11-08 2:14 ` [LTP] [PATCH v3 3/3] max_map_count: replace ifdefs by tst_arch Li Wang 2021-11-08 12:34 ` [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure Cyril Hrubis 2 siblings, 1 reply; 11+ messages in thread From: Li Wang @ 2021-11-08 2:14 UTC (permalink / raw) To: ltp In some places, we have to keep ifdefs to make the compiler work with unportable code. Signed-off-by: Li Wang <liwang@redhat.com> --- configure.ac | 1 + testcases/cve/meltdown.c | 16 ++++++----- testcases/kernel/syscalls/ptrace/ptrace07.c | 11 ++++---- testcases/kernel/syscalls/ptrace/ptrace08.c | 22 ++++++++------- testcases/kernel/syscalls/ptrace/ptrace09.c | 11 +++++--- testcases/kernel/syscalls/ptrace/ptrace10.c | 12 +++++---- .../syscalls/set_mempolicy/set_mempolicy05.c | 27 +++++++++---------- 7 files changed, 54 insertions(+), 46 deletions(-) diff --git a/configure.ac b/configure.ac index 5bf3c52ec..7efd9df87 100644 --- a/configure.ac +++ b/configure.ac @@ -42,6 +42,7 @@ AC_CHECK_DECLS([SEM_STAT_ANY],,,[#include <sys/sem.h>]) AC_CHECK_HEADERS_ONCE([ \ asm/ldt.h \ + emmintrin.h \ ifaddrs.h \ keyutils.h \ linux/can.h \ diff --git a/testcases/cve/meltdown.c b/testcases/cve/meltdown.c index 5a984aba3..2577c1a80 100644 --- a/testcases/cve/meltdown.c +++ b/testcases/cve/meltdown.c @@ -6,8 +6,6 @@ #include "config.h" #include "tst_test.h" -#if defined(__x86_64__) || defined(__i386__) - #include <stdio.h> #include <string.h> #include <signal.h> @@ -17,6 +15,7 @@ #include <ctype.h> #include <sys/utsname.h> +#ifdef HAVE_EMMINTRIN_H #include <emmintrin.h> #include "tst_tsc.h" @@ -378,14 +377,17 @@ static struct tst_test test = { .test_all = run, .cleanup = cleanup, .min_kver = "2.6.32", + .supported_archs = (const char *const []) { + "i386", + "x86_64", + NULL + }, .tags = (const struct tst_tag[]) { {"CVE", "2017-5754"}, {} } }; -#else /* #if defined(__x86_64__) || defined(__i386__) */ - -TST_TEST_TCONF("not x86_64 or i386"); - -#endif /* #else #if defined(__x86_64__) || defined(__i386__) */ +#else /* HAVE_EMMINTRIN_H */ + TST_TEST_TCONF("<emmintrin.h> is not supported"); +#endif diff --git a/testcases/kernel/syscalls/ptrace/ptrace07.c b/testcases/kernel/syscalls/ptrace/ptrace07.c index 9e3f7511d..da62cadb0 100644 --- a/testcases/kernel/syscalls/ptrace/ptrace07.c +++ b/testcases/kernel/syscalls/ptrace/ptrace07.c @@ -48,13 +48,13 @@ # define NT_X86_XSTATE 0x202 #endif -#ifdef __x86_64__ static void check_regs_loop(uint32_t initval) { const unsigned long num_iters = 1000000000; uint32_t xmm0[4] = { initval, initval, initval, initval }; int status = 1; +#ifdef __x86_64__ asm volatile(" movdqu %0, %%xmm0\n" " mov %0, %%rbx\n" "1: dec %2\n" @@ -68,6 +68,7 @@ static void check_regs_loop(uint32_t initval) "3:\n" : "+m" (xmm0), "+r" (status) : "r" (num_iters) : "rax", "rbx", "xmm0"); +#endif if (status) { tst_res(TFAIL, @@ -178,6 +179,10 @@ static struct tst_test test = { .test_all = do_test, .forks_child = 1, .needs_checkpoints = 1, + .supported_archs = (const char *const []) { + "x86_64", + NULL + }, .tags = (const struct tst_tag[]) { {"linux-git", "814fb7bb7db5"}, {"CVE", "2017-15537"}, @@ -185,7 +190,3 @@ static struct tst_test test = { } }; - -#else /* !__x86_64__ */ - TST_TEST_TCONF("this test is only supported on x86_64"); -#endif /* __x86_64__ */ diff --git a/testcases/kernel/syscalls/ptrace/ptrace08.c b/testcases/kernel/syscalls/ptrace/ptrace08.c index f86f69a9c..77c4c10c3 100644 --- a/testcases/kernel/syscalls/ptrace/ptrace08.c +++ b/testcases/kernel/syscalls/ptrace/ptrace08.c @@ -43,18 +43,16 @@ #include "tst_test.h" #include "tst_safe_stdio.h" -#if defined(__i386__) || defined(__x86_64__) - static pid_t child_pid; -#if defined(__x86_64__) -# define KERN_ADDR_MIN 0xffff800000000000 -# define KERN_ADDR_MAX 0xffffffffffffffff -# define KERN_ADDR_BITS 64 -#elif defined(__i386__) +#if defined(__i386__) # define KERN_ADDR_MIN 0xc0000000 # define KERN_ADDR_MAX 0xffffffff # define KERN_ADDR_BITS 32 +#else +# define KERN_ADDR_MIN 0xffff800000000000 +# define KERN_ADDR_MAX 0xffffffffffffffff +# define KERN_ADDR_BITS 64 #endif static int deffered_check; @@ -106,6 +104,7 @@ static void ptrace_try_kern_addr(unsigned long kern_addr) if (SAFE_WAITPID(child_pid, &status, WUNTRACED) != child_pid) tst_brk(TBROK, "Received event from unexpected PID"); +#if defined(__i386__) || defined(__x86_64__) SAFE_PTRACE(PTRACE_ATTACH, child_pid, NULL, NULL); SAFE_PTRACE(PTRACE_POKEUSER, child_pid, (void *)offsetof(struct user, u_debugreg[0]), (void *)1); @@ -135,6 +134,7 @@ static void ptrace_try_kern_addr(unsigned long kern_addr) addr = ptrace(PTRACE_PEEKUSER, child_pid, (void*)offsetof(struct user, u_debugreg[0]), NULL); +#endif if (!deffered_check && addr == kern_addr) tst_res(TFAIL, "Was able to set breakpoint on kernel addr"); @@ -164,6 +164,11 @@ static struct tst_test test = { .setup = setup, .cleanup = cleanup, .forks_child = 1, + .supported_archs = (const char *const []) { + "i386", + "x86_64", + NULL + }, .tags = (const struct tst_tag[]) { {"linux-git", "f67b15037a7a"}, {"CVE", "2018-1000199"}, @@ -171,6 +176,3 @@ static struct tst_test test = { {} } }; -#else -TST_TEST_TCONF("This test is only supported on x86 systems"); -#endif diff --git a/testcases/kernel/syscalls/ptrace/ptrace09.c b/testcases/kernel/syscalls/ptrace/ptrace09.c index 85875ce65..8ccdfcc4b 100644 --- a/testcases/kernel/syscalls/ptrace/ptrace09.c +++ b/testcases/kernel/syscalls/ptrace/ptrace09.c @@ -22,7 +22,6 @@ #include <signal.h> #include "tst_test.h" -#if defined(__i386__) || defined(__x86_64__) static short watchpoint; static pid_t child_pid; @@ -46,6 +45,7 @@ static void run(void) { int status; +#if defined(__i386__) || defined(__x86_64__) child_pid = SAFE_FORK(); if (!child_pid) { @@ -60,6 +60,7 @@ static void run(void) SAFE_PTRACE(PTRACE_POKEUSER, child_pid, (void *)offsetof(struct user, u_debugreg[7]), (void *)0x30001); SAFE_PTRACE(PTRACE_CONT, child_pid, NULL, NULL); +#endif while (1) { if (SAFE_WAITPID(child_pid, &status, 0) != child_pid) @@ -92,12 +93,14 @@ static struct tst_test test = { .test_all = run, .cleanup = cleanup, .forks_child = 1, + .supported_archs = (const char *const []) { + "i386", + "x86_64", + NULL + }, .tags = (const struct tst_tag[]) { {"linux-git", "d8ba61ba58c8"}, {"CVE", "2018-8897"}, {} } }; -#else -TST_TEST_TCONF("This test is only supported on x86 systems"); -#endif diff --git a/testcases/kernel/syscalls/ptrace/ptrace10.c b/testcases/kernel/syscalls/ptrace/ptrace10.c index b5d6b9f8f..3bd8ca1a9 100644 --- a/testcases/kernel/syscalls/ptrace/ptrace10.c +++ b/testcases/kernel/syscalls/ptrace/ptrace10.c @@ -22,8 +22,6 @@ #include <signal.h> #include "tst_test.h" -#if defined(__i386__) || defined(__x86_64__) - static pid_t child_pid; static void child_main(void) @@ -45,6 +43,7 @@ static void run(void) if (SAFE_WAITPID(child_pid, &status, WUNTRACED) != child_pid) tst_brk(TBROK, "Received event from unexpected PID"); +#if defined(__i386__) || defined(__x86_64__) SAFE_PTRACE(PTRACE_ATTACH, child_pid, NULL, NULL); SAFE_PTRACE(PTRACE_POKEUSER, child_pid, (void *)offsetof(struct user, u_debugreg[0]), (void *)1); @@ -53,6 +52,7 @@ static void run(void) addr = ptrace(PTRACE_PEEKUSER, child_pid, (void*)offsetof(struct user, u_debugreg[0]), NULL); +#endif if (addr == 2) tst_res(TPASS, "The rd0 was set on second PTRACE_POKEUSR"); @@ -76,11 +76,13 @@ static struct tst_test test = { .test_all = run, .cleanup = cleanup, .forks_child = 1, + .supported_archs = (const char *const []) { + "i386", + "x86_64", + NULL + }, .tags = (const struct tst_tag[]) { {"linux-git", "bd14406b78e6"}, {} } }; -#else -TST_TEST_TCONF("This test is only supported on x86 systems"); -#endif diff --git a/testcases/kernel/syscalls/set_mempolicy/set_mempolicy05.c b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy05.c index 86f6a95dc..f56b7b26f 100644 --- a/testcases/kernel/syscalls/set_mempolicy/set_mempolicy05.c +++ b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy05.c @@ -37,18 +37,10 @@ #include "config.h" #include "tst_test.h" -#if defined(__i386__) || defined(__powerpc__) - #include <string.h> static unsigned int i; static int sys_ret; -#ifdef __i386__ -static const int sys_num = 276; -static const int mode; -static const int node_mask_ptr = UINT_MAX; -static const int node_mask_sz = UINT_MAX; -#endif static volatile char *stack_ptr; static void run(void) @@ -58,6 +50,11 @@ static void run(void) register long mode __asm__("r3"); register long node_mask_ptr __asm__("r4"); register long node_mask_sz __asm__("r5"); +#else + static const int sys_num = 276; + static const int mode; + static const int node_mask_ptr = UINT_MAX; + static const int node_mask_sz = UINT_MAX; #endif char stack_pattern[0x400]; @@ -78,7 +75,8 @@ static void run(void) : "memory", "cr0", "r6", "r7", "r8", "r9", "r10", "r11", "r12"); sys_ret = mode; -#else /* __i386__ */ +#endif +#ifdef __i386__ asm volatile ( "add $0x400, %%esp\n\t" "int $0x80\n\t" @@ -114,15 +112,14 @@ static void run(void) static struct tst_test test = { .test_all = run, + .supported_archs = (const char *const []) { + "i386", + "ppc", + NULL + }, .tags = (const struct tst_tag[]) { {"linux-git", "cf01fb9985e8"}, {"CVE", "CVE-2017-7616"}, {} } }; - -#else /* #if defined(__x86_64__) || defined(__powerpc__) */ - -TST_TEST_TCONF("not i386 or powerpc"); - -#endif /* #else #if defined(__x86_64__) || defined(__powerpc__) */ -- 2.31.1 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3 2/3] testcase: make use of .supported_archs 2021-11-08 2:14 ` [LTP] [PATCH v3 2/3] testcase: make use of .supported_archs Li Wang @ 2021-11-08 12:41 ` Cyril Hrubis 2021-11-09 6:52 ` Li Wang 0 siblings, 1 reply; 11+ messages in thread From: Cyril Hrubis @ 2021-11-08 12:41 UTC (permalink / raw) To: Li Wang; +Cc: ltp Hi! > asm volatile ( > "add $0x400, %%esp\n\t" > "int $0x80\n\t" > @@ -114,15 +112,14 @@ static void run(void) > > static struct tst_test test = { > .test_all = run, > + .supported_archs = (const char *const []) { > + "i386", > + "ppc", > + NULL > + }, > .tags = (const struct tst_tag[]) { > {"linux-git", "cf01fb9985e8"}, > {"CVE", "CVE-2017-7616"}, > {} > } > }; > - > -#else /* #if defined(__x86_64__) || defined(__powerpc__) */ > - > -TST_TEST_TCONF("not i386 or powerpc"); > - > -#endif /* #else #if defined(__x86_64__) || defined(__powerpc__) */ Accordingly to this table: https://wiki.debian.org/ArchitectureSpecificsMemo __powerpc__ matches both 32bit and 64bit variants. I guess that we would have to change the checks in the library as: #ifdef __powerpc__ # ifdef __powerpc64__ || __ppc64__ .arch = "ppc64", .type = TST_PPC64, # else .arch = "ppc", .type = "TST_PPC" # endif #endif Also I guess that gcc does not define __x86__ for historical reasons and __i386__ really means __x86__, but I haven't checked that one. -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3 2/3] testcase: make use of .supported_archs 2021-11-08 12:41 ` Cyril Hrubis @ 2021-11-09 6:52 ` Li Wang 0 siblings, 0 replies; 11+ messages in thread From: Li Wang @ 2021-11-09 6:52 UTC (permalink / raw) To: Cyril Hrubis; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 1916 bytes --] On Mon, Nov 8, 2021 at 8:40 PM Cyril Hrubis <chrubis@suse.cz> wrote: > Hi! > > asm volatile ( > > "add $0x400, %%esp\n\t" > > "int $0x80\n\t" > > @@ -114,15 +112,14 @@ static void run(void) > > > > static struct tst_test test = { > > .test_all = run, > > + .supported_archs = (const char *const []) { > > + "i386", > > + "ppc", > > + NULL > > + }, > > .tags = (const struct tst_tag[]) { > > {"linux-git", "cf01fb9985e8"}, > > {"CVE", "CVE-2017-7616"}, > > {} > > } > > }; > > - > > -#else /* #if defined(__x86_64__) || defined(__powerpc__) */ > > - > > -TST_TEST_TCONF("not i386 or powerpc"); > > - > > -#endif /* #else #if defined(__x86_64__) || defined(__powerpc__) */ > > Accordingly to this table: > > https://wiki.debian.org/ArchitectureSpecificsMemo > > __powerpc__ matches both 32bit and 64bit variants. > > I guess that we would have to change the checks in the library as: > Yes, but I think we can simply reverse the order to solve this. It will try to match 64bit firstly otherwise 32bit. The s390 does as well. .... #elif defined(__powerpc64__ || __ppc64__) .name = "ppc64", .type = TST_PPC64, #elif defined(__powerpc__ || __ppc__) .name = "ppc", .type = TST_PPC, #elif defined(__s390x__) .name = "s390x", .type = TST_S390X, #elif defined(__s390__) .name = "s390", .type = TST_S390, .... > > #ifdef __powerpc__ > # ifdef __powerpc64__ || __ppc64__ > .arch = "ppc64", > .type = TST_PPC64, > # else > .arch = "ppc", > .type = "TST_PPC" > # endif > #endif > > Also I guess that gcc does not define __x86__ for historical reasons and > __i386__ really means __x86__, but I haven't checked that one. > You are right. And next we have to get rid of __x86__ from ltp testcase. -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 3607 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3 3/3] max_map_count: replace ifdefs by tst_arch 2021-11-08 2:14 [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure Li Wang 2021-11-08 2:14 ` [LTP] [PATCH v3 2/3] testcase: make use of .supported_archs Li Wang @ 2021-11-08 2:14 ` Li Wang 2021-11-08 12:44 ` Cyril Hrubis 2021-11-08 12:34 ` [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure Cyril Hrubis 2 siblings, 1 reply; 11+ messages in thread From: Li Wang @ 2021-11-08 2:14 UTC (permalink / raw) To: ltp Signed-off-by: Li Wang <liwang@redhat.com> --- testcases/kernel/mem/tunable/max_map_count.c | 41 +++++++++++--------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/testcases/kernel/mem/tunable/max_map_count.c b/testcases/kernel/mem/tunable/max_map_count.c index 4f0ad0037..9da67520c 100644 --- a/testcases/kernel/mem/tunable/max_map_count.c +++ b/testcases/kernel/mem/tunable/max_map_count.c @@ -91,24 +91,29 @@ static bool filter_map(const char *line) if (ret != 1) return false; -#if defined(__x86_64__) || defined(__x86__) - /* On x86, there's an old compat vsyscall page */ - if (!strcmp(buf, "[vsyscall]")) - return true; -#elif defined(__ia64__) - /* On ia64, the vdso is not a proper mapping */ - if (!strcmp(buf, "[vdso]")) - return true; -#elif defined(__arm__) - /* Skip it when run it in aarch64 */ - if ((!strcmp(un.machine, "aarch64")) - || (!strcmp(un.machine, "aarch64_be"))) - return false; - - /* Older arm kernels didn't label their vdso maps */ - if (!strncmp(line, "ffff0000-ffff1000", 17)) - return true; -#endif + switch (tst_arch.type) { + case TST_X86: + case TST_X86_64: + /* On x86, there's an old compat vsyscall page */ + if (!strcmp(buf, "[vsyscall]")) + return true; + break; + case TST_IA64: + /* On ia64, the vdso is not a proper mapping */ + if (!strcmp(buf, "[vdso]")) + return true; + break; + case TST_ARM: + /* Skip it when run it in aarch64 */ + if ((!strcmp(un.machine, "aarch64")) + || (!strcmp(un.machine, "aarch64_be"))) + return false; + + /* Older arm kernels didn't label their vdso maps */ + if (!strncmp(line, "ffff0000-ffff1000", 17)) + return true; + break; + }; return false; } -- 2.31.1 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3 3/3] max_map_count: replace ifdefs by tst_arch 2021-11-08 2:14 ` [LTP] [PATCH v3 3/3] max_map_count: replace ifdefs by tst_arch Li Wang @ 2021-11-08 12:44 ` Cyril Hrubis 2021-11-09 9:35 ` Li Wang 0 siblings, 1 reply; 11+ messages in thread From: Cyril Hrubis @ 2021-11-08 12:44 UTC (permalink / raw) To: Li Wang; +Cc: ltp On Mon, Nov 08, 2021 at 10:14:50AM +0800, Li Wang wrote: > Signed-off-by: Li Wang <liwang@redhat.com> > --- > testcases/kernel/mem/tunable/max_map_count.c | 41 +++++++++++--------- > 1 file changed, 23 insertions(+), 18 deletions(-) > > diff --git a/testcases/kernel/mem/tunable/max_map_count.c b/testcases/kernel/mem/tunable/max_map_count.c > index 4f0ad0037..9da67520c 100644 > --- a/testcases/kernel/mem/tunable/max_map_count.c > +++ b/testcases/kernel/mem/tunable/max_map_count.c > @@ -91,24 +91,29 @@ static bool filter_map(const char *line) > if (ret != 1) > return false; > > -#if defined(__x86_64__) || defined(__x86__) > - /* On x86, there's an old compat vsyscall page */ > - if (!strcmp(buf, "[vsyscall]")) > - return true; > -#elif defined(__ia64__) > - /* On ia64, the vdso is not a proper mapping */ > - if (!strcmp(buf, "[vdso]")) > - return true; > -#elif defined(__arm__) > - /* Skip it when run it in aarch64 */ > - if ((!strcmp(un.machine, "aarch64")) > - || (!strcmp(un.machine, "aarch64_be"))) > - return false; > - > - /* Older arm kernels didn't label their vdso maps */ > - if (!strncmp(line, "ffff0000-ffff1000", 17)) > - return true; > -#endif > + switch (tst_arch.type) { > + case TST_X86: > + case TST_X86_64: > + /* On x86, there's an old compat vsyscall page */ > + if (!strcmp(buf, "[vsyscall]")) > + return true; > + break; > + case TST_IA64: > + /* On ia64, the vdso is not a proper mapping */ > + if (!strcmp(buf, "[vdso]")) > + return true; > + break; > + case TST_ARM: > + /* Skip it when run it in aarch64 */ > + if ((!strcmp(un.machine, "aarch64")) > + || (!strcmp(un.machine, "aarch64_be"))) > + return false; I wonder if this would be better as: if (tst_kernel_bits() == 64) return false; Other than this the code looks actually better this way. > + /* Older arm kernels didn't label their vdso maps */ > + if (!strncmp(line, "ffff0000-ffff1000", 17)) > + return true; > + break; > + }; > > return false; > } > -- > 2.31.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3 3/3] max_map_count: replace ifdefs by tst_arch 2021-11-08 12:44 ` Cyril Hrubis @ 2021-11-09 9:35 ` Li Wang 2021-11-09 10:47 ` Cyril Hrubis 0 siblings, 1 reply; 11+ messages in thread From: Li Wang @ 2021-11-09 9:35 UTC (permalink / raw) To: Cyril Hrubis; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 939 bytes --] > > + switch (tst_arch.type) { > > + case TST_X86: > > + case TST_X86_64: > > + /* On x86, there's an old compat vsyscall page */ > > + if (!strcmp(buf, "[vsyscall]")) > > + return true; > > + break; > > + case TST_IA64: > > + /* On ia64, the vdso is not a proper mapping */ > > + if (!strcmp(buf, "[vdso]")) > > + return true; > > + break; > > + case TST_ARM: > > + /* Skip it when run it in aarch64 */ > > + if ((!strcmp(un.machine, "aarch64")) > > + || (!strcmp(un.machine, "aarch64_be"))) > > + return false; > > I wonder if this would be better as: > > if (tst_kernel_bits() == 64) > return false; > Actually, we have TST_AARCH64 already, I'd go with switch to that. -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 1636 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3 3/3] max_map_count: replace ifdefs by tst_arch 2021-11-09 9:35 ` Li Wang @ 2021-11-09 10:47 ` Cyril Hrubis 2021-11-09 11:41 ` Li Wang 0 siblings, 1 reply; 11+ messages in thread From: Cyril Hrubis @ 2021-11-09 10:47 UTC (permalink / raw) To: Li Wang; +Cc: LTP List Hi! > > > + switch (tst_arch.type) { > > > + case TST_X86: > > > + case TST_X86_64: > > > + /* On x86, there's an old compat vsyscall page */ > > > + if (!strcmp(buf, "[vsyscall]")) > > > + return true; > > > + break; > > > + case TST_IA64: > > > + /* On ia64, the vdso is not a proper mapping */ > > > + if (!strcmp(buf, "[vdso]")) > > > + return true; > > > + break; > > > + case TST_ARM: > > > + /* Skip it when run it in aarch64 */ > > > + if ((!strcmp(un.machine, "aarch64")) > > > + || (!strcmp(un.machine, "aarch64_be"))) > > > + return false; > > > > I wonder if this would be better as: > > > > if (tst_kernel_bits() == 64) > > return false; > > > > Actually, we have TST_AARCH64 already, I'd go with switch to that. That wouldn't work right? Since we are checking here if 32bit ARM binary runs on 64bit AARCH64 kernel. The tst_arch defines for which architecture the binary was build while the tst_kernel_bits() checks if the kernel is 32bit or 64bit. -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3 3/3] max_map_count: replace ifdefs by tst_arch 2021-11-09 10:47 ` Cyril Hrubis @ 2021-11-09 11:41 ` Li Wang 0 siblings, 0 replies; 11+ messages in thread From: Li Wang @ 2021-11-09 11:41 UTC (permalink / raw) To: Cyril Hrubis; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 578 bytes --] > > > I wonder if this would be better as: > > > > > > if (tst_kernel_bits() == 64) > > > return false; > > > > > > > Actually, we have TST_AARCH64 already, I'd go with switch to that. > > That wouldn't work right? Since we are checking here if 32bit ARM binary > runs on 64bit AARCH64 kernel. The tst_arch defines for which > architecture the binary was build while the tst_kernel_bits() checks if > the kernel is 32bit or 64bit. > Right, I didn't realize that situation. Thanks for the reminder~ -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 1100 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-08 2:14 [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure Li Wang 2021-11-08 2:14 ` [LTP] [PATCH v3 2/3] testcase: make use of .supported_archs Li Wang 2021-11-08 2:14 ` [LTP] [PATCH v3 3/3] max_map_count: replace ifdefs by tst_arch Li Wang @ 2021-11-08 12:34 ` Cyril Hrubis 2021-11-09 7:52 ` Li Wang 2 siblings, 1 reply; 11+ messages in thread From: Cyril Hrubis @ 2021-11-08 12:34 UTC (permalink / raw) To: Li Wang; +Cc: ltp Hi! > Testcases for specific arch should be limited on that only being supported > platform to run, we now involve a .supported_archs to achieve this feature > in LTP library. All you need to run a test on the expected arch is to set > the '.supported_archs' array in the 'struct tst_test' to choose the required > arch list. e.g. > > .supported_archs = (const char *const []){"x86_64", "ppc64", NULL} > > This helps move the TCONF info from code to tst_test metadata as well. > > And, we also export a struct tst_arch to save the system architecture > for using in the whole test cases. > > extern struct arch { > char name[16]; > enum tst_arch_type type; > } tst_arch; > > Signed-off-by: Li Wang <liwang@redhat.com> > --- > doc/c-test-api.txt | 36 +++++++++++++++ > include/tst_arch.h | 39 ++++++++++++++++ > include/tst_test.h | 10 +++++ > lib/tst_arch.c | 108 +++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 193 insertions(+) > create mode 100644 include/tst_arch.h > create mode 100644 lib/tst_arch.c > > diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt > index 3127018a4..5e32b7075 100644 > --- a/doc/c-test-api.txt > +++ b/doc/c-test-api.txt > @@ -2261,6 +2261,42 @@ struct tst_test test = { > }; > ------------------------------------------------------------------------------- > > +1.39 Testing on the specific architecture > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > +Testcases for specific arch should be limited on that only being supported > +platform to run, we now involve a .supported_archs to achieve this feature > +in LTP library. All you need to run a test on the expected arch is to set > +the '.supported_archs' array in the 'struct tst_test' to choose the required > +arch list. e.g. > + > + .supported_archs = (const char *const []){"x86_64", "ppc64", NULL} > + > +This helps move the TCONF info from code to tst_test metadata as well. > + > +And, we also export a struct tst_arch to save the system architecture for > +using in the whole test cases. > + > + extern struct arch { > + char name[16]; > + enum tst_arch_type type; > + } tst_arch; > + > +[source,c] > +------------------------------------------------------------------------------- > +#include "tst_test.h" > + > +static struct tst_test test = { > + ... > + .setup = setup, > + .supported_archs = (const char *const []) { > + "x86_64", > + "ppc64", > + "s390x", > + NULL > + }, > +}; > +------------------------------------------------------------------------------- > + > 2. Common problems > ------------------ > > diff --git a/include/tst_arch.h b/include/tst_arch.h > new file mode 100644 > index 000000000..784c3093b > --- /dev/null > +++ b/include/tst_arch.h > @@ -0,0 +1,39 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later > + * Copyright (c) 2021 Li Wang <liwang@redhat.com> > + */ > + > +#ifndef TST_ARCH_H__ > +#define TST_ARCH_H__ > + > +enum tst_arch_type { > + TST_I386, > + TST_X86, Why do we have both i386 and X86 here? Isn't __i386__ synonymous for __x86__ does gcc even define __x86__? I doubt that we care about the differencies between i386, i586 and i686 at all. I would just keep TST_X86 in the list for any 32bit intel compatible hardware. > + TST_X86_64, > + TST_IA64, > + TST_PPC, > + TST_PPC64, > + TST_S390, > + TST_S390X, > + TST_ARM, > + TST_AARCH64, > + TST_SPARC, > +}; > + > +/* > + * This tst_arch is to save the system architecture for > + * using in the whole testcase. > + */ > +extern struct arch { > + char name[16]; > + enum tst_arch_type type; > +} tst_arch; the struct should be prefixed with tst_ as well. i.e. extern struct tst_arch { ... > +/* > + * Check if test platform is in the given arch list. If yes return 1, > + * otherwise return 0. > + * > + * @archlist A NULL terminated array of architectures to support. > + */ > +int tst_is_on_arch(const char *const *archlist); > + > +#endif /* TST_ARCH_H__ */ > diff --git a/include/tst_test.h b/include/tst_test.h > index 3dcb45de0..7611520ee 100644 > --- a/include/tst_test.h > +++ b/include/tst_test.h > @@ -43,6 +43,7 @@ > #include "tst_fips.h" > #include "tst_taint.h" > #include "tst_memutils.h" > +#include "tst_arch.h" > > /* > * Reports testcase result. > @@ -139,6 +140,12 @@ struct tst_test { > > const char *min_kver; > > + /* > + * The supported_archs is a NULL terminated list of archs the test > + * does support. > + */ > + const char *const *supported_archs; > + > /* If set the test is compiled out */ > const char *tconf_msg; > > @@ -316,6 +323,9 @@ static struct tst_test test; > > int main(int argc, char *argv[]) > { > + if (!tst_is_on_arch(test.supported_archs)) > + tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name); Why are you putting this check into the header? All the other checks are in the do_setup() in lib/tst_test.c > tst_run_tcases(argc, argv, &test); > } > > diff --git a/lib/tst_arch.c b/lib/tst_arch.c > new file mode 100644 > index 000000000..ae5817075 > --- /dev/null > +++ b/lib/tst_arch.c > @@ -0,0 +1,108 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later > + * Copyright (c) 2021 Li Wang <liwang@redhat.com> > + */ > + > +#include <string.h> > +#include <stdlib.h> > + > +#define TST_NO_DEFAULT_MAIN > +#include "tst_arch.h" > +#include "tst_test.h" > + > +struct arch tst_arch; > + > +static const char *const arch_type_list[] = { > + "i386", > + "x86", > + "x86_64", > + "ia64", > + "ppc", > + "ppc64", > + "s390", > + "s390x", > + "arm", > + "aarch64", > + "sparc", > + NULL > +}; > + > +static int is_matched_arch(const char *arch, const char *name) > +{ > + if (strcmp(arch, name) == 0) > + return 1; > + > + return 0; > +} > + > +static int is_valid_arch_name(const char *name) > +{ > + unsigned int i; > + > + for (i = 0; arch_type_list[i]; i++) { > + if (is_matched_arch(arch_type_list[i], name)) > + return 1; > + } > + > + return 0; > +} > + > +static void get_system_arch(void) > +{ > +#if defined(__i386__) > + strcpy(tst_arch.name, "i386"); > + tst_arch.type = TST_I386; > +#elif defined(__x86__) > + strcpy(tst_arch.name, "x86"); > + tst_arch.type = TST_X86; > +#elif defined(__x86_64__) > + strcpy(tst_arch.name, "x86_64"); > + tst_arch.type = TST_X86_64; > +#elif defined(__ia64__) > + strcpy(tst_arch.name, "ia64"); > + tst_arch.type = TST_IA64; > +#elif defined(__powerpc__) > + strcpy(tst_arch.name, "ppc"); > + tst_arch.type = TST_PPC; > +#elif defined(__powerpc64__) > + strcpy(tst_arch.name, "ppc64"); > + tst_arch.type = TST_PPC64; > +#elif defined(__s390x__) > + strcpy(tst_arch.name, "s390x"); > + tst_arch.type = TST_S390X; > +#elif defined(__s390__) > + strcpy(tst_arch.name, "s390"); > + tst_arch.type = TST_S390; > +#elif defined(__arm__) > + strcpy(tst_arch.name, "s390x"); > + tst_arch.type = TST_ARM; > +#elif defined(__aarch64__) > + strcpy(tst_arch.name, "aarch64"); > + tst_arch.type = TST_AARCH64; > +#elif defined(__sparc__) > + strcpy(tst_arch.name, "sparc"); > + tst_arch.type = TST_SPARC; > +#endif > +} Actually we can initialize this staically as: const struct arch tst_arch = { #if defined(__x86_64__) .name = "x86_64", .type = TST_X86_64, #elif ... #else .name = "unknown", .type = TST_ARCH_UNKNOWN, #endif }; > +int tst_is_on_arch(const char *const *archlist) > +{ > + unsigned int i; > + > + get_system_arch(); > + > + if (archlist == NULL) > + return 1; Just if (!archlist) > + for (i = 0; archlist[i]; i++) { > + if (!is_valid_arch_name(archlist[i])) > + tst_brk(TBROK, "%s is invalid arch, please reset!", ^ fix? > + archlist[i]); > + } > + > + for (i = 0; archlist[i]; i++) { > + if (is_matched_arch(tst_arch.name, archlist[i])) I would just do !strcmp() here, there is no point in havin a function that just calls strcmp() and returns the result. > + return 1; > + } > + > + return 0; > +} > -- > 2.31.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-08 12:34 ` [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure Cyril Hrubis @ 2021-11-09 7:52 ` Li Wang 0 siblings, 0 replies; 11+ messages in thread From: Li Wang @ 2021-11-09 7:52 UTC (permalink / raw) To: Cyril Hrubis; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 1068 bytes --] > > diff --git a/include/tst_arch.h b/include/tst_arch.h > > new file mode 100644 > > index 000000000..784c3093b > > --- /dev/null > > +++ b/include/tst_arch.h > > @@ -0,0 +1,39 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later > > + * Copyright (c) 2021 Li Wang <liwang@redhat.com> > > + */ > > + > > +#ifndef TST_ARCH_H__ > > +#define TST_ARCH_H__ > > + > > +enum tst_arch_type { > > + TST_I386, > > + TST_X86, > > Why do we have both i386 and X86 here? Isn't __i386__ synonymous for > __x86__ does gcc even define __x86__? > My fault, I just copy that from testcase defines and didn't check if __x86__ validatable or not. > > I doubt that we care about the differencies between i386, i586 and i686 > at all. I would just keep TST_X86 in the list for any 32bit intel > compatible hardware. > I prefer to only keep TST_I386 and TST_X86_64 for use. Becuase so far I didn't see there is a requirement on i[4|5|6]86 in LTP at all. And, we can add that if we really need it in the future. The rest of the comments looks good, thanks. -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 2119 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-11-09 11:42 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-11-08 2:14 [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure Li Wang 2021-11-08 2:14 ` [LTP] [PATCH v3 2/3] testcase: make use of .supported_archs Li Wang 2021-11-08 12:41 ` Cyril Hrubis 2021-11-09 6:52 ` Li Wang 2021-11-08 2:14 ` [LTP] [PATCH v3 3/3] max_map_count: replace ifdefs by tst_arch Li Wang 2021-11-08 12:44 ` Cyril Hrubis 2021-11-09 9:35 ` Li Wang 2021-11-09 10:47 ` Cyril Hrubis 2021-11-09 11:41 ` Li Wang 2021-11-08 12:34 ` [LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure Cyril Hrubis 2021-11-09 7:52 ` Li Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.