On Mon, Jul 22, 2024 at 05:08:22PM -0500, Andrew Jones wrote: > 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 <[email protected]> > > > > --- > > > > 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
Yes, running v_initval and vstate_prctl standalone does provide TAP output. - Charlie > > > > > 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 > >
