[PATCH 6/6] symbols: centralize and re-arrange $(all_symbols) calculation

Jan Beulich posted 6 patches 11 months ago
There is a newer version of this series
[PATCH 6/6] symbols: centralize and re-arrange $(all_symbols) calculation
Posted by Jan Beulich 11 months ago
For one there's no need for each architecture to have the same logic.
Move to the root Makefile, also to calculate just once.

And then re-arrange to permit FAST_SYMBOL_LOOKUP to be independent of
LIVEPATCH, which may be useful in (at least) debugging.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Likely syms-warn-dup-y wants to follow suit; it doesn't even have an Arm
counterpart right now.

--- a/xen/Makefile
+++ b/xen/Makefile
@@ -460,6 +460,10 @@ ALL_OBJS-$(CONFIG_CRYPTO) += crypto/buil
 
 ALL_LIBS-y                := lib/lib.a
 
+all-symbols-y :=
+all-symbols-$(CONFIG_LIVEPATCH) += --all-symbols
+all-symbols-$(CONFIG_FAST_SYMBOL_LOOKUP) += --sort-by-name
+
 include $(srctree)/arch/$(SRCARCH)/arch.mk
 
 # define new variables to avoid the ones defined in Config.mk
@@ -612,7 +616,8 @@ $(TARGET): outputmakefile asm-generic FO
 	$(Q)$(MAKE) $(build)=include all
 	$(Q)$(MAKE) $(build)=arch/$(SRCARCH) include
 	$(Q)$(MAKE) $(build)=. arch/$(SRCARCH)/include/asm/asm-offsets.h
-	$(Q)$(MAKE) $(build)=. MKRELOC=$(MKRELOC) 'ALL_OBJS=$(ALL_OBJS-y)' 'ALL_LIBS=$(ALL_LIBS-y)' $@
+	$(Q)$(MAKE) $(build)=. MKRELOC=$(MKRELOC) 'ALL_OBJS=$(ALL_OBJS-y)' \
+	            'ALL_LIBS=$(ALL_LIBS-y)' 'all_symbols=$(all-symbols-y)' $@
 
 SUBDIRS = xsm arch common crypto drivers lib test
 define all_sources
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -81,15 +81,6 @@ ifneq ($(CONFIG_DTB_FILE),"")
 obj-y += dtb.o
 endif
 
-ifdef CONFIG_LIVEPATCH
-all_symbols = --all-symbols
-ifdef CONFIG_FAST_SYMBOL_LOOKUP
-all_symbols = --all-symbols --sort-by-name
-endif
-else
-all_symbols =
-endif
-
 $(TARGET): $(TARGET)-syms
 	$(OBJCOPY) -O binary -S $< $@
 ifeq ($(CONFIG_ARM_64),y)
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -111,15 +111,6 @@ notes_phdrs = --notes
 endif
 endif
 
-ifdef CONFIG_LIVEPATCH
-all_symbols = --all-symbols
-ifdef CONFIG_FAST_SYMBOL_LOOKUP
-all_symbols = --all-symbols --sort-by-name
-endif
-else
-all_symbols =
-endif
-
 syms-warn-dup-y := --warn-dup
 syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
 syms-warn-dup-$(CONFIG_ENFORCE_UNIQUE_SYMBOLS) := --error-dup
Re: [PATCH 6/6] symbols: centralize and re-arrange $(all_symbols) calculation
Posted by Andrew Cooper 11 months ago
On 13/03/2025 1:55 pm, Jan Beulich wrote:
> For one there's no need for each architecture to have the same logic.
> Move to the root Makefile, also to calculate just once.
>
> And then re-arrange to permit FAST_SYMBOL_LOOKUP to be independent of
> LIVEPATCH, which may be useful in (at least) debugging.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Likely syms-warn-dup-y wants to follow suit; it doesn't even have an Arm
> counterpart right now.

Recently, I thought the same about --orphan-handling={warn,error} too. 
We need to up it to error, and enforce it consistently.

There's actually a lot of $(TARGET)-syms which ought to be less
copy&paste.  I'll submit my cleanup so far, which doesn't interact here
I don't think, but is also incomplete.

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -460,6 +460,10 @@ ALL_OBJS-$(CONFIG_CRYPTO) += crypto/buil
>  
>  ALL_LIBS-y                := lib/lib.a
>  
> +all-symbols-y :=
> +all-symbols-$(CONFIG_LIVEPATCH) += --all-symbols
> +all-symbols-$(CONFIG_FAST_SYMBOL_LOOKUP) += --sort-by-name
> +

I presume this works, so it's after we've processed Kconfig, but is
there really nowhere better for it to live?

If we're moving others, this is going to turn into a lot, and it's
specific to one final stage.

~Andrew

Re: [PATCH 6/6] symbols: centralize and re-arrange $(all_symbols) calculation
Posted by Jan Beulich 11 months ago
On 13.03.2025 18:02, Andrew Cooper wrote:
> On 13/03/2025 1:55 pm, Jan Beulich wrote:
>> For one there's no need for each architecture to have the same logic.
>> Move to the root Makefile, also to calculate just once.
>>
>> And then re-arrange to permit FAST_SYMBOL_LOOKUP to be independent of
>> LIVEPATCH, which may be useful in (at least) debugging.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Likely syms-warn-dup-y wants to follow suit; it doesn't even have an Arm
>> counterpart right now.
> 
> Recently, I thought the same about --orphan-handling={warn,error} too. 
> We need to up it to error, and enforce it consistently.
> 
> There's actually a lot of $(TARGET)-syms which ought to be less
> copy&paste.

Indeed. Iirc like me I think you indicated you'd like to also break up
those multi-step rules, to properly work through dependencies instead.

>  I'll submit my cleanup so far, which doesn't interact here
> I don't think, but is also incomplete.
> 
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -460,6 +460,10 @@ ALL_OBJS-$(CONFIG_CRYPTO) += crypto/buil
>>  
>>  ALL_LIBS-y                := lib/lib.a
>>  
>> +all-symbols-y :=
>> +all-symbols-$(CONFIG_LIVEPATCH) += --all-symbols
>> +all-symbols-$(CONFIG_FAST_SYMBOL_LOOKUP) += --sort-by-name
>> +
> 
> I presume this works, so it's after we've processed Kconfig, but is
> there really nowhere better for it to live?

What do you mean by "better"? If we're to (slowly) centralize the
linking, this is where all of that would move. xen/Makefile is processed
just once (twice if .config changed), and ...

> If we're moving others, this is going to turn into a lot, and it's
> specific to one final stage.

... applicable in exactly that one case. Or are you suggesting to
introduce a new helper makefile where just the linking settings and
(eventually) logic live? That would be a bigger piece of work, which I'm
afraid I'm not up to right now.

For now it seemed quite natural to place all-symbols next to ALL_OBJS
and ALL_LIBS.

Jan