[PATCH 0/2] Enable --gc-sections

[PATCH 0/2] Enable --gc-sections

[Jason Andryuk]

The first patch removes the scheduler duplication in the linker script.
The second patch adds CONFIG_GC_SECTIONS and makes it available for all
architectures.

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

--print-gc-sections is enabled, and ARM builds in particular show a good
bit of removal.

Jason Andryuk (2):
  xen: Centralize scheduler linker definition
  xen: Add CONFIG_GC_SECTIONS

 xen/Makefile              |  3 +++
 xen/arch/arm/Makefile     |  6 +++---
 xen/arch/arm/xen.lds.S    | 27 ++++++++++++---------------
 xen/arch/ppc/Makefile     |  6 +++---
 xen/arch/ppc/xen.lds.S    | 19 ++++++++-----------
 xen/arch/riscv/Makefile   |  6 +++---
 xen/arch/riscv/xen.lds.S  | 19 ++++++++-----------
 xen/arch/x86/Makefile     |  6 +++---
 xen/arch/x86/xen.lds.S    | 39 ++++++++++++++++++---------------------
 xen/common/Kconfig        |  9 +++++++++
 xen/include/xen/xen.lds.h | 24 +++++++++++++++---------
 11 files changed, 85 insertions(+), 79 deletions(-)

-- 
2.52.0

[PATCH 1/2] xen: Centralize scheduler linker definition

[Jason Andryuk]

Use a define to centralize the common scheduler data in the per-arch
linker scripts.  This is in preparation for marking it KEEP().

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
 xen/arch/arm/xen.lds.S    | 5 +----
 xen/arch/ppc/xen.lds.S    | 5 +----
 xen/arch/riscv/xen.lds.S  | 5 +----
 xen/arch/x86/xen.lds.S    | 5 +----
 xen/include/xen/xen.lds.h | 6 ++++++
 5 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index db17ff1efa..2d5f1c516d 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -92,11 +92,8 @@ SECTIONS
   . = ALIGN(SMP_CACHE_BYTES);
   .data : {                    /* Data */
        *(.data.page_aligned)
-       . = ALIGN(8);
-       __start_schedulers_array = .;
-       *(.data.schedulers)
-       __end_schedulers_array = .;
 
+       SCHEDULER_ARRAY
        HYPFS_PARAM
 
        *(.data .data.*)
diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S
index 1de0b77fc6..d0f2ed43f1 100644
--- a/xen/arch/ppc/xen.lds.S
+++ b/xen/arch/ppc/xen.lds.S
@@ -83,11 +83,8 @@ SECTIONS
     . = ALIGN(PAGE_SIZE);
     DECL_SECTION(.data) {                    /* Data */
         *(.data.page_aligned)
-        . = ALIGN(8);
-        __start_schedulers_array = .;
-        *(.data.schedulers)
-        __end_schedulers_array = .;
 
+        SCHEDULER_ARRAY
         HYPFS_PARAM
 
         *(.data .data.*)
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index edcadff90b..45d2e053d0 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -78,11 +78,8 @@ SECTIONS
     . = ALIGN(PAGE_SIZE);
     .data : {                    /* Data */
         *(.data.page_aligned)
-        . = ALIGN(8);
-        __start_schedulers_array = .;
-        *(.data.schedulers)
-        __end_schedulers_array = .;
 
+        SCHEDULER_ARRAY
         HYPFS_PARAM
 
         *(.data .data.*)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 1ee08a3ea3..2aa41306ca 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -309,11 +309,8 @@ SECTIONS
   . = ALIGN(SMP_CACHE_BYTES);
   DECL_SECTION(.data.read_mostly) {
        *(.data.read_mostly)
-       . = ALIGN(8);
-       __start_schedulers_array = .;
-       *(.data.schedulers)
-       __end_schedulers_array = .;
 
+       SCHEDULER_ARRAY
        HYPFS_PARAM
   } PHDR(text)
 
diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
index f54fb2d152..2d66d618b3 100644
--- a/xen/include/xen/xen.lds.h
+++ b/xen/include/xen/xen.lds.h
@@ -173,6 +173,12 @@
        _edevice = .;        \
   } :text
 
+#define SCHEDULER_ARRAY              \
+       . = ALIGN(8);                 \
+       __start_schedulers_array = .; \
+       *(.data.schedulers)           \
+       __end_schedulers_array = .;
+
 #ifdef CONFIG_HYPFS
 #define HYPFS_PARAM              \
        . = ALIGN(POINTER_ALIGN); \
-- 
2.52.0

[PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jason Andryuk]

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 1/2] xen: Centralize scheduler linker definition

[Jan Beulich]

On 09.12.2025 22:47, Jason Andryuk wrote:
> --- a/xen/include/xen/xen.lds.h
> +++ b/xen/include/xen/xen.lds.h
> @@ -173,6 +173,12 @@
>         _edevice = .;        \
>    } :text
>  
> +#define SCHEDULER_ARRAY              \
> +       . = ALIGN(8);                 \

While indeed it was 8 in all original locations, I question that for Arm32
(and a possible future RV32, for example); imo it wants to be ...

> +       __start_schedulers_array = .; \
> +       *(.data.schedulers)           \
> +       __end_schedulers_array = .;
> +
>  #ifdef CONFIG_HYPFS
>  #define HYPFS_PARAM              \
>         . = ALIGN(POINTER_ALIGN); \

... exactly like this. Preferably with that change (happy to carry out while
committing, alongside a respective addition to the description, so long as
there's agreement):
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jan Beulich]

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

[Anthony PERARD]

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 1/2] xen: Centralize scheduler linker definition

[Andrew Cooper]

On 10/12/2025 8:08 am, Jan Beulich wrote:
> On 09.12.2025 22:47, Jason Andryuk wrote:
>> --- a/xen/include/xen/xen.lds.h
>> +++ b/xen/include/xen/xen.lds.h
>> @@ -173,6 +173,12 @@
>>         _edevice = .;        \
>>    } :text
>>  
>> +#define SCHEDULER_ARRAY              \
>> +       . = ALIGN(8);                 \
> While indeed it was 8 in all original locations, I question that for Arm32
> (and a possible future RV32, for example); imo it wants to be ...
>
>> +       __start_schedulers_array = .; \
>> +       *(.data.schedulers)           \
>> +       __end_schedulers_array = .;
>> +
>>  #ifdef CONFIG_HYPFS
>>  #define HYPFS_PARAM              \
>>         . = ALIGN(POINTER_ALIGN); \
> ... exactly like this. Preferably with that change (happy to carry out while
> committing, alongside a respective addition to the description, so long as
> there's agreement):
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

I thought the same.  struct scheduler is entirely pointers, and one
unsigned int.

I'm pretty sure that this "array" predates the introduction of
POINTER_ALIGN.

So yes, with it converted to POINTER_ALIGN, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>

Re: [PATCH 1/2] xen: Centralize scheduler linker definition

[Jason Andryuk]

On 2025-12-10 11:32, Andrew Cooper wrote:
> On 10/12/2025 8:08 am, Jan Beulich wrote:
>> On 09.12.2025 22:47, Jason Andryuk wrote:
>>> --- a/xen/include/xen/xen.lds.h
>>> +++ b/xen/include/xen/xen.lds.h
>>> @@ -173,6 +173,12 @@
>>>          _edevice = .;        \
>>>     } :text
>>>   
>>> +#define SCHEDULER_ARRAY              \
>>> +       . = ALIGN(8);                 \
>> While indeed it was 8 in all original locations, I question that for Arm32
>> (and a possible future RV32, for example); imo it wants to be ...
>>
>>> +       __start_schedulers_array = .; \
>>> +       *(.data.schedulers)           \
>>> +       __end_schedulers_array = .;
>>> +
>>>   #ifdef CONFIG_HYPFS
>>>   #define HYPFS_PARAM              \
>>>          . = ALIGN(POINTER_ALIGN); \
>> ... exactly like this. Preferably with that change (happy to carry out while
>> committing, alongside a respective addition to the description, so long as
>> there's agreement):
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> I thought the same.  struct scheduler is entirely pointers, and one
> unsigned int.
> 
> I'm pretty sure that this "array" predates the introduction of
> POINTER_ALIGN.
> 
> So yes, with it converted to POINTER_ALIGN, Reviewed-by: Andrew Cooper
> <andrew.cooper3@citrix.com>

Converting to POINTER_ALIGN works for me.

Thanks,
Jason

KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Andrew Cooper]

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: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jason Andryuk]

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

[Jason Andryuk]

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: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jason Andryuk]

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

Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jason Andryuk]

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

[Jason Andryuk]

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

[Andrew Cooper]

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: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jan Beulich]

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

Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jan Beulich]

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: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jan Beulich]

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

[Jan Beulich]

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

[Jason Andryuk]

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: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Grygorii Strashko]

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: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jan Beulich]

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

[Jason Andryuk]

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

[Jason Andryuk]

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: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jason Andryuk]

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: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jan Beulich]

On 12.12.2025 16:48, Jason Andryuk wrote:
> 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.

Oh, sorry - I had managed to overlook the use of '?' above.

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

Hmm, looking at a random example, .debug_info (and other Dwarf sections) is
still monolithic with -ffunction-sections. I'm having a hard time seeing
how that would work nicely. And while I'm sure I saw gcc emit section
groups in certain cases (e.g. while building tools/), I can't observe it
doing so there, so that must also depend on something I haven't figured out
yet.

Jan

Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jason Andryuk]

(trimmed the CC list down)

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

Earlier in the file, there is
              .section        .init.text,"ax",@progbits
but the later .section .init.text entries don't have the extra string.

tl;dr: If I add "ax",@progbits to all the .init.text entries, the file 
assembles.

I opened a bug here: https://sourceware.org/bugzilla/show_bug.cgi?id=33779

Regards,
Jason

Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS

[Jan Beulich]

On 10.01.2026 00:32, Jason Andryuk wrote:
> (trimmed the CC list down)
> 
> 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
> 
> Earlier in the file, there is
>               .section        .init.text,"ax",@progbits
> but the later .section .init.text entries don't have the extra string.
> 
> tl;dr: If I add "ax",@progbits to all the .init.text entries, the file 
> assembles.
> 
> I opened a bug here: https://sourceware.org/bugzilla/show_bug.cgi?id=33779

And I've replied there. I think gas is working correctly here.

Jan