Some RISC-V platforms expose the SSTC extension, but its CSRs are not properly saved and restored by Xen. Using SSTC in Xen could therefore lead to unexpected behaviour.
To avoid this in QEMU, disable SSTC by passing "sstc=off". On real hardware, OpenSBI does not provide a mechanism to disable SSTC via the DTS (riscv,isa or similar property), as it does not rely on that property to determine extension availability. Instead, it directly probes the CSR_STIMECMP register. Introduce struct trap_info together with the do_expected_trap() handler to safely probe CSRs. The helper csr_read_allowed() attempts to read a CSR while catching traps, allowing Xen to detect whether the register is accessible. This mechanism is used at boot to verify SSTC support and panic if the CSR is not available. The trap handling infrastructure may also be reused for other cases where controlled trap handling is required (e.g. probing instructions such as HLV*). Also reorder header inclusion in asm-offsets.c to follow Xen coding style: <xen/types.h> should be included before <asm/*> headers as there is no any specific reason to remain the current order. Signed-off-by: Oleksii Kurochko <[email protected]> --- Changes in v7: - new patch. IMO, it is okay to have this patch separetely as at the moment it won't be an issue if Xen will use CSR_STIMECMP to setup its timer. The issue will start to occur when a guest will run. --- automation/scripts/qemu-smoke-riscv64.sh | 2 +- xen/arch/riscv/cpufeature.c | 8 ++++++ xen/arch/riscv/entry.S | 24 ++++++++++++++++++ xen/arch/riscv/include/asm/csr.h | 32 ++++++++++++++++++++++++ xen/arch/riscv/include/asm/traps.h | 7 ++++++ xen/arch/riscv/riscv64/asm-offsets.c | 7 +++++- 6 files changed, 78 insertions(+), 2 deletions(-) diff --git a/automation/scripts/qemu-smoke-riscv64.sh b/automation/scripts/qemu-smoke-riscv64.sh index c0b1082a08fd..1909abb7af32 100755 --- a/automation/scripts/qemu-smoke-riscv64.sh +++ b/automation/scripts/qemu-smoke-riscv64.sh @@ -7,7 +7,7 @@ rm -f smoke.serial export TEST_CMD="qemu-system-riscv64 \ -M virt,aia=aplic-imsic \ - -cpu rv64,svpbmt=on \ + -cpu rv64,svpbmt=on,sstc=off \ -smp 1 \ -nographic \ -m 2g \ diff --git a/xen/arch/riscv/cpufeature.c b/xen/arch/riscv/cpufeature.c index 03e27b037be0..987d36dc7eee 100644 --- a/xen/arch/riscv/cpufeature.c +++ b/xen/arch/riscv/cpufeature.c @@ -17,6 +17,8 @@ #include <xen/sections.h> #include <asm/cpufeature.h> +#include <asm/csr.h> +#include <asm/traps.h> #ifdef CONFIG_ACPI # error "cpufeature.c functions should be updated to support ACPI" @@ -483,6 +485,7 @@ void __init riscv_fill_hwcap(void) unsigned int i; const size_t req_extns_amount = ARRAY_SIZE(required_extensions); bool all_extns_available = true; + struct trap_info trap; riscv_fill_hwcap_from_isa_string(); @@ -509,4 +512,9 @@ void __init riscv_fill_hwcap(void) if ( !all_extns_available ) panic("Look why the extensions above are needed in " "https://xenbits.xenproject.org/docs/unstable/misc/riscv/booting.txt\n";); + + csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap); + + if ( !trap.scause ) + panic("SSTC isn't supported\n"); } diff --git a/xen/arch/riscv/entry.S b/xen/arch/riscv/entry.S index 202a35fb03a8..b434948da3a4 100644 --- a/xen/arch/riscv/entry.S +++ b/xen/arch/riscv/entry.S @@ -99,3 +99,27 @@ restore_registers: sret END(handle_trap) + + /* + * We assume that the faulting instruction is 4 bytes long and blindly + * increment SEPC by 4. + * + * This should be safe because all places that may trigger this handler + * use ".option norvc" around the instruction that could cause the trap, + * or the instruction is not available in the RVC instruction set. + * + * do_expected_trap(a3, a4): + * a3 <- pointer to struct trap_info + * a4 <- temporary register + */ +FUNC(do_expected_trap) + csrr a4, CSR_SEPC + REG_S a4, RISCV_TRAP_SEPC(a3) + csrr a4, CSR_SCAUSE + REG_S a4, RISCV_TRAP_SCAUSE(a3) + + csrr a4, CSR_SEPC + addi a4, a4, 4 + csrw CSR_SEPC, a4 + sret +END(do_expected_trap) diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h index 01876f828981..b318cbdf35c3 100644 --- a/xen/arch/riscv/include/asm/csr.h +++ b/xen/arch/riscv/include/asm/csr.h @@ -9,6 +9,7 @@ #include <asm/asm.h> #include <xen/const.h> #include <asm/riscv_encoding.h> +#include <asm/traps.h> #ifndef __ASSEMBLER__ @@ -78,6 +79,37 @@ : "memory" ); \ }) +/* + * Some functions inside asm/system.h requires some of the macros above, + * so this header should be included after the macros above are introduced. + */ +#include <asm/system.h> + +#define csr_read_allowed(csr_num, trap) \ +({ \ + register unsigned long tinfo asm("a3") = (unsigned long)trap; \ + register unsigned long ttmp asm("a4"); \ + register unsigned long stvec = (unsigned long)&do_expected_trap; \ + register unsigned long ret = 0; \ + unsigned long flags; \ + ((struct trap_info *)(trap))->scause = 0; \ + local_irq_save(flags); \ + asm volatile ( \ + ".option push\n" \ + ".option norvc\n" \ + "add %[ttmp], %[tinfo], zero\n" \ + "csrrw %[stvec], " STR(CSR_STVEC) ", %[stvec]\n" \ + "csrr %[ret], %[csr]\n" \ + "csrw " STR(CSR_STVEC) ", %[stvec]\n" \ + ".option pop" \ + : [stvec] "+&r" (stvec), [tinfo] "+&r" (tinfo), \ + [ttmp] "+&r" (ttmp), [ret] "=&r" (ret) \ + : [csr] "i" (csr_num) \ + : "memory" ); \ + local_irq_restore(flags); \ + ret; \ +}) + #endif /* __ASSEMBLER__ */ #endif /* ASM__RISCV__CSR_H */ diff --git a/xen/arch/riscv/include/asm/traps.h b/xen/arch/riscv/include/asm/traps.h index 21fa3c3259b3..194d9a72f3ed 100644 --- a/xen/arch/riscv/include/asm/traps.h +++ b/xen/arch/riscv/include/asm/traps.h @@ -7,10 +7,17 @@ #ifndef __ASSEMBLER__ +struct trap_info { + register_t sepc; + register_t scause; +}; + void do_trap(struct cpu_user_regs *cpu_regs); void handle_trap(void); void trap_init(void); +void do_expected_trap(void); + #endif /* __ASSEMBLER__ */ #endif /* ASM__RISCV__TRAPS_H */ diff --git a/xen/arch/riscv/riscv64/asm-offsets.c b/xen/arch/riscv/riscv64/asm-offsets.c index 472cced4f8af..b0e2a4d86bd3 100644 --- a/xen/arch/riscv/riscv64/asm-offsets.c +++ b/xen/arch/riscv/riscv64/asm-offsets.c @@ -1,8 +1,10 @@ #define COMPILE_OFFSETS +#include <xen/types.h> + #include <asm/current.h> #include <asm/processor.h> -#include <xen/types.h> +#include <asm/traps.h> #define DEFINE(_sym, _val) \ asm volatile ( "\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\""\ @@ -54,4 +56,7 @@ void asm_offsets(void) BLANK(); DEFINE(PCPU_INFO_SIZE, sizeof(struct pcpu_info)); BLANK(); + OFFSET(RISCV_TRAP_SEPC, struct trap_info, sepc); + OFFSET(RISCV_TRAP_SCAUSE, struct trap_info, scause); + BLANK(); } -- 2.53.0
