Received-SPF: pass (google.com: domain of linux-kernel+bounces-143387-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 12 Apr 2024 22:34:10 +0100
From: Conor Dooley <conor@kernel.org>
To: Charlie Jenkins <charlie@rivosinc.com>
Cc: Conor Dooley <conor.dooley@microchip.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>, Guo Ren <guoren@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Samuel Holland <samuel@sholland.org>,
	Evan Green <evan@rivosinc.com>,
	=?iso-8859-1?Q?Cl=E9ment_L=E9ger?= <cleger@rivosinc.com>,
	Jonathan Corbet <corbet@lwn.net>, Shuah Khan <shuah@kernel.org>,
	linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Palmer Dabbelt <palmer@rivosinc.com>,
	linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev,
	linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH 08/19] riscv: Introduce vendor variants of extension
 helpers
Message-ID: <20240412-geranium-monotone-b47f414715d1@spud>
References: <20240411-dev-charlie-support_thead_vector_6_9-v1-0-4af9815ec746@rivosinc.com>
 <20240411-dev-charlie-support_thead_vector_6_9-v1-8-4af9815ec746@rivosinc.com>
 <20240412-dwarf-shower-5a7300fcd283@wendy>
 <ZhlypvTdsFPZBr08@ghost>
 <20240412-drab-french-55d8ff2c8cfa@spud>
 <ZhmhtLS5NuRrPOfu@ghost>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="SiZFDomrOPZV4p3t"
Content-Disposition: inline
In-Reply-To: <ZhmhtLS5NuRrPOfu@ghost>


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

On Fri, Apr 12, 2024 at 02:03:48PM -0700, Charlie Jenkins wrote:
> On Fri, Apr 12, 2024 at 09:40:03PM +0100, Conor Dooley wrote:
> > On Fri, Apr 12, 2024 at 10:43:02AM -0700, Charlie Jenkins wrote:
> > > On Fri, Apr 12, 2024 at 12:49:57PM +0100, Conor Dooley wrote:
> > > > On Thu, Apr 11, 2024 at 09:11:14PM -0700, Charlie Jenkins wrote:
> > > > > Create vendor variants of the existing extension helpers. If the
> > > > > existing functions were instead modified to support vendor extens=
ions, a
> > > > > branch based on the ext value being greater than
> > > > > RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This addit=
ional
> > > > > branch would have an unnecessary performance impact.
> > > > >=20
> > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > > >=20
> > > > I've not looked at the "main" patch in the series that adds all of =
the
> > > > probing and structures for representing this info yet beyond a curs=
ory
> > > > glance, but it feels like we're duplicating a bunch of infrastructu=
re
> > > > here before it is necessary. The IDs are all internal to Linux, so =
I'd
> > > > rather we kept everything in the same structure until we have more =
than
> > > > a handful of vendor extensions. With this patch (and the theadpmu s=
tuff)
> > > > we will have three vendor extensions which feels like a drop in the
> > > > bucket compared to the standard ones.
> > >=20
> > > It is not duplicating infrastructure. If we merge this into the exist=
ing
> > > infrastructure, we would be littering if (ext > RISCV_ISA_VENDOR_EXT_=
BASE)
> > > in __riscv_isa_extension_available. This is particularily important
> > > exactly because we have so few vendor extensions currently so this ch=
eck
> > > would be irrelevant in the vast majority of cases.
> >=20
> > That's only because of your implementation. The existing vendor extensi=
on
> > works fine without this littering. That's another thing actually, you
> > forgot to convert over the user we already have :)
>=20
> Oh right, I will convert them over. The fundemental goal of this patch
> is to allow a way for vendors to support their own extensions without
> needing to populate riscv_isa_ext. This is to create separation between
> vendors so they do not impact each other.

The one that needs converting is xandespmu. As I said on the other patch
a minute I don't think isolating vendors for the internal representation
is needed and can be left in hwprobe. I also don't think we can rely on
a behaviour of "SiFive CPUs will always have SiFive's mvendorid" or that
kinda thing, I've heard talk of the SoC vendor getting their mvendorid
for custom CPU cores instead of the CPU vendor and it's possible for
the SBI implementation to "adjust" the values also.

> xlinuxenvcfg does not fit into this scheme however. This scheme assumes
> that a hart cannot have multiple vendors which that extension breaks.
> xlinuxenvcfg is really filling a hole in the standard isa that is
> applicible to all vendors and does not appear in the device tree so it
> is okay for that to live outside this scheme.

Ye, xlinuxenvcfg is an internal psuedo-extension that should be treated
more like a standard one than something vendor.

> > > It is also unecessary to push off the refactoring until we have some
> > > "sufficient" amount of vendor extensions to deem changing the
> > > infrastructure when I already have the patch available here. This does
> > > not introduce any extra overhead to existing functions and will be ab=
le
> > > to support vendors into the future.
> >=20
> > Yeah, maybe that's true but this was my gut reaction before reading the
> > other patch in detail (which I've still yet to do).
>=20
> - Charlie
>=20

--SiZFDomrOPZV4p3t
Content-Type: application/pgp-signature; name="signature.asc"

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

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZhmo0QAKCRB4tDGHoIJi
0sB2AP9FeUoact1liM4pnP8xEk+uEek2FyqTjYcZiaib0RlcGwD/RBzXv10rgGru
eAFY4Rmq36PhVYfHdF4UI+zCC4yTAgw=
=xSsQ
-----END PGP SIGNATURE-----

--SiZFDomrOPZV4p3t--