[U-Boot] [PATCH] riscv: cache: Implement i/dcache [status, enable, disable]
Auer, Lukas
lukas.auer at aisec.fraunhofer.de
Mon Oct 29 12:13:15 UTC 2018
Hi Rick,
On Mon, 2018-10-29 at 11:16 +0800, Rick Chen wrote:
> Auer, Lukas <lukas.auer at aisec.fraunhofer.de> 於 2018年10月27日 週六
> 上午12:32寫道：
> >
> > Hi Rick,
> >
> > On Mon, 2018-10-22 at 16:16 +0800, Andes wrote:
> > > From: Rick Chen <rick at andestech.com>
> > >
> > > AndeStar V5 provide mcache_ctl register which can configure
> > > I/D cache as enabled or disabled.
> > >
> > > This CSR will be encapsulated by CONFIG_NDS_V5.
> > > If you want to configure cache on AndeStar V5
> > > AE350 platform. YOu can enable [*] AndeStar V5 ISA support
> > > by make menuconfig.
> > >
> > > Signed-off-by: Rick Chen <rick at andestech.com>
> > > Cc: Greentime Hu <greentime at andestech.com>
> > > ---
> > > arch/riscv/Kconfig | 8 +++
> > > arch/riscv/cpu/ax25/cpu.c | 4 ++
> > > arch/riscv/cpu/start.S | 19 ++++++
> > > arch/riscv/include/asm/cache.h | 9 +++
> > > arch/riscv/lib/cache.c | 131
> > > ++++++++++++++++++++++++++++++++++++++---
> > > 5 files changed, 163 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 168ca3d..de7fd9e 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -43,4 +43,12 @@ config 32BIT
> > > config 64BIT
> > > bool
> > >
> > > +config NDS_V5
> > > + bool "AndeStar V5 ISA support"
> > > + def_bool n
> >
> > You can use just "default n" instead of "def_bool n" here, since
> > you
> > already have a "bool" above it.
> >
>
> I will reword it.
>
> > > + help
> > > + Say Y here if you plan to run U-Boot on AndeStar v5
> > > + platforms and use some specific features which are
> > > + provided by Andes Technology AndeStar V5 Families.
> > > +
> > > endmenu
> > > diff --git a/arch/riscv/cpu/ax25/cpu.c
> > > b/arch/riscv/cpu/ax25/cpu.c
> > > index fddcc15..76689b2 100644
> > > --- a/arch/riscv/cpu/ax25/cpu.c
> > > +++ b/arch/riscv/cpu/ax25/cpu.c
> > > @@ -6,6 +6,7 @@
> > >
> > > /* CPU specific code */
> > > #include <common.h>
> > > +#include <asm/cache.h>
> > >
> > > /*
> > > * cleanup_before_linux() is called just before we call linux
> > > @@ -18,6 +19,9 @@ int cleanup_before_linux(void)
> > > disable_interrupts();
> > >
> > > /* turn off I/D-cache */
> > > + cache_flush();
> >
> > I don't think you need to flush the data cache here. This should be
> > handled by bootm_load_os() in common/bootm.c.
> >
>
> Do you mean
> flush_cache(flush_start, ALIGN(flush_len, ARCH_DMA_MINALIGN)); ?
> Ii is different cache API and implement.
>
Yes I did mean flush_cache(), however I did not check if it is
implemented. Perhaps it's best to implement it and rely on it to flush
the cache?
> If flush_cache is implemented, why you still add
> invalidate_icache_all(); in boot_jump_linux 0f bootm.c
> Do it have special reason ?
>
RISC-V does not guarantee that stores to instruction memory are visible
to instruction fetches (i.e. incoherent instruction caches). You are
right, we do not need to flush the instruction cache explicitly to make
the kernel visible to instruction fetches. We need it for a separate
reason. Simplified, we do the following to start Linux.
kernel = kernel_address;
kernel();
Without a fence.i in between, we can not guarantee that kernel() will
jump to the address specified in kernel_address. This is because the
instruction cache is incoherent with the rest of the memory system, it
must therefore be synchronized first.
This is the reason, why I added the instruction cache invalidation to
the bootm code.
>
> > > + icache_disable();
> > > + dcache_disable();
> > >
> > > return 0;
> > > }
> > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> > > index 7cd7755..a8764df 100644
> > > --- a/arch/riscv/cpu/start.S
> > > +++ b/arch/riscv/cpu/start.S
> > > @@ -51,6 +51,23 @@ handle_reset:
> > > csrwi mstatus, 0
> > > csrwi mie, 0
> > >
> > > +/* Enable cache */
> > > +#ifndef CONFIG_SYS_ICACHE_OFF
> > > +#ifdef CONFIG_NDS_V5
> > > + csrr t1, mcache_ctl
> > > + ori t0, t1, 0x1
> > > + csrw mcache_ctl, t0
> > > +#endif
> > > +#endif
> > > +
> > > +#ifndef CONFIG_SYS_DCACHE_OFF
> > > +#ifdef CONFIG_NDS_V5
> > > + csrr t1, mcache_ctl
> > > + ori t0, t1, 0x2
> > > + csrw mcache_ctl, t0
> > > +#endif
> > > +#endif
> > > +
> > > /*
> > > * Do CPU critical regs init only at reboot,
> > > * not when booting from ram
> > > @@ -191,6 +208,8 @@ clbss_l:
> > > * initialization, now running from RAM.
> > > */
> > > call_board_init_r:
> > > + jal invalidate_icache_all
> > > + jal flush_dcache_all
> > > la t0, board_init_r
> > > mv t4, t0 /* offset of board_init_r()
> > > */
> > > add t4, t4, t6 /* real address of
> > > board_init_r() */
> > > diff --git a/arch/riscv/include/asm/cache.h
> > > b/arch/riscv/include/asm/cache.h
> > > index ca83dd6..e76ca13 100644
> > > --- a/arch/riscv/include/asm/cache.h
> > > +++ b/arch/riscv/include/asm/cache.h
> > > @@ -7,6 +7,15 @@
> > > #ifndef _ASM_RISCV_CACHE_H
> > > #define _ASM_RISCV_CACHE_H
> > >
> > > +/* cache */
> > > +int icache_status(void);
> > > +void icache_enable(void);
> > > +void icache_disable(void);
> > > +int dcache_status(void);
> > > +void dcache_enable(void);
> > > +void dcache_disable(void);
> > > +void cache_flush(void);
> > > +
> > > /*
> > > * The current upper bound for RISCV L1 data cache line sizes is
> > > 32
> > > bytes.
> > > * We use that value for aligning DMA buffers unless the board
> > > config has
> > > diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c
> > > index 1d67c49..1837fb8 100644
> > > --- a/arch/riscv/lib/cache.c
> > > +++ b/arch/riscv/lib/cache.c
> > > @@ -6,19 +6,39 @@
> > >
> > > #include <common.h>
> > >
> > > -void flush_dcache_range(unsigned long start, unsigned long end)
> > > +#ifndef CONFIG_SYS_ICACHE_OFF
> > > +void invalidate_icache_all(void)
> > > {
> > > + asm volatile("FENCE.I":::"memory");
> > > }
> >
> > fence.i will likely still be needed even if the instruction cache
> > is
> > disabled. It should therefore not be removed using the #ifndef.
> >
>
> I can remove here.
> No matter i-cache is enabled or disabled, let invalidate_icache_all
> still work with fence.i
>
I thought about this a bit more. We need invalidate_icache_all() to be
always available, because there are systems, where the instruction
cache can not be disabled. SiFive's U54-MC on the Freedom Unleashed
board is an example of such a system. If CONFIG_SYS_ICACHE_OFF is
defined by mistake, these systems might show unpredictable behavior.
> > >
> > > -void invalidate_icache_range(unsigned long start, unsigned long
> > > end)
> > > +void icache_enable(void)
> > > {
> > > +#ifndef CONFIG_SYS_ICACHE_OFF
> >
> > This #ifndef (also the ones below) is redundant since it is already
> > tested above.
> >
>
> I do not understand your words here.
> If CONFIG_NDS_V5 is define, it still can be disabled by
> CONFIG_SYS_ICACHE_OFF definition here.
>
I meant that the #ifndef CONFIG_SYS_ICACHE_OFF is redundant :)
>
> > > +#ifdef CONFIG_NDS_V5
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "ori t0, t1, 0x1\n\t"
> > > + "csrw mcache_ctl, t0\n\t"
> > > + );
> > > +#endif
> > > +#endif
> > > }
> > >
> >
> > I think the cpu-specific functions should go into a separate file
> > under
> > cpu/ax25. In this case, all functions in this file (lib/cache.c)
> > should
> > be declared weak, so that they can be overwritten. In addition, we
> > could also move the enable cache functions into that file. The
> > functions could then be called, like with ARM, from board_init_r()
> > by
> > defining the function enable_caches / initr_caches.
> > What do you think?
> >
>
> It is a good advice.
> I will try to separate it as you said.
>
> Rick
Thanks!
Lukas
>
> > Thanks,
> > Lukas
> >
> > > -void invalidate_dcache_range(unsigned long start, unsigned long
> > > end)
> > > +void icache_disable(void)
> > > {
> > > +#ifndef CONFIG_SYS_ICACHE_OFF
> > > +#ifdef CONFIG_NDS_V5
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "andi t0, t1, ~0x1\n\t"
> > > + "csrw mcache_ctl, t0\n\t"
> > > + );
> > > +#endif
> > > +#endif
> > > }
> > > -
> > > -void flush_cache(unsigned long addr, unsigned long size)
> > > +#else
> > > +void invalidate_icache_all(void)
> > > {
> > > }
> > >
> > > @@ -30,9 +50,44 @@ void icache_disable(void)
> > > {
> > > }
> > >
> > > -int icache_status(void)
> > > +#endif
> > > +
> > > +#ifndef CONFIG_SYS_DCACHE_OFF
> > > +void flush_dcache_all(void)
> > > +{
> > > + asm volatile("FENCE":::"memory");
> > > +}
> > > +
> > > +void dcache_enable(void)
> > > +{
> > > +#ifndef CONFIG_SYS_DCACHE_OFF
> > > +#ifdef CONFIG_NDS_V5
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "ori t0, t1, 0x2\n\t"
> > > + "csrw mcache_ctl, t0\n\t"
> > > + );
> > > +#endif
> > > +#endif
> > > +}
> > > +
> > > +void dcache_disable(void)
> > > +{
> > > +#ifndef CONFIG_SYS_DCACHE_OFF
> > > +#ifdef CONFIG_NDS_V5
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "andi t0, t1, ~0x2\n\t"
> > > + "csrw mcache_ctl, t0\n\t"
> > > + );
> > > +#endif
> > > +#endif
> > > +}
> > > +
> > > +
> > > +#else
> > > +void flush_dcache_all(void)
> > > {
> > > - return 0;
> > > }
> > >
> > > void dcache_enable(void)
> > > @@ -42,8 +97,68 @@ void dcache_enable(void)
> > > void dcache_disable(void)
> > > {
> > > }
> > > +#endif
> > > +
> > > +int icache_status(void)
> > > +{
> > > +#ifndef CONFIG_SYS_ICACHE_OFF
> > > + int ret = 1;
> > > +#else
> > > + int ret = 0;
> > > +#endif
> > > +
> > > +#ifdef CONFIG_NDS_V5
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "andi %0, t1, 0x01\n\t"
> > > + : "=r" (ret)
> > > + :
> > > + : "memory"
> > > + );
> > > +#endif
> > > +
> > > + return ret;
> > > +}
> > >
> > > int dcache_status(void)
> > > {
> > > - return 0;
> > > +#ifndef CONFIG_SYS_DCACHE_OFF
> > > + int ret = 1;
> > > +#else
> > > + int ret = 0;
> > > +#endif
> > > +
> > > +#ifdef CONFIG_NDS_V5
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "andi %0, t1, 0x02\n\t"
> > > + : "=r" (ret)
> > > + :
> > > + : "memory"
> > > + );
> > > +#endif
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +void invalidate_icache_range(unsigned long start, unsigned long
> > > end)
> > > +{
> > > +}
> > > +
> > > +void invalidate_dcache_range(unsigned long start, unsigned long
> > > end)
> > > +{
> > > +}
> > > +
> > > +void flush_dcache_range(unsigned long start, unsigned long end)
> > > +{
> > > +}
> > > +
> > > +void cache_flush(void)
> > > +{
> > > + invalidate_icache_all();
> > > + flush_dcache_all();
> > > +}
> > > +
> > > +void flush_cache(unsigned long addr, unsigned long size)
> > > +{
> > > }
More information about the U-Boot
mailing list