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, 13 Jul 2023 14:16:43 +0100
From:   Conor Dooley <conor.dooley@microchip.com>
To:     Andrew Jones <ajones@ventanamicro.com>
CC:     Evan Green <evan@rivosinc.com>, Samuel Ortiz <sameo@rivosinc.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        <linux-riscv@lists.infradead.org>,
        "Hongren (Zenithal) Zheng" <i@zenithal.me>, <linux@rivosinc.com>,
        Heiko Stuebner <heiko.stuebner@vrull.eu>,
        Anup Patel <apatel@ventanamicro.com>,
        <linux-kernel@vger.kernel.org>, Guo Ren <guoren@kernel.org>,
        Atish Patra <atishp@rivosinc.com>,
        =?iso-8859-1?Q?Bj=F6rn_T=F6pel?= <bjorn@rivosinc.com>,
        <devicetree@vger.kernel.org>, <sorear@fastmail.com>,
        Jiatai He <jiatai2021@iscas.ac.cn>
Subject: Re: [PATCH v4 1/4] RISC-V: Add Bitmanip/Scalar Crypto parsing from DT
Message-ID: <20230713-bootleg-tray-c5bfe58b5673@wendy>
References: <20230712084134.1648008-1-sameo@rivosinc.com>
 <20230712084134.1648008-2-sameo@rivosinc.com>
 <20230712-bulldozer-affected-199042dc3afd@wendy>
 <CALs-HsuxxVcwX=mSwktPiEiAFkfK+5qJ6zg1Bzf2t37L=pZWjw@mail.gmail.com>
 <20230713-3f574332a06678f908cee21e@orel>
 <20230713-aggregate-uncut-e16ee9270b14@wendy>
 <20230713-692c967c7a08c15dacbcab10@orel>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="gTE48Nq7TkHCRHEc"
Content-Disposition: inline
In-Reply-To: <20230713-692c967c7a08c15dacbcab10@orel>
Precedence: bulk

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

On Thu, Jul 13, 2023 at 02:45:57PM +0200, Andrew Jones wrote:
> On Thu, Jul 13, 2023 at 12:27:24PM +0100, Conor Dooley wrote:

> > +#define __RISCV_ISA_EXT_BUNDLE(_name, _bundled_exts) {	\
> > +	.name =3D #_name,					\
> > +	.property =3D #_name,				\
> > +	.bundle_ids =3D _bundled_exts,			\
> > +	.bundle_size =3D ARRAY_SIZE(_bundled_exts)	\
> > +}
> > +
> > +static const unsigned int riscv_zk_bundled_exts[] =3D {
> > +	RISCV_ISA_EXT_ZBKB,
> > +	RISCV_ISA_EXT_ZBKC,
> > +	RISCV_ISA_EXT_ZBKX,
> > +	RISCV_ISA_EXT_ZKND,
> > +	RISCV_ISA_EXT_ZKNE,
> > +	RISCV_ISA_EXT_ZKR,
> > +	RISCV_ISA_EXT_ZKT,
>=20
> I think RISCV_ISA_EXT_ZKNH also belongs in this bundle,
> since the spec says zk is the zkn bundle plus zkr and zkt.
>=20
> > +};
> > +
> > +static const unsigned int riscv_zkn_bundled_exts[] =3D {
> > +	RISCV_ISA_EXT_ZBKB,
> > +	RISCV_ISA_EXT_ZBKC,
> > +	RISCV_ISA_EXT_ZBKX,
> > +	RISCV_ISA_EXT_ZKND,
> > +	RISCV_ISA_EXT_ZKNE,
> > +	RISCV_ISA_EXT_ZKNH,
> > +};
> > +
> > +static const unsigned int riscv_zks_bundled_exts[] =3D {
> > +	RISCV_ISA_EXT_ZBKB,
> > +	RISCV_ISA_EXT_ZBKC,
> > +	RISCV_ISA_EXT_ZKSED,
> > +	RISCV_ISA_EXT_ZKSH
>=20
> And, per the spec, this one appears to be missing RISCV_ISA_EXT_ZBKX.

Yeah, these do look wrong. I should've cross-checked it.

> I found [1] which calls these shorthands "group names", so maybe we should
> use the term "group" instead of "bundle"?

WFM at least.

> I'm tempted to try to directly
> code that graphic in [1] with something like...
>=20
> #define Zks_group1 \
> 	RISCV_ISA_EXT_ZKSED, \
> 	RISCV_ISA_EXT_ZKSH
>=20
> #define Zks_group2 \
> 	RISCV_ISA_EXT_ZBKB, \
> 	RISCV_ISA_EXT_ZBKC, \
> 	RISCV_ISA_EXT_ZBKX
>=20
> #define Zks_group \
> 	Zks_group1, \
> 	Zks_group2
>=20
> #define Zkn_group1 \
> 	RISCV_ISA_EXT_ZKND, \
> 	RISCV_ISA_EXT_ZKNE, \
> 	RISCV_ISA_EXT_ZKNH
>=20
> #define Zkn_group2 \
> 	Zks_group2
>=20
> #define Zkn_group \
> 	Zkn_group1, \
> 	Zkn_group2
>=20
> static const unsigned int riscv_zks_group[] =3D {
> 	Zks_group,
> };
>=20
> static const unsigned int riscv_zkn_group[] =3D {
> 	Zkn_group,
> };
>=20
> static const unsigned int riscv_zk_group[] =3D {
> 	Zks_group,
> 	Zkn_group,
> 	RISCV_ISA_EXT_ZKR,
> 	RISCV_ISA_EXT_ZKT,
> };
>=20
> ...but now that I have, I'm not sure I like the looks of it...

If you called them RISCV_ISA_EXT_GROUP_ZKN (or similar) it would look a
lot less out of place IMO. I'd probably drop the "group2" dance & pick a
better name for "Zks_group2", maybe just do something like
RISCV_ISA_EXT_GROUP_SCALAR_CRYPTO_BITMANIP? Mouthful, but seemed better
than trying to be clever with ZBK or something.

> [1] https://wiki.riscv.org/display/HOME/Scalar+Cryptography+Instruction+S=
et+Extension+Group+Names+Diagram


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

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

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZK/5OwAKCRB4tDGHoIJi
0ry5AP99PnMNEOYct0QDkIe2N2+G1CGzJmhUN6A1xqT/KkLs0QD/aE6I9W7tY9lW
A0LBQi1Gw8uUta0WbR/Ph1YYceZVtAU=
=6z5J
-----END PGP SIGNATURE-----

--gTE48Nq7TkHCRHEc--