Messages in this thread | | | Date | Thu, 6 Jul 2023 11:53:12 +0100 | From | Conor Dooley <> | Subject | Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo |
| |
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 > >
[unhandled content-type:application/pgp-signature] | |