On Mon, Jul 10, 2023 at 2:36 AM Conor Dooley <conor.dooley@xxxxxxxxxxxxx> wrote:
>
> In riscv_fill_hwcap() riscv_isa_ext array can be looped over, rather
> than duplicating the list of extensions with individual
> SET_ISA_EXT_MAP() usage. While at it, drop the statement-of-the-obvious
> comments from the struct, rename uprop to something more suitable for
> its new use & constify the members.
>
> Reviewed-by: Andrew Jones <ajones@xxxxxxxxxxxxxxxx>
> Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> ---
> Changes in v2:
> - Delete the now unused definition
> ---
> arch/riscv/include/asm/hwcap.h | 7 ++-----
> arch/riscv/kernel/cpu.c | 5 +++--
> arch/riscv/kernel/cpufeature.c | 26 +++++++-------------------
> 3 files changed, 12 insertions(+), 26 deletions(-)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 7a57e6109aef..2460ac2fc7ed 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -55,7 +55,6 @@
> #define RISCV_ISA_EXT_ZIHPM 42
>
> #define RISCV_ISA_EXT_MAX 64
> -#define RISCV_ISA_EXT_NAME_LEN_MAX 32
>
> #ifdef CONFIG_RISCV_M_MODE
> #define RISCV_ISA_EXT_SxAIA RISCV_ISA_EXT_SMAIA
> @@ -70,10 +69,8 @@
> unsigned long riscv_get_elf_hwcap(void);
>
> struct riscv_isa_ext_data {
> - /* Name of the extension displayed to userspace via /proc/cpuinfo */
> - char uprop[RISCV_ISA_EXT_NAME_LEN_MAX];
> - /* The logical ISA extension ID */
> - unsigned int isa_ext_id;
> + const unsigned int id;
> + const char *name;
> };
>
> extern const struct riscv_isa_ext_data riscv_isa_ext[];
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index bf93293d51f3..aa17eeb0ec9a 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -168,9 +168,10 @@ static void print_isa_ext(struct seq_file *f)
> {
> 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))
> + if (!__riscv_isa_extension_available(NULL, edata->id))
> continue;
> - seq_printf(f, "_%s", edata->uprop);
> +
> + seq_printf(f, "_%s", edata->name);
> }
> }
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index fb476153fffc..6d8cd45af723 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -99,11 +99,10 @@ static bool riscv_isa_extension_check(int id)
> return true;
> }
>
> -#define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
> - { \
> - .uprop = #UPROP, \
> - .isa_ext_id = EXTID, \
> - }
> +#define __RISCV_ISA_EXT_DATA(_name, _id) { \
> + .name = #_name, \
> + .id = _id, \
> +}
>
> /*
> * The canonical order of ISA extension names in the ISA string is defined in
> @@ -366,20 +365,9 @@ void __init riscv_fill_hwcap(void)
> set_bit(nr, isainfo->isa);
> }
> } else {
> - /* sorted alphabetically */
> - SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
> - SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
> - 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("svnapot", RISCV_ISA_EXT_SVNAPOT);
> - SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> - SET_ISA_EXT_MAP("zba", RISCV_ISA_EXT_ZBA);
> - SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> - SET_ISA_EXT_MAP("zbs", RISCV_ISA_EXT_ZBS);
> - SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
> - SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
> - SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> + for (int i = 0; i < riscv_isa_ext_count; i++)
> + SET_ISA_EXT_MAP(riscv_isa_ext[i].name,
Does this still work, given that SET_ISA_EXT_MAP() does sizeof(name)
to get the string length? I worry that line of the macro is now
evaluating to a constant sizeof(pointer), and the macro needs to
change to use strlen().
-Evan
