Re: [PATCH v3 1/3] xen/riscv: introduce vSBI extension framework

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.12.2025 17:37, Oleksii Kurochko wrote:
> 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@xxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

I'd like to note though that ...

> --- /dev/null
> +++ b/xen/arch/riscv/vsbi/core.c
> @@ -0,0 +1,57 @@
> +/* 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)
> +{
> +    for ( const struct vsbi_ext *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 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(eid, fid, regs);
> +    else
> +    {
> +        gprintk(XENLOG_ERR, "Unsupported Guest SBI EID #%#lx, FID #%lu\n",
> +                eid, regs->a1);

... I still consider this too verbose. Before switching RISC-V into "supported"
status, changing this and alike to gdprintk() may want considering.

Jan