From 99e2266f2460e5778560f81982b6301dd2a16502 Mon Sep 17 00:00:00 2001 From: Conor Dooley Date: Mon, 5 Dec 2022 14:45:24 +0000 Subject: [PATCH 1/3] RISC-V: clarify ISA string ordering rules in cpu.c While the current list of rules may have been accurate when created it now lacks some clarity in the face of isa-manual updates. Instead of trying to continuously align this rule-set with the one in the specifications, change the role of this comment. This particular comment is important, as the array it "decorates" defines the order in which the ISA string appears to userspace in /proc/cpuinfo. Re-jig and strengthen the wording to provide contributors with a set order in which to add entries & note why this particular struct needs more attention than others. While in the area, add some whitespace and tweak some wording for readability's sake. Suggested-by: Andrew Jones Reviewed-by: Andrew Jones Signed-off-by: Conor Dooley Link: https://lore.kernel.org/r/20221205144525.2148448-2-conor.dooley@microchip.com Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/cpu.c | 49 ++++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index fa427bdcf773d..4480c2833ecc8 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -120,22 +120,45 @@ device_initcall(riscv_cpuinfo_init); .uprop = #UPROP, \ .isa_ext_id = EXTID, \ } + /* - * Here are the ordering rules of extension naming defined by RISC-V - * specification : - * 1. All extensions should be separated from other multi-letter extensions - * by an underscore. - * 2. The first letter following the 'Z' conventionally indicates the most + * The canonical order of ISA extension names in the ISA string is defined in + * chapter 27 of the unprivileged specification. + * + * Ordinarily, for in-kernel data structures, this order is unimportant but + * isa_ext_arr defines the order of the ISA string in /proc/cpuinfo. + * + * 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 should be ordered first - * by category, then alphabetically within a category. - * 3. Standard supervisor-level extensions (starts with 'S') should be - * listed after standard unprivileged extensions. If multiple - * supervisor-level extensions are listed, they should be ordered + * If multiple 'Z' extensions are named, they must be ordered first by + * category, then alphabetically within a category. + * + * 3. 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. + * + * 4. 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. - * 4. Non-standard extensions (starts with 'X') must be listed after all - * standard extensions. They must be separated from other multi-letter - * extensions by an underscore. + * + * 5. 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. */ static struct riscv_isa_ext_data isa_ext_arr[] = { __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), From 80c200b34ee8a0a3378d2073bd8eaae09651c60e Mon Sep 17 00:00:00 2001 From: Conor Dooley Date: Mon, 5 Dec 2022 14:45:25 +0000 Subject: [PATCH 2/3] RISC-V: resort all extensions in consistent orders Ordering between each and every list of extensions is wildly inconsistent. Per discussion on the lists pick the following policy: - The array defining order in /proc/cpuinfo follows a narrow interpretation of the ISA specifications, described in a comment immediately presiding it. - All other lists of extensions are sorted alphabetically. This will hopefully allow for easier review & future additions, and reduce conflicts between patchsets as the number of extensions grows. Link: https://lore.kernel.org/all/20221129144742.2935581-2-conor.dooley@microchip.com/ Suggested-by: Andrew Jones Reviewed-by: Andrew Jones Reviewed-by: Heiko Stuebner Signed-off-by: Conor Dooley Link: https://lore.kernel.org/r/20221205144525.2148448-3-conor.dooley@microchip.com Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/hwcap.h | 12 +++++++----- arch/riscv/kernel/cpu.c | 4 ++-- arch/riscv/kernel/cpufeature.c | 6 ++++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index b225252900730..ce522aad641a2 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -51,14 +51,15 @@ extern unsigned long elf_hwcap; * RISCV_ISA_EXT_MAX. 0-25 range is reserved for single letter * extensions while all the multi-letter extensions should define the next * available logical extension id. + * Entries are sorted alphabetically. */ enum riscv_isa_ext_id { RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, + RISCV_ISA_EXT_SSTC, + RISCV_ISA_EXT_SVINVAL, RISCV_ISA_EXT_SVPBMT, RISCV_ISA_EXT_ZICBOM, RISCV_ISA_EXT_ZIHINTPAUSE, - RISCV_ISA_EXT_SSTC, - RISCV_ISA_EXT_SVINVAL, RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, }; @@ -66,11 +67,12 @@ enum riscv_isa_ext_id { * This enum represents the logical ID for each RISC-V ISA extension static * keys. We can use static key to optimize code path if some ISA extensions * are available. + * Entries are sorted alphabetically. */ enum riscv_isa_ext_key { RISCV_ISA_EXT_KEY_FPU, /* For 'F' and 'D' */ - RISCV_ISA_EXT_KEY_ZIHINTPAUSE, RISCV_ISA_EXT_KEY_SVINVAL, + RISCV_ISA_EXT_KEY_ZIHINTPAUSE, RISCV_ISA_EXT_KEY_MAX, }; @@ -90,10 +92,10 @@ static __always_inline int riscv_isa_ext2key(int num) return RISCV_ISA_EXT_KEY_FPU; case RISCV_ISA_EXT_d: return RISCV_ISA_EXT_KEY_FPU; - case RISCV_ISA_EXT_ZIHINTPAUSE: - return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; case RISCV_ISA_EXT_SVINVAL: return RISCV_ISA_EXT_KEY_SVINVAL; + case RISCV_ISA_EXT_ZIHINTPAUSE: + return RISCV_ISA_EXT_KEY_ZIHINTPAUSE; default: return -EINVAL; } diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index 4480c2833ecc8..b8127bfc8f0fd 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -161,12 +161,12 @@ device_initcall(riscv_cpuinfo_init); * New entries to this struct should follow the ordering rules described above. */ static struct riscv_isa_ext_data isa_ext_arr[] = { + __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), + __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC), __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), - __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), - __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX), }; diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 694267d1fe814..8a76a6ce70cf3 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -199,12 +199,13 @@ void __init riscv_fill_hwcap(void) this_hwcap |= isa2hwcap[(unsigned char)(*ext)]; set_bit(*ext - 'a', this_isa); } else { + /* sorted alphabetically */ SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); + SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC); + SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL); SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM); SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); - SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC); - SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL); } #undef SET_ISA_EXT_MAP } @@ -284,6 +285,7 @@ static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage) * This code may also be executed before kernel relocation, so we cannot use * addresses generated by the address-of operator as they won't be valid in * this context. + * Tests, unless otherwise required, are to be added in alphabetical order. */ static u32 __init_or_module cpufeature_probe(unsigned int stage) { From f07b2b3f9d47fea308af3ae05613b6b4801e68a3 Mon Sep 17 00:00:00 2001 From: Conor Dooley Date: Mon, 5 Dec 2022 14:45:26 +0000 Subject: [PATCH 3/3] Documentation: riscv: add a section about ISA string ordering in /proc/cpuinfo The RISC-V specs are permissive in what they allow as the ISA string, but how we output this to userspace in /proc/cpuinfo is quasi uABI. Formalise this as part of the uABI, by documenting the list of rules we use at this point in time. Signed-off-by: Conor Dooley Link: https://lore.kernel.org/r/20221205144525.2148448-4-conor.dooley@microchip.com Signed-off-by: Palmer Dabbelt --- Documentation/riscv/uabi.rst | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/Documentation/riscv/uabi.rst b/Documentation/riscv/uabi.rst index 21a82cfb6c4dd..2ebec4c522308 100644 --- a/Documentation/riscv/uabi.rst +++ b/Documentation/riscv/uabi.rst @@ -3,4 +3,46 @@ RISC-V Linux User ABI ===================== +ISA string ordering in /proc/cpuinfo +------------------------------------ + +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: + +#. Single-letter extensions come first, in canonical order. + The canonical order is "IMAFDQLCBKJTPVH". + +#. All multi-letter extensions will be separated from other extensions by an + underscore. + +#. Additional standard extensions (starting with 'Z') will be sorted after + single-letter extensions and before any higher-privileged extensions. + +#. For additional standard extensions, the first letter following the 'Z' + conventionally indicates the most closely related alphabetical + extension category. If multiple 'Z' extensions are named, they will be ordered + first by category, in canonical order, as listed above, then alphabetically + within a category. + +#. Standard supervisor-level extensions (starting with 'S') will be listed + after standard unprivileged extensions. If multiple supervisor-level + extensions are listed, they will be ordered alphabetically. + +#. Standard machine-level extensions (starting with 'Zxm') will be listed + after any lower-privileged, standard extensions. If multiple machine-level + extensions are listed, they will be ordered alphabetically. + +#. Non-standard extensions (starting with 'X') will be listed after all standard + extensions. If multiple non-standard extensions are listed, they will be + ordered alphabetically. + +An example string following the order is:: + + rv64imadc_zifoo_zigoo_zafoo_sbar_scar_zxmbaz_xqux_xrux + +Misaligned accesses +------------------- + Misaligned accesses are supported in userspace, but they may perform poorly.