[v1] dt-bindings: riscv: deprecate riscv,isa

Commit Message

Conor Dooley May 18, 2023, 8:58 a.m. UTC

intro
=====

When the RISC-V dt-bindings were accepted upstream in Linux, the base
ISA etc had yet to be ratified. By the ratification of the base ISA,
incompatible changes had snuck into the specifications - for example the
Zicsr and Zifencei extensions were spun out of the base ISA.

Fast forward to today, and the reason for this patch.
Currently the riscv,isa dt property permits only a specific subset of
the ISA string - in particular it excludes version numbering.
With the current constraints, it is not possible to discern whether
"rv64i" means that the hart supports the fence.i instruction, for
example.
Future systems may choose to implement their own instruction fencing,
perhaps using a vendor extension, or they may not implement the optional
counter extensions. Software needs a way to determine this.

versioning schemes
==================

"Use the extension versions that are described in the ISA manual" you
may say, and it's not like this has not been considered.
Firstly, software that parses the riscv,isa property at runtime will
need to contain a lookup table of some sort that maps arbitrary versions
to versions it understands. There is not a consistent application of
version number applied to extensions, with a higgledy-piggledy
collection of tags, "bare" and version documents awaiting the reader on
the "recently ratified extensions" page:
https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions

	As an aside, this is reflected in the patch too, since many
	extensions have yet to appear in a release of the ISA specs,
	and are defined by commits in their respective "working draft"
	repositories.

Secondly, there is an issue of backwards compatibility, whereby allowing
numbers in the ISA string, some parsers may be broken. This would
require an additional property to be created to even use the versions in
this manner.

boolean properties
==================

If a new property is needed, the whole approach may as well be looked at
from the bottom up. A string with limited character choices etc is
hardly the best approach for communicating extension information to
software.

Switching to using boolean properties, one per extension, allows us to
define explicit meanings for the DT representation of each extension -
rather than the current situation where different operating systems or
other bits of software may impart different meanings to characters in
the string. Clearly the best source of meanings is the specifications
themselves, this just provides us the ability to choose at what point
in time the meaning is set. If an extension changes incompatibility in
the future, a new property will be required.

Off-list, some of the RVI folks have committed to shoring up the wording
in either the ISA specifications, the riscv-isa-manual or
so that in the future, modifications to and additions or removals of
features will require a new extension. Codifying that assertion
somewhere would make it quite unlikely that compatibility would be
broken, but we have the tools required to deal with it, if & when it
crops up.
It is in our collective interest, as consumers of extension meanings, to
define a scheme that enforces compatibility.

The use of boolean properties, rather than elements in a string, will
also permit validation that the strings have a meaning, as well as
potentially reject mutually exclusive combinations, or enforce
dependencies between instructions. That would not be possible with the
current dt-schema infrastructure for arbitrary strings, as we would need
to add a riscv,isa parser to dt-validate!
	That's not implemented in this patch, but rather left as
	future work!

acpi
====

The current ACPI ECR is based on having a string unfortunately, but
ideally ACPI will move to another method, perhaps GUIDs, that give
explicit meaning to extensions.

parser simplicity
=================

Many systems that parse DT at runtime already implement an function that
can check for the presence of boolean properties, rather than having to
implement - although unfortunately for backwards compatibility with old
dtbs, existing parsers may not be removable - which may greatly simplify
dt parsing code. For example, in Linux, checking for an extension
becomes as simple as:
	of_property_present(node, "riscv,isa-extension-zicbom")

vendor extensions
=================

Compared to riscv,isa, this proposed scheme promotes vendor extensions,
oft touted as the strength of RISC-V, to first-class citizens.
At present, extensions are defined as meaning what the RISC-V ISA
specifications say they do. There is no realistic way of using that
interface to provide cross-platform definitions for what vendor
extensions mean. Vendor extensions may also have even less consistency
than RVI do in terms of versioning, or no care about backwards
compatibility.
A boolean property allows us to assign explicit meanings on a per vendor
extension basis, backed up by a description of their meanings.

fin
===

Create a new file to store the extension meanings, each in the form
riscv,isa-extension-<foo> and a new riscv,isa-base property to replace
the missing aspect of riscv,isa - the base ISA implemented by a hart.
As a starting point, properties were added for extensions currently used
in Linux.

Finally, mark riscv,isa as deprecated. o7.

CC: Palmer Dabbelt <[e-mail]>
CC: Paul Walmsley <[e-mail]>
CC: Rob Herring <[e-mail]>
CC: Krzysztof Kozlowski <[e-mail]>
CC: Alistair Francis <[e-mail]>
CC: Andrew Jones <[e-mail]>
CC: Anup Patel <[e-mail]>
CC: Atish Patra <[e-mail]>
CC: Jessica Clarke <[e-mail]>
CC: Rick Chen <[e-mail]>
CC: Leo <[e-mail]>
CC: [e-mail]
CC: [e-mail]
CC: [e-mail]
CC: [e-mail]
CC: [e-mail]
Signed-off-by: Conor Dooley <[e-mail]>
---
I've tried to CC a few folks here that would care about this, but I am
sure there are more. I'll go cross-post it to sw-dev, if it allows me to
post there...
---
 .../devicetree/bindings/riscv/cpus.yaml       |  45 +--
 .../devicetree/bindings/riscv/extensions.yaml | 259 ++++++++++++++++++
 2 files changed, 282 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/riscv/extensions.yaml

Comments

Andrew Jones May 18, 2023, 10:31 a.m. UTC | #1

On Thu, May 18, 2023 at 09:58:30AM +0100, Conor Dooley wrote:
> intro
> =====
> 
> When the RISC-V dt-bindings were accepted upstream in Linux, the base
> ISA etc had yet to be ratified. By the ratification of the base ISA,
> incompatible changes had snuck into the specifications - for example the
> Zicsr and Zifencei extensions were spun out of the base ISA.
> 
> Fast forward to today, and the reason for this patch.
> Currently the riscv,isa dt property permits only a specific subset of
> the ISA string - in particular it excludes version numbering.
> With the current constraints, it is not possible to discern whether
> "rv64i" means that the hart supports the fence.i instruction, for
> example.
> Future systems may choose to implement their own instruction fencing,
> perhaps using a vendor extension, or they may not implement the optional
> counter extensions. Software needs a way to determine this.
> 
> versioning schemes
> ==================
> 
> "Use the extension versions that are described in the ISA manual" you
> may say, and it's not like this has not been considered.
> Firstly, software that parses the riscv,isa property at runtime will
> need to contain a lookup table of some sort that maps arbitrary versions
> to versions it understands. There is not a consistent application of
> version number applied to extensions, with a higgledy-piggledy
> collection of tags, "bare" and version documents awaiting the reader on
> the "recently ratified extensions" page:
> https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions
> 
> 	As an aside, this is reflected in the patch too, since many
> 	extensions have yet to appear in a release of the ISA specs,
> 	and are defined by commits in their respective "working draft"
> 	repositories.
> 
> Secondly, there is an issue of backwards compatibility, whereby allowing
> numbers in the ISA string, some parsers may be broken. This would
> require an additional property to be created to even use the versions in
> this manner.
> 
> boolean properties
> ==================
> 
> If a new property is needed, the whole approach may as well be looked at
> from the bottom up. A string with limited character choices etc is
> hardly the best approach for communicating extension information to
> software.
> 
> Switching to using boolean properties, one per extension, allows us to
> define explicit meanings for the DT representation of each extension -
> rather than the current situation where different operating systems or
> other bits of software may impart different meanings to characters in
> the string. Clearly the best source of meanings is the specifications
> themselves, this just provides us the ability to choose at what point
> in time the meaning is set. If an extension changes incompatibility in
> the future, a new property will be required.
> 
> Off-list, some of the RVI folks have committed to shoring up the wording
> in either the ISA specifications, the riscv-isa-manual or
> so that in the future, modifications to and additions or removals of
> features will require a new extension. Codifying that assertion
> somewhere would make it quite unlikely that compatibility would be
> broken, but we have the tools required to deal with it, if & when it
> crops up.
> It is in our collective interest, as consumers of extension meanings, to
> define a scheme that enforces compatibility.
> 
> The use of boolean properties, rather than elements in a string, will
> also permit validation that the strings have a meaning, as well as
> potentially reject mutually exclusive combinations, or enforce
> dependencies between instructions. That would not be possible with the
> current dt-schema infrastructure for arbitrary strings, as we would need
> to add a riscv,isa parser to dt-validate!
> 	That's not implemented in this patch, but rather left as
> 	future work!
> 
> acpi
> ====
> 
> The current ACPI ECR is based on having a string unfortunately, but
> ideally ACPI will move to another method, perhaps GUIDs, that give
> explicit meaning to extensions.
> 
> parser simplicity
> =================
> 
> Many systems that parse DT at runtime already implement an function that
> can check for the presence of boolean properties, rather than having to
> implement - although unfortunately for backwards compatibility with old
> dtbs, existing parsers may not be removable - which may greatly simplify
> dt parsing code. For example, in Linux, checking for an extension
> becomes as simple as:
> 	of_property_present(node, "riscv,isa-extension-zicbom")
> 
> vendor extensions
> =================
> 
> Compared to riscv,isa, this proposed scheme promotes vendor extensions,
> oft touted as the strength of RISC-V, to first-class citizens.
> At present, extensions are defined as meaning what the RISC-V ISA
> specifications say they do. There is no realistic way of using that
> interface to provide cross-platform definitions for what vendor
> extensions mean. Vendor extensions may also have even less consistency
> than RVI do in terms of versioning, or no care about backwards
> compatibility.
> A boolean property allows us to assign explicit meanings on a per vendor
> extension basis, backed up by a description of their meanings.
> 
> fin
> ===
> 
> Create a new file to store the extension meanings, each in the form
> riscv,isa-extension-<foo> and a new riscv,isa-base property to replace
> the missing aspect of riscv,isa - the base ISA implemented by a hart.
> As a starting point, properties were added for extensions currently used
> in Linux.
> 
> Finally, mark riscv,isa as deprecated. o7.
> 
> CC: Palmer Dabbelt <[e-mail]>
> CC: Paul Walmsley <[e-mail]>
> CC: Rob Herring <[e-mail]>
> CC: Krzysztof Kozlowski <[e-mail]>
> CC: Alistair Francis <[e-mail]>
> CC: Andrew Jones <[e-mail]>
> CC: Anup Patel <[e-mail]>
> CC: Atish Patra <[e-mail]>
> CC: Jessica Clarke <[e-mail]>
> CC: Rick Chen <[e-mail]>
> CC: Leo <[e-mail]>
> CC: [e-mail]
> CC: [e-mail]
> CC: [e-mail]
> CC: [e-mail]
> CC: [e-mail]
> Signed-off-by: Conor Dooley <[e-mail]>
> ---
> I've tried to CC a few folks here that would care about this, but I am
> sure there are more. I'll go cross-post it to sw-dev, if it allows me to
> post there...
> ---
>  .../devicetree/bindings/riscv/cpus.yaml       |  45 +--
>  .../devicetree/bindings/riscv/extensions.yaml | 259 ++++++++++++++++++
>  2 files changed, 282 insertions(+), 22 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/riscv/extensions.yaml
> 
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index 3d2934b15e80..446801fb7495 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -23,6 +23,9 @@ description: |
>    two cores, each of which has two hyperthreads, could be described as
>    having four harts.
>  
> +allOf:
> +  - $ref: extensions.yaml
> +
>  properties:
>    compatible:
>      oneOf:
> @@ -79,25 +82,6 @@ properties:
>      description:
>        The blocksize in bytes for the Zicboz cache operations.
>  
> -  riscv,isa:
> -    description:
> -      Identifies the specific RISC-V instruction set architecture
> -      supported by the hart.  These are documented in the RISC-V
> -      User-Level ISA document, available from
> -      https://riscv.org/specifications/
> -
> -      Due to revisions of the ISA specification, some deviations
> -      have arisen over time.
> -      Notably, riscv,isa was defined prior to the creation of the
> -      Zicsr and Zifencei extensions and thus "i" implies
> -      "zicsr_zifencei".
> -
> -      While the isa strings in ISA specification are case
> -      insensitive, letters in the riscv,isa string must be all
> -      lowercase to simplify parsing.
> -    $ref: "/schemas/types.yaml#/definitions/string"
> -    pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
> -
>    # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
>    timebase-frequency: false
>  
> @@ -133,8 +117,13 @@ properties:
>        DMIPS/MHz, relative to highest capacity-dmips-mhz
>        in the system.
>  
> +oneOf:
> +  - required:
> +      - riscv,isa

This is the part Anup keeps reminding me about. We can create better ways
to handle extensions in DT and ACPI, but we'll still need to parse ISA
strings to handle legacy DTs and holdouts that keep creating ISA strings,
at least during the deprecation period, since ISA strings are still "the
way to do it" according to the spec.

Also, if we assume the wording in the spec does get shored up, then,
unless I'm missing something, the list of advantages for this boolean
proposal from your commit message would be

* More character choices for name -- probably not a huge gain for ratified
  extensions, since the boolean properties will likely still use the same
  name as the ISA string (riscv,isa-extension-<name>). But, for vendor
  extensions, this is indeed a major improvement, since vendor extension
  boolean property names may need to be extended in unambiguous ways to
  handle changes in the extension.

* Simpler, more complete DT validation (but we still need a best effort
  for legacy ISA strings)

* Simpler DT parsing (but we still need the current parser for legacy ISA
  strings)

> +  - required:
> +      - riscv,isa-base
> +
>  required:
> -  - riscv,isa
>    - interrupt-controller
>  
>  additionalProperties: true
> @@ -177,7 +166,13 @@ examples:
>                  i-tlb-size = <32>;
>                  mmu-type = "riscv,sv39";
>                  reg = <1>;
> -                riscv,isa = "rv64imafdc";
> +                riscv,isa-base = "rv64i";
> +                riscv,isa-extension-i;
> +                riscv,isa-extension-m;
> +                riscv,isa-extension-a;
> +                riscv,isa-extension-f;
> +                riscv,isa-extension-d;
> +                riscv,isa-extension-c;
>                  tlb-split;
>                  cpu_intc1: interrupt-controller {
>                          #interrupt-cells = <1>;
> @@ -196,7 +191,13 @@ examples:
>                  device_type = "cpu";
>                  reg = <0>;
>                  compatible = "riscv";
> -                riscv,isa = "rv64imafdc";
> +                riscv,isa-base = "rv64i";
> +                riscv,isa-extension-i;
> +                riscv,isa-extension-m;
> +                riscv,isa-extension-a;
> +                riscv,isa-extension-f;
> +                riscv,isa-extension-d;
> +                riscv,isa-extension-c;
>                  mmu-type = "riscv,sv48";
>                  interrupt-controller {
>                          #interrupt-cells = <1>;
> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> new file mode 100644
> index 000000000000..1b4d726f7174
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> @@ -0,0 +1,259 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/riscv/extensions.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RISC-V ISA extensions
> +
> +maintainers:
> +  - Paul Walmsley <[e-mail]>
> +  - Palmer Dabbelt <[e-mail]>
> +  - Conor Dooley <[e-mail]>
> +
> +description: |
> +  RISC-V has large number of extensions, some of which "standard" extensions,
               ^ a                                       ^ are

> +  meaning they are ratified by RISC-V International, and others are "vendor"
> +  extensions.  This document defines properties that indicate whether a hart
> +  supports a given extensions.

drop 'a' or depluralize 'extensions'

> +
> +  Once a standard extension has been ratified, no features can be added or

I'd change 'features' to 'changes in behavior', and then...

> +  removed without the creation of a new extension for that sub- or super-set.

...drop 'for that sub- or super-set'

> +  The properties for standard extensions therefore map to their originally
> +  ratified states, with the exception of the I, Zicntr & Zihpm extensions.

Can you elaborate on the exceptions? Or, if the exceptions are described
below, maybe a '(see below)' here would help ease the reader's
insecurities about their lack of knowledge about these exceptions, as
they'll see that the education is coming :-)

> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        const: riscv
> +
> +properties:
> +  riscv,isa:
> +    description:
> +      Identifies the specific RISC-V instruction set architecture
> +      supported by the hart.  These are documented in the RISC-V
> +      User-Level ISA document, available from
> +      https://riscv.org/specifications/
> +
> +      Due to revisions of the ISA specification, some deviations
> +      have arisen over time.
> +      Notably, riscv,isa was defined prior to the creation of the
> +      Zicsr and Zifencei extensions and thus "i" implies
> +      "zicsr_zifencei".
> +
> +      While the isa strings in ISA specification are case
                                 ^ the

				 (but I see this was a faithful move
				 of the current text, so maybe better
				 to fix it up separately)

> +      insensitive, letters in the riscv,isa string must be all
> +      lowercase to simplify parsing.
> +
> +      This property has been deprecated due to disparity between the
> +      extension at the time of its creation and ratification of the
> +      base ISA.
> +
> +    $ref: /schemas/types.yaml#/definitions/string
> +    pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
> +    deprecated: true
> +
> +  riscv,isa-base:
> +    description:
> +      The base ISA implemented by this hart, as described by the 20191213
> +      version of the unprivileged ISA specification.
> +    enum:
> +      - rv32i
> +      - rv64i
> +
> +  riscv,isa-extension-i:
> +    type: boolean
> +    description:
> +      The base integer instruction set, as ratified in the 20191213 version of the
> +      unprivileged ISA specification.
> +
> +  riscv,isa-extension-m:
> +    type: boolean
> +    description:
> +      The standard M extension for integer multiplication and division, as
> +      ratified in the 20191213 version of the unprivileged ISA specification.
> +
> +  riscv,isa-extension-a:
> +    type: boolean
> +    description:
> +      The standard A extension for atomic instructions, as ratified in the
> +      20191213 version of the unprivileged ISA specification.
> +
> +  riscv,isa-extension-f:
> +    type: boolean
> +    description:
> +      The standard M extension for single-precision floating point, as
                      ^ F

> +      ratified in the 20191213 version of the unprivileged ISA specification.
> +
> +  riscv,isa-extension-d:
> +    type: boolean
> +    description:
> +      The standard M extension for double-precision floating-point, as
                      ^ D

> +      ratified in the 20191213 version of the unprivileged ISA specification.
> +
> +  riscv,isa-extension-q:
> +    type: boolean
> +    description:
> +      The standard M extension for quad-precision floating-point, as ratified in
                      ^ Q

> +      the 20191213 version of the unprivileged ISA specification.
> +
> +  riscv,isa-extension-c:
> +    type: boolean
> +    description:
> +      The standard M extension for compressed instructions, as ratified in the
                      ^ C

> +      20191213 version of the unprivileged ISA specification.
> +
> +  riscv,isa-extension-v:
> +    type: boolean
> +    description:
> +      The standard V extension for vector operations, as ratified in-and-around
> +      commit 7a6c8ae ("Fix text that describes vfmv.v.f encoding") of the
> +      riscv-v-spec.
> +
> +  riscv,isa-extension-h:
> +    type: boolean
> +    description:
> +      The standard h extension for hypervisors as ratified in the 20191213
                      ^ H (might as well keep the case consistent)

> +      version of the privileged ISA specification.
> +
> +  # Additional Standard Extensions, sorted by category then alphabetically

Can we just do pure alphabetically? And the single-letter extensions above
don't have a "sorted by" comment above them. I guess they need one, or
maybe they can also be alphabetical?

> +
> +  riscv,isa-extension-zicntr:
> +    type: boolean
> +    description:
> +      The standard Zicntr extension for base counters and timers, as ratified
> +      in the 20191213 version of the unprivileged ISA specification.
> +
> +  riscv,isa-extension-zicsr:
> +    type: boolean
> +    description:
> +      The standard Zicsr extension for control and status register instructions,
> +      as ratified in the 20191213 version of the unprivileged ISA specification.
> +
> +  riscv,isa-extension-zifencei:
> +    type: boolean
> +    description:
> +      The standard Zifencei extension for instruction-fetch fence, as ratified
> +      in the 20191213 version of the unprivileged ISA specification.
> +
> +  riscv,isa-extension-zihpm:
> +    type: boolean
> +    description:
> +      The standard Zihpm extension for hardware performance counters, as
> +      ratified in the 20191213 version of the unprivileged ISA specification.
> +
> +  riscv,isa-extension-zicbom:
> +    type: boolean
> +    description:
> +      The standard Zicbom extension for base cache management operations as
> +      ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> +
> +  riscv,isa-extension-zicbop:
> +    type: boolean
> +    description:
> +      The standard Zicbop extension for cache-block prefetch instructions as
> +      ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> +
> +  riscv,isa-extension-zicboz:
> +    type: boolean
> +    description:
> +      The standard  Zicbomz extension for cache-block zeroing as ratified in
                     ^ ^Zicboz
                     ^ extra space

(The repetition is making my vision blur, so I'm feeling like I should
write a script to compare $a and $b, where $a is riscv,isa-extension-$a
and $b is 'The standard $b' to make sure they match :-) But I probably
won't...

> +      commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> +
> +  riscv,isa-extension-zihintpause:
> +    type: boolean
> +    description: |
> +      The standard Zihintpause extension for pause hints, as ratified in
> +      commit d8ab5c7 ("Zihintpause is ratified") of the riscv-isa-manual.
> +
> +  riscv,isa-extension-zba:
> +    type: boolean
> +    description: |
> +      The standard Zba bit-manipulation extension for address generation
> +      acceleration instructions as ratified at commit 6d33919 ("Merge pull
> +      request #158 from hirooih/clmul-fix-loop-end-condition") of
> +      riscv-bitmanip.
> +
> +  riscv,isa-extension-zbb:
> +    type: boolean
> +    description: |
> +      The standard Zbb bit-manipulation extension for basic bit-manipulation as
> +      atified at commit 6d33919 ("Merge pull request #158 from
         ^ ratified

> +      hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
> +
> +  riscv,isa-extension-zbc:
> +    type: boolean
> +    description: |
> +      The standard Zbc bit-manipulation extension for carry-less multiplication
> +      as ratified at commit 6d33919 ("Merge pull request #158 from
> +      hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
> +
> +  riscv,isa-extension-zbs:
> +    type: boolean
> +    description: |
> +      The standard Zbs bit-manipulation extension for single-bit instructions
> +      as ratified at commit 6d33919 ("Merge pull request #158 from
> +      hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
> +
> +  riscv,isa-extension-ztso:
> +    type: boolean
> +    description:
> +      The standard Ztso extension for total store ordering, as ratified in
> +      commit 2e5236 ("Ztso is now ratified.") of the riscv-isa-manual.
> +
> + # Standard Supervisor-level Extensions, sorted by category then alphabetically

The spec only says alphabetical sorting for supervisor-level extensions,
no category.

> +
> +  'riscv,isa-extension-smaia':
> +    type: boolean
> +    description: |
> +      The standard Smaia supervisor-level extension for the advanced interrupt
> +      architecture for machine-mode-visible csr and behavioural changes to
> +      interrupts as frozen at commit ccbddab ("Merge pull request #42 from
> +      riscv/jhauser-2023-RC4") of riscv-aia.
> +
> +  riscv,isa-extension-ssaia:
> +    type: boolean
> +    description: |
> +      The standard Ssaia supervisor-level extension for the advanced interrupt
> +      architecture for supervisor-mode-visible csr and behavioural changes to
> +      interrupts as frozen at commit ccbddab ("Merge pull request #42 from
> +      riscv/jhauser-2023-RC4") of riscv-aia.
> +
> +  riscv,isa-extension-sscofpmf:
> +    type: boolean
> +    description: |
> +      The standard supervisor-level extension for count overflow and mode-based
                     ^ Sscofpmf

> +      filtering as ratified at commit 01d1df0 ("Add ability to manually trigger
> +      workflow. (#2)") of riscv-count-overflow.
> +
> +  riscv,isa-extension-sstc:
> +    type: boolean
> +    description:
> +      The standard supervisor-level extension for time compare
                     ^ Sstc

> +      as ratified at commit 3f9ed34 ("Add ability to manually trigger
> +      workflow. (#2)") of riscv-time-compare.
> +
> +  riscv,isa-extension-svinval:
> +    type: boolean
> +    description:
> +      The standard Svinval supervisor-level extension for fine-grained
> +      address-translation cache invalidation as ratified in the 20191213 version
> +      of the privileged ISA specification.
> +
> +  riscv,isa-extension-svnapot:
> +    type: boolean
> +    description:
> +      The standard Svnapot supervisor-level extensions for napot translation
> +      contiguity as ratified in the 20191213 version of the privileged ISA
> +      specification.
> +
> +  riscv,isa-extension-svpbmt:
> +    type: boolean
> +    description:
> +      The standard Svpbmt supervisor-level extensions for page-based memory
> +      types as ratified in the 20191213 version of the privileged ISA
> +      specification.
> +
> +additionalProperties: true
> +...
> -- 
> 2.39.2
>

I'll take your word for it on the versions/dates/commit hashes referenced,
at least until we get closer to actually merging this.

Thanks,
drew

Conor Dooley May 18, 2023, 11:15 a.m. UTC | #2

Hey Drew,

On Thu, May 18, 2023 at 12:31:51PM +0200, Andrew Jones wrote:
> On Thu, May 18, 2023 at 09:58:30AM +0100, Conor Dooley wrote:

> > -  riscv,isa:
> > -    description:
> > -      Identifies the specific RISC-V instruction set architecture
> > -      supported by the hart.  These are documented in the RISC-V
> > -      User-Level ISA document, available from
> > -      https://riscv.org/specifications/
> > -
> > -      Due to revisions of the ISA specification, some deviations
> > -      have arisen over time.
> > -      Notably, riscv,isa was defined prior to the creation of the
> > -      Zicsr and Zifencei extensions and thus "i" implies
> > -      "zicsr_zifencei".
> > -
> > -      While the isa strings in ISA specification are case
> > -      insensitive, letters in the riscv,isa string must be all
> > -      lowercase to simplify parsing.
> > -    $ref: "/schemas/types.yaml#/definitions/string"
> > -    pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
> > -
> >    # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
> >    timebase-frequency: false
> >  
> > @@ -133,8 +117,13 @@ properties:
> >        DMIPS/MHz, relative to highest capacity-dmips-mhz
> >        in the system.
> >  
> > +oneOf:
> > +  - required:
> > +      - riscv,isa
> 
> This is the part Anup keeps reminding me about. We can create better ways
> to handle extensions in DT and ACPI, but we'll still need to parse ISA
> strings to handle legacy DTs and holdouts that keep creating ISA strings,
> at least during the deprecation period,

Yeah, we cannot remove the string parser from the kernel as we will
break existing users.
I don't see keeping it as a problem, we should be nice to people rather
than intentionally trip them up. Let them trip themselves up when their
implicit meaning doesn't match whatever bit of software they are
running's.

> since ISA strings are still "the
> way to do it" according to the spec.

I am not sure what this means, dt-bindings determine the DT ABI, not
what RVI puts in specs.

> Also, if we assume the wording in the spec does get shored up, then,
> unless I'm missing something, the list of advantages for this boolean
> proposal from your commit message would be

Well, shored up _AND_ adhered to. Actions speak far, far louder than
words in that respect unfortunately!

> * More character choices for name -- probably not a huge gain for ratified
>   extensions, since the boolean properties will likely still use the same
>   name as the ISA string (riscv,isa-extension-<name>). But, for vendor
>   extensions, this is indeed a major improvement, since vendor extension
>   boolean property names may need to be extended in unambiguous ways to
>   handle changes in the extension.
> 
> * Simpler, more complete DT validation (but we still need a best effort
>   for legacy ISA strings)

The "best effort" validation via dt-bindings is the current regex. I've
intentionally marked it deprecated, rather than delete it partly for
that reason & partly out of consideration for other users.

> * Simpler DT parsing (but we still need the current parser for legacy ISA
>   strings)

Speaking only about Linux, we can use these meanings here for interpreting
the strings and then apply the fixups that correspond to the difference
between the defined meanings & those at the time of the dt-binding
originally being merged - unconditionally setting zifencei, zicsr, zicntr,
etc. If you don't implement those things, then expect to fall over.
Other operating systems etc may have different implicit meanings and
their own decisions to make!
Oh, and if the "foo" bit of "riscv,isa-extension-foo" doesn't match what
you put in riscv,isa then you keep the pieces. For example, if a vendor
has a vendor extension Xconor where version 1.0.1 is incompatible with
v1.0.0, the properties may be "riscv,isa-extension-xconor" and
"riscv,isa-extension-xconor-no-insnx". In that case, for Linux, I would
assert that there should/would be no way to get the later version of
that extension via riscv,isa.
There are no existing situations like this, so no backwards
compatibility is broken here. If/when it happens, the property is
deprecated and we can direct such cases to the new interface :)
Basically the same as:
https://lore.kernel.org/linux-riscv/20230504-oncoming-antihero-1ed69bb8f57d@spud/

	That reminds me, I need to re-spin that with more extensions set
	unconditionally.

If some new OS comes along, they don't need to implement riscv,isa at
all as it's deprecated.

I'd like to add another one:
* Unified meaning of extensions across bits of software. I actually have
  no idea what versions of things other OSes map the characters to.

> > diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > new file mode 100644
> > index 000000000000..1b4d726f7174
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > @@ -0,0 +1,259 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/riscv/extensions.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: RISC-V ISA extensions
> > +
> > +maintainers:
> > +  - Paul Walmsley <[e-mail]>
> > +  - Palmer Dabbelt <[e-mail]>
> > +  - Conor Dooley <[e-mail]>
> > +
> > +description: |
> > +  RISC-V has large number of extensions, some of which "standard" extensions,
>                ^ a                                       ^ are
> 
> > +  meaning they are ratified by RISC-V International, and others are "vendor"
> > +  extensions.  This document defines properties that indicate whether a hart
> > +  supports a given extensions.
> 
> drop 'a' or depluralize 'extensions'
> 
> > +
> > +  Once a standard extension has been ratified, no features can be added or
> 
> I'd change 'features' to 'changes in behavior', and then...
> 
> > +  removed without the creation of a new extension for that sub- or super-set.
> 
> ...drop 'for that sub- or super-set'
> 
> > +  The properties for standard extensions therefore map to their originally
> > +  ratified states, with the exception of the I, Zicntr & Zihpm extensions.
> 
> Can you elaborate on the exceptions? Or, if the exceptions are described
> below, maybe a '(see below)' here would help ease the reader's
> insecurities about their lack of knowledge about these exceptions, as
> they'll see that the education is coming :-)

Ah crap, I meant to note in the I section that timers were moved to
their own home. Since we are defining this stuff now, I felt that it'd
make sense to define riscv,isa-extension-i as meaning the ratified spec,
sans the counters.

> > +  riscv,isa-extension-f:
> > +    type: boolean
> > +    description:
> > +      The standard M extension for single-precision floating point, as
>                       ^ F

Whoops. All of these are me realising this morning, after checking
everything a few times last night, that there was a dt_binding_check
complaint which prevented me implementing non-patterns as
patternProperties. As a result, I split everything back out into single
properties. It's always the last minute bits...

I could have had a mix of properties and pattern properties, but I
preferred to keep the ordering as something that people could more
easily use to locate properties.

> > +      20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-v:
> > +    type: boolean
> > +    description:
> > +      The standard V extension for vector operations, as ratified in-and-around
> > +      commit 7a6c8ae ("Fix text that describes vfmv.v.f encoding") of the
> > +      riscv-v-spec.
> > +
> > +  riscv,isa-extension-h:
> > +    type: boolean
> > +    description:
> > +      The standard h extension for hypervisors as ratified in the 20191213
>                       ^ H (might as well keep the case consistent)
> 
> > +      version of the privileged ISA specification.
> > +
> > +  # Additional Standard Extensions, sorted by category then alphabetically
> 
> Can we just do pure alphabetically? And the single-letter extensions above
> don't have a "sorted by" comment above them. I guess they need one, or
> maybe they can also be alphabetical?

I don't really have a preference for ordering. I'm happy to do pure
alphabetical.

> > +  riscv,isa-extension-zicboz:
> > +    type: boolean
> > +    description:
> > +      The standard  Zicbomz extension for cache-block zeroing as ratified in
>                      ^ ^Zicboz
>                      ^ extra space
> 
> (The repetition is making my vision blur, so I'm feeling like I should
> write a script to compare $a and $b, where $a is riscv,isa-extension-$a
> and $b is 'The standard $b' to make sure they match :-) But I probably
> won't...

Again, same excuse! Silly me and all that.

> I'll take your word for it on the versions/dates/commit hashes referenced,
> at least until we get closer to actually merging this.

Aye & I am hoping that by the time that this could actually be
considered for merging that perhaps some of the items will migrate to
the riscv-isa-manual, instead of being flung to the four winds.

Thanks,
Conor.

Conor Dooley May 18, 2023, 11:25 a.m. UTC | #3

On Thu, May 18, 2023 at 12:31:51PM +0200, Andrew Jones wrote:
> > +  # Additional Standard Extensions, sorted by category then alphabetically
> 
> Can we just do pure alphabetically? And the single-letter extensions above
> don't have a "sorted by" comment above them. I guess they need one, or
> maybe they can also be alphabetical?

Maybe it is just me, but my brain is too used to seeing those ones in
something approaching canonical order. I'd rather keep them that way &
then alphanumerical for everything else?

I also noticed that the CMO stuff wasn't actually in the comment's order
anyway, so needs a re-sort to begin with. A vote in favour of
alphanumerical.

Anup Patel May 18, 2023, 1:43 p.m. UTC | #4

On Thu, May 18, 2023 at 4:02 PM Andrew Jones <[e-mail]> wrote:
>
> On Thu, May 18, 2023 at 09:58:30AM +0100, Conor Dooley wrote:
> > intro
> > =====
> >
> > When the RISC-V dt-bindings were accepted upstream in Linux, the base
> > ISA etc had yet to be ratified. By the ratification of the base ISA,
> > incompatible changes had snuck into the specifications - for example the
> > Zicsr and Zifencei extensions were spun out of the base ISA.
> >
> > Fast forward to today, and the reason for this patch.
> > Currently the riscv,isa dt property permits only a specific subset of
> > the ISA string - in particular it excludes version numbering.
> > With the current constraints, it is not possible to discern whether
> > "rv64i" means that the hart supports the fence.i instruction, for
> > example.
> > Future systems may choose to implement their own instruction fencing,
> > perhaps using a vendor extension, or they may not implement the optional
> > counter extensions. Software needs a way to determine this.
> >
> > versioning schemes
> > ==================
> >
> > "Use the extension versions that are described in the ISA manual" you
> > may say, and it's not like this has not been considered.
> > Firstly, software that parses the riscv,isa property at runtime will
> > need to contain a lookup table of some sort that maps arbitrary versions
> > to versions it understands. There is not a consistent application of
> > version number applied to extensions, with a higgledy-piggledy
> > collection of tags, "bare" and version documents awaiting the reader on
> > the "recently ratified extensions" page:
> > https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions
> >
> >       As an aside, this is reflected in the patch too, since many
> >       extensions have yet to appear in a release of the ISA specs,
> >       and are defined by commits in their respective "working draft"
> >       repositories.
> >
> > Secondly, there is an issue of backwards compatibility, whereby allowing
> > numbers in the ISA string, some parsers may be broken. This would
> > require an additional property to be created to even use the versions in
> > this manner.
> >
> > boolean properties
> > ==================
> >
> > If a new property is needed, the whole approach may as well be looked at
> > from the bottom up. A string with limited character choices etc is
> > hardly the best approach for communicating extension information to
> > software.
> >
> > Switching to using boolean properties, one per extension, allows us to
> > define explicit meanings for the DT representation of each extension -
> > rather than the current situation where different operating systems or
> > other bits of software may impart different meanings to characters in
> > the string. Clearly the best source of meanings is the specifications
> > themselves, this just provides us the ability to choose at what point
> > in time the meaning is set. If an extension changes incompatibility in
> > the future, a new property will be required.
> >
> > Off-list, some of the RVI folks have committed to shoring up the wording
> > in either the ISA specifications, the riscv-isa-manual or
> > so that in the future, modifications to and additions or removals of
> > features will require a new extension. Codifying that assertion
> > somewhere would make it quite unlikely that compatibility would be
> > broken, but we have the tools required to deal with it, if & when it
> > crops up.
> > It is in our collective interest, as consumers of extension meanings, to
> > define a scheme that enforces compatibility.
> >
> > The use of boolean properties, rather than elements in a string, will
> > also permit validation that the strings have a meaning, as well as
> > potentially reject mutually exclusive combinations, or enforce
> > dependencies between instructions. That would not be possible with the
> > current dt-schema infrastructure for arbitrary strings, as we would need
> > to add a riscv,isa parser to dt-validate!
> >       That's not implemented in this patch, but rather left as
> >       future work!
> >
> > acpi
> > ====
> >
> > The current ACPI ECR is based on having a string unfortunately, but
> > ideally ACPI will move to another method, perhaps GUIDs, that give
> > explicit meaning to extensions.
> >
> > parser simplicity
> > =================
> >
> > Many systems that parse DT at runtime already implement an function that
> > can check for the presence of boolean properties, rather than having to
> > implement - although unfortunately for backwards compatibility with old
> > dtbs, existing parsers may not be removable - which may greatly simplify
> > dt parsing code. For example, in Linux, checking for an extension
> > becomes as simple as:
> >       of_property_present(node, "riscv,isa-extension-zicbom")
> >
> > vendor extensions
> > =================
> >
> > Compared to riscv,isa, this proposed scheme promotes vendor extensions,
> > oft touted as the strength of RISC-V, to first-class citizens.
> > At present, extensions are defined as meaning what the RISC-V ISA
> > specifications say they do. There is no realistic way of using that
> > interface to provide cross-platform definitions for what vendor
> > extensions mean. Vendor extensions may also have even less consistency
> > than RVI do in terms of versioning, or no care about backwards
> > compatibility.
> > A boolean property allows us to assign explicit meanings on a per vendor
> > extension basis, backed up by a description of their meanings.
> >
> > fin
> > ===
> >
> > Create a new file to store the extension meanings, each in the form
> > riscv,isa-extension-<foo> and a new riscv,isa-base property to replace
> > the missing aspect of riscv,isa - the base ISA implemented by a hart.
> > As a starting point, properties were added for extensions currently used
> > in Linux.
> >
> > Finally, mark riscv,isa as deprecated. o7.
> >
> > CC: Palmer Dabbelt <[e-mail]>
> > CC: Paul Walmsley <[e-mail]>
> > CC: Rob Herring <[e-mail]>
> > CC: Krzysztof Kozlowski <[e-mail]>
> > CC: Alistair Francis <[e-mail]>
> > CC: Andrew Jones <[e-mail]>
> > CC: Anup Patel <[e-mail]>
> > CC: Atish Patra <[e-mail]>
> > CC: Jessica Clarke <[e-mail]>
> > CC: Rick Chen <[e-mail]>
> > CC: Leo <[e-mail]>
> > CC: [e-mail]
> > CC: [e-mail]
> > CC: [e-mail]
> > CC: [e-mail]
> > CC: [e-mail]
> > Signed-off-by: Conor Dooley <[e-mail]>
> > ---
> > I've tried to CC a few folks here that would care about this, but I am
> > sure there are more. I'll go cross-post it to sw-dev, if it allows me to
> > post there...
> > ---
> >  .../devicetree/bindings/riscv/cpus.yaml       |  45 +--
> >  .../devicetree/bindings/riscv/extensions.yaml | 259 ++++++++++++++++++
> >  2 files changed, 282 insertions(+), 22 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/riscv/extensions.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > index 3d2934b15e80..446801fb7495 100644
> > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > @@ -23,6 +23,9 @@ description: |
> >    two cores, each of which has two hyperthreads, could be described as
> >    having four harts.
> >
> > +allOf:
> > +  - $ref: extensions.yaml
> > +
> >  properties:
> >    compatible:
> >      oneOf:
> > @@ -79,25 +82,6 @@ properties:
> >      description:
> >        The blocksize in bytes for the Zicboz cache operations.
> >
> > -  riscv,isa:
> > -    description:
> > -      Identifies the specific RISC-V instruction set architecture
> > -      supported by the hart.  These are documented in the RISC-V
> > -      User-Level ISA document, available from
> > -      https://riscv.org/specifications/
> > -
> > -      Due to revisions of the ISA specification, some deviations
> > -      have arisen over time.
> > -      Notably, riscv,isa was defined prior to the creation of the
> > -      Zicsr and Zifencei extensions and thus "i" implies
> > -      "zicsr_zifencei".
> > -
> > -      While the isa strings in ISA specification are case
> > -      insensitive, letters in the riscv,isa string must be all
> > -      lowercase to simplify parsing.
> > -    $ref: "/schemas/types.yaml#/definitions/string"
> > -    pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
> > -
> >    # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
> >    timebase-frequency: false
> >
> > @@ -133,8 +117,13 @@ properties:
> >        DMIPS/MHz, relative to highest capacity-dmips-mhz
> >        in the system.
> >
> > +oneOf:
> > +  - required:
> > +      - riscv,isa
>
> This is the part Anup keeps reminding me about. We can create better ways
> to handle extensions in DT and ACPI, but we'll still need to parse ISA
> strings to handle legacy DTs and holdouts that keep creating ISA strings,
> at least during the deprecation period, since ISA strings are still "the
> way to do it" according to the spec.

Coming up with an alternate way in DT is fine but we can't deprecate
ISA strings since ISA strings are widely used:
1) Various bootloaders
2) It is part of /proc/cpuinfo
3) Hypervisors use it to communicate HW features to Guest/VM.
Hypervisors can't get away from generating ISA strings because
Hypervisors don't know what is running inside Guest/VM.

In the case of ACPI, it is a very different situation. Like Sunil mentioned,
ACPI will always follow mechanisms defined by RVI (such as ISA string).
Other ACPI approaches such as GUID for ISA extension are simply not
scalable and will take a lot more memory for ACPI tables compared to
ISA strings.

>
> Also, if we assume the wording in the spec does get shored up, then,
> unless I'm missing something, the list of advantages for this boolean
> proposal from your commit message would be

IMO, we should try our best to have the wordings changed in RVI spec.

>
> * More character choices for name -- probably not a huge gain for ratified
>   extensions, since the boolean properties will likely still use the same
>   name as the ISA string (riscv,isa-extension-<name>). But, for vendor
>   extensions, this is indeed a major improvement, since vendor extension
>   boolean property names may need to be extended in unambiguous ways to
>   handle changes in the extension.
>
> * Simpler, more complete DT validation (but we still need a best effort
>   for legacy ISA strings)
>
> * Simpler DT parsing (but we still need the current parser for legacy ISA
>   strings)
>
> > +  - required:
> > +      - riscv,isa-base
> > +
> >  required:
> > -  - riscv,isa
> >    - interrupt-controller
> >
> >  additionalProperties: true
> > @@ -177,7 +166,13 @@ examples:
> >                  i-tlb-size = <32>;
> >                  mmu-type = "riscv,sv39";
> >                  reg = <1>;
> > -                riscv,isa = "rv64imafdc";
> > +                riscv,isa-base = "rv64i";
> > +                riscv,isa-extension-i;
> > +                riscv,isa-extension-m;
> > +                riscv,isa-extension-a;
> > +                riscv,isa-extension-f;
> > +                riscv,isa-extension-d;
> > +                riscv,isa-extension-c;

One downside of this new approach is it will increase the size of DTB.
Imaging 50 such DT properties in 46 CPU DT nodes.

Regards,
Anup

> >                  tlb-split;
> >                  cpu_intc1: interrupt-controller {
> >                          #interrupt-cells = <1>;
> > @@ -196,7 +191,13 @@ examples:
> >                  device_type = "cpu";
> >                  reg = <0>;
> >                  compatible = "riscv";
> > -                riscv,isa = "rv64imafdc";
> > +                riscv,isa-base = "rv64i";
> > +                riscv,isa-extension-i;
> > +                riscv,isa-extension-m;
> > +                riscv,isa-extension-a;
> > +                riscv,isa-extension-f;
> > +                riscv,isa-extension-d;
> > +                riscv,isa-extension-c;
> >                  mmu-type = "riscv,sv48";
> >                  interrupt-controller {
> >                          #interrupt-cells = <1>;
> > diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > new file mode 100644
> > index 000000000000..1b4d726f7174
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > @@ -0,0 +1,259 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/riscv/extensions.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: RISC-V ISA extensions
> > +
> > +maintainers:
> > +  - Paul Walmsley <[e-mail]>
> > +  - Palmer Dabbelt <[e-mail]>
> > +  - Conor Dooley <[e-mail]>
> > +
> > +description: |
> > +  RISC-V has large number of extensions, some of which "standard" extensions,
>                ^ a                                       ^ are
>
> > +  meaning they are ratified by RISC-V International, and others are "vendor"
> > +  extensions.  This document defines properties that indicate whether a hart
> > +  supports a given extensions.
>
> drop 'a' or depluralize 'extensions'
>
> > +
> > +  Once a standard extension has been ratified, no features can be added or
>
> I'd change 'features' to 'changes in behavior', and then...
>
> > +  removed without the creation of a new extension for that sub- or super-set.
>
> ...drop 'for that sub- or super-set'
>
> > +  The properties for standard extensions therefore map to their originally
> > +  ratified states, with the exception of the I, Zicntr & Zihpm extensions.
>
> Can you elaborate on the exceptions? Or, if the exceptions are described
> below, maybe a '(see below)' here would help ease the reader's
> insecurities about their lack of knowledge about these exceptions, as
> they'll see that the education is coming :-)
>
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        const: riscv
> > +
> > +properties:
> > +  riscv,isa:
> > +    description:
> > +      Identifies the specific RISC-V instruction set architecture
> > +      supported by the hart.  These are documented in the RISC-V
> > +      User-Level ISA document, available from
> > +      https://riscv.org/specifications/
> > +
> > +      Due to revisions of the ISA specification, some deviations
> > +      have arisen over time.
> > +      Notably, riscv,isa was defined prior to the creation of the
> > +      Zicsr and Zifencei extensions and thus "i" implies
> > +      "zicsr_zifencei".
> > +
> > +      While the isa strings in ISA specification are case
>                                  ^ the
>
>                                  (but I see this was a faithful move
>                                  of the current text, so maybe better
>                                  to fix it up separately)
>
> > +      insensitive, letters in the riscv,isa string must be all
> > +      lowercase to simplify parsing.
> > +
> > +      This property has been deprecated due to disparity between the
> > +      extension at the time of its creation and ratification of the
> > +      base ISA.
> > +
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
> > +    deprecated: true
> > +
> > +  riscv,isa-base:
> > +    description:
> > +      The base ISA implemented by this hart, as described by the 20191213
> > +      version of the unprivileged ISA specification.
> > +    enum:
> > +      - rv32i
> > +      - rv64i
> > +
> > +  riscv,isa-extension-i:
> > +    type: boolean
> > +    description:
> > +      The base integer instruction set, as ratified in the 20191213 version of the
> > +      unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-m:
> > +    type: boolean
> > +    description:
> > +      The standard M extension for integer multiplication and division, as
> > +      ratified in the 20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-a:
> > +    type: boolean
> > +    description:
> > +      The standard A extension for atomic instructions, as ratified in the
> > +      20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-f:
> > +    type: boolean
> > +    description:
> > +      The standard M extension for single-precision floating point, as
>                       ^ F
>
> > +      ratified in the 20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-d:
> > +    type: boolean
> > +    description:
> > +      The standard M extension for double-precision floating-point, as
>                       ^ D
>
> > +      ratified in the 20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-q:
> > +    type: boolean
> > +    description:
> > +      The standard M extension for quad-precision floating-point, as ratified in
>                       ^ Q
>
> > +      the 20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-c:
> > +    type: boolean
> > +    description:
> > +      The standard M extension for compressed instructions, as ratified in the
>                       ^ C
>
> > +      20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-v:
> > +    type: boolean
> > +    description:
> > +      The standard V extension for vector operations, as ratified in-and-around
> > +      commit 7a6c8ae ("Fix text that describes vfmv.v.f encoding") of the
> > +      riscv-v-spec.
> > +
> > +  riscv,isa-extension-h:
> > +    type: boolean
> > +    description:
> > +      The standard h extension for hypervisors as ratified in the 20191213
>                       ^ H (might as well keep the case consistent)
>
> > +      version of the privileged ISA specification.
> > +
> > +  # Additional Standard Extensions, sorted by category then alphabetically
>
> Can we just do pure alphabetically? And the single-letter extensions above
> don't have a "sorted by" comment above them. I guess they need one, or
> maybe they can also be alphabetical?
>
> > +
> > +  riscv,isa-extension-zicntr:
> > +    type: boolean
> > +    description:
> > +      The standard Zicntr extension for base counters and timers, as ratified
> > +      in the 20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-zicsr:
> > +    type: boolean
> > +    description:
> > +      The standard Zicsr extension for control and status register instructions,
> > +      as ratified in the 20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-zifencei:
> > +    type: boolean
> > +    description:
> > +      The standard Zifencei extension for instruction-fetch fence, as ratified
> > +      in the 20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-zihpm:
> > +    type: boolean
> > +    description:
> > +      The standard Zihpm extension for hardware performance counters, as
> > +      ratified in the 20191213 version of the unprivileged ISA specification.
> > +
> > +  riscv,isa-extension-zicbom:
> > +    type: boolean
> > +    description:
> > +      The standard Zicbom extension for base cache management operations as
> > +      ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> > +
> > +  riscv,isa-extension-zicbop:
> > +    type: boolean
> > +    description:
> > +      The standard Zicbop extension for cache-block prefetch instructions as
> > +      ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> > +
> > +  riscv,isa-extension-zicboz:
> > +    type: boolean
> > +    description:
> > +      The standard  Zicbomz extension for cache-block zeroing as ratified in
>                      ^ ^Zicboz
>                      ^ extra space
>
> (The repetition is making my vision blur, so I'm feeling like I should
> write a script to compare $a and $b, where $a is riscv,isa-extension-$a
> and $b is 'The standard $b' to make sure they match :-) But I probably
> won't...
>
> > +      commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> > +
> > +  riscv,isa-extension-zihintpause:
> > +    type: boolean
> > +    description: |
> > +      The standard Zihintpause extension for pause hints, as ratified in
> > +      commit d8ab5c7 ("Zihintpause is ratified") of the riscv-isa-manual.
> > +
> > +  riscv,isa-extension-zba:
> > +    type: boolean
> > +    description: |
> > +      The standard Zba bit-manipulation extension for address generation
> > +      acceleration instructions as ratified at commit 6d33919 ("Merge pull
> > +      request #158 from hirooih/clmul-fix-loop-end-condition") of
> > +      riscv-bitmanip.
> > +
> > +  riscv,isa-extension-zbb:
> > +    type: boolean
> > +    description: |
> > +      The standard Zbb bit-manipulation extension for basic bit-manipulation as
> > +      atified at commit 6d33919 ("Merge pull request #158 from
>          ^ ratified
>
> > +      hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
> > +
> > +  riscv,isa-extension-zbc:
> > +    type: boolean
> > +    description: |
> > +      The standard Zbc bit-manipulation extension for carry-less multiplication
> > +      as ratified at commit 6d33919 ("Merge pull request #158 from
> > +      hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
> > +
> > +  riscv,isa-extension-zbs:
> > +    type: boolean
> > +    description: |
> > +      The standard Zbs bit-manipulation extension for single-bit instructions
> > +      as ratified at commit 6d33919 ("Merge pull request #158 from
> > +      hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
> > +
> > +  riscv,isa-extension-ztso:
> > +    type: boolean
> > +    description:
> > +      The standard Ztso extension for total store ordering, as ratified in
> > +      commit 2e5236 ("Ztso is now ratified.") of the riscv-isa-manual.
> > +
> > + # Standard Supervisor-level Extensions, sorted by category then alphabetically
>
> The spec only says alphabetical sorting for supervisor-level extensions,
> no category.
>
> > +
> > +  'riscv,isa-extension-smaia':
> > +    type: boolean
> > +    description: |
> > +      The standard Smaia supervisor-level extension for the advanced interrupt
> > +      architecture for machine-mode-visible csr and behavioural changes to
> > +      interrupts as frozen at commit ccbddab ("Merge pull request #42 from
> > +      riscv/jhauser-2023-RC4") of riscv-aia.
> > +
> > +  riscv,isa-extension-ssaia:
> > +    type: boolean
> > +    description: |
> > +      The standard Ssaia supervisor-level extension for the advanced interrupt
> > +      architecture for supervisor-mode-visible csr and behavioural changes to
> > +      interrupts as frozen at commit ccbddab ("Merge pull request #42 from
> > +      riscv/jhauser-2023-RC4") of riscv-aia.
> > +
> > +  riscv,isa-extension-sscofpmf:
> > +    type: boolean
> > +    description: |
> > +      The standard supervisor-level extension for count overflow and mode-based
>                      ^ Sscofpmf
>
> > +      filtering as ratified at commit 01d1df0 ("Add ability to manually trigger
> > +      workflow. (#2)") of riscv-count-overflow.
> > +
> > +  riscv,isa-extension-sstc:
> > +    type: boolean
> > +    description:
> > +      The standard supervisor-level extension for time compare
>                      ^ Sstc
>
> > +      as ratified at commit 3f9ed34 ("Add ability to manually trigger
> > +      workflow. (#2)") of riscv-time-compare.
> > +
> > +  riscv,isa-extension-svinval:
> > +    type: boolean
> > +    description:
> > +      The standard Svinval supervisor-level extension for fine-grained
> > +      address-translation cache invalidation as ratified in the 20191213 version
> > +      of the privileged ISA specification.
> > +
> > +  riscv,isa-extension-svnapot:
> > +    type: boolean
> > +    description:
> > +      The standard Svnapot supervisor-level extensions for napot translation
> > +      contiguity as ratified in the 20191213 version of the privileged ISA
> > +      specification.
> > +
> > +  riscv,isa-extension-svpbmt:
> > +    type: boolean
> > +    description:
> > +      The standard Svpbmt supervisor-level extensions for page-based memory
> > +      types as ratified in the 20191213 version of the privileged ISA
> > +      specification.
> > +
> > +additionalProperties: true
> > +...
> > --
> > 2.39.2
> >
>
> I'll take your word for it on the versions/dates/commit hashes referenced,
> at least until we get closer to actually merging this.
>
> Thanks,
> drew

Conor Dooley May 18, 2023, 2:06 p.m. UTC | #5

On Thu, May 18, 2023 at 07:13:15PM +0530, Anup Patel wrote:
> On Thu, May 18, 2023 at 4:02 PM Andrew Jones <[e-mail]> wrote:
> > On Thu, May 18, 2023 at 09:58:30AM +0100, Conor Dooley wrote:

> > > -  riscv,isa:
> > > -    description:
> > > -      Identifies the specific RISC-V instruction set architecture
> > > -      supported by the hart.  These are documented in the RISC-V
> > > -      User-Level ISA document, available from
> > > -      https://riscv.org/specifications/
> > > -
> > > -      Due to revisions of the ISA specification, some deviations
> > > -      have arisen over time.
> > > -      Notably, riscv,isa was defined prior to the creation of the
> > > -      Zicsr and Zifencei extensions and thus "i" implies
> > > -      "zicsr_zifencei".
> > > -
> > > -      While the isa strings in ISA specification are case
> > > -      insensitive, letters in the riscv,isa string must be all
> > > -      lowercase to simplify parsing.
> > > -    $ref: "/schemas/types.yaml#/definitions/string"
> > > -    pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
> > > -
> > >    # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
> > >    timebase-frequency: false
> > >
> > > @@ -133,8 +117,13 @@ properties:
> > >        DMIPS/MHz, relative to highest capacity-dmips-mhz
> > >        in the system.
> > >
> > > +oneOf:
> > > +  - required:
> > > +      - riscv,isa
> >
> > This is the part Anup keeps reminding me about. We can create better ways
> > to handle extensions in DT and ACPI, but we'll still need to parse ISA
> > strings to handle legacy DTs and holdouts that keep creating ISA strings,
> > at least during the deprecation period, since ISA strings are still "the
> > way to do it" according to the spec.
> 
> Coming up with an alternate way in DT is fine but we can't deprecate
> ISA strings since ISA strings are widely used:
> 1) Various bootloaders

Aye, for the reason, as I mentioned earlier and in the RFC thread,
removing existing parsers isn't a good idea.

> 2) It is part of /proc/cpuinfo

That is irrelevant.

> 3) Hypervisors use it to communicate HW features to Guest/VM.
> Hypervisors can't get away from generating ISA strings because
> Hypervisors don't know what is running inside Guest/VM.

Generate both :) As things stand, your guests could interpret what you
communicate to them via riscv,isa differently!

> In the case of ACPI, it is a very different situation. Like Sunil mentioned,
> ACPI will always follow mechanisms defined by RVI (such as ISA string).
> Other ACPI approaches such as GUID for ISA extension are simply not
> scalable and will take a lot more memory for ACPI tables compared to
> ISA strings.

My proposal should actually suit ACPI, at least for Linux, as it would
be a chance to align currently misaligned definitions. I won't speak to
GUIDs or whatever as that's someone else's problem :)

> > Also, if we assume the wording in the spec does get shored up, then,
> > unless I'm missing something, the list of advantages for this boolean
> > proposal from your commit message would be
> 
> IMO, we should try our best to have the wordings changed in RVI spec.

Yes, doing so is beneficial for all of us regardless of what happens
here. I do think that it is partially orthogonal - it allows us to not
design an interface that needs to be capable of communicating a wide
variety of versions, but I don't think it solves some of the issues
that riscv,isa has. If I thought it did, I would not have gone to the
trouble of respinning this patch out of the other approach.

> > * More character choices for name -- probably not a huge gain for ratified
> >   extensions, since the boolean properties will likely still use the same
> >   name as the ISA string (riscv,isa-extension-<name>). But, for vendor
> >   extensions, this is indeed a major improvement, since vendor extension
> >   boolean property names may need to be extended in unambiguous ways to
> >   handle changes in the extension.
> >
> > * Simpler, more complete DT validation (but we still need a best effort
> >   for legacy ISA strings)
> >
> > * Simpler DT parsing (but we still need the current parser for legacy ISA
> >   strings)
> >
> > > +  - required:
> > > +      - riscv,isa-base
> > > +
> > >  required:
> > > -  - riscv,isa
> > >    - interrupt-controller
> > >
> > >  additionalProperties: true
> > > @@ -177,7 +166,13 @@ examples:
> > >                  i-tlb-size = <32>;
> > >                  mmu-type = "riscv,sv39";
> > >                  reg = <1>;
> > > -                riscv,isa = "rv64imafdc";
> > > +                riscv,isa-base = "rv64i";
> > > +                riscv,isa-extension-i;
> > > +                riscv,isa-extension-m;
> > > +                riscv,isa-extension-a;
> > > +                riscv,isa-extension-f;
> > > +                riscv,isa-extension-d;
> > > +                riscv,isa-extension-c;
> 
> One downside of this new approach is it will increase the size of DTB.
> Imaging 50 such DT properties in 46 CPU DT nodes.

I should do a comparison between 50 extensions in riscv,isa and doing
this 50 times and see what the sizes are.

Palmer Dabbelt May 18, 2023, 2:41 p.m. UTC | #6

On Thu, 18 May 2023 07:06:17 PDT (-0700), Conor Dooley wrote:
> On Thu, May 18, 2023 at 07:13:15PM +0530, Anup Patel wrote:
>> On Thu, May 18, 2023 at 4:02 PM Andrew Jones <[e-mail]> wrote:
>> > On Thu, May 18, 2023 at 09:58:30AM +0100, Conor Dooley wrote:
>
>> > > -  riscv,isa:
>> > > -    description:
>> > > -      Identifies the specific RISC-V instruction set architecture
>> > > -      supported by the hart.  These are documented in the RISC-V
>> > > -      User-Level ISA document, available from
>> > > -      https://riscv.org/specifications/
>> > > -
>> > > -      Due to revisions of the ISA specification, some deviations
>> > > -      have arisen over time.
>> > > -      Notably, riscv,isa was defined prior to the creation of the
>> > > -      Zicsr and Zifencei extensions and thus "i" implies
>> > > -      "zicsr_zifencei".
>> > > -
>> > > -      While the isa strings in ISA specification are case
>> > > -      insensitive, letters in the riscv,isa string must be all
>> > > -      lowercase to simplify parsing.
>> > > -    $ref: "/schemas/types.yaml#/definitions/string"
>> > > -    pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
>> > > -
>> > >    # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
>> > >    timebase-frequency: false
>> > >
>> > > @@ -133,8 +117,13 @@ properties:
>> > >        DMIPS/MHz, relative to highest capacity-dmips-mhz
>> > >        in the system.
>> > >
>> > > +oneOf:
>> > > +  - required:
>> > > +      - riscv,isa
>> >
>> > This is the part Anup keeps reminding me about. We can create better ways
>> > to handle extensions in DT and ACPI, but we'll still need to parse ISA
>> > strings to handle legacy DTs and holdouts that keep creating ISA strings,
>> > at least during the deprecation period, since ISA strings are still "the
>> > way to do it" according to the spec.
>> 
>> Coming up with an alternate way in DT is fine but we can't deprecate
>> ISA strings since ISA strings are widely used:
>> 1) Various bootloaders
>
> Aye, for the reason, as I mentioned earlier and in the RFC thread,
> removing existing parsers isn't a good idea.

Removing and deprecating are different.  We can deprecate stuff.

>> 2) It is part of /proc/cpuinfo
>
> That is irrelevant.
>
>> 3) Hypervisors use it to communicate HW features to Guest/VM.
>> Hypervisors can't get away from generating ISA strings because
>> Hypervisors don't know what is running inside Guest/VM.
>
> Generate both :) As things stand, your guests could interpret what you
> communicate to them via riscv,isa differently!
>
>> In the case of ACPI, it is a very different situation. Like Sunil mentioned,
>> ACPI will always follow mechanisms defined by RVI (such as ISA string).
>> Other ACPI approaches such as GUID for ISA extension are simply not
>> scalable and will take a lot more memory for ACPI tables compared to
>> ISA strings.
>
> My proposal should actually suit ACPI, at least for Linux, as it would
> be a chance to align currently misaligned definitions. I won't speak to
> GUIDs or whatever as that's someone else's problem :)

We talked a bit in the patchwork meeting with Drew about ACPI.  Any 
actual spec/encoding would need to be different, of course, but 
conceptually it seems to fit fine.  It's also broadly similar to what 
we've done with riscv_hwprobe() for userspace, which is nice.

>> > Also, if we assume the wording in the spec does get shored up, then,
>> > unless I'm missing something, the list of advantages for this boolean
>> > proposal from your commit message would be
>> 
>> IMO, we should try our best to have the wordings changed in RVI spec.
>
> Yes, doing so is beneficial for all of us regardless of what happens
> here. I do think that it is partially orthogonal - it allows us to not
> design an interface that needs to be capable of communicating a wide
> variety of versions, but I don't think it solves some of the issues
> that riscv,isa has. If I thought it did, I would not have gone to the
> trouble of respinning this patch out of the other approach.
>
>> > * More character choices for name -- probably not a huge gain for ratified
>> >   extensions, since the boolean properties will likely still use the same
>> >   name as the ISA string (riscv,isa-extension-<name>). But, for vendor
>> >   extensions, this is indeed a major improvement, since vendor extension
>> >   boolean property names may need to be extended in unambiguous ways to
>> >   handle changes in the extension.
>> >
>> > * Simpler, more complete DT validation (but we still need a best effort
>> >   for legacy ISA strings)
>> >
>> > * Simpler DT parsing (but we still need the current parser for legacy ISA
>> >   strings)
>> >
>> > > +  - required:
>> > > +      - riscv,isa-base
>> > > +
>> > >  required:
>> > > -  - riscv,isa
>> > >    - interrupt-controller
>> > >
>> > >  additionalProperties: true
>> > > @@ -177,7 +166,13 @@ examples:
>> > >                  i-tlb-size = <32>;
>> > >                  mmu-type = "riscv,sv39";
>> > >                  reg = <1>;
>> > > -                riscv,isa = "rv64imafdc";
>> > > +                riscv,isa-base = "rv64i";
>> > > +                riscv,isa-extension-i;
>> > > +                riscv,isa-extension-m;
>> > > +                riscv,isa-extension-a;
>> > > +                riscv,isa-extension-f;
>> > > +                riscv,isa-extension-d;
>> > > +                riscv,isa-extension-c;
>> 
>> One downside of this new approach is it will increase the size of DTB.
>> Imaging 50 such DT properties in 46 CPU DT nodes.
>
> I should do a comparison between 50 extensions in riscv,isa and doing
> this 50 times and see what the sizes are.

I'm not sure how sensitive people are to DT size (presumably it'd be DTB 
size)?

It's also not clear what we can do about it: RISC-V has lots of 
extensions, that's going to take encoding space.  Sticking with an 
ambiguous encoding because it's smaller seems like a way to get 
burned in the long run.

Conor Dooley May 18, 2023, 5:06 p.m. UTC | #7

On Thu, May 18, 2023 at 07:41:17AM -0700, Palmer Dabbelt wrote:
> On Thu, 18 May 2023 07:06:17 PDT (-0700), Conor Dooley wrote:
> > On Thu, May 18, 2023 at 07:13:15PM +0530, Anup Patel wrote:
> > > On Thu, May 18, 2023 at 4:02 PM Andrew Jones <[e-mail]> wrote:
> > > > On Thu, May 18, 2023 at 09:58:30AM +0100, Conor Dooley wrote:

> > > One downside of this new approach is it will increase the size of DTB.
> > > Imaging 50 such DT properties in 46 CPU DT nodes.
> > 
> > I should do a comparison between 50 extensions in riscv,isa and doing
> > this 50 times and see what the sizes are.
> 
> I'm not sure how sensitive people are to DT size (presumably it'd be DTB
> size)?
> 
> It's also not clear what we can do about it: RISC-V has lots of extensions,
> that's going to take encoding space.  Sticking with an ambiguous encoding
> because it's smaller seems like a way to get burned in the long run.

I did actually go an look at this. I cheated a little and renamed the
properties to "riscv,isa-ext-foo", which is about as communicative IMO
as the longer name I currently have, but may seem more agreeable to the
size conscious.
I added 30 cpu nodes to mpfs.dtsi, each with 100 extensions of 6 chars
long. With just the string, containing "rv64imafdc_zabcde_...", it was
unreadable, but "only" took up 46k.
I then removed the multiletter extensions from riscv,isa & switched to
riscv,isa-base & 100 booleans. IMO it was more readable (although still
quite bad!), but took up 62k.
Removing all of the boolean properties, leaving me with 30 addtional harts
with "rv64imafdc" only, was 26k.

I think the generic limit for dtb files is 2 MiB? To me the size
increase doesn't sound like a bit problem.

Sean Anderson May 18, 2023, 6:30 p.m. UTC | #8

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

Why not just have something like

mycpu {
	...
	riscv,isa {
		i;
		m;
		a;
		zicsr;
		...
	};
};

?

--Sean

Conor Dooley May 18, 2023, 9:42 p.m. UTC | #9

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

Naming of the node aside (perhaps that could be riscv,isa-extensions)
there's not something hitting me immediately as to why that is a no-no.
If the size is a concern, this would certainly be more efficient & not
like the probing would be anything other than trivial more difficult
what I have in my proposal.

Rob's AFK at the moment, and I was hoping that he would take a look at
the idea, so I won't respin til he is back, but I'll give this a go in
the interim.

Cheers,
Conor.

Conor Dooley May 30, 2023, 2:12 p.m. UTC | #10

On Thu, May 18, 2023 at 10:42:34PM +0100, Conor Dooley wrote:
> On Thu, May 18, 2023 at 02:30:53PM -0400, Sean Anderson wrote:

> > 
> > Why not just have something like
> > 
> > mycpu {
> > 	...
> > 	riscv,isa {
> > 		i;
> > 		m;
> > 		a;
> > 		zicsr;
> > 		...
> > 	};
> > };
>
> Naming of the node aside (perhaps that could be riscv,isa-extensions)
> there's not something hitting me immediately as to why that is a no-no.
> If the size is a concern, this would certainly be more efficient & not
> like the probing would be anything other than trivial more difficult
> what I have in my proposal.

Having started messing around with this, one of the main advantages, to
me, of this approach is proper validation.
cpus.yaml has additionalProperties: true in it, which would have had to
be sorted out, or worked around, but creating a child-node with the
properties in it allows setting additionalProperties: false.

> Rob's AFK at the moment, and I was hoping that he would take a look at
> the idea, so I won't respin til he is back, but I'll give this a go in
> the interim.

Mechanically, the conversion of the patch isn't difficult, but I'll still
wait for Rob to come back before sending a v2. But that v2 will more
than likely implement your suggestion.

Cheers,
Conor.

Rob Herring (Arm) June 8, 2023, 7:15 p.m. UTC | #11

On Tue, May 30, 2023 at 03:12:12PM +0100, Conor Dooley wrote:
> On Thu, May 18, 2023 at 10:42:34PM +0100, Conor Dooley wrote:
> > On Thu, May 18, 2023 at 02:30:53PM -0400, Sean Anderson wrote:
> 
> > > 
> > > Why not just have something like
> > > 
> > > mycpu {
> > > 	...
> > > 	riscv,isa {
> > > 		i;
> > > 		m;
> > > 		a;
> > > 		zicsr;
> > > 		...

I prefer property names be globally unique. The tools are geared towards 
that too. That's largely a symptom of having 0 type information in the 
DT.

For example if you had an extension called 'reg', it would be a problem.

> > > 	};
> > > };
> >
> > Naming of the node aside (perhaps that could be riscv,isa-extensions)
> > there's not something hitting me immediately as to why that is a no-no.
> > If the size is a concern, this would certainly be more efficient & not
> > like the probing would be anything other than trivial more difficult
> > what I have in my proposal.
> 
> Having started messing around with this, one of the main advantages, to
> me, of this approach is proper validation.
> cpus.yaml has additionalProperties: true in it, which would have had to
> be sorted out, or worked around, but creating a child-node with the
> properties in it allows setting additionalProperties: false.

That's an issue on my radar to fix. I started that for the Arm cpus.yaml 
a while back. Sadly it involves adding all the misc properties vendors 
added. It's not a lot, but still better to get in front of that for 
Risc-V.

> > Rob's AFK at the moment, and I was hoping that he would take a look at
> > the idea, so I won't respin til he is back, but I'll give this a go in
> > the interim.
> 
> Mechanically, the conversion of the patch isn't difficult, but I'll still
> wait for Rob to come back before sending a v2. But that v2 will more
> than likely implement your suggestion.

I haven't read the whole thread, but the initial proposal looks okay to 
me.

Another way you could do this is a list of strings:

riscv,isa-ext = "i", "m", "zicsr";

I think we have a helper to test is a string in the list.

Rob

Conor Dooley June 8, 2023, 7:30 p.m. UTC | #12

On Thu, Jun 08, 2023 at 01:15:37PM -0600, Rob Herring wrote:
> On Tue, May 30, 2023 at 03:12:12PM +0100, Conor Dooley wrote:
> > On Thu, May 18, 2023 at 10:42:34PM +0100, Conor Dooley wrote:
> > > On Thu, May 18, 2023 at 02:30:53PM -0400, Sean Anderson wrote:
> > 
> > > > 
> > > > Why not just have something like
> > > > 
> > > > mycpu {
> > > > 	...
> > > > 	riscv,isa {
> > > > 		i;
> > > > 		m;
> > > > 		a;
> > > > 		zicsr;
> > > > 		...
> 
> I prefer property names be globally unique. The tools are geared towards 
> that too. That's largely a symptom of having 0 type information in the 
> DT.
> 
> For example if you had an extension called 'reg', it would be a problem.

Per the current ISA rules, that'd not be valid. But then again, I do
have trust issues & it's not like "reg" is the only property name in DT
land.

> > > Naming of the node aside (perhaps that could be riscv,isa-extensions)
> > > there's not something hitting me immediately as to why that is a no-no.
> > > If the size is a concern, this would certainly be more efficient & not
> > > like the probing would be anything other than trivial more difficult
> > > what I have in my proposal.
> > 
> > Having started messing around with this, one of the main advantages, to
> > me, of this approach is proper validation.
> > cpus.yaml has additionalProperties: true in it, which would have had to
> > be sorted out, or worked around, but creating a child-node with the
> > properties in it allows setting additionalProperties: false.
> 
> That's an issue on my radar to fix. I started that for the Arm cpus.yaml 
> a while back. Sadly it involves adding all the misc properties vendors 
> added. It's not a lot, but still better to get in front of that for 
> Risc-V.

Yeah, guess I can append that to my todo list.

> > > Rob's AFK at the moment, and I was hoping that he would take a look at
> > > the idea, so I won't respin til he is back, but I'll give this a go in
> > > the interim.
> > 
> > Mechanically, the conversion of the patch isn't difficult, but I'll still
> > wait for Rob to come back before sending a v2. But that v2 will more
> > than likely implement your suggestion.
> 
> I haven't read the whole thread, but the initial proposal looks okay to 
> me.
> 
> Another way you could do this is a list of strings:
> 
> riscv,isa-ext = "i", "m", "zicsr";
> 
> I think we have a helper to test is a string in the list.

Perhaps I should've waited a bit longer than a month for a v2 - I sent
one this afternoon doing what Sean suggested:
https://lore.kernel.org/all/20230608-sitting-bath-31eddc03c5a5@spud/

I'll do some poking at the strings and see what I think of it.

Cheers,
Conor.

Conor Dooley June 12, 2023, 9:23 p.m. UTC | #13

Rob,
Before I press on with more versions...

On Thu, Jun 08, 2023 at 08:30:28PM +0100, Conor Dooley wrote:
> On Thu, Jun 08, 2023 at 01:15:37PM -0600, Rob Herring wrote:
> > On Tue, May 30, 2023 at 03:12:12PM +0100, Conor Dooley wrote:
> > > On Thu, May 18, 2023 at 10:42:34PM +0100, Conor Dooley wrote:
> > > > On Thu, May 18, 2023 at 02:30:53PM -0400, Sean Anderson wrote:
> > > 
> > > > > 
> > > > > Why not just have something like
> > > > > 
> > > > > mycpu {
> > > > > 	...
> > > > > 	riscv,isa {
> > > > > 		i;
> > > > > 		m;
> > > > > 		a;
> > > > > 		zicsr;
> > > > > 		...
> > 
> > I prefer property names be globally unique. The tools are geared towards 
> > that too. That's largely a symptom of having 0 type information in the 
> > DT.
> > 
> > For example if you had an extension called 'reg', it would be a problem.
> 
> Per the current ISA rules, that'd not be valid. But then again, I do
> have trust issues & it's not like "reg" is the only property name in DT
> land.

...you say "prefer" here. Is that a NAK, or a "you keep the pieces"?

Rob Herring (Arm) June 13, 2023, 1:28 p.m. UTC | #14

On Mon, Jun 12, 2023 at 3:23 PM Conor Dooley <[e-mail]> wrote:
>
> Rob,
> Before I press on with more versions...
>
> On Thu, Jun 08, 2023 at 08:30:28PM +0100, Conor Dooley wrote:
> > On Thu, Jun 08, 2023 at 01:15:37PM -0600, Rob Herring wrote:
> > > On Tue, May 30, 2023 at 03:12:12PM +0100, Conor Dooley wrote:
> > > > On Thu, May 18, 2023 at 10:42:34PM +0100, Conor Dooley wrote:
> > > > > On Thu, May 18, 2023 at 02:30:53PM -0400, Sean Anderson wrote:
> > > >
> > > > > >
> > > > > > Why not just have something like
> > > > > >
> > > > > > mycpu {
> > > > > >       ...
> > > > > >       riscv,isa {
> > > > > >               i;
> > > > > >               m;
> > > > > >               a;
> > > > > >               zicsr;
> > > > > >               ...
> > >
> > > I prefer property names be globally unique. The tools are geared towards
> > > that too. That's largely a symptom of having 0 type information in the
> > > DT.
> > >
> > > For example if you had an extension called 'reg', it would be a problem.
> >
> > Per the current ISA rules, that'd not be valid. But then again, I do
> > have trust issues & it's not like "reg" is the only property name in DT
> > land.
>
> ...you say "prefer" here. Is that a NAK, or a "you keep the pieces"?

Don't do the above node.

Rob

Conor Dooley June 13, 2023, 2:11 p.m. UTC | #15

On Tue, Jun 13, 2023 at 07:28:34AM -0600, Rob Herring wrote:
> On Mon, Jun 12, 2023 at 3:23 PM Conor Dooley <[e-mail]> wrote:
> > On Thu, Jun 08, 2023 at 08:30:28PM +0100, Conor Dooley wrote:
> > > On Thu, Jun 08, 2023 at 01:15:37PM -0600, Rob Herring wrote:
> > > > On Tue, May 30, 2023 at 03:12:12PM +0100, Conor Dooley wrote:
> > > > > On Thu, May 18, 2023 at 10:42:34PM +0100, Conor Dooley wrote:
> > > > > > On Thu, May 18, 2023 at 02:30:53PM -0400, Sean Anderson wrote:
> > > > >
> > > > > > >
> > > > > > > Why not just have something like
> > > > > > >
> > > > > > > mycpu {
> > > > > > >       ...
> > > > > > >       riscv,isa {
> > > > > > >               i;
> > > > > > >               m;
> > > > > > >               a;
> > > > > > >               zicsr;
> > > > > > >               ...
> > > >
> > > > I prefer property names be globally unique. The tools are geared towards
> > > > that too. That's largely a symptom of having 0 type information in the
> > > > DT.
> > > >
> > > > For example if you had an extension called 'reg', it would be a problem.
> > >
> > > Per the current ISA rules, that'd not be valid. But then again, I do
> > > have trust issues & it's not like "reg" is the only property name in DT
> > > land.
> >
> > ...you say "prefer" here. Is that a NAK, or a "you keep the pieces"?
> 
> Don't do the above node.

Yeah, that's more helpful wording than "prefer" for sure!

If that's a no-go & so are the booleans prefixed with "riscv,whatever-",
since people have size concerns, I guess that leaves your string
suggestion (there is a helper in Linux at least, haven't checked
elsewhere yet).

I guess that means something like:

  riscv,isa-extensions:
    $ref: /schemas/types.yaml#/definitions/string-array
    minItems: 1
    description: Extensions supported by the hart.
    items:
      anyOf:
        - const: i
          description: foo
        - const: m
          description: foo
        - const: a
          description: foo
        - const: f
          description: foo
        - const: d
          description: foo
        - const: c
          description: foo
        - const: zifencei
          description: foo
        - etc

Obviously with "foo" replaced by the existing descriptions in this
patch.

Patch

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index 3d2934b15e80..446801fb7495 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -23,6 +23,9 @@  description: |
   two cores, each of which has two hyperthreads, could be described as
   having four harts.
 
+allOf:
+  - $ref: extensions.yaml
+
 properties:
   compatible:
     oneOf:
@@ -79,25 +82,6 @@  properties:
     description:
       The blocksize in bytes for the Zicboz cache operations.
 
-  riscv,isa:
-    description:
-      Identifies the specific RISC-V instruction set architecture
-      supported by the hart.  These are documented in the RISC-V
-      User-Level ISA document, available from
-      https://riscv.org/specifications/
-
-      Due to revisions of the ISA specification, some deviations
-      have arisen over time.
-      Notably, riscv,isa was defined prior to the creation of the
-      Zicsr and Zifencei extensions and thus "i" implies
-      "zicsr_zifencei".
-
-      While the isa strings in ISA specification are case
-      insensitive, letters in the riscv,isa string must be all
-      lowercase to simplify parsing.
-    $ref: "/schemas/types.yaml#/definitions/string"
-    pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
-
   # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
   timebase-frequency: false
 
@@ -133,8 +117,13 @@  properties:
       DMIPS/MHz, relative to highest capacity-dmips-mhz
       in the system.
 
+oneOf:
+  - required:
+      - riscv,isa
+  - required:
+      - riscv,isa-base
+
 required:
-  - riscv,isa
   - interrupt-controller
 
 additionalProperties: true
@@ -177,7 +166,13 @@  examples:
                 i-tlb-size = <32>;
                 mmu-type = "riscv,sv39";
                 reg = <1>;
-                riscv,isa = "rv64imafdc";
+                riscv,isa-base = "rv64i";
+                riscv,isa-extension-i;
+                riscv,isa-extension-m;
+                riscv,isa-extension-a;
+                riscv,isa-extension-f;
+                riscv,isa-extension-d;
+                riscv,isa-extension-c;
                 tlb-split;
                 cpu_intc1: interrupt-controller {
                         #interrupt-cells = <1>;
@@ -196,7 +191,13 @@  examples:
                 device_type = "cpu";
                 reg = <0>;
                 compatible = "riscv";
-                riscv,isa = "rv64imafdc";
+                riscv,isa-base = "rv64i";
+                riscv,isa-extension-i;
+                riscv,isa-extension-m;
+                riscv,isa-extension-a;
+                riscv,isa-extension-f;
+                riscv,isa-extension-d;
+                riscv,isa-extension-c;
                 mmu-type = "riscv,sv48";
                 interrupt-controller {
                         #interrupt-cells = <1>;
diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
new file mode 100644
index 000000000000..1b4d726f7174
--- /dev/null
+++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
@@ -0,0 +1,259 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR MIT)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/riscv/extensions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V ISA extensions
+
+maintainers:
+  - Paul Walmsley <[e-mail]>
+  - Palmer Dabbelt <[e-mail]>
+  - Conor Dooley <[e-mail]>
+
+description: |
+  RISC-V has large number of extensions, some of which "standard" extensions,
+  meaning they are ratified by RISC-V International, and others are "vendor"
+  extensions.  This document defines properties that indicate whether a hart
+  supports a given extensions.
+
+  Once a standard extension has been ratified, no features can be added or
+  removed without the creation of a new extension for that sub- or super-set.
+  The properties for standard extensions therefore map to their originally
+  ratified states, with the exception of the I, Zicntr & Zihpm extensions.
+
+select:
+  properties:
+    compatible:
+      contains:
+        const: riscv
+
+properties:
+  riscv,isa:
+    description:
+      Identifies the specific RISC-V instruction set architecture
+      supported by the hart.  These are documented in the RISC-V
+      User-Level ISA document, available from
+      https://riscv.org/specifications/
+
+      Due to revisions of the ISA specification, some deviations
+      have arisen over time.
+      Notably, riscv,isa was defined prior to the creation of the
+      Zicsr and Zifencei extensions and thus "i" implies
+      "zicsr_zifencei".
+
+      While the isa strings in ISA specification are case
+      insensitive, letters in the riscv,isa string must be all
+      lowercase to simplify parsing.
+
+      This property has been deprecated due to disparity between the
+      extension at the time of its creation and ratification of the
+      base ISA.
+
+    $ref: /schemas/types.yaml#/definitions/string
+    pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
+    deprecated: true
+
+  riscv,isa-base:
+    description:
+      The base ISA implemented by this hart, as described by the 20191213
+      version of the unprivileged ISA specification.
+    enum:
+      - rv32i
+      - rv64i
+
+  riscv,isa-extension-i:
+    type: boolean
+    description:
+      The base integer instruction set, as ratified in the 20191213 version of the
+      unprivileged ISA specification.
+
+  riscv,isa-extension-m:
+    type: boolean
+    description:
+      The standard M extension for integer multiplication and division, as
+      ratified in the 20191213 version of the unprivileged ISA specification.
+
+  riscv,isa-extension-a:
+    type: boolean
+    description:
+      The standard A extension for atomic instructions, as ratified in the
+      20191213 version of the unprivileged ISA specification.
+
+  riscv,isa-extension-f:
+    type: boolean
+    description:
+      The standard M extension for single-precision floating point, as
+      ratified in the 20191213 version of the unprivileged ISA specification.
+
+  riscv,isa-extension-d:
+    type: boolean
+    description:
+      The standard M extension for double-precision floating-point, as
+      ratified in the 20191213 version of the unprivileged ISA specification.
+
+  riscv,isa-extension-q:
+    type: boolean
+    description:
+      The standard M extension for quad-precision floating-point, as ratified in
+      the 20191213 version of the unprivileged ISA specification.
+
+  riscv,isa-extension-c:
+    type: boolean
+    description:
+      The standard M extension for compressed instructions, as ratified in the
+      20191213 version of the unprivileged ISA specification.
+
+  riscv,isa-extension-v:
+    type: boolean
+    description:
+      The standard V extension for vector operations, as ratified in-and-around
+      commit 7a6c8ae ("Fix text that describes vfmv.v.f encoding") of the
+      riscv-v-spec.
+
+  riscv,isa-extension-h:
+    type: boolean
+    description:
+      The standard h extension for hypervisors as ratified in the 20191213
+      version of the privileged ISA specification.
+
+  # Additional Standard Extensions, sorted by category then alphabetically
+
+  riscv,isa-extension-zicntr:
+    type: boolean
+    description:
+      The standard Zicntr extension for base counters and timers, as ratified
+      in the 20191213 version of the unprivileged ISA specification.
+
+  riscv,isa-extension-zicsr:
+    type: boolean
+    description:
+      The standard Zicsr extension for control and status register instructions,
+      as ratified in the 20191213 version of the unprivileged ISA specification.
+
+  riscv,isa-extension-zifencei:
+    type: boolean
+    description:
+      The standard Zifencei extension for instruction-fetch fence, as ratified
+      in the 20191213 version of the unprivileged ISA specification.
+
+  riscv,isa-extension-zihpm:
+    type: boolean
+    description:
+      The standard Zihpm extension for hardware performance counters, as
+      ratified in the 20191213 version of the unprivileged ISA specification.
+
+  riscv,isa-extension-zicbom:
+    type: boolean
+    description:
+      The standard Zicbom extension for base cache management operations as
+      ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
+
+  riscv,isa-extension-zicbop:
+    type: boolean
+    description:
+      The standard Zicbop extension for cache-block prefetch instructions as
+      ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
+
+  riscv,isa-extension-zicboz:
+    type: boolean
+    description:
+      The standard  Zicbomz extension for cache-block zeroing as ratified in
+      commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
+
+  riscv,isa-extension-zihintpause:
+    type: boolean
+    description: |
+      The standard Zihintpause extension for pause hints, as ratified in
+      commit d8ab5c7 ("Zihintpause is ratified") of the riscv-isa-manual.
+
+  riscv,isa-extension-zba:
+    type: boolean
+    description: |
+      The standard Zba bit-manipulation extension for address generation
+      acceleration instructions as ratified at commit 6d33919 ("Merge pull
+      request #158 from hirooih/clmul-fix-loop-end-condition") of
+      riscv-bitmanip.
+
+  riscv,isa-extension-zbb:
+    type: boolean
+    description: |
+      The standard Zbb bit-manipulation extension for basic bit-manipulation as
+      atified at commit 6d33919 ("Merge pull request #158 from
+      hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
+
+  riscv,isa-extension-zbc:
+    type: boolean
+    description: |
+      The standard Zbc bit-manipulation extension for carry-less multiplication
+      as ratified at commit 6d33919 ("Merge pull request #158 from
+      hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
+
+  riscv,isa-extension-zbs:
+    type: boolean
+    description: |
+      The standard Zbs bit-manipulation extension for single-bit instructions
+      as ratified at commit 6d33919 ("Merge pull request #158 from
+      hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
+
+  riscv,isa-extension-ztso:
+    type: boolean
+    description:
+      The standard Ztso extension for total store ordering, as ratified in
+      commit 2e5236 ("Ztso is now ratified.") of the riscv-isa-manual.
+
+ # Standard Supervisor-level Extensions, sorted by category then alphabetically
+
+  'riscv,isa-extension-smaia':
+    type: boolean
+    description: |
+      The standard Smaia supervisor-level extension for the advanced interrupt
+      architecture for machine-mode-visible csr and behavioural changes to
+      interrupts as frozen at commit ccbddab ("Merge pull request #42 from
+      riscv/jhauser-2023-RC4") of riscv-aia.
+
+  riscv,isa-extension-ssaia:
+    type: boolean
+    description: |
+      The standard Ssaia supervisor-level extension for the advanced interrupt
+      architecture for supervisor-mode-visible csr and behavioural changes to
+      interrupts as frozen at commit ccbddab ("Merge pull request #42 from
+      riscv/jhauser-2023-RC4") of riscv-aia.
+
+  riscv,isa-extension-sscofpmf:
+    type: boolean
+    description: |
+      The standard supervisor-level extension for count overflow and mode-based
+      filtering as ratified at commit 01d1df0 ("Add ability to manually trigger
+      workflow. (#2)") of riscv-count-overflow.
+
+  riscv,isa-extension-sstc:
+    type: boolean
+    description:
+      The standard supervisor-level extension for time compare
+      as ratified at commit 3f9ed34 ("Add ability to manually trigger
+      workflow. (#2)") of riscv-time-compare.
+
+  riscv,isa-extension-svinval:
+    type: boolean
+    description:
+      The standard Svinval supervisor-level extension for fine-grained
+      address-translation cache invalidation as ratified in the 20191213 version
+      of the privileged ISA specification.
+
+  riscv,isa-extension-svnapot:
+    type: boolean
+    description:
+      The standard Svnapot supervisor-level extensions for napot translation
+      contiguity as ratified in the 20191213 version of the privileged ISA
+      specification.
+
+  riscv,isa-extension-svpbmt:
+    type: boolean
+    description:
+      The standard Svpbmt supervisor-level extensions for page-based memory
+      types as ratified in the 20191213 version of the privileged ISA
+      specification.
+
+additionalProperties: true
+...