Re: [PATCH v2 1/7] x86: split xen-syms/xen.efi linking rules

[Jan Beulich <jbeulich@suse.com>]

On 02.09.2026 16:52, Anthony PERARD wrote:
> On Wed, Aug 26, 2026 at 02:00:22PM +0200, Jan Beulich wrote:
>> Doing so, besides (hopefully) adding clarity (not the least by way of
>> using pattern rules where possible), also avoids explicit recursive
>> $(MAKE) invocations. For xen-syms move re-usable helper rules to a new
>> scripts/Makefile.link.
>>
>> While doing so, re-order .map file creation (which can in principle fail)
>> and check-endbr.sh invocation ahead of putting in place the final image
>> (which is now the result of a simple rename).
>>
>> Also drop --source-name= from the tools/symbols invocation which has
>> --empty passed, for being meaningless there.
>>
>> Note that the original "rm" at the end of the rule needs limiting:
>> Removing intermediate files (which $(MAKE) doesn't itself remove) would
>> cause re-linking even when installing as root (when common/version.o is
>> left unaltered, and hence an incremental build should do nothing as long
>> as nothing else changed in the source tree).
> 
> But as far as I can tell, both `rm` command are still the same,
> unaltered. And both command do removes file mark as intermediate via
> .INTERMEDIATE, before make would do so. Without the `rm` commands, make
> would leave alone ".xen*.*.o.sym" and "..xen*.*.o.d".

Oh, I'm sorry - this paragraph is stale from v1. I've now dropped it.

>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -102,12 +102,6 @@ notes_phdrs = --notes
>> +LAST_LINKING_PASS := 2
>> +
>> +final-image-check-$(CONFIG_XEN_IBT) = $(SHELL) $(srctree)/tools/check-endbr.sh $<
> 
> How about removing $< from this macro, and letting the users of
> $(final-image-check-y) decide which argument to use?

I did consider doing so, but decided against: The placement of the argument
within the command may (in principle) matter. Now that you also mention this,
I think I'll switch to

final-image-check-$(CONFIG_XEN_IBT) = $(SHELL) $(srctree)/tools/check-endbr.sh $(1)

using

	$(call final-image-check-y, $<)

at the use sites.

>> @@ -191,51 +165,69 @@ note_file_option ?= $(note_file)
>>  
>>  extra-$(XEN_BUILD_PE) += efi.lds
>>  ifeq ($(XEN_BUILD_PE),y)
>> -$(TARGET).efi: $(obj)/efi/relocs-dummy.o $(obj)/efi/relocs-empty.o $(obj)/efi/mkreloc
>> -$(TARGET).efi: $(objtree)/prelink.o $(note_file) $(obj)/efi.lds
>> +
>> +.INTERMEDIATE: $(addprefix .$(TARGET).efi., \
>> +                           $(foreach n, 0 1 2, \
>> +                                     $(n) alt.$(n) $(n)r.o $(n)s.o $(n)r.S $(n)s.S))
>> +
>> +.$(TARGET).efi.%.o: .$(TARGET).efi.%.S FORCE
> 
> Left over "FORCE" from v1. Without if_changed we should let make decide
> to execute the recipe or not.

Oh, indeed. The adjustments to the xen.efi machinery were done merely
to mirror the xen-syms ones; they weren't strictly necessary to do
(and hence this went unnoticed).

>> +	$(call cmd,cc_o_S)
>> +
>> +.$(TARGET).efi.1r.S: .$(TARGET).efi.0 $(if $(relocs-dummy),.$(TARGET).efi.alt.0)
>> +.$(TARGET).efi.2r.S: .$(TARGET).efi.1 $(if $(relocs-dummy),.$(TARGET).efi.alt.1)
>> +
>> +.$(TARGET).efi.0r.o: $(obj)/efi/relocs-dummy.o $(obj)/efi/mkreloc
>> +	ln -sf $< $@
> 
> Why mkreloc is a prerequisite of this rule? It's not use here.
> 
> It could be move to the next rule, where it is actually used, and we
> could use order-only prerequisite, so $^ won't be altered. I've check,
> order-only prereq where introduced in make 3.80 according to the
> changelog of 3.81. And they are not part of the $^ variable.
> 
> But the target won't get rebuilt if mkreloc is changed. So order-only
> might not be the right type of prerequisite.

Indeed, it wants to be a real prereq. And rather than ...

>> +.$(TARGET).efi.%r.S:
>> +	$(MKRELOC) $^ > $@

... filtering it out of $^ I think it's easier the way it is. I can add a
comment, unless you think I need to move it here and do the filtering.

But wait - it really needs to move here, as the tool having been rebuilt
needs to cause rebuilding of these .S files (while .$(TARGET).efi.0r.o
wouldn't change at all).

>> +.$(TARGET).efi.0s.S:
>> +	$(objtree)/tools/symbols $(all_symbols) --empty > $@
>> +
>> +.$(TARGET).efi.1s.S: .$(TARGET).efi.0
>> +.$(TARGET).efi.2s.S: .$(TARGET).efi.1
>> +
>> +.$(TARGET).efi.%s.S:
>> +	$(NM) -pa --format=sysv $< \
>> +	  | $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>> + 	    --source-name=$(TARGET).efi.S \
>> +	  > $@
>> +
>> +# See above for why $(note_file) needs removing here.
>> +efi-objs = $(filter-out $(note_file),$(filter %.o,$^))
>> +
>> +.$(TARGET).efi.%: $(objtree)/prelink.o .$(TARGET).efi.%r.o \
>> +                  .$(TARGET).efi.%s.o $(note_file) $(obj)/efi.lds
>> +	$(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T $(obj)/efi.lds $(efi-objs) \
>> +	      --strip-debug $(note_file_option) -o $@
> 
> This command have changed compared to what we have currently, for the
> step ".xen.efi.0". In the case where $(relocs-dummy) is empty, this
> command doesn't have relocs-dummy.o on the command line. With this patch,
> the object is added, via .xen.efi.1r.o. Is this fine?

For .xen.efi.0 it's .xen.efi.0r.o, and the rule for the latter is making
a symlink to relocs-dummy.o.

>> --- /dev/null
>> +++ b/xen/scripts/Makefile.link
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# ==========================================================================
>> +# Helper rules for linking xen-syms
>> +# ==========================================================================
>> +
>> +syms-warn-dup-y := --warn-dup
>> +syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
>> +syms-warn-dup-$(CONFIG_ENFORCE_UNIQUE_SYMBOLS) := --error-dup
>> +
>> +orphan-handling-$(call ld-option,--orphan-handling=warn) := --orphan-handling=warn
>> +
>> +final-image-check-y ?= true
>> +
>> +.INTERMEDIATE: $(addprefix .$(TARGET)-syms.,$(foreach n,0 1 2 3,$(n) $(n).o $(n).S))
>> +
>> +.$(TARGET)-syms.%.o: .$(TARGET)-syms.%.S
>> +	$(call cmd,cc_o_S)
> 
> That recipe change slight we what's currently in tree, there's now
> "-DXEN_BUILD_EFI -DBUILD_ID_EFI", but that's probably fine, CFLAGS-y
> from xen/arch/x86/Makefile are now taken into account. (That's
> likely the case also for .xen.efi.%.o but I haven't checked.)

Yes, the same applies there, and yes, the two extra -D are entirely
benign (and strictly speaking more correct, if either would matter for
these .S files; right now xen.lds.S is their only consumer).

Jan