On Sun, 2 Feb 2020 at 18:13, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote: > > At handover entry in efi32_stub_entry, the firmware's GDT is still > installed. We save the GDTR for later use in __efi64_thunk but we are > assuming that descriptor 2 (__KERNEL_CS) is a valid 32-bit code segment > descriptor and that descriptor 3 (__KERNEL_DS/__BOOT_DS) is a valid data > segment descriptor. > > This happens to be true for OVMF (it actually uses descriptor 1 for data > segments, but descriptor 3 is also setup as data), but we shouldn't > depend on this being the case. > > Fix this by saving the code and data selectors in addition to the GDTR > in efi32_stub_entry, and restoring them in __efi64_thunk before calling > the firmware. The UEFI specification guarantees that selectors will be > flat, so using the DS selector for all the segment registers should be > enough. > > We also need to install our own GDT before initializing segment > registers in startup_32, so move the GDT load up to the beginning of the > function. > > Signed-off-by: Arvind Sankar <nivedita@xxxxxxxxxxxx> It might be useful to mention /somewhere/ in the commit log that this applies to mixed mode > --- > arch/x86/boot/compressed/efi_thunk_64.S | 29 +++++++++++++++++++----- > arch/x86/boot/compressed/head_64.S | 30 +++++++++++++++---------- > 2 files changed, 42 insertions(+), 17 deletions(-) > > diff --git a/arch/x86/boot/compressed/efi_thunk_64.S b/arch/x86/boot/compressed/efi_thunk_64.S > index 8fb7f6799c52..2b2049259619 100644 > --- a/arch/x86/boot/compressed/efi_thunk_64.S > +++ b/arch/x86/boot/compressed/efi_thunk_64.S > @@ -54,11 +54,16 @@ SYM_FUNC_START(__efi64_thunk) > * Switch to gdt with 32-bit segments. This is the firmware GDT > * that was installed when the kernel started executing. This > * pointer was saved at the EFI stub entry point in head_64.S. > + * > + * Pass the saved DS selector to the 32-bit code, and use far return to > + * restore the saved CS selector. > */ > leaq efi32_boot_gdt(%rip), %rax > lgdt (%rax) > > - pushq $__KERNEL_CS > + movzwl efi32_boot_ds(%rip), %edx > + movzwq efi32_boot_cs(%rip), %rax > + pushq %rax > leaq efi_enter32(%rip), %rax > pushq %rax > lretq > @@ -73,6 +78,10 @@ SYM_FUNC_START(__efi64_thunk) > movl %ebx, %es > pop %rbx > movl %ebx, %ds > + /* Clear out 32-bit selector from FS and GS */ > + xorl %ebx, %ebx > + movl %ebx, %fs > + movl %ebx, %gs > > /* > * Convert 32-bit status code into 64-bit. > @@ -92,10 +101,12 @@ SYM_FUNC_END(__efi64_thunk) > * The stack should represent the 32-bit calling convention. > */ > SYM_FUNC_START_LOCAL(efi_enter32) > - movl $__KERNEL_DS, %eax > - movl %eax, %ds > - movl %eax, %es > - movl %eax, %ss > + /* Load firmware selector into data and stack segment registers */ > + movl %edx, %ds > + movl %edx, %es > + movl %edx, %fs > + movl %edx, %gs > + movl %edx, %ss > > /* Reload pgtables */ > movl %cr3, %eax > @@ -157,6 +168,14 @@ SYM_DATA_START(efi32_boot_gdt) > .quad 0 > SYM_DATA_END(efi32_boot_gdt) > > +SYM_DATA_START(efi32_boot_cs) > + .word 0 > +SYM_DATA_END(efi32_boot_cs) > + > +SYM_DATA_START(efi32_boot_ds) > + .word 0 > +SYM_DATA_END(efi32_boot_ds) > + > SYM_DATA_START(efi_gdt64) > .word efi_gdt64_end - efi_gdt64 > .long 0 /* Filled out by user */ > diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S > index bd44d89540d3..c56b30bd9c7b 100644 > --- a/arch/x86/boot/compressed/head_64.S > +++ b/arch/x86/boot/compressed/head_64.S > @@ -54,10 +54,6 @@ SYM_FUNC_START(startup_32) > */ > cld > cli > - movl $(__BOOT_DS), %eax > - movl %eax, %ds > - movl %eax, %es > - movl %eax, %ss > > /* > * Calculate the delta between where we were compiled to run > @@ -72,10 +68,20 @@ SYM_FUNC_START(startup_32) > 1: popl %ebp > subl $1b, %ebp > > + /* Load new GDT with the 64bit segments using 32bit descriptor */ > + addl %ebp, gdt+2(%ebp) > + lgdt gdt(%ebp) > + > + /* Load segment registers with our descriptors */ > + movl $__BOOT_DS, %eax > + movl %eax, %ds > + movl %eax, %es > + movl %eax, %fs > + movl %eax, %gs > + movl %eax, %ss > + > /* setup a stack and make sure cpu supports long mode. */ > - movl $boot_stack_end, %eax > - addl %ebp, %eax > - movl %eax, %esp > + leal boot_stack_end(%ebp), %esp > > call verify_cpu > testl %eax, %eax > @@ -112,10 +118,6 @@ SYM_FUNC_START(startup_32) > * Prepare for entering 64 bit mode > */ > > - /* Load new GDT with the 64bit segments using 32bit descriptor */ > - addl %ebp, gdt+2(%ebp) > - lgdt gdt(%ebp) > - > /* Enable PAE mode */ > movl %cr4, %eax > orl $X86_CR4_PAE, %eax > @@ -232,9 +234,13 @@ SYM_FUNC_START(efi32_stub_entry) > > movl %ecx, efi32_boot_args(%ebp) > movl %edx, efi32_boot_args+4(%ebp) > - sgdtl efi32_boot_gdt(%ebp) > movb $0, efi_is64(%ebp) > > + /* Save firmware GDTR and code/data selectors */ > + sgdtl efi32_boot_gdt(%ebp) > + movw %cs, efi32_boot_cs(%ebp) > + movw %ds, efi32_boot_ds(%ebp) > + > /* Disable paging */ > movl %cr0, %eax > btrl $X86_CR0_PG_BIT, %eax > -- > 2.24.1 >
