[PATCH 2/2] selftests/fchmodat2: fix clang build failure due to -static-libasan

2024-05-03 Thread John Hubbard
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

2024-05-03 Thread John Hubbard
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

2024-05-03 Thread John Hubbard
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

2024-05-03 Thread John Hubbard
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

2024-05-03 Thread Masami Hiramatsu (Google)
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

2024-05-03 Thread Masami Hiramatsu (Google)
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

2024-05-03 Thread Masami Hiramatsu (Google)
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

2024-05-03 Thread Masami Hiramatsu (Google)
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

2024-05-03 Thread Jakub Kicinski
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)

2024-05-03 Thread kernelci.org bot
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)

2024-05-03 Thread kernelci.org bot
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

2024-05-03 Thread John Hubbard
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

2024-05-03 Thread Joe Damato
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

2024-05-03 Thread Jakub Kicinski
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)

2024-05-03 Thread kernelci.org bot
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

2024-05-03 Thread John Hubbard

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

2024-05-03 Thread Willem de Bruijn
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

2024-05-03 Thread Shengyu Li
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

2024-05-03 Thread Sean Christopherson
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

2024-05-03 Thread Evan Green
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

2024-05-03 Thread Evan Green
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

2024-05-03 Thread Reinette Chatre
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

2024-05-03 Thread Willem de Bruijn
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

2024-05-03 Thread Mina Almasry
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

2024-05-03 Thread Shuah Khan

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

2024-05-03 Thread Nhat Pham
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

2024-05-03 Thread Shuah Khan

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

2024-05-03 Thread Shuah Khan

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

2024-05-03 Thread Shuah Khan

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

2024-05-03 Thread Shuah Khan

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

2024-05-03 Thread John Hubbard

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

2024-05-03 Thread Shuah Khan

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

2024-05-03 Thread Tejun Heo
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

2024-05-03 Thread Shuah Khan

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

2024-05-03 Thread Sean Christopherson
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

2024-05-03 Thread Reinette Chatre



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

2024-05-03 Thread Reinette Chatre



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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread SeongJae Park
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

2024-05-03 Thread SeongJae Park
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'

2024-05-03 Thread SeongJae Park
_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

2024-05-03 Thread SeongJae Park
_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

2024-05-03 Thread SeongJae Park
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

2024-05-03 Thread SeongJae Park
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

2024-05-03 Thread Roman Gushchin
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}

2024-05-03 Thread Roman Gushchin
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

2024-05-03 Thread Roman Gushchin
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

2024-05-03 Thread Roman Gushchin
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Conor Dooley
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

2024-05-03 Thread Muhammad Usama Anjum
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Evan Green
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

2024-05-03 Thread Charlie Jenkins
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

2024-05-03 Thread Conor Dooley
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

2024-05-03 Thread John Hubbard

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

2024-05-03 Thread Evan Green
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

2024-05-03 Thread Evan Green
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

2024-05-03 Thread Evan Green
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

2024-05-03 Thread Daniel Borkmann

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

2024-05-03 Thread Daniel Borkmann

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

2024-05-03 Thread Sean Christopherson
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

2024-05-03 Thread Muhammad Usama Anjum
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

2024-05-03 Thread Dmitry Safonov
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

2024-05-03 Thread Bagas Sanjaya
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

2024-05-03 Thread Joseph Jang

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

2024-05-03 Thread Ivan Orlov

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

2024-05-03 Thread Joseph Jang

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

2024-05-03 Thread Ivan Orlov

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

2024-05-03 Thread Ryan Roberts
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Mickaël Salaün
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

2024-05-03 Thread Joseph Jang




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

2024-05-03 Thread Michael Ellerman
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



  1   2   >