On 22/04/2024 13:19, Conor Dooley wrote:
On Mon, Apr 22, 2024 at 10:53:04AM +0200, Clément Léger wrote:
On 19/04/2024 17:49, Conor Dooley wrote:
On Thu, Apr 18, 2024 at 02:42:26PM +0200, Clément Léger wrote:
As stated by Zc* spec:
"As C defines the same instructions as Zca, Zcf and Zcd, the rule is that:
- C always implies Zca
- C+F implies Zcf (RV32 only)
- C+D implies Zcd"
Add additionnal validation rules to enforce this in dts.
I'll get it out of the way: NAK, and the dts patch is the perfect example of why. I don't want us to have to continually update devicetrees. If these are implied due to being subsets of other extensions, then software should be able to enable them when that other extension is present.
Acked.
My fear is that, and a quick look at the "add probing" commit seemed to confirm it, new subsets would require updates to the dts, even though the existing extension is perfectly sufficient to determine presence.
I definitely want to avoid continual updates to the devicetree for churn reasons whenever subsets are added, but not turning on the likes of Zca when C is present because "the bindings were updated to enforce this" is a complete blocker. I do concede that having two parents makes that more difficult and will likely require some changes to how we probe - do we need to have a "second round" type thing?
Yeah, I understand. At first, I actually did the modifications in the ISA probing loop with some dependency probing (ie loop while we don't have a stable extension state). But I thought that it was not actually our problem but rather the ISA string provider. For instance, Qemu provides them.
A newer version of QEMU might, but not all do, so I'm not sure that using it is a good example. My expectations is that a devicetree will be written to the standards of the day and not be updated as subsets are released.
If this were the first instance of a superset/bundle I'd be prepared to accept an argument that we should not infer anything - but it's not and we'd be introducing inconsistency with the crypto stuff. I know that both scenarios are different in terms of extension history given that this is splitting things into a subset and that was a superset/bundle created at the same time, but they're not really that different in terms of the DT/ACPI to user "interface".
Taking Zcf as an example, maybe something like making both of C and F into "standard" supersets and adding a case to riscv_isa_extension_check() that would mandate that Zca and F are enabled before enabling it, and we would ensure that C implies Zca before it implies Zcf?
I'm afraid that riscv_isa_extension_check() will become a rat nest so rather than going that way, I would be in favor of adding a validation callback for the extensions if needed.
IOW, extension check split out per extension moving to be a callback?
Given we'd be relying on ordering, we have to perform the same implication for both F and C and make sure that the "implies" struct has Zca before Zcf. I don't really like that suggestion, hopefully there's a nicer way of doing that, but I don't like the dt stuff here.
I guess the "cleanest" way would be to have some "defered-like" mechanism in ISA probing which would allow to handle ordering as well as dependencies/implies for extensions. For Zca, Zcf, we actually do not have ordering problems but I think it would be a bit broken not to support that as well.
We could, I suppose, enable all detected extensions on a CPU and run the aforemention callback, disabling them if conditions are not met?
Is that something like what you're suggesting?
Yep, exactly. First parse the ISA blindly in a bitmap, (either from riscv,isa string, riscv,isa-extensions, or ACPI). Then in a second time, verify the ISA extensions by validating extension and looping until we reach a stable set.
Clément