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