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

[Jan Beulich <jbeulich@xxxxxxxx>]

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