diff mbox series

[v2,1/3] dt-bindings: riscv: document cbom-block-size

Message ID 20220511214132.2281431-2-heiko@sntech.de (mailing list archive)
State New
Headers show
Series riscv: implement Zicbom-based CMO instructions + the t-head variant | expand

Commit Message

Heiko Stübner May 11, 2022, 9:41 p.m. UTC
The Zicbom operates on a block-size defined for the cpu-core,
which does not necessarily match other cache-sizes used.

So add the necessary property for the system to know the core's
block-size.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 Documentation/devicetree/bindings/riscv/cpus.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Anup Patel May 12, 2022, 4:18 a.m. UTC | #1
On Thu, May 12, 2022 at 3:11 AM Heiko Stuebner <heiko@sntech.de> wrote:
>
> The Zicbom operates on a block-size defined for the cpu-core,
> which does not necessarily match other cache-sizes used.
>
> So add the necessary property for the system to know the core's
> block-size.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index d632ac76532e..b179bfd155a3 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -63,6 +63,13 @@ properties:
>        - riscv,sv48
>        - riscv,none
>
> +  riscv,cbom-block-size:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Blocksize in bytes for the Zicbom cache operations. The block
> +      size is a property of the core itself and does not necessarily
> +      match other software defined cache sizes.
> +
>    riscv,isa:
>      description:
>        Identifies the specific RISC-V instruction set architecture
> --
> 2.35.1
>
Christoph Müllner May 13, 2022, 10:28 a.m. UTC | #2
Hi Anup and Heiko,

The CBO specification says:
"""
2.7. Software Discovery
The initial set of CMO extensions requires the following information
to be discovered by software:
• The size of the cache block for management and prefetch instructions
• The size of the cache block for zero instructions
"""

Therefore we should add riscv,cboz-block-size as well, or?
Additionally, should we add riscv,cbop-block-size as well or rename
riscv,cbom-block-size into
riscv,cbom-cbop-block-size to reflect that this size is also used for
prefetch instructions?

BR
Christoph

On Thu, May 12, 2022 at 6:18 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Thu, May 12, 2022 at 3:11 AM Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > The Zicbom operates on a block-size defined for the cpu-core,
> > which does not necessarily match other cache-sizes used.
> >
> > So add the necessary property for the system to know the core's
> > block-size.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>
> Looks good to me.
>
> Reviewed-by: Anup Patel <anup@brainfault.org>
>
> Regards,
> Anup
>
> > ---
> >  Documentation/devicetree/bindings/riscv/cpus.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > index d632ac76532e..b179bfd155a3 100644
> > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > @@ -63,6 +63,13 @@ properties:
> >        - riscv,sv48
> >        - riscv,none
> >
> > +  riscv,cbom-block-size:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Blocksize in bytes for the Zicbom cache operations. The block
> > +      size is a property of the core itself and does not necessarily
> > +      match other software defined cache sizes.
> > +
> >    riscv,isa:
> >      description:
> >        Identifies the specific RISC-V instruction set architecture
> > --
> > 2.35.1
> >
Rob Herring May 18, 2022, 12:25 a.m. UTC | #3
On Wed, May 11, 2022 at 11:41:30PM +0200, Heiko Stuebner wrote:
> The Zicbom operates on a block-size defined for the cpu-core,
> which does not necessarily match other cache-sizes used.
> 
> So add the necessary property for the system to know the core's
> block-size.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index d632ac76532e..b179bfd155a3 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -63,6 +63,13 @@ properties:
>        - riscv,sv48
>        - riscv,none
>  
> +  riscv,cbom-block-size:
> +    $ref: /schemas/types.yaml#/definitions/uint32

Any value 0-2^32 is valid? 

> +    description:
> +      Blocksize in bytes for the Zicbom cache operations. The block
> +      size is a property of the core itself and does not necessarily
> +      match other software defined cache sizes.

What about hardware defined cache sizes? I'm scratching my head as to 
what a 'software defined cache size' is.

> +
>    riscv,isa:
>      description:
>        Identifies the specific RISC-V instruction set architecture
> -- 
> 2.35.1
> 
>
Philipp Tomsich May 18, 2022, 8:22 a.m. UTC | #4
+David Kruckemyer (who is chairing the CMO task-group within RVI).

On Wed, 18 May 2022 at 02:25, Rob Herring <robh@kernel.org> wrote:
>
> On Wed, May 11, 2022 at 11:41:30PM +0200, Heiko Stuebner wrote:
> > The Zicbom operates on a block-size defined for the cpu-core,
> > which does not necessarily match other cache-sizes used.
> >
> > So add the necessary property for the system to know the core's
> > block-size.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  Documentation/devicetree/bindings/riscv/cpus.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > index d632ac76532e..b179bfd155a3 100644
> > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > @@ -63,6 +63,13 @@ properties:
> >        - riscv,sv48
> >        - riscv,none
> >
> > +  riscv,cbom-block-size:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
>
> Any value 0-2^32 is valid?
>
> > +    description:
> > +      Blocksize in bytes for the Zicbom cache operations. The block
> > +      size is a property of the core itself and does not necessarily
> > +      match other software defined cache sizes.
>
> What about hardware defined cache sizes? I'm scratching my head as to
> what a 'software defined cache size' is.

This seems to be a misnomer, as the specification doesn't use the term
and rather talks about the "size of a cache block for [operation
name]".

There are currently two such 'operation sizes' discoverable by software:
- size of the cache block for management and prefetch instructions
- size of the cache block for zero instructions

For whatever it's worth, cache operations in RISC-V attempt to
disassociate the underlying hardware cache geometry from software.
See https://github.com/riscv/riscv-CMOs/blob/master/specifications/cmobase-v1.0.1.pdf
for the CMO specification, and the discoverable parameters are listed
in section 2.7.

Philipp.

> > +
> >    riscv,isa:
> >      description:
> >        Identifies the specific RISC-V instruction set architecture
> > --
> > 2.35.1
> >
> >
Heiko Stübner May 18, 2022, 9:02 a.m. UTC | #5
Am Mittwoch, 18. Mai 2022, 10:22:17 CEST schrieb Philipp Tomsich:
> +David Kruckemyer (who is chairing the CMO task-group within RVI).
> 
> On Wed, 18 May 2022 at 02:25, Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, May 11, 2022 at 11:41:30PM +0200, Heiko Stuebner wrote:
> > > The Zicbom operates on a block-size defined for the cpu-core,
> > > which does not necessarily match other cache-sizes used.
> > >
> > > So add the necessary property for the system to know the core's
> > > block-size.
> > >
> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > ---
> > >  Documentation/devicetree/bindings/riscv/cpus.yaml | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > index d632ac76532e..b179bfd155a3 100644
> > > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > @@ -63,6 +63,13 @@ properties:
> > >        - riscv,sv48
> > >        - riscv,none
> > >
> > > +  riscv,cbom-block-size:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> >
> > Any value 0-2^32 is valid?
> >
> > > +    description:
> > > +      Blocksize in bytes for the Zicbom cache operations. The block
> > > +      size is a property of the core itself and does not necessarily
> > > +      match other software defined cache sizes.
> >
> > What about hardware defined cache sizes? I'm scratching my head as to
> > what a 'software defined cache size' is.

I agree that this should be worded better. The intent was to tell that this
is different from say the l1-cache-block-size.

I.e. these values can be the same but don't need to be. But I guess I got
too much lead on by a kernel implementation detail (L1_CACHE_BYTES constant)


> This seems to be a misnomer, as the specification doesn't use the term
> and rather talks about the "size of a cache block for [operation
> name]".
> 
> There are currently two such 'operation sizes' discoverable by software:
> - size of the cache block for management and prefetch instructions
> - size of the cache block for zero instructions
> 
> For whatever it's worth, cache operations in RISC-V attempt to
> disassociate the underlying hardware cache geometry from software.
> See https://github.com/riscv/riscv-CMOs/blob/master/specifications/cmobase-v1.0.1.pdf
> for the CMO specification, and the discoverable parameters are listed
> in section 2.7.
> 
> Philipp.
> 
> > > +
> > >    riscv,isa:
> > >      description:
> > >        Identifies the specific RISC-V instruction set architecture
> > > --
> > > 2.35.1
> > >
> > >
>
Anup Patel May 18, 2022, 9:10 a.m. UTC | #6
On Wed, May 18, 2022 at 2:33 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Am Mittwoch, 18. Mai 2022, 10:22:17 CEST schrieb Philipp Tomsich:
> > +David Kruckemyer (who is chairing the CMO task-group within RVI).
> >
> > On Wed, 18 May 2022 at 02:25, Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Wed, May 11, 2022 at 11:41:30PM +0200, Heiko Stuebner wrote:
> > > > The Zicbom operates on a block-size defined for the cpu-core,
> > > > which does not necessarily match other cache-sizes used.
> > > >
> > > > So add the necessary property for the system to know the core's
> > > > block-size.
> > > >
> > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > > ---
> > > >  Documentation/devicetree/bindings/riscv/cpus.yaml | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > > index d632ac76532e..b179bfd155a3 100644
> > > > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > > @@ -63,6 +63,13 @@ properties:
> > > >        - riscv,sv48
> > > >        - riscv,none
> > > >
> > > > +  riscv,cbom-block-size:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > >
> > > Any value 0-2^32 is valid?
> > >
> > > > +    description:
> > > > +      Blocksize in bytes for the Zicbom cache operations. The block
> > > > +      size is a property of the core itself and does not necessarily
> > > > +      match other software defined cache sizes.
> > >
> > > What about hardware defined cache sizes? I'm scratching my head as to
> > > what a 'software defined cache size' is.
>
> I agree that this should be worded better. The intent was to tell that this
> is different from say the l1-cache-block-size.
>
> I.e. these values can be the same but don't need to be. But I guess I got
> too much lead on by a kernel implementation detail (L1_CACHE_BYTES constant)

Better to just call it as "the cache block-size expected by Zicbom cache
operations" without getting details of relation with L1 cache block size.

Regards,
Anup

>
>
> > This seems to be a misnomer, as the specification doesn't use the term
> > and rather talks about the "size of a cache block for [operation
> > name]".
> >
> > There are currently two such 'operation sizes' discoverable by software:
> > - size of the cache block for management and prefetch instructions
> > - size of the cache block for zero instructions
> >
> > For whatever it's worth, cache operations in RISC-V attempt to
> > disassociate the underlying hardware cache geometry from software.
> > See https://github.com/riscv/riscv-CMOs/blob/master/specifications/cmobase-v1.0.1.pdf
> > for the CMO specification, and the discoverable parameters are listed
> > in section 2.7.
> >
> > Philipp.
> >
> > > > +
> > > >    riscv,isa:
> > > >      description:
> > > >        Identifies the specific RISC-V instruction set architecture
> > > > --
> > > > 2.35.1
> > > >
> > > >
> >
>
>
>
>
Philipp Tomsich May 18, 2022, 9:20 a.m. UTC | #7
On Wed, 18 May 2022 at 11:10, Anup Patel <anup@brainfault.org> wrote:
> > > > > +    description:
> > > > > +      Blocksize in bytes for the Zicbom cache operations. The block
> > > > > +      size is a property of the core itself and does not necessarily
> > > > > +      match other software defined cache sizes.
> > > >
> > > > What about hardware defined cache sizes? I'm scratching my head as to
> > > > what a 'software defined cache size' is.
> >
> > I agree that this should be worded better. The intent was to tell that this
> > is different from say the l1-cache-block-size.
> >
> > I.e. these values can be the same but don't need to be. But I guess I got
> > too much lead on by a kernel implementation detail (L1_CACHE_BYTES constant)
>
> Better to just call it as "the cache block-size expected by Zicbom cache
> operations" without getting details of relation with L1 cache block size.

I would make this an even stronger statement and assert that Anup's
recommended rewording (and staying away from L1 block/line sizes in
terminology) is required to accurately reflect the design of the
RISC-V CMOs.

The Zicbom operation size is in fact decoupled from the
l1-cache-block-size (as that would be the cache line size — and
therefore the size of fetches/replacements to the cache) as the
deliberations within the CMO group showed.   This is only the granule
that Zicbom instructions operate on (and there might be additional
mechanisms at work in the background that ensure that this is safe for
any given underlying cache implementation).

Cheers,
Philipp.
Heiko Stübner May 25, 2022, 3:14 p.m. UTC | #8
Am Mittwoch, 18. Mai 2022, 02:25:29 CEST schrieb Rob Herring:
> On Wed, May 11, 2022 at 11:41:30PM +0200, Heiko Stuebner wrote:
> > The Zicbom operates on a block-size defined for the cpu-core,
> > which does not necessarily match other cache-sizes used.
> > 
> > So add the necessary property for the system to know the core's
> > block-size.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  Documentation/devicetree/bindings/riscv/cpus.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > index d632ac76532e..b179bfd155a3 100644
> > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > @@ -63,6 +63,13 @@ properties:
> >        - riscv,sv48
> >        - riscv,none
> >  
> > +  riscv,cbom-block-size:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> Any value 0-2^32 is valid?

funnily enough there really seems to be _no_ constraints defined in the
spec [0] regarding the actual cache-block size.

It essentially only states
"The capacity and organization of a cache and the size of a
cache block are both implementation-specific"

and later in software-discovery:
"The initial set of CMO extensions requires the following information to
be discovered by software:
- The size of the cache block for management and prefetch instructions
- The size of the cache block for zero instructions"



[0] https://github.com/riscv/riscv-CMOs/blob/master/specifications/cmobase-v1.0.pdf


> 
> > +    description:
> > +      Blocksize in bytes for the Zicbom cache operations. The block
> > +      size is a property of the core itself and does not necessarily
> > +      match other software defined cache sizes.
> 
> What about hardware defined cache sizes? I'm scratching my head as to 
> what a 'software defined cache size' is.
> 
> > +
> >    riscv,isa:
> >      description:
> >        Identifies the specific RISC-V instruction set architecture
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index d632ac76532e..b179bfd155a3 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -63,6 +63,13 @@  properties:
       - riscv,sv48
       - riscv,none
 
+  riscv,cbom-block-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Blocksize in bytes for the Zicbom cache operations. The block
+      size is a property of the core itself and does not necessarily
+      match other software defined cache sizes.
+
   riscv,isa:
     description:
       Identifies the specific RISC-V instruction set architecture