Re: [PATCH v5 12/13] selftests: riscv: Fix vector tests

On Mon, Jul 22, 2024 at 02:14:44PM GMT, Charlie Jenkins wrote:

On Mon, Jul 22, 2024 at 01:47:29PM -0500, Andrew Jones wrote:

...

On Fri, Jul 19, 2024 at 09:19:07AM GMT, Charlie Jenkins wrote:

...

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 charlie@rivosinc.com

tools/testing/selftests/riscv/abi/ptrace | Bin 0 -> 759368 bytes 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 ++++++++++++--------- 9 files changed, 324 insertions(+), 191 deletions(-)

diff --git a/tools/testing/selftests/riscv/abi/ptrace b/tools/testing/selftests/riscv/abi/ptrace new file mode 100755 index 000000000000..2b03e77b4dcf Binary files /dev/null and b/tools/testing/selftests/riscv/abi/ptrace differ 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 000000000000..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) {							\
  

• 	first = 0;						\
  

• } else if (value != prev_value || !(value == 0x00 || value == 0xff)) { \
  

• 	printf("Register " __stringify(register) " values not clean! value: %u\n", value);	\
  

• 	exit(-1);						\
  

I think we should ensure all tests in tools/testing/selftests/riscv/ use TAP output, exiting with ksft_finished(), or at least exit with 0 for success. For example, vstate_exec_nolibc exits with 2 for success since it exits with the return value of prctl(PR_RISCV_V_GET_CONTROL). And vstate_prctl.c exits with several different negative values, which means it'll exit with several different values around 255. To figure what went wrong, one will have to convert those exit codes to the original negative values in order to look them up. Having these types of inconsistent exit values complicates QA.

Thanks, drew

I do not follow. I am using the kselftest_harness (tools/testing/selftests/kselftest_harness.h) that does output using the TAP format. vstate_exec_nolibc is not a test in itself but is a helper.

Ah, this was my misunderstanding, as I'm used to all files with main() being independent tests and I only grepped exit codes. I'll have to look to see how this all fits together.

The Makefile for the vector tests describes this as:

TEST_GEN_PROGS := v_initval vstate_prctl

If you run the riscv collection of tests with:

$ ./run_kselftest.sh --collection riscv

I'll look later, but I hope those tests can also be run standalone and still get TAP output.

Thanks, drew

You will see that the only vector test cases are v_initval and vstate_prctl, which both output as expected in TAP format.

I do see that I messed up the return type of the is_vector_supported() function, I had meant to change that to a bool from an int since RISCV_HWPROBE_EXT_ZVE32X is greater than 32 bits. I will send a new version.

• Charlie