| Message ID | 017478f9-76d2-4dc4-de93-b662c4552968@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/EFI: build adjustments | expand |
On Thu, Apr 01, 2021 at 11:45:09AM +0200, Jan Beulich wrote: > While they apparently do no harm when building xen.efi, their use is > potentially misleading. Conditionalize their use to be for just the ELF > binary we produce. > > No change to the resulting binaries. The GNU Linker manual notes that program headers would be ignored when not generating an ELF file, so I'm not sure it's worth us adding more churn to the linker script to hide something that's already ignored by ld already. Maybe adding a comment noting program headers are ignored when not generating an ELF output would be enough? Thanks, Roger.
On 21.04.2021 11:11, Roger Pau Monné wrote: > On Thu, Apr 01, 2021 at 11:45:09AM +0200, Jan Beulich wrote: >> While they apparently do no harm when building xen.efi, their use is >> potentially misleading. Conditionalize their use to be for just the ELF >> binary we produce. >> >> No change to the resulting binaries. > > The GNU Linker manual notes that program headers would be ignored when > not generating an ELF file, so I'm not sure it's worth us adding more > churn to the linker script to hide something that's already ignored by > ld already. > > Maybe adding a comment noting program headers are ignored when not > generating an ELF output would be enough? Maybe, but I'd prefer this to be explicit, and I'd prefer for efi.lds to not have any PE-unrelated baggage. The churn by this patch isn't all this significant, is it? In fact in two cases it actually deletes meaningless stuff. Jan
On Wed, Apr 21, 2021 at 12:36:16PM +0200, Jan Beulich wrote: > On 21.04.2021 11:11, Roger Pau Monné wrote: > > On Thu, Apr 01, 2021 at 11:45:09AM +0200, Jan Beulich wrote: > >> While they apparently do no harm when building xen.efi, their use is > >> potentially misleading. Conditionalize their use to be for just the ELF > >> binary we produce. > >> > >> No change to the resulting binaries. > > > > The GNU Linker manual notes that program headers would be ignored when > > not generating an ELF file, so I'm not sure it's worth us adding more > > churn to the linker script to hide something that's already ignored by > > ld already. > > > > Maybe adding a comment noting program headers are ignored when not > > generating an ELF output would be enough? > > Maybe, but I'd prefer this to be explicit, and I'd prefer for efi.lds > to not have any PE-unrelated baggage. The churn by this patch isn't > all this significant, is it? In fact in two cases it actually deletes > meaningless stuff. Acked-by: Roger Pau Monné <roger.pau@citrix.com> I would prefer if the new PHDR macro was used for all program headers directives for consistency though. Thanks, Roger.
On 21.04.2021 16:21, Roger Pau Monné wrote: > On Wed, Apr 21, 2021 at 12:36:16PM +0200, Jan Beulich wrote: >> On 21.04.2021 11:11, Roger Pau Monné wrote: >>> On Thu, Apr 01, 2021 at 11:45:09AM +0200, Jan Beulich wrote: >>>> While they apparently do no harm when building xen.efi, their use is >>>> potentially misleading. Conditionalize their use to be for just the ELF >>>> binary we produce. >>>> >>>> No change to the resulting binaries. >>> >>> The GNU Linker manual notes that program headers would be ignored when >>> not generating an ELF file, so I'm not sure it's worth us adding more >>> churn to the linker script to hide something that's already ignored by >>> ld already. >>> >>> Maybe adding a comment noting program headers are ignored when not >>> generating an ELF output would be enough? >> >> Maybe, but I'd prefer this to be explicit, and I'd prefer for efi.lds >> to not have any PE-unrelated baggage. The churn by this patch isn't >> all this significant, is it? In fact in two cases it actually deletes >> meaningless stuff. > > Acked-by: Roger Pau Monné <roger.pau@citrix.com> Thanks. > I would prefer if the new PHDR macro was used for all program headers > directives for consistency though. Well, yes, I can certainly do so. Jan
--- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -34,13 +34,19 @@ OUTPUT_FORMAT(FORMAT, FORMAT, FORMAT) OUTPUT_ARCH(i386:x86-64) +#ifndef EFI PHDRS { text PT_LOAD ; -#if (defined(BUILD_ID) || defined (CONFIG_PVH_GUEST)) && !defined(EFI) +#if defined(BUILD_ID) || defined(CONFIG_PVH_GUEST) note PT_NOTE ; #endif } +#define PHDR(x) :x +#else +#define PHDR(x) +#endif + SECTIONS { #if !defined(EFI) @@ -83,7 +89,7 @@ SECTIONS *(.text.kexec) *(.gnu.warning) _etext = .; /* End of text section */ - } :text = 0x9090 + } PHDR(text) = 0x9090 . = ALIGN(SECTION_ALIGN); __2M_text_end = .; @@ -134,7 +140,7 @@ SECTIONS *(SORT(.data.vpci.*)) __end_vpci_array = .; #endif - } :text + } PHDR(text) #if defined(CONFIG_PVH_GUEST) && !defined(EFI) DECL_SECTION(.note.Xen) { @@ -160,7 +166,7 @@ SECTIONS __note_gnu_build_id_start = .; *(.buildid) __note_gnu_build_id_end = .; - } :text + } #endif #endif @@ -260,7 +266,7 @@ SECTIONS *(SORT(.data.vpci.*)) __end_vpci_array = .; #endif - } :text + } PHDR(text) . = ALIGN(SECTION_ALIGN); __init_end = .; @@ -281,7 +287,7 @@ SECTIONS *(.data.paramhypfs) __paramhypfs_end = .; #endif - } :text + } PHDR(text) DECL_SECTION(.data) { *(.data.page_aligned) @@ -289,7 +295,7 @@ SECTIONS *(.data.rel) *(.data.rel.*) CONSTRUCTORS - } :text + } PHDR(text) DECL_SECTION(.bss) { __bss_start = .; @@ -306,7 +312,7 @@ SECTIONS *(.bss) . = ALIGN(POINTER_ALIGN); __bss_end = .; - } :text + } PHDR(text) _end = . ; . = ALIGN(SECTION_ALIGN); @@ -316,12 +322,12 @@ SECTIONS . = ALIGN(4); DECL_SECTION(.reloc) { *(.reloc) - } :text + } /* Trick the linker into setting the image size to exactly 16Mb. */ . = ALIGN(__section_alignment__); DECL_SECTION(.pad) { . = ALIGN(MB(16)); - } :text + } #endif #ifndef XEN_BUILD_EFI
While they apparently do no harm when building xen.efi, their use is potentially misleading. Conditionalize their use to be for just the ELF binary we produce. No change to the resulting binaries. Signed-off-by: Jan Beulich <jbeulich@suse.com>