diff mbox series

[v2] RISC-V: Add mvendorid, marchid, and mimpid to /proc/cpuinfo output

Message ID 20220727043829.151794-1-apatel@ventanamicro.com (mailing list archive)
State New, archived
Headers show
Series [v2] RISC-V: Add mvendorid, marchid, and mimpid to /proc/cpuinfo output | expand

Commit Message

Anup Patel July 27, 2022, 4:38 a.m. UTC
Identifying the underlying RISC-V implementation can be important
for some of the user space applications. For example, the perf tool
uses arch specific CPU implementation id (i.e. CPUID) to select a
JSON file describing custom perf events on a CPU.

Currently, there is no way to identify RISC-V implementation so we
add mvendorid, marchid, and mimpid to /proc/cpuinfo output.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Tested-by: Nikita Shubin <n.shubin@yadro.com>
---
Changes since v1:
 - Use IS_ENABLED() to check CONFIG defines
 - Added RB and TB tags in commit description
---
 arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

Comments

Conor Dooley July 27, 2022, 8:31 a.m. UTC | #1
On 27/07/2022 05:38, Anup Patel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Identifying the underlying RISC-V implementation can be important
> for some of the user space applications. For example, the perf tool
> uses arch specific CPU implementation id (i.e. CPUID) to select a
> JSON file describing custom perf events on a CPU.
> 
> Currently, there is no way to identify RISC-V implementation so we
> add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Tested-by: Nikita Shubin <n.shubin@yadro.com>

Read back the boring zeros on polarfire, if only I could write a
patch for the core complex..

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> ---
> Changes since v1:
>   - Use IS_ENABLED() to check CONFIG defines
>   - Added RB and TB tags in commit description
> ---
>   arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
> 
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fba9e9f46a8c..04bcc91c91ea 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -3,10 +3,13 @@
>    * Copyright (C) 2012 Regents of the University of California
>    */
> 
> +#include <linux/cpu.h>
>   #include <linux/init.h>
>   #include <linux/seq_file.h>
>   #include <linux/of.h>
> +#include <asm/csr.h>
>   #include <asm/hwcap.h>
> +#include <asm/sbi.h>
>   #include <asm/smp.h>
>   #include <asm/pgtable.h>
> 
> @@ -64,6 +67,50 @@ int riscv_of_parent_hartid(struct device_node *node)
>   }
> 
>   #ifdef CONFIG_PROC_FS
> +
> +struct riscv_cpuinfo {
> +       unsigned long mvendorid;
> +       unsigned long marchid;
> +       unsigned long mimpid;
> +};
> +static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
> +
> +static int riscv_cpuinfo_starting(unsigned int cpu)
> +{
> +       struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
> +
> +#if IS_ENABLED(CONFIG_RISCV_SBI)
> +       ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
> +       ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
> +       ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
> +#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
> +       ci->mvendorid = csr_read(CSR_MVENDORID);
> +       ci->marchid = csr_read(CSR_MARCHID);
> +       ci->mimpid = csr_read(CSR_MIMPID);
> +#else
> +       ci->mvendorid = 0;
> +       ci->marchid = 0;
> +       ci->mimpid = 0;
> +#endif
> +
> +       return 0;
> +}
> +
> +static int __init riscv_cpuinfo_init(void)
> +{
> +       int ret;
> +
> +       ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/cpuinfo:starting",
> +                               riscv_cpuinfo_starting, NULL);
> +       if (ret < 0) {
> +               pr_err("cpuinfo: failed to register hotplug callbacks.\n");
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +device_initcall(riscv_cpuinfo_init);
> +
>   #define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
>          {                                                       \
>                  .uprop = #UPROP,                                \
> @@ -178,6 +225,7 @@ static int c_show(struct seq_file *m, void *v)
>   {
>          unsigned long cpu_id = (unsigned long)v - 1;
>          struct device_node *node = of_get_cpu_node(cpu_id, NULL);
> +       struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
>          const char *compat, *isa;
> 
>          seq_printf(m, "processor\t: %lu\n", cpu_id);
> @@ -188,6 +236,9 @@ static int c_show(struct seq_file *m, void *v)
>          if (!of_property_read_string(node, "compatible", &compat)
>              && strcmp(compat, "riscv"))
>                  seq_printf(m, "uarch\t\t: %s\n", compat);
> +       seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
> +       seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid);
> +       seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid);
>          seq_puts(m, "\n");
>          of_node_put(node);
> 
> --
> 2.34.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Ben Dooks July 27, 2022, 8:55 a.m. UTC | #2
On 27/07/2022 05:38, Anup Patel wrote:
> Identifying the underlying RISC-V implementation can be important
> for some of the user space applications. For example, the perf tool
> uses arch specific CPU implementation id (i.e. CPUID) to select a
> JSON file describing custom perf events on a CPU.
> 
> Currently, there is no way to identify RISC-V implementation so we
> add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Tested-by: Nikita Shubin <n.shubin@yadro.com>
> ---
> Changes since v1:
>   - Use IS_ENABLED() to check CONFIG defines
>   - Added RB and TB tags in commit description
> ---
>   arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
> 
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fba9e9f46a8c..04bcc91c91ea 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -3,10 +3,13 @@
>    * Copyright (C) 2012 Regents of the University of California
>    */
>   
> +#include <linux/cpu.h>
>   #include <linux/init.h>
>   #include <linux/seq_file.h>
>   #include <linux/of.h>
> +#include <asm/csr.h>
>   #include <asm/hwcap.h>
> +#include <asm/sbi.h>
>   #include <asm/smp.h>
>   #include <asm/pgtable.h>
>   
> @@ -64,6 +67,50 @@ int riscv_of_parent_hartid(struct device_node *node)
>   }
>   
>   #ifdef CONFIG_PROC_FS
> +
> +struct riscv_cpuinfo {
> +	unsigned long mvendorid;
> +	unsigned long marchid;
> +	unsigned long mimpid;
> +};
> +static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
> +
> +static int riscv_cpuinfo_starting(unsigned int cpu)
> +{
> +	struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
> +
> +#if IS_ENABLED(CONFIG_RISCV_SBI)
> +	ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
> +	ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
> +	ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();

how about:

	if (IS_ENABLED(CONFIG_RISCV_SBI)) {
		...	
	} ... {

or maybe even:


	if (IS_ENABLED(CONFIG_RISCV_SBI)) {
		if (sbi_spec_is_0_1()) {
			...
		}	
	} ... {

would mean better compile coverage (at the slight exepnese of
having "false" sbi_spec_is_0_1() implemenation

> +#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
> +	ci->mvendorid = csr_read(CSR_MVENDORID);
> +	ci->marchid = csr_read(CSR_MARCHID);
> +	ci->mimpid = csr_read(CSR_MIMPID);
> +#else
> +	ci->mvendorid = 0;
> +	ci->marchid = 0;
> +	ci->mimpid = 0;
> +#endif

Would it be easier to zero out all the fields first and then fill them
in if supported?

> +
> +	return 0;
> +}
> +
> +static int __init riscv_cpuinfo_init(void)
> +{
> +	int ret;
> +
> +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/cpuinfo:starting",
> +				riscv_cpuinfo_starting, NULL);
> +	if (ret < 0) {
> +		pr_err("cpuinfo: failed to register hotplug callbacks.\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +device_initcall(riscv_cpuinfo_init);
> +
>   #define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
>   	{							\
>   		.uprop = #UPROP,				\
> @@ -178,6 +225,7 @@ static int c_show(struct seq_file *m, void *v)
>   {
>   	unsigned long cpu_id = (unsigned long)v - 1;
>   	struct device_node *node = of_get_cpu_node(cpu_id, NULL);
> +	struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
>   	const char *compat, *isa;
>   
>   	seq_printf(m, "processor\t: %lu\n", cpu_id);
> @@ -188,6 +236,9 @@ static int c_show(struct seq_file *m, void *v)
>   	if (!of_property_read_string(node, "compatible", &compat)
>   	    && strcmp(compat, "riscv"))
>   		seq_printf(m, "uarch\t\t: %s\n", compat);
> +	seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
> +	seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid);
> +	seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid);
>   	seq_puts(m, "\n");
>   	of_node_put(node);
>
Anup Patel July 27, 2022, 10:06 a.m. UTC | #3
On Wed, Jul 27, 2022 at 2:25 PM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>
> On 27/07/2022 05:38, Anup Patel wrote:
> > Identifying the underlying RISC-V implementation can be important
> > for some of the user space applications. For example, the perf tool
> > uses arch specific CPU implementation id (i.e. CPUID) to select a
> > JSON file describing custom perf events on a CPU.
> >
> > Currently, there is no way to identify RISC-V implementation so we
> > add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > Tested-by: Nikita Shubin <n.shubin@yadro.com>
> > ---
> > Changes since v1:
> >   - Use IS_ENABLED() to check CONFIG defines
> >   - Added RB and TB tags in commit description
> > ---
> >   arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 51 insertions(+)
> >
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index fba9e9f46a8c..04bcc91c91ea 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -3,10 +3,13 @@
> >    * Copyright (C) 2012 Regents of the University of California
> >    */
> >
> > +#include <linux/cpu.h>
> >   #include <linux/init.h>
> >   #include <linux/seq_file.h>
> >   #include <linux/of.h>
> > +#include <asm/csr.h>
> >   #include <asm/hwcap.h>
> > +#include <asm/sbi.h>
> >   #include <asm/smp.h>
> >   #include <asm/pgtable.h>
> >
> > @@ -64,6 +67,50 @@ int riscv_of_parent_hartid(struct device_node *node)
> >   }
> >
> >   #ifdef CONFIG_PROC_FS
> > +
> > +struct riscv_cpuinfo {
> > +     unsigned long mvendorid;
> > +     unsigned long marchid;
> > +     unsigned long mimpid;
> > +};
> > +static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
> > +
> > +static int riscv_cpuinfo_starting(unsigned int cpu)
> > +{
> > +     struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
> > +
> > +#if IS_ENABLED(CONFIG_RISCV_SBI)
> > +     ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
> > +     ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
> > +     ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
>
> how about:
>
>         if (IS_ENABLED(CONFIG_RISCV_SBI)) {
>                 ...
>         } ... {
>
> or maybe even:
>
>
>         if (IS_ENABLED(CONFIG_RISCV_SBI)) {
>                 if (sbi_spec_is_0_1()) {
>                         ...
>                 }
>         } ... {
>
> would mean better compile coverage (at the slight exepnese of
> having "false" sbi_spec_is_0_1() implemenation

Most of the sbi_xyz() functions are not available for NoMMU
kernel so using "if (IS_ENABLED())" results in compile error.

>
> > +#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
> > +     ci->mvendorid = csr_read(CSR_MVENDORID);
> > +     ci->marchid = csr_read(CSR_MARCHID);
> > +     ci->mimpid = csr_read(CSR_MIMPID);
> > +#else
> > +     ci->mvendorid = 0;
> > +     ci->marchid = 0;
> > +     ci->mimpid = 0;
> > +#endif
>
> Would it be easier to zero out all the fields first and then fill them
> in if supported?

Clearing out fields before "#if" ladder results in dead assignments.

Regards,
Anup

>
> > +
> > +     return 0;
> > +}
> > +
> > +static int __init riscv_cpuinfo_init(void)
> > +{
> > +     int ret;
> > +
> > +     ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/cpuinfo:starting",
> > +                             riscv_cpuinfo_starting, NULL);
> > +     if (ret < 0) {
> > +             pr_err("cpuinfo: failed to register hotplug callbacks.\n");
> > +             return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +device_initcall(riscv_cpuinfo_init);
> > +
> >   #define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
> >       {                                                       \
> >               .uprop = #UPROP,                                \
> > @@ -178,6 +225,7 @@ static int c_show(struct seq_file *m, void *v)
> >   {
> >       unsigned long cpu_id = (unsigned long)v - 1;
> >       struct device_node *node = of_get_cpu_node(cpu_id, NULL);
> > +     struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
> >       const char *compat, *isa;
> >
> >       seq_printf(m, "processor\t: %lu\n", cpu_id);
> > @@ -188,6 +236,9 @@ static int c_show(struct seq_file *m, void *v)
> >       if (!of_property_read_string(node, "compatible", &compat)
> >           && strcmp(compat, "riscv"))
> >               seq_printf(m, "uarch\t\t: %s\n", compat);
> > +     seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
> > +     seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid);
> > +     seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid);
> >       seq_puts(m, "\n");
> >       of_node_put(node);
> >
>
>
> --
> Ben Dooks                               http://www.codethink.co.uk/
> Senior Engineer                         Codethink - Providing Genius
>
> https://www.codethink.co.uk/privacy.html
Ben Dooks July 27, 2022, 10:12 a.m. UTC | #4
On 27/07/2022 11:06, Anup Patel wrote:
> On Wed, Jul 27, 2022 at 2:25 PM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>>
>> On 27/07/2022 05:38, Anup Patel wrote:
>>> Identifying the underlying RISC-V implementation can be important
>>> for some of the user space applications. For example, the perf tool
>>> uses arch specific CPU implementation id (i.e. CPUID) to select a
>>> JSON file describing custom perf events on a CPU.
>>>
>>> Currently, there is no way to identify RISC-V implementation so we
>>> add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
>>>
>>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>>> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>> Tested-by: Nikita Shubin <n.shubin@yadro.com>
>>> ---
>>> Changes since v1:
>>>    - Use IS_ENABLED() to check CONFIG defines
>>>    - Added RB and TB tags in commit description
>>> ---
>>>    arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 51 insertions(+)
>>>
>>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
>>> index fba9e9f46a8c..04bcc91c91ea 100644
>>> --- a/arch/riscv/kernel/cpu.c
>>> +++ b/arch/riscv/kernel/cpu.c
>>> @@ -3,10 +3,13 @@
>>>     * Copyright (C) 2012 Regents of the University of California
>>>     */
>>>
>>> +#include <linux/cpu.h>
>>>    #include <linux/init.h>
>>>    #include <linux/seq_file.h>
>>>    #include <linux/of.h>
>>> +#include <asm/csr.h>
>>>    #include <asm/hwcap.h>
>>> +#include <asm/sbi.h>
>>>    #include <asm/smp.h>
>>>    #include <asm/pgtable.h>
>>>
>>> @@ -64,6 +67,50 @@ int riscv_of_parent_hartid(struct device_node *node)
>>>    }
>>>
>>>    #ifdef CONFIG_PROC_FS
>>> +
>>> +struct riscv_cpuinfo {
>>> +     unsigned long mvendorid;
>>> +     unsigned long marchid;
>>> +     unsigned long mimpid;
>>> +};
>>> +static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
>>> +
>>> +static int riscv_cpuinfo_starting(unsigned int cpu)
>>> +{
>>> +     struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
>>> +
>>> +#if IS_ENABLED(CONFIG_RISCV_SBI)
>>> +     ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
>>> +     ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
>>> +     ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
>>
>> how about:
>>
>>          if (IS_ENABLED(CONFIG_RISCV_SBI)) {
>>                  ...
>>          } ... {
>>
>> or maybe even:
>>
>>
>>          if (IS_ENABLED(CONFIG_RISCV_SBI)) {
>>                  if (sbi_spec_is_0_1()) {
>>                          ...
>>                  }
>>          } ... {
>>
>> would mean better compile coverage (at the slight exepnese of
>> having "false" sbi_spec_is_0_1() implemenation
> 
> Most of the sbi_xyz() functions are not available for NoMMU
> kernel so using "if (IS_ENABLED())" results in compile error.

How about defining "false" versions for no-mmu case and try
and avoid these #if mountains?

>>
>>> +#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
>>> +     ci->mvendorid = csr_read(CSR_MVENDORID);
>>> +     ci->marchid = csr_read(CSR_MARCHID);
>>> +     ci->mimpid = csr_read(CSR_MIMPID);
>>> +#else
>>> +     ci->mvendorid = 0;
>>> +     ci->marchid = 0;
>>> +     ci->mimpid = 0;
>>> +#endif
>>
>> Would it be easier to zero out all the fields first and then fill them
>> in if supported?
> 
> Clearing out fields before "#if" ladder results in dead assignments.

Not sure which is worse here, the #if ladder or some possibly dead
assignments.
Anup Patel July 27, 2022, 11:53 a.m. UTC | #5
On Wed, Jul 27, 2022 at 3:42 PM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>
> On 27/07/2022 11:06, Anup Patel wrote:
> > On Wed, Jul 27, 2022 at 2:25 PM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> >>
> >> On 27/07/2022 05:38, Anup Patel wrote:
> >>> Identifying the underlying RISC-V implementation can be important
> >>> for some of the user space applications. For example, the perf tool
> >>> uses arch specific CPU implementation id (i.e. CPUID) to select a
> >>> JSON file describing custom perf events on a CPU.
> >>>
> >>> Currently, there is no way to identify RISC-V implementation so we
> >>> add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
> >>>
> >>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> >>> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >>> Tested-by: Nikita Shubin <n.shubin@yadro.com>
> >>> ---
> >>> Changes since v1:
> >>>    - Use IS_ENABLED() to check CONFIG defines
> >>>    - Added RB and TB tags in commit description
> >>> ---
> >>>    arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
> >>>    1 file changed, 51 insertions(+)
> >>>
> >>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> >>> index fba9e9f46a8c..04bcc91c91ea 100644
> >>> --- a/arch/riscv/kernel/cpu.c
> >>> +++ b/arch/riscv/kernel/cpu.c
> >>> @@ -3,10 +3,13 @@
> >>>     * Copyright (C) 2012 Regents of the University of California
> >>>     */
> >>>
> >>> +#include <linux/cpu.h>
> >>>    #include <linux/init.h>
> >>>    #include <linux/seq_file.h>
> >>>    #include <linux/of.h>
> >>> +#include <asm/csr.h>
> >>>    #include <asm/hwcap.h>
> >>> +#include <asm/sbi.h>
> >>>    #include <asm/smp.h>
> >>>    #include <asm/pgtable.h>
> >>>
> >>> @@ -64,6 +67,50 @@ int riscv_of_parent_hartid(struct device_node *node)
> >>>    }
> >>>
> >>>    #ifdef CONFIG_PROC_FS
> >>> +
> >>> +struct riscv_cpuinfo {
> >>> +     unsigned long mvendorid;
> >>> +     unsigned long marchid;
> >>> +     unsigned long mimpid;
> >>> +};
> >>> +static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
> >>> +
> >>> +static int riscv_cpuinfo_starting(unsigned int cpu)
> >>> +{
> >>> +     struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
> >>> +
> >>> +#if IS_ENABLED(CONFIG_RISCV_SBI)
> >>> +     ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
> >>> +     ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
> >>> +     ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
> >>
> >> how about:
> >>
> >>          if (IS_ENABLED(CONFIG_RISCV_SBI)) {
> >>                  ...
> >>          } ... {
> >>
> >> or maybe even:
> >>
> >>
> >>          if (IS_ENABLED(CONFIG_RISCV_SBI)) {
> >>                  if (sbi_spec_is_0_1()) {
> >>                          ...
> >>                  }
> >>          } ... {
> >>
> >> would mean better compile coverage (at the slight exepnese of
> >> having "false" sbi_spec_is_0_1() implemenation
> >
> > Most of the sbi_xyz() functions are not available for NoMMU
> > kernel so using "if (IS_ENABLED())" results in compile error.
>
> How about defining "false" versions for no-mmu case and try
> and avoid these #if mountains?

Well, we are not simplifying anything by moving from a "#if" ladder
to "if ()" ladder. Also, I don't see how the "#if" ladder will grow over
time.

Regards,
Anup

>
> >>
> >>> +#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
> >>> +     ci->mvendorid = csr_read(CSR_MVENDORID);
> >>> +     ci->marchid = csr_read(CSR_MARCHID);
> >>> +     ci->mimpid = csr_read(CSR_MIMPID);
> >>> +#else
> >>> +     ci->mvendorid = 0;
> >>> +     ci->marchid = 0;
> >>> +     ci->mimpid = 0;
> >>> +#endif
> >>
> >> Would it be easier to zero out all the fields first and then fill them
> >> in if supported?
> >
> > Clearing out fields before "#if" ladder results in dead assignments.
>
> Not sure which is worse here, the #if ladder or some possibly dead
> assignments.
>
> --
> Ben Dooks                               http://www.codethink.co.uk/
> Senior Engineer                         Codethink - Providing Genius
>
> https://www.codethink.co.uk/privacy.html
Anup Patel Aug. 11, 2022, 5:05 a.m. UTC | #6
Hi Palmer,

On Wed, Jul 27, 2022 at 10:09 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Identifying the underlying RISC-V implementation can be important
> for some of the user space applications. For example, the perf tool
> uses arch specific CPU implementation id (i.e. CPUID) to select a
> JSON file describing custom perf events on a CPU.
>
> Currently, there is no way to identify RISC-V implementation so we
> add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Tested-by: Nikita Shubin <n.shubin@yadro.com>

Can this patch be considered for 5.20 ?

Regards,
Anup


> ---
> Changes since v1:
>  - Use IS_ENABLED() to check CONFIG defines
>  - Added RB and TB tags in commit description
> ---
>  arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fba9e9f46a8c..04bcc91c91ea 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -3,10 +3,13 @@
>   * Copyright (C) 2012 Regents of the University of California
>   */
>
> +#include <linux/cpu.h>
>  #include <linux/init.h>
>  #include <linux/seq_file.h>
>  #include <linux/of.h>
> +#include <asm/csr.h>
>  #include <asm/hwcap.h>
> +#include <asm/sbi.h>
>  #include <asm/smp.h>
>  #include <asm/pgtable.h>
>
> @@ -64,6 +67,50 @@ int riscv_of_parent_hartid(struct device_node *node)
>  }
>
>  #ifdef CONFIG_PROC_FS
> +
> +struct riscv_cpuinfo {
> +       unsigned long mvendorid;
> +       unsigned long marchid;
> +       unsigned long mimpid;
> +};
> +static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
> +
> +static int riscv_cpuinfo_starting(unsigned int cpu)
> +{
> +       struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
> +
> +#if IS_ENABLED(CONFIG_RISCV_SBI)
> +       ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
> +       ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
> +       ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
> +#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
> +       ci->mvendorid = csr_read(CSR_MVENDORID);
> +       ci->marchid = csr_read(CSR_MARCHID);
> +       ci->mimpid = csr_read(CSR_MIMPID);
> +#else
> +       ci->mvendorid = 0;
> +       ci->marchid = 0;
> +       ci->mimpid = 0;
> +#endif
> +
> +       return 0;
> +}
> +
> +static int __init riscv_cpuinfo_init(void)
> +{
> +       int ret;
> +
> +       ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/cpuinfo:starting",
> +                               riscv_cpuinfo_starting, NULL);
> +       if (ret < 0) {
> +               pr_err("cpuinfo: failed to register hotplug callbacks.\n");
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +device_initcall(riscv_cpuinfo_init);
> +
>  #define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
>         {                                                       \
>                 .uprop = #UPROP,                                \
> @@ -178,6 +225,7 @@ static int c_show(struct seq_file *m, void *v)
>  {
>         unsigned long cpu_id = (unsigned long)v - 1;
>         struct device_node *node = of_get_cpu_node(cpu_id, NULL);
> +       struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
>         const char *compat, *isa;
>
>         seq_printf(m, "processor\t: %lu\n", cpu_id);
> @@ -188,6 +236,9 @@ static int c_show(struct seq_file *m, void *v)
>         if (!of_property_read_string(node, "compatible", &compat)
>             && strcmp(compat, "riscv"))
>                 seq_printf(m, "uarch\t\t: %s\n", compat);
> +       seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
> +       seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid);
> +       seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid);
>         seq_puts(m, "\n");
>         of_node_put(node);
>
> --
> 2.34.1
>
Palmer Dabbelt Oct. 4, 2022, 2:54 a.m. UTC | #7
On Tue, 26 Jul 2022 21:38:29 PDT (-0700), apatel@ventanamicro.com wrote:
> Identifying the underlying RISC-V implementation can be important
> for some of the user space applications. For example, the perf tool
> uses arch specific CPU implementation id (i.e. CPUID) to select a
> JSON file describing custom perf events on a CPU.
>
> Currently, there is no way to identify RISC-V implementation so we
> add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Tested-by: Nikita Shubin <n.shubin@yadro.com>
> ---
> Changes since v1:
>  - Use IS_ENABLED() to check CONFIG defines
>  - Added RB and TB tags in commit description
> ---
>  arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)

Sorry for being slow on this one.  I'd been worried about sticking more 
stuff into /proc/cpuinfo, as having the only user interface for these 
involve parsing /proc/cpuinfo is really just a recipe for disaster.  I 
didn't get around do doing something better, though, and waiting for 
another release seems kind of silly.

I was going to go put this on for-next, but it looks like it's causing 
kasan boot failures.  I'm not actually quite sure how this is triggering 
these, at least based on the backtrace, but there's a bunch of them and 
boot hangs.

Here's the first of them:

[    3.830416] Hardware name: riscv-virtio,qemu (DT)
[    3.830828] epc : kasan_check_range+0x116/0x14c
[    3.832377]  ra : memset+0x1e/0x4c
[    3.832699] epc : ffffffff8026468e ra : ffffffff80264d6e sp : ff600000832afa60
[    3.833051]  gp : ffffffff81d800a0 tp : ff600000831d2400 t0 : ff60000083224360
[    3.833397]  t1 : ffebfffefffef000 t2 : 0000000000000000 s0 : ff600000832afa90
[    3.833811]  s1 : 0000000000000004 a0 : 0000000000000010 a1 : 0000000000000004
[    3.834210]  a2 : 0000000000000001 a3 : ffffffff801fa22c a4 : ff5ffffffff78000
[    3.834558]  a5 : ffebfffefffef000 a6 : ffebfffefffef001 a7 : ff5ffffffff78003
[    3.834931]  s2 : ff5ffffffff78000 s3 : 0000000000000000 s4 : ffffffff815b59b0
[    3.835292]  s5 : ffffffff81d815e0 s6 : 0000000000000000 s7 : 0000000000000008
[    3.836400]  s8 : ffffffff81d8b0e0 s9 : 0000000000000000 s10: 0000000000000008
[    3.836817]  s11: ff5ffffffff78000 t3 : 0000000000000000 t4 : ffebfffefffef000
[    3.837154]  t5 : ffebfffefffef001 t6 : 0000000000000002
[    3.837471] status: 0000000000000120 badaddr: ffebfffefffef000 cause: 000000000000000d
[    3.837933] [<ffffffff801fa22c>] pcpu_alloc+0x494/0x838
[    3.838324] [<ffffffff801fa5fc>] __alloc_percpu+0x14/0x1c
[    3.838623] [<ffffffff800930fa>] __percpu_init_rwsem+0x1a/0x98
[    3.838995] [<ffffffff80286c5a>] alloc_super+0x148/0x3da
[    3.839631] [<ffffffff80287af2>] sget_fc+0x90/0x2c4
[    3.840240] [<ffffffff80288498>] get_tree_nodev+0x24/0xa4
[    3.840598] [<ffffffff801e919a>] shmem_get_tree+0x14/0x1c
[    3.840891] [<ffffffff80286436>] vfs_get_tree+0x3a/0x11a
[    3.841172] [<ffffffff802ba746>] path_mount+0x2ea/0xb44
[    3.841468] [<ffffffff802bb69a>] sys_mount+0x1b2/0x270
[    3.841790] [<ffffffff80003d1c>] ret_from_syscall+0x0/0x2
[    3.842480] ---[ end trace 0000000000000000 ]---

>
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fba9e9f46a8c..04bcc91c91ea 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -3,10 +3,13 @@
>   * Copyright (C) 2012 Regents of the University of California
>   */
>
> +#include <linux/cpu.h>
>  #include <linux/init.h>
>  #include <linux/seq_file.h>
>  #include <linux/of.h>
> +#include <asm/csr.h>
>  #include <asm/hwcap.h>
> +#include <asm/sbi.h>
>  #include <asm/smp.h>
>  #include <asm/pgtable.h>
>
> @@ -64,6 +67,50 @@ int riscv_of_parent_hartid(struct device_node *node)
>  }
>
>  #ifdef CONFIG_PROC_FS
> +
> +struct riscv_cpuinfo {
> +	unsigned long mvendorid;
> +	unsigned long marchid;
> +	unsigned long mimpid;
> +};
> +static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
> +
> +static int riscv_cpuinfo_starting(unsigned int cpu)
> +{
> +	struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
> +
> +#if IS_ENABLED(CONFIG_RISCV_SBI)
> +	ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
> +	ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
> +	ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
> +#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
> +	ci->mvendorid = csr_read(CSR_MVENDORID);
> +	ci->marchid = csr_read(CSR_MARCHID);
> +	ci->mimpid = csr_read(CSR_MIMPID);
> +#else
> +	ci->mvendorid = 0;
> +	ci->marchid = 0;
> +	ci->mimpid = 0;
> +#endif
> +
> +	return 0;
> +}
> +
> +static int __init riscv_cpuinfo_init(void)
> +{
> +	int ret;
> +
> +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/cpuinfo:starting",
> +				riscv_cpuinfo_starting, NULL);
> +	if (ret < 0) {
> +		pr_err("cpuinfo: failed to register hotplug callbacks.\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +device_initcall(riscv_cpuinfo_init);
> +
>  #define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
>  	{							\
>  		.uprop = #UPROP,				\
> @@ -178,6 +225,7 @@ static int c_show(struct seq_file *m, void *v)
>  {
>  	unsigned long cpu_id = (unsigned long)v - 1;
>  	struct device_node *node = of_get_cpu_node(cpu_id, NULL);
> +	struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
>  	const char *compat, *isa;
>
>  	seq_printf(m, "processor\t: %lu\n", cpu_id);
> @@ -188,6 +236,9 @@ static int c_show(struct seq_file *m, void *v)
>  	if (!of_property_read_string(node, "compatible", &compat)
>  	    && strcmp(compat, "riscv"))
>  		seq_printf(m, "uarch\t\t: %s\n", compat);
> +	seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
> +	seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid);
> +	seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid);
>  	seq_puts(m, "\n");
>  	of_node_put(node);
Anup Patel Oct. 4, 2022, 3:44 a.m. UTC | #8
On Tue, Oct 4, 2022 at 8:24 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Tue, 26 Jul 2022 21:38:29 PDT (-0700), apatel@ventanamicro.com wrote:
> > Identifying the underlying RISC-V implementation can be important
> > for some of the user space applications. For example, the perf tool
> > uses arch specific CPU implementation id (i.e. CPUID) to select a
> > JSON file describing custom perf events on a CPU.
> >
> > Currently, there is no way to identify RISC-V implementation so we
> > add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > Tested-by: Nikita Shubin <n.shubin@yadro.com>
> > ---
> > Changes since v1:
> >  - Use IS_ENABLED() to check CONFIG defines
> >  - Added RB and TB tags in commit description
> > ---
> >  arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
>
> Sorry for being slow on this one.  I'd been worried about sticking more
> stuff into /proc/cpuinfo, as having the only user interface for these
> involve parsing /proc/cpuinfo is really just a recipe for disaster.  I
> didn't get around do doing something better, though, and waiting for
> another release seems kind of silly.

In addition to being useful for perf tool, printing vendor and
implementation details in /proc/cpuinfo is a very useful info.

Users can easily know the underlying CPU implementer using
"cat /proc/cpuinfo".

>
> I was going to go put this on for-next, but it looks like it's causing
> kasan boot failures.  I'm not actually quite sure how this is triggering
> these, at least based on the backtrace, but there's a bunch of them and
> boot hangs.
>
> Here's the first of them:
>
> [    3.830416] Hardware name: riscv-virtio,qemu (DT)
> [    3.830828] epc : kasan_check_range+0x116/0x14c
> [    3.832377]  ra : memset+0x1e/0x4c
> [    3.832699] epc : ffffffff8026468e ra : ffffffff80264d6e sp : ff600000832afa60
> [    3.833051]  gp : ffffffff81d800a0 tp : ff600000831d2400 t0 : ff60000083224360
> [    3.833397]  t1 : ffebfffefffef000 t2 : 0000000000000000 s0 : ff600000832afa90
> [    3.833811]  s1 : 0000000000000004 a0 : 0000000000000010 a1 : 0000000000000004
> [    3.834210]  a2 : 0000000000000001 a3 : ffffffff801fa22c a4 : ff5ffffffff78000
> [    3.834558]  a5 : ffebfffefffef000 a6 : ffebfffefffef001 a7 : ff5ffffffff78003
> [    3.834931]  s2 : ff5ffffffff78000 s3 : 0000000000000000 s4 : ffffffff815b59b0
> [    3.835292]  s5 : ffffffff81d815e0 s6 : 0000000000000000 s7 : 0000000000000008
> [    3.836400]  s8 : ffffffff81d8b0e0 s9 : 0000000000000000 s10: 0000000000000008
> [    3.836817]  s11: ff5ffffffff78000 t3 : 0000000000000000 t4 : ffebfffefffef000
> [    3.837154]  t5 : ffebfffefffef001 t6 : 0000000000000002
> [    3.837471] status: 0000000000000120 badaddr: ffebfffefffef000 cause: 000000000000000d
> [    3.837933] [<ffffffff801fa22c>] pcpu_alloc+0x494/0x838
> [    3.838324] [<ffffffff801fa5fc>] __alloc_percpu+0x14/0x1c
> [    3.838623] [<ffffffff800930fa>] __percpu_init_rwsem+0x1a/0x98
> [    3.838995] [<ffffffff80286c5a>] alloc_super+0x148/0x3da
> [    3.839631] [<ffffffff80287af2>] sget_fc+0x90/0x2c4
> [    3.840240] [<ffffffff80288498>] get_tree_nodev+0x24/0xa4
> [    3.840598] [<ffffffff801e919a>] shmem_get_tree+0x14/0x1c
> [    3.840891] [<ffffffff80286436>] vfs_get_tree+0x3a/0x11a
> [    3.841172] [<ffffffff802ba746>] path_mount+0x2ea/0xb44
> [    3.841468] [<ffffffff802bb69a>] sys_mount+0x1b2/0x270
> [    3.841790] [<ffffffff80003d1c>] ret_from_syscall+0x0/0x2
> [    3.842480] ---[ end trace 0000000000000000 ]---

Sure, I will try to fix this ASAP.

Are you willing to take this if the above issue is fixed ?

Regards,
Anup

>
> >
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index fba9e9f46a8c..04bcc91c91ea 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -3,10 +3,13 @@
> >   * Copyright (C) 2012 Regents of the University of California
> >   */
> >
> > +#include <linux/cpu.h>
> >  #include <linux/init.h>
> >  #include <linux/seq_file.h>
> >  #include <linux/of.h>
> > +#include <asm/csr.h>
> >  #include <asm/hwcap.h>
> > +#include <asm/sbi.h>
> >  #include <asm/smp.h>
> >  #include <asm/pgtable.h>
> >
> > @@ -64,6 +67,50 @@ int riscv_of_parent_hartid(struct device_node *node)
> >  }
> >
> >  #ifdef CONFIG_PROC_FS
> > +
> > +struct riscv_cpuinfo {
> > +     unsigned long mvendorid;
> > +     unsigned long marchid;
> > +     unsigned long mimpid;
> > +};
> > +static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
> > +
> > +static int riscv_cpuinfo_starting(unsigned int cpu)
> > +{
> > +     struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
> > +
> > +#if IS_ENABLED(CONFIG_RISCV_SBI)
> > +     ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
> > +     ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
> > +     ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
> > +#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
> > +     ci->mvendorid = csr_read(CSR_MVENDORID);
> > +     ci->marchid = csr_read(CSR_MARCHID);
> > +     ci->mimpid = csr_read(CSR_MIMPID);
> > +#else
> > +     ci->mvendorid = 0;
> > +     ci->marchid = 0;
> > +     ci->mimpid = 0;
> > +#endif
> > +
> > +     return 0;
> > +}
> > +
> > +static int __init riscv_cpuinfo_init(void)
> > +{
> > +     int ret;
> > +
> > +     ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/cpuinfo:starting",
> > +                             riscv_cpuinfo_starting, NULL);
> > +     if (ret < 0) {
> > +             pr_err("cpuinfo: failed to register hotplug callbacks.\n");
> > +             return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +device_initcall(riscv_cpuinfo_init);
> > +
> >  #define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
> >       {                                                       \
> >               .uprop = #UPROP,                                \
> > @@ -178,6 +225,7 @@ static int c_show(struct seq_file *m, void *v)
> >  {
> >       unsigned long cpu_id = (unsigned long)v - 1;
> >       struct device_node *node = of_get_cpu_node(cpu_id, NULL);
> > +     struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
> >       const char *compat, *isa;
> >
> >       seq_printf(m, "processor\t: %lu\n", cpu_id);
> > @@ -188,6 +236,9 @@ static int c_show(struct seq_file *m, void *v)
> >       if (!of_property_read_string(node, "compatible", &compat)
> >           && strcmp(compat, "riscv"))
> >               seq_printf(m, "uarch\t\t: %s\n", compat);
> > +     seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
> > +     seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid);
> > +     seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid);
> >       seq_puts(m, "\n");
> >       of_node_put(node);
Anup Patel Oct. 4, 2022, 4:30 a.m. UTC | #9
Hi Palmer,

On Tue, Oct 4, 2022 at 8:24 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Tue, 26 Jul 2022 21:38:29 PDT (-0700), apatel@ventanamicro.com wrote:
> > Identifying the underlying RISC-V implementation can be important
> > for some of the user space applications. For example, the perf tool
> > uses arch specific CPU implementation id (i.e. CPUID) to select a
> > JSON file describing custom perf events on a CPU.
> >
> > Currently, there is no way to identify RISC-V implementation so we
> > add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > Tested-by: Nikita Shubin <n.shubin@yadro.com>
> > ---
> > Changes since v1:
> >  - Use IS_ENABLED() to check CONFIG defines
> >  - Added RB and TB tags in commit description
> > ---
> >  arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
>
> Sorry for being slow on this one.  I'd been worried about sticking more
> stuff into /proc/cpuinfo, as having the only user interface for these
> involve parsing /proc/cpuinfo is really just a recipe for disaster.  I
> didn't get around do doing something better, though, and waiting for
> another release seems kind of silly.
>
> I was going to go put this on for-next, but it looks like it's causing
> kasan boot failures.  I'm not actually quite sure how this is triggering
> these, at least based on the backtrace, but there's a bunch of them and
> boot hangs.
>
> Here's the first of them:
>
> [    3.830416] Hardware name: riscv-virtio,qemu (DT)
> [    3.830828] epc : kasan_check_range+0x116/0x14c
> [    3.832377]  ra : memset+0x1e/0x4c
> [    3.832699] epc : ffffffff8026468e ra : ffffffff80264d6e sp : ff600000832afa60
> [    3.833051]  gp : ffffffff81d800a0 tp : ff600000831d2400 t0 : ff60000083224360
> [    3.833397]  t1 : ffebfffefffef000 t2 : 0000000000000000 s0 : ff600000832afa90
> [    3.833811]  s1 : 0000000000000004 a0 : 0000000000000010 a1 : 0000000000000004
> [    3.834210]  a2 : 0000000000000001 a3 : ffffffff801fa22c a4 : ff5ffffffff78000
> [    3.834558]  a5 : ffebfffefffef000 a6 : ffebfffefffef001 a7 : ff5ffffffff78003
> [    3.834931]  s2 : ff5ffffffff78000 s3 : 0000000000000000 s4 : ffffffff815b59b0
> [    3.835292]  s5 : ffffffff81d815e0 s6 : 0000000000000000 s7 : 0000000000000008
> [    3.836400]  s8 : ffffffff81d8b0e0 s9 : 0000000000000000 s10: 0000000000000008
> [    3.836817]  s11: ff5ffffffff78000 t3 : 0000000000000000 t4 : ffebfffefffef000
> [    3.837154]  t5 : ffebfffefffef001 t6 : 0000000000000002
> [    3.837471] status: 0000000000000120 badaddr: ffebfffefffef000 cause: 000000000000000d
> [    3.837933] [<ffffffff801fa22c>] pcpu_alloc+0x494/0x838
> [    3.838324] [<ffffffff801fa5fc>] __alloc_percpu+0x14/0x1c
> [    3.838623] [<ffffffff800930fa>] __percpu_init_rwsem+0x1a/0x98
> [    3.838995] [<ffffffff80286c5a>] alloc_super+0x148/0x3da
> [    3.839631] [<ffffffff80287af2>] sget_fc+0x90/0x2c4
> [    3.840240] [<ffffffff80288498>] get_tree_nodev+0x24/0xa4
> [    3.840598] [<ffffffff801e919a>] shmem_get_tree+0x14/0x1c
> [    3.840891] [<ffffffff80286436>] vfs_get_tree+0x3a/0x11a
> [    3.841172] [<ffffffff802ba746>] path_mount+0x2ea/0xb44
> [    3.841468] [<ffffffff802bb69a>] sys_mount+0x1b2/0x270
> [    3.841790] [<ffffffff80003d1c>] ret_from_syscall+0x0/0x2
> [    3.842480] ---[ end trace 0000000000000000 ]---
>

I tried several times but I am not able to reproduce this issue
with this patch applied on the latest Linux-6.0. It seems some
other patch is causing the KASAN trace which you are seeing.

Here's the complete boot log .....

$ qemu-system-riscv64 -M virt -m 512M -nographic -bios
opensbi/build/platform/generic/firmware/fw_jump.bin -kernel
./build-riscv64/arch/riscv/boot/Image -append "root=/dev/ram rw
console=ttyS0 earlycon" -initrd ./rootfs_riscv64.img -smp 4

OpenSBI v1.1-54-g7f09fba
   ____                    _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|____/_____|
        | |
        |_|

Platform Name             : riscv-virtio,qemu
Platform Features         : medeleg
Platform HART Count       : 4
Platform IPI Device       : aclint-mswi
Platform Timer Device     : aclint-mtimer @ 10000000Hz
Platform Console Device   : uart8250
Platform HSM Device       : ---
Platform PMU Device       : ---
Platform Reboot Device    : sifive_test
Platform Shutdown Device  : sifive_test
Firmware Base             : 0x80000000
Firmware Size             : 232 KB
Runtime SBI Version       : 1.0

Domain0 Name              : root
Domain0 Boot HART         : 3
Domain0 HARTs             : 0*,1*,2*,3*
Domain0 Region00          : 0x0000000002000000-0x000000000200ffff (I)
Domain0 Region01          : 0x0000000080000000-0x000000008003ffff ()
Domain0 Region02          : 0x0000000000000000-0xffffffffffffffff (R,W,X)
Domain0 Next Address      : 0x0000000080200000
Domain0 Next Arg1         : 0x0000000082200000
Domain0 Next Mode         : S-mode
Domain0 SysReset          : yes

Boot HART ID              : 3
Boot HART Domain          : root
Boot HART Priv Version    : v1.12
Boot HART Base ISA        : rv64imafdch
Boot HART ISA Extensions  : time,sstc
Boot HART PMP Count       : 16
Boot HART PMP Granularity : 4
Boot HART PMP Address Bits: 54
Boot HART MHPM Count      : 16
Boot HART MIDELEG         : 0x0000000000001666
Boot HART MEDELEG         : 0x0000000000f0b509
[    0.000000] Linux version 6.0.0-00001-g04097fca4bd6
(anup@anup-ubuntu64-vm) (riscv64-unknown-linux-gnu-gcc (g1ea978e3066)
12.1.0, GNU ld (GNU Binutils) 2.39) #1 SMP Mon Oct 3 11:04:02 IST 2022
[    0.000000] random: crng init done
[    0.000000] OF: fdt: Ignoring memory range 0x80000000 - 0x80200000
[    0.000000] Machine model: riscv-virtio,qemu
[    0.000000] earlycon: ns16550a0 at MMIO 0x0000000010000000 (options '')
[    0.000000] printk: bootconsole [ns16550a0] enabled
[    0.000000] efi: UEFI not found.
[    0.000000] Zone ranges:
[    0.000000]   DMA32    [mem 0x0000000080200000-0x000000009fffffff]
[    0.000000]   Normal   empty
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000080200000-0x000000009fffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000080200000-0x000000009fffffff]
[    0.000000] SBI specification v1.0 detected
[    0.000000] SBI implementation ID=0x1 Version=0x10001
[    0.000000] SBI TIME extension detected
[    0.000000] SBI IPI extension detected
[    0.000000] SBI RFENCE extension detected
[    0.000000] SBI SRST extension detected
[    0.000000] SBI HSM extension detected
[    0.000000] riscv: base ISA extensions acdfhim
[    0.000000] riscv: ELF capabilities acdfim
[    0.000000] percpu: Embedded 17 pages/cpu s30840 r8192 d30600 u69632
[    0.000000] Built 1 zonelists, mobility grouping on.  Total pages: 128520
[    0.000000] Kernel command line: root=/dev/ram rw console=ttyS0 earlycon
[    0.000000] Dentry cache hash table entries: 65536 (order: 7,
524288 bytes, linear)
[    0.000000] Inode-cache hash table entries: 32768 (order: 6, 262144
bytes, linear)
[    0.000000] mem auto-init: stack:all(zero), heap alloc:off, heap free:off
[    0.000000] stackdepot hash table entries: 1048576 (order: 11,
8388608 bytes, linear)
[    0.000000] Virtual kernel memory layout:
[    0.000000]       fixmap : 0xff1bfffffee00000 - 0xff1bffffff000000
 (2048 kB)
[    0.000000]       pci io : 0xff1bffffff000000 - 0xff1c000000000000
 (  16 MB)
[    0.000000]      vmemmap : 0xff1c000000000000 - 0xff20000000000000
 (1024 TB)
[    0.000000]      vmalloc : 0xff20000000000000 - 0xff60000000000000
 (16384 TB)
[    0.000000]      modules : 0xffffffff01e22000 - 0xffffffff80000000
 (2017 MB)
[    0.000000]       lowmem : 0xff60000000000000 - 0xff6000001fe00000
 ( 510 MB)
[    0.000000]        kasan : 0xffdf000000000000 - 0xffffffff00000000
 (8447 TB)
[    0.000000]       kernel : 0xffffffff80000000 - 0xffffffffffffffff
 (2047 MB)
[    0.000000] Memory: 132036K/522240K available (12642K kernel code,
7564K rwdata, 6144K rodata, 2193K init, 612K bss, 390204K reserved, 0K
cma-reserved)
[    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[    0.000000] rcu: Hierarchical RCU implementation.
[    0.000000] rcu:     RCU restricting CPUs from NR_CPUS=8 to nr_cpu_ids=4.
[    0.000000] rcu:     RCU debug extended QS entry/exit.
[    0.000000]     Tracing variant of Tasks RCU enabled.
[    0.000000] rcu: RCU calculated value of scheduler-enlistment delay
is 25 jiffies.
[    0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=4
[    0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[    0.000000] riscv-intc: 64 local interrupts mapped
[    0.000000] plic: plic@c000000: mapped 96 interrupts with 4
handlers for 8 contexts.
[    0.000000] rcu: srcu_init: Setting srcu_struct sizes based on contention.
[    0.000000] riscv-timer: riscv_timer_init_dt: Registering
clocksource cpuid [0] hartid [3]
[    0.000000] clocksource: riscv_clocksource: mask:
0xffffffffffffffff max_cycles: 0x24e6a1710, max_idle_ns: 440795202120
ns
[    0.000167] sched_clock: 64 bits at 10MHz, resolution 100ns, wraps
every 4398046511100ns
[    0.006196] riscv-timer: Timer interrupt in S-mode is available via
sstc extension
[    0.033533] Console: colour dummy device 80x25
[    0.039665] Calibrating delay loop (skipped), value calculated
using timer frequency.. 20.00 BogoMIPS (lpj=40000)
[    0.041873] pid_max: default: 32768 minimum: 301
[    0.047970] LSM: Security Framework initializing
[    0.056472] Mount-cache hash table entries: 1024 (order: 1, 8192
bytes, linear)
[    0.057651] Mountpoint-cache hash table entries: 1024 (order: 1,
8192 bytes, linear)
[    0.194551] cblist_init_generic: Setting adjustable number of
callback queues.
[    0.196019] cblist_init_generic: Setting shift to 2 and lim to 1.
[    0.198820] riscv: ELF compat mode supported
[    0.199475] ASID allocator using 16 bits (65536 entries)
[    0.204535] rcu: Hierarchical SRCU implementation.
[    0.205144] rcu:     Max phase no-delay instances is 1000.
[    0.220950] EFI services will not be available.
[    0.257578] smp: Bringing up secondary CPUs ...
[    0.330519] smp: Brought up 1 node, 4 CPUs
[    0.364217] devtmpfs: initialized
[    0.441404] clocksource: jiffies: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 7645041785100000 ns
[    0.442783] futex hash table entries: 1024 (order: 4, 65536 bytes, linear)
[    0.449422] pinctrl core: initialized pinctrl subsystem
[    0.463479] NET: Registered PF_NETLINK/PF_ROUTE protocol family
[    0.480895] DMA: preallocated 128 KiB GFP_KERNEL pool for atomic allocations
[    0.482798] DMA: preallocated 128 KiB GFP_KERNEL|GFP_DMA32 pool for
atomic allocations
[    0.487607] audit: initializing netlink subsys (disabled)
[    0.496356] audit: type=2000 audit(0.352:1): state=initialized
audit_enabled=0 res=1
[    0.521944] cpuidle: using governor menu
[    0.764786] HugeTLB: registered 2.00 MiB page size, pre-allocated 0 pages
[    0.765347] HugeTLB: 0 KiB vmemmap can be freed for a 2.00 MiB page
[    0.778167] iommu: Default domain type: Translated
[    0.778797] iommu: DMA domain TLB invalidation policy: strict mode
[    0.785936] SCSI subsystem initialized
[    0.793815] usbcore: registered new interface driver usbfs
[    0.795355] usbcore: registered new interface driver hub
[    0.796867] usbcore: registered new device driver usb
[    0.862701] vgaarb: loaded
[    0.873160] clocksource: Switched to clocksource riscv_clocksource
[    1.090734] NET: Registered PF_INET protocol family
[    1.094533] IP idents hash table entries: 8192 (order: 4, 65536
bytes, linear)
[    1.116613] tcp_listen_portaddr_hash hash table entries: 256
(order: 1, 8192 bytes, linear)
[    1.119396] Table-perturb hash table entries: 65536 (order: 6,
262144 bytes, linear)
[    1.122509] TCP established hash table entries: 4096 (order: 3,
32768 bytes, linear)
[    1.125009] TCP bind hash table entries: 4096 (order: 5, 131072
bytes, linear)
[    1.130044] TCP: Hash tables configured (established 4096 bind 4096)
[    1.136668] UDP hash table entries: 256 (order: 2, 24576 bytes, linear)
[    1.138039] UDP-Lite hash table entries: 256 (order: 2, 24576 bytes, linear)
[    1.142935] NET: Registered PF_UNIX/PF_LOCAL protocol family
[    1.166580] RPC: Registered named UNIX socket transport module.
[    1.168292] RPC: Registered udp transport module.
[    1.169043] RPC: Registered tcp transport module.
[    1.169616] RPC: Registered tcp NFSv4.1 backchannel transport module.
[    1.170766] PCI: CLS 0 bytes, default 64
[    1.193634] Unpacking initramfs...
[    1.234821] workingset: timestamp_bits=46 max_order=16 bucket_order=0
[    1.481070] NFS: Registering the id_resolver key type
[    1.486758] Key type id_resolver registered
[    1.488052] Key type id_legacy registered
[    1.492906] nfs4filelayout_init: NFSv4 File Layout Driver Registering...
[    1.494170] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver
Registering...
[    1.499783] 9p: Installing v9fs 9p2000 file system support
[    1.509514] NET: Registered PF_ALG protocol family
[    1.512535] Block layer SCSI generic (bsg) driver version 0.4
loaded (major 250)
[    1.514517] io scheduler mq-deadline registered
[    1.515786] io scheduler kyber registered
[    1.625664] pci-host-generic 30000000.pci: host bridge
/soc/pci@30000000 ranges:
[    1.628610] pci-host-generic 30000000.pci:       IO
0x0003000000..0x000300ffff -> 0x0000000000
[    1.631279] pci-host-generic 30000000.pci:      MEM
0x0040000000..0x007fffffff -> 0x0040000000
[    1.632791] pci-host-generic 30000000.pci:      MEM
0x0400000000..0x07ffffffff -> 0x0400000000
[    1.635612] pci-host-generic 30000000.pci: Memory resource size
exceeds max for 32 bits
[    1.651366] pci-host-generic 30000000.pci: ECAM at [mem
0x30000000-0x3fffffff] for [bus 00-ff]
[    1.663042] pci-host-generic 30000000.pci: PCI host bridge to bus 0000:00
[    1.665769] pci_bus 0000:00: root bus resource [bus 00-ff]
[    1.667976] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
[    1.669508] pci_bus 0000:00: root bus resource [mem 0x40000000-0x7fffffff]
[    1.670762] pci_bus 0000:00: root bus resource [mem 0x400000000-0x7ffffffff]
[    1.676881] pci 0000:00:00.0: [1b36:0008] type 00 class 0x060000
[    1.811192] Freeing initrd memory: 12484K
[    2.748960] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[    2.788856] printk: console [ttyS0] disabled
[    2.796353] 10000000.serial: ttyS0 at MMIO 0x10000000 (irq = 1,
base_baud = 230400) is a 16550A
[    2.800108] printk: console [ttyS0] enabled
[    2.800108] printk: console [ttyS0] enabled
[    2.801134] printk: bootconsole [ns16550a0] disabled
[    2.801134] printk: bootconsole [ns16550a0] disabled
[    2.942562] loop: module loaded
[    2.977068] e1000e: Intel(R) PRO/1000 Network Driver
[    2.977716] e1000e: Copyright(c) 1999 - 2015 Intel Corporation.
[    2.981896] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    2.983611] ehci-pci: EHCI PCI platform driver
[    2.984953] ehci-platform: EHCI generic platform driver
[    2.986438] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[    2.988260] ohci-pci: OHCI PCI platform driver
[    2.990070] ohci-platform: OHCI generic platform driver
[    2.996940] usbcore: registered new interface driver uas
[    2.998376] usbcore: registered new interface driver usb-storage
[    3.003800] mousedev: PS/2 mouse device common for all mice
[    3.025924] goldfish_rtc 101000.rtc: registered as rtc0
[    3.027210] goldfish_rtc 101000.rtc: setting system clock to
2022-10-04T04:25:44 UTC (1664857544)
[    3.050953] syscon-poweroff poweroff: pm_power_off already claimed
for sbi_srst_power_off
[    3.053152] syscon-poweroff: probe of poweroff failed with error -16
[    3.062649] sdhci: Secure Digital Host Controller Interface driver
[    3.063888] sdhci: Copyright(c) Pierre Ossman
[    3.064920] sdhci-pltfm: SDHCI platform and OF driver helper
[    3.071575] usbcore: registered new interface driver usbhid
[    3.074086] usbhid: USB HID core driver
[    3.078343] riscv-pmu-sbi: SBI PMU extension is available
[    3.082211] riscv-pmu-sbi: 16 firmware and 18 hardware counters
[    3.082844] riscv-pmu-sbi: Perf sampling/filtering is not supported
as sscof extension is not available
[    3.117166] NET: Registered PF_INET6 protocol family
[    3.157784] Segment Routing with IPv6
[    3.159593] In-situ OAM (IOAM) with IPv6
[    3.162181] sit: IPv6, IPv4 and MPLS over IPv4 tunneling driver
[    3.175768] NET: Registered PF_PACKET protocol family
[    3.181457] 9pnet: Installing 9P2000 support
[    3.183152] Key type dns_resolver registered
[    3.210151] debug_vm_pgtable: [debug_vm_pgtable         ]:
Validating architecture page table helpers
[    3.335692] Freeing unused kernel image (initmem) memory: 2192K
[    3.359821] Run /init as init process
           _  _
          | ||_|
          | | _ ____  _   _  _  _
          | || |  _ \| | | |\ \/ /
          | || | | | | |_| |/    \
          |_||_|_| |_|\____|\_/\_/

               Busybox Rootfs

Please press Enter to activate this console.
/ #
/ # dmesg | grep -i kasan
[    0.000000]        kasan : 0xffdf000000000000 - 0xffffffff00000000
 (8447 TB)
/ #
/ #
/ #
/ #
/ # uptime
 04:26:31 up 0 min,  0 users,  load average: 0.00, 0.00, 0.00
/ # cat /proc/cpuinfo
processor    : 0
hart        : 3
isa        : rv64imafdch_zihintpause_sstc
mmu        : sv57
mvendorid    : 0x0
marchid        : 0x70132
mimpid        : 0x70132

processor    : 1
hart        : 0
isa        : rv64imafdch_zihintpause_sstc
mmu        : sv57
mvendorid    : 0x0
marchid        : 0x70132
mimpid        : 0x70132

processor    : 2
hart        : 1
isa        : rv64imafdch_zihintpause_sstc
mmu        : sv57
mvendorid    : 0x0
marchid        : 0x70132
mimpid        : 0x70132

processor    : 3
hart        : 2
isa        : rv64imafdch_zihintpause_sstc
mmu        : sv57
mvendorid    : 0x0
marchid        : 0x70132
mimpid        : 0x70132

Regards,
Anup
Palmer Dabbelt Oct. 4, 2022, 4:35 a.m. UTC | #10
On Mon, 03 Oct 2022 20:44:54 PDT (-0700), anup@brainfault.org wrote:
> On Tue, Oct 4, 2022 at 8:24 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> On Tue, 26 Jul 2022 21:38:29 PDT (-0700), apatel@ventanamicro.com wrote:
>> > Identifying the underlying RISC-V implementation can be important
>> > for some of the user space applications. For example, the perf tool
>> > uses arch specific CPU implementation id (i.e. CPUID) to select a
>> > JSON file describing custom perf events on a CPU.
>> >
>> > Currently, there is no way to identify RISC-V implementation so we
>> > add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
>> >
>> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>> > Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> > Tested-by: Nikita Shubin <n.shubin@yadro.com>
>> > ---
>> > Changes since v1:
>> >  - Use IS_ENABLED() to check CONFIG defines
>> >  - Added RB and TB tags in commit description
>> > ---
>> >  arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 51 insertions(+)
>>
>> Sorry for being slow on this one.  I'd been worried about sticking more
>> stuff into /proc/cpuinfo, as having the only user interface for these
>> involve parsing /proc/cpuinfo is really just a recipe for disaster.  I
>> didn't get around do doing something better, though, and waiting for
>> another release seems kind of silly.
>
> In addition to being useful for perf tool, printing vendor and
> implementation details in /proc/cpuinfo is a very useful info.
>
> Users can easily know the underlying CPU implementer using
> "cat /proc/cpuinfo".
>
>>
>> I was going to go put this on for-next, but it looks like it's causing
>> kasan boot failures.  I'm not actually quite sure how this is triggering
>> these, at least based on the backtrace, but there's a bunch of them and
>> boot hangs.
>>
>> Here's the first of them:
>>
>> [    3.830416] Hardware name: riscv-virtio,qemu (DT)
>> [    3.830828] epc : kasan_check_range+0x116/0x14c
>> [    3.832377]  ra : memset+0x1e/0x4c
>> [    3.832699] epc : ffffffff8026468e ra : ffffffff80264d6e sp : ff600000832afa60
>> [    3.833051]  gp : ffffffff81d800a0 tp : ff600000831d2400 t0 : ff60000083224360
>> [    3.833397]  t1 : ffebfffefffef000 t2 : 0000000000000000 s0 : ff600000832afa90
>> [    3.833811]  s1 : 0000000000000004 a0 : 0000000000000010 a1 : 0000000000000004
>> [    3.834210]  a2 : 0000000000000001 a3 : ffffffff801fa22c a4 : ff5ffffffff78000
>> [    3.834558]  a5 : ffebfffefffef000 a6 : ffebfffefffef001 a7 : ff5ffffffff78003
>> [    3.834931]  s2 : ff5ffffffff78000 s3 : 0000000000000000 s4 : ffffffff815b59b0
>> [    3.835292]  s5 : ffffffff81d815e0 s6 : 0000000000000000 s7 : 0000000000000008
>> [    3.836400]  s8 : ffffffff81d8b0e0 s9 : 0000000000000000 s10: 0000000000000008
>> [    3.836817]  s11: ff5ffffffff78000 t3 : 0000000000000000 t4 : ffebfffefffef000
>> [    3.837154]  t5 : ffebfffefffef001 t6 : 0000000000000002
>> [    3.837471] status: 0000000000000120 badaddr: ffebfffefffef000 cause: 000000000000000d
>> [    3.837933] [<ffffffff801fa22c>] pcpu_alloc+0x494/0x838
>> [    3.838324] [<ffffffff801fa5fc>] __alloc_percpu+0x14/0x1c
>> [    3.838623] [<ffffffff800930fa>] __percpu_init_rwsem+0x1a/0x98
>> [    3.838995] [<ffffffff80286c5a>] alloc_super+0x148/0x3da
>> [    3.839631] [<ffffffff80287af2>] sget_fc+0x90/0x2c4
>> [    3.840240] [<ffffffff80288498>] get_tree_nodev+0x24/0xa4
>> [    3.840598] [<ffffffff801e919a>] shmem_get_tree+0x14/0x1c
>> [    3.840891] [<ffffffff80286436>] vfs_get_tree+0x3a/0x11a
>> [    3.841172] [<ffffffff802ba746>] path_mount+0x2ea/0xb44
>> [    3.841468] [<ffffffff802bb69a>] sys_mount+0x1b2/0x270
>> [    3.841790] [<ffffffff80003d1c>] ret_from_syscall+0x0/0x2
>> [    3.842480] ---[ end trace 0000000000000000 ]---
>
> Sure, I will try to fix this ASAP.
>
> Are you willing to take this if the above issue is fixed ?

I still think the other interface is better, but it's taking too long 
and some stuff is blocked on getting these to userspace so I'm fine with 
it.

>
> Regards,
> Anup
>
>>
>> >
>> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
>> > index fba9e9f46a8c..04bcc91c91ea 100644
>> > --- a/arch/riscv/kernel/cpu.c
>> > +++ b/arch/riscv/kernel/cpu.c
>> > @@ -3,10 +3,13 @@
>> >   * Copyright (C) 2012 Regents of the University of California
>> >   */
>> >
>> > +#include <linux/cpu.h>
>> >  #include <linux/init.h>
>> >  #include <linux/seq_file.h>
>> >  #include <linux/of.h>
>> > +#include <asm/csr.h>
>> >  #include <asm/hwcap.h>
>> > +#include <asm/sbi.h>
>> >  #include <asm/smp.h>
>> >  #include <asm/pgtable.h>
>> >
>> > @@ -64,6 +67,50 @@ int riscv_of_parent_hartid(struct device_node *node)
>> >  }
>> >
>> >  #ifdef CONFIG_PROC_FS
>> > +
>> > +struct riscv_cpuinfo {
>> > +     unsigned long mvendorid;
>> > +     unsigned long marchid;
>> > +     unsigned long mimpid;
>> > +};
>> > +static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
>> > +
>> > +static int riscv_cpuinfo_starting(unsigned int cpu)
>> > +{
>> > +     struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
>> > +
>> > +#if IS_ENABLED(CONFIG_RISCV_SBI)
>> > +     ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
>> > +     ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
>> > +     ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
>> > +#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
>> > +     ci->mvendorid = csr_read(CSR_MVENDORID);
>> > +     ci->marchid = csr_read(CSR_MARCHID);
>> > +     ci->mimpid = csr_read(CSR_MIMPID);
>> > +#else
>> > +     ci->mvendorid = 0;
>> > +     ci->marchid = 0;
>> > +     ci->mimpid = 0;
>> > +#endif
>> > +
>> > +     return 0;
>> > +}
>> > +
>> > +static int __init riscv_cpuinfo_init(void)
>> > +{
>> > +     int ret;
>> > +
>> > +     ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/cpuinfo:starting",
>> > +                             riscv_cpuinfo_starting, NULL);
>> > +     if (ret < 0) {
>> > +             pr_err("cpuinfo: failed to register hotplug callbacks.\n");
>> > +             return ret;
>> > +     }
>> > +
>> > +     return 0;
>> > +}
>> > +device_initcall(riscv_cpuinfo_init);
>> > +
>> >  #define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
>> >       {                                                       \
>> >               .uprop = #UPROP,                                \
>> > @@ -178,6 +225,7 @@ static int c_show(struct seq_file *m, void *v)
>> >  {
>> >       unsigned long cpu_id = (unsigned long)v - 1;
>> >       struct device_node *node = of_get_cpu_node(cpu_id, NULL);
>> > +     struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
>> >       const char *compat, *isa;
>> >
>> >       seq_printf(m, "processor\t: %lu\n", cpu_id);
>> > @@ -188,6 +236,9 @@ static int c_show(struct seq_file *m, void *v)
>> >       if (!of_property_read_string(node, "compatible", &compat)
>> >           && strcmp(compat, "riscv"))
>> >               seq_printf(m, "uarch\t\t: %s\n", compat);
>> > +     seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
>> > +     seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid);
>> > +     seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid);
>> >       seq_puts(m, "\n");
>> >       of_node_put(node);
Heiko Stübner Oct. 11, 2022, 7:59 a.m. UTC | #11
Am Mittwoch, 27. Juli 2022, 06:38:29 CEST schrieb Anup Patel:
> Identifying the underlying RISC-V implementation can be important
> for some of the user space applications. For example, the perf tool
> uses arch specific CPU implementation id (i.e. CPUID) to select a
> JSON file describing custom perf events on a CPU.
> 
> Currently, there is no way to identify RISC-V implementation so we
> add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Tested-by: Nikita Shubin <n.shubin@yadro.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
[on Qemu and Allwinner D1]
Tested-by: Heiko Stuebner <heiko@sntech.de>
Palmer Dabbelt Oct. 13, 2022, 9:25 p.m. UTC | #12
On Mon, 03 Oct 2022 21:35:25 PDT (-0700), Palmer Dabbelt wrote:
> On Mon, 03 Oct 2022 20:44:54 PDT (-0700), anup@brainfault.org wrote:
>> On Tue, Oct 4, 2022 at 8:24 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>>
>>> On Tue, 26 Jul 2022 21:38:29 PDT (-0700), apatel@ventanamicro.com wrote:
>>> > Identifying the underlying RISC-V implementation can be important
>>> > for some of the user space applications. For example, the perf tool
>>> > uses arch specific CPU implementation id (i.e. CPUID) to select a
>>> > JSON file describing custom perf events on a CPU.
>>> >
>>> > Currently, there is no way to identify RISC-V implementation so we
>>> > add mvendorid, marchid, and mimpid to /proc/cpuinfo output.
>>> >
>>> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>>> > Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>> > Tested-by: Nikita Shubin <n.shubin@yadro.com>
>>> > ---
>>> > Changes since v1:
>>> >  - Use IS_ENABLED() to check CONFIG defines
>>> >  - Added RB and TB tags in commit description
>>> > ---
>>> >  arch/riscv/kernel/cpu.c | 51 +++++++++++++++++++++++++++++++++++++++++
>>> >  1 file changed, 51 insertions(+)
>>>
>>> Sorry for being slow on this one.  I'd been worried about sticking more
>>> stuff into /proc/cpuinfo, as having the only user interface for these
>>> involve parsing /proc/cpuinfo is really just a recipe for disaster.  I
>>> didn't get around do doing something better, though, and waiting for
>>> another release seems kind of silly.
>>
>> In addition to being useful for perf tool, printing vendor and
>> implementation details in /proc/cpuinfo is a very useful info.
>>
>> Users can easily know the underlying CPU implementer using
>> "cat /proc/cpuinfo".
>>
>>>
>>> I was going to go put this on for-next, but it looks like it's causing
>>> kasan boot failures.  I'm not actually quite sure how this is triggering
>>> these, at least based on the backtrace, but there's a bunch of them and
>>> boot hangs.
>>>
>>> Here's the first of them:
>>>
>>> [    3.830416] Hardware name: riscv-virtio,qemu (DT)
>>> [    3.830828] epc : kasan_check_range+0x116/0x14c
>>> [    3.832377]  ra : memset+0x1e/0x4c
>>> [    3.832699] epc : ffffffff8026468e ra : ffffffff80264d6e sp : ff600000832afa60
>>> [    3.833051]  gp : ffffffff81d800a0 tp : ff600000831d2400 t0 : ff60000083224360
>>> [    3.833397]  t1 : ffebfffefffef000 t2 : 0000000000000000 s0 : ff600000832afa90
>>> [    3.833811]  s1 : 0000000000000004 a0 : 0000000000000010 a1 : 0000000000000004
>>> [    3.834210]  a2 : 0000000000000001 a3 : ffffffff801fa22c a4 : ff5ffffffff78000
>>> [    3.834558]  a5 : ffebfffefffef000 a6 : ffebfffefffef001 a7 : ff5ffffffff78003
>>> [    3.834931]  s2 : ff5ffffffff78000 s3 : 0000000000000000 s4 : ffffffff815b59b0
>>> [    3.835292]  s5 : ffffffff81d815e0 s6 : 0000000000000000 s7 : 0000000000000008
>>> [    3.836400]  s8 : ffffffff81d8b0e0 s9 : 0000000000000000 s10: 0000000000000008
>>> [    3.836817]  s11: ff5ffffffff78000 t3 : 0000000000000000 t4 : ffebfffefffef000
>>> [    3.837154]  t5 : ffebfffefffef001 t6 : 0000000000000002
>>> [    3.837471] status: 0000000000000120 badaddr: ffebfffefffef000 cause: 000000000000000d
>>> [    3.837933] [<ffffffff801fa22c>] pcpu_alloc+0x494/0x838
>>> [    3.838324] [<ffffffff801fa5fc>] __alloc_percpu+0x14/0x1c
>>> [    3.838623] [<ffffffff800930fa>] __percpu_init_rwsem+0x1a/0x98
>>> [    3.838995] [<ffffffff80286c5a>] alloc_super+0x148/0x3da
>>> [    3.839631] [<ffffffff80287af2>] sget_fc+0x90/0x2c4
>>> [    3.840240] [<ffffffff80288498>] get_tree_nodev+0x24/0xa4
>>> [    3.840598] [<ffffffff801e919a>] shmem_get_tree+0x14/0x1c
>>> [    3.840891] [<ffffffff80286436>] vfs_get_tree+0x3a/0x11a
>>> [    3.841172] [<ffffffff802ba746>] path_mount+0x2ea/0xb44
>>> [    3.841468] [<ffffffff802bb69a>] sys_mount+0x1b2/0x270
>>> [    3.841790] [<ffffffff80003d1c>] ret_from_syscall+0x0/0x2
>>> [    3.842480] ---[ end trace 0000000000000000 ]---
>>
>> Sure, I will try to fix this ASAP.
>>
>> Are you willing to take this if the above issue is fixed ?
>
> I still think the other interface is better, but it's taking too long
> and some stuff is blocked on getting these to userspace so I'm fine with
> it.

Replying to my own email here as I'm a bit mixed up right now from 
trying to convert over and I can't find Anup's response saying he can't 
reproduce the issues:

It looks like this was just bad luck, for some reason it happens these 
random failures started triggering for me right when I was merging the 
patch (I even went back and they went away).  So I think these were just 
spurious failures.  
https://lore.kernel.org/all/20221009083050.3814850-1-panqinglin2020@iscas.ac.cn/ 
is out as a proposed fix, I've got it in my testing repo as it's making 
the crashes stop -- it smells a bit fishy, though, so I want to get some 
more time to look before merging it.

This one, though, is on for-next.

Thanks!

>
>>
>> Regards,
>> Anup
>>
>>>
>>> >
>>> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
>>> > index fba9e9f46a8c..04bcc91c91ea 100644
>>> > --- a/arch/riscv/kernel/cpu.c
>>> > +++ b/arch/riscv/kernel/cpu.c
>>> > @@ -3,10 +3,13 @@
>>> >   * Copyright (C) 2012 Regents of the University of California
>>> >   */
>>> >
>>> > +#include <linux/cpu.h>
>>> >  #include <linux/init.h>
>>> >  #include <linux/seq_file.h>
>>> >  #include <linux/of.h>
>>> > +#include <asm/csr.h>
>>> >  #include <asm/hwcap.h>
>>> > +#include <asm/sbi.h>
>>> >  #include <asm/smp.h>
>>> >  #include <asm/pgtable.h>
>>> >
>>> > @@ -64,6 +67,50 @@ int riscv_of_parent_hartid(struct device_node *node)
>>> >  }
>>> >
>>> >  #ifdef CONFIG_PROC_FS
>>> > +
>>> > +struct riscv_cpuinfo {
>>> > +     unsigned long mvendorid;
>>> > +     unsigned long marchid;
>>> > +     unsigned long mimpid;
>>> > +};
>>> > +static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
>>> > +
>>> > +static int riscv_cpuinfo_starting(unsigned int cpu)
>>> > +{
>>> > +     struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
>>> > +
>>> > +#if IS_ENABLED(CONFIG_RISCV_SBI)
>>> > +     ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
>>> > +     ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
>>> > +     ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
>>> > +#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
>>> > +     ci->mvendorid = csr_read(CSR_MVENDORID);
>>> > +     ci->marchid = csr_read(CSR_MARCHID);
>>> > +     ci->mimpid = csr_read(CSR_MIMPID);
>>> > +#else
>>> > +     ci->mvendorid = 0;
>>> > +     ci->marchid = 0;
>>> > +     ci->mimpid = 0;
>>> > +#endif
>>> > +
>>> > +     return 0;
>>> > +}
>>> > +
>>> > +static int __init riscv_cpuinfo_init(void)
>>> > +{
>>> > +     int ret;
>>> > +
>>> > +     ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/cpuinfo:starting",
>>> > +                             riscv_cpuinfo_starting, NULL);
>>> > +     if (ret < 0) {
>>> > +             pr_err("cpuinfo: failed to register hotplug callbacks.\n");
>>> > +             return ret;
>>> > +     }
>>> > +
>>> > +     return 0;
>>> > +}
>>> > +device_initcall(riscv_cpuinfo_init);
>>> > +
>>> >  #define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
>>> >       {                                                       \
>>> >               .uprop = #UPROP,                                \
>>> > @@ -178,6 +225,7 @@ static int c_show(struct seq_file *m, void *v)
>>> >  {
>>> >       unsigned long cpu_id = (unsigned long)v - 1;
>>> >       struct device_node *node = of_get_cpu_node(cpu_id, NULL);
>>> > +     struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
>>> >       const char *compat, *isa;
>>> >
>>> >       seq_printf(m, "processor\t: %lu\n", cpu_id);
>>> > @@ -188,6 +236,9 @@ static int c_show(struct seq_file *m, void *v)
>>> >       if (!of_property_read_string(node, "compatible", &compat)
>>> >           && strcmp(compat, "riscv"))
>>> >               seq_printf(m, "uarch\t\t: %s\n", compat);
>>> > +     seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
>>> > +     seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid);
>>> > +     seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid);
>>> >       seq_puts(m, "\n");
>>> >       of_node_put(node);
diff mbox series

Patch

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index fba9e9f46a8c..04bcc91c91ea 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -3,10 +3,13 @@ 
  * Copyright (C) 2012 Regents of the University of California
  */
 
+#include <linux/cpu.h>
 #include <linux/init.h>
 #include <linux/seq_file.h>
 #include <linux/of.h>
+#include <asm/csr.h>
 #include <asm/hwcap.h>
+#include <asm/sbi.h>
 #include <asm/smp.h>
 #include <asm/pgtable.h>
 
@@ -64,6 +67,50 @@  int riscv_of_parent_hartid(struct device_node *node)
 }
 
 #ifdef CONFIG_PROC_FS
+
+struct riscv_cpuinfo {
+	unsigned long mvendorid;
+	unsigned long marchid;
+	unsigned long mimpid;
+};
+static DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
+
+static int riscv_cpuinfo_starting(unsigned int cpu)
+{
+	struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
+
+#if IS_ENABLED(CONFIG_RISCV_SBI)
+	ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
+	ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
+	ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
+#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
+	ci->mvendorid = csr_read(CSR_MVENDORID);
+	ci->marchid = csr_read(CSR_MARCHID);
+	ci->mimpid = csr_read(CSR_MIMPID);
+#else
+	ci->mvendorid = 0;
+	ci->marchid = 0;
+	ci->mimpid = 0;
+#endif
+
+	return 0;
+}
+
+static int __init riscv_cpuinfo_init(void)
+{
+	int ret;
+
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/cpuinfo:starting",
+				riscv_cpuinfo_starting, NULL);
+	if (ret < 0) {
+		pr_err("cpuinfo: failed to register hotplug callbacks.\n");
+		return ret;
+	}
+
+	return 0;
+}
+device_initcall(riscv_cpuinfo_init);
+
 #define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
 	{							\
 		.uprop = #UPROP,				\
@@ -178,6 +225,7 @@  static int c_show(struct seq_file *m, void *v)
 {
 	unsigned long cpu_id = (unsigned long)v - 1;
 	struct device_node *node = of_get_cpu_node(cpu_id, NULL);
+	struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
 	const char *compat, *isa;
 
 	seq_printf(m, "processor\t: %lu\n", cpu_id);
@@ -188,6 +236,9 @@  static int c_show(struct seq_file *m, void *v)
 	if (!of_property_read_string(node, "compatible", &compat)
 	    && strcmp(compat, "riscv"))
 		seq_printf(m, "uarch\t\t: %s\n", compat);
+	seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
+	seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid);
+	seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid);
 	seq_puts(m, "\n");
 	of_node_put(node);