diff mbox series

[v1,2/2] selftests: Test RISC-V Vector's first-use handler

Message ID 20230627015556.12329-3-andy.chiu@sifive.com (mailing list archive)
State Accepted
Commit 5c93c4c72fbc69f0f1cdf43c9402b923314e67c8
Headers show
Series Initialize Vector registers in the first-use trap | expand

Checks

Context Check Description
conchuod/cover_letter success Series has a cover letter
conchuod/tree_selection success Guessed tree name to be for-next at HEAD 488833ccdcac
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 6 and now 6
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/build_rv64_clang_allmodconfig success Errors and warnings before: 8 this patch: 8
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig success Errors and warnings before: 8 this patch: 8
conchuod/build_rv32_defconfig success Build OK
conchuod/dtb_warn_rv64 success Errors and warnings before: 20 this patch: 20
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch warning CHECK: Lines should not end with a '(' WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success No Fixes tag
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Andy Chiu June 27, 2023, 1:55 a.m. UTC 
This add a test to check if the kernel zero-initializes all V registers
after the first-use trap handler returns.

If V registers are not zero-initialized, then the test should fail one
out of several runs:

```
 root@sifive-fpga:~# ./v_initval_nolibc
 # vl = 256
 not ok 1 detect stale values on v-regesters
 0 0 0 0 0 0 0 0   0 0 0 0 0 0 0 0
 0 4c 41 4e 47 3d 43 0   50 41 54 48 3d 2f 75 73
 72 2f 6c 6f 63 61 6c 2f   73 62 69 6e 3a 2f 75 73
 72 2f 6c 6f 63 61 6c 2f   62 69 6e 3a 2f 75 73 72
 ff ff 81 0 0 0 0 0   0 0 0 0 0 0 0 0
```

Otherwise, the test passes without errors each run.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
 .../testing/selftests/riscv/vector/.gitignore |  1 +
 tools/testing/selftests/riscv/vector/Makefile |  6 +-
 .../selftests/riscv/vector/v_initval_nolibc.c | 68 +++++++++++++++++++
 3 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/riscv/vector/v_initval_nolibc.c

Comments

Björn Töpel June 27, 2023, 7:46 a.m. UTC | #1 
Andy Chiu <andy.chiu@sifive.com> writes:

> This add a test to check if the kernel zero-initializes all V registers
> after the first-use trap handler returns.
>
> If V registers are not zero-initialized, then the test should fail one
> out of several runs:
>
> ```
>  root@sifive-fpga:~# ./v_initval_nolibc
>  # vl = 256
>  not ok 1 detect stale values on v-regesters
>  0 0 0 0 0 0 0 0   0 0 0 0 0 0 0 0
>  0 4c 41 4e 47 3d 43 0   50 41 54 48 3d 2f 75 73
>  72 2f 6c 6f 63 61 6c 2f   73 62 69 6e 3a 2f 75 73
>  72 2f 6c 6f 63 61 6c 2f   62 69 6e 3a 2f 75 73 72
>  ff ff 81 0 0 0 0 0   0 0 0 0 0 0 0 0
> ```
>
> Otherwise, the test passes without errors each run.
>
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
>  .../testing/selftests/riscv/vector/.gitignore |  1 +
>  tools/testing/selftests/riscv/vector/Makefile |  6 +-
>  .../selftests/riscv/vector/v_initval_nolibc.c | 68 +++++++++++++++++++
>  3 files changed, 74 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/riscv/vector/v_initval_nolibc.c
>
> diff --git a/tools/testing/selftests/riscv/vector/.gitignore b/tools/testing/selftests/riscv/vector/.gitignore
> index 4f2b4e8a3b08..9ae7964491d5 100644
> --- a/tools/testing/selftests/riscv/vector/.gitignore
> +++ b/tools/testing/selftests/riscv/vector/.gitignore
> @@ -1,2 +1,3 @@
>  vstate_exec_nolibc
>  vstate_prctl
> +v_initval_nolibc
> diff --git a/tools/testing/selftests/riscv/vector/Makefile b/tools/testing/selftests/riscv/vector/Makefile
> index cd6e80bf995d..bfff0ff4f3be 100644
> --- a/tools/testing/selftests/riscv/vector/Makefile
> +++ b/tools/testing/selftests/riscv/vector/Makefile
> @@ -2,7 +2,7 @@
>  # Copyright (C) 2021 ARM Limited
>  # Originally tools/testing/arm64/abi/Makefile
>  
> -TEST_GEN_PROGS := vstate_prctl
> +TEST_GEN_PROGS := vstate_prctl v_initval_nolibc
>  TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc
>  
>  include ../../lib.mk
> @@ -13,3 +13,7 @@ $(OUTPUT)/vstate_prctl: vstate_prctl.c ../hwprobe/sys_hwprobe.S
>  $(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
> +	$(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \
> +		-Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc

Hmm, does this build with clang? (No, bigge on my end, and can be fixed
later.)

> diff --git a/tools/testing/selftests/riscv/vector/v_initval_nolibc.c b/tools/testing/selftests/riscv/vector/v_initval_nolibc.c
> new file mode 100644
> index 000000000000..66764edb0d52
> --- /dev/null
> +++ b/tools/testing/selftests/riscv/vector/v_initval_nolibc.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include "../../kselftest.h"
> +#define MAX_VSIZE	(8192 * 32)
> +
> +void dump(char *ptr, int size)
> +{
> +	int i = 0;
> +
> +	for (i = 0; i < size; i++) {
> +		if (i != 0) {
> +			if (i % 16 == 0)
> +				printf("\n");
> +			else if (i % 8 == 0)
> +				printf("  ");
> +		}
> +		printf("%02x ", ptr[i]);
> +	}
> +	printf("\n");
> +}
> +
> +int main(void)
> +{
> +	int i;
> +	unsigned long vl;
> +	char *datap, *tmp;
> +
> +	datap = malloc(MAX_VSIZE);
> +	if (!datap) {
> +		ksft_test_result_fail("fail to allocate memory for size = %lu\n", MAX_VSIZE);
> +		exit(-1);
> +	}
> +
> +	tmp = datap;
> +	asm volatile (
> +		".option push\n\t"
> +		".option arch, +v\n\t"
> +		"vsetvli	%0, x0, e8, m8, ta, ma\n\t"
> +		"vse8.v		v0, (%2)\n\t"
> +		"add		%1, %2, %0\n\t"
> +		"vse8.v		v8, (%1)\n\t"
> +		"add		%1, %1, %0\n\t"
> +		"vse8.v		v16, (%1)\n\t"
> +		"add		%1, %1, %0\n\t"
> +		"vse8.v		v24, (%1)\n\t"
> +		".option pop\n\t"
> +		: "=&r" (vl), "=r" (tmp) : "r" (datap) : "memory");
> +
> +	ksft_print_msg("vl = %lu\n", vl);
> +
> +	if (datap[0] != 0x00 && datap[0] != 0xff) {
> +		ksft_test_result_fail("v-regesters are not properly initialized\n");

Nit: "v-registers"

> +		dump(datap, vl * 4);
> +		exit(-1);
> +	}
> +
> +	for (i = 1; i < vl * 4; i++) {
> +		if (datap[i] != datap[0]) {
> +			ksft_test_result_fail("detect stale values on v-regesters\n");

Nit (dito): "v-registers", and maybe "detected".


With, or without the changes above,
Reviewed-by: Björn Töpel <bjorn@rivosinc.com>

Björn
Andy Chiu June 27, 2023, 3:39 p.m. UTC | #2 
On Tue, Jun 27, 2023 at 3:46 PM Björn Töpel <bjorn@kernel.org> wrote:
>
> Andy Chiu <andy.chiu@sifive.com> writes:
>
> > This add a test to check if the kernel zero-initializes all V registers
> > after the first-use trap handler returns.
> >
> > If V registers are not zero-initialized, then the test should fail one
> > out of several runs:
> >
> > ```
> >  root@sifive-fpga:~# ./v_initval_nolibc
> >  # vl = 256
> >  not ok 1 detect stale values on v-regesters
> >  0 0 0 0 0 0 0 0   0 0 0 0 0 0 0 0
> >  0 4c 41 4e 47 3d 43 0   50 41 54 48 3d 2f 75 73
> >  72 2f 6c 6f 63 61 6c 2f   73 62 69 6e 3a 2f 75 73
> >  72 2f 6c 6f 63 61 6c 2f   62 69 6e 3a 2f 75 73 72
> >  ff ff 81 0 0 0 0 0   0 0 0 0 0 0 0 0
> > ```
> >
> > Otherwise, the test passes without errors each run.
> >
> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > ---
> >  .../testing/selftests/riscv/vector/.gitignore |  1 +
> >  tools/testing/selftests/riscv/vector/Makefile |  6 +-
> >  .../selftests/riscv/vector/v_initval_nolibc.c | 68 +++++++++++++++++++
> >  3 files changed, 74 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/testing/selftests/riscv/vector/v_initval_nolibc.c
> >
> > diff --git a/tools/testing/selftests/riscv/vector/.gitignore b/tools/testing/selftests/riscv/vector/.gitignore
> > index 4f2b4e8a3b08..9ae7964491d5 100644
> > --- a/tools/testing/selftests/riscv/vector/.gitignore
> > +++ b/tools/testing/selftests/riscv/vector/.gitignore
> > @@ -1,2 +1,3 @@
> >  vstate_exec_nolibc
> >  vstate_prctl
> > +v_initval_nolibc
> > diff --git a/tools/testing/selftests/riscv/vector/Makefile b/tools/testing/selftests/riscv/vector/Makefile
> > index cd6e80bf995d..bfff0ff4f3be 100644
> > --- a/tools/testing/selftests/riscv/vector/Makefile
> > +++ b/tools/testing/selftests/riscv/vector/Makefile
> > @@ -2,7 +2,7 @@
> >  # Copyright (C) 2021 ARM Limited
> >  # Originally tools/testing/arm64/abi/Makefile
> >
> > -TEST_GEN_PROGS := vstate_prctl
> > +TEST_GEN_PROGS := vstate_prctl v_initval_nolibc
> >  TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc
> >
> >  include ../../lib.mk
> > @@ -13,3 +13,7 @@ $(OUTPUT)/vstate_prctl: vstate_prctl.c ../hwprobe/sys_hwprobe.S
> >  $(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
> > +     $(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \
> > +             -Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc
>
> Hmm, does this build with clang? (No, bigge on my end, and can be fixed
> later.)

Oops, I didn't try. I will respin the series if clang runs into
problems. I am not very confident here, but do we need to gate build
these tests if CONFIG_RISCV_ISA_V is not set? If my code fits clang
well then it probably would be some toolchain dependency issues
(guessing the ".option arch" part). Kconfig should be able to resolve
this part.

>
> > diff --git a/tools/testing/selftests/riscv/vector/v_initval_nolibc.c b/tools/testing/selftests/riscv/vector/v_initval_nolibc.c
> > new file mode 100644
> > index 000000000000..66764edb0d52
> > --- /dev/null
> > +++ b/tools/testing/selftests/riscv/vector/v_initval_nolibc.c
> > @@ -0,0 +1,68 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include "../../kselftest.h"
> > +#define MAX_VSIZE    (8192 * 32)
> > +
> > +void dump(char *ptr, int size)
> > +{
> > +     int i = 0;
> > +
> > +     for (i = 0; i < size; i++) {
> > +             if (i != 0) {
> > +                     if (i % 16 == 0)
> > +                             printf("\n");
> > +                     else if (i % 8 == 0)
> > +                             printf("  ");
> > +             }
> > +             printf("%02x ", ptr[i]);
> > +     }
> > +     printf("\n");
> > +}
> > +
> > +int main(void)
> > +{
> > +     int i;
> > +     unsigned long vl;
> > +     char *datap, *tmp;
> > +
> > +     datap = malloc(MAX_VSIZE);
> > +     if (!datap) {
> > +             ksft_test_result_fail("fail to allocate memory for size = %lu\n", MAX_VSIZE);
> > +             exit(-1);
> > +     }
> > +
> > +     tmp = datap;
> > +     asm volatile (
> > +             ".option push\n\t"
> > +             ".option arch, +v\n\t"
> > +             "vsetvli        %0, x0, e8, m8, ta, ma\n\t"
> > +             "vse8.v         v0, (%2)\n\t"
> > +             "add            %1, %2, %0\n\t"
> > +             "vse8.v         v8, (%1)\n\t"
> > +             "add            %1, %1, %0\n\t"
> > +             "vse8.v         v16, (%1)\n\t"
> > +             "add            %1, %1, %0\n\t"
> > +             "vse8.v         v24, (%1)\n\t"
> > +             ".option pop\n\t"
> > +             : "=&r" (vl), "=r" (tmp) : "r" (datap) : "memory");
> > +
> > +     ksft_print_msg("vl = %lu\n", vl);
> > +
> > +     if (datap[0] != 0x00 && datap[0] != 0xff) {
> > +             ksft_test_result_fail("v-regesters are not properly initialized\n");
>
> Nit: "v-registers"
>
> > +             dump(datap, vl * 4);
> > +             exit(-1);
> > +     }
> > +
> > +     for (i = 1; i < vl * 4; i++) {
> > +             if (datap[i] != datap[0]) {
> > +                     ksft_test_result_fail("detect stale values on v-regesters\n");
>
> Nit (dito): "v-registers", and maybe "detected".
>
>
> With, or without the changes above,
> Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
>
> Björn

Thanks,
Andy
diff mbox series

Patch

diff --git a/tools/testing/selftests/riscv/vector/.gitignore b/tools/testing/selftests/riscv/vector/.gitignore
index 4f2b4e8a3b08..9ae7964491d5 100644
--- a/tools/testing/selftests/riscv/vector/.gitignore
+++ b/tools/testing/selftests/riscv/vector/.gitignore
@@ -1,2 +1,3 @@ 
 vstate_exec_nolibc
 vstate_prctl
+v_initval_nolibc
diff --git a/tools/testing/selftests/riscv/vector/Makefile b/tools/testing/selftests/riscv/vector/Makefile
index cd6e80bf995d..bfff0ff4f3be 100644
--- a/tools/testing/selftests/riscv/vector/Makefile
+++ b/tools/testing/selftests/riscv/vector/Makefile
@@ -2,7 +2,7 @@ 
 # Copyright (C) 2021 ARM Limited
 # Originally tools/testing/arm64/abi/Makefile
 
-TEST_GEN_PROGS := vstate_prctl
+TEST_GEN_PROGS := vstate_prctl v_initval_nolibc
 TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc
 
 include ../../lib.mk
@@ -13,3 +13,7 @@  $(OUTPUT)/vstate_prctl: vstate_prctl.c ../hwprobe/sys_hwprobe.S
 $(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
+	$(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \
+		-Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc
diff --git a/tools/testing/selftests/riscv/vector/v_initval_nolibc.c b/tools/testing/selftests/riscv/vector/v_initval_nolibc.c
new file mode 100644
index 000000000000..66764edb0d52
--- /dev/null
+++ b/tools/testing/selftests/riscv/vector/v_initval_nolibc.c
@@ -0,0 +1,68 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "../../kselftest.h"
+#define MAX_VSIZE	(8192 * 32)
+
+void dump(char *ptr, int size)
+{
+	int i = 0;
+
+	for (i = 0; i < size; i++) {
+		if (i != 0) {
+			if (i % 16 == 0)
+				printf("\n");
+			else if (i % 8 == 0)
+				printf("  ");
+		}
+		printf("%02x ", ptr[i]);
+	}
+	printf("\n");
+}
+
+int main(void)
+{
+	int i;
+	unsigned long vl;
+	char *datap, *tmp;
+
+	datap = malloc(MAX_VSIZE);
+	if (!datap) {
+		ksft_test_result_fail("fail to allocate memory for size = %lu\n", MAX_VSIZE);
+		exit(-1);
+	}
+
+	tmp = datap;
+	asm volatile (
+		".option push\n\t"
+		".option arch, +v\n\t"
+		"vsetvli	%0, x0, e8, m8, ta, ma\n\t"
+		"vse8.v		v0, (%2)\n\t"
+		"add		%1, %2, %0\n\t"
+		"vse8.v		v8, (%1)\n\t"
+		"add		%1, %1, %0\n\t"
+		"vse8.v		v16, (%1)\n\t"
+		"add		%1, %1, %0\n\t"
+		"vse8.v		v24, (%1)\n\t"
+		".option pop\n\t"
+		: "=&r" (vl), "=r" (tmp) : "r" (datap) : "memory");
+
+	ksft_print_msg("vl = %lu\n", vl);
+
+	if (datap[0] != 0x00 && datap[0] != 0xff) {
+		ksft_test_result_fail("v-regesters are not properly initialized\n");
+		dump(datap, vl * 4);
+		exit(-1);
+	}
+
+	for (i = 1; i < vl * 4; i++) {
+		if (datap[i] != datap[0]) {
+			ksft_test_result_fail("detect stale values on v-regesters\n");
+			dump(datap, vl * 4);
+			exit(-2);
+		}
+	}
+
+	free(datap);
+	ksft_exit_pass();
+	return 0;
+}