Re: [PATCH for 4-21 v4] xen/riscv: identify specific ISA supported by cpu

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.02.2025 21:00, Oleksii Kurochko wrote:
> 
> On 2/7/25 5:47 PM, Oleksii Kurochko wrote:
>>
>>
>> On 2/4/25 12:47 PM, Jan Beulich wrote:
>>>> +const struct riscv_isa_ext_data __initconst riscv_isa_ext[] = {
>>>> +    RISCV_ISA_EXT_DATA(i, RISCV_ISA_EXT_i),
>>>> +    RISCV_ISA_EXT_DATA(m, RISCV_ISA_EXT_m),
>>>> +    RISCV_ISA_EXT_DATA(a, RISCV_ISA_EXT_a),
>>>> +    RISCV_ISA_EXT_DATA(f, RISCV_ISA_EXT_f),
>>>> +    RISCV_ISA_EXT_DATA(d, RISCV_ISA_EXT_d),
>>>> +    RISCV_ISA_EXT_DATA(q, RISCV_ISA_EXT_q),
>>>> +    RISCV_ISA_EXT_DATA(h, RISCV_ISA_EXT_h),
>>>> +    RISCV_ISA_EXT_DATA(zicntr, RISCV_ISA_EXT_ZICNTR),
>>>> +    RISCV_ISA_EXT_DATA(zicsr, RISCV_ISA_EXT_ZICSR),
>>>> +    RISCV_ISA_EXT_DATA(zifencei, RISCV_ISA_EXT_ZIFENCEI),
>>>> +    RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
>>>> +    RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
>>>> +    RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
>>>> +    RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
>>>> +    RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
>>>> +};
>>>> +
>>>> +static const struct riscv_isa_ext_data __initconst required_extensions[] 
>>>> = {
>>>> +    RISCV_ISA_EXT_DATA(zicsr, RISCV_ISA_EXT_ZICSR),
>>>> +    RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
>>>> +    RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
>>>> +};
>>> Coming back to my earlier question regarding the B (pseudo-)extension:
>>> Since riscv_isa_ext[] only contains Zbb, is it precluded anywhere in
>>> the spec that DT may mention just B when all of its constituents are
>>> supported?
>>>
>>> Which gets me on to G, which is somewhat similar in nature to B. We
>>> require G when RISCV_ISA_RV64G=y, yet required_extensions[] doesn't
>>> name it or its constituents. Much like we require C when RISCV_ISA_C=y,
>>> yet it's not in the table.
>> Another one thing I am thinking about if we really need a separate 
>> required_extensions[] array.
>>
>> We can leave only riscv_isa_ext[] and then just do a check:
>>   bitmap_weight(riscv_isa, ...) == ARRAY_SIZE(riscv_isa_ext)
> 
> It seems like we still need to have two arrays: one for what Xen is supported 
> (and could be passed to guest
> by riscv,isa) and one for what is required for boot.

Well, you can get away with just one array, but only if adding a boolean
to struct riscv_isa_ext_data (indicating whether an extension is required).
I'm not sure though how well that would work overall.

Jan