Re: [PATCH] xen/x86: fix xen.efi boot crash from some bootloaders

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 23/07/2025 3:21 pm, Jan Beulich wrote:
> On 23.07.2025 16:13, Andrew Cooper wrote:
>> On 23/07/2025 2:56 pm, Yann Sionneau wrote:
>>> xen.efi PE does not boot when loaded from shim or some patched
>>> downstream grub2.
>>>
>>> What happens is the bootloader would honour the MEM_DISCARDABLE
>>> flag of the .reloc section meaning it would not load its content
>>> into memory.
>>>
>>> But Xen is parsing the .reloc section content twice at boot:
>>> * https://elixir.bootlin.com/xen/v4.20.1/source/xen/common/efi/boot.c#L1362
>>> * 
>>> https://elixir.bootlin.com/xen/v4.20.1/source/xen/arch/x86/efi/efi-boot.h#L237
>>>
>>> Therefore it would crash with the following message:
>>> "Unsupported relocation type" as reported there:
>>>
>>> * 
>>> https://github.com/QubesOS/qubes-issues/issues/8206#issuecomment-2619048838
>>> * 
>>> https://lore.kernel.org/xen-devel/7e039262-1f54-46e1-8f70-ac3f03607d5a@xxxxxxxx/T/#me122b9e6c27cd98db917da2c9f67e74a2c6ad7a5
>>>
>>> This commit adds a small C host tool named keeprelocs
>>> that is called after xen.efi is produced by the build system
>>> in order to remove this bit from its .reloc section header.
>>>
>>> Signed-off-by: Yann Sionneau <yann.sionneau@xxxxxxxxxx>
>>> ---
>>>  xen/Makefile           |   5 +-
>>>  xen/arch/x86/Makefile  |   1 +
>>>  xen/tools/Makefile     |   3 ++
>>>  xen/tools/keeprelocs.c | 119 +++++++++++++++++++++++++++++++++++++++++
>>>  4 files changed, 127 insertions(+), 1 deletion(-)
>>>  create mode 100644 xen/tools/keeprelocs.c
>> I'm sick and tired of the hoops we have to jump through for broken
>> tooling.  This is now rewriting the PE+ metadata because apparently the
>> linker can't do it correctly.
> The linker is doing it correctly. It is us (and very likely just us) who
> have special needs here.
>
>> Either fix the linker (or the way we drive it/etc), or we're doing away
>> with PE+ emulation entirely and writing the MZ/PE headers by hand like
>> literally every other kernel does.
> "Fixing" the linker was suggested, but with my binutils maintainer hat on
> I'm inclined to not accept a Xen-only option into the linker.

Either Xen is doing something wrong, and should be doing it differently,
or Xen is doing something right and the tooling is
wrong/incomplete/whatever.

As a related question, is anyone other than Xen using PE+ emulation in
anger?  Every other kernel/OS level tool I'm aware of writes the MZ/PE
header by hand, and frankly, the list of bugs we've found in PE+
emulation would strongly suggest that noone else is using it.

~Andrew