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

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

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@xxxxxxx>
> ---
> 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