* [PATCH 0/2] riscv,isa fixups @ 2022-11-24 13:04 Conor Dooley 2022-11-24 13:04 ` [PATCH 1/2] dt-bindings: riscv: fix underscore requirement for addtional standard extensions Conor Dooley 2022-11-24 13:04 ` [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order Conor Dooley 0 siblings, 2 replies; 31+ messages in thread From: Conor Dooley @ 2022-11-24 13:04 UTC (permalink / raw) To: linux-riscv Cc: Conor Dooley, Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Stuebner, Andrew Jones, Guo Ren, devicetree, linux-kernel I noticed today while looking at the isa manual that I had not accounted for another couple of edge cases with my regex. As before, I think attempting to validate the canonical order for multiletter stuff makes no sense - but we should totally try to avoid false-positives for combinations that are known to be valid. Thanks, Conor. CC: Conor Dooley <conor@kernel.org> CC: Rob Herring <robh+dt@kernel.org> CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> CC: Paul Walmsley <paul.walmsley@sifive.com> CC: Palmer Dabbelt <palmer@dabbelt.com> CC: Albert Ou <aou@eecs.berkeley.edu> CC: Heiko Stuebner <heiko@sntech.de> CC: Andrew Jones <ajones@ventanamicro.com> CC: Guo Ren <guoren@kernel.org> CC: linux-riscv@lists.infradead.org CC: devicetree@vger.kernel.org CC: linux-kernel@vger.kernel.org Conor Dooley (2): dt-bindings: riscv: fix underscore requirement for addtional standard extensions dt-bindings: riscv: fix single letter canonical order Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.38.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/2] dt-bindings: riscv: fix underscore requirement for addtional standard extensions 2022-11-24 13:04 [PATCH 0/2] riscv,isa fixups Conor Dooley @ 2022-11-24 13:04 ` Conor Dooley 2022-11-25 1:12 ` Guo Ren 2022-11-24 13:04 ` [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order Conor Dooley 1 sibling, 1 reply; 31+ messages in thread From: Conor Dooley @ 2022-11-24 13:04 UTC (permalink / raw) To: linux-riscv Cc: Conor Dooley, Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Stuebner, Andrew Jones, Guo Ren, devicetree, linux-kernel The RISC-V ISA Manual allows for the first Additional Standard Extension having no leading underscore. Only if there are multiple Additional Standard Extensions is it needed to have an underscore. The dt-binding does not validate that a multi-letter extension is canonically ordered, as that'd need an even worse regex than is here, but it should not fail validation for valid ISA strings. Allow the first Z multi-letter extension to appear immediately prior after the single-letter extensions. Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") Signed-off-by: Conor Dooley <conor.dooley@microchip.com> --- Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml index 90a7cabf58fe..e80c967a4fa4 100644 --- a/Documentation/devicetree/bindings/riscv/cpus.yaml +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml @@ -80,7 +80,7 @@ properties: 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?v?k?h?(?:_[hsxz](?:[a-z])+)*$ + pattern: ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here timebase-frequency: false -- 2.38.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] dt-bindings: riscv: fix underscore requirement for addtional standard extensions 2022-11-24 13:04 ` [PATCH 1/2] dt-bindings: riscv: fix underscore requirement for addtional standard extensions Conor Dooley @ 2022-11-25 1:12 ` Guo Ren 0 siblings, 0 replies; 31+ messages in thread From: Guo Ren @ 2022-11-25 1:12 UTC (permalink / raw) To: Conor Dooley Cc: linux-riscv, Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Stuebner, Andrew Jones, devicetree, linux-kernel On Thu, Nov 24, 2022 at 9:06 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > The RISC-V ISA Manual allows for the first Additional Standard > Extension having no leading underscore. Only if there are multiple > Additional Standard Extensions is it needed to have an underscore. > > The dt-binding does not validate that a multi-letter extension is > canonically ordered, as that'd need an even worse regex than is here, > but it should not fail validation for valid ISA strings. > > Allow the first Z multi-letter extension to appear immediately prior > after the single-letter extensions. > > Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 > Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml > index 90a7cabf58fe..e80c967a4fa4 100644 > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml > @@ -80,7 +80,7 @@ properties: > 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?v?k?h?(?:_[hsxz](?:[a-z])+)*$ > + pattern: ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ Acked-by: Guo Ren <guoren@kernel.org> > > # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here > timebase-frequency: false > -- > 2.38.1 > -- Best Regards Guo Ren _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-24 13:04 [PATCH 0/2] riscv,isa fixups Conor Dooley 2022-11-24 13:04 ` [PATCH 1/2] dt-bindings: riscv: fix underscore requirement for addtional standard extensions Conor Dooley @ 2022-11-24 13:04 ` Conor Dooley 2022-11-24 13:42 ` Heiko Stübner 2022-11-25 1:13 ` [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order Guo Ren 1 sibling, 2 replies; 31+ messages in thread From: Conor Dooley @ 2022-11-24 13:04 UTC (permalink / raw) To: linux-riscv Cc: Conor Dooley, Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Stuebner, Andrew Jones, Guo Ren, devicetree, linux-kernel I used the wikipedia table for ordering extensions when updating the pattern here in foo. Unfortunately that table did not match canonical order, as defined by the RISC-V ISA Manual, which defines extension ordering in (what is currently) Table 41, "Standard ISA extension names". Fix things up by re-sorting v (vector) and adding p (packed-simd) & j (dynamic languages). The e (reduced integer) and g (general) extensions are still intentionally left out. Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") Signed-off-by: Conor Dooley <conor.dooley@microchip.com> --- Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml index e80c967a4fa4..b7462ea2dbe4 100644 --- a/Documentation/devicetree/bindings/riscv/cpus.yaml +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml @@ -80,7 +80,7 @@ properties: 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?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ + pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here timebase-frequency: false -- 2.38.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-24 13:04 ` [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order Conor Dooley @ 2022-11-24 13:42 ` Heiko Stübner 2022-11-24 13:52 ` Conor Dooley 2022-11-28 17:41 ` Palmer Dabbelt 2022-11-25 1:13 ` [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order Guo Ren 1 sibling, 2 replies; 31+ messages in thread From: Heiko Stübner @ 2022-11-24 13:42 UTC (permalink / raw) To: linux-riscv, Conor Dooley Cc: Conor Dooley, Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrew Jones, Guo Ren, devicetree, linux-kernel Am Donnerstag, 24. November 2022, 14:04:41 CET schrieb Conor Dooley: > I used the wikipedia table for ordering extensions when updating the > pattern here in foo. ^ foo? :-) > Unfortunately that table did not match canonical order, as defined by > the RISC-V ISA Manual, which defines extension ordering in (what is > currently) Table 41, "Standard ISA extension names". Fix things up by > re-sorting v (vector) and adding p (packed-simd) & j (dynamic > languages). The e (reduced integer) and g (general) extensions are still > intentionally left out. > > Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 > Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> So I have compared the new pattern to the isa manual, and it looks like the order checks out, so Reviewed-by: Heiko Stuebner <heiko@sntech.de> > --- > Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml > index e80c967a4fa4..b7462ea2dbe4 100644 > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml > @@ -80,7 +80,7 @@ properties: > 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?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ > + pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ > > # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here > timebase-frequency: false > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-24 13:42 ` Heiko Stübner @ 2022-11-24 13:52 ` Conor Dooley 2022-11-28 17:41 ` Palmer Dabbelt 1 sibling, 0 replies; 31+ messages in thread From: Conor Dooley @ 2022-11-24 13:52 UTC (permalink / raw) To: Heiko Stübner Cc: linux-riscv, Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrew Jones, Guo Ren, devicetree, linux-kernel On Thu, Nov 24, 2022 at 02:42:20PM +0100, Heiko Stübner wrote: > Am Donnerstag, 24. November 2022, 14:04:41 CET schrieb Conor Dooley: > > I used the wikipedia table for ordering extensions when updating the > > pattern here in foo. > > ^ foo? :-) God damn it! I wrote foo, left nvim to check what the fixes tag would be, came back and added it below but did not update this part of the commit log... > > Unfortunately that table did not match canonical order, as defined by > > the RISC-V ISA Manual, which defines extension ordering in (what is > > currently) Table 41, "Standard ISA extension names". Fix things up by > > re-sorting v (vector) and adding p (packed-simd) & j (dynamic > > languages). The e (reduced integer) and g (general) extensions are still > > intentionally left out. > > > > Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 > > Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > So I have compared the new pattern to the isa manual, > and it looks like the order checks out, so > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> Thanks! _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-24 13:42 ` Heiko Stübner 2022-11-24 13:52 ` Conor Dooley @ 2022-11-28 17:41 ` Palmer Dabbelt 2022-11-28 18:08 ` Conor Dooley 1 sibling, 1 reply; 31+ messages in thread From: Palmer Dabbelt @ 2022-11-28 17:41 UTC (permalink / raw) To: heiko Cc: linux-riscv, Conor Dooley, Conor Dooley, Conor Dooley, robh+dt, krzysztof.kozlowski+dt, Paul Walmsley, aou, ajones, guoren, devicetree, linux-kernel On Thu, 24 Nov 2022 05:42:20 PST (-0800), heiko@sntech.de wrote: > Am Donnerstag, 24. November 2022, 14:04:41 CET schrieb Conor Dooley: >> I used the wikipedia table for ordering extensions when updating the >> pattern here in foo. > > ^ foo? :-) > >> Unfortunately that table did not match canonical order, as defined by >> the RISC-V ISA Manual, which defines extension ordering in (what is >> currently) Table 41, "Standard ISA extension names". Fix things up by >> re-sorting v (vector) and adding p (packed-simd) & j (dynamic >> languages). The e (reduced integer) and g (general) extensions are still >> intentionally left out. >> >> Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 >> Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > So I have compared the new pattern to the isa manual, > and it looks like the order checks out, so Which ISA manual? There have been many mutually incompatible ISA string encoding rules, at least one of them was a change to the extension ordering. It's not entirely clear what the right answer is here, as we can't really parse ISA strings without also knowing the version of the ISA manual we're meant to parse them against. Maybe we just accept everything? IMO it's time to just stop using the ISA string. It's not a stable interface, trying to treat it as such just leads to headaches. We should just come up with some DT-specific way of encoding whatever HW features are in question. Sure it'll be a bit of work to write that all down in the DT bindings, but it's going to be way less work than trying to keep around all this ISA string parsing code. I know I've said the opposite before, but there's just been way too many breakages here to assume they're going to stop. > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > >> --- >> Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml >> index e80c967a4fa4..b7462ea2dbe4 100644 >> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml >> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml >> @@ -80,7 +80,7 @@ properties: >> 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?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ >> + pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ >> >> # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here >> timebase-frequency: false >> _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-28 17:41 ` Palmer Dabbelt @ 2022-11-28 18:08 ` Conor Dooley 2022-11-28 18:12 ` Palmer Dabbelt 0 siblings, 1 reply; 31+ messages in thread From: Conor Dooley @ 2022-11-28 18:08 UTC (permalink / raw) To: Palmer Dabbelt Cc: heiko, linux-riscv, Conor Dooley, robh+dt, krzysztof.kozlowski+dt, Paul Walmsley, aou, ajones, guoren, devicetree, linux-kernel On Mon, Nov 28, 2022 at 09:41:03AM -0800, Palmer Dabbelt wrote: > On Thu, 24 Nov 2022 05:42:20 PST (-0800), heiko@sntech.de wrote: > > Am Donnerstag, 24. November 2022, 14:04:41 CET schrieb Conor Dooley: > > > I used the wikipedia table for ordering extensions when updating the > > > pattern here in foo. > > > > ^ foo? :-) > > > > > Unfortunately that table did not match canonical order, as defined by > > > the RISC-V ISA Manual, which defines extension ordering in (what is > > > currently) Table 41, "Standard ISA extension names". Fix things up by > > > re-sorting v (vector) and adding p (packed-simd) & j (dynamic > > > languages). The e (reduced integer) and g (general) extensions are still > > > intentionally left out. > > > > > > Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 > > > Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > > > So I have compared the new pattern to the isa manual, > > and it looks like the order checks out, so > > Which ISA manual? For me, isa manual is the above github repo. > There have been many mutually incompatible ISA string > encoding rules, at least one of them was a change to the extension ordering. > It's not entirely clear what the right answer is here, as we can't really > parse ISA strings without also knowing the version of the ISA manual we're > meant to parse them against. Maybe we just accept everything? I don't think accepting everything is the right thing to do. A minimal amount of validation is still needed here, but I think we can deprecate the DT property entirely & make it optional if a new-and-improved way of encoding the in DT is used. > IMO it's time to just stop using the ISA string. It's not a stable > interface, trying to treat it as such just leads to headaches. We should > just come up with some DT-specific way of encoding whatever HW features are > in question. Sure it'll be a bit of work to write that all down in the DT > bindings, but it's going to be way less work than trying to keep around all > this ISA string parsing code. I'm a glutton for punishment, I'll try and come up with some sort of other way to encode this information in DT that requires less parsing and validation. As I said on IRC, something that more resembles: if (of_property_wahtever("riscv,isa-foo")) { do_enable_foo() } > I know I've said the opposite before, but there's just been way too many > breakages here to assume they're going to stop. :upside_down_face: Either way, I think these two patches are worth taking in the mean time. > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > > > > > --- > > > Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml > > > index e80c967a4fa4..b7462ea2dbe4 100644 > > > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml > > > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml > > > @@ -80,7 +80,7 @@ properties: > > > 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?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ > > > + pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ > > > > > > # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here > > > timebase-frequency: false > > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-28 18:08 ` Conor Dooley @ 2022-11-28 18:12 ` Palmer Dabbelt 2022-11-28 19:17 ` Conor Dooley 0 siblings, 1 reply; 31+ messages in thread From: Palmer Dabbelt @ 2022-11-28 18:12 UTC (permalink / raw) To: Conor Dooley Cc: heiko, linux-riscv, Conor Dooley, robh+dt, krzysztof.kozlowski+dt, Paul Walmsley, aou, ajones, guoren, devicetree, linux-kernel On Mon, 28 Nov 2022 10:08:05 PST (-0800), Conor Dooley wrote: > On Mon, Nov 28, 2022 at 09:41:03AM -0800, Palmer Dabbelt wrote: >> On Thu, 24 Nov 2022 05:42:20 PST (-0800), heiko@sntech.de wrote: >> > Am Donnerstag, 24. November 2022, 14:04:41 CET schrieb Conor Dooley: >> > > I used the wikipedia table for ordering extensions when updating the >> > > pattern here in foo. >> > >> > ^ foo? :-) >> > >> > > Unfortunately that table did not match canonical order, as defined by >> > > the RISC-V ISA Manual, which defines extension ordering in (what is >> > > currently) Table 41, "Standard ISA extension names". Fix things up by >> > > re-sorting v (vector) and adding p (packed-simd) & j (dynamic >> > > languages). The e (reduced integer) and g (general) extensions are still >> > > intentionally left out. >> > > >> > > Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 >> > > Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") >> > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >> > >> > So I have compared the new pattern to the isa manual, >> > and it looks like the order checks out, so >> >> Which ISA manual? > > For me, isa manual is the above github repo. Which commit, though? > >> There have been many mutually incompatible ISA string >> encoding rules, at least one of them was a change to the extension ordering. >> It's not entirely clear what the right answer is here, as we can't really >> parse ISA strings without also knowing the version of the ISA manual we're >> meant to parse them against. Maybe we just accept everything? > > I don't think accepting everything is the right thing to do. A minimal > amount of validation is still needed here, but I think we can deprecate > the DT property entirely & make it optional if a new-and-improved way of > encoding the in DT is used. Sorry, by "everything" I meant "everything that's even been allowed by the ISA manual". Just accetping anything would be bad ;) >> IMO it's time to just stop using the ISA string. It's not a stable >> interface, trying to treat it as such just leads to headaches. We should >> just come up with some DT-specific way of encoding whatever HW features are >> in question. Sure it'll be a bit of work to write that all down in the DT >> bindings, but it's going to be way less work than trying to keep around all >> this ISA string parsing code. > > I'm a glutton for punishment, I'll try and come up with some sort of > other way to encode this information in DT that requires less parsing > and validation. As I said on IRC, something that more resembles: > if (of_property_wahtever("riscv,isa-foo")) { do_enable_foo() } That seems way simpler to me, thanks! We'll still need to support whatever was here as a legacy format, but at least we won't need to add a bunch of new stuff to it -- that's where the parsing starts to get really complicated. FWIW, there's a similar dicussion going on in GCC land right now. >> I know I've said the opposite before, but there's just been way too many >> breakages here to assume they're going to stop. > > :upside_down_face: > > Either way, I think these two patches are worth taking in the mean time. Yep, just as long as it doesn't break any of the strings that were valid according to previous versions of the ISA manual I'm fine with it. >> > Reviewed-by: Heiko Stuebner <heiko@sntech.de> >> > >> > > --- >> > > Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- >> > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > >> > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml >> > > index e80c967a4fa4..b7462ea2dbe4 100644 >> > > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml >> > > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml >> > > @@ -80,7 +80,7 @@ properties: >> > > 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?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ >> > > + pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ >> > > >> > > # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here >> > > timebase-frequency: false >> > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-28 18:12 ` Palmer Dabbelt @ 2022-11-28 19:17 ` Conor Dooley 2022-11-28 23:41 ` Palmer Dabbelt 0 siblings, 1 reply; 31+ messages in thread From: Conor Dooley @ 2022-11-28 19:17 UTC (permalink / raw) To: Palmer Dabbelt Cc: heiko, linux-riscv, Conor Dooley, robh+dt, krzysztof.kozlowski+dt, Paul Walmsley, aou, ajones, guoren, devicetree, linux-kernel On Mon, Nov 28, 2022 at 10:12:17AM -0800, Palmer Dabbelt wrote: > On Mon, 28 Nov 2022 10:08:05 PST (-0800), Conor Dooley wrote: > > On Mon, Nov 28, 2022 at 09:41:03AM -0800, Palmer Dabbelt wrote: > > > On Thu, 24 Nov 2022 05:42:20 PST (-0800), heiko@sntech.de wrote: > > > > Am Donnerstag, 24. November 2022, 14:04:41 CET schrieb Conor Dooley: > > > > > I used the wikipedia table for ordering extensions when updating the > > > > > pattern here in foo. > > > > > > > > ^ foo? :-) > > > > > > > > > Unfortunately that table did not match canonical order, as defined by > > > > > the RISC-V ISA Manual, which defines extension ordering in (what is > > > > > currently) Table 41, "Standard ISA extension names". Fix things up by > > > > > re-sorting v (vector) and adding p (packed-simd) & j (dynamic > > > > > languages). The e (reduced integer) and g (general) extensions are still > > > > > intentionally left out. > > > > > > > > > > Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 > > > > > Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") > > > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > > > > > > > So I have compared the new pattern to the isa manual, > > > > and it looks like the order checks out, so > > > > > > Which ISA manual? > > > > For me, isa manual is the above github repo. > > Which commit, though? mutt won't let me paste a clown face emoticon. > > > There have been many mutually incompatible ISA string > > > encoding rules, at least one of them was a change to the extension ordering. > > > It's not entirely clear what the right answer is here, as we can't really > > > parse ISA strings without also knowing the version of the ISA manual we're > > > meant to parse them against. Maybe we just accept everything? > > > > I don't think accepting everything is the right thing to do. A minimal > > amount of validation is still needed here, but I think we can deprecate > > the DT property entirely & make it optional if a new-and-improved way of > > encoding the in DT is used. > > Sorry, by "everything" I meant "everything that's even been allowed by the > ISA manual". Just accetping anything would be bad ;) > > > > IMO it's time to just stop using the ISA string. It's not a stable > > > interface, trying to treat it as such just leads to headaches. We should > > > just come up with some DT-specific way of encoding whatever HW features are > > > in question. Sure it'll be a bit of work to write that all down in the DT > > > bindings, but it's going to be way less work than trying to keep around all > > > this ISA string parsing code. > > > > I'm a glutton for punishment, I'll try and come up with some sort of > > other way to encode this information in DT that requires less parsing > > and validation. As I said on IRC, something that more resembles: > > if (of_property_wahtever("riscv,isa-foo")) { do_enable_foo() } > > That seems way simpler to me, thanks! We'll still need to support whatever > was here as a legacy format, but at least we won't need to add a bunch of > new stuff to it -- that's where the parsing starts to get really > complicated. Yah, and "deprecated" in dt-schema doesn't actually do anything at the moment other than let humans know not to use something. Just gonna have to do some sort of "feature-wise AND" between the existing things we parse from the isa string & whatever riscv,isa-foo stuff later on. > FWIW, there's a similar dicussion going on in GCC land right now. > > > > I know I've said the opposite before, but there's just been way too many > > > breakages here to assume they're going to stop. > > > > :upside_down_face: > > > > Either way, I think these two patches are worth taking in the mean time. > > Yep, just as long as it doesn't break any of the strings that were valid > according to previous versions of the ISA manual I'm fine with it. I don't think so. I had been looking around for a supposed order for where to actually put H, which had been dropped - and the only place I recall seeing that was Wikipedia - which now seems like an awful decision since the order there looks kinda off anything I see in dozen or so spec PDFs I have downloaded. But that's where I got the K & V ordering from that I now think is wrong (and doesn't match any PDF I have). The other changes relax rules and add letters so they should be okay too. > > > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > > > > > > > > > --- > > > > > Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml > > > > > index e80c967a4fa4..b7462ea2dbe4 100644 > > > > > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml > > > > > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml > > > > > @@ -80,7 +80,7 @@ properties: > > > > > 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?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ > > > > > + pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ > > > > > > > > > > # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here > > > > > timebase-frequency: false > > > > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-28 19:17 ` Conor Dooley @ 2022-11-28 23:41 ` Palmer Dabbelt 2022-11-29 5:19 ` Andrew Jones 0 siblings, 1 reply; 31+ messages in thread From: Palmer Dabbelt @ 2022-11-28 23:41 UTC (permalink / raw) To: Conor Dooley Cc: heiko, linux-riscv, Conor Dooley, robh+dt, krzysztof.kozlowski+dt, Paul Walmsley, aou, ajones, guoren, devicetree, linux-kernel On Mon, 28 Nov 2022 11:17:21 PST (-0800), Conor Dooley wrote: > On Mon, Nov 28, 2022 at 10:12:17AM -0800, Palmer Dabbelt wrote: >> On Mon, 28 Nov 2022 10:08:05 PST (-0800), Conor Dooley wrote: >> > On Mon, Nov 28, 2022 at 09:41:03AM -0800, Palmer Dabbelt wrote: >> > > On Thu, 24 Nov 2022 05:42:20 PST (-0800), heiko@sntech.de wrote: >> > > > Am Donnerstag, 24. November 2022, 14:04:41 CET schrieb Conor Dooley: >> > > > > I used the wikipedia table for ordering extensions when updating the >> > > > > pattern here in foo. >> > > > >> > > > ^ foo? :-) >> > > > >> > > > > Unfortunately that table did not match canonical order, as defined by >> > > > > the RISC-V ISA Manual, which defines extension ordering in (what is >> > > > > currently) Table 41, "Standard ISA extension names". Fix things up by >> > > > > re-sorting v (vector) and adding p (packed-simd) & j (dynamic >> > > > > languages). The e (reduced integer) and g (general) extensions are still >> > > > > intentionally left out. >> > > > > >> > > > > Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 >> > > > > Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") >> > > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >> > > > >> > > > So I have compared the new pattern to the isa manual, >> > > > and it looks like the order checks out, so >> > > >> > > Which ISA manual? >> > >> > For me, isa manual is the above github repo. >> >> Which commit, though? > > mutt won't let me paste a clown face emoticon. > >> > > There have been many mutually incompatible ISA string >> > > encoding rules, at least one of them was a change to the extension ordering. >> > > It's not entirely clear what the right answer is here, as we can't really >> > > parse ISA strings without also knowing the version of the ISA manual we're >> > > meant to parse them against. Maybe we just accept everything? >> > >> > I don't think accepting everything is the right thing to do. A minimal >> > amount of validation is still needed here, but I think we can deprecate >> > the DT property entirely & make it optional if a new-and-improved way of >> > encoding the in DT is used. >> >> Sorry, by "everything" I meant "everything that's even been allowed by the >> ISA manual". Just accetping anything would be bad ;) >> >> > > IMO it's time to just stop using the ISA string. It's not a stable >> > > interface, trying to treat it as such just leads to headaches. We should >> > > just come up with some DT-specific way of encoding whatever HW features are >> > > in question. Sure it'll be a bit of work to write that all down in the DT >> > > bindings, but it's going to be way less work than trying to keep around all >> > > this ISA string parsing code. >> > >> > I'm a glutton for punishment, I'll try and come up with some sort of >> > other way to encode this information in DT that requires less parsing >> > and validation. As I said on IRC, something that more resembles: >> > if (of_property_wahtever("riscv,isa-foo")) { do_enable_foo() } >> >> That seems way simpler to me, thanks! We'll still need to support whatever >> was here as a legacy format, but at least we won't need to add a bunch of >> new stuff to it -- that's where the parsing starts to get really >> complicated. > > Yah, and "deprecated" in dt-schema doesn't actually do anything at the > moment other than let humans know not to use something. Just gonna have > to do some sort of "feature-wise AND" between the existing things we > parse from the isa string & whatever riscv,isa-foo stuff later on. I suppose this is more of a Rob question, but could we just make the DT bindings match the current ISA manual's rules and then have the kernel's "riscv,isa" string parser accept more orderings to remain compatible? Sort of a API vs ABI stability question, but for DTB and bindngs. >> FWIW, there's a similar dicussion going on in GCC land right now. >> >> > > I know I've said the opposite before, but there's just been way too many >> > > breakages here to assume they're going to stop. >> > >> > :upside_down_face: >> > >> > Either way, I think these two patches are worth taking in the mean time. >> >> Yep, just as long as it doesn't break any of the strings that were valid >> according to previous versions of the ISA manual I'm fine with it. > > I don't think so. I had been looking around for a supposed order for > where to actually put H, which had been dropped - and the only place I > recall seeing that was Wikipedia - which now seems like an awful > decision since the order there looks kinda off anything I see in dozen > or so spec PDFs I have downloaded. But that's where I got the K & V > ordering from that I now think is wrong (and doesn't match any PDF I > have). The other changes relax rules and add letters so they should be > okay too. Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> as it sounds like there aren't even any fixed rules, so I guess none of this even matters? > >> > > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> >> > > > >> > > > > --- >> > > > > Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- >> > > > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > > > >> > > > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml >> > > > > index e80c967a4fa4..b7462ea2dbe4 100644 >> > > > > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml >> > > > > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml >> > > > > @@ -80,7 +80,7 @@ properties: >> > > > > 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?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ >> > > > > + pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ >> > > > > >> > > > > # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here >> > > > > timebase-frequency: false >> > > > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-28 23:41 ` Palmer Dabbelt @ 2022-11-29 5:19 ` Andrew Jones 2022-11-29 11:40 ` Conor Dooley 0 siblings, 1 reply; 31+ messages in thread From: Andrew Jones @ 2022-11-29 5:19 UTC (permalink / raw) To: Palmer Dabbelt Cc: Conor Dooley, heiko, linux-riscv, Conor Dooley, robh+dt, krzysztof.kozlowski+dt, Paul Walmsley, aou, guoren, devicetree, linux-kernel, Sunil V L On Mon, Nov 28, 2022 at 03:41:19PM -0800, Palmer Dabbelt wrote: > On Mon, 28 Nov 2022 11:17:21 PST (-0800), Conor Dooley wrote: > > On Mon, Nov 28, 2022 at 10:12:17AM -0800, Palmer Dabbelt wrote: > > > On Mon, 28 Nov 2022 10:08:05 PST (-0800), Conor Dooley wrote: > > > > On Mon, Nov 28, 2022 at 09:41:03AM -0800, Palmer Dabbelt wrote: > > > > > On Thu, 24 Nov 2022 05:42:20 PST (-0800), heiko@sntech.de wrote: > > > > > > Am Donnerstag, 24. November 2022, 14:04:41 CET schrieb Conor Dooley: > > > > > > > I used the wikipedia table for ordering extensions when updating the > > > > > > > pattern here in foo. > > > > > > > > > > > > ^ foo? :-) > > > > > > > > > > > > > Unfortunately that table did not match canonical order, as defined by > > > > > > > the RISC-V ISA Manual, which defines extension ordering in (what is > > > > > > > currently) Table 41, "Standard ISA extension names". Fix things up by > > > > > > > re-sorting v (vector) and adding p (packed-simd) & j (dynamic > > > > > > > languages). The e (reduced integer) and g (general) extensions are still > > > > > > > intentionally left out. > > > > > > > > > > > > > > Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 > > > > > > > Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") > > > > > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > > > > > > > > > > > So I have compared the new pattern to the isa manual, > > > > > > and it looks like the order checks out, so > > > > > > > > > > Which ISA manual? > > > > > > > > For me, isa manual is the above github repo. > > > > > > Which commit, though? > > > > mutt won't let me paste a clown face emoticon. > > > > > > > There have been many mutually incompatible ISA string > > > > > encoding rules, at least one of them was a change to the extension ordering. > > > > > It's not entirely clear what the right answer is here, as we can't really > > > > > parse ISA strings without also knowing the version of the ISA manual we're > > > > > meant to parse them against. Maybe we just accept everything? > > > > > > > > I don't think accepting everything is the right thing to do. A minimal > > > > amount of validation is still needed here, but I think we can deprecate > > > > the DT property entirely & make it optional if a new-and-improved way of > > > > encoding the in DT is used. > > > > > > Sorry, by "everything" I meant "everything that's even been allowed by the > > > ISA manual". Just accetping anything would be bad ;) > > > > > > > > IMO it's time to just stop using the ISA string. It's not a stable > > > > > interface, trying to treat it as such just leads to headaches. We should > > > > > just come up with some DT-specific way of encoding whatever HW features are > > > > > in question. Sure it'll be a bit of work to write that all down in the DT > > > > > bindings, but it's going to be way less work than trying to keep around all > > > > > this ISA string parsing code. > > > > > > > > I'm a glutton for punishment, I'll try and come up with some sort of > > > > other way to encode this information in DT that requires less parsing > > > > and validation. As I said on IRC, something that more resembles: > > > > if (of_property_wahtever("riscv,isa-foo")) { do_enable_foo() } > > > > > > That seems way simpler to me, thanks! We'll still need to support whatever > > > was here as a legacy format, but at least we won't need to add a bunch of > > > new stuff to it -- that's where the parsing starts to get really > > > complicated. While it's easy for Linux to add new DT nodes when new extension support is added, it's not so easy to add new ACPI objects. The current plan for ACPI is to use the ISA string[1]. With that in mind, I think Linux should continue to parse the string for DT as well. [1] https://docs.google.com/document/d/1LlCefO_0GQ_7Tf3lzfMPETEfMlGo2FfLRJ09IMqJKEk/edit > > > > Yah, and "deprecated" in dt-schema doesn't actually do anything at the > > moment other than let humans know not to use something. Just gonna have > > to do some sort of "feature-wise AND" between the existing things we > > parse from the isa string & whatever riscv,isa-foo stuff later on. > > I suppose this is more of a Rob question, but could we just make the DT > bindings match the current ISA manual's rules and then have the kernel's > "riscv,isa" string parser accept more orderings to remain compatible? > > Sort of a API vs ABI stability question, but for DTB and bindngs. > > > > FWIW, there's a similar dicussion going on in GCC land right now. > > > > > > > > I know I've said the opposite before, but there's just been way too many > > > > > breakages here to assume they're going to stop. > > > > > > > > :upside_down_face: > > > > > > > > Either way, I think these two patches are worth taking in the mean time. > > > > > > Yep, just as long as it doesn't break any of the strings that were valid > > > according to previous versions of the ISA manual I'm fine with it. > > > > I don't think so. I had been looking around for a supposed order for > > where to actually put H, which had been dropped - and the only place I > > recall seeing that was Wikipedia - which now seems like an awful > > decision since the order there looks kinda off anything I see in dozen > > or so spec PDFs I have downloaded. But that's where I got the K & V > > ordering from that I now think is wrong (and doesn't match any PDF I > > have). The other changes relax rules and add letters so they should be > > okay too. > > Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > > as it sounds like there aren't even any fixed rules, so I guess none of this > even matters? IMHO, we should try to get the spec changed to say that the only order which matters is that single letter extensions come first (in any order) and then multi-letter extensions come after (in any order). Parsing becomes simple and we can publish the string in proc in any order too, maybe preferring alphabetical order for neatness. Thanks, drew > > > > > > > > > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > > > > > > > > > > > > > --- > > > > > > > Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml > > > > > > > index e80c967a4fa4..b7462ea2dbe4 100644 > > > > > > > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml > > > > > > > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml > > > > > > > @@ -80,7 +80,7 @@ properties: > > > > > > > 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?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ > > > > > > > + pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ > > > > > > > > > > > > > > # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here > > > > > > > timebase-frequency: false > > > > > > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-29 5:19 ` Andrew Jones @ 2022-11-29 11:40 ` Conor Dooley 2022-11-29 14:47 ` [RFC 0/2] Putting some basic order on isa extension stuff Conor Dooley ` (3 more replies) 0 siblings, 4 replies; 31+ messages in thread From: Conor Dooley @ 2022-11-29 11:40 UTC (permalink / raw) To: Andrew Jones, palmer Cc: devicetree, aou, heiko, linux-kernel, Conor Dooley, robh+dt, Palmer Dabbelt, krzysztof.kozlowski+dt, Paul Walmsley, guoren, linux-riscv Yo Drew, Palmer, (preserving the conversation in case any DT folks want to chime in on the ABI comment) On 29/11/2022 05:19, Andrew Jones wrote: > On Mon, Nov 28, 2022 at 03:41:19PM -0800, Palmer Dabbelt wrote: >> On Mon, 28 Nov 2022 11:17:21 PST (-0800), Conor Dooley wrote: >>> On Mon, Nov 28, 2022 at 10:12:17AM -0800, Palmer Dabbelt wrote: >>>> On Mon, 28 Nov 2022 10:08:05 PST (-0800), Conor Dooley wrote: >>>>> On Mon, Nov 28, 2022 at 09:41:03AM -0800, Palmer Dabbelt wrote: >>>>>> On Thu, 24 Nov 2022 05:42:20 PST (-0800), heiko@sntech.de wrote: >>>>>>> Am Donnerstag, 24. November 2022, 14:04:41 CET schrieb Conor Dooley: >>>>>>>> I used the wikipedia table for ordering extensions when updating the >>>>>>>> pattern here in foo. >>>>>>> >>>>>>> ^ foo? :-) >>>>>>> >>>>>>>> Unfortunately that table did not match canonical order, as defined by >>>>>>>> the RISC-V ISA Manual, which defines extension ordering in (what is >>>>>>>> currently) Table 41, "Standard ISA extension names". Fix things up by >>>>>>>> re-sorting v (vector) and adding p (packed-simd) & j (dynamic >>>>>>>> languages). The e (reduced integer) and g (general) extensions are still >>>>>>>> intentionally left out. >>>>>>>> >>>>>>>> Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 >>>>>>>> Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") >>>>>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >>>>>>> >>>>>>> So I have compared the new pattern to the isa manual, >>>>>>> and it looks like the order checks out, so >>>>>> >>>>>> Which ISA manual? >>>>> >>>>> For me, isa manual is the above github repo. >>>> >>>> Which commit, though? >>> >>> mutt won't let me paste a clown face emoticon. >>> >>>>>> There have been many mutually incompatible ISA string >>>>>> encoding rules, at least one of them was a change to the extension ordering. >>>>>> It's not entirely clear what the right answer is here, as we can't really >>>>>> parse ISA strings without also knowing the version of the ISA manual we're >>>>>> meant to parse them against. Maybe we just accept everything? >>>>> >>>>> I don't think accepting everything is the right thing to do. A minimal >>>>> amount of validation is still needed here, but I think we can deprecate >>>>> the DT property entirely & make it optional if a new-and-improved way of >>>>> encoding the in DT is used. >>>> >>>> Sorry, by "everything" I meant "everything that's even been allowed by the >>>> ISA manual". Just accetping anything would be bad ;) I agree. I can always relax the regex if something comes to light - but it's already pretty forgiving about the ordering of stuff - especially the multiletter extensions. >>>> >>>>>> IMO it's time to just stop using the ISA string. It's not a stable >>>>>> interface, trying to treat it as such just leads to headaches. We should >>>>>> just come up with some DT-specific way of encoding whatever HW features are >>>>>> in question. Sure it'll be a bit of work to write that all down in the DT >>>>>> bindings, but it's going to be way less work than trying to keep around all >>>>>> this ISA string parsing code. >>>>> >>>>> I'm a glutton for punishment, I'll try and come up with some sort of >>>>> other way to encode this information in DT that requires less parsing >>>>> and validation. As I said on IRC, something that more resembles: >>>>> if (of_property_wahtever("riscv,isa-foo")) { do_enable_foo() } >>>> >>>> That seems way simpler to me, thanks! We'll still need to support whatever >>>> was here as a legacy format, but at least we won't need to add a bunch of >>>> new stuff to it -- that's where the parsing starts to get really >>>> complicated. > > While it's easy for Linux to add new DT nodes when new extension support is > added, it's not so easy to add new ACPI objects. The current plan for ACPI > is to use the ISA string[1]. With that in mind, I think Linux should > continue to parse the string for DT as well. I was thinking about how to implement this at the gym this morning, before even reading this mail, and I have gotten cold feet about doing riscv,isa-foo stuff anyway. I don't really want to create a confusing, two-location way of turning features on. We would be either forcing all other operating systems to switch away from using riscv,isa too or creating a difference between how Linux handles things compared to, say, FreeBSD or U-Boot. IOW, if the "v" in riscv,isa turned it on for FreeBSD and a boolean DT property of riscv,isa-v did it for Linux. I really dislike the idea of that... If we are going to have to parse it *anyway* for ACPI, then I think there's little merit unfortunately. I think we would also end up ballooning the code with a bunch of checks for the different extensions which I am not keen on either. Completely relaxing any parsing rules, as discussed below, seems like a less user-hostile & more "application" independent way of proceeding. > [1] https://docs.google.com/document/d/1LlCefO_0GQ_7Tf3lzfMPETEfMlGo2FfLRJ09IMqJKEk/edit > >>> >>> Yah, and "deprecated" in dt-schema doesn't actually do anything at the >>> moment other than let humans know not to use something. Just gonna have >>> to do some sort of "feature-wise AND" between the existing things we That should be s/AND/OR/ >>> parse from the isa string & whatever riscv,isa-foo stuff later on. >> >> I suppose this is more of a Rob question, but could we just make the DT >> bindings match the current ISA manual's rules and then have the kernel's >> "riscv,isa" string parser accept more orderings to remain compatible? >> >> Sort of a API vs ABI stability question, but for DTB and bindngs. AFAIU, the kernel needs to keep supporting only what was ever in a dt-binding to satisfy the DT ABI and our current regex is compatible with that so that's fine. Whether or not we have laxer rules within the kernel is fine, as long as we support whatever we specified previously in the binding. >>>> FWIW, there's a similar dicussion going on in GCC land right now. >>>> >>>>>> I know I've said the opposite before, but there's just been way too many >>>>>> breakages here to assume they're going to stop. >>>>> >>>>> :upside_down_face: >>>>> >>>>> Either way, I think these two patches are worth taking in the mean time. >>>> >>>> Yep, just as long as it doesn't break any of the strings that were valid >>>> according to previous versions of the ISA manual I'm fine with it. >>> >>> I don't think so. I had been looking around for a supposed order for >>> where to actually put H, which had been dropped - and the only place I >>> recall seeing that was Wikipedia - which now seems like an awful >>> decision since the order there looks kinda off anything I see in dozen >>> or so spec PDFs I have downloaded. But that's where I got the K & V >>> ordering from that I now think is wrong (and doesn't match any PDF I >>> have). The other changes relax rules and add letters so they should be >>> okay too. >> >> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> I am assuming that w/ these tags I can take this with other DT stuff, since they're kinda unrelated to the ongoing conversation. >> as it sounds like there aren't even any fixed rules, so I guess none of this >> even matters? > > IMHO, we should try to get the spec changed to say that the only order > which matters is that single letter extensions come first (in any order) Good luck, I'll +1 your PR. > and then multi-letter extensions come after (in any order). Parsing > becomes simple and we can publish the string in proc in any order too, > maybe preferring alphabetical order for neatness. Devil's advocate, then we break someone's parser that relied expects things to be how they were ordered by a prior version of the spec? Either way, I am happy to vote for a world in which the stuff we put into /proc follows the loosest possible wording that has ever been using in the spec (and in fact, I think we *need* to do that). Per the thread on the Zbb stuff, the table claims it "defines the canonical order in which extension names must appear in the name string, with top-to-bottom in table indicating first-to-last in the name string, e.g., RV32IMACV is legal, whereas RV32IMAVC is not." It does not contain an entry for Zihintpause & I cannot see anywhere in the current incarnation* of the naming.tex document that tells us how to sort unmentioned Standard Additional Extensions - just that "Standard supervisor-level extensions should be listed after standard unprivileged extensions" & "Standard machine-level extensions should be listed after standard lesser-privileged extensions" (as an aside, I think that second quote is missing a comma after standard) The implication there, is that something like Zihintpause could, per this version of the spec*, be placed anywhere in the string once the "Base Integer ISA" constraint is satisfied (or not permitted in the string at all?). Have I missed something? This particular situation has been the cause of all of my confusion lately - the doc says it defines the order by the table, which then has no generic entry for "Additional Standard Extensions". * 10eea63205f371ed649355f4cf7a80716335958f Anyways, I feel hopelessly confused by all of this stuff & I wish we could just write down exactly what *we* are doing w.r.t. internal kernel representation (yeah yeah https://xkcd.com/927/) as absolutely noone seems to be sure what to do. Maybe in the "patch acceptance" document. In terms of our own parsing of the isa string, assuming single-letter followed by multiletter is about the only assumption that I think we should be making. I don't think that parsing in any order after that really becomes an issue unless we have x depends on y sort of things where x is not explicitly required. The specs currently say: "Some ISA extensions depend on the presence of other extensions, e.g., ``D'' depends on ``F'' and ``F'' depends on ``Zicsr''. These dependences may be implicit in the ISA name: for example, RV32IF is equivalent to RV32IFZicsr, and RV32ID is equivalent to RV32IFD and RV32IFDZicsr." That'd mean that out-of-canonical-order processing isn't too bad since the more expansive extension would imply the others too. Who knows if that will hold for future extensions, but maybe we can deal with things down the road. I'll take a look at the current parser to at least get things straightened out in my own head and then maybe submit a patch for the docs as mentioned above so we can at least point patch submitters at something concrete? I am really sorry for all of the "chaos" that I've sown over the last few days on this sort of subject, but everyone is doing different things and it triggers my OCD heh. Knowing where we stand would be nice :) Thanks, Conor. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC 0/2] Putting some basic order on isa extension stuff 2022-11-29 11:40 ` Conor Dooley @ 2022-11-29 14:47 ` Conor Dooley 2022-11-29 15:48 ` Andrew Jones 2022-11-29 14:47 ` [RFC 1/2] RISC-V: clarify ISA string ordering rules in cpu.c Conor Dooley ` (2 subsequent siblings) 3 siblings, 1 reply; 31+ messages in thread From: Conor Dooley @ 2022-11-29 14:47 UTC (permalink / raw) To: linux-riscv Cc: Conor Dooley, ajones, aou, conor, devicetree, guoren, heiko, krzysztof.kozlowski+dt, linux-kernel, palmer, paul.walmsley, robh+dt RFC: - I have not even tested this, I just did an allmodconfig - I don't know if I re-ordered something that is sacrosanct - I don't know if I changed all of the instances - I didn't write a proper commit message for "patch" 2/2 With those caveats out of the way - all I did here was try to make things consistent so that it'd be easier to point patch submitters at a "do this order please". I never know which of these can be moved without breaking stuff - but they all seem to be internal use stuff since they're not in uapi? @drew, I didn't touch the KVM ones - are they re-sortable too? My base here is rc7 so if you did a reorder at any point there I'd not see it ;) CC: conor.dooley@microchip.com CC: ajones@ventanamicro.com CC: aou@eecs.berkeley.edu CC: conor@kernel.org CC: devicetree@vger.kernel.org CC: guoren@kernel.org CC: heiko@sntech.de CC: krzysztof.kozlowski+dt@linaro.org CC: linux-kernel@vger.kernel.org CC: linux-riscv@lists.infradead.org CC: palmer@dabbelt.com CC: paul.walmsley@sifive.com CC: robh+dt@kernel.org Conor Dooley (2): RISC-V: clarify ISA string ordering rules in cpu.c RISC-V: resort all extensions in "canonical" order arch/riscv/include/asm/hwcap.h | 6 +++--- arch/riscv/kernel/cpu.c | 26 +++++++++++++++++++------- arch/riscv/kernel/cpufeature.c | 4 ++-- 3 files changed, 24 insertions(+), 12 deletions(-) -- 2.38.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 0/2] Putting some basic order on isa extension stuff 2022-11-29 14:47 ` [RFC 0/2] Putting some basic order on isa extension stuff Conor Dooley @ 2022-11-29 15:48 ` Andrew Jones 2022-11-29 16:50 ` Conor Dooley 0 siblings, 1 reply; 31+ messages in thread From: Andrew Jones @ 2022-11-29 15:48 UTC (permalink / raw) To: Conor Dooley Cc: linux-riscv, aou, conor, devicetree, guoren, heiko, krzysztof.kozlowski+dt, linux-kernel, palmer, paul.walmsley, robh+dt On Tue, Nov 29, 2022 at 02:47:41PM +0000, Conor Dooley wrote: > RFC: > - I have not even tested this, I just did an allmodconfig > - I don't know if I re-ordered something that is sacrosanct > - I don't know if I changed all of the instances > - I didn't write a proper commit message for "patch" 2/2 > > With those caveats out of the way - all I did here was try to make > things consistent so that it'd be easier to point patch submitters at a > "do this order please". > > I never know which of these can be moved without breaking stuff - but > they all seem to be internal use stuff since they're not in uapi? > > @drew, I didn't touch the KVM ones - are they re-sortable too? My base > here is rc7 so if you did a reorder at any point there I'd not see it ;) Right, we can't touch enum KVM_RISCV_ISA_EXT_ID as that's UAPI. All new extensions must be added at the bottom. We originally also had to keep kvm_isa_ext_arr[] in that order, but commit 1b5cbb8733f9 ("RISC-V: KVM: Make ISA ext mappings explicit") allows us to list its elements in any order, which means we could sort them in canonical order, if we wanted to. I think I'd rather have them in alphabetical order, though (they nearly are at the moment, except for the bottom two...) The only other place we have ISA extensions listed in KVM is in a switch statement, which of course doesn't matter, and it's currently in alphabetical order. Thanks, drew _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 0/2] Putting some basic order on isa extension stuff 2022-11-29 15:48 ` Andrew Jones @ 2022-11-29 16:50 ` Conor Dooley 0 siblings, 0 replies; 31+ messages in thread From: Conor Dooley @ 2022-11-29 16:50 UTC (permalink / raw) To: Andrew Jones Cc: Conor Dooley, linux-riscv, aou, devicetree, guoren, heiko, krzysztof.kozlowski+dt, linux-kernel, palmer, paul.walmsley, robh+dt On Tue, Nov 29, 2022 at 04:48:32PM +0100, Andrew Jones wrote: > On Tue, Nov 29, 2022 at 02:47:41PM +0000, Conor Dooley wrote: > > RFC: > > - I have not even tested this, I just did an allmodconfig > > - I don't know if I re-ordered something that is sacrosanct > > - I don't know if I changed all of the instances > > - I didn't write a proper commit message for "patch" 2/2 > > > > With those caveats out of the way - all I did here was try to make > > things consistent so that it'd be easier to point patch submitters at a > > "do this order please". > > > > I never know which of these can be moved without breaking stuff - but > > they all seem to be internal use stuff since they're not in uapi? > > > > @drew, I didn't touch the KVM ones - are they re-sortable too? My base > > here is rc7 so if you did a reorder at any point there I'd not see it ;) > > Right, we can't touch enum KVM_RISCV_ISA_EXT_ID as that's UAPI. All new > extensions must be added at the bottom. We originally also had to keep > kvm_isa_ext_arr[] in that order, but commit 1b5cbb8733f9 ("RISC-V: KVM: Right, I knew that something had been changed in KVM land. It's probably a good idea to say sort them all alphabetically apart from whichever ones must be in other orders & explicitly note the reasons in-place. > Make ISA ext mappings explicit") allows us to list its elements in any > order, which means we could sort them in canonical order, if we wanted > to. I think I'd rather have them in alphabetical order, though (they > nearly are at the moment, except for the bottom two...) The only other > place we have ISA extensions listed in KVM is in a switch statement, > which of course doesn't matter, and it's currently in alphabetical order. I did see the one in uAPI for KVM. Your idea in 2/2 of doing alphabetical unless otherwise stated works for me as I just want something concrete! If it works for the chief too, I'll resubmit and drop the RFC... _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC 1/2] RISC-V: clarify ISA string ordering rules in cpu.c 2022-11-29 11:40 ` Conor Dooley 2022-11-29 14:47 ` [RFC 0/2] Putting some basic order on isa extension stuff Conor Dooley @ 2022-11-29 14:47 ` Conor Dooley 2022-11-29 16:12 ` Andrew Jones 2022-11-29 14:47 ` [RFC 2/2] RISC-V: resort all extensions in "canonical" order Conor Dooley 2022-12-01 13:51 ` [PATCH] Documentation: riscv: note that counter access is part of the uABI Conor Dooley 3 siblings, 1 reply; 31+ messages in thread From: Conor Dooley @ 2022-11-29 14:47 UTC (permalink / raw) To: linux-riscv Cc: Conor Dooley, ajones, aou, conor, devicetree, guoren, heiko, krzysztof.kozlowski+dt, linux-kernel, palmer, paul.walmsley, robh+dt While the list of rules may have been accurate when created, it now lacks some clarity in the face of isa-manual updates. Specifically: - there is no mention here of a distinction between regular 'Z' extensions which are "Additional Standard Extensions" and "Zxm" extensions which are "Standard Machine-Level Extensions" - there is also no explicit mention of where either should be sorted in the list - underscores are only required between two *multi-letter* extensions but the list of rules implies that this is required between a multi-letter extension and any other extension. IOW "rv64imafdzicsr_zifencei" is a valid string Attempt to clean up the list of rules, by adding information on the above & sprinkling in some white space for readability. Signed-off-by: Conor Dooley <conor.dooley@microchip.com> --- arch/riscv/kernel/cpu.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index 852ecccd8920..5e42c92a8456 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -120,20 +120,32 @@ device_initcall(riscv_cpuinfo_init); .uprop = #UPROP, \ .isa_ext_id = EXTID, \ } + /* * Here are the ordering rules of extension naming defined by RISC-V * specification : - * 1. All extensions should be separated from other multi-letter extensions - * by an underscore. + * + * 1. All multi-letter extensions should be separated from other multi-letter + * extensions by an underscore. + * * 2. The first letter following the 'Z' conventionally indicates the most * closely related alphabetical extension category, IMAFDQLCBKJTPVH. - * If multiple 'Z' extensions are named, they should be ordered first - * by category, then alphabetically within a category. + * 'Z' extensions should be sorted after single-letter extensions and before + * any higher-privileged extensions. + * If multiple 'Z' extensions are named, they should be ordered first by + * category, then alphabetically within a category. + * * 3. Standard supervisor-level extensions (starts with 'S') should be * listed after standard unprivileged extensions. If multiple * supervisor-level extensions are listed, they should be ordered * alphabetically. - * 4. Non-standard extensions (starts with 'X') must be listed after all + * + * 4 Standard machine-level extensions (starts with 'Zxm') should be + * listed after any lower-privileged, standard extensions. If multiple + * machine-level extensions are listed, they should be ordered + * alphabetically. + * + * 5. Non-standard extensions (starts with 'X') must be listed after all * standard extensions. They must be separated from other multi-letter * extensions by an underscore. */ -- 2.38.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [RFC 1/2] RISC-V: clarify ISA string ordering rules in cpu.c 2022-11-29 14:47 ` [RFC 1/2] RISC-V: clarify ISA string ordering rules in cpu.c Conor Dooley @ 2022-11-29 16:12 ` Andrew Jones 2022-11-29 16:54 ` Conor Dooley 0 siblings, 1 reply; 31+ messages in thread From: Andrew Jones @ 2022-11-29 16:12 UTC (permalink / raw) To: Conor Dooley Cc: linux-riscv, aou, conor, devicetree, guoren, heiko, krzysztof.kozlowski+dt, linux-kernel, palmer, paul.walmsley, robh+dt On Tue, Nov 29, 2022 at 02:47:42PM +0000, Conor Dooley wrote: > While the list of rules may have been accurate when created, it now > lacks some clarity in the face of isa-manual updates. Specifically: > > - there is no mention here of a distinction between regular 'Z' > extensions which are "Additional Standard Extensions" and "Zxm" > extensions which are "Standard Machine-Level Extensions" > > - there is also no explicit mention of where either should be sorted in > the list > > - underscores are only required between two *multi-letter* extensions but > the list of rules implies that this is required between a multi-letter > extension and any other extension. IOW "rv64imafdzicsr_zifencei" is a > valid string > > Attempt to clean up the list of rules, by adding information on the > above & sprinkling in some white space for readability. > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > arch/riscv/kernel/cpu.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > index 852ecccd8920..5e42c92a8456 100644 > --- a/arch/riscv/kernel/cpu.c > +++ b/arch/riscv/kernel/cpu.c > @@ -120,20 +120,32 @@ device_initcall(riscv_cpuinfo_init); > .uprop = #UPROP, \ > .isa_ext_id = EXTID, \ > } > + > /* > * Here are the ordering rules of extension naming defined by RISC-V > * specification : > - * 1. All extensions should be separated from other multi-letter extensions > - * by an underscore. > + * > + * 1. All multi-letter extensions should be separated from other multi-letter > + * extensions by an underscore. > + * > * 2. The first letter following the 'Z' conventionally indicates the most > * closely related alphabetical extension category, IMAFDQLCBKJTPVH. > - * If multiple 'Z' extensions are named, they should be ordered first > - * by category, then alphabetically within a category. > + * 'Z' extensions should be sorted after single-letter extensions and before > + * any higher-privileged extensions. > + * If multiple 'Z' extensions are named, they should be ordered first by > + * category, then alphabetically within a category. > + * > * 3. Standard supervisor-level extensions (starts with 'S') should be > * listed after standard unprivileged extensions. If multiple > * supervisor-level extensions are listed, they should be ordered > * alphabetically. > - * 4. Non-standard extensions (starts with 'X') must be listed after all > + * > + * 4 Standard machine-level extensions (starts with 'Zxm') should be > + * listed after any lower-privileged, standard extensions. If multiple > + * machine-level extensions are listed, they should be ordered > + * alphabetically. > + * > + * 5. Non-standard extensions (starts with 'X') must be listed after all > * standard extensions. They must be separated from other multi-letter > * extensions by an underscore. > */ > -- > 2.38.1 > Alternatively, we could change the comment to just point out the spec chapter and provide an example, e.g. /* * The canonical order of ISA extension names in the ISA string is defined in * chapter 27 of the unprivileged spec. An example string following the * order is * * rv64imadc_zifoo_zigoo_zafoo_sbar_scar_zxmbaz_xqux_xrux * * Notice how Z-extensions are first sorted by category using the canonical * order of the first letter following the Z. Extension groups are in the * order specified in chapter 27. Extensions within each group are sorted * alphabetically. */ Thanks, drew _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 1/2] RISC-V: clarify ISA string ordering rules in cpu.c 2022-11-29 16:12 ` Andrew Jones @ 2022-11-29 16:54 ` Conor Dooley 2022-11-29 17:19 ` Andrew Jones 0 siblings, 1 reply; 31+ messages in thread From: Conor Dooley @ 2022-11-29 16:54 UTC (permalink / raw) To: Andrew Jones Cc: Conor Dooley, linux-riscv, aou, devicetree, guoren, heiko, krzysztof.kozlowski+dt, linux-kernel, palmer, paul.walmsley, robh+dt On Tue, Nov 29, 2022 at 05:12:23PM +0100, Andrew Jones wrote: > On Tue, Nov 29, 2022 at 02:47:42PM +0000, Conor Dooley wrote: > > While the list of rules may have been accurate when created, it now > > lacks some clarity in the face of isa-manual updates. Specifically: > > > > - there is no mention here of a distinction between regular 'Z' > > extensions which are "Additional Standard Extensions" and "Zxm" > > extensions which are "Standard Machine-Level Extensions" > > > > - there is also no explicit mention of where either should be sorted in > > the list > > > > - underscores are only required between two *multi-letter* extensions but > > the list of rules implies that this is required between a multi-letter > > extension and any other extension. IOW "rv64imafdzicsr_zifencei" is a > > valid string > > > > Attempt to clean up the list of rules, by adding information on the > > above & sprinkling in some white space for readability. > > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > --- > > arch/riscv/kernel/cpu.c | 22 +++++++++++++++++----- > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > > index 852ecccd8920..5e42c92a8456 100644 > > --- a/arch/riscv/kernel/cpu.c > > +++ b/arch/riscv/kernel/cpu.c > > @@ -120,20 +120,32 @@ device_initcall(riscv_cpuinfo_init); > > .uprop = #UPROP, \ > > .isa_ext_id = EXTID, \ > > } > > + > > /* > > * Here are the ordering rules of extension naming defined by RISC-V > > * specification : > > - * 1. All extensions should be separated from other multi-letter extensions > > - * by an underscore. > > + * > > + * 1. All multi-letter extensions should be separated from other multi-letter > > + * extensions by an underscore. > > + * > > * 2. The first letter following the 'Z' conventionally indicates the most > > * closely related alphabetical extension category, IMAFDQLCBKJTPVH. > > - * If multiple 'Z' extensions are named, they should be ordered first > > - * by category, then alphabetically within a category. > > + * 'Z' extensions should be sorted after single-letter extensions and before > > + * any higher-privileged extensions. > > + * If multiple 'Z' extensions are named, they should be ordered first by > > + * category, then alphabetically within a category. > > + * > > * 3. Standard supervisor-level extensions (starts with 'S') should be > > * listed after standard unprivileged extensions. If multiple > > * supervisor-level extensions are listed, they should be ordered > > * alphabetically. > > - * 4. Non-standard extensions (starts with 'X') must be listed after all > > + * > > + * 4 Standard machine-level extensions (starts with 'Zxm') should be > > + * listed after any lower-privileged, standard extensions. If multiple > > + * machine-level extensions are listed, they should be ordered > > + * alphabetically. > > + * > > + * 5. Non-standard extensions (starts with 'X') must be listed after all > > * standard extensions. They must be separated from other multi-letter > > * extensions by an underscore. > > */ > > -- > > 2.38.1 > > > > Alternatively, we could change the comment to just point out the spec > chapter and provide an example, e.g. IDK, maybe add the reference & the example but keep the summary? > /* > * The canonical order of ISA extension names in the ISA string is defined in > * chapter 27 of the unprivileged spec. An example string following the > * order is > * > * rv64imadc_zifoo_zigoo_zafoo_sbar_scar_zxmbaz_xqux_xrux > * > * Notice how Z-extensions are first sorted by category using the canonical > * order of the first letter following the Z. Extension groups are in the > * order specified in chapter 27. Extensions within each group are sorted > * alphabetically. > */ > > > Thanks, > drew _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 1/2] RISC-V: clarify ISA string ordering rules in cpu.c 2022-11-29 16:54 ` Conor Dooley @ 2022-11-29 17:19 ` Andrew Jones 2022-11-29 17:48 ` Conor Dooley 0 siblings, 1 reply; 31+ messages in thread From: Andrew Jones @ 2022-11-29 17:19 UTC (permalink / raw) To: Conor Dooley Cc: Conor Dooley, linux-riscv, aou, devicetree, guoren, heiko, krzysztof.kozlowski+dt, linux-kernel, palmer, paul.walmsley, robh+dt On Tue, Nov 29, 2022 at 04:54:19PM +0000, Conor Dooley wrote: > On Tue, Nov 29, 2022 at 05:12:23PM +0100, Andrew Jones wrote: > > On Tue, Nov 29, 2022 at 02:47:42PM +0000, Conor Dooley wrote: > > > While the list of rules may have been accurate when created, it now > > > lacks some clarity in the face of isa-manual updates. Specifically: > > > > > > - there is no mention here of a distinction between regular 'Z' > > > extensions which are "Additional Standard Extensions" and "Zxm" > > > extensions which are "Standard Machine-Level Extensions" > > > > > > - there is also no explicit mention of where either should be sorted in > > > the list > > > > > > - underscores are only required between two *multi-letter* extensions but > > > the list of rules implies that this is required between a multi-letter > > > extension and any other extension. IOW "rv64imafdzicsr_zifencei" is a > > > valid string > > > > > > Attempt to clean up the list of rules, by adding information on the > > > above & sprinkling in some white space for readability. > > > > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > > --- > > > arch/riscv/kernel/cpu.c | 22 +++++++++++++++++----- > > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > > > index 852ecccd8920..5e42c92a8456 100644 > > > --- a/arch/riscv/kernel/cpu.c > > > +++ b/arch/riscv/kernel/cpu.c > > > @@ -120,20 +120,32 @@ device_initcall(riscv_cpuinfo_init); > > > .uprop = #UPROP, \ > > > .isa_ext_id = EXTID, \ > > > } > > > + > > > /* > > > * Here are the ordering rules of extension naming defined by RISC-V > > > * specification : > > > - * 1. All extensions should be separated from other multi-letter extensions > > > - * by an underscore. > > > + * > > > + * 1. All multi-letter extensions should be separated from other multi-letter > > > + * extensions by an underscore. > > > + * > > > * 2. The first letter following the 'Z' conventionally indicates the most > > > * closely related alphabetical extension category, IMAFDQLCBKJTPVH. > > > - * If multiple 'Z' extensions are named, they should be ordered first > > > - * by category, then alphabetically within a category. > > > + * 'Z' extensions should be sorted after single-letter extensions and before > > > + * any higher-privileged extensions. > > > + * If multiple 'Z' extensions are named, they should be ordered first by > > > + * category, then alphabetically within a category. > > > + * > > > * 3. Standard supervisor-level extensions (starts with 'S') should be > > > * listed after standard unprivileged extensions. If multiple > > > * supervisor-level extensions are listed, they should be ordered > > > * alphabetically. > > > - * 4. Non-standard extensions (starts with 'X') must be listed after all > > > + * > > > + * 4 Standard machine-level extensions (starts with 'Zxm') should be > > > + * listed after any lower-privileged, standard extensions. If multiple > > > + * machine-level extensions are listed, they should be ordered > > > + * alphabetically. > > > + * > > > + * 5. Non-standard extensions (starts with 'X') must be listed after all > > > * standard extensions. They must be separated from other multi-letter > > > * extensions by an underscore. > > > */ > > > -- > > > 2.38.1 > > > > > > > Alternatively, we could change the comment to just point out the spec > > chapter and provide an example, e.g. > > IDK, maybe add the reference & the example but keep the summary? It risks needing to synchronize the comment with the spec. Also, the comment doesn't need to reproduce the flexible specifications, since Linux has a single implementation (it always puts an underscore between single-letter extensions and the first multi-letter extension, for example). So, rather than paraphrase too much of the spec, we could just point out Linux's specific implementation (with the help of an example). I don't feel that strongly about it though, so we can keep the text the spec paraphrasing too. Thanks, drew > > > /* > > * The canonical order of ISA extension names in the ISA string is defined in > > * chapter 27 of the unprivileged spec. An example string following the > > * order is > > * > > * rv64imadc_zifoo_zigoo_zafoo_sbar_scar_zxmbaz_xqux_xrux > > * > > * Notice how Z-extensions are first sorted by category using the canonical > > * order of the first letter following the Z. Extension groups are in the > > * order specified in chapter 27. Extensions within each group are sorted > > * alphabetically. > > */ > > > > > > Thanks, > > drew _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 1/2] RISC-V: clarify ISA string ordering rules in cpu.c 2022-11-29 17:19 ` Andrew Jones @ 2022-11-29 17:48 ` Conor Dooley 0 siblings, 0 replies; 31+ messages in thread From: Conor Dooley @ 2022-11-29 17:48 UTC (permalink / raw) To: Andrew Jones Cc: Conor Dooley, linux-riscv, aou, devicetree, guoren, heiko, krzysztof.kozlowski+dt, linux-kernel, palmer, paul.walmsley, robh+dt On Tue, Nov 29, 2022 at 06:19:51PM +0100, Andrew Jones wrote: > On Tue, Nov 29, 2022 at 04:54:19PM +0000, Conor Dooley wrote: > > On Tue, Nov 29, 2022 at 05:12:23PM +0100, Andrew Jones wrote: > > > On Tue, Nov 29, 2022 at 02:47:42PM +0000, Conor Dooley wrote: > > > > While the list of rules may have been accurate when created, it now > > > > lacks some clarity in the face of isa-manual updates. Specifically: > > > > > > > > - there is no mention here of a distinction between regular 'Z' > > > > extensions which are "Additional Standard Extensions" and "Zxm" > > > > extensions which are "Standard Machine-Level Extensions" > > > > > > > > - there is also no explicit mention of where either should be sorted in > > > > the list > > > > > > > > - underscores are only required between two *multi-letter* extensions but > > > > the list of rules implies that this is required between a multi-letter > > > > extension and any other extension. IOW "rv64imafdzicsr_zifencei" is a > > > > valid string > > > > > > > > Attempt to clean up the list of rules, by adding information on the > > > > above & sprinkling in some white space for readability. > > > > > > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > > > --- > > > > arch/riscv/kernel/cpu.c | 22 +++++++++++++++++----- > > > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > > > > index 852ecccd8920..5e42c92a8456 100644 > > > > --- a/arch/riscv/kernel/cpu.c > > > > +++ b/arch/riscv/kernel/cpu.c > > > > @@ -120,20 +120,32 @@ device_initcall(riscv_cpuinfo_init); > > > > .uprop = #UPROP, \ > > > > .isa_ext_id = EXTID, \ > > > > } > > > > + > > > > /* > > > > * Here are the ordering rules of extension naming defined by RISC-V > > > > * specification : > > > > - * 1. All extensions should be separated from other multi-letter extensions > > > > - * by an underscore. > > > > + * > > > > + * 1. All multi-letter extensions should be separated from other multi-letter > > > > + * extensions by an underscore. > > > > + * > > > > * 2. The first letter following the 'Z' conventionally indicates the most > > > > * closely related alphabetical extension category, IMAFDQLCBKJTPVH. > > > > - * If multiple 'Z' extensions are named, they should be ordered first > > > > - * by category, then alphabetically within a category. > > > > + * 'Z' extensions should be sorted after single-letter extensions and before > > > > + * any higher-privileged extensions. > > > > + * If multiple 'Z' extensions are named, they should be ordered first by > > > > + * category, then alphabetically within a category. > > > > + * > > > > * 3. Standard supervisor-level extensions (starts with 'S') should be > > > > * listed after standard unprivileged extensions. If multiple > > > > * supervisor-level extensions are listed, they should be ordered > > > > * alphabetically. > > > > - * 4. Non-standard extensions (starts with 'X') must be listed after all > > > > + * > > > > + * 4 Standard machine-level extensions (starts with 'Zxm') should be > > > > + * listed after any lower-privileged, standard extensions. If multiple > > > > + * machine-level extensions are listed, they should be ordered > > > > + * alphabetically. > > > > + * > > > > + * 5. Non-standard extensions (starts with 'X') must be listed after all > > > > * standard extensions. They must be separated from other multi-letter > > > > * extensions by an underscore. > > > > */ > > > > -- > > > > 2.38.1 > > > > > > > > > > Alternatively, we could change the comment to just point out the spec > > > chapter and provide an example, e.g. > > > > IDK, maybe add the reference & the example but keep the summary? > > It risks needing to synchronize the comment with the spec. Heh, see I almost see this as an *advantage* of this approach! > Also, the > comment doesn't need to reproduce the flexible specifications, since > Linux has a single implementation (it always puts an underscore between > single-letter extensions and the first multi-letter extension, for > example). So, rather than paraphrase too much of the spec, we could just > point out Linux's specific implementation (with the help of an example). You're right here though. I've had my head stuck in the incoming-string side of things rather than outgoing. I'll wait for the all clear from Above, but ideally I'll reword this explaining where this info shows up (and therefore why it needs to be ordered), what elements of the rules matter here (entirely ordering) and then include your bits from below? > I don't feel that strongly about it though, so we can keep the text > the spec paraphrasing too. > > > /* > > > * The canonical order of ISA extension names in the ISA string is defined in > > > * chapter 27 of the unprivileged spec. An example string following the > > > * order is > > > * > > > * rv64imadc_zifoo_zigoo_zafoo_sbar_scar_zxmbaz_xqux_xrux > > > * > > > * Notice how Z-extensions are first sorted by category using the canonical > > > * order of the first letter following the Z. Extension groups are in the > > > * order specified in chapter 27. Extensions within each group are sorted > > > * alphabetically. > > > */ _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC 2/2] RISC-V: resort all extensions in "canonical" order 2022-11-29 11:40 ` Conor Dooley 2022-11-29 14:47 ` [RFC 0/2] Putting some basic order on isa extension stuff Conor Dooley 2022-11-29 14:47 ` [RFC 1/2] RISC-V: clarify ISA string ordering rules in cpu.c Conor Dooley @ 2022-11-29 14:47 ` Conor Dooley 2022-11-29 16:35 ` Andrew Jones 2022-12-01 13:51 ` [PATCH] Documentation: riscv: note that counter access is part of the uABI Conor Dooley 3 siblings, 1 reply; 31+ messages in thread From: Conor Dooley @ 2022-11-29 14:47 UTC (permalink / raw) To: linux-riscv Cc: Conor Dooley, ajones, aou, conor, devicetree, guoren, heiko, krzysztof.kozlowski+dt, linux-kernel, palmer, paul.walmsley, robh+dt Per the comment in cpu.c, re-sort all lists/tables/enums/whatever in arch/riscv (apart from KVM) in the current edition of what the isa manual considers to be "canonical" order. None of this is in uapi, so we are free to re-order it? I'm never sure when it comes to hwcap... Signed-off-by: Conor Dooley <conor.dooley@microchip.com> --- arch/riscv/include/asm/hwcap.h | 6 +++--- arch/riscv/kernel/cpu.c | 4 ++-- arch/riscv/kernel/cpufeature.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index b22525290073..d7d5f27619ee 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -53,12 +53,12 @@ extern unsigned long elf_hwcap; * available logical extension id. */ enum riscv_isa_ext_id { - RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, - RISCV_ISA_EXT_SVPBMT, - RISCV_ISA_EXT_ZICBOM, + RISCV_ISA_EXT_ZICBOM = RISCV_ISA_EXT_BASE, RISCV_ISA_EXT_ZIHINTPAUSE, + RISCV_ISA_EXT_SSCOFPMF, RISCV_ISA_EXT_SSTC, RISCV_ISA_EXT_SVINVAL, + RISCV_ISA_EXT_SVPBMT, RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, }; diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index 5e42c92a8456..1d0fa0ebf6a8 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -150,12 +150,12 @@ device_initcall(riscv_cpuinfo_init); * extensions by an underscore. */ static struct riscv_isa_ext_data isa_ext_arr[] = { + __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), + __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC), __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), - __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), - __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX), }; diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 694267d1fe81..d3df72c4b94f 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -199,12 +199,12 @@ void __init riscv_fill_hwcap(void) this_hwcap |= isa2hwcap[(unsigned char)(*ext)]; set_bit(*ext - 'a', this_isa); } else { - SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); - SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM); SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); + SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC); SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL); + SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); } #undef SET_ISA_EXT_MAP } -- 2.38.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [RFC 2/2] RISC-V: resort all extensions in "canonical" order 2022-11-29 14:47 ` [RFC 2/2] RISC-V: resort all extensions in "canonical" order Conor Dooley @ 2022-11-29 16:35 ` Andrew Jones 0 siblings, 0 replies; 31+ messages in thread From: Andrew Jones @ 2022-11-29 16:35 UTC (permalink / raw) To: Conor Dooley Cc: linux-riscv, aou, conor, devicetree, guoren, heiko, krzysztof.kozlowski+dt, linux-kernel, palmer, paul.walmsley, robh+dt On Tue, Nov 29, 2022 at 02:47:43PM +0000, Conor Dooley wrote: > Per the comment in cpu.c, re-sort all lists/tables/enums/whatever in > arch/riscv (apart from KVM) in the current edition of what the isa > manual considers to be "canonical" order. > > None of this is in uapi, so we are free to re-order it? I'm never sure > when it comes to hwcap... > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > arch/riscv/include/asm/hwcap.h | 6 +++--- > arch/riscv/kernel/cpu.c | 4 ++-- > arch/riscv/kernel/cpufeature.c | 4 ++-- > 3 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h > index b22525290073..d7d5f27619ee 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -53,12 +53,12 @@ extern unsigned long elf_hwcap; > * available logical extension id. > */ > enum riscv_isa_ext_id { > - RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, > - RISCV_ISA_EXT_SVPBMT, > - RISCV_ISA_EXT_ZICBOM, > + RISCV_ISA_EXT_ZICBOM = RISCV_ISA_EXT_BASE, > RISCV_ISA_EXT_ZIHINTPAUSE, > + RISCV_ISA_EXT_SSCOFPMF, > RISCV_ISA_EXT_SSTC, > RISCV_ISA_EXT_SVINVAL, > + RISCV_ISA_EXT_SVPBMT, > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, Or, @@ -48,17 +48,16 @@ extern unsigned long elf_hwcap; /* * This enum represent the logical ID for each multi-letter RISC-V ISA extension. * The logical ID should start from RISCV_ISA_EXT_BASE and must not exceed - * RISCV_ISA_EXT_MAX. 0-25 range is reserved for single letter - * extensions while all the multi-letter extensions should define the next - * available logical extension id. + * RISCV_ISA_EXT_MAX. While the order doesn't matter, we keep it sorted + * alphabetically for neatness. */ enum riscv_isa_ext_id { RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE, + RISCV_ISA_EXT_SSTC, + RISCV_ISA_EXT_SVINVAL, RISCV_ISA_EXT_SVPBMT, RISCV_ISA_EXT_ZICBOM, RISCV_ISA_EXT_ZIHINTPAUSE, - RISCV_ISA_EXT_SSTC, - RISCV_ISA_EXT_SVINVAL, RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX, }; > }; > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > index 5e42c92a8456..1d0fa0ebf6a8 100644 > --- a/arch/riscv/kernel/cpu.c > +++ b/arch/riscv/kernel/cpu.c > @@ -150,12 +150,12 @@ device_initcall(riscv_cpuinfo_init); > * extensions by an underscore. > */ > static struct riscv_isa_ext_data isa_ext_arr[] = { > + __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), > + __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC), > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), > __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), > - __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), > - __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), > __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX), > }; Yes, this one should be put in canonical order (I guess). > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > index 694267d1fe81..d3df72c4b94f 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -199,12 +199,12 @@ void __init riscv_fill_hwcap(void) > this_hwcap |= isa2hwcap[(unsigned char)(*ext)]; > set_bit(*ext - 'a', this_isa); > } else { > - SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > - SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); > SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM); > SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); > + SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); > SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC); > SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL); > + SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); Or, @@ -199,12 +199,16 @@ void __init riscv_fill_hwcap(void) this_hwcap |= isa2hwcap[(unsigned char)(*ext)]; set_bit(*ext - 'a', this_isa); } else { + /* + * While the order doesn't matter here, we sort + * alphabetically for neatness. + */ SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); + SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC); + SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL); SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT); SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM); SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE); - SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC); - SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL); } #undef SET_ISA_EXT_MAP } > } > #undef SET_ISA_EXT_MAP > } > -- > 2.38.1 > Thanks, drew _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH] Documentation: riscv: note that counter access is part of the uABI 2022-11-29 11:40 ` Conor Dooley ` (2 preceding siblings ...) 2022-11-29 14:47 ` [RFC 2/2] RISC-V: resort all extensions in "canonical" order Conor Dooley @ 2022-12-01 13:51 ` Conor Dooley 2022-12-01 19:21 ` Palmer Dabbelt 3 siblings, 1 reply; 31+ messages in thread From: Conor Dooley @ 2022-12-01 13:51 UTC (permalink / raw) To: Jonathan Corbet, Palmer Dabbelt Cc: Conor Dooley, linux-doc, linux-riscv, linux-kernel Commit 5a5294fbe020 ("RISC-V: Re-enable counter access from userspace") fixed userspace access to CYCLE, TIME & INSTRET counters and left a nice comment in-place about why they must not be restricted. Since we now have a uABI doc in RISC-V land, add a section documenting it. Signed-off-by: Conor Dooley <conor.dooley@microchip.com> --- Based on an, as yet, unsent v2 of my other uABI changes. I don't expect it to be applicable, just getting a patch into patchwork while I don't forget about this. --- Documentation/riscv/uabi.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/riscv/uabi.rst b/Documentation/riscv/uabi.rst index 8d2651e42fda..638ddce56700 100644 --- a/Documentation/riscv/uabi.rst +++ b/Documentation/riscv/uabi.rst @@ -3,6 +3,13 @@ RISC-V Linux User ABI ===================== +Counter access +-------------- + +Access to the CYCLE, TIME and INSTRET counters, now controlled by the SBI PMU +extension, were part of the ISA when the uABI was frozen & so remain accessible +from userspace. + ISA string ordering in /proc/cpuinfo ------------------------------------ base-commit: 13ee7ef407cfcf63f4f047460ac5bb6ba5a3447d prerequisite-patch-id: d17a9ffb6fcf99eb683728da98cd50e18cd28fe8 prerequisite-patch-id: 0df4127e3f4a0c02a235fea00bcb69cd94fabb38 prerequisite-patch-id: 171724b870ba212b714ebbded480269accd83733 -- 2.38.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] Documentation: riscv: note that counter access is part of the uABI 2022-12-01 13:51 ` [PATCH] Documentation: riscv: note that counter access is part of the uABI Conor Dooley @ 2022-12-01 19:21 ` Palmer Dabbelt 2022-12-03 10:38 ` Jonathan Corbet 0 siblings, 1 reply; 31+ messages in thread From: Palmer Dabbelt @ 2022-12-01 19:21 UTC (permalink / raw) To: Conor Dooley; +Cc: corbet, Conor Dooley, linux-doc, linux-riscv, linux-kernel On Thu, 01 Dec 2022 05:51:10 PST (-0800), Conor Dooley wrote: > Commit 5a5294fbe020 ("RISC-V: Re-enable counter access from userspace") > fixed userspace access to CYCLE, TIME & INSTRET counters and left a nice > comment in-place about why they must not be restricted. Since we now > have a uABI doc in RISC-V land, add a section documenting it. > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > Based on an, as yet, unsent v2 of my other uABI changes. I don't expect > it to be applicable, just getting a patch into patchwork while I don't > forget about this. > --- > Documentation/riscv/uabi.rst | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/riscv/uabi.rst b/Documentation/riscv/uabi.rst > index 8d2651e42fda..638ddce56700 100644 > --- a/Documentation/riscv/uabi.rst > +++ b/Documentation/riscv/uabi.rst > @@ -3,6 +3,13 @@ > RISC-V Linux User ABI > ===================== > > +Counter access > +-------------- > + > +Access to the CYCLE, TIME and INSTRET counters, now controlled by the SBI PMU > +extension, were part of the ISA when the uABI was frozen & so remain accessible > +from userspace. > + > ISA string ordering in /proc/cpuinfo > ------------------------------------ > > > base-commit: 13ee7ef407cfcf63f4f047460ac5bb6ba5a3447d > prerequisite-patch-id: d17a9ffb6fcf99eb683728da98cd50e18cd28fe8 > prerequisite-patch-id: 0df4127e3f4a0c02a235fea00bcb69cd94fabb38 > prerequisite-patch-id: 171724b870ba212b714ebbded480269accd83733 Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> I think I merged the last one of these, but if the doc folks pick it up that's fine with me. Otherwise I'll take it when it comes back around, so folks have time to take a look. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Documentation: riscv: note that counter access is part of the uABI 2022-12-01 19:21 ` Palmer Dabbelt @ 2022-12-03 10:38 ` Jonathan Corbet 2022-12-03 10:45 ` Jonathan Corbet 0 siblings, 1 reply; 31+ messages in thread From: Jonathan Corbet @ 2022-12-03 10:38 UTC (permalink / raw) To: Palmer Dabbelt, Conor Dooley Cc: Conor Dooley, linux-doc, linux-riscv, linux-kernel Palmer Dabbelt <palmer@dabbelt.com> writes: > On Thu, 01 Dec 2022 05:51:10 PST (-0800), Conor Dooley wrote: >> Commit 5a5294fbe020 ("RISC-V: Re-enable counter access from userspace") >> fixed userspace access to CYCLE, TIME & INSTRET counters and left a nice >> comment in-place about why they must not be restricted. Since we now >> have a uABI doc in RISC-V land, add a section documenting it. >> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >> --- >> Based on an, as yet, unsent v2 of my other uABI changes. I don't expect >> it to be applicable, just getting a patch into patchwork while I don't >> forget about this. >> --- >> Documentation/riscv/uabi.rst | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/Documentation/riscv/uabi.rst b/Documentation/riscv/uabi.rst >> index 8d2651e42fda..638ddce56700 100644 >> --- a/Documentation/riscv/uabi.rst >> +++ b/Documentation/riscv/uabi.rst >> @@ -3,6 +3,13 @@ >> RISC-V Linux User ABI >> ===================== >> >> +Counter access >> +-------------- >> + >> +Access to the CYCLE, TIME and INSTRET counters, now controlled by the SBI PMU >> +extension, were part of the ISA when the uABI was frozen & so remain accessible >> +from userspace. >> + >> ISA string ordering in /proc/cpuinfo >> ------------------------------------ >> >> >> base-commit: 13ee7ef407cfcf63f4f047460ac5bb6ba5a3447d >> prerequisite-patch-id: d17a9ffb6fcf99eb683728da98cd50e18cd28fe8 >> prerequisite-patch-id: 0df4127e3f4a0c02a235fea00bcb69cd94fabb38 >> prerequisite-patch-id: 171724b870ba212b714ebbded480269accd83733 > > Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > > I think I merged the last one of these, but if the doc folks pick it up > that's fine with me. Otherwise I'll take it when it comes back around, > so folks have time to take a look. "Doc folks" applied it, thanks. :) jon _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Documentation: riscv: note that counter access is part of the uABI 2022-12-03 10:38 ` Jonathan Corbet @ 2022-12-03 10:45 ` Jonathan Corbet 2022-12-03 10:56 ` Conor Dooley 0 siblings, 1 reply; 31+ messages in thread From: Jonathan Corbet @ 2022-12-03 10:45 UTC (permalink / raw) To: Palmer Dabbelt, Conor Dooley Cc: Conor Dooley, linux-doc, linux-riscv, linux-kernel Jonathan Corbet <corbet@lwn.net> writes: > Palmer Dabbelt <palmer@dabbelt.com> writes: >> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> >> >> I think I merged the last one of these, but if the doc folks pick it up >> that's fine with me. Otherwise I'll take it when it comes back around, >> so folks have time to take a look. > > "Doc folks" applied it, thanks. :) Actually, I take that back. I'd missed this part from the patch: > Based on an, as yet, unsent v2 of my other uABI changes. I don't expect > it to be applicable, just getting a patch into patchwork while I don't > forget about this. ...but b4 happily picked up a couple of *other* patches from this thread and applied them instead; I've now undone that. Sorry for the noise. jon _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Documentation: riscv: note that counter access is part of the uABI 2022-12-03 10:45 ` Jonathan Corbet @ 2022-12-03 10:56 ` Conor Dooley 2023-01-09 21:36 ` Atish Patra 0 siblings, 1 reply; 31+ messages in thread From: Conor Dooley @ 2022-12-03 10:56 UTC (permalink / raw) To: Jonathan Corbet Cc: Palmer Dabbelt, Conor Dooley, linux-doc, linux-riscv, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 1238 bytes --] On Sat, Dec 03, 2022 at 03:45:35AM -0700, Jonathan Corbet wrote: > Jonathan Corbet <corbet@lwn.net> writes: > > > Palmer Dabbelt <palmer@dabbelt.com> writes: > >> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> > >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > >> > >> I think I merged the last one of these, but if the doc folks pick it up > >> that's fine with me. Otherwise I'll take it when it comes back around, > >> so folks have time to take a look. > > > > "Doc folks" applied it, thanks. :) > > Actually, I take that back. I'd missed this part from the patch: > > > Based on an, as yet, unsent v2 of my other uABI changes. I don't expect > > it to be applicable, just getting a patch into patchwork while I don't > > forget about this. > > ...but b4 happily picked up a couple of *other* patches from this thread > and applied them instead; I've now undone that. Sorry for the noise. Huh, I accidentally put an "in-reply-to" header on this patch. I have been updating some of my submission helper scripts & I must have left the field populated from sending another set by accident: https://lore.kernel.org/linux-riscv/20221129144742.2935581-1-conor.dooley@microchip.com/ Apologies! [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 161 bytes --] _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Documentation: riscv: note that counter access is part of the uABI 2022-12-03 10:56 ` Conor Dooley @ 2023-01-09 21:36 ` Atish Patra 2023-01-09 21:46 ` Conor Dooley 0 siblings, 1 reply; 31+ messages in thread From: Atish Patra @ 2023-01-09 21:36 UTC (permalink / raw) To: Conor Dooley Cc: Jonathan Corbet, Palmer Dabbelt, Conor Dooley, linux-doc, linux-riscv, linux-kernel On Sat, Dec 3, 2022 at 2:57 AM Conor Dooley <conor@kernel.org> wrote: > > On Sat, Dec 03, 2022 at 03:45:35AM -0700, Jonathan Corbet wrote: > > Jonathan Corbet <corbet@lwn.net> writes: > > > > > Palmer Dabbelt <palmer@dabbelt.com> writes: > > >> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> > > >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > > >> > > >> I think I merged the last one of these, but if the doc folks pick it up > > >> that's fine with me. Otherwise I'll take it when it comes back around, > > >> so folks have time to take a look. > > > > > > "Doc folks" applied it, thanks. :) > > > > Actually, I take that back. I'd missed this part from the patch: > > > > > Based on an, as yet, unsent v2 of my other uABI changes. I don't expect > > > it to be applicable, just getting a patch into patchwork while I don't > > > forget about this. > > > > ...but b4 happily picked up a couple of *other* patches from this thread > > and applied them instead; I've now undone that. Sorry for the noise. > > Huh, I accidentally put an "in-reply-to" header on this patch. I have > been updating some of my submission helper scripts & I must have left > the field populated from sending another set by accident: > https://lore.kernel.org/linux-riscv/20221129144742.2935581-1-conor.dooley@microchip.com/ > I don't see the patch upstream. Is this patch merged already ? If not, please hold on merging this for now. We had some discussions around this in the perf community. Here is the thread https://lore.kernel.org/lkml/Y7gN32eHJNyWBvVD@FVFF77S0Q05N/T/ TLDR; Even though x86 allows unrestricted access through rdpmc (not default), it still reads zero unless a privileged root user modifies the MSR interface exposed by the kernel. Quoting PeterZ "RDPMC is only useful if you read counters you own on yourself -- IOW selfmonitoring, using the interface outlined in uapi/linux/perf_events.h near struct perf_event_mmap_page. Any other usage -- you get to keep the pieces." "Anyway, given RISC-V being a very young platform, I would try really *really* *REALLY* hard to stomp on these applications and get them to change in order to reclaim the PMU usage." We need to decide what's the best approach for RISC-V. The current text in uABI will let users assume that cycle/instret can be read without any issues which is wrong. There are few options what we can do for RISC-V: 1. We can trap n emulate and report 0 always as suggested by Mark in that thread. 2. Continue to allow the user to read the counters directly but expects the garbage value depending on the other activities on the system. Hopefully, folks will fix their application by that time. Once we have the procfs interface, we enforce the behavior by breaking the application. In either case, the uABI needs to be updated accordingly. > Apologies! > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv -- Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Documentation: riscv: note that counter access is part of the uABI 2023-01-09 21:36 ` Atish Patra @ 2023-01-09 21:46 ` Conor Dooley 0 siblings, 0 replies; 31+ messages in thread From: Conor Dooley @ 2023-01-09 21:46 UTC (permalink / raw) To: Atish Patra Cc: Jonathan Corbet, Palmer Dabbelt, Conor Dooley, linux-doc, linux-riscv, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 3572 bytes --] On Mon, Jan 09, 2023 at 01:36:19PM -0800, Atish Patra wrote: > On Sat, Dec 3, 2022 at 2:57 AM Conor Dooley <conor@kernel.org> wrote: > > > > On Sat, Dec 03, 2022 at 03:45:35AM -0700, Jonathan Corbet wrote: > > > Jonathan Corbet <corbet@lwn.net> writes: > > > > > > > Palmer Dabbelt <palmer@dabbelt.com> writes: > > > >> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> > > > >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > > > >> > > > >> I think I merged the last one of these, but if the doc folks pick it up > > > >> that's fine with me. Otherwise I'll take it when it comes back around, > > > >> so folks have time to take a look. > > > > > > > > "Doc folks" applied it, thanks. :) > > > > > > Actually, I take that back. I'd missed this part from the patch: > > > > > > > Based on an, as yet, unsent v2 of my other uABI changes. I don't expect > > > > it to be applicable, just getting a patch into patchwork while I don't > > > > forget about this. > > > > > > ...but b4 happily picked up a couple of *other* patches from this thread > > > and applied them instead; I've now undone that. Sorry for the noise. > > > > Huh, I accidentally put an "in-reply-to" header on this patch. I have > > been updating some of my submission helper scripts & I must have left > > the field populated from sending another set by accident: > > https://lore.kernel.org/linux-riscv/20221129144742.2935581-1-conor.dooley@microchip.com/ > > > > I don't see the patch upstream. Is this patch merged already ? No it's not. I was going to resend it actually, but I noticed the thread yesterday or something. Not sure why you CC'ed half the world on it but not linux-riscv? Or me if you didn't want me resending it! > If not, please hold on merging this for now. We had some discussions > around this in the perf community. > Here is the thread > https://lore.kernel.org/lkml/Y7gN32eHJNyWBvVD@FVFF77S0Q05N/T/ > > TLDR; Even though x86 allows unrestricted access through rdpmc (not > default), it still reads zero unless a privileged root user modifies > the MSR interface exposed by the kernel. > > Quoting PeterZ > > "RDPMC is only useful if you read counters you own on yourself -- IOW > selfmonitoring, using the interface outlined in uapi/linux/perf_events.h > near struct perf_event_mmap_page. > > Any other usage -- you get to keep the pieces." > > "Anyway, given RISC-V being a very young platform, I would try really > *really* *REALLY* hard to stomp on these applications and get them to > change in order to reclaim the PMU usage." > > We need to decide what's the best approach for RISC-V. The current > text in uABI will let users assume that > cycle/instret can be read without any issues which is wrong. > > There are few options what we can do for RISC-V: > > 1. We can trap n emulate and report 0 always as suggested by Mark in > that thread. > 2. Continue to allow the user to read the counters directly but > expects the garbage value depending on the other activities > on the system. Hopefully, folks will fix their application by that time. > > Once we have the procfs interface, we enforce the behavior by breaking > the application. > > In either case, the uABI needs to be updated accordingly. I don't disagree with what I saw either PeterZ or Mark say. I'm happy not to resend this, I've got no skin in the game other than not wanting something like that "documented" only in the driver. What do you intend doing to instigate the "stomping"? Thanks, Conor. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 161 bytes --] _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order 2022-11-24 13:04 ` [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order Conor Dooley 2022-11-24 13:42 ` Heiko Stübner @ 2022-11-25 1:13 ` Guo Ren 1 sibling, 0 replies; 31+ messages in thread From: Guo Ren @ 2022-11-25 1:13 UTC (permalink / raw) To: Conor Dooley Cc: linux-riscv, Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Stuebner, Andrew Jones, devicetree, linux-kernel On Thu, Nov 24, 2022 at 9:06 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > I used the wikipedia table for ordering extensions when updating the > pattern here in foo. > Unfortunately that table did not match canonical order, as defined by > the RISC-V ISA Manual, which defines extension ordering in (what is > currently) Table 41, "Standard ISA extension names". Fix things up by > re-sorting v (vector) and adding p (packed-simd) & j (dynamic > languages). The e (reduced integer) and g (general) extensions are still > intentionally left out. > > Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5 > Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators") > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml > index e80c967a4fa4..b7462ea2dbe4 100644 > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml > @@ -80,7 +80,7 @@ properties: > 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?v?k?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ > + pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:z(?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ Acked-by: Guo Ren <guoren@kernel.org> > > # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here > timebase-frequency: false > -- > 2.38.1 > -- Best Regards Guo Ren _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2023-01-09 21:52 UTC | newest] Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-24 13:04 [PATCH 0/2] riscv,isa fixups Conor Dooley 2022-11-24 13:04 ` [PATCH 1/2] dt-bindings: riscv: fix underscore requirement for addtional standard extensions Conor Dooley 2022-11-25 1:12 ` Guo Ren 2022-11-24 13:04 ` [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order Conor Dooley 2022-11-24 13:42 ` Heiko Stübner 2022-11-24 13:52 ` Conor Dooley 2022-11-28 17:41 ` Palmer Dabbelt 2022-11-28 18:08 ` Conor Dooley 2022-11-28 18:12 ` Palmer Dabbelt 2022-11-28 19:17 ` Conor Dooley 2022-11-28 23:41 ` Palmer Dabbelt 2022-11-29 5:19 ` Andrew Jones 2022-11-29 11:40 ` Conor Dooley 2022-11-29 14:47 ` [RFC 0/2] Putting some basic order on isa extension stuff Conor Dooley 2022-11-29 15:48 ` Andrew Jones 2022-11-29 16:50 ` Conor Dooley 2022-11-29 14:47 ` [RFC 1/2] RISC-V: clarify ISA string ordering rules in cpu.c Conor Dooley 2022-11-29 16:12 ` Andrew Jones 2022-11-29 16:54 ` Conor Dooley 2022-11-29 17:19 ` Andrew Jones 2022-11-29 17:48 ` Conor Dooley 2022-11-29 14:47 ` [RFC 2/2] RISC-V: resort all extensions in "canonical" order Conor Dooley 2022-11-29 16:35 ` Andrew Jones 2022-12-01 13:51 ` [PATCH] Documentation: riscv: note that counter access is part of the uABI Conor Dooley 2022-12-01 19:21 ` Palmer Dabbelt 2022-12-03 10:38 ` Jonathan Corbet 2022-12-03 10:45 ` Jonathan Corbet 2022-12-03 10:56 ` Conor Dooley 2023-01-09 21:36 ` Atish Patra 2023-01-09 21:46 ` Conor Dooley 2022-11-25 1:13 ` [PATCH 2/2] dt-bindings: riscv: fix single letter canonical order Guo Ren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).