On Thu, May 18, 2023 at 07:13:15PM +0530, Anup Patel wrote: > On Thu, May 18, 2023 at 4:02 PM Andrew Jones <ajones@xxxxxxxxxxxxxxxx> wrote: > > On Thu, May 18, 2023 at 09:58:30AM +0100, Conor Dooley wrote: > > > - riscv,isa: > > > - description: > > > - Identifies the specific RISC-V instruction set architecture > > > - supported by the hart. These are documented in the RISC-V > > > - User-Level ISA document, available from > > > - https://riscv.org/specifications/ > > > - > > > - Due to revisions of the ISA specification, some deviations > > > - have arisen over time. > > > - Notably, riscv,isa was defined prior to the creation of the > > > - Zicsr and Zifencei extensions and thus "i" implies > > > - "zicsr_zifencei". > > > - > > > - While the isa strings in ISA specification are case > > > - insensitive, letters in the riscv,isa string must be all > > > - lowercase to simplify parsing. > > > - $ref: "/schemas/types.yaml#/definitions/string" > > > - pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ > > > - > > > # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here > > > timebase-frequency: false > > > > > > @@ -133,8 +117,13 @@ properties: > > > DMIPS/MHz, relative to highest capacity-dmips-mhz > > > in the system. > > > > > > +oneOf: > > > + - required: > > > + - riscv,isa > > > > This is the part Anup keeps reminding me about. We can create better ways > > to handle extensions in DT and ACPI, but we'll still need to parse ISA > > strings to handle legacy DTs and holdouts that keep creating ISA strings, > > at least during the deprecation period, since ISA strings are still "the > > way to do it" according to the spec. > > Coming up with an alternate way in DT is fine but we can't deprecate > ISA strings since ISA strings are widely used: > 1) Various bootloaders Aye, for the reason, as I mentioned earlier and in the RFC thread, removing existing parsers isn't a good idea. > 2) It is part of /proc/cpuinfo That is irrelevant. > 3) Hypervisors use it to communicate HW features to Guest/VM. > Hypervisors can't get away from generating ISA strings because > Hypervisors don't know what is running inside Guest/VM. Generate both :) As things stand, your guests could interpret what you communicate to them via riscv,isa differently! > In the case of ACPI, it is a very different situation. Like Sunil mentioned, > ACPI will always follow mechanisms defined by RVI (such as ISA string). > Other ACPI approaches such as GUID for ISA extension are simply not > scalable and will take a lot more memory for ACPI tables compared to > ISA strings. My proposal should actually suit ACPI, at least for Linux, as it would be a chance to align currently misaligned definitions. I won't speak to GUIDs or whatever as that's someone else's problem :) > > Also, if we assume the wording in the spec does get shored up, then, > > unless I'm missing something, the list of advantages for this boolean > > proposal from your commit message would be > > IMO, we should try our best to have the wordings changed in RVI spec. Yes, doing so is beneficial for all of us regardless of what happens here. I do think that it is partially orthogonal - it allows us to not design an interface that needs to be capable of communicating a wide variety of versions, but I don't think it solves some of the issues that riscv,isa has. If I thought it did, I would not have gone to the trouble of respinning this patch out of the other approach. > > * More character choices for name -- probably not a huge gain for ratified > > extensions, since the boolean properties will likely still use the same > > name as the ISA string (riscv,isa-extension-<name>). But, for vendor > > extensions, this is indeed a major improvement, since vendor extension > > boolean property names may need to be extended in unambiguous ways to > > handle changes in the extension. > > > > * Simpler, more complete DT validation (but we still need a best effort > > for legacy ISA strings) > > > > * Simpler DT parsing (but we still need the current parser for legacy ISA > > strings) > > > > > + - required: > > > + - riscv,isa-base > > > + > > > required: > > > - - riscv,isa > > > - interrupt-controller > > > > > > additionalProperties: true > > > @@ -177,7 +166,13 @@ examples: > > > i-tlb-size = <32>; > > > mmu-type = "riscv,sv39"; > > > reg = <1>; > > > - riscv,isa = "rv64imafdc"; > > > + riscv,isa-base = "rv64i"; > > > + riscv,isa-extension-i; > > > + riscv,isa-extension-m; > > > + riscv,isa-extension-a; > > > + riscv,isa-extension-f; > > > + riscv,isa-extension-d; > > > + riscv,isa-extension-c; > > One downside of this new approach is it will increase the size of DTB. > Imaging 50 such DT properties in 46 CPU DT nodes. I should do a comparison between 50 extensions in riscv,isa and doing this 50 times and see what the sizes are.
Attachment:
signature.asc
Description: PGP signature
