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@xxxxxxxxxxxx> > > --- > > 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. 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 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