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

[Jan Beulich <jbeulich@xxxxxxxx>]

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