On 11.02.2025 10:53, Oleksii Kurochko wrote:
> On 2/10/25 5:19 PM, Jan Beulich wrote:
>> On 07.02.2025 21:07, Oleksii Kurochko wrote:
>>> +/*
>>> + * The canonical order of ISA extension names in the ISA string is defined
>>> in
>>> + * chapter 27 of the unprivileged specification.
>>> + *
>>> + * The specification uses vague wording, such as should, when it comes to
>>> + * ordering, so for our purposes the following rules apply:
>>> + *
>>> + * 1. All multi-letter extensions must be separated from other extensions
>>> by an
>>> + * underscore.
>>> + *
>>> + * 2. Additional standard extensions (starting with 'Z') must be sorted
>>> after
>>> + * single-letter extensions and before any higher-privileged extensions.
>>> + *
>>> + * 3. The first letter following the 'Z' conventionally indicates the most
>>> + * closely related alphabetical extension category, IMAFDQLCBKJTPVH.
>>> + * If multiple 'Z' extensions are named, they must be ordered first by
>>> + * category, then alphabetically within a category.
>>> + *
>>> + * 4. Standard supervisor-level extensions (starting with 'S') must be
>>> listed
>>> + * after standard unprivileged extensions. If multiple supervisor-level
>>> + * extensions are listed, they must be ordered alphabetically.
>>> + *
>>> + * 5. Standard machine-level extensions (starting with 'Zxm') must be
>>> listed
>>> + * after any lower-privileged, standard extensions. If multiple
>>> + * machine-level extensions are listed, they must be ordered
>>> + * alphabetically.
>>> + *
>>> + * 6. Non-standard extensions (starting with 'X') must be listed after all
>>> + * standard extensions. If multiple non-standard extensions are listed,
>>> they
>>> + * must be ordered alphabetically.
>>> + *
>>> + * An example string following the order is:
>>> + * rv64imadc_zifoo_zigoo_zafoo_sbar_scar_zxmbaz_xqux_xrux
>>> + *
>>> + * New entries to this struct should follow the ordering rules described
>>> above.
>>> + *
>>> + * Extension name must be all lowercase (according to device-tree binding)
>>> + * 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_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(c, RISCV_ISA_EXT_c),
>>> + 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(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),
>> Why would these last four (Zifencei below) not be included in #ifdef
>> CONFIG_RISCV_ISA_RV64G, just like ...
>>
>>> +#ifdef CONFIG_RISCV_ISA_C
>>> + RISCV_ISA_EXT_DATA(c, RISCV_ISA_EXT_c),
>>> +#endif
>> .. this one is?
>
> I'm not sure if it was the best decision, but I did it this way because
> I believe other bitnesses (32, 128) will also need G. So, I left them
> without|#ifdef| to avoid adding|#ifdef CONFIG_RV{32,128}G| in the future.
That's fine, but then imo RISCV_ISA_RV64G ought to be dropped, and we use
G unconditionally. Whether that's a good move I don't know. I could
imagine embedded use cases which want to run an very simple processors.
> I also spent some time considering whether 'f' and 'd' are necessary
> for Xen. In the end, I decided that if they aren't needed for Xen,
> it might be better not to use "G" for compilation and instead explicitly
> specify "ima". But it will be better to do as a separate patch (if it
> makes sense).
That's certainly an option; use of floating point registers / insns will
need suppressing one way or another anyway, sooner or later. And yes, I
agree this wants to be a separate change. Even their relative order is
not important, as long as things remain consistent at any point in time.
Jan
