On 1/19/26 9:34 AM, Jan Beulich wrote:
On 16.01.2026 17:16, Oleksii Kurochko wrote:
Provide more context on the exception state when an unexpected
exception occurs by dumping various Supervisor, Virtual Supervisor
and Hypervisor CSRs, and GPRs pertaining to the trap.
Signed-off-by: Oleksii Kurochko<[email protected]>
---
Changes in v2
- Address coments about print_csr() macros and dump_csrs().
- Add dumping of GPRs pertaining to the trap.
---
xen/arch/riscv/traps.c | 82 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index e9c967786312..4e0df698552f 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -17,6 +17,13 @@
#include <asm/traps.h>
#include <asm/vsbi.h>
+#define print_csr(csr) \
+ printk("\t" #csr ": %016lx\n", csr_read(csr));
This prints the CSR_ prefix of the internally used constants. Is this useful
(rather than extra clutter)?
No, the prefix isn't really useful. It was printed so only because all CSRs
registers
defintions start on CSR_*.
Unlike for the GPRs this also prints the register
names in upper case. That may be fine, or may not be.
I will follow then like it is written in RISC-V spec for consistency.
The values printed also
won't align, which may make reading more difficult.
Do you expect the similar alignment like for GPRs?
+#define print_gprs(regs, reg1, reg2) \
+ printk("\t%-7s: %016lx %-7s: %016lx\n", \
+ #reg1, (regs)->reg1, #reg2, (regs)->reg2)
Yes, two per line is certainly helpful. Why not also for some of the CSRs.
It is less clear how it would be better to group them. I thought about
CSR_STVEC: .... CSR_VSTVEC: ....
CSR_STTATUS: ... CSR_VSSTATUS: ....
So group them in S-mode mode register and VS-mode register.
@@ -99,12 +106,87 @@ static const char *decode_cause(unsigned long cause)
return decode_trap_cause(cause);
}
+static void dump_general_regs(const struct cpu_user_regs *regs)
+{
+ printk("\nDumping GPRs...\n");
Register names alone will be meaningful enough? Be mindful of serial line
bandwidth as well as screen resolution.
Agree, we could drop this print. (Then probably the same could be for
Supervisor CSRs
and Virtual Supervisor CSRs, etc as it is clear based on the name which one CSR
it
is)
+ print_gprs(regs, ra, sp);
+ print_gprs(regs, gp, tp);
+ print_gprs(regs, t0, t1);
+ print_gprs(regs, t2, s0);
+ print_gprs(regs, s1, a0);
+ print_gprs(regs, a1, a2);
+ print_gprs(regs, a3, a4);
+ print_gprs(regs, a5, a6);
+ print_gprs(regs, a7, s2);
+ print_gprs(regs, s3, s4);
+ print_gprs(regs, s5, s6);
+ print_gprs(regs, s7, s8);
+ print_gprs(regs, s9, s10);
+ print_gprs(regs, s11, t3);
+ print_gprs(regs, t4, t5);
+ print_gprs(regs, t6, sepc);
+ print_gprs(regs, sstatus, hstatus);
The last three aren't GPRs. Why would they be logged here? (All three also
being logged in dump_csrs() would further require some disambiguation in
the output, for people to not need to go look at the source code every
time.)
Agree, that it would be better to print it with the CSRs. It was print here
only as they are in the same structure with GPRs.
+}
+
+static void dump_csrs(unsigned long cause)
+{
+ unsigned long hstatus;
+
+ printk("\nDumping CSRs...\n");
+
+ printk("Supervisor CSRs\n");
+ print_csr(CSR_STVEC);
+ print_csr(CSR_SATP);
+ print_csr(CSR_SEPC);
+
+ hstatus = csr_read(CSR_HSTATUS);
+
+ printk("\tCSR_STVAL: %016lx%s\n", csr_read(CSR_STVAL),
+ (hstatus & HSTATUS_GVA) ? ", (guest virtual address)" : "");
+
+ printk("\tCSR_SCAUSE: %016lx\n", cause);
+ printk("\t\tDescription: %s\n", decode_cause(cause));
+ print_csr(CSR_SSTATUS);
+
+ printk("\nVirtual Supervisor CSRs\n");
+ print_csr(CSR_VSTVEC);
+ print_csr(CSR_VSATP);
+ print_csr(CSR_VSEPC);
+ print_csr(CSR_VSTVAL);
+ cause = csr_read(CSR_VSCAUSE);
+ printk("\tCSR_VSCAUSE: %016lx\n", cause);
+ printk("\t\tDescription: %s\n", decode_cause(cause));
+ print_csr(CSR_VSSTATUS);
As before, imo justification is wanted (in the description) for logging
VS* values.
It is hard to describe in general why they could be needed. The best I can
come up is:
Dump VS* CSRs to provide full guest exception context. When handling traps
originating
from VS-mode, host CSRs alone do not describe the fault; VSCAUSE, VSEPC,
VSTVAL, and
related state are required to diagnose guest crashes and hypervisor
misconfiguration,
and to correlate host-side handling with guest-visible exceptions.
Does it good enough justification?
~ Oleksii
