[RFC PATCH v4 1/2] arch: riscv: cpu: Add callback to init each core

Rick Chen rickchen36 at gmail.com
Tue Apr 13 06:12:27 CEST 2021


Hi Sean

> On 4/12/21 10:39 PM, Rick Chen wrote:
> > Hi Green,
> >
> >> From: Green Wan [mailto:green.wan at 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.
>

Maybe we can add two callbacks (early_lottery_init and
late_lottery_init) before and after lottery individually for all
scenarios.

Thanks,
Rick

> --Sean
>
> >
> > Thanks,
> > Rick
> >
> >>
> >> Regards,
> >> Green
> >>
> >> On Sun, Apr 11, 2021 at 11:43 PM Sean Anderson <seanga2 at 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 <seanga2 at gmail.com> wrote:
> >>>>>
> >>>>> On 4/9/21 4:16 AM, Rick Chen wrote:
> >>>>>> Hi Sean ,Bin
> >>>>>>
> >>>>>>> From: Bin Meng [mailto:bmeng.cn at 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 <seanga2 at 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.wan at 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 <rick at 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 <rick at andestech.com>
> >>>>>>>        Cc: Greentime Hu <greentime at 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
> >>>>>
> >>>>>
> >>>
>
>


More information about the U-Boot mailing list