| Message ID | 20240820093800.5436-3-mchitale@ventanamicro.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | Risc-V cache operations | expand |
On 20.08.24 11:37, Mayuresh Chitale wrote: > Define CBO inval and flush instructions and use those for the > dcache inval and flush operations respectively. > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> > --- > arch/riscv/Kconfig | 4 ++ > arch/riscv/include/asm/cache.h | 3 ++ > arch/riscv/lib/cache.c | 90 ++++++++++++++++++++++++++++++++++ > 3 files changed, 97 insertions(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index fa3b016c52..0f89d07be7 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -310,6 +310,10 @@ endmenu > config RISCV_ISA_A > def_bool y > > +config RISCV_ISA_ZICBOM > + bool "Zicbom support" > + depends on !SYS_DISABLE_DCACHE_OPS > + > config DMA_ADDR_T_64BIT > bool > default y if 64BIT > diff --git a/arch/riscv/include/asm/cache.h b/arch/riscv/include/asm/cache.h > index 874963d731..42dbce5b4f 100644 > --- a/arch/riscv/include/asm/cache.h > +++ b/arch/riscv/include/asm/cache.h > @@ -9,6 +9,9 @@ > > /* cache */ > void cache_flush(void); > +void riscv_zicbom_init(void); > +void cbo_flush(unsigned long start, unsigned long end); > +void cbo_inval(unsigned long start, unsigned long end); > > /* > * The current upper bound for RISCV L1 data cache line sizes is 32 bytes. > diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c > index afad7e117f..456353d9c1 100644 > --- a/arch/riscv/lib/cache.c > +++ b/arch/riscv/lib/cache.c > @@ -5,6 +5,95 @@ > */ > > #include <cpu_func.h> > +#include <dm.h> > +#include <asm/insn-def.h> > +#include <linux/const.h> > + > +#define CBO_INVAL(base) \ > + INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \ > + RS1(base), SIMM12(0)) > +#define CBO_CLEAN(base) \ > + INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \ > + RS1(base), SIMM12(1)) > +#define CBO_FLUSH(base) \ > + INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \ > + RS1(base), SIMM12(2)) > +enum { > + CBO_CLEAN, > + CBO_FLUSH, > + CBO_INVAL > +} riscv_cbo_ops; > +static int zicbom_block_size; > + > +static inline void do_cbo_clean(unsigned long base) > +{ > + asm volatile ("add a0, %0, zero\n" CBO_CLEAN(%0) :: > + "r"(base) : "memory"); > +} > + > +static inline void do_cbo_flush(unsigned long base) > +{ > + asm volatile ("add a0, %0, zero\n" CBO_FLUSH(%0) :: > + "r"(base) : "memory"); > +} > + > +static inline void do_cbo_inval(unsigned long base) > +{ > + asm volatile ("add a0, %0, zero\n" CBO_INVAL(%0) :: > + "r"(base) : "memory"); > +} > + > +static void cbo_op(int op_type, unsigned long start, > + unsigned long end) > +{ > + unsigned long op_size = end - start, size = 0; > + void (*fn)(unsigned long base); > + > + switch (op_type) { > + case CBO_CLEAN: > + fn = do_cbo_clean; > + break; > + case CBO_FLUSH: > + fn = do_cbo_flush; > + break; > + case CBO_INVAL: > + fn = do_cbo_inval; > + break; > + } > + start &= ~(UL(zicbom_block_size - 1)); > + while (size < op_size) { > + fn(start + size); > + size += zicbom_block_size; > + } > +} > + > +void cbo_flush(unsigned long start, unsigned long end) > +{ > + if (zicbom_block_size) > + cbo_op(CBO_FLUSH, start, end); > +} > + > +void cbo_inval(unsigned long start, unsigned long end) > +{ > + if (zicbom_block_size) > + cbo_op(CBO_INVAL, start, end); > +} > + > +void riscv_zicbom_init(void) > +{ > + struct udevice *dev; > + > + if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) > + return; > + > + uclass_first_device(UCLASS_CPU, &dev); > + if (!dev) { > + log_err("Failed to get cpu device!\n"); > + return; Please, return an error code. > + } > + > + (void)dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size); Please, do not ignore errors. Best regards Heinrich > +} > > void invalidate_icache_all(void) > { > @@ -72,4 +161,5 @@ __weak int dcache_status(void) > > __weak void enable_caches(void) > { > + puts("WARNING: Caches not enabled\n"); > }
On Tue, Aug 20, 2024 at 5:49 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 20.08.24 11:37, Mayuresh Chitale wrote: > > Define CBO inval and flush instructions and use those for the > > dcache inval and flush operations respectively. > > > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> > > --- > > arch/riscv/Kconfig | 4 ++ > > arch/riscv/include/asm/cache.h | 3 ++ > > arch/riscv/lib/cache.c | 90 ++++++++++++++++++++++++++++++++++ > > 3 files changed, 97 insertions(+) > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index fa3b016c52..0f89d07be7 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -310,6 +310,10 @@ endmenu > > config RISCV_ISA_A > > def_bool y > > > > +config RISCV_ISA_ZICBOM > > + bool "Zicbom support" > > + depends on !SYS_DISABLE_DCACHE_OPS > > + > > config DMA_ADDR_T_64BIT > > bool > > default y if 64BIT > > diff --git a/arch/riscv/include/asm/cache.h b/arch/riscv/include/asm/cache.h > > index 874963d731..42dbce5b4f 100644 > > --- a/arch/riscv/include/asm/cache.h > > +++ b/arch/riscv/include/asm/cache.h > > @@ -9,6 +9,9 @@ > > > > /* cache */ > > void cache_flush(void); > > +void riscv_zicbom_init(void); > > +void cbo_flush(unsigned long start, unsigned long end); > > +void cbo_inval(unsigned long start, unsigned long end); > > > > /* > > * The current upper bound for RISCV L1 data cache line sizes is 32 bytes. > > diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c > > index afad7e117f..456353d9c1 100644 > > --- a/arch/riscv/lib/cache.c > > +++ b/arch/riscv/lib/cache.c > > @@ -5,6 +5,95 @@ > > */ > > > > #include <cpu_func.h> > > +#include <dm.h> > > +#include <asm/insn-def.h> > > +#include <linux/const.h> > > + > > +#define CBO_INVAL(base) \ > > + INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \ > > + RS1(base), SIMM12(0)) > > +#define CBO_CLEAN(base) \ > > + INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \ > > + RS1(base), SIMM12(1)) > > +#define CBO_FLUSH(base) \ > > + INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \ > > + RS1(base), SIMM12(2)) > > +enum { > > + CBO_CLEAN, > > + CBO_FLUSH, > > + CBO_INVAL > > +} riscv_cbo_ops; > > +static int zicbom_block_size; > > + > > +static inline void do_cbo_clean(unsigned long base) > > +{ > > + asm volatile ("add a0, %0, zero\n" CBO_CLEAN(%0) :: > > + "r"(base) : "memory"); > > +} > > + > > +static inline void do_cbo_flush(unsigned long base) > > +{ > > + asm volatile ("add a0, %0, zero\n" CBO_FLUSH(%0) :: > > + "r"(base) : "memory"); > > +} > > + > > +static inline void do_cbo_inval(unsigned long base) > > +{ > > + asm volatile ("add a0, %0, zero\n" CBO_INVAL(%0) :: > > + "r"(base) : "memory"); > > +} > > + > > +static void cbo_op(int op_type, unsigned long start, > > + unsigned long end) > > +{ > > + unsigned long op_size = end - start, size = 0; > > + void (*fn)(unsigned long base); > > + > > + switch (op_type) { > > + case CBO_CLEAN: > > + fn = do_cbo_clean; > > + break; > > + case CBO_FLUSH: > > + fn = do_cbo_flush; > > + break; > > + case CBO_INVAL: > > + fn = do_cbo_inval; > > + break; > > + } > > + start &= ~(UL(zicbom_block_size - 1)); > > + while (size < op_size) { > > + fn(start + size); > > + size += zicbom_block_size; > > + } > > +} > > + > > +void cbo_flush(unsigned long start, unsigned long end) > > +{ > > + if (zicbom_block_size) > > + cbo_op(CBO_FLUSH, start, end); > > +} > > + > > +void cbo_inval(unsigned long start, unsigned long end) > > +{ > > + if (zicbom_block_size) > > + cbo_op(CBO_INVAL, start, end); > > +} > > + > > +void riscv_zicbom_init(void) > > +{ > > + struct udevice *dev; > > + > > + if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) > > + return; > > + > > + uclass_first_device(UCLASS_CPU, &dev); > > + if (!dev) { > > + log_err("Failed to get cpu device!\n"); > > + return; > > Please, return an error code. > > > + } > > + > > + (void)dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size); > > Please, do not ignore errors. Ok. > > Best regards > > Heinrich > > > +} > > > > void invalidate_icache_all(void) > > { > > @@ -72,4 +161,5 @@ __weak int dcache_status(void) > > > > __weak void enable_caches(void) > > { > > + puts("WARNING: Caches not enabled\n"); > > } >
On Tue, Aug 20, 2024 at 02:14:01PM +0200, Heinrich Schuchardt wrote: > On 20.08.24 11:37, Mayuresh Chitale wrote: > > +void riscv_zicbom_init(void) > > +{ > > + struct udevice *dev; > > + > > + if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) > > + return; > > + > > + uclass_first_device(UCLASS_CPU, &dev); > > + if (!dev) { > > + log_err("Failed to get cpu device!\n"); > > + return; > > Please, return an error code. > > > + } > > + > > + (void)dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size); > > Please, do not ignore errors. I'm curious what the policy of U-Boot is w.r.t. extension probing. Is it okay to enable the option for Zicbom even if your hardware does not support it, in which case riscv_zicbom_init() would be expected to fail gracefully and no CMOs done? Say, for example, you had two very similar chips, one with DMA non-coherent peripherals and one that only differed by having DMA coherent ones, and you wanted to run the same U-Boot binary on both devices using a devicetree passed from firmware.
On 21/08/2024 10:57, Conor Dooley wrote: > On Tue, Aug 20, 2024 at 02:14:01PM +0200, Heinrich Schuchardt wrote: >> On 20.08.24 11:37, Mayuresh Chitale wrote: >>> +void riscv_zicbom_init(void) >>> +{ >>> + struct udevice *dev; >>> + >>> + if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) >>> + return; >>> + >>> + uclass_first_device(UCLASS_CPU, &dev); >>> + if (!dev) { >>> + log_err("Failed to get cpu device!\n"); >>> + return; >> >> Please, return an error code. >> >>> + } >>> + >>> + (void)dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size); >> >> Please, do not ignore errors. > > I'm curious what the policy of U-Boot is w.r.t. extension probing. Is it > okay to enable the option for Zicbom even if your hardware does not > support it, in which case riscv_zicbom_init() would be expected to fail > gracefully and no CMOs done? > Say, for example, you had two very similar chips, one with DMA non-coherent > peripherals and one that only differed by having DMA coherent ones, and you > wanted to run the same U-Boot binary on both devices using a devicetree > passed from firmware. To be clear, I did notice that if the dev_read_u32() does not populate zicbom_block_size no cache ops will be done. My question was about the behaviour relating to the config option and or emitting warnings etc. Cheers, Conor.
On Wed, Aug 21, 2024 at 3:28 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > On Tue, Aug 20, 2024 at 02:14:01PM +0200, Heinrich Schuchardt wrote: > > On 20.08.24 11:37, Mayuresh Chitale wrote: > > > +void riscv_zicbom_init(void) > > > +{ > > > + struct udevice *dev; > > > + > > > + if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) > > > + return; > > > + > > > + uclass_first_device(UCLASS_CPU, &dev); > > > + if (!dev) { > > > + log_err("Failed to get cpu device!\n"); > > > + return; > > > > Please, return an error code. > > > > > + } > > > + > > > + (void)dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size); > > > > Please, do not ignore errors. > > I'm curious what the policy of U-Boot is w.r.t. extension probing. Is it > okay to enable the option for Zicbom even if your hardware does not > support it, in which case riscv_zicbom_init() would be expected to fail > gracefully and no CMOs done? > Say, for example, you had two very similar chips, one with DMA non-coherent > peripherals and one that only differed by having DMA coherent ones, and you > wanted to run the same U-Boot binary on both devices using a devicetree > passed from firmware. I think in such a case the config option can always be enabled with the proper dt being passed by the firmware.
On Wed, Aug 21, 2024 at 3:33 PM <Conor.Dooley@microchip.com> wrote: > > On 21/08/2024 10:57, Conor Dooley wrote: > > On Tue, Aug 20, 2024 at 02:14:01PM +0200, Heinrich Schuchardt wrote: > >> On 20.08.24 11:37, Mayuresh Chitale wrote: > >>> +void riscv_zicbom_init(void) > >>> +{ > >>> + struct udevice *dev; > >>> + > >>> + if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) > >>> + return; > >>> + > >>> + uclass_first_device(UCLASS_CPU, &dev); > >>> + if (!dev) { > >>> + log_err("Failed to get cpu device!\n"); > >>> + return; > >> > >> Please, return an error code. > >> > >>> + } > >>> + > >>> + (void)dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size); > >> > >> Please, do not ignore errors. > > > > I'm curious what the policy of U-Boot is w.r.t. extension probing. Is it > > okay to enable the option for Zicbom even if your hardware does not > > support it, in which case riscv_zicbom_init() would be expected to fail > > gracefully and no CMOs done? > > Say, for example, you had two very similar chips, one with DMA non-coherent > > peripherals and one that only differed by having DMA coherent ones, and you > > wanted to run the same U-Boot binary on both devices using a devicetree > > passed from firmware. > > To be clear, I did notice that if the dev_read_u32() does not populate > zicbom_block_size no cache ops will be done. My question was about > the behaviour relating to the config option and or emitting warnings etc. I am planning to add a info message if the config option is enabled but the dt property is not found. > > Cheers, > Conor.
On Wed, Aug 21, 2024 at 09:27:03PM +0530, Mayuresh Chitale wrote: > On Wed, Aug 21, 2024 at 3:28 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > On Tue, Aug 20, 2024 at 02:14:01PM +0200, Heinrich Schuchardt wrote: > > > On 20.08.24 11:37, Mayuresh Chitale wrote: > > > > +void riscv_zicbom_init(void) > > > > +{ > > > > + struct udevice *dev; > > > > + > > > > + if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) > > > > + return; > > > > + > > > > + uclass_first_device(UCLASS_CPU, &dev); > > > > + if (!dev) { > > > > + log_err("Failed to get cpu device!\n"); > > > > + return; > > > > > > Please, return an error code. > > > > > > > + } > > > > + > > > > + (void)dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size); > > > > > > Please, do not ignore errors. > > > > I'm curious what the policy of U-Boot is w.r.t. extension probing. Is it > > okay to enable the option for Zicbom even if your hardware does not > > support it, in which case riscv_zicbom_init() would be expected to fail > > gracefully and no CMOs done? > > Say, for example, you had two very similar chips, one with DMA non-coherent > > peripherals and one that only differed by having DMA coherent ones, and you > > wanted to run the same U-Boot binary on both devices using a devicetree > > passed from firmware. > I think in such a case the config option can always be enabled with > the proper dt being passed by the firmware. I don't understand what you mean, sorry. Both cases I mention would have a "proper" devicetree that matches the hardware. I'm interested in whether or not U-Boot's maintainers think that enabling the config option should warn on, or preclude use of, systems that do not not support Zicbom. Cheers, Conor. Also, technically there's no guarantee that "riscv,cbom-block-size" being present means that zicbom also is. Realistically, it should never be the case that it is present without zicbom in the isa string representation, but correctness would require checking that it is. supports_extension() only supports single-letter extensions at present, making it support multi-letter extensions should not be too difficult.
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index fa3b016c52..0f89d07be7 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -310,6 +310,10 @@ endmenu config RISCV_ISA_A def_bool y +config RISCV_ISA_ZICBOM + bool "Zicbom support" + depends on !SYS_DISABLE_DCACHE_OPS + config DMA_ADDR_T_64BIT bool default y if 64BIT diff --git a/arch/riscv/include/asm/cache.h b/arch/riscv/include/asm/cache.h index 874963d731..42dbce5b4f 100644 --- a/arch/riscv/include/asm/cache.h +++ b/arch/riscv/include/asm/cache.h @@ -9,6 +9,9 @@ /* cache */ void cache_flush(void); +void riscv_zicbom_init(void); +void cbo_flush(unsigned long start, unsigned long end); +void cbo_inval(unsigned long start, unsigned long end); /* * The current upper bound for RISCV L1 data cache line sizes is 32 bytes. diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c index afad7e117f..456353d9c1 100644 --- a/arch/riscv/lib/cache.c +++ b/arch/riscv/lib/cache.c @@ -5,6 +5,95 @@ */ #include <cpu_func.h> +#include <dm.h> +#include <asm/insn-def.h> +#include <linux/const.h> + +#define CBO_INVAL(base) \ + INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \ + RS1(base), SIMM12(0)) +#define CBO_CLEAN(base) \ + INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \ + RS1(base), SIMM12(1)) +#define CBO_FLUSH(base) \ + INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \ + RS1(base), SIMM12(2)) +enum { + CBO_CLEAN, + CBO_FLUSH, + CBO_INVAL +} riscv_cbo_ops; +static int zicbom_block_size; + +static inline void do_cbo_clean(unsigned long base) +{ + asm volatile ("add a0, %0, zero\n" CBO_CLEAN(%0) :: + "r"(base) : "memory"); +} + +static inline void do_cbo_flush(unsigned long base) +{ + asm volatile ("add a0, %0, zero\n" CBO_FLUSH(%0) :: + "r"(base) : "memory"); +} + +static inline void do_cbo_inval(unsigned long base) +{ + asm volatile ("add a0, %0, zero\n" CBO_INVAL(%0) :: + "r"(base) : "memory"); +} + +static void cbo_op(int op_type, unsigned long start, + unsigned long end) +{ + unsigned long op_size = end - start, size = 0; + void (*fn)(unsigned long base); + + switch (op_type) { + case CBO_CLEAN: + fn = do_cbo_clean; + break; + case CBO_FLUSH: + fn = do_cbo_flush; + break; + case CBO_INVAL: + fn = do_cbo_inval; + break; + } + start &= ~(UL(zicbom_block_size - 1)); + while (size < op_size) { + fn(start + size); + size += zicbom_block_size; + } +} + +void cbo_flush(unsigned long start, unsigned long end) +{ + if (zicbom_block_size) + cbo_op(CBO_FLUSH, start, end); +} + +void cbo_inval(unsigned long start, unsigned long end) +{ + if (zicbom_block_size) + cbo_op(CBO_INVAL, start, end); +} + +void riscv_zicbom_init(void) +{ + struct udevice *dev; + + if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) + return; + + uclass_first_device(UCLASS_CPU, &dev); + if (!dev) { + log_err("Failed to get cpu device!\n"); + return; + } + + (void)dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size); +} void invalidate_icache_all(void) { @@ -72,4 +161,5 @@ __weak int dcache_status(void) __weak void enable_caches(void) { + puts("WARNING: Caches not enabled\n"); }
Define CBO inval and flush instructions and use those for the dcache inval and flush operations respectively. Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> --- arch/riscv/Kconfig | 4 ++ arch/riscv/include/asm/cache.h | 3 ++ arch/riscv/lib/cache.c | 90 ++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+)