Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 6 Jul 2023 11:53:12 +0100
From:   Conor Dooley <conor.dooley@microchip.com>
To:     Andrew Jones <ajones@ventanamicro.com>
CC:     Evan Green <evan@rivosinc.com>,
        Palmer Dabbelt <palmer@rivosinc.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Anup Patel <apatel@ventanamicro.com>,
        Heiko Stuebner <heiko.stuebner@vrull.eu>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Sunil V L <sunilvl@ventanamicro.com>,
        <linux-kernel@vger.kernel.org>, <linux-riscv@lists.infradead.org>
Subject: Re: [PATCH v2] RISC-V: Show accurate per-hart isa in /proc/cpuinfo
Message-ID: <20230706-murkiness-oven-444f25924546@wendy>
References: <20230705172931.1099183-1-evan@rivosinc.com>
 <20230706-dbdee6eba0c4a16084587b10@orel>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="0qpcjaRKI05o/cBU"
Content-Disposition: inline
In-Reply-To: <20230706-dbdee6eba0c4a16084587b10@orel>
Precedence: bulk

--0qpcjaRKI05o/cBU
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
> >=20
> > 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.
> >=20
> > Signed-off-by: Evan Green <evan@rivosinc.com>
> >=20
> > ---
> >=20
> > Changes in v2:
> >  - Added new "hart isa" line rather than altering behavior of existing
> >    "isa" line (Conor, Palmer)
> >=20
> >=20
> > 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.
> >=20
> > 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.
>=20
> 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(-)
> >=20
> > 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);
> > =20
> >  #ifdef CONFIG_PROC_FS
> > =20
> > -static void print_isa(struct seq_file *f)
> > +static void print_isa(struct seq_file *f, const unsigned long *isa_bit=
map)
> >  {
> > -	seq_puts(f, "isa\t\t: ");
> > =20
> >  	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);
> > =20
> >  	for (int i =3D 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;
> > =20
> >  		/* Only multi-letter extensions are split by underscores */
> > @@ -271,7 +270,15 @@ static int c_show(struct seq_file *m, void *v)
> > =20
> >  	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 com=
mon
> > +	 * 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);
> > =20
> >  	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");
> > =20
> >  	return 0;
> > --=20
> > 2.34.1
> >


--0qpcjaRKI05o/cBU
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZKadGAAKCRB4tDGHoIJi
0pK8AQCYZpuP+ABbmojkwmSlqKPYl+thQSqwUqrJsDOgD51+aAEAvyq/zo3OaIWj
saOw+IqxHnyBVjVvUCz1KNx9ABaCBQ4=
=UJ59
-----END PGP SIGNATURE-----

--0qpcjaRKI05o/cBU--