Re: [PATCH v3 19/75] x86/boot/compressed/64: Add stage1 #VC handler

Borislav Petkov
2020-05-09T09:05:57Z

On Tue, Apr 28, 2020 at 05:16:29PM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Add the first handler for #VC exceptions. At stage 1 there is no GHCB
> yet becaue we might still be on the EFI page table and thus can't map

"... because the kernel might still be running on the EFI page table... "

> memory unencrypted.
>
> The stage 1 handler is limited to the MSR based protocol to talk to
> the hypervisor and can only support CPUID exit-codes, but that is
> enough to get to stage 2.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> arch/x86/boot/compressed/Makefile | 1 +
> arch/x86/boot/compressed/idt_64.c | 4 ++
> arch/x86/boot/compressed/idt_handlers_64.S | 4 ++
> arch/x86/boot/compressed/misc.h | 1 +
> arch/x86/boot/compressed/sev-es.c | 45 +++++++++++++++
> arch/x86/include/asm/msr-index.h | 1 +
> arch/x86/include/asm/sev-es.h | 37 ++++++++++++
> arch/x86/include/asm/trap_defs.h | 1 +
> arch/x86/kernel/sev-es-shared.c | 65 ++++++++++++++++++++++
> 9 files changed, 159 insertions(+)
> create mode 100644 arch/x86/boot/compressed/sev-es.c
> create mode 100644 arch/x86/include/asm/sev-es.h
> create mode 100644 arch/x86/kernel/sev-es-shared.c
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index c6909d10a6b9..a7847a1ef63a 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -85,6 +85,7 @@ ifdef CONFIG_X86_64
> vmlinux-objs-y += $(obj)/idt_64.o $(obj)/idt_handlers_64.o
> vmlinux-objs-y += $(obj)/mem_encrypt.o
> vmlinux-objs-y += $(obj)/pgtable_64.o
> + vmlinux-objs-$(CONFIG_AMD_MEM_ENCRYPT) += $(obj)/sev-es.o
> endif
>
> vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
> diff --git a/arch/x86/boot/compressed/idt_64.c b/arch/x86/boot/compressed/idt_64.c
> index 99cc78062684..f8295d68b3e1 100644
> --- a/arch/x86/boot/compressed/idt_64.c
> +++ b/arch/x86/boot/compressed/idt_64.c
> @@ -31,6 +31,10 @@ void load_stage1_idt(void)
> {
> boot_idt_desc.address = (unsigned long)boot_idt;
>
> +#ifdef CONFIG_AMD_MEM_ENCRYPT
> + set_idt_entry(X86_TRAP_VC, boot_stage1_vc);
> +#endif

if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT))

seems to work too and drops the ifdeffery ugliness.

...

> +void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
> +{
> + unsigned int fn = lower_bits(regs->ax, 32);
> + unsigned long val;
> +
> + /* Only CPUID is supported via MSR protocol */
> + if (exit_code != SVM_EXIT_CPUID)
> + goto fail;
> +
> + sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EAX));
> + VMGEXIT();
> + val = sev_es_rd_ghcb_msr();
> + if (GHCB_SEV_GHCB_RESP_CODE(val) != GHCB_SEV_CPUID_RESP)
> + goto fail;
> + regs->ax = val >> 32;
> +
> + sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EBX));
> + VMGEXIT();
> + val = sev_es_rd_ghcb_msr();
> + if (GHCB_SEV_GHCB_RESP_CODE(val) != GHCB_SEV_CPUID_RESP)
> + goto fail;
> + regs->bx = val >> 32;
> +
> + sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_ECX));
> + VMGEXIT();
> + val = sev_es_rd_ghcb_msr();
> + if (GHCB_SEV_GHCB_RESP_CODE(val) != GHCB_SEV_CPUID_RESP)
> + goto fail;
> + regs->cx = val >> 32;
> +
> + sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EDX));
> + VMGEXIT();
> + val = sev_es_rd_ghcb_msr();
> + if (GHCB_SEV_GHCB_RESP_CODE(val) != GHCB_SEV_CPUID_RESP)
> + goto fail;
> + regs->dx = val >> 32;

This could use a comment:

/* Skip over the CPUID two-byte opcode */

or so.

--
Regards/Gruss,
Boris.