Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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, <roger.pau@xxxxxxxxxx> 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, <roger.pau@xxxxxxxxxx> wrote:
> > >> > > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote:
> > >> > >> >>> On 19.06.19 at 13:02, <roger.pau@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel