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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan