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