Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary

[Daniel Kiper <daniel.kiper@oracle.com>]

On Wed, May 19, 2021 at 04:35:00PM +0200, Jan Beulich wrote:
> On 19.05.2021 14:48, Daniel Kiper wrote:
> > On Wed, May 19, 2021 at 11:29:43AM +0200, Jan Beulich wrote:
> >> On 18.05.2021 19:46, Daniel Kiper wrote:
> >>> On Mon, May 17, 2021 at 03:24:28PM +0200, Jan Beulich wrote:
> >>>> On 17.05.2021 15:20, Daniel Kiper wrote:
> >>>>> On Mon, May 17, 2021 at 08:48:32AM +0200, Jan Beulich wrote:
> >>>>>> On 07.05.2021 22:26, Bob Eshleman wrote:
> >>>>>>> What is your intuition WRT the idea that instead of trying add a PE/COFF hdr
> >>>>>>> in front of Xen's mb2 bin, we instead go the route of introducing valid mb2
> >>>>>>> entry points into xen.efi?
> >>>>>>
> >>>>>> At the first glance I think this is going to be less intrusive, and hence
> >>>>>> to be preferred. But of course I haven't experimented in any way ...
> >>>>>
> >>>>> When I worked on this a few years ago I tried that way. Sadly I failed
> >>>>> because I was not able to produce "linear" PE image using binutils
> >>>>> exiting that days.
> >>>>
> >>>> What is a "linear" PE image?
> >>>
> >>> The problem with Multiboot family protocols is that all code and data
> >>> sections have to be glued together in the image and as such loaded into
> >>> the memory (IIRC BSS is an exception but it has to live behind the
> >>> image). So, you cannot use PE image which has different representation
> >>> in file and memory. IIRC by default at least code and data sections in
> >>> xen.efi have different sizes in PE file and in memory. I tried to fix
> >>> that using linker script and objcopy but it did not work. Sadly I do
> >>> not remember the details but there is pretty good chance you can find
> >>> relevant emails in Xen-devel archive with me explaining what kind of
> >>> problems I met.
> >>
> >> Ah, this rings a bell. Even the .bss-is-last assumption doesn't hold,
> >> because .reloc (for us as well as in general) comes later, but needs
> >> loading (in the right place). Since even xen.gz isn't simply the
> >
> > However, IIRC it is not used when Xen is loaded through Multiboot2
> > protocol. So, I think it may stay in the image as is and the Mutliboot2
> > header should not cover .reloc section.
> >
> > By the way, why do we need .reloc section in the PE image? Is not %rip
> > relative addressing sufficient? IIRC the Linux kernel just contains
> > a stub .reloc section. Could not we do the same?
>
> %rip-relative addressing can (obviously, I think) help only for text.
> But we also have data containing pointers, which need relocating.

Ahhh, right, I totally forgot about it.

> >> compressed linker output, but a post-processed (by mkelf32) image,
> >> maybe what we need is a build tool doing similar post-processing on
> >> xen.efi? Otoh getting disk image and in-memory image aligned ought
> >
> > Yep, this should work too.
> >
> >> to be possible by setting --section-alignment= and --file-alignment=
> >> to the same value (resulting in a much larger file) - adjusting file
> >
> > IIRC this did not work for some reason. Maybe it would be better to
> > enforce correct alignment and required padding using linker script.
>
> I'm not convinced the linker script is the correct vehicle here. It
> is mainly about placement in the address space (i.e. laying out how
> things will end up in memory), not about file layout.

OK but at least I would check what is possible and do it then.

> >> positions would effectively be what a post-processing tool would need
> >> to do (like with mkelf32 perhaps we could then at least save the
> >> first ~2Mb of space). Which would still leave .reloc to be dealt with
> >> - maybe we could place this after .init, but still ahead of
> >> __init_end (such that the memory would get freed late in the boot
> >> process). Not sure whether EFI loaders would "like" such an unusual
> >> placement.
> >
> > Yeah, good question...
> >
> >> Also not sure what to do with Dwarf debug info, which just recently
> >> we managed to avoid needing to strip unconditionally.
> >
> > I think debug info may stay as is. Just Multiboot2 header should not
> > cover it if it is not needed.
>
> You did say that .bss is expected to be last, which both .reloc and
> debug info violate.

The .bss section has to be last one in memory from Multiboot2 protocol
point of view. However, nothing, AFAICT, forbids to have something
behind in the file. Of course if you ignore the data at the end of file
when you load the image using Multiboot2 protocol.

Daniel