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
