* [U-Boot] [PATCH v2] riscv: cache: Implement i/dcache [status, enable, disable]
@ 2018-11-01 4:08 Andes
2018-11-04 14:21 ` Auer, Lukas
2018-11-04 14:31 ` Bin Meng
0 siblings, 2 replies; 7+ messages in thread
From: Andes @ 2018-11-01 4:08 UTC (permalink / raw)
To: u-boot
From: Rick Chen <rick@andestech.com>
AndeStar RISC-V(V5) provide mcache_ctl register which
can configure I/D cache as enabled or disabled.
This CSR will be encapsulated by CONFIG_RISCV_NDS.
If you want to configure cache on AndeStar V5
AE350 platform. YOu can enable [*] AndeStar V5 ISA support
by make menuconfig.
This approach also provide the expansion when the
vender specific features are going to join in.
Signed-off-by: Rick Chen <rick@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
---
arch/riscv/Kconfig | 8 ++++
arch/riscv/cpu/ax25/Makefile | 1 +
arch/riscv/cpu/ax25/cache.c | 89 ++++++++++++++++++++++++++++++++++++++++++
arch/riscv/cpu/ax25/cpu.c | 4 ++
arch/riscv/cpu/qemu/cpu.c | 2 +-
arch/riscv/cpu/start.S | 6 +++
arch/riscv/include/asm/cache.h | 9 +++++
arch/riscv/lib/cache.c | 30 +++++++++-----
8 files changed, 138 insertions(+), 11 deletions(-)
create mode 100644 arch/riscv/cpu/ax25/cache.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 371921b..a356729 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -74,4 +74,12 @@ config 32BIT
config 64BIT
bool
+config RISCV_NDS
+ bool "AndeStar V5 ISA support"
+ default n
+ 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/Makefile b/arch/riscv/cpu/ax25/Makefile
index 2ab0342..318bacc 100644
--- a/arch/riscv/cpu/ax25/Makefile
+++ b/arch/riscv/cpu/ax25/Makefile
@@ -4,3 +4,4 @@
# Rick Chen, Andes Technology Corporation <rick@andestech.com>
obj-y := cpu.o
+obj-y += cache.o
diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c
new file mode 100644
index 0000000..e0bcaa2
--- /dev/null
+++ b/arch/riscv/cpu/ax25/cache.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2017 Andes Technology Corporation
+ * Rick Chen, Andes Technology Corporation <rick@andestech.com>
+ */
+
+#include <common.h>
+
+void icache_enable(void)
+{
+#ifndef CONFIG_SYS_ICACHE_OFF
+#ifdef CONFIG_RISCV_NDS
+ asm volatile (
+ "csrr t1, mcache_ctl\n\t"
+ "ori t0, t1, 0x1\n\t"
+ "csrw mcache_ctl, t0\n\t"
+ );
+#endif
+#endif
+}
+
+void icache_disable(void)
+{
+#ifdef CONFIG_RISCV_NDS
+ asm volatile (
+ "csrr t1, mcache_ctl\n\t"
+ "andi t0, t1, ~0x1\n\t"
+ "csrw mcache_ctl, t0\n\t"
+ );
+#endif
+}
+
+void dcache_enable(void)
+{
+#ifndef CONFIG_SYS_ICACHE_OFF
+#ifdef CONFIG_RISCV_NDS
+ 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)
+{
+#ifdef CONFIG_RISCV_NDS
+ asm volatile (
+ "csrr t1, mcache_ctl\n\t"
+ "andi t0, t1, ~0x2\n\t"
+ "csrw mcache_ctl, t0\n\t"
+ );
+#endif
+}
+
+int icache_status(void)
+{
+ int ret = 0;
+
+#ifdef CONFIG_RISCV_NDS
+ asm volatile (
+ "csrr t1, mcache_ctl\n\t"
+ "andi %0, t1, 0x01\n\t"
+ : "=r" (ret)
+ :
+ : "memory"
+ );
+#endif
+
+ return ret;
+}
+
+int dcache_status(void)
+{
+ int ret = 0;
+
+#ifdef CONFIG_RISCV_NDS
+ asm volatile (
+ "csrr t1, mcache_ctl\n\t"
+ "andi %0, t1, 0x02\n\t"
+ : "=r" (ret)
+ :
+ : "memory"
+ );
+#endif
+
+ return ret;
+}
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();
+ icache_disable();
+ dcache_disable();
return 0;
}
diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
index 6c7a327..25d97d0 100644
--- a/arch/riscv/cpu/qemu/cpu.c
+++ b/arch/riscv/cpu/qemu/cpu.c
@@ -15,7 +15,7 @@ int cleanup_before_linux(void)
{
disable_interrupts();
- /* turn off I/D-cache */
+ cache_flush();
return 0;
}
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
index 331a534..0e21679 100644
--- a/arch/riscv/cpu/start.S
+++ b/arch/riscv/cpu/start.S
@@ -46,6 +46,10 @@ _start:
/* mask all interrupts */
csrw mie, zero
+/* Enable cache */
+jal icache_enable
+jal dcache_enable
+
/*
* Set stackpointer in internal/ex RAM to call board_init_f
*/
@@ -181,6 +185,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 d642a38..121db09 100644
--- a/arch/riscv/lib/cache.c
+++ b/arch/riscv/lib/cache.c
@@ -6,6 +6,15 @@
#include <common.h>
+void invalidate_icache_all(void)
+{
+ asm volatile ("fence.i" ::: "memory");
+}
+
+void flush_dcache_all(void)
+{
+ asm volatile("fence" :::"memory");
+}
void flush_dcache_range(unsigned long start, unsigned long end)
{
}
@@ -19,41 +28,42 @@ void invalidate_icache_range(unsigned long start, unsigned long end)
invalidate_icache_all();
}
-void invalidate_icache_all(void)
+void invalidate_dcache_range(unsigned long start, unsigned long end)
{
- asm volatile ("fence.i" ::: "memory");
}
-void invalidate_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)
+__weak void flush_cache(unsigned long addr, unsigned long size)
{
}
-void icache_enable(void)
+__weak void icache_enable(void)
{
}
-void icache_disable(void)
+__weak void icache_disable(void)
{
}
-int icache_status(void)
+__weak int icache_status(void)
{
return 0;
}
-void dcache_enable(void)
+__weak void dcache_enable(void)
{
}
-void dcache_disable(void)
+__weak void dcache_disable(void)
{
}
-int dcache_status(void)
+__weak int dcache_status(void)
{
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v2] riscv: cache: Implement i/dcache [status, enable, disable]
2018-11-01 4:08 [U-Boot] [PATCH v2] riscv: cache: Implement i/dcache [status, enable, disable] Andes
@ 2018-11-04 14:21 ` Auer, Lukas
2018-11-06 2:28 ` Rick Chen
2018-11-04 14:31 ` Bin Meng
1 sibling, 1 reply; 7+ messages in thread
From: Auer, Lukas @ 2018-11-04 14:21 UTC (permalink / raw)
To: u-boot
Hi Rick,
On Thu, 2018-11-01 at 12:08 +0800, Andes wrote:
> From: Rick Chen <rick@andestech.com>
>
> AndeStar RISC-V(V5) provide mcache_ctl register which
> can configure I/D cache as enabled or disabled.
>
> This CSR will be encapsulated by CONFIG_RISCV_NDS.
> If you want to configure cache on AndeStar V5
> AE350 platform. YOu can enable [*] AndeStar V5 ISA support
> by make menuconfig.
>
> This approach also provide the expansion when the
> vender specific features are going to join in.
>
> Signed-off-by: Rick Chen <rick@andestech.com>
> Cc: Greentime Hu <greentime@andestech.com>
> ---
> arch/riscv/Kconfig | 8 ++++
> arch/riscv/cpu/ax25/Makefile | 1 +
> arch/riscv/cpu/ax25/cache.c | 89
> ++++++++++++++++++++++++++++++++++++++++++
> arch/riscv/cpu/ax25/cpu.c | 4 ++
> arch/riscv/cpu/qemu/cpu.c | 2 +-
> arch/riscv/cpu/start.S | 6 +++
> arch/riscv/include/asm/cache.h | 9 +++++
> arch/riscv/lib/cache.c | 30 +++++++++-----
> 8 files changed, 138 insertions(+), 11 deletions(-)
> create mode 100644 arch/riscv/cpu/ax25/cache.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 371921b..a356729 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -74,4 +74,12 @@ config 32BIT
> config 64BIT
> bool
>
> +config RISCV_NDS
> + bool "AndeStar V5 ISA support"
> + default n
> + 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/Makefile
> b/arch/riscv/cpu/ax25/Makefile
> index 2ab0342..318bacc 100644
> --- a/arch/riscv/cpu/ax25/Makefile
> +++ b/arch/riscv/cpu/ax25/Makefile
> @@ -4,3 +4,4 @@
> # Rick Chen, Andes Technology Corporation <rick@andestech.com>
>
> obj-y := cpu.o
> +obj-y += cache.o
> diff --git a/arch/riscv/cpu/ax25/cache.c
> b/arch/riscv/cpu/ax25/cache.c
> new file mode 100644
> index 0000000..e0bcaa2
> --- /dev/null
> +++ b/arch/riscv/cpu/ax25/cache.c
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2017 Andes Technology Corporation
> + * Rick Chen, Andes Technology Corporation <rick@andestech.com>
> + */
> +
> +#include <common.h>
> +
> +void icache_enable(void)
> +{
> +#ifndef CONFIG_SYS_ICACHE_OFF
> +#ifdef CONFIG_RISCV_NDS
> + asm volatile (
> + "csrr t1, mcache_ctl\n\t"
> + "ori t0, t1, 0x1\n\t"
> + "csrw mcache_ctl, t0\n\t"
> + );
> +#endif
> +#endif
> +}
> +
> +void icache_disable(void)
Just wondering, why do you not have the same #ifndef
CONFIG_SYS_ICACHE_OFF as above here?
> +{
> +#ifdef CONFIG_RISCV_NDS
> + asm volatile (
> + "csrr t1, mcache_ctl\n\t"
> + "andi t0, t1, ~0x1\n\t"
> + "csrw mcache_ctl, t0\n\t"
> + );
> +#endif
> +}
> +
> +void dcache_enable(void)
> +{
> +#ifndef CONFIG_SYS_ICACHE_OFF
> +#ifdef CONFIG_RISCV_NDS
> + 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)
> +{
> +#ifdef CONFIG_RISCV_NDS
> + asm volatile (
> + "csrr t1, mcache_ctl\n\t"
> + "andi t0, t1, ~0x2\n\t"
> + "csrw mcache_ctl, t0\n\t"
> + );
> +#endif
> +}
> +
> +int icache_status(void)
> +{
> + int ret = 0;
> +
> +#ifdef CONFIG_RISCV_NDS
> + asm volatile (
> + "csrr t1, mcache_ctl\n\t"
> + "andi %0, t1, 0x01\n\t"
> + : "=r" (ret)
> + :
> + : "memory"
> + );
> +#endif
> +
> + return ret;
> +}
> +
> +int dcache_status(void)
> +{
> + int ret = 0;
> +
> +#ifdef CONFIG_RISCV_NDS
> + asm volatile (
> + "csrr t1, mcache_ctl\n\t"
> + "andi %0, t1, 0x02\n\t"
> + : "=r" (ret)
> + :
> + : "memory"
> + );
> +#endif
> +
> + return ret;
> +}
> 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();
> + icache_disable();
> + dcache_disable();
This is a separate change from the one described in the commit message,
please move this into a new patch.
I think we should do the same thing as ARM here. This is the
implementation on armv8, for example. This sequence is chosen so that
any new entries to the cache just before it is disabled get invalidated
as well.
/*
* Turn off I-cache and invalidate it
*/
icache_disable();
invalidate_icache_all();
/*
* turn off D-cache
* dcache_disable() in turn flushes the d-cache and disables MMU
*/
dcache_disable();
invalidate_dcache_all();
>
> return 0;
> }
> diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
> index 6c7a327..25d97d0 100644
> --- a/arch/riscv/cpu/qemu/cpu.c
> +++ b/arch/riscv/cpu/qemu/cpu.c
> @@ -15,7 +15,7 @@ int cleanup_before_linux(void)
> {
> disable_interrupts();
>
> - /* turn off I/D-cache */
> + cache_flush();
>
> return 0;
> }
> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> index 331a534..0e21679 100644
> --- a/arch/riscv/cpu/start.S
> +++ b/arch/riscv/cpu/start.S
> @@ -46,6 +46,10 @@ _start:
> /* mask all interrupts */
> csrw mie, zero
>
> +/* Enable cache */
> +jal icache_enable
> +jal dcache_enable
> +
I think we should also define a enable_caches() function, like on ARM,
to enable the caches. This works as well, but I think it makes sense to
copy what other architectures do. What do you think?
> /*
> * Set stackpointer in internal/ex RAM to call board_init_f
> */
> @@ -181,6 +185,8 @@ clbss_l:
> * initialization, now running from RAM.
> */
> call_board_init_r:
> + jal invalidate_icache_all
> + jal flush_dcache_all
Is this required, perhaps this should be called from
icache/dcache_enable?
> 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);
These are not required, include/common.h already has the function
prototypes.
> +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 d642a38..121db09 100644
> --- a/arch/riscv/lib/cache.c
> +++ b/arch/riscv/lib/cache.c
> @@ -6,6 +6,15 @@
>
> #include <common.h>
>
> +void invalidate_icache_all(void)
> +{
> + asm volatile ("fence.i" ::: "memory");
> +}
> +
> +void flush_dcache_all(void)
> +{
> + asm volatile("fence" :::"memory");
nit: there should be a space after "volatile" and ":::" to match the
style from invalidate_icache_all :)
> +}
> void flush_dcache_range(unsigned long start, unsigned long end)
Can you also implement the flush_dcache_range and flush_cache
functions? We might run into unexpected behavior if we don't flush the
cache here.
> {
> }
> @@ -19,41 +28,42 @@ void invalidate_icache_range(unsigned long start,
> unsigned long end)
> invalidate_icache_all();
> }
>
> -void invalidate_icache_all(void)
> +void invalidate_dcache_range(unsigned long start, unsigned long end)
> {
> - asm volatile ("fence.i" ::: "memory");
> }
>
> -void invalidate_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)
> +__weak void flush_cache(unsigned long addr, unsigned long size)
You are not overwriting this function, so it doesn't have to be defined
as weak.
Thanks,
Lukas
> {
> }
>
> -void icache_enable(void)
> +__weak void icache_enable(void)
> {
> }
>
> -void icache_disable(void)
> +__weak void icache_disable(void)
> {
> }
>
> -int icache_status(void)
> +__weak int icache_status(void)
> {
> return 0;
> }
>
> -void dcache_enable(void)
> +__weak void dcache_enable(void)
> {
> }
>
> -void dcache_disable(void)
> +__weak void dcache_disable(void)
> {
> }
>
> -int dcache_status(void)
> +__weak int dcache_status(void)
> {
> return 0;
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v2] riscv: cache: Implement i/dcache [status, enable, disable]
2018-11-01 4:08 [U-Boot] [PATCH v2] riscv: cache: Implement i/dcache [status, enable, disable] Andes
2018-11-04 14:21 ` Auer, Lukas
@ 2018-11-04 14:31 ` Bin Meng
2018-11-06 6:02 ` Rick Chen
1 sibling, 1 reply; 7+ messages in thread
From: Bin Meng @ 2018-11-04 14:31 UTC (permalink / raw)
To: u-boot
Hi Rick,
On Thu, Nov 1, 2018 at 12:10 PM Andes <uboot@andestech.com> wrote:
>
> From: Rick Chen <rick@andestech.com>
>
> AndeStar RISC-V(V5) provide mcache_ctl register which
> can configure I/D cache as enabled or disabled.
>
> This CSR will be encapsulated by CONFIG_RISCV_NDS.
> If you want to configure cache on AndeStar V5
> AE350 platform. YOu can enable [*] AndeStar V5 ISA support
> by make menuconfig.
>
> This approach also provide the expansion when the
> vender specific features are going to join in.
>
> Signed-off-by: Rick Chen <rick@andestech.com>
> Cc: Greentime Hu <greentime@andestech.com>
> ---
> arch/riscv/Kconfig | 8 ++++
> arch/riscv/cpu/ax25/Makefile | 1 +
> arch/riscv/cpu/ax25/cache.c | 89 ++++++++++++++++++++++++++++++++++++++++++
> arch/riscv/cpu/ax25/cpu.c | 4 ++
> arch/riscv/cpu/qemu/cpu.c | 2 +-
> arch/riscv/cpu/start.S | 6 +++
> arch/riscv/include/asm/cache.h | 9 +++++
> arch/riscv/lib/cache.c | 30 +++++++++-----
> 8 files changed, 138 insertions(+), 11 deletions(-)
> create mode 100644 arch/riscv/cpu/ax25/cache.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 371921b..a356729 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -74,4 +74,12 @@ config 32BIT
> config 64BIT
> bool
>
> +config RISCV_NDS
This needs better be moved to arch/riscv/cpu/ax25/Kconfig. I have a
patch to organize the Kconfig options in a hierarchy way @
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=5a650689410482907a37f77b2a4257d81bb4daa2;hp=11f2be3a168230d6e3afddb75b1a63adb0c1b838
> + bool "AndeStar V5 ISA support"
> + default n
nits: default n is not needed.
If we move this to arch/riscv/cpu/ax25/Kconfig, this option can be
selected by the board config.
> + 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/Makefile b/arch/riscv/cpu/ax25/Makefile
> index 2ab0342..318bacc 100644
> --- a/arch/riscv/cpu/ax25/Makefile
> +++ b/arch/riscv/cpu/ax25/Makefile
> @@ -4,3 +4,4 @@
> # Rick Chen, Andes Technology Corporation <rick@andestech.com>
>
> obj-y := cpu.o
> +obj-y += cache.o
> diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c
> new file mode 100644
> index 0000000..e0bcaa2
> --- /dev/null
> +++ b/arch/riscv/cpu/ax25/cache.c
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2017 Andes Technology Corporation
> + * Rick Chen, Andes Technology Corporation <rick@andestech.com>
> + */
> +
> +#include <common.h>
> +
> +void icache_enable(void)
> +{
> +#ifndef CONFIG_SYS_ICACHE_OFF
> +#ifdef CONFIG_RISCV_NDS
There is no need to #ifdef here.
> + asm volatile (
> + "csrr t1, mcache_ctl\n\t"
> + "ori t0, t1, 0x1\n\t"
> + "csrw mcache_ctl, t0\n\t"
> + );
> +#endif
> +#endif
> +}
> +
[snip]
Regards,
Bin
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v2] riscv: cache: Implement i/dcache [status, enable, disable]
2018-11-04 14:21 ` Auer, Lukas
@ 2018-11-06 2:28 ` Rick Chen
2018-11-06 15:35 ` Auer, Lukas
0 siblings, 1 reply; 7+ messages in thread
From: Rick Chen @ 2018-11-06 2:28 UTC (permalink / raw)
To: u-boot
Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2018年11月4日 週日 下午10:21寫道：
>
> Hi Rick,
>
> On Thu, 2018-11-01 at 12:08 +0800, Andes wrote:
> > From: Rick Chen <rick@andestech.com>
> >
> > AndeStar RISC-V(V5) provide mcache_ctl register which
> > can configure I/D cache as enabled or disabled.
> >
> > This CSR will be encapsulated by CONFIG_RISCV_NDS.
> > If you want to configure cache on AndeStar V5
> > AE350 platform. YOu can enable [*] AndeStar V5 ISA support
> > by make menuconfig.
> >
> > This approach also provide the expansion when the
> > vender specific features are going to join in.
> >
> > Signed-off-by: Rick Chen <rick@andestech.com>
> > Cc: Greentime Hu <greentime@andestech.com>
> > ---
> > arch/riscv/Kconfig | 8 ++++
> > arch/riscv/cpu/ax25/Makefile | 1 +
> > arch/riscv/cpu/ax25/cache.c | 89
> > ++++++++++++++++++++++++++++++++++++++++++
> > arch/riscv/cpu/ax25/cpu.c | 4 ++
> > arch/riscv/cpu/qemu/cpu.c | 2 +-
> > arch/riscv/cpu/start.S | 6 +++
> > arch/riscv/include/asm/cache.h | 9 +++++
> > arch/riscv/lib/cache.c | 30 +++++++++-----
> > 8 files changed, 138 insertions(+), 11 deletions(-)
> > create mode 100644 arch/riscv/cpu/ax25/cache.c
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 371921b..a356729 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -74,4 +74,12 @@ config 32BIT
> > config 64BIT
> > bool
> >
> > +config RISCV_NDS
> > + bool "AndeStar V5 ISA support"
> > + default n
> > + 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/Makefile
> > b/arch/riscv/cpu/ax25/Makefile
> > index 2ab0342..318bacc 100644
> > --- a/arch/riscv/cpu/ax25/Makefile
> > +++ b/arch/riscv/cpu/ax25/Makefile
> > @@ -4,3 +4,4 @@
> > # Rick Chen, Andes Technology Corporation <rick@andestech.com>
> >
> > obj-y := cpu.o
> > +obj-y += cache.o
> > diff --git a/arch/riscv/cpu/ax25/cache.c
> > b/arch/riscv/cpu/ax25/cache.c
> > new file mode 100644
> > index 0000000..e0bcaa2
> > --- /dev/null
> > +++ b/arch/riscv/cpu/ax25/cache.c
> > @@ -0,0 +1,89 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2017 Andes Technology Corporation
> > + * Rick Chen, Andes Technology Corporation <rick@andestech.com>
> > + */
> > +
> > +#include <common.h>
> > +
> > +void icache_enable(void)
> > +{
> > +#ifndef CONFIG_SYS_ICACHE_OFF
> > +#ifdef CONFIG_RISCV_NDS
> > + asm volatile (
> > + "csrr t1, mcache_ctl\n\t"
> > + "ori t0, t1, 0x1\n\t"
> > + "csrw mcache_ctl, t0\n\t"
> > + );
> > +#endif
> > +#endif
> > +}
> > +
> > +void icache_disable(void)
>
> Just wondering, why do you not have the same #ifndef
> CONFIG_SYS_ICACHE_OFF as above here?
>
I only add CONFIG_SYS_XXX_OFF in enable function, not in disable funtion.
This can control cache enable or disable when u-boot start-up, on other reason.
If you care about this, I can add CONFIG_SYS_XXX_OFF for all.
> > +{
> > +#ifdef CONFIG_RISCV_NDS
> > + asm volatile (
> > + "csrr t1, mcache_ctl\n\t"
> > + "andi t0, t1, ~0x1\n\t"
> > + "csrw mcache_ctl, t0\n\t"
> > + );
> > +#endif
> > +}
> > +
> > +void dcache_enable(void)
> > +{
> > +#ifndef CONFIG_SYS_ICACHE_OFF
> > +#ifdef CONFIG_RISCV_NDS
> > + 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)
> > +{
> > +#ifdef CONFIG_RISCV_NDS
> > + asm volatile (
> > + "csrr t1, mcache_ctl\n\t"
> > + "andi t0, t1, ~0x2\n\t"
> > + "csrw mcache_ctl, t0\n\t"
> > + );
> > +#endif
> > +}
> > +
> > +int icache_status(void)
> > +{
> > + int ret = 0;
> > +
> > +#ifdef CONFIG_RISCV_NDS
> > + asm volatile (
> > + "csrr t1, mcache_ctl\n\t"
> > + "andi %0, t1, 0x01\n\t"
> > + : "=r" (ret)
> > + :
> > + : "memory"
> > + );
> > +#endif
> > +
> > + return ret;
> > +}
> > +
> > +int dcache_status(void)
> > +{
> > + int ret = 0;
> > +
> > +#ifdef CONFIG_RISCV_NDS
> > + asm volatile (
> > + "csrr t1, mcache_ctl\n\t"
> > + "andi %0, t1, 0x02\n\t"
> > + : "=r" (ret)
> > + :
> > + : "memory"
> > + );
> > +#endif
> > +
> > + return ret;
> > +}
> > 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();
> > + icache_disable();
> > + dcache_disable();
>
> This is a separate change from the one described in the commit message,
> please move this into a new patch.
>
> I think we should do the same thing as ARM here. This is the
> implementation on armv8, for example. This sequence is chosen so that
> any new entries to the cache just before it is disabled get invalidated
> as well.
This is RISC-V not armv8.
/lib/cache.c is for RISC-V generic cache implement.
/cpu/ax25/cache.c is for Andes RISC-V cache implement.
You can add a platform which is armv8 relative as below
And modify the necessary flow as you want
/cpu/armv8/cache.c
>
> /*
> * Turn off I-cache and invalidate it
> */
> icache_disable();
> invalidate_icache_all();
>
> /*
> * turn off D-cache
> * dcache_disable() in turn flushes the d-cache and disables MMU
> */
> dcache_disable();
> invalidate_dcache_all();
>
> >
> > return 0;
> > }
> > diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
> > index 6c7a327..25d97d0 100644
> > --- a/arch/riscv/cpu/qemu/cpu.c
> > +++ b/arch/riscv/cpu/qemu/cpu.c
> > @@ -15,7 +15,7 @@ int cleanup_before_linux(void)
> > {
> > disable_interrupts();
> >
> > - /* turn off I/D-cache */
> > + cache_flush();
> >
> > return 0;
> > }
> > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> > index 331a534..0e21679 100644
> > --- a/arch/riscv/cpu/start.S
> > +++ b/arch/riscv/cpu/start.S
> > @@ -46,6 +46,10 @@ _start:
> > /* mask all interrupts */
> > csrw mie, zero
> >
> > +/* Enable cache */
> > +jal icache_enable
> > +jal dcache_enable
> > +
>
> I think we should also define a enable_caches() function, like on ARM,
> to enable the caches. This works as well, but I think it makes sense to
> copy what other architectures do. What do you think?
>
Both coding style are exist in whole U-Boot
I can not find enable_caches be used in start.S
But it can be found
bl dcache_enable
in arch\powerpc\cpu\mpc86xx\start.S
So I prefer to not modify this flow.
Unless it indeed affect your platform and cause some error.
And I modify it as jal icache_enable the coding flow.
It is because you do not like CONFIG_SYS_ICACHE_OFF.
It may let you make some mistakes.
So I move CONFIG_SYS_ICACHE_OFF in /cpi/ax25/cache.c
> > /*
> > * Set stackpointer in internal/ex RAM to call board_init_f
> > */
> > @@ -181,6 +185,8 @@ clbss_l:
> > * initialization, now running from RAM.
> > */
> > call_board_init_r:
> > + jal invalidate_icache_all
> > + jal flush_dcache_all
>
> Is this required, perhaps this should be called from
> icache/dcache_enable?
Yes. It is required.
It can not be called to icache/dcache_enable
Cache shall be inval and flush after relocation.
>
> > 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);
>
> These are not required, include/common.h already has the function
> prototypes.
>
I will remove them from cache.h.
> > +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 d642a38..121db09 100644
> > --- a/arch/riscv/lib/cache.c
> > +++ b/arch/riscv/lib/cache.c
> > @@ -6,6 +6,15 @@
> >
> > #include <common.h>
> >
> > +void invalidate_icache_all(void)
> > +{
> > + asm volatile ("fence.i" ::: "memory");
> > +}
> > +
> > +void flush_dcache_all(void)
> > +{
> > + asm volatile("fence" :::"memory");
>
> nit: there should be a space after "volatile" and ":::" to match the
> style from invalidate_icache_all :)
I will modify it.
>
> > +}
> > void flush_dcache_range(unsigned long start, unsigned long end)
>
> Can you also implement the flush_dcache_range and flush_cache
> functions? We might run into unexpected behavior if we don't flush the
> cache here.
>
I do not modify it for the reason:
I separate one cache.c to
/lib/cache.c
/cpu/ax25/cache.c
I plan /lib/cache.c will be maintained by you
That is why I do not implement this two function.
If you said that I will implement flush_dcache_range and flush_cache in V2
> > {
> > }
> > @@ -19,41 +28,42 @@ void invalidate_icache_range(unsigned long start,
> > unsigned long end)
> > invalidate_icache_all();
> > }
> >
> > -void invalidate_icache_all(void)
> > +void invalidate_dcache_range(unsigned long start, unsigned long end)
> > {
> > - asm volatile ("fence.i" ::: "memory");
> > }
> >
> > -void invalidate_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)
> > +__weak void flush_cache(unsigned long addr, unsigned long size)
>
> You are not overwriting this function, so it doesn't have to be defined
> as weak.
>
Actually I will have a
flush_cache(unsigned long addr, unsigned long size)
in /cpu/ax25/cache.c in the future.
And I declare it as weak in /lib/cache.c
So next time when I send a patch
/lib/cache.c can't be modified anymore.
Maybe it is not a good idea.
I will recovery it as not weak here in V2.
> Thanks,
> Lukas
>
> > {
> > }
> >
> > -void icache_enable(void)
> > +__weak void icache_enable(void)
> > {
> > }
> >
> > -void icache_disable(void)
> > +__weak void icache_disable(void)
> > {
> > }
> >
> > -int icache_status(void)
> > +__weak int icache_status(void)
> > {
> > return 0;
> > }
> >
> > -void dcache_enable(void)
> > +__weak void dcache_enable(void)
> > {
> > }
> >
> > -void dcache_disable(void)
> > +__weak void dcache_disable(void)
> > {
> > }
> >
> > -int dcache_status(void)
> > +__weak int dcache_status(void)
> > {
> > return 0;
> > }
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v2] riscv: cache: Implement i/dcache [status, enable, disable]
2018-11-04 14:31 ` Bin Meng
@ 2018-11-06 6:02 ` Rick Chen
2018-11-06 6:35 ` Bin Meng
0 siblings, 1 reply; 7+ messages in thread
From: Rick Chen @ 2018-11-06 6:02 UTC (permalink / raw)
To: u-boot
Hi Bin
Bin Meng <bmeng.cn@gmail.com> 於 2018年11月4日 週日 下午10:31寫道：
>
> Hi Rick,
>
> On Thu, Nov 1, 2018 at 12:10 PM Andes <uboot@andestech.com> wrote:
> >
> > From: Rick Chen <rick@andestech.com>
> >
> > AndeStar RISC-V(V5) provide mcache_ctl register which
> > can configure I/D cache as enabled or disabled.
> >
> > This CSR will be encapsulated by CONFIG_RISCV_NDS.
> > If you want to configure cache on AndeStar V5
> > AE350 platform. YOu can enable [*] AndeStar V5 ISA support
> > by make menuconfig.
> >
> > This approach also provide the expansion when the
> > vender specific features are going to join in.
> >
> > Signed-off-by: Rick Chen <rick@andestech.com>
> > Cc: Greentime Hu <greentime@andestech.com>
> > ---
> > arch/riscv/Kconfig | 8 ++++
> > arch/riscv/cpu/ax25/Makefile | 1 +
> > arch/riscv/cpu/ax25/cache.c | 89 ++++++++++++++++++++++++++++++++++++++++++
> > arch/riscv/cpu/ax25/cpu.c | 4 ++
> > arch/riscv/cpu/qemu/cpu.c | 2 +-
> > arch/riscv/cpu/start.S | 6 +++
> > arch/riscv/include/asm/cache.h | 9 +++++
> > arch/riscv/lib/cache.c | 30 +++++++++-----
> > 8 files changed, 138 insertions(+), 11 deletions(-)
> > create mode 100644 arch/riscv/cpu/ax25/cache.c
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 371921b..a356729 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -74,4 +74,12 @@ config 32BIT
> > config 64BIT
> > bool
> >
> > +config RISCV_NDS
>
> This needs better be moved to arch/riscv/cpu/ax25/Kconfig. I have a
> patch to organize the Kconfig options in a hierarchy way @
I will move this to arch/riscv/cpu/ax25/Kconfig
> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=5a650689410482907a37f77b2a4257d81bb4daa2;hp=11f2be3a168230d6e3afddb75b1a63adb0c1b838
>
> > + bool "AndeStar V5 ISA support"
> > + default n
>
> nits: default n is not needed.
It may still needed.
Or it will fail in auto build with generic RISC-V toolchain.
It do not recognize this mcache_ctl csr .
>
> If we move this to arch/riscv/cpu/ax25/Kconfig, this option can be
> selected by the board config.
>
> > + 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/Makefile b/arch/riscv/cpu/ax25/Makefile
> > index 2ab0342..318bacc 100644
> > --- a/arch/riscv/cpu/ax25/Makefile
> > +++ b/arch/riscv/cpu/ax25/Makefile
> > @@ -4,3 +4,4 @@
> > # Rick Chen, Andes Technology Corporation <rick@andestech.com>
> >
> > obj-y := cpu.o
> > +obj-y += cache.o
> > diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c
> > new file mode 100644
> > index 0000000..e0bcaa2
> > --- /dev/null
> > +++ b/arch/riscv/cpu/ax25/cache.c
> > @@ -0,0 +1,89 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2017 Andes Technology Corporation
> > + * Rick Chen, Andes Technology Corporation <rick@andestech.com>
> > + */
> > +
> > +#include <common.h>
> > +
> > +void icache_enable(void)
> > +{
> > +#ifndef CONFIG_SYS_ICACHE_OFF
> > +#ifdef CONFIG_RISCV_NDS
>
> There is no need to #ifdef here.
>
> > + asm volatile (
> > + "csrr t1, mcache_ctl\n\t"
> > + "ori t0, t1, 0x1\n\t"
> > + "csrw mcache_ctl, t0\n\t"
> > + );
> > +#endif
> > +#endif
> > +}
> > +
>
> [snip]
>
> Regards,
> Bin
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v2] riscv: cache: Implement i/dcache [status, enable, disable]
2018-11-06 6:02 ` Rick Chen
@ 2018-11-06 6:35 ` Bin Meng
0 siblings, 0 replies; 7+ messages in thread
From: Bin Meng @ 2018-11-06 6:35 UTC (permalink / raw)
To: u-boot
Hi Rick,
On Tue, Nov 6, 2018 at 2:01 PM Rick Chen <rickchen36@gmail.com> wrote:
>
> Hi Bin
>
> Bin Meng <bmeng.cn@gmail.com> 於 2018年11月4日 週日 下午10:31寫道：
> >
> > Hi Rick,
> >
> > On Thu, Nov 1, 2018 at 12:10 PM Andes <uboot@andestech.com> wrote:
> > >
> > > From: Rick Chen <rick@andestech.com>
> > >
> > > AndeStar RISC-V(V5) provide mcache_ctl register which
> > > can configure I/D cache as enabled or disabled.
> > >
> > > This CSR will be encapsulated by CONFIG_RISCV_NDS.
> > > If you want to configure cache on AndeStar V5
> > > AE350 platform. YOu can enable [*] AndeStar V5 ISA support
> > > by make menuconfig.
> > >
> > > This approach also provide the expansion when the
> > > vender specific features are going to join in.
> > >
> > > Signed-off-by: Rick Chen <rick@andestech.com>
> > > Cc: Greentime Hu <greentime@andestech.com>
> > > ---
> > > arch/riscv/Kconfig | 8 ++++
> > > arch/riscv/cpu/ax25/Makefile | 1 +
> > > arch/riscv/cpu/ax25/cache.c | 89 ++++++++++++++++++++++++++++++++++++++++++
> > > arch/riscv/cpu/ax25/cpu.c | 4 ++
> > > arch/riscv/cpu/qemu/cpu.c | 2 +-
> > > arch/riscv/cpu/start.S | 6 +++
> > > arch/riscv/include/asm/cache.h | 9 +++++
> > > arch/riscv/lib/cache.c | 30 +++++++++-----
> > > 8 files changed, 138 insertions(+), 11 deletions(-)
> > > create mode 100644 arch/riscv/cpu/ax25/cache.c
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 371921b..a356729 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -74,4 +74,12 @@ config 32BIT
> > > config 64BIT
> > > bool
> > >
> > > +config RISCV_NDS
> >
> > This needs better be moved to arch/riscv/cpu/ax25/Kconfig. I have a
> > patch to organize the Kconfig options in a hierarchy way @
>
> I will move this to arch/riscv/cpu/ax25/Kconfig
>
> > http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=5a650689410482907a37f77b2a4257d81bb4daa2;hp=11f2be3a168230d6e3afddb75b1a63adb0c1b838
> >
> > > + bool "AndeStar V5 ISA support"
> > > + default n
> >
> > nits: default n is not needed.
>
> It may still needed.
> Or it will fail in auto build with generic RISC-V toolchain.
> It do not recognize this mcache_ctl csr .
>
I mean from Kconfig language perspective, specifying 'default n', or
omitting it, does not make any difference. So you need need explicitly
write 'default n' here.
> >
> > If we move this to arch/riscv/cpu/ax25/Kconfig, this option can be
> > selected by the board config.
> >
> > > + 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/Makefile b/arch/riscv/cpu/ax25/Makefile
> > > index 2ab0342..318bacc 100644
> > > --- a/arch/riscv/cpu/ax25/Makefile
> > > +++ b/arch/riscv/cpu/ax25/Makefile
> > > @@ -4,3 +4,4 @@
> > > # Rick Chen, Andes Technology Corporation <rick@andestech.com>
> > >
> > > obj-y := cpu.o
> > > +obj-y += cache.o
> > > diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c
> > > new file mode 100644
> > > index 0000000..e0bcaa2
> > > --- /dev/null
> > > +++ b/arch/riscv/cpu/ax25/cache.c
> > > @@ -0,0 +1,89 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2017 Andes Technology Corporation
> > > + * Rick Chen, Andes Technology Corporation <rick@andestech.com>
> > > + */
> > > +
> > > +#include <common.h>
> > > +
> > > +void icache_enable(void)
> > > +{
> > > +#ifndef CONFIG_SYS_ICACHE_OFF
> > > +#ifdef CONFIG_RISCV_NDS
> >
> > There is no need to #ifdef here.
> >
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "ori t0, t1, 0x1\n\t"
> > > + "csrw mcache_ctl, t0\n\t"
> > > + );
> > > +#endif
> > > +#endif
> > > +}
> > > +
> >
Regards,
Bin
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v2] riscv: cache: Implement i/dcache [status, enable, disable]
2018-11-06 2:28 ` Rick Chen
@ 2018-11-06 15:35 ` Auer, Lukas
0 siblings, 0 replies; 7+ messages in thread
From: Auer, Lukas @ 2018-11-06 15:35 UTC (permalink / raw)
To: u-boot
Hi Rick,
On Tue, 2018-11-06 at 10:28 +0800, Rick Chen wrote:
> Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2018年11月4日 週日
> 下午10:21寫道：
> >
> > Hi Rick,
> >
> > On Thu, 2018-11-01 at 12:08 +0800, Andes wrote:
> > > From: Rick Chen <rick@andestech.com>
> > >
> > > AndeStar RISC-V(V5) provide mcache_ctl register which
> > > can configure I/D cache as enabled or disabled.
> > >
> > > This CSR will be encapsulated by CONFIG_RISCV_NDS.
> > > If you want to configure cache on AndeStar V5
> > > AE350 platform. YOu can enable [*] AndeStar V5 ISA support
> > > by make menuconfig.
> > >
> > > This approach also provide the expansion when the
> > > vender specific features are going to join in.
> > >
> > > Signed-off-by: Rick Chen <rick@andestech.com>
> > > Cc: Greentime Hu <greentime@andestech.com>
> > > ---
> > > arch/riscv/Kconfig | 8 ++++
> > > arch/riscv/cpu/ax25/Makefile | 1 +
> > > arch/riscv/cpu/ax25/cache.c | 89
> > > ++++++++++++++++++++++++++++++++++++++++++
> > > arch/riscv/cpu/ax25/cpu.c | 4 ++
> > > arch/riscv/cpu/qemu/cpu.c | 2 +-
> > > arch/riscv/cpu/start.S | 6 +++
> > > arch/riscv/include/asm/cache.h | 9 +++++
> > > arch/riscv/lib/cache.c | 30 +++++++++-----
> > > 8 files changed, 138 insertions(+), 11 deletions(-)
> > > create mode 100644 arch/riscv/cpu/ax25/cache.c
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 371921b..a356729 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -74,4 +74,12 @@ config 32BIT
> > > config 64BIT
> > > bool
> > >
> > > +config RISCV_NDS
> > > + bool "AndeStar V5 ISA support"
> > > + default n
> > > + 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/Makefile
> > > b/arch/riscv/cpu/ax25/Makefile
> > > index 2ab0342..318bacc 100644
> > > --- a/arch/riscv/cpu/ax25/Makefile
> > > +++ b/arch/riscv/cpu/ax25/Makefile
> > > @@ -4,3 +4,4 @@
> > > # Rick Chen, Andes Technology Corporation <rick@andestech.com>
> > >
> > > obj-y := cpu.o
> > > +obj-y += cache.o
> > > diff --git a/arch/riscv/cpu/ax25/cache.c
> > > b/arch/riscv/cpu/ax25/cache.c
> > > new file mode 100644
> > > index 0000000..e0bcaa2
> > > --- /dev/null
> > > +++ b/arch/riscv/cpu/ax25/cache.c
> > > @@ -0,0 +1,89 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2017 Andes Technology Corporation
> > > + * Rick Chen, Andes Technology Corporation <rick@andestech.com>
> > > + */
> > > +
> > > +#include <common.h>
> > > +
> > > +void icache_enable(void)
> > > +{
> > > +#ifndef CONFIG_SYS_ICACHE_OFF
> > > +#ifdef CONFIG_RISCV_NDS
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "ori t0, t1, 0x1\n\t"
> > > + "csrw mcache_ctl, t0\n\t"
> > > + );
> > > +#endif
> > > +#endif
> > > +}
> > > +
> > > +void icache_disable(void)
> >
> > Just wondering, why do you not have the same #ifndef
> > CONFIG_SYS_ICACHE_OFF as above here?
> >
>
> I only add CONFIG_SYS_XXX_OFF in enable function, not in disable
> funtion.
> This can control cache enable or disable when u-boot start-up, on
> other reason.
> If you care about this, I can add CONFIG_SYS_XXX_OFF for all.
>
I was just wondering why, you can leave it as is.
> > > +{
> > > +#ifdef CONFIG_RISCV_NDS
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "andi t0, t1, ~0x1\n\t"
> > > + "csrw mcache_ctl, t0\n\t"
> > > + );
> > > +#endif
> > > +}
> > > +
> > > +void dcache_enable(void)
> > > +{
> > > +#ifndef CONFIG_SYS_ICACHE_OFF
> > > +#ifdef CONFIG_RISCV_NDS
> > > + 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)
> > > +{
> > > +#ifdef CONFIG_RISCV_NDS
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "andi t0, t1, ~0x2\n\t"
> > > + "csrw mcache_ctl, t0\n\t"
> > > + );
> > > +#endif
> > > +}
> > > +
> > > +int icache_status(void)
> > > +{
> > > + int ret = 0;
> > > +
> > > +#ifdef CONFIG_RISCV_NDS
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "andi %0, t1, 0x01\n\t"
> > > + : "=r" (ret)
> > > + :
> > > + : "memory"
> > > + );
> > > +#endif
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +int dcache_status(void)
> > > +{
> > > + int ret = 0;
> > > +
> > > +#ifdef CONFIG_RISCV_NDS
> > > + asm volatile (
> > > + "csrr t1, mcache_ctl\n\t"
> > > + "andi %0, t1, 0x02\n\t"
> > > + : "=r" (ret)
> > > + :
> > > + : "memory"
> > > + );
> > > +#endif
> > > +
> > > + return ret;
> > > +}
> > > 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();
> > > + icache_disable();
> > > + dcache_disable();
> >
> > This is a separate change from the one described in the commit
> > message,
> > please move this into a new patch.
> >
> > I think we should do the same thing as ARM here. This is the
> > implementation on armv8, for example. This sequence is chosen so
> > that
> > any new entries to the cache just before it is disabled get
> > invalidated
> > as well.
>
> This is RISC-V not armv8.
> /lib/cache.c is for RISC-V generic cache implement.
> /cpu/ax25/cache.c is for Andes RISC-V cache implement.
>
> You can add a platform which is armv8 relative as below
> And modify the necessary flow as you want
> /cpu/armv8/cache.c
>
Of course this is not armv8. I was referring to the sequence in which
the cache is disabled, i.e. first disable, then invalidate. This might
not be needed, but is nevertheless a good idea to ensure all entries in
the cache are invalidated.
> >
> > /*
> > * Turn off I-cache and invalidate it
> > */
> > icache_disable();
> > invalidate_icache_all();
> >
> > /*
> > * turn off D-cache
> > * dcache_disable() in turn flushes the d-cache and disables MMU
> > */
> > dcache_disable();
> > invalidate_dcache_all();
> >
> > >
> > > return 0;
> > > }
> > > diff --git a/arch/riscv/cpu/qemu/cpu.c
> > > b/arch/riscv/cpu/qemu/cpu.c
> > > index 6c7a327..25d97d0 100644
> > > --- a/arch/riscv/cpu/qemu/cpu.c
> > > +++ b/arch/riscv/cpu/qemu/cpu.c
> > > @@ -15,7 +15,7 @@ int cleanup_before_linux(void)
> > > {
> > > disable_interrupts();
> > >
> > > - /* turn off I/D-cache */
> > > + cache_flush();
> > >
> > > return 0;
> > > }
> > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> > > index 331a534..0e21679 100644
> > > --- a/arch/riscv/cpu/start.S
> > > +++ b/arch/riscv/cpu/start.S
> > > @@ -46,6 +46,10 @@ _start:
> > > /* mask all interrupts */
> > > csrw mie, zero
> > >
> > > +/* Enable cache */
> > > +jal icache_enable
> > > +jal dcache_enable
> > > +
> >
> > I think we should also define a enable_caches() function, like on
> > ARM,
> > to enable the caches. This works as well, but I think it makes
> > sense to
> > copy what other architectures do. What do you think?
> >
>
> Both coding style are exist in whole U-Boot
> I can not find enable_caches be used in start.S
> But it can be found
> bl dcache_enable
> in arch\powerpc\cpu\mpc86xx\start.S
> So I prefer to not modify this flow.
> Unless it indeed affect your platform and cause some error.
>
enable_caches is, for example, defined in
arch/arm/cpu/armv8/cache_v8.c. It is called by initr_caches() in
board_init_r, this means the caches are only enabled once U-Boot has
been relocated.
I prefer this way, because I think it is better if the caches are
enabled in a file common to all platforms, which also initializes other
parts of the system. Of course, this is my personal opinion, if you
still prefer this way, you can leave it like this.
> And I modify it as jal icache_enable the coding flow.
> It is because you do not like CONFIG_SYS_ICACHE_OFF.
> It may let you make some mistakes.
> So I move CONFIG_SYS_ICACHE_OFF in /cpi/ax25/cache.c
>
I am completely fine with CONFIG_SYS_ICACHE_OFF, in fact, I think it
should be used here :)
>
> > > /*
> > > * Set stackpointer in internal/ex RAM to call board_init_f
> > > */
> > > @@ -181,6 +185,8 @@ clbss_l:
> > > * initialization, now running from RAM.
> > > */
> > > call_board_init_r:
> > > + jal invalidate_icache_all
> > > + jal flush_dcache_all
> >
> > Is this required, perhaps this should be called from
> > icache/dcache_enable?
>
> Yes. It is required.
> It can not be called to icache/dcache_enable
> Cache shall be inval and flush after relocation.
>
Ah I see, yes that is definitely required. However, I think this should
go into relocate_code of start.S, just so it's clear why the caches are
cleared at this point.
> >
> > > 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);
> >
> > These are not required, include/common.h already has the function
> > prototypes.
> >
>
> I will remove them from cache.h.
>
>
> > > +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 d642a38..121db09 100644
> > > --- a/arch/riscv/lib/cache.c
> > > +++ b/arch/riscv/lib/cache.c
> > > @@ -6,6 +6,15 @@
> > >
> > > #include <common.h>
> > >
> > > +void invalidate_icache_all(void)
> > > +{
> > > + asm volatile ("fence.i" ::: "memory");
> > > +}
> > > +
> > > +void flush_dcache_all(void)
> > > +{
> > > + asm volatile("fence" :::"memory");
> >
> > nit: there should be a space after "volatile" and ":::" to match
> > the
> > style from invalidate_icache_all :)
>
> I will modify it.
>
> >
> > > +}
> > > void flush_dcache_range(unsigned long start, unsigned long end)
> >
> > Can you also implement the flush_dcache_range and flush_cache
> > functions? We might run into unexpected behavior if we don't flush
> > the
> > cache here.
> >
>
> I do not modify it for the reason:
> I separate one cache.c to
> /lib/cache.c
> /cpu/ax25/cache.c
>
> I plan /lib/cache.c will be maintained by you
> That is why I do not implement this two function.
>
> If you said that I will implement flush_dcache_range and flush_cache
> in V2
>
Ok, I can send a patch to implement them.
>
> > > {
> > > }
> > > @@ -19,41 +28,42 @@ void invalidate_icache_range(unsigned long
> > > start,
> > > unsigned long end)
> > > invalidate_icache_all();
> > > }
> > >
> > > -void invalidate_icache_all(void)
> > > +void invalidate_dcache_range(unsigned long start, unsigned long
> > > end)
> > > {
> > > - asm volatile ("fence.i" ::: "memory");
> > > }
> > >
> > > -void invalidate_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)
> > > +__weak void flush_cache(unsigned long addr, unsigned long size)
> >
> > You are not overwriting this function, so it doesn't have to be
> > defined
> > as weak.
> >
>
> Actually I will have a
> flush_cache(unsigned long addr, unsigned long size)
> in /cpu/ax25/cache.c in the future.
>
> And I declare it as weak in /lib/cache.c
> So next time when I send a patch
> /lib/cache.c can't be modified anymore.
>
> Maybe it is not a good idea.
> I will recovery it as not weak here in V2.
>
That makes sense, the weak doesn't break anything, so you can also
leave it I think.
Thanks,
Lukas
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-11-06 15:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-01 4:08 [U-Boot] [PATCH v2] riscv: cache: Implement i/dcache [status, enable, disable] Andes
2018-11-04 14:21 ` Auer, Lukas
2018-11-06 2:28 ` Rick Chen
2018-11-06 15:35 ` Auer, Lukas
2018-11-04 14:31 ` Bin Meng
2018-11-06 6:02 ` Rick Chen
2018-11-06 6:35 ` Bin Meng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.