On 11.02.2025 16:28, Oleksii Kurochko wrote:
>
> On 2/11/25 11:01 AM, Jan Beulich wrote:
>> 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.
>
> Actually, I think that we should drop 'f' and 'd' from required_extensions[]
> array as they aren't really needed for Xen. Or make them conditional just to
> be sure that if "G" was used for compilation and the code with using of them
> was generated then they are really supported by a h/w.
As said, that's okay. But as also said you then need to also keep the
compiler from potentially using F or D insns / registers.
> Regarding #ifdef-ing with RISCV_ISA_RV64G, I think that we have to keep them
> mentioned unconditionally as 'i', 'm', 'a', 'zicsr' and 'zifencei' which are
> part of 'G' as all of them are needed by Xen to work.
Yet then why do we have CONFIG_RISCV_ISA_RV64G?
Jan
