[U-Boot] [PATCH] riscv: cache: Implement i/dcache [status, enable, disable]

Rick Chen rickchen36 at gmail.com
Mon Oct 29 03:16:45 UTC 2018


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.

If flush_cache is implemented, why you still add
invalidate_icache_all(); in boot_jump_linux 0f bootm.c
Do it have special reason ?


> > +     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

> >
> > -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.


> > +#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
>
> > -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