Re: [PATCH v2] x86: Put trampoline in separate .init.trampoline section

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Sat, Sep 14, 2024 at 7:16 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 11.09.2024 11:55, Frediano Ziglio wrote:
> > --- a/xen/arch/x86/boot/head.S
> > +++ b/xen/arch/x86/boot/head.S
> > @@ -882,8 +882,9 @@ cmdline_parse_early:
> >  reloc:
> >          .incbin "reloc.bin"
> >
> > +#include "x86_64.S"
> > +
> > +        .section .init.trampoline, "aw", @progbits
> >  ENTRY(trampoline_start)
> >  #include "trampoline.S"
> >  ENTRY(trampoline_end)
>
> Hmm, nice - this turns out rather easier than I first thought.
>
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -269,6 +269,11 @@ SECTIONS
> >         __ctors_end = .;
> >    } PHDR(text)
> >
> > +  . = ALIGN(PAGE_SIZE);
>
> Why? There's no special alignment right now.
>

UEFI CA Memory Mitigation requirements, I'll remove from this commit.

> > +  DECL_SECTION(.init.trampoline) {
> > +       *(.init.trampoline)
> > +  } PHDR(text)
> > +
> >  #ifndef EFI
>
> If this is to be a separate section also for ELF, I think that
> wants mentioning explicitly. "Easily disassemble" is too vague
> a reason for my taste.

It's not clear if either you changed your mind on that reason or if
the commit message is not clear. I'm assuming the latter, I'll improve
the commit message.
Not clear why you specify "ELF" in the above sentence, I mean, why
should it matter if it applies to EFI and/or ELF? And why having it
different from ELF to EFI?

>
> Jan

Frediano