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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 12/18/25 2:52 PM, Jan Beulich wrote:
> 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()?
I decided that it would be better then the following alignment of for loop:
    for ( const struct vsbi_ext *ext = _svsbi_exts;
          ext != _evsbi_exts;
          ext++ )

I am okay to change that to be aligned with the code above.

> > +        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.
Good question. I tried to keep this more generic, but after your comment I
cannot find a case where this argument is actually used with something
other than|current in downstream code.|

Let’s drop the|vcpu| argument for now, and if it is needed in the future,
we can reintroduce it then.

Thanks.

~ Oleksii