| Message ID | 20240403-vector_sigreturn_tests-v1-1-2e68b7a3b8d7@rivosinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: selftests: Add signal handling vector tests | expand |
On 4/3/24 16:50, Charlie Jenkins wrote: > Add two tests to check vector save/restore when a signal is received > during a vector routine. One test ensures that a value is not clobbered > during signal handling. The other verifies that vector registers > modified in the signal handler are properly reflected when the signal > handling is complete. Hmm, isn't this testing two contradictory things ? We do want V regs to be not clobbered across a handled signal, o/w V enabled code would just not work at all. That implies that anything done by signal handler should just be discarded - no ? Am I missing something. -Vineet > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > --- > These tests came about to highlight the bug fixed in > https://lore.kernel.org/lkml/20240403072638.567446-1-bjorn@kernel.org/ > and will only pass with that fix applied. > --- > tools/testing/selftests/riscv/Makefile | 2 +- > tools/testing/selftests/riscv/sigreturn/.gitignore | 1 + > tools/testing/selftests/riscv/sigreturn/Makefile | 12 ++++ > .../testing/selftests/riscv/sigreturn/sigreturn.c | 82 ++++++++++++++++++++++ > 4 files changed, 96 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/riscv/Makefile b/tools/testing/selftests/riscv/Makefile > index 4a9ff515a3a0..7ce03d832b64 100644 > --- a/tools/testing/selftests/riscv/Makefile > +++ b/tools/testing/selftests/riscv/Makefile > @@ -5,7 +5,7 @@ > ARCH ?= $(shell uname -m 2>/dev/null || echo not) > > ifneq (,$(filter $(ARCH),riscv)) > -RISCV_SUBTARGETS ?= hwprobe vector mm > +RISCV_SUBTARGETS ?= hwprobe vector mm sigreturn > else > RISCV_SUBTARGETS := > endif > diff --git a/tools/testing/selftests/riscv/sigreturn/.gitignore b/tools/testing/selftests/riscv/sigreturn/.gitignore > new file mode 100644 > index 000000000000..35002b8ae780 > --- /dev/null > +++ b/tools/testing/selftests/riscv/sigreturn/.gitignore > @@ -0,0 +1 @@ > +sigreturn > diff --git a/tools/testing/selftests/riscv/sigreturn/Makefile b/tools/testing/selftests/riscv/sigreturn/Makefile > new file mode 100644 > index 000000000000..eb8bac9279a8 > --- /dev/null > +++ b/tools/testing/selftests/riscv/sigreturn/Makefile > @@ -0,0 +1,12 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (C) 2021 ARM Limited > +# Originally tools/testing/arm64/abi/Makefile > + > +CFLAGS += -I$(top_srcdir)/tools/include > + > +TEST_GEN_PROGS := sigreturn > + > +include ../../lib.mk > + > +$(OUTPUT)/sigreturn: sigreturn.c > + $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^ > diff --git a/tools/testing/selftests/riscv/sigreturn/sigreturn.c b/tools/testing/selftests/riscv/sigreturn/sigreturn.c > new file mode 100644 > index 000000000000..62397d5934f1 > --- /dev/null > +++ b/tools/testing/selftests/riscv/sigreturn/sigreturn.c > @@ -0,0 +1,82 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#include <signal.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <ucontext.h> > +#include <linux/ptrace.h> > +#include "../../kselftest_harness.h" > + > +#define RISCV_V_MAGIC 0x53465457 > +#define DEFAULT_VALUE 2 > +#define SIGNAL_HANDLER_OVERRIDE 3 > + > +static void simple_handle(int sig_no, siginfo_t *info, void *vcontext) > +{ > + ucontext_t *context = vcontext; > + > + context->uc_mcontext.__gregs[REG_PC] = context->uc_mcontext.__gregs[REG_PC] + 4; > +} > + > +static void vector_override(int sig_no, siginfo_t *info, void *vcontext) > +{ > + ucontext_t *context = vcontext; > + > + // vector state > + struct __riscv_extra_ext_header *ext; > + struct __riscv_v_ext_state *v_ext_state; > + > + /* Find the vector context. */ > + ext = (void *)(&context->uc_mcontext.__fpregs); > + if (ext->hdr.magic != RISCV_V_MAGIC) { > + fprintf(stderr, "bad vector magic: %x\n", ext->hdr.magic); > + abort(); > + } > + > + v_ext_state = (void *)((char *)(ext) + sizeof(*ext)); > + > + *(int *)v_ext_state->datap = SIGNAL_HANDLER_OVERRIDE; > + > + context->uc_mcontext.__gregs[REG_PC] = context->uc_mcontext.__gregs[REG_PC] + 4; > +} > + > +static int vector_sigreturn(int data, void (*handler)(int, siginfo_t *, void *)) > +{ > + int after_sigreturn; > + struct sigaction sig_action = { > + .sa_sigaction = handler, > + .sa_flags = SA_SIGINFO > + }; > + > + sigaction(SIGSEGV, &sig_action, 0); > + > + asm(".option push \n\ > + .option arch, +v \n\ > + vsetivli x0, 1, e32, ta, ma \n\ > + vmv.s.x v0, %1 \n\ > + # Generate SIGSEGV \n\ > + lw a0, 0(x0) \n\ > + vmv.x.s %0, v0 \n\ > + .option pop" : "=r" (after_sigreturn) : "r" (data)); > + > + return after_sigreturn; > +} > + > +TEST(vector_restore) > +{ > + int result; > + > + result = vector_sigreturn(DEFAULT_VALUE, &simple_handle); > + > + EXPECT_EQ(DEFAULT_VALUE, result); > +} > + > +TEST(vector_restore_signal_handler_override) > +{ > + int result; > + > + result = vector_sigreturn(DEFAULT_VALUE, &vector_override); > + > + EXPECT_EQ(SIGNAL_HANDLER_OVERRIDE, result); > +} > + > +TEST_HARNESS_MAIN > > --- > base-commit: 4cece764965020c22cff7665b18a012006359095 > change-id: 20240403-vector_sigreturn_tests-8118f0ac54fa
On Wed, Apr 03, 2024 at 05:27:23PM -0700, Vineet Gupta wrote: > On 4/3/24 16:50, Charlie Jenkins wrote: > > Add two tests to check vector save/restore when a signal is received > > during a vector routine. One test ensures that a value is not clobbered > > during signal handling. The other verifies that vector registers > > modified in the signal handler are properly reflected when the signal > > handling is complete. > > Hmm, isn't this testing two contradictory things ? > > We do want V regs to be not clobbered across a handled signal, o/w V > enabled code would just not work at all. > That implies that anything done by signal handler should just be > discarded - no ? > > Am I missing something. > > -Vineet This is referring to the user-implemented signal handler. The test in question is testing the ability of the user-implemented signal handler to be able to modify the vector registers. In the test: static void vector_override(int sig_no, siginfo_t *info, void *vcontext) { ... *(int *)v_ext_state->datap = SIGNAL_HANDLER_OVERRIDE; ... } This line is changing the saved state of the user program's vector registers. When the signal handling concludes and the original user program resumes execution, the content of the user-context registers that were modified by the signal handler should be restored into the hardware registers. Hence the test case checks that the v0 register contains the value SIGNAL_HANDLER_OVERRIDE. EXPECT_EQ(SIGNAL_HANDLER_OVERRIDE, result); - Charlie > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > --- > > These tests came about to highlight the bug fixed in > > https://lore.kernel.org/lkml/20240403072638.567446-1-bjorn@kernel.org/ > > and will only pass with that fix applied. > > --- > > tools/testing/selftests/riscv/Makefile | 2 +- > > tools/testing/selftests/riscv/sigreturn/.gitignore | 1 + > > tools/testing/selftests/riscv/sigreturn/Makefile | 12 ++++ > > .../testing/selftests/riscv/sigreturn/sigreturn.c | 82 ++++++++++++++++++++++ > > 4 files changed, 96 insertions(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/riscv/Makefile b/tools/testing/selftests/riscv/Makefile > > index 4a9ff515a3a0..7ce03d832b64 100644 > > --- a/tools/testing/selftests/riscv/Makefile > > +++ b/tools/testing/selftests/riscv/Makefile > > @@ -5,7 +5,7 @@ > > ARCH ?= $(shell uname -m 2>/dev/null || echo not) > > > > ifneq (,$(filter $(ARCH),riscv)) > > -RISCV_SUBTARGETS ?= hwprobe vector mm > > +RISCV_SUBTARGETS ?= hwprobe vector mm sigreturn > > else > > RISCV_SUBTARGETS := > > endif > > diff --git a/tools/testing/selftests/riscv/sigreturn/.gitignore b/tools/testing/selftests/riscv/sigreturn/.gitignore > > new file mode 100644 > > index 000000000000..35002b8ae780 > > --- /dev/null > > +++ b/tools/testing/selftests/riscv/sigreturn/.gitignore > > @@ -0,0 +1 @@ > > +sigreturn > > diff --git a/tools/testing/selftests/riscv/sigreturn/Makefile b/tools/testing/selftests/riscv/sigreturn/Makefile > > new file mode 100644 > > index 000000000000..eb8bac9279a8 > > --- /dev/null > > +++ b/tools/testing/selftests/riscv/sigreturn/Makefile > > @@ -0,0 +1,12 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (C) 2021 ARM Limited > > +# Originally tools/testing/arm64/abi/Makefile > > + > > +CFLAGS += -I$(top_srcdir)/tools/include > > + > > +TEST_GEN_PROGS := sigreturn > > + > > +include ../../lib.mk > > + > > +$(OUTPUT)/sigreturn: sigreturn.c > > + $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^ > > diff --git a/tools/testing/selftests/riscv/sigreturn/sigreturn.c b/tools/testing/selftests/riscv/sigreturn/sigreturn.c > > new file mode 100644 > > index 000000000000..62397d5934f1 > > --- /dev/null > > +++ b/tools/testing/selftests/riscv/sigreturn/sigreturn.c > > @@ -0,0 +1,82 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +#include <signal.h> > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <ucontext.h> > > +#include <linux/ptrace.h> > > +#include "../../kselftest_harness.h" > > + > > +#define RISCV_V_MAGIC 0x53465457 > > +#define DEFAULT_VALUE 2 > > +#define SIGNAL_HANDLER_OVERRIDE 3 > > + > > +static void simple_handle(int sig_no, siginfo_t *info, void *vcontext) > > +{ > > + ucontext_t *context = vcontext; > > + > > + context->uc_mcontext.__gregs[REG_PC] = context->uc_mcontext.__gregs[REG_PC] + 4; > > +} > > + > > +static void vector_override(int sig_no, siginfo_t *info, void *vcontext) > > +{ > > + ucontext_t *context = vcontext; > > + > > + // vector state > > + struct __riscv_extra_ext_header *ext; > > + struct __riscv_v_ext_state *v_ext_state; > > + > > + /* Find the vector context. */ > > + ext = (void *)(&context->uc_mcontext.__fpregs); > > + if (ext->hdr.magic != RISCV_V_MAGIC) { > > + fprintf(stderr, "bad vector magic: %x\n", ext->hdr.magic); > > + abort(); > > + } > > + > > + v_ext_state = (void *)((char *)(ext) + sizeof(*ext)); > > + > > + *(int *)v_ext_state->datap = SIGNAL_HANDLER_OVERRIDE; > > + > > + context->uc_mcontext.__gregs[REG_PC] = context->uc_mcontext.__gregs[REG_PC] + 4; > > +} > > + > > +static int vector_sigreturn(int data, void (*handler)(int, siginfo_t *, void *)) > > +{ > > + int after_sigreturn; > > + struct sigaction sig_action = { > > + .sa_sigaction = handler, > > + .sa_flags = SA_SIGINFO > > + }; > > + > > + sigaction(SIGSEGV, &sig_action, 0); > > + > > + asm(".option push \n\ > > + .option arch, +v \n\ > > + vsetivli x0, 1, e32, ta, ma \n\ > > + vmv.s.x v0, %1 \n\ > > + # Generate SIGSEGV \n\ > > + lw a0, 0(x0) \n\ > > + vmv.x.s %0, v0 \n\ > > + .option pop" : "=r" (after_sigreturn) : "r" (data)); > > + > > + return after_sigreturn; > > +} > > + > > +TEST(vector_restore) > > +{ > > + int result; > > + > > + result = vector_sigreturn(DEFAULT_VALUE, &simple_handle); > > + > > + EXPECT_EQ(DEFAULT_VALUE, result); > > +} > > + > > +TEST(vector_restore_signal_handler_override) > > +{ > > + int result; > > + > > + result = vector_sigreturn(DEFAULT_VALUE, &vector_override); > > + > > + EXPECT_EQ(SIGNAL_HANDLER_OVERRIDE, result); > > +} > > + > > +TEST_HARNESS_MAIN > > > > --- > > base-commit: 4cece764965020c22cff7665b18a012006359095 > > change-id: 20240403-vector_sigreturn_tests-8118f0ac54fa >
Charlie Jenkins <charlie@rivosinc.com> writes: > Add two tests to check vector save/restore when a signal is received > during a vector routine. One test ensures that a value is not clobbered > during signal handling. The other verifies that vector registers > modified in the signal handler are properly reflected when the signal > handling is complete. > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> Nice! Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
On Thu, Apr 4, 2024 at 1:47 PM Björn Töpel <bjorn@kernel.org> wrote: > > Charlie Jenkins <charlie@rivosinc.com> writes: > > > Add two tests to check vector save/restore when a signal is received > > during a vector routine. One test ensures that a value is not clobbered > > during signal handling. The other verifies that vector registers > > modified in the signal handler are properly reflected when the signal > > handling is complete. > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > Nice! > > Reviewed-by: Björn Töpel <bjorn@rivosinc.com> Thanks! Reviewed-by: Andy Chiu <andy.chiu@sifive.com> Tested-by: Andy Chiu <andy.chiu@sifive.com>
diff --git a/tools/testing/selftests/riscv/Makefile b/tools/testing/selftests/riscv/Makefile index 4a9ff515a3a0..7ce03d832b64 100644 --- a/tools/testing/selftests/riscv/Makefile +++ b/tools/testing/selftests/riscv/Makefile @@ -5,7 +5,7 @@ ARCH ?= $(shell uname -m 2>/dev/null || echo not) ifneq (,$(filter $(ARCH),riscv)) -RISCV_SUBTARGETS ?= hwprobe vector mm +RISCV_SUBTARGETS ?= hwprobe vector mm sigreturn else RISCV_SUBTARGETS := endif diff --git a/tools/testing/selftests/riscv/sigreturn/.gitignore b/tools/testing/selftests/riscv/sigreturn/.gitignore new file mode 100644 index 000000000000..35002b8ae780 --- /dev/null +++ b/tools/testing/selftests/riscv/sigreturn/.gitignore @@ -0,0 +1 @@ +sigreturn diff --git a/tools/testing/selftests/riscv/sigreturn/Makefile b/tools/testing/selftests/riscv/sigreturn/Makefile new file mode 100644 index 000000000000..eb8bac9279a8 --- /dev/null +++ b/tools/testing/selftests/riscv/sigreturn/Makefile @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2021 ARM Limited +# Originally tools/testing/arm64/abi/Makefile + +CFLAGS += -I$(top_srcdir)/tools/include + +TEST_GEN_PROGS := sigreturn + +include ../../lib.mk + +$(OUTPUT)/sigreturn: sigreturn.c + $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^ diff --git a/tools/testing/selftests/riscv/sigreturn/sigreturn.c b/tools/testing/selftests/riscv/sigreturn/sigreturn.c new file mode 100644 index 000000000000..62397d5934f1 --- /dev/null +++ b/tools/testing/selftests/riscv/sigreturn/sigreturn.c @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <signal.h> +#include <stdio.h> +#include <stdlib.h> +#include <ucontext.h> +#include <linux/ptrace.h> +#include "../../kselftest_harness.h" + +#define RISCV_V_MAGIC 0x53465457 +#define DEFAULT_VALUE 2 +#define SIGNAL_HANDLER_OVERRIDE 3 + +static void simple_handle(int sig_no, siginfo_t *info, void *vcontext) +{ + ucontext_t *context = vcontext; + + context->uc_mcontext.__gregs[REG_PC] = context->uc_mcontext.__gregs[REG_PC] + 4; +} + +static void vector_override(int sig_no, siginfo_t *info, void *vcontext) +{ + ucontext_t *context = vcontext; + + // vector state + struct __riscv_extra_ext_header *ext; + struct __riscv_v_ext_state *v_ext_state; + + /* Find the vector context. */ + ext = (void *)(&context->uc_mcontext.__fpregs); + if (ext->hdr.magic != RISCV_V_MAGIC) { + fprintf(stderr, "bad vector magic: %x\n", ext->hdr.magic); + abort(); + } + + v_ext_state = (void *)((char *)(ext) + sizeof(*ext)); + + *(int *)v_ext_state->datap = SIGNAL_HANDLER_OVERRIDE; + + context->uc_mcontext.__gregs[REG_PC] = context->uc_mcontext.__gregs[REG_PC] + 4; +} + +static int vector_sigreturn(int data, void (*handler)(int, siginfo_t *, void *)) +{ + int after_sigreturn; + struct sigaction sig_action = { + .sa_sigaction = handler, + .sa_flags = SA_SIGINFO + }; + + sigaction(SIGSEGV, &sig_action, 0); + + asm(".option push \n\ + .option arch, +v \n\ + vsetivli x0, 1, e32, ta, ma \n\ + vmv.s.x v0, %1 \n\ + # Generate SIGSEGV \n\ + lw a0, 0(x0) \n\ + vmv.x.s %0, v0 \n\ + .option pop" : "=r" (after_sigreturn) : "r" (data)); + + return after_sigreturn; +} + +TEST(vector_restore) +{ + int result; + + result = vector_sigreturn(DEFAULT_VALUE, &simple_handle); + + EXPECT_EQ(DEFAULT_VALUE, result); +} + +TEST(vector_restore_signal_handler_override) +{ + int result; + + result = vector_sigreturn(DEFAULT_VALUE, &vector_override); + + EXPECT_EQ(SIGNAL_HANDLER_OVERRIDE, result); +} + +TEST_HARNESS_MAIN
Add two tests to check vector save/restore when a signal is received during a vector routine. One test ensures that a value is not clobbered during signal handling. The other verifies that vector registers modified in the signal handler are properly reflected when the signal handling is complete. Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> --- These tests came about to highlight the bug fixed in https://lore.kernel.org/lkml/20240403072638.567446-1-bjorn@kernel.org/ and will only pass with that fix applied. --- tools/testing/selftests/riscv/Makefile | 2 +- tools/testing/selftests/riscv/sigreturn/.gitignore | 1 + tools/testing/selftests/riscv/sigreturn/Makefile | 12 ++++ .../testing/selftests/riscv/sigreturn/sigreturn.c | 82 ++++++++++++++++++++++ 4 files changed, 96 insertions(+), 1 deletion(-) --- base-commit: 4cece764965020c22cff7665b18a012006359095 change-id: 20240403-vector_sigreturn_tests-8118f0ac54fa