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