This series is based on the alternatives changes done in my svpbmt series
and thus also depends on Atish's isa-extension parsing series.
It implements using the cache-management instructions from the Zicbom-
extension to handle cache flush, etc actions on platforms needing them.
SoCs using cpu cores from T-Head like the Allwinne D1 implement a
different set of cache instructions. But while they are different,
instructions they provide the same functionality, so a variant can
easly hook into the existing alternatives mechanism on those.
As Palmer suggested, merging might have to wait until the cache
instructions have landed in compilers, but I wanted to put the
block-size changes out there for people to look at already and
also update the series to match the current svpbmt state.
changes in v2:
- cbom-block-size is hardware-specific and comes from firmware
- update Kconfig name to use the ISA extension name
- select the ALTERNATIVES symbol when enabled
- shorten the line lengths of the errata-assembly
Heiko Stuebner (3):
dt-bindings: riscv: document cbom-block-size
riscv: Implement Zicbom-based cache management operations
riscv: implement cache-management errata for T-Head SoCs
.../devicetree/bindings/riscv/cpus.yaml | 7 ++
arch/riscv/Kconfig | 15 +++
arch/riscv/Kconfig.erratas | 10 ++
arch/riscv/errata/thead/errata.c | 5 +
arch/riscv/include/asm/cacheflush.h | 6 ++
arch/riscv/include/asm/errata_list.h | 80 +++++++++++++++-
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/kernel/cpu.c | 1 +
arch/riscv/kernel/cpufeature.c | 17 ++++
arch/riscv/kernel/setup.c | 2 +
arch/riscv/mm/Makefile | 1 +
arch/riscv/mm/dma-noncoherent.c | 92 +++++++++++++++++++
12 files changed, 235 insertions(+), 2 deletions(-)
create mode 100644 arch/riscv/mm/dma-noncoherent.c
--
2.35.1
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 <[email protected]>
---
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
The T-Head C906 and C910 implement a scheme for handling
cache operations different from the generic Zicbom extension.
Add an errata for it next to the generic dma coherency ops.
Tested-by: Samuel Holland <[email protected]>
Signed-off-by: Heiko Stuebner <[email protected]>
---
arch/riscv/Kconfig.erratas | 10 ++++++
arch/riscv/errata/thead/errata.c | 5 +++
arch/riscv/include/asm/errata_list.h | 47 +++++++++++++++++++++++++---
3 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
index ebfcd5cc6eaf..213629bac5d7 100644
--- a/arch/riscv/Kconfig.erratas
+++ b/arch/riscv/Kconfig.erratas
@@ -54,4 +54,14 @@ config ERRATA_THEAD_PBMT
If you don't know what to do here, say "Y".
+config ERRATA_THEAD_CMO
+ bool "Apply T-Head cache management errata"
+ depends on ERRATA_THEAD && RISCV_ISA_ZICBOM
+ default y
+ help
+ This will apply the cache management errata to handle the
+ non-standard handling on non-coherent operations on T-Head SoCs.
+
+ If you don't know what to do here, say "Y".
+
endmenu
diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
index e5d75270b99c..9545f43d3504 100644
--- a/arch/riscv/errata/thead/errata.c
+++ b/arch/riscv/errata/thead/errata.c
@@ -33,6 +33,11 @@ static const struct errata_info errata_list[ERRATA_THEAD_NUMBER] = {
.stage = RISCV_ALTERNATIVES_EARLY_BOOT,
.check_func = errata_mt_check_func
},
+ {
+ .name = "cache-management",
+ .stage = RISCV_ALTERNATIVES_BOOT,
+ .check_func = errata_mt_check_func
+ },
};
static u32 thead_errata_probe(unsigned int stage, unsigned long archid, unsigned long impid)
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index eebcd4415049..1da311fc5126 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -16,7 +16,8 @@
#ifdef CONFIG_ERRATA_THEAD
#define ERRATA_THEAD_PBMT 0
-#define ERRATA_THEAD_NUMBER 1
+#define ERRATA_THEAD_CMO 1
+#define ERRATA_THEAD_NUMBER 2
#endif
#define CPUFEATURE_SVPBMT 0
@@ -111,8 +112,37 @@ asm volatile(ALTERNATIVE( \
#define CBO_CLEAN_A0 ".long 0x25200F"
#define CBO_FLUSH_A0 ".long 0x05200F"
+/*
+ * dcache.ipa rs1 (invalidate, physical address)
+ * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
+ * 0000001 01010 rs1 000 00000 0001011
+ * dache.iva rs1 (invalida, virtual address)
+ * 0000001 00110 rs1 000 00000 0001011
+ *
+ * dcache.cpa rs1 (clean, physical address)
+ * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
+ * 0000001 01001 rs1 000 00000 0001011
+ * dcache.cva rs1 (clean, virtual address)
+ * 0000001 00100 rs1 000 00000 0001011
+ *
+ * dcache.cipa rs1 (clean then invalidate, physical address)
+ * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
+ * 0000001 01011 rs1 000 00000 0001011
+ * dcache.civa rs1 (... virtual address)
+ * 0000001 00111 rs1 000 00000 0001011
+ *
+ * sync.s (make sure all cache operations finished)
+ * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
+ * 0000000 11001 00000 000 00000 0001011
+ */
+#define THEAD_INVAL_A0 ".long 0x0265000b"
+#define THEAD_CLEAN_A0 ".long 0x0245000b"
+#define THEAD_FLUSH_A0 ".long 0x0275000b"
+#define THEAD_SYNC_S ".long 0x0190000b"
+
#define ALT_CMO_OP(_op, _start, _size, _cachesize) \
-asm volatile(ALTERNATIVE( \
+asm volatile(ALTERNATIVE_2( \
+ "nop\n\t" \
"nop\n\t" \
"nop\n\t" \
"nop\n\t" \
@@ -124,8 +154,17 @@ asm volatile(ALTERNATIVE( \
CBO_##_op##_A0 "\n\t" \
"add a0, a0, %0\n\t" \
"2:\n\t" \
- "bltu a0, %2, 3b\n\t", 0, \
- CPUFEATURE_CMO, CONFIG_RISCV_ISA_ZICBOM) \
+ "bltu a0, %2, 3b\n\t" \
+ "nop", 0, CPUFEATURE_CMO, CONFIG_RISCV_ISA_ZICBOM, \
+ "mv a0, %1\n\t" \
+ "j 2f\n\t" \
+ "3:\n\t" \
+ THEAD_##_op##_A0 "\n\t" \
+ "add a0, a0, %0\n\t" \
+ "2:\n\t" \
+ "bltu a0, %2, 3b\n\t" \
+ THEAD_SYNC_S, THEAD_VENDOR_ID, \
+ ERRATA_THEAD_CMO, CONFIG_ERRATA_THEAD_CMO) \
: : "r"(_cachesize), \
"r"(ALIGN((_start), (_cachesize))), \
"r"(ALIGN((_start) + (_size), (_cachesize))))
--
2.35.1
On Thu, May 12, 2022 at 3:11 AM Heiko Stuebner <[email protected]> wrote:
>
> The T-Head C906 and C910 implement a scheme for handling
> cache operations different from the generic Zicbom extension.
>
> Add an errata for it next to the generic dma coherency ops.
>
> Tested-by: Samuel Holland <[email protected]>
> Signed-off-by: Heiko Stuebner <[email protected]>
Looks good to me.
Reviewed-by: Anup Patel <[email protected]>
Regards,
Anup
> ---
> arch/riscv/Kconfig.erratas | 10 ++++++
> arch/riscv/errata/thead/errata.c | 5 +++
> arch/riscv/include/asm/errata_list.h | 47 +++++++++++++++++++++++++---
> 3 files changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
> index ebfcd5cc6eaf..213629bac5d7 100644
> --- a/arch/riscv/Kconfig.erratas
> +++ b/arch/riscv/Kconfig.erratas
> @@ -54,4 +54,14 @@ config ERRATA_THEAD_PBMT
>
> If you don't know what to do here, say "Y".
>
> +config ERRATA_THEAD_CMO
> + bool "Apply T-Head cache management errata"
> + depends on ERRATA_THEAD && RISCV_ISA_ZICBOM
> + default y
> + help
> + This will apply the cache management errata to handle the
> + non-standard handling on non-coherent operations on T-Head SoCs.
> +
> + If you don't know what to do here, say "Y".
> +
> endmenu
> diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> index e5d75270b99c..9545f43d3504 100644
> --- a/arch/riscv/errata/thead/errata.c
> +++ b/arch/riscv/errata/thead/errata.c
> @@ -33,6 +33,11 @@ static const struct errata_info errata_list[ERRATA_THEAD_NUMBER] = {
> .stage = RISCV_ALTERNATIVES_EARLY_BOOT,
> .check_func = errata_mt_check_func
> },
> + {
> + .name = "cache-management",
> + .stage = RISCV_ALTERNATIVES_BOOT,
> + .check_func = errata_mt_check_func
> + },
> };
>
> static u32 thead_errata_probe(unsigned int stage, unsigned long archid, unsigned long impid)
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index eebcd4415049..1da311fc5126 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -16,7 +16,8 @@
>
> #ifdef CONFIG_ERRATA_THEAD
> #define ERRATA_THEAD_PBMT 0
> -#define ERRATA_THEAD_NUMBER 1
> +#define ERRATA_THEAD_CMO 1
> +#define ERRATA_THEAD_NUMBER 2
> #endif
>
> #define CPUFEATURE_SVPBMT 0
> @@ -111,8 +112,37 @@ asm volatile(ALTERNATIVE( \
> #define CBO_CLEAN_A0 ".long 0x25200F"
> #define CBO_FLUSH_A0 ".long 0x05200F"
>
> +/*
> + * dcache.ipa rs1 (invalidate, physical address)
> + * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
> + * 0000001 01010 rs1 000 00000 0001011
> + * dache.iva rs1 (invalida, virtual address)
> + * 0000001 00110 rs1 000 00000 0001011
> + *
> + * dcache.cpa rs1 (clean, physical address)
> + * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
> + * 0000001 01001 rs1 000 00000 0001011
> + * dcache.cva rs1 (clean, virtual address)
> + * 0000001 00100 rs1 000 00000 0001011
> + *
> + * dcache.cipa rs1 (clean then invalidate, physical address)
> + * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
> + * 0000001 01011 rs1 000 00000 0001011
> + * dcache.civa rs1 (... virtual address)
> + * 0000001 00111 rs1 000 00000 0001011
> + *
> + * sync.s (make sure all cache operations finished)
> + * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
> + * 0000000 11001 00000 000 00000 0001011
> + */
> +#define THEAD_INVAL_A0 ".long 0x0265000b"
> +#define THEAD_CLEAN_A0 ".long 0x0245000b"
> +#define THEAD_FLUSH_A0 ".long 0x0275000b"
> +#define THEAD_SYNC_S ".long 0x0190000b"
> +
> #define ALT_CMO_OP(_op, _start, _size, _cachesize) \
> -asm volatile(ALTERNATIVE( \
> +asm volatile(ALTERNATIVE_2( \
> + "nop\n\t" \
> "nop\n\t" \
> "nop\n\t" \
> "nop\n\t" \
> @@ -124,8 +154,17 @@ asm volatile(ALTERNATIVE( \
> CBO_##_op##_A0 "\n\t" \
> "add a0, a0, %0\n\t" \
> "2:\n\t" \
> - "bltu a0, %2, 3b\n\t", 0, \
> - CPUFEATURE_CMO, CONFIG_RISCV_ISA_ZICBOM) \
> + "bltu a0, %2, 3b\n\t" \
> + "nop", 0, CPUFEATURE_CMO, CONFIG_RISCV_ISA_ZICBOM, \
> + "mv a0, %1\n\t" \
> + "j 2f\n\t" \
> + "3:\n\t" \
> + THEAD_##_op##_A0 "\n\t" \
> + "add a0, a0, %0\n\t" \
> + "2:\n\t" \
> + "bltu a0, %2, 3b\n\t" \
> + THEAD_SYNC_S, THEAD_VENDOR_ID, \
> + ERRATA_THEAD_CMO, CONFIG_ERRATA_THEAD_CMO) \
> : : "r"(_cachesize), \
> "r"(ALIGN((_start), (_cachesize))), \
> "r"(ALIGN((_start) + (_size), (_cachesize))))
> --
> 2.35.1
>
Reviewed-by: Guo Ren <[email protected]>
On Thu, May 12, 2022 at 12:41 PM Anup Patel <[email protected]> wrote:
>
> On Thu, May 12, 2022 at 3:11 AM Heiko Stuebner <[email protected]> wrote:
> >
> > The T-Head C906 and C910 implement a scheme for handling
> > cache operations different from the generic Zicbom extension.
> >
> > Add an errata for it next to the generic dma coherency ops.
> >
> > Tested-by: Samuel Holland <[email protected]>
> > Signed-off-by: Heiko Stuebner <[email protected]>
>
> Looks good to me.
>
> Reviewed-by: Anup Patel <[email protected]>
>
> Regards,
> Anup
>
> > ---
> > arch/riscv/Kconfig.erratas | 10 ++++++
> > arch/riscv/errata/thead/errata.c | 5 +++
> > arch/riscv/include/asm/errata_list.h | 47 +++++++++++++++++++++++++---
> > 3 files changed, 58 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
> > index ebfcd5cc6eaf..213629bac5d7 100644
> > --- a/arch/riscv/Kconfig.erratas
> > +++ b/arch/riscv/Kconfig.erratas
> > @@ -54,4 +54,14 @@ config ERRATA_THEAD_PBMT
> >
> > If you don't know what to do here, say "Y".
> >
> > +config ERRATA_THEAD_CMO
> > + bool "Apply T-Head cache management errata"
> > + depends on ERRATA_THEAD && RISCV_ISA_ZICBOM
> > + default y
> > + help
> > + This will apply the cache management errata to handle the
> > + non-standard handling on non-coherent operations on T-Head SoCs.
> > +
> > + If you don't know what to do here, say "Y".
> > +
> > endmenu
> > diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> > index e5d75270b99c..9545f43d3504 100644
> > --- a/arch/riscv/errata/thead/errata.c
> > +++ b/arch/riscv/errata/thead/errata.c
> > @@ -33,6 +33,11 @@ static const struct errata_info errata_list[ERRATA_THEAD_NUMBER] = {
> > .stage = RISCV_ALTERNATIVES_EARLY_BOOT,
> > .check_func = errata_mt_check_func
> > },
> > + {
> > + .name = "cache-management",
> > + .stage = RISCV_ALTERNATIVES_BOOT,
> > + .check_func = errata_mt_check_func
> > + },
> > };
> >
> > static u32 thead_errata_probe(unsigned int stage, unsigned long archid, unsigned long impid)
> > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > index eebcd4415049..1da311fc5126 100644
> > --- a/arch/riscv/include/asm/errata_list.h
> > +++ b/arch/riscv/include/asm/errata_list.h
> > @@ -16,7 +16,8 @@
> >
> > #ifdef CONFIG_ERRATA_THEAD
> > #define ERRATA_THEAD_PBMT 0
> > -#define ERRATA_THEAD_NUMBER 1
> > +#define ERRATA_THEAD_CMO 1
> > +#define ERRATA_THEAD_NUMBER 2
> > #endif
> >
> > #define CPUFEATURE_SVPBMT 0
> > @@ -111,8 +112,37 @@ asm volatile(ALTERNATIVE( \
> > #define CBO_CLEAN_A0 ".long 0x25200F"
> > #define CBO_FLUSH_A0 ".long 0x05200F"
> >
> > +/*
> > + * dcache.ipa rs1 (invalidate, physical address)
> > + * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
> > + * 0000001 01010 rs1 000 00000 0001011
> > + * dache.iva rs1 (invalida, virtual address)
> > + * 0000001 00110 rs1 000 00000 0001011
> > + *
> > + * dcache.cpa rs1 (clean, physical address)
> > + * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
> > + * 0000001 01001 rs1 000 00000 0001011
> > + * dcache.cva rs1 (clean, virtual address)
> > + * 0000001 00100 rs1 000 00000 0001011
> > + *
> > + * dcache.cipa rs1 (clean then invalidate, physical address)
> > + * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
> > + * 0000001 01011 rs1 000 00000 0001011
> > + * dcache.civa rs1 (... virtual address)
> > + * 0000001 00111 rs1 000 00000 0001011
> > + *
> > + * sync.s (make sure all cache operations finished)
> > + * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
> > + * 0000000 11001 00000 000 00000 0001011
> > + */
> > +#define THEAD_INVAL_A0 ".long 0x0265000b"
> > +#define THEAD_CLEAN_A0 ".long 0x0245000b"
> > +#define THEAD_FLUSH_A0 ".long 0x0275000b"
> > +#define THEAD_SYNC_S ".long 0x0190000b"
> > +
> > #define ALT_CMO_OP(_op, _start, _size, _cachesize) \
> > -asm volatile(ALTERNATIVE( \
> > +asm volatile(ALTERNATIVE_2( \
> > + "nop\n\t" \
> > "nop\n\t" \
> > "nop\n\t" \
> > "nop\n\t" \
> > @@ -124,8 +154,17 @@ asm volatile(ALTERNATIVE( \
> > CBO_##_op##_A0 "\n\t" \
> > "add a0, a0, %0\n\t" \
> > "2:\n\t" \
> > - "bltu a0, %2, 3b\n\t", 0, \
> > - CPUFEATURE_CMO, CONFIG_RISCV_ISA_ZICBOM) \
> > + "bltu a0, %2, 3b\n\t" \
> > + "nop", 0, CPUFEATURE_CMO, CONFIG_RISCV_ISA_ZICBOM, \
> > + "mv a0, %1\n\t" \
> > + "j 2f\n\t" \
> > + "3:\n\t" \
> > + THEAD_##_op##_A0 "\n\t" \
> > + "add a0, a0, %0\n\t" \
> > + "2:\n\t" \
> > + "bltu a0, %2, 3b\n\t" \
> > + THEAD_SYNC_S, THEAD_VENDOR_ID, \
> > + ERRATA_THEAD_CMO, CONFIG_ERRATA_THEAD_CMO) \
> > : : "r"(_cachesize), \
> > "r"(ALIGN((_start), (_cachesize))), \
> > "r"(ALIGN((_start) + (_size), (_cachesize))))
> > --
> > 2.35.1
> >
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
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 <[email protected]>
> ---
> 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 <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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 <[email protected]> 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 <[email protected]>
> > ---
> > 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
>