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

[Yao Zi <ziyao@disroot.org>]

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