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

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

[Green Wan]

Add a callback interface, riscv_hart_early_init() to perform riscv CPU 
hart-dependent configuration for each hart. riscv_hart_early_init() is
placed after stack of harts are initialized and before main boot hart is
picked up. Several conditions should be aware of or avoided are listed:

  - cannot access gd
    At the moment, gd hasn't initialized yet.

  - all operations in riscv_hart_early_init() should only affect core itself
    For example, the operations for board level should be moved to mach_xxx()
    or board_init_xxx() functions instead.

  - Common resource need protection if multicore
    Since all harts might enter riscv_hart_early_init() in parallel, common
    resource need a mechanism to handle race condition.

  - A dummy implementation is added in ./arch/riscv/cpu/cpu.c
    The implementation is declared with '__weak' and can be overridden.

Green Wan (2):
  arch: riscv: cpu: Add callback to init each core
  board: sifive: unmatched: clear feature disable CSR

 arch/riscv/cpu/cpu.c         | 15 +++++++++++++++
 arch/riscv/cpu/start.S       | 14 ++++++++++++++
 board/sifive/unmatched/spl.c | 16 ++++++++++++++++
 3 files changed, 45 insertions(+)

-- 
2.31.0

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

[Green Wan]

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@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..c779a477c7 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, to turn on or clear chicken bits.
+ *
+ * This function is executed by each core after stack is initialized and not
+ * expect to access gd since gd is not initialized. All operations in this
+ * 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.
+ */
+__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
+
 #ifndef CONFIG_XIP
 	/*
 	 * Pick hart to initialize global data and run U-Boot. The other harts
-- 
2.31.0

[RFC PATCH v3 2/2] board: sifive: unmatched: clear feature disable CSR

[Green Wan]

Clear feature disable CSR to turn on all features of hart. The detail
is specified at section, 'SiFive Feature Disable CSR', in user manual

https://sifive.cdn.prismic.io/sifive/aee0dd4c-d156-496e-a6c4-db0cf54bbe68_sifive_U74MC_rtl_full_20G1.03.00_manual.pdf

Signed-off-by: Green Wan <green.wan@sifive.com>
---
 board/sifive/unmatched/spl.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/board/sifive/unmatched/spl.c b/board/sifive/unmatched/spl.c
index 5e1333b09a..e3f3d82df4 100644
--- a/board/sifive/unmatched/spl.c
+++ b/board/sifive/unmatched/spl.c
@@ -12,6 +12,7 @@
 #include <log.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <asm/csr.h>
 #include <asm/gpio.h>
 #include <asm/arch/gpio.h>
 #include <asm/arch/spl.h>
@@ -22,6 +23,21 @@
 #define MODE_SELECT_SD		0xb
 #define MODE_SELECT_MASK	GENMASK(3, 0)
 
+#define CSR_U74_FEATURE_DISABLE	0x7c1
+
+void riscv_hart_early_init(void)
+{
+	/*
+	 * Feature Disable CSR
+	 *
+	 * Clear feature disable CSR to '0' to turn on all features for
+	 * each core. This operation must be in m-mode.
+	 */
+	if (CONFIG_IS_ENABLED(RISCV_MMODE)) {
+		csr_write(CSR_U74_FEATURE_DISABLE, 0);
+	}
+}
+
 int spl_board_init_f(void)
 {
 	int ret;
-- 
2.31.0

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

[Sean Anderson]

On 3/26/21 10:18 PM, 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@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..c779a477c7 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, to turn on or clear chicken bits.

What's a chicken bit?

> + *
> + * This function is executed by each core after stack is initialized and not
> + * expect to access gd since gd is not initialized. All operations in this
> + * 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.

Perhaps just

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.

--Sean

> + */
> +__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
> +
>   #ifndef CONFIG_XIP
>   	/*
>   	 * Pick hart to initialize global data and run U-Boot. The other harts
> 

[RFC PATCH v3 2/2] board: sifive: unmatched: clear feature disable CSR

[Sean Anderson]

On 3/26/21 10:18 PM, Green Wan wrote:
> Clear feature disable CSR to turn on all features of hart. The detail
> is specified at section, 'SiFive Feature Disable CSR', in user manual
> 
> https://sifive.cdn.prismic.io/sifive/aee0dd4c-d156-496e-a6c4-db0cf54bbe68_sifive_U74MC_rtl_full_20G1.03.00_manual.pdf
> 
> Signed-off-by: Green Wan <green.wan@sifive.com>
> ---
>   board/sifive/unmatched/spl.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/board/sifive/unmatched/spl.c b/board/sifive/unmatched/spl.c
> index 5e1333b09a..e3f3d82df4 100644
> --- a/board/sifive/unmatched/spl.c
> +++ b/board/sifive/unmatched/spl.c
> @@ -12,6 +12,7 @@
>   #include <log.h>
>   #include <linux/delay.h>
>   #include <linux/io.h>
> +#include <asm/csr.h>
>   #include <asm/gpio.h>
>   #include <asm/arch/gpio.h>
>   #include <asm/arch/spl.h>
> @@ -22,6 +23,21 @@
>   #define MODE_SELECT_SD		0xb
>   #define MODE_SELECT_MASK	GENMASK(3, 0)
>   
> +#define CSR_U74_FEATURE_DISABLE	0x7c1
> +
> +void riscv_hart_early_init(void)
> +{
> +	/*
> +	 * Feature Disable CSR
> +	 *
> +	 * Clear feature disable CSR to '0' to turn on all features for
> +	 * each core. This operation must be in m-mode.
> +	 */
> +	if (CONFIG_IS_ENABLED(RISCV_MMODE)) {
> +		csr_write(CSR_U74_FEATURE_DISABLE, 0);
> +	}

These braces are not strictly necessary.

> +}
> +
>   int spl_board_init_f(void)
>   {
>   	int ret;
> 

Reviewed-by: Sean Anderson <seanga2@gmail.com>

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

[Green Wan]

On Mon, Mar 29, 2021 at 10:26 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> On 3/26/21 10:18 PM, 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@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..c779a477c7 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, to turn on or clear chicken bits.
>
> What's a chicken bit?

Perhaps I can rephrase to
"For example, turn on or off the functional block of CPU harts."

Chicken bits are the bit to control the functional blocks of a chip.

>
> > + *
> > + * This function is executed by each core after stack is initialized and not
> > + * expect to access gd since gd is not initialized. All operations in this
> > + * 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.
>
> Perhaps just
>
> 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.

okay, thanks for revising. will replace it.

>
> --Sean
>
> > + */
> > +__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
> > +
> >   #ifndef CONFIG_XIP
> >       /*
> >        * Pick hart to initialize global data and run U-Boot. The other harts
> >
>

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

[Green Wan]

Oaky, thanks for sharing the defconfig info.

-
Green

Leo Liang <ycliang@andestech.com>? 2021?3?29? ?????10:24???

> On Fri, Mar 26, 2021 at 05:03:54PM +0800, Green Wan wrote:
> > On Fri, Mar 26, 2021 at 9:34 AM Sean Anderson <seanga2@gmail.com> wrote:
> > >
> > > On 3/25/21 9:22 PM, Leo Liang wrote:
> > > > Hi Green,
> > > >
> > > > On Tue, Mar 23, 2021 at 01:35:38AM -0700, 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 calback, they must be
> able
> > > >> to run the same setup.
> > > >>
> > > >> Signed-off-by: Green Wan <green.wan@sifive.com>
> > > >> ---
> > > >>   arch/riscv/cpu/start.S | 5 +++++
> > > >>   arch/riscv/lib/spl.c   | 4 ++++
> > > >
> > > > This patch fails to compile for some defconfigs.
> > > > (CI result:
> https://dev.azure.com/ycliang-tw/u-boot-riscv/_build/results?buildId=15&view=results
> )
> > > >
> > > > Maybe we should come up with a better place to add this function.
> > >
> > > arch/riscv/cpu/cpu.c is a good candidate.
> >
> > Hi Leo,
> >
> > Thanks for reporting the result. Looks like the test doesn't use
> > "CONFIG_SPL_BUILD"? Thanks to Sean's suggestion, I will move dummy
> > riscv_hart_early_init() from ./arch/riscv/lib/spl.c to
> > ./arch/riscv/cpu/cpu.c to create the next patch.
> >
> > BTW, it's nice if you can share the .config file or CONFIG_* related
> > to CPU? I'd like to check around whether there are other possibilities
> > to break the code building? Thanks,
> >
> > -- Green
> >
> Hi Green,
>
> Sorry for the late reply.
> It seems that this v2 patch would fail to compile with but not limited to
> the following configs.
>         qemu-riscv[32|64]_defconfig, ae350_rv[32|64]_defconfig...
>
> The v3 patch seems to fix the compile error, thanks!
>
> Best regards,
> Leo
> >
> > >
> > > --Sean
> > >
> > > >
> > > > Best regards,
> > > > Leo
> > > >
> > > >>   2 files changed, 9 insertions(+)
> > > >>
> > > >> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> > > >> index 8589509e01..5c7d4da9e2 100644
> > > >> --- a/arch/riscv/cpu/start.S
> > > >> +++ b/arch/riscv/cpu/start.S
> > > >> @@ -117,6 +117,11 @@ call_board_init_f_0:
> > > >>      mv      sp, a0
> > > >>   #endif
> > > >>
> > > >> +#if CONFIG_IS_ENABLED(RISCV_MMODE)
> > > >> +call_riscv_hart_early_init:
> > > >> +    jal     riscv_hart_early_init
> > > >> +#endif
> > > >> +
> > > >>   #ifndef CONFIG_XIP
> > > >>      /*
> > > >>       * Pick hart to initialize global data and run U-Boot. The
> other harts
> > > >> diff --git a/arch/riscv/lib/spl.c b/arch/riscv/lib/spl.c
> > > >> index 8baee07bea..e5b1affbfc 100644
> > > >> --- a/arch/riscv/lib/spl.c
> > > >> +++ b/arch/riscv/lib/spl.c
> > > >> @@ -14,6 +14,10 @@
> > > >>
> > > >>   DECLARE_GLOBAL_DATA_PTR;
> > > >>
> > > >> +__weak void riscv_hart_early_init(void)
> > > >> +{
> > > >> +}
> > > >> +
> > > >>   __weak int spl_board_init_f(void)
> > > >>   {
> > > >>      return 0;
> > >
>

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

[Leo Liang]

On Fri, Mar 26, 2021 at 05:03:54PM +0800, Green Wan wrote:
> On Fri, Mar 26, 2021 at 9:34 AM Sean Anderson <seanga2@gmail.com> wrote:
> >
> > On 3/25/21 9:22 PM, Leo Liang wrote:
> > > Hi Green,
> > >
> > > On Tue, Mar 23, 2021 at 01:35:38AM -0700, 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 calback, they must be able
> > >> to run the same setup.
> > >>
> > >> Signed-off-by: Green Wan <green.wan@sifive.com>
> > >> ---
> > >>   arch/riscv/cpu/start.S | 5 +++++
> > >>   arch/riscv/lib/spl.c   | 4 ++++
> > >
> > > This patch fails to compile for some defconfigs.
> > > (CI result: https://dev.azure.com/ycliang-tw/u-boot-riscv/_build/results?buildId=15&view=results)
> > >
> > > Maybe we should come up with a better place to add this function.
> >
> > arch/riscv/cpu/cpu.c is a good candidate.
> 
> Hi Leo,
> 
> Thanks for reporting the result. Looks like the test doesn't use
> "CONFIG_SPL_BUILD"? Thanks to Sean's suggestion, I will move dummy
> riscv_hart_early_init() from ./arch/riscv/lib/spl.c to
> ./arch/riscv/cpu/cpu.c to create the next patch.
> 
> BTW, it's nice if you can share the .config file or CONFIG_* related
> to CPU? I'd like to check around whether there are other possibilities
> to break the code building? Thanks,
> 
> -- Green
> 
Hi Green,

Sorry for the late reply.
It seems that this v2 patch would fail to compile with but not limited to the following configs. 
	qemu-riscv[32|64]_defconfig, ae350_rv[32|64]_defconfig...

The v3 patch seems to fix the compile error, thanks!

Best regards,
Leo
> 
> >
> > --Sean
> >
> > >
> > > Best regards,
> > > Leo
> > >
> > >>   2 files changed, 9 insertions(+)
> > >>
> > >> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> > >> index 8589509e01..5c7d4da9e2 100644
> > >> --- a/arch/riscv/cpu/start.S
> > >> +++ b/arch/riscv/cpu/start.S
> > >> @@ -117,6 +117,11 @@ call_board_init_f_0:
> > >>      mv      sp, a0
> > >>   #endif
> > >>
> > >> +#if CONFIG_IS_ENABLED(RISCV_MMODE)
> > >> +call_riscv_hart_early_init:
> > >> +    jal     riscv_hart_early_init
> > >> +#endif
> > >> +
> > >>   #ifndef CONFIG_XIP
> > >>      /*
> > >>       * Pick hart to initialize global data and run U-Boot. The other harts
> > >> diff --git a/arch/riscv/lib/spl.c b/arch/riscv/lib/spl.c
> > >> index 8baee07bea..e5b1affbfc 100644
> > >> --- a/arch/riscv/lib/spl.c
> > >> +++ b/arch/riscv/lib/spl.c
> > >> @@ -14,6 +14,10 @@
> > >>
> > >>   DECLARE_GLOBAL_DATA_PTR;
> > >>
> > >> +__weak void riscv_hart_early_init(void)
> > >> +{
> > >> +}
> > >> +
> > >>   __weak int spl_board_init_f(void)
> > >>   {
> > >>      return 0;
> >