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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

>> 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.

> 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).

Jan