1. Patchew
  2. Xen
  3. View series

[PATCH] Arm: do a 4th linking pass if necessary

Jan Beulich posted 1 patch 3 days, 8 hours ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/7b23e011-56b8-4f6c-bd55-83e1d9b0453f@suse.com
There is a newer version of this series
[PATCH] Arm: do a 4th linking pass if necessary
Posted by Jan Beulich 3 days, 8 hours ago 
The .rodata growth between 1st and 2nd linking passes may affect the
number of stubs the linker wants to insert, which in turn affects the
number of symbols. If symbol table sizes change after the 2nd linking
pass, insert another one before the final one. (As a comment in Linux puts
it, "In theory it's possible this results in even more stubs, but
unlikely.")

To use the $(compare-symbol-tables) macro in a shell "if", it needs
slightly adjusting (and then wrapping to be run in a sub-shell).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
There's imo no good Fixes: tag, since (aiui) stubs (veneers) may also be
inserted for reasons other than errata workarounds.

For context: The Cortex A53 erratum 843419 workaround in GNU ld comes in
two flavors: In the general case a stub is inserted, but if the problem
ADRP can be replaced by ADR (i.e. the target symbol is within ±1Mb), then
that's preferred and no stub is needed. The addition of the symbol table
data is what then results in the stub-less form to no longer be usable in
affected configurations. A possible countermeasure could be to move
.rodata ahead of .text. (Also, from looking at generated code, it may well
be that newer gcc simply avoids producing problematic insn sequences.)

Linux simply compares object file sizes, but I consider that fragile: A
change in size of one of the symbol table constituents may not necessarily
change the object file size, due to padding which may be in use.

Once we generalize linking, we may want to introduce an equivalent of
Linux'es KALLSYMS_EXTRA_PASS as well. I don't think doing this right here
would make overly much sense, though.

--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -99,9 +99,20 @@ $(TARGET)-syms: $(objtree)/prelink.o $(o
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).2.S
 	$(MAKE) $(build)=$(@D) $(dot-target).2.o
-	$(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o)
+	if ! { $(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o) >/dev/null; }; \
+	then \
+		$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
+		    $(dot-target).2.o -o $(dot-target).2; \
+		$(NM) -pa --format=sysv $(dot-target).2 \
+			| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
+			> $(dot-target).3.S; \
+		$(MAKE) $(build)=$(@D) $(dot-target).3.o; \
+		$(call compare-symbol-tables, $(dot-target).2.o, $(dot-target).3.o); \
+	else \
+		ln -sf $(dot-target).2.o $(dot-target).3.o; \
+	fi
 	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
-	    $(dot-target).2.o -o $@
+	    $(dot-target).3.o -o $@
 	$(NM) -pa --format=sysv $@ \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
 		> $@.map
--- a/xen/scripts/Kbuild.include
+++ b/xen/scripts/Kbuild.include
@@ -65,7 +65,7 @@ define compare-symbol-tables
     $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(1).sym; \
     ln -f $(2) $(@D)/.cst.$$$$; \
     $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(2).sym; \
-    rm -f $(@D)/.cst.$$$$
+    rm -f $(@D)/.cst.$$$$; \
     diff -u $(1).sym $(2).sym
 endef
Re: [PATCH] Arm: do a 4th linking pass if necessary
Posted by Anthony PERARD 3 days, 4 hours ago 
On Wed, May 20, 2026 at 01:53:34PM +0200, Jan Beulich wrote:
> Once we generalize linking, we may want to introduce an equivalent of
> Linux'es KALLSYMS_EXTRA_PASS as well. I don't think doing this right here
> would make overly much sense, though.

If you generalise linking, you are going to put it in a shell script,
right? Because this recipe is getting very complicated, for within a
makefile.

> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -99,9 +99,20 @@ $(TARGET)-syms: $(objtree)/prelink.o $(o
>  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>  		> $(dot-target).2.S
>  	$(MAKE) $(build)=$(@D) $(dot-target).2.o
> -	$(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o)
> +	if ! { $(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o) >/dev/null; }; \

This `>/dev/null` seems to only suppress the output of the `diff` of the
macro, is it what is intended?

> +	then \
> +		$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> +		    $(dot-target).2.o -o $(dot-target).2; \
> +		$(NM) -pa --format=sysv $(dot-target).2 \
> +			| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
> +			> $(dot-target).3.S; \
> +		$(MAKE) $(build)=$(@D) $(dot-target).3.o; \

This new block ignore all errors, from LD, NM and MAKE. We want
a `set -e` before the if.

> +		$(call compare-symbol-tables, $(dot-target).2.o, $(dot-target).3.o); \

At least, an error returned by `diff` in that macro should be taken into
account, for now.

> +	else \
> +		ln -sf $(dot-target).2.o $(dot-target).3.o; \
> +	fi
>  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> -	    $(dot-target).2.o -o $@
> +	    $(dot-target).3.o -o $@
>  	$(NM) -pa --format=sysv $@ \
>  		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
>  		> $@.map
> --- a/xen/scripts/Kbuild.include
> +++ b/xen/scripts/Kbuild.include
> @@ -65,7 +65,7 @@ define compare-symbol-tables
>      $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(1).sym; \
>      ln -f $(2) $(@D)/.cst.$$$$; \
>      $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(2).sym; \
> -    rm -f $(@D)/.cst.$$$$
> +    rm -f $(@D)/.cst.$$$$; \
>      diff -u $(1).sym $(2).sym

This macro is missing `set -e`, if both OBJDUMP command fails and create
an empty file, `diff` will return success. But looks like `set -e` in
this macro isn't going to work in the condition of the `if`.


Thanks,


-- 
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH] Arm: do a 4th linking pass if necessary
Posted by Jan Beulich 2 days, 14 hours ago 
On 20.05.2026 18:03, Anthony PERARD wrote:
> On Wed, May 20, 2026 at 01:53:34PM +0200, Jan Beulich wrote:
>> Once we generalize linking, we may want to introduce an equivalent of
>> Linux'es KALLSYMS_EXTRA_PASS as well. I don't think doing this right here
>> would make overly much sense, though.
> 
> If you generalise linking, you are going to put it in a shell script,
> right? Because this recipe is getting very complicated, for within a
> makefile.

Well. Prior to this change I was definitely hoping to spit the big rule up
into small ones. Whether that's still feasible with an optional path I'll
have to see; I very much would prefer if I could keep everything in make
logic.

>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -99,9 +99,20 @@ $(TARGET)-syms: $(objtree)/prelink.o $(o
>>  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>>  		> $(dot-target).2.S
>>  	$(MAKE) $(build)=$(@D) $(dot-target).2.o
>> -	$(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o)
>> +	if ! { $(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o) >/dev/null; }; \
> 
> This `>/dev/null` seems to only suppress the output of the `diff` of the
> macro, is it what is intended?

Yes. All errors and alike should appear normally.

>> +	then \
>> +		$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
>> +		    $(dot-target).2.o -o $(dot-target).2; \
>> +		$(NM) -pa --format=sysv $(dot-target).2 \
>> +			| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>> +			> $(dot-target).3.S; \
>> +		$(MAKE) $(build)=$(@D) $(dot-target).3.o; \
> 
> This new block ignore all errors, from LD, NM and MAKE. We want
> a `set -e` before the if.

Hmm, perhaps I should add that, yes, albeit ...

>> +		$(call compare-symbol-tables, $(dot-target).2.o, $(dot-target).3.o); \
> 
> At least, an error returned by `diff` in that macro should be taken into
> account, for now.

... I expect this would fail if there was an earlier error.

>> --- a/xen/scripts/Kbuild.include
>> +++ b/xen/scripts/Kbuild.include
>> @@ -65,7 +65,7 @@ define compare-symbol-tables
>>      $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(1).sym; \
>>      ln -f $(2) $(@D)/.cst.$$$$; \
>>      $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(2).sym; \
>> -    rm -f $(@D)/.cst.$$$$
>> +    rm -f $(@D)/.cst.$$$$; \
>>      diff -u $(1).sym $(2).sym
> 
> This macro is missing `set -e`, if both OBJDUMP command fails and create
> an empty file, `diff` will return success.

Whether to have "set -e" here is an independent question, I guess. To avoid
the case you mention, maybe better

      $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(1).sym || rm -f $(1).sym; \

?

> But looks like `set -e` in
> this macro isn't going to work in the condition of the `if`.

Whereas the above would be compatible with both uses, I think.

Jan
Re: [PATCH] Arm: do a 4th linking pass if necessary
Posted by Anthony PERARD 2 days, 7 hours ago 
On Thu, May 21, 2026 at 08:08:44AM +0200, Jan Beulich wrote:
> On 20.05.2026 18:03, Anthony PERARD wrote:
> > On Wed, May 20, 2026 at 01:53:34PM +0200, Jan Beulich wrote:
> >> +	then \
> >> +		$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> >> +		    $(dot-target).2.o -o $(dot-target).2; \
> >> +		$(NM) -pa --format=sysv $(dot-target).2 \
> >> +			| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
> >> +			> $(dot-target).3.S; \
> >> +		$(MAKE) $(build)=$(@D) $(dot-target).3.o; \
> > 
> > This new block ignore all errors, from LD, NM and MAKE. We want
> > a `set -e` before the if.
> 
> Hmm, perhaps I should add that, yes, albeit ...
> 
> >> +		$(call compare-symbol-tables, $(dot-target).2.o, $(dot-target).3.o); \
> > 
> > At least, an error returned by `diff` in that macro should be taken into
> > account, for now.
> 
> ... I expect this would fail if there was an earlier error.

Yes, but that's fragile, and that's not how `make` behave. It's better if
every command behave the same way, that is the recipe fails on the first
command that fail. So adding `set -e` would be useful.

> >> --- a/xen/scripts/Kbuild.include
> >> +++ b/xen/scripts/Kbuild.include
> >> @@ -65,7 +65,7 @@ define compare-symbol-tables
> >>      $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(1).sym; \
> >>      ln -f $(2) $(@D)/.cst.$$$$; \
> >>      $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(2).sym; \
> >> -    rm -f $(@D)/.cst.$$$$
> >> +    rm -f $(@D)/.cst.$$$$; \
> >>      diff -u $(1).sym $(2).sym
> > 
> > This macro is missing `set -e`, if both OBJDUMP command fails and create
> > an empty file, `diff` will return success.
> 
> Whether to have "set -e" here is an independent question, I guess. To avoid
> the case you mention, maybe better
> 
>       $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(1).sym || rm -f $(1).sym; \
> 
> ?

That sounds fine. (Replacing all the `;` by `&&` would work too.)

> > But looks like `set -e` in
> > this macro isn't going to work in the condition of the `if`.
> 
> Whereas the above would be compatible with both uses, I think.

Yes.


With at lest a `set -e` for the `if` block:
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,


-- 
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH] Arm: do a 4th linking pass if necessary
Posted by Oleksii Kurochko 3 days, 6 hours ago 
On 5/20/26 1:53 PM, Jan Beulich wrote:
> The .rodata growth between 1st and 2nd linking passes may affect the
> number of stubs the linker wants to insert, which in turn affects the
> number of symbols. If symbol table sizes change after the 2nd linking
> pass, insert another one before the final one. (As a comment in Linux puts
> it, "In theory it's possible this results in even more stubs, but
> unlikely.")
> 
> To use the $(compare-symbol-tables) macro in a shell "if", it needs
> slightly adjusting (and then wrapping to be run in a sub-shell).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> There's imo no good Fixes: tag, since (aiui) stubs (veneers) may also be
> inserted for reasons other than errata workarounds.
> 
> For context: The Cortex A53 erratum 843419 workaround in GNU ld comes in
> two flavors: In the general case a stub is inserted, but if the problem
> ADRP can be replaced by ADR (i.e. the target symbol is within ±1Mb), then
> that's preferred and no stub is needed. The addition of the symbol table
> data is what then results in the stub-less form to no longer be usable in
> affected configurations. A possible countermeasure could be to move
> .rodata ahead of .text. (Also, from looking at generated code, it may well
> be that newer gcc simply avoids producing problematic insn sequences.)
> 
> Linux simply compares object file sizes, but I consider that fragile: A
> change in size of one of the symbol table constituents may not necessarily
> change the object file size, due to padding which may be in use.
> 
> Once we generalize linking, we may want to introduce an equivalent of
> Linux'es KALLSYMS_EXTRA_PASS as well. I don't think doing this right here
> would make overly much sense, though.

Probably we want to have the similar for RISC-V. Not so far time ago in 
downstream we faced the following issue:
https://gitlab.com/xen-project/people/olkur/xen/-/jobs/14389897308

Unfortunately I wasn't able to reproduce it locally and it was only on 
CI but I suggested the similar to this fix and it helped:

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index a1124eb5d072..72b9789cfd9c 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -52,9 +52,15 @@ $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
  		> $(dot-target).2.S
  	$(MAKE) $(build)=$(@D) $(dot-target).2.o
-	$(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o)
  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
-	    $(dot-target).2.o -o $@
+	    $(dot-target).2.o -o $(dot-target).2
+	$(NM) -pa --format=sysv $(dot-target).2 \
+		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
+		> $(dot-target).3.S
+	$(MAKE) $(build)=$(@D) $(dot-target).3.o
+	$(call compare-symbol-tables, $(dot-target).2.o, $(dot-target).3.o)
+	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
+	    $(dot-target).3.o -o $@
  	$(NM) -pa --format=sysv $@ \
  		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
  		> $@.map

The fix wasn't sent as after merging some extra patches on top of it the 
issue just disappear.

> 
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -99,9 +99,20 @@ $(TARGET)-syms: $(objtree)/prelink.o $(o
>   		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>   		> $(dot-target).2.S
>   	$(MAKE) $(build)=$(@D) $(dot-target).2.o
> -	$(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o)
> +	if ! { $(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o) >/dev/null; }; \
> +	then \
> +		$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> +		    $(dot-target).2.o -o $(dot-target).2; \
> +		$(NM) -pa --format=sysv $(dot-target).2 \
> +			| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
> +			> $(dot-target).3.S; \
> +		$(MAKE) $(build)=$(@D) $(dot-target).3.o; \
> +		$(call compare-symbol-tables, $(dot-target).2.o, $(dot-target).3.o); \
> +	else \
> +		ln -sf $(dot-target).2.o $(dot-target).3.o; \
> +	fi
>   	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> -	    $(dot-target).2.o -o $@
> +	    $(dot-target).3.o -o $@
>   	$(NM) -pa --format=sysv $@ \
>   		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
>   		> $@.map
> --- a/xen/scripts/Kbuild.include
> +++ b/xen/scripts/Kbuild.include
> @@ -65,7 +65,7 @@ define compare-symbol-tables
>       $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(1).sym; \
>       ln -f $(2) $(@D)/.cst.$$$$; \
>       $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(2).sym; \
> -    rm -f $(@D)/.cst.$$$$
> +    rm -f $(@D)/.cst.$$$$; \
>       diff -u $(1).sym $(2).sym
>   endef
>   

LGTM:
  Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.