On Sun, Feb 18, 2024 at 09:00:14AM -0600, Samuel Holland wrote:
Or maybe we can still with the properties you have, but instead of treating them like any other extension, handle these separately, focusing on the numbering, so that only having the exact version supported by a cpu is possible.
Maybe I'm misunderstanding what you're saying here, but it is already the case that the DT for a CPU would only contain the exact version of the privileged ISA supported by that CPU.
If privileged spec versions are boolean extensions, then you would say "ss1p11", "ss1p12", "ss1p13" as separate/simultaneous extensions.
This is needed in order to allow simple support checks as described in the riscv,isa-extensions cover letter.
Yes, it is explicitly a goal of riscv,isa-extensions that you can look for a specific extension in the list if that is all you care about. If you go and drop ss1p11 because you support ss1p12 it defeats that.
Okay, that makes sense, but that is not how the parsing code works right now, which is probably what led me down the wrong path. :)
To have the intended semantics, we cannot parse _anything_ in riscv,isa-extensions as a "bundled" or "superset" extension.
That's not true I don't think. You can parse as a "bundle" but...
Because to accomplish your goal, each extension in the bundle must be listed explicitly, in case the consumer only cares about that one extension.
...as you note here, the extensions also have to be listed explicitly so that they can be detected in isolation if that is all that a consumer does.
So it sounds like riscv_fill_hwcap_from_ext_list() needs to ignore subset_ext_size/subset_ext_ids.
Do you mean this: if (ext->subset_ext_size) { for (int j = 0; j < ext->subset_ext_size; j++) { if (riscv_isa_extension_check(ext->subset_ext_ids[i])) set_bit(ext->subset_ext_ids[j], isainfo->isa); } }
I think that is fine? If you find the "superset" you can enable the individual elements. The problem would just be if someone put only the superset in a DT (or ACPI tables) and the software looked for the individual element only, but IIRC the kernel currently checks both the superset and individual elements. It would be possible to check a bundle and then skip looking for the individual elements if the bundle was already found if the parsing is wont to be sped up.
I think all we need to do is enforce that all individual elements are present on a schema validation level (I have no clue what we can do for ACPI) and no change is required in the kernel.
Am I misunderstanding what you think is the problem here?
I don't know off the top of my head how to enforce ss1p12 requiring ss1p11 in json schema, but I do have an idea of where to start...
Yeah, this is different from normal "dependencies:" because it is a string list.
I think it is actually doable, just will look sorta clunky. I meant to go and do it this weekend, but I've been rather sick unfortunately. Something similar is definitely doable for compatibles, so either it'll "just work" or I may have to extend the validation tooling.
Cheers, Conor.