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

["Yann Sionneau" <yann.sionneau@xxxxxxxxxx>]

On 8/4/25 11:34, Jan Beulich wrote:
> On 24.07.2025 16:07, 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>
> 
> So I found a way to deal with this at the linker side, without any new command
> line options. Behavior is solely driven by the attributes of any incoming 
> .reloc
> sections (of which there would be none by default, retaining original 
> behavior).
> The important patch is [1], but at least the first patch of the series [2] 
> would
> in most cases also be wanted/needed (patch 04 is obviously a mechanical prereq
> for the main patch). Need for other of the prereqs there depends on the scope
> and purpose of one's binutils build(s).
> 
> Jan
> 
> [1] https://sourceware.org/pipermail/binutils/2025-August/143153.html
> [2] https://sourceware.org/pipermail/binutils/2025-August/143141.html

That sounds great!
It's clearly better to fix the issue by changing/improving binutils.
Let's drop my patch in Xen if this gets accepted in binutils!
It would be nice if you could keep us posted in xen-devel of the 
status/progress of the binutils patches.

By the number of patches needed for binutils it seems you opened a can 
of worms/pandora box with this issue ^^

Also, in patch 12/17, you state that the logic would be that if .reloc 
is generated partly by the code itself instead of solely by the linker 
this means we want to use the section at runtime.
While I kind of understand this idea, it also feels a bit as a hack, 
doesn't it?
One could argue that even if .reloc is just generated by the linker, the
program could still want to access it at runtime.
I've looked at Xen code to see if it does put something in .reloc itself 
and it seems so: 
https://elixir.bootlin.com/xen/v4.20.1/source/xen/arch/x86/efi/relocs-dummy.S
The "code puts data in .reloc section" would just serve as a "hint" for 
the linker if I understand your patch well, just as well as a 
`--keep-reloc` command line option would.

Anyway, I won't comment much further on the binutils patchset since I'm 
not well versed in linker black magic.

Thanks for the patchset!

Yann

-- 


Yann Sionneau | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech