This commit introduces support for handling virtual SBI extensions in Xen.

The changes include:
- Added new vsbi/core.c and vsbi.h files to implement virtual SBI extension
  handling.
- Modified traps.c to handle CAUSE_VIRTUAL_SUPERVISOR_ECALL by calling
  vsbi_handle_ecall() when the trap originates from VS-mode.
- Updated xen.lds.S to include a new .vsbi.exts section for virtual SBI
  extension data.
- Updated Makefile to include the new vsbi/ directory in the build.
- Add hstatus register to struct cpu_user_regs as it is needed for
  a check that CAUSE_VIRTUAL_SUPERVISOR_ECALL happens from VS-mode.
  Also, add storing/restoring of hstatus register in handle_trap().
- Introduce vsbi_find_extension() to check if vsbi extension is supported
  by Xen. For now it is called only inside vsbi/core.c, but in future
  it is going to be called from other files.
- Introduce check_vsbi_ext_ranges() to check if there EIDs ranges
  overlapping between extensions.

The implementation allows for registration and handling of SBI
extensions via a new vsbi_ext structure and ".vsbi.exts" section,
enabling extensible virtual SBI support for RISC-V guests.

Note: All EIDs are printed in the format #%#lx and all FIDs in #%#lu, as
the SBI spec uses these formats. Printing them this way makes it easier to
search for them in the SBI spec.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v2:
 - s/struct regs/struct cpu_user_regs.
 - s/handle/handler.
 - Drop extid_ prefix inside VSBI_EXT_START().
 - use BUG_ON() instead of panic to be sure that 
   CAUSE_VIRTUAL_SUPERVISOR_ECALL comes from VS-mode.
 - s/ext_id/eid for vsbi_find_extension().
 - Add the comment above VSBI_EXT_START about [start,end] range.
 - Drop check "&& ext->handler" in vsbi_handle_ecall() as it isn't
   be so that handler() isn't provided.
 - s/vsbi.c/core.c
 - s/vsbi_ext/ext for local variable inside vsbi_find_extension().
 - Update the commit message: add a note about FID and EID printing formats,
   add some information about vsbo_find_extension() function, and add info
   about check_vsbi_ext_ranges().
 - Introduce check_vsbi_ext_ranges() to be sure that there is no overlapping
   in EIDs range(s).
 - Add storing/restoring of hstatus register in handle_trap()[entry.S].
---
 xen/arch/riscv/Makefile                |  1 +
 xen/arch/riscv/entry.S                 |  6 +++
 xen/arch/riscv/include/asm/processor.h |  1 +
 xen/arch/riscv/include/asm/vsbi.h      | 32 +++++++++++++++
 xen/arch/riscv/riscv64/asm-offsets.c   |  1 +
 xen/arch/riscv/setup.c                 |  3 ++
 xen/arch/riscv/traps.c                 |  8 ++++
 xen/arch/riscv/vsbi/Makefile           |  1 +
 xen/arch/riscv/vsbi/core.c             | 56 ++++++++++++++++++++++++++
 xen/arch/riscv/xen.lds.S               |  7 ++++
 10 files changed, 116 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/vsbi.h
 create mode 100644 xen/arch/riscv/vsbi/Makefile
 create mode 100644 xen/arch/riscv/vsbi/core.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 9dde693db4..87c1148b00 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -20,6 +20,7 @@ obj-y += time.o
 obj-y += traps.o
 obj-y += vmid.o
 obj-y += vm_event.o
+obj-y += vsbi/
 
 $(TARGET): $(TARGET)-syms
 	$(OBJCOPY) -O binary -S $< $@
diff --git a/xen/arch/riscv/entry.S b/xen/arch/riscv/entry.S
index 4db818ba8d..202a35fb03 100644
--- a/xen/arch/riscv/entry.S
+++ b/xen/arch/riscv/entry.S
@@ -48,11 +48,17 @@ save_to_stack:
         csrr    t0, CSR_SSTATUS
         REG_S   t0, CPU_USER_REGS_SSTATUS(sp)
 
+        csrr    t0, CSR_HSTATUS
+        REG_S   t0, CPU_USER_REGS_HSTATUS(sp)
+
         mv      a0, sp
         call    do_trap
 
 restore_registers:
         /* Restore stack_cpu_regs */
+        REG_L	t0, CPU_USER_REGS_HSTATUS(sp)
+        csrw	CSR_HSTATUS, t0
+
         REG_L   t0, CPU_USER_REGS_SEPC(sp)
         csrw    CSR_SEPC, t0
         REG_L   t0, CPU_USER_REGS_SSTATUS(sp)
diff --git a/xen/arch/riscv/include/asm/processor.h b/xen/arch/riscv/include/asm/processor.h
index 2502045642..6b89df4a2d 100644
--- a/xen/arch/riscv/include/asm/processor.h
+++ b/xen/arch/riscv/include/asm/processor.h
@@ -49,6 +49,7 @@ struct cpu_user_regs
     unsigned long t6;
     unsigned long sepc;
     unsigned long sstatus;
+    unsigned long hstatus;
     /* pointer to previous stack_cpu_regs */
     unsigned long pregs;
 };
diff --git a/xen/arch/riscv/include/asm/vsbi.h b/xen/arch/riscv/include/asm/vsbi.h
new file mode 100644
index 0000000000..47a38dce02
--- /dev/null
+++ b/xen/arch/riscv/include/asm/vsbi.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier:  GPL-2.0-only */
+
+#ifndef ASM_RISCV_VSBI_H
+#define ASM_RISCV_VSBI_H
+
+struct cpu_user_regs;
+struct vcpu;
+
+struct vsbi_ext {
+    const char *name;
+    unsigned long eid_start;
+    unsigned long eid_end;
+    int (*handler)(struct vcpu *vcpu, unsigned long eid,
+                   unsigned long fid, struct cpu_user_regs *regs);
+};
+
+/* Ranges (start and end) are inclusive within an extension */
+#define VSBI_EXT(ext, start, end, handle)           \
+static const struct vsbi_ext vsbi_ext_##ext __used  \
+__section(".vsbi.exts") = {                         \
+    .name = #ext,                                   \
+    .eid_start = start,                             \
+    .eid_end = end,                                 \
+    .handler = handle,                              \
+};
+
+void vsbi_handle_ecall(struct vcpu *vcpu, struct cpu_user_regs *regs);
+const struct vsbi_ext *vsbi_find_extension(unsigned long eid);
+
+void check_vsbi_ext_ranges(void);
+
+#endif
diff --git a/xen/arch/riscv/riscv64/asm-offsets.c b/xen/arch/riscv/riscv64/asm-offsets.c
index 3b5daf3b36..472cced4f8 100644
--- a/xen/arch/riscv/riscv64/asm-offsets.c
+++ b/xen/arch/riscv/riscv64/asm-offsets.c
@@ -49,6 +49,7 @@ void asm_offsets(void)
     OFFSET(CPU_USER_REGS_T6, struct cpu_user_regs, t6);
     OFFSET(CPU_USER_REGS_SEPC, struct cpu_user_regs, sepc);
     OFFSET(CPU_USER_REGS_SSTATUS, struct cpu_user_regs, sstatus);
+    OFFSET(CPU_USER_REGS_HSTATUS, struct cpu_user_regs, hstatus);
     OFFSET(CPU_USER_REGS_PREGS, struct cpu_user_regs, pregs);
     BLANK();
     DEFINE(PCPU_INFO_SIZE, sizeof(struct pcpu_info));
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 8f46f1a1de..9b4835960d 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -26,6 +26,7 @@
 #include <asm/sbi.h>
 #include <asm/setup.h>
 #include <asm/traps.h>
+#include <asm/vsbi.h>
 
 /* Xen stack for bringing up the first CPU. */
 unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
@@ -110,6 +111,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
 
     end_boot_allocator();
 
+    check_vsbi_ext_ranges();
+
     /*
      * The memory subsystem has been initialized, we can now switch from
      * early_boot -> boot.
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index f061004d83..5c3d1988d7 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -15,6 +15,7 @@
 #include <asm/processor.h>
 #include <asm/riscv_encoding.h>
 #include <asm/traps.h>
+#include <asm/vsbi.h>
 
 /*
  * Initialize the trap handling.
@@ -114,6 +115,13 @@ void do_trap(struct cpu_user_regs *cpu_regs)
 
     switch ( cause )
     {
+    case CAUSE_VIRTUAL_SUPERVISOR_ECALL:
+        /* CAUSE_VIRTUAL_SUPERVISOR_ECALL should come from VS-mode */
+        BUG_ON(!(cpu_regs->hstatus & HSTATUS_SPV));
+
+        vsbi_handle_ecall(current, cpu_regs);
+        break;
+
     case CAUSE_ILLEGAL_INSTRUCTION:
         if ( do_bug_frame(cpu_regs, pc) >= 0 )
         {
diff --git a/xen/arch/riscv/vsbi/Makefile b/xen/arch/riscv/vsbi/Makefile
new file mode 100644
index 0000000000..820eb10ac2
--- /dev/null
+++ b/xen/arch/riscv/vsbi/Makefile
@@ -0,0 +1 @@
+obj-y += core.o
diff --git a/xen/arch/riscv/vsbi/core.c b/xen/arch/riscv/vsbi/core.c
new file mode 100644
index 0000000000..5ac4fee379
--- /dev/null
+++ b/xen/arch/riscv/vsbi/core.c
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/sched.h>
+
+#include <asm/processor.h>
+#include <asm/sbi.h>
+#include <asm/vsbi.h>
+
+extern const struct vsbi_ext _svsbi_exts[], _evsbi_exts[];
+
+void __init check_vsbi_ext_ranges(void)
+{
+    for ( const struct vsbi_ext *a = _svsbi_exts; a != _evsbi_exts; a++ )
+        for ( const struct vsbi_ext *b = a + 1; b != _evsbi_exts; b++ )
+            if ( !(a->eid_end < b->eid_start || b->eid_end < a->eid_start) )
+                panic("EID range overlap detected: "
+                      "%s:[#%#lx..#%#lx] vs %s:[#%#lx..#%#lx]\n",
+                      a->name, a->eid_start, a->eid_end,
+                      b->name, b->eid_start, b->eid_end);
+}
+
+const struct vsbi_ext *vsbi_find_extension(unsigned long eid)
+{
+    const struct vsbi_ext *ext;
+
+    for ( ext = _svsbi_exts; ext != _evsbi_exts; ext++ )
+        if ( (eid >= ext->eid_start) && (eid <= ext->eid_end) )
+            return ext;
+
+    return NULL;
+}
+
+void vsbi_handle_ecall(struct vcpu *vcpu, struct cpu_user_regs *regs)
+{
+    const unsigned long eid = regs->a7;
+    const unsigned long fid = regs->a6;
+    const struct vsbi_ext *ext = vsbi_find_extension(eid);
+    int ret;
+
+    if ( ext )
+        ret = ext->handler(vcpu, eid, fid, regs);
+    else
+    {
+        printk("Unsupported Guest SBI EID #%#lx, FID #%lu\n", eid, regs->a1);
+        ret = SBI_ERR_NOT_SUPPORTED;
+    }
+
+    /*
+     * The ecall instruction is not part of the RISC-V C extension (compressed
+     * instructions), so it is always 4 bytes long. Therefore, it is safe to
+     * use a fixed length of 4 bytes instead of reading guest memory to
+     * determine the instruction length.
+     */
+    regs->sepc += 4;
+    regs->a0 = ret;
+}
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index 45d2e053d0..331a7d63d3 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -61,6 +61,13 @@ SECTIONS
         __note_gnu_build_id_end = .;
     } :note :text
     #endif
+
+    . = ALIGN(POINTER_ALIGN);
+    DECL_SECTION(.vsbi.exts) {
+        _svsbi_exts = .;
+        *(.vsbi.exts)
+        _evsbi_exts = .;
+    } :text
     _erodata = .;                /* End of read-only data */
 
     . = ALIGN(PAGE_SIZE);
-- 
2.52.0

On 17.12.2025 17:54, Oleksii Kurochko wrote:
> This commit introduces support for handling virtual SBI extensions in Xen.

Nit: As before, no "this commit" in commit messages, please.

> The changes include:
> - Added new vsbi/core.c and vsbi.h files to implement virtual SBI extension
>   handling.
> - Modified traps.c to handle CAUSE_VIRTUAL_SUPERVISOR_ECALL by calling
>   vsbi_handle_ecall() when the trap originates from VS-mode.
> - Updated xen.lds.S to include a new .vsbi.exts section for virtual SBI
>   extension data.
> - Updated Makefile to include the new vsbi/ directory in the build.
> - Add hstatus register to struct cpu_user_regs as it is needed for
>   a check that CAUSE_VIRTUAL_SUPERVISOR_ECALL happens from VS-mode.
>   Also, add storing/restoring of hstatus register in handle_trap().
> - Introduce vsbi_find_extension() to check if vsbi extension is supported
>   by Xen. For now it is called only inside vsbi/core.c, but in future
>   it is going to be called from other files.
> - Introduce check_vsbi_ext_ranges() to check if there EIDs ranges
>   overlapping between extensions.
> 
> The implementation allows for registration and handling of SBI
> extensions via a new vsbi_ext structure and ".vsbi.exts" section,
> enabling extensible virtual SBI support for RISC-V guests.
> 
> Note: All EIDs are printed in the format #%#lx and all FIDs in #%#lu, as
> the SBI spec uses these formats. Printing them this way makes it easier to
> search for them in the SBI spec.

Luckily the oddity exists only here: I don't think '#' would be very
useful with 'u'.

Jan

On 17.12.2025 17:54, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/vsbi/core.c
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/sched.h>
> +
> +#include <asm/processor.h>
> +#include <asm/sbi.h>
> +#include <asm/vsbi.h>
> +
> +extern const struct vsbi_ext _svsbi_exts[], _evsbi_exts[];
> +
> +void __init check_vsbi_ext_ranges(void)
> +{
> +    for ( const struct vsbi_ext *a = _svsbi_exts; a != _evsbi_exts; a++ )
> +        for ( const struct vsbi_ext *b = a + 1; b != _evsbi_exts; b++ )

Like here, ...

> +            if ( !(a->eid_end < b->eid_start || b->eid_end < a->eid_start) )
> +                panic("EID range overlap detected: "
> +                      "%s:[#%#lx..#%#lx] vs %s:[#%#lx..#%#lx]\n",
> +                      a->name, a->eid_start, a->eid_end,
> +                      b->name, b->eid_start, b->eid_end);
> +}
> +
> +const struct vsbi_ext *vsbi_find_extension(unsigned long eid)
> +{
> +    const struct vsbi_ext *ext;
> +
> +    for ( ext = _svsbi_exts; ext != _evsbi_exts; ext++ )

... declare "ext" inside the for()?

> +        if ( (eid >= ext->eid_start) && (eid <= ext->eid_end) )
> +            return ext;
> +
> +    return NULL;
> +}
> +
> +void vsbi_handle_ecall(struct vcpu *vcpu, struct cpu_user_regs *regs)
> +{
> +    const unsigned long eid = regs->a7;
> +    const unsigned long fid = regs->a6;
> +    const struct vsbi_ext *ext = vsbi_find_extension(eid);
> +    int ret;
> +
> +    if ( ext )
> +        ret = ext->handler(vcpu, eid, fid, regs);
> +    else
> +    {
> +        printk("Unsupported Guest SBI EID #%#lx, FID #%lu\n", eid, regs->a1);

As before - anything guest triggered must not cause log spam issues.
Minimally you want to use XENLOG_GUEST in such cases, but I think you
really mean gprintk() here.

A connected question then arises: Why is "vcpu" being passed in, when
the sole caller only ever passes "current"? (The connection here is
that gprintk() also uses current, and hence would be wrong to use when
vcpu != current.) Same question goes for the ->handler() hook.

Jan

On 12/18/25 2:52 PM, Jan Beulich wrote:
> On 17.12.2025 17:54, Oleksii Kurochko wrote:
>> --- /dev/null
>> +++ b/xen/arch/riscv/vsbi/core.c
>> @@ -0,0 +1,56 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#include <xen/sched.h>
>> +
>> +#include <asm/processor.h>
>> +#include <asm/sbi.h>
>> +#include <asm/vsbi.h>
>> +
>> +extern const struct vsbi_ext _svsbi_exts[], _evsbi_exts[];
>> +
>> +void __init check_vsbi_ext_ranges(void)
>> +{
>> +    for ( const struct vsbi_ext *a = _svsbi_exts; a != _evsbi_exts; a++ )
>> +        for ( const struct vsbi_ext *b = a + 1; b != _evsbi_exts; b++ )
> Like here, ...
>
>> +            if ( !(a->eid_end < b->eid_start || b->eid_end < a->eid_start) )
>> +                panic("EID range overlap detected: "
>> +                      "%s:[#%#lx..#%#lx] vs %s:[#%#lx..#%#lx]\n",
>> +                      a->name, a->eid_start, a->eid_end,
>> +                      b->name, b->eid_start, b->eid_end);
>> +}
>> +
>> +const struct vsbi_ext *vsbi_find_extension(unsigned long eid)
>> +{
>> +    const struct vsbi_ext *ext;
>> +
>> +    for ( ext = _svsbi_exts; ext != _evsbi_exts; ext++ )
> ... declare "ext" inside the for()?

I decided that it would be better then the following alignment of for loop:
     for ( const struct vsbi_ext *ext = _svsbi_exts;
           ext != _evsbi_exts;
           ext++ )

I am okay to change that to be aligned with the code above.

>
>> +        if ( (eid >= ext->eid_start) && (eid <= ext->eid_end) )
>> +            return ext;
>> +
>> +    return NULL;
>> +}
>> +
>> +void vsbi_handle_ecall(struct vcpu *vcpu, struct cpu_user_regs *regs)
>> +{
>> +    const unsigned long eid = regs->a7;
>> +    const unsigned long fid = regs->a6;
>> +    const struct vsbi_ext *ext = vsbi_find_extension(eid);
>> +    int ret;
>> +
>> +    if ( ext )
>> +        ret = ext->handler(vcpu, eid, fid, regs);
>> +    else
>> +    {
>> +        printk("Unsupported Guest SBI EID #%#lx, FID #%lu\n", eid, regs->a1);
> As before - anything guest triggered must not cause log spam issues.
> Minimally you want to use XENLOG_GUEST in such cases, but I think you
> really mean gprintk() here.
>
> A connected question then arises: Why is "vcpu" being passed in, when
> the sole caller only ever passes "current"? (The connection here is
> that gprintk() also uses current, and hence would be wrong to use when
> vcpu != current.) Same question goes for the ->handler() hook.

Good question. I tried to keep this more generic, but after your comment I
cannot find a case where this argument is actually used with something
other than|current in downstream code.|

Let’s drop the|vcpu| argument for now, and if it is needed in the future,
we can reintroduce it then.

Thanks.

~ Oleksii