| Message ID | 20250407033744.4025-3-ziyao@disroot.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Andes |
| Headers | show |
| Series | Fix binman_sym functionality on RISC-V port | expand |
+Heinrich Schuchardt On Mon, 7 Apr 2025 at 15:38, Yao Zi <ziyao@disroot.org> wrote: > > Binman looks for __image_copy_start to determine the base address of an > entry if elf-base-sym isn't specified, which is missing in RISC-V port. > This causes binman skips RISC-V SPL entries without filling addresses > into its .binman_sym_table section. > > This patch defines __image_copy_start in linkerscript of both SPL and > proper U-Boot to ensure binman_sym functions correctly with the default > binman.dtsi. The paired symbol, __image_copy_end, is introduced as well > for completeness. > > Signed-off-by: Yao Zi <ziyao@disroot.org> > --- > arch/riscv/cpu/u-boot-spl.lds | 2 ++ > arch/riscv/cpu/u-boot.lds | 3 +++ > 2 files changed, 5 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> > > diff --git a/arch/riscv/cpu/u-boot-spl.lds b/arch/riscv/cpu/u-boot-spl.lds > index 907094620bd..18eb62c01c6 100644 > --- a/arch/riscv/cpu/u-boot-spl.lds > +++ b/arch/riscv/cpu/u-boot-spl.lds > @@ -16,6 +16,7 @@ ENTRY(_start) > SECTIONS > { > . = ALIGN(4); > + __image_copy_start = ADDR(.text); > .text : { > arch/riscv/cpu/start.o (.text) > *(.text*) > @@ -46,6 +47,7 @@ SECTIONS > > _end = .; > _image_binary_end = .; > + _image_copy_end = .; > > .bss : { > __bss_start = .; > diff --git a/arch/riscv/cpu/u-boot.lds b/arch/riscv/cpu/u-boot.lds > index 2ffe6ba3c8f..b11ea8b56d2 100644 > --- a/arch/riscv/cpu/u-boot.lds > +++ b/arch/riscv/cpu/u-boot.lds > @@ -10,6 +10,7 @@ ENTRY(_start) > SECTIONS > { > . = ALIGN(4); > + __image_copy_start = ADDR(.text); > .text : { > arch/riscv/cpu/start.o (.text) > } > @@ -57,6 +58,8 @@ SECTIONS > __efi_runtime_rel_stop = .; > } > > + __image_copy_end = .; > + > /DISCARD/ : { *(.rela.plt*) } > .rela.dyn : { > __rel_dyn_start = .; > -- > 2.49.0 >
Hi, On 2025-04-07 05:37, Yao Zi wrote: > Binman looks for __image_copy_start to determine the base address of an > entry if elf-base-sym isn't specified, which is missing in RISC-V port. > This causes binman skips RISC-V SPL entries without filling addresses > into its .binman_sym_table section. > > This patch defines __image_copy_start in linkerscript of both SPL and > proper U-Boot to ensure binman_sym functions correctly with the default > binman.dtsi. The paired symbol, __image_copy_end, is introduced as well > for completeness. > > Signed-off-by: Yao Zi <ziyao@disroot.org> > --- > arch/riscv/cpu/u-boot-spl.lds | 2 ++ > arch/riscv/cpu/u-boot.lds | 3 +++ > 2 files changed, 5 insertions(+) > > diff --git a/arch/riscv/cpu/u-boot-spl.lds b/arch/riscv/cpu/u-boot-spl.lds > index 907094620bd..18eb62c01c6 100644 > --- a/arch/riscv/cpu/u-boot-spl.lds > +++ b/arch/riscv/cpu/u-boot-spl.lds > @@ -16,6 +16,7 @@ ENTRY(_start) > SECTIONS > { > . = ALIGN(4); > + __image_copy_start = ADDR(.text); > .text : { > arch/riscv/cpu/start.o (.text) > *(.text*) > @@ -46,6 +47,7 @@ SECTIONS > > _end = .; > _image_binary_end = .; > + _image_copy_end = .; This looks like a typo, should probably be with two _ ? Regards, Jonas > > .bss : { > __bss_start = .; > diff --git a/arch/riscv/cpu/u-boot.lds b/arch/riscv/cpu/u-boot.lds > index 2ffe6ba3c8f..b11ea8b56d2 100644 > --- a/arch/riscv/cpu/u-boot.lds > +++ b/arch/riscv/cpu/u-boot.lds > @@ -10,6 +10,7 @@ ENTRY(_start) > SECTIONS > { > . = ALIGN(4); > + __image_copy_start = ADDR(.text); > .text : { > arch/riscv/cpu/start.o (.text) > } > @@ -57,6 +58,8 @@ SECTIONS > __efi_runtime_rel_stop = .; > } > > + __image_copy_end = .; > + > /DISCARD/ : { *(.rela.plt*) } > .rela.dyn : { > __rel_dyn_start = .;
On Mon, Apr 07, 2025 at 01:10:32PM +0200, Jonas Karlman wrote: > Hi, > > On 2025-04-07 05:37, Yao Zi wrote: > > Binman looks for __image_copy_start to determine the base address of an > > entry if elf-base-sym isn't specified, which is missing in RISC-V port. > > This causes binman skips RISC-V SPL entries without filling addresses > > into its .binman_sym_table section. > > > > This patch defines __image_copy_start in linkerscript of both SPL and > > proper U-Boot to ensure binman_sym functions correctly with the default > > binman.dtsi. The paired symbol, __image_copy_end, is introduced as well > > for completeness. > > > > Signed-off-by: Yao Zi <ziyao@disroot.org> > > --- > > arch/riscv/cpu/u-boot-spl.lds | 2 ++ > > arch/riscv/cpu/u-boot.lds | 3 +++ > > 2 files changed, 5 insertions(+) > > > > diff --git a/arch/riscv/cpu/u-boot-spl.lds b/arch/riscv/cpu/u-boot-spl.lds > > index 907094620bd..18eb62c01c6 100644 > > --- a/arch/riscv/cpu/u-boot-spl.lds > > +++ b/arch/riscv/cpu/u-boot-spl.lds > > @@ -16,6 +16,7 @@ ENTRY(_start) > > SECTIONS > > { > > . = ALIGN(4); > > + __image_copy_start = ADDR(.text); > > .text : { > > arch/riscv/cpu/start.o (.text) > > *(.text*) > > @@ -46,6 +47,7 @@ SECTIONS > > > > _end = .; > > _image_binary_end = .; > > + _image_copy_end = .; > > This looks like a typo, should probably be with two _ ? Yes, thanks for catching this. I'll correct it and apply Simon's tag in v2. Simon, is it okay for you? Thanks, Yao Zi > Regards, > Jonas > > > > > .bss : { > > __bss_start = .; > > diff --git a/arch/riscv/cpu/u-boot.lds b/arch/riscv/cpu/u-boot.lds > > index 2ffe6ba3c8f..b11ea8b56d2 100644 > > --- a/arch/riscv/cpu/u-boot.lds > > +++ b/arch/riscv/cpu/u-boot.lds > > @@ -10,6 +10,7 @@ ENTRY(_start) > > SECTIONS > > { > > . = ALIGN(4); > > + __image_copy_start = ADDR(.text); > > .text : { > > arch/riscv/cpu/start.o (.text) > > } > > @@ -57,6 +58,8 @@ SECTIONS > > __efi_runtime_rel_stop = .; > > } > > > > + __image_copy_end = .; > > + > > /DISCARD/ : { *(.rela.plt*) } > > .rela.dyn : { > > __rel_dyn_start = .; >
Hi Yao, On Tue, 8 Apr 2025 at 03:32, Yao Zi <ziyao@disroot.org> wrote: > > On Mon, Apr 07, 2025 at 01:10:32PM +0200, Jonas Karlman wrote: > > Hi, > > > > On 2025-04-07 05:37, Yao Zi wrote: > > > Binman looks for __image_copy_start to determine the base address of an > > > entry if elf-base-sym isn't specified, which is missing in RISC-V port. > > > This causes binman skips RISC-V SPL entries without filling addresses > > > into its .binman_sym_table section. > > > > > > This patch defines __image_copy_start in linkerscript of both SPL and > > > proper U-Boot to ensure binman_sym functions correctly with the default > > > binman.dtsi. The paired symbol, __image_copy_end, is introduced as well > > > for completeness. > > > > > > Signed-off-by: Yao Zi <ziyao@disroot.org> > > > --- > > > arch/riscv/cpu/u-boot-spl.lds | 2 ++ > > > arch/riscv/cpu/u-boot.lds | 3 +++ > > > 2 files changed, 5 insertions(+) > > > > > > diff --git a/arch/riscv/cpu/u-boot-spl.lds b/arch/riscv/cpu/u-boot-spl.lds > > > index 907094620bd..18eb62c01c6 100644 > > > --- a/arch/riscv/cpu/u-boot-spl.lds > > > +++ b/arch/riscv/cpu/u-boot-spl.lds > > > @@ -16,6 +16,7 @@ ENTRY(_start) > > > SECTIONS > > > { > > > . = ALIGN(4); > > > + __image_copy_start = ADDR(.text); > > > .text : { > > > arch/riscv/cpu/start.o (.text) > > > *(.text*) > > > @@ -46,6 +47,7 @@ SECTIONS > > > > > > _end = .; > > > _image_binary_end = .; > > > + _image_copy_end = .; > > > > This looks like a typo, should probably be with two _ ? > > Yes, thanks for catching this. I'll correct it and apply Simon's tag in > v2. Simon, is it okay for you? Yes. Regards, Simon
diff --git a/arch/riscv/cpu/u-boot-spl.lds b/arch/riscv/cpu/u-boot-spl.lds index 907094620bd..18eb62c01c6 100644 --- a/arch/riscv/cpu/u-boot-spl.lds +++ b/arch/riscv/cpu/u-boot-spl.lds @@ -16,6 +16,7 @@ ENTRY(_start) SECTIONS { . = ALIGN(4); + __image_copy_start = ADDR(.text); .text : { arch/riscv/cpu/start.o (.text) *(.text*) @@ -46,6 +47,7 @@ SECTIONS _end = .; _image_binary_end = .; + _image_copy_end = .; .bss : { __bss_start = .; diff --git a/arch/riscv/cpu/u-boot.lds b/arch/riscv/cpu/u-boot.lds index 2ffe6ba3c8f..b11ea8b56d2 100644 --- a/arch/riscv/cpu/u-boot.lds +++ b/arch/riscv/cpu/u-boot.lds @@ -10,6 +10,7 @@ ENTRY(_start) SECTIONS { . = ALIGN(4); + __image_copy_start = ADDR(.text); .text : { arch/riscv/cpu/start.o (.text) } @@ -57,6 +58,8 @@ SECTIONS __efi_runtime_rel_stop = .; } + __image_copy_end = .; + /DISCARD/ : { *(.rela.plt*) } .rela.dyn : { __rel_dyn_start = .;
Binman looks for __image_copy_start to determine the base address of an entry if elf-base-sym isn't specified, which is missing in RISC-V port. This causes binman skips RISC-V SPL entries without filling addresses into its .binman_sym_table section. This patch defines __image_copy_start in linkerscript of both SPL and proper U-Boot to ensure binman_sym functions correctly with the default binman.dtsi. The paired symbol, __image_copy_end, is introduced as well for completeness. Signed-off-by: Yao Zi <ziyao@disroot.org> --- arch/riscv/cpu/u-boot-spl.lds | 2 ++ arch/riscv/cpu/u-boot.lds | 3 +++ 2 files changed, 5 insertions(+)