1. Patchew
  2. Xen
  3. [PATCH 0/2] Enable --gc-sections
  4. View patch

[PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

Jason Andryuk posted 2 patches 2 days, 21 hours ago
[PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jason Andryuk 2 days, 21 hours ago 
Add a new Kconfig CONFIG_GC_SECTIONS to control linking with
--gc-sections.  --gc-sections witll garbage collect unused sections.
Combined with CONFIG_CC_SPLIT_SECTIONS, this will remove unreachable
code and data.

Linker scripts need to add KEEP() assorted places to retain
appropriate data - especially the arrays created by the linker script.

This has some affect, but it is inomplete.  In a test where memory_add()
is unreachable, it is still included.  I'm not sure, but it seems that
alternatives keep a relocation reference to it.

Only ELF xen is supported.  xen.efi fails to link with many undefined
references when using --gc-sections.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v1:
Add Kconfig
select CC_SPLIT_SECTIONS
remove KEEP from .fixup
Add KEEP to .text.entry.* (Only needed with Jan's "common: honor
CONFIG_CC_SPLIT_SECTIONS also for assembly functions" ?)
Add ARM, RISC-V and PPC

Pipeline passes:
https://gitlab.com/xen-project/people/jandryuk-amd/xen/-/pipelines/2205223331

It defaults CONFIG_GC_SECTIONS=y and adds --print-gc-sections

With --print-gc-sections on x86 defconfig + GC_SECTIONS=y debug=y:
ld: removing unused section '.text.__bitmap_full' in file 'prelink.o'
ld: removing unused section '.text.__bitmap_xor' in file 'prelink.o'
ld: removing unused section '.text.__bitmap_set' in file 'prelink.o'
ld: removing unused section '.text.__bitmap_clear' in file 'prelink.o'
ld: removing unused section '.text.bitmap_find_free_region' in file 'prelink.o'
ld: removing unused section '.text.bitmap_release_region' in file 'prelink.o'
ld: removing unused section '.text.domain_has_ioreq_server' in file 'prelink.o'
ld: removing unused section '.text.compat_kexec_op' in file 'prelink.o'
ld: removing unused section '.text.in_atomic' in file 'prelink.o'
ld: removing unused section '.text.radix_tree_next_hole' in file 'prelink.o'
ld: removing unused section '.text.radix_tree_prev_hole' in file 'prelink.o'
ld: removing unused section '.text.radix_tree_gang_lookup_slot' in file 'prelink.o'
ld: removing unused section '.text._nrspin_trylock' in file 'prelink.o'
ld: removing unused section '.text.xen_compile_host' in file 'prelink.o'
ld: removing unused section '.text.vscnprintf' in file 'prelink.o'
ld: removing unused section '.text.wake_up_one' in file 'prelink.o'
ld: removing unused section '.text.xmem_pool_get_used_size' in file 'prelink.o'
ld: removing unused section '.text.xmem_pool_get_total_size' in file 'prelink.o'
ld: removing unused section '.text.xmem_pool_maxalloc' in file 'prelink.o'
ld: removing unused section '.text.xlat_start_info' in file 'prelink.o'
ld: removing unused section '.text.elf_sym_by_name' in file 'prelink.o'
ld: removing unused section '.text.elf_sym_by_index' in file 'prelink.o'
ld: removing unused section '.text.elf_get_ptr' in file 'prelink.o'
ld: removing unused section '.text.elf_lookup_addr' in file 'prelink.o'
ld: removing unused section '.text.serial_vuart_info' in file 'prelink.o'
ld: removing unused section '.text.pci_find_next_cap' in file 'prelink.o'
ld: removing unused section '.text.free_hvm_irq_dpci' in file 'prelink.o'
ld: removing unused section '.text.mce_barrier_init' in file 'prelink.o'
ld: removing unused section '.text.mce_barrier_dec' in file 'prelink.o'
ld: removing unused section '.text.mce_barrier' in file 'prelink.o'
ld: removing unused section '.text.apei_read_mce' in file 'prelink.o'
ld: removing unused section '.text.apei_check_mce' in file 'prelink.o'
ld: removing unused section '.text.apei_clear_mce' in file 'prelink.o'
ld: removing unused section '.text.efi_halt_system' in file 'prelink.o'
ld: removing unused section '.text.get_vvmcs_virtual_safe' in file 'prelink.o'
ld: removing unused section '.text.get_vvmcs_real_safe' in file 'prelink.o'
ld: removing unused section '.text.set_vvmcs_real' in file 'prelink.o'
ld: removing unused section '.text.set_vvmcs_virtual_safe' in file 'prelink.o'
ld: removing unused section '.text.set_vvmcs_real_safe' in file 'prelink.o'
ld: removing unused section '.text.domain_set_alloc_bitsize' in file 'prelink.o'
ld: removing unused section '.text.watchdog_enabled' in file 'prelink.o'
ld: removing unused section '.text.unset_nmi_callback' in file 'prelink.o'
ld: removing unused section '.text.sha2_256_init' in file 'prelink.o'
ld: removing unused section '.text.xxh64_copy_state' in file 'prelink.o'
ld: removing unused section '.text.xxh64' in file 'prelink.o'
ld: removing unused section '.discard' in file 'prelink.o'
ld: removing unused section '.rodata.xen_compile_host.str1.1' in file 'prelink.o'
ld: removing unused section '.rodata.elf_lookup_addr.str1.1' in file 'prelink.o'
ld: removing unused section '.rodata.apei_read_mce.str1.8' in file 'prelink.o'
ld: removing unused section '.rodata.efi_halt_system.str1.8' in file 'prelink.o'
ld: removing unused section '.rodata.play_dead.str1.1' in file 'prelink.o'
ld: removing unused section '.data.rel.ro.local.fetch_type_names' in file 'prelink.o'
---
 xen/Makefile              |  3 +++
 xen/arch/arm/Makefile     |  6 +++---
 xen/arch/arm/xen.lds.S    | 22 +++++++++++-----------
 xen/arch/ppc/Makefile     |  6 +++---
 xen/arch/ppc/xen.lds.S    | 14 +++++++-------
 xen/arch/riscv/Makefile   |  6 +++---
 xen/arch/riscv/xen.lds.S  | 14 +++++++-------
 xen/arch/x86/Makefile     |  6 +++---
 xen/arch/x86/xen.lds.S    | 34 +++++++++++++++++-----------------
 xen/common/Kconfig        |  9 +++++++++
 xen/include/xen/xen.lds.h | 20 ++++++++++----------
 11 files changed, 76 insertions(+), 64 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index e6cf287425..aeb5dcf2ee 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -469,10 +469,13 @@ all-symbols-$(CONFIG_FAST_SYMBOL_LOOKUP) += --sort-by-name
 
 include $(srctree)/arch/$(SRCARCH)/arch.mk
 
+XEN_FINAL_LDFLAGS-$(CONFIG_GC_SECTIONS) := --gc-sections
+
 # define new variables to avoid the ones defined in Config.mk
 export XEN_CFLAGS := $(CFLAGS)
 export XEN_AFLAGS := $(AFLAGS)
 export XEN_LDFLAGS := $(LDFLAGS)
+export XEN_FINAL_LDFLAGS := $(LDFLAGS) $(XEN_FINAL_LDFLAGS-y)
 export CFLAGS_UBSAN
 
 endif # need-config
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 7494a0f926..3ac5ff88cc 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -87,19 +87,19 @@ endif
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
 	$(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0.S
 	$(MAKE) $(build)=$(@D) $(dot-target).0.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< \
 	      $(dot-target).0.o -o $(dot-target).0
 	$(NM) -pa --format=sysv $(dot-target).0 \
 		| $(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 $< \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< \
 	    $(dot-target).1.o -o $(dot-target).1
 	$(NM) -pa --format=sysv $(dot-target).1 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).2.S
 	$(MAKE) $(build)=$(@D) $(dot-target).2.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
 	    $(dot-target).2.o -o $@
 	$(NM) -pa --format=sysv $@ \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 2d5f1c516d..178af612a2 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -63,7 +63,7 @@ SECTIONS
 
        . = ALIGN(4);
        __proc_info_start = .;
-       *(.proc.info)
+       KEEP(*(.proc.info))
        __proc_info_end = .;
   } :text
 
@@ -103,7 +103,7 @@ SECTIONS
   . = ALIGN(8);
   .arch.info : {
       _splatform = .;
-      *(.arch.info)
+      KEEP(*(.arch.info))
       _eplatform = .;
   } :text
 
@@ -116,7 +116,7 @@ SECTIONS
   . = ALIGN(8);
   .teemediator.info : {
       _steemediator = .;
-      *(.teemediator.info)
+      KEEP(*(.teemediator.info))
       _eteemediator = .;
   } :text
 
@@ -127,7 +127,7 @@ SECTIONS
        *(.init.text)
        _einittext = .;
        . = ALIGN(PAGE_SIZE);        /* Avoid mapping alt insns executable */
-       *(.altinstr_replacement)
+       KEEP(*(.altinstr_replacement))
   } :text
   . = ALIGN(PAGE_SIZE);
   __init_data_begin = .;
@@ -137,18 +137,18 @@ SECTIONS
 
        . = ALIGN(POINTER_ALIGN);
        __setup_start = .;
-       *(.init.setup)
+       KEEP(*(.init.setup))
        __setup_end = .;
 
        __initcall_start = .;
-       *(.initcallpresmp.init)
+       KEEP(*(.initcallpresmp.init))
        __presmp_initcall_end = .;
-       *(.initcall1.init)
+       KEEP(*(.initcall1.init))
        __initcall_end = .;
 
        . = ALIGN(4);
        __alt_instructions = .;
-       *(.altinstructions)
+       KEEP(*(.altinstructions))
        __alt_instructions_end = .;
 
        LOCK_PROFILE_DATA
@@ -159,9 +159,9 @@ SECTIONS
 
        . = ALIGN(8);
        __ctors_start = .;
-       *(.ctors)
-       *(.init_array)
-       *(SORT(.init_array.*))
+       KEEP(*(.ctors))
+       KEEP(*(.init_array))
+       KEEP(*(SORT(.init_array.*)))
        __ctors_end = .;
   } :text
   __init_end_efi = .;
diff --git a/xen/arch/ppc/Makefile b/xen/arch/ppc/Makefile
index e80690d3b8..42db3d6f2c 100644
--- a/xen/arch/ppc/Makefile
+++ b/xen/arch/ppc/Makefile
@@ -14,19 +14,19 @@ $(TARGET): $(TARGET)-syms
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
 	$(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0.S
 	$(MAKE) $(build)=$(@D) $(dot-target).0.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< \
 	      $(dot-target).0.o -o $(dot-target).0
 	$(NM) -pa --format=sysv $(dot-target).0 \
 		| $(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 $< \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< \
 	    $(dot-target).1.o -o $(dot-target).1
 	$(NM) -pa --format=sysv $(dot-target).1 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).2.S
 	$(MAKE) $(build)=$(@D) $(dot-target).2.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
 	    $(dot-target).2.o -o $@
 	$(NM) -pa --format=sysv $@ \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S
index d0f2ed43f1..c91df79468 100644
--- a/xen/arch/ppc/xen.lds.S
+++ b/xen/arch/ppc/xen.lds.S
@@ -24,7 +24,7 @@ SECTIONS
 
     DECL_SECTION(.text) {
         _stext = .;            /* Text section */
-        *(.text.header)
+        KEEP(*(.text.header))
 
         . = ALIGN(256);
         HIDDEN(_stext_exceptions = .);
@@ -109,13 +109,13 @@ SECTIONS
 
         . = ALIGN(POINTER_ALIGN);
         __setup_start = .;
-        *(.init.setup)
+        KEEP(*(.init.setup))
         __setup_end = .;
 
         __initcall_start = .;
-        *(.initcallpresmp.init)
+        KEEP(*(.initcallpresmp.init))
         __presmp_initcall_end = .;
-        *(.initcall1.init)
+        KEEP(*(.initcall1.init))
         __initcall_end = .;
 
         LOCK_PROFILE_DATA
@@ -126,9 +126,9 @@ SECTIONS
 
         . = ALIGN(8);
         __ctors_start = .;
-        *(.ctors)
-        *(.init_array)
-        *(SORT(.init_array.*))
+        KEEP(*(.ctors))
+        KEEP(*(.init_array))
+        KEEP(*(SORT(.init_array.*)))
         __ctors_end = .;
     } :text
 
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index d667234949..0cb0e88a72 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -24,19 +24,19 @@ $(TARGET): $(TARGET)-syms
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
 	$(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0.S
 	$(MAKE) $(build)=$(@D) $(dot-target).0.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< \
 	      $(dot-target).0.o -o $(dot-target).0
 	$(NM) -pa --format=sysv $(dot-target).0 \
 		| $(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 $< \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< \
 	    $(dot-target).1.o -o $(dot-target).1
 	$(NM) -pa --format=sysv $(dot-target).1 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).2.S
 	$(MAKE) $(build)=$(@D) $(dot-target).2.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
 	    $(dot-target).2.o -o $@
 	$(NM) -pa --format=sysv $@ \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index 45d2e053d0..e57db6b914 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -18,7 +18,7 @@ SECTIONS
     _start = .;
     .text : {
         _stext = .;            /* Text section */
-        *(.text.header)
+        KEEP(*(.text.header))
 
         *(.text.cold)
         *(.text.unlikely .text.*_unlikely .text.unlikely.*)
@@ -103,13 +103,13 @@ SECTIONS
 
         . = ALIGN(POINTER_ALIGN);
         __setup_start = .;
-        *(.init.setup)
+        KEEP(*(.init.setup))
         __setup_end = .;
 
         __initcall_start = .;
-        *(.initcallpresmp.init)
+        KEEP(*(.initcallpresmp.init))
         __presmp_initcall_end = .;
-        *(.initcall1.init)
+        KEEP(*(.initcall1.init))
         __initcall_end = .;
 
         LOCK_PROFILE_DATA
@@ -120,9 +120,9 @@ SECTIONS
 
         . = ALIGN(8);
         __ctors_start = .;
-        *(.ctors)
-        *(.init_array)
-        *(SORT(.init_array.*))
+        KEEP(*(.ctors))
+        KEEP(*(.init_array))
+        KEEP(*(SORT(.init_array.*)))
         __ctors_end = .;
     } :text
 
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 300cc67407..3fd4cf44ab 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -136,19 +136,19 @@ CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
 	$(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0.S
 	$(MAKE) $(build)=$(@D) $(dot-target).0.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
 	      $(dot-target).0.o -o $(dot-target).0
 	$(NM) -pa --format=sysv $(dot-target).0 \
 		| $(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) \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
 	    $(dot-target).1.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).2.S
 	$(MAKE) $(build)=$(@D) $(dot-target).2.o
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
+	$(LD) $(XEN_FINAL_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
 	    $(orphan-handling-y) $(dot-target).2.o -o $@
 	$(NM) -pa --format=sysv $@ \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 2aa41306ca..e4135edd28 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -76,12 +76,12 @@ SECTIONS
   _start = .;
   DECL_SECTION(.text) {
         _stext = .;            /* Text and read-only data */
-       *(.text.header)
+       KEEP(*(.text.header))
 
        . = ALIGN(PAGE_SIZE);
        _stextentry = .;
        *(.text.entry)
-       *(.text.entry.*)
+       KEEP(*(.text.entry.*))
        . = ALIGN(PAGE_SIZE);
        _etextentry = .;
 
@@ -116,7 +116,7 @@ SECTIONS
        . = ALIGN(8);
        /* Exception table */
        __start___ex_table = .;
-       *(.ex_table)
+       KEEP(*(.ex_table))
        __stop___ex_table = .;
 
        . = ALIGN(PAGE_SIZE);
@@ -207,7 +207,7 @@ SECTIONS
         * as binary blobs. The .altinstructions has enough data to get
         * the address and the length of them to patch the kernel safely.
         */
-       *(.altinstr_replacement)
+       KEEP(*(.altinstr_replacement))
 
 #ifdef EFI /* EFI wants to merge all of .init.*  ELF doesn't. */
        . = ALIGN(SMP_CACHE_BYTES);
@@ -220,8 +220,8 @@ SECTIONS
 
        . = ALIGN(POINTER_ALIGN);
        __initdata_cf_clobber_start = .;
-       *(.init.data.cf_clobber)
-       *(.init.rodata.cf_clobber)
+       KEEP(*(.init.data.cf_clobber))
+       KEEP(*(.init.rodata.cf_clobber))
        __initdata_cf_clobber_end = .;
 
        *(.init.rodata)
@@ -229,13 +229,13 @@ SECTIONS
 
        . = ALIGN(POINTER_ALIGN);
        __setup_start = .;
-       *(.init.setup)
+       KEEP(*(.init.setup))
        __setup_end = .;
 
        __initcall_start = .;
-       *(.initcallpresmp.init)
+       KEEP(*(.initcallpresmp.init))
        __presmp_initcall_end = .;
-       *(.initcall1.init)
+       KEEP(*(.initcall1.init))
        __initcall_end = .;
 
        *(.init.data)
@@ -243,10 +243,10 @@ SECTIONS
        *(.init.data.rel.*)
        . = ALIGN(4);
        __trampoline_rel_start = .;
-       *(.trampoline_rel)
+       KEEP(*(.trampoline_rel))
        __trampoline_rel_stop = .;
        __trampoline_seg_start = .;
-       *(.trampoline_seg)
+       KEEP(*(.trampoline_seg))
        __trampoline_seg_stop = .;
        /*
         * struct alt_inst entries. From the header (alternative.h):
@@ -255,21 +255,21 @@ SECTIONS
         */
        . = ALIGN(8);
         __alt_instructions = .;
-        *(.altinstructions)
+        KEEP(*(.altinstructions))
         __alt_instructions_end = .;
         . = ALIGN(4);
         __alt_call_sites_start = .;
-        *(.alt_call_sites)
+        KEEP(*(.alt_call_sites))
         __alt_call_sites_end = .;
 
        LOCK_PROFILE_DATA
 
        . = ALIGN(8);
        __ctors_start = .;
-       *(SORT_BY_INIT_PRIORITY(.init_array.*))
-       *(SORT_BY_INIT_PRIORITY(.ctors.*))
-       *(.init_array)
-       *(.ctors)
+       KEEP(*(SORT_BY_INIT_PRIORITY(.init_array.*)))
+       KEEP(*(SORT_BY_INIT_PRIORITY(.ctors.*)))
+       KEEP(*(.init_array))
+       KEEP(*(.ctors))
        __ctors_end = .;
   } PHDR(text)
 
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 401d5046f6..7e40a921a7 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -680,4 +680,13 @@ config PM_STATS
 	  Enable collection of performance management statistics to aid in
 	  analyzing and tuning power/performance characteristics of the system
 
+config GC_SECTIONS
+	bool "Garbage Collect Sections"
+	select CC_SPLIT_SECTIONS
+	help
+	  During final linking, garbage collect unused sections.  This will
+	  reduce the size of the final Xen binary
+
+	  Only supported for ELF/Multiboot xen/xen.gz, not EFI xen.efi.
+
 endmenu
diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
index 2d66d618b3..4703523cb2 100644
--- a/xen/include/xen/xen.lds.h
+++ b/xen/include/xen/xen.lds.h
@@ -144,46 +144,46 @@
   . = ALIGN(POINTER_ALIGN);  \
   DECL_SECTION(.adev.info) { \
       _asdevice = .;         \
-      *(.adev.info)          \
+      KEEP(*(.adev.info))    \
       _aedevice = .;         \
   } :text
 
 #define BUGFRAMES                               \
     __start_bug_frames_0 = .;                   \
-    *(.bug_frames.0)                            \
+    KEEP(*(.bug_frames.0))                      \
     __stop_bug_frames_0 = .;                    \
                                                 \
     __start_bug_frames_1 = .;                   \
-    *(.bug_frames.1)                            \
+    KEEP(*(.bug_frames.1))                      \
     __stop_bug_frames_1 = .;                    \
                                                 \
     __start_bug_frames_2 = .;                   \
-    *(.bug_frames.2)                            \
+    KEEP(*(.bug_frames.2))                      \
     __stop_bug_frames_2 = .;                    \
                                                 \
     __start_bug_frames_3 = .;                   \
-    *(.bug_frames.3)                            \
+    KEEP(*(.bug_frames.3))                      \
     __stop_bug_frames_3 = .;
 
 #define DT_DEV_INFO         \
   . = ALIGN(POINTER_ALIGN); \
   DECL_SECTION(.dev.info) { \
        _sdevice = .;        \
-       *(.dev.info)         \
+       KEEP(*(.dev.info))   \
        _edevice = .;        \
   } :text
 
 #define SCHEDULER_ARRAY              \
        . = ALIGN(8);                 \
        __start_schedulers_array = .; \
-       *(.data.schedulers)           \
+       KEEP(*(.data.schedulers))     \
        __end_schedulers_array = .;
 
 #ifdef CONFIG_HYPFS
 #define HYPFS_PARAM              \
        . = ALIGN(POINTER_ALIGN); \
        __paramhypfs_start = .;   \
-       *(.data.paramhypfs)       \
+       KEEP(*(.data.paramhypfs)) \
        __paramhypfs_end = .;
 #else
 #define HYPFS_PARAM
@@ -193,7 +193,7 @@
 #define LOCK_PROFILE_DATA        \
        . = ALIGN(POINTER_ALIGN); \
        __lock_profile_start = .; \
-       *(.lockprofile.data)      \
+       KEEP(*(.lockprofile.data))\
        __lock_profile_end = .;
 #else
 #define LOCK_PROFILE_DATA
@@ -213,7 +213,7 @@
 #define VPCI_ARRAY               \
        . = ALIGN(POINTER_ALIGN); \
        __start_vpci_array = .;   \
-       *(.data.rel.ro.vpci)      \
+       KEEP(*(.data.rel.ro.vpci))\
        __end_vpci_array = .;
 #else
 #define VPCI_ARRAY
-- 
2.52.0
Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Grygorii Strashko 8 hours ago 
Hi Jason,

On 09.12.25 23:47, Jason Andryuk wrote:
> Add a new Kconfig CONFIG_GC_SECTIONS to control linking with
> --gc-sections.  --gc-sections witll garbage collect unused sections.
> Combined with CONFIG_CC_SPLIT_SECTIONS, this will remove unreachable
> code and data.
> 
> Linker scripts need to add KEEP() assorted places to retain
> appropriate data - especially the arrays created by the linker script.
> 
> This has some affect, but it is inomplete.  In a test where memory_add()
> is unreachable, it is still included.  I'm not sure, but it seems that
> alternatives keep a relocation reference to it.
> 
> Only ELF xen is supported.  xen.efi fails to link with many undefined
> references when using --gc-sections.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> v1:
> Add Kconfig
> select CC_SPLIT_SECTIONS
> remove KEEP from .fixup
> Add KEEP to .text.entry.* (Only needed with Jan's "common: honor
> CONFIG_CC_SPLIT_SECTIONS also for assembly functions" ?)
> Add ARM, RISC-V and PPC
> 
> Pipeline passes:
> https://gitlab.com/xen-project/people/jandryuk-amd/xen/-/pipelines/2205223331
> 
> It defaults CONFIG_GC_SECTIONS=y and adds --print-gc-sections
> 

[...]

> ---
>   xen/Makefile              |  3 +++
>   xen/arch/arm/Makefile     |  6 +++---
>   xen/arch/arm/xen.lds.S    | 22 +++++++++++-----------
>   xen/arch/ppc/Makefile     |  6 +++---
>   xen/arch/ppc/xen.lds.S    | 14 +++++++-------
>   xen/arch/riscv/Makefile   |  6 +++---
>   xen/arch/riscv/xen.lds.S  | 14 +++++++-------
>   xen/arch/x86/Makefile     |  6 +++---
>   xen/arch/x86/xen.lds.S    | 34 +++++++++++++++++-----------------
>   xen/common/Kconfig        |  9 +++++++++
>   xen/include/xen/xen.lds.h | 20 ++++++++++----------
>   11 files changed, 76 insertions(+), 64 deletions(-)
> 
> diff --git a/xen/Makefile b/xen/Makefile
> index e6cf287425..aeb5dcf2ee 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -469,10 +469,13 @@ all-symbols-$(CONFIG_FAST_SYMBOL_LOOKUP) += --sort-by-name
>   
>   include $(srctree)/arch/$(SRCARCH)/arch.mk
>   
> +XEN_FINAL_LDFLAGS-$(CONFIG_GC_SECTIONS) := --gc-sections
> +
>   # define new variables to avoid the ones defined in Config.mk
>   export XEN_CFLAGS := $(CFLAGS)
>   export XEN_AFLAGS := $(AFLAGS)
>   export XEN_LDFLAGS := $(LDFLAGS)
> +export XEN_FINAL_LDFLAGS := $(LDFLAGS) $(XEN_FINAL_LDFLAGS-y)
>   export CFLAGS_UBSAN
>   
>   endif # need-config

[...]

> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index 2d5f1c516d..178af612a2 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -63,7 +63,7 @@ SECTIONS
>   
>          . = ALIGN(4);
>          __proc_info_start = .;
> -       *(.proc.info)
> +       KEEP(*(.proc.info))
>          __proc_info_end = .;
>     } :text
>   
> @@ -103,7 +103,7 @@ SECTIONS
>     . = ALIGN(8);
>     .arch.info : {
>         _splatform = .;
> -      *(.arch.info)
> +      KEEP(*(.arch.info))
>         _eplatform = .;
>     } :text
>   
> @@ -116,7 +116,7 @@ SECTIONS
>     . = ALIGN(8);
>     .teemediator.info : {
>         _steemediator = .;
> -      *(.teemediator.info)
> +      KEEP(*(.teemediator.info))
>         _eteemediator = .;
>     } :text
>   
> @@ -127,7 +127,7 @@ SECTIONS
>          *(.init.text)
>          _einittext = .;
>          . = ALIGN(PAGE_SIZE);        /* Avoid mapping alt insns executable */
> -       *(.altinstr_replacement)
> +       KEEP(*(.altinstr_replacement))
>     } :text
>     . = ALIGN(PAGE_SIZE);
>     __init_data_begin = .;
> @@ -137,18 +137,18 @@ SECTIONS
>   
>          . = ALIGN(POINTER_ALIGN);
>          __setup_start = .;
> -       *(.init.setup)
> +       KEEP(*(.init.setup))
>          __setup_end = .;
>   
>          __initcall_start = .;
> -       *(.initcallpresmp.init)
> +       KEEP(*(.initcallpresmp.init))
>          __presmp_initcall_end = .;
> -       *(.initcall1.init)
> +       KEEP(*(.initcall1.init))
>          __initcall_end = .;

Wouldn't it be reasonable to do the same here for "initcall/setup" as was done for
"schedulers_array"?


[...]

-- 
Best regards,
-grygorii
Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jason Andryuk 3 hours ago 
On 2025-12-12 05:42, Grygorii Strashko wrote:
> 
> Wouldn't it be reasonable to do the same here for "initcall/setup" as 
> was done for
> "schedulers_array"?

Hi Grygorii,

Yes, I think you are correct.

Thanks,
Jason
Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Anthony PERARD 2 days, 5 hours ago 
On Tue, Dec 09, 2025 at 04:47:28PM -0500, Jason Andryuk wrote:
> diff --git a/xen/Makefile b/xen/Makefile
> index e6cf287425..aeb5dcf2ee 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -469,10 +469,13 @@ all-symbols-$(CONFIG_FAST_SYMBOL_LOOKUP) += --sort-by-name
>  
>  include $(srctree)/arch/$(SRCARCH)/arch.mk
>  
> +XEN_FINAL_LDFLAGS-$(CONFIG_GC_SECTIONS) := --gc-sections

Is there a good reason to add this flags after the arch-specific
makefiles? If not, could you move that just before, and right after the
definition of "$(all-symbols)" as it's a variable that is used in the
same phase of the build. (With Jan's other feedback)

>  # define new variables to avoid the ones defined in Config.mk
>  export XEN_CFLAGS := $(CFLAGS)
>  export XEN_AFLAGS := $(AFLAGS)
>  export XEN_LDFLAGS := $(LDFLAGS)
> +export XEN_FINAL_LDFLAGS := $(LDFLAGS) $(XEN_FINAL_LDFLAGS-y)

"FINAL" isn't very descriptive. A completely wrong interpretation might
be that we should use the "final" variable instead of "XEN_LDFLAGS". How
about a name that describe where this set of flags is going to be used,
like "XEN_LDFLAGS_xen_syms" (which unfortunately doesn't exactly fit
with x86 xen.efi target), or maybe suffix it with "_target" or just
"_xen"? (In Linux build system, they use "LDFLAGS_vmlinux", but I don't
know what would be the equivalent of "vmlinux" in our build system.)

The prefix "XEN_" is used as namespace, with one reason described in the
comment.

Also, could you use $(XEN_LDFLAGS) instead of $(LDFLAGS) ?

Cheers,

-- 
Anthony PERARD
Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jason Andryuk 2 days, 2 hours ago 
On 2025-12-10 09:40, Anthony PERARD wrote:
> On Tue, Dec 09, 2025 at 04:47:28PM -0500, Jason Andryuk wrote:
>> diff --git a/xen/Makefile b/xen/Makefile
>> index e6cf287425..aeb5dcf2ee 100644
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -469,10 +469,13 @@ all-symbols-$(CONFIG_FAST_SYMBOL_LOOKUP) += --sort-by-name
>>   
>>   include $(srctree)/arch/$(SRCARCH)/arch.mk
>>   
>> +XEN_FINAL_LDFLAGS-$(CONFIG_GC_SECTIONS) := --gc-sections
> 
> Is there a good reason to add this flags after the arch-specific
> makefiles? If not, could you move that just before, and right after the
> definition of "$(all-symbols)" as it's a variable that is used in the
> same phase of the build. (With Jan's other feedback)

No, there is no reason for its location.  I can move it.

>>   # define new variables to avoid the ones defined in Config.mk
>>   export XEN_CFLAGS := $(CFLAGS)
>>   export XEN_AFLAGS := $(AFLAGS)
>>   export XEN_LDFLAGS := $(LDFLAGS)
>> +export XEN_FINAL_LDFLAGS := $(LDFLAGS) $(XEN_FINAL_LDFLAGS-y)
> 
> "FINAL" isn't very descriptive. A completely wrong interpretation might
> be that we should use the "final" variable instead of "XEN_LDFLAGS". How
> about a name that describe where this set of flags is going to be used,
> like "XEN_LDFLAGS_xen_syms" (which unfortunately doesn't exactly fit
> with x86 xen.efi target), or maybe suffix it with "_target" or just
> "_xen"? (In Linux build system, they use "LDFLAGS_vmlinux", but I don't
> know what would be the equivalent of "vmlinux" in our build system.)

I plan to use "_xen" unless anyone objects.  "_xen_lds" could be another 
option, but again that doesn't match efi.lds.

Hmmm - maybe my earlier xen.efi link failure was from efi.lds needing to 
be updated.

> 
> The prefix "XEN_" is used as namespace, with one reason described in the
> comment.

I'm not sure what you mean here.  Are you just pointing out that XEN_ is 
the prefix and not the target?

> Also, could you use $(XEN_LDFLAGS) instead of $(LDFLAGS) ?

Yes.

Thanks,
Jason
Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jan Beulich 1 day, 11 hours ago 
On 10.12.2025 18:08, Jason Andryuk wrote:
> On 2025-12-10 09:40, Anthony PERARD wrote:
>> On Tue, Dec 09, 2025 at 04:47:28PM -0500, Jason Andryuk wrote:
>>> diff --git a/xen/Makefile b/xen/Makefile
>>> index e6cf287425..aeb5dcf2ee 100644
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -469,10 +469,13 @@ all-symbols-$(CONFIG_FAST_SYMBOL_LOOKUP) += --sort-by-name
>>>   
>>>   include $(srctree)/arch/$(SRCARCH)/arch.mk
>>>   
>>> +XEN_FINAL_LDFLAGS-$(CONFIG_GC_SECTIONS) := --gc-sections
>>
>> Is there a good reason to add this flags after the arch-specific
>> makefiles? If not, could you move that just before, and right after the
>> definition of "$(all-symbols)" as it's a variable that is used in the
>> same phase of the build. (With Jan's other feedback)
> 
> No, there is no reason for its location.  I can move it.
> 
>>>   # define new variables to avoid the ones defined in Config.mk
>>>   export XEN_CFLAGS := $(CFLAGS)
>>>   export XEN_AFLAGS := $(AFLAGS)
>>>   export XEN_LDFLAGS := $(LDFLAGS)
>>> +export XEN_FINAL_LDFLAGS := $(LDFLAGS) $(XEN_FINAL_LDFLAGS-y)
>>
>> "FINAL" isn't very descriptive. A completely wrong interpretation might
>> be that we should use the "final" variable instead of "XEN_LDFLAGS". How
>> about a name that describe where this set of flags is going to be used,
>> like "XEN_LDFLAGS_xen_syms" (which unfortunately doesn't exactly fit
>> with x86 xen.efi target), or maybe suffix it with "_target" or just
>> "_xen"? (In Linux build system, they use "LDFLAGS_vmlinux", but I don't
>> know what would be the equivalent of "vmlinux" in our build system.)
> 
> I plan to use "_xen" unless anyone objects.  "_xen_lds" could be another 
> option, but again that doesn't match efi.lds.

_lds would also be wrong - that rather refers to the linker script than the
final binary linking of which these flags influence.

Jan
Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jan Beulich 2 days, 11 hours ago 
On 09.12.2025 22:47, Jason Andryuk wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -469,10 +469,13 @@ all-symbols-$(CONFIG_FAST_SYMBOL_LOOKUP) += --sort-by-name
>  
>  include $(srctree)/arch/$(SRCARCH)/arch.mk
>  
> +XEN_FINAL_LDFLAGS-$(CONFIG_GC_SECTIONS) := --gc-sections
> +
>  # define new variables to avoid the ones defined in Config.mk
>  export XEN_CFLAGS := $(CFLAGS)
>  export XEN_AFLAGS := $(AFLAGS)
>  export XEN_LDFLAGS := $(LDFLAGS)
> +export XEN_FINAL_LDFLAGS := $(LDFLAGS) $(XEN_FINAL_LDFLAGS-y)
>  export CFLAGS_UBSAN

Imo the introduction of XEN_FINAL_LDFLAGS would best be a separate, prereq
change. That could then also go in already while the KEEP() issue is still
being sorted.

The appending of --gc-sections should then also be truly appending, so make
sure that e.g. anything set by arch/$(SRCARCH)/arch.mk wouldn't be purged
again. IOW I think ahead of that include we want

XEN_FINAL_LDFLAGS-y :=

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -680,4 +680,13 @@ config PM_STATS
>  	  Enable collection of performance management statistics to aid in
>  	  analyzing and tuning power/performance characteristics of the system
>  
> +config GC_SECTIONS
> +	bool "Garbage Collect Sections"
> +	select CC_SPLIT_SECTIONS
> +	help
> +	  During final linking, garbage collect unused sections.  This will
> +	  reduce the size of the final Xen binary
> +
> +	  Only supported for ELF/Multiboot xen/xen.gz, not EFI xen.efi.

This last sentence is x86-centric, which it shouldn't be here (or it should
say that this is an x86-only aspect).

I also wonder whether this wouldn't better live next to CC_SPLIT_SECTIONS.

Jan
Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jason Andryuk 2 days, 2 hours ago 
On 2025-12-10 03:17, Jan Beulich wrote:
> On 09.12.2025 22:47, Jason Andryuk wrote:
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -469,10 +469,13 @@ all-symbols-$(CONFIG_FAST_SYMBOL_LOOKUP) += --sort-by-name
>>   
>>   include $(srctree)/arch/$(SRCARCH)/arch.mk
>>   
>> +XEN_FINAL_LDFLAGS-$(CONFIG_GC_SECTIONS) := --gc-sections
>> +
>>   # define new variables to avoid the ones defined in Config.mk
>>   export XEN_CFLAGS := $(CFLAGS)
>>   export XEN_AFLAGS := $(AFLAGS)
>>   export XEN_LDFLAGS := $(LDFLAGS)
>> +export XEN_FINAL_LDFLAGS := $(LDFLAGS) $(XEN_FINAL_LDFLAGS-y)
>>   export CFLAGS_UBSAN
> 
> Imo the introduction of XEN_FINAL_LDFLAGS would best be a separate, prereq
> change. That could then also go in already while the KEEP() issue is still
> being sorted.
> 
> The appending of --gc-sections should then also be truly appending, so make
> sure that e.g. anything set by arch/$(SRCARCH)/arch.mk wouldn't be purged
> again. IOW I think ahead of that include we want
> 
> XEN_FINAL_LDFLAGS-y :=

This all sounds fine, though with Anthony's response the variable name 
may change.

> 
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -680,4 +680,13 @@ config PM_STATS
>>   	  Enable collection of performance management statistics to aid in
>>   	  analyzing and tuning power/performance characteristics of the system
>>   
>> +config GC_SECTIONS
>> +	bool "Garbage Collect Sections"
>> +	select CC_SPLIT_SECTIONS
>> +	help
>> +	  During final linking, garbage collect unused sections.  This will
>> +	  reduce the size of the final Xen binary
>> +
>> +	  Only supported for ELF/Multiboot xen/xen.gz, not EFI xen.efi.
> 
> This last sentence is x86-centric, which it shouldn't be here (or it should
> say that this is an x86-only aspect).
> 
> I also wonder whether this wouldn't better live next to CC_SPLIT_SECTIONS.

If I put it immediately after CC_SPLIT_SECTIONS, menuconfig puts it as a 
top level option:

│ │    [*] Garbage Collect Sections
│ │        Architecture Features  --->
│ │        Common Features  --->

I thought Common Features was a better place for it.

Also, I think it should probably gain " if EXPERT" as well.

Thanks,
Jason
Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jan Beulich 1 day, 11 hours ago 
On 10.12.2025 17:57, Jason Andryuk wrote:
> On 2025-12-10 03:17, Jan Beulich wrote:
>> On 09.12.2025 22:47, Jason Andryuk wrote:
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -680,4 +680,13 @@ config PM_STATS
>>>   	  Enable collection of performance management statistics to aid in
>>>   	  analyzing and tuning power/performance characteristics of the system
>>>   
>>> +config GC_SECTIONS
>>> +	bool "Garbage Collect Sections"
>>> +	select CC_SPLIT_SECTIONS
>>> +	help
>>> +	  During final linking, garbage collect unused sections.  This will
>>> +	  reduce the size of the final Xen binary
>>> +
>>> +	  Only supported for ELF/Multiboot xen/xen.gz, not EFI xen.efi.
>>
>> This last sentence is x86-centric, which it shouldn't be here (or it should
>> say that this is an x86-only aspect).
>>
>> I also wonder whether this wouldn't better live next to CC_SPLIT_SECTIONS.
> 
> If I put it immediately after CC_SPLIT_SECTIONS, menuconfig puts it as a 
> top level option:
> 
> │ │    [*] Garbage Collect Sections
> │ │        Architecture Features  --->
> │ │        Common Features  --->
> 
> I thought Common Features was a better place for it.

Oh, right. I didn't recall CC_SPLIT_SECTIONS wouldn't even have a prompt. I
wonder if it shouldn't gain one, move somewhere here, and then this new
option could be placed next to it.

> Also, I think it should probably gain " if EXPERT" as well.

Not sure there. If this works reliably, I don't see why it would take an
expert to enable. Eventually, after all, this may want to be the default.

Jan
KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Andrew Cooper 2 days, 2 hours ago 
On 09/12/2025 9:47 pm, Jason Andryuk wrote:
>         . = ALIGN(4);
>         __alt_instructions = .;
> -       *(.altinstructions)
> +       KEEP(*(.altinstructions))
>         __alt_instructions_end = .;

Thinking about this, what we need is for there to be a reference tied to
the source location, referencing the replacement metadata and
replacement instructions.

Looking at https://maskray.me/blog/2021-02-28-linker-garbage-collection
might be able to do this with .reloc of type none.  In fact,
BFD_RELOC_NONE seems to have been invented for precisely this purpose.

This means that if and only if the source function gets included, so
does the metadata and replacement.

~Andrew
Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jason Andryuk 1 day, 18 hours ago 
On 2025-12-10 11:55, Andrew Cooper wrote:
> On 09/12/2025 9:47 pm, Jason Andryuk wrote:
>>          . = ALIGN(4);
>>          __alt_instructions = .;
>> -       *(.altinstructions)
>> +       KEEP(*(.altinstructions))
>>          __alt_instructions_end = .;
> 
> Thinking about this, what we need is for there to be a reference tied to
> the source location, referencing the replacement metadata and
> replacement instructions.
> 
> Looking at https://maskray.me/blog/2021-02-28-linker-garbage-collection
> might be able to do this with .reloc of type none.  In fact,
> BFD_RELOC_NONE seems to have been invented for precisely this purpose.
> 
> This means that if and only if the source function gets included, so
> does the metadata and replacement.

With Jan's -Wa,--sectname-subst changes added to CFLAGS, this seems to 
work somewhat.  I'm trying to make the ALTERNATIVE place relocations 
against the .altinstructions.%%S and .altinstr_replacement sections:

diff --git c/xen/arch/x86/include/asm/alternative.h 
w/xen/arch/x86/include/asm/alternative.h
index 18109e3dc5..e871bfc04c 100644
--- c/xen/arch/x86/include/asm/alternative.h
+++ w/xen/arch/x86/include/asm/alternative.h
@@ -90,18 +90,25 @@ extern void alternative_instructions(void);
  /* alternative assembly primitive: */
  #define ALTERNATIVE(oldinstr, newinstr, feature)                      \
          OLDINSTR_1(oldinstr, 1)                                       \
-        ".pushsection .altinstructions, \"a\", @progbits\n"           \
+        ".reloc ., BFD_RELOC_NONE, 567f\n"                            \
+        ".reloc ., BFD_RELOC_NONE, 568f\n"                            \
+        ".pushsection .altinstructions.%%S, \"a\", @progbits\n"       \
+        "567:\n"                                                      \
          ALTINSTR_ENTRY(feature, 1)                                    \
          ".section .discard, \"a\", @progbits\n"                       \
          ".byte " alt_total_len "\n" /* total_len <= 255 */            \
          DISCARD_ENTRY(1)                                              \
          ".section .altinstr_replacement, \"ax\", @progbits\n"         \
+        "568:\n"                                                      \
          ALTINSTR_REPLACEMENT(newinstr, 1)                             \
          ".popsection\n"

--print-gc-sections shows a few .altinstructions.%%S dropped - as an 
example:

ld: removing unused section '.text.reserve_lapic_nmi' in file 'prelink.o'
ld: removing unused section '.text.release_lapic_nmi' in file 'prelink.o'
...
ld: removing unused section '.altinstructions..text.reserve_lapic_nmi' 
in file 'prelink.o'
ld: removing unused section '.altinstructions..text.release_lapic_nmi' 
in file 'prelink.o'

The full list is below.

Unfortunately, EFI doesn't like it with ~14000 lines of:
ld: error: 0-bit reloc in dll

Also, my test of removing the path to memory_add() still doesn't drop 
memory_add().

There is still something wrong where I get a fault in some shadow code. 
I'm still investigating that.

Regards,
Jason


ld: removing unused section '.text.__bitmap_full' in file 'prelink.o'
ld: removing unused section '.text.__bitmap_xor' in file 'prelink.o'
ld: removing unused section '.text.__bitmap_set' in file 'prelink.o'
ld: removing unused section '.text.__bitmap_clear' in file 'prelink.o'
ld: removing unused section '.text.bitmap_find_free_region' in file 
'prelink.o'
ld: removing unused section '.text.bitmap_release_region' in file 
'prelink.o'
ld: removing unused section '.text.safe_copy_string_from_guest' in file 
'prelink.o'
ld: removing unused section '.text.domain_has_ioreq_server' in file 
'prelink.o'
ld: removing unused section '.text.compat_kexec_op' in file 'prelink.o'
ld: removing unused section '.text.in_atomic' in file 'prelink.o'
ld: removing unused section '.text.radix_tree_next_hole' in file 'prelink.o'
ld: removing unused section '.text.radix_tree_prev_hole' in file 'prelink.o'
ld: removing unused section '.text.radix_tree_gang_lookup_slot' in file 
'prelink.o'
ld: removing unused section '.text._nrspin_trylock' in file 'prelink.o'
ld: removing unused section '.text.xen_compile_host' in file 'prelink.o'
ld: removing unused section '.text.vm_event_cancel_slot' in file 'prelink.o'
ld: removing unused section '.text.vscnprintf' in file 'prelink.o'
ld: removing unused section '.text.wake_up_one' in file 'prelink.o'
ld: removing unused section '.text.xmem_pool_get_used_size' in file 
'prelink.o'
ld: removing unused section '.text.xmem_pool_get_total_size' in file 
'prelink.o'
ld: removing unused section '.text.xmem_pool_destroy' in file 'prelink.o'
ld: removing unused section '.text.xmem_pool_maxalloc' in file 'prelink.o'
ld: removing unused section '.text.xlat_start_info' in file 'prelink.o'
ld: removing unused section '.text.elf_sym_by_name' in file 'prelink.o'
ld: removing unused section '.text.elf_sym_by_index' in file 'prelink.o'
ld: removing unused section '.text.elf_get_ptr' in file 'prelink.o'
ld: removing unused section '.text.elf_lookup_addr' in file 'prelink.o'
ld: removing unused section '.text.serial_vuart_info' in file 'prelink.o'
ld: removing unused section '.text.pci_find_next_cap' in file 'prelink.o'
ld: removing unused section '.text.free_hvm_irq_dpci' in file 'prelink.o'
ld: removing unused section '.text.iommu_has_feature' in file 'prelink.o'
ld: removing unused section '.text.__erst_get_next_record_id' in file 
'prelink.o'
ld: removing unused section '.text.__erst_read' in file 'prelink.o'
ld: removing unused section '.text.erst_get_record_count' in file 
'prelink.o'
ld: removing unused section '.text.erst_get_next_record_id' in file 
'prelink.o'
ld: removing unused section '.text.erst_read' in file 'prelink.o'
ld: removing unused section '.text.erst_read_next' in file 'prelink.o'
ld: removing unused section '.text.erst_clear' in file 'prelink.o'
ld: removing unused section '.text.mce_barrier_init' in file 'prelink.o'
ld: removing unused section '.text.mce_barrier_dec' in file 'prelink.o'
ld: removing unused section '.text.mce_barrier' in file 'prelink.o'
ld: removing unused section '.text.apei_read_mce' in file 'prelink.o'
ld: removing unused section '.text.apei_check_mce' in file 'prelink.o'
ld: removing unused section '.text.apei_clear_mce' in file 'prelink.o'
ld: removing unused section '.text.efi_halt_system' in file 'prelink.o'
ld: removing unused section '.text.get_vvmcs_virtual_safe' in file 
'prelink.o'
ld: removing unused section '.text.get_vvmcs_real_safe' in file 'prelink.o'
ld: removing unused section '.text.set_vvmcs_real' in file 'prelink.o'
ld: removing unused section '.text.set_vvmcs_virtual_safe' in file 
'prelink.o'
ld: removing unused section '.text.set_vvmcs_real_safe' in file 'prelink.o'
ld: removing unused section '.init.text.early_page_fault' in file 
'prelink.o'
ld: removing unused section '.text.domain_set_alloc_bitsize' in file 
'prelink.o'
ld: removing unused section '.text.reserve_lapic_nmi' in file 'prelink.o'
ld: removing unused section '.text.release_lapic_nmi' in file 'prelink.o'
ld: removing unused section '.text.watchdog_enabled' in file 'prelink.o'
ld: removing unused section '.text.unset_nmi_callback' in file 'prelink.o'
ld: removing unused section '.text.sha2_256_init' in file 'prelink.o'
ld: removing unused section '.text.xxh64_copy_state' in file 'prelink.o'
ld: removing unused section '.text.xxh64' in file 'prelink.o'
ld: removing unused section '.discard' in file 'prelink.o'
ld: removing unused section 
'.altinstructions..text.safe_copy_string_from_guest' in file 'prelink.o'
ld: removing unused section '.rodata.xen_compile_host.str1.1' in file 
'prelink.o'
ld: removing unused section 
'.altinstructions..text.vm_event_cancel_slot' in file 'prelink.o'
ld: removing unused section '.rodata.xmem_pool_destroy.str1.8' in file 
'prelink.o'
ld: removing unused section '.altinstructions..text.xmem_pool_destroy' 
in file 'prelink.o'
ld: removing unused section '.rodata.elf_lookup_addr.str1.1' in file 
'prelink.o'
ld: removing unused section '.altinstructions..text.iommu_has_feature' 
in file 'prelink.o'
ld: removing unused section '.rodata.__erst_read.str1.8' in file 'prelink.o'
ld: removing unused section 
'.altinstructions..text.erst_get_record_count' in file 'prelink.o'
ld: removing unused section 
'.altinstructions..text.erst_get_next_record_id' in file 'prelink.o'
ld: removing unused section '.altinstructions..text.erst_read' in file 
'prelink.o'
ld: removing unused section '.altinstructions..text.erst_read_next' in 
file 'prelink.o'
ld: removing unused section '.altinstructions..text.erst_clear' in file 
'prelink.o'
ld: removing unused section '.rodata.apei_read_mce.str1.8' in file 
'prelink.o'
ld: removing unused section '.rodata.efi_halt_system.str1.8' in file 
'prelink.o'
ld: removing unused section '.rodata.play_dead.str1.1' in file 'prelink.o'
ld: removing unused section '.altinstructions..text.reserve_lapic_nmi' 
in file 'prelink.o'
ld: removing unused section '.altinstructions..text.release_lapic_nmi' 
in file 'prelink.o'
ld: removing unused section '.data.rel.ro.local.fetch_type_names' in 
file 'prelink.o'
ld: removing unused section '.data.lapic_nmi_owner_lock' in file 'prelink.o'
Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jan Beulich 1 day, 11 hours ago 
On 11.12.2025 02:28, Jason Andryuk wrote:
> On 2025-12-10 11:55, Andrew Cooper wrote:
>> On 09/12/2025 9:47 pm, Jason Andryuk wrote:
>>>          . = ALIGN(4);
>>>          __alt_instructions = .;
>>> -       *(.altinstructions)
>>> +       KEEP(*(.altinstructions))
>>>          __alt_instructions_end = .;
>>
>> Thinking about this, what we need is for there to be a reference tied to
>> the source location, referencing the replacement metadata and
>> replacement instructions.
>>
>> Looking at https://maskray.me/blog/2021-02-28-linker-garbage-collection
>> might be able to do this with .reloc of type none.  In fact,
>> BFD_RELOC_NONE seems to have been invented for precisely this purpose.
>>
>> This means that if and only if the source function gets included, so
>> does the metadata and replacement.
> 
> With Jan's -Wa,--sectname-subst changes added to CFLAGS, this seems to 
> work somewhat.  I'm trying to make the ALTERNATIVE place relocations 
> against the .altinstructions.%%S and .altinstr_replacement sections:
> 
> diff --git c/xen/arch/x86/include/asm/alternative.h 
> w/xen/arch/x86/include/asm/alternative.h
> index 18109e3dc5..e871bfc04c 100644
> --- c/xen/arch/x86/include/asm/alternative.h
> +++ w/xen/arch/x86/include/asm/alternative.h
> @@ -90,18 +90,25 @@ extern void alternative_instructions(void);
>   /* alternative assembly primitive: */
>   #define ALTERNATIVE(oldinstr, newinstr, feature)                      \
>           OLDINSTR_1(oldinstr, 1)                                       \
> -        ".pushsection .altinstructions, \"a\", @progbits\n"           \
> +        ".reloc ., BFD_RELOC_NONE, 567f\n"                            \
> +        ".reloc ., BFD_RELOC_NONE, 568f\n"                            \

When I saw this, ...

> +        ".pushsection .altinstructions.%%S, \"a\", @progbits\n"       \
> +        "567:\n"                                                      \
>           ALTINSTR_ENTRY(feature, 1)                                    \
>           ".section .discard, \"a\", @progbits\n"                       \
>           ".byte " alt_total_len "\n" /* total_len <= 255 */            \
>           DISCARD_ENTRY(1)                                              \
>           ".section .altinstr_replacement, \"ax\", @progbits\n"         \
> +        "568:\n"                                                      \
>           ALTINSTR_REPLACEMENT(newinstr, 1)                             \
>           ".popsection\n"
> 
> --print-gc-sections shows a few .altinstructions.%%S dropped - as an 
> example:
> 
> ld: removing unused section '.text.reserve_lapic_nmi' in file 'prelink.o'
> ld: removing unused section '.text.release_lapic_nmi' in file 'prelink.o'
> ...
> ld: removing unused section '.altinstructions..text.reserve_lapic_nmi' 
> in file 'prelink.o'
> ld: removing unused section '.altinstructions..text.release_lapic_nmi' 
> in file 'prelink.o'
> 
> The full list is below.
> 
> Unfortunately, EFI doesn't like it with ~14000 lines of:
> ld: error: 0-bit reloc in dll

... I immediately wanted to mention this. Another of the things on my binutils
todo list.

Jan
Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jason Andryuk 1 day, 17 hours ago 
On 2025-12-10 20:28, Jason Andryuk wrote:
> Also, my test of removing the path to memory_add() still doesn't drop 
> memory_add().

Splitting .altinstr_replacements and .bug_frame.* seems to be needed to 
drop memory_add().

Regards,
Jason
Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Andrew Cooper 1 day, 16 hours ago 
On 11/12/2025 1:28 am, Jason Andryuk wrote:
> On 2025-12-10 11:55, Andrew Cooper wrote:
>> On 09/12/2025 9:47 pm, Jason Andryuk wrote:
>>>          . = ALIGN(4);
>>>          __alt_instructions = .;
>>> -       *(.altinstructions)
>>> +       KEEP(*(.altinstructions))
>>>          __alt_instructions_end = .;
>>
>> Thinking about this, what we need is for there to be a reference tied to
>> the source location, referencing the replacement metadata and
>> replacement instructions.
>>
>> Looking at https://maskray.me/blog/2021-02-28-linker-garbage-collection
>> might be able to do this with .reloc of type none.  In fact,
>> BFD_RELOC_NONE seems to have been invented for precisely this purpose.
>>
>> This means that if and only if the source function gets included, so
>> does the metadata and replacement.
>
> With Jan's -Wa,--sectname-subst changes added to CFLAGS, this seems to
> work somewhat.  I'm trying to make the ALTERNATIVE place relocations
> against the .altinstructions.%%S and .altinstr_replacement sections:
>
> diff --git c/xen/arch/x86/include/asm/alternative.h
> w/xen/arch/x86/include/asm/alternative.h
> index 18109e3dc5..e871bfc04c 100644
> --- c/xen/arch/x86/include/asm/alternative.h
> +++ w/xen/arch/x86/include/asm/alternative.h
> @@ -90,18 +90,25 @@ extern void alternative_instructions(void);
>  /* alternative assembly primitive: */
>  #define ALTERNATIVE(oldinstr, newinstr, feature)                      \
>          OLDINSTR_1(oldinstr, 1)                                       \
> -        ".pushsection .altinstructions, \"a\", @progbits\n"           \
> +        ".reloc ., BFD_RELOC_NONE, 567f\n"                            \
> +        ".reloc ., BFD_RELOC_NONE, 568f\n"                            \
> +        ".pushsection .altinstructions.%%S, \"a\", @progbits\n"       \
> +        "567:\n"                                                      \
>          ALTINSTR_ENTRY(feature, 1)                                    \
>          ".section .discard, \"a\", @progbits\n"                       \
>          ".byte " alt_total_len "\n" /* total_len <= 255 */            \
>          DISCARD_ENTRY(1)                                              \
>          ".section .altinstr_replacement, \"ax\", @progbits\n"         \
> +        "568:\n"                                                      \
>          ALTINSTR_REPLACEMENT(newinstr, 1)                             \
>          ".popsection\n"
>

There's already a symbol for the start of the replacement.  We only need
to introduce a symbol for the metadata.  Try something like this:

diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h
index 18109e3dc594..a1295ed816f5 100644
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -55,6 +55,10 @@ extern void alternative_instructions(void);
 
 #define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE("("a") < ("b")")")))"
 
+#define REF(num)                                        \
+    ".reloc ., BFD_RELOC_NONE, .LXEN%=_alt" #num "\n\t" \
+    ".reloc ., BFD_RELOC_NONE, "alt_repl_s(num)  "\n\t"
+
 #define OLDINSTR(oldinstr, padding)                              \
     ".LXEN%=_orig_s:\n\t" oldinstr "\n .LXEN%=_orig_e:\n\t"      \
     ".LXEN%=_diff = " padding "\n\t"                             \
@@ -62,17 +66,21 @@ extern void alternative_instructions(void);
     ".LXEN%=_orig_p:\n\t"
 
 #define OLDINSTR_1(oldinstr, n1)                                 \
-    OLDINSTR(oldinstr, alt_repl_len(n1) "-" alt_orig_len)
+    OLDINSTR(oldinstr, alt_repl_len(n1) "-" alt_orig_len)        \
+    REF(n1)
 
 #define OLDINSTR_2(oldinstr, n1, n2)                             \
     OLDINSTR(oldinstr,                                           \
              as_max(alt_repl_len(n1),                            \
-                    alt_repl_len(n2)) "-" alt_orig_len)
+                    alt_repl_len(n2)) "-" alt_orig_len)          \
+    REF(n1)                                                      \
+    REF(n2)
 
 #define ALTINSTR_ENTRY(feature, num)                                    \
         " .if (" STR(feature & ~ALT_FLAG_NOT) ") >= " STR(NCAPINTS * 32) "\n" \
         " .error \"alternative feature outside of featureset range\"\n" \
         " .endif\n"                                                     \
+        ".LXEN%=_alt" #num ":\n"                                        \
         " .long .LXEN%=_orig_s - .\n"             /* label           */ \
         " .long " alt_repl_s(num)" - .\n"         /* new instruction */ \
         " .word " STR(feature) "\n"               /* feature bit     */ \


which also avoids the timebomb of using blind numbers and hoping for no
collision.

In principle we should reference the discard entry too, as that's the
(very obscure) build assertion that we got the lengths correct, but

`.discard' referenced in section `.text' of prelink.o: defined in discarded section `.discard' of prelink.o


gets in the way.  I'm really not sure what to do here.

> --print-gc-sections shows a few .altinstructions.%%S dropped - as an
> example:
>
> ld: removing unused section '.text.reserve_lapic_nmi' in file 'prelink.o'
> ld: removing unused section '.text.release_lapic_nmi' in file 'prelink.o'
> ...
> ld: removing unused section '.altinstructions..text.reserve_lapic_nmi'
> in file 'prelink.o'
> ld: removing unused section '.altinstructions..text.release_lapic_nmi'
> in file 'prelink.o'

Excellent.  This shows we're making progress.

>
> The full list is below.
>
> Unfortunately, EFI doesn't like it with ~14000 lines of:
> ld: error: 0-bit reloc in dll

Yeah, I found that too.  And because of the way we derive xen.efi from
prelink.o, we can't simply exclude these .reloc's in the xen.efi case.

>
> Also, my test of removing the path to memory_add() still doesn't drop
> memory_add().

Hmm.  There's nothing obvious I can see in the generated .s that ought
to keep it referenced.

>
> There is still something wrong where I get a fault in some shadow
> code. I'm still investigating that.

The hunk above builds for me, but I'm not using %%S yet.  The shadow
code has multi.c build 3 times and linked together, so you may need to
account for GUEST_PAGING_LEVELS specially.

~Andrew
Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jason Andryuk 4 hours ago 
On 2025-12-10 21:47, Andrew Cooper wrote:
> On 11/12/2025 1:28 am, Jason Andryuk wrote:

>> +        "567:\n"                                                      \

> which also avoids the timebomb of using blind numbers and hoping for no
> collision.

Timebomb?  The bomb already went off!  That's why it's not 1.  :(

Regards,
Jason
Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jan Beulich 1 day, 11 hours ago 
On 11.12.2025 03:47, Andrew Cooper wrote:
> On 11/12/2025 1:28 am, Jason Andryuk wrote:
>> On 2025-12-10 11:55, Andrew Cooper wrote:
>>> On 09/12/2025 9:47 pm, Jason Andryuk wrote:
>>>>          . = ALIGN(4);
>>>>          __alt_instructions = .;
>>>> -       *(.altinstructions)
>>>> +       KEEP(*(.altinstructions))
>>>>          __alt_instructions_end = .;
>>>
>>> Thinking about this, what we need is for there to be a reference tied to
>>> the source location, referencing the replacement metadata and
>>> replacement instructions.
>>>
>>> Looking at https://maskray.me/blog/2021-02-28-linker-garbage-collection
>>> might be able to do this with .reloc of type none.  In fact,
>>> BFD_RELOC_NONE seems to have been invented for precisely this purpose.
>>>
>>> This means that if and only if the source function gets included, so
>>> does the metadata and replacement.
>>
>> With Jan's -Wa,--sectname-subst changes added to CFLAGS, this seems to
>> work somewhat.  I'm trying to make the ALTERNATIVE place relocations
>> against the .altinstructions.%%S and .altinstr_replacement sections:
>>
>> diff --git c/xen/arch/x86/include/asm/alternative.h
>> w/xen/arch/x86/include/asm/alternative.h
>> index 18109e3dc5..e871bfc04c 100644
>> --- c/xen/arch/x86/include/asm/alternative.h
>> +++ w/xen/arch/x86/include/asm/alternative.h
>> @@ -90,18 +90,25 @@ extern void alternative_instructions(void);
>>  /* alternative assembly primitive: */
>>  #define ALTERNATIVE(oldinstr, newinstr, feature)                      \
>>          OLDINSTR_1(oldinstr, 1)                                       \
>> -        ".pushsection .altinstructions, \"a\", @progbits\n"           \
>> +        ".reloc ., BFD_RELOC_NONE, 567f\n"                            \
>> +        ".reloc ., BFD_RELOC_NONE, 568f\n"                            \
>> +        ".pushsection .altinstructions.%%S, \"a\", @progbits\n"       \
>> +        "567:\n"                                                      \
>>          ALTINSTR_ENTRY(feature, 1)                                    \
>>          ".section .discard, \"a\", @progbits\n"                       \
>>          ".byte " alt_total_len "\n" /* total_len <= 255 */            \
>>          DISCARD_ENTRY(1)                                              \
>>          ".section .altinstr_replacement, \"ax\", @progbits\n"         \
>> +        "568:\n"                                                      \
>>          ALTINSTR_REPLACEMENT(newinstr, 1)                             \
>>          ".popsection\n"
>>
> 
> There's already a symbol for the start of the replacement.  We only need
> to introduce a symbol for the metadata.  Try something like this:
> 
> diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h
> index 18109e3dc594..a1295ed816f5 100644
> --- a/xen/arch/x86/include/asm/alternative.h
> +++ b/xen/arch/x86/include/asm/alternative.h
> @@ -55,6 +55,10 @@ extern void alternative_instructions(void);
>  
>  #define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE("("a") < ("b")")")))"
>  
> +#define REF(num)                                        \
> +    ".reloc ., BFD_RELOC_NONE, .LXEN%=_alt" #num "\n\t" \
> +    ".reloc ., BFD_RELOC_NONE, "alt_repl_s(num)  "\n\t"

Is it even worthwhile trying to further pursue this route if xen.efi can't
be built with it?

Jan
Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jason Andryuk 18 hours ago 
On 2025-12-11 03:29, Jan Beulich wrote:
> On 11.12.2025 03:47, Andrew Cooper wrote:
>> On 11/12/2025 1:28 am, Jason Andryuk wrote:
>>> On 2025-12-10 11:55, Andrew Cooper wrote:
>>>> On 09/12/2025 9:47 pm, Jason Andryuk wrote:
>>>>>           . = ALIGN(4);
>>>>>           __alt_instructions = .;
>>>>> -       *(.altinstructions)
>>>>> +       KEEP(*(.altinstructions))
>>>>>           __alt_instructions_end = .;
>>>>
>>>> Thinking about this, what we need is for there to be a reference tied to
>>>> the source location, referencing the replacement metadata and
>>>> replacement instructions.
>>>>
>>>> Looking at https://maskray.me/blog/2021-02-28-linker-garbage-collection
>>>> might be able to do this with .reloc of type none.  In fact,
>>>> BFD_RELOC_NONE seems to have been invented for precisely this purpose.
>>>>
>>>> This means that if and only if the source function gets included, so
>>>> does the metadata and replacement.
>>>
>>> With Jan's -Wa,--sectname-subst changes added to CFLAGS, this seems to
>>> work somewhat.  I'm trying to make the ALTERNATIVE place relocations
>>> against the .altinstructions.%%S and .altinstr_replacement sections:
>>>
>>> diff --git c/xen/arch/x86/include/asm/alternative.h
>>> w/xen/arch/x86/include/asm/alternative.h
>>> index 18109e3dc5..e871bfc04c 100644
>>> --- c/xen/arch/x86/include/asm/alternative.h
>>> +++ w/xen/arch/x86/include/asm/alternative.h
>>> @@ -90,18 +90,25 @@ extern void alternative_instructions(void);
>>>   /* alternative assembly primitive: */
>>>   #define ALTERNATIVE(oldinstr, newinstr, feature)                      \
>>>           OLDINSTR_1(oldinstr, 1)                                       \
>>> -        ".pushsection .altinstructions, \"a\", @progbits\n"           \
>>> +        ".reloc ., BFD_RELOC_NONE, 567f\n"                            \
>>> +        ".reloc ., BFD_RELOC_NONE, 568f\n"                            \
>>> +        ".pushsection .altinstructions.%%S, \"a\", @progbits\n"       \
>>> +        "567:\n"                                                      \
>>>           ALTINSTR_ENTRY(feature, 1)                                    \
>>>           ".section .discard, \"a\", @progbits\n"                       \
>>>           ".byte " alt_total_len "\n" /* total_len <= 255 */            \
>>>           DISCARD_ENTRY(1)                                              \
>>>           ".section .altinstr_replacement, \"ax\", @progbits\n"         \
>>> +        "568:\n"                                                      \
>>>           ALTINSTR_REPLACEMENT(newinstr, 1)                             \
>>>           ".popsection\n"
>>>
>>
>> There's already a symbol for the start of the replacement.  We only need
>> to introduce a symbol for the metadata.  Try something like this:
>>
>> diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h
>> index 18109e3dc594..a1295ed816f5 100644
>> --- a/xen/arch/x86/include/asm/alternative.h
>> +++ b/xen/arch/x86/include/asm/alternative.h
>> @@ -55,6 +55,10 @@ extern void alternative_instructions(void);
>>   
>>   #define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE("("a") < ("b")")")))"
>>   
>> +#define REF(num)                                        \
>> +    ".reloc ., BFD_RELOC_NONE, .LXEN%=_alt" #num "\n\t" \
>> +    ".reloc ., BFD_RELOC_NONE, "alt_repl_s(num)  "\n\t"
> 
> Is it even worthwhile trying to further pursue this route if xen.efi can't
> be built with it?

The alternative is section groups?  I'm trying that, and it kinda works 
sometimes, but .attach_to_group fails when .init.text is involved.

Here's an example that I think would work, I could make it to 
--gc-sectrions:
group section [    3] `.group' [.text.vpmu_do_msr] contains 5 sections:
    [Index]    Name
    [   43]   .text.vpmu_do_msr
    [   44]   .rela.text.vpmu_do_msr
    [   45]   .altinstructions..text.vpmu_do_msr
    [   46]   .rela.altinstructions..text.vpmu_do_msr
    [   47]   .altinstr_replacement..text.vpmu_do_msr

But I don't make it that far.  Other files blow up with tons of:
{standard input}:9098: Warning: dwarf line number information for 
.init.text ignored
and
{standard input}:50083: Error: leb128 operand is an undefined symbol: 
.LVU4040

Line 9098 of apic.s is .loc below:
"""
         .section        .init.text
         .globl  setup_boot_APIC_clock
         .hidden setup_boot_APIC_clock
         .type   setup_boot_APIC_clock, @function
setup_boot_APIC_clock:
.LFB827:
         .loc 1 1150 1 is_stmt 1 view -0
         .cfi_startproc
         pushq   %rbp
"""

diff below.  Any ideas?

Thanks,
Jason

diff --git a/xen/arch/x86/include/asm/alternative.h 
b/xen/arch/x86/include/asm/alternative.h
index 18109e3dc5..8701d0e0a7 100644
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -90,25 +90,31 @@ extern void alternative_instructions(void);
  /* alternative assembly primitive: */
  #define ALTERNATIVE(oldinstr, newinstr, feature)                      \
          OLDINSTR_1(oldinstr, 1)                                       \
-        ".pushsection .altinstructions, \"a\", @progbits\n"           \
+        ".attach_to_group %%S\n"                                      \
+        ".pushsection .altinstructions.%%S, \"a?\", @progbits\n"      \
          ALTINSTR_ENTRY(feature, 1)                                    \
-        ".section .discard, \"a\", @progbits\n"                       \
+        ".popsection\n"                                               \
+        ".pushsection .discard, \"a\", @progbits\n"                   \
          ".byte " alt_total_len "\n" /* total_len <= 255 */            \
          DISCARD_ENTRY(1)                                              \
-        ".section .altinstr_replacement, \"ax\", @progbits\n"         \
+        ".popsection\n"                                               \
+        ".pushsection .altinstr_replacement.%%S, \"ax?\", @progbits\n"\
          ALTINSTR_REPLACEMENT(newinstr, 1)                             \
          ".popsection\n"

  #define ALTERNATIVE_2(oldinstr, newinstr1, feature1, newinstr2, 
feature2) \
          OLDINSTR_2(oldinstr, 1, 2)                                    \
-        ".pushsection .altinstructions, \"a\", @progbits\n"           \
+        ".attach_to_group %%S\n"                                      \
+        ".pushsection .altinstructions.%%S, \"a?\", @progbits\n"      \
          ALTINSTR_ENTRY(feature1, 1)                                   \
          ALTINSTR_ENTRY(feature2, 2)                                   \
-        ".section .discard, \"a\", @progbits\n"                       \
+        ".popsection\n"                                               \
+        ".pushsection .discard, \"a\", @progbits\n"                   \
          ".byte " alt_total_len "\n" /* total_len <= 255 */            \
          DISCARD_ENTRY(1)                                              \
          DISCARD_ENTRY(2)                                              \
-        ".section .altinstr_replacement, \"ax\", @progbits\n"         \
+        ".popsection\n"                                               \
+        ".pushsection .altinstr_replacement.%%S, \"ax?\", @progbits\n"\
          ALTINSTR_REPLACEMENT(newinstr1, 1)                            \
          ALTINSTR_REPLACEMENT(newinstr2, 2)                            \
          ".popsection\n"
Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jan Beulich 6 hours ago 
On 12.12.2025 02:34, Jason Andryuk wrote:
> The alternative is section groups?  I'm trying that, and it kinda works 
> sometimes, but .attach_to_group fails when .init.text is involved.
> 
> Here's an example that I think would work, I could make it to 
> --gc-sectrions:
> group section [    3] `.group' [.text.vpmu_do_msr] contains 5 sections:
>     [Index]    Name
>     [   43]   .text.vpmu_do_msr
>     [   44]   .rela.text.vpmu_do_msr
>     [   45]   .altinstructions..text.vpmu_do_msr
>     [   46]   .rela.altinstructions..text.vpmu_do_msr
>     [   47]   .altinstr_replacement..text.vpmu_do_msr
> 
> But I don't make it that far.  Other files blow up with tons of:
> {standard input}:9098: Warning: dwarf line number information for 
> .init.text ignored
> and
> {standard input}:50083: Error: leb128 operand is an undefined symbol: 
> .LVU4040
> 
> Line 9098 of apic.s is .loc below:
> """
>          .section        .init.text
>          .globl  setup_boot_APIC_clock
>          .hidden setup_boot_APIC_clock
>          .type   setup_boot_APIC_clock, @function
> setup_boot_APIC_clock:
> .LFB827:
>          .loc 1 1150 1 is_stmt 1 view -0
>          .cfi_startproc
>          pushq   %rbp
> """
> 
> diff below.  Any ideas?

I haven't looked into this in detail yet, but ...

> --- a/xen/arch/x86/include/asm/alternative.h
> +++ b/xen/arch/x86/include/asm/alternative.h
> @@ -90,25 +90,31 @@ extern void alternative_instructions(void);
>   /* alternative assembly primitive: */
>   #define ALTERNATIVE(oldinstr, newinstr, feature)                      \
>           OLDINSTR_1(oldinstr, 1)                                       \
> -        ".pushsection .altinstructions, \"a\", @progbits\n"           \
> +        ".attach_to_group %%S\n"                                      \
> +        ".pushsection .altinstructions.%%S, \"a?\", @progbits\n"      \

... wouldn't you need another .attach_to_group here and ...

>           ALTINSTR_ENTRY(feature, 1)                                    \
> -        ".section .discard, \"a\", @progbits\n"                       \
> +        ".popsection\n"                                               \
> +        ".pushsection .discard, \"a\", @progbits\n"                   \
>           ".byte " alt_total_len "\n" /* total_len <= 255 */            \
>           DISCARD_ENTRY(1)                                              \
> -        ".section .altinstr_replacement, \"ax\", @progbits\n"         \
> +        ".popsection\n"                                               \
> +        ".pushsection .altinstr_replacement.%%S, \"ax?\", @progbits\n"\

... here? Or alternatively use the 'G' section flag to the specify the group
name?

As to debug info, I wonder whether playing with groups behind the back of the
compiler is going to work well. Iirc it groups sections itself, too. Did you
look at the generated assembly with this in mind?

Jan
Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jason Andryuk 3 hours ago 
On 2025-12-12 08:22, Jan Beulich wrote:
> On 12.12.2025 02:34, Jason Andryuk wrote:
>> The alternative is section groups?  I'm trying that, and it kinda works
>> sometimes, but .attach_to_group fails when .init.text is involved.
>>
>> Here's an example that I think would work, I could make it to
>> --gc-sectrions:
>> group section [    3] `.group' [.text.vpmu_do_msr] contains 5 sections:
>>      [Index]    Name
>>      [   43]   .text.vpmu_do_msr
>>      [   44]   .rela.text.vpmu_do_msr
>>      [   45]   .altinstructions..text.vpmu_do_msr
>>      [   46]   .rela.altinstructions..text.vpmu_do_msr
>>      [   47]   .altinstr_replacement..text.vpmu_do_msr
>>
>> But I don't make it that far.  Other files blow up with tons of:
>> {standard input}:9098: Warning: dwarf line number information for
>> .init.text ignored
>> and
>> {standard input}:50083: Error: leb128 operand is an undefined symbol:
>> .LVU4040
>>
>> Line 9098 of apic.s is .loc below:
>> """
>>           .section        .init.text
>>           .globl  setup_boot_APIC_clock
>>           .hidden setup_boot_APIC_clock
>>           .type   setup_boot_APIC_clock, @function
>> setup_boot_APIC_clock:
>> .LFB827:
>>           .loc 1 1150 1 is_stmt 1 view -0
>>           .cfi_startproc
>>           pushq   %rbp
>> """
>>
>> diff below.  Any ideas?
> 
> I haven't looked into this in detail yet, but ...
> 
>> --- a/xen/arch/x86/include/asm/alternative.h
>> +++ b/xen/arch/x86/include/asm/alternative.h
>> @@ -90,25 +90,31 @@ extern void alternative_instructions(void);
>>    /* alternative assembly primitive: */
>>    #define ALTERNATIVE(oldinstr, newinstr, feature)                      \
>>            OLDINSTR_1(oldinstr, 1)                                       \
>> -        ".pushsection .altinstructions, \"a\", @progbits\n"           \
>> +        ".attach_to_group %%S\n"                                      \
>> +        ".pushsection .altinstructions.%%S, \"a?\", @progbits\n"      \
> 
> ... wouldn't you need another .attach_to_group here and ...
> 
>>            ALTINSTR_ENTRY(feature, 1)                                    \
>> -        ".section .discard, \"a\", @progbits\n"                       \
>> +        ".popsection\n"                                               \
>> +        ".pushsection .discard, \"a\", @progbits\n"                   \
>>            ".byte " alt_total_len "\n" /* total_len <= 255 */            \
>>            DISCARD_ENTRY(1)                                              \
>> -        ".section .altinstr_replacement, \"ax\", @progbits\n"         \
>> +        ".popsection\n"                                               \
>> +        ".pushsection .altinstr_replacement.%%S, \"ax?\", @progbits\n"\
> 
> ... here? Or alternatively use the 'G' section flag to the specify the group
> name?

The '?' flag puts the new section in the previous group, so it doesn't 
have to be specified.  I have used 'G' and %%S with similar results. 
The example vpmu output above shows that is working.  I can't get to 
linking with --gc-sections yes to see if %%S is no longer necessary with 
proper groups.

The problem is "the current function" needs to be assigned to the same 
group, and that is what I hoped to address with .attach_to_group.  From 
what I can tell, the function-section is not assigned to a group without 
.attach_to_group.

> As to debug info, I wonder whether playing with groups behind the back of the
> compiler is going to work well. Iirc it groups sections itself, too. Did you
> look at the generated assembly with this in mind?

The generated assembly differs only by the presence of .attach_to_group 
for build vs. doesn't build.  Is the debug information expected to 
differ according to groups?  (This is all new to me).  I have more to 
look into, I figured I'd post what I have in case anyone had seen it before.

Thanks,
Jason
Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS
Posted by Jason Andryuk 2 days, 2 hours ago 
On 2025-12-10 11:55, Andrew Cooper wrote:
> On 09/12/2025 9:47 pm, Jason Andryuk wrote:
>>          . = ALIGN(4);
>>          __alt_instructions = .;
>> -       *(.altinstructions)
>> +       KEEP(*(.altinstructions))
>>          __alt_instructions_end = .;
> 
> Thinking about this, what we need is for there to be a reference tied to
> the source location, referencing the replacement metadata and
> replacement instructions.
> 
> Looking at https://maskray.me/blog/2021-02-28-linker-garbage-collection
> might be able to do this with .reloc of type none.  In fact,
> BFD_RELOC_NONE seems to have been invented for precisely this purpose.
> 
> This means that if and only if the source function gets included, so
> does the metadata and replacement.

Yes.  Thanks for the reference.  I'll take a look.

Regards,
Jason

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.