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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.05.2026 16:49, Oleksii Kurochko wrote:
> On 5/19/26 3:40 PM, Jan Beulich wrote:
>> On 19.05.2026 15:24, Oleksii Kurochko wrote:
>>> 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.
>>
>> This might be yet better with only a single function. Otherwise the two are
>> always at risk of going out of sync. After all you can use snprintf() to
>> determine just the size needed; if you go look, there may even be an
>> example or two in the tree.
> 
> I will do than in the following way:
> 
> static int build_guest_isa_str(char *buf, size_t size,
>                                 const unsigned long *isa_bitmap)
> {
>      int total = 0;
>      int ret;
> 
> #if defined(CONFIG_RISCV_32)
>      ret = snprintf(buf, size, "rv32");
> #elif defined(CONFIG_RISCV_64)
>      ret = snprintf(buf, size, "rv64");
> #else
> #   error "Unsupported RISC-V bitness"
> #endif
>      if ( ret < 0 )
>          return ret;

You can use total here right away, and limit ...

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

... ret's scope to this loop. This then also justifies total to be of a
signed type.

>          if ( !riscv_isa_extension_available(isa_bitmap, ext->id) )
>              continue;
> 
>          ret = snprintf(buf ? buf + total : NULL,
>                         buf ? size - total : 0, "%s%s",
>                         ext->id >= RISCV_ISA_EXT_BASE ? "_" : "",
>                         ext->name);
>          if ( ret < 0 )
>              return ret;
>          total += ret;
>      }
> 
>      return total;
> }
> 
> int init_guest_isa(struct domain *d)
> {
>      int len;
> 
>      bitmap_andnot(d->arch.isa, riscv_isa, guest_unsupp,
>                    RISCV_ISA_EXT_MAX);
> 
>      len = build_guest_isa_str(NULL, 0, d->arch.isa);
>      if ( len < 0 )
>          return len;
> 
>      d->arch.isa_str = xmalloc_array(char, len + 1);
>      if ( !d->arch.isa_str )
>          return -ENOMEM;
> 
>      build_guest_isa_str(d->arch.isa_str, len + 1, d->arch.isa);

At least ASSERT() the success of this?

Jan