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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.08.2025 16:17, Yann Sionneau wrote:
> 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).
>>
>> [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!

Luckily I'm in a position where I don't need "acceptance", but merely
"absence of objections". The sole reason for the present delay is with
a colliding MIPS patch, which I'd rather see go in first.

> It would be nice if you could keep us posted in xen-devel of the 
> status/progress of the binutils patches.

I'll try to remember.

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

Well, that's been only one of the tinier cans.

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

Yes and no. Assigning purpose to sections merely from their names is
already a hack. Yet since we need to live with that concept on PE/COFF
(and even ELF is quite far from being free of such), making this small
extra distinction feels quite acceptable to me.

> One could argue that even if .reloc is just generated by the linker, the
> program could still want to access it at runtime.

Not really, no. Ordinary programs hardly have a need to access their
own .reloc. And if so, having a simple, command-line-option-less way
to distinguish both intentions is probably the best we can have for
both worlds.

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

And something similar would then be needed for the other two linking
steps, just that there would be no actual data in that .reloc section
"contribution".

Jan