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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.07.18 at 18:48, <daniel.kiper@xxxxxxxxxx> wrote:
> On Wed, Jul 04, 2018 at 09:34:09AM -0600, Jan Beulich wrote:
>> >>> On 04.07.18 at 16:25, <daniel.kiper@xxxxxxxxxx> wrote:
>> > On Thu, Jun 28, 2018 at 07:51:52AM -0600, Jan Beulich wrote:
>> >> >>> On 19.06.18 at 16:35, <daniel.kiper@xxxxxxxxxx> wrote:
>> >> > Then rename xen.mb.efi to xen.efi and drop all related
>> >> > differentiators in the code.
>> >>
>> >> For this you'll first of all need to convince me that the binary you 
>> >> build is
>> >> a drop-in replacement for xen.efi. As noted in the replies to earlier
>> >> patches, I'm getting the impression of this not being the case. A further
>> >> hint towards this is the outright deletion of xen/arch/x86/efi/mkreloc.c:
>> >> How is the Xen image going to be relocated that way, when loaded from
>> >> the EFI shell or boot loader?
>> >
>> > It works because all addressing is relative to %rip. To be precise, new
>> > xen.efi, earlier xen.mb.efi, contains exactly the same code as ELF does.
>> > So, if ELF works without any relocations why PE should not. Especially on
>> > x86-64. Additionally, my tests showed that in general UEFI implementations
>> > just require Base Relocation Table entry in PE Data Directories to relocate
>> > the image.
>>
>> The thing I'm missing in the series is the generation of the relocations to
>> go into the section pointed to by this Data Directory entry.
> 
> There is no such thing because xen.mb.efi does not need that.

Well, fine. But you still owe me an answer to the "why" part here.

>> > Even it can be empty. As it is in current patchset.
>>
>> No, it can't be empty. Just try removing the relocations from xen.efi and
>> see what you get.
> 
> I am pretty sure that current xen.efi will not work without .reloc section.
> However, xen.mb.efi works without any issue. So, I am not sure where is the
> problem. +/- fake .reloc entry which I was talking about earlier.

As per above: If this is the case - fine. But I want this to be explained,
not the least because I'd like to understand if I wasted effort back when
I added to code to produce and handle the relocations.

>> > Though I am
>> > afraid about more picky UEFI stuff and considering addition of at least
>> > one .reloc entry, fake one, as Linux kernel does. And this rises another
>> > question: should not we add .bss section into PE header? Right now it is
>> > embedded/hidden in PE .text section.
>>
>> My xen.efi does have a .bss section.
> 
> It is completely valid to have BSS embedded in .text section or living
> as a separate entity in PE section table. So, the question is: which one
> do we prefer? Currently existing xen.mb.efi layout is similar to ELF layout.

I'd prefer any binary to look as natural as possible. However, our ELF
binary doesn't, and hence I wouldn't insist on this to be the case for
the PE one. There's a reason for the ELF one to be the way it is, though.
If no similar reason exists for the PE one, then please have it have a
"normal" set of sections.

Jan



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