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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.12.2025 17:54, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/include/asm/sbi.h
> +++ b/xen/arch/riscv/include/asm/sbi.h
> @@ -14,6 +14,10 @@
>  
>  #include <xen/cpumask.h>
>  
> +#define XEN_SBI_VER_MAJOR 0
> +#define XEN_SBI_VER_MINOR 2
> +#define XEN_SBI_IMPID 7

Are these numbers part of the spec (sorry, lack of a reference makes me wonder,
plus if that were the case, I'd kind of expect the names to be SBI_XEN_..., not
XEN_SBI_...)?

> --- /dev/null
> +++ b/xen/arch/riscv/vsbi/base-extension.c
> @@ -0,0 +1,71 @@
> +
> +/* 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>
> +
> +static int vsbi_base_ecall_handler(struct vcpu *vcpu, 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 ) {

Nit: Brace placement.

> +    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;
> +    case SBI_EXT_BASE_GET_IMP_ID:
> +        regs->a1 = XEN_SBI_IMPID;
> +        break;
> +    case SBI_EXT_BASE_GET_IMP_VERSION:
> +        regs->a1 = (xen_major_version() << 16) | xen_minor_version();
> +        break;

Along those lines here - are we free to use an arbitrary layout (shifting major 
by
16 bits), or is this mandated by the spec? At least in the latter case, the 16 
will
want to gain a #define.

Also - blank lines please between non-fall-through case blocks.

Jan