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

[Rick Chen]

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

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