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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.02.2021 18:17, Andrew Cooper wrote:
> 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.

Yes, in particular it wants to move out of Makefile (so it
won't get executed multiple times).

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

Backporting - yes, definitely. And hence minimally invasive would
indeed be helpful.

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

Thanks.

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

My objections to doing such stuff in Kconfig have remained
unresponded to, iirc. Plus doing this in Kconfig would help on its
own - we'd also need to further split which object files get linked
into which binary. (In fact a patch in the 4.16 series I have now
to use linker produced base relocations and to retain debug info I
do away with prelink-efi.o, for becoming identical to prelink.o.)

Jan