Re: [PATCH v2 05/26] xen/riscv: introduce guest riscv,isa string

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 5/19/26 2:12 PM, Jan Beulich wrote:
> On 19.05.2026 13:59, Oleksii Kurochko wrote:
> > On 5/18/26 5:51 PM, Jan Beulich wrote:
> > > On 08.05.2026 16:43, Oleksii Kurochko wrote:
> > > > --- a/xen/arch/riscv/include/asm/cpufeature.h
> > > > +++ b/xen/arch/riscv/include/asm/cpufeature.h
> > > > @@ -17,6 +17,8 @@
> > > >     */
> > > >    #define RISCV_ISA_EXT_BASE  26
> > > >    
> > > > +#define RISCV_GUEST_ISA_STR_MAX 256
> > > This looks like it won't be good for very long, seeing how long ISA strings can
> > > get. I wonder anyway whether ...
> > >
> > > > @@ -94,6 +95,9 @@ struct arch_domain {
> > > >        struct p2m_domain p2m;
> > > >    
> > > >        struct paging_domain paging;
> > > > +
> > > > +    DECLARE_BITMAP(guest_isa, RISCV_ISA_EXT_MAX);
> > > > +    char guest_isa_str[RISCV_GUEST_ISA_STR_MAX];
> > > ... a compile-time sized buffer is suitable here. Can't you allocate a buffer
> > > just large enough to hold the string?
> > It could be allocated dynamically.
> >
> > Does it make sense to evaluate in run-time what should be a buffer size?
> > For this case I can't find analogue of realloc() in Xen.
> Hmm, I see xrealloc_array(), and surely we could gain xvrealloc_array()
> which we'll need anyway once xrealloc_array() uses get converted. (I also
> see x{,v}realloc_flex_struct(), but that's of no use here as it looks.)
Oh, I missed to turned off "full match" during search...

> > Or it would be
> > fine just to take something bigger as a const (lets say 2048) and use it
> > for dynamic allocation?
> I'd rather not. Can't you determine how much space the string is going to
> occupy?
I thought about two options as alternatives:

1. Take as a length host RISC-V ISA string but theoretically we can 
emulate some extensions which aren't mentioned in host RISC-V ISA string 
so it could be longer. So not a good option.
2. Having two walks in init_guest_isa():
   Introduce the following function:
static size_t guest_isa_str_len(const unsigned long *isa_bitmap)
{
    size_t len = 4; /* rvX prefix */

    for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
    {
        const struct riscv_isa_ext_data *ext = &riscv_isa_ext[i];

        if ( !riscv_isa_extension_available(isa_bitmap, ext->id) )
            continue;

        if ( ext->id >= RISCV_ISA_EXT_BASE )
            len++; /* '_' separator */

        len += strlen(ext->name);
    }

    return len + 1; /* NUL terminator */
}

  and then:

int init_guest_isa(struct domain *d)
{
    bitmap_andnot(d->arch.guest_isa, riscv_isa, guest_unsupp,
                  RISCV_ISA_EXT_MAX);

    size_t len = guest_isa_str_len(d->arch.guest_isa);
    d->arch.guest_isa_str = xzalloc_array(char, len);
    if ( !d->arch.guest_isa_str )
        return -ENOMEM;

    /* ... existing snprintf + strlcat loop unchanged ... */
}

If approach 2 is a good one I can follow it.

~ Oleksii