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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

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

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> I'd like to keep the "beautification" part, i.e. transforming to more use
> of Kbuild.include machinery, separate.

Sounds good to me. One step at a time.

> The check-endbr.sh invocation doesn't fit neatly into this model. I was
> considering to move it into $(TARGET)'s rule, but that's not very nice
> either (both because it'd be odd [strictly speaking: wrong] for xen.efi,
> and because it would reduce parallelism).
> ---
> v2: Mark intermediate files as such. Don't use $(if_changed ...).
> 
> --- 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?

> +
> +include scripts/Makefile.link
>  
>  $(obj)/note.o: $(TARGET)-syms
>       $(OBJCOPY) -O binary --only-section=.note.gnu.build-id $< $@.bin
> @@ -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.

> +     $(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.

> +
> +.$(TARGET).efi.%r.S:
> +     $(MKRELOC) $^ > $@
> +
> +.$(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?

> +
> +.$(TARGET).efi.alt.%: $(objtree)/prelink.o .$(TARGET).efi.%r.o \
> +                      .$(TARGET).efi.%s.o $(note_file) $(obj)/efi.lds
> +     $(LD) $(call EFI_LDFLAGS,$(ALT_BASE)) -T $(obj)/efi.lds $(efi-objs) \
> +           --strip-debug $(note_file_option) -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.)


Overall, the changes looks good to me.

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech