| Message ID | 1699de5384a71c0d0ad535cdc48cc2b95087719d.1727968902.git.jerome.forissier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce the lwIP network stack | expand |
On Thu, 3 Oct 2024 at 18:23, Jerome Forissier <jerome.forissier@linaro.org> 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 > default 4 > help > Force the each linker list to be aligned to this boundary. This > -- > 2.40.1 > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
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 @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. Best regards Heinrich > default 4 > help > Force the each linker list to be aligned to this boundary. This
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. Thanks,
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.
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 default 4 help Force the each linker list to be aligned to this boundary. This
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(-)