Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
On 09.06.2021 15:18, Daniel Kiper wrote: > 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: 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. Well, debug info can be ignored. If MB2 would work like it does today, then .reloc also would never be touched. Feels a little fragile, but might be okay then. Jan
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
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
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
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. >> 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. >> 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. > Maybe > newer binutils are more flexible and will be able to produce a PE image > with properties required by Multiboot2 protocol. Isn't all you need the MB2 header within the first so many bytes of the (disk) image? Or was it the image as loaded into memory? Both should be possible to arrange for. >>> >>> IIRC Multiboot2 protocol requires the header in first 32 kiB of an image. >>> So, this is not a problem. >> >> I was about to ask "Disk image or in-memory image?" But this won't >> matter if the image as a whole got linearized, as long as the first >> section doesn't start to high up. I notice that xen-syms doesn't fit >> this requirement either, only the output of mkelf32 does. Which >> suggests that there may not be a way around a post-processing tool. > > Could not we drop 2 MiB padding at the beginning of xen-syms by changing > some things in the linker script? Not sure, but at the first glance I don't think so. If we want section and file alignment to match, and if we want sections at 2Mb boundaries, then the first
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
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? > 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. > 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. > >>> Maybe > >>> newer binutils are more flexible and will be able to produce a PE image > >>> with properties required by Multiboot2 protocol. > >> > >> Isn't all you need the MB2 header within the first so many bytes of the > >> (disk) image? Or was it the image as loaded into memory? Both should be > >> possible to arrange for. > > > > IIRC Multiboot2 protocol requires the header in first 32 kiB of an image. > > So, this is not a problem. > > I was about to ask "Disk image or in-memory image?" But this won't > matter if the image as a whole got linearized, as long as the first > section doesn't start to high up. I notice that xen-syms doesn't fit > this requirement either, only the output of mkelf32 does. Which > suggests that there may not be a way around a post-processing tool. Could not we drop 2 MiB padding at the beginning of xen-syms by changing some things in the linker script? Daniel
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
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 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 to be possible by setting --section-alignment= and --file-alignment= to the same value (resulting in a much larger file) - adjusting file 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. Also not sure what to do with Dwarf debug info, which just recently we managed to avoid needing to strip unconditionally. >>> Maybe >>> newer binutils are more flexible and will be able to produce a PE image >>> with properties required by Multiboot2 protocol. >> >> Isn't all you need the MB2 header within the first so many bytes of the >> (disk) image? Or was it the image as loaded into memory? Both should be >> possible to arrange for. > > IIRC Multiboot2 protocol requires the header in first 32 kiB of an image. > So, this is not a problem. I was about to ask "Disk image or in-memory image?" But this won't matter if the image as a whole got linearized, as long as the first section doesn't start to high up. I notice that xen-syms doesn't fit this requirement either, only the output of mkelf32 does. Which suggests that there may not be a way around a post-processing tool. Jan
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
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. > > Maybe > > newer binutils are more flexible and will be able to produce a PE image > > with properties required by Multiboot2 protocol. > > Isn't all you need the MB2 header within the first so many bytes of the > (disk) image? Or was it the image as loaded into memory? Both should be > possible to arrange for. IIRC Multiboot2 protocol requires the header in first 32 kiB of an image. So, this is not a problem. Daniel
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
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? > Maybe > newer binutils are more flexible and will be able to produce a PE image > with properties required by Multiboot2 protocol. Isn't all you need the MB2 header within the first so many bytes of the (disk) image? Or was it the image as loaded into memory? Both should be possible to arrange for. Jan
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
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. Though I think you should try once again. Maybe newer binutils are more flexible and will be able to produce a PE image with properties required by Multiboot2 protocol. Daniel
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
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 ... Jan
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
Jan, I mulled over your feedback and I think I can now see your reservations with this series. I'm wondering if the long-term goal of using the xen mb1/mb2 binary as the basis for creating a EFI loadable mb1/mb2 payload is actually the wrong approach. After all, I do not see a feasible way to maintain the comprehensive sectioning, the proper reloc table, the proper debug directory, etc... that is found in the current xen.efi using the approach in this series, which would mean maintaining a third binary forever. 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 end of the day, our goal is just to have a binary that meets these requirements: * Is verifiable with shim (PE/COFF) * May boot on BIOS platforms via grub2 * May boot on EFI platforms via grub2 or EFI loader Thanks -- Bobby Eshleman SE at Vates SAS
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
On 22.01.2021 01:51, Bobby Eshleman wrote:
> From: Daniel Kiper
>
> This patch introduces xen.mb.efi which contains a manually built PE
> header.
>
> This allows us to support Xen on UEFI Secure Boot-enabled hosts via
> multiboot2.
>
> xen.mb.efi is a single binary that is loadable by a UEFI loader or via
> the Multiboot/Multiboot2 protocols.
What's missing here yet very important is why the existing xen.efi
doesn't fit and can't be made fit.
> Signed-off-by: Daniel Kiper
> Signed-off-by: Bobby Eshleman
> ---
Besides (or instead of) the series-wide change log, please have
per-patch changes info here.
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -266,29 +266,31 @@ endif
> .PHONY: _build
> _build: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
>
> +define install_xen_links
> + $(INSTALL_DATA) $(TARGET)$1 $(D)$(BOOT_DIR)/$(T)-$(XEN_FULLVERSION)$1
> + ln -f -s $(T)-$(XEN_FULLVERSION)$1
> $(D)$(BOOT_DIR)/$(T)-$(XEN_VERSION).$(XEN_SUBVERSION)$1
> + ln -f -s $(T)-$(XEN_FULLVERSION)$1 $(D)$(BOOT_DIR)/$(T)-$(XEN_VERSION)$1
> + ln -f -s $(T)-$(XEN_FULLVERSION)$1 $(D)$(BOOT_DIR)/$(T)$1
> +endef
If you abstract this away, please take the opportunity to fold
"-f -s" into a single option.
> .PHONY: _install
> _install: D=$(DESTDIR)
> _install: T=$(notdir $(TARGET))
> _install: Z=$(CONFIG_XEN_INSTALL_SUFFIX)
> _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
> [ -d $(D)$(BOOT_DIR) ] || $(INSTALL_DIR) $(D)$(BOOT_DIR)
> - $(INSTALL_DATA) $(TARGET)$(Z)
> $(D)$(BOOT_DIR)/$(T)-$(XEN_FULLVERSION)$(Z)
> - ln -f -s $(T)-$(XEN_FULLVERSION)$(Z)
> $(D)$(BOOT_DIR)/$(T)-$(XEN_VERSION).$(XEN_SUBVERSION)$(Z)
> - ln -f -s $(T)-$(XEN_FULLVERSION)$(Z)
> $(D)$(BOOT_DIR)/$(T)-$(XEN_VERSION)$(Z)
> - ln -f -s $(T)-$(XEN_FULLVERSION)$(Z) $(D)$(BOOT_DIR)/$(T)$(Z)
> + $(call install_xen_links,$(Z))
> + $(call install_xen_links,.mb.efi)
This is common code, so will affect Arm as well. I don't think
your addition can be unconditional.
> [ -d "$(D)$(DEBUG_DIR)" ] || $(INSTALL_DIR) $(D)$(DEBUG_DIR)
> $(INSTALL_DATA) $(TARGET)-syms
> $(D)$(DEBUG_DIR)/$(T)-syms-$(XEN_FULLVERSION)
> $(INSTALL_DATA) $(TARGET)-syms.map
> $(D)$(DEBUG_DIR)/$(T)-syms-$(XEN_FULLVERSION).map
> $(INSTALL_DATA) $(KCONFIG_CONFIG)
> $(D)$(BOOT_DIR)/$(T)-$(XEN_FULLVERSION).config
> if [ -r $(TARGET).efi -a -n '$(EFI_DIR)' ]; then \
> [ -d $(D)$(EFI_DIR) ] || $(INSTALL_DIR) $(D)$(EFI_DIR); \
> - $(INSTALL_DATA) $(TARGET).efi
> $(D)$(EFI_DIR)/$(T)-$(XEN_FULLVERSION).efi; \
> if [ -e $(TARGET).efi.map ]; then \
> $(INSTALL_DATA) $(TARGET).efi.map
> $(D)$(DEBUG_DIR)/$(T)-$(XEN_FULLVERSION).efi.map; \
> fi; \
> - ln -sf $(T)-$(XEN_FULLVERSION).efi
> $(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).$(XEN_SUBVERSION).efi; \
> - ln -sf $(T)-$(XEN_FULLVERSION).efi
> $(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).efi; \
> - ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T).efi; \
> + $(call install_xen_links,.efi)) \
> if [ -n '$(EFI_MOUNTPOINT)' -a -n '$(EFI_VENDOR)' ]; then \
> $(INSTALL_DATA) $(TARGET).efi
> $(D)$(EFI_MOUNTPOINT)/efi/$(EFI_VENDOR)/$(T)-$(XEN_FULLVERSION).efi; \
> elif [ "$(D)" = "$(patsubst $(shell cd $(XEN_ROOT) &&
> pwd)/%,%,$(D))" ]; then \
Since this part of the patch is a non-negligible fraction of the
patch and since this installation step doesn't need to be an
integral part of the change, may I suggest / ask that you split
this off into a separate change? Possibly the installing of the
new binary could remain here, but then the breaking out of the
install_xen_links macro (which imo also would better use dashes
in place of the underscores) could still be factored out.
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -110,7 +110,7 @@
> syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
> syms-warn-dup-$(CONFIG_ENFORCE_UNIQUE_SYMBOLS) := --error-dup
>
> $(TARGET): TMP = $(@D)/.$(@F).elf32
> -$(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
> +$(TARGET): $(TARGET).mb.efi $(TARGET)-syms $(efi-y) boot/mkelf32
While perhaps mostly cosmetic, I'd prefer additions to be done
after the existing (pseudo-)dependencies, not as the very first
item. $(TARGET)-syms still is the main dependency here, and it
should remain this way.
Speaking of (pseudo-)dependencies - I was hoping that we could
avoid further extending this sub-optimal approach.
> @@ -119,6 +119,11 @@ $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
> { echo "No Multiboot2 header found" >&2; false; }
> mv $(TMP) $(TARGET)
>
> +$(TARGET).mb.efi: $(TARGET)-syms
> + $(OBJCOPY) -O binary -S --change-section-address \
> + ".efi.pe.header-`$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) .
> __image_base__$$/0x\1/p'`" \
> + $(TARGET)-syms $(TARGET).mb.efi
The quoting is very hard to follow
[PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
From: Daniel Kiper
This patch introduces xen.mb.efi which contains a manually built PE
header.
This allows us to support Xen on UEFI Secure Boot-enabled hosts via
multiboot2.
xen.mb.efi is a single binary that is loadable by a UEFI loader or via
the Multiboot/Multiboot2 protocols.
Signed-off-by: Daniel Kiper
Signed-off-by: Bobby Eshleman
---
xen/Makefile| 20 +++---
xen/arch/x86/Makefile | 7 +-
xen/arch/x86/arch.mk| 2 +
xen/arch/x86/boot/Makefile | 1 +
xen/arch/x86/boot/head.S| 1 +
xen/arch/x86/boot/pecoff.S | 123
xen/arch/x86/efi/efi-boot.h | 24 ++-
xen/arch/x86/efi/stub.c | 12 +++-
xen/arch/x86/xen.lds.S | 34 ++
xen/include/xen/efi.h | 1 +
10 files changed, 212 insertions(+), 13 deletions(-)
create mode 100644 xen/arch/x86/boot/pecoff.S
diff --git a/xen/Makefile b/xen/Makefile
index 85f9b763a4..9c689c2095 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -266,29 +266,31 @@ endif
.PHONY: _build
_build: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
+define install_xen_links
+ $(INSTALL_DATA) $(TARGET)$1 $(D)$(BOOT_DIR)/$(T)-$(XEN_FULLVERSION)$1
+ ln -f -s $(T)-$(XEN_FULLVERSION)$1
$(D)$(BOOT_DIR)/$(T)-$(XEN_VERSION).$(XEN_SUBVERSION)$1
+ ln -f -s $(T)-$(XEN_FULLVERSION)$1 $(D)$(BOOT_DIR)/$(T)-$(XEN_VERSION)$1
+ ln -f -s $(T)-$(XEN_FULLVERSION)$1 $(D)$(BOOT_DIR)/$(T)$1
+endef
+
.PHONY: _install
_install: D=$(DESTDIR)
_install: T=$(notdir $(TARGET))
_install: Z=$(CONFIG_XEN_INSTALL_SUFFIX)
_install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
[ -d $(D)$(BOOT_DIR) ] || $(INSTALL_DIR) $(D)$(BOOT_DIR)
- $(INSTALL_DATA) $(TARGET)$(Z)
$(D)$(BOOT_DIR)/$(T)-$(XEN_FULLVERSION)$(Z)
- ln -f -s $(T)-$(XEN_FULLVERSION)$(Z)
$(D)$(BOOT_DIR)/$(T)-$(XEN_VERSION).$(XEN_SUBVERSION)$(Z)
- ln -f -s $(T)-$(XEN_FULLVERSION)$(Z)
$(D)$(BOOT_DIR)/$(T)-$(XEN_VERSION)$(Z)
- ln -f -s $(T)-$(XEN_FULLVERSION)$(Z) $(D)$(BOOT_DIR)/$(T)$(Z)
+ $(call install_xen_links,$(Z))
+ $(call install_xen_links,.mb.efi)
[ -d "$(D)$(DEBUG_DIR)" ] || $(INSTALL_DIR) $(D)$(DEBUG_DIR)
$(INSTALL_DATA) $(TARGET)-syms
$(D)$(DEBUG_DIR)/$(T)-syms-$(XEN_FULLVERSION)
$(INSTALL_DATA) $(TARGET)-syms.map
$(D)$(DEBUG_DIR)/$(T)-syms-$(XEN_FULLVERSION).map
$(INSTALL_DATA) $(KCONFIG_CONFIG)
$(D)$(BOOT_DIR)/$(T)-$(XEN_FULLVERSION).config
if [ -r $(TARGET).efi -a -n '$(EFI_DIR)' ]; then \
[ -d $(D)$(EFI_DIR) ] || $(INSTALL_DIR) $(D)$(EFI_DIR); \
- $(INSTALL_DATA) $(TARGET).efi
$(D)$(EFI_DIR)/$(T)-$(XEN_FULLVERSION).efi; \
if [ -e $(TARGET).efi.map ]; then \
$(INSTALL_DATA) $(TARGET).efi.map
$(D)$(DEBUG_DIR)/$(T)-$(XEN_FULLVERSION).efi.map; \
fi; \
- ln -sf $(T)-$(XEN_FULLVERSION).efi
$(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).$(XEN_SUBVERSION).efi; \
- ln -sf $(T)-$(XEN_FULLVERSION).efi
$(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).efi; \
- ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T).efi; \
+ $(call install_xen_links,.efi)) \
if [ -n '$(EFI_MOUNTPOINT)' -a -n '$(EFI_VENDOR)' ]; then \
$(INSTALL_DATA) $(TARGET).efi
$(D)$(EFI_MOUNTPOINT)/efi/$(EFI_VENDOR)/$(T)-$(XEN_FULLVERSION).efi; \
elif [ "$(D)" = "$(patsubst $(shell cd $(XEN_ROOT) &&
pwd)/%,%,$(D))" ]; then \
@@ -341,7 +343,7 @@ _clean: delete-unfresh-files
$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH)
SRCARCH=$(SRCARCH) clean
find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" \
-o -name "*.gcno" -o -name ".*.cmd" \) -exec rm -f {} \;
- rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi
$(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
+ rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET)*.efi
$(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
rm -f include/asm-*/asm-offsets.h
rm -f .banner
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 7769bb40d7..49c61adabf 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -110,7 +110,7 @@ syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS)
:=
syms-warn-dup-$(CONFIG_ENFORCE_UNIQUE_SYMBOLS) := --error-dup
$(TARGET): TMP = $(@D)/.$(@F).elf32
-$(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
+$(TARGET): $(TARGET).mb.efi $(TARGET)-syms $(efi-y) boot/mkelf32
./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \
`$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) .
__2M_rwdata_end$$/0x\1/p'`
od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || \
@@ -119,6 +119,11 @@ $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
{ echo "No Multiboot2 header found" >&2; false; }
mv $(TMP) $(TARGET)
+$(TARGET).mb.efi: $(TARGET)-syms
+
