Re: [PATCH v4 1/1] xen/riscv: add RISC-V virtual SBI base extension support for guests

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 1/5/26 5:26 PM, Jan Beulich wrote:
> On 30.12.2025 16:50, Oleksii Kurochko wrote:
> > Add support of virtual SBI base extension calls for RISC-V guests, delegating
> > hardware-specific queries to the underlying SBI and handling version and
> > firmware ID queries directly.
> >
> > The changes include:
> > 1. Define new SBI base extension function IDs (SBI_EXT_BASE_GET_MVENDORID,
> >     SBI_EXT_BASE_GET_MARCHID, SBI_EXT_BASE_GET_MIMPID).
> > 2. Introduce XEN_SBI_VER_MAJOR, XEN_SBI_VER_MINOR for imeplenataion of
> >     SBI_EXT_BASE_GET_SPEC_VERSION.
> > 4. Introduce SBI_XEN_IMPID to implement SBI_EXT_BASE_GET_IMP_ID.
> > 5. Implement handling of SBI base extension functions, including version,
> >     firmware ID, and machine-specific queries.
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
Thanks.

> Albeit with a question:
>
> > --- /dev/null
> > +++ b/xen/arch/riscv/vsbi/base-extension.c
> > @@ -0,0 +1,82 @@
> > +
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#include <xen/lib.h>
> > +#include <xen/sched.h>
> > +#include <xen/version.h>
> > +
> > +#include <asm/processor.h>
> > +#include <asm/sbi.h>
> > +#include <asm/vsbi.h>
> > +
> > +/* Xen-controlled SBI version reported to guests */
> > +#define XEN_SBI_VER_MAJOR 0
> > +#define XEN_SBI_VER_MINOR 2
> Is it clear from whatever spec it is that is ...
>
> > +static int vsbi_base_ecall_handler(unsigned long eid, unsigned long fid,
> > +                                   struct cpu_user_regs *regs)
> > +{
> > +    int ret = 0;
> > +    struct sbiret sbi_ret;
> > +
> > +    ASSERT(eid == SBI_EXT_BASE);
> > +
> > +    switch ( fid )
> > +    {
> > +    case SBI_EXT_BASE_GET_SPEC_VERSION:
> > +        regs->a1 = MASK_INSR(XEN_SBI_VER_MAJOR, SBI_SPEC_VERSION_MAJOR_MASK) |
> > +                   XEN_SBI_VER_MINOR;
> > +        break;
> ... implied here (it's ..._SPEC_VERSION after all) under what conditions the
> version would need bumping and what effects this would have on existing (e.g.
> migrating-in) guests? Recall that ...
For example, sooner or later we will want to use the SBI DBCN (Debug Console
Extension) for early debug output for guests, as it provides an API to work with
strings instead of single characters. This will require bumping the SBI version
to 2.0.

I don’t think this should cause any migration issues. If a guest was fully 
booted
and running with Xen SBI version 0.2, it would continue to use the legacy 
extension
for early console output (or for hvc console which is using SBI calls in Linux 
for
the moment). If the guest was still in the initialization stage (before SBI
extensions were probed), it would simply use the newer SBI DBCN extension 
instead
of the Legacy one.

~ Oleksii

> > +    case SBI_EXT_BASE_GET_IMP_ID:
> > +        regs->a1 = SBI_XEN_IMPID;
> > +        break;
> > +
> > +    case SBI_EXT_BASE_GET_IMP_VERSION:
> > +        regs->a1 = (xen_major_version() << 16) | xen_minor_version();
> > +        break;
> > +
> > +    case SBI_EXT_BASE_GET_MVENDORID:
> > +    case SBI_EXT_BASE_GET_MARCHID:
> > +    case SBI_EXT_BASE_GET_MIMPID:
> > +        if ( is_hardware_domain(current->domain) )
> > +        {
> > +            sbi_ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0);
> > +            ret = sbi_ret.error;
> > +            regs->a1 = sbi_ret.value;
> > +        }
> > +        else
> > +            /*
> > +             * vSBI should present a consistent, virtualized view to guests.
> > +             * In particular, DomU-visible data must remain stable across
> > +             * migration and must not expose hardware-specific details.
> ... what is being said here applies to other sub-functions as well. IOW it
> looks to me as if the version reported needs to be a per-guest property.
>
> Jan