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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.07.2025 16:45, Jan Beulich wrote:
> On 23.07.2025 16:38, Andrew Cooper wrote:
>> 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,
> 
> Well, no-one else would require access to its own executable's .reloc
> section. Of course we can do things differently (like duplicate .reloc
> into .init.data), but why have the same data in two places?
> 
>> or Xen is doing something right and the tooling is
>> wrong/incomplete/whatever.

One more aspect, for completeness: Xen's need to self-relocate stems from
the fact that we combine OS loader and OS into a single binary. The original
EFI concept was to have an OS loader as EFI binary, which would then load
the OS kernel binary in whatever format it likes. That way, the OS loader
can run at wherever EFI loads it, and the OS kernel can be loaded right at
its designated address.

Jan