Re: [PATCH v4 1/4] RISC-V: Add Bitmanip/Scalar Crypto parsing from DT

[Evan Green]

On Wed, Jul 12, 2023 at 3:39 AM Conor Dooley <conor.dooley@xxxxxxxxxxxxx> wrote:
>
> Hey Samuel, Evan,
>
> On Wed, Jul 12, 2023 at 10:41:17AM +0200, Samuel Ortiz wrote:
> > From: "Hongren (Zenithal) Zheng" <i@xxxxxxxxxxx>
> >
> > Parse Zb/Zk related string from DT and output them to cpuinfo.
>
> One thing that has sprung to mind is that this is not limited to DT
> anymore, since the information could in theory come from ACPI too.
> Ditto the title I guess.
>
> > It is worth noting that the Scalar Crypto extension defines "zk" as a
> > shorthand for the Zkn, Zkr and Zkt extensions. Since the Zkn one also
> > implies the Zbkb, Zbkc and Zbkx extensions, simply passing the valid
> > "zk" extension name through a DT will enable all of the  Zbkb, Zbkc,
> > Zbkx, Zkn, Zkr and Zkt extensions.
> >
> > Also, since there currently is no mechanism to merge all enabled
> > extensions, the generated cpuinfo output could be relatively large.
> > For example, setting the "riscv,isa" DT property to "rv64imafdc_zk_zks"
> > will generate the following cpuinfo output:
> > "rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed_zksh_zkt".
>
> On that note, I've created another version of what checking for
> supersets could look like, since it'll be needed either by my series or
> this one, depending on what gets merged first. I've yet to test the
> dedicated extensions part of it, but I wanted to get this out before I
> went looking at other fixes in the area.
>
> Evan, since it was you that commented on this stuff last time around,
> could you take another look? I'm still not keen on the "subset_of"
> arrays, but they're an improvement on what I had last time around for
> sure.
>

This looks alright to me. At the risk of getting into bikeshedding
territory, the only awkward bit of it is it composes the extensions in
sort of the opposite way you'd expect. I tend to think of Zks as being
comprised of {zbkb, zbkc, zksed, zksh}, rather than zbkb being a part
of {zks, zkn, zk}, though both are of course correct. Here's an
untested version of the other way. You can decide if you like it
better or worse than what you've got, and I'm fine either way. Sorry
gmail mangles it, if you want the patch for real I can get it to you: