[v1] riscv: Add support for xtheadvector

[PATCH 02/13] dt-bindings: thead: add a vlen register length property

Posted by Charlie Jenkins 1 year, 8 months ago

Add a property analogous to the vlenb CSR so that software can detect
the vector length of each CPU prior to it being brought online.
Currently software has to assume that the vector length read from the
boot CPU applies to all possible CPUs. On T-Head CPUs implementing
pre-ratification vector, reading the th.vlenb CSR may produce an illegal
instruction trap, so this property is required on such systems.

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
---
 Documentation/devicetree/bindings/riscv/thead.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/thead.yaml b/Documentation/devicetree/bindings/riscv/thead.yaml
index 301912dcd290..5e578df36ac5 100644
--- a/Documentation/devicetree/bindings/riscv/thead.yaml
+++ b/Documentation/devicetree/bindings/riscv/thead.yaml
@@ -28,6 +28,13 @@ properties:
           - const: sipeed,lichee-module-4a
           - const: thead,th1520
 
+thead,vlenb:
+  $ref: /schemas/types.yaml#/definitions/uint32
+  description:
+    VLEN/8, the vector register length in bytes. This property is required in
+    systems where the vector register length is not identical on all harts, or
+    the vlenb CSR is not available.
+
 additionalProperties: true
 
 ...

-- 
2.44.0

Re: [PATCH 02/13] dt-bindings: thead: add a vlen register length property

Posted by Conor Dooley 1 year, 8 months ago

On Sun, Jun 09, 2024 at 09:45:07PM -0700, Charlie Jenkins wrote:
> Add a property analogous to the vlenb CSR so that software can detect
> the vector length of each CPU prior to it being brought online.
> Currently software has to assume that the vector length read from the
> boot CPU applies to all possible CPUs. On T-Head CPUs implementing
> pre-ratification vector, reading the th.vlenb CSR may produce an illegal
> instruction trap, so this property is required on such systems.
> 
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> ---
>  Documentation/devicetree/bindings/riscv/thead.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/thead.yaml b/Documentation/devicetree/bindings/riscv/thead.yaml
> index 301912dcd290..5e578df36ac5 100644
> --- a/Documentation/devicetree/bindings/riscv/thead.yaml
> +++ b/Documentation/devicetree/bindings/riscv/thead.yaml
> @@ -28,6 +28,13 @@ properties:
>            - const: sipeed,lichee-module-4a
>            - const: thead,th1520
>  
> +thead,vlenb:

This needs to move back into cpus.yaml, this file documents root node
compatibles (boards and socs etc) and is not for CPUs. If you want to
restrict this to T-Head CPUs only, it must be done in cpus.yaml with
a conditional `if: not: ... then: properties: thead,vlenb: false`.

Please test your bindings.

Thanks,
Conor.

> +  $ref: /schemas/types.yaml#/definitions/uint32
> +  description:
> +    VLEN/8, the vector register length in bytes. This property is required in
> +    systems where the vector register length is not identical on all harts, or
> +    the vlenb CSR is not available.
> +
>  additionalProperties: true
>  
>  ...
> 
> -- 
> 2.44.0
>

Re: [PATCH 02/13] dt-bindings: thead: add a vlen register length property

Posted by Charlie Jenkins 1 year, 8 months ago

On Mon, Jun 10, 2024 at 05:29:23PM +0100, Conor Dooley wrote:
> On Sun, Jun 09, 2024 at 09:45:07PM -0700, Charlie Jenkins wrote:
> > Add a property analogous to the vlenb CSR so that software can detect
> > the vector length of each CPU prior to it being brought online.
> > Currently software has to assume that the vector length read from the
> > boot CPU applies to all possible CPUs. On T-Head CPUs implementing
> > pre-ratification vector, reading the th.vlenb CSR may produce an illegal
> > instruction trap, so this property is required on such systems.
> > 
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > ---
> >  Documentation/devicetree/bindings/riscv/thead.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/riscv/thead.yaml b/Documentation/devicetree/bindings/riscv/thead.yaml
> > index 301912dcd290..5e578df36ac5 100644
> > --- a/Documentation/devicetree/bindings/riscv/thead.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/thead.yaml
> > @@ -28,6 +28,13 @@ properties:
> >            - const: sipeed,lichee-module-4a
> >            - const: thead,th1520
> >  
> > +thead,vlenb:
> 
> This needs to move back into cpus.yaml, this file documents root node
> compatibles (boards and socs etc) and is not for CPUs. If you want to
> restrict this to T-Head CPUs only, it must be done in cpus.yaml with
> a conditional `if: not: ... then: properties: thead,vlenb: false`.
> 
> Please test your bindings.

Now that I know `make dt_binding_check` exists I will use that in the
future!

- Charlie

> 
> Thanks,
> Conor.
> 
> > +  $ref: /schemas/types.yaml#/definitions/uint32
> > +  description:
> > +    VLEN/8, the vector register length in bytes. This property is required in
> > +    systems where the vector register length is not identical on all harts, or
> > +    the vlenb CSR is not available.
> > +
> >  additionalProperties: true
> >  
> >  ...
> > 
> > -- 
> > 2.44.0
> >

Re: [PATCH 02/13] dt-bindings: thead: add a vlen register length property

Posted by Conor Dooley 1 year, 8 months ago

On Mon, Jun 10, 2024 at 09:38:06AM -0700, Charlie Jenkins wrote:
> On Mon, Jun 10, 2024 at 05:29:23PM +0100, Conor Dooley wrote:
> > On Sun, Jun 09, 2024 at 09:45:07PM -0700, Charlie Jenkins wrote:
> > > Add a property analogous to the vlenb CSR so that software can detect
> > > the vector length of each CPU prior to it being brought online.
> > > Currently software has to assume that the vector length read from the
> > > boot CPU applies to all possible CPUs. On T-Head CPUs implementing
> > > pre-ratification vector, reading the th.vlenb CSR may produce an illegal
> > > instruction trap, so this property is required on such systems.
> > > 
> > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > > ---
> > >  Documentation/devicetree/bindings/riscv/thead.yaml | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/riscv/thead.yaml b/Documentation/devicetree/bindings/riscv/thead.yaml
> > > index 301912dcd290..5e578df36ac5 100644
> > > --- a/Documentation/devicetree/bindings/riscv/thead.yaml
> > > +++ b/Documentation/devicetree/bindings/riscv/thead.yaml
> > > @@ -28,6 +28,13 @@ properties:
> > >            - const: sipeed,lichee-module-4a
> > >            - const: thead,th1520
> > >  
> > > +thead,vlenb:
> > 
> > This needs to move back into cpus.yaml, this file documents root node
> > compatibles (boards and socs etc) and is not for CPUs. If you want to
> > restrict this to T-Head CPUs only, it must be done in cpus.yaml with
> > a conditional `if: not: ... then: properties: thead,vlenb: false`.
> > 
> > Please test your bindings.
> 
> Now that I know `make dt_binding_check` exists I will use that in the
> future!

And in this case dtbs_check would also complain about riscv,vlenb being
undocumented - and complain about thead,vlenb not being a permitted cpu
property either.

Thanks,
Conor.

Re: [PATCH 02/13] dt-bindings: thead: add a vlen register length property

Posted by Rob Herring (Arm) 1 year, 8 months ago

On Sun, 09 Jun 2024 21:45:07 -0700, Charlie Jenkins wrote:
> Add a property analogous to the vlenb CSR so that software can detect
> the vector length of each CPU prior to it being brought online.
> Currently software has to assume that the vector length read from the
> boot CPU applies to all possible CPUs. On T-Head CPUs implementing
> pre-ratification vector, reading the th.vlenb CSR may produce an illegal
> instruction trap, so this property is required on such systems.
> 
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> ---
>  Documentation/devicetree/bindings/riscv/thead.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/riscv/thead.yaml: 'thead,vlenb' is not one of ['$id', '$schema', 'title', 'description', 'examples', 'required', 'allOf', 'anyOf', 'oneOf', 'definitions', '$defs', 'additionalProperties', 'dependencies', 'dependentRequired', 'dependentSchemas', 'patternProperties', 'properties', 'not', 'if', 'then', 'else', 'unevaluatedProperties', 'deprecated', 'maintainers', 'select', '$ref']
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240609-xtheadvector-v1-2-3fe591d7f109@rivosinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.