Re: [Xen-devel] [PATCH v2 8/8] efi: drop original xen.efi code and build mechanism

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Tue, Jul 10, 2018 at 08:05:56AM -0600, Jan Beulich wrote:
> >>> On 10.07.18 at 13:35, <daniel.kiper@xxxxxxxxxx> wrote:
> > On Fri, Jul 06, 2018 at 09:16:38AM -0600, Jan Beulich wrote:
> >> >>> On 06.07.18 at 16:46, <daniel.kiper@xxxxxxxxxx> wrote:
> >> > OK, xen.mb.efi does not need relocs because:
> >> >   - we generate PE file from xen-syms file like we do with ELF output;
> >> >     so, the code in the PE file is the same like in the ELF file;
> >> >     hence, if ELF works why PE should not,
> >> >   - all addressing is relative to %rip as it is in ELF file,
> >>
> >> What are the several hundred base relocs in xen.efi doing then? Sure
> >> some of them wouldn't really be needed, but I doubt that's true for
> >> all of them. The first and foremost case of non-RIP-relative addressing
> >> is data with static initializers pointing elsewhere in the image. These
> >> need relocations applied to work.
> >>
> >> Once again - a fundamental criteria is whether your binary can be used
> >> in place of the current xen.efi. I can't convince myself that you've
> >> actually tried that out. At the very least I'd expect the static array in
> >> PrintErrMesg() to present problems here.
> >
> > Ugh... You are right. I forgot about that. Sadly the problem applies to
> > the EFI boot code in the xen.gz too. So, both things have to be fixed.
> > At first sight it seems to me that we can leave relocs in the xen-syms
> > and then attach them to the xen.mb.efi/xen.gz somehow. It would be nice
> > to do that just only for the EFI boot code. Should not we put it in
> > separate section then? Another question is the size of the .reloc section.
> > We do not know it in advance. So, probably we should build the code in
> > two steps as we do now. Or prealloc a static place and fill it later.
> > This way we would have just one phase build.
>
> Any static allocation/reservation scheme is wasteful at first and eventually
> not allocating/reserving enough space. Unless there's a way to reasonably
> well estimate the size ahead of time, I'd be opposed to such a model. As

I have the same concerns in regards to that.

> to a separate section - sure, why not? Relocations are in a separate section
> in xen.efi as well.

I was thinking about separate section for EFI boot code itself, e.g. .text.efi.
Then probably it will be much easier to identify and use/get relocs needed only
for that code.

> > Or another option. Add Xen mappings in the early EFI boot code. However,
> > it seems crazy and may not work on all EFI implementations. Hmmm...
> > Have to check the UEFI spec...
>
> I'm afraid I don't understand anyway what you think of here.

All non-rip-relative addresses are in Xen address space. So, I was
thinking about adding required mappings to avoid .reloc section
requirement. Though UEFI spec may not allow such play with page
tables before ExitBootServices() call.

> > By the way, I am not sure why mkreloc is executed twice. Could you
> > explain that?
>
> Its needed as many times as we link a binary, minus the very first time,
> where stub (dummy) objects are used. I don't see how you think we
> could get away with just one invocation. Are you thinking we could get
> away with fewer linking steps?

That would be nice. At least I will try...

> - Link step 0 produces a binary without kallsyms table, but with all
>   symbols. Hence a symbol table generated from it will have the correct
>   number of entries and hence the correct size.
> - Link step 1 produces a binary with kallsyms table taken from the
>   step 0 binary. This results in all addresses in the resulting binary now
>   being correct (no more code/data is going to be added), but the
>   symbol table itself is wrong (as coming from step 0).
> - Link step 2 produces a binary with a _correct_ kallsyms table.
> If we omitted relocations from step 1, we'd risk other addresses
> changing in step 2 (maybe this is just a theoretical risk, but anyway).

OK, thanks for explanation.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel