On 4/12/21 10:39 PM, Rick Chen wrote:
Hi Green,

From: Green Wan [mailto:green....@sifive.com]
Sent: Monday, April 12, 2021 10:33 AM
To: Sean Anderson
Cc: Rick Chen; Rick Jian-Zhi Chen(陳建志); Bin Meng; U-Boot Mailing List; Paul 
Walmsley; Pragnesh Patel; Simon Glass; Atish Patra; Leo Yu-Chi Liang(梁育齊); Brad 
Kim
Subject: Re: [RFC PATCH v4 1/2] arch: riscv: cpu: Add callback to init each core

Hi Bin and Sean,

While we keep the consistency of cache control discussion going, later
today I'd like to send the v5 patch which is not directly relevant to
cache control.

I will prefer not to mix cache control issue into this patch.
Like I said, this callback is a init for all harts before lottery.

Yes, but enabling caches is a very similar thing (this proposal even
uses it to turn on caches, among other things). At the moment we have
two calls to enable caches at almost the same time as what Green
proposes. These calls only translate into work done on one platform. I
think having one call (or perhaps two) for this purpose would help
reduce codepaths across different platforms going forward.

--Sean


Thanks,
Rick


Regards,
Green

On Sun, Apr 11, 2021 at 11:43 PM Sean Anderson <sean...@gmail.com> wrote:

On 4/9/21 12:05 PM, Green Wan wrote:
Hi folks,

Correct me if I'm wrong, like Rick mentioned, i/dcache
enable/disable() is only called on the main hart. Right now the dummy
i/dcache enable/disable are empty and shared among all riscv CPU. The
ax25 is the only one that has its own implementation for now.

Right, so why are caches are disabled on all harts before booting Linux
on ax25? Is there a requirement for this on ax25 which that other
platforms (which have always-on caches like k210, or which have
non-disableable caches like fuX40) do not have?

--Sean


FU540/FU740 also leverages the dummy i/dcache enable/disable()
functions (only main hart calls them). L2 cache on FU540/FU740 is
enabled as SRAM purpose. And according to the HW design behavior, once
L2 is enabled, it can't be disabled unless doing a reset.[1] The Linux
L2$ driver will handle that according to the configuration of L2
registers.

[1] https://static.dev.sifive.com/FU540-C000-v1.0.pdf

Thanks,

On Fri, Apr 9, 2021 at 9:18 PM Sean Anderson <sean...@gmail.com> wrote:

On 4/9/21 4:16 AM, Rick Chen wrote:
Hi Sean ,Bin

From: Bin Meng [mailto:bmeng...@gmail.com]
Sent: Tuesday, April 06, 2021 5:16 PM
To: Sean Anderson
Cc: Green Wan; Rick Jian-Zhi Chen(陳建志); Paul Walmsley; Pragnesh Patel; Bin 
Meng; Simon Glass; Atish Patra; Leo Yu-Chi Liang(梁育齊); Brad Kim; U-Boot Mailing 
List
Subject: Re: [RFC PATCH v4 1/2] arch: riscv: cpu: Add callback to init each core

On Sat, Apr 3, 2021 at 6:53 AM Sean Anderson <sean...@gmail.com> wrote:

On 3/30/21 1:26 AM, Green Wan wrote:
Add a callback riscv_hart_early_init() to ./arch/riscv/cpu/start.S to
allow different riscv hart perform setup code for each hart as early
as possible. Since all the harts enter the callback, they must be able
to run the same setup.

Signed-off-by: Green Wan <green....@sifive.com>
---
     arch/riscv/cpu/cpu.c   | 15 +++++++++++++++
     arch/riscv/cpu/start.S | 14 ++++++++++++++
     2 files changed, 29 insertions(+)

diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index 85592f5bee..1652e51137 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -140,3 +140,18 @@ int arch_early_init_r(void)
     {
         return riscv_cpu_probe();
     }
+
+/**
+ * riscv_hart_early_init() - A dummy function called by
+ * ./arch/riscv/cpu/start.S to allow to disable/enable features of each core.
+ * For example, turn on or off the functional block of CPU harts.
+ *
+ * In a multi-core system, this function must not access shared resources.
+ *
+ * Any access to such resources would probably be better done with
+ * available_harts_lock held. However, I doubt that any such access will be
+ * necessary.
+ */
+__weak void riscv_hart_early_init(void)
+{
+}
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
index 8589509e01..ab73008f23 100644
--- a/arch/riscv/cpu/start.S
+++ b/arch/riscv/cpu/start.S
@@ -117,6 +117,20 @@ call_board_init_f_0:
         mv      sp, a0
     #endif

+#if CONFIG_IS_ENABLED(RISCV_MMODE)
+     /*
+      * Jump to riscv_hart_early_init() to perform init for each core. Not
+      * expect to access gd since gd is not initialized. All operations in the
+      * function should affect core itself only. In multi-core system, any 
access
+      * to common resource or registers outside core should be avoided or need 
a
+      * protection for multicore.
+      *
+      * A dummy implementation is provided in ./arch/riscv/cpu/cpu.c.
+      */
+call_riscv_hart_early_init:
+     jal     riscv_hart_early_init
+#endif
+

I wonder if we could move the calls to icache_enable and dcache_enable
into this function. Though this would have the consequence of enabling
caches on all harts for CPUs which previously only enabled them for the
boot hart. I think ax25 is the only CPU which currently does this. Bin,
would this be an issue?

No, they are functions shall be called in different stage about lottery.
riscv_hart_early_init() is called before lottery for all harts.
It shall not move icache_enable() and dcache_enable() into
riscv_hart_early_init(), they are belong to the stage after lottery
only for main hart.


Good catch. I believe AX25 cache support is currently somehow broken?

No, AX25 cache support is currently work well.


I think Rick has to comment on this as he added this via commit:

commit 52923c6db7f00e0197ec894c8c1bb8a7681974bb
Author: Rick Chen <r...@andestech.com>
Date:   Wed Nov 7 09:34:06 2018 +0800

       riscv: cache: Implement i/dcache [status, enable, disable]

       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 <r...@andestech.com>
       Cc: Greentime Hu <greent...@andestech.com>

The original commit has i/d cache enabled on all harts. But it was
later moved to boot hart due to SMP support.

Not all harts will enable i/d cache during startup currently, only
main hart will enable i/d cache here.
So only main hart will disable i/d cache in cleanup_before_linux().

Ok, so we have ax25 where cache is disabled on all harts before Linux,
and fu740 where cache will be enabled on all harts. Is there any
guidance from Linux on what should be happening here?

--Sean

Thanks,
Rick


However on the cleanup side, it looks only the boot hart calls i/d
cache disable?


See arch/riscv/cpu/ax25/cpu.c:: cleanup_before_linux()

Regards,
Bin





Reply via email to