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