[PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Evan Green]

In /proc/cpuinfo, most of the information we show for each processor is
specific to that hart: marchid, mvendorid, mimpid, processor, hart,
compatible, and the mmu size. But the ISA string gets filtered through a
lowest common denominator mask, so that if one CPU is missing an ISA
extension, no CPUs will show it.

Now that we track the ISA extensions for each hart, let's report ISA
extension info accurately per-hart in /proc/cpuinfo. We cannot change
the "isa:" line, as usermode may be relying on that line to show only
the common set of extensions supported across all harts. Add a new "hart
isa" line instead, which reports the true set of extensions for that
hart. This matches what is returned in riscv_hwprobe() when querying a
given hart.

Signed-off-by: Evan Green <evan@rivosinc.com>

---

Changes in v2:
 - Added new "hart isa" line rather than altering behavior of existing
   "isa" line (Conor, Palmer)


I based this series on top of Conor's riscv-extensions-strings branch
from July 3rd, since otherwise this change gets hopelessly entangled
with that series.

I was unsure if I could snuggle the new "hart isa" line in just below
"isa". Aesthetically it would be quite pleasing, but it runs the risk of
breaking brittle usermode parsers that are assuming a specific line
order. So I put it at the end.

---
 arch/riscv/kernel/cpu.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 1acf3679600d..6264b7b94945 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -197,9 +197,8 @@ arch_initcall(riscv_cpuinfo_init);
 
 #ifdef CONFIG_PROC_FS
 
-static void print_isa(struct seq_file *f)
+static void print_isa(struct seq_file *f, const unsigned long *isa_bitmap)
 {
-	seq_puts(f, "isa\t\t: ");
 
 	if (IS_ENABLED(CONFIG_32BIT))
 		seq_write(f, "rv32", 4);
@@ -207,7 +206,7 @@ static void print_isa(struct seq_file *f)
 		seq_write(f, "rv64", 4);
 
 	for (int i = 0; i < riscv_isa_ext_count; i++) {
-		if (!__riscv_isa_extension_available(NULL, riscv_isa_ext[i].id))
+		if (!__riscv_isa_extension_available(isa_bitmap, riscv_isa_ext[i].id))
 			continue;
 
 		/* Only multi-letter extensions are split by underscores */
@@ -271,7 +270,15 @@ static int c_show(struct seq_file *m, void *v)
 
 	seq_printf(m, "processor\t: %lu\n", cpu_id);
 	seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id));
-	print_isa(m);
+
+	/*
+	 * For historical raisins, the isa: line is limited to the lowest common
+	 * denominator of extensions supported across all harts. A true list of
+	 * extensions supported on this hart is printed later in the hart_isa:
+	 * line.
+	 */
+	seq_puts(m, "isa\t\t: ");
+	print_isa(m, NULL);
 	print_mmu(m);
 
 	if (acpi_disabled) {
@@ -287,6 +294,13 @@ static int c_show(struct seq_file *m, void *v)
 	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);
+
+	/*
+	 * Print the ISA extensions specific to this hart, which may show
+	 * additional extensions not present across all harts.
+	 */
+	seq_puts(m, "hart isa\t: ");
+	print_isa(m, hart_isa[cpu_id].isa);
 	seq_puts(m, "\n");
 
 	return 0;
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Evan Green]

In /proc/cpuinfo, most of the information we show for each processor is
specific to that hart: marchid, mvendorid, mimpid, processor, hart,
compatible, and the mmu size. But the ISA string gets filtered through a
lowest common denominator mask, so that if one CPU is missing an ISA
extension, no CPUs will show it.

Now that we track the ISA extensions for each hart, let's report ISA
extension info accurately per-hart in /proc/cpuinfo. We cannot change
the "isa:" line, as usermode may be relying on that line to show only
the common set of extensions supported across all harts. Add a new "hart
isa" line instead, which reports the true set of extensions for that
hart. This matches what is returned in riscv_hwprobe() when querying a
given hart.

Signed-off-by: Evan Green <evan@rivosinc.com>

---

Changes in v2:
 - Added new "hart isa" line rather than altering behavior of existing
   "isa" line (Conor, Palmer)


I based this series on top of Conor's riscv-extensions-strings branch
from July 3rd, since otherwise this change gets hopelessly entangled
with that series.

I was unsure if I could snuggle the new "hart isa" line in just below
"isa". Aesthetically it would be quite pleasing, but it runs the risk of
breaking brittle usermode parsers that are assuming a specific line
order. So I put it at the end.

---
 arch/riscv/kernel/cpu.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 1acf3679600d..6264b7b94945 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -197,9 +197,8 @@ arch_initcall(riscv_cpuinfo_init);
 
 #ifdef CONFIG_PROC_FS
 
-static void print_isa(struct seq_file *f)
+static void print_isa(struct seq_file *f, const unsigned long *isa_bitmap)
 {
-	seq_puts(f, "isa\t\t: ");
 
 	if (IS_ENABLED(CONFIG_32BIT))
 		seq_write(f, "rv32", 4);
@@ -207,7 +206,7 @@ static void print_isa(struct seq_file *f)
 		seq_write(f, "rv64", 4);
 
 	for (int i = 0; i < riscv_isa_ext_count; i++) {
-		if (!__riscv_isa_extension_available(NULL, riscv_isa_ext[i].id))
+		if (!__riscv_isa_extension_available(isa_bitmap, riscv_isa_ext[i].id))
 			continue;
 
 		/* Only multi-letter extensions are split by underscores */
@@ -271,7 +270,15 @@ static int c_show(struct seq_file *m, void *v)
 
 	seq_printf(m, "processor\t: %lu\n", cpu_id);
 	seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id));
-	print_isa(m);
+
+	/*
+	 * For historical raisins, the isa: line is limited to the lowest common
+	 * denominator of extensions supported across all harts. A true list of
+	 * extensions supported on this hart is printed later in the hart_isa:
+	 * line.
+	 */
+	seq_puts(m, "isa\t\t: ");
+	print_isa(m, NULL);
 	print_mmu(m);
 
 	if (acpi_disabled) {
@@ -287,6 +294,13 @@ static int c_show(struct seq_file *m, void *v)
 	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);
+
+	/*
+	 * Print the ISA extensions specific to this hart, which may show
+	 * additional extensions not present across all harts.
+	 */
+	seq_puts(m, "hart isa\t: ");
+	print_isa(m, hart_isa[cpu_id].isa);
 	seq_puts(m, "\n");
 
 	return 0;
-- 
2.34.1

Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Andrew Jones]

On Wed, Jul 05, 2023 at 10:29:31AM -0700, Evan Green wrote:
> In /proc/cpuinfo, most of the information we show for each processor is
> specific to that hart: marchid, mvendorid, mimpid, processor, hart,
> compatible, and the mmu size. But the ISA string gets filtered through a
> lowest common denominator mask, so that if one CPU is missing an ISA
> extension, no CPUs will show it.
> 
> Now that we track the ISA extensions for each hart, let's report ISA
> extension info accurately per-hart in /proc/cpuinfo. We cannot change
> the "isa:" line, as usermode may be relying on that line to show only
> the common set of extensions supported across all harts. Add a new "hart
> isa" line instead, which reports the true set of extensions for that
> hart. This matches what is returned in riscv_hwprobe() when querying a
> given hart.
> 
> Signed-off-by: Evan Green <evan@rivosinc.com>
> 
> ---
> 
> Changes in v2:
>  - Added new "hart isa" line rather than altering behavior of existing
>    "isa" line (Conor, Palmer)
> 
> 
> I based this series on top of Conor's riscv-extensions-strings branch
> from July 3rd, since otherwise this change gets hopelessly entangled
> with that series.
> 
> I was unsure if I could snuggle the new "hart isa" line in just below
> "isa". Aesthetically it would be quite pleasing, but it runs the risk of
> breaking brittle usermode parsers that are assuming a specific line
> order. So I put it at the end.

Actually, they're probably only aesthetically pleasing when they match. If
there are differences, then I'd guess having them side by side, almost the
same, but different, would make them even harder to look at then they
already are. So I think I'll be happier with them separated by a few lines
anyway.

> 
> ---
>  arch/riscv/kernel/cpu.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 1acf3679600d..6264b7b94945 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -197,9 +197,8 @@ arch_initcall(riscv_cpuinfo_init);
>  
>  #ifdef CONFIG_PROC_FS
>  
> -static void print_isa(struct seq_file *f)
> +static void print_isa(struct seq_file *f, const unsigned long *isa_bitmap)
>  {
> -	seq_puts(f, "isa\t\t: ");
>  
>  	if (IS_ENABLED(CONFIG_32BIT))
>  		seq_write(f, "rv32", 4);
> @@ -207,7 +206,7 @@ static void print_isa(struct seq_file *f)
>  		seq_write(f, "rv64", 4);
>  
>  	for (int i = 0; i < riscv_isa_ext_count; i++) {
> -		if (!__riscv_isa_extension_available(NULL, riscv_isa_ext[i].id))
> +		if (!__riscv_isa_extension_available(isa_bitmap, riscv_isa_ext[i].id))
>  			continue;
>  
>  		/* Only multi-letter extensions are split by underscores */
> @@ -271,7 +270,15 @@ static int c_show(struct seq_file *m, void *v)
>  
>  	seq_printf(m, "processor\t: %lu\n", cpu_id);
>  	seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id));
> -	print_isa(m);
> +
> +	/*
> +	 * For historical raisins, the isa: line is limited to the lowest common
> +	 * denominator of extensions supported across all harts. A true list of
> +	 * extensions supported on this hart is printed later in the hart_isa:
> +	 * line.
> +	 */
> +	seq_puts(m, "isa\t\t: ");
> +	print_isa(m, NULL);
>  	print_mmu(m);
>  
>  	if (acpi_disabled) {
> @@ -287,6 +294,13 @@ static int c_show(struct seq_file *m, void *v)
>  	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);
> +
> +	/*
> +	 * Print the ISA extensions specific to this hart, which may show
> +	 * additional extensions not present across all harts.
> +	 */
> +	seq_puts(m, "hart isa\t: ");
> +	print_isa(m, hart_isa[cpu_id].isa);
>  	seq_puts(m, "\n");
>  
>  	return 0;
> -- 
> 2.34.1
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Andrew Jones]

On Wed, Jul 05, 2023 at 10:29:31AM -0700, Evan Green wrote:
> In /proc/cpuinfo, most of the information we show for each processor is
> specific to that hart: marchid, mvendorid, mimpid, processor, hart,
> compatible, and the mmu size. But the ISA string gets filtered through a
> lowest common denominator mask, so that if one CPU is missing an ISA
> extension, no CPUs will show it.
> 
> Now that we track the ISA extensions for each hart, let's report ISA
> extension info accurately per-hart in /proc/cpuinfo. We cannot change
> the "isa:" line, as usermode may be relying on that line to show only
> the common set of extensions supported across all harts. Add a new "hart
> isa" line instead, which reports the true set of extensions for that
> hart. This matches what is returned in riscv_hwprobe() when querying a
> given hart.
> 
> Signed-off-by: Evan Green <evan@rivosinc.com>
> 
> ---
> 
> Changes in v2:
>  - Added new "hart isa" line rather than altering behavior of existing
>    "isa" line (Conor, Palmer)
> 
> 
> I based this series on top of Conor's riscv-extensions-strings branch
> from July 3rd, since otherwise this change gets hopelessly entangled
> with that series.
> 
> I was unsure if I could snuggle the new "hart isa" line in just below
> "isa". Aesthetically it would be quite pleasing, but it runs the risk of
> breaking brittle usermode parsers that are assuming a specific line
> order. So I put it at the end.

Actually, they're probably only aesthetically pleasing when they match. If
there are differences, then I'd guess having them side by side, almost the
same, but different, would make them even harder to look at then they
already are. So I think I'll be happier with them separated by a few lines
anyway.

> 
> ---
>  arch/riscv/kernel/cpu.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 1acf3679600d..6264b7b94945 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -197,9 +197,8 @@ arch_initcall(riscv_cpuinfo_init);
>  
>  #ifdef CONFIG_PROC_FS
>  
> -static void print_isa(struct seq_file *f)
> +static void print_isa(struct seq_file *f, const unsigned long *isa_bitmap)
>  {
> -	seq_puts(f, "isa\t\t: ");
>  
>  	if (IS_ENABLED(CONFIG_32BIT))
>  		seq_write(f, "rv32", 4);
> @@ -207,7 +206,7 @@ static void print_isa(struct seq_file *f)
>  		seq_write(f, "rv64", 4);
>  
>  	for (int i = 0; i < riscv_isa_ext_count; i++) {
> -		if (!__riscv_isa_extension_available(NULL, riscv_isa_ext[i].id))
> +		if (!__riscv_isa_extension_available(isa_bitmap, riscv_isa_ext[i].id))
>  			continue;
>  
>  		/* Only multi-letter extensions are split by underscores */
> @@ -271,7 +270,15 @@ static int c_show(struct seq_file *m, void *v)
>  
>  	seq_printf(m, "processor\t: %lu\n", cpu_id);
>  	seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id));
> -	print_isa(m);
> +
> +	/*
> +	 * For historical raisins, the isa: line is limited to the lowest common
> +	 * denominator of extensions supported across all harts. A true list of
> +	 * extensions supported on this hart is printed later in the hart_isa:
> +	 * line.
> +	 */
> +	seq_puts(m, "isa\t\t: ");
> +	print_isa(m, NULL);
>  	print_mmu(m);
>  
>  	if (acpi_disabled) {
> @@ -287,6 +294,13 @@ static int c_show(struct seq_file *m, void *v)
>  	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);
> +
> +	/*
> +	 * Print the ISA extensions specific to this hart, which may show
> +	 * additional extensions not present across all harts.
> +	 */
> +	seq_puts(m, "hart isa\t: ");
> +	print_isa(m, hart_isa[cpu_id].isa);
>  	seq_puts(m, "\n");
>  
>  	return 0;
> -- 
> 2.34.1
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 4659 bytes --]

On Thu, Jul 06, 2023 at 10:01:31AM +0200, Andrew Jones wrote:
> On Wed, Jul 05, 2023 at 10:29:31AM -0700, Evan Green wrote:
> > In /proc/cpuinfo, most of the information we show for each processor is
> > specific to that hart: marchid, mvendorid, mimpid, processor, hart,
> > compatible, and the mmu size. But the ISA string gets filtered through a
> > lowest common denominator mask, so that if one CPU is missing an ISA
> > extension, no CPUs will show it.
> > 
> > Now that we track the ISA extensions for each hart, let's report ISA
> > extension info accurately per-hart in /proc/cpuinfo. We cannot change
> > the "isa:" line, as usermode may be relying on that line to show only
> > the common set of extensions supported across all harts. Add a new "hart
> > isa" line instead, which reports the true set of extensions for that
> > hart. This matches what is returned in riscv_hwprobe() when querying a
> > given hart.
> > 
> > Signed-off-by: Evan Green <evan@rivosinc.com>
> > 
> > ---
> > 
> > Changes in v2:
> >  - Added new "hart isa" line rather than altering behavior of existing
> >    "isa" line (Conor, Palmer)
> > 
> > 
> > I based this series on top of Conor's riscv-extensions-strings branch
> > from July 3rd, since otherwise this change gets hopelessly entangled
> > with that series.
> > 
> > I was unsure if I could snuggle the new "hart isa" line in just below
> > "isa". Aesthetically it would be quite pleasing, but it runs the risk of
> > breaking brittle usermode parsers that are assuming a specific line
> > order. So I put it at the end.
> 
> Actually, they're probably only aesthetically pleasing when they match. If
> there are differences, then I'd guess having them side by side, almost the
> same, but different, would make them even harder to look at then they
> already are. So I think I'll be happier with them separated by a few lines
> anyway.

This list is eventually going to be so big that I don't think doing
by-eye anything is going to be useful, so aesthetics be damned.
That said, a parser that relies on the order of individual lines like
that might deserve to be broken ;)

Anyway, change looks good to me:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

I was thinking the uabi doc might need an update - should we add to it
that "isa" means the common set & "hart isa"?

Cheers,
Conor.

> > ---
> >  arch/riscv/kernel/cpu.c | 22 ++++++++++++++++++----
> >  1 file changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index 1acf3679600d..6264b7b94945 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -197,9 +197,8 @@ arch_initcall(riscv_cpuinfo_init);
> >  
> >  #ifdef CONFIG_PROC_FS
> >  
> > -static void print_isa(struct seq_file *f)
> > +static void print_isa(struct seq_file *f, const unsigned long *isa_bitmap)
> >  {
> > -	seq_puts(f, "isa\t\t: ");
> >  
> >  	if (IS_ENABLED(CONFIG_32BIT))
> >  		seq_write(f, "rv32", 4);
> > @@ -207,7 +206,7 @@ static void print_isa(struct seq_file *f)
> >  		seq_write(f, "rv64", 4);
> >  
> >  	for (int i = 0; i < riscv_isa_ext_count; i++) {
> > -		if (!__riscv_isa_extension_available(NULL, riscv_isa_ext[i].id))
> > +		if (!__riscv_isa_extension_available(isa_bitmap, riscv_isa_ext[i].id))
> >  			continue;
> >  
> >  		/* Only multi-letter extensions are split by underscores */
> > @@ -271,7 +270,15 @@ static int c_show(struct seq_file *m, void *v)
> >  
> >  	seq_printf(m, "processor\t: %lu\n", cpu_id);
> >  	seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id));
> > -	print_isa(m);
> > +
> > +	/*
> > +	 * For historical raisins, the isa: line is limited to the lowest common
> > +	 * denominator of extensions supported across all harts. A true list of
> > +	 * extensions supported on this hart is printed later in the hart_isa:
> > +	 * line.
> > +	 */
> > +	seq_puts(m, "isa\t\t: ");
> > +	print_isa(m, NULL);
> >  	print_mmu(m);
> >  
> >  	if (acpi_disabled) {
> > @@ -287,6 +294,13 @@ static int c_show(struct seq_file *m, void *v)
> >  	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);
> > +
> > +	/*
> > +	 * Print the ISA extensions specific to this hart, which may show
> > +	 * additional extensions not present across all harts.
> > +	 */
> > +	seq_puts(m, "hart isa\t: ");
> > +	print_isa(m, hart_isa[cpu_id].isa);
> >  	seq_puts(m, "\n");
> >  
> >  	return 0;
> > -- 
> > 2.34.1
> >


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4659 bytes --]

On Thu, Jul 06, 2023 at 10:01:31AM +0200, Andrew Jones wrote:
> On Wed, Jul 05, 2023 at 10:29:31AM -0700, Evan Green wrote:
> > In /proc/cpuinfo, most of the information we show for each processor is
> > specific to that hart: marchid, mvendorid, mimpid, processor, hart,
> > compatible, and the mmu size. But the ISA string gets filtered through a
> > lowest common denominator mask, so that if one CPU is missing an ISA
> > extension, no CPUs will show it.
> > 
> > Now that we track the ISA extensions for each hart, let's report ISA
> > extension info accurately per-hart in /proc/cpuinfo. We cannot change
> > the "isa:" line, as usermode may be relying on that line to show only
> > the common set of extensions supported across all harts. Add a new "hart
> > isa" line instead, which reports the true set of extensions for that
> > hart. This matches what is returned in riscv_hwprobe() when querying a
> > given hart.
> > 
> > Signed-off-by: Evan Green <evan@rivosinc.com>
> > 
> > ---
> > 
> > Changes in v2:
> >  - Added new "hart isa" line rather than altering behavior of existing
> >    "isa" line (Conor, Palmer)
> > 
> > 
> > I based this series on top of Conor's riscv-extensions-strings branch
> > from July 3rd, since otherwise this change gets hopelessly entangled
> > with that series.
> > 
> > I was unsure if I could snuggle the new "hart isa" line in just below
> > "isa". Aesthetically it would be quite pleasing, but it runs the risk of
> > breaking brittle usermode parsers that are assuming a specific line
> > order. So I put it at the end.
> 
> Actually, they're probably only aesthetically pleasing when they match. If
> there are differences, then I'd guess having them side by side, almost the
> same, but different, would make them even harder to look at then they
> already are. So I think I'll be happier with them separated by a few lines
> anyway.

This list is eventually going to be so big that I don't think doing
by-eye anything is going to be useful, so aesthetics be damned.
That said, a parser that relies on the order of individual lines like
that might deserve to be broken ;)

Anyway, change looks good to me:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

I was thinking the uabi doc might need an update - should we add to it
that "isa" means the common set & "hart isa"?

Cheers,
Conor.

> > ---
> >  arch/riscv/kernel/cpu.c | 22 ++++++++++++++++++----
> >  1 file changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index 1acf3679600d..6264b7b94945 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -197,9 +197,8 @@ arch_initcall(riscv_cpuinfo_init);
> >  
> >  #ifdef CONFIG_PROC_FS
> >  
> > -static void print_isa(struct seq_file *f)
> > +static void print_isa(struct seq_file *f, const unsigned long *isa_bitmap)
> >  {
> > -	seq_puts(f, "isa\t\t: ");
> >  
> >  	if (IS_ENABLED(CONFIG_32BIT))
> >  		seq_write(f, "rv32", 4);
> > @@ -207,7 +206,7 @@ static void print_isa(struct seq_file *f)
> >  		seq_write(f, "rv64", 4);
> >  
> >  	for (int i = 0; i < riscv_isa_ext_count; i++) {
> > -		if (!__riscv_isa_extension_available(NULL, riscv_isa_ext[i].id))
> > +		if (!__riscv_isa_extension_available(isa_bitmap, riscv_isa_ext[i].id))
> >  			continue;
> >  
> >  		/* Only multi-letter extensions are split by underscores */
> > @@ -271,7 +270,15 @@ static int c_show(struct seq_file *m, void *v)
> >  
> >  	seq_printf(m, "processor\t: %lu\n", cpu_id);
> >  	seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id));
> > -	print_isa(m);
> > +
> > +	/*
> > +	 * For historical raisins, the isa: line is limited to the lowest common
> > +	 * denominator of extensions supported across all harts. A true list of
> > +	 * extensions supported on this hart is printed later in the hart_isa:
> > +	 * line.
> > +	 */
> > +	seq_puts(m, "isa\t\t: ");
> > +	print_isa(m, NULL);
> >  	print_mmu(m);
> >  
> >  	if (acpi_disabled) {
> > @@ -287,6 +294,13 @@ static int c_show(struct seq_file *m, void *v)
> >  	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);
> > +
> > +	/*
> > +	 * Print the ISA extensions specific to this hart, which may show
> > +	 * additional extensions not present across all harts.
> > +	 */
> > +	seq_puts(m, "hart isa\t: ");
> > +	print_isa(m, hart_isa[cpu_id].isa);
> >  	seq_puts(m, "\n");
> >  
> >  	return 0;
> > -- 
> > 2.34.1
> >


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Evan Green]

On Thu, Jul 6, 2023 at 3:53 AM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Thu, Jul 06, 2023 at 10:01:31AM +0200, Andrew Jones wrote:
> > On Wed, Jul 05, 2023 at 10:29:31AM -0700, Evan Green wrote:
> > > In /proc/cpuinfo, most of the information we show for each processor is
> > > specific to that hart: marchid, mvendorid, mimpid, processor, hart,
> > > compatible, and the mmu size. But the ISA string gets filtered through a
> > > lowest common denominator mask, so that if one CPU is missing an ISA
> > > extension, no CPUs will show it.
> > >
> > > Now that we track the ISA extensions for each hart, let's report ISA
> > > extension info accurately per-hart in /proc/cpuinfo. We cannot change
> > > the "isa:" line, as usermode may be relying on that line to show only
> > > the common set of extensions supported across all harts. Add a new "hart
> > > isa" line instead, which reports the true set of extensions for that
> > > hart. This matches what is returned in riscv_hwprobe() when querying a
> > > given hart.
> > >
> > > Signed-off-by: Evan Green <evan@rivosinc.com>
> > >
> > > ---
> > >
> > > Changes in v2:
> > >  - Added new "hart isa" line rather than altering behavior of existing
> > >    "isa" line (Conor, Palmer)
> > >
> > >
> > > I based this series on top of Conor's riscv-extensions-strings branch
> > > from July 3rd, since otherwise this change gets hopelessly entangled
> > > with that series.
> > >
> > > I was unsure if I could snuggle the new "hart isa" line in just below
> > > "isa". Aesthetically it would be quite pleasing, but it runs the risk of
> > > breaking brittle usermode parsers that are assuming a specific line
> > > order. So I put it at the end.
> >
> > Actually, they're probably only aesthetically pleasing when they match. If
> > there are differences, then I'd guess having them side by side, almost the
> > same, but different, would make them even harder to look at then they
> > already are. So I think I'll be happier with them separated by a few lines
> > anyway.
>
> This list is eventually going to be so big that I don't think doing
> by-eye anything is going to be useful, so aesthetics be damned.
> That said, a parser that relies on the order of individual lines like
> that might deserve to be broken ;)
>
> Anyway, change looks good to me:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>
> I was thinking the uabi doc might need an update - should we add to it
> that "isa" means the common set & "hart isa"?

I did do a little search for documentation to update, but failed to
come up with any. Which doc are you thinking of?
-Evan

Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Evan Green]

On Thu, Jul 6, 2023 at 3:53 AM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Thu, Jul 06, 2023 at 10:01:31AM +0200, Andrew Jones wrote:
> > On Wed, Jul 05, 2023 at 10:29:31AM -0700, Evan Green wrote:
> > > In /proc/cpuinfo, most of the information we show for each processor is
> > > specific to that hart: marchid, mvendorid, mimpid, processor, hart,
> > > compatible, and the mmu size. But the ISA string gets filtered through a
> > > lowest common denominator mask, so that if one CPU is missing an ISA
> > > extension, no CPUs will show it.
> > >
> > > Now that we track the ISA extensions for each hart, let's report ISA
> > > extension info accurately per-hart in /proc/cpuinfo. We cannot change
> > > the "isa:" line, as usermode may be relying on that line to show only
> > > the common set of extensions supported across all harts. Add a new "hart
> > > isa" line instead, which reports the true set of extensions for that
> > > hart. This matches what is returned in riscv_hwprobe() when querying a
> > > given hart.
> > >
> > > Signed-off-by: Evan Green <evan@rivosinc.com>
> > >
> > > ---
> > >
> > > Changes in v2:
> > >  - Added new "hart isa" line rather than altering behavior of existing
> > >    "isa" line (Conor, Palmer)
> > >
> > >
> > > I based this series on top of Conor's riscv-extensions-strings branch
> > > from July 3rd, since otherwise this change gets hopelessly entangled
> > > with that series.
> > >
> > > I was unsure if I could snuggle the new "hart isa" line in just below
> > > "isa". Aesthetically it would be quite pleasing, but it runs the risk of
> > > breaking brittle usermode parsers that are assuming a specific line
> > > order. So I put it at the end.
> >
> > Actually, they're probably only aesthetically pleasing when they match. If
> > there are differences, then I'd guess having them side by side, almost the
> > same, but different, would make them even harder to look at then they
> > already are. So I think I'll be happier with them separated by a few lines
> > anyway.
>
> This list is eventually going to be so big that I don't think doing
> by-eye anything is going to be useful, so aesthetics be damned.
> That said, a parser that relies on the order of individual lines like
> that might deserve to be broken ;)
>
> Anyway, change looks good to me:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>
> I was thinking the uabi doc might need an update - should we add to it
> that "isa" means the common set & "hart isa"?

I did do a little search for documentation to update, but failed to
come up with any. Which doc are you thinking of?
-Evan

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Palmer Dabbelt]

On Thu, 06 Jul 2023 12:34:30 PDT (-0700), Evan Green wrote:
> On Thu, Jul 6, 2023 at 3:53 AM Conor Dooley <conor.dooley@microchip.com> wrote:
>>
>> On Thu, Jul 06, 2023 at 10:01:31AM +0200, Andrew Jones wrote:
>> > On Wed, Jul 05, 2023 at 10:29:31AM -0700, Evan Green wrote:
>> > > In /proc/cpuinfo, most of the information we show for each processor is
>> > > specific to that hart: marchid, mvendorid, mimpid, processor, hart,
>> > > compatible, and the mmu size. But the ISA string gets filtered through a
>> > > lowest common denominator mask, so that if one CPU is missing an ISA
>> > > extension, no CPUs will show it.
>> > >
>> > > Now that we track the ISA extensions for each hart, let's report ISA
>> > > extension info accurately per-hart in /proc/cpuinfo. We cannot change
>> > > the "isa:" line, as usermode may be relying on that line to show only
>> > > the common set of extensions supported across all harts. Add a new "hart
>> > > isa" line instead, which reports the true set of extensions for that
>> > > hart. This matches what is returned in riscv_hwprobe() when querying a
>> > > given hart.
>> > >
>> > > Signed-off-by: Evan Green <evan@rivosinc.com>
>> > >
>> > > ---
>> > >
>> > > Changes in v2:
>> > >  - Added new "hart isa" line rather than altering behavior of existing
>> > >    "isa" line (Conor, Palmer)
>> > >
>> > >
>> > > I based this series on top of Conor's riscv-extensions-strings branch
>> > > from July 3rd, since otherwise this change gets hopelessly entangled
>> > > with that series.
>> > >
>> > > I was unsure if I could snuggle the new "hart isa" line in just below
>> > > "isa". Aesthetically it would be quite pleasing, but it runs the risk of
>> > > breaking brittle usermode parsers that are assuming a specific line
>> > > order. So I put it at the end.
>> >
>> > Actually, they're probably only aesthetically pleasing when they match. If
>> > there are differences, then I'd guess having them side by side, almost the
>> > same, but different, would make them even harder to look at then they
>> > already are. So I think I'll be happier with them separated by a few lines
>> > anyway.
>>
>> This list is eventually going to be so big that I don't think doing
>> by-eye anything is going to be useful, so aesthetics be damned.
>> That said, a parser that relies on the order of individual lines like
>> that might deserve to be broken ;)
>>
>> Anyway, change looks good to me:
>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>>
>> I was thinking the uabi doc might need an update - should we add to it
>> that "isa" means the common set & "hart isa"?
>
> I did do a little search for documentation to update, but failed to
> come up with any. Which doc are you thinking of?

Should be somewhere like Documentation/arch/riscv/uabi.rst

Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Palmer Dabbelt]

On Thu, 06 Jul 2023 12:34:30 PDT (-0700), Evan Green wrote:
> On Thu, Jul 6, 2023 at 3:53 AM Conor Dooley <conor.dooley@microchip.com> wrote:
>>
>> On Thu, Jul 06, 2023 at 10:01:31AM +0200, Andrew Jones wrote:
>> > On Wed, Jul 05, 2023 at 10:29:31AM -0700, Evan Green wrote:
>> > > In /proc/cpuinfo, most of the information we show for each processor is
>> > > specific to that hart: marchid, mvendorid, mimpid, processor, hart,
>> > > compatible, and the mmu size. But the ISA string gets filtered through a
>> > > lowest common denominator mask, so that if one CPU is missing an ISA
>> > > extension, no CPUs will show it.
>> > >
>> > > Now that we track the ISA extensions for each hart, let's report ISA
>> > > extension info accurately per-hart in /proc/cpuinfo. We cannot change
>> > > the "isa:" line, as usermode may be relying on that line to show only
>> > > the common set of extensions supported across all harts. Add a new "hart
>> > > isa" line instead, which reports the true set of extensions for that
>> > > hart. This matches what is returned in riscv_hwprobe() when querying a
>> > > given hart.
>> > >
>> > > Signed-off-by: Evan Green <evan@rivosinc.com>
>> > >
>> > > ---
>> > >
>> > > Changes in v2:
>> > >  - Added new "hart isa" line rather than altering behavior of existing
>> > >    "isa" line (Conor, Palmer)
>> > >
>> > >
>> > > I based this series on top of Conor's riscv-extensions-strings branch
>> > > from July 3rd, since otherwise this change gets hopelessly entangled
>> > > with that series.
>> > >
>> > > I was unsure if I could snuggle the new "hart isa" line in just below
>> > > "isa". Aesthetically it would be quite pleasing, but it runs the risk of
>> > > breaking brittle usermode parsers that are assuming a specific line
>> > > order. So I put it at the end.
>> >
>> > Actually, they're probably only aesthetically pleasing when they match. If
>> > there are differences, then I'd guess having them side by side, almost the
>> > same, but different, would make them even harder to look at then they
>> > already are. So I think I'll be happier with them separated by a few lines
>> > anyway.
>>
>> This list is eventually going to be so big that I don't think doing
>> by-eye anything is going to be useful, so aesthetics be damned.
>> That said, a parser that relies on the order of individual lines like
>> that might deserve to be broken ;)
>>
>> Anyway, change looks good to me:
>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>>
>> I was thinking the uabi doc might need an update - should we add to it
>> that "isa" means the common set & "hart isa"?
>
> I did do a little search for documentation to update, but failed to
> come up with any. Which doc are you thinking of?

Should be somewhere like Documentation/arch/riscv/uabi.rst

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Evan Green]

On Thu, Jul 6, 2023 at 12:45 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 06 Jul 2023 12:34:30 PDT (-0700), Evan Green wrote:
> > On Thu, Jul 6, 2023 at 3:53 AM Conor Dooley <conor.dooley@microchip.com> wrote:
> >>
> >> On Thu, Jul 06, 2023 at 10:01:31AM +0200, Andrew Jones wrote:
> >> > On Wed, Jul 05, 2023 at 10:29:31AM -0700, Evan Green wrote:
> >> > > In /proc/cpuinfo, most of the information we show for each processor is
> >> > > specific to that hart: marchid, mvendorid, mimpid, processor, hart,
> >> > > compatible, and the mmu size. But the ISA string gets filtered through a
> >> > > lowest common denominator mask, so that if one CPU is missing an ISA
> >> > > extension, no CPUs will show it.
> >> > >
> >> > > Now that we track the ISA extensions for each hart, let's report ISA
> >> > > extension info accurately per-hart in /proc/cpuinfo. We cannot change
> >> > > the "isa:" line, as usermode may be relying on that line to show only
> >> > > the common set of extensions supported across all harts. Add a new "hart
> >> > > isa" line instead, which reports the true set of extensions for that
> >> > > hart. This matches what is returned in riscv_hwprobe() when querying a
> >> > > given hart.
> >> > >
> >> > > Signed-off-by: Evan Green <evan@rivosinc.com>
> >> > >
> >> > > ---
> >> > >
> >> > > Changes in v2:
> >> > >  - Added new "hart isa" line rather than altering behavior of existing
> >> > >    "isa" line (Conor, Palmer)
> >> > >
> >> > >
> >> > > I based this series on top of Conor's riscv-extensions-strings branch
> >> > > from July 3rd, since otherwise this change gets hopelessly entangled
> >> > > with that series.
> >> > >
> >> > > I was unsure if I could snuggle the new "hart isa" line in just below
> >> > > "isa". Aesthetically it would be quite pleasing, but it runs the risk of
> >> > > breaking brittle usermode parsers that are assuming a specific line
> >> > > order. So I put it at the end.
> >> >
> >> > Actually, they're probably only aesthetically pleasing when they match. If
> >> > there are differences, then I'd guess having them side by side, almost the
> >> > same, but different, would make them even harder to look at then they
> >> > already are. So I think I'll be happier with them separated by a few lines
> >> > anyway.
> >>
> >> This list is eventually going to be so big that I don't think doing
> >> by-eye anything is going to be useful, so aesthetics be damned.
> >> That said, a parser that relies on the order of individual lines like
> >> that might deserve to be broken ;)
> >>
> >> Anyway, change looks good to me:
> >> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> >>
> >> I was thinking the uabi doc might need an update - should we add to it
> >> that "isa" means the common set & "hart isa"?
> >
> > I did do a little search for documentation to update, but failed to
> > come up with any. Which doc are you thinking of?
>
> Should be somewhere like Documentation/arch/riscv/uabi.rst

Thanks, I'll spin this patch and add something here.
-Evan

Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Evan Green]

On Thu, Jul 6, 2023 at 12:45 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 06 Jul 2023 12:34:30 PDT (-0700), Evan Green wrote:
> > On Thu, Jul 6, 2023 at 3:53 AM Conor Dooley <conor.dooley@microchip.com> wrote:
> >>
> >> On Thu, Jul 06, 2023 at 10:01:31AM +0200, Andrew Jones wrote:
> >> > On Wed, Jul 05, 2023 at 10:29:31AM -0700, Evan Green wrote:
> >> > > In /proc/cpuinfo, most of the information we show for each processor is
> >> > > specific to that hart: marchid, mvendorid, mimpid, processor, hart,
> >> > > compatible, and the mmu size. But the ISA string gets filtered through a
> >> > > lowest common denominator mask, so that if one CPU is missing an ISA
> >> > > extension, no CPUs will show it.
> >> > >
> >> > > Now that we track the ISA extensions for each hart, let's report ISA
> >> > > extension info accurately per-hart in /proc/cpuinfo. We cannot change
> >> > > the "isa:" line, as usermode may be relying on that line to show only
> >> > > the common set of extensions supported across all harts. Add a new "hart
> >> > > isa" line instead, which reports the true set of extensions for that
> >> > > hart. This matches what is returned in riscv_hwprobe() when querying a
> >> > > given hart.
> >> > >
> >> > > Signed-off-by: Evan Green <evan@rivosinc.com>
> >> > >
> >> > > ---
> >> > >
> >> > > Changes in v2:
> >> > >  - Added new "hart isa" line rather than altering behavior of existing
> >> > >    "isa" line (Conor, Palmer)
> >> > >
> >> > >
> >> > > I based this series on top of Conor's riscv-extensions-strings branch
> >> > > from July 3rd, since otherwise this change gets hopelessly entangled
> >> > > with that series.
> >> > >
> >> > > I was unsure if I could snuggle the new "hart isa" line in just below
> >> > > "isa". Aesthetically it would be quite pleasing, but it runs the risk of
> >> > > breaking brittle usermode parsers that are assuming a specific line
> >> > > order. So I put it at the end.
> >> >
> >> > Actually, they're probably only aesthetically pleasing when they match. If
> >> > there are differences, then I'd guess having them side by side, almost the
> >> > same, but different, would make them even harder to look at then they
> >> > already are. So I think I'll be happier with them separated by a few lines
> >> > anyway.
> >>
> >> This list is eventually going to be so big that I don't think doing
> >> by-eye anything is going to be useful, so aesthetics be damned.
> >> That said, a parser that relies on the order of individual lines like
> >> that might deserve to be broken ;)
> >>
> >> Anyway, change looks good to me:
> >> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> >>
> >> I was thinking the uabi doc might need an update - should we add to it
> >> that "isa" means the common set & "hart isa"?
> >
> > I did do a little search for documentation to update, but failed to
> > come up with any. Which doc are you thinking of?
>
> Should be somewhere like Documentation/arch/riscv/uabi.rst

Thanks, I'll spin this patch and add something here.
-Evan

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv