Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220222204811.2281949-1-atishp@rivosinc.com> <20220222204811.2281949-7-atishp@rivosinc.com>
In-Reply-To: <20220222204811.2281949-7-atishp@rivosinc.com>
From:   Anup Patel <anup@brainfault.org>
Date:   Mon, 28 Feb 2022 15:37:31 +0530
Message-ID: <CAAhSdy3SrHsDLt87uPwOHWDM0QtEg6Do7Zw_ymM52dOVbEz12Q@mail.gmail.com>
Subject: Re: [PATCH v5 6/6] RISC-V: Improve /proc/cpuinfo output for ISA extensions
To:     Atish Patra <atishp@rivosinc.com>
Cc:     "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>,
        Heiko Stuebner <heiko@sntech.de>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Atish Patra <atishp@atishpatra.org>,
        Damien Le Moal <damien.lemoal@wdc.com>,
        DTML <devicetree@vger.kernel.org>,
        Jisheng Zhang <jszhang@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Rob Herring <robh+dt@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Feb 23, 2022 at 2:18 AM Atish Patra <atishp@rivosinc.com> 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 <heiko@sntech.de>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  arch/riscv/include/asm/hwcap.h |  7 +++++
>  arch/riscv/kernel/cpu.c        | 51 ++++++++++++++++++++++++++++++++--
>  2 files changed, 56 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..031ad15a059f 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,57 @@ 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,                            \
> +       }
> +
> +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;
> +
> +       seq_puts(f, "isa-ext\t\t: ");
> +       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);
> +       }
> +       seq_puts(f, "\n");
> +}
> +
> +/**
> + * 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);
> +       }
>         seq_puts(f, "\n");
>  }
>
> @@ -115,6 +161,7 @@ static int c_show(struct seq_file *m, void *v)
>         seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id));
>         if (!of_property_read_string(node, "riscv,isa", &isa))
>                 print_isa(m, isa);
> +       print_isa_ext(m);
>         print_mmu(m);
>         if (!of_property_read_string(node, "compatible", &compat)
>             && strcmp(compat, "riscv"))
> --
> 2.30.2
>