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
Attachment:
signature.asc
Description: PGP signature
