From: Charlie Jenkins <charlie@rivosinc.com>
To: Andrew Jones <ajones@ventanamicro.com>
Cc: Conor Dooley <conor@kernel.org>, Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Jisheng Zhang <jszhang@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
Jernej Skrabec <jernej.skrabec@gmail.com>,
Samuel Holland <samuel@sholland.org>,
Samuel Holland <samuel.holland@sifive.com>,
Jonathan Corbet <corbet@lwn.net>, Shuah Khan <shuah@kernel.org>,
Guo Ren <guoren@kernel.org>, Evan Green <evan@rivosinc.com>,
Andy Chiu <andy.chiu@sifive.com>,
Jessica Clarke <jrtc27@jrtc27.com>,
linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev,
linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v5 12/13] selftests: riscv: Fix vector tests
Date: Mon, 22 Jul 2024 15:14:24 -0700 [thread overview]
Message-ID: <Zp7ZwDzpFt7iJ6iT@ghost> (raw)
In-Reply-To: <20240722-86a2b1fa474d71c4dbc557c7@orel>
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 <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
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
> >
next prev parent reply other threads:[~2024-07-22 22:14 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-19 16:18 [PATCH v5 00/13] riscv: Add support for xtheadvector Charlie Jenkins
2024-07-19 16:18 ` [PATCH v5 01/13] dt-bindings: riscv: Add xtheadvector ISA extension description Charlie Jenkins
2024-07-19 16:18 ` [PATCH v5 02/13] dt-bindings: cpus: add a thead vlen register length property Charlie Jenkins
2024-07-19 16:18 ` [PATCH v5 03/13] riscv: dts: allwinner: Add xtheadvector to the D1/D1s devicetree Charlie Jenkins
2024-07-19 16:18 ` [PATCH v5 04/13] riscv: Add thead and xtheadvector as a vendor extension Charlie Jenkins
2024-07-19 16:19 ` [PATCH v5 05/13] riscv: vector: Use vlenb from DT for thead Charlie Jenkins
2024-07-19 16:19 ` [PATCH v5 06/13] RISC-V: define the elements of the VCSR vector CSR Charlie Jenkins
2024-07-19 16:19 ` [PATCH v5 07/13] riscv: csr: Add CSR encodings for CSR_VXRM/CSR_VXSAT Charlie Jenkins
2024-07-19 16:19 ` [PATCH v5 08/13] riscv: Add xtheadvector instruction definitions Charlie Jenkins
2024-07-19 16:19 ` [PATCH v5 09/13] riscv: vector: Support xtheadvector save/restore Charlie Jenkins
2024-07-19 16:19 ` [PATCH v5 10/13] riscv: hwprobe: Add thead vendor extension probing Charlie Jenkins
2024-07-19 16:19 ` [PATCH v5 11/13] riscv: hwprobe: Document thead vendor extensions and xtheadvector extension Charlie Jenkins
2024-07-19 16:19 ` [PATCH v5 12/13] selftests: riscv: Fix vector tests Charlie Jenkins
2024-07-22 18:47 ` Andrew Jones
2024-07-22 21:14 ` Charlie Jenkins
2024-07-22 22:08 ` Andrew Jones
2024-07-22 22:14 ` Charlie Jenkins [this message]
2024-07-19 16:19 ` [PATCH v5 13/13] selftests: riscv: Support xtheadvector in " Charlie Jenkins
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zp7ZwDzpFt7iJ6iT@ghost \
--to=charlie@rivosinc.com \
--cc=ajones@ventanamicro.com \
--cc=andy.chiu@sifive.com \
--cc=aou@eecs.berkeley.edu \
--cc=conor@kernel.org \
--cc=corbet@lwn.net \
--cc=devicetree@vger.kernel.org \
--cc=evan@rivosinc.com \
--cc=guoren@kernel.org \
--cc=jernej.skrabec@gmail.com \
--cc=jrtc27@jrtc27.com \
--cc=jszhang@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=robh@kernel.org \
--cc=samuel.holland@sifive.com \
--cc=samuel@sholland.org \
--cc=shuah@kernel.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).