[PATCH 1/1] riscv: consider CONFIG_RISCV_ISA_ZAAMO in SPL too

[PATCH 1/1] riscv: consider CONFIG_RISCV_ISA_ZAAMO in SPL too

[Heinrich Schuchardt]

Commit a681cfecb434 ("riscv: Add a Zalrsc-only alternative for
synchronization in start.S") changed the hart synchronization in start.S.
It uses CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO) to determine which method to
use. If the macro evaluates to true the old behavior is maintained.

The macro evaluates to false for SPL builds which was unintended. Use
IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO) instead.

This fixes a boot failure on StarFive JH7110 based boards.

Fixes: a681cfecb434 ("riscv: Add a Zalrsc-only alternative for synchronization in start.S")
Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 arch/riscv/cpu/start.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
index 6324ff585d4..87b3ff0f93f 100644
--- a/arch/riscv/cpu/start.S
+++ b/arch/riscv/cpu/start.S
@@ -151,7 +151,7 @@ call_harts_early_init:
 	 */
 	la	t0, hart_lottery
 	li	t1, 1
-#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
+#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
 	amoswap.w s2, t1, 0(t0)
 	bnez	s2, wait_for_gd_init
 #else
@@ -184,7 +184,7 @@ call_harts_early_init:
 #if !CONFIG_IS_ENABLED(XIP)
 #ifdef CONFIG_AVAILABLE_HARTS
 	la	t0, available_harts_lock
-#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
+#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
 	amoswap.w.rl zero, zero, 0(t0)
 #else
 	fence	rw, w
@@ -203,7 +203,7 @@ wait_for_gd_init:
 	la	t0, available_harts_lock
 	li	t1, 1
 1:
-#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
+#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
 	amoswap.w.aq t1, t1, 0(t0)
 #else
 	lr.w.aq	t1, 0(t0)
@@ -219,7 +219,7 @@ wait_for_gd_init:
 	or	t2, t2, t1
 	SREG	t2, GD_AVAILABLE_HARTS(gp)
 
-#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
+#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
 	amoswap.w.rl zero, zero, 0(t0)
 #else
 	fence	rw, w
-- 
2.51.0

Re: [PATCH 1/1] riscv: consider CONFIG_RISCV_ISA_ZAAMO in SPL too

[Heinrich Schuchardt]

On 10/16/25 18:58, Heinrich Schuchardt wrote:
> Commit a681cfecb434 ("riscv: Add a Zalrsc-only alternative for
> synchronization in start.S") changed the hart synchronization in start.S.
> It uses CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO) to determine which method to
> use. If the macro evaluates to true the old behavior is maintained.
> 
> The macro evaluates to false for SPL builds which was unintended. Use
> IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO) instead.
> 
> This fixes a boot failure on StarFive JH7110 based boards.
> 
> Fixes: a681cfecb434 ("riscv: Add a Zalrsc-only alternative for synchronization in start.S")
> Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---

Hello Yao,

It would be worthwhile to understand why your new method does not work 
on the StarFive VisionFive 2. I only addressed the Kconfig issue. But I 
suspect there is something broken in the alternative pass.

Best regards

Heinrich

>   arch/riscv/cpu/start.S | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> index 6324ff585d4..87b3ff0f93f 100644
> --- a/arch/riscv/cpu/start.S
> +++ b/arch/riscv/cpu/start.S
> @@ -151,7 +151,7 @@ call_harts_early_init:
>   	 */
>   	la	t0, hart_lottery
>   	li	t1, 1
> -#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
> +#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
>   	amoswap.w s2, t1, 0(t0)
>   	bnez	s2, wait_for_gd_init
>   #else
> @@ -184,7 +184,7 @@ call_harts_early_init:
>   #if !CONFIG_IS_ENABLED(XIP)
>   #ifdef CONFIG_AVAILABLE_HARTS
>   	la	t0, available_harts_lock
> -#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
> +#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
>   	amoswap.w.rl zero, zero, 0(t0)
>   #else
>   	fence	rw, w
> @@ -203,7 +203,7 @@ wait_for_gd_init:
>   	la	t0, available_harts_lock
>   	li	t1, 1
>   1:
> -#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
> +#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
>   	amoswap.w.aq t1, t1, 0(t0)
>   #else
>   	lr.w.aq	t1, 0(t0)
> @@ -219,7 +219,7 @@ wait_for_gd_init:
>   	or	t2, t2, t1
>   	SREG	t2, GD_AVAILABLE_HARTS(gp)
>   
> -#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
> +#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
>   	amoswap.w.rl zero, zero, 0(t0)
>   #else
>   	fence	rw, w

Re: [PATCH 1/1] riscv: consider CONFIG_RISCV_ISA_ZAAMO in SPL too

[E Shattow]

Hi Heinrich, thank you for your attention to this.

On 10/16/25 09:58, Heinrich Schuchardt wrote:
> Commit a681cfecb434 ("riscv: Add a Zalrsc-only alternative for
> synchronization in start.S") changed the hart synchronization in start.S.
> It uses CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO) to determine which method to
> use. If the macro evaluates to true the old behavior is maintained.
> 
> The macro evaluates to false for SPL builds which was unintended. Use
> IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO) instead.
> 
> This fixes a boot failure on StarFive JH7110 based boards.
> 
> Fixes: a681cfecb434 ("riscv: Add a Zalrsc-only alternative for synchronization in start.S")
> Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  arch/riscv/cpu/start.S | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> index 6324ff585d4..87b3ff0f93f 100644
> --- a/arch/riscv/cpu/start.S
> +++ b/arch/riscv/cpu/start.S
> @@ -151,7 +151,7 @@ call_harts_early_init:
>  	 */
>  	la	t0, hart_lottery
>  	li	t1, 1
> -#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
> +#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
>  	amoswap.w s2, t1, 0(t0)
>  	bnez	s2, wait_for_gd_init
>  #else
> @@ -184,7 +184,7 @@ call_harts_early_init:
>  #if !CONFIG_IS_ENABLED(XIP)
>  #ifdef CONFIG_AVAILABLE_HARTS
>  	la	t0, available_harts_lock
> -#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
> +#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
>  	amoswap.w.rl zero, zero, 0(t0)
>  #else
>  	fence	rw, w
> @@ -203,7 +203,7 @@ wait_for_gd_init:
>  	la	t0, available_harts_lock
>  	li	t1, 1
>  1:
> -#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
> +#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
>  	amoswap.w.aq t1, t1, 0(t0)
>  #else
>  	lr.w.aq	t1, 0(t0)
> @@ -219,7 +219,7 @@ wait_for_gd_init:
>  	or	t2, t2, t1
>  	SREG	t2, GD_AVAILABLE_HARTS(gp)
>  
> -#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
> +#if IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO)
>  	amoswap.w.rl zero, zero, 0(t0)
>  #else
>  	fence	rw, w

I confirm this restores Pine64 Star64 (JH-7110) SPL boot in origin/next

Are we reverting the bad commit or papering over it with a fix?

Ref. "Revert "riscv: Add a Zalrsc-only alternative for synchronization
in start.S":

https://lore.kernel.org/u-boot/aOylBeAxyoKuPJYa@pie/

In any event, for this patch,

Tested-by: E Shattow <e@freeshell.de>

Re: [PATCH 1/1] riscv: consider CONFIG_RISCV_ISA_ZAAMO in SPL too

[Yao Zi]

On Thu, Oct 16, 2025 at 07:04:48PM +0200, Heinrich Schuchardt wrote:
> On 10/16/25 18:58, Heinrich Schuchardt wrote:
> > Commit a681cfecb434 ("riscv: Add a Zalrsc-only alternative for
> > synchronization in start.S") changed the hart synchronization in start.S.
> > It uses CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO) to determine which method to
> > use. If the macro evaluates to true the old behavior is maintained.
> > 
> > The macro evaluates to false for SPL builds which was unintended. Use
> > IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO) instead.
> > 
> > This fixes a boot failure on StarFive JH7110 based boards.
> > 
> > Fixes: a681cfecb434 ("riscv: Add a Zalrsc-only alternative for synchronization in start.S")
> > Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > ---

Hi Heinrich,

Thanks for sending the patch!

> 
> Hello Yao,
> 
> It would be worthwhile to understand why your new method does not work on
> the StarFive VisionFive 2. I only addressed the Kconfig issue. But I suspect
> there is something broken in the alternative pass.

A possible reason is that, on JH7110 not all the cores support LR/SC
instructions: the S7 small core always raises access exception for it
since the implementation of LR/SC requires data cache to present, while
S7 doesn't have one.

This is documented in SiFive U74-MC Core Complex Manual,

> Load-reserved (LR) and store-conditional (SC) instructions are special
> atomic instructions that are only supported in data cacheable regions.
> As the S7 core does not have a data cache, the LR and SC instructions
> will always generate a precise access exception.[1]

As S7 also takes part in the initial HART lottery, usage of LR/SC
instructions might just crash its boot process with exception.

However I could only confirm the idea days later when I have access to
a JH7110 board. Since earlier Tom has merged the patch[2] that reverts
the problematic commit, do you think it's better to wait for me to
confirm the cause and write v3 of the patch, instead of fixing it up
now? I'll carry your Co-developed-by tag in v3 if you're okay with this.

> Best regards
> 
> Heinrich

Best regards,
Yao Zi

[1]: https://starfivetech.com/uploads/u74mc_core_complex_manual_21G1.pdf
(Chapter 3.6, Atomic Memory Operations)
[2]: https://lore.kernel.org/u-boot/176063629917.212270.1145034876136991424.b4-ty@konsulko.com/

Re: [PATCH 1/1] riscv: consider CONFIG_RISCV_ISA_ZAAMO in SPL too

[Heinrich Schuchardt]

On 10/17/25 03:33, Yao Zi wrote:
> On Thu, Oct 16, 2025 at 07:04:48PM +0200, Heinrich Schuchardt wrote:
>> On 10/16/25 18:58, Heinrich Schuchardt wrote:
>>> Commit a681cfecb434 ("riscv: Add a Zalrsc-only alternative for
>>> synchronization in start.S") changed the hart synchronization in start.S.
>>> It uses CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO) to determine which method to
>>> use. If the macro evaluates to true the old behavior is maintained.
>>>
>>> The macro evaluates to false for SPL builds which was unintended. Use
>>> IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO) instead.
>>>
>>> This fixes a boot failure on StarFive JH7110 based boards.
>>>
>>> Fixes: a681cfecb434 ("riscv: Add a Zalrsc-only alternative for synchronization in start.S")
>>> Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

e6646b35f410 ("Revert "riscv: Add a Zalrsc-only alternative for 
synchronization in start.S"") has been merged this night. Hence the 
current patch is superseded.

ibex-ast2700_defconfig for which the original patch was developed does 
not build w/ or w/o the revert ("undefined reference to `__ashldi3'") on 
Ubuntu 25.10 using GCC 15.2.

>>> ---
> 
> Hi Heinrich,
> 
> Thanks for sending the patch!
> 
>>
>> Hello Yao,
>>
>> It would be worthwhile to understand why your new method does not work on
>> the StarFive VisionFive 2. I only addressed the Kconfig issue. But I suspect
>> there is something broken in the alternative pass.
> 
> A possible reason is that, on JH7110 not all the cores support LR/SC
> instructions: the S7 small core always raises access exception for it
> since the implementation of LR/SC requires data cache to present, while
> S7 doesn't have one.

@Hal, @Minda
does the S7 core enter U-Boot SPL at all?

Best regards

Heinrich

> 
> This is documented in SiFive U74-MC Core Complex Manual,
> 
>> Load-reserved (LR) and store-conditional (SC) instructions are special
>> atomic instructions that are only supported in data cacheable regions.
>> As the S7 core does not have a data cache, the LR and SC instructions
>> will always generate a precise access exception.[1]
> 
> As S7 also takes part in the initial HART lottery, usage of LR/SC
> instructions might just crash its boot process with exception.
> 
> However I could only confirm the idea days later when I have access to
> a JH7110 board. Since earlier Tom has merged the patch[2] that reverts
> the problematic commit, do you think it's better to wait for me to
> confirm the cause and write v3 of the patch, instead of fixing it up
> now? I'll carry your Co-developed-by tag in v3 if you're okay with this.
> 
>> Best regards
>>
>> Heinrich
> 
> Best regards,
> Yao Zi
> 
> [1]: https://starfivetech.com/uploads/u74mc_core_complex_manual_21G1.pdf
> (Chapter 3.6, Atomic Memory Operations)
> [2]: https://lore.kernel.org/u-boot/176063629917.212270.1145034876136991424.b4-ty@konsulko.com/

RE: [PATCH 1/1] riscv: consider CONFIG_RISCV_ISA_ZAAMO in SPL too

[Hal Feng]

> On 17.10.25 15:12, Heinrich Schuchardt wrote:
> On 10/17/25 03:33, Yao Zi wrote:
> > On Thu, Oct 16, 2025 at 07:04:48PM +0200, Heinrich Schuchardt wrote:
> >> On 10/16/25 18:58, Heinrich Schuchardt wrote:
> >>> Commit a681cfecb434 ("riscv: Add a Zalrsc-only alternative for
> >>> synchronization in start.S") changed the hart synchronization in start.S.
> >>> It uses CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO) to determine which
> method
> >>> to use. If the macro evaluates to true the old behavior is maintained.
> >>>
> >>> The macro evaluates to false for SPL builds which was unintended.
> >>> Use
> >>> IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO) instead.
> >>>
> >>> This fixes a boot failure on StarFive JH7110 based boards.
> >>>
> >>> Fixes: a681cfecb434 ("riscv: Add a Zalrsc-only alternative for
> >>> synchronization in start.S")
> >>> Reported-by: Emil Renner Berthing
> >>> <emil.renner.berthing@canonical.com>
> >>> Signed-off-by: Heinrich Schuchardt
> >>> <heinrich.schuchardt@canonical.com>
> 
> e6646b35f410 ("Revert "riscv: Add a Zalrsc-only alternative for
> synchronization in start.S"") has been merged this night. Hence the current
> patch is superseded.
> 
> ibex-ast2700_defconfig for which the original patch was developed does not
> build w/ or w/o the revert ("undefined reference to `__ashldi3'") on Ubuntu
> 25.10 using GCC 15.2.
> 
> >>> ---
> >
> > Hi Heinrich,
> >
> > Thanks for sending the patch!
> >
> >>
> >> Hello Yao,
> >>
> >> It would be worthwhile to understand why your new method does not
> >> work on the StarFive VisionFive 2. I only addressed the Kconfig
> >> issue. But I suspect there is something broken in the alternative pass.
> >
> > A possible reason is that, on JH7110 not all the cores support LR/SC
> > instructions: the S7 small core always raises access exception for it
> > since the implementation of LR/SC requires data cache to present,
> > while
> > S7 doesn't have one.
> 
> @Hal, @Minda
> does the S7 core enter U-Boot SPL at all?

No, I think SPL runs on U74 at all and S7 is disabled in the device tree.

Best regards,
Hal

> 
> >
> > This is documented in SiFive U74-MC Core Complex Manual,
> >
> >> Load-reserved (LR) and store-conditional (SC) instructions are
> >> special atomic instructions that are only supported in data cacheable
> regions.
> >> As the S7 core does not have a data cache, the LR and SC instructions
> >> will always generate a precise access exception.[1]
> >
> > As S7 also takes part in the initial HART lottery, usage of LR/SC
> > instructions might just crash its boot process with exception.
> >
> > However I could only confirm the idea days later when I have access to
> > a JH7110 board. Since earlier Tom has merged the patch[2] that reverts
> > the problematic commit, do you think it's better to wait for me to
> > confirm the cause and write v3 of the patch, instead of fixing it up
> > now? I'll carry your Co-developed-by tag in v3 if you're okay with this.
> >
> >> Best regards
> >>
> >> Heinrich
> >
> > Best regards,
> > Yao Zi
> >
> > [1]:
> > https://starfivetech.com/uploads/u74mc_core_complex_manual_21G1.pdf
> > (Chapter 3.6, Atomic Memory Operations)
> > [2]:
> > https://lore.kernel.org/u-
> boot/176063629917.212270.1145034876136991424
> > .b4-ty@konsulko.com/

Re: [PATCH 1/1] riscv: consider CONFIG_RISCV_ISA_ZAAMO in SPL too

[Yao Zi]

On Fri, Oct 17, 2025 at 01:33:11AM +0000, Yao Zi wrote:
> On Thu, Oct 16, 2025 at 07:04:48PM +0200, Heinrich Schuchardt wrote:
> > On 10/16/25 18:58, Heinrich Schuchardt wrote:
> > > Commit a681cfecb434 ("riscv: Add a Zalrsc-only alternative for
> > > synchronization in start.S") changed the hart synchronization in start.S.
> > > It uses CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO) to determine which method to
> > > use. If the macro evaluates to true the old behavior is maintained.
> > > 
> > > The macro evaluates to false for SPL builds which was unintended. Use
> > > IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO) instead.
> > > 
> > > This fixes a boot failure on StarFive JH7110 based boards.
> > > 
> > > Fixes: a681cfecb434 ("riscv: Add a Zalrsc-only alternative for synchronization in start.S")
> > > Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > ---
> 
> Hi Heinrich,
> 
> Thanks for sending the patch!
> 
> > 
> > Hello Yao,
> > 
> > It would be worthwhile to understand why your new method does not work on
> > the StarFive VisionFive 2. I only addressed the Kconfig issue. But I suspect
> > there is something broken in the alternative pass.
> 
> A possible reason is that, on JH7110 not all the cores support LR/SC
> instructions: the S7 small core always raises access exception for it
> since the implementation of LR/SC requires data cache to present, while
> S7 doesn't have one.
> 
> This is documented in SiFive U74-MC Core Complex Manual,
> 
> > Load-reserved (LR) and store-conditional (SC) instructions are special
> > atomic instructions that are only supported in data cacheable regions.
> > As the S7 core does not have a data cache, the LR and SC instructions
> > will always generate a precise access exception.[1]
> 
> As S7 also takes part in the initial HART lottery, usage of LR/SC
> instructions might just crash its boot process with exception.
> 
> However I could only confirm the idea days later when I have access to
> a JH7110 board. Since earlier Tom has merged the patch[2] that reverts
> the problematic commit, do you think it's better to wait for me to
> confirm the cause and write v3 of the patch, instead of fixing it up
> now? I'll carry your Co-developed-by tag in v3 if you're okay with this.

Hi Heinrich,

I finally got a JH7110 board on hand and could find out what happens
here. As stated in the U74-MC Core Complex Manual,

> > Load-reserved (LR) and store-conditional (SC) instructions are special
> > atomic instructions that are only supported in data cacheable regions.
> > As the S7 core does not have a data cache, the LR and SC instructions
> > will always generate a precise access exception.[1]

This is correct, and with my patch the S7 core does trigger an
exception when doing LR/SC stuff, but this alone isn't enough to break
the whole SPL booting process up without even a banner shown up.

With some hand-written assembly added, I found that not only the S7
core, but also the four U74 cores are trapped in exceptions as well.
This is because U-Boot SPL, including the available_harts_lock variable,
is loaded in 0x8000000, as pointed out by CONFIG_SPL_TEXT_BASE and
spl/u-boot-spl.sym. According to StarFive JH7110's manual[3], the region
is mapped to the L2 Loosely-Integrated Memory.

And the U74-MC Core Complex Manual again points out,

> When cache ways are disabled, they are addressable in the L2
> Loosely-Integrated Memory (L2 LIM) address space as described in the
> U74-MC Core Complex memory map in Section 5.2. The L2 LIM is an
> uncacheable port into unused L2 SRAM and provides deterministic
> access time.

The L2 LIM region is uncachable, too. Since U74 and S7 could only
perform LR/SC instructions in cachable regions, the new LR/SC
implementation will always trigger an Load Access Fault exception, thus
doesn't work for JH7110.

Another simple patch[4] could confirm this, which just adds a simple
load-reserved operation on available_harts_lock in
arch/riscv/cpu/start.S, but still hangs the boot process on JH7110.

I've reviewed v2 of my patch again, and think besides the usage of
CONFIG_IS_ENALBED() which you just pointed out, there should be no logic
error. I'll probably send v3 of the patch carrying your fix along with
a new patch to explicitly disable RISCV_ISA_ZALRSC on JH7110 platforms,

Thanks again for sending the fix.

Best regards,
Yao Zi

> > Best regards
> > 
> > Heinrich
> 
> Best regards,
> Yao Zi
> 
> [1]: https://starfivetech.com/uploads/u74mc_core_complex_manual_21G1.pdf
> (Chapter 3.6, Atomic Memory Operations)
> [2]: https://lore.kernel.org/u-boot/176063629917.212270.1145034876136991424.b4-ty@konsulko.com/

[3]: https://doc-en.rvspace.org/JH7110/TRM/JH7110_TRM/u74_memory_map.html
[4]: https://gist.github.com/ziyao233/75fc0e40956a19e4383ab360dd06c784

Re: [PATCH 1/1] riscv: consider CONFIG_RISCV_ISA_ZAAMO in SPL too

[Heinrich Schuchardt]

On 10/20/25 11:13, Yao Zi wrote:
> On Fri, Oct 17, 2025 at 01:33:11AM +0000, Yao Zi wrote:
>> On Thu, Oct 16, 2025 at 07:04:48PM +0200, Heinrich Schuchardt wrote:
>>> On 10/16/25 18:58, Heinrich Schuchardt wrote:
>>>> Commit a681cfecb434 ("riscv: Add a Zalrsc-only alternative for
>>>> synchronization in start.S") changed the hart synchronization in start.S.
>>>> It uses CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO) to determine which method to
>>>> use. If the macro evaluates to true the old behavior is maintained.
>>>>
>>>> The macro evaluates to false for SPL builds which was unintended. Use
>>>> IS_ENABLED(CONFIG_RISCV_ISA_ZAAMO) instead.
>>>>
>>>> This fixes a boot failure on StarFive JH7110 based boards.
>>>>
>>>> Fixes: a681cfecb434 ("riscv: Add a Zalrsc-only alternative for synchronization in start.S")
>>>> Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
>>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>>> ---
>>
>> Hi Heinrich,
>>
>> Thanks for sending the patch!
>>
>>>
>>> Hello Yao,
>>>
>>> It would be worthwhile to understand why your new method does not work on
>>> the StarFive VisionFive 2. I only addressed the Kconfig issue. But I suspect
>>> there is something broken in the alternative pass.
>>
>> A possible reason is that, on JH7110 not all the cores support LR/SC
>> instructions: the S7 small core always raises access exception for it
>> since the implementation of LR/SC requires data cache to present, while
>> S7 doesn't have one.
>>
>> This is documented in SiFive U74-MC Core Complex Manual,
>>
>>> Load-reserved (LR) and store-conditional (SC) instructions are special
>>> atomic instructions that are only supported in data cacheable regions.
>>> As the S7 core does not have a data cache, the LR and SC instructions
>>> will always generate a precise access exception.[1]
>>
>> As S7 also takes part in the initial HART lottery, usage of LR/SC
>> instructions might just crash its boot process with exception.
>>
>> However I could only confirm the idea days later when I have access to
>> a JH7110 board. Since earlier Tom has merged the patch[2] that reverts
>> the problematic commit, do you think it's better to wait for me to
>> confirm the cause and write v3 of the patch, instead of fixing it up
>> now? I'll carry your Co-developed-by tag in v3 if you're okay with this.
> 
> Hi Heinrich,
> 
> I finally got a JH7110 board on hand and could find out what happens
> here. As stated in the U74-MC Core Complex Manual,
> 
>>> Load-reserved (LR) and store-conditional (SC) instructions are special
>>> atomic instructions that are only supported in data cacheable regions.
>>> As the S7 core does not have a data cache, the LR and SC instructions
>>> will always generate a precise access exception.[1]
> 
> This is correct, and with my patch the S7 core does trigger an
> exception when doing LR/SC stuff, but this alone isn't enough to break
> the whole SPL booting process up without even a banner shown up.
> 
> With some hand-written assembly added, I found that not only the S7
> core, but also the four U74 cores are trapped in exceptions as well.
> This is because U-Boot SPL, including the available_harts_lock variable,
> is loaded in 0x8000000, as pointed out by CONFIG_SPL_TEXT_BASE and
> spl/u-boot-spl.sym. According to StarFive JH7110's manual[3], the region
> is mapped to the L2 Loosely-Integrated Memory.
> 
> And the U74-MC Core Complex Manual again points out,
> 
>> When cache ways are disabled, they are addressable in the L2
>> Loosely-Integrated Memory (L2 LIM) address space as described in the
>> U74-MC Core Complex memory map in Section 5.2. The L2 LIM is an
>> uncacheable port into unused L2 SRAM and provides deterministic
>> access time.
> 
> The L2 LIM region is uncachable, too. Since U74 and S7 could only
> perform LR/SC instructions in cachable regions, the new LR/SC
> implementation will always trigger an Load Access Fault exception, thus
> doesn't work for JH7110.
> 
> Another simple patch[4] could confirm this, which just adds a simple
> load-reserved operation on available_harts_lock in
> arch/riscv/cpu/start.S, but still hangs the boot process on JH7110.
> 
> I've reviewed v2 of my patch again, and think besides the usage of
> CONFIG_IS_ENALBED() which you just pointed out, there should be no logic
> error. I'll probably send v3 of the patch carrying your fix along with
> a new patch to explicitly disable RISCV_ISA_ZALRSC on JH7110 platforms,
> 
> Thanks again for sending the fix.
> 
> Best regards,
> Yao Zi

Thank you for investigating

+CC Hal, Minda as JH7110 maintainers

Best regards

Heinrich

> 
>>> Best regards
>>>
>>> Heinrich
>>
>> Best regards,
>> Yao Zi
>>
>> [1]: https://starfivetech.com/uploads/u74mc_core_complex_manual_21G1.pdf
>> (Chapter 3.6, Atomic Memory Operations)
>> [2]: https://lore.kernel.org/u-boot/176063629917.212270.1145034876136991424.b4-ty@konsulko.com/
> 
> [3]: https://doc-en.rvspace.org/JH7110/TRM/JH7110_TRM/u74_memory_map.html
> [4]: https://gist.github.com/ziyao233/75fc0e40956a19e4383ab360dd06c784