On Tue, Jun 03, 2025 at 05:46:50PM +0200, Clément Léger wrote: > This test allows to test the double trap implementation of hardware as > well as the SBI FWFT and SSE support for double trap. The tests will try > to trigger double trap using various sequences and will test to receive > the SSE double trap event if supported. > > It is provided as a separate test from the SBI one for two reasons: > - It isn't specifically testing SBI "per se". > - It ends up by trying to crash into in M-mode. > > Currently, the test uses a page fault to raise a trap programatically. > Some concern was raised by a github user on the original branch [1] > saying that the spec doesn't mandate any trap to be delegatable and that > we would need a way to detect which ones are delegatable. I think we can > safely assume that PAGE FAULT is delegatable and if a hardware that does > not have support comes up then it will probably be the vendor > responsibility to provide a way to do so. > > Link: https://github.com/clementleger/kvm-unit-tests/issues/1 [1] > Signed-off-by: Clément Léger <cleger@xxxxxxxxxxxx> > --- > riscv/Makefile | 1 + > lib/riscv/asm/csr.h | 1 + > lib/riscv/asm/processor.h | 10 ++ > riscv/isa-dbltrp.c | 211 ++++++++++++++++++++++++++++++++++++++ > riscv/unittests.cfg | 4 + > 5 files changed, 227 insertions(+) > create mode 100644 riscv/isa-dbltrp.c > > diff --git a/riscv/Makefile b/riscv/Makefile > index 11e68eae..d71c9d2e 100644 > --- a/riscv/Makefile > +++ b/riscv/Makefile > @@ -14,6 +14,7 @@ tests = > tests += $(TEST_DIR)/sbi.$(exe) > tests += $(TEST_DIR)/selftest.$(exe) > tests += $(TEST_DIR)/sieve.$(exe) > +tests += $(TEST_DIR)/isa-dbltrp.$(exe) > > all: $(tests) > > diff --git a/lib/riscv/asm/csr.h b/lib/riscv/asm/csr.h > index 3e4b5fca..6a8e0578 100644 > --- a/lib/riscv/asm/csr.h > +++ b/lib/riscv/asm/csr.h > @@ -18,6 +18,7 @@ > > #define SR_SIE _AC(0x00000002, UL) > #define SR_SPP _AC(0x00000100, UL) > +#define SR_SDT _AC(0x01000000, UL) /* Supervisor Double Trap */ > > /* Exception cause high bit - is an interrupt if set */ > #define CAUSE_IRQ_FLAG (_AC(1, UL) << (__riscv_xlen - 1)) > diff --git a/lib/riscv/asm/processor.h b/lib/riscv/asm/processor.h > index 40104272..87a41312 100644 > --- a/lib/riscv/asm/processor.h > +++ b/lib/riscv/asm/processor.h > @@ -48,6 +48,16 @@ static inline void ipi_ack(void) > csr_clear(CSR_SIP, IE_SSIE); > } > > +static inline void local_dlbtrp_enable(void) > +{ > + csr_set(CSR_SSTATUS, SR_SDT); > +} > + > +static inline void local_dlbtrp_disable(void) > +{ > + csr_clear(CSR_SSTATUS, SR_SDT); > +} > + > void install_exception_handler(unsigned long cause, void (*handler)(struct pt_regs *)); > void install_irq_handler(unsigned long cause, void (*handler)(struct pt_regs *)); > void do_handle_exception(struct pt_regs *regs); > diff --git a/riscv/isa-dbltrp.c b/riscv/isa-dbltrp.c > new file mode 100644 > index 00000000..a4545096 > --- /dev/null > +++ b/riscv/isa-dbltrp.c > @@ -0,0 +1,211 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * SBI verification > + * > + * Copyright (C) 2025, Rivos Inc., Clément Léger <cleger@xxxxxxxxxxxx> > + */ > +#include <alloc.h> > +#include <alloc_page.h> > +#include <libcflat.h> > +#include <stdlib.h> > + > +#include <asm/csr.h> > +#include <asm/page.h> > +#include <asm/processor.h> > +#include <asm/ptrace.h> > +#include <asm/sbi.h> > + > +#include <sbi-tests.h> > + > +static bool double_trap; > +static bool clear_sdt; > + > +#define INSN_LEN(insn) ((((insn) & 0x3) < 0x3) ? 2 : 4) > + > +#define GEN_TRAP() \ > +do { \ > + void *ptr = NULL; \ > + unsigned long value = 0; \ > + asm volatile( \ > + " .option push\n" \ > + " .option arch,-c\n" \ > + " sw %0, 0(%1)\n" \ > + " .option pop\n" \ > + : : "r" (value), "r" (ptr) : "memory"); \ > +} while (0) > + > +static void pagefault_trap_handler(struct pt_regs *regs) > +{ > + if (READ_ONCE(clear_sdt)) > + local_dlbtrp_disable(); > + > + if (READ_ONCE(double_trap)) { > + WRITE_ONCE(double_trap, false); > + GEN_TRAP(); > + } > + > + /* Skip trapping instruction */ > + regs->epc += 4; > + > + local_dlbtrp_enable(); > +} > + > +static bool sse_dbltrp_called; > + > +static void sse_dbltrp_handler(void *data, struct pt_regs *regs, unsigned int hartid) > +{ > + struct sbiret ret; > + unsigned long flags; > + unsigned long expected_flags = SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPP | > + SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SDT; > + > + ret = sbi_sse_read_attrs(SBI_SSE_EVENT_LOCAL_DOUBLE_TRAP, SBI_SSE_ATTR_INTERRUPTED_FLAGS, 1, > + &flags); > + sbiret_report_error(&ret, SBI_SUCCESS, "Get double trap event flags"); > + report(flags == expected_flags, "SSE flags == 0x%lx", expected_flags); > + > + WRITE_ONCE(sse_dbltrp_called, true); > + > + /* Skip trapping instruction */ > + regs->epc += 4; > +} > + > +static int sse_double_trap(void) > +{ > + struct sbiret ret; > + int err = 0; > + > + struct sbi_sse_handler_arg handler_arg = { > + .handler = sse_dbltrp_handler, > + .stack = alloc_page() + PAGE_SIZE, > + }; > + > + report_prefix_push("sse"); > + > + ret = sbi_sse_hart_unmask(); > + if (!sbiret_report_error(&ret, SBI_SUCCESS, "SSE hart unmask ok")) { > + report_skip("Failed to unmask SSE events, skipping test"); > + goto out_free_page; > + } > + > + ret = sbi_sse_register(SBI_SSE_EVENT_LOCAL_DOUBLE_TRAP, &handler_arg); > + if (ret.error == SBI_ERR_NOT_SUPPORTED) { > + report_skip("SSE double trap event is not supported"); > + goto out_mask_sse; > + } > + sbiret_report_error(&ret, SBI_SUCCESS, "SSE double trap register"); > + > + ret = sbi_sse_enable(SBI_SSE_EVENT_LOCAL_DOUBLE_TRAP); > + if (!sbiret_report_error(&ret, SBI_SUCCESS, "SSE double trap enable")) { > + err = ret.error; I'm not sure we need to return an error for this case. We won't be leaving an SSE event handler registered. > + goto out_unregister; > + } > + > + /* > + * Generate a double crash so that an SSE event should be generated. The SPEC (ISA nor SBI) > + * does not explicitly tell that if supported it should generate an SSE event but that's > + * a reasonable assumption to do so if both FWFT and SSE are supported. > + */ > + WRITE_ONCE(clear_sdt, false); > + WRITE_ONCE(double_trap, true); > + GEN_TRAP(); > + > + report(READ_ONCE(sse_dbltrp_called), "SSE double trap event generated"); > + > + ret = sbi_sse_disable(SBI_SSE_EVENT_LOCAL_DOUBLE_TRAP); > + sbiret_report_error(&ret, SBI_SUCCESS, "SSE double trap disable"); > + > +out_unregister: > + ret = sbi_sse_unregister(SBI_SSE_EVENT_LOCAL_DOUBLE_TRAP); > + sbiret_report_error(&ret, SBI_SUCCESS, "SSE double trap unregister"); Needs to be if (!sbiret_report_error(&ret, SBI_SUCCESS, "SSE double trap unregister")) err = ret.error; > + > +out_mask_sse: > + sbi_sse_hart_mask(); > + > +out_free_page: > + free_page(handler_arg.stack - PAGE_SIZE); > + report_prefix_pop(); > + > + return err; > +} > + > +static void check_double_trap(void) > +{ > + struct sbiret ret; > + > + /* Disable double trap */ > + ret = sbi_fwft_set(SBI_FWFT_DOUBLE_TRAP, 0, 0); > + sbiret_report_error(&ret, SBI_SUCCESS, "Set double trap enable feature value == 0"); > + ret = sbi_fwft_get(SBI_FWFT_DOUBLE_TRAP); > + sbiret_report(&ret, SBI_SUCCESS, 0, "Get double trap enable feature value == 0"); > + > + install_exception_handler(EXC_STORE_PAGE_FAULT, pagefault_trap_handler); > + > + WRITE_ONCE(clear_sdt, true); > + WRITE_ONCE(double_trap, true); > + GEN_TRAP(); > + report_pass("Double trap disabled, trap first time ok"); > + > + /* Enable double trap */ > + ret = sbi_fwft_set(SBI_FWFT_DOUBLE_TRAP, 1, 0); > + sbiret_report_error(&ret, SBI_SUCCESS, "Set double trap enable feature value == 1"); > + ret = sbi_fwft_get(SBI_FWFT_DOUBLE_TRAP); > + if (!sbiret_report(&ret, SBI_SUCCESS, 1, "Get double trap enable feature value == 1")) > + return; > + > + /* First time, clear the double trap flag (SDT) so that it doesn't generate a double trap */ > + WRITE_ONCE(clear_sdt, true); > + WRITE_ONCE(double_trap, true); > + > + GEN_TRAP(); > + report_pass("Trapped twice allowed ok"); > + > + if (sbi_probe(SBI_EXT_SSE)) { > + if (sse_double_trap()) { > + report_skip("Could not correctly unregister SSE event, skipping last test"); > + return; > + } > + } else { > + report_skip("SSE double trap event will not be tested, extension is not available"); Missing return > + } How about rearranging as if (!sbi_probe(SBI_EXT_SSE)) { report_skip("SSE double trap event will not be tested, extension is not available"); return; } if (sse_double_trap()) { report_skip("Could not correctly unregister SSE event, skipping last test"); return; } > + > + if (!env_or_skip("DOUBLE_TRAP_TEST_CRASH")) > + return; > + > + /* > + * Third time, keep the double trap flag (SDT) and generate another trap, this should > + * generate a double trap. Since there is no SSE handler registered, it should crash to > + * M-mode. > + */ > + WRITE_ONCE(clear_sdt, false); > + WRITE_ONCE(double_trap, true); > + report_info("Should generate a double trap and crash!"); > + GEN_TRAP(); > + report_fail("Should have crashed!"); > +} > + > +int main(int argc, char **argv) > +{ > + struct sbiret ret; > + > + report_prefix_push("dbltrp"); > + > + if (!sbi_probe(SBI_EXT_FWFT)) { > + report_skip("FWFT extension is not available, can not enable double traps"); > + goto out; > + } > + > + ret = sbi_fwft_get(SBI_FWFT_DOUBLE_TRAP); > + if (ret.error == SBI_ERR_NOT_SUPPORTED) { > + report_skip("SBI_FWFT_DOUBLE_TRAP is not supported!"); > + goto out; > + } > + > + if (sbiret_report_error(&ret, SBI_SUCCESS, "SBI_FWFT_DOUBLE_TRAP get value")) > + check_double_trap(); > + > +out: > + report_prefix_pop(); > + > + return report_summary(); > +} > diff --git a/riscv/unittests.cfg b/riscv/unittests.cfg > index 2eb760ec..286e1cc7 100644 > --- a/riscv/unittests.cfg > +++ b/riscv/unittests.cfg > @@ -18,3 +18,7 @@ groups = selftest > file = sbi.flat > smp = $MAX_SMP > groups = sbi > + > +[dbltrp] > +file = isa-dbltrp.flat > +groups = isa sbi > -- > 2.49.0 > Thanks, drew
