2024-04-24 16:04:53

by Ard Biesheuvel

[permalink] [raw]
Subject: [RFC PATCH 2/9] x86/purgatory: Simplify stack handling

From: Ard Biesheuvel <[email protected]>

The x86 purgatory, which does little more than verify a SHA-256 hash of
the loaded segments, currently uses three different stacks:
- one in .bss that is used to call the purgatory C code
- one in .rodata that is only used to switch to an updated code segment
descriptor in the GDT
- one in .data, which allows it to be prepopulated from the kexec loader
in theory, but this is not actually being taken advantage of.

Simplify this, by dropping the latter two stacks, as well as the loader
logic that programs RSP.

Both the stacks in .bss and .data are 4k aligned, but 16 byte alignment
is more than sufficient.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/include/asm/kexec.h | 1 -
arch/x86/kernel/kexec-bzimage64.c | 8 --------
arch/x86/purgatory/entry64.S | 8 --------
arch/x86/purgatory/setup-x86_64.S | 2 +-
arch/x86/purgatory/stack.S | 18 ------------------
5 files changed, 1 insertion(+), 36 deletions(-)

diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index 91ca9a9ee3a2..ee7b32565e5f 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -163,7 +163,6 @@ struct kexec_entry64_regs {
uint64_t rcx;
uint64_t rdx;
uint64_t rbx;
- uint64_t rsp;
uint64_t rbp;
uint64_t rsi;
uint64_t rdi;
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index cde167b0ea92..f5bf1b7d01a6 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -400,7 +400,6 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
unsigned long bootparam_load_addr, kernel_load_addr, initrd_load_addr;
struct bzimage64_data *ldata;
struct kexec_entry64_regs regs64;
- void *stack;
unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr);
unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset;
struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
@@ -550,14 +549,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
regs64.rbx = 0; /* Bootstrap Processor */
regs64.rsi = bootparam_load_addr;
regs64.rip = kernel_load_addr + 0x200;
- stack = kexec_purgatory_get_symbol_addr(image, "stack_end");
- if (IS_ERR(stack)) {
- pr_err("Could not find address of symbol stack_end\n");
- ret = -EINVAL;
- goto out_free_params;
- }

- regs64.rsp = (unsigned long)stack;
ret = kexec_purgatory_get_set_symbol(image, "entry64_regs", &regs64,
sizeof(regs64), 0);
if (ret)
diff --git a/arch/x86/purgatory/entry64.S b/arch/x86/purgatory/entry64.S
index 0b4390ce586b..9913877b0dbe 100644
--- a/arch/x86/purgatory/entry64.S
+++ b/arch/x86/purgatory/entry64.S
@@ -26,8 +26,6 @@ SYM_CODE_START(entry64)
movl %eax, %fs
movl %eax, %gs

- /* Setup new stack */
- leaq stack_init(%rip), %rsp
pushq $0x10 /* CS */
leaq new_cs_exit(%rip), %rax
pushq %rax
@@ -41,7 +39,6 @@ new_cs_exit:
movq rdx(%rip), %rdx
movq rsi(%rip), %rsi
movq rdi(%rip), %rdi
- movq rsp(%rip), %rsp
movq rbp(%rip), %rbp
movq r8(%rip), %r8
movq r9(%rip), %r9
@@ -63,7 +60,6 @@ rax: .quad 0x0
rcx: .quad 0x0
rdx: .quad 0x0
rbx: .quad 0x0
-rsp: .quad 0x0
rbp: .quad 0x0
rsi: .quad 0x0
rdi: .quad 0x0
@@ -97,7 +93,3 @@ SYM_DATA_START_LOCAL(gdt)
/* 0x18 4GB flat data segment */
.word 0xFFFF, 0x0000, 0x9200, 0x00CF
SYM_DATA_END_LABEL(gdt, SYM_L_LOCAL, gdt_end)
-
-SYM_DATA_START_LOCAL(stack)
- .quad 0, 0
-SYM_DATA_END_LABEL(stack, SYM_L_LOCAL, stack_init)
diff --git a/arch/x86/purgatory/setup-x86_64.S b/arch/x86/purgatory/setup-x86_64.S
index 89d9e9e53fcd..2d10ff88851d 100644
--- a/arch/x86/purgatory/setup-x86_64.S
+++ b/arch/x86/purgatory/setup-x86_64.S
@@ -53,7 +53,7 @@ SYM_DATA_START_LOCAL(gdt)
SYM_DATA_END_LABEL(gdt, SYM_L_LOCAL, gdt_end)

.bss
- .balign 4096
+ .balign 16
SYM_DATA_START_LOCAL(lstack)
.skip 4096
SYM_DATA_END_LABEL(lstack, SYM_L_LOCAL, lstack_end)
diff --git a/arch/x86/purgatory/stack.S b/arch/x86/purgatory/stack.S
deleted file mode 100644
index 1ef507ca50a5..000000000000
--- a/arch/x86/purgatory/stack.S
+++ /dev/null
@@ -1,18 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * purgatory: stack
- *
- * Copyright (C) 2014 Red Hat Inc.
- */
-
-#include <linux/linkage.h>
-
- /* A stack for the loaded kernel.
- * Separate and in the data section so it can be prepopulated.
- */
- .data
- .balign 4096
-
-SYM_DATA_START(stack)
- .skip 4096
-SYM_DATA_END_LABEL(stack, SYM_L_GLOBAL, stack_end)
--
2.44.0.769.g3c40516874-goog



2024-04-24 18:32:17

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [RFC PATCH 2/9] x86/purgatory: Simplify stack handling

On Wed, Apr 24, 2024 at 05:53:12PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <[email protected]>
>
> The x86 purgatory, which does little more than verify a SHA-256 hash of
> the loaded segments, currently uses three different stacks:
> - one in .bss that is used to call the purgatory C code
> - one in .rodata that is only used to switch to an updated code segment
> descriptor in the GDT
> - one in .data, which allows it to be prepopulated from the kexec loader
> in theory, but this is not actually being taken advantage of.
>
> Simplify this, by dropping the latter two stacks, as well as the loader
> logic that programs RSP.
>
> Both the stacks in .bss and .data are 4k aligned, but 16 byte alignment
> is more than sufficient.
>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> arch/x86/include/asm/kexec.h | 1 -
> arch/x86/kernel/kexec-bzimage64.c | 8 --------
> arch/x86/purgatory/entry64.S | 8 --------
> arch/x86/purgatory/setup-x86_64.S | 2 +-
> arch/x86/purgatory/stack.S | 18 ------------------

This needs a small fix up to build.

make[6]: *** No rule to make target 'arch/x86/purgatory/stack.o', needed by 'arch/x86/purgatory/purgatory.ro'.

diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index acc09799af2a..2b6b2fb033d6 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
OBJECT_FILES_NON_STANDARD := y

-purgatory-y := purgatory.o stack.o setup-x86_$(BITS).o sha256.o entry64.o string.o
+purgatory-y := purgatory.o setup-x86_$(BITS).o sha256.o entry64.o string.o

targets += $(purgatory-y)
PURGATORY_OBJS = $(addprefix $(obj)/,$(purgatory-y))

2024-04-26 21:33:04

by Justin Stitt

[permalink] [raw]
Subject: Re: [RFC PATCH 2/9] x86/purgatory: Simplify stack handling

Hi,

On Wed, Apr 24, 2024 at 11:26:59AM -0700, Nathan Chancellor wrote:
> On Wed, Apr 24, 2024 at 05:53:12PM +0200, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <[email protected]>
> >
> > The x86 purgatory, which does little more than verify a SHA-256 hash of
> > the loaded segments, currently uses three different stacks:
> > - one in .bss that is used to call the purgatory C code
> > - one in .rodata that is only used to switch to an updated code segment
> > descriptor in the GDT
> > - one in .data, which allows it to be prepopulated from the kexec loader
> > in theory, but this is not actually being taken advantage of.
> >
> > Simplify this, by dropping the latter two stacks, as well as the loader
> > logic that programs RSP.
> >
> > Both the stacks in .bss and .data are 4k aligned, but 16 byte alignment
> > is more than sufficient.
> >
> > Signed-off-by: Ard Biesheuvel <[email protected]>
> > ---
> > arch/x86/include/asm/kexec.h | 1 -
> > arch/x86/kernel/kexec-bzimage64.c | 8 --------
> > arch/x86/purgatory/entry64.S | 8 --------
> > arch/x86/purgatory/setup-x86_64.S | 2 +-
> > arch/x86/purgatory/stack.S | 18 ------------------
>
> This needs a small fix up to build.
>
> make[6]: *** No rule to make target 'arch/x86/purgatory/stack.o', needed by 'arch/x86/purgatory/purgatory.ro'.
>

I was trying to reproduce this build failure, but to no avail. I am
curious what your build target / build command was.

It is clear that stack.S has been removed so your change makes sense, I
don't doubt that -- I just cannot get that specific error message you
encountered (what is a .ro file supposed to be, anyway?).

> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> index acc09799af2a..2b6b2fb033d6 100644
> --- a/arch/x86/purgatory/Makefile
> +++ b/arch/x86/purgatory/Makefile
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
> OBJECT_FILES_NON_STANDARD := y
>
> -purgatory-y := purgatory.o stack.o setup-x86_$(BITS).o sha256.o entry64.o string.o
> +purgatory-y := purgatory.o setup-x86_$(BITS).o sha256.o entry64.o string.o
>
> targets += $(purgatory-y)
> PURGATORY_OBJS = $(addprefix $(obj)/,$(purgatory-y))

Thanks
Justin

2024-04-26 21:54:02

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [RFC PATCH 2/9] x86/purgatory: Simplify stack handling

On Fri, Apr 26, 2024 at 09:32:52PM +0000, Justin Stitt wrote:
> Hi,
>
> On Wed, Apr 24, 2024 at 11:26:59AM -0700, Nathan Chancellor wrote:
> > On Wed, Apr 24, 2024 at 05:53:12PM +0200, Ard Biesheuvel wrote:
> > > arch/x86/purgatory/stack.S | 18 ------------------
> >
> > This needs a small fix up to build.
> >
> > make[6]: *** No rule to make target 'arch/x86/purgatory/stack.o', needed by 'arch/x86/purgatory/purgatory.ro'.
> >
>
> I was trying to reproduce this build failure, but to no avail. I am
> curious what your build target / build command was.
>
> It is clear that stack.S has been removed so your change makes sense, I
> don't doubt that -- I just cannot get that specific error message you

Odd, I was using my distribution configuration for the test but it is
easily reproducible with allmodconfig:

$ make -skj"$(nproc)" ARCH=x86_64 LLVM=1 allmodconfig arch/x86/purgatory/
make[6]: *** No rule to make target 'arch/x86/purgatory/stack.o', needed by 'arch/x86/purgatory/purgatory.ro'.
...

> encountered (what is a .ro file supposed to be, anyway?).

Read only? Relocatable object? *shrug*

Cheers,
Nathan

2024-04-26 22:02:20

by Justin Stitt

[permalink] [raw]
Subject: Re: [RFC PATCH 2/9] x86/purgatory: Simplify stack handling

On Fri, Apr 26, 2024 at 2:53 PM Nathan Chancellor <[email protected]> wrote:
>
> On Fri, Apr 26, 2024 at 09:32:52PM +0000, Justin Stitt wrote:
> > Hi,
> >
> > On Wed, Apr 24, 2024 at 11:26:59AM -0700, Nathan Chancellor wrote:
> > > On Wed, Apr 24, 2024 at 05:53:12PM +0200, Ard Biesheuvel wrote:
> > > > arch/x86/purgatory/stack.S | 18 ------------------
> > >
> > > This needs a small fix up to build.
> > >
> > > make[6]: *** No rule to make target 'arch/x86/purgatory/stack.o', needed by 'arch/x86/purgatory/purgatory.ro'.
> > >
> >
> > I was trying to reproduce this build failure, but to no avail. I am
> > curious what your build target / build command was.
> >
> > It is clear that stack.S has been removed so your change makes sense, I
> > don't doubt that -- I just cannot get that specific error message you
>
> Odd, I was using my distribution configuration for the test but it is
> easily reproducible with allmodconfig:
>
> $ make -skj"$(nproc)" ARCH=x86_64 LLVM=1 allmodconfig arch/x86/purgatory/
> make[6]: *** No rule to make target 'arch/x86/purgatory/stack.o', needed by 'arch/x86/purgatory/purgatory.ro'.
> ...

Agh, I was just doing a defconfig followed by a menuconfig to manually
enable all the kexec and purgatory stuff. I wonder which one I missed.

allyes/allmodconfig is what I needed here :thumbs_up:

>
> > encountered (what is a .ro file supposed to be, anyway?).
>
> Read only? Relocatable object? *shrug*
>
> Cheers,
> Nathan