2022-03-16 14:14:48

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v6 6/6] RISC-V: Improve /proc/cpuinfo output for ISA extensions

On Tue, Mar 15, 2022 at 2:09 AM Atish Patra <[email protected]> wrote:
>
> Currently, the /proc/cpuinfo outputs the entire riscv,isa string which
> is not ideal when we have multiple ISA extensions present in the ISA
> string. Some of them may not be enabled in kernel as well.
> Same goes for the single letter extensions as well which prints the
> entire ISA string. Some of they may not be valid ISA extensions as
> well (e.g 'su')
>
> Parse only the valid & enabled ISA extension and print them.
>
> Tested-by: Heiko Stuebner <[email protected]>
> Signed-off-by: Atish Patra <[email protected]>

Reviewed-by: Anup Patel <[email protected]>

Regards,
Anup

> ---
> arch/riscv/include/asm/hwcap.h | 7 ++++
> arch/riscv/kernel/cpu.c | 65 ++++++++++++++++++++++++++++++++--
> 2 files changed, 70 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 170bd80da520..691fc9c8099b 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -54,6 +54,13 @@ enum riscv_isa_ext_id {
> RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> };
>
> +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;
> +};
> +
> 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 ad0a7e9f828b..fc115e307ef5 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -6,6 +6,7 @@
> #include <linux/init.h>
> #include <linux/seq_file.h>
> #include <linux/of.h>
> +#include <asm/hwcap.h>
> #include <asm/smp.h>
> #include <asm/pgtable.h>
>
> @@ -63,12 +64,72 @@ int riscv_of_parent_hartid(struct device_node *node)
> }
>
> #ifdef CONFIG_PROC_FS
> +#define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
> + { \
> + .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
> + * from other multi-letter extensions by an underscore.
> + * 2. 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
> + * 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.
> + */
> +static struct riscv_isa_ext_data isa_ext_arr[] = {
> + __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;
> +
> + /* No extension support available */
> + if (arr_sz <= 0)
> + return;
> +
> + for (i = 0; i <= arr_sz; i++) {
> + edata = &isa_ext_arr[i];
> + if (!__riscv_isa_extension_available(NULL, edata->isa_ext_id))
> + continue;
> + seq_printf(f, "_%s", edata->uprop);
> + }
> +}
> +
> +/**
> + * These are the only valid base (single letter) ISA extensions as per the spec.
> + * It also specifies the canonical order in which it appears in the spec.
> + * Some of the extension may just be a place holder for now (B, K, P, J).
> + * This should be updated once corresponding extensions are ratified.
> + */
> +static const char base_riscv_exts[13] = "imafdqcbkjpvh";
>
> static void print_isa(struct seq_file *f, const char *isa)
> {
> - /* Print the entire ISA as it is */
> + int i;
> +
> seq_puts(f, "isa\t\t: ");
> - seq_write(f, isa, strlen(isa));
> + /* Print the rv[64/32] part */
> + seq_write(f, isa, 4);
> + for (i = 0; i < sizeof(base_riscv_exts); i++) {
> + if (__riscv_isa_extension_available(NULL, base_riscv_exts[i] - 'a'))
> + /* Print only enabled the base ISA extensions */
> + seq_write(f, &base_riscv_exts[i], 1);
> + }
> + print_isa_ext(f);
> seq_puts(f, "\n");
> }
>
> --
> 2.30.2
>