diff mbox series

[4/6] x86/smpbppt: Remove early_gdt_descr on 64-bit

Message ID 20230222221301.245890-5-brgerst@gmail.com
State New
Headers
Series x86-64: Remove global variables from boot |

Commit Message

Brian Gerst Feb. 22, 2023, 10:12 p.m. UTC 
  Build the GDT descriptor on the stack instead.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
---
 arch/x86/kernel/acpi/sleep.c |  2 --
 arch/x86/kernel/head_64.S    | 19 +++++++------------
 2 files changed, 7 insertions(+), 14 deletions(-)

Comments

H. Peter Anvin Feb. 23, 2023, 6:49 a.m. UTC | #1 
On February 22, 2023 2:12:59 PM PST, Brian Gerst <brgerst@gmail.com> wrote:
>Build the GDT descriptor on the stack instead.
>
>Signed-off-by: Brian Gerst <brgerst@gmail.com>
>---
> arch/x86/kernel/acpi/sleep.c |  2 --
> arch/x86/kernel/head_64.S    | 19 +++++++------------
> 2 files changed, 7 insertions(+), 14 deletions(-)
>
>diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
>index 008fda8b1982..6538ddb55f28 100644
>--- a/arch/x86/kernel/acpi/sleep.c
>+++ b/arch/x86/kernel/acpi/sleep.c
>@@ -114,8 +114,6 @@ int x86_acpi_suspend_lowlevel(void)
> #else /* CONFIG_64BIT */
> #ifdef CONFIG_SMP
> 	current->thread.sp = (unsigned long)temp_stack + sizeof(temp_stack);
>-	early_gdt_descr.address =
>-			(unsigned long)get_cpu_gdt_rw(smp_processor_id());
> 	initial_gs = per_cpu_offset(smp_processor_id());
> 	/* Force the startup into boot mode */
> 	saved_smpboot_ctrl = xchg(&smpboot_control, 0);
>diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
>index f7905ba4b992..0dd57d573a0e 100644
>--- a/arch/x86/kernel/head_64.S
>+++ b/arch/x86/kernel/head_64.S
>@@ -245,7 +245,7 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
> #ifdef CONFIG_SMP
> 	/*
> 	 * Is this the boot CPU coming up? If so everything is available
>-	 * in initial_gs and early_gdt_descr.
>+	 * in initial_gs.
> 	 */
> 	movl	smpboot_control(%rip), %edx
> 	testl	$STARTUP_SECONDARY, %edx
>@@ -313,11 +313,6 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
> 	/* Save it for GS BASE setup */
> 	movq	%rbx, initial_gs(%rip)
> 
>-	/* Calculate the GDT address */
>-	movq	$gdt_page, %rcx
>-	addq	%rbx, %rcx
>-	movq	%rcx, early_gdt_descr_base(%rip)
>-
> 	movq	%rbx, %rdx
> #else
> 	xorl	%edx, %edx
>@@ -339,7 +334,12 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
> 	 * addresses where we're currently running on. We have to do that here
> 	 * because in 32bit we couldn't load a 64bit linear address.
> 	 */
>-	lgdt	early_gdt_descr(%rip)
>+	subq	$16, %rsp
>+	movw	$(GDT_SIZE-1), (%rsp)
>+	leaq	gdt_page(%rdx), %rax
>+	movq	%rax, 2(%rsp)
>+	lgdt	(%rsp)
>+	addq	$16, %rsp
> 
> 	/* set up data segments */
> 	xorl %eax,%eax
>@@ -754,11 +754,6 @@ SYM_DATA_END(level1_fixmap_pgt)
> 
> 	.data
> 	.align 16
>-
>-SYM_DATA(early_gdt_descr,		.word GDT_ENTRIES*8-1)
>-SYM_DATA_LOCAL(early_gdt_descr_base,	.quad INIT_PER_CPU_VAR(gdt_page))
>-
>-	.align 16
> SYM_DATA(smpboot_control,		.long 0)
> 
> 	.align 16

This is known to break at least some hypervisors, probably old by now, but...
Brian Gerst Feb. 23, 2023, 12:10 p.m. UTC | #2 
On Thu, Feb 23, 2023 at 1:50 AM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On February 22, 2023 2:12:59 PM PST, Brian Gerst <brgerst@gmail.com> wrote:
> >Build the GDT descriptor on the stack instead.
> >
> >Signed-off-by: Brian Gerst <brgerst@gmail.com>
> >---
> > arch/x86/kernel/acpi/sleep.c |  2 --
> > arch/x86/kernel/head_64.S    | 19 +++++++------------
> > 2 files changed, 7 insertions(+), 14 deletions(-)
> >
> >diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
> >index 008fda8b1982..6538ddb55f28 100644
> >--- a/arch/x86/kernel/acpi/sleep.c
> >+++ b/arch/x86/kernel/acpi/sleep.c
> >@@ -114,8 +114,6 @@ int x86_acpi_suspend_lowlevel(void)
> > #else /* CONFIG_64BIT */
> > #ifdef CONFIG_SMP
> >       current->thread.sp = (unsigned long)temp_stack + sizeof(temp_stack);
> >-      early_gdt_descr.address =
> >-                      (unsigned long)get_cpu_gdt_rw(smp_processor_id());
> >       initial_gs = per_cpu_offset(smp_processor_id());
> >       /* Force the startup into boot mode */
> >       saved_smpboot_ctrl = xchg(&smpboot_control, 0);
> >diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> >index f7905ba4b992..0dd57d573a0e 100644
> >--- a/arch/x86/kernel/head_64.S
> >+++ b/arch/x86/kernel/head_64.S
> >@@ -245,7 +245,7 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
> > #ifdef CONFIG_SMP
> >       /*
> >        * Is this the boot CPU coming up? If so everything is available
> >-       * in initial_gs and early_gdt_descr.
> >+       * in initial_gs.
> >        */
> >       movl    smpboot_control(%rip), %edx
> >       testl   $STARTUP_SECONDARY, %edx
> >@@ -313,11 +313,6 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
> >       /* Save it for GS BASE setup */
> >       movq    %rbx, initial_gs(%rip)
> >
> >-      /* Calculate the GDT address */
> >-      movq    $gdt_page, %rcx
> >-      addq    %rbx, %rcx
> >-      movq    %rcx, early_gdt_descr_base(%rip)
> >-
> >       movq    %rbx, %rdx
> > #else
> >       xorl    %edx, %edx
> >@@ -339,7 +334,12 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
> >        * addresses where we're currently running on. We have to do that here
> >        * because in 32bit we couldn't load a 64bit linear address.
> >        */
> >-      lgdt    early_gdt_descr(%rip)
> >+      subq    $16, %rsp
> >+      movw    $(GDT_SIZE-1), (%rsp)
> >+      leaq    gdt_page(%rdx), %rax
> >+      movq    %rax, 2(%rsp)
> >+      lgdt    (%rsp)
> >+      addq    $16, %rsp
> >
> >       /* set up data segments */
> >       xorl %eax,%eax
> >@@ -754,11 +754,6 @@ SYM_DATA_END(level1_fixmap_pgt)
> >
> >       .data
> >       .align 16
> >-
> >-SYM_DATA(early_gdt_descr,             .word GDT_ENTRIES*8-1)
> >-SYM_DATA_LOCAL(early_gdt_descr_base,  .quad INIT_PER_CPU_VAR(gdt_page))
> >-
> >-      .align 16
> > SYM_DATA(smpboot_control,             .long 0)
> >
> >       .align 16
>
> This is known to break at least some hypervisors, probably old by now, but...

I remember some ancient versions of Xen stored the address of the
descriptor, instead of the contents.  But looking at load_direct_gdt()
and load_fixmap_gdt() which both load a descriptor on the stack, I
don't think that is still a concern.

--
Brian Gerst
diff mbox series

Patch

diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 008fda8b1982..6538ddb55f28 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -114,8 +114,6 @@  int x86_acpi_suspend_lowlevel(void)
 #else /* CONFIG_64BIT */
 #ifdef CONFIG_SMP
 	current->thread.sp = (unsigned long)temp_stack + sizeof(temp_stack);
-	early_gdt_descr.address =
-			(unsigned long)get_cpu_gdt_rw(smp_processor_id());
 	initial_gs = per_cpu_offset(smp_processor_id());
 	/* Force the startup into boot mode */
 	saved_smpboot_ctrl = xchg(&smpboot_control, 0);
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index f7905ba4b992..0dd57d573a0e 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -245,7 +245,7 @@  SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 #ifdef CONFIG_SMP
 	/*
 	 * Is this the boot CPU coming up? If so everything is available
-	 * in initial_gs and early_gdt_descr.
+	 * in initial_gs.
 	 */
 	movl	smpboot_control(%rip), %edx
 	testl	$STARTUP_SECONDARY, %edx
@@ -313,11 +313,6 @@  SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	/* Save it for GS BASE setup */
 	movq	%rbx, initial_gs(%rip)
 
-	/* Calculate the GDT address */
-	movq	$gdt_page, %rcx
-	addq	%rbx, %rcx
-	movq	%rcx, early_gdt_descr_base(%rip)
-
 	movq	%rbx, %rdx
 #else
 	xorl	%edx, %edx
@@ -339,7 +334,12 @@  SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	 * addresses where we're currently running on. We have to do that here
 	 * because in 32bit we couldn't load a 64bit linear address.
 	 */
-	lgdt	early_gdt_descr(%rip)
+	subq	$16, %rsp
+	movw	$(GDT_SIZE-1), (%rsp)
+	leaq	gdt_page(%rdx), %rax
+	movq	%rax, 2(%rsp)
+	lgdt	(%rsp)
+	addq	$16, %rsp
 
 	/* set up data segments */
 	xorl %eax,%eax
@@ -754,11 +754,6 @@  SYM_DATA_END(level1_fixmap_pgt)
 
 	.data
 	.align 16
-
-SYM_DATA(early_gdt_descr,		.word GDT_ENTRIES*8-1)
-SYM_DATA_LOCAL(early_gdt_descr_base,	.quad INIT_PER_CPU_VAR(gdt_page))
-
-	.align 16
 SYM_DATA(smpboot_control,		.long 0)
 
 	.align 16