Re: [PATCH 4/8] x86/EFI: redo .reloc section bounds determination

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

On Wed, Apr 21, 2021 at 12:44:13PM +0200, Jan Beulich wrote:
> On 21.04.2021 11:46, Roger Pau Monné wrote:
> > On Thu, Apr 01, 2021 at 11:45:38AM +0200, Jan Beulich wrote:
> >> There's no need to link relocs-dummy.o into the ELF binary. The two
> >> symbols needed can as well be provided by the linker script. Then our
> >> mkreloc tool also doesn't need to put them in the generated assembler
> >> source.
> > 
> > Maybe I'm just dense today, but I think the message needs to be
> > expanded a bit to mention that while the __base_relocs_{start,end} are
> > not used when loaded as an EFI application, they are used by the EFI
> > code in Xen when booted using the multiboot2 protocol for example, as
> > they are used by efi_arch_relocate_image.
> > 
> > I think relocation is not needed when natively loaded as an EFI
> > application, as then the load address matches the one expected by
> > Xen?
> 
> It's quite the other way around: The EFI loader applies relocations
> to put the binary at its loaded _physical_ address (the image gets
> linked for the final virtual address). Hence we need to apply the
> same relocations a 2nd time (undoing what the EFI loader did)
> before we can branch from the physical (identity mapped) address
> range where xen.efi was loaded to the intended virtual address
> range where we mean to run Xen from.
> 
> For the ELF binary the symbols are needed solely to make ld happy.
> 
> > I also wonder, at some point there where plans for providing a single
> > binary that would work as multiboot{1,2} and also be capable of being
> > loaded as an EFI application (ie: have a PE/COFF header also I assume
> > together with the ELF one), won't the changes here make it more
> > difficult to reach that goal or require reverting later on, as I feel
> > they are adding more differences between the PE binary and the ELF
> > one.
> 
> There were such plans, yes, but from the last round of that series
> I seem to recall that there was at least one issue breaking this
> idea. So no, at this point I'm not intending to take precautions to
> make that work easier (or not further complicate it). This said, I
> don't think the change here complicates anything there.
> 
> > The code LGTM, but I think at least I would like the commit message to
> > be expanded.
> 
> Well, once I know what exactly you're missing there, I can certainly
> try to expand it.

OK, I think I now have a clearer view, the commit message is likely
fine as it already mentions the ELF binary only needs the dummy
__base_relocs_{start,end}, hence it's the EFI binary the one that
requires the relocation symbols.

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks, Roger.