Provide additional context when an unexpected exception occurs by dumping
the relevant Supervisor, Virtual Supervisor (VS), and Hypervisor CSRs,
along with the general-purpose registers associated with the trap.

Dumping VS-mode CSRs in addition to host CSRs is beneficial when analysing
VS-mode traps. VSCAUSE, VSEPC, VSTVAL, and related VS state are required to
properly diagnose unexpected guest traps and potential hypervisor
misconfiguration.
For example, on an illegal-instruction exception the hardware may record
the faulting instruction in VSTVAL. If VSTVAL is zero, VSEPC should always
be inspected, and can be used together with objdump to identify the
faulting instruction. Dumping VSCAUSE is also useful when the guest does
not report it, or when the hypervisor redirects a trap to the guest using
VSCAUSE, VSTATUS, and VSTVEC, allowing verification that a subsequent trap
is not caused by incorrect VS state.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v3:
 - Refactor the code dumping of CSRs and GPRs:
   - Introduce new macros
   - Re-group some registers when values are displayed.
 - Print all registers name in lower case.
 - Drop unnessary "Dumping CSRs", "Dumping GSRs" as it
   is clear based on names.
 - Update the commit message: add justification of dumping of
   some VS* registers.
 - Drop printing of VSSATP as I don't know usecase for now
   where it could be needed.
---
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 | 69 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index e9c967786312..7b9bcb97035f 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -99,12 +99,81 @@ static const char *decode_cause(unsigned long cause)
     return decode_trap_cause(cause);
 }
 
+static void dump_general_regs(const struct cpu_user_regs *regs)
+{
+#define GPR_LIST \
+    X(regs, ra, " ") X(regs, sp, "\n") \
+    X(regs, gp, " ") X(regs, tp, "\n") \
+    X(regs, t0, " ") X(regs, t1, "\n") \
+    X(regs, t2, " ") X(regs, s0, "\n") \
+    X(regs, s1, " ") X(regs, a0, "\n") \
+    X(regs, a1, " ") X(regs, a2, "\n") \
+    X(regs, a3, " ") X(regs, a4, "\n") \
+    X(regs, a5, " ") X(regs, a6, "\n") \
+    X(regs, a7, " ") X(regs, s2, "\n") \
+    X(regs, s3, " ") X(regs, s4, "\n") \
+    X(regs, s5, " ") X(regs, s6, "\n") \
+    X(regs, s7, " ") X(regs, s8, "\n") \
+    X(regs, s9, " ") X(regs, s10, "\n") \
+    X(regs, s11, " ") X(regs, t3, "\n") \
+    X(regs, t4, " ") X(regs, t4, "\n")
+
+#define X(regs, name, delim) \
+    printk("\t %-4s: %016lx" delim, #name, (regs)->name);
+
+    GPR_LIST
+
+#undef X
+#undef GPR_LIST
+}
+
+static void dump_csrs(unsigned long cause)
+{
+#define CSR_LIST \
+    X(stvec, CSR_STVEC, " ") X(vstvec, CSR_VSTVEC, "\n") \
+    X(sepc, CSR_SEPC, " ") X(vsepc, CSR_VSEPC, "\n") \
+    X(stval, CSR_STVAL, " ") X(vstval, CSR_VSTVAL, "\n") \
+    X(status, CSR_SSTATUS, " ") X(vsstatus, CSR_VSSTATUS, "\n") \
+    X(satp, CSR_SATP, "\n") \
+    X(scause, CSR_SCAUSE, " [%s]\n", \
+      decode_cause(v)) \
+    X(vscause, CSR_VSCAUSE, " [%s]\n\n", \
+      decode_cause(v)) \
+    X(hstatus, CSR_HSTATUS, " [%s%s%s%s%s%s ]\n", \
+      (v & HSTATUS_VTSR) ? " VTSR" : "", \
+      (v & HSTATUS_VTVM) ? " VTVM" : "", \
+      (v & HSTATUS_HU)   ? " HU"   : "", \
+      (v & HSTATUS_SPVP) ? " SPVP" : "", \
+      (v & HSTATUS_SPV)  ? " SPV"  : "", \
+      (v & HSTATUS_GVA)  ? " GVA"  : "") \
+    X(hgatp, CSR_HGATP, "\n") \
+    X(htval, CSR_HTVAL, "\n") \
+    X(htinst, CSR_HTINST, "\n") \
+    X(hedeleg, CSR_HEDELEG, "\n") \
+    X(hideleg, CSR_HIDELEG, "\n") \
+    X(hstateen0, CSR_HSTATEEN0, "\n\n")
+
+#define X(name, csr, fmt, ...) \
+do { \
+    unsigned long v = csr_read(csr); \
+    printk("\t %-10s: %016lx" fmt, #name, v, ##__VA_ARGS__); \
+} while (0);
+
+    CSR_LIST
+
+#undef X
+#undef CSR_LIST
+}
+
 static void do_unexpected_trap(const struct cpu_user_regs *regs)
 {
     unsigned long cause = csr_read(CSR_SCAUSE);
 
     printk("Unhandled exception: %s\n", decode_cause(cause));
 
+    dump_csrs(cause);
+    dump_general_regs(regs);
+
     die();
 }
 
-- 
2.52.0

On 26.01.2026 09:38, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/traps.c
> +++ b/xen/arch/riscv/traps.c
> @@ -99,12 +99,81 @@ static const char *decode_cause(unsigned long cause)
>      return decode_trap_cause(cause);
>  }
>  
> +static void dump_general_regs(const struct cpu_user_regs *regs)
> +{
> +#define GPR_LIST \
> +    X(regs, ra, " ") X(regs, sp, "\n") \
> +    X(regs, gp, " ") X(regs, tp, "\n") \
> +    X(regs, t0, " ") X(regs, t1, "\n") \
> +    X(regs, t2, " ") X(regs, s0, "\n") \
> +    X(regs, s1, " ") X(regs, a0, "\n") \
> +    X(regs, a1, " ") X(regs, a2, "\n") \
> +    X(regs, a3, " ") X(regs, a4, "\n") \
> +    X(regs, a5, " ") X(regs, a6, "\n") \
> +    X(regs, a7, " ") X(regs, s2, "\n") \
> +    X(regs, s3, " ") X(regs, s4, "\n") \
> +    X(regs, s5, " ") X(regs, s6, "\n") \
> +    X(regs, s7, " ") X(regs, s8, "\n") \
> +    X(regs, s9, " ") X(regs, s10, "\n") \
> +    X(regs, s11, " ") X(regs, t3, "\n") \
> +    X(regs, t4, " ") X(regs, t4, "\n")
> +
> +#define X(regs, name, delim) \
> +    printk("\t %-4s: %016lx" delim, #name, (regs)->name);

No semicolon here please; that should be supplied at the use sites of
such a macro.

As to the format string: If past the leading tab you want an extra
padding blank, why not "\t%-5s ..."? Question however is why this deep
indentation is wanted in the first place. I'd suggest to omit the tab
in particular.

> +    GPR_LIST

What use is this macro? All it does is hamper readability, by requiring
the line-continuation backslashes in its definition.

> +#undef X
> +#undef GPR_LIST
> +}
> +
> +static void dump_csrs(unsigned long cause)
> +{
> +#define CSR_LIST \
> +    X(stvec, CSR_STVEC, " ") X(vstvec, CSR_VSTVEC, "\n") \
> +    X(sepc, CSR_SEPC, " ") X(vsepc, CSR_VSEPC, "\n") \
> +    X(stval, CSR_STVAL, " ") X(vstval, CSR_VSTVAL, "\n") \
> +    X(status, CSR_SSTATUS, " ") X(vsstatus, CSR_VSSTATUS, "\n") \
> +    X(satp, CSR_SATP, "\n") \
> +    X(scause, CSR_SCAUSE, " [%s]\n", \
> +      decode_cause(v)) \
> +    X(vscause, CSR_VSCAUSE, " [%s]\n\n", \
> +      decode_cause(v)) \

Anything that can help save space (as indicated, output may go to a limited size
screen) should imo be leveraged. IOW better no double newline here, nor ...

> +    X(hstatus, CSR_HSTATUS, " [%s%s%s%s%s%s ]\n", \
> +      (v & HSTATUS_VTSR) ? " VTSR" : "", \
> +      (v & HSTATUS_VTVM) ? " VTVM" : "", \
> +      (v & HSTATUS_HU)   ? " HU"   : "", \
> +      (v & HSTATUS_SPVP) ? " SPVP" : "", \
> +      (v & HSTATUS_SPV)  ? " SPV"  : "", \
> +      (v & HSTATUS_GVA)  ? " GVA"  : "") \
> +    X(hgatp, CSR_HGATP, "\n") \
> +    X(htval, CSR_HTVAL, "\n") \
> +    X(htinst, CSR_HTINST, "\n") \
> +    X(hedeleg, CSR_HEDELEG, "\n") \
> +    X(hideleg, CSR_HIDELEG, "\n") \
> +    X(hstateen0, CSR_HSTATEEN0, "\n\n")

... here. In this latter case it's questionable anyway, as apparently you
have this here to separate from the GPRs being logged subsequently. Just
that right here you don't know whether your caller actually means to do
so.

As to grouping: How about further putting hedeleg and hideleg on a single
line? Maybe also htval and htinst?

> +#define X(name, csr, fmt, ...) \
> +do { \
> +    unsigned long v = csr_read(csr); \
> +    printk("\t %-10s: %016lx" fmt, #name, v, ##__VA_ARGS__); \
> +} while (0);
> +
> +    CSR_LIST

Same remarks here as above. The issue is actually worse here, as CSR_LIST
uses "v" which it isn't passed.

Jan

On 1/26/26 11:09 AM, Jan Beulich wrote:
> On 26.01.2026 09:38, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/traps.c
>> +++ b/xen/arch/riscv/traps.c
>> @@ -99,12 +99,81 @@ static const char *decode_cause(unsigned long cause)
>>       return decode_trap_cause(cause);
>>   }
>>   
>> +static void dump_general_regs(const struct cpu_user_regs *regs)
>> +{
>> +#define GPR_LIST \
>> +    X(regs, ra, " ") X(regs, sp, "\n") \
>> +    X(regs, gp, " ") X(regs, tp, "\n") \
>> +    X(regs, t0, " ") X(regs, t1, "\n") \
>> +    X(regs, t2, " ") X(regs, s0, "\n") \
>> +    X(regs, s1, " ") X(regs, a0, "\n") \
>> +    X(regs, a1, " ") X(regs, a2, "\n") \
>> +    X(regs, a3, " ") X(regs, a4, "\n") \
>> +    X(regs, a5, " ") X(regs, a6, "\n") \
>> +    X(regs, a7, " ") X(regs, s2, "\n") \
>> +    X(regs, s3, " ") X(regs, s4, "\n") \
>> +    X(regs, s5, " ") X(regs, s6, "\n") \
>> +    X(regs, s7, " ") X(regs, s8, "\n") \
>> +    X(regs, s9, " ") X(regs, s10, "\n") \
>> +    X(regs, s11, " ") X(regs, t3, "\n") \
>> +    X(regs, t4, " ") X(regs, t4, "\n")
>> +
>> +#define X(regs, name, delim) \
>> +    printk("\t %-4s: %016lx" delim, #name, (regs)->name);
> No semicolon here please; that should be supplied at the use sites of
> such a macro.

I thought about this option, but decided it would be fine to do in this
way as it is a local marco and isn't expected to be used anywhere else,
so it will help to avoid a bunch of semicolons where X macro used now.
Anyway, I am okay to rework that.

>
> As to the format string: If past the leading tab you want an extra
> padding blank, why not "\t%-5s ..."? Question however is why this deep
> indentation is wanted in the first place. I'd suggest to omit the tab
> in particular.

I will check how it will look, probably we could really just drop \t and
use only "%-4s".


>
>> +    GPR_LIST
> What use is this macro? All it does is hamper readability, by requiring
> the line-continuation backslashes in its definition.

Agree, it doesn't really useful to have GPR_LIST. I will just leave X macro
and open-coded what is now in GPR_LIST instead.

>
>> +#undef X
>> +#undef GPR_LIST
>> +}
>> +
>> +static void dump_csrs(unsigned long cause)
>> +{
>> +#define CSR_LIST \
>> +    X(stvec, CSR_STVEC, " ") X(vstvec, CSR_VSTVEC, "\n") \
>> +    X(sepc, CSR_SEPC, " ") X(vsepc, CSR_VSEPC, "\n") \
>> +    X(stval, CSR_STVAL, " ") X(vstval, CSR_VSTVAL, "\n") \
>> +    X(status, CSR_SSTATUS, " ") X(vsstatus, CSR_VSSTATUS, "\n") \
>> +    X(satp, CSR_SATP, "\n") \
>> +    X(scause, CSR_SCAUSE, " [%s]\n", \
>> +      decode_cause(v)) \
>> +    X(vscause, CSR_VSCAUSE, " [%s]\n\n", \
>> +      decode_cause(v)) \
> Anything that can help save space (as indicated, output may go to a limited size
> screen) should imo be leveraged. IOW better no double newline here, nor ...

For it could be useful visually separate group of hypervisor-related registers.

>
>> +    X(hstatus, CSR_HSTATUS, " [%s%s%s%s%s%s ]\n", \
>> +      (v & HSTATUS_VTSR) ? " VTSR" : "", \
>> +      (v & HSTATUS_VTVM) ? " VTVM" : "", \
>> +      (v & HSTATUS_HU)   ? " HU"   : "", \
>> +      (v & HSTATUS_SPVP) ? " SPVP" : "", \
>> +      (v & HSTATUS_SPV)  ? " SPV"  : "", \
>> +      (v & HSTATUS_GVA)  ? " GVA"  : "") \
>> +    X(hgatp, CSR_HGATP, "\n") \
>> +    X(htval, CSR_HTVAL, "\n") \
>> +    X(htinst, CSR_HTINST, "\n") \
>> +    X(hedeleg, CSR_HEDELEG, "\n") \
>> +    X(hideleg, CSR_HIDELEG, "\n") \
>> +    X(hstateen0, CSR_HSTATEEN0, "\n\n")
> ... here. In this latter case it's questionable anyway, as apparently you
> have this here to separate from the GPRs being logged subsequently. Just
> that right here you don't know whether your caller actually means to do
> so.

With this one, I agree that it is not the best one place for double newline.
It was necessary to avoid printk("\n") before GPRs dumping.

In general, I am fine with dropping the double newline and using a single
newline, as this separation is probably only useful to me.

>
> As to grouping: How about further putting hedeleg and hideleg on a single
> line? Maybe also htval and htinst?

Good point. I will apply theses suggestions.

>
>> +#define X(name, csr, fmt, ...) \
>> +do { \
>> +    unsigned long v = csr_read(csr); \
>> +    printk("\t %-10s: %016lx" fmt, #name, v, ##__VA_ARGS__); \
>> +} while (0);
>> +
>> +    CSR_LIST
> Same remarks here as above. The issue is actually worse here, as CSR_LIST
> uses "v" which it isn't passed.

I will declare local variable v in dump_csrs() and use it. Considering that
this macros is declared and used only inside this function I think I can
not pass it to X macro.

~ Oleksii

On 26.01.2026 11:43, Oleksii Kurochko wrote:
> On 1/26/26 11:09 AM, Jan Beulich wrote:
>> On 26.01.2026 09:38, Oleksii Kurochko wrote:
>>> +#define X(name, csr, fmt, ...) \
>>> +do { \
>>> +    unsigned long v = csr_read(csr); \
>>> +    printk("\t %-10s: %016lx" fmt, #name, v, ##__VA_ARGS__); \
>>> +} while (0);
>>> +
>>> +    CSR_LIST
>> Same remarks here as above. The issue is actually worse here, as CSR_LIST
>> uses "v" which it isn't passed.
> 
> I will declare local variable v in dump_csrs() and use it. Considering that
> this macros is declared and used only inside this function I think I can
> not pass it to X macro.

Yes, that should indeed be fine.

Jan