Re: [PATCH 6/8] x86/EFI: avoid use of GNU ld's --disable-reloc-section when possible

From: Jan Beulich <jbeulich@suse.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH 6/8] x86/EFI: avoid use of GNU ld's --disable-reloc-section when possible
Date: Thu, 22 Apr 2021 12:42:44 +0200	[thread overview]
Message-ID: <0e102d7d-d633-7865-ea03-5d56238fb3c3@suse.com> (raw)
In-Reply-To: <YIEkPorW1vEibACD@Air-de-Roger>

On 22.04.2021 09:22, Roger Pau Monné wrote:
> On Wed, Apr 21, 2021 at 05:34:29PM +0200, Jan Beulich wrote:
>> On 21.04.2021 17:20, Roger Pau Monné wrote:
>>> On Wed, Apr 21, 2021 at 02:03:49PM +0200, Jan Beulich wrote:
>>>> On 21.04.2021 12:21, Roger Pau Monné wrote:
>>>>> On Thu, Apr 01, 2021 at 11:46:44AM +0200, Jan Beulich wrote:
>>>>>> @@ -189,7 +208,11 @@ EFI_LDFLAGS += --no-insert-timestamp
>>>>>>  endif
>>>>>>  
>>>>>>  $(TARGET).efi: VIRT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A VIRT_START$$,,p')
>>>>>> +ifeq ($(MKRELOC),:)
>>>>>> +$(TARGET).efi: ALT_BASE :=
>>>>>> +else
>>>>>>  $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A ALT_START$$,,p')
>>>>>
>>>>> Could you maybe check whether $(relocs-dummy) is set as the condition
>>>>> here and use it here instead of efi/relocs-dummy.o?
>>>>
>>>> I can use it in the ifeq() if you think that's neater (the current way
>>>> is minimally shorter), but using it in the ALT_BASE assignment would
>>>> make this differ more from the VIRT_BASE one, which I'd like to avoid.
>>>
>>> Sorry, I think I'm slightly confused because when the linker can
>>> produce the required relocations relocs-dummy variable is left empty,
>>> so it won't be added to $(TARGET).efi.
>>>
>>> But we still need to generate the efi/relocs-dummy.o file for the ELF
>>> build I assume?
>>
>> Not anymore (after "x86/EFI: redo .reloc section bounds determination").
>> We need it, as said, to fish VIRT_BASE out of it. It indeed wouldn't get
>> linked into any of the final binaries anymore.
> 
> Could you please add some text to the commit message to note that
> while relocs-dummy is not linked into the final binary it's still
> needed to get VIRT_BASE?

I've added

"Note that in the case that we leave base relocation generation to ld,
 while efi/relocs-dummy.o then won't be linked into any executable
 anymore, it still needs generating (and hence dependencies need to be
 kept as they are) in order to have VIRT_BASE pulled out of it."

> With that:
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

> FWIW, it would also be nice to add some comments to the $(TARGET).efi
> target commands, as it's quite impenetrable right now.

Well, my preferred plan was to pull it (and $(TARGET)-sym's) apart
into separate steps, suitably linked together via dependencies. This
may not eliminate altogether the need for some comments, but we then
may get away with brief ones.

Jan