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
