[PATCH 2/2] selftests/fchmodat2: fix clang build failure due to -static-libasan
gcc requires -static-libasan in order to ensure that Address Sanitizer's library is the first one loaded. However, this leads to build failures on clang, when building via: make LLVM=1 -C tools/testing/selftests However, clang already does the right thing by default: it statically links the Address Sanitizer if -fsanitize is specified. Therefore, simply omit -static-libasan for clang builds. And leave behind a comment, because the whole reason for static linking might not be obvious. Cc: Ryan Roberts Signed-off-by: John Hubbard --- tools/testing/selftests/fchmodat2/Makefile | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/fchmodat2/Makefile b/tools/testing/selftests/fchmodat2/Makefile index 71ec34bf1501..4373cea79b79 100644 --- a/tools/testing/selftests/fchmodat2/Makefile +++ b/tools/testing/selftests/fchmodat2/Makefile @@ -1,6 +1,15 @@ # SPDX-License-Identifier: GPL-2.0-or-later -CFLAGS += -Wall -O2 -g -fsanitize=address -fsanitize=undefined -static-libasan $(KHDR_INCLUDES) +CFLAGS += -Wall -O2 -g -fsanitize=address -fsanitize=undefined $(KHDR_INCLUDES) + +# gcc requires -static-libasan in order to ensure that Address Sanitizer's +# library is the first one loaded. However, clang already statically links the +# Address Sanitizer if -fsanitize is specified. Therefore, simply omit +# -static-libasan for clang builds. +ifeq ($(LLVM),) +CFLAGS += -static-libasan +endif + TEST_GEN_PROGS := fchmodat2_test include ../lib.mk -- 2.45.0
[PATCH 1/2] selftests/openat2: fix clang build failures: -static-libasan, LOCAL_HDRS
When building with clang via: make LLVM=1 -C tools/testing/selftests two distinct failures occur: 1) gcc requires -static-libasan in order to ensure that Address Sanitizer's library is the first one loaded. However, this leads to build failures on clang, when building via: make LLVM=1 -C tools/testing/selftests However, clang already does the right thing by default: it statically links the Address Sanitizer if -fsanitize is specified. Therefore, fix this by simply omitting -static-libasan for clang builds. And leave behind a comment, because the whole reason for static linking might not be obvious. 2) clang won't accept invocations of this form, but gcc will: $(CC) file1.c header2.h Fix this by using selftests/lib.mk facilities for tracking local header file dependencies: add them to LOCAL_HDRS, leaving only the .c files to be passed to the compiler. Cc: Ryan Roberts Signed-off-by: John Hubbard --- tools/testing/selftests/openat2/Makefile | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/openat2/Makefile b/tools/testing/selftests/openat2/Makefile index 254d676a2689..185dc76ebb5f 100644 --- a/tools/testing/selftests/openat2/Makefile +++ b/tools/testing/selftests/openat2/Makefile @@ -1,8 +1,18 @@ # SPDX-License-Identifier: GPL-2.0-or-later -CFLAGS += -Wall -O2 -g -fsanitize=address -fsanitize=undefined -static-libasan +CFLAGS += -Wall -O2 -g -fsanitize=address -fsanitize=undefined TEST_GEN_PROGS := openat2_test resolve_test rename_attack_test +# gcc requires -static-libasan in order to ensure that Address Sanitizer's +# library is the first one loaded. However, clang already statically links the +# Address Sanitizer if -fsanitize is specified. Therefore, simply omit +# -static-libasan for clang builds. +ifeq ($(LLVM),) +CFLAGS += -static-libasan +endif + +LOCAL_HDRS += helpers.h + include ../lib.mk -$(TEST_GEN_PROGS): helpers.c helpers.h +$(TEST_GEN_PROGS): helpers.c base-commit: ddb4c3f25b7b95df3d6932db0b379d768a6ebdf7 prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27 -- 2.45.0
[PATCH] selftests/exec: build with -fPIE instead of -pie, to make clang happy
clang doesn't deal well with "-pie -static": it warns that -pie is an unused option here. Changing to "-fPIE -static" solves this problem for clang, while keeping the gcc results identical. The problem is visible when building via: make LLVM=1 -C tools/testing/selftests Again: gcc 13 produces identical binaries for all of these programs, both before and after this commit (using "-pie"), and after (using "-fPIE"). Also, the runtime results are the same for both clang and gcc builds. Signed-off-by: John Hubbard --- tools/testing/selftests/exec/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile index fb4472ddffd8..b7b54d442378 100644 --- a/tools/testing/selftests/exec/Makefile +++ b/tools/testing/selftests/exec/Makefile @@ -29,8 +29,8 @@ $(OUTPUT)/execveat.denatured: $(OUTPUT)/execveat cp $< $@ chmod -x $@ $(OUTPUT)/load_address_4096: load_address.c - $(CC) $(CFLAGS) $(LDFLAGS) -Wl,-z,max-page-size=0x1000 -pie -static $< -o $@ + $(CC) $(CFLAGS) $(LDFLAGS) -Wl,-z,max-page-size=0x1000 -fPIE -static $< -o $@ $(OUTPUT)/load_address_2097152: load_address.c - $(CC) $(CFLAGS) $(LDFLAGS) -Wl,-z,max-page-size=0x20 -pie -static $< -o $@ + $(CC) $(CFLAGS) $(LDFLAGS) -Wl,-z,max-page-size=0x20 -fPIE -static $< -o $@ $(OUTPUT)/load_address_16777216: load_address.c - $(CC) $(CFLAGS) $(LDFLAGS) -Wl,-z,max-page-size=0x100 -pie -static $< -o $@ + $(CC) $(CFLAGS) $(LDFLAGS) -Wl,-z,max-page-size=0x100 -fPIE -static $< -o $@ base-commit: ddb4c3f25b7b95df3d6932db0b379d768a6ebdf7 prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27 -- 2.45.0
[PATCH] selftests/alsa: fix a build warning: return a value in all cases
dump_config_tree() is declared to return an int, but the compiler cannot prove that it always returns any value at all. This leads to a clang warning, when building via: make LLVM=1 -C tools/testing/selftests Fix this by unconditionally returning the "err" variable if the code reaches the end of the function. Signed-off-by: John Hubbard --- tools/testing/selftests/alsa/conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/alsa/conf.c b/tools/testing/selftests/alsa/conf.c index 89e3656a042d..0109fde53e6f 100644 --- a/tools/testing/selftests/alsa/conf.c +++ b/tools/testing/selftests/alsa/conf.c @@ -116,6 +116,8 @@ static int dump_config_tree(snd_config_t *top) if (snd_config_save(top, out)) ksft_exit_fail_msg("config save\n"); snd_output_close(out); + + return err; } snd_config_t *conf_load_from_file(const char *filename) base-commit: ddb4c3f25b7b95df3d6932db0b379d768a6ebdf7 prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27 -- 2.45.0
[PATCH for-next] selftests/ftrace: Fix required features for VFS type test case
From: Masami Hiramatsu (Google) Since the VFS type argument test case uses fprobe events, it must check the availablity of dynamic_events file and fprobe events syntax in README. Without this fix, the test fails if CONFIG_FPROBE_EVENTS=n. Fixes: ee97e5e135c6 ("selftests/ftrace: add fprobe test cases for VFS type "%pd" and "%pD"") Signed-off-by: Masami Hiramatsu (Google) --- .../ftrace/test.d/dynevent/fprobe_args_vfs.tc |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_args_vfs.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_args_vfs.tc index 49a833bf334c..c6a9d2466a71 100644 --- a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_args_vfs.tc +++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_args_vfs.tc @@ -1,7 +1,8 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 # description: Fprobe event VFS type argument -# requires: kprobe_events "%pd/%pD":README +# requires: dynamic_events "%pd/%pD":README "f[:[/][]] [%return] []":README + : "Test argument %pd with name for fprobe" echo 'f:testprobe dput name=$arg1:%pd' > dynamic_events
[PATCH 2/2] selftests/ftrace: Fix checkbashisms errors
From: Masami Hiramatsu (Google) Fix the below checkbashisms errors. Because of these errors, these tests will fail on dash shell. possible bashism in test.d/kprobe/kretprobe_entry_arg.tc line 14 ('function' is useless): function streq() { possible bashism in test.d/dynevent/fprobe_entry_arg.tc line 14 ('function' is useless): function streq() { Fixes: f6e2253a617c ("selftests/ftrace: Add test cases for entry args at function exit") Cc: sta...@vger.kernel.org Signed-off-by: Masami Hiramatsu (Google) --- .../ftrace/test.d/dynevent/fprobe_entry_arg.tc |2 +- .../ftrace/test.d/kprobe/kretprobe_entry_arg.tc|2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_entry_arg.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_entry_arg.tc index d183b8a8ecf8..1e251ce2998e 100644 --- a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_entry_arg.tc +++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_entry_arg.tc @@ -11,7 +11,7 @@ echo 1 > events/tests/enable echo > trace cat trace > /dev/null -function streq() { +streq() { test $1 = $2 } diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_entry_arg.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_entry_arg.tc index 53b82f36a1d0..e50470b53164 100644 --- a/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_entry_arg.tc +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_entry_arg.tc @@ -11,7 +11,7 @@ echo 1 > events/kprobes/enable echo > trace cat trace > /dev/null -function streq() { +streq() { test $1 = $2 }
[PATCH 1/2] selftests/ftrace: Fix BTFARG testcase to check fprobe is enabled correctly
From: Masami Hiramatsu (Google) Since the dynevent/add_remove_btfarg.tc test case forgets to ensure that fprobe is enabled for some structure field access tests which uses the fprobe, it fails if CONFIG_FPROBE=n or CONFIG_FPROBE_EVENTS=n. Fixes it to ensure the fprobe events are supported. Fixes: d892d3d3d885 ("selftests/ftrace: Add BTF fields access testcases") Cc: sta...@vger.kernel.org Signed-off-by: Masami Hiramatsu (Google) --- .../ftrace/test.d/dynevent/add_remove_btfarg.tc|2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_btfarg.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_btfarg.tc index b9c21a81d248..c0cdad4c400e 100644 --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_btfarg.tc +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_btfarg.tc @@ -53,7 +53,7 @@ fi echo > dynamic_events -if [ "$FIELDS" ] ; then +if [ "$FIELDS" -a "$FPROBES" ] ; then echo "t:tpevent ${TP2} obj_size=s->object_size" >> dynamic_events echo "f:fpevent ${TP3}%return path=\$retval->name:string" >> dynamic_events echo "t:tpevent2 ${TP4} p->se.group_node.next->prev" >> dynamic_events
[PATCH 0/2] selftests/ftrace: Fix some errors
Here is a couple of patches for fixing errors on ftracetest. Shuah, can you pick these to your fixes branch? Or I also can push it. Thank you, --- Masami Hiramatsu (Google) (2): selftests/ftrace: Fix BTFARG testcase to check fprobe is enabled correctly selftests/ftrace: Fix checkbashisms errors .../ftrace/test.d/dynevent/add_remove_btfarg.tc|2 +- .../ftrace/test.d/dynevent/fprobe_entry_arg.tc |2 +- .../ftrace/test.d/kprobe/kretprobe_entry_arg.tc|2 +- 3 files changed, 3 insertions(+), 3 deletions(-) -- Masami Hiramatsu (Google)
Re: [PATCH net-next] selftest: epoll_busy_poll: epoll busy poll tests
On Fri, 3 May 2024 16:09:45 -0700 Joe Damato wrote: > > "GEN" is for files which are built for other tests to use. > > IOW unless there's also a wrapper script under TEST_PROGS > > (or the C code is itself under TEST_PROGS) this test won't > > be executed by most CIs. > > Ah, I see. OK. > > If I decided to go with the kselftest_harness as mentioned below, I'd need > to include a wrapper script to run the binary with the right cmd line > arg(s) and put that in TEST_PROGS? harness or not, the only two real requirements for including in TEST_PROGS directly is to: - return non-zero exit code on failure; and - not require any command line arguments.
kselftest/next kselftest-lkdtm: 2 runs, 1 regressions (v6.9-rc4-36-g70bfefe4252d7)
kselftest/next kselftest-lkdtm: 2 runs, 1 regressions (v6.9-rc4-36-g70bfefe4252d7) Regressions Summary --- platform| arch | lab | compiler | defconfig | regressions +--+---+--+--+ imx6q-sabrelite | arm | lab-collabora | gcc-10 | multi_v7_defconfig+kselftest | 1 Details: https://kernelci.org/test/job/kselftest/branch/next/kernel/v6.9-rc4-36-g70bfefe4252d7/plan/kselftest-lkdtm/ Test: kselftest-lkdtm Tree: kselftest Branch: next Describe: v6.9-rc4-36-g70bfefe4252d7 URL: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git SHA: 70bfefe4252d7ab57fb49348ca5b66ad9298e46e Test Regressions platform| arch | lab | compiler | defconfig | regressions +--+---+--+--+ imx6q-sabrelite | arm | lab-collabora | gcc-10 | multi_v7_defconfig+kselftest | 1 Details: https://kernelci.org/test/plan/id/66356a3dc9262a7ecb4c42de Results: 0 PASS, 1 FAIL, 0 SKIP Full config: multi_v7_defconfig+kselftest Compiler:gcc-10 (arm-linux-gnueabihf-gcc (Debian 10.2.1-6) 10.2.1 20210110) Plain log: https://storage.kernelci.org//kselftest/next/v6.9-rc4-36-g70bfefe4252d7/arm/multi_v7_defconfig+kselftest/gcc-10/lab-collabora/kselftest-lkdtm-imx6q-sabrelite.txt HTML log: https://storage.kernelci.org//kselftest/next/v6.9-rc4-36-g70bfefe4252d7/arm/multi_v7_defconfig+kselftest/gcc-10/lab-collabora/kselftest-lkdtm-imx6q-sabrelite.html Rootfs: http://storage.kernelci.org/images/rootfs/debian/bookworm-kselftest/20240313.0/armhf/initrd.cpio.gz * kselftest-lkdtm.login: https://kernelci.org/test/case/id/66356a3dc9262a7ecb4c42df failing since 8 days (last pass: v6.9-rc4-19-g00ab560eb0e3, first fail: v6.9-rc4-32-g693fe2f6a9ea)
kselftest/next kselftest-livepatch: 1 runs, 1 regressions (v6.9-rc4-36-g70bfefe4252d7)
kselftest/next kselftest-livepatch: 1 runs, 1 regressions (v6.9-rc4-36-g70bfefe4252d7) Regressions Summary --- platform| arch | lab | compiler | defconfig | regressions +--+---+--+--+ imx6q-sabrelite | arm | lab-collabora | gcc-10 | multi_v7_defconfig+kselftest | 1 Details: https://kernelci.org/test/job/kselftest/branch/next/kernel/v6.9-rc4-36-g70bfefe4252d7/plan/kselftest-livepatch/ Test: kselftest-livepatch Tree: kselftest Branch: next Describe: v6.9-rc4-36-g70bfefe4252d7 URL: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git SHA: 70bfefe4252d7ab57fb49348ca5b66ad9298e46e Test Regressions platform| arch | lab | compiler | defconfig | regressions +--+---+--+--+ imx6q-sabrelite | arm | lab-collabora | gcc-10 | multi_v7_defconfig+kselftest | 1 Details: https://kernelci.org/test/plan/id/6635687051b08916c84c42da Results: 1 PASS, 1 FAIL, 0 SKIP Full config: multi_v7_defconfig+kselftest Compiler:gcc-10 (arm-linux-gnueabihf-gcc (Debian 10.2.1-6) 10.2.1 20210110) Plain log: https://storage.kernelci.org//kselftest/next/v6.9-rc4-36-g70bfefe4252d7/arm/multi_v7_defconfig+kselftest/gcc-10/lab-collabora/kselftest-livepatch-imx6q-sabrelite.txt HTML log: https://storage.kernelci.org//kselftest/next/v6.9-rc4-36-g70bfefe4252d7/arm/multi_v7_defconfig+kselftest/gcc-10/lab-collabora/kselftest-livepatch-imx6q-sabrelite.html Rootfs: http://storage.kernelci.org/images/rootfs/debian/bookworm-kselftest/20240313.0/armhf/initrd.cpio.gz * kselftest-livepatch.shardfile-livepatch: https://kernelci.org/test/case/id/6635687051b08916c84c42dc failing since 79 days (last pass: v6.8-rc1, first fail: v6.8-rc1-32-g345e8abe4c355) 2024-05-03T22:43:22.173115 / # 2024-05-03T22:43:22.183109 2024-05-03T22:43:27.326213 / # export NFS_ROOTFS='/var/lib/lava/dispatcher/tmp/13633047/extract-nfsrootfs-c03ooedj' 2024-05-03T22:43:27.342473 export NFS_ROOTFS='/var/lib/lava/dispatcher/tmp/13633047/extract-nfsrootfs-c03ooedj' 2024-05-03T22:43:29.569322 / # export NFS_SERVER_IP='192.168.201.1' 2024-05-03T22:43:29.580248 export NFS_SERVER_IP='192.168.201.1' 2024-05-03T22:43:29.697807 / # # 2024-05-03T22:43:29.705994 # 2024-05-03T22:43:29.823718 / # export SHELL=/bin/bash 2024-05-03T22:43:29.834056 export SHELL=/bin/bash ... (94 line(s) more)
[PATCH v2] selftests/resctrl: fix clang build warnings related to abs(), labs() calls
First of all, in order to build with clang at all, one must first apply Valentin Obst's build fix for LLVM [1]. Furthermore, for this particular resctrl directory, my pending fix [2] must also be applied. Once those fixes are in place, then when building with clang, via: make LLVM=1 -C tools/testing/selftests ...two types of warnings occur: warning: absolute value function 'abs' given an argument of type 'long' but has parameter of type 'int' which may cause truncation of value warning: taking the absolute value of unsigned type 'unsigned long' has no effect Fix these by: a) using labs() in place of abs(), when long integers are involved, and b) Change to use signed integer data types, in places where subtraction is used (and could end up with negative values). [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ [2] https://lore.kernel.org/all/20240503021712.78601-1-jhubb...@nvidia.com/ Cc: Reinette Chatre Signed-off-by: John Hubbard --- Hi Reinette, This v2 includes a fix for the bugs that you pointed out (thanks!) in v1. I kept the changes to signed integers minimal: only what is required in order to get a clean clang build. thanks, John Hubbard tools/testing/selftests/resctrl/cmt_test.c | 12 ++-- tools/testing/selftests/resctrl/mba_test.c | 4 ++-- tools/testing/selftests/resctrl/mbm_test.c | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index a81f91222a89..af33abd1cca7 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -29,22 +29,22 @@ static int cmt_setup(const struct resctrl_test *test, return 0; } -static int show_results_info(unsigned long sum_llc_val, int no_of_bits, -unsigned long cache_span, unsigned long max_diff, -unsigned long max_diff_percent, unsigned long num_of_runs, +static int show_results_info(long sum_llc_val, int no_of_bits, +long cache_span, long max_diff, +long max_diff_percent, long num_of_runs, bool platform) { - unsigned long avg_llc_val = 0; + long avg_llc_val = 0; float diff_percent; long avg_diff = 0; int ret; avg_llc_val = sum_llc_val / num_of_runs; - avg_diff = (long)abs(cache_span - avg_llc_val); + avg_diff = labs(cache_span - avg_llc_val); diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100; ret = platform && abs((int)diff_percent) > max_diff_percent && - abs(avg_diff) > max_diff; + labs(avg_diff) > max_diff; ksft_print_msg("%s Check cache miss rate within %lu%%\n", ret ? "Fail:" : "Pass:", max_diff_percent); diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c index 7946e32e85c8..707b07687249 100644 --- a/tools/testing/selftests/resctrl/mba_test.c +++ b/tools/testing/selftests/resctrl/mba_test.c @@ -60,8 +60,8 @@ static bool show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc) /* Memory bandwidth from 100% down to 10% */ for (allocation = 0; allocation < ALLOCATION_MAX / ALLOCATION_STEP; allocation++) { - unsigned long avg_bw_imc, avg_bw_resc; - unsigned long sum_bw_imc = 0, sum_bw_resc = 0; + long avg_bw_imc, avg_bw_resc; + long sum_bw_imc = 0, sum_bw_resc = 0; int avg_diff_per; float avg_diff; diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index d67ffa3ec63a..30af15020731 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -17,8 +17,8 @@ static int show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, size_t span) { - unsigned long avg_bw_imc = 0, avg_bw_resc = 0; - unsigned long sum_bw_imc = 0, sum_bw_resc = 0; + long avg_bw_imc = 0, avg_bw_resc = 0; + long sum_bw_imc = 0, sum_bw_resc = 0; int runs, ret, avg_diff_per; float avg_diff = 0; base-commit: ddb4c3f25b7b95df3d6932db0b379d768a6ebdf7 prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27 prerequisite-patch-id: 8d96c4b8c3ed6d9ea2588ef7f594ae0f9f83c279 -- 2.45.0
Re: [PATCH net-next] selftest: epoll_busy_poll: epoll busy poll tests
On Fri, May 03, 2024 at 03:49:39PM -0700, Jakub Kicinski wrote: > On Thu, 2 May 2024 21:20:11 + Joe Damato wrote: > > --- a/tools/testing/selftests/net/Makefile > > +++ b/tools/testing/selftests/net/Makefile > > @@ -84,6 +84,7 @@ TEST_GEN_FILES += sctp_hello > > TEST_GEN_FILES += csum > > TEST_GEN_FILES += ip_local_port_range > > TEST_GEN_FILES += bind_wildcard > > +TEST_GEN_FILES += epoll_busy_poll > > "GEN" is for files which are built for other tests to use. > IOW unless there's also a wrapper script under TEST_PROGS > (or the C code is itself under TEST_PROGS) this test won't > be executed by most CIs. Ah, I see. OK. If I decided to go with the kselftest_harness as mentioned below, I'd need to include a wrapper script to run the binary with the right cmd line arg(s) and put that in TEST_PROGS? > FWIW here's how we run the tests in our CI upstream CI: > https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style Thanks for the link, I'll give this a close read. > > TEST_PROGS += test_vxlan_mdb.sh > > TEST_PROGS += test_bridge_neigh_suppress.sh > > TEST_PROGS += test_vxlan_nolocalbypass.sh > > > +static void do_simple_test(void) > > +{ > > + int fd; > > + > > + fd = epoll_create1(0); > > + if (fd == -1) > > + error(1, errno, "epoll_create"); > > + > > + do_simple_test_invalid_fd(); > > + do_simple_test_invalid_ioctl(fd); > > + do_simple_test_get_params(fd); > > + do_simple_test_set_invalid(fd); > > + do_simple_test_set_and_get_valid(fd); > > You don't want to use the kselftest_harness for this? > No strong preference here, but seems like you could > pop the epoll_create1 into a FIXTURE() and then the > test cases into TEST_F() and we'd get the KTAP output > formatting, ability to run the tests selectively etc. > for free. I have no preference. I looked at some random .c file test in the directory and it wasn't using the kselftest_harness stuff so I just went with that. The advantages of kselftest_harness make sense, so I can give it a rewrite to use kselftest_harness in v2. > tools/testing/selftests/net/tap.c is probably a good example > to take a look at Thanks, I'll look at that one. I had previously just kinda scanned reuseaddr_conflict.c and rxtimestamp.c and some other ones. Seemed like a bunch were just regular C programs so I went that route, but the advantages you list make a lot of sense.
Re: [PATCH net-next] selftest: epoll_busy_poll: epoll busy poll tests
On Thu, 2 May 2024 21:20:11 + Joe Damato wrote: > --- a/tools/testing/selftests/net/Makefile > +++ b/tools/testing/selftests/net/Makefile > @@ -84,6 +84,7 @@ TEST_GEN_FILES += sctp_hello > TEST_GEN_FILES += csum > TEST_GEN_FILES += ip_local_port_range > TEST_GEN_FILES += bind_wildcard > +TEST_GEN_FILES += epoll_busy_poll "GEN" is for files which are built for other tests to use. IOW unless there's also a wrapper script under TEST_PROGS (or the C code is itself under TEST_PROGS) this test won't be executed by most CIs. FWIW here's how we run the tests in our CI upstream CI: https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style > TEST_PROGS += test_vxlan_mdb.sh > TEST_PROGS += test_bridge_neigh_suppress.sh > TEST_PROGS += test_vxlan_nolocalbypass.sh > +static void do_simple_test(void) > +{ > + int fd; > + > + fd = epoll_create1(0); > + if (fd == -1) > + error(1, errno, "epoll_create"); > + > + do_simple_test_invalid_fd(); > + do_simple_test_invalid_ioctl(fd); > + do_simple_test_get_params(fd); > + do_simple_test_set_invalid(fd); > + do_simple_test_set_and_get_valid(fd); You don't want to use the kselftest_harness for this? No strong preference here, but seems like you could pop the epoll_create1 into a FIXTURE() and then the test cases into TEST_F() and we'd get the KTAP output formatting, ability to run the tests selectively etc. for free. tools/testing/selftests/net/tap.c is probably a good example to take a look at
kselftest/next build: 4 builds: 0 failed, 4 passed, 1 warning (v6.9-rc4-36-g70bfefe4252d7)
kselftest/next build: 4 builds: 0 failed, 4 passed, 1 warning (v6.9-rc4-36-g70bfefe4252d7) Full Build Summary: https://kernelci.org/build/kselftest/branch/next/kernel/v6.9-rc4-36-g70bfefe4252d7/ Tree: kselftest Branch: next Git Describe: v6.9-rc4-36-g70bfefe4252d7 Git Commit: 70bfefe4252d7ab57fb49348ca5b66ad9298e46e Git URL: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git Built: 3 unique architectures Warnings Detected: arm: i386: x86_64: x86_64_defconfig+kselftest (clang-16): 1 warning Warnings summary: 1vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x23: relocation to !ENDBR: .text+0x14ea89 Detailed per-defconfig build reports: i386_defconfig+kselftest (i386, gcc-10) — PASS, 0 errors, 0 warnings, 0 section mismatches multi_v7_defconfig+kselftest (arm, gcc-10) — PASS, 0 errors, 0 warnings, 0 section mismatches x86_64_defconfig+kselftest (x86_64, gcc-10) — PASS, 0 errors, 0 warnings, 0 section mismatches x86_64_defconfig+kselftest (x86_64, clang-16) — PASS, 0 errors, 1 warning, 0 section mismatches Warnings: vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x23: relocation to !ENDBR: .text+0x14ea89 --- For more info write to
Re: [PATCH] selftests/resctrl: fix clang build warnings related to abs(), labs() calls
On 5/3/24 1:46 PM, Reinette Chatre wrote: Hi John, On 5/3/2024 12:12 PM, John Hubbard wrote: On 5/3/24 11:37 AM, Reinette Chatre wrote: On 5/3/2024 9:52 AM, John Hubbard wrote: On 5/3/24 1:00 AM, Ilpo Järvinen wrote: On Thu, 2 May 2024, John Hubbard wrote: ... ... I assumed that this code did not expect to handle negative numbers, because it is using unsigned math throughout. If you do expect it to handle cases where, for example, this happens: avg_bw_imc > avg_bw_resc The existing code seems to handle this ok. A sample program with this scenario comparing existing computation with your first proposal is below: #include #include void main(void) { unsigned long avg_bw_resc = 2; unsigned long avg_bw_imc = 4; float avg_diff; /* Existing code */ avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc; printf("Existing code: avg_diff = %f\n", avg_diff); /* Original proposed fix */ avg_diff = (float)(avg_bw_resc - avg_bw_imc) / avg_bw_imc; printf("Original proposed fix: avg_diff = %f\n", avg_diff); } output: Existing code: avg_diff = 0.50 Original proposed fix: avg_diff = 461168590192640.00 That seems "a little bit" wrong. haha :) ...then a proper solution is easy, and looks like this: diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index c873793d016d..b87f91a41494 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -17,8 +17,8 @@ static int show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, size_t span) { - unsigned long avg_bw_imc = 0, avg_bw_resc = 0; - unsigned long sum_bw_imc = 0, sum_bw_resc = 0; + long avg_bw_imc = 0, avg_bw_resc = 0; + long sum_bw_imc = 0, sum_bw_resc = 0; int runs, ret, avg_diff_per; float avg_diff = 0; Should I resend the patch with that approach? ok. That indeed makes the computations easier to understand. I assume you intend to fix the snippet in mba_test.c also? Yes, will do that. Thanks for spotting the bug in the original "fix"! thanks, -- John Hubbard NVIDIA
Re: [PATCH net-next v2] selftests: drv-net: add checksum tests
Willem de Bruijn wrote: > From: Willem de Bruijn > > Run tools/testing/selftest/net/csum.c as part of drv-net. > This binary covers multiple scenarios, based on arguments given, > for both IPv4 and IPv6: > > - Accept UDP correct checksum > - Detect UDP invalid checksum > - Accept TCP correct checksum > - Detect TCP invalid checksum > > - Transmit UDP: basic checksum offload > - Transmit UDP: zero checksum conversion > > The test direction is reversed between receive and transmit tests, so > that the NIC under test is always the local machine. > > In total this adds up to 12 testcases, with more to follow. For > conciseness, I replaced individual functions with a function factory. > > Also detect hardware offload feature availability using Ethtool > netlink and skip tests when either feature is off. This need may be > common for offload feature tests and eventually deserving of a thin > wrapper in lib.py. > > Missing are the PF_PACKET based send tests ('-P'). These use > virtio_net_hdr to program hardware checksum offload. Which requires > looking up the local MAC address and (harder) the MAC of the next hop. > I'll have to give it some though how to do that robustly and where > that code would belong. > > Tested: > > make -C tools/testing/selftests/ \ > TARGETS="drivers/net drivers/net/hw" \ > install INSTALL_PATH=/tmp/ksft > cd /tmp/ksft > > sudo NETIF=ens4 REMOTE_TYPE=ssh \ > REMOTE_ARGS="root@10.40.0.2" \ > LOCAL_V4="10.40.0.1" Missing backslash > REMOTE_V4="10.40.0.2" \ > ./run_kselftest.sh -t drivers/net/hw:csum.py > > Signed-off-by: Willem de Bruijn > > --- > > Changes > - v1->v2 > - remove dependency on tools/testing/selftests/net: move csum > - remove test output from git commit message: > has noisy (expected) failures on test platform after bkg changes > --- > .../testing/selftests/drivers/net/hw/Makefile | 1 + > .../testing/selftests/drivers/net/hw/csum.py | 114 ++ > tools/testing/selftests/net/.gitignore| 1 - > tools/testing/selftests/net/Makefile | 1 - > tools/testing/selftests/net/lib/.gitignore| 2 + > tools/testing/selftests/net/lib/Makefile | 7 ++ > tools/testing/selftests/net/{ => lib}/csum.c | 0 > 7 files changed, 124 insertions(+), 2 deletions(-) > create mode 100755 tools/testing/selftests/drivers/net/hw/csum.py > create mode 100644 tools/testing/selftests/net/lib/.gitignore > rename tools/testing/selftests/net/{ => lib}/csum.c (100%) > > diff --git a/tools/testing/selftests/drivers/net/hw/Makefile > b/tools/testing/selftests/drivers/net/hw/Makefile > index 1dd732855d76..4933d045ab66 100644 > --- a/tools/testing/selftests/drivers/net/hw/Makefile > +++ b/tools/testing/selftests/drivers/net/hw/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0+ OR MIT > > TEST_PROGS = \ > + csum.py \ > devlink_port_split.py \ > ethtool.sh \ > ethtool_extended_state.sh \ > diff --git a/tools/testing/selftests/drivers/net/hw/csum.py > b/tools/testing/selftests/drivers/net/hw/csum.py > new file mode 100755 > index ..7e3a955fc426 > --- /dev/null > +++ b/tools/testing/selftests/drivers/net/hw/csum.py > @@ -0,0 +1,114 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: GPL-2.0 > + > +"""Run the tools/testing/selftests/net/csum testsuite.""" > + > +from os import path > + > +from lib.py import ksft_run, ksft_exit, KsftSkipEx > +from lib.py import EthtoolFamily, NetDrvEpEnv > +from lib.py import bkg, cmd, wait_port_listen > + > +def test_receive(cfg, ipv4=False, extra_args=None): > +"""Test local nic checksum receive. Remote host sends crafted packets.""" > +if not cfg.have_rx_csum: > +raise KsftSkipEx(f"Test requires rx checksum offload on > {cfg.ifname}") > + > +if ipv4: > +ip_args = f"-4 -S {cfg.remote_v4} -D {cfg.v4}" > +else: > +ip_args = f"-6 -S {cfg.remote_v6} -D {cfg.v6}" > + > +rx_cmd = f"{cfg.bin_local} -i {cfg.ifname} -n 100 {ip_args} -r 1 -R > {extra_args}" > +tx_cmd = f"{cfg.bin_remote} -i {cfg.ifname} -n 100 {ip_args} -r 1 -T > {extra_args}" > + > +with bkg(rx_cmd, exit_wait=True): > +wait_port_listen(34000, proto='udp') > +cmd(tx_cmd, host=cfg.remote) > + > + > +def test_transmit(cfg, ipv4=False, extra_args=None): > +"""Test local nic checksum transmit. Remote host verifies packets.""" > +if not cfg.have_tx_csum: > +raise KsftSkipEx(f"Test requires tx checksum offload on > {cfg.ifname}") > + > +if ipv4: > +ip_args = f"-4 -S {cfg.v4} -D {cfg.remote_v4}" > +else: > +ip_args = f"-6 -S {cfg.v6} -D {cfg.remote_v6}" > + > +# Cannot randomize input when calculating zero checksum > +if extra_args != "-U -Z": > +extra_args += " -r 1" > + > +rx_cmd = f"{cfg.bin_remote} -i {cfg.ifname} -L 1 -n 100 {ip_args}
[PATCH v3] selftest/tty: Use harness framework in tty
Use kselftest_harness.h to simplify the code structure by eliminating conditional logic. Enhance diagnostics by directly printing relevant info, such as access and modification times, upon test failure. Reflecting common I/O optimizations, the access time usually remains unchanged, while the modify time is expected to update. Accordingly, these elements have been logically separated. Signed-off-by: Shengyu Li --- v3: Explain the need for refactoring v2: Fixed the last Assert --- .../testing/selftests/tty/tty_tstamp_update.c | 49 +-- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/tools/testing/selftests/tty/tty_tstamp_update.c b/tools/testing/selftests/tty/tty_tstamp_update.c index 0ee97943dccc..38de211e0715 100644 --- a/tools/testing/selftests/tty/tty_tstamp_update.c +++ b/tools/testing/selftests/tty/tty_tstamp_update.c @@ -9,7 +9,7 @@ #include #include -#include "../kselftest.h" +#include "../kselftest_harness.h" #define MIN_TTY_PATH_LEN 8 @@ -42,47 +42,42 @@ static int write_dev_tty(void) return r; } -int main(int argc, char **argv) +TEST(tty_tstamp_update) { int r; char tty[PATH_MAX] = {}; struct stat st1, st2; - ksft_print_header(); - ksft_set_plan(1); + ASSERT_GE(readlink("/proc/self/fd/0", tty, PATH_MAX), 0) + TH_LOG("readlink on /proc/self/fd/0 failed: %m"); - r = readlink("/proc/self/fd/0", tty, PATH_MAX); - if (r < 0) - ksft_exit_fail_msg("readlink on /proc/self/fd/0 failed: %m\n"); - - if (!tty_valid(tty)) - ksft_exit_skip("invalid tty path '%s'\n", tty); + ASSERT_TRUE(tty_valid(tty)) { + TH_LOG("SKIP: invalid tty path '%s'", tty); + _exit(KSFT_SKIP); + } - r = stat(tty, ); - if (r < 0) - ksft_exit_fail_msg("stat failed on tty path '%s': %m\n", tty); + ASSERT_GE(stat(tty, ), 0) + TH_LOG("stat failed on tty path '%s': %m", tty); /* We need to wait at least 8 seconds in order to observe timestamp change */ /* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fbf47635315ab308c9b58a1ea0906e711a9228de */ sleep(10); r = write_dev_tty(); - if (r < 0) - ksft_exit_fail_msg("failed to write to /dev/tty: %s\n", - strerror(-r)); + ASSERT_GE(r, 0) + TH_LOG("failed to write to /dev/tty: %s", strerror(-r)); - r = stat(tty, ); - if (r < 0) - ksft_exit_fail_msg("stat failed on tty path '%s': %m\n", tty); + ASSERT_GE(stat(tty, ), 0) + TH_LOG("stat failed on tty path '%s': %m", tty); + + /* Validate unchanged atime under 'relatime' to ensure minimal disk I/O */ + EXPECT_EQ(st1.st_atim.tv_sec, st2.st_atim.tv_sec); /* We wrote to the terminal so timestamps should have been updated */ - if (st1.st_atim.tv_sec == st2.st_atim.tv_sec && - st1.st_mtim.tv_sec == st2.st_mtim.tv_sec) { - ksft_test_result_fail("tty timestamps not updated\n"); - ksft_exit_fail(); - } + ASSERT_NE(st1.st_mtim.tv_sec, st2.st_mtim.tv_sec) + TH_LOG("tty timestamps not updated"); - ksft_test_result_pass( - "timestamps of terminal '%s' updated after write to /dev/tty\n", tty); - return EXIT_SUCCESS; + TH_LOG("timestamps of terminal '%s' updated after write to /dev/tty", + tty); } +TEST_HARNESS_MAIN -- 2.25.1
Re: [PATCH V5] KVM: selftests: Add a new option to rseq_test
On Thu, 02 May 2024 14:39:36 -0700, Zide Chen wrote: > Currently, the migration worker delays 1-10 us, assuming that one > KVM_RUN iteration only takes a few microseconds. But if the CPU low > power wakeup latency is large enough, for example, hundreds or even > thousands of microseconds deep C-state exit latencies on x86 server > CPUs, it may happen that it's not able to wakeup the target CPU before > the migration worker starts to migrate the vCPU thread to the next CPU. > > [...] Applied to kvm-x86 selftests, thanks! I tweaked the changelog slightly to call out the new comment and assert message. I also added an extra newline so that the "help" part of the assert message is isolated from the primary explanation of why the assert fired. E.g. the output looks like: Test Assertion Failure rseq_test.c:290: skip_sanity_check || i > (NR_TASK_MIGRATIONS * 2002) pid=20283 tid=20283 errno=4 - Interrupted system call 1 0x0040210a: main at rseq_test.c:286 2 0x7f07fa821c86: ?? ??:0 3 0x00402209: _start at ??:? Only performed 11162 KVM_RUNs, task stalled too much? Try disabling deep sleep states to reduce CPU wakeup latency, e.g. via cpuidle.off=1 or setting /dev/cpu_dma_latency to '0', or run with -u to disable this sanity check. [1/1] KVM: selftests: Add a new option to rseq_test https://github.com/kvm-x86/linux/commit/20ecf595b513 -- https://github.com/kvm-x86/linux/tree/next
Re: [PATCH v6 06/17] riscv: Add vendor extensions to /proc/cpuinfo
On Fri, May 3, 2024 at 11:18 AM Charlie Jenkins wrote: > > All of the supported vendor extensions that have been listed in > riscv_isa_vendor_ext_list can be exported through /proc/cpuinfo. > > Signed-off-by: Charlie Jenkins Reviewed-by: Evan Green
Re: [PATCH v6 05/17] riscv: Extend cpufeature.c to detect vendor extensions
On Fri, May 3, 2024 at 11:18 AM Charlie Jenkins wrote: > > Separate vendor extensions out into one struct per vendor > instead of adding vendor extensions onto riscv_isa_ext. > > Add a hidden config RISCV_ISA_VENDOR_EXT to conditionally include this > code. > > The xtheadvector vendor extension is added using these changes. > > Signed-off-by: Charlie Jenkins Reviewed-by: Evan Green
Re: [PATCH] selftests/resctrl: fix clang build warnings related to abs(), labs() calls
Hi John, On 5/3/2024 12:12 PM, John Hubbard wrote: > On 5/3/24 11:37 AM, Reinette Chatre wrote: >> On 5/3/2024 9:52 AM, John Hubbard wrote: >>> On 5/3/24 1:00 AM, Ilpo Järvinen wrote: On Thu, 2 May 2024, John Hubbard wrote: >>> ... > diff --git a/tools/testing/selftests/resctrl/mbm_test.c > b/tools/testing/selftests/resctrl/mbm_test.c > index d67ffa3ec63a..c873793d016d 100644 > --- a/tools/testing/selftests/resctrl/mbm_test.c > +++ b/tools/testing/selftests/resctrl/mbm_test.c > @@ -33,7 +33,7 @@ show_bw_info(unsigned long *bw_imc, unsigned long > *bw_resc, size_t span) > avg_bw_imc = sum_bw_imc / 4; > avg_bw_resc = sum_bw_resc / 4; > - avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc; > + avg_diff = (float)(avg_bw_resc - avg_bw_imc) / avg_bw_imc; > avg_diff_per = (int)(avg_diff * 100); > ret = avg_diff_per > MAX_DIFF_PERCENT; But how are these two cases same after your change when you ended up removing taking the absolute value entirely? >>> >>> All of the arguments are unsigned integers, so all arithmetic results >>> are interpreted as unsigned, so taking the absolute value of that is >>> always a no-op. >> >> It does not seem as though clang can see when values have been casted. >> I tried to do so explicitly with a: >> avg_diff = labs((long)avg_bw_resc - avg_bw_imc) / (float)avg_bw_imc; > > The subtraction result will get promoted to an unsigned long, before being > passed into labs(3). > >> >> But that still triggers: >> warning: taking the absolute value of unsigned type 'unsigned long' has no >> effect [-Wabsolute-value] > > As expected, yes. > >> >> Looks like we may need to be more explicit types and not rely on casting so >> much >> to make the compiler happy. >> > > I assumed that this code did not expect to handle negative numbers, > because it is using unsigned math throughout. > > If you do expect it to handle cases where, for example, this happens: > > avg_bw_imc > avg_bw_resc The existing code seems to handle this ok. A sample program with this scenario comparing existing computation with your first proposal is below: #include #include void main(void) { unsigned long avg_bw_resc = 2; unsigned long avg_bw_imc = 4; float avg_diff; /* Existing code */ avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc; printf("Existing code: avg_diff = %f\n", avg_diff); /* Original proposed fix */ avg_diff = (float)(avg_bw_resc - avg_bw_imc) / avg_bw_imc; printf("Original proposed fix: avg_diff = %f\n", avg_diff); } output: Existing code: avg_diff = 0.50 Original proposed fix: avg_diff = 461168590192640.00 > > ...then a proper solution is easy, and looks like this: > > diff --git a/tools/testing/selftests/resctrl/mbm_test.c > b/tools/testing/selftests/resctrl/mbm_test.c > index c873793d016d..b87f91a41494 100644 > --- a/tools/testing/selftests/resctrl/mbm_test.c > +++ b/tools/testing/selftests/resctrl/mbm_test.c > @@ -17,8 +17,8 @@ > static int > show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, size_t span) > { > - unsigned long avg_bw_imc = 0, avg_bw_resc = 0; > - unsigned long sum_bw_imc = 0, sum_bw_resc = 0; > + long avg_bw_imc = 0, avg_bw_resc = 0; > + long sum_bw_imc = 0, sum_bw_resc = 0; > int runs, ret, avg_diff_per; > float avg_diff = 0; > > Should I resend the patch with that approach? ok. That indeed makes the computations easier to understand. I assume you intend to fix the snippet in mba_test.c also? Reinette
[PATCH net-next v2] selftests: drv-net: add checksum tests
From: Willem de Bruijn Run tools/testing/selftest/net/csum.c as part of drv-net. This binary covers multiple scenarios, based on arguments given, for both IPv4 and IPv6: - Accept UDP correct checksum - Detect UDP invalid checksum - Accept TCP correct checksum - Detect TCP invalid checksum - Transmit UDP: basic checksum offload - Transmit UDP: zero checksum conversion The test direction is reversed between receive and transmit tests, so that the NIC under test is always the local machine. In total this adds up to 12 testcases, with more to follow. For conciseness, I replaced individual functions with a function factory. Also detect hardware offload feature availability using Ethtool netlink and skip tests when either feature is off. This need may be common for offload feature tests and eventually deserving of a thin wrapper in lib.py. Missing are the PF_PACKET based send tests ('-P'). These use virtio_net_hdr to program hardware checksum offload. Which requires looking up the local MAC address and (harder) the MAC of the next hop. I'll have to give it some though how to do that robustly and where that code would belong. Tested: make -C tools/testing/selftests/ \ TARGETS="drivers/net drivers/net/hw" \ install INSTALL_PATH=/tmp/ksft cd /tmp/ksft sudo NETIF=ens4 REMOTE_TYPE=ssh \ REMOTE_ARGS="root@10.40.0.2" \ LOCAL_V4="10.40.0.1" REMOTE_V4="10.40.0.2" \ ./run_kselftest.sh -t drivers/net/hw:csum.py Signed-off-by: Willem de Bruijn --- Changes - v1->v2 - remove dependency on tools/testing/selftests/net: move csum - remove test output from git commit message: has noisy (expected) failures on test platform after bkg changes --- .../testing/selftests/drivers/net/hw/Makefile | 1 + .../testing/selftests/drivers/net/hw/csum.py | 114 ++ tools/testing/selftests/net/.gitignore| 1 - tools/testing/selftests/net/Makefile | 1 - tools/testing/selftests/net/lib/.gitignore| 2 + tools/testing/selftests/net/lib/Makefile | 7 ++ tools/testing/selftests/net/{ => lib}/csum.c | 0 7 files changed, 124 insertions(+), 2 deletions(-) create mode 100755 tools/testing/selftests/drivers/net/hw/csum.py create mode 100644 tools/testing/selftests/net/lib/.gitignore rename tools/testing/selftests/net/{ => lib}/csum.c (100%) diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile index 1dd732855d76..4933d045ab66 100644 --- a/tools/testing/selftests/drivers/net/hw/Makefile +++ b/tools/testing/selftests/drivers/net/hw/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ OR MIT TEST_PROGS = \ + csum.py \ devlink_port_split.py \ ethtool.sh \ ethtool_extended_state.sh \ diff --git a/tools/testing/selftests/drivers/net/hw/csum.py b/tools/testing/selftests/drivers/net/hw/csum.py new file mode 100755 index ..7e3a955fc426 --- /dev/null +++ b/tools/testing/selftests/drivers/net/hw/csum.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0 + +"""Run the tools/testing/selftests/net/csum testsuite.""" + +from os import path + +from lib.py import ksft_run, ksft_exit, KsftSkipEx +from lib.py import EthtoolFamily, NetDrvEpEnv +from lib.py import bkg, cmd, wait_port_listen + +def test_receive(cfg, ipv4=False, extra_args=None): +"""Test local nic checksum receive. Remote host sends crafted packets.""" +if not cfg.have_rx_csum: +raise KsftSkipEx(f"Test requires rx checksum offload on {cfg.ifname}") + +if ipv4: +ip_args = f"-4 -S {cfg.remote_v4} -D {cfg.v4}" +else: +ip_args = f"-6 -S {cfg.remote_v6} -D {cfg.v6}" + +rx_cmd = f"{cfg.bin_local} -i {cfg.ifname} -n 100 {ip_args} -r 1 -R {extra_args}" +tx_cmd = f"{cfg.bin_remote} -i {cfg.ifname} -n 100 {ip_args} -r 1 -T {extra_args}" + +with bkg(rx_cmd, exit_wait=True): +wait_port_listen(34000, proto='udp') +cmd(tx_cmd, host=cfg.remote) + + +def test_transmit(cfg, ipv4=False, extra_args=None): +"""Test local nic checksum transmit. Remote host verifies packets.""" +if not cfg.have_tx_csum: +raise KsftSkipEx(f"Test requires tx checksum offload on {cfg.ifname}") + +if ipv4: +ip_args = f"-4 -S {cfg.v4} -D {cfg.remote_v4}" +else: +ip_args = f"-6 -S {cfg.v6} -D {cfg.remote_v6}" + +# Cannot randomize input when calculating zero checksum +if extra_args != "-U -Z": +extra_args += " -r 1" + +rx_cmd = f"{cfg.bin_remote} -i {cfg.ifname} -L 1 -n 100 {ip_args} -R {extra_args}" +tx_cmd = f"{cfg.bin_local} -i {cfg.ifname} -L 1 -n 100 {ip_args} -T {extra_args}" + +with bkg(rx_cmd, host=cfg.remote, exit_wait=True): +wait_port_listen(34000, proto='udp', host=cfg.remote) +cmd(tx_cmd) + + +def test_builder(name, cfg, ipv4=False,
Re: [RFC PATCH net-next v8 02/14] net: page_pool: create hooks for custom page providers
Sorry for the late reply. On Wed, May 1, 2024 at 12:55 AM Christoph Hellwig wrote: > > Still NAK to creating aⅺbitrary hooks here. Is the concern still that folks may be able to hook proprietary stuff into this like you mentioned before[1]? I don't see how that can be done as currently written. The page_pool grabs the memory_provider_ops from the netdev_rx_queue struct managed by core net stack and not really overridable by external modules. When the netdev creates the page_pool, it gets the core-managed netdev_rx_queue via something like __netif_get_rx_queue() and passes that to page_pool_create(). We could make the memory_provider_ops even more opaque by only allowing the device to only pass in the netdev + queue num to the page_pool_create, and have the page_pool_create query the netdev_rx_queue struct, to make sure we're getting the one managed by core. Long story short is that as currently written I think it's pretty much impossible for someone to plug in a proprietary out-of-tree memory provider using these hooks, and if desired I can change the code slightly to make it even more difficult (but maybe that's pointless, I don't think it's possible even in the current iteration). The only way to get a memory_provider_ops in is to seek to merge it as part of the kernel with community approval. Is there something I'm missing here? > This should be a page or > dmabuf pool and not an indirect call abstraction allowing random > crap to hook into it. > What is the suggested fix here? I do something like: cp net/core/page_pool.c net/core/dmabuf_pool.c and then modify it such that the net stack maintains 2 page_pools? There are a lot of cons to that: 1. Code duplication/maintenance (page_pool.c + dmabuf_pool.c will look very similar). 2. The hooks enable more use cases than dmabuf_pool + standard pages. In addition to those, I'm thinking of (but not working on): a. Limited memory pools. I.e. a page_pool limited to a certain amount of memory (for overcommited VMs). b. dmabuf pools with GPU virtual addresses. Currently we seek to support dmabuf memory where the virtual address is an offset into the dmabuf for CPU access. For GPU memory accessible to the GPU we need dmabuf memory where the virtual address is the GPU virtual address. 3. Support for multiple page_pools is actually more proprietary friendly IMO. Currently the page_pool is internal to core. If we start adding additional pools we need to have some uniform behavior between all the pools so core can operate on memory that originated from any one of them. In that case it becomes actually easier for someone to develop an out of tree pool and use it from their out-of-tree driver and as long as their out of tree page_pool behaves similarly enough to the decided uniform behavior, it may be able to fool core into thinking it's an in-tree pool... [1] https://lore.kernel.org/linux-kernel/zfegzb341onc_...@infradead.org/ -- Thanks, Mina
Re: [PATCH] selftests: default to host arch for LLVM builds
On 4/28/24 06:08, Valentin Obst wrote: Align the behavior for gcc and clang builds by interpreting unset `ARCH` and `CROSS_COMPILE` variables in `LLVM` builds as a sign that the user wants to build for the host architecture. This patch preserves the properties that setting the `ARCH` variable to an unknown value will trigger an error that complains about insufficient information, and that a set `CROSS_COMPILE` variable will override the target triple that is determined based on presence/absence of `ARCH`. When compiling with clang, i.e., `LLVM` is set, an unset `ARCH` variable in combination with an unset `CROSS_COMPILE` variable, i.e., compiling for the host architecture, leads to compilation failures since `lib.mk` can not determine the clang target triple. In this case, the following error message is displayed for each subsystem that does not set `ARCH` in its own Makefile before including `lib.mk` (lines wrapped at 75 chrs): make[1]: Entering directory '/mnt/build/linux/tools/testing/selftests/ sysctl' ../lib.mk:33: *** Specify CROSS_COMPILE or add '--target=' option to lib.mk. Stop. make[1]: Leaving directory '/mnt/build/linux/tools/testing/selftests/ sysctl' Thanks for fixing this. And yes, the selftests "normal" (non-cross-compile) build is *broken* right now, for clang. I didn't realize from the patch title that this is actually a significant fix. Maybe we should change the subject line (patch title) to something like: [PATCH] selftests: fix the clang build: default to host arch for LLVM builds Yes, I agree that the title should contain the word 'fix' somewhere. For me its okay if maintainers reword the title when applying the patch, alternatively I can send a v2. (Is it still a v2 if I change the title, or rather a new patch?). Any thoughts on whether this also needs a 'Cc stable'? Its not quite clear to me if this fix meets the requirements. As above, no objections if maintainers should decide to add it. ? Just a thought. The "Fixes:" tag covers it already, I realize. Anyway, this looks correct, and fixes that aspect of the build for me, so either way, please feel free to add: Reviewed-by: John Hubbard Thanks for the patch. Applied to linux-kselftest next for Linux 6.10-rc1 thanks, -- Shuah
Re: [PATCH 4/4] selftests/cgroup: fix uninitialized variables in test_zswap.c
On Thu, May 2, 2024 at 8:51 PM John Hubbard wrote: > > First of all, in order to build with clang at all, one must first apply > Valentin Obst's build fix for LLVM [1]. Once that is done, then when > building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...clang finds and warning about some uninitialized variables. Fix these > by initializing them. > > [1] > https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ > > Signed-off-by: John Hubbard Reviewed-by: Nhat Pham > ---
Re: [PATCH v2] selftest/tty: Use harness framework in tty
On 4/30/24 10:18, Shengyu Li wrote: Similarly, this one is based on automated tools and a very small percentage of manual modifications to automatically refactor the version that uses kselftest_harness.h, which is logically clearer. Signed-off-by: Shengyu Li --- v2: Fixed the last Assert See feedback on your v1. Same comments apply here. Explain why this refactor is necessary. thanks, -- Shuah
Re: [PATCH] selftest/tty: Use harness framework in tty
On 4/30/24 09:05, Shengyu Li wrote: Similarly, this one is based on automated tools and a very small percentage of manual modifications to automatically refactor the version that uses kselftest_harness.h, which is logically clearer. Similar to what? How does refactoring help? Follow the imperative mood to write change logs: https://www.kernel.org/doc/html/latest/process/submitting-patches.html Signed-off-by: Shengyu Li --- thanks, -- Shuah
Re: [PATCH] selftests/binderfs: use the Makefile's rules, not Make's implicit rules
On 5/3/24 03:10, Christian Brauner wrote: On Thu, May 02, 2024 at 06:58:20PM -0700, John Hubbard wrote: First of all, in order to build with clang at all, one must first apply Valentin Obst's build fix for LLVM [1]. Once that is done, then when building with clang, via: make LLVM=1 -C tools/testing/selftests ...the following error occurs: clang: error: cannot specify -o when generating multiple output files This is because clang, unlike gcc, won't accept invocations of this form: clang file1.c header2.h While trying to fix this, I noticed that: a) selftests/lib.mk already avoids the problem, and b) The binderfs Makefile indavertently bypasses the selftests/lib.mk build system, and quitely uses Make's implicit build rules for .c files instead. The Makefile attempts to set up both a dependency and a source file, neither of which was needed, because lib.mk is able to automatically handle both. This line: binderfs_test: binderfs_test.c ...causes Make's implicit rules to run, which builds binderfs_test without ever looking at lib.mk. Fix this by simply deleting the "binderfs_test:" Makefile target and letting lib.mk handle it instead. [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ Fixes: 6e29225af902 ("binderfs: port tests to test harness infrastructure") Cc: Christian Brauner Signed-off-by: John Hubbard --- Reviewed-by: Christian Brauner Thank you. Applied to linunx-kselftest next for Linux 6.10-rc1 thanks, -- Shuah
Re: [PATCH] selftests/resctrl: fix clang build failure: use LOCAL_HDRS
On 5/3/24 12:39, Reinette Chatre wrote: On 5/2/2024 7:17 PM, John Hubbard wrote: First of all, in order to build with clang at all, one must first apply Valentin Obst's build fix for LLVM [1]. Once that is done, then when building with clang, via: make LLVM=1 -C tools/testing/selftests ...the following error occurs: clang: error: cannot specify -o when generating multiple output files This is because clang, unlike gcc, won't accept invocations of this form: clang file1.c header2.h Fix this by using selftests/lib.mk facilities for tracking local header file dependencies: add them to LOCAL_HDRS, leaving only the .c files to be passed to the compiler. [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ Fixes: 8e289f454289 ("selftests/resctrl: Add resctrl.h into build deps") Cc: Ilpo Järvinen Signed-off-by: John Hubbard --- Thank you. Acked-by: Reinette Chatre Reinette Thank you. Applied to linux-kselftest next for Linux 6.10-rc1 thanks, -- Shuah
Re: [PATCH] selftests/resctrl: fix clang build warnings related to abs(), labs() calls
On 5/3/24 11:37 AM, Reinette Chatre wrote: On 5/3/2024 9:52 AM, John Hubbard wrote: On 5/3/24 1:00 AM, Ilpo Järvinen wrote: On Thu, 2 May 2024, John Hubbard wrote: ... diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index d67ffa3ec63a..c873793d016d 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -33,7 +33,7 @@ show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, size_t span) avg_bw_imc = sum_bw_imc / 4; avg_bw_resc = sum_bw_resc / 4; - avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc; + avg_diff = (float)(avg_bw_resc - avg_bw_imc) / avg_bw_imc; avg_diff_per = (int)(avg_diff * 100); ret = avg_diff_per > MAX_DIFF_PERCENT; But how are these two cases same after your change when you ended up removing taking the absolute value entirely? All of the arguments are unsigned integers, so all arithmetic results are interpreted as unsigned, so taking the absolute value of that is always a no-op. It does not seem as though clang can see when values have been casted. I tried to do so explicitly with a: avg_diff = labs((long)avg_bw_resc - avg_bw_imc) / (float)avg_bw_imc; The subtraction result will get promoted to an unsigned long, before being passed into labs(3). But that still triggers: warning: taking the absolute value of unsigned type 'unsigned long' has no effect [-Wabsolute-value] As expected, yes. Looks like we may need to be more explicit types and not rely on casting so much to make the compiler happy. I assumed that this code did not expect to handle negative numbers, because it is using unsigned math throughout. If you do expect it to handle cases where, for example, this happens: avg_bw_imc > avg_bw_resc ...then a proper solution is easy, and looks like this: diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index c873793d016d..b87f91a41494 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -17,8 +17,8 @@ static int show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, size_t span) { - unsigned long avg_bw_imc = 0, avg_bw_resc = 0; - unsigned long sum_bw_imc = 0, sum_bw_resc = 0; + long avg_bw_imc = 0, avg_bw_resc = 0; + long sum_bw_imc = 0, sum_bw_resc = 0; int runs, ret, avg_diff_per; float avg_diff = 0; Should I resend the patch with that approach? thanks, -- John Hubbard NVIDIA
Re: [PATCH v22 5/5] ring-buffer/selftest: Add ring-buffer mapping test
On 4/30/24 05:13, Vincent Donnefort wrote: This test maps a ring-buffer and validate the meta-page after reset and after emitting few events. Changelog needs to be imperative - refer to the following: https://www.kernel.org/doc/html/latest/process/submitting-patches.html Update the change log and describe what the test does and include test output. If the test requires root privileges - make sure add a check to skip when a normal use runs the test. The rest looks good. thanks, -- Shuah
Re: [PATCH 0/4] selftests/cgroups: fix clang build failures, warnings
On Thu, May 02, 2024 at 08:51:01PM -0700, John Hubbard wrote: > Hi, > > Just a bunch of fixes as part of my work to make selftests build cleanly > with clang. > > Enjoy! > > thanks, > John Hubbard > > > John Hubbard (4): > selftests/cgroup: fix clang build failures for abs() calls > selftests/cgroup: fix clang warnings: uninitialized fd variable > selftests/cgroup: cpu_hogger init: use {} instead of {NULL} > selftests/cgroup: fix uninitialized variables in test_zswap.c Applied to cgroup/for-6.10. Thanks. -- tejun
Re: [PATCH] Documentation: kselftest: fix codeblock
On 4/29/24 10:50, Yo-Jung (Leo) Lin wrote: Add extra colon to mark command in the next paragraph as codeblock Signed-off-by: Yo-Jung (Leo) Lin <0xf...@gmail.com> --- Documentation/dev-tools/kselftest.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst index ff10dc6eef5d..dcf634e411bd 100644 --- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -183,7 +183,7 @@ expected time it takes to run a test. If you have control over the systems which will run the tests you can configure a test runner on those systems to use a greater or lower timeout on the command line as with the `-o` or the `--override-timeout` argument. For example to use 165 seconds instead -one would use: +one would use:: $ ./run_kselftest.sh --override-timeout 165 Thank you. Applied to linux=kselftest next for Linux 6.10-rc1. thanks, -- Shuah
Re: [PATCH v2] KVM: selftests: Use TAP interface in the set_memory_region test
On Fri, May 03, 2024, Thomas Huth wrote: > On 02/05/2024 21.37, Sean Christopherson wrote: > > On Fri, Apr 26, 2024, Thomas Huth wrote: > > I like that we can actually report sub-tests as being skipped, but I don't > > like > > having multiple ways to express requirements. And IMO, this is much less > > readable > > than TEST_REQUIRE(has_cap_guest_memfd()); > > > > AIUI, each test runs in a child process, so TEST_REQUIRE() can simply > > exit(), it > > just needs to avoid ksft_exit_skip() so that a sub-test doesn't spit out > > the full > > test summary. > > > > And if using exit() isn't an option, setjmp()+longjmp() will do the trick > > (I got > > that working for KVM_ONE_VCPU_TEST() before I realized tests run as a > > child). > > > > The below is lightly tested, but I think it does what we want? > > Not quite ... for example, if I force vmx_pmu_caps_test to skip the last > test, I get: ... > As you can see, the "ok 5" line is duplicated now, once marked with "# SKIP" > and once as successfull. I don't think that this is valid TAP anymore? Ah, IIUC, it's because the test reports a SKIP, and then also eventually exits with KSFT_SKIP too. > > I also think we would effectively forbid direct use of TEST(). Partly > > because > > it's effectively necessary to use TEST_REQUIRE(), but also so that all > > tests will > > have an existing single point of contact if we need/want to make similar > > changes > > in the future. > > Ok, but I wrote in the patch description, KVM_ONE_VCPU_TEST_SUITE() does not > work for the set_memory_region test since it does not like to have a > pre-defined vcpu ... so if we want to forbid TEST(), I assume we'd need > another macro like KVM_BAREBONE_TEST_SUITE() ? Yeah, though we probably don't need BAREBONE, e.g. KVM_TEST_SUITE() would suffice. The "barebones" terminology exists for VMs because the vanilla "create VM" helpers do waay more than the bare minimum, whereas I don't think we'll have the same issues here. > Not sure whether I really like it, though, since I'd prefer if we could keep > the possibility to use the original selftest macros (for people who are > already used to those macros from other selftests). The more I fiddle with the kselftests harness, the more I'm opposed to using it directly. The harness code heavily assumes a "simple" environment, i.e. a test environment without libraries. E.g. including kselftest_harness.h without invoking test_harness_run() fails due to unused functions, and including it in multiple compilation units, e.g. to allow using its macros in utilities, fails due to duplicate symbols. It's obviously possible to split kselftest_harness.h to get around the immediate issues, but that doesn't help with SKIP() (and other macros) only being usable at the top-level TEST(). And using the undersored globals and functions params, i.e. the "private" stuff, directly seems like a bad idea, e.g. the odds of KVM selftests being broken by changes to the common code would be too high for my taste. While I agree it would be nice to not diverge from the common kselftest framework, as far as things like SKIP and ASSERT macros go, that ship sailed a long time ago, as TEST_REQUIRE() and TEST_ASSERT() usage is ubiquitous throughout KVM selftests. And given the limitations of the common framework versus what we have in KVM's framework, I don't see us converging on the common framework. It's not perfect, but the best idea I can come up with is to trampoline the skip out through KVM's harness and on to the common harness. --- .../selftests/kvm/include/kvm_test_harness.h | 11 ++- .../testing/selftests/kvm/include/test_util.h | 31 ++- tools/testing/selftests/kvm/lib/kvm_util.c| 2 ++ .../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 3 +- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/kvm/include/kvm_test_harness.h b/tools/testing/selftests/kvm/include/kvm_test_harness.h index 8f7c6858e8e2..fa4b5f707135 100644 --- a/tools/testing/selftests/kvm/include/kvm_test_harness.h +++ b/tools/testing/selftests/kvm/include/kvm_test_harness.h @@ -9,6 +9,7 @@ #define SELFTEST_KVM_TEST_HARNESS_H #include "kselftest_harness.h" +#include "test_util.h" #define KVM_ONE_VCPU_TEST_SUITE(name) \ FIXTURE(name) { \ @@ -28,8 +29,16 @@ static void __suite##_##test(struct kvm_vcpu *vcpu); \ \ TEST_F(suite, test)\ { \ + struct kvm_selftests_subtest subtest; \ + \ vcpu_arch_set_entry_point(self->vcpu, guestcode); \ - __suite##_##test(self->vcpu);
Re: [PATCH] selftests/resctrl: fix clang build failure: use LOCAL_HDRS
On 5/2/2024 7:17 PM, John Hubbard wrote: > First of all, in order to build with clang at all, one must first apply > Valentin Obst's build fix for LLVM [1]. Once that is done, then when > building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...the following error occurs: > >clang: error: cannot specify -o when generating multiple output files > > This is because clang, unlike gcc, won't accept invocations of this > form: > > clang file1.c header2.h > > Fix this by using selftests/lib.mk facilities for tracking local header > file dependencies: add them to LOCAL_HDRS, leaving only the .c files to > be passed to the compiler. > > [1] > https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ > > Fixes: 8e289f454289 ("selftests/resctrl: Add resctrl.h into build deps") > Cc: Ilpo Järvinen > Signed-off-by: John Hubbard > --- Thank you. Acked-by: Reinette Chatre Reinette
Re: [PATCH] selftests/resctrl: fix clang build warnings related to abs(), labs() calls
On 5/3/2024 9:52 AM, John Hubbard wrote: > On 5/3/24 1:00 AM, Ilpo Järvinen wrote: >> On Thu, 2 May 2024, John Hubbard wrote: > ... >>> diff --git a/tools/testing/selftests/resctrl/mbm_test.c >>> b/tools/testing/selftests/resctrl/mbm_test.c >>> index d67ffa3ec63a..c873793d016d 100644 >>> --- a/tools/testing/selftests/resctrl/mbm_test.c >>> +++ b/tools/testing/selftests/resctrl/mbm_test.c >>> @@ -33,7 +33,7 @@ show_bw_info(unsigned long *bw_imc, unsigned long >>> *bw_resc, size_t span) >>> avg_bw_imc = sum_bw_imc / 4; >>> avg_bw_resc = sum_bw_resc / 4; >>> - avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc; >>> + avg_diff = (float)(avg_bw_resc - avg_bw_imc) / avg_bw_imc; >>> avg_diff_per = (int)(avg_diff * 100); >>> ret = avg_diff_per > MAX_DIFF_PERCENT; >> >> But how are these two cases same after your change when you ended up >> removing taking the absolute value entirely? > > All of the arguments are unsigned integers, so all arithmetic results > are interpreted as unsigned, so taking the absolute value of that is > always a no-op. It does not seem as though clang can see when values have been casted. I tried to do so explicitly with a: avg_diff = labs((long)avg_bw_resc - avg_bw_imc) / (float)avg_bw_imc; But that still triggers: warning: taking the absolute value of unsigned type 'unsigned long' has no effect [-Wabsolute-value] Looks like we may need to be more explicit types and not rely on casting so much to make the compiler happy. Reinette
[PATCH v6 17/17] selftests: riscv: Support xtheadvector in vector tests
Extend existing vector tests to be compatible with the xtheadvector instruction set. Signed-off-by: Charlie Jenkins --- .../selftests/riscv/vector/v_exec_initval_nolibc.c | 23 -- tools/testing/selftests/riscv/vector/v_helpers.c | 17 +++- tools/testing/selftests/riscv/vector/v_helpers.h | 4 +- tools/testing/selftests/riscv/vector/v_initval.c | 12 ++- .../selftests/riscv/vector/vstate_exec_nolibc.c| 20 +++-- .../testing/selftests/riscv/vector/vstate_prctl.c | 91 ++ 6 files changed, 115 insertions(+), 52 deletions(-) diff --git a/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c b/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c index 74b13806baf0..58c29ea91b80 100644 --- a/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c +++ b/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c @@ -18,13 +18,22 @@ int main(int argc, char **argv) unsigned long vl; int first = 1; - asm volatile ( - ".option push\n\t" - ".option arch, +v\n\t" - "vsetvli%[vl], x0, e8, m1, ta, ma\n\t" - ".option pop\n\t" - : [vl] "=r" (vl) - ); + if (argc > 2 && strcmp(argv[2], "x")) + asm volatile ( + // 0 | zimm[10:0] | rs1 | 1 1 1 | rd |1010111| vsetvli + // vsetvli t4, x0, e8, m1, d1 + ".insn 0b011011010111\n\t" + "mv %[vl], t4\n\t" + : [vl] "=r" (vl) : : "t4" + ); + else + asm volatile ( + ".option push\n\t" + ".option arch, +v\n\t" + "vsetvli%[vl], x0, e8, m1, ta, ma\n\t" + ".option pop\n\t" + : [vl] "=r" (vl) + ); #define CHECK_VECTOR_REGISTER(register) ({ \ for (int i = 0; i < vl; i++) { \ diff --git a/tools/testing/selftests/riscv/vector/v_helpers.c b/tools/testing/selftests/riscv/vector/v_helpers.c index 15c22318db72..2c4df76eefe9 100644 --- a/tools/testing/selftests/riscv/vector/v_helpers.c +++ b/tools/testing/selftests/riscv/vector/v_helpers.c @@ -1,11 +1,21 @@ // SPDX-License-Identifier: GPL-2.0-only #include "../hwprobe/hwprobe.h" +#include #include #include #include #include +int is_xtheadvector_supported(void) +{ + struct riscv_hwprobe pair; + + pair.key = RISCV_HWPROBE_KEY_VENDOR_EXT_THEAD_0; + riscv_hwprobe(, 1, 0, NULL, 0); + return pair.value & RISCV_HWPROBE_VENDOR_EXT_XTHEADVECTOR; +} + int is_vector_supported(void) { struct riscv_hwprobe pair; @@ -15,9 +25,9 @@ int is_vector_supported(void) return pair.value & RISCV_HWPROBE_IMA_V; } -int launch_test(char *next_program, int test_inherit) +int launch_test(char *next_program, int test_inherit, int xtheadvector) { - char *exec_argv[3], *exec_envp[1]; + char *exec_argv[4], *exec_envp[1]; int rc, pid, status; pid = fork(); @@ -29,7 +39,8 @@ int launch_test(char *next_program, int test_inherit) if (!pid) { exec_argv[0] = next_program; exec_argv[1] = test_inherit != 0 ? "x" : NULL; - exec_argv[2] = NULL; + exec_argv[2] = xtheadvector != 0 ? "x" : NULL; + exec_argv[3] = NULL; exec_envp[0] = NULL; /* launch the program again to check inherit */ rc = execve(next_program, exec_argv, exec_envp); diff --git a/tools/testing/selftests/riscv/vector/v_helpers.h b/tools/testing/selftests/riscv/vector/v_helpers.h index 88719c4be496..67d41cb6f871 100644 --- a/tools/testing/selftests/riscv/vector/v_helpers.h +++ b/tools/testing/selftests/riscv/vector/v_helpers.h @@ -1,5 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +int is_xtheadvector_supported(void); + int is_vector_supported(void); -int launch_test(char *next_program, int test_inherit); +int launch_test(char *next_program, int test_inherit, int xtheadvector); diff --git a/tools/testing/selftests/riscv/vector/v_initval.c b/tools/testing/selftests/riscv/vector/v_initval.c index f38b5797fa31..be9e1d18ad29 100644 --- a/tools/testing/selftests/riscv/vector/v_initval.c +++ b/tools/testing/selftests/riscv/vector/v_initval.c @@ -7,10 +7,16 @@ TEST(v_initval) { - if (!is_vector_supported()) - SKIP(return, "Vector not supported"); + int xtheadvector = 0; - ASSERT_EQ(0, launch_test(NEXT_PROGRAM, 0)); + if (!is_vector_supported()) { + if (is_xtheadvector_supported()) + xtheadvector = 1; + else + SKIP(return, "Vector not supported"); + } + + ASSERT_EQ(0, launch_test(NEXT_PROGRAM,
[PATCH v6 16/17] selftests: riscv: Fix vector tests
Overhaul the riscv vector tests to use kselftest_harness to help the test cases correctly report the results and decouple the individual test cases from each other. With this refactoring, only run the test cases is vector is reported and properly report the test case as skipped otherwise. The v_initval_nolibc test was previously not checking if vector was supported and used a function (malloc) which invalidates the state of the vector registers. Signed-off-by: Charlie Jenkins --- tools/testing/selftests/riscv/vector/.gitignore| 3 +- tools/testing/selftests/riscv/vector/Makefile | 17 +- .../selftests/riscv/vector/v_exec_initval_nolibc.c | 84 +++ tools/testing/selftests/riscv/vector/v_helpers.c | 56 + tools/testing/selftests/riscv/vector/v_helpers.h | 5 + tools/testing/selftests/riscv/vector/v_initval.c | 16 ++ .../selftests/riscv/vector/v_initval_nolibc.c | 68 -- .../testing/selftests/riscv/vector/vstate_prctl.c | 266 - 8 files changed, 324 insertions(+), 191 deletions(-) diff --git a/tools/testing/selftests/riscv/vector/.gitignore b/tools/testing/selftests/riscv/vector/.gitignore index 9ae7964491d5..7d9c87cd0649 100644 --- a/tools/testing/selftests/riscv/vector/.gitignore +++ b/tools/testing/selftests/riscv/vector/.gitignore @@ -1,3 +1,4 @@ vstate_exec_nolibc vstate_prctl -v_initval_nolibc +v_initval +v_exec_initval_nolibc diff --git a/tools/testing/selftests/riscv/vector/Makefile b/tools/testing/selftests/riscv/vector/Makefile index bfff0ff4f3be..995746359477 100644 --- a/tools/testing/selftests/riscv/vector/Makefile +++ b/tools/testing/selftests/riscv/vector/Makefile @@ -2,18 +2,27 @@ # Copyright (C) 2021 ARM Limited # Originally tools/testing/arm64/abi/Makefile -TEST_GEN_PROGS := vstate_prctl v_initval_nolibc -TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc +TEST_GEN_PROGS := v_initval vstate_prctl +TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc v_exec_initval_nolibc sys_hwprobe.o v_helpers.o include ../../lib.mk -$(OUTPUT)/vstate_prctl: vstate_prctl.c ../hwprobe/sys_hwprobe.S +$(OUTPUT)/sys_hwprobe.o: ../hwprobe/sys_hwprobe.S + $(CC) -static -c -o$@ $(CFLAGS) $^ + +$(OUTPUT)/v_helpers.o: v_helpers.c + $(CC) -static -c -o$@ $(CFLAGS) $^ + +$(OUTPUT)/vstate_prctl: vstate_prctl.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^ $(OUTPUT)/vstate_exec_nolibc: vstate_exec_nolibc.c $(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \ -Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc -$(OUTPUT)/v_initval_nolibc: v_initval_nolibc.c +$(OUTPUT)/v_initval: v_initval.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o + $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^ + +$(OUTPUT)/v_exec_initval_nolibc: v_exec_initval_nolibc.c $(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \ -Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc diff --git a/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c b/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c new file mode 100644 index ..74b13806baf0 --- /dev/null +++ b/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Get values of vector registers as soon as the program starts to test if + * is properly cleaning the values before starting a new program. Vector + * registers are caller saved, so no function calls may happen before reading + * the values. To further ensure consistency, this file is compiled without + * libc and without auto-vectorization. + * + * To be "clean" all values must be either all ones or all zeroes. + */ + +#define __stringify_1(x...)#x +#define __stringify(x...) __stringify_1(x) + +int main(int argc, char **argv) +{ + char prev_value = 0, value; + unsigned long vl; + int first = 1; + + asm volatile ( + ".option push\n\t" + ".option arch, +v\n\t" + "vsetvli%[vl], x0, e8, m1, ta, ma\n\t" + ".option pop\n\t" + : [vl] "=r" (vl) + ); + +#define CHECK_VECTOR_REGISTER(register) ({ \ + for (int i = 0; i < vl; i++) { \ + asm volatile ( \ + ".option push\n\t" \ + ".option arch, +v\n\t" \ + "vmv.x.s %0, " __stringify(register) "\n\t" \ + "vsrl.vi " __stringify(register) ", " __stringify(register) ", 8\n\t" \ + ".option pop\n\t" \ + : "=r" (value)); \ + if (first) {
[PATCH v6 15/17] riscv: hwprobe: Document thead vendor extensions and xtheadvector extension
Document support for thead vendor extensions using the key RISCV_HWPROBE_KEY_VENDOR_EXT_THEAD_0 and xtheadvector extension using the key RISCV_HWPROBE_VENDOR_EXT_XTHEADVECTOR. Signed-off-by: Charlie Jenkins Reviewed-by: Evan Green --- Documentation/arch/riscv/hwprobe.rst | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst index b2bcc9eed9aa..b2bb305140aa 100644 --- a/Documentation/arch/riscv/hwprobe.rst +++ b/Documentation/arch/riscv/hwprobe.rst @@ -210,3 +210,13 @@ The following keys are defined: * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which represents the size of the Zicboz block in bytes. + +* :c:macro:`RISCV_HWPROBE_KEY_VENDOR_EXT_THEAD_0`: A bitmask containing the + thead vendor extensions that are compatible with the + :c:macro:`RISCV_HWPROBE_BASE_BEHAVIOR_IMA`: base system behavior. + + * T-HEAD + +* :c:macro:`RISCV_HWPROBE_VENDOR_EXT_XTHEADVECTOR`: The xtheadvector vendor +extension is supported in the T-Head ISA extensions spec starting from + commit a18c801634 ("Add T-Head VECTOR vendor extension. "). -- 2.44.0
[PATCH v6 14/17] riscv: hwprobe: Add thead vendor extension probing
Add a new hwprobe key "RISCV_HWPROBE_KEY_VENDOR_EXT_THEAD_0" which allows userspace to probe for the new RISCV_ISA_VENDOR_EXT_XTHEADVECTOR vendor extension. This new key will allow userspace code to probe for which thead vendor extensions are supported. This API is modeled to be consistent with RISCV_HWPROBE_KEY_IMA_EXT_0. The bitmask returned will have each bit corresponding to a supported thead vendor extension of the cpumask set. Just like RISCV_HWPROBE_KEY_IMA_EXT_0, this allows a userspace program to determine all of the supported thead vendor extensions in one call. Signed-off-by: Charlie Jenkins Reviewed-by: Evan Green --- arch/riscv/include/asm/hwprobe.h | 4 +-- .../include/asm/vendor_extensions/thead_hwprobe.h | 18 +++ .../include/asm/vendor_extensions/vendor_hwprobe.h | 37 ++ arch/riscv/include/uapi/asm/hwprobe.h | 3 +- arch/riscv/include/uapi/asm/vendor/thead.h | 3 ++ arch/riscv/kernel/sys_hwprobe.c| 5 +++ arch/riscv/kernel/vendor_extensions/Makefile | 1 + .../riscv/kernel/vendor_extensions/thead_hwprobe.c | 19 +++ 8 files changed, 87 insertions(+), 3 deletions(-) diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h index 630507dff5ea..e68496b4f8de 100644 --- a/arch/riscv/include/asm/hwprobe.h +++ b/arch/riscv/include/asm/hwprobe.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ /* - * Copyright 2023 Rivos, Inc + * Copyright 2023-2024 Rivos, Inc */ #ifndef _ASM_HWPROBE_H @@ -8,7 +8,7 @@ #include -#define RISCV_HWPROBE_MAX_KEY 6 +#define RISCV_HWPROBE_MAX_KEY 7 static inline bool riscv_hwprobe_key_is_valid(__s64 key) { diff --git a/arch/riscv/include/asm/vendor_extensions/thead_hwprobe.h b/arch/riscv/include/asm/vendor_extensions/thead_hwprobe.h new file mode 100644 index ..925fef39a2c0 --- /dev/null +++ b/arch/riscv/include/asm/vendor_extensions/thead_hwprobe.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_RISCV_VENDOR_EXTENSIONS_THEAD_HWPROBE_H +#define _ASM_RISCV_VENDOR_EXTENSIONS_THEAD_HWPROBE_H + +#include + +#include + +#ifdef CONFIG_RISCV_ISA_VENDOR_EXT_THEAD +void hwprobe_isa_vendor_ext_thead_0(struct riscv_hwprobe *pair, const struct cpumask *cpus); +#else +static inline void hwprobe_isa_vendor_ext_thead_0(struct riscv_hwprobe *pair, const struct cpumask *cpus) +{ + pair->value = 0; +} +#endif + +#endif diff --git a/arch/riscv/include/asm/vendor_extensions/vendor_hwprobe.h b/arch/riscv/include/asm/vendor_extensions/vendor_hwprobe.h new file mode 100644 index ..b6222e7b519e --- /dev/null +++ b/arch/riscv/include/asm/vendor_extensions/vendor_hwprobe.h @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright 2024 Rivos, Inc + */ + +#ifndef _ASM_RISCV_SYS_HWPROBE_H +#define _ASM_RISCV_SYS_HWPROBE_H + +#include + +#define EXT_KEY(ext) \ + do { \ + if (__riscv_isa_extension_available(isainfo->isa, RISCV_ISA_VENDOR_EXT_##ext)) \ + pair->value |= RISCV_HWPROBE_VENDOR_EXT_##ext; \ + else \ + missing |= RISCV_HWPROBE_VENDOR_EXT_##ext; \ + } while (false) + +/* + * Loop through and record extensions that 1) anyone has, and 2) anyone + * doesn't have. + * + * _extension_checks is an arbitrary C block to set the values of pair->value + * and missing. It should be filled with EXT_KEY expressions. + */ +#define VENDOR_EXTENSION_SUPPORTED(pair, cpus, per_hart_thead_bitmap, _extension_checks) \ + do { \ + int cpu; \ + u64 missing; \ + for_each_cpu(cpu, (cpus)) { \ + struct riscv_isavendorinfo *isainfo = &(per_hart_thead_bitmap)[cpu];\ + _extension_checks \ + } \ + (pair)->value &= ~missing; \ + } while (false) \ + +#endif /* _ASM_RISCV_SYS_HWPROBE_H */ diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h index 9f2a8e3ff204..21e96a63f9ea 100644 --- a/arch/riscv/include/uapi/asm/hwprobe.h +++
[PATCH v6 13/17] riscv: vector: Support xtheadvector save/restore
Use alternatives to add support for xtheadvector vector save/restore routines. Signed-off-by: Charlie Jenkins --- arch/riscv/Kconfig.vendor | 13 ++ arch/riscv/include/asm/csr.h | 6 + arch/riscv/include/asm/switch_to.h | 2 +- arch/riscv/include/asm/vector.h| 247 ++--- arch/riscv/kernel/cpufeature.c | 2 +- arch/riscv/kernel/kernel_mode_vector.c | 8 +- arch/riscv/kernel/process.c| 4 +- arch/riscv/kernel/signal.c | 6 +- arch/riscv/kernel/vector.c | 13 +- 9 files changed, 233 insertions(+), 68 deletions(-) diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor index aa5a191e659e..edf49f3065ac 100644 --- a/arch/riscv/Kconfig.vendor +++ b/arch/riscv/Kconfig.vendor @@ -13,6 +13,19 @@ config RISCV_ISA_VENDOR_EXT_THEAD extensions. Without this option enabled, T-Head vendor extensions will not be detected at boot and their presence not reported to userspace. + If you don't know what to do here, say Y. + +config RISCV_ISA_XTHEADVECTOR + bool "xtheadvector extension support" + depends on RISCV_ISA_VENDOR_EXT_THEAD + depends on RISCV_ISA_V + depends on FPU + default y + help + Say N here if you want to disable all xtheadvector related procedure + in the kernel. This will disable vector for any T-Head board that + contains xtheadvector rather than the standard vector. + If you don't know what to do here, say Y. endmenu diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h index e5a35efd56e0..13657d096e7d 100644 --- a/arch/riscv/include/asm/csr.h +++ b/arch/riscv/include/asm/csr.h @@ -30,6 +30,12 @@ #define SR_VS_CLEAN_AC(0x0400, UL) #define SR_VS_DIRTY_AC(0x0600, UL) +#define SR_VS_THEAD_AC(0x0180, UL) /* xtheadvector Status */ +#define SR_VS_OFF_THEAD_AC(0x, UL) +#define SR_VS_INITIAL_THEAD_AC(0x0080, UL) +#define SR_VS_CLEAN_THEAD _AC(0x0100, UL) +#define SR_VS_DIRTY_THEAD _AC(0x0180, UL) + #define SR_XS _AC(0x00018000, UL) /* Extension Status */ #define SR_XS_OFF _AC(0x, UL) #define SR_XS_INITIAL _AC(0x8000, UL) diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h index 7efdb0584d47..ada6b5cf2d94 100644 --- a/arch/riscv/include/asm/switch_to.h +++ b/arch/riscv/include/asm/switch_to.h @@ -78,7 +78,7 @@ do { \ struct task_struct *__next = (next);\ if (has_fpu()) \ __switch_to_fpu(__prev, __next);\ - if (has_vector()) \ + if (has_vector() || has_xtheadvector()) \ __switch_to_vector(__prev, __next); \ ((last) = __switch_to(__prev, __next)); \ } while (0) diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h index 731dcd0ed4de..db851dc81870 100644 --- a/arch/riscv/include/asm/vector.h +++ b/arch/riscv/include/asm/vector.h @@ -18,6 +18,27 @@ #include #include #include +#include +#include +#include + +#define __riscv_v_vstate_or(_val, TYPE) ({ \ + typeof(_val) _res = _val; \ + if (has_xtheadvector()) \ + _res = (_res & ~SR_VS_THEAD) | SR_VS_##TYPE##_THEAD;\ + else\ + _res = (_res & ~SR_VS) | SR_VS_##TYPE; \ + _res; \ +}) + +#define __riscv_v_vstate_check(_val, TYPE) ({ \ + bool _res; \ + if (has_xtheadvector()) \ + _res = ((_val) & SR_VS_THEAD) == SR_VS_##TYPE##_THEAD; \ + else\ + _res = ((_val) & SR_VS) == SR_VS_##TYPE;\ + _res; \ +}) extern unsigned long riscv_v_vsize; int riscv_v_setup_vsize(void); @@ -40,39 +61,62 @@ static __always_inline bool has_vector(void) return riscv_has_extension_unlikely(RISCV_ISA_EXT_v); } +static __always_inline bool has_xtheadvector_no_alternatives(void) +{ + if (IS_ENABLED(CONFIG_RISCV_ISA_XTHEADVECTOR)) + return riscv_isa_vendor_extension_available(THEAD_VENDOR_ID, XTHEADVECTOR); + else + return false; +} + +static __always_inline bool has_xtheadvector(void) +{ + if (IS_ENABLED(CONFIG_RISCV_ISA_XTHEADVECTOR)) + return riscv_has_vendor_extension_unlikely(THEAD_VENDOR_ID, +
[PATCH v6 11/17] riscv: csr: Add CSR encodings for VCSR_VXRM/VCSR_VXSAT
The VXRM vector csr for xtheadvector has an encoding of 0xa and VXSAT has an encoding of 0x9. Co-developed-by: Heiko Stuebner Signed-off-by: Charlie Jenkins --- arch/riscv/include/asm/csr.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h index 13bc99c995d1..e5a35efd56e0 100644 --- a/arch/riscv/include/asm/csr.h +++ b/arch/riscv/include/asm/csr.h @@ -219,6 +219,8 @@ #define VCSR_VXRM_MASK 3 #define VCSR_VXRM_SHIFT1 #define VCSR_VXSAT_MASK1 +#define VCSR_VXSAT 0x9 +#define VCSR_VXRM 0xa /* symbolic CSR names: */ #define CSR_CYCLE 0xc00 -- 2.44.0
[PATCH v6 10/17] RISC-V: define the elements of the VCSR vector CSR
From: Heiko Stuebner The VCSR CSR contains two elements VXRM[2:1] and VXSAT[0]. Define constants for those to access the elements in a readable way. Acked-by: Guo Ren Reviewed-by: Conor Dooley Signed-off-by: Heiko Stuebner Signed-off-by: Charlie Jenkins --- arch/riscv/include/asm/csr.h | 5 + 1 file changed, 5 insertions(+) diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h index 2468c55933cd..13bc99c995d1 100644 --- a/arch/riscv/include/asm/csr.h +++ b/arch/riscv/include/asm/csr.h @@ -215,6 +215,11 @@ #define SMSTATEEN0_SSTATEEN0_SHIFT 63 #define SMSTATEEN0_SSTATEEN0 (_ULL(1) << SMSTATEEN0_SSTATEEN0_SHIFT) +/* VCSR flags */ +#define VCSR_VXRM_MASK 3 +#define VCSR_VXRM_SHIFT1 +#define VCSR_VXSAT_MASK1 + /* symbolic CSR names: */ #define CSR_CYCLE 0xc00 #define CSR_TIME 0xc01 -- 2.44.0
[PATCH v6 09/17] riscv: Convert xandespmu to use the vendor extension framework
Migrate xandespmu out of riscv_isa_ext and into a new Andes-specific vendor namespace. Signed-off-by: Charlie Jenkins Reviewed-by: Conor Dooley --- arch/riscv/Kconfig.vendor| 12 arch/riscv/errata/andes/errata.c | 2 ++ arch/riscv/include/asm/hwcap.h | 1 - arch/riscv/include/asm/vendor_extensions/andes.h | 19 +++ arch/riscv/kernel/cpufeature.c | 1 - arch/riscv/kernel/vendor_extensions.c| 10 ++ arch/riscv/kernel/vendor_extensions/Makefile | 1 + arch/riscv/kernel/vendor_extensions/andes.c | 18 ++ drivers/perf/riscv_pmu_sbi.c | 9 ++--- 9 files changed, 68 insertions(+), 5 deletions(-) diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor index 85ac30496b0e..aa5a191e659e 100644 --- a/arch/riscv/Kconfig.vendor +++ b/arch/riscv/Kconfig.vendor @@ -16,4 +16,16 @@ config RISCV_ISA_VENDOR_EXT_THEAD If you don't know what to do here, say Y. endmenu +menu "Andes" +config RISCV_ISA_VENDOR_EXT_ANDES + bool "Andes vendor extension support" + default y + help + Say N here if you want to disable all Andes vendor extension + support. This will cause any Andes vendor extensions that are + requested by hardware probing to be ignored. + + If you don't know what to do here, say Y. +endmenu + endmenu diff --git a/arch/riscv/errata/andes/errata.c b/arch/riscv/errata/andes/errata.c index f2708a9494a1..a5d96a7a4682 100644 --- a/arch/riscv/errata/andes/errata.c +++ b/arch/riscv/errata/andes/errata.c @@ -65,6 +65,8 @@ void __init_or_module andes_errata_patch_func(struct alt_entry *begin, struct al unsigned long archid, unsigned long impid, unsigned int stage) { + BUILD_BUG_ON(ERRATA_ANDES_NUMBER >= RISCV_VENDOR_EXT_ALTERNATIVES_BASE); + if (stage == RISCV_ALTERNATIVES_BOOT) errata_probe_iocp(stage, archid, impid); diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index e17d0078a651..1f2d2599c655 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -80,7 +80,6 @@ #define RISCV_ISA_EXT_ZFA 71 #define RISCV_ISA_EXT_ZTSO 72 #define RISCV_ISA_EXT_ZACAS73 -#define RISCV_ISA_EXT_XANDESPMU74 #define RISCV_ISA_EXT_XLINUXENVCFG 127 diff --git a/arch/riscv/include/asm/vendor_extensions/andes.h b/arch/riscv/include/asm/vendor_extensions/andes.h new file mode 100644 index ..7bb2fc43438f --- /dev/null +++ b/arch/riscv/include/asm/vendor_extensions/andes.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_RISCV_VENDOR_EXTENSIONS_ANDES_H +#define _ASM_RISCV_VENDOR_EXTENSIONS_ANDES_H + +#include + +#include + +#define RISCV_ISA_VENDOR_EXT_XANDESPMU 0 + +/* + * Extension keys should be strictly less than max. + * It is safe to increment this when necessary. + */ +#define RISCV_ISA_VENDOR_EXT_MAX_ANDES 32 + +extern struct riscv_isa_vendor_ext_data_list riscv_isa_vendor_ext_list_andes; + +#endif diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 2a5527020d0f..2993318b8ea2 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -289,7 +289,6 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT), __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), - __RISCV_ISA_EXT_DATA(xandespmu, RISCV_ISA_EXT_XANDESPMU), }; const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext); diff --git a/arch/riscv/kernel/vendor_extensions.c b/arch/riscv/kernel/vendor_extensions.c index 7910890c17de..e4d58938e6ce 100644 --- a/arch/riscv/kernel/vendor_extensions.c +++ b/arch/riscv/kernel/vendor_extensions.c @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -14,6 +15,9 @@ struct riscv_isa_vendor_ext_data_list *riscv_isa_vendor_ext_list[] = { #ifdef CONFIG_RISCV_ISA_VENDOR_EXT_THEAD _isa_vendor_ext_list_thead, #endif +#ifdef CONFIG_RISCV_ISA_VENDOR_EXT_ANDES + _isa_vendor_ext_list_andes, +#endif }; const size_t riscv_isa_vendor_ext_list_size = ARRAY_SIZE(riscv_isa_vendor_ext_list); @@ -40,6 +44,12 @@ bool __riscv_isa_vendor_extension_available(int cpu, unsigned long vendor, unsig bmap = _isa_vendor_ext_list_thead.all_harts_isa_bitmap; cpu_bmap = _isa_vendor_ext_list_thead.per_hart_isa_bitmap[cpu]; break; +#endif +#ifdef CONFIG_RISCV_ISA_VENDOR_EXT_ANDES + case ANDES_VENDOR_ID: + bmap = _isa_vendor_ext_list_andes.all_harts_isa_bitmap; + cpu_bmap =
[PATCH v6 08/17] riscv: cpufeature: Extract common elements from extension checking
The __riscv_has_extension_likely() and __riscv_has_extension_unlikely() functions from the vendor_extensions.h can be used to simplify the standard extension checking code as well. Migrate those functions to cpufeature.h and reorganize the code in the file to use the functions. Signed-off-by: Charlie Jenkins Reviewed-by: Conor Dooley --- arch/riscv/include/asm/cpufeature.h| 78 +- arch/riscv/include/asm/vendor_extensions.h | 28 --- arch/riscv/kernel/vendor_extensions.c | 16 +++--- 3 files changed, 51 insertions(+), 71 deletions(-) diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index fedd479ccfd1..88723ac2d26e 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -98,59 +98,66 @@ extern bool riscv_isa_fallback; unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap); +#define STANDARD_EXT 0 + bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, unsigned int bit); #define riscv_isa_extension_available(isa_bitmap, ext) \ __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext) -static __always_inline bool -riscv_has_extension_likely(const unsigned long ext) +static __always_inline bool __riscv_has_extension_likely(const unsigned long vendor, +const unsigned long ext) { - compiletime_assert(ext < RISCV_ISA_EXT_MAX, - "ext must be < RISCV_ISA_EXT_MAX"); - - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { - asm goto( - ALTERNATIVE("j %l[l_no]", "nop", 0, %[ext], 1) - : - : [ext] "i" (ext) - : - : l_no); - } else { - if (!__riscv_isa_extension_available(NULL, ext)) - goto l_no; - } + asm goto(ALTERNATIVE("j %l[l_no]", "nop", %[vendor], %[ext], 1) + : + : [vendor] "i" (vendor), [ext] "i" (ext) + : + : l_no); return true; l_no: return false; } -static __always_inline bool -riscv_has_extension_unlikely(const unsigned long ext) +static __always_inline bool __riscv_has_extension_unlikely(const unsigned long vendor, + const unsigned long ext) { - compiletime_assert(ext < RISCV_ISA_EXT_MAX, - "ext must be < RISCV_ISA_EXT_MAX"); - - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { - asm goto( - ALTERNATIVE("nop", "j %l[l_yes]", 0, %[ext], 1) - : - : [ext] "i" (ext) - : - : l_yes); - } else { - if (__riscv_isa_extension_available(NULL, ext)) - goto l_yes; - } + asm goto(ALTERNATIVE("nop", "j %l[l_yes]", %[vendor], %[ext], 1) + : + : [vendor] "i" (vendor), [ext] "i" (ext) + : + : l_yes); return false; l_yes: return true; } +static __always_inline bool riscv_has_extension_unlikely(const unsigned long ext) +{ + compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_unlikely(STANDARD_EXT, ext); + + return __riscv_isa_extension_available(NULL, ext); +} + +static __always_inline bool riscv_has_extension_likely(const unsigned long ext) +{ + compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_likely(STANDARD_EXT, ext); + + return __riscv_isa_extension_available(NULL, ext); +} + static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext) { - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext)) + compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && + __riscv_has_extension_likely(STANDARD_EXT, ext)) return true; return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); @@ -158,7 +165,10 @@ static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsign static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext) { - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext)) + compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && + __riscv_has_extension_unlikely(STANDARD_EXT, ext)) return true; return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); diff --git a/arch/riscv/include/asm/vendor_extensions.h
[PATCH v6 07/17] riscv: Introduce vendor variants of extension helpers
Vendor extensions are maintained in per-vendor structs (separate from standard extensions which live in riscv_isa). Create vendor variants for the existing extension helpers to interface with the riscv_isa_vendor bitmaps. Signed-off-by: Charlie Jenkins Reviewed-by: Conor Dooley --- arch/riscv/errata/sifive/errata.c | 3 + arch/riscv/errata/thead/errata.c | 3 + arch/riscv/include/asm/vendor_extensions.h | 97 ++ arch/riscv/kernel/cpufeature.c | 32 +++--- arch/riscv/kernel/vendor_extensions.c | 40 5 files changed, 167 insertions(+), 8 deletions(-) diff --git a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c index 3d9a32d791f7..b68b023115c2 100644 --- a/arch/riscv/errata/sifive/errata.c +++ b/arch/riscv/errata/sifive/errata.c @@ -12,6 +12,7 @@ #include #include #include +#include struct errata_info_t { char name[32]; @@ -91,6 +92,8 @@ void sifive_errata_patch_func(struct alt_entry *begin, struct alt_entry *end, u32 cpu_apply_errata = 0; u32 tmp; + BUILD_BUG_ON(ERRATA_SIFIVE_NUMBER >= RISCV_VENDOR_EXT_ALTERNATIVES_BASE); + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT) return; diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c index b1c410bbc1ae..6d5d7f8eebbc 100644 --- a/arch/riscv/errata/thead/errata.c +++ b/arch/riscv/errata/thead/errata.c @@ -18,6 +18,7 @@ #include #include #include +#include static bool errata_probe_pbmt(unsigned int stage, unsigned long arch_id, unsigned long impid) @@ -160,6 +161,8 @@ void thead_errata_patch_func(struct alt_entry *begin, struct alt_entry *end, u32 tmp; void *oldptr, *altptr; + BUILD_BUG_ON(ERRATA_THEAD_NUMBER >= RISCV_VENDOR_EXT_ALTERNATIVES_BASE); + for (alt = begin; alt < end; alt++) { if (alt->vendor_id != THEAD_VENDOR_ID) continue; diff --git a/arch/riscv/include/asm/vendor_extensions.h b/arch/riscv/include/asm/vendor_extensions.h index bf4dac66e6e6..a6959836f895 100644 --- a/arch/riscv/include/asm/vendor_extensions.h +++ b/arch/riscv/include/asm/vendor_extensions.h @@ -31,4 +31,101 @@ extern struct riscv_isa_vendor_ext_data_list *riscv_isa_vendor_ext_list[]; extern const size_t riscv_isa_vendor_ext_list_size; +/* + * The alternatives need some way of distinguishing between vendor extensions + * and errata. Incrementing all of the vendor extension keys so they are at + * least 0x8000 accomplishes that. + */ +#define RISCV_VENDOR_EXT_ALTERNATIVES_BASE 0x8000 + +#define VENDOR_EXT_ALL_CPUS-1 + +bool __riscv_isa_vendor_extension_available(int cpu, unsigned long vendor, unsigned int bit); +#define riscv_cpu_isa_vendor_extension_available(cpu, vendor, ext) \ + __riscv_isa_vendor_extension_available(cpu, vendor, RISCV_ISA_VENDOR_EXT_##ext) +#define riscv_isa_vendor_extension_available(vendor, ext) \ + __riscv_isa_vendor_extension_available(VENDOR_EXT_ALL_CPUS, vendor, \ + RISCV_ISA_VENDOR_EXT_##ext) + +static __always_inline bool __riscv_has_extension_likely(const unsigned long vendor, +const unsigned long ext) +{ + asm goto(ALTERNATIVE("j %l[l_no]", "nop", %[vendor], %[ext], 1) + : + : [vendor] "i" (vendor), [ext] "i" (ext) + : + : l_no); + + return true; +l_no: + return false; +} + +static __always_inline bool __riscv_has_extension_unlikely(const unsigned long vendor, + const unsigned long ext) +{ + asm goto(ALTERNATIVE("nop", "j %l[l_yes]", %[vendor], %[ext], 1) + : + : [vendor] "i" (vendor), [ext] "i" (ext) + : + : l_yes); + + return false; +l_yes: + return true; +} + +static __always_inline bool riscv_has_vendor_extension_likely(const unsigned long vendor, + const unsigned long ext) +{ + if (!IS_ENABLED(CONFIG_RISCV_ISA_VENDOR_EXT)) + return false; + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_likely(vendor, + ext + RISCV_VENDOR_EXT_ALTERNATIVES_BASE); + + return __riscv_isa_vendor_extension_available(VENDOR_EXT_ALL_CPUS, vendor, ext); +} + +static __always_inline bool riscv_has_vendor_extension_unlikely(const unsigned long vendor, + const unsigned long ext) +{ + if (!IS_ENABLED(CONFIG_RISCV_ISA_VENDOR_EXT)) + return false; + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_unlikely(vendor, +
[PATCH v6 06/17] riscv: Add vendor extensions to /proc/cpuinfo
All of the supported vendor extensions that have been listed in riscv_isa_vendor_ext_list can be exported through /proc/cpuinfo. Signed-off-by: Charlie Jenkins --- arch/riscv/kernel/cpu.c | 35 --- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index d11d6320fb0d..2a7924dd809b 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -16,6 +16,7 @@ #include #include #include +#include bool arch_match_cpu_phys_id(int cpu, u64 phys_id) { @@ -203,7 +204,33 @@ arch_initcall(riscv_cpuinfo_init); #ifdef CONFIG_PROC_FS -static void print_isa(struct seq_file *f, const unsigned long *isa_bitmap) +#define ALL_CPUS -1 + +static void print_vendor_isa(struct seq_file *f, int cpu) +{ + struct riscv_isavendorinfo *vendor_bitmap; + struct riscv_isa_vendor_ext_data_list *ext_list; + const struct riscv_isa_ext_data *ext_data; + + for (int i = 0; i < riscv_isa_vendor_ext_list_size; i++) { + ext_list = riscv_isa_vendor_ext_list[i]; + ext_data = riscv_isa_vendor_ext_list[i]->ext_data; + + if (cpu == ALL_CPUS) + vendor_bitmap = _list->all_harts_isa_bitmap; + else + vendor_bitmap = _list->per_hart_isa_bitmap[cpu]; + + for (int j = 0; j < ext_list->ext_data_count; j++) { + if (!__riscv_isa_extension_available(vendor_bitmap->isa, ext_data[j].id)) + continue; + + seq_printf(f, "_%s", ext_data[j].name); + } + } +} + +static void print_isa(struct seq_file *f, const unsigned long *isa_bitmap, int cpu) { if (IS_ENABLED(CONFIG_32BIT)) @@ -222,6 +249,8 @@ static void print_isa(struct seq_file *f, const unsigned long *isa_bitmap) seq_printf(f, "%s", riscv_isa_ext[i].name); } + print_vendor_isa(f, cpu); + seq_puts(f, "\n"); } @@ -284,7 +313,7 @@ static int c_show(struct seq_file *m, void *v) * line. */ seq_puts(m, "isa\t\t: "); - print_isa(m, NULL); + print_isa(m, NULL, ALL_CPUS); print_mmu(m); if (acpi_disabled) { @@ -306,7 +335,7 @@ static int c_show(struct seq_file *m, void *v) * additional extensions not present across all harts. */ seq_puts(m, "hart isa\t: "); - print_isa(m, hart_isa[cpu_id].isa); + print_isa(m, hart_isa[cpu_id].isa, cpu_id); seq_puts(m, "\n"); return 0; -- 2.44.0
[PATCH v6 05/17] riscv: Extend cpufeature.c to detect vendor extensions
Separate vendor extensions out into one struct per vendor instead of adding vendor extensions onto riscv_isa_ext. Add a hidden config RISCV_ISA_VENDOR_EXT to conditionally include this code. The xtheadvector vendor extension is added using these changes. Signed-off-by: Charlie Jenkins --- arch/riscv/Kconfig | 2 + arch/riscv/Kconfig.vendor| 19 + arch/riscv/include/asm/cpufeature.h | 18 + arch/riscv/include/asm/vendor_extensions.h | 34 + arch/riscv/include/asm/vendor_extensions/thead.h | 16 arch/riscv/kernel/Makefile | 2 + arch/riscv/kernel/cpufeature.c | 93 +++- arch/riscv/kernel/vendor_extensions.c| 18 + arch/riscv/kernel/vendor_extensions/Makefile | 3 + arch/riscv/kernel/vendor_extensions/thead.c | 18 + 10 files changed, 203 insertions(+), 20 deletions(-) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index be09c8836d56..fec86fba3acd 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -759,6 +759,8 @@ config RISCV_EFFICIENT_UNALIGNED_ACCESS endchoice +source "arch/riscv/Kconfig.vendor" + endmenu # "Platform type" menu "Kernel features" diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor new file mode 100644 index ..85ac30496b0e --- /dev/null +++ b/arch/riscv/Kconfig.vendor @@ -0,0 +1,19 @@ +menu "Vendor extensions" + +config RISCV_ISA_VENDOR_EXT + bool + +menu "T-Head" +config RISCV_ISA_VENDOR_EXT_THEAD + bool "T-Head vendor extension support" + select RISCV_ISA_VENDOR_EXT + default y + help + Say N here to disable detection of and support for all T-Head vendor + extensions. Without this option enabled, T-Head vendor extensions will + not be detected at boot and their presence not reported to userspace. + + If you don't know what to do here, say Y. +endmenu + +endmenu diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index 0c4f08577015..fedd479ccfd1 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -35,6 +35,24 @@ extern u32 riscv_vlenb_of; void riscv_user_isa_enable(void); +#define _RISCV_ISA_EXT_DATA(_name, _id, _subset_exts, _subset_exts_size) { \ + .name = #_name, \ + .property = #_name, \ + .id = _id, \ + .subset_ext_ids = _subset_exts, \ + .subset_ext_size = _subset_exts_size \ +} + +#define __RISCV_ISA_EXT_DATA(_name, _id) _RISCV_ISA_EXT_DATA(_name, _id, NULL, 0) + +/* Used to declare pure "lasso" extension (Zk for instance) */ +#define __RISCV_ISA_EXT_BUNDLE(_name, _bundled_exts) \ + _RISCV_ISA_EXT_DATA(_name, RISCV_ISA_EXT_INVALID, _bundled_exts, ARRAY_SIZE(_bundled_exts)) + +/* Used to declare extensions that are a superset of other extensions (Zvbb for instance) */ +#define __RISCV_ISA_EXT_SUPERSET(_name, _id, _sub_exts) \ + _RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, ARRAY_SIZE(_sub_exts)) + #if defined(CONFIG_RISCV_MISALIGNED) bool check_unaligned_access_emulated_all_cpus(void); void unaligned_emulation_finish(void); diff --git a/arch/riscv/include/asm/vendor_extensions.h b/arch/riscv/include/asm/vendor_extensions.h new file mode 100644 index ..bf4dac66e6e6 --- /dev/null +++ b/arch/riscv/include/asm/vendor_extensions.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright 2024 Rivos, Inc + */ + +#ifndef _ASM_VENDOR_EXTENSIONS_H +#define _ASM_VENDOR_EXTENSIONS_H + +#include + +#include +#include + +/* + * The extension keys of each vendor must be strictly less than this value. + */ +#define RISCV_ISA_VENDOR_EXT_MAX 32 + +struct riscv_isavendorinfo { + DECLARE_BITMAP(isa, RISCV_ISA_VENDOR_EXT_MAX); +}; + +struct riscv_isa_vendor_ext_data_list { + const size_t ext_data_count; + const struct riscv_isa_ext_data *ext_data; + struct riscv_isavendorinfo per_hart_isa_bitmap[NR_CPUS]; + struct riscv_isavendorinfo all_harts_isa_bitmap; +}; + +extern struct riscv_isa_vendor_ext_data_list *riscv_isa_vendor_ext_list[]; + +extern const size_t riscv_isa_vendor_ext_list_size; + +#endif /* _ASM_VENDOR_EXTENSIONS_H */ diff --git a/arch/riscv/include/asm/vendor_extensions/thead.h b/arch/riscv/include/asm/vendor_extensions/thead.h new file mode 100644 index ..48421d1553ad --- /dev/null +++ b/arch/riscv/include/asm/vendor_extensions/thead.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_RISCV_VENDOR_EXTENSIONS_THEAD_H +#define _ASM_RISCV_VENDOR_EXTENSIONS_THEAD_H + +#include + +#include + +/* + * Extension keys must
[PATCH v6 04/17] riscv: dts: allwinner: Add xtheadvector to the D1/D1s devicetree
The D1/D1s SoCs support xtheadvector so it can be included in the devicetree. Also include vlenb for the cpu. Signed-off-by: Charlie Jenkins --- arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi b/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi index 64c3c2e6cbe0..50c9f4ec8a7f 100644 --- a/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi +++ b/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi @@ -27,7 +27,8 @@ cpu0: cpu@0 { riscv,isa = "rv64imafdc"; riscv,isa-base = "rv64i"; riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr", - "zifencei", "zihpm"; + "zifencei", "zihpm", "xtheadvector"; + riscv,vlenb = <128>; #cooling-cells = <2>; cpu0_intc: interrupt-controller { -- 2.44.0
[PATCH v6 03/17] riscv: vector: Use vlenb from DT
If vlenb is provided in the device tree, prefer that over reading the vlenb csr. Signed-off-by: Charlie Jenkins --- arch/riscv/include/asm/cpufeature.h | 2 ++ arch/riscv/kernel/cpufeature.c | 47 + arch/riscv/kernel/vector.c | 12 +- 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index 347805446151..0c4f08577015 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -31,6 +31,8 @@ DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo); /* Per-cpu ISA extensions. */ extern struct riscv_isainfo hart_isa[NR_CPUS]; +extern u32 riscv_vlenb_of; + void riscv_user_isa_enable(void); #if defined(CONFIG_RISCV_MISALIGNED) diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 3ed2359eae35..6c143ea9592b 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -35,6 +35,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly; /* Per-cpu ISA extensions. */ struct riscv_isainfo hart_isa[NR_CPUS]; +u32 riscv_vlenb_of; + /** * riscv_isa_extension_base() - Get base extension word * @@ -648,6 +650,46 @@ static int __init riscv_isa_fallback_setup(char *__unused) early_param("riscv_isa_fallback", riscv_isa_fallback_setup); #endif +static int has_riscv_homogeneous_vlenb(void) +{ + int cpu; + u32 prev_vlenb = 0; + u32 vlenb; + + /* Ignore vlenb if vector is not enabled in the kernel */ + if (!IS_ENABLED(CONFIG_RISCV_ISA_V)) + return 0; + + for_each_possible_cpu(cpu) { + struct device_node *cpu_node; + + cpu_node = of_cpu_device_node_get(cpu); + if (!cpu_node) { + pr_warn("Unable to find cpu node\n"); + return -ENOENT; + } + + if (of_property_read_u32(cpu_node, "riscv,vlenb", )) { + of_node_put(cpu_node); + + if (prev_vlenb) + return -ENOENT; + continue; + } + + if (prev_vlenb && vlenb != prev_vlenb) { + of_node_put(cpu_node); + return -ENOENT; + } + + prev_vlenb = vlenb; + of_node_put(cpu_node); + } + + riscv_vlenb_of = vlenb; + return 0; +} + void __init riscv_fill_hwcap(void) { char print_str[NUM_ALPHA_EXTS + 1]; @@ -671,6 +713,11 @@ void __init riscv_fill_hwcap(void) pr_info("Falling back to deprecated \"riscv,isa\"\n"); riscv_fill_hwcap_from_isa_string(isa2hwcap); } + + if (elf_hwcap & COMPAT_HWCAP_ISA_V && has_riscv_homogeneous_vlenb() < 0) { + pr_warn("Unsupported heterogeneous vlen detected, vector extension disabled.\n"); + elf_hwcap &= ~COMPAT_HWCAP_ISA_V; + } } /* diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c index 6727d1d3b8f2..e04586cdb7f0 100644 --- a/arch/riscv/kernel/vector.c +++ b/arch/riscv/kernel/vector.c @@ -33,7 +33,17 @@ int riscv_v_setup_vsize(void) { unsigned long this_vsize; - /* There are 32 vector registers with vlenb length. */ + /* +* There are 32 vector registers with vlenb length. +* +* If the riscv,vlenb property was provided by the firmware, use that +* instead of probing the CSRs. +*/ + if (riscv_vlenb_of) { + this_vsize = riscv_vlenb_of * 32; + return 0; + } + riscv_v_enable(); this_vsize = csr_read(CSR_VLENB) * 32; riscv_v_disable(); -- 2.44.0
[PATCH v6 02/17] dt-bindings: riscv: cpus: add a vlen register length property
From: Conor Dooley Add a property analogous to the vlenb CSR so that software can detect the vector length of each CPU prior to it being brought online. Currently software has to assume that the vector length read from the boot CPU applies to all possible CPUs. On T-Head CPUs implementing pre-ratification vector, reading the th.vlenb CSR may produce an illegal instruction trap, so this property is required on such systems. Signed-off-by: Conor Dooley Signed-off-by: Charlie Jenkins --- Documentation/devicetree/bindings/riscv/cpus.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml index d87dd50f1a4b..edcb6a7d9319 100644 --- a/Documentation/devicetree/bindings/riscv/cpus.yaml +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml @@ -94,6 +94,12 @@ properties: description: The blocksize in bytes for the Zicboz cache operations. + riscv,vlenb: +$ref: /schemas/types.yaml#/definitions/uint32 +description: + VLEN/8, the vector register length in bytes. This property is required in + systems where the vector register length is not identical on all harts. + # RISC-V has multiple properties for cache op block sizes as the sizes # differ between individual CBO extensions cache-op-block-size: false -- 2.44.0
[PATCH v6 01/17] dt-bindings: riscv: Add xtheadvector ISA extension description
The xtheadvector ISA extension is described on the T-Head extension spec Github page [1] at commit 95358cb2cca9. Link: https://github.com/T-head-Semi/thead-extension-spec/blob/95358cb2cca9489361c61d335e03d3134b14133f/xtheadvector.adoc [1] Signed-off-by: Charlie Jenkins Reviewed-by: Conor Dooley --- Documentation/devicetree/bindings/riscv/extensions.yaml | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml index 468c646247aa..99d2a9e8c52d 100644 --- a/Documentation/devicetree/bindings/riscv/extensions.yaml +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml @@ -477,6 +477,10 @@ properties: latency, as ratified in commit 56ed795 ("Update riscv-crypto-spec-vector.adoc") of riscv-crypto. +# vendor extensions, each extension sorted alphanumerically under the +# vendor they belong to. Vendors are sorted alphanumerically as well. + +# Andes - const: xandespmu description: The Andes Technology performance monitor extension for counter overflow @@ -484,5 +488,11 @@ properties: Registers in the AX45MP datasheet. https://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf +# T-HEAD +- const: xtheadvector + description: +The T-HEAD specific 0.7.1 vector implementation as written in + https://github.com/T-head-Semi/thead-extension-spec/blob/95358cb2cca9489361c61d335e03d3134b14133f/xtheadvector.adoc. + additionalProperties: true ... -- 2.44.0
[PATCH v6 00/17] riscv: Support vendor extensions and xtheadvector
This patch series ended up much larger than expected, please bear with me! The goal here is to support vendor extensions, starting at probing the device tree and ending with reporting to userspace. The main design objective was to allow vendors to operate independently of each other. This has been achieved by delegating vendor extensions to a their own files and then accumulating the extensions in arch/riscv/kernel/vendor_extensions.c. Each vendor will have their own list of extensions they support. There is a new hwprobe key RISCV_HWPROBE_KEY_VENDOR_EXT_THEAD_0 that is used to request which thead vendor extensions are supported on the current platform. This allows future vendors to allocate hwprobe keys for their vendor. On to the xtheadvector specific code. xtheadvector is a custom extension that is based upon riscv vector version 0.7.1 [1]. All of the vector routines have been modified to support this alternative vector version based upon whether xtheadvector was determined to be supported at boot. I have tested this with an Allwinner Nezha board. I ran into issues booting the board on 6.9-rc1 so I applied these patches to 6.8. There are a couple of minor merge conflicts that do arrise when doing that, so please let me know if you have been able to boot this board with a 6.9 kernel. I used SkiffOS [2] to manage building the image, but upgraded the U-Boot version to Samuel Holland's more up-to-date version [3] and changed out the device tree used by U-Boot with the device trees that are present in upstream linux and this series. Thank you Samuel for all of the work you did to make this task possible. To test the integration, I used the riscv vector kselftests. I modified the test cases to be able to more easily extend them, and then added a xtheadvector target that works by calling hwprobe and swapping out the vector asm if needed. [1] https://github.com/T-head-Semi/thead-extension-spec/blob/95358cb2cca9489361c61d335e03d3134b14133f/xtheadvector.adoc [2] https://github.com/skiffos/SkiffOS/tree/master/configs/allwinner/nezha [3] https://github.com/smaeul/u-boot/commit/2e89b706f5c956a70c989cd31665f1429e9a0b48 Signed-off-by: Charlie Jenkins --- Changes in v6: - Only check vlenb from of if vector enabled in kernel (Conor) - No need for variadic args in VENDOR_EXTENSION_SUPPORTED so just use a standard argument - Make 'first' variable in riscv_fill_vendor_ext_list() static so that the variable value remains across calls to the function (Evan) - Link to v5: https://lore.kernel.org/r/20240502-dev-charlie-support_thead_vector_6_9-v5-0-d1b5c013a...@rivosinc.com Changes in v5: - Make all vendors have the same size bitmap - Extract vendor hwprobe code into helper macro - Fix bug related to the handling of vendor extensions in the parsing of the isa string (Conor) - Fix bug with the vendor bitmap being incorrectly populated (Evan) - Add vendor extensions to /proc/cpuinfo - Link to v4: https://lore.kernel.org/r/20240426-dev-charlie-support_thead_vector_6_9-v4-0-5cf53b5bc...@rivosinc.com Changes in v4: - Disable vector immediately if vlenb from the device tree is not homogeneous - Hide vendor extension code behind a hidden config that vendor extensions select to eliminate the code when kernel is compiled without vendor extensions - Clear up naming conventions and introduce some defines to make the vendor extension code clearer - Link to v3: https://lore.kernel.org/r/20240420-dev-charlie-support_thead_vector_6_9-v3-0-67cff4271...@rivosinc.com Changes in v3: - Allow any hardware to support any vendor extension, rather than restricting the vendor extensions to the same vendor as the hardware - Introduce config options to enable/disable a vendor's extensions - Link to v2: https://lore.kernel.org/r/20240415-dev-charlie-support_thead_vector_6_9-v2-0-c7d68c603...@rivosinc.com Changes in v2: - Added commit hash to xtheadvector - Simplified riscv,isa vector removal fix to not mess with the DT riscv,vendorid - Moved riscv,vendorid parsing into a different patch and cache the value to be used by alternative patching - Reduce riscv,vendorid missing severity to "info" - Separate vendor extension list to vendor files - xtheadvector no longer puts v in the elf_hwcap - Only patch vendor extension if all harts are associated with the same vendor. This is the best chance the kernel has for working properly if there are multiple vendors. - Split hwprobe vendor keys out into vendor file - Add attribution for Heiko's patches - Link to v1: https://lore.kernel.org/r/20240411-dev-charlie-support_thead_vector_6_9-v1-0-4af9815ec...@rivosinc.com --- Charlie Jenkins (15): dt-bindings: riscv: Add xtheadvector ISA extension description riscv: vector: Use vlenb from DT riscv: dts: allwinner: Add xtheadvector to the D1/D1s devicetree riscv: Extend cpufeature.c to detect vendor extensions riscv: Add vendor extensions to /proc/cpuinfo riscv: Introduce vendor variants of
Re: [PATCH 00/10] mm/damon: misc fixes and improvements
Andrew, please add DAMON selftests patchset[1] that I posted yesterday before this patchset. Otherwise, patches would get conflicts. [1] https://lore.kernel.org/20240502172718.74166-1...@kernel.org Thanks, SJ On Fri, 3 May 2024 11:03:08 -0700 SeongJae Park wrote: > Add miscelleneous and non-urgent fixes and improvements for DAMON code, > selftests, and documents. > > SeongJae Park (10): > mm/damon/core: initialize ->esz_bp from damos_quota_init_priv() > selftests/damon/_damon_sysfs: check errors from nr_schemes file reads > selftests/damon/_damon_sysfs: find sysfs mount point from /proc/mounts > selftests/damon/_damon_sysfs: use 'is' instead of '==' for 'None' > selftests/damon: classify tests for functionalities and regressions > Docs/admin-guide/mm/damon/usage: fix wrong example of DAMOS filter > matching sysfs file > Docs/admin-guide/mm/damon/usage: fix wrong schemes effective quota > update command > Docs/mm/damon/design: use a list for supported filters > Docs/mm/damon/maintainer-profile: change the maintainer's timezone > from PST to PT > Docs/mm/damon/maintainer-profile: allow posting patches based on > damon/next tree > > Documentation/admin-guide/mm/damon/usage.rst | 6 +- > Documentation/mm/damon/design.rst | 46 + > Documentation/mm/damon/maintainer-profile.rst | 13 +-- > mm/damon/core.c | 1 + > tools/testing/selftests/damon/Makefile| 13 ++- > tools/testing/selftests/damon/_damon_sysfs.py | 95 +++ > 6 files changed, 100 insertions(+), 74 deletions(-) > > > base-commit: fc7314cb6b750187a1366e0bf9da4c3ca8cfd064 > -- > 2.39.2
[PATCH 05/10] selftests/damon: classify tests for functionalities and regressions
DAMON selftests can be classified into two categories: functionalities and regressions. Functionality tests are for checking if the function is working as specified, while the regression tests are basically reproducers of previously reported and fixed bugs. The tests of the categories are mixed in the selftests Makefile. Separate those for easier understanding of the types of tests. Signed-off-by: SeongJae Park --- tools/testing/selftests/damon/Makefile | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/damon/Makefile b/tools/testing/selftests/damon/Makefile index 06c248880172..29a22f50e762 100644 --- a/tools/testing/selftests/damon/Makefile +++ b/tools/testing/selftests/damon/Makefile @@ -7,16 +7,21 @@ TEST_GEN_FILES += debugfs_target_ids_pid_leak TEST_GEN_FILES += access_memory TEST_FILES = _chk_dependency.sh _debugfs_common.sh + +# functionality tests TEST_PROGS = debugfs_attrs.sh debugfs_schemes.sh debugfs_target_ids.sh +TEST_PROGS += sysfs.sh +TEST_PROGS += sysfs_update_schemes_tried_regions_wss_estimation.py +TEST_PROGS += damos_quota.py damos_quota_goal.py damos_apply_interval.py +TEST_PROGS += reclaim.sh lru_sort.sh + +# regression tests (reproducers of previously found bugs) TEST_PROGS += debugfs_empty_targets.sh debugfs_huge_count_read_write.sh TEST_PROGS += debugfs_duplicate_context_creation.sh TEST_PROGS += debugfs_rm_non_contexts.sh TEST_PROGS += debugfs_target_ids_read_before_terminate_race.sh TEST_PROGS += debugfs_target_ids_pid_leak.sh -TEST_PROGS += sysfs.sh sysfs_update_removed_scheme_dir.sh +TEST_PROGS += sysfs_update_removed_scheme_dir.sh TEST_PROGS += sysfs_update_schemes_tried_regions_hang.py -TEST_PROGS += sysfs_update_schemes_tried_regions_wss_estimation.py -TEST_PROGS += damos_quota.py damos_quota_goal.py damos_apply_interval.py -TEST_PROGS += reclaim.sh lru_sort.sh include ../lib.mk -- 2.39.2
[PATCH 04/10] selftests/damon/_damon_sysfs: use 'is' instead of '==' for 'None'
_damon_sysfs.py is using '==' or '!=' for 'None'. Since 'None' is a singleton, using 'is' or 'is not' is more efficient. Use the more efficient one. Signed-off-by: SeongJae Park --- tools/testing/selftests/damon/_damon_sysfs.py | 80 +-- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py index 5367e98817a9..01d4b8022d50 100644 --- a/tools/testing/selftests/damon/_damon_sysfs.py +++ b/tools/testing/selftests/damon/_damon_sysfs.py @@ -45,11 +45,11 @@ class DamosAccessPattern: self.nr_accesses = nr_accesses self.age = age -if self.size == None: +if self.size is None: self.size = [0, 2**64 - 1] -if self.nr_accesses == None: +if self.nr_accesses is None: self.nr_accesses = [0, 2**64 - 1] -if self.age == None: +if self.age is None: self.age = [0, 2**64 - 1] def sysfs_dir(self): @@ -58,27 +58,27 @@ class DamosAccessPattern: def stage(self): err = write_file( os.path.join(self.sysfs_dir(), 'sz', 'min'), self.size[0]) -if err != None: +if err is not None: return err err = write_file( os.path.join(self.sysfs_dir(), 'sz', 'max'), self.size[1]) -if err != None: +if err is not None: return err err = write_file(os.path.join(self.sysfs_dir(), 'nr_accesses', 'min'), self.nr_accesses[0]) -if err != None: +if err is not None: return err err = write_file(os.path.join(self.sysfs_dir(), 'nr_accesses', 'max'), self.nr_accesses[1]) -if err != None: +if err is not None: return err err = write_file( os.path.join(self.sysfs_dir(), 'age', 'min'), self.age[0]) -if err != None: +if err is not None: return err err = write_file( os.path.join(self.sysfs_dir(), 'age', 'max'), self.age[1]) -if err != None: +if err is not None: return err qgoal_metric_user_input = 'user_input' @@ -137,14 +137,14 @@ class DamosQuota: def stage(self): err = write_file(os.path.join(self.sysfs_dir(), 'bytes'), self.sz) -if err != None: +if err is not None: return err err = write_file(os.path.join(self.sysfs_dir(), 'ms'), self.ms) -if err != None: +if err is not None: return err err = write_file(os.path.join(self.sysfs_dir(), 'reset_interval_ms'), self.reset_interval_ms) -if err != None: +if err is not None: return err nr_goals_file = os.path.join(self.sysfs_dir(), 'goals', 'nr_goals') @@ -201,30 +201,30 @@ class Damos: def stage(self): err = write_file(os.path.join(self.sysfs_dir(), 'action'), self.action) -if err != None: +if err is not None: return err err = self.access_pattern.stage() -if err != None: +if err is not None: return err err = write_file(os.path.join(self.sysfs_dir(), 'apply_interval_us'), '%d' % self.apply_interval_us) -if err != None: +if err is not None: return err err = self.quota.stage() -if err != None: +if err is not None: return err # disable watermarks err = write_file( os.path.join(self.sysfs_dir(), 'watermarks', 'metric'), 'none') -if err != None: +if err is not None: return err # disable filters err = write_file( os.path.join(self.sysfs_dir(), 'filters', 'nr_filters'), '0') -if err != None: +if err is not None: return err class DamonTarget: @@ -243,7 +243,7 @@ class DamonTarget: def stage(self): err = write_file( os.path.join(self.sysfs_dir(), 'regions', 'nr_regions'), '0') -if err != None: +if err is not None: return err return write_file( os.path.join(self.sysfs_dir(), 'pid_target'), self.pid) @@ -275,27 +275,27 @@ class DamonAttrs: def stage(self): err = write_file(os.path.join(self.interval_sysfs_dir(), 'sample_us'), self.sample_us) -if err != None: +if err is not None: return err err = write_file(os.path.join(self.interval_sysfs_dir(), 'aggr_us'), self.aggr_us) -if err != None: +if err is not None: return err err = write_file(os.path.join(self.interval_sysfs_dir(), 'update_us'), self.update_us) -if err != None: +if err is not None:
[PATCH 03/10] selftests/damon/_damon_sysfs: find sysfs mount point from /proc/mounts
_damon_sysfs.py assumes sysfs is mounted at /sys. In some systems, that might not be true. Find the mount point from /proc/mounts file content. Signed-off-by: SeongJae Park --- tools/testing/selftests/damon/_damon_sysfs.py | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py index fffa74a78bd7..5367e98817a9 100644 --- a/tools/testing/selftests/damon/_damon_sysfs.py +++ b/tools/testing/selftests/damon/_damon_sysfs.py @@ -2,7 +2,18 @@ import os -sysfs_root = '/sys/kernel/mm/damon/admin' +ksft_skip=4 + +sysfs_root = None +with open('/proc/mounts', 'r') as f: +for line in f: +dev_name, mount_point, dev_fs = line.split()[:3] +if dev_fs == 'sysfs': +sysfs_root = '%s/kernel/mm/damon/admin' % mount_point +break +if sysfs_root is None: +print('Seems sysfs not mounted?') +exit(ksft_skip) def write_file(path, string): "Returns error string if failed, or None otherwise" -- 2.39.2
[PATCH 02/10] selftests/damon/_damon_sysfs: check errors from nr_schemes file reads
DAMON context staging method in _damon_sysfs.py is not checking the returned error from nr_schemes file read. Check it. Fixes: f5f0e5a2bef9 ("selftests/damon/_damon_sysfs: implement kdamonds start function") Signed-off-by: SeongJae Park --- tools/testing/selftests/damon/_damon_sysfs.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py index f80fdcef507c..fffa74a78bd7 100644 --- a/tools/testing/selftests/damon/_damon_sysfs.py +++ b/tools/testing/selftests/damon/_damon_sysfs.py @@ -341,6 +341,8 @@ class DamonCtx: nr_schemes_file = os.path.join( self.sysfs_dir(), 'schemes', 'nr_schemes') content, err = read_file(nr_schemes_file) +if err is not None: +return err if int(content) != len(self.schemes): err = write_file(nr_schemes_file, '%d' % len(self.schemes)) if err != None: -- 2.39.2
[PATCH 00/10] mm/damon: misc fixes and improvements
Add miscelleneous and non-urgent fixes and improvements for DAMON code, selftests, and documents. SeongJae Park (10): mm/damon/core: initialize ->esz_bp from damos_quota_init_priv() selftests/damon/_damon_sysfs: check errors from nr_schemes file reads selftests/damon/_damon_sysfs: find sysfs mount point from /proc/mounts selftests/damon/_damon_sysfs: use 'is' instead of '==' for 'None' selftests/damon: classify tests for functionalities and regressions Docs/admin-guide/mm/damon/usage: fix wrong example of DAMOS filter matching sysfs file Docs/admin-guide/mm/damon/usage: fix wrong schemes effective quota update command Docs/mm/damon/design: use a list for supported filters Docs/mm/damon/maintainer-profile: change the maintainer's timezone from PST to PT Docs/mm/damon/maintainer-profile: allow posting patches based on damon/next tree Documentation/admin-guide/mm/damon/usage.rst | 6 +- Documentation/mm/damon/design.rst | 46 + Documentation/mm/damon/maintainer-profile.rst | 13 +-- mm/damon/core.c | 1 + tools/testing/selftests/damon/Makefile| 13 ++- tools/testing/selftests/damon/_damon_sysfs.py | 95 +++ 6 files changed, 100 insertions(+), 74 deletions(-) base-commit: fc7314cb6b750187a1366e0bf9da4c3ca8cfd064 -- 2.39.2
Re: [PATCH 4/4] selftests/cgroup: fix uninitialized variables in test_zswap.c
On Thu, May 02, 2024 at 08:51:05PM -0700, John Hubbard wrote: > First of all, in order to build with clang at all, one must first apply > Valentin Obst's build fix for LLVM [1]. Once that is done, then when > building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...clang finds and warning about some uninitialized variables. Fix these > by initializing them. > > [1] > https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ > > Signed-off-by: John Hubbard Reviewed-by: Roman Gushchin Thanks!
Re: [PATCH 3/4] selftests/cgroup: cpu_hogger init: use {} instead of {NULL}
On Thu, May 02, 2024 at 08:51:04PM -0700, John Hubbard wrote: > First of all, in order to build with clang at all, one must first apply > Valentin Obst's build fix for LLVM [1]. Once that is done, then when > building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...clang generates warning here, because struct cpu_hogger has multiple > fields, and the code is initializing an array of these structs, and it > is incorrect to specify a single NULL value as the initializer. > > Fix this by initializing with {}, so that the compiler knows to use > default initializer values for all fields in each array entry. > > [1] > https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ > > Signed-off-by: John Hubbard Reviewed-by: Roman Gushchin
Re: [PATCH 2/4] selftests/cgroup: fix clang warnings: uninitialized fd variable
On Thu, May 02, 2024 at 08:51:03PM -0700, John Hubbard wrote: > First of all, in order to build with clang at all, one must first apply > Valentin Obst's build fix for LLVM [1]. Once that is done, then when > building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...clang warns about fd being used uninitialized, in > test_memcg_reclaim()'s error handling path. > > Fix this by initializing fd to -1. > > [1] > https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ > > Signed-off-by: John Hubbard Reviewed-by: Roman Gushchin
Re: [PATCH 1/4] selftests/cgroup: fix clang build failures for abs() calls
On Thu, May 02, 2024 at 08:51:02PM -0700, John Hubbard wrote: > First of all, in order to build with clang at all, one must first apply > Valentin Obst's build fix for LLVM [1]. Once that is done, then when > building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...clang is pickier than gcc, about which version of abs(3) to call, > depending on the argument type: > >int abs(int j); >long labs(long j); >long long llabs(long long j); > > ...and this is causing both build failures and warnings, when running: > > make LLVM=1 -C tools/testing/selftests > > Fix this by calling labs() in value_close(), because the arguments are > unambiguously "long" type. > > [1] > https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ > > Signed-off-by: John Hubbard Reviewed-by: Roman Gushchin Thanks!
Re: [PATCH v5 03/17] riscv: vector: Use vlenb from DT
On Fri, May 03, 2024 at 06:26:58PM +0100, Conor Dooley wrote: > On Fri, May 03, 2024 at 10:15:16AM -0700, Charlie Jenkins wrote: > > The DT is improperly > > formatted since it has heterogeneous vlenb entries and has V enabled, > > but since the user disabled V in the kernel skipping the warning is > > reasonable. > > I wouldn't go as far as "improperly formatted", as if the harts really > do have differing vector lengths, it's correctly formatted. It's just > not something we support in Linux. Fair enough, not supported is a better term here. - Charlie
Re: [PATCH v5 05/17] riscv: Extend cpufeature.c to detect vendor extensions
On Fri, May 03, 2024 at 10:13:33AM -0700, Evan Green wrote: > On Fri, May 3, 2024 at 10:08 AM Charlie Jenkins wrote: > > > > On Fri, May 03, 2024 at 09:28:24AM -0700, Evan Green wrote: > > > On Thu, May 2, 2024 at 9:46 PM Charlie Jenkins > > > wrote: > > > > > > > > Separate vendor extensions out into one struct per vendor > > > > instead of adding vendor extensions onto riscv_isa_ext. > > > > > > > > Add a hidden config RISCV_ISA_VENDOR_EXT to conditionally include this > > > > code. > > > > > > > > The xtheadvector vendor extension is added using these changes. > > > > > > > > Signed-off-by: Charlie Jenkins > > > > --- > > > > arch/riscv/Kconfig | 2 + > > > > arch/riscv/Kconfig.vendor| 19 + > > > > arch/riscv/include/asm/cpufeature.h | 18 + > > > > arch/riscv/include/asm/vendor_extensions.h | 34 + > > > > arch/riscv/include/asm/vendor_extensions/thead.h | 16 > > > > arch/riscv/kernel/Makefile | 2 + > > > > arch/riscv/kernel/cpufeature.c | 93 > > > > +++- > > > > arch/riscv/kernel/vendor_extensions.c| 18 + > > > > arch/riscv/kernel/vendor_extensions/Makefile | 3 + > > > > arch/riscv/kernel/vendor_extensions/thead.c | 18 + > > > > 10 files changed, 203 insertions(+), 20 deletions(-) > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > index be09c8836d56..fec86fba3acd 100644 > > > > --- a/arch/riscv/Kconfig > > > > +++ b/arch/riscv/Kconfig > > > > @@ -759,6 +759,8 @@ config RISCV_EFFICIENT_UNALIGNED_ACCESS > > > > > > > > endchoice > > > > > > > > +source "arch/riscv/Kconfig.vendor" > > > > + > > > > endmenu # "Platform type" > > > > > > > > menu "Kernel features" > > > > diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor > > > > new file mode 100644 > > > > index ..85ac30496b0e > > > > --- /dev/null > > > > +++ b/arch/riscv/Kconfig.vendor > > > > @@ -0,0 +1,19 @@ > > > > +menu "Vendor extensions" > > > > + > > > > +config RISCV_ISA_VENDOR_EXT > > > > + bool > > > > + > > > > +menu "T-Head" > > > > +config RISCV_ISA_VENDOR_EXT_THEAD > > > > + bool "T-Head vendor extension support" > > > > + select RISCV_ISA_VENDOR_EXT > > > > + default y > > > > + help > > > > + Say N here to disable detection of and support for all T-Head > > > > vendor > > > > + extensions. Without this option enabled, T-Head vendor > > > > extensions will > > > > + not be detected at boot and their presence not reported to > > > > userspace. > > > > + > > > > + If you don't know what to do here, say Y. > > > > +endmenu > > > > + > > > > +endmenu > > > > diff --git a/arch/riscv/include/asm/cpufeature.h > > > > b/arch/riscv/include/asm/cpufeature.h > > > > index 0c4f08577015..fedd479ccfd1 100644 > > > > --- a/arch/riscv/include/asm/cpufeature.h > > > > +++ b/arch/riscv/include/asm/cpufeature.h > > > > @@ -35,6 +35,24 @@ extern u32 riscv_vlenb_of; > > > > > > > > void riscv_user_isa_enable(void); > > > > > > > > +#define _RISCV_ISA_EXT_DATA(_name, _id, _subset_exts, > > > > _subset_exts_size) { \ > > > > + .name = #_name, > > > > \ > > > > + .property = #_name, > > > > \ > > > > + .id = _id, > > > > \ > > > > + .subset_ext_ids = _subset_exts, > > > > \ > > > > + .subset_ext_size = _subset_exts_size > > > > \ > > > > +} > > > > + > > > > +#define __RISCV_ISA_EXT_DATA(_name, _id) _RISCV_ISA_EXT_DATA(_name, > > > > _id, NULL, 0) > > > > + > > > > +/* Used to declare pure "lasso" extension (Zk for instance) */ > > > > +#define __RISCV_ISA_EXT_BUNDLE(_name, _bundled_exts) \ > > > > + _RISCV_ISA_EXT_DATA(_name, RISCV_ISA_EXT_INVALID, > > > > _bundled_exts, ARRAY_SIZE(_bundled_exts)) > > > > + > > > > +/* Used to declare extensions that are a superset of other extensions > > > > (Zvbb for instance) */ > > > > +#define __RISCV_ISA_EXT_SUPERSET(_name, _id, _sub_exts) \ > > > > + _RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, > > > > ARRAY_SIZE(_sub_exts)) > > > > + > > > > #if defined(CONFIG_RISCV_MISALIGNED) > > > > bool check_unaligned_access_emulated_all_cpus(void); > > > > void unaligned_emulation_finish(void); > > > > diff --git a/arch/riscv/include/asm/vendor_extensions.h > > > > b/arch/riscv/include/asm/vendor_extensions.h > > > > new file mode 100644 > > > > index ..bf4dac66e6e6 > > > > --- /dev/null > > > > +++ b/arch/riscv/include/asm/vendor_extensions.h > > > > @@ -0,0 +1,34 @@ > > > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > > > +/* > > > > + * Copyright 2024 Rivos, Inc > > > > + */ > > > > + > > > >
Re: [PATCH v5 03/17] riscv: vector: Use vlenb from DT
On Fri, May 03, 2024 at 10:15:16AM -0700, Charlie Jenkins wrote: > The DT is improperly > formatted since it has heterogeneous vlenb entries and has V enabled, > but since the user disabled V in the kernel skipping the warning is > reasonable. I wouldn't go as far as "improperly formatted", as if the harts really do have differing vector lengths, it's correctly formatted. It's just not something we support in Linux. signature.asc Description: PGP signature
Re: [PATCH] selftest/timerns: fix clang build failures for abs() calls
On 5/3/24 8:29 AM, John Hubbard wrote: > First of all, in order to build with clang at all, one must first apply > Valentin Obst's build fix for LLVM [1]. Once that is done, then when > building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...then clang warns about mismatches between the expected and required > integer length being supplied to abs(3). > > Fix this by using the correct variant of abs(3): labs(3) or llabs(3), in > these cases. > > [1] > https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ > > Signed-off-by: John Hubbard Thanks for fixing! Reviewed-by: Muhammad Usama Anjum > --- > tools/testing/selftests/timens/exec.c | 6 +++--- > tools/testing/selftests/timens/timer.c | 2 +- > tools/testing/selftests/timens/timerfd.c| 2 +- > tools/testing/selftests/timens/vfork_exec.c | 4 ++-- > 4 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/tools/testing/selftests/timens/exec.c > b/tools/testing/selftests/timens/exec.c > index e40dc5be2f66..d12ff955de0d 100644 > --- a/tools/testing/selftests/timens/exec.c > +++ b/tools/testing/selftests/timens/exec.c > @@ -30,7 +30,7 @@ int main(int argc, char *argv[]) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, , i); > - if (abs(tst.tv_sec - now.tv_sec) > 5) > + if (labs(tst.tv_sec - now.tv_sec) > 5) > return pr_fail("%ld %ld\n", now.tv_sec, > tst.tv_sec); > } > return 0; > @@ -50,7 +50,7 @@ int main(int argc, char *argv[]) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, , i); > - if (abs(tst.tv_sec - now.tv_sec) > 5) > + if (labs(tst.tv_sec - now.tv_sec) > 5) > return pr_fail("%ld %ld\n", > now.tv_sec, tst.tv_sec); > } > @@ -70,7 +70,7 @@ int main(int argc, char *argv[]) > /* Check that a child process is in the new timens. */ > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, , i); > - if (abs(tst.tv_sec - now.tv_sec - OFFSET) > 5) > + if (labs(tst.tv_sec - now.tv_sec - OFFSET) > 5) > return pr_fail("%ld %ld\n", > now.tv_sec + OFFSET, > tst.tv_sec); > } > diff --git a/tools/testing/selftests/timens/timer.c > b/tools/testing/selftests/timens/timer.c > index 5e7f0051bd7b..5b939f59dfa4 100644 > --- a/tools/testing/selftests/timens/timer.c > +++ b/tools/testing/selftests/timens/timer.c > @@ -56,7 +56,7 @@ int run_test(int clockid, struct timespec now) > return pr_perror("timerfd_gettime"); > > elapsed = new_value.it_value.tv_sec; > - if (abs(elapsed - 3600) > 60) { > + if (llabs(elapsed - 3600) > 60) { > ksft_test_result_fail("clockid: %d elapsed: %lld\n", > clockid, elapsed); > return 1; > diff --git a/tools/testing/selftests/timens/timerfd.c > b/tools/testing/selftests/timens/timerfd.c > index 9edd43d6b2c1..a4196bbd6e33 100644 > --- a/tools/testing/selftests/timens/timerfd.c > +++ b/tools/testing/selftests/timens/timerfd.c > @@ -61,7 +61,7 @@ int run_test(int clockid, struct timespec now) > return pr_perror("timerfd_gettime(%d)", clockid); > > elapsed = new_value.it_value.tv_sec; > - if (abs(elapsed - 3600) > 60) { > + if (llabs(elapsed - 3600) > 60) { > ksft_test_result_fail("clockid: %d elapsed: %lld\n", > clockid, elapsed); > return 1; > diff --git a/tools/testing/selftests/timens/vfork_exec.c > b/tools/testing/selftests/timens/vfork_exec.c > index beb7614941fb..5b8907bf451d 100644 > --- a/tools/testing/selftests/timens/vfork_exec.c > +++ b/tools/testing/selftests/timens/vfork_exec.c > @@ -32,7 +32,7 @@ static void *tcheck(void *_args) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, , i); > - if (abs(tst.tv_sec - now->tv_sec) > 5) { > + if (labs(tst.tv_sec - now->tv_sec) > 5) { > pr_fail("%s: in-thread: unexpected value: %ld (%ld)\n", > args->tst_name, tst.tv_sec, now->tv_sec); > return (void *)1UL; > @@ -64,7 +64,7 @@ static int check(char *tst_name, struct timespec *now) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, , i); > - if (abs(tst.tv_sec - now->tv_sec) > 5) > + if (labs(tst.tv_sec - now->tv_sec) > 5) > return pr_fail("%s: unexpected value: %ld (%ld)\n", >
Re: [PATCH v5 03/17] riscv: vector: Use vlenb from DT
On Fri, May 03, 2024 at 05:59:33PM +0100, Conor Dooley wrote: > On Thu, May 02, 2024 at 09:46:38PM -0700, Charlie Jenkins wrote: > > If vlenb is provided in the device tree, prefer that over reading the > > vlenb csr. > > > > Signed-off-by: Charlie Jenkins > > --- > > arch/riscv/include/asm/cpufeature.h | 2 ++ > > arch/riscv/kernel/cpufeature.c | 43 > > + > > arch/riscv/kernel/vector.c | 12 ++- > > 3 files changed, 56 insertions(+), 1 deletion(-) > > > > diff --git a/arch/riscv/include/asm/cpufeature.h > > b/arch/riscv/include/asm/cpufeature.h > > index 347805446151..0c4f08577015 100644 > > --- a/arch/riscv/include/asm/cpufeature.h > > +++ b/arch/riscv/include/asm/cpufeature.h > > @@ -31,6 +31,8 @@ DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo); > > /* Per-cpu ISA extensions. */ > > extern struct riscv_isainfo hart_isa[NR_CPUS]; > > > > +extern u32 riscv_vlenb_of; > > + > > void riscv_user_isa_enable(void); > > > > #if defined(CONFIG_RISCV_MISALIGNED) > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > > index 3ed2359eae35..12c79db0b0bb 100644 > > --- a/arch/riscv/kernel/cpufeature.c > > +++ b/arch/riscv/kernel/cpufeature.c > > @@ -35,6 +35,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) > > __read_mostly; > > /* Per-cpu ISA extensions. */ > > struct riscv_isainfo hart_isa[NR_CPUS]; > > > > +u32 riscv_vlenb_of; > > + > > /** > > * riscv_isa_extension_base() - Get base extension word > > * > > @@ -648,6 +650,42 @@ static int __init riscv_isa_fallback_setup(char > > *__unused) > > early_param("riscv_isa_fallback", riscv_isa_fallback_setup); > > #endif > > > > +static int has_riscv_homogeneous_vlenb(void) > > +{ > > + int cpu; > > + u32 prev_vlenb = 0; > > + u32 vlenb; > > + > > + for_each_possible_cpu(cpu) { > > + struct device_node *cpu_node; > > + > > + cpu_node = of_cpu_device_node_get(cpu); > > + if (!cpu_node) { > > + pr_warn("Unable to find cpu node\n"); > > + return -ENOENT; > > + } > > + > > + if (of_property_read_u32(cpu_node, "riscv,vlenb", )) { > > + of_node_put(cpu_node); > > + > > + if (prev_vlenb) > > + return -ENOENT; > > + continue; > > + } > > + > > + if (prev_vlenb && vlenb != prev_vlenb) { > > + of_node_put(cpu_node); > > + return -ENOENT; > > + } > > + > > + prev_vlenb = vlenb; > > + of_node_put(cpu_node); > > + } > > + > > + riscv_vlenb_of = vlenb; > > + return 0; > > +} > > + > > void __init riscv_fill_hwcap(void) > > { > > char print_str[NUM_ALPHA_EXTS + 1]; > > @@ -671,6 +709,11 @@ void __init riscv_fill_hwcap(void) > > pr_info("Falling back to deprecated \"riscv,isa\"\n"); > > riscv_fill_hwcap_from_isa_string(isa2hwcap); > > } > > + > > + if (elf_hwcap & COMPAT_HWCAP_ISA_V && > > has_riscv_homogeneous_vlenb() < 0) { > > I still think this isn't quite right, as it will emit a warning when > RISCV_ISA_V is disabled. The simplest thing to do probably is just > add an `if (IS_ENABLED(CONFIG_RISCV_ISA_V) return 0` shortcut the to > function? It'll get disabled a few lines later so I think a zero is > safe. That seems like a good idea. It is weird to throw a warning about this even when they have V disabled in the kernel. The DT is improperly formatted since it has heterogeneous vlenb entries and has V enabled, but since the user disabled V in the kernel skipping the warning is reasonable. - Charlie > > > + pr_warn("Unsupported heterogeneous vlen detected, > > vector extension disabled.\n"); > > + elf_hwcap &= ~COMPAT_HWCAP_ISA_V; > > + } > > } > > > > /* > > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > > index 6727d1d3b8f2..e04586cdb7f0 100644 > > --- a/arch/riscv/kernel/vector.c > > +++ b/arch/riscv/kernel/vector.c > > @@ -33,7 +33,17 @@ int riscv_v_setup_vsize(void) > > { > > unsigned long this_vsize; > > > > - /* There are 32 vector registers with vlenb length. */ > > + /* > > +* There are 32 vector registers with vlenb length. > > +* > > +* If the riscv,vlenb property was provided by the firmware, use that > > +* instead of probing the CSRs. > > +*/ > > + if (riscv_vlenb_of) { > > + this_vsize = riscv_vlenb_of * 32; > > + return 0; > > + } > > + > > riscv_v_enable(); > > this_vsize = csr_read(CSR_VLENB) * 32; > > riscv_v_disable(); > > > > -- > > 2.44.0 > >
Re: [PATCH v5 05/17] riscv: Extend cpufeature.c to detect vendor extensions
On Fri, May 3, 2024 at 10:08 AM Charlie Jenkins wrote: > > On Fri, May 03, 2024 at 09:28:24AM -0700, Evan Green wrote: > > On Thu, May 2, 2024 at 9:46 PM Charlie Jenkins wrote: > > > > > > Separate vendor extensions out into one struct per vendor > > > instead of adding vendor extensions onto riscv_isa_ext. > > > > > > Add a hidden config RISCV_ISA_VENDOR_EXT to conditionally include this > > > code. > > > > > > The xtheadvector vendor extension is added using these changes. > > > > > > Signed-off-by: Charlie Jenkins > > > --- > > > arch/riscv/Kconfig | 2 + > > > arch/riscv/Kconfig.vendor| 19 + > > > arch/riscv/include/asm/cpufeature.h | 18 + > > > arch/riscv/include/asm/vendor_extensions.h | 34 + > > > arch/riscv/include/asm/vendor_extensions/thead.h | 16 > > > arch/riscv/kernel/Makefile | 2 + > > > arch/riscv/kernel/cpufeature.c | 93 > > > +++- > > > arch/riscv/kernel/vendor_extensions.c| 18 + > > > arch/riscv/kernel/vendor_extensions/Makefile | 3 + > > > arch/riscv/kernel/vendor_extensions/thead.c | 18 + > > > 10 files changed, 203 insertions(+), 20 deletions(-) > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > index be09c8836d56..fec86fba3acd 100644 > > > --- a/arch/riscv/Kconfig > > > +++ b/arch/riscv/Kconfig > > > @@ -759,6 +759,8 @@ config RISCV_EFFICIENT_UNALIGNED_ACCESS > > > > > > endchoice > > > > > > +source "arch/riscv/Kconfig.vendor" > > > + > > > endmenu # "Platform type" > > > > > > menu "Kernel features" > > > diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor > > > new file mode 100644 > > > index ..85ac30496b0e > > > --- /dev/null > > > +++ b/arch/riscv/Kconfig.vendor > > > @@ -0,0 +1,19 @@ > > > +menu "Vendor extensions" > > > + > > > +config RISCV_ISA_VENDOR_EXT > > > + bool > > > + > > > +menu "T-Head" > > > +config RISCV_ISA_VENDOR_EXT_THEAD > > > + bool "T-Head vendor extension support" > > > + select RISCV_ISA_VENDOR_EXT > > > + default y > > > + help > > > + Say N here to disable detection of and support for all T-Head > > > vendor > > > + extensions. Without this option enabled, T-Head vendor > > > extensions will > > > + not be detected at boot and their presence not reported to > > > userspace. > > > + > > > + If you don't know what to do here, say Y. > > > +endmenu > > > + > > > +endmenu > > > diff --git a/arch/riscv/include/asm/cpufeature.h > > > b/arch/riscv/include/asm/cpufeature.h > > > index 0c4f08577015..fedd479ccfd1 100644 > > > --- a/arch/riscv/include/asm/cpufeature.h > > > +++ b/arch/riscv/include/asm/cpufeature.h > > > @@ -35,6 +35,24 @@ extern u32 riscv_vlenb_of; > > > > > > void riscv_user_isa_enable(void); > > > > > > +#define _RISCV_ISA_EXT_DATA(_name, _id, _subset_exts, _subset_exts_size) > > > { \ > > > + .name = #_name, > > > \ > > > + .property = #_name, > > > \ > > > + .id = _id, > > > \ > > > + .subset_ext_ids = _subset_exts, > > > \ > > > + .subset_ext_size = _subset_exts_size > > > \ > > > +} > > > + > > > +#define __RISCV_ISA_EXT_DATA(_name, _id) _RISCV_ISA_EXT_DATA(_name, _id, > > > NULL, 0) > > > + > > > +/* Used to declare pure "lasso" extension (Zk for instance) */ > > > +#define __RISCV_ISA_EXT_BUNDLE(_name, _bundled_exts) \ > > > + _RISCV_ISA_EXT_DATA(_name, RISCV_ISA_EXT_INVALID, _bundled_exts, > > > ARRAY_SIZE(_bundled_exts)) > > > + > > > +/* Used to declare extensions that are a superset of other extensions > > > (Zvbb for instance) */ > > > +#define __RISCV_ISA_EXT_SUPERSET(_name, _id, _sub_exts) \ > > > + _RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, ARRAY_SIZE(_sub_exts)) > > > + > > > #if defined(CONFIG_RISCV_MISALIGNED) > > > bool check_unaligned_access_emulated_all_cpus(void); > > > void unaligned_emulation_finish(void); > > > diff --git a/arch/riscv/include/asm/vendor_extensions.h > > > b/arch/riscv/include/asm/vendor_extensions.h > > > new file mode 100644 > > > index ..bf4dac66e6e6 > > > --- /dev/null > > > +++ b/arch/riscv/include/asm/vendor_extensions.h > > > @@ -0,0 +1,34 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > > +/* > > > + * Copyright 2024 Rivos, Inc > > > + */ > > > + > > > +#ifndef _ASM_VENDOR_EXTENSIONS_H > > > +#define _ASM_VENDOR_EXTENSIONS_H > > > + > > > +#include > > > + > > > +#include > > > +#include > > > + > > > +/* > > > + * The extension keys of each vendor must be strictly less than this > > > value. > > > + */ > > > +#define RISCV_ISA_VENDOR_EXT_MAX 32 > > > + > > >
Re: [PATCH v5 05/17] riscv: Extend cpufeature.c to detect vendor extensions
On Fri, May 03, 2024 at 09:28:24AM -0700, Evan Green wrote: > On Thu, May 2, 2024 at 9:46 PM Charlie Jenkins wrote: > > > > Separate vendor extensions out into one struct per vendor > > instead of adding vendor extensions onto riscv_isa_ext. > > > > Add a hidden config RISCV_ISA_VENDOR_EXT to conditionally include this > > code. > > > > The xtheadvector vendor extension is added using these changes. > > > > Signed-off-by: Charlie Jenkins > > --- > > arch/riscv/Kconfig | 2 + > > arch/riscv/Kconfig.vendor| 19 + > > arch/riscv/include/asm/cpufeature.h | 18 + > > arch/riscv/include/asm/vendor_extensions.h | 34 + > > arch/riscv/include/asm/vendor_extensions/thead.h | 16 > > arch/riscv/kernel/Makefile | 2 + > > arch/riscv/kernel/cpufeature.c | 93 > > +++- > > arch/riscv/kernel/vendor_extensions.c| 18 + > > arch/riscv/kernel/vendor_extensions/Makefile | 3 + > > arch/riscv/kernel/vendor_extensions/thead.c | 18 + > > 10 files changed, 203 insertions(+), 20 deletions(-) > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index be09c8836d56..fec86fba3acd 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -759,6 +759,8 @@ config RISCV_EFFICIENT_UNALIGNED_ACCESS > > > > endchoice > > > > +source "arch/riscv/Kconfig.vendor" > > + > > endmenu # "Platform type" > > > > menu "Kernel features" > > diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor > > new file mode 100644 > > index ..85ac30496b0e > > --- /dev/null > > +++ b/arch/riscv/Kconfig.vendor > > @@ -0,0 +1,19 @@ > > +menu "Vendor extensions" > > + > > +config RISCV_ISA_VENDOR_EXT > > + bool > > + > > +menu "T-Head" > > +config RISCV_ISA_VENDOR_EXT_THEAD > > + bool "T-Head vendor extension support" > > + select RISCV_ISA_VENDOR_EXT > > + default y > > + help > > + Say N here to disable detection of and support for all T-Head > > vendor > > + extensions. Without this option enabled, T-Head vendor extensions > > will > > + not be detected at boot and their presence not reported to > > userspace. > > + > > + If you don't know what to do here, say Y. > > +endmenu > > + > > +endmenu > > diff --git a/arch/riscv/include/asm/cpufeature.h > > b/arch/riscv/include/asm/cpufeature.h > > index 0c4f08577015..fedd479ccfd1 100644 > > --- a/arch/riscv/include/asm/cpufeature.h > > +++ b/arch/riscv/include/asm/cpufeature.h > > @@ -35,6 +35,24 @@ extern u32 riscv_vlenb_of; > > > > void riscv_user_isa_enable(void); > > > > +#define _RISCV_ISA_EXT_DATA(_name, _id, _subset_exts, _subset_exts_size) { > > \ > > + .name = #_name, > > \ > > + .property = #_name, > > \ > > + .id = _id, > > \ > > + .subset_ext_ids = _subset_exts, > > \ > > + .subset_ext_size = _subset_exts_size > > \ > > +} > > + > > +#define __RISCV_ISA_EXT_DATA(_name, _id) _RISCV_ISA_EXT_DATA(_name, _id, > > NULL, 0) > > + > > +/* Used to declare pure "lasso" extension (Zk for instance) */ > > +#define __RISCV_ISA_EXT_BUNDLE(_name, _bundled_exts) \ > > + _RISCV_ISA_EXT_DATA(_name, RISCV_ISA_EXT_INVALID, _bundled_exts, > > ARRAY_SIZE(_bundled_exts)) > > + > > +/* Used to declare extensions that are a superset of other extensions > > (Zvbb for instance) */ > > +#define __RISCV_ISA_EXT_SUPERSET(_name, _id, _sub_exts) \ > > + _RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, ARRAY_SIZE(_sub_exts)) > > + > > #if defined(CONFIG_RISCV_MISALIGNED) > > bool check_unaligned_access_emulated_all_cpus(void); > > void unaligned_emulation_finish(void); > > diff --git a/arch/riscv/include/asm/vendor_extensions.h > > b/arch/riscv/include/asm/vendor_extensions.h > > new file mode 100644 > > index ..bf4dac66e6e6 > > --- /dev/null > > +++ b/arch/riscv/include/asm/vendor_extensions.h > > @@ -0,0 +1,34 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +/* > > + * Copyright 2024 Rivos, Inc > > + */ > > + > > +#ifndef _ASM_VENDOR_EXTENSIONS_H > > +#define _ASM_VENDOR_EXTENSIONS_H > > + > > +#include > > + > > +#include > > +#include > > + > > +/* > > + * The extension keys of each vendor must be strictly less than this value. > > + */ > > +#define RISCV_ISA_VENDOR_EXT_MAX 32 > > + > > +struct riscv_isavendorinfo { > > + DECLARE_BITMAP(isa, RISCV_ISA_VENDOR_EXT_MAX); > > +}; > > Nice, I think this was a good compromise: being honest with the > compiler about the fixed array sizes, with the tradeoff that all > vendors have to use the same ceiling for the number of bits. If one > vendor raises this ceiling
Re: [PATCH v5 03/17] riscv: vector: Use vlenb from DT
On Thu, May 02, 2024 at 09:46:38PM -0700, Charlie Jenkins wrote: > If vlenb is provided in the device tree, prefer that over reading the > vlenb csr. > > Signed-off-by: Charlie Jenkins > --- > arch/riscv/include/asm/cpufeature.h | 2 ++ > arch/riscv/kernel/cpufeature.c | 43 > + > arch/riscv/kernel/vector.c | 12 ++- > 3 files changed, 56 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/cpufeature.h > b/arch/riscv/include/asm/cpufeature.h > index 347805446151..0c4f08577015 100644 > --- a/arch/riscv/include/asm/cpufeature.h > +++ b/arch/riscv/include/asm/cpufeature.h > @@ -31,6 +31,8 @@ DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo); > /* Per-cpu ISA extensions. */ > extern struct riscv_isainfo hart_isa[NR_CPUS]; > > +extern u32 riscv_vlenb_of; > + > void riscv_user_isa_enable(void); > > #if defined(CONFIG_RISCV_MISALIGNED) > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > index 3ed2359eae35..12c79db0b0bb 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -35,6 +35,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) > __read_mostly; > /* Per-cpu ISA extensions. */ > struct riscv_isainfo hart_isa[NR_CPUS]; > > +u32 riscv_vlenb_of; > + > /** > * riscv_isa_extension_base() - Get base extension word > * > @@ -648,6 +650,42 @@ static int __init riscv_isa_fallback_setup(char > *__unused) > early_param("riscv_isa_fallback", riscv_isa_fallback_setup); > #endif > > +static int has_riscv_homogeneous_vlenb(void) > +{ > + int cpu; > + u32 prev_vlenb = 0; > + u32 vlenb; > + > + for_each_possible_cpu(cpu) { > + struct device_node *cpu_node; > + > + cpu_node = of_cpu_device_node_get(cpu); > + if (!cpu_node) { > + pr_warn("Unable to find cpu node\n"); > + return -ENOENT; > + } > + > + if (of_property_read_u32(cpu_node, "riscv,vlenb", )) { > + of_node_put(cpu_node); > + > + if (prev_vlenb) > + return -ENOENT; > + continue; > + } > + > + if (prev_vlenb && vlenb != prev_vlenb) { > + of_node_put(cpu_node); > + return -ENOENT; > + } > + > + prev_vlenb = vlenb; > + of_node_put(cpu_node); > + } > + > + riscv_vlenb_of = vlenb; > + return 0; > +} > + > void __init riscv_fill_hwcap(void) > { > char print_str[NUM_ALPHA_EXTS + 1]; > @@ -671,6 +709,11 @@ void __init riscv_fill_hwcap(void) > pr_info("Falling back to deprecated \"riscv,isa\"\n"); > riscv_fill_hwcap_from_isa_string(isa2hwcap); > } > + > + if (elf_hwcap & COMPAT_HWCAP_ISA_V && > has_riscv_homogeneous_vlenb() < 0) { I still think this isn't quite right, as it will emit a warning when RISCV_ISA_V is disabled. The simplest thing to do probably is just add an `if (IS_ENABLED(CONFIG_RISCV_ISA_V) return 0` shortcut the to function? It'll get disabled a few lines later so I think a zero is safe. > + pr_warn("Unsupported heterogeneous vlen detected, > vector extension disabled.\n"); > + elf_hwcap &= ~COMPAT_HWCAP_ISA_V; > + } > } > > /* > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > index 6727d1d3b8f2..e04586cdb7f0 100644 > --- a/arch/riscv/kernel/vector.c > +++ b/arch/riscv/kernel/vector.c > @@ -33,7 +33,17 @@ int riscv_v_setup_vsize(void) > { > unsigned long this_vsize; > > - /* There are 32 vector registers with vlenb length. */ > + /* > + * There are 32 vector registers with vlenb length. > + * > + * If the riscv,vlenb property was provided by the firmware, use that > + * instead of probing the CSRs. > + */ > + if (riscv_vlenb_of) { > + this_vsize = riscv_vlenb_of * 32; > + return 0; > + } > + > riscv_v_enable(); > this_vsize = csr_read(CSR_VLENB) * 32; > riscv_v_disable(); > > -- > 2.44.0 > signature.asc Description: PGP signature
Re: [PATCH] selftests/resctrl: fix clang build warnings related to abs(), labs() calls
On 5/3/24 1:00 AM, Ilpo Järvinen wrote: On Thu, 2 May 2024, John Hubbard wrote: ... diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index d67ffa3ec63a..c873793d016d 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -33,7 +33,7 @@ show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, size_t span) avg_bw_imc = sum_bw_imc / 4; avg_bw_resc = sum_bw_resc / 4; - avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc; + avg_diff = (float)(avg_bw_resc - avg_bw_imc) / avg_bw_imc; avg_diff_per = (int)(avg_diff * 100); ret = avg_diff_per > MAX_DIFF_PERCENT; But how are these two cases same after your change when you ended up removing taking the absolute value entirely? All of the arguments are unsigned integers, so all arithmetic results are interpreted as unsigned, so taking the absolute value of that is always a no-op. thanks, -- John Hubbard NVIDIA
Re: [PATCH v5 15/17] riscv: hwprobe: Document thead vendor extensions and xtheadvector extension
On Thu, May 2, 2024 at 9:47 PM Charlie Jenkins wrote: > > Document support for thead vendor extensions using the key > RISCV_HWPROBE_KEY_VENDOR_EXT_THEAD_0 and xtheadvector extension using > the key RISCV_HWPROBE_VENDOR_EXT_XTHEADVECTOR. > > Signed-off-by: Charlie Jenkins Reviewed-by: Evan Green
Re: [PATCH v5 14/17] riscv: hwprobe: Add thead vendor extension probing
On Thu, May 2, 2024 at 9:47 PM Charlie Jenkins wrote: > > Add a new hwprobe key "RISCV_HWPROBE_KEY_VENDOR_EXT_THEAD_0" which > allows userspace to probe for the new RISCV_ISA_VENDOR_EXT_XTHEADVECTOR > vendor extension. > > This new key will allow userspace code to probe for which thead vendor > extensions are supported. This API is modeled to be consistent with > RISCV_HWPROBE_KEY_IMA_EXT_0. The bitmask returned will have each bit > corresponding to a supported thead vendor extension of the cpumask set. > Just like RISCV_HWPROBE_KEY_IMA_EXT_0, this allows a userspace program > to determine all of the supported thead vendor extensions in one call. > > Signed-off-by: Charlie Jenkins Reviewed-by: Evan Green > --- > arch/riscv/include/asm/hwprobe.h | 4 +-- > .../include/asm/vendor_extensions/thead_hwprobe.h | 18 > .../include/asm/vendor_extensions/vendor_hwprobe.h | 34 > ++ > arch/riscv/include/uapi/asm/hwprobe.h | 3 +- > arch/riscv/include/uapi/asm/vendor/thead.h | 3 ++ > arch/riscv/kernel/sys_hwprobe.c| 5 > arch/riscv/kernel/vendor_extensions/Makefile | 1 + > .../riscv/kernel/vendor_extensions/thead_hwprobe.c | 19 > 8 files changed, 84 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/include/asm/hwprobe.h > b/arch/riscv/include/asm/hwprobe.h > index 630507dff5ea..e68496b4f8de 100644 > --- a/arch/riscv/include/asm/hwprobe.h > +++ b/arch/riscv/include/asm/hwprobe.h > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > /* > - * Copyright 2023 Rivos, Inc > + * Copyright 2023-2024 Rivos, Inc > */ > > #ifndef _ASM_HWPROBE_H > @@ -8,7 +8,7 @@ > > #include > > -#define RISCV_HWPROBE_MAX_KEY 6 > +#define RISCV_HWPROBE_MAX_KEY 7 > > static inline bool riscv_hwprobe_key_is_valid(__s64 key) > { > diff --git a/arch/riscv/include/asm/vendor_extensions/thead_hwprobe.h > b/arch/riscv/include/asm/vendor_extensions/thead_hwprobe.h > new file mode 100644 > index ..925fef39a2c0 > --- /dev/null > +++ b/arch/riscv/include/asm/vendor_extensions/thead_hwprobe.h > @@ -0,0 +1,18 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_RISCV_VENDOR_EXTENSIONS_THEAD_HWPROBE_H > +#define _ASM_RISCV_VENDOR_EXTENSIONS_THEAD_HWPROBE_H > + > +#include > + > +#include > + > +#ifdef CONFIG_RISCV_ISA_VENDOR_EXT_THEAD > +void hwprobe_isa_vendor_ext_thead_0(struct riscv_hwprobe *pair, const struct > cpumask *cpus); > +#else > +static inline void hwprobe_isa_vendor_ext_thead_0(struct riscv_hwprobe > *pair, const struct cpumask *cpus) > +{ > + pair->value = 0; > +} > +#endif > + > +#endif > diff --git a/arch/riscv/include/asm/vendor_extensions/vendor_hwprobe.h > b/arch/riscv/include/asm/vendor_extensions/vendor_hwprobe.h > new file mode 100644 > index ..2a29f1a5cae3 > --- /dev/null > +++ b/arch/riscv/include/asm/vendor_extensions/vendor_hwprobe.h > @@ -0,0 +1,34 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright 2024 Rivos, Inc > + */ > + > +#ifndef _ASM_RISCV_SYS_HWPROBE_H > +#define _ASM_RISCV_SYS_HWPROBE_H > + > +#include > + > +#define EXT_KEY(ext) > \ > + do { > \ > + if (__riscv_isa_extension_available(isainfo->isa, > RISCV_ISA_VENDOR_EXT_##ext)) \ > + pair->value |= RISCV_HWPROBE_VENDOR_EXT_##ext; > \ > + else > \ > + missing |= RISCV_HWPROBE_VENDOR_EXT_##ext; > \ > + } while (false) > + > +/* > + * Loop through and record extensions that 1) anyone has, and 2) anyone > + * doesn't have. > + */ > +#define VENDOR_EXTENSION_SUPPORTED(pair, cpus, per_hart_thead_bitmap, ...) > \ > + do { > \ > + int cpu; > \ > + u64 missing; > \ > + for_each_cpu(cpu, (cpus)) { > \ > + struct riscv_isavendorinfo *isainfo = > &(per_hart_thead_bitmap)[cpu];\ > + __VA_ARGS__ > \ > + } > \ > + (pair)->value &= ~missing; > \ > + } while (false) > \ > + > +#endif /* _ASM_RISCV_SYS_HWPROBE_H */ > diff --git
Re: [PATCH v5 05/17] riscv: Extend cpufeature.c to detect vendor extensions
On Thu, May 2, 2024 at 9:46 PM Charlie Jenkins wrote: > > Separate vendor extensions out into one struct per vendor > instead of adding vendor extensions onto riscv_isa_ext. > > Add a hidden config RISCV_ISA_VENDOR_EXT to conditionally include this > code. > > The xtheadvector vendor extension is added using these changes. > > Signed-off-by: Charlie Jenkins > --- > arch/riscv/Kconfig | 2 + > arch/riscv/Kconfig.vendor| 19 + > arch/riscv/include/asm/cpufeature.h | 18 + > arch/riscv/include/asm/vendor_extensions.h | 34 + > arch/riscv/include/asm/vendor_extensions/thead.h | 16 > arch/riscv/kernel/Makefile | 2 + > arch/riscv/kernel/cpufeature.c | 93 > +++- > arch/riscv/kernel/vendor_extensions.c| 18 + > arch/riscv/kernel/vendor_extensions/Makefile | 3 + > arch/riscv/kernel/vendor_extensions/thead.c | 18 + > 10 files changed, 203 insertions(+), 20 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index be09c8836d56..fec86fba3acd 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -759,6 +759,8 @@ config RISCV_EFFICIENT_UNALIGNED_ACCESS > > endchoice > > +source "arch/riscv/Kconfig.vendor" > + > endmenu # "Platform type" > > menu "Kernel features" > diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor > new file mode 100644 > index ..85ac30496b0e > --- /dev/null > +++ b/arch/riscv/Kconfig.vendor > @@ -0,0 +1,19 @@ > +menu "Vendor extensions" > + > +config RISCV_ISA_VENDOR_EXT > + bool > + > +menu "T-Head" > +config RISCV_ISA_VENDOR_EXT_THEAD > + bool "T-Head vendor extension support" > + select RISCV_ISA_VENDOR_EXT > + default y > + help > + Say N here to disable detection of and support for all T-Head vendor > + extensions. Without this option enabled, T-Head vendor extensions > will > + not be detected at boot and their presence not reported to > userspace. > + > + If you don't know what to do here, say Y. > +endmenu > + > +endmenu > diff --git a/arch/riscv/include/asm/cpufeature.h > b/arch/riscv/include/asm/cpufeature.h > index 0c4f08577015..fedd479ccfd1 100644 > --- a/arch/riscv/include/asm/cpufeature.h > +++ b/arch/riscv/include/asm/cpufeature.h > @@ -35,6 +35,24 @@ extern u32 riscv_vlenb_of; > > void riscv_user_isa_enable(void); > > +#define _RISCV_ISA_EXT_DATA(_name, _id, _subset_exts, _subset_exts_size) { > \ > + .name = #_name, > \ > + .property = #_name, > \ > + .id = _id, > \ > + .subset_ext_ids = _subset_exts, > \ > + .subset_ext_size = _subset_exts_size > \ > +} > + > +#define __RISCV_ISA_EXT_DATA(_name, _id) _RISCV_ISA_EXT_DATA(_name, _id, > NULL, 0) > + > +/* Used to declare pure "lasso" extension (Zk for instance) */ > +#define __RISCV_ISA_EXT_BUNDLE(_name, _bundled_exts) \ > + _RISCV_ISA_EXT_DATA(_name, RISCV_ISA_EXT_INVALID, _bundled_exts, > ARRAY_SIZE(_bundled_exts)) > + > +/* Used to declare extensions that are a superset of other extensions (Zvbb > for instance) */ > +#define __RISCV_ISA_EXT_SUPERSET(_name, _id, _sub_exts) \ > + _RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, ARRAY_SIZE(_sub_exts)) > + > #if defined(CONFIG_RISCV_MISALIGNED) > bool check_unaligned_access_emulated_all_cpus(void); > void unaligned_emulation_finish(void); > diff --git a/arch/riscv/include/asm/vendor_extensions.h > b/arch/riscv/include/asm/vendor_extensions.h > new file mode 100644 > index ..bf4dac66e6e6 > --- /dev/null > +++ b/arch/riscv/include/asm/vendor_extensions.h > @@ -0,0 +1,34 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright 2024 Rivos, Inc > + */ > + > +#ifndef _ASM_VENDOR_EXTENSIONS_H > +#define _ASM_VENDOR_EXTENSIONS_H > + > +#include > + > +#include > +#include > + > +/* > + * The extension keys of each vendor must be strictly less than this value. > + */ > +#define RISCV_ISA_VENDOR_EXT_MAX 32 > + > +struct riscv_isavendorinfo { > + DECLARE_BITMAP(isa, RISCV_ISA_VENDOR_EXT_MAX); > +}; Nice, I think this was a good compromise: being honest with the compiler about the fixed array sizes, with the tradeoff that all vendors have to use the same ceiling for the number of bits. If one vendor raises this ceiling absurdly and starts creating huge amounts of waste we can revisit. > + > +struct riscv_isa_vendor_ext_data_list { > + const size_t ext_data_count; > + const struct riscv_isa_ext_data *ext_data; > + struct riscv_isavendorinfo per_hart_isa_bitmap[NR_CPUS]; > + struct riscv_isavendorinfo all_harts_isa_bitmap; > +}; >
Re: [PATCH bpf-next 4/4] selftests/bpf: Add a null pointer check for the serial_test_tp_attach_query
On 5/3/24 5:47 PM, Daniel Borkmann wrote: On 4/24/24 4:04 AM, Kunwu Chan wrote: There is a 'malloc' call, which can be unsuccessful. Add the malloc failure checking to avoid possible null dereference. Signed-off-by: Kunwu Chan --- tools/testing/selftests/bpf/prog_tests/tp_attach_query.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c index 655d69f0ff0b..302b25408a53 100644 --- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c +++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c @@ -39,6 +39,9 @@ void serial_test_tp_attach_query(void) attr.wakeup_events = 1; query = malloc(sizeof(*query) + sizeof(__u32) * num_progs); + if (CHECK(!query, "malloc()", "error:%s\n", strerror(errno))) Series looks reasonable, small nit on CHECK() : Lets use ASSERT*() macros given they are preferred over the latter : if (!ASSERT_OK_PTR(buf, "malloc")) ( Also as a side-note: Fixes tag on all these patches is not needed given this will just end up spamming stable tree. If you indeed end up with NULL then the tests will just segfault & fail. ) + return; + for (i = 0; i < num_progs; i++) { err = bpf_prog_test_load(file, BPF_PROG_TYPE_TRACEPOINT, [i], _fd[i]);
Re: [PATCH bpf-next 4/4] selftests/bpf: Add a null pointer check for the serial_test_tp_attach_query
On 4/24/24 4:04 AM, Kunwu Chan wrote: There is a 'malloc' call, which can be unsuccessful. Add the malloc failure checking to avoid possible null dereference. Signed-off-by: Kunwu Chan --- tools/testing/selftests/bpf/prog_tests/tp_attach_query.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c index 655d69f0ff0b..302b25408a53 100644 --- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c +++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c @@ -39,6 +39,9 @@ void serial_test_tp_attach_query(void) attr.wakeup_events = 1; query = malloc(sizeof(*query) + sizeof(__u32) * num_progs); + if (CHECK(!query, "malloc()", "error:%s\n", strerror(errno))) Series looks reasonable, small nit on CHECK() : Lets use ASSERT*() macros given they are preferred over the latter : if (!ASSERT_OK_PTR(buf, "malloc")) + return; + for (i = 0; i < num_progs; i++) { err = bpf_prog_test_load(file, BPF_PROG_TYPE_TRACEPOINT, [i], _fd[i]);
Re: [PATCH v5 10/10] selftests/harness: Handle TEST_F()'s explicit exit codes
On Fri, May 03, 2024, Mickaël Salaün wrote: > If TEST_F() explicitly calls exit(code) with code different than 0, then > _metadata->exit_code is set to this code (e.g. KVM_ONE_VCPU_TEST()). We > need to keep in mind that _metadata->exit_code can be KSFT_SKIP while > the process exit code is 0. > > Initial patch written by Sean Christopherson [1]. Heh, my pseudo patch barely has any relevance at this point. How about replacing that with: Reported-by: Sean Christopherson Closes: https://lore.kernel.org/r/zjpelw6-abtyv...@google.com > Cc: Jakub Kicinski > Cc: Kees Cook > Cc: Mark Brown > Cc: Sean Christopherson > Cc: Shuah Khan > Cc: Will Drewry > Link: https://lore.kernel.org/r/zjpelw6-abtyv...@google.com [1] > Fixes: 0710a1a73fb4 ("selftests/harness: Merge TEST_F_FORK() into TEST_F()") > Signed-off-by: Mickaël Salaün > Link: https://lore.kernel.org/r/20240503105820.300927-11-...@digikod.net > --- > > Changes since v4: > * Check abort status when the grandchild exited. > * Keep the _exit(0) calls because _metadata->exit_code is always > checked. > * Only set _metadata->exit_code to WEXITSTATUS() if it is not zero. > > Changes since v3: > * New patch mainly from Sean Christopherson. > --- > tools/testing/selftests/kselftest_harness.h | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/kselftest_harness.h > b/tools/testing/selftests/kselftest_harness.h > index eb25f7c11949..7612bf09c5f8 100644 > --- a/tools/testing/selftests/kselftest_harness.h > +++ b/tools/testing/selftests/kselftest_harness.h > @@ -462,9 +462,13 @@ static inline pid_t clone3_vfork(void) > munmap(teardown, sizeof(*teardown)); \ > if (self && fixture_name##_teardown_parent) \ > munmap(self, sizeof(*self)); \ > - if (!WIFEXITED(status) && WIFSIGNALED(status)) \ > + if (WIFEXITED(status)) { \ > + if (WEXITSTATUS(status)) \ > + _metadata->exit_code = WEXITSTATUS(status); \ Ah, IIUC, this works because __run_test() effectively forwards the exit_code? } else if (t->pid == 0) { setpgrp(); t->fn(t, variant); _exit(t->exit_code); } Tested-by: Sean Christopherson > + } else if (WIFSIGNALED(status)) { \ > /* Forward signal to __wait_for_test(). */ \ > kill(getpid(), WTERMSIG(status)); \ > + } \ > __test_check_assert(_metadata); \ > } \ > static void __attribute__((constructor)) \ > -- > 2.45.0 >
Re: [PATCH bpf-next v3] selftests/bpf: Move test_dev_cgroup to prog_tests
On 4/5/24 1:06 AM, Yonghong Song wrote: > > On 4/3/24 5:03 AM, Muhammad Usama Anjum wrote: >> On 4/3/24 7:36 AM, Yonghong Song wrote: >>> On 4/2/24 8:16 AM, Muhammad Usama Anjum wrote: Yonghong Song, Thank you so much for replying. I was missing how to run pipeline manually. Thanks a ton. On 4/1/24 11:53 PM, Yonghong Song wrote: > On 4/1/24 5:34 AM, Muhammad Usama Anjum wrote: >> Move test_dev_cgroup.c to prog_tests/dev_cgroup.c to be able to run it >> with test_progs. Replace dev_cgroup.bpf.o with skel header file, >> dev_cgroup.skel.h and load program from it accourdingly. >> >> ./test_progs -t dev_cgroup >> mknod: /tmp/test_dev_cgroup_null: Operation not permitted >> 64+0 records in >> 64+0 records out >> 32768 bytes (33 kB, 32 KiB) copied, 0.000856684 s, 38.2 MB/s >> dd: failed to open '/dev/full': Operation not permitted >> dd: failed to open '/dev/random': Operation not permitted >> #72 test_dev_cgroup:OK >> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED >> Signed-off-by: Muhammad Usama Anjum >> --- >> Changes since v2: >> - Replace test_dev_cgroup with serial_test_dev_cgroup as there is >> probability that the test is racing against another cgroup test >> - Minor changes to the commit message above >> >> I've tested the patch with vmtest.sh on bpf-next/for-next and linux >> next. It is passing on both. Not sure why it was failed on BPFCI. >> Test run with vmtest.h: >> sudo LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh >> ./test_progs -t dev_cgroup >> ./test_progs -t dev_cgroup >> mknod: /tmp/test_dev_cgroup_null: Operation not permitted >> 64+0 records in >> 64+0 records out >> 32768 bytes (33 kB, 32 KiB) copied, 0.000403432 s, 81.2 MB/s >> dd: failed to open '/dev/full': Operation not permitted >> dd: failed to open '/dev/random': Operation not permitted >> #69 dev_cgroup:OK >> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED > The CI failure: > > > Error: #72 dev_cgroup > serial_test_dev_cgroup:PASS:skel_open_and_load 0 nsec > serial_test_dev_cgroup:PASS:cgroup_setup_and_join 0 nsec > serial_test_dev_cgroup:PASS:bpf_attach 0 nsec > serial_test_dev_cgroup:PASS:bpf_query 0 nsec > serial_test_dev_cgroup:PASS:bpf_query 0 nsec > serial_test_dev_cgroup:PASS:rm 0 nsec > serial_test_dev_cgroup:PASS:mknod 0 nsec > serial_test_dev_cgroup:PASS:rm 0 nsec > serial_test_dev_cgroup:PASS:rm 0 nsec > serial_test_dev_cgroup:FAIL:mknod unexpected mknod: actual 256 != > expected 0 > serial_test_dev_cgroup:PASS:rm 0 nsec > serial_test_dev_cgroup:PASS:dd 0 nsec > serial_test_dev_cgroup:PASS:dd 0 nsec > serial_test_dev_cgroup:PASS:dd 0 nsec > > (cgroup_helpers.c:353: errno: Device or resource busy) umount cgroup2 > > The error code 256 means mknod execution has some issues. Maybe you > need to > find specific errno to find out what is going on. I think you can do ci > on-demanding test to debug. errno is 2 --> No such file or directory Locally I'm unable to reproduce it until I don't remove rm -f /tmp/test_dev_cgroup_zero such that the /tmp/test_dev_cgroup_zero node is present before test execution. The error code is 256 with errno 2. I'm debugging by placing system("ls /tmp 1>&2"); to find out which files are already present in /tmp. But ls's output doesn't appear on the CI logs. >>> errno 2 means ENOENT. >>> From mknod man page (https://linux.die.net/man/2/mknod), it means >>> A directory component in/pathname/ does not exist or is a dangling >>> symbolic link. >>> >>> It means /tmp does not exist or a dangling symbolic link. >>> It is indeed very strange. To make the test robust, maybe creating a temp >>> directory with mkdtemp and use it as the path? The temp directory >>> creation should be done before bpf prog attach. >> I've tried following but still no luck: >> * /tmp is already present. Then I thought maybe the desired file is already >> present. I've verified that there isn't file of same name is present inside >> /tmp. >> * I thought maybe mknod isn't present in the system. But mknod --help >> succeeds. >> * I switched from /tmp to current directory to create the mknod. But the >> result is same error. >> * I've tried to use the same kernel config as the BPF CI is using. I'm not >> able to reproduce it. >> >> Not sure which edge case or what's going on. The problem is appearing >> because of some limitation in the rootfs. > > Maybe you could collect /tmp mount options to see whether anything is > suspicious? In my vm, I have > tmpfs on /tmp type tmpfs (rw,nosuid,nodev,size=3501540k,nr_inodes=1048576) > and the test works fine. > > My test system: tmpfs /tmp tmpfs rw,relatime 0 0 On the CI, /tmp is present. But it isn't
Re: [PATCH] selftest/timerns: fix clang build failures for abs() calls
On Fri, May 3, 2024 at 4:30 AM John Hubbard wrote: > > First of all, in order to build with clang at all, one must first apply > Valentin Obst's build fix for LLVM [1]. Once that is done, then when > building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...then clang warns about mismatches between the expected and required > integer length being supplied to abs(3). > > Fix this by using the correct variant of abs(3): labs(3) or llabs(3), in > these cases. > > [1] > https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/ > > Signed-off-by: John Hubbard LGTM, even potentially fixes the testing post-2038. Reviewed-by: Dmitry Safonov > --- > tools/testing/selftests/timens/exec.c | 6 +++--- > tools/testing/selftests/timens/timer.c | 2 +- > tools/testing/selftests/timens/timerfd.c| 2 +- > tools/testing/selftests/timens/vfork_exec.c | 4 ++-- > 4 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/tools/testing/selftests/timens/exec.c > b/tools/testing/selftests/timens/exec.c > index e40dc5be2f66..d12ff955de0d 100644 > --- a/tools/testing/selftests/timens/exec.c > +++ b/tools/testing/selftests/timens/exec.c > @@ -30,7 +30,7 @@ int main(int argc, char *argv[]) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, , i); > - if (abs(tst.tv_sec - now.tv_sec) > 5) > + if (labs(tst.tv_sec - now.tv_sec) > 5) > return pr_fail("%ld %ld\n", now.tv_sec, > tst.tv_sec); > } > return 0; > @@ -50,7 +50,7 @@ int main(int argc, char *argv[]) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, , i); > - if (abs(tst.tv_sec - now.tv_sec) > 5) > + if (labs(tst.tv_sec - now.tv_sec) > 5) > return pr_fail("%ld %ld\n", > now.tv_sec, tst.tv_sec); > } > @@ -70,7 +70,7 @@ int main(int argc, char *argv[]) > /* Check that a child process is in the new timens. */ > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, , i); > - if (abs(tst.tv_sec - now.tv_sec - OFFSET) > 5) > + if (labs(tst.tv_sec - now.tv_sec - OFFSET) > 5) > return pr_fail("%ld %ld\n", > now.tv_sec + OFFSET, > tst.tv_sec); > } > diff --git a/tools/testing/selftests/timens/timer.c > b/tools/testing/selftests/timens/timer.c > index 5e7f0051bd7b..5b939f59dfa4 100644 > --- a/tools/testing/selftests/timens/timer.c > +++ b/tools/testing/selftests/timens/timer.c > @@ -56,7 +56,7 @@ int run_test(int clockid, struct timespec now) > return pr_perror("timerfd_gettime"); > > elapsed = new_value.it_value.tv_sec; > - if (abs(elapsed - 3600) > 60) { > + if (llabs(elapsed - 3600) > 60) { > ksft_test_result_fail("clockid: %d elapsed: %lld\n", > clockid, elapsed); > return 1; > diff --git a/tools/testing/selftests/timens/timerfd.c > b/tools/testing/selftests/timens/timerfd.c > index 9edd43d6b2c1..a4196bbd6e33 100644 > --- a/tools/testing/selftests/timens/timerfd.c > +++ b/tools/testing/selftests/timens/timerfd.c > @@ -61,7 +61,7 @@ int run_test(int clockid, struct timespec now) > return pr_perror("timerfd_gettime(%d)", clockid); > > elapsed = new_value.it_value.tv_sec; > - if (abs(elapsed - 3600) > 60) { > + if (llabs(elapsed - 3600) > 60) { > ksft_test_result_fail("clockid: %d elapsed: %lld\n", > clockid, elapsed); > return 1; > diff --git a/tools/testing/selftests/timens/vfork_exec.c > b/tools/testing/selftests/timens/vfork_exec.c > index beb7614941fb..5b8907bf451d 100644 > --- a/tools/testing/selftests/timens/vfork_exec.c > +++ b/tools/testing/selftests/timens/vfork_exec.c > @@ -32,7 +32,7 @@ static void *tcheck(void *_args) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, , i); > - if (abs(tst.tv_sec - now->tv_sec) > 5) { > + if (labs(tst.tv_sec - now->tv_sec) > 5) { > pr_fail("%s: in-thread: unexpected value: %ld > (%ld)\n", > args->tst_name, tst.tv_sec, now->tv_sec); > return (void *)1UL; > @@ -64,7 +64,7 @@ static int check(char *tst_name, struct timespec *now) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, , i); > - if (abs(tst.tv_sec - now->tv_sec) > 5) > + if (labs(tst.tv_sec -
Re: [RFC PATCH net-next v8 13/14] net: add devmem TCP documentation
On Tue, Apr 02, 2024 at 05:20:50PM -0700, Mina Almasry wrote: > +ncdevmem has a validation mode as well that expects a repeating pattern of > +incoming data and validates it as such:: > + > + # On server: > + ncdevmem -s -c -f eth1 -d 3 -n :06:00.0 -l \ > + -p 5201 -v 7 > + > + # On client: > + yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | \ > + tr \\n \\0 | head -c 5G | nc 5201 -p 5201 What about splitting server and client usage? >8 diff --git a/Documentation/networking/devmem.rst b/Documentation/networking/devmem.rst index e4e978fbcdbd5f..f32acfd62075d2 100644 --- a/Documentation/networking/devmem.rst +++ b/Documentation/networking/devmem.rst @@ -245,12 +245,14 @@ To run ncdevmem, you need to run it on a server on the machine under test, and you need to run netcat on a peer to provide the TX data. ncdevmem has a validation mode as well that expects a repeating pattern of -incoming data and validates it as such:: +incoming data and validates it as such. For example, you can launch +ncdevmem on the server by:: - # On server: ncdevmem -s -c -f eth1 -d 3 -n :06:00.0 -l \ -p 5201 -v 7 - # On client: +On client side, use regular netcat to send TX data to ncdevmem process +on the server:: + yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | \ tr \\n \\0 | head -c 5G | nc 5201 -p 5201 Thanks. -- An old man doll... just what I always wanted! - Clara signature.asc Description: PGP signature
Re: [PATCH 1/1] selftest: rtc: Add support rtc alarm content check
Hi Alexandre, Thanks for your promptly response, I try to remove all HTML links and resend the email again to avoid the security scanner to disrupt the external link. Hope you can see this email without problems. On 2024/5/3 8:20 PM, Joseph Jang wrote: On 02/05/2024 18:41:02-0700, Joseph Jang wrote: > Some platforms do not support WAKEUP service by default, we use a shell > script to check the absence of alarm content in /proc/driver/rtc. procfs for the RTC has been deprecated for a while, don't use it. Instead, you can use the RTC_PARAM_GET ioctl to get RTC_PARAM_FEATURES and then look at RTC_FEATURE_ALARM. I found old version kernel doesn't support RTC_PARAM_GET ioctl. In order support old version kernel testing, is it possible to use rtc procfs to validate wakealarm function for old version kernel ? Can I move this rtc alarm validation to /tools/testing/selftests/rtc/rtctest.c ? So, we could try to use RTC_PARAM_GET ioctl first and then roll back to use rtc procfs if new RTC_PARAM_GET ioctl was not supported. Thank you, Joseph > > The script will validate /proc/driver/rtc when it is not empty and then > check if could find alarm content in it according to the rtc wakealarm > is supported or not. > > Requires commit 101ca8d05913b ("rtc: efi: Enable SET/GET WAKEUP services > as optional") > > Reviewed-by: Matthew R. Ochs > Signed-off-by: Joseph Jang > --- > tools/testing/selftests/Makefile | 1 + > tools/testing/selftests/rtc/property/Makefile | 5 > .../selftests/rtc/property/rtc-alarm-test.sh | 27 +++ > 3 files changed, 33 insertions(+) > create mode 100644 tools/testing/selftests/rtc/property/Makefile > create mode 100755 tools/testing/selftests/rtc/property/rtc-alarm-test.sh > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index e1504833654d..f5d43e2132e8 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -80,6 +80,7 @@ TARGETS += riscv > TARGETS += rlimits > TARGETS += rseq > TARGETS += rtc > +TARGETS += rtc/property > TARGETS += rust > TARGETS += seccomp > TARGETS += sgx > diff --git a/tools/testing/selftests/rtc/property/Makefile b/tools/testing/selftests/rtc/property/Makefile > new file mode 100644 > index ..c6f7aa4f0e29 > --- /dev/null > +++ b/tools/testing/selftests/rtc/property/Makefile > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: GPL-2.0 > +TEST_PROGS := rtc-alarm-test.sh > + > +include ../../lib.mk > + > diff --git a/tools/testing/selftests/rtc/property/rtc-alarm-test.sh b/tools/testing/selftests/rtc/property/rtc-alarm-test.sh > new file mode 100755 > index ..3bee1dd5fbd0 > --- /dev/null > +++ b/tools/testing/selftests/rtc/property/rtc-alarm-test.sh > @@ -0,0 +1,27 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > + > +if [ ! -f /proc/driver/rtc ]; then > + echo "SKIP: the /proc/driver/rtc is empty." > + exit 4 > +fi > + > +# Check if could find alarm content in /proc/driver/rtc according to > +# the rtc wakealarm is supported or not. > +if [ -n "$(ls /sys/class/rtc/rtc* | grep -i wakealarm)" ]; then > + if [ -n "$(grep -i alarm /proc/driver/rtc)" ]; then > + exit 0 > + else > + echo "ERROR: The alarm content is not found." > + cat /proc/driver/rtc > + exit 1 > + fi > +else > + if [ -n "$(grep -i alarm /proc/driver/rtc)" ]; then > + echo "ERROR: The alarm content is found." > + cat /proc/driver/rtc > + exit 1 > + else > + exit 0 > + fi > +fi > -- > 2.34.1 > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering
Re: [PATCH] kunit: Cover 'assert.c' with tests
On 5/3/24 12:10, Ivan Orlov wrote: On 5/2/24 00:20, Rae Moar wrote: On Sat, Apr 27, 2024 at 6:04 PM Ivan Orlov wrote: There are multiple assertion formatting functions in the `assert.c` file, which are not covered with tests yet. Implement the KUnit test for these functions. The test consists of 11 test cases for the following functions: 1) 'is_literal' 2) 'is_str_literal' 3) 'kunit_assert_prologue', test case for multiple assert types 4) 'kunit_assert_print_msg' 5) 'kunit_unary_assert_format' 6) 'kunit_ptr_not_err_assert_format' 7) 'kunit_binary_assert_format' 8) 'kunit_binary_ptr_assert_format' 9) 'kunit_binary_str_assert_format' 10) 'kunit_assert_hexdump' 11) 'kunit_mem_assert_format' The test aims at maximizing the branch coverage for the assertion formatting functions. As you can see, it covers some of the static helper functions as well, so we have to import the test source in the `assert.c` file in order to be able to call and validate them. Signed-off-by: Ivan Orlov Hello! This is a great patch and addition of KUnit tests. Happy to see it. Thank you very much! I do have a few comments below. But none of them are deal breakers. Hi Rae, Thank you so much for the detailed review. --- lib/kunit/assert.c | 4 + lib/kunit/assert_test.c | 416 2 files changed, 420 insertions(+) create mode 100644 lib/kunit/assert_test.c diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index dd1d633d0fe2..ab68c6daf546 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -270,3 +270,7 @@ void kunit_mem_assert_format(const struct kunit_assert *assert, } } EXPORT_SYMBOL_GPL(kunit_mem_assert_format); + +#if IS_ENABLED(CONFIG_KUNIT_TEST) +#include "assert_test.c" +#endif I might consider using the macro VISIBLE_IF_KUNIT macro, found in include/kunit/visibility.h, to make the static functions in assert.c visible only if KUnit is enabled. To avoid having to add the include here. What do you think? Wow, I haven't seen this macro before, thank you for the suggestion! I'll use it in the V2 of the patch. I assume we need to use it in combination with EXPORT_SYMBOL_IF_KUNIT, otherwise GCC will complain on use of functions without definitions, right? s/definitions/declarations/g :) -- Kind regards, Ivan Orlov
Re: [PATCH 1/1] selftest: rtc: Add support rtc alarm content check
Hi Alexandre, Thanks for your promptly response, I try to re-send the email again and avoid the security scanner to disrupt the external link. > procfs for the RTC has been deprecated for a while, don't use it. > > Instead, you can use the RTC_PARAM_GET ioctl to get RTC_PARAM_FEATURES > and then look at RTC_FEATURE_ALARM. I found old version kernel doesn't support RTC_PARAM_GET ioctl. In order support old version kernel testing, is it possible to use rtc procfs to validate wakealarm function for old version kernel ? Can I move this rtc alarm validation to /tools/testing/selftests/rtc/rtctest.c ? So we could try to use RTC_PARAM_GET ioctl first and then roll back to use rtc procfs if RTC_PARAM_GET ioctl was not supported. Thank you, Joseph. On 2024/5/3 2:49 PM, Alexandre Belloni wrote: On 02/05/2024 18:41:02-0700, Joseph Jang wrote: Some platforms do not support WAKEUP service by default, we use a shell script to check the absence of alarm content in /proc/driver/rtc. procfs for the RTC has been deprecated for a while, don't use it. Instead, you can use the RTC_PARAM_GET ioctl to get RTC_PARAM_FEATURES and then look at RTC_FEATURE_ALARM. See https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/rtc-tools.git/tree/rtc.c The script will validate /proc/driver/rtc when it is not empty and then check if could find alarm content in it according to the rtc wakealarm is supported or not. Requires commit 101ca8d05913b ("rtc: efi: Enable SET/GET WAKEUP services as optional") Reviewed-by: Matthew R. Ochs Signed-off-by: Joseph Jang --- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/rtc/property/Makefile | 5 .../selftests/rtc/property/rtc-alarm-test.sh | 27 +++ 3 files changed, 33 insertions(+) create mode 100644 tools/testing/selftests/rtc/property/Makefile create mode 100755 tools/testing/selftests/rtc/property/rtc-alarm-test.sh diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index e1504833654d..f5d43e2132e8 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -80,6 +80,7 @@ TARGETS += riscv TARGETS += rlimits TARGETS += rseq TARGETS += rtc +TARGETS += rtc/property TARGETS += rust TARGETS += seccomp TARGETS += sgx diff --git a/tools/testing/selftests/rtc/property/Makefile b/tools/testing/selftests/rtc/property/Makefile new file mode 100644 index ..c6f7aa4f0e29 --- /dev/null +++ b/tools/testing/selftests/rtc/property/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 +TEST_PROGS := rtc-alarm-test.sh + +include ../../lib.mk + diff --git a/tools/testing/selftests/rtc/property/rtc-alarm-test.sh b/tools/testing/selftests/rtc/property/rtc-alarm-test.sh new file mode 100755 index ..3bee1dd5fbd0 --- /dev/null +++ b/tools/testing/selftests/rtc/property/rtc-alarm-test.sh @@ -0,0 +1,27 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +if [ ! -f /proc/driver/rtc ]; then + echo "SKIP: the /proc/driver/rtc is empty." + exit 4 +fi + +# Check if could find alarm content in /proc/driver/rtc according to +# the rtc wakealarm is supported or not. +if [ -n "$(ls /sys/class/rtc/rtc* | grep -i wakealarm)" ]; then + if [ -n "$(grep -i alarm /proc/driver/rtc)" ]; then + exit 0 + else + echo "ERROR: The alarm content is not found." + cat /proc/driver/rtc + exit 1 + fi +else + if [ -n "$(grep -i alarm /proc/driver/rtc)" ]; then + echo "ERROR: The alarm content is found." + cat /proc/driver/rtc + exit 1 + else + exit 0 + fi +fi -- 2.34.1
Re: [PATCH] kunit: Cover 'assert.c' with tests
On 5/2/24 00:20, Rae Moar wrote: On Sat, Apr 27, 2024 at 6:04 PM Ivan Orlov wrote: There are multiple assertion formatting functions in the `assert.c` file, which are not covered with tests yet. Implement the KUnit test for these functions. The test consists of 11 test cases for the following functions: 1) 'is_literal' 2) 'is_str_literal' 3) 'kunit_assert_prologue', test case for multiple assert types 4) 'kunit_assert_print_msg' 5) 'kunit_unary_assert_format' 6) 'kunit_ptr_not_err_assert_format' 7) 'kunit_binary_assert_format' 8) 'kunit_binary_ptr_assert_format' 9) 'kunit_binary_str_assert_format' 10) 'kunit_assert_hexdump' 11) 'kunit_mem_assert_format' The test aims at maximizing the branch coverage for the assertion formatting functions. As you can see, it covers some of the static helper functions as well, so we have to import the test source in the `assert.c` file in order to be able to call and validate them. Signed-off-by: Ivan Orlov Hello! This is a great patch and addition of KUnit tests. Happy to see it. Thank you very much! I do have a few comments below. But none of them are deal breakers. Hi Rae, Thank you so much for the detailed review. --- lib/kunit/assert.c | 4 + lib/kunit/assert_test.c | 416 2 files changed, 420 insertions(+) create mode 100644 lib/kunit/assert_test.c diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index dd1d633d0fe2..ab68c6daf546 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -270,3 +270,7 @@ void kunit_mem_assert_format(const struct kunit_assert *assert, } } EXPORT_SYMBOL_GPL(kunit_mem_assert_format); + +#if IS_ENABLED(CONFIG_KUNIT_TEST) +#include "assert_test.c" +#endif I might consider using the macro VISIBLE_IF_KUNIT macro, found in include/kunit/visibility.h, to make the static functions in assert.c visible only if KUnit is enabled. To avoid having to add the include here. What do you think? Wow, I haven't seen this macro before, thank you for the suggestion! I'll use it in the V2 of the patch. I assume we need to use it in combination with EXPORT_SYMBOL_IF_KUNIT, otherwise GCC will complain on use of functions without definitions, right? Also, should the assertion test be in a different file in such a case, or we could merge it with one of the existing test files, for instance `kunit_test.c`? Having these static functions exported would allow us to do that. diff --git a/lib/kunit/assert_test.c b/lib/kunit/assert_test.c new file mode 100644 index ..d54841740761 --- /dev/null +++ b/lib/kunit/assert_test.c @@ -0,0 +1,416 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * KUnit test for the assertion formatting functions. + * Author: Ivan Orlov + */ +#include + +#define TEST_PTR_EXPECTED_BUF_SIZE 128 + +static void kunit_test_is_literal(struct kunit *test) +{ + KUNIT_EXPECT_TRUE(test, is_literal("5", 5)); + KUNIT_EXPECT_TRUE(test, is_literal("0", 0)); + KUNIT_EXPECT_TRUE(test, is_literal("1234567890", 1234567890)); + KUNIT_EXPECT_TRUE(test, is_literal("-1234567890", -1234567890)); + KUNIT_EXPECT_FALSE(test, is_literal("05", 5)); + KUNIT_EXPECT_FALSE(test, is_literal("", 0)); + KUNIT_EXPECT_FALSE(test, is_literal("-0", 0)); + KUNIT_EXPECT_FALSE(test, is_literal("12#45", 1245)); +} + +static void kunit_test_is_str_literal(struct kunit *test) +{ + KUNIT_EXPECT_TRUE(test, is_str_literal("\"Hello, World!\"", "Hello, World!")); + KUNIT_EXPECT_TRUE(test, is_str_literal("\"\"", "")); + KUNIT_EXPECT_TRUE(test, is_str_literal("\"\"\"", "\"")); + KUNIT_EXPECT_FALSE(test, is_str_literal("", "")); + KUNIT_EXPECT_FALSE(test, is_str_literal("\"", "\"")); + KUNIT_EXPECT_FALSE(test, is_str_literal("\"Abacaba", "Abacaba")); + KUNIT_EXPECT_FALSE(test, is_str_literal("Abacaba\"", "Abacaba")); + KUNIT_EXPECT_FALSE(test, is_str_literal("\"Abacaba\"", "\"Abacaba\"")); +} + +KUNIT_DEFINE_ACTION_WRAPPER(kfree_wrapper, kfree, const void *); + +/* this function is used to get a "char *" string from the string stream and defer its cleanup */ +static char *get_str_from_stream(struct kunit *test, struct string_stream *stream) +{ + char *str = string_stream_get_string(stream); + + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, str); + kunit_add_action(test, kfree_wrapper, (void *)str); + + return str; +} + +static void kunit_test_assert_prologue(struct kunit *test) +{ + struct string_stream *stream; + const struct kunit_loc location = { + .file = "testfile.c", + .line = 1337, + }; + + stream = kunit_alloc_string_stream(test, GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream); + + /* Test an expectation fail prologue */ + kunit_assert_prologue(, KUNIT_EXPECTATION, stream); + KUNIT_EXPECT_STREQ(test, get_str_from_stream(test, stream), +
Re: [PATCH v1 1/1] selftest mm/mseal: fix arm build
On 02/05/2024 23:53, jef...@chromium.org wrote: > From: Jeff Xu > > add include linux/mman.h to fix arm build > fix a typo > > Signed-off-by: Jeff Xu > Suggested-by: Ryan Roberts I confirm this has fixed our issue. Thanks! Tested-by: Ryan Roberts Reviewed-by: Ryan Roberts > --- > tools/testing/selftests/mm/mseal_test.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/mm/mseal_test.c > b/tools/testing/selftests/mm/mseal_test.c > index ca8dbee0c612..41998cf1dcf5 100644 > --- a/tools/testing/selftests/mm/mseal_test.c > +++ b/tools/testing/selftests/mm/mseal_test.c > @@ -1,5 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > #define _GNU_SOURCE > +#include > #include > #include > #include > @@ -29,7 +30,7 @@ > # define PKEY_DISABLE_WRITE 0x2 > #endif > > -#ifndef PKEY_BITS_PER_KEY > +#ifndef PKEY_BITS_PER_PKEY > #define PKEY_BITS_PER_PKEY 2 > #endif >
[PATCH v5 07/10] selftests/pidfd: Fix wrong expectation
Replace a wrong EXPECT_GT(self->child_pid_exited, 0) with EXPECT_GE(), which will be actually tested on the parent and child sides with a following commit. Cc: Shuah Khan Reviewed-by: Kees Cook Reviewed-by: Christian Brauner Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240503105820.300927-8-...@digikod.net --- Changes since v1: * Extract change from a bigger patch (suggested by Kees). --- tools/testing/selftests/pidfd/pidfd_setns_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/pidfd/pidfd_setns_test.c b/tools/testing/selftests/pidfd/pidfd_setns_test.c index 6e2f2cd400ca..47746b0c6acd 100644 --- a/tools/testing/selftests/pidfd/pidfd_setns_test.c +++ b/tools/testing/selftests/pidfd/pidfd_setns_test.c @@ -158,7 +158,7 @@ FIXTURE_SETUP(current_nsset) /* Create task that exits right away. */ self->child_pid_exited = create_child(>child_pidfd_exited, CLONE_NEWUSER | CLONE_NEWNET); - EXPECT_GT(self->child_pid_exited, 0); + EXPECT_GE(self->child_pid_exited, 0); if (self->child_pid_exited == 0) _exit(EXIT_SUCCESS); -- 2.45.0
[PATCH v5 10/10] selftests/harness: Handle TEST_F()'s explicit exit codes
If TEST_F() explicitly calls exit(code) with code different than 0, then _metadata->exit_code is set to this code (e.g. KVM_ONE_VCPU_TEST()). We need to keep in mind that _metadata->exit_code can be KSFT_SKIP while the process exit code is 0. Initial patch written by Sean Christopherson [1]. Cc: Jakub Kicinski Cc: Kees Cook Cc: Mark Brown Cc: Sean Christopherson Cc: Shuah Khan Cc: Will Drewry Link: https://lore.kernel.org/r/zjpelw6-abtyv...@google.com [1] Fixes: 0710a1a73fb4 ("selftests/harness: Merge TEST_F_FORK() into TEST_F()") Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240503105820.300927-11-...@digikod.net --- Changes since v4: * Check abort status when the grandchild exited. * Keep the _exit(0) calls because _metadata->exit_code is always checked. * Only set _metadata->exit_code to WEXITSTATUS() if it is not zero. Changes since v3: * New patch mainly from Sean Christopherson. --- tools/testing/selftests/kselftest_harness.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index eb25f7c11949..7612bf09c5f8 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -462,9 +462,13 @@ static inline pid_t clone3_vfork(void) munmap(teardown, sizeof(*teardown)); \ if (self && fixture_name##_teardown_parent) \ munmap(self, sizeof(*self)); \ - if (!WIFEXITED(status) && WIFSIGNALED(status)) \ + if (WIFEXITED(status)) { \ + if (WEXITSTATUS(status)) \ + _metadata->exit_code = WEXITSTATUS(status); \ + } else if (WIFSIGNALED(status)) { \ /* Forward signal to __wait_for_test(). */ \ kill(getpid(), WTERMSIG(status)); \ + } \ __test_check_assert(_metadata); \ } \ static void __attribute__((constructor)) \ -- 2.45.0
[PATCH v5 09/10] selftests/harness: Fix vfork() side effects
Setting the time namespace with CLONE_NEWTIME returns -EUSERS if the calling thread shares memory with another thread (because of the shared vDSO), which is the case when it is created with vfork(). Fix pidfd_setns_test by replacing test harness's vfork() call with a clone3() call with CLONE_VFORK, and an explicit sharing of the _metadata and self objects. Replace _metadata->teardown_parent with a new FIXTURE_TEARDOWN_PARENT() helper that can replace FIXTURE_TEARDOWN(). This is a cleaner approach and it enables to selectively share the fixture data between the child process running tests and the parent process running the fixture teardown. This also avoids updating several tests to not rely on the self object's copy-on-write property (e.g. storing the returned value of a fork() call). Cc: Christian Brauner Cc: David S. Miller Cc: Günther Noack Cc: Jakub Kicinski Cc: Mark Brown Cc: Shuah Khan Cc: Will Drewry Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-lkp/202403291015.1fcfa957-oliver.s...@intel.com Fixes: 0710a1a73fb4 ("selftests/harness: Merge TEST_F_FORK() into TEST_F()") Reviewed-by: Kees Cook Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240503105820.300927-10-...@digikod.net --- Changes since v1: * Split changes (suggested by Kees). * Improve documentation. * Remove the static fixture_name##_teardown_parent initialisation to false (as suggested by checkpatch.pl). --- tools/testing/selftests/kselftest_harness.h | 66 - tools/testing/selftests/landlock/fs_test.c | 16 ++--- 2 files changed, 57 insertions(+), 25 deletions(-) diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index ea78bec5856f..eb25f7c11949 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -294,6 +294,32 @@ static inline pid_t clone3_vfork(void) * A bare "return;" statement may be used to return early. */ #define FIXTURE_TEARDOWN(fixture_name) \ + static const bool fixture_name##_teardown_parent; \ + __FIXTURE_TEARDOWN(fixture_name) + +/** + * FIXTURE_TEARDOWN_PARENT() + * *_metadata* is included so that EXPECT_*, ASSERT_* etc. work correctly. + * + * @fixture_name: fixture name + * + * .. code-block:: c + * + * FIXTURE_TEARDOWN_PARENT(fixture_name) { implementation } + * + * Same as FIXTURE_TEARDOWN() but run this code in a parent process. This + * enables the test process to drop its privileges without impacting the + * related FIXTURE_TEARDOWN_PARENT() (e.g. to remove files from a directory + * where write access was dropped). + * + * To make it possible for the parent process to use *self*, share (MAP_SHARED) + * the fixture data between all forked processes. + */ +#define FIXTURE_TEARDOWN_PARENT(fixture_name) \ + static const bool fixture_name##_teardown_parent = true; \ + __FIXTURE_TEARDOWN(fixture_name) + +#define __FIXTURE_TEARDOWN(fixture_name) \ void fixture_name##_teardown( \ struct __test_metadata __attribute__((unused)) *_metadata, \ FIXTURE_DATA(fixture_name) __attribute__((unused)) *self, \ @@ -368,10 +394,11 @@ static inline pid_t clone3_vfork(void) * Very similar to TEST() except that *self* is the setup instance of fixture's * datatype exposed for use by the implementation. * - * The @test_name code is run in a separate process sharing the same memory - * (i.e. vfork), which means that the test process can update its privileges - * without impacting the related FIXTURE_TEARDOWN() (e.g. to remove files from - * a directory where write access was dropped). + * The _metadata object is shared (MAP_SHARED) with all the potential forked + * processes, which enables them to use EXCEPT_*() and ASSERT_*(). + * + * The *self* object is only shared with the potential forked processes if + * FIXTURE_TEARDOWN_PARENT() is used instead of FIXTURE_TEARDOWN(). */ #define TEST_F(fixture_name, test_name) \ __TEST_F_IMPL(fixture_name, test_name, -1, TEST_TIMEOUT_DEFAULT) @@ -392,39 +419,49 @@ static inline pid_t clone3_vfork(void) struct __fixture_variant_metadata *variant) \ { \ /* fixture data is alloced, setup, and torn down per call. */ \ - FIXTURE_DATA(fixture_name) self; \ + FIXTURE_DATA(fixture_name) self_private, *self = NULL; \ pid_t child = 1; \ int status = 0; \ /* Makes sure there is only one teardown, even when child forks again. */ \ bool *teardown = mmap(NULL, sizeof(*teardown), \ PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0); \ *teardown = false; \ - memset(, 0, sizeof(FIXTURE_DATA(fixture_name))); \ + if (sizeof(*self) > 0) { \ + if (fixture_name##_teardown_parent) { \ + self =
[PATCH v5 03/10] selftests/harness: Fix fixture teardown
Make sure fixture teardowns are run when test cases failed, including when _metadata->teardown_parent is set to true. Make sure only one fixture teardown is run per test case, handling the case where the test child forks. Cc: Jakub Kicinski Cc: Shengyu Li Cc: Shuah Khan Fixes: 72d7cb5c190b ("selftests/harness: Prevent infinite loop due to Assert in FIXTURE_TEARDOWN") Fixes: 0710a1a73fb4 ("selftests/harness: Merge TEST_F_FORK() into TEST_F()") Reviewed-by: Kees Cook Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240503105820.300927-4-...@digikod.net --- tools/testing/selftests/kselftest_harness.h | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index d98702b6955d..55699a762c45 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -382,7 +382,10 @@ FIXTURE_DATA(fixture_name) self; \ pid_t child = 1; \ int status = 0; \ - bool jmp = false; \ + /* Makes sure there is only one teardown, even when child forks again. */ \ + bool *teardown = mmap(NULL, sizeof(*teardown), \ + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0); \ + *teardown = false; \ memset(, 0, sizeof(FIXTURE_DATA(fixture_name))); \ if (setjmp(_metadata->env) == 0) { \ /* Use the same _metadata. */ \ @@ -399,15 +402,16 @@ _metadata->exit_code = KSFT_FAIL; \ } \ } \ - else \ - jmp = true; \ if (child == 0) { \ - if (_metadata->setup_completed && !_metadata->teardown_parent && !jmp) \ + if (_metadata->setup_completed && !_metadata->teardown_parent && \ + __sync_bool_compare_and_swap(teardown, false, true)) \ fixture_name##_teardown(_metadata, , variant->data); \ _exit(0); \ } \ - if (_metadata->setup_completed && _metadata->teardown_parent) \ + if (_metadata->setup_completed && _metadata->teardown_parent && \ + __sync_bool_compare_and_swap(teardown, false, true)) \ fixture_name##_teardown(_metadata, , variant->data); \ + munmap(teardown, sizeof(*teardown)); \ if (!WIFEXITED(status) && WIFSIGNALED(status)) \ /* Forward signal to __wait_for_test(). */ \ kill(getpid(), WTERMSIG(status)); \ -- 2.45.0
[PATCH v5 08/10] selftests/harness: Share _metadata between forked processes
Unconditionally share _metadata between all forked processes, which enables to actually catch errors which were previously ignored. This is required for a following commit replacing vfork() with clone3() and CLONE_VFORK (i.e. not sharing the full memory) . It should also be useful to share _metadata to extend expectations to test process's forks. For instance, this change identified a wrong expectation in pidfd_setns_test. Cc: Jakub Kicinski Cc: Shuah Khan Cc: Will Drewry Reviewed-by: Kees Cook Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240503105820.300927-9-...@digikod.net --- Changes since v4: * Reset _metadata's aborted and setup_completed fields. Changes since v1: * Extract change from a bigger patch (suggested by Kees). --- tools/testing/selftests/kselftest_harness.h | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index 201040207c85..ea78bec5856f 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -430,19 +430,17 @@ static inline pid_t clone3_vfork(void) kill(getpid(), WTERMSIG(status)); \ __test_check_assert(_metadata); \ } \ - static struct __test_metadata \ - _##fixture_name##_##test_name##_object = { \ - .name = #test_name, \ - .fn = _##fixture_name##_##test_name, \ - .fixture = &_##fixture_name##_fixture_object, \ - .termsig = signal, \ - .timeout = tmout, \ - .teardown_parent = false, \ -}; \ static void __attribute__((constructor)) \ _register_##fixture_name##_##test_name(void) \ { \ - __register_test(&_##fixture_name##_##test_name##_object); \ + struct __test_metadata *object = mmap(NULL, sizeof(*object), \ + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0); \ + object->name = #test_name; \ + object->fn = _##fixture_name##_##test_name; \ + object->fixture = &_##fixture_name##_fixture_object; \ + object->termsig = signal; \ + object->timeout = tmout; \ + __register_test(object); \ } \ static void fixture_name##_##test_name( \ struct __test_metadata __attribute__((unused)) *_metadata, \ @@ -1181,6 +1179,9 @@ void __run_test(struct __fixture_metadata *f, /* reset test struct */ t->exit_code = KSFT_PASS; t->trigger = 0; + t->aborted = false; + t->setup_completed = false; + memset(t->env, 0, sizeof(t->env)); memset(t->results->reason, 0, sizeof(t->results->reason)); if (asprintf(_name, "%s%s%s.%s", f->name, -- 2.45.0
[PATCH v5 02/10] selftests/landlock: Fix FS tests when run on a private mount point
According to the test environment, the mount point of the test's working directory may be shared or not, which changes the visibility of the nested "tmp" mount point for the test's parent process calling umount("tmp"). This was spotted while running tests in containers [1], where mount points are private. Cc: Günther Noack Cc: Shuah Khan Link: https://github.com/landlock-lsm/landlock-test-tools/pull/4 [1] Fixes: 41cca0542d7c ("selftests/harness: Fix TEST_F()'s vfork handling") Reviewed-by: Kees Cook Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240503105820.300927-3-...@digikod.net --- Changes since v1: * Update commit description. --- tools/testing/selftests/landlock/fs_test.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 9a6036fbf289..46b9effd53e4 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -293,7 +293,15 @@ static void prepare_layout(struct __test_metadata *const _metadata) static void cleanup_layout(struct __test_metadata *const _metadata) { set_cap(_metadata, CAP_SYS_ADMIN); - EXPECT_EQ(0, umount(TMP_DIR)); + if (umount(TMP_DIR)) { + /* +* According to the test environment, the mount point of the +* current directory may be shared or not, which changes the +* visibility of the nested TMP_DIR mount point for the test's +* parent process doing this cleanup. +*/ + ASSERT_EQ(EINVAL, errno); + } clear_cap(_metadata, CAP_SYS_ADMIN); EXPECT_EQ(0, remove_path(TMP_DIR)); } -- 2.45.0
[PATCH v5 06/10] selftests/harness: Constify fixture variants
FIXTURE_VARIANT_ADD() types are passed as const pointers to FIXTURE_TEARDOWN(). Make that explicit by constifying the variants declarations. Cc: Shuah Khan Cc: Will Drewry Reviewed-by: Kees Cook Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240503105820.300927-7-...@digikod.net --- Changes since v1: * Extract change from a bigger patch (suggested by Kees). --- tools/testing/selftests/kselftest_harness.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index 9d7178a71c2c..201040207c85 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -338,7 +338,7 @@ static inline pid_t clone3_vfork(void) * variant. */ #define FIXTURE_VARIANT_ADD(fixture_name, variant_name) \ - extern FIXTURE_VARIANT(fixture_name) \ + extern const FIXTURE_VARIANT(fixture_name) \ _##fixture_name##_##variant_name##_variant; \ static struct __fixture_variant_metadata \ _##fixture_name##_##variant_name##_object = \ @@ -350,7 +350,7 @@ static inline pid_t clone3_vfork(void) __register_fixture_variant(&_##fixture_name##_fixture_object, \ &_##fixture_name##_##variant_name##_object);\ } \ - FIXTURE_VARIANT(fixture_name) \ + const FIXTURE_VARIANT(fixture_name) \ _##fixture_name##_##variant_name##_variant = /** -- 2.45.0
[PATCH v5 01/10] selftests/pidfd: Fix config for pidfd_setns_test
Required by switch_timens() to open /proc/self/ns/time_for_children. CONFIG_GENERIC_VDSO_TIME_NS is not available on UML, so pidfd_setns_test cannot be run successfully on this architecture. Cc: Shuah Khan Fixes: 2b40c5db73e2 ("selftests/pidfd: add pidfd setns tests") Reviewed-by: Kees Cook Reviewed-by: Christian Brauner Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240503105820.300927-2-...@digikod.net --- tools/testing/selftests/pidfd/config | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/pidfd/config b/tools/testing/selftests/pidfd/config index f6f2965e17af..6133524710f7 100644 --- a/tools/testing/selftests/pidfd/config +++ b/tools/testing/selftests/pidfd/config @@ -3,5 +3,7 @@ CONFIG_IPC_NS=y CONFIG_USER_NS=y CONFIG_PID_NS=y CONFIG_NET_NS=y +CONFIG_TIME_NS=y +CONFIG_GENERIC_VDSO_TIME_NS=y CONFIG_CGROUPS=y CONFIG_CHECKPOINT_RESTORE=y -- 2.45.0
[PATCH v5 05/10] selftests/landlock: Do not allocate memory in fixture data
Do not allocate self->dir_path in the test process because this would not be visible in the FIXTURE_TEARDOWN() process when relying on fork()/clone3() instead of vfork(). This change is required for a following commit removing vfork() call to not break the layout3_fs.* test cases. Cc: Günther Noack Cc: Shuah Khan Reviewed-by: Kees Cook Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240503105820.300927-6-...@digikod.net --- Changes since v1: * Extract change from a bigger patch (suggested by Kees). --- tools/testing/selftests/landlock/fs_test.c | 57 +- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 46b9effd53e4..1e2cffde02b5 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -9,6 +9,7 @@ #define _GNU_SOURCE #include +#include #include #include #include @@ -4624,7 +4625,6 @@ FIXTURE(layout3_fs) { bool has_created_dir; bool has_created_file; - char *dir_path; bool skip_test; }; @@ -4683,11 +4683,24 @@ FIXTURE_VARIANT_ADD(layout3_fs, hostfs) { .cwd_fs_magic = HOSTFS_SUPER_MAGIC, }; +static char *dirname_alloc(const char *path) +{ + char *dup; + + if (!path) + return NULL; + + dup = strdup(path); + if (!dup) + return NULL; + + return dirname(dup); +} + FIXTURE_SETUP(layout3_fs) { struct stat statbuf; - const char *slash; - size_t dir_len; + char *dir_path = dirname_alloc(variant->file_path); if (!supports_filesystem(variant->mnt.type) || !cwd_matches_fs(variant->cwd_fs_magic)) { @@ -4697,25 +4710,15 @@ FIXTURE_SETUP(layout3_fs) _metadata->teardown_parent = true; - slash = strrchr(variant->file_path, '/'); - ASSERT_NE(slash, NULL); - dir_len = (size_t)slash - (size_t)variant->file_path; - ASSERT_LT(0, dir_len); - self->dir_path = malloc(dir_len + 1); - self->dir_path[dir_len] = '\0'; - strncpy(self->dir_path, variant->file_path, dir_len); - prepare_layout_opt(_metadata, >mnt); /* Creates directory when required. */ - if (stat(self->dir_path, )) { + if (stat(dir_path, )) { set_cap(_metadata, CAP_DAC_OVERRIDE); - EXPECT_EQ(0, mkdir(self->dir_path, 0700)) + EXPECT_EQ(0, mkdir(dir_path, 0700)) { TH_LOG("Failed to create directory \"%s\": %s", - self->dir_path, strerror(errno)); - free(self->dir_path); - self->dir_path = NULL; + dir_path, strerror(errno)); } self->has_created_dir = true; clear_cap(_metadata, CAP_DAC_OVERRIDE); @@ -4736,6 +4739,8 @@ FIXTURE_SETUP(layout3_fs) self->has_created_file = true; clear_cap(_metadata, CAP_DAC_OVERRIDE); } + + free(dir_path); } FIXTURE_TEARDOWN(layout3_fs) @@ -4754,16 +4759,17 @@ FIXTURE_TEARDOWN(layout3_fs) } if (self->has_created_dir) { + char *dir_path = dirname_alloc(variant->file_path); + set_cap(_metadata, CAP_DAC_OVERRIDE); /* * Don't check for error because the directory might already * have been removed (cf. release_inode test). */ - rmdir(self->dir_path); + rmdir(dir_path); clear_cap(_metadata, CAP_DAC_OVERRIDE); + free(dir_path); } - free(self->dir_path); - self->dir_path = NULL; cleanup_layout(_metadata); } @@ -4830,7 +4836,10 @@ TEST_F_FORK(layout3_fs, tag_inode_dir_mnt) TEST_F_FORK(layout3_fs, tag_inode_dir_child) { - layer3_fs_tag_inode(_metadata, self, variant, self->dir_path); + char *dir_path = dirname_alloc(variant->file_path); + + layer3_fs_tag_inode(_metadata, self, variant, dir_path); + free(dir_path); } TEST_F_FORK(layout3_fs, tag_inode_file) @@ -4857,9 +4866,13 @@ TEST_F_FORK(layout3_fs, release_inodes) if (self->has_created_file) EXPECT_EQ(0, remove_path(variant->file_path)); - if (self->has_created_dir) + if (self->has_created_dir) { + char *dir_path = dirname_alloc(variant->file_path); + /* Don't check for error because of cgroup specificities. */ - remove_path(self->dir_path); + remove_path(dir_path); + free(dir_path); + } ruleset_fd = create_ruleset(_metadata, LANDLOCK_ACCESS_FS_READ_DIR, layer1); -- 2.45.0
[PATCH v5 04/10] selftests/harness: Fix interleaved scheduling leading to race conditions
Fix a race condition when running several FIXTURE_TEARDOWN() managing the same resource. This fixes a race condition in the Landlock file system tests when creating or unmounting the same directory. Using clone3() with CLONE_VFORK guarantees that the child and grandchild test processes are sequentially scheduled. This is implemented with a new clone3_vfork() helper replacing the fork() call. This avoids triggering this error in __wait_for_test(): Test ended in some other way [127] Cc: Christian Brauner Cc: David S. Miller Cc: Günther Noack Cc: Jakub Kicinski Cc: Mark Brown Cc: Shuah Khan Cc: Will Drewry Fixes: 41cca0542d7c ("selftests/harness: Fix TEST_F()'s vfork handling") Reviewed-by: Kees Cook Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240503105820.300927-5-...@digikod.net --- Changes since v2: * Replace __attribute__((__unused__)) with inline for clone3_vfork() (suggested by Kees and Jakub) --- tools/testing/selftests/kselftest_harness.h | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index 55699a762c45..9d7178a71c2c 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -66,6 +66,8 @@ #include #include #include +#include +#include #include "kselftest.h" @@ -80,6 +82,17 @@ # define TH_LOG_ENABLED 1 #endif +/* Wait for the child process to end but without sharing memory mapping. */ +static inline pid_t clone3_vfork(void) +{ + struct clone_args args = { + .flags = CLONE_VFORK, + .exit_signal = SIGCHLD, + }; + + return syscall(__NR_clone3, , sizeof(args)); +} + /** * TH_LOG() * @@ -1183,7 +1196,7 @@ void __run_test(struct __fixture_metadata *f, fflush(stdout); fflush(stderr); - t->pid = fork(); + t->pid = clone3_vfork(); if (t->pid < 0) { ksft_print_msg("ERROR SPAWNING TEST CHILD\n"); t->exit_code = KSFT_FAIL; -- 2.45.0
[PATCH v5 00/10] Fix Kselftest's vfork() side effects
Hi, This fifth series fixes _metadata reset and fixes the last patch to handle code set with direct calls to _exit(). As reported by Kernel Test Robot [1], some pidfd tests fail. This is due to the use of vfork() which introduced some side effects. Similarly, while making it more generic, a previous commit made some Landlock file system tests flaky, and subject to the host's file system mount configuration. This series fixes all these side effects by replacing vfork() with clone3() and CLONE_VFORK, which is cleaner (no arbitrary shared memory) and makes the Kselftest framework more robust. I tried different approaches and I found this one to be the cleaner and less invasive for current test cases. I successfully ran the following tests (using TEST_F and fork/clone/clone3, and KVM_ONE_VCPU_TEST) with this series: - kvm:fix_hypercall_test - kvm:sync_regs_test - kvm:userspace_msr_exit_test - kvm:vmx_pmu_caps_test - landlock:fs_test - landlock:net_test - landlock:ptrace_test - move_mount_set_group:move_mount_set_group_test - net/af_unix:scm_pidfd - perf_events:remove_on_exec - pidfd:pidfd_getfd_test - pidfd:pidfd_setns_test - seccomp:seccomp_bpf - user_events:abi_test [1] https://lore.kernel.org/oe-lkp/202403291015.1fcfa957-oliver.s...@intel.com Previous versions: v1: https://lore.kernel.org/r/20240426172252.1862930-1-...@digikod.net v2: https://lore.kernel.org/r/20240429130931.2394118-1-...@digikod.net v3: https://lore.kernel.org/r/20240429191911.2552580-1-...@digikod.net v4: https://lore.kernel.org/r/20240502210926.145539-1-...@digikod.net Regards, Mickaël Salaün (10): selftests/pidfd: Fix config for pidfd_setns_test selftests/landlock: Fix FS tests when run on a private mount point selftests/harness: Fix fixture teardown selftests/harness: Fix interleaved scheduling leading to race conditions selftests/landlock: Do not allocate memory in fixture data selftests/harness: Constify fixture variants selftests/pidfd: Fix wrong expectation selftests/harness: Share _metadata between forked processes selftests/harness: Fix vfork() side effects selftests/harness: Handle TEST_F()'s explicit exit codes tools/testing/selftests/kselftest_harness.h | 122 +- tools/testing/selftests/landlock/fs_test.c| 83 +++- tools/testing/selftests/pidfd/config | 2 + .../selftests/pidfd/pidfd_setns_test.c| 2 +- 4 files changed, 143 insertions(+), 66 deletions(-) base-commit: e67572cd2204894179d89bd7b984072f19313b03 -- 2.45.0
Re: [PATCH 1/1] selftest: rtc: Add support rtc alarm content check
On 2024/5/3 2:49 PM, Alexandre Belloni wrote: On 02/05/2024 18:41:02-0700, Joseph Jang wrote: Some platforms do not support WAKEUP service by default, we use a shell script to check the absence of alarm content in /proc/driver/rtc. procfs for the RTC has been deprecated for a while, don't use it. Instead, you can use the RTC_PARAM_GET ioctl to get RTC_PARAM_FEATURES and then look at RTC_FEATURE_ALARM. See https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/rtc-tools.git/tree/rtc.c I found old version kernel doesn't support RTC_PARAM_GET ioctl. In order support old version kernel testing, is it possible to use rtc procfs to validate wakealarm function for old version kernel ? Can I move this rtc alarm validation to /tools/testing/selftests/rtc/rtctest.c ? So we could try to use RTC_PARAM_GET ioctl first and then roll back to use rtc procfs if RTC_PARAM_GET ioctl was not supported. Thank you, Joseph. The script will validate /proc/driver/rtc when it is not empty and then check if could find alarm content in it according to the rtc wakealarm is supported or not. Requires commit 101ca8d05913b ("rtc: efi: Enable SET/GET WAKEUP services as optional") Reviewed-by: Matthew R. Ochs Signed-off-by: Joseph Jang --- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/rtc/property/Makefile | 5 .../selftests/rtc/property/rtc-alarm-test.sh | 27 +++ 3 files changed, 33 insertions(+) create mode 100644 tools/testing/selftests/rtc/property/Makefile create mode 100755 tools/testing/selftests/rtc/property/rtc-alarm-test.sh diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index e1504833654d..f5d43e2132e8 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -80,6 +80,7 @@ TARGETS += riscv TARGETS += rlimits TARGETS += rseq TARGETS += rtc +TARGETS += rtc/property TARGETS += rust TARGETS += seccomp TARGETS += sgx diff --git a/tools/testing/selftests/rtc/property/Makefile b/tools/testing/selftests/rtc/property/Makefile new file mode 100644 index ..c6f7aa4f0e29 --- /dev/null +++ b/tools/testing/selftests/rtc/property/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 +TEST_PROGS := rtc-alarm-test.sh + +include ../../lib.mk + diff --git a/tools/testing/selftests/rtc/property/rtc-alarm-test.sh b/tools/testing/selftests/rtc/property/rtc-alarm-test.sh new file mode 100755 index ..3bee1dd5fbd0 --- /dev/null +++ b/tools/testing/selftests/rtc/property/rtc-alarm-test.sh @@ -0,0 +1,27 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +if [ ! -f /proc/driver/rtc ]; then + echo "SKIP: the /proc/driver/rtc is empty." + exit 4 +fi + +# Check if could find alarm content in /proc/driver/rtc according to +# the rtc wakealarm is supported or not. +if [ -n "$(ls /sys/class/rtc/rtc* | grep -i wakealarm)" ]; then + if [ -n "$(grep -i alarm /proc/driver/rtc)" ]; then + exit 0 + else + echo "ERROR: The alarm content is not found." + cat /proc/driver/rtc + exit 1 + fi +else + if [ -n "$(grep -i alarm /proc/driver/rtc)" ]; then + echo "ERROR: The alarm content is found." + cat /proc/driver/rtc + exit 1 + else + exit 0 + fi +fi -- 2.34.1
Re: [PATCH v2 1/2] selftests/powerpc: Convert pmu Makefile to for loop style
On Mon, 22 Apr 2024 23:34:52 +1000, Michael Ellerman wrote: > The pmu Makefile has grown more sub directories over the years. Rather > than open coding the rules for each subdir, use for loops. > > Applied to powerpc/next. [1/2] selftests/powerpc: Convert pmu Makefile to for loop style https://git.kernel.org/powerpc/c/822a04957cc5e675570645f506270797a1cf2865 [2/2] selftests/powerpc: Install tests in sub-directories https://git.kernel.org/powerpc/c/dda32e37d397f5937cc24a6e98b71d3645f51afa cheers