On 24/10/2023 18:37, Evan Green wrote: > On Tue, Oct 24, 2023 at 2:30 AM Clément Léger <cleger@xxxxxxxxxxxx> wrote: >> >> >> >> On 24/10/2023 09:18, Clément Léger wrote: >>> >>> >>> On 23/10/2023 18:21, Evan Green wrote: >>>> On Tue, Oct 17, 2023 at 6:15 AM Clément Léger <cleger@xxxxxxxxxxxx> wrote: >>>>> >>>>> From: Evan Green <evan@xxxxxxxxxxxx> >>>>> >>>>> The Scalar Crypto specification defines Zk as a shorthand for the >>>>> Zkn, Zkr and Zkt extensions. The same follows for both Zkn, Zks and Zbk, >>>>> which are all shorthands for various other extensions. The detailed >>>>> breakdown can be found in their dt-binding entries. >>>>> >>>>> Since Zkn also implies the Zbkb, Zbkc and Zbkx extensions, simply passing >>>>> "zk" through a DT should enable all of Zbkb, Zbkc, Zbkx, Zkn, Zkr and Zkt. >>>>> For example, setting the "riscv,isa" DT property to "rv64imafdc_zk" >>>>> should generate the following cpuinfo output: >>>>> "rv64imafdc_zicntr_zicsr_zifencei_zihpm_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zkt" >>>>> >>>>> riscv_isa_ext_data grows a pair of new members, to permit setting the >>>>> relevant bits for "bundled" extensions, both while parsing the ISA string >>>>> and the new dedicated extension properties >>>>> >>>>> Co-developed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> >>>>> Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> >>>>> Signed-off-by: Evan Green <evan@xxxxxxxxxxxx> >>>>> Signed-off-by: Clément Léger <cleger@xxxxxxxxxxxx> >>>> >>>> My tree might be out of sync, but in my search for riscv_isa_ext, I >>>> also found a use in print_isa() (cpu.c) where we're reaching into >>>> riscv_isa_ext[].id and assuming it's always valid. If that's still in >>>> there we'll want to fix up that spot too, since now with bundles .id >>>> may or may not be valid. >>> >>> Oh indeed, the array is visible outside of this compilation unit :/. >>> I'll check that before sending V3. >> >> After looking a bit more at that, it actually seems that id is used in >> cpuinfo to determine which extensions are present which means you are >> right, bundle_size needs to be accounted for. >> >> Looking at it also raises the question (again) of exposing the "bundles" >> extensions themselves or not in cpuinfo output. With the current setup, >> the bundles extensions won't be visible in cpuinfo output. For instance >> if Zk was in the isa string, then it will not be visible in the cpuinfo >> output, only the child extensions. One solution would be to always have >> a valid id for each extension. So we would have one for Zk for instance. >> >> We would then have a similar setup for all "bundles" or "subset" >> extensions, they would have a id for all of them. For instance, Zk would >> become: >> >> __RISCV_ISA_EXT_DATA_BUNDLE(zk, RISCV_ISA_EXT_ZK, riscv_zk_bundled_exts) >> >> Same would go for zvbb (riscv_zvbb_subset_exts would only contain Zvkb): >> >> __RISCV_ISA_EXT_DATA_BUNDLE(zk, RISCV_ISA_EXT_ZVBB, riscv_zvbb_subset_exts) >> >> For the sake of completeness, I feel like it would be good to have all >> the extensions (bundles or not) visible in the riscv_isa_ext. >> >> Any objection ? > > I could be persuaded that it's a good idea, but there are arguments to > be made for not defining them as separate bits: > > 1. Having two (sets of) bits that mean the same thing means usermode > now has to decide which one to query, or query both. Multiple values > that mean the same thing is always something that makes me nervous. That is indeed an acceptable cons. > 2. To avoid these two sets having different answers, we'd have to > solve the reverse problem too: If all of the bundled extensions that > make up Zk are on, we'd need to detect that and turn Zk on as well. Oh yes, sorry, already forgot that point :/ Well, I guess things sorted out by themselves. Let's do what you proposed: - Pure lasso extensions (Zk) will simply result in all the sub extensions being enable, there won't be any .id specified for these ones, simply a bundle of extensions. - "Superset" extensions (Zvbb for instance) will have their own .id as well as a bundle made of subsets extensions. Clément > That code would also need to know the difference between a pure lasso > like Zk, where you should flip it on if its components are on, and the > loose change variant we were discussing on the other thread (Zvkb?), > where you cannot. > > Pretending pure lasso extensions didn't exist on their own was a way > to sidestep the problem. > -Evan
