Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
>>> On 25.06.19 at 14:48, wrote: > On Tue, Jun 25, 2019 at 01:08:49PM +0200, Roger Pau Monné wrote: >> Sorry for not being clear. By remove I mean `git rm >> xen/arch/x86/efi/relocs-dummy.S` and fix the build, like the diff >> appended below. > > The chunk below will not work because relocs-dummy is also needed > by the EFI build. I'm however lost at why this is required, and the > commit message that introduced the file (bf6501a62e) doesn't add any > reasoning. > > Is maybe .reloc mandatory for PE format? Yes, almost. You _can_ have one without .reloc, but then you're tied to it loading at the linked address. That's fine with an ordinary boot loader, but it's not an option when this is to get loaded just like a normal binary, without knowing at which address it'll be placed. Remember that the EFI boot environment runs in (pseudo)physical mode, i.e. there's a 1:1 mapping between linear and physical addresses. Therefore there's no way to predict a memory range that's always going to be available (and that hence xen.efi could be linked to load at). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
>>> On 25.06.19 at 13:08, wrote: > Sorry for not being clear. By remove I mean `git rm > xen/arch/x86/efi/relocs-dummy.S` and fix the build, like the diff > appended below. > > Is there any reason we should keep the dummy .reloc in the ELF > output? Yes, there is. And yes, I was afraid you'd mean this. > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > @@ -39,6 +39,7 @@ extern const intpte_t __page_tables_start[], > __page_tables_end[]; > #define PE_BASE_RELOC_HIGHLOW 3 > #define PE_BASE_RELOC_DIR64 10 > > +#ifdef XEN_BUILD_PE This is an identifier available to Makefiles only. You also can't propagate it to .c files, as these get compiled just once to produce _both_ PE and ELF binary. So while what you suggest may build, it'll result in a broken xen.efi. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
On Tue, Jun 25, 2019 at 01:08:49PM +0200, Roger Pau Monné wrote: > On Tue, Jun 25, 2019 at 03:18:14AM -0600, Jan Beulich wrote: > > >>> On 25.06.19 at 10:10, wrote: > > > On Mon, Jun 24, 2019 at 01:24:02PM +0200, Daniel Kiper wrote: > > >> On Fri, Jun 21, 2019 at 12:34:13AM -0600, Jan Beulich wrote: > > >> > >>> On 19.06.19 at 17:06, wrote: > > >> > > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote: > > >> > >> >>> On 19.06.19 at 13:02, wrote: > > >> > >> > If the hypervisor has been built with EFI support (ie: > > >> > >> > multiboot2). > > >> > >> > This allows to position the .reloc section correctly in the output > > >> > >> > binary, or else the linker might place .reloc before the .text > > >> > >> > section. > > >> > >> > > > >> > >> > Note that the .reloc section is moved before .bss for two > > >> > >> > reasons: in > > >> > >> > order for the resulting binary to not contain any section with > > >> > >> > data > > >> > >> > after .bss, so that the file size can be smaller than the loaded > > >> > >> > memory size, and because the data it contains is read-only, so it > > >> > >> > belongs with the other sections containing read-only data. > > >> > >> > > >> > >> While this may be fine for ELF, I'm afraid it would be calling for > > >> > >> subtle issues with xen.efi (i.e. the PE binary): There a .reloc > > >> > >> section is generally expected to come after "normal" data > > >> > >> sections. > > >> > > > > >> > > OK, would you like me to leave the .reloc section at the previous > > >> > > position for EFI builds then? > > >> > > > >> > Well, this part is a requirement, not a question of me liking you > > >> > to do so. > > >> > > > >> > > Or do we prefer to leave .reloc orphaned in the ELF build? > > >> > > > >> > Daniel might have an opinion here with his plans to actually > > >> > add relocations there in the non-linker-generated-PE build. I > > >> > don't have a strong opinion either way, as long as the > > >> > current method of building gets left as is (or even simplified). > > >> > > >> I would not drop .reloc section from xen-syms because it can be useful > > >> for "manual" EFI image relocs generation. However, I am not strongly > > >> tied to it. If you wish to drop it go ahead. I can readd it latter if > > >> I get back to my new PE build work. > > > > > > Do you mean that the dummy .reloc section added to non-PE builds can > > > be dropped? (ie: remove xen/arch/x86/efi/relocs-dummy.S from the build) > > > > Given my earlier reply it's not clear to me what you mean by "remove" > > here. As a result ... > > > > > I'm slightly lost, .reloc begin a section that's explicitly added to > > > non-PE builds by relocs-dummy.S I assumed it was needed for some > > > reason. > > > > ... it's also not clear what exactly you mean here, and hence whether > > there's any reason needed beyond the reference to the two bounding > > symbols by efi_arch_relocate_image(). > > Sorry for not being clear. By remove I mean `git rm > xen/arch/x86/efi/relocs-dummy.S` and fix the build, like the diff > appended below. The chunk below will not work because relocs-dummy is also needed by the EFI build. I'm however lost at why this is required, and the commit message that introduced the file (bf6501a62e) doesn't add any reasoning. Is maybe .reloc mandatory for PE format? Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
On Tue, Jun 25, 2019 at 03:18:14AM -0600, Jan Beulich wrote:
> >>> On 25.06.19 at 10:10, wrote:
> > On Mon, Jun 24, 2019 at 01:24:02PM +0200, Daniel Kiper wrote:
> >> On Fri, Jun 21, 2019 at 12:34:13AM -0600, Jan Beulich wrote:
> >> > >>> On 19.06.19 at 17:06, wrote:
> >> > > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote:
> >> > >> >>> On 19.06.19 at 13:02, wrote:
> >> > >> > If the hypervisor has been built with EFI support (ie: multiboot2).
> >> > >> > This allows to position the .reloc section correctly in the output
> >> > >> > binary, or else the linker might place .reloc before the .text
> >> > >> > section.
> >> > >> >
> >> > >> > Note that the .reloc section is moved before .bss for two reasons:
> >> > >> > in
> >> > >> > order for the resulting binary to not contain any section with data
> >> > >> > after .bss, so that the file size can be smaller than the loaded
> >> > >> > memory size, and because the data it contains is read-only, so it
> >> > >> > belongs with the other sections containing read-only data.
> >> > >>
> >> > >> While this may be fine for ELF, I'm afraid it would be calling for
> >> > >> subtle issues with xen.efi (i.e. the PE binary): There a .reloc
> >> > >> section is generally expected to come after "normal" data
> >> > >> sections.
> >> > >
> >> > > OK, would you like me to leave the .reloc section at the previous
> >> > > position for EFI builds then?
> >> >
> >> > Well, this part is a requirement, not a question of me liking you
> >> > to do so.
> >> >
> >> > > Or do we prefer to leave .reloc orphaned in the ELF build?
> >> >
> >> > Daniel might have an opinion here with his plans to actually
> >> > add relocations there in the non-linker-generated-PE build. I
> >> > don't have a strong opinion either way, as long as the
> >> > current method of building gets left as is (or even simplified).
> >>
> >> I would not drop .reloc section from xen-syms because it can be useful
> >> for "manual" EFI image relocs generation. However, I am not strongly
> >> tied to it. If you wish to drop it go ahead. I can readd it latter if
> >> I get back to my new PE build work.
> >
> > Do you mean that the dummy .reloc section added to non-PE builds can
> > be dropped? (ie: remove xen/arch/x86/efi/relocs-dummy.S from the build)
>
> Given my earlier reply it's not clear to me what you mean by "remove"
> here. As a result ...
>
> > I'm slightly lost, .reloc begin a section that's explicitly added to
> > non-PE builds by relocs-dummy.S I assumed it was needed for some
> > reason.
>
> ... it's also not clear what exactly you mean here, and hence whether
> there's any reason needed beyond the reference to the two bounding
> symbols by efi_arch_relocate_image().
Sorry for not being clear. By remove I mean `git rm
xen/arch/x86/efi/relocs-dummy.S` and fix the build, like the diff
appended below.
Is there any reason we should keep the dummy .reloc in the ELF
output?
Thanks, Roger.
---8<---
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 4bc0a196e9..5849604766 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -11,6 +11,6 @@ $(call
cc-option-add,cflags-stack-boundary,CC,-mpreferred-stack-boundary=4)
$(EFIOBJ): CFLAGS-stack-boundary := $(cflags-stack-boundary)
obj-y := stub.o
-obj-$(XEN_BUILD_EFI) := $(EFIOBJ) relocs-dummy.o
+obj-$(XEN_BUILD_EFI) := $(EFIOBJ)
extra-$(XEN_BUILD_EFI) += buildid.o
nocov-$(XEN_BUILD_EFI) += stub.o
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 7a13a30bc0..2cf440e2ae 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -39,6 +39,7 @@ extern const intpte_t __page_tables_start[],
__page_tables_end[];
#define PE_BASE_RELOC_HIGHLOW 3
#define PE_BASE_RELOC_DIR64 10
+#ifdef XEN_BUILD_PE
extern const struct pe_base_relocs {
u32 rva;
u32 size;
@@ -97,6 +98,12 @@ static void __init efi_arch_relocate_image(unsigned long
delta)
base_relocs = (const void *)(base_relocs->entries + i + (i & 1));
}
}
+#else /* !XEN_BUILD_PE */
+static void __init efi_arch_relocate_image(unsigned long delta)
+{
+ASSERT_UNREACHABLE();
+}
+#endif /* XEN_BUILD_PE */
extern const s32 __trampoline_rel_start[], __trampoline_rel_stop[];
extern const s32 __trampoline_seg_start[], __trampoline_seg_stop[];
diff --git a/xen/arch/x86/efi/relocs-dummy.S b/xen/arch/x86/efi/relocs-dummy.S
deleted file mode 100644
index d928a82d53..00
--- a/xen/arch/x86/efi/relocs-dummy.S
+++ /dev/null
@@ -1,11 +0,0 @@
-
- .section .reloc, "a", @progbits
- .balign 4
-GLOBAL(__base_relocs_start)
- .long 0
- .long 8
-GLOBAL(__base_relocs_end)
-
- .globl VIRT_START, ALT_START
- .equ VIRT_START, XEN_VIRT_START
- .equ ALT_START, XEN_VIRT_END
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
>>> On 25.06.19 at 10:10, wrote: > On Mon, Jun 24, 2019 at 01:24:02PM +0200, Daniel Kiper wrote: >> On Fri, Jun 21, 2019 at 12:34:13AM -0600, Jan Beulich wrote: >> > >>> On 19.06.19 at 17:06, wrote: >> > > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote: >> > >> >>> On 19.06.19 at 13:02, wrote: >> > >> > If the hypervisor has been built with EFI support (ie: multiboot2). >> > >> > This allows to position the .reloc section correctly in the output >> > >> > binary, or else the linker might place .reloc before the .text >> > >> > section. >> > >> > >> > >> > Note that the .reloc section is moved before .bss for two reasons: in >> > >> > order for the resulting binary to not contain any section with data >> > >> > after .bss, so that the file size can be smaller than the loaded >> > >> > memory size, and because the data it contains is read-only, so it >> > >> > belongs with the other sections containing read-only data. >> > >> >> > >> While this may be fine for ELF, I'm afraid it would be calling for >> > >> subtle issues with xen.efi (i.e. the PE binary): There a .reloc >> > >> section is generally expected to come after "normal" data >> > >> sections. >> > > >> > > OK, would you like me to leave the .reloc section at the previous >> > > position for EFI builds then? >> > >> > Well, this part is a requirement, not a question of me liking you >> > to do so. >> > >> > > Or do we prefer to leave .reloc orphaned in the ELF build? >> > >> > Daniel might have an opinion here with his plans to actually >> > add relocations there in the non-linker-generated-PE build. I >> > don't have a strong opinion either way, as long as the >> > current method of building gets left as is (or even simplified). >> >> I would not drop .reloc section from xen-syms because it can be useful >> for "manual" EFI image relocs generation. However, I am not strongly >> tied to it. If you wish to drop it go ahead. I can readd it latter if >> I get back to my new PE build work. > > Do you mean that the dummy .reloc section added to non-PE builds can > be dropped? (ie: remove xen/arch/x86/efi/relocs-dummy.S from the build) Given my earlier reply it's not clear to me what you mean by "remove" here. As a result ... > I'm slightly lost, .reloc begin a section that's explicitly added to > non-PE builds by relocs-dummy.S I assumed it was needed for some > reason. ... it's also not clear what exactly you mean here, and hence whether there's any reason needed beyond the reference to the two bounding symbols by efi_arch_relocate_image(). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
On Mon, Jun 24, 2019 at 01:24:02PM +0200, Daniel Kiper wrote: > On Fri, Jun 21, 2019 at 12:34:13AM -0600, Jan Beulich wrote: > > >>> On 19.06.19 at 17:06, wrote: > > > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote: > > >> >>> On 19.06.19 at 13:02, wrote: > > >> > If the hypervisor has been built with EFI support (ie: multiboot2). > > >> > This allows to position the .reloc section correctly in the output > > >> > binary, or else the linker might place .reloc before the .text > > >> > section. > > >> > > > >> > Note that the .reloc section is moved before .bss for two reasons: in > > >> > order for the resulting binary to not contain any section with data > > >> > after .bss, so that the file size can be smaller than the loaded > > >> > memory size, and because the data it contains is read-only, so it > > >> > belongs with the other sections containing read-only data. > > >> > > >> While this may be fine for ELF, I'm afraid it would be calling for > > >> subtle issues with xen.efi (i.e. the PE binary): There a .reloc > > >> section is generally expected to come after "normal" data > > >> sections. > > > > > > OK, would you like me to leave the .reloc section at the previous > > > position for EFI builds then? > > > > Well, this part is a requirement, not a question of me liking you > > to do so. > > > > > Or do we prefer to leave .reloc orphaned in the ELF build? > > > > Daniel might have an opinion here with his plans to actually > > add relocations there in the non-linker-generated-PE build. I > > don't have a strong opinion either way, as long as the > > current method of building gets left as is (or even simplified). > > I would not drop .reloc section from xen-syms because it can be useful > for "manual" EFI image relocs generation. However, I am not strongly > tied to it. If you wish to drop it go ahead. I can readd it latter if > I get back to my new PE build work. Do you mean that the dummy .reloc section added to non-PE builds can be dropped? (ie: remove xen/arch/x86/efi/relocs-dummy.S from the build) I'm slightly lost, .reloc begin a section that's explicitly added to non-PE builds by relocs-dummy.S I assumed it was needed for some reason. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
On Fri, Jun 21, 2019 at 12:34:13AM -0600, Jan Beulich wrote: > >>> On 19.06.19 at 17:06, wrote: > > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote: > >> >>> On 19.06.19 at 13:02, wrote: > >> > If the hypervisor has been built with EFI support (ie: multiboot2). > >> > This allows to position the .reloc section correctly in the output > >> > binary, or else the linker might place .reloc before the .text > >> > section. > >> > > >> > Note that the .reloc section is moved before .bss for two reasons: in > >> > order for the resulting binary to not contain any section with data > >> > after .bss, so that the file size can be smaller than the loaded > >> > memory size, and because the data it contains is read-only, so it > >> > belongs with the other sections containing read-only data. > >> > >> While this may be fine for ELF, I'm afraid it would be calling for > >> subtle issues with xen.efi (i.e. the PE binary): There a .reloc > >> section is generally expected to come after "normal" data > >> sections. > > > > OK, would you like me to leave the .reloc section at the previous > > position for EFI builds then? > > Well, this part is a requirement, not a question of me liking you > to do so. > > > Or do we prefer to leave .reloc orphaned in the ELF build? > > Daniel might have an opinion here with his plans to actually > add relocations there in the non-linker-generated-PE build. I > don't have a strong opinion either way, as long as the > current method of building gets left as is (or even simplified). I would not drop .reloc section from xen-syms because it can be useful for "manual" EFI image relocs generation. However, I am not strongly tied to it. If you wish to drop it go ahead. I can readd it latter if I get back to my new PE build work. Daniel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
On Fri, Jun 21, 2019 at 06:07:25AM -0600, Jan Beulich wrote: > >>> On 21.06.19 at 13:46, wrote: > > On Fri, Jun 21, 2019 at 12:34:13AM -0600, Jan Beulich wrote: > >> >>> On 19.06.19 at 17:06, wrote: > >> > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote: > >> >> >>> On 19.06.19 at 13:02, wrote: > >> >> > If the hypervisor has been built with EFI support (ie: multiboot2). > >> >> > This allows to position the .reloc section correctly in the output > >> >> > binary, or else the linker might place .reloc before the .text > >> >> > section. > >> >> > > >> >> > Note that the .reloc section is moved before .bss for two reasons: in > >> >> > order for the resulting binary to not contain any section with data > >> >> > after .bss, so that the file size can be smaller than the loaded > >> >> > memory size, and because the data it contains is read-only, so it > >> >> > belongs with the other sections containing read-only data. > >> >> > >> >> While this may be fine for ELF, I'm afraid it would be calling for > >> >> subtle issues with xen.efi (i.e. the PE binary): There a .reloc > >> >> section is generally expected to come after "normal" data > >> >> sections. > >> > > >> > OK, would you like me to leave the .reloc section at the previous > >> > position for EFI builds then? > >> > >> Well, this part is a requirement, not a question of me liking you > >> to do so. > >> > >> > Or do we prefer to leave .reloc orphaned in the ELF build? > >> > >> Daniel might have an opinion here with his plans to actually > >> add relocations there in the non-linker-generated-PE build. I > >> don't have a strong opinion either way, as long as the > >> current method of building gets left as is (or even simplified). > >> > >> Also a remark regarding the title - in my builds there already is > >> a .reloc section in the ELF images, so "add" doesn't really seem > >> correct to me. It sits right after .rodata, and I would it doesn't > >> get folded into there because - for some reason - .rodata is > >> actually marked writable. > > > > AFAICT .rodata is marked writable because it contains .data.param and > > .data.rel.ro. I'm unsure why we need .data.rel.ro, I would assume that > > once the final binary has been linked .data.rel.ro would be empty, > > since there's no run time linking or relocation as Xen is a standalone > > binary. > > No - contents of sections don't get moved to other sections while > linking, unless instructed so by the linker script. In all the > relocatable objects there's going to be .data.rel.ro, and hence the > linker script has to put them somewhere (or leave it to default > placement by the linker). Right, so as long as we place .data.rel.ro inside of .rodata the resulting section is always going to be writable, due to the input .data.rel.ro being writable. > Hmm, thinking about it - are you perhaps mixing up .data.rel / > .data.rel.ro with .rel.data / .rela.data? Yes, I think I messed up. As you say, contents of sections don't move unless explicitly done by the linker script. > > Regarding .data.param it should be renamed to .rodata.param, and I > > should take a look at why it's marked as 'WA' instead of 'A'. > > Well, there's no "const" on the structure instantiations. I think there is indeed a const on the instantiation, see __param macro in init.h which is used in the declarations done with __rtparam. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
>>> On 21.06.19 at 13:46, wrote: > On Fri, Jun 21, 2019 at 12:34:13AM -0600, Jan Beulich wrote: >> >>> On 19.06.19 at 17:06, wrote: >> > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote: >> >> >>> On 19.06.19 at 13:02, wrote: >> >> > If the hypervisor has been built with EFI support (ie: multiboot2). >> >> > This allows to position the .reloc section correctly in the output >> >> > binary, or else the linker might place .reloc before the .text >> >> > section. >> >> > >> >> > Note that the .reloc section is moved before .bss for two reasons: in >> >> > order for the resulting binary to not contain any section with data >> >> > after .bss, so that the file size can be smaller than the loaded >> >> > memory size, and because the data it contains is read-only, so it >> >> > belongs with the other sections containing read-only data. >> >> >> >> While this may be fine for ELF, I'm afraid it would be calling for >> >> subtle issues with xen.efi (i.e. the PE binary): There a .reloc >> >> section is generally expected to come after "normal" data >> >> sections. >> > >> > OK, would you like me to leave the .reloc section at the previous >> > position for EFI builds then? >> >> Well, this part is a requirement, not a question of me liking you >> to do so. >> >> > Or do we prefer to leave .reloc orphaned in the ELF build? >> >> Daniel might have an opinion here with his plans to actually >> add relocations there in the non-linker-generated-PE build. I >> don't have a strong opinion either way, as long as the >> current method of building gets left as is (or even simplified). >> >> Also a remark regarding the title - in my builds there already is >> a .reloc section in the ELF images, so "add" doesn't really seem >> correct to me. It sits right after .rodata, and I would it doesn't >> get folded into there because - for some reason - .rodata is >> actually marked writable. > > AFAICT .rodata is marked writable because it contains .data.param and > .data.rel.ro. I'm unsure why we need .data.rel.ro, I would assume that > once the final binary has been linked .data.rel.ro would be empty, > since there's no run time linking or relocation as Xen is a standalone > binary. No - contents of sections don't get moved to other sections while linking, unless instructed so by the linker script. In all the relocatable objects there's going to be .data.rel.ro, and hence the linker script has to put them somewhere (or leave it to default placement by the linker). Hmm, thinking about it - are you perhaps mixing up .data.rel / .data.rel.ro with .rel.data / .rela.data? > Regarding .data.param it should be renamed to .rodata.param, and I > should take a look at why it's marked as 'WA' instead of 'A'. Well, there's no "const" on the structure instantiations. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
On Fri, Jun 21, 2019 at 12:34:13AM -0600, Jan Beulich wrote: > >>> On 19.06.19 at 17:06, wrote: > > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote: > >> >>> On 19.06.19 at 13:02, wrote: > >> > If the hypervisor has been built with EFI support (ie: multiboot2). > >> > This allows to position the .reloc section correctly in the output > >> > binary, or else the linker might place .reloc before the .text > >> > section. > >> > > >> > Note that the .reloc section is moved before .bss for two reasons: in > >> > order for the resulting binary to not contain any section with data > >> > after .bss, so that the file size can be smaller than the loaded > >> > memory size, and because the data it contains is read-only, so it > >> > belongs with the other sections containing read-only data. > >> > >> While this may be fine for ELF, I'm afraid it would be calling for > >> subtle issues with xen.efi (i.e. the PE binary): There a .reloc > >> section is generally expected to come after "normal" data > >> sections. > > > > OK, would you like me to leave the .reloc section at the previous > > position for EFI builds then? > > Well, this part is a requirement, not a question of me liking you > to do so. > > > Or do we prefer to leave .reloc orphaned in the ELF build? > > Daniel might have an opinion here with his plans to actually > add relocations there in the non-linker-generated-PE build. I > don't have a strong opinion either way, as long as the > current method of building gets left as is (or even simplified). > > Also a remark regarding the title - in my builds there already is > a .reloc section in the ELF images, so "add" doesn't really seem > correct to me. It sits right after .rodata, and I would it doesn't > get folded into there because - for some reason - .rodata is > actually marked writable. AFAICT .rodata is marked writable because it contains .data.param and .data.rel.ro. I'm unsure why we need .data.rel.ro, I would assume that once the final binary has been linked .data.rel.ro would be empty, since there's no run time linking or relocation as Xen is a standalone binary. Regarding .data.param it should be renamed to .rodata.param, and I should take a look at why it's marked as 'WA' instead of 'A'. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
>>> On 19.06.19 at 17:06, wrote: > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote: >> >>> On 19.06.19 at 13:02, wrote: >> > If the hypervisor has been built with EFI support (ie: multiboot2). >> > This allows to position the .reloc section correctly in the output >> > binary, or else the linker might place .reloc before the .text >> > section. >> > >> > Note that the .reloc section is moved before .bss for two reasons: in >> > order for the resulting binary to not contain any section with data >> > after .bss, so that the file size can be smaller than the loaded >> > memory size, and because the data it contains is read-only, so it >> > belongs with the other sections containing read-only data. >> >> While this may be fine for ELF, I'm afraid it would be calling for >> subtle issues with xen.efi (i.e. the PE binary): There a .reloc >> section is generally expected to come after "normal" data >> sections. > > OK, would you like me to leave the .reloc section at the previous > position for EFI builds then? Well, this part is a requirement, not a question of me liking you to do so. > Or do we prefer to leave .reloc orphaned in the ELF build? Daniel might have an opinion here with his plans to actually add relocations there in the non-linker-generated-PE build. I don't have a strong opinion either way, as long as the current method of building gets left as is (or even simplified). Also a remark regarding the title - in my builds there already is a .reloc section in the ELF images, so "add" doesn't really seem correct to me. It sits right after .rodata, and I would it doesn't get folded into there because - for some reason - .rodata is actually marked writable. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
>>> On 19.06.19 at 16:30, wrote: > On Wed, Jun 19, 2019 at 12:20:40PM +0100, Andrew Cooper wrote: >> Since the MB1/MB2 builds aren't relocatable, I think we might be able to >> get away with simply excluding them in the non-EFI build. > > Hm, OK. I'm slightly loss then. I've taken a look at the history of > xen/arch/x86/efi/relocs-dummy.S and it's not clear to me why such a > dummy file was added. My guess is that it's done in order to prevent > missing symbols errors. If that's the case I guess the code that makes > use of such symbols can be guarded, and the dummy file removed? Missing symbols errors - yes. But how would you remove the dummy one, which is a place holder in stage 1 linking for what will be real data in stages 2 and 3? I don't think we should ignore unresolved symbols in stage 1. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
On Wed, Jun 19, 2019 at 12:20:40PM +0100, Andrew Cooper wrote:
> On 19/06/2019 12:02, Roger Pau Monne wrote:
> > If the hypervisor has been built with EFI support (ie: multiboot2).
>
> Seeing as this continues the sentence from the subject, it should start
> without a capital. Otherwise the result is werd to read.
>
> > This allows to position the .reloc section correctly in the output
> > binary, or else the linker might place .reloc before the .text
> > section.
>
> Really? How can this be a legitimate transformation for the linker to make?
I've already submitted a bug report:
https://bugs.llvm.org/show_bug.cgi?id=42327
GNU ld behaviour is to place orphaned sections at the end.
> >
> > Note that the .reloc section is moved before .bss for two reasons: in
> > order for the resulting binary to not contain any section with data
> > after .bss, so that the file size can be smaller than the loaded
> > memory size, and because the data it contains is read-only, so it
> > belongs with the other sections containing read-only data.
>
> The content of .relocs is transformed via mkreloc to become
> __base_relocs_{start,end} and shouldn't exist into the final linked
> image AFAICT.
__base_relocs_{start/end} is actually what's contained in the .relocs
section, or at least that was mny impression based on the contents of
xen/arch/x86/efi/relocs-dummy.S
> Since the MB1/MB2 builds aren't relocatable, I think we might be able to
> get away with simply excluding them in the non-EFI build.
Hm, OK. I'm slightly loss then. I've taken a look at the history of
xen/arch/x86/efi/relocs-dummy.S and it's not clear to me why such a
dummy file was added. My guess is that it's done in order to prevent
missing symbols errors. If that's the case I guess the code that makes
use of such symbols can be guarded, and the dummy file removed?
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
>>> On 19.06.19 at 13:20, wrote:
> On 19/06/2019 12:02, Roger Pau Monne wrote:
>> Note that the .reloc section is moved before .bss for two reasons: in
>> order for the resulting binary to not contain any section with data
>> after .bss, so that the file size can be smaller than the loaded
>> memory size, and because the data it contains is read-only, so it
>> belongs with the other sections containing read-only data.
>
> The content of .relocs is transformed via mkreloc to become
> __base_relocs_{start,end} and shouldn't exist into the final linked
> image AFAICT.
The labels are referenced from code, and hence ...
> Since the MB1/MB2 builds aren't relocatable, I think we might be able to
> get away with simply excluding them in the non-EFI build.
... this won't be possible without further trickery, I'm afraid.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary
On 19/06/2019 12:02, Roger Pau Monne wrote:
> If the hypervisor has been built with EFI support (ie: multiboot2).
Seeing as this continues the sentence from the subject, it should start
without a capital. Otherwise the result is werd to read.
> This allows to position the .reloc section correctly in the output
> binary, or else the linker might place .reloc before the .text
> section.
Really? How can this be a legitimate transformation for the linker to make?
>
> Note that the .reloc section is moved before .bss for two reasons: in
> order for the resulting binary to not contain any section with data
> after .bss, so that the file size can be smaller than the loaded
> memory size, and because the data it contains is read-only, so it
> belongs with the other sections containing read-only data.
The content of .relocs is transformed via mkreloc to become
__base_relocs_{start,end} and shouldn't exist into the final linked
image AFAICT.
Since the MB1/MB2 builds aren't relocatable, I think we might be able to
get away with simply excluding them in the non-EFI build.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
