Re: [PATCH v7 04/20] xen/riscv: introduce guest riscv,isa string

On 8/13/26 5:43 PM, Jan Beulich wrote:
> On 13.08.2026 17:37, Oleksii Kurochko wrote:
>> On 8/13/26 9:19 AM, Jan Beulich wrote:
>>> On 04.08.2026 17:47, Oleksii Kurochko wrote:
>>>> @@ -120,29 +148,30 @@ static int __init dt_get_cpuid_from_node(const struct dt_device_node *cpu,
>>>>     * and strncmp() is used in match_isa_ext() to compare extension names instead
>>>>     * of strncasecmp().
>>>>     */
>>>> -const struct riscv_isa_ext_data __initconst riscv_isa_ext[] = {
>>>> -    RISCV_ISA_EXT_DATA(i),
>>>> -    RISCV_ISA_EXT_DATA(m),
>>>> -    RISCV_ISA_EXT_DATA(a),
>>>> -    RISCV_ISA_EXT_DATA(f),
>>>> -    RISCV_ISA_EXT_DATA(d),
>>>> -    RISCV_ISA_EXT_DATA(q),
>>>> -    RISCV_ISA_EXT_DATA(c),
>>>> -    RISCV_ISA_EXT_DATA(h),
>>>> -    RISCV_ISA_EXT_DATA(zicntr),
>>>> -    RISCV_ISA_EXT_DATA(zicsr),
>>>> -    RISCV_ISA_EXT_DATA(zifencei),
>>>> -    RISCV_ISA_EXT_DATA(zihintpause),
>>>> -    RISCV_ISA_EXT_DATA(zihpm),
>>>> -    RISCV_ISA_EXT_DATA(zba),
>>>> -    RISCV_ISA_EXT_DATA(zbb),
>>>> -    RISCV_ISA_EXT_DATA(zbs),
>>>> -    RISCV_ISA_EXT_DATA(smaia),
>>>> -    RISCV_ISA_EXT_DATA(smstateen),
>>>> -    RISCV_ISA_EXT_DATA(ssaia),
>>>> -    RISCV_ISA_EXT_DATA(sstc),
>>>> -    RISCV_ISA_EXT_DATA(svade),
>>>> -    RISCV_ISA_EXT_DATA(svpbmt),
>>>> +static const struct riscv_isa_ext_entry __initconstrel riscv_isa_ext[] = {
>>>> +    RISCV_ISA_EXT_ENTRY(i,            true),
>>>> +    RISCV_ISA_EXT_ENTRY(m,            true),
>>>> +    RISCV_ISA_EXT_ENTRY(a,            true),
>>>> +    RISCV_ISA_EXT_ENTRY(f,            false),
>>>> +    RISCV_ISA_EXT_ENTRY(d,            false),
>>>> +    RISCV_ISA_EXT_ENTRY(q,            false),
>>>> +    RISCV_ISA_EXT_ENTRY(c,            true),
>>>> +    RISCV_ISA_EXT_ENTRY(v,            false),
>>>> +    RISCV_ISA_EXT_ENTRY(h,            false),
>>>> +    RISCV_ISA_EXT_ENTRY(zicntr,       true),
>>>> +    RISCV_ISA_EXT_ENTRY(zicsr,        true),
>>>> +    RISCV_ISA_EXT_ENTRY(zifencei,     true),
>>>> +    RISCV_ISA_EXT_ENTRY(zihintpause,  true),
>>>> +    RISCV_ISA_EXT_ENTRY(zihpm,        true),
>>>> +    RISCV_ISA_EXT_ENTRY(zba,          true),
>>>> +    RISCV_ISA_EXT_ENTRY(zbb,          true),
>>>> +    RISCV_ISA_EXT_ENTRY(zbs,          true),
>>>> +    RISCV_ISA_EXT_ENTRY(smaia,        true),
>>>> +    RISCV_ISA_EXT_ENTRY(smstateen,    true),
>>>> +    RISCV_ISA_EXT_ENTRY(ssaia,        true),
>>>> +    RISCV_ISA_EXT_ENTRY(sstc,         false),
>>>> +    RISCV_ISA_EXT_ENTRY(svade,        false),
>>>> +    RISCV_ISA_EXT_ENTRY(svpbmt,       false),
>>>>    };
>>>
>>> Just as an independent, up front remark after having looked at patch 16/17 of
>>> the other series: Is a mere boolean going to suffice in the longer run? I could
>>> see some extensions wanting exposing to only RV32 or only RV64 guests. E.g.
>>> Zilsd is RV32-only, while Zqinx quite likely would want restricting to RV64.
>>
>> Good point generally.
>>
>> Right now the distinction can't be observed: RV32 isn't buildable
>> (#error "RV32 isn't supported" in asm/config.h), and guest XLEN is
>> hard-wired to host XLEN — build_guest_isa_str() emits the rv32/rv64
>> prefix from the Kconfig symbol, and riscv_isa_parse_string() rejects a
>> host ISA string of the other width.
>>
>> On top of that, extensions with an architectural XLEN restriction are
>> already filtered out for free: compute_guest_isa() masks the table
>> against the host bitmap, so an RV32-only extension like Zilsd can't have
>> its bit set on an RV64 build regardless of what the table says. The
>> boolean only ever subtracts from what the host actually reports.
>>
>> That leaves purely policy-driven per-XLEN restrictions — e.g. exposing
>> Zqinx to RV64 guests but not RV32 ones, since Zqinx is architecturally
>> defined for both.
> 
> Is it? Can you point me at a spec, as I wasn't able to find any?

Well, it is not defined now but in the spec 
(unpriv-isa-asciidoc_20240411.pdf) it is mentioned:

In the future, an RV64Zqinx quad-precision extension could be defined 
analogously to RV32Zdinx. An RV32Zqinx extension could also be defined 
but would require quad-register groups.

> 
>> Those only become meaningful once guest XLEN can
>> differ from host XLEN (i.e. hstatus.VSXL support), and at that point the
>> shared guest_isa bitmap has to become per-domain as well, as the comment
>> above it already notes.
> 
> Not necessarily - you could have an RV64 one and an RV32 one.>
>> So I'd rather keep the plain bool for now and widen it to a flags field
>> when there's an actual case; it's a mechanical change to the struct, the
>> macro and the single test in compute_guest_isa(), all local to
>> cpufeature.c. I can add a comment stating the "guest XLEN == host XLEN"
>> assumption so the reason is on record. If you'd prefer it as flags from
>> the start I don't mind doing it now either. I just don't have a way to
>> give either value a meaning yet. So if to do that now I would suggest
>> the following:
> 
> I'm fine with a comment, and I'd also be fine with the more extensive
> logic. Main question being whether "guest XLEN < host XLEN" support is
> meant to be added within the foreseeable future.

We don't have such plans at the moment so it won't be in the nearest 
future even in downstream.

~ Oleksii