| Message ID | 20240820093800.5436-4-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 enable_caches function for the qemu-riscv board which probes for > the cbom-block-size dt property when RISCV_ISA_ZICBOM is enabled. Also > add flush_dcache_range and invalidate_dcache_range functions which use > the corresponding CBO ops. > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> > --- > board/emulation/qemu-riscv/qemu-riscv.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c > index e5193e31e3..1795d2f831 100644 > --- a/board/emulation/qemu-riscv/qemu-riscv.c > +++ b/board/emulation/qemu-riscv/qemu-riscv.c > @@ -14,6 +14,7 @@ > #include <usb.h> > #include <virtio_types.h> > #include <virtio.h> > +#include <asm/cache.h> > > DECLARE_GLOBAL_DATA_PTR; > > @@ -70,3 +71,18 @@ void *board_fdt_blob_setup(int *err) > /* Stored the DTB address there during our init */ > return (void *)(ulong)gd->arch.firmware_fdt_addr; > } > + > +void enable_caches(void) > +{ > + riscv_zicbom_init(); > +} Enable caches may be called multiple times. But riscv_zicbom_init() only needs to be called once. > + > +void flush_dcache_range(unsigned long start, unsigned long end) > +{ > + cbo_flush(start, end); > +} > + > +void invalidate_dcache_range(unsigned long start, unsigned long end) > +{ > + cbo_inval(start, end); > +} The ZICBOM extension is not specific to a single board. We should not add this code to board files but into the central place for cache operations, i.e. arch/riscv/lib/cache.c. E.g. static int get_zicbom_block_size() { if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) return 0; if (zicbom_block_size) return zicbom_block_size; uclass_first_device(UCLASS_CPU, &dev); if (!dev) { log_err("Failed to get CPU device!\n"); return 0; } if (dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size)) return 0; return zicbom_block_size } __weak void invalidate_icache_range(unsigned long start, unsigned long end) { if (CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM) && get_zicbom_block_size()) cbo_inval(start, end); else invalidate_icache_all(); } Cc: Rick as one of the RISC-V maintainers. Best regards Heinrich
Hi Heinrich, On Tue, Aug 20, 2024 at 5:43 PM Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote: > > On 20.08.24 11:37, Mayuresh Chitale wrote: > > Define enable_caches function for the qemu-riscv board which probes for > > the cbom-block-size dt property when RISCV_ISA_ZICBOM is enabled. Also > > add flush_dcache_range and invalidate_dcache_range functions which use > > the corresponding CBO ops. > > > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> > > --- > > board/emulation/qemu-riscv/qemu-riscv.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c > > index e5193e31e3..1795d2f831 100644 > > --- a/board/emulation/qemu-riscv/qemu-riscv.c > > +++ b/board/emulation/qemu-riscv/qemu-riscv.c > > @@ -14,6 +14,7 @@ > > #include <usb.h> > > #include <virtio_types.h> > > #include <virtio.h> > > +#include <asm/cache.h> > > > > DECLARE_GLOBAL_DATA_PTR; > > > > @@ -70,3 +71,18 @@ void *board_fdt_blob_setup(int *err) > > /* Stored the DTB address there during our init */ > > return (void *)(ulong)gd->arch.firmware_fdt_addr; > > } > > + > > +void enable_caches(void) > > +{ > > + riscv_zicbom_init(); > > +} > > Enable caches may be called multiple times. But riscv_zicbom_init() only > needs to be called once. Ok. > > > + > > +void flush_dcache_range(unsigned long start, unsigned long end) > > +{ > > + cbo_flush(start, end); > > +} > > + > > +void invalidate_dcache_range(unsigned long start, unsigned long end) > > +{ > > + cbo_inval(start, end); > > +} > > The ZICBOM extension is not specific to a single board. We should not > add this code to board files but into the central place for cache > operations, i.e. arch/riscv/lib/cache.c. Most of the code is already cache.c. I didn't know if it would suffice for all platforms so I decided to override the weak functions like in ARM. > E.g. > > static int get_zicbom_block_size() > { > if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) > return 0; > if (zicbom_block_size) > return zicbom_block_size; > > uclass_first_device(UCLASS_CPU, &dev); > if (!dev) { > log_err("Failed to get CPU device!\n"); > return 0; > } > > if (dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size)) > return 0; > > return zicbom_block_size > } > > __weak void invalidate_icache_range(unsigned long start, unsigned long end) > { > if (CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM) && get_zicbom_block_size()) > cbo_inval(start, end); > else > invalidate_icache_all(); > } > > Cc: Rick as one of the RISC-V maintainers. Ok. > > Best regards > > Heinrich >
On 21.08.24 11:11, Mayuresh Chitale wrote: > Hi Heinrich, > > On Tue, Aug 20, 2024 at 5:43 PM Heinrich Schuchardt > <heinrich.schuchardt@canonical.com> wrote: >> >> On 20.08.24 11:37, Mayuresh Chitale wrote: >>> Define enable_caches function for the qemu-riscv board which probes for >>> the cbom-block-size dt property when RISCV_ISA_ZICBOM is enabled. Also >>> add flush_dcache_range and invalidate_dcache_range functions which use >>> the corresponding CBO ops. >>> >>> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> >>> --- >>> board/emulation/qemu-riscv/qemu-riscv.c | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c >>> index e5193e31e3..1795d2f831 100644 >>> --- a/board/emulation/qemu-riscv/qemu-riscv.c >>> +++ b/board/emulation/qemu-riscv/qemu-riscv.c >>> @@ -14,6 +14,7 @@ >>> #include <usb.h> >>> #include <virtio_types.h> >>> #include <virtio.h> >>> +#include <asm/cache.h> >>> >>> DECLARE_GLOBAL_DATA_PTR; >>> >>> @@ -70,3 +71,18 @@ void *board_fdt_blob_setup(int *err) >>> /* Stored the DTB address there during our init */ >>> return (void *)(ulong)gd->arch.firmware_fdt_addr; >>> } >>> + >>> +void enable_caches(void) >>> +{ >>> + riscv_zicbom_init(); >>> +} >> >> Enable caches may be called multiple times. But riscv_zicbom_init() only >> needs to be called once. > Ok. >> >>> + >>> +void flush_dcache_range(unsigned long start, unsigned long end) >>> +{ >>> + cbo_flush(start, end); >>> +} >>> + >>> +void invalidate_dcache_range(unsigned long start, unsigned long end) >>> +{ >>> + cbo_inval(start, end); >>> +} >> >> The ZICBOM extension is not specific to a single board. We should not >> add this code to board files but into the central place for cache >> operations, i.e. arch/riscv/lib/cache.c. > Most of the code is already cache.c. I didn't know if it would suffice > for all platforms so I decided to override the weak functions like in > ARM. With the suggestion below boards needing a board specific solution can still override the weak functions. But I would expect that in future new boards converge on zicbom. Best regards Heinrich > >> E.g. >> >> static int get_zicbom_block_size() >> { >> if (!CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM)) >> return 0; >> if (zicbom_block_size) >> return zicbom_block_size; >> >> uclass_first_device(UCLASS_CPU, &dev); >> if (!dev) { >> log_err("Failed to get CPU device!\n"); >> return 0; >> } >> >> if (dev_read_u32(dev, "riscv,cbom-block-size", &zicbom_block_size)) >> return 0; >> >> return zicbom_block_size >> } >> >> __weak void invalidate_icache_range(unsigned long start, unsigned long end) >> { >> if (CONFIG_IS_ENABLED(RISCV_ISA_ZICBOM) && get_zicbom_block_size()) >> cbo_inval(start, end); >> else >> invalidate_icache_all(); >> } >> >> Cc: Rick as one of the RISC-V maintainers. > Ok. >> >> Best regards >> >> Heinrich >>
diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c index e5193e31e3..1795d2f831 100644 --- a/board/emulation/qemu-riscv/qemu-riscv.c +++ b/board/emulation/qemu-riscv/qemu-riscv.c @@ -14,6 +14,7 @@ #include <usb.h> #include <virtio_types.h> #include <virtio.h> +#include <asm/cache.h> DECLARE_GLOBAL_DATA_PTR; @@ -70,3 +71,18 @@ void *board_fdt_blob_setup(int *err) /* Stored the DTB address there during our init */ return (void *)(ulong)gd->arch.firmware_fdt_addr; } + +void enable_caches(void) +{ + riscv_zicbom_init(); +} + +void flush_dcache_range(unsigned long start, unsigned long end) +{ + cbo_flush(start, end); +} + +void invalidate_dcache_range(unsigned long start, unsigned long end) +{ + cbo_inval(start, end); +}
Define enable_caches function for the qemu-riscv board which probes for the cbom-block-size dt property when RISCV_ISA_ZICBOM is enabled. Also add flush_dcache_range and invalidate_dcache_range functions which use the corresponding CBO ops. Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> --- board/emulation/qemu-riscv/qemu-riscv.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)