* [PATCH 0/2] Enable --gc-sections @ 2025-12-09 21:47 Jason Andryuk 2025-12-09 21:47 ` [PATCH 1/2] xen: Centralize scheduler linker definition Jason Andryuk 2025-12-09 21:47 ` [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS Jason Andryuk 0 siblings, 2 replies; 28+ messages in thread From: Jason Andryuk @ 2025-12-09 21:47 UTC (permalink / raw) To: xen-devel Cc: Victor Lira, Jason Andryuk, Jan Beulich, Andrew Cooper, Roger Pau Monné, Anthony PERARD, Michal Orzel, Julien Grall, Stefano Stabellini, Grygorii Strashko, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/2] xen: Centralize scheduler linker definition 2025-12-09 21:47 [PATCH 0/2] Enable --gc-sections Jason Andryuk @ 2025-12-09 21:47 ` Jason Andryuk 2025-12-10 8:08 ` Jan Beulich 2025-12-09 21:47 ` [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS Jason Andryuk 1 sibling, 1 reply; 28+ messages in thread From: Jason Andryuk @ 2025-12-09 21:47 UTC (permalink / raw) To: xen-devel Cc: Victor Lira, Jason Andryuk, Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper, Anthony PERARD, Jan Beulich, Roger Pau Monné, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko 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 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] xen: Centralize scheduler linker definition 2025-12-09 21:47 ` [PATCH 1/2] xen: Centralize scheduler linker definition Jason Andryuk @ 2025-12-10 8:08 ` Jan Beulich 2025-12-10 16:32 ` Andrew Cooper 0 siblings, 1 reply; 28+ messages in thread From: Jan Beulich @ 2025-12-10 8:08 UTC (permalink / raw) To: Jason Andryuk Cc: Victor Lira, Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper, Anthony PERARD, Roger Pau Monné, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, xen-devel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] xen: Centralize scheduler linker definition 2025-12-10 8:08 ` Jan Beulich @ 2025-12-10 16:32 ` Andrew Cooper 2025-12-10 16:49 ` Jason Andryuk 0 siblings, 1 reply; 28+ messages in thread From: Andrew Cooper @ 2025-12-10 16:32 UTC (permalink / raw) To: Jan Beulich, Jason Andryuk Cc: Andrew Cooper, Victor Lira, Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Anthony PERARD, Roger Pau Monné, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, xen-devel 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> ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] xen: Centralize scheduler linker definition 2025-12-10 16:32 ` Andrew Cooper @ 2025-12-10 16:49 ` Jason Andryuk 0 siblings, 0 replies; 28+ messages in thread From: Jason Andryuk @ 2025-12-10 16:49 UTC (permalink / raw) To: Andrew Cooper, Jan Beulich Cc: Victor Lira, Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Anthony PERARD, Roger Pau Monné, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, xen-devel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-09 21:47 [PATCH 0/2] Enable --gc-sections Jason Andryuk 2025-12-09 21:47 ` [PATCH 1/2] xen: Centralize scheduler linker definition Jason Andryuk @ 2025-12-09 21:47 ` Jason Andryuk 2025-12-10 8:17 ` Jan Beulich ` (3 more replies) 1 sibling, 4 replies; 28+ messages in thread From: Jason Andryuk @ 2025-12-09 21:47 UTC (permalink / raw) To: xen-devel Cc: Victor Lira, Jason Andryuk, Andrew Cooper, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko 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 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-09 21:47 ` [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS Jason Andryuk @ 2025-12-10 8:17 ` Jan Beulich 2025-12-10 16:57 ` Jason Andryuk 2025-12-10 14:40 ` Anthony PERARD ` (2 subsequent siblings) 3 siblings, 1 reply; 28+ messages in thread From: Jan Beulich @ 2025-12-10 8:17 UTC (permalink / raw) To: Jason Andryuk Cc: Victor Lira, Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, xen-devel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-10 8:17 ` Jan Beulich @ 2025-12-10 16:57 ` Jason Andryuk 2025-12-11 8:20 ` Jan Beulich 0 siblings, 1 reply; 28+ messages in thread From: Jason Andryuk @ 2025-12-10 16:57 UTC (permalink / raw) To: Jan Beulich Cc: Victor Lira, Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, xen-devel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-10 16:57 ` Jason Andryuk @ 2025-12-11 8:20 ` Jan Beulich 0 siblings, 0 replies; 28+ messages in thread From: Jan Beulich @ 2025-12-11 8:20 UTC (permalink / raw) To: Jason Andryuk Cc: Victor Lira, Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, xen-devel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-09 21:47 ` [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS Jason Andryuk 2025-12-10 8:17 ` Jan Beulich @ 2025-12-10 14:40 ` Anthony PERARD 2025-12-10 17:08 ` Jason Andryuk 2025-12-10 16:55 ` KEEP " Andrew Cooper 2025-12-12 10:42 ` Grygorii Strashko 3 siblings, 1 reply; 28+ messages in thread From: Anthony PERARD @ 2025-12-10 14:40 UTC (permalink / raw) To: Jason Andryuk Cc: xen-devel, Victor Lira, Andrew Cooper, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-10 14:40 ` Anthony PERARD @ 2025-12-10 17:08 ` Jason Andryuk 2025-12-11 8:23 ` Jan Beulich 0 siblings, 1 reply; 28+ messages in thread From: Jason Andryuk @ 2025-12-10 17:08 UTC (permalink / raw) To: Anthony PERARD Cc: xen-devel, Victor Lira, Andrew Cooper, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-10 17:08 ` Jason Andryuk @ 2025-12-11 8:23 ` Jan Beulich 0 siblings, 0 replies; 28+ messages in thread From: Jan Beulich @ 2025-12-11 8:23 UTC (permalink / raw) To: Jason Andryuk Cc: xen-devel, Victor Lira, Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, Anthony PERARD 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-09 21:47 ` [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS Jason Andryuk 2025-12-10 8:17 ` Jan Beulich 2025-12-10 14:40 ` Anthony PERARD @ 2025-12-10 16:55 ` Andrew Cooper 2025-12-10 17:11 ` Jason Andryuk 2025-12-11 1:28 ` Jason Andryuk 2025-12-12 10:42 ` Grygorii Strashko 3 siblings, 2 replies; 28+ messages in thread From: Andrew Cooper @ 2025-12-10 16:55 UTC (permalink / raw) To: Jason Andryuk, xen-devel Cc: Andrew Cooper, Victor Lira, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-10 16:55 ` KEEP " Andrew Cooper @ 2025-12-10 17:11 ` Jason Andryuk 2025-12-11 1:28 ` Jason Andryuk 1 sibling, 0 replies; 28+ messages in thread From: Jason Andryuk @ 2025-12-10 17:11 UTC (permalink / raw) To: Andrew Cooper, xen-devel Cc: Victor Lira, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-10 16:55 ` KEEP " Andrew Cooper 2025-12-10 17:11 ` Jason Andryuk @ 2025-12-11 1:28 ` Jason Andryuk 2025-12-11 1:53 ` Jason Andryuk ` (2 more replies) 1 sibling, 3 replies; 28+ messages in thread From: Jason Andryuk @ 2025-12-11 1:28 UTC (permalink / raw) To: Andrew Cooper, xen-devel Cc: Victor Lira, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko 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' ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-11 1:28 ` Jason Andryuk @ 2025-12-11 1:53 ` Jason Andryuk 2025-12-11 2:47 ` Andrew Cooper 2025-12-11 8:27 ` Jan Beulich 2 siblings, 0 replies; 28+ messages in thread From: Jason Andryuk @ 2025-12-11 1:53 UTC (permalink / raw) To: Andrew Cooper, xen-devel Cc: Victor Lira, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-11 1:28 ` Jason Andryuk 2025-12-11 1:53 ` Jason Andryuk @ 2025-12-11 2:47 ` Andrew Cooper 2025-12-11 8:29 ` Jan Beulich 2025-12-12 15:39 ` Jason Andryuk 2025-12-11 8:27 ` Jan Beulich 2 siblings, 2 replies; 28+ messages in thread From: Andrew Cooper @ 2025-12-11 2:47 UTC (permalink / raw) To: Jason Andryuk, xen-devel Cc: Andrew Cooper, Victor Lira, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko 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 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-11 2:47 ` Andrew Cooper @ 2025-12-11 8:29 ` Jan Beulich 2025-12-12 1:34 ` Jason Andryuk 2025-12-12 15:39 ` Jason Andryuk 1 sibling, 1 reply; 28+ messages in thread From: Jan Beulich @ 2025-12-11 8:29 UTC (permalink / raw) To: Andrew Cooper Cc: Victor Lira, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, Jason Andryuk, xen-devel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-11 8:29 ` Jan Beulich @ 2025-12-12 1:34 ` Jason Andryuk 2025-12-12 13:22 ` Jan Beulich 0 siblings, 1 reply; 28+ messages in thread From: Jason Andryuk @ 2025-12-12 1:34 UTC (permalink / raw) To: Jan Beulich, Andrew Cooper Cc: Victor Lira, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, xen-devel 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" ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-12 1:34 ` Jason Andryuk @ 2025-12-12 13:22 ` Jan Beulich 2025-12-12 15:48 ` Jason Andryuk 2026-01-09 23:32 ` Jason Andryuk 0 siblings, 2 replies; 28+ messages in thread From: Jan Beulich @ 2025-12-12 13:22 UTC (permalink / raw) To: Jason Andryuk Cc: Victor Lira, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, xen-devel, Andrew Cooper 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-12 13:22 ` Jan Beulich @ 2025-12-12 15:48 ` Jason Andryuk 2025-12-15 8:59 ` Jan Beulich 2026-01-09 23:32 ` Jason Andryuk 1 sibling, 1 reply; 28+ messages in thread From: Jason Andryuk @ 2025-12-12 15:48 UTC (permalink / raw) To: Jan Beulich Cc: Victor Lira, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, xen-devel, Andrew Cooper 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-12 15:48 ` Jason Andryuk @ 2025-12-15 8:59 ` Jan Beulich 0 siblings, 0 replies; 28+ messages in thread From: Jan Beulich @ 2025-12-15 8:59 UTC (permalink / raw) To: Jason Andryuk Cc: Victor Lira, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, xen-devel, Andrew Cooper 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-12 13:22 ` Jan Beulich 2025-12-12 15:48 ` Jason Andryuk @ 2026-01-09 23:32 ` Jason Andryuk 2026-01-12 10:37 ` Jan Beulich 1 sibling, 1 reply; 28+ messages in thread From: Jason Andryuk @ 2026-01-09 23:32 UTC (permalink / raw) To: Jan Beulich Cc: Victor Lira, Anthony PERARD, Michal Orzel, Roger Pau Monné, Stefano Stabellini, Grygorii Strashko, xen-devel, Andrew Cooper (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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2026-01-09 23:32 ` Jason Andryuk @ 2026-01-12 10:37 ` Jan Beulich 0 siblings, 0 replies; 28+ messages in thread From: Jan Beulich @ 2026-01-12 10:37 UTC (permalink / raw) To: Jason Andryuk Cc: Victor Lira, Anthony PERARD, Michal Orzel, Roger Pau Monné, Stefano Stabellini, Grygorii Strashko, xen-devel, Andrew Cooper 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-11 2:47 ` Andrew Cooper 2025-12-11 8:29 ` Jan Beulich @ 2025-12-12 15:39 ` Jason Andryuk 1 sibling, 0 replies; 28+ messages in thread From: Jason Andryuk @ 2025-12-12 15:39 UTC (permalink / raw) To: Andrew Cooper, xen-devel Cc: Victor Lira, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KEEP Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-11 1:28 ` Jason Andryuk 2025-12-11 1:53 ` Jason Andryuk 2025-12-11 2:47 ` Andrew Cooper @ 2025-12-11 8:27 ` Jan Beulich 2 siblings, 0 replies; 28+ messages in thread From: Jan Beulich @ 2025-12-11 8:27 UTC (permalink / raw) To: Jason Andryuk Cc: Victor Lira, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko, Grygorii Strashko, Andrew Cooper, xen-devel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-09 21:47 ` [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS Jason Andryuk ` (2 preceding siblings ...) 2025-12-10 16:55 ` KEEP " Andrew Cooper @ 2025-12-12 10:42 ` Grygorii Strashko 2025-12-12 15:54 ` Jason Andryuk 3 siblings, 1 reply; 28+ messages in thread From: Grygorii Strashko @ 2025-12-12 10:42 UTC (permalink / raw) To: Jason Andryuk, xen-devel Cc: Victor Lira, Andrew Cooper, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS 2025-12-12 10:42 ` Grygorii Strashko @ 2025-12-12 15:54 ` Jason Andryuk 0 siblings, 0 replies; 28+ messages in thread From: Jason Andryuk @ 2025-12-12 15:54 UTC (permalink / raw) To: Grygorii Strashko, xen-devel Cc: Victor Lira, Andrew Cooper, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk, Timothy Pearson, Alistair Francis, Bob Eshleman, Connor Davis, Oleksii Kurochko 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2026-01-12 10:37 UTC | newest] Thread overview: 28+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-09 21:47 [PATCH 0/2] Enable --gc-sections Jason Andryuk 2025-12-09 21:47 ` [PATCH 1/2] xen: Centralize scheduler linker definition Jason Andryuk 2025-12-10 8:08 ` Jan Beulich 2025-12-10 16:32 ` Andrew Cooper 2025-12-10 16:49 ` Jason Andryuk 2025-12-09 21:47 ` [PATCH 2/2] xen: Add CONFIG_GC_SECTIONS Jason Andryuk 2025-12-10 8:17 ` Jan Beulich 2025-12-10 16:57 ` Jason Andryuk 2025-12-11 8:20 ` Jan Beulich 2025-12-10 14:40 ` Anthony PERARD 2025-12-10 17:08 ` Jason Andryuk 2025-12-11 8:23 ` Jan Beulich 2025-12-10 16:55 ` KEEP " Andrew Cooper 2025-12-10 17:11 ` Jason Andryuk 2025-12-11 1:28 ` Jason Andryuk 2025-12-11 1:53 ` Jason Andryuk 2025-12-11 2:47 ` Andrew Cooper 2025-12-11 8:29 ` Jan Beulich 2025-12-12 1:34 ` Jason Andryuk 2025-12-12 13:22 ` Jan Beulich 2025-12-12 15:48 ` Jason Andryuk 2025-12-15 8:59 ` Jan Beulich 2026-01-09 23:32 ` Jason Andryuk 2026-01-12 10:37 ` Jan Beulich 2025-12-12 15:39 ` Jason Andryuk 2025-12-11 8:27 ` Jan Beulich 2025-12-12 10:42 ` Grygorii Strashko 2025-12-12 15:54 ` Jason Andryuk
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.