Re: [PATCH v11 03/29] linker_lists: set LINKER_LIST_ALIGN to 8 for CPU_MIPS64

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2408 bytes --]

On Fri, Oct 04, 2024 at 02:55:58PM +0200, Jerome Forissier wrote:
> 
> 
> On 10/4/24 14:04, Heinrich Schuchardt wrote:
> > On 03.10.24 17:22, Jerome Forissier wrote:
> >> Note: Patch posted separately [0].
> >>
> >> [0] https://patchwork.ozlabs.org/project/uboot/patch/20241003142030.1610222-1-jerome.forissier@linaro.org/
> >>
> >> CPU_MIPS64 needs 8-byte alignment on the linker lists, otherwise an
> >> exception may occur. Fixes an issue found on malta64 with QEMU:
> >>
> >>   Breakpoint 1, lists_driver_lookup_name (name=0xffffffffbe043578 "root_driver") at /home/uboot/u-boot/drivers/core/lists.c:31
> >>   31                      if (!strcmp(name, entry->name))
> >>   [...]
> >>   ld      a1,0(s0)
> >>
> >>   (gdb) p/x &entry->name
> >>   0xffffffffbe04b0d4
> >>   (gdb) p/x $s0
> >>   0xffffffffbe04b0d4
> >>
> >>   $ grep __u_boot_list /tmp/malta64/u-boot.objdump
> >>   4 __u_boot_list 000018e0  ffffffffbe04a4d4  ffffffffbe04a4d4  0004a584  2**2
> >>
> >> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> >> ---
> >>   arch/Kconfig | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/Kconfig b/arch/Kconfig
> >> index 8f1f4667012..8f4df849801 100644
> >> --- a/arch/Kconfig
> >> +++ b/arch/Kconfig
> >> @@ -45,7 +45,7 @@ config SYS_CACHELINE_SIZE
> >>   config LINKER_LIST_ALIGN
> >>       int
> >>       default 32 if SANDBOX
> >> -    default 8 if ARM64 || X86
> >> +    default 8 if ARM64 || X86 || CPU_MIPS64
> > 
> > Shouldn't we set 8 byte alignment on all 64bit architectures including
> > riscv64?
> > 
> >     default 8 if 64BIT
> 
> Makes sense.
> 
> > 
> > @Simon
> > I would not know why 32bit X86 should need 8 byte alignment.
> > I am a bit astonished that you chose 32 byte alignment for the Sandbox.
> > Is there any justification to use more than 4 on the 32bit Sandbox and
> > more than 8 on the 64bit Sandbox? If yes, we should document it via a
> > comment.
> 
> I tried running what CI does manually (.azure_pipelines.yml) with
> "default 32 if SANDBOX" removed, and I got SIGSEGV with
> TEST_PY_BD=sandbox. OTOH, TEST_PY_BD=sandbox64 worked fine but it might
> have been a fluke.

I think we can leave sandbox as a future cleanup, and perhaps file an
issue at https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/ so
it doesn't get lost.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]