2023-06-26 11:35:57

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v1 3/9] RISC-V: shunt isa_ext_arr to cpufeature.c

To facilitate using one struct to define extensions, rather than having
several, shunt isa_ext_arr to cpufeature.c, where it will be used for
probing extension presence also.
As that scope of the array as widened, prefix it with riscv & drop the
type from the variable name.

Since the new array is const, print_isa() needs a wee bit of cleanup to
avoid complaints about losing the const qualifier.

Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/include/asm/hwcap.h | 3 ++
arch/riscv/kernel/cpu.c | 75 +---------------------------------
arch/riscv/kernel/cpufeature.c | 68 ++++++++++++++++++++++++++++++
3 files changed, 73 insertions(+), 73 deletions(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index f041bfa7f6a0..7a57e6109aef 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -76,6 +76,9 @@ struct riscv_isa_ext_data {
unsigned int isa_ext_id;
};

+extern const struct riscv_isa_ext_data riscv_isa_ext[];
+extern const size_t riscv_isa_ext_count;
+
unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);

#define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext)
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 01f7e5c62997..61fb92e7d524 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -160,81 +160,10 @@ arch_initcall(riscv_cpuinfo_init);

#ifdef CONFIG_PROC_FS

-#define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
- { \
- .uprop = #UPROP, \
- .isa_ext_id = EXTID, \
- }
-
-/*
- * 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 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.
- *
- * 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(zicbom, RISCV_ISA_EXT_ZICBOM),
- __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
- __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(zba, RISCV_ISA_EXT_ZBA),
- __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
- __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
- __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
- __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
- __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(svnapot, RISCV_ISA_EXT_SVNAPOT),
- __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
- __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
-};
-
static void print_isa_ext(struct seq_file *f)
{
- struct riscv_isa_ext_data *edata;
- int i = 0, arr_sz;
-
- arr_sz = ARRAY_SIZE(isa_ext_arr) - 1;
-
- for (i = 0; i <= arr_sz; i++) {
- edata = &isa_ext_arr[i];
+ for (int i = 0; i < riscv_isa_ext_count; i++) {
+ const struct riscv_isa_ext_data *edata = &riscv_isa_ext[i];
if (!__riscv_isa_extension_available(NULL, edata->isa_ext_id))
continue;
seq_printf(f, "_%s", edata->uprop);
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index bdcf460ea53d..f0ae310006de 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -99,6 +99,74 @@ static bool riscv_isa_extension_check(int id)
return true;
}

+#define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
+ { \
+ .uprop = #UPROP, \
+ .isa_ext_id = EXTID, \
+ }
+
+/*
+ * 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 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.
+ *
+ * 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.
+ */
+const struct riscv_isa_ext_data riscv_isa_ext[] = {
+ __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
+ __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
+ __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(zba, RISCV_ISA_EXT_ZBA),
+ __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
+ __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
+ __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
+ __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
+ __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(svnapot, RISCV_ISA_EXT_SVNAPOT),
+ __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
+ __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
+};
+
+const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
+
void __init riscv_fill_hwcap(void)
{
struct device_node *node;
--
2.40.1



2023-06-26 15:44:54

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v1 3/9] RISC-V: shunt isa_ext_arr to cpufeature.c

On Mon, Jun 26, 2023 at 12:19:41PM +0100, Conor Dooley wrote:
> To facilitate using one struct to define extensions, rather than having
> several, shunt isa_ext_arr to cpufeature.c, where it will be used for
> probing extension presence also.
> As that scope of the array as widened, prefix it with riscv & drop the
> type from the variable name.
>
> Since the new array is const, print_isa() needs a wee bit of cleanup to
> avoid complaints about losing the const qualifier.
>
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> arch/riscv/include/asm/hwcap.h | 3 ++
> arch/riscv/kernel/cpu.c | 75 +---------------------------------
> arch/riscv/kernel/cpufeature.c | 68 ++++++++++++++++++++++++++++++
> 3 files changed, 73 insertions(+), 73 deletions(-)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index f041bfa7f6a0..7a57e6109aef 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -76,6 +76,9 @@ struct riscv_isa_ext_data {
> unsigned int isa_ext_id;
> };
>
> +extern const struct riscv_isa_ext_data riscv_isa_ext[];
> +extern const size_t riscv_isa_ext_count;
> +
> unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
>
> #define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext)
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 01f7e5c62997..61fb92e7d524 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -160,81 +160,10 @@ arch_initcall(riscv_cpuinfo_init);
>
> #ifdef CONFIG_PROC_FS
>
> -#define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
> - { \
> - .uprop = #UPROP, \
> - .isa_ext_id = EXTID, \
> - }
> -
> -/*
> - * 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 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.
> - *
> - * 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(zicbom, RISCV_ISA_EXT_ZICBOM),
> - __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
> - __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(zba, RISCV_ISA_EXT_ZBA),
> - __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> - __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> - __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> - __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> - __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(svnapot, RISCV_ISA_EXT_SVNAPOT),
> - __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> - __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> -};
> -
> static void print_isa_ext(struct seq_file *f)
> {
> - struct riscv_isa_ext_data *edata;
> - int i = 0, arr_sz;
> -
> - arr_sz = ARRAY_SIZE(isa_ext_arr) - 1;
> -
> - for (i = 0; i <= arr_sz; i++) {
> - edata = &isa_ext_arr[i];
> + for (int i = 0; i < riscv_isa_ext_count; i++) {
> + const struct riscv_isa_ext_data *edata = &riscv_isa_ext[i];
> if (!__riscv_isa_extension_available(NULL, edata->isa_ext_id))
> continue;
> seq_printf(f, "_%s", edata->uprop);
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index bdcf460ea53d..f0ae310006de 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -99,6 +99,74 @@ static bool riscv_isa_extension_check(int id)
> return true;
> }
>
> +#define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
> + { \
> + .uprop = #UPROP, \
> + .isa_ext_id = EXTID, \
> + }
> +
> +/*
> + * 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 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.
> + *
> + * 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.
> + */
> +const struct riscv_isa_ext_data riscv_isa_ext[] = {
> + __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> + __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
> + __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(zba, RISCV_ISA_EXT_ZBA),
> + __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> + __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> + __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(svnapot, RISCV_ISA_EXT_SVNAPOT),
> + __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> + __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),

I think we can either drop this null entry or drop the count variable
below. My preference would be to drop the count variable, and always
loop to the null.

> +};
> +
> +const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
> +
> void __init riscv_fill_hwcap(void)
> {
> struct device_node *node;
> --
> 2.40.1
>

Otherwise,

Reviewed-by: Andrew Jones <[email protected]>

Thanks,
drew

2023-06-26 16:04:13

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v1 3/9] RISC-V: shunt isa_ext_arr to cpufeature.c

On Mon, Jun 26, 2023 at 05:29:04PM +0200, Andrew Jones wrote:
> On Mon, Jun 26, 2023 at 12:19:41PM +0100, Conor Dooley wrote:
...
> > +const struct riscv_isa_ext_data riscv_isa_ext[] = {
> > + __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> > + __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
> > + __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(zba, RISCV_ISA_EXT_ZBA),
> > + __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > + __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> > + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> > + __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(svnapot, RISCV_ISA_EXT_SVNAPOT),
> > + __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> > + __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
>
> I think we can either drop this null entry or drop the count variable
> below. My preference would be to drop the count variable, and always
> loop to the null.

Eh, never mind, the entry isn't null, it's "". Why do we have that entry
though? I guess it can be dropped?

Thanks,
drew

2023-06-26 16:23:13

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1 3/9] RISC-V: shunt isa_ext_arr to cpufeature.c

On Mon, Jun 26, 2023 at 05:44:18PM +0200, Andrew Jones wrote:
> On Mon, Jun 26, 2023 at 05:29:04PM +0200, Andrew Jones wrote:
> > On Mon, Jun 26, 2023 at 12:19:41PM +0100, Conor Dooley wrote:
> ...
> > > +const struct riscv_isa_ext_data riscv_isa_ext[] = {
> > > + __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> > > + __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
> > > + __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(zba, RISCV_ISA_EXT_ZBA),
> > > + __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > > + __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> > > + __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > > + __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> > > + __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(svnapot, RISCV_ISA_EXT_SVNAPOT),
> > > + __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> > > + __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> >
> > I think we can either drop this null entry or drop the count variable
> > below. My preference would be to drop the count variable, and always
> > loop to the null.
>
> Eh, never mind, the entry isn't null, it's "". Why do we have that entry
> though? I guess it can be dropped?

It may just be cargo culting? The commit that added this, a9b202606c69
("RISC-V: Improve /proc/cpuinfo output for ISA extensions") [1], added
an empty array other than this element & perhaps it was just not removed
when real entries were added? Symptom of adding a feature without an
actual user (multiletter extension) perhaps?

1 - https://lore.kernel.org/all/[email protected]/


Attachments:
(No filename) (2.01 kB)
signature.asc (235.00 B)
Download all attachments