This change allows to put the trampoline in a separate, not executable
section. The trampoline contains a mix of code and data (data which
is modified from C code during early start so must be writable).
This is in preparation for W^X patch in order to satisfy UEFI CA
memory mitigation requirements.
At the moment .init.text and .init.data in EFI mode are put together
so they will be in the same final section as before this patch.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
 xen/arch/x86/boot/head.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index d8ac0f0494..f027ff45fd 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -870,6 +870,8 @@ cmdline_parse_early:
 reloc:
         .incbin "reloc.bin"
 
+        .section .init.data, "aw", @progbits
+        .align 4
 ENTRY(trampoline_start)
 #include "trampoline.S"
 ENTRY(trampoline_end)
-- 
2.45.2

On 07.08.2024 15:48, Alejandro Vallejo wrote:
> This change allows to put the trampoline in a separate, not executable
> section. The trampoline contains a mix of code and data (data which
> is modified from C code during early start so must be writable).
> This is in preparation for W^X patch in order to satisfy UEFI CA
> memory mitigation requirements.

Which, aiui, has the downside of disassembly of the section no longer
happening by default, when using objdump or similar tools, which go from
section attributes. Why is it being in .init.text (and hence RX) not
appropriate? It should - in principle at least - be possible to avoid
all in-place writing to it, but instead only ever write to its relocated
copy. Quite a bit more code churn of course.

I wonder if we shouldn't put the trampoline in its own section, RWX in
the object file, and switched to whatever appropriate in the binary
(which really may be RX, not RW).

> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -870,6 +870,8 @@ cmdline_parse_early:
>  reloc:
>          .incbin "reloc.bin"
>  
> +        .section .init.data, "aw", @progbits
> +        .align 4

Is the .align really needed here? I think ...

>  ENTRY(trampoline_start)

... ENTRY() covers this properly? And actually in a better way, using
CODE_FILL (which ultimately we will want to switch from 0x90 to 0xcc, I
suppose) rather than whatever the assembler puts in by default for data
sections.

Jan

>  #include "trampoline.S"
>  ENTRY(trampoline_end)