Re: [PATCH Part1 RFC v3 21/22] x86/sev: Register SNP guest request platform device

From: Sergio Lopez
Date: Fri Jun 04 2021 - 07:28:48 EST

On Wed, Jun 02, 2021 at 09:04:15AM -0500, Brijesh Singh wrote:
> Version 2 of GHCB specification provides NAEs that can be used by the SNP
> guest to communicate with the PSP without risk from a malicious hypervisor
> who wishes to read, alter, drop or replay the messages sent.
>
> The hypervisor uses the SNP_GUEST_REQUEST command interface provided by
> the SEV-SNP firmware to forward the guest messages to the PSP.
>
> In order to communicate with the PSP, the guest need to locate the secrets
> page inserted by the hypervisor during the SEV-SNP guest launch. The
> secrets page contains the communication keys used to send and receive the
> encrypted messages between the guest and the PSP.
>
> The secrets page is located either through the setup_data cc_blob_address
> or EFI configuration table.
>
> Create a platform device that the SNP guest driver can bind to get the
> platform resources. The SNP guest driver can provide userspace interface
> to get the attestation report, key derivation etc.
>
> The helper snp_issue_guest_request() will be used by the drivers to
> send the guest message request to the hypervisor. The guest message header
> contains a message count. The message count is used in the IV. The
> firmware increments the message count by 1, and expects that next message
> will be using the incremented count.
>
> The helper snp_msg_seqno() will be used by driver to get and message
> sequence counter, and it will be automatically incremented by the
> snp_issue_guest_request(). The incremented value is be saved in the
> secrets page so that the kexec'ed kernel knows from where to begin.
>
> See SEV-SNP and GHCB spec for more details.
>
> Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
> ---
> arch/x86/include/asm/sev.h | 12 +++
> arch/x86/include/uapi/asm/svm.h | 2 +
> arch/x86/kernel/sev.c | 176 ++++++++++++++++++++++++++++++++
> arch/x86/platform/efi/efi.c | 2 +
> include/linux/efi.h | 1 +
> include/linux/sev-guest.h | 76 ++++++++++++++
> 6 files changed, 269 insertions(+)
> create mode 100644 include/linux/sev-guest.h
>
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 640108402ae9..da2f757cd9bc 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -59,6 +59,18 @@ extern void vc_no_ghcb(void);
> extern void vc_boot_ghcb(void);
> extern bool handle_vc_boot_ghcb(struct pt_regs *regs);
>
> +/* AMD SEV Confidential computing blob structure */
> +#define CC_BLOB_SEV_HDR_MAGIC 0x45444d41
> +struct cc_blob_sev_info {
> + u32 magic;
> + u16 version;
> + u16 reserved;
> + u64 secrets_phys;
> + u32 secrets_len;
> + u64 cpuid_phys;
> + u32 cpuid_len;
> +};
> +
> /* Software defined (when rFlags.CF = 1) */
> #define PVALIDATE_FAIL_NOUPDATE 255
>
> diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
> index c0152186a008..bd64f2b98ac7 100644
> --- a/arch/x86/include/uapi/asm/svm.h
> +++ b/arch/x86/include/uapi/asm/svm.h
> @@ -109,6 +109,7 @@
> #define SVM_VMGEXIT_SET_AP_JUMP_TABLE 0
> #define SVM_VMGEXIT_GET_AP_JUMP_TABLE 1
> #define SVM_VMGEXIT_PSC 0x80000010
> +#define SVM_VMGEXIT_GUEST_REQUEST 0x80000011
> #define SVM_VMGEXIT_AP_CREATION 0x80000013
> #define SVM_VMGEXIT_AP_CREATE_ON_INIT 0
> #define SVM_VMGEXIT_AP_CREATE 1
> @@ -222,6 +223,7 @@
> { SVM_VMGEXIT_AP_JUMP_TABLE, "vmgexit_ap_jump_table" }, \
> { SVM_VMGEXIT_PSC, "vmgexit_page_state_change" }, \
> { SVM_VMGEXIT_AP_CREATION, "vmgexit_ap_creation" }, \
> + { SVM_VMGEXIT_GUEST_REQUEST, "vmgexit_guest_request" }, \
> { SVM_EXIT_ERR, "invalid_guest_state" }
>
>
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index 8f7ef35a25ef..8aae1166f52e 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -9,6 +9,7 @@
>
> #define pr_fmt(fmt) "SEV-ES: " fmt
>
> +#include <linux/platform_device.h>
> #include <linux/sched/debug.h> /* For show_regs() */
> #include <linux/percpu-defs.h>
> #include <linux/mem_encrypt.h>
> @@ -16,10 +17,13 @@
> #include <linux/printk.h>
> #include <linux/mm_types.h>
> #include <linux/set_memory.h>
> +#include <linux/sev-guest.h>
> #include <linux/memblock.h>
> #include <linux/kernel.h>
> +#include <linux/efi.h>
> #include <linux/mm.h>
> #include <linux/cpumask.h>
> +#include <linux/io.h>
>
> #include <asm/cpu_entry_area.h>
> #include <asm/stacktrace.h>
> @@ -33,6 +37,7 @@
> #include <asm/smp.h>
> #include <asm/cpu.h>
> #include <asm/apic.h>
> +#include <asm/setup.h> /* For struct boot_params */
>
> #include "sev-internal.h"
>
> @@ -47,6 +52,8 @@ static struct ghcb boot_ghcb_page __bss_decrypted __aligned(PAGE_SIZE);
> */
> static struct ghcb __initdata *boot_ghcb;
>
> +static unsigned long snp_secrets_phys;
> +
> /* #VC handler runtime per-CPU data */
> struct sev_es_runtime_data {
> struct ghcb ghcb_page;
> @@ -105,6 +112,10 @@ struct ghcb_state {
> struct ghcb *ghcb;
> };
>
> +#ifdef CONFIG_EFI
> +extern unsigned long cc_blob_phys;
> +#endif
> +
> static DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
> DEFINE_STATIC_KEY_FALSE(sev_es_enable_key);
>
> @@ -1909,3 +1920,168 @@ bool __init handle_vc_boot_ghcb(struct pt_regs *regs)
> while (true)
> halt();
> }
> +
> +static struct resource guest_req_res[0];
> +static struct platform_device guest_req_device = {
> + .name = "snp-guest",
> + .id = -1,
> + .resource = guest_req_res,
> + .num_resources = 1,
> +};

Perhaps I'm missing something, but I can't find where the memory for
"guest_req_res" is allocated. In my tests I had to turn this
zero-length array into a single struct to prevent the kernel from
crashing.

Thanks,
Sergio.

Attachment: signature.asc
Description: PGP signature