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

[Jason Andryuk <jason.andryuk@xxxxxxx>]

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"