linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arvind Sankar <nivedita@alum.mit.edu>
To: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Ard Biesheuvel <ardb@kernel.org>
Cc: linux-efi@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 2/7] efi/x86: Don't depend on firmware GDT layout
Date: Sun,  2 Feb 2020 12:13:48 -0500	[thread overview]
Message-ID: <20200202171353.3736319-3-nivedita@alum.mit.edu> (raw)
In-Reply-To: <20200202171353.3736319-1-nivedita@alum.mit.edu>

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@alum.mit.edu>
---
 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


  parent reply	other threads:[~2020-02-02 17:14 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-30 20:04 [PATCH 0/8] Remove 64-bit GDT setup in efi_main + doc fixes Arvind Sankar
2020-01-30 20:04 ` [PATCH 1/8] efi/x86: Use C wrapper instead of inline assembly Arvind Sankar
2020-01-30 20:04 ` [PATCH 2/8] efi/x86: Allocate the GDT pointer on the stack Arvind Sankar
2020-01-30 20:04 ` [PATCH 3/8] efi/x86: Factor GDT setup code into a function Arvind Sankar
2020-01-30 20:04 ` [PATCH 4/8] efi/x86: Only setup the GDT for 32-bit kernel Arvind Sankar
2020-01-30 20:04 ` [PATCH 5/8] efi/x86: Allocate only the required 32 bytes for the GDT Arvind Sankar
2020-01-30 20:04 ` [PATCH 6/8] efi/x86: Change __KERNEL_{CS,DS} to __BOOT_{CS,DS} Arvind Sankar
2020-01-30 20:04 ` [PATCH 7/8] Documentation/x86/boot: Clarify segment requirements for EFI handover Arvind Sankar
2020-01-31 19:24   ` Arvind Sankar
2020-01-30 20:04 ` [PATCH 8/8] Documentation/x86/boot: Correct segment requirements for 64-bit boot Arvind Sankar
2020-01-31  8:42 ` [PATCH 0/8] Remove 64-bit GDT setup in efi_main + doc fixes Ard Biesheuvel
2020-01-31  9:31   ` Ard Biesheuvel
2020-01-31 19:10     ` Arvind Sankar
2020-02-02 17:13 ` [PATCH v2 0/7] x86/efi,boot: GDT handling cleanup/fixes Arvind Sankar
2020-02-02 17:13   ` [PATCH v2 1/7] x86/boot: Remove KEEP_SEGMENTS support Arvind Sankar
2020-02-02 17:13   ` Arvind Sankar [this message]
2020-02-02 17:54     ` [PATCH v2 2/7] efi/x86: Don't depend on firmware GDT layout Ard Biesheuvel
2020-02-02 18:18       ` Arvind Sankar
2020-02-02 17:13   ` [PATCH v2 3/7] x86/boot: Reload GDTR after copying to the end of the buffer Arvind Sankar
2020-02-02 17:13   ` [PATCH v2 4/7] x86/boot: Clear direction and interrupt flags in startup_64 Arvind Sankar
2020-02-02 17:13   ` [PATCH v2 5/7] efi/x86: Remove GDT setup from efi_main Arvind Sankar
2020-02-02 17:13   ` [PATCH v2 6/7] x86/boot: GDT limit value should be size - 1 Arvind Sankar
2020-02-02 17:13   ` [PATCH v2 7/7] x86/boot: Micro-optimize GDT loading instructions Arvind Sankar
2020-02-02 18:01   ` [PATCH v2 0/7] x86/efi,boot: GDT handling cleanup/fixes Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200202171353.3736319-3-nivedita@alum.mit.edu \
    --to=nivedita@alum.mit.edu \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).