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

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

On Fri, Jun 21, 2019 at 06:07:25AM -0600, Jan Beulich wrote:
> >>> On 21.06.19 at 13:46, <roger.pau@xxxxxxxxxx> 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).
> >> 
> >> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel