Re: [PATCH v4 2/4] x86/efi: discard multiboot support for PE binary

[Frediano Ziglio <freddy77@xxxxxxxxx>]

On Thu, 25 Jun 2026 at 12:18, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 25.06.2026 12:15, Frediano Ziglio wrote:
> > On Wed, 24 Jun 2026 at 15:18, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>
> >> On 16.06.2026 19:28, Frediano Ziglio wrote:
> >>> From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >>>
> >>> Multiboot and PVH booting are not supported for PE, hence discards them
> >>> in the linker script when doing a PE build.
> >>>
> >>> That removes some relocations that otherwise appear due to the usage of 
> >>> the
> >>> start and __efi64_mb2_start symbols in the multiboot2 header.
> >>>
> >>> Section discarding is not done updating DISCARD_SECTIONS definition as the
> >>> change is specific for x86.
> >>>
> >>> No functional change intended.
> >>>
> >>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >>> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
> >>> Acked-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> >>
> >> While on the surface this looks okay, there are still concerns:
> >>
> >> For one, this also discards the PVH entry point. That's technically fine 
> >> aiui,
> >> yet shouldn't go without mentioning.
> >>
> >
> > Considering that the code/data is not exported in EFI as
> >
> > #if defined(CONFIG_PVH_GUEST) && !defined(EFI)
> >   /*
> >    * In principle this should be fine to live in .note (below), but let's 
> > keep
> >    * it separate in case anyone decided to find these notes by section name.
> >    */
> >   DECL_SECTION(.note.Xen) {
> >       *(.note.Xen)
> >   } PHDR(note) PHDR(text)
> > #endif
> >
> > yes, technically it's surely fine.
> >
> > There's a mention in the commit log:
> >
> >     Multiboot and PVH booting are not supported for PE, hence discards them
> >     in the linker script when doing a PE build.
> >
> > But not in the subject:
> >
> >     x86/efi: discard multiboot support for PE binary
> >
> > What about simply changing the subject to:
> >
> >     x86/efi: discard multiboot and PVH support for PE binary
>
> Perhaps.
>

Updated.

> >> Otoh you discard call sites of functions without discarding the functions
> >> themselves, violating Misra's "no unreachable code" rule. Eclair may not be
> >> able to spot this, but imo we should still adhere to the rule. Proper
> >> coverage analysis, for example, would likely turn this up.
> >>
> >
> > That makes sense. Given that most code in head.S is now discarded most
> > data sections are now not used and the only thing left will be the
> > trampoline.
> > It'll take a bit of time to search for removed symbols.
> >
> > About the "no unreachable code" I think we are violating that anyway.
>
> Perhaps, but we should get the number of such violations down, not up.
>

It was not meant to be an excuse, more of a question if the problem is known.
The "It'll take a bit of time to search for removed symbols" was a "I will do".
I now have the fixup patch for "x86/efi: discard multiboot and PVH
support for PE binary" (the commit we are talking about here). About
sending an updated series, what is the best way to send a fixup patch?
Send the fixup as separate? Merge into the base patch and remove the
"acked-by"? Keep the "acked-by"?

> > We package "built-in.o" files and then use them to craft the final
> > executable. I don't think the linker will be able to discard unused
> > functions for that reason. That does not mean that more things can be
> > discarded.
> At least not until we engage it garbage collection, which as per Jason
> proves problematic when linking xen.efi (due to linker issues as it looks).
>

In my experience linker garbage collector on Linux is a bit
problematic, I think LTO is more tested and working. Still not that
automatic.

I tried to remove the BIOS code using sections "trick" like what was
done in this commit without success.

> Jan

Frediano