1. Patchew
  2. Xen
  3. View series

[PATCH] xen/arch: Simplify $(TARGET)-syms rule

Andrew Cooper posted 1 patch 7 months ago
Failed in applying to current master (apply log)
xen/arch/arm/Makefile   | 10 ++++------
xen/arch/ppc/Makefile   | 10 ++++------
xen/arch/riscv/Makefile | 10 ++++------
xen/arch/x86/Makefile   | 10 ++++------
4 files changed, 16 insertions(+), 24 deletions(-)
[PATCH] xen/arch: Simplify $(TARGET)-syms rule
Posted by Andrew Cooper 7 months ago 
In all cases, "-T $(obj)/xen.lds" can be factored out by appending to
XEN_LDFLAGS.  This takes the $(LD) commands from multi-line to single-line.

x86 uses $(build_id_linker) for all links, so factor that out too.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Shawn Anastasio <sanastasio@raptorengineering.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>

This is based on top of Roger's "xen: remove -N from the linker command line".

x86 might want to follow ARM's lead and only use $(build_id_linker) on the
final link.  It also wants extending to PPC/RISC-V

$(orphan-handling-y) needs making common and using ouside of x86 too.
---
 xen/arch/arm/Makefile   | 10 ++++------
 xen/arch/ppc/Makefile   | 10 ++++------
 xen/arch/riscv/Makefile | 10 ++++------
 xen/arch/x86/Makefile   | 10 ++++------
 4 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 129a109d6ec5..811adf3567c4 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -96,21 +96,19 @@ ifeq ($(CONFIG_ARM_64),y)
 	ln -sf $(@F) $@.efi
 endif
 
+$(TARGET)-syms: XEN_LDFLAGS += -T $(obj)/xen.lds
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< \
-	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0
+	$(LD) $(XEN_LDFLAGS) $< $(objtree)/common/symbols-dummy.o -o $(dot-target).0
 	$(NM) -pa --format=sysv $(dot-target).0 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).0.S
 	$(MAKE) $(build)=$(@D) $(dot-target).0.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< \
-	    $(dot-target).0.o -o $(dot-target).1
+	$(LD) $(XEN_LDFLAGS) $< $(dot-target).0.o -o $(dot-target).1
 	$(NM) -pa --format=sysv $(dot-target).1 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).1.S
 	$(MAKE) $(build)=$(@D) $(dot-target).1.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
-	    $(dot-target).1.o -o $@
+	$(LD) $(XEN_LDFLAGS) $< $(build_id_linker) $(dot-target).1.o -o $@
 	$(NM) -pa --format=sysv $@ \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
 		> $@.map
diff --git a/xen/arch/ppc/Makefile b/xen/arch/ppc/Makefile
index cf27bcebb25a..9ba23ecfff7e 100644
--- a/xen/arch/ppc/Makefile
+++ b/xen/arch/ppc/Makefile
@@ -11,21 +11,19 @@ obj-y += tlb-radix.o
 $(TARGET): $(TARGET)-syms
 	cp -f $< $@
 
+$(TARGET)-syms: XEN_LDFLAGS += -T $(obj)/xen.lds
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< \
-	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0
+	$(LD) $(XEN_LDFLAGS) $< $(objtree)/common/symbols-dummy.o -o $(dot-target).0
 	$(NM) -pa --format=sysv $(dot-target).0 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).0.S
 	$(MAKE) $(build)=$(@D) $(dot-target).0.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< \
-	    $(dot-target).0.o -o $(dot-target).1
+	$(LD) $(XEN_LDFLAGS) $< $(dot-target).0.o -o $(dot-target).1
 	$(NM) -pa --format=sysv $(dot-target).1 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).1.S
 	$(MAKE) $(build)=$(@D) $(dot-target).1.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
-	    $(dot-target).1.o -o $@
+	$(LD) $(XEN_LDFLAGS) $< $(build_id_linker) $(dot-target).1.o -o $@
 	$(NM) -pa --format=sysv $@ \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
 		> $@.map
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 516f5d505ca8..92e084db91a2 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -15,21 +15,19 @@ obj-y += vm_event.o
 $(TARGET): $(TARGET)-syms
 	$(OBJCOPY) -O binary -S $< $@
 
+$(TARGET)-syms: XEN_LDFLAGS += -T $(obj)/xen.lds
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< \
-	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0
+	$(LD) $(XEN_LDFLAGS) $< $(objtree)/common/symbols-dummy.o -o $(dot-target).0
 	$(NM) -pa --format=sysv $(dot-target).0 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).0.S
 	$(MAKE) $(build)=$(@D) $(dot-target).0.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< \
-	    $(dot-target).0.o -o $(dot-target).1
+	$(LD) $(XEN_LDFLAGS) $< $(dot-target).0.o -o $(dot-target).1
 	$(NM) -pa --format=sysv $(dot-target).1 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).1.S
 	$(MAKE) $(build)=$(@D) $(dot-target).1.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
-	    $(dot-target).1.o -o $@
+	$(LD) $(XEN_LDFLAGS) $< $(build_id_linker) $(dot-target).1.o -o $@
 	$(NM) -pa --format=sysv $@ \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
 		> $@.map
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index c2f1dcf301d6..b5227aeabb81 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -138,21 +138,19 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
 
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 
+$(TARGET)-syms: XEN_LDFLAGS += -T $(obj)/xen.lds $(build_id_linker)
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
-	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0
+	$(LD) $(XEN_LDFLAGS) $< $(objtree)/common/symbols-dummy.o -o $(dot-target).0
 	$(NM) -pa --format=sysv $(dot-target).0 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).0.S
 	$(MAKE) $(build)=$(@D) $(dot-target).0.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
-	    $(dot-target).0.o -o $(dot-target).1
+	$(LD) $(XEN_LDFLAGS) $< $(dot-target).0.o -o $(dot-target).1
 	$(NM) -pa --format=sysv $(dot-target).1 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \
 		> $(dot-target).1.S
 	$(MAKE) $(build)=$(@D) $(dot-target).1.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
-	    $(orphan-handling-y) $(dot-target).1.o -o $@
+	$(LD) $(XEN_LDFLAGS) $< $(orphan-handling-y) $(dot-target).1.o -o $@
 	$(NM) -pa --format=sysv $@ \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
 		> $@.map

base-commit: debe8bf537ec2c69a4734393cd2b0c7f57f74c0c
prerequisite-patch-id: 7911818c8833f061c9476332975b9aecb69bc26d
-- 
2.39.5
Re: [PATCH] xen/arch: Simplify $(TARGET)-syms rule
Posted by Anthony PERARD 7 months ago 
On Tue, Apr 01, 2025 at 03:47:03PM +0100, Andrew Cooper wrote:
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 129a109d6ec5..811adf3567c4 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -96,21 +96,19 @@ ifeq ($(CONFIG_ARM_64),y)
>  	ln -sf $(@F) $@.efi
>  endif
>  
> +$(TARGET)-syms: XEN_LDFLAGS += -T $(obj)/xen.lds

I would advise againts this change. Target-specific variables values
tend to apply to prerequist as well, so XEN_LDFLAGS will also be
modified when building prelink.o and xen.lds. It's might be ok in this
case because xen.lds doesn't use it, and prelink.o happen to be linked
in a different Makefile, but that's kind of setting a trap for someone
to fall into.

There's a keyword to keep the variable change strictly to $(TARGET)-syms
but that requires a more recent version of GNU make. That would be
"private" if we could use it. Looks like we need GNU make 3.82 to use
"private":
    "GNU make 3.82 is now available"
    https://lists.gnu.org/archive/html/info-gnu/2010-07/msg00023.html

>  $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
> -	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< \
> -	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0
> +	$(LD) $(XEN_LDFLAGS) $< $(objtree)/common/symbols-dummy.o -o $(dot-target).0

Cheers,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH] xen/arch: Simplify $(TARGET)-syms rule
Posted by Jan Beulich 7 months ago 
On 01.04.2025 16:47, Andrew Cooper wrote:
> In all cases, "-T $(obj)/xen.lds" can be factored out by appending to
> XEN_LDFLAGS.  This takes the $(LD) commands from multi-line to single-line.
> 
> x86 uses $(build_id_linker) for all links, so factor that out too.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

However, ...

> --- a/xen/arch/arm/Makefile
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -138,21 +138,19 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
>  
>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>  
> +$(TARGET)-syms: XEN_LDFLAGS += -T $(obj)/xen.lds $(build_id_linker)
>  $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
> -	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> -	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0
> +	$(LD) $(XEN_LDFLAGS) $< $(objtree)/common/symbols-dummy.o -o $(dot-target).0
>  	$(NM) -pa --format=sysv $(dot-target).0 \
>  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>  		> $(dot-target).0.S
>  	$(MAKE) $(build)=$(@D) $(dot-target).0.o
> -	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> -	    $(dot-target).0.o -o $(dot-target).1
> +	$(LD) $(XEN_LDFLAGS) $< $(dot-target).0.o -o $(dot-target).1
>  	$(NM) -pa --format=sysv $(dot-target).1 \
>  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \
>  		> $(dot-target).1.S
>  	$(MAKE) $(build)=$(@D) $(dot-target).1.o
> -	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> -	    $(orphan-handling-y) $(dot-target).1.o -o $@
> +	$(LD) $(XEN_LDFLAGS) $< $(orphan-handling-y) $(dot-target).1.o -o $@
>  	$(NM) -pa --format=sysv $@ \
>  		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
>  		> $@.map

... any reason not to do the same for xen.efi's EFI_LDFLAGS and its
-T $(obj)/efi.lds? I'd really like to keep the two rules as closely in sync
as possible, to make it a little easier to eventually split them into a chain
of smaller rules.

Jan

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.