| Message ID | 20240517-mips-efi-v1-3-79096e3ca3b3@flygoat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | MIPS: Enable EFI support | expand |
Hi Jiaxun On Fri, 17 May 2024 at 19:33, Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > > This is required in performing objcopy to MIPS EFI files. > This seems not mips specific. What was missing for your case? Thanks /Ilias > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > --- > scripts/Makefile.lib | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 62f87517c09c..52aed7a65d47 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -469,7 +469,7 @@ $(obj)/%_efi.S: $(obj)/%.efi > > quiet_cmd_efi_objcopy = OBJCOPY $@ > cmd_efi_objcopy = $(OBJCOPY) -j .header -j .text -j .sdata -j .data -j \ > - .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc \ > + .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc -j .rodata \ > $(if $(EFI_TARGET),$(EFI_TARGET),-O binary) $^ $@ > > $(obj)/%.efi: $(obj)/%_efi.so > > -- > 2.34.1 >
在2024年6月11日六月 下午2:49，Ilias Apalodimas写道： > Hi Jiaxun > > > On Fri, 17 May 2024 at 19:33, Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: >> >> This is required in performing objcopy to MIPS EFI files. >> > > This seems not mips specific. What was missing for your case? So MIPS EFI file would come with .rodata section. If we don't preserve it here, the whole section would be stripped away. .rodata is always necessary in binary if it's in ELF as it contains valid program data. Other architectures won't generate .rodata section in their ELF file so it's harmless. Besides GNU-EFI's universal strip commands also have -j .rodata argument. Thanks - Jiaxun > > Thanks > /Ilias >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> >> --- >> scripts/Makefile.lib | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >> index 62f87517c09c..52aed7a65d47 100644 >> --- a/scripts/Makefile.lib >> +++ b/scripts/Makefile.lib >> @@ -469,7 +469,7 @@ $(obj)/%_efi.S: $(obj)/%.efi >> >> quiet_cmd_efi_objcopy = OBJCOPY $@ >> cmd_efi_objcopy = $(OBJCOPY) -j .header -j .text -j .sdata -j .data -j \ >> - .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc \ >> + .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc -j .rodata \ >> $(if $(EFI_TARGET),$(EFI_TARGET),-O binary) $^ $@ >> >> $(obj)/%.efi: $(obj)/%_efi.so >> >> -- >> 2.34.1 >>
On 11.06.24 16:06, Jiaxun Yang wrote: > > > 在2024年6月11日六月 下午2:49，Ilias Apalodimas写道： >> Hi Jiaxun >> >> >> On Fri, 17 May 2024 at 19:33, Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: >>> >>> This is required in performing objcopy to MIPS EFI files. >>> >> >> This seems not mips specific. What was missing for your case? > > So MIPS EFI file would come with .rodata section. If we don't preserve > it here, the whole section would be stripped away. > > .rodata is always necessary in binary if it's in ELF as it contains > valid program data. > > Other architectures won't generate .rodata section in their ELF file > so it's harmless. Besides GNU-EFI's universal strip commands also have > -j .rodata argument. > > Thanks > - Jiaxun >> >> Thanks >> /Ilias >>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> >>> --- >>> scripts/Makefile.lib | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >>> index 62f87517c09c..52aed7a65d47 100644 >>> --- a/scripts/Makefile.lib >>> +++ b/scripts/Makefile.lib >>> @@ -469,7 +469,7 @@ $(obj)/%_efi.S: $(obj)/%.efi >>> >>> quiet_cmd_efi_objcopy = OBJCOPY $@ >>> cmd_efi_objcopy = $(OBJCOPY) -j .header -j .text -j .sdata -j .data -j \ >>> - .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc \ >>> + .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc -j .rodata \ >>> $(if $(EFI_TARGET),$(EFI_TARGET),-O binary) $^ $@ >>> >>> $(obj)/%.efi: $(obj)/%_efi.so >>> >>> -- >>> 2.34.1 >>> > We have: arch/arm/lib/elf_aarch64_efi.lds:26: *(.rodata*) arch/arm/lib/elf_arm_efi.lds:26: *(.rodata*) arch/riscv/lib/elf_riscv32_efi.lds:26: *(.rodata*) arch/riscv/lib/elf_riscv64_efi.lds:26: *(.rodata*) arch/x86/lib/elf_ia32_efi.lds:35: *(.rodata*) arch/x86/lib/elf_x86_64_efi.lds:37: *(.rodata*) Not considering .rodata in objcopy looks inconsistent. As we use -fdata-sections wouldn't we expect a .rodata* section per function with constants on LoongArch? Shouldn't we use: + .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc -j .rodata* \ Best regards Heinrich
在2024年6月11日六月 下午3:28，Heinrich Schuchardt写道： [...] > > We have: > > arch/arm/lib/elf_aarch64_efi.lds:26: *(.rodata*) > arch/arm/lib/elf_arm_efi.lds:26: *(.rodata*) > arch/riscv/lib/elf_riscv32_efi.lds:26: *(.rodata*) > arch/riscv/lib/elf_riscv64_efi.lds:26: *(.rodata*) > arch/x86/lib/elf_ia32_efi.lds:35: *(.rodata*) > arch/x86/lib/elf_x86_64_efi.lds:37: *(.rodata*) > > Not considering .rodata in objcopy looks inconsistent. > > As we use -fdata-sections wouldn't we expect a .rodata* section per > function with constants on LoongArch? Shouldn't we use: > > + .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc -j .rodata* \ That makes sense, will take in next version. Thanks > > Best regards > > Heinrich
On Tue, 11 Jun 2024 at 17:07, Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > > > > 在2024年6月11日六月 下午2:49，Ilias Apalodimas写道： > > Hi Jiaxun > > > > > > On Fri, 17 May 2024 at 19:33, Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > >> > >> This is required in performing objcopy to MIPS EFI files. > >> > > > > This seems not mips specific. What was missing for your case? > > So MIPS EFI file would come with .rodata section. If we don't preserve > it here, the whole section would be stripped away. > > .rodata is always necessary in binary if it's in ELF as it contains > valid program data. > > Other architectures won't generate .rodata section in their ELF file > so it's harmless. > Besides GNU-EFI's universal strip commands also have > -j .rodata argument. Yea I am not arguing that this is useless or dangerous, I was just wondering if more architectures need .rodata so we could make the commit message a bit more generic Cheers /Ilias > > Thanks > - Jiaxun > > > > Thanks > > /Ilias > >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > >> --- > >> scripts/Makefile.lib | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > >> index 62f87517c09c..52aed7a65d47 100644 > >> --- a/scripts/Makefile.lib > >> +++ b/scripts/Makefile.lib > >> @@ -469,7 +469,7 @@ $(obj)/%_efi.S: $(obj)/%.efi > >> > >> quiet_cmd_efi_objcopy = OBJCOPY $@ > >> cmd_efi_objcopy = $(OBJCOPY) -j .header -j .text -j .sdata -j .data -j \ > >> - .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc \ > >> + .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc -j .rodata \ > >> $(if $(EFI_TARGET),$(EFI_TARGET),-O binary) $^ $@ > >> > >> $(obj)/%.efi: $(obj)/%_efi.so > >> > >> -- > >> 2.34.1 > >> > > -- > - Jiaxun
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 62f87517c09c..52aed7a65d47 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -469,7 +469,7 @@ $(obj)/%_efi.S: $(obj)/%.efi quiet_cmd_efi_objcopy = OBJCOPY $@ cmd_efi_objcopy = $(OBJCOPY) -j .header -j .text -j .sdata -j .data -j \ - .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc \ + .dynamic -j .dynsym -j .rel* -j .rela* -j .reloc -j .rodata \ $(if $(EFI_TARGET),$(EFI_TARGET),-O binary) $^ $@ $(obj)/%.efi: $(obj)/%_efi.so
This is required in performing objcopy to MIPS EFI files. Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> --- scripts/Makefile.lib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)