Re: [PATCH v5 09/26] xen/riscv: introduce guest riscv,isa string

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.07.2026 17:00, Oleksii Kurochko wrote:
> On 7/9/26 3:05 PM, Jan Beulich wrote:
>> On 06.07.2026 17:57, Oleksii Kurochko wrote:
>>> +void init_guest_isa(struct domain *d)
>>> +{
>>> +    for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
>>> +    {
>>> +        const struct riscv_isa_ext_entry *ext = &riscv_isa_ext[i];
>>> +
>>> +        if ( ext->guest_supported &&
>>> +             riscv_isa_extension_available(NULL, ext->id) )
>>> +            __set_bit(ext->id, d->arch.isa);
>>> +    }
>>> +}
>>
>> Right now what this function does is dependent on only global variables.
>> IOW each guest gets the same bitmap. Is this going to change soon?
> 
> At the moment, we don't have a use case where domain has unique bitmap. 
> But IIRC correctly we agreed that generally it is good idea to have isa 
> bitmap per domain and let the use to chose.

Indeed. But what you're doing (in this series alone) is a mix of things:
Some properties are per-domain, some are global. Such a mix might be okay
if it's justified by something. Otherwise I think consistency it to be
valued higher.

>   Else
>> why not calculate that bitmap once, taking the same shortcut as you take
>> elsewhere for the time being?
> 
> static __ro_after_init DECLARE_BITMAP(guest_isa, RISCV_ISA_EXT_MAX);
> 
> /* called once from riscv_fill_hwcap() or similar */
> void __init init_guest_isa_mask(void)
> {
>      for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
>          if ( riscv_isa_ext[i].guest_supported &&
>               riscv_isa_extension_available(NULL, riscv_isa_ext[i].id) )
>              __set_bit(riscv_isa_ext[i].id, guest_isa);
> }
> 
> void init_guest_isa(struct domain *d)   /* called per-domain */
> {
>      bitmap_copy(d->arch.isa, guest_isa, RISCV_ISA_EXT_MAX);
> }
> 
> Do you mean something like that?

Yes. Not necessarily with bitmap_copy() though; perhaps with a pointer in
struct domain.

>   Then allowing riscv_isa_ext[] to remain
>> __initconst (should really have been __initconstrel).
> 
> I think you explained me already why but I forgot. Could you please 
> remind me again why __initconstrel should be here? Is it because of 
> pointers used inside struct riscv_isa_ext_entry?

Yes, pointers incur relocations, and the need for relocations requires
that the compiler emit the data to a writable section. As opposed to
relocation-free data, which (when declared const) can go into a r/o
section.

Jan