[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v7 14/14] xen/riscv: Disable SSTC extension and add trap-based CSR probing



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 <oleksii.kurochko@xxxxxxxxx>
---
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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.