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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.01.2026 10:30, Oleksii Kurochko wrote:
> 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 fear there's a misunderstanding here, likely on my side: Why would it be 2.0?
Didn't you say the version is Xen controlled? If so, why not 0.3 or 1.0?

Contrary to what you said previously, it now looks to me as if the version
wasn't "Xen-controlled", but instead what we pick reflects functionality
required by a particular spec version of a spec we do not control. That's
"SBI version implemented by Xen" to me though, not really a "Xen-controlled"
version.

Jan

> 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