[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230713-bootleg-tray-c5bfe58b5673@wendy>
Date: Thu, 13 Jul 2023 14:16:43 +0100
From: Conor Dooley <conor.dooley@...rochip.com>
To: Andrew Jones <ajones@...tanamicro.com>
CC: Evan Green <evan@...osinc.com>, Samuel Ortiz <sameo@...osinc.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
<linux-riscv@...ts.infradead.org>,
"Hongren (Zenithal) Zheng" <i@...ithal.me>, <linux@...osinc.com>,
Heiko Stuebner <heiko.stuebner@...ll.eu>,
Anup Patel <apatel@...tanamicro.com>,
<linux-kernel@...r.kernel.org>, Guo Ren <guoren@...nel.org>,
Atish Patra <atishp@...osinc.com>,
Björn Töpel <bjorn@...osinc.com>,
<devicetree@...r.kernel.org>, <sorear@...tmail.com>,
Jiatai He <jiatai2021@...as.ac.cn>
Subject: Re: [PATCH v4 1/4] RISC-V: Add Bitmanip/Scalar Crypto parsing from DT
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 = #_name, \
> > + .property = #_name, \
> > + .bundle_ids = _bundled_exts, \
> > + .bundle_size = ARRAY_SIZE(_bundled_exts) \
> > +}
> > +
> > +static const unsigned int riscv_zk_bundled_exts[] = {
> > + 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,
>
> I think RISCV_ISA_EXT_ZKNH also belongs in this bundle,
> since the spec says zk is the zkn bundle plus zkr and zkt.
>
> > +};
> > +
> > +static const unsigned int riscv_zkn_bundled_exts[] = {
> > + 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[] = {
> > + RISCV_ISA_EXT_ZBKB,
> > + RISCV_ISA_EXT_ZBKC,
> > + RISCV_ISA_EXT_ZKSED,
> > + RISCV_ISA_EXT_ZKSH
>
> 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...
>
> #define Zks_group1 \
> RISCV_ISA_EXT_ZKSED, \
> RISCV_ISA_EXT_ZKSH
>
> #define Zks_group2 \
> RISCV_ISA_EXT_ZBKB, \
> RISCV_ISA_EXT_ZBKC, \
> RISCV_ISA_EXT_ZBKX
>
> #define Zks_group \
> Zks_group1, \
> Zks_group2
>
> #define Zkn_group1 \
> RISCV_ISA_EXT_ZKND, \
> RISCV_ISA_EXT_ZKNE, \
> RISCV_ISA_EXT_ZKNH
>
> #define Zkn_group2 \
> Zks_group2
>
> #define Zkn_group \
> Zkn_group1, \
> Zkn_group2
>
> static const unsigned int riscv_zks_group[] = {
> Zks_group,
> };
>
> static const unsigned int riscv_zkn_group[] = {
> Zkn_group,
> };
>
> static const unsigned int riscv_zk_group[] = {
> Zks_group,
> Zkn_group,
> RISCV_ISA_EXT_ZKR,
> RISCV_ISA_EXT_ZKT,
> };
>
> ...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+Set+Extension+Group+Names+Diagram
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists