Received-SPF: pass (google.com: domain of linux-kernel+bounces-70445-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Sun, 18 Feb 2024 17:02:46 +0000
From: Conor Dooley <conor@kernel.org>
To: Samuel Holland <samuel.holland@sifive.com>
Cc: Stefan O'Rear <sorear@fastmail.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Andrew Jones <ajones@ventanamicro.com>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH -fixes v2 3/4] riscv: Add ISA extension parsing for Sm
 and Ss
Message-ID: <20240218-rotunda-discover-d6d84709807e@spud>
References: <20240213033744.4069020-1-samuel.holland@sifive.com>
 <20240213033744.4069020-4-samuel.holland@sifive.com>
 <20240213-dangle-taco-2742f6087a3e@spud>
 <ff3bd436-12f8-4cde-881d-89a005ad85c0@sifive.com>
 <b7ad2bd4-a19e-486e-8be2-3b56f288d5d0@app.fastmail.com>
 <20240213-earflap-easing-370492840507@spud>
 <90a7443c-feca-4fbb-8c2e-fa050c0e6141@sifive.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="r0a/WySfeyXRLwE+"
Content-Disposition: inline
In-Reply-To: <90a7443c-feca-4fbb-8c2e-fa050c0e6141@sifive.com>


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

On Sun, Feb 18, 2024 at 09:00:14AM -0600, Samuel Holland wrote:
> >>>> Or maybe we can still with the properties you have, but instead of
> >>>> treating them like any other extension, handle these separately,
> >>>> focusing on the numbering, so that only having the exact version
> >>>> supported by a cpu is possible.
> >>>
> >>> Maybe I'm misunderstanding what you're saying here, but it is already=
 the case
> >>> that the DT for a CPU would only contain the exact version of the pri=
vileged ISA
> >>> supported by that CPU.
> >>
> >> If privileged spec versions are boolean extensions, then you would say=
 "ss1p11",
> >> "ss1p12", "ss1p13" as separate/simultaneous extensions.
> >=20
> >> This is needed in order
> >> to allow simple support checks as described in the riscv,isa-extension=
s cover
> >> letter.
> >=20
> > Yes, it is explicitly a goal of riscv,isa-extensions that you can look
> > for a specific extension in the list if that is all you care about. If
> > you go and drop ss1p11 because you support ss1p12 it defeats that.
>=20
> Okay, that makes sense, but that is not how the parsing code works right =
now,
> which is probably what led me down the wrong path. :)
>=20
> To have the intended semantics, we cannot parse _anything_ in
> riscv,isa-extensions as a "bundled" or "superset" extension.

That's not true I don't think. You can parse as a "bundle" but...

> Because to
> accomplish your goal, each extension in the bundle must be listed explici=
tly, in
> case the consumer only cares about that one extension.

=2E..as you note here, the extensions also have to be listed explicitly so
that they can be detected in isolation if that is all that a consumer
does.

> So it sounds like
> riscv_fill_hwcap_from_ext_list() needs to ignore subset_ext_size/subset_e=
xt_ids.

Do you mean this:
if (ext->subset_ext_size) {
	for (int j =3D 0; j < ext->subset_ext_size; j++) {
		if (riscv_isa_extension_check(ext->subset_ext_ids[i]))
			set_bit(ext->subset_ext_ids[j], isainfo->isa);
	}
}

I think that is fine? If you find the "superset" you can enable the
individual elements. The problem would just be if someone put only the
superset in a DT (or ACPI tables) and the software looked for the
individual element only, but IIRC the kernel currently checks both the
superset and individual elements.
It would be possible to check a bundle and then skip looking for the
individual elements if the bundle was already found if the parsing is
wont to be sped up.

I think all we need to do is enforce that all individual elements are
present on a schema validation level (I have no clue what we can do
for ACPI) and no change is required in the kernel.

Am I misunderstanding what you think is the problem here?

> > I don't know off the top of my head how to enforce ss1p12 requiring ss1=
p11
> > in json schema, but I do have an idea of where to start...
>=20
> Yeah, this is different from normal "dependencies:" because it is a strin=
g list.

I think it is actually doable, just will look sorta clunky. I meant to
go and do it this weekend, but I've been rather sick unfortunately.
Something similar is definitely doable for compatibles, so either it'll
"just work" or I may have to extend the validation tooling.

Cheers,
Conor.

--r0a/WySfeyXRLwE+
Content-Type: application/pgp-signature; name="signature.asc"

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

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZdI3+gAKCRB4tDGHoIJi
0op+AQD+yswfcNC39kGvy+5LlZblpT5uVA4VMEPH51eOdf6opgEA64Aq9nCRWaUY
XrJa2JzMMscuYkn0xKBnFl5H5ed3sAs=
=oPKw
-----END PGP SIGNATURE-----

--r0a/WySfeyXRLwE+--