Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 23/02/2021 07:53, Jan Beulich wrote:
> On 22.02.2021 17:36, Andrew Cooper wrote:
>> On 19/02/2021 08:09, Jan Beulich wrote:
>>> --- a/xen/arch/x86/Makefile
>>> +++ b/xen/arch/x86/Makefile
>>> @@ -123,8 +123,13 @@ ifneq ($(efi-y),)
>>>  # Check if the compiler supports the MS ABI.
>>>  export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
>>>  # Check if the linker supports PE.
>>> -XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -S -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>>> +EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10 --strip-debug
>>> +XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) $(EFI_LDFLAGS) -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>>>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>>> +# Check if the linker produces fixups in PE by default (we need to disable it doing so for now).
>>> +XEN_NO_PE_FIXUPS := $(if $(XEN_BUILD_EFI), \
>>> +                         $(shell $(LD) $(EFI_LDFLAGS) --disable-reloc-section -o efi/check.efi efi/check.o 2>/dev/null && \
>>> +                                 echo --disable-reloc-section))
>> Why does --strip-debug move?
> -S and --strip-debug are the same. I'm simply accumulating in
> EFI_LDFLAGS all that's needed for the use in the probing construct.

Oh ok.

It occurs to me that EFI_LDFLAGS now only gets started in an ifneq
block, but appended to later on while unprotected.  That said, I'm
fairly sure it is only consumed inside a different ifeq section, so I
think there is a reasonable quantity of tidying which ought to be done here.

> Also I meanwhile have a patch to retain debug info, for which this
> movement turns out to be a prereq. (I've yet to test that the
> produced binary actually works, and what's more I first need to get
> a couple of changes accepted into binutils for the linker to actually
> cope.)
>
>> What's wrong with $(call ld-option ...) ?  Actually, lots of this block
>> of code looks to be opencoding of standard constructs.
> It looks like ld-option could indeed be used here (there are marginal
> differences which are likely acceptable), despite its brief comment
> talking of just "flag" (singular, plus not really covering e.g. input
> files).
>
> But:
> - It working differently than cc-option makes it inconsistent to
>   use (the setting of XEN_BUILD_EFI can't very well be switched to
>   use cc-option); because of this I'm not surprised that we have
>   only exactly one use right now in the tree.
> - While XEN_BUILD_PE wants to be set to "y", for XEN_NO_PE_FIXUPS
>   another transformation would then be necessary to translate "y"
>   into "--disable-reloc-section".
> - Do you really suggest to re-do this at this point in the release
>   cycle?

I'm looking to prevent this almost-incompressible mess from getting worse.

But I suppose you want this to backport, so I suppose it ought to be
minimally invasive.

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

This logic all actually needs moving into Kconfig so we can also go
about fixing the other bugs we have such as having Multiboot headers in
xen.efi pointing at unusable entrypoints.

~Andrew