From: Joerg Roedel <[email protected]>
Hi,
these patches add support for the 32-bit boot in the decompressor
code. This is needed to boot an SEV-ES guest on some firmware and grub
versions. The patches also add the necessary CPUID sanity checks and a
32-bit version of the C-bit check.
Other updates included here:
1. Add code to shut down exception handling in the
decompressor code before jumping to the real kernel.
Once in the real kernel it is not safe anymore to jump
back to the decompressor code via exceptions.
2. Replace open-coded hlt loops with proper calls to
sev_es_terminate().
Please review.
Thanks,
Joerg
Joerg Roedel (7):
x86/boot/compressed/64: Cleanup exception handling before booting
kernel
x86/boot/compressed/64: Reload CS in startup_32
x86/boot/compressed/64: Setup IDT in startup_32 boot path
x86/boot/compressed/64: Add 32-bit boot #VC handler
x86/boot/compressed/64: Add CPUID sanity check to 32-bit boot-path
x86/boot/compressed/64: Check SEV encryption in 32-bit boot-path
x86/sev-es: Replace open-coded hlt-loops with sev_es_terminate()
arch/x86/boot/compressed/head_64.S | 168 ++++++++++++++++++++++++-
arch/x86/boot/compressed/idt_64.c | 14 +++
arch/x86/boot/compressed/mem_encrypt.S | 114 ++++++++++++++++-
arch/x86/boot/compressed/misc.c | 7 +-
arch/x86/boot/compressed/misc.h | 6 +
arch/x86/boot/compressed/sev-es.c | 12 +-
arch/x86/kernel/sev-es-shared.c | 10 +-
7 files changed, 307 insertions(+), 24 deletions(-)
--
2.30.0
From: Joerg Roedel <[email protected]>
Add a #VC exception handler which is used when the kernel still executes
in protected mode. This boot-path already uses CPUID, which will cause #VC
exceptions in an SEV-ES guest.
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/boot/compressed/head_64.S | 6 ++
arch/x86/boot/compressed/mem_encrypt.S | 77 +++++++++++++++++++++++++-
2 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 8deeec78cdb4..eadaa0a082b8 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -34,6 +34,7 @@
#include <asm/asm-offsets.h>
#include <asm/bootparam.h>
#include <asm/desc_defs.h>
+#include <asm/trapnr.h>
#include "pgtable.h"
/*
@@ -856,6 +857,11 @@ SYM_FUNC_START(startup32_set_idt_entry)
SYM_FUNC_END(startup32_set_idt_entry)
SYM_FUNC_START(startup32_load_idt)
+ /* #VC handler */
+ leal rva(startup32_vc_handler)(%ebp), %eax
+ movl $X86_TRAP_VC, %edx
+ call startup32_set_idt_entry
+
/* Load IDT */
leal rva(boot32_idt)(%ebp), %eax
movl %eax, rva(boot32_idt_desc+2)(%ebp)
diff --git a/arch/x86/boot/compressed/mem_encrypt.S b/arch/x86/boot/compressed/mem_encrypt.S
index aa561795efd1..350ecb56c7e4 100644
--- a/arch/x86/boot/compressed/mem_encrypt.S
+++ b/arch/x86/boot/compressed/mem_encrypt.S
@@ -67,10 +67,85 @@ SYM_FUNC_START(get_sev_encryption_bit)
ret
SYM_FUNC_END(get_sev_encryption_bit)
+/*
+ * Emit code to request an CPUID register from the Hypervisor using
+ * the MSR-based protocol.
+ *
+ * fn: The register containing the CPUID function
+ * reg: Register requested
+ * 1 = EAX
+ * 2 = EBX
+ * 3 = ECX
+ * 4 = EDX
+ *
+ * Result is in EDX. Jumps to .Lfail on error
+ */
+.macro SEV_ES_REQ_CPUID fn:req reg:req
+ /* Request CPUID[%ebx].EAX */
+ movl $\reg, %eax
+ shll $30, %eax
+ orl $0x00000004, %eax
+ movl \fn, %edx
+ movl $MSR_AMD64_SEV_ES_GHCB, %ecx
+ wrmsr
+ rep; vmmcall
+ rdmsr
+ /* Check response code */
+ andl $0xfff, %eax
+ cmpl $5, %eax
+ jne .Lfail
+ /* All good */
+.endm
+
+SYM_CODE_START(startup32_vc_handler)
+ pushl %eax
+ pushl %ebx
+ pushl %ecx
+ pushl %edx
+
+ /* Keep CPUID function in %ebx */
+ movl %eax, %ebx
+
+ /* Check if error-code == SVM_EXIT_CPUID */
+ cmpl $0x72, 16(%esp)
+ jne .Lfail
+
+ /* Request CPUID[%ebx].EAX */
+ SEV_ES_REQ_CPUID fn=%ebx reg=0
+ movl %edx, 12(%esp)
+
+ /* Request CPUID[%ebx].EBX */
+ SEV_ES_REQ_CPUID fn=%ebx reg=1
+ movl %edx, 8(%esp)
+
+ /* Request CPUID[%ebx].ECX */
+ SEV_ES_REQ_CPUID fn=%ebx reg=2
+ movl %edx, 4(%esp)
+
+ /* Request CPUID[%ebx].EDX */
+ SEV_ES_REQ_CPUID fn=%ebx reg=3
+ movl %edx, (%esp)
+
+ popl %edx
+ popl %ecx
+ popl %ebx
+ popl %eax
+
+ /* Remove error code */
+ addl $4, %esp
+
+ /* Jump over CPUID instruction */
+ addl $2, (%esp)
+
+ iret
+.Lfail:
+ hlt
+ jmp .Lfail
+SYM_CODE_END(startup32_vc_handler)
+
.code64
#include "../../kernel/sev_verify_cbit.S"
-
SYM_FUNC_START(set_sev_encryption_mask)
#ifdef CONFIG_AMD_MEM_ENCRYPT
push %rbp
--
2.30.0
From: Joerg Roedel <[email protected]>
Disable the exception handling before booting the kernel to make sure
any exceptions that happen during early kernel boot are not directed to
the pre-decompression code.
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/boot/compressed/idt_64.c | 14 ++++++++++++++
arch/x86/boot/compressed/misc.c | 7 ++-----
arch/x86/boot/compressed/misc.h | 6 ++++++
3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/arch/x86/boot/compressed/idt_64.c b/arch/x86/boot/compressed/idt_64.c
index 804a502ee0d2..9b93567d663a 100644
--- a/arch/x86/boot/compressed/idt_64.c
+++ b/arch/x86/boot/compressed/idt_64.c
@@ -52,3 +52,17 @@ void load_stage2_idt(void)
load_boot_idt(&boot_idt_desc);
}
+
+void cleanup_exception_handling(void)
+{
+ /*
+ * Flush GHCB from cache and map it encrypted again when running as
+ * SEV-ES guest.
+ */
+ sev_es_shutdown_ghcb();
+
+ /* Set a null-idt, disabling #PF and #VC handling */
+ boot_idt_desc.size = 0;
+ boot_idt_desc.address = 0;
+ load_boot_idt(&boot_idt_desc);
+}
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 267e7f93050e..cc9fd0e8766a 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -443,11 +443,8 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
handle_relocations(output, output_len, virt_addr);
debug_putstr("done.\nBooting the kernel.\n");
- /*
- * Flush GHCB from cache and map it encrypted again when running as
- * SEV-ES guest.
- */
- sev_es_shutdown_ghcb();
+ /* Disable exception handling before booting the kernel */
+ cleanup_exception_handling();
return output;
}
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 901ea5ebec22..e5612f035498 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -155,6 +155,12 @@ extern pteval_t __default_kernel_pte_mask;
extern gate_desc boot_idt[BOOT_IDT_ENTRIES];
extern struct desc_ptr boot_idt_desc;
+#ifdef CONFIG_X86_64
+void cleanup_exception_handling(void);
+#else
+static inline void cleanup_exception_handling(void) { }
+#endif
+
/* IDT Entry Points */
void boot_page_fault(void);
void boot_stage1_vc(void);
--
2.30.0
From: Joerg Roedel <[email protected]>
There are a few places left in the SEV-ES C code where hlt loops and/or
terminate requests are implemented. Replace them all with calls to
sev_es_terminate().
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/boot/compressed/sev-es.c | 12 +++---------
arch/x86/kernel/sev-es-shared.c | 10 +++-------
2 files changed, 6 insertions(+), 16 deletions(-)
diff --git a/arch/x86/boot/compressed/sev-es.c b/arch/x86/boot/compressed/sev-es.c
index 27826c265aab..d904bd56b3e3 100644
--- a/arch/x86/boot/compressed/sev-es.c
+++ b/arch/x86/boot/compressed/sev-es.c
@@ -200,14 +200,8 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
}
finish:
- if (result == ES_OK) {
+ if (result == ES_OK)
vc_finish_insn(&ctxt);
- } else if (result != ES_RETRY) {
- /*
- * For now, just halt the machine. That makes debugging easier,
- * later we just call sev_es_terminate() here.
- */
- while (true)
- asm volatile("hlt\n");
- }
+ else if (result != ES_RETRY)
+ sev_es_terminate(GHCB_SEV_ES_REASON_GENERAL_REQUEST);
}
diff --git a/arch/x86/kernel/sev-es-shared.c b/arch/x86/kernel/sev-es-shared.c
index cdc04d091242..7c34be61258e 100644
--- a/arch/x86/kernel/sev-es-shared.c
+++ b/arch/x86/kernel/sev-es-shared.c
@@ -24,7 +24,7 @@ static bool __init sev_es_check_cpu_features(void)
return true;
}
-static void sev_es_terminate(unsigned int reason)
+static void __noreturn sev_es_terminate(unsigned int reason)
{
u64 val = GHCB_SEV_TERMINATE;
@@ -210,12 +210,8 @@ void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
return;
fail:
- sev_es_wr_ghcb_msr(GHCB_SEV_TERMINATE);
- VMGEXIT();
-
- /* Shouldn't get here - if we do halt the machine */
- while (true)
- asm volatile("hlt\n");
+ /* Terminate the guest */
+ sev_es_terminate(GHCB_SEV_ES_REASON_GENERAL_REQUEST);
}
static enum es_result vc_insn_string_read(struct es_em_ctxt *ctxt,
--
2.30.0
From: Joerg Roedel <[email protected]>
Exception handling in the startup_32 boot path requires the CS
selector to be correctly set up. Reload it from the current GDT.
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/boot/compressed/head_64.S | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index e94874f4bbc1..c59c80ca546d 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -107,9 +107,16 @@ SYM_FUNC_START(startup_32)
movl %eax, %gs
movl %eax, %ss
-/* setup a stack and make sure cpu supports long mode. */
+ /* Setup a stack and load CS from current GDT */
leal rva(boot_stack_end)(%ebp), %esp
+ pushl $__KERNEL32_CS
+ leal rva(1f)(%ebp), %eax
+ pushl %eax
+ lretl
+1:
+
+ /* Make sure cpu supports long mode. */
call verify_cpu
testl %eax, %eax
jnz .Lno_longmode
--
2.30.0
On Wed, Feb 10, 2021 at 11:21:28AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> Hi,
>
> these patches add support for the 32-bit boot in the decompressor
> code. This is needed to boot an SEV-ES guest on some firmware and grub
> versions. The patches also add the necessary CPUID sanity checks and a
Could you expand a bit please?
What GRUB versions are we talking about (CC-ing Daniel Kiper, who owns
GRUB).
By 'some firmware' we talking SeaBIOS?
> 32-bit version of the C-bit check.
>
> Other updates included here:
>
> 1. Add code to shut down exception handling in the
> decompressor code before jumping to the real kernel.
> Once in the real kernel it is not safe anymore to jump
> back to the decompressor code via exceptions.
>
> 2. Replace open-coded hlt loops with proper calls to
> sev_es_terminate().
>
> Please review.
>
> Thanks,
>
> Joerg
>
> Joerg Roedel (7):
> x86/boot/compressed/64: Cleanup exception handling before booting
> kernel
> x86/boot/compressed/64: Reload CS in startup_32
> x86/boot/compressed/64: Setup IDT in startup_32 boot path
> x86/boot/compressed/64: Add 32-bit boot #VC handler
> x86/boot/compressed/64: Add CPUID sanity check to 32-bit boot-path
> x86/boot/compressed/64: Check SEV encryption in 32-bit boot-path
> x86/sev-es: Replace open-coded hlt-loops with sev_es_terminate()
>
> arch/x86/boot/compressed/head_64.S | 168 ++++++++++++++++++++++++-
> arch/x86/boot/compressed/idt_64.c | 14 +++
> arch/x86/boot/compressed/mem_encrypt.S | 114 ++++++++++++++++-
> arch/x86/boot/compressed/misc.c | 7 +-
> arch/x86/boot/compressed/misc.h | 6 +
> arch/x86/boot/compressed/sev-es.c | 12 +-
> arch/x86/kernel/sev-es-shared.c | 10 +-
> 7 files changed, 307 insertions(+), 24 deletions(-)
>
> --
> 2.30.0
>
Hi Konrad,
On Wed, Feb 10, 2021 at 09:58:35AM -0500, Konrad Rzeszutek Wilk wrote:
> What GRUB versions are we talking about (CC-ing Daniel Kiper, who owns
> GRUB).
I think this was about 32-bit GRUB builds used by distributions. I
personally tested it with a kernel which has EFI support disabled, in
this case the OVMF firmware will also boot into the startup_32 boot
path.
> By 'some firmware' we talking SeaBIOS?
No, SeaBIOS is not supported for SEV-ES, only OVMF has handling for #VC
so far.
Regards,
Joerg
On Wed, Feb 10, 2021 at 04:12:25PM +0100, Joerg Roedel wrote:
> Hi Konrad,
>
> On Wed, Feb 10, 2021 at 09:58:35AM -0500, Konrad Rzeszutek Wilk wrote:
> > What GRUB versions are we talking about (CC-ing Daniel Kiper, who owns
> > GRUB).
>
> I think this was about 32-bit GRUB builds used by distributions. I
> personally tested it with a kernel which has EFI support disabled, in
> this case the OVMF firmware will also boot into the startup_32 boot
> path.
I think I am missing something obvious here - but why would you want
EFI support disabled?
Or is the idea that "legacy" OSes can nicely run under AMD SEV?
But since you are having a kernel patch that is not "legacy OS" anymore.
>
> > By 'some firmware' we talking SeaBIOS?
>
> No, SeaBIOS is not supported for SEV-ES, only OVMF has handling for #VC
> so far.
>
> Regards,
>
> Joerg
On Wed, Feb 10, 2021 at 10:19:38AM -0500, Konrad Rzeszutek Wilk wrote:
> I think I am missing something obvious here - but why would you want
> EFI support disabled?
I don't want EFI support disabled, this is just a way to trigger this
boot-path. In real life it is triggered by 32-bit GRUB EFI builds. But I
havn't had one of those for testing, so I used another way to trigger
this path.
Regards,
Joerg
On Wed, Feb 10, 2021 at 11:21:32AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> Add a #VC exception handler which is used when the kernel still executes
> in protected mode. This boot-path already uses CPUID, which will cause #VC
> exceptions in an SEV-ES guest.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> arch/x86/boot/compressed/head_64.S | 6 ++
> arch/x86/boot/compressed/mem_encrypt.S | 77 +++++++++++++++++++++++++-
> 2 files changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index 8deeec78cdb4..eadaa0a082b8 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -34,6 +34,7 @@
> #include <asm/asm-offsets.h>
> #include <asm/bootparam.h>
> #include <asm/desc_defs.h>
> +#include <asm/trapnr.h>
> #include "pgtable.h"
>
> /*
> @@ -856,6 +857,11 @@ SYM_FUNC_START(startup32_set_idt_entry)
> SYM_FUNC_END(startup32_set_idt_entry)
>
> SYM_FUNC_START(startup32_load_idt)
> + /* #VC handler */
> + leal rva(startup32_vc_handler)(%ebp), %eax
> + movl $X86_TRAP_VC, %edx
> + call startup32_set_idt_entry
> +
> /* Load IDT */
> leal rva(boot32_idt)(%ebp), %eax
> movl %eax, rva(boot32_idt_desc+2)(%ebp)
> diff --git a/arch/x86/boot/compressed/mem_encrypt.S b/arch/x86/boot/compressed/mem_encrypt.S
> index aa561795efd1..350ecb56c7e4 100644
> --- a/arch/x86/boot/compressed/mem_encrypt.S
> +++ b/arch/x86/boot/compressed/mem_encrypt.S
> @@ -67,10 +67,85 @@ SYM_FUNC_START(get_sev_encryption_bit)
> ret
> SYM_FUNC_END(get_sev_encryption_bit)
>
> +/*
> + * Emit code to request an CPUID register from the Hypervisor using
> + * the MSR-based protocol.
> + *
> + * fn: The register containing the CPUID function
> + * reg: Register requested
> + * 1 = EAX
> + * 2 = EBX
> + * 3 = ECX
> + * 4 = EDX
> + *
> + * Result is in EDX. Jumps to .Lfail on error
> + */
> +.macro SEV_ES_REQ_CPUID fn:req reg:req
I'm wondering - instead of replicating this 4 times, can this be a
function which you CALL? You do have a stack so you should be able to.
> + /* Request CPUID[%ebx].EAX */
> + movl $\reg, %eax
> + shll $30, %eax
> + orl $0x00000004, %eax
> + movl \fn, %edx
> + movl $MSR_AMD64_SEV_ES_GHCB, %ecx
> + wrmsr
> + rep; vmmcall
> + rdmsr
> + /* Check response code */
Before you do that, I guess you wanna check:
GHCBData[29:12] – Reserved, must be zero
in the HV response.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette