On Thu, 18 May 2023 at 00:40, Tom Lendacky <thomas.lendacky@xxxxxxx> wrote: > > On 5/8/23 02:03, Ard Biesheuvel wrote: > > The 32-bit trampoline no longer uses the stack for anything except > > performing a long return back to long mode. Currently, this stack is > > allocated in the same page that carries the trampoline code, which means > > this page must be mapped writable and executable, and the stack is > > therefore executable as well. > > > > So let's do a long jump instead: that way, we can pre-calculate the > > return address and poke it into the code before we call it. In a later > > patch, we will take advantage of this by removing writable permissions > > (and adding executable ones) explicitly when booting via the EFI stub. > > > > Not playing with the stack pointer also makes it more straight-forward > > to call the trampoline code as an ordinary 64-bit function from C code. > > > > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx> > > --- > > arch/x86/boot/compressed/head_64.S | 34 ++++---------------- > > arch/x86/boot/compressed/pgtable.h | 6 ++-- > > arch/x86/boot/compressed/pgtable_64.c | 12 ++++++- > > 3 files changed, 21 insertions(+), 31 deletions(-) > > > > diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S > > index b1f8a867777120bb..3b5fc851737ffc39 100644 > > --- a/arch/x86/boot/compressed/head_64.S > > +++ b/arch/x86/boot/compressed/head_64.S > > @@ -460,9 +460,6 @@ SYM_CODE_START(startup_64) > > leaq TRAMPOLINE_32BIT_CODE_OFFSET(%rax), %rax > > call *%rax > > > > - /* Restore the stack, the 32-bit trampoline uses its own stack */ > > - leaq rva(boot_stack_end)(%rbx), %rsp > > - > > /* > > * cleanup_trampoline() would restore trampoline memory. > > * > > @@ -563,24 +560,17 @@ SYM_FUNC_END(.Lrelocated) > > * EDI contains the base address of the trampoline memory. > > * Non-zero ESI means trampoline needs to enable 5-level paging. > > */ > > + .section ".rodata", "a", @progbits > > SYM_CODE_START(trampoline_32bit_src) > > - popq %r8 > > /* Switch to compatibility mode (CS.L = 0 CS.D = 1) via far return */ > > pushq $__KERNEL32_CS > > leaq 0f(%rip), %rax > > pushq %rax > > lretq > > +.Lret: retq > > Maybe just add a comment above this to explain that this is a target of > the long jump below to get back into long mode and be able to return > without setting up a new stack for the 32-bit code. > > And then a corresponding comment on the long jump itself. I think it would > make it easier to understand what is going on in this part of the code. > Fair point. I'll add that in the next version.
