Message-ID: <YMEVedGOrYgI1Klc@work-vm>
Date:   Wed, 9 Jun 2021 20:24:41 +0100
From:   "Dr. David Alan Gilbert" <dgilbert@...hat.com>
To:     Brijesh Singh <brijesh.singh@....com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-efi@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        linux-coco@...ts.linux.dev, linux-mm@...ck.org,
        linux-crypto@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Joerg Roedel <jroedel@...e.de>,
        Tom Lendacky <thomas.lendacky@....com>,
        "H. Peter Anvin" <hpa@...or.com>, Ard Biesheuvel <ardb@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Sergio Lopez <slp@...hat.com>, Peter Gonda <pgonda@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>, tony.luck@...el.com,
        npmccallum@...hat.com
Subject: Re: [PATCH Part1 RFC v3 21/22] x86/sev: Register SNP guest request
 platform device

* Brijesh Singh (brijesh.singh@....com) 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@....com>
> ---
>  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,
> +};
> +
> +static struct snp_secrets_page_layout *snp_map_secrets_page(void)
> +{
> +	u16 __iomem *secrets;
> +
> +	if (!snp_secrets_phys || !sev_feature_enabled(SEV_SNP))
> +		return NULL;
> +
> +	secrets = ioremap_encrypted(snp_secrets_phys, PAGE_SIZE);
> +	if (!secrets)
> +		return NULL;
> +
> +	return (struct snp_secrets_page_layout *)secrets;
> +}
> +
> +u64 snp_msg_seqno(void)
> +{
> +	struct snp_secrets_page_layout *layout;
> +	u64 count;
> +
> +	layout = snp_map_secrets_page();
> +	if (layout == NULL)
> +		return 0;
> +
> +	/* Read the current message sequence counter from secrets pages */
> +	count = readl(&layout->os_area.msg_seqno_0);

Why is this seqno_0 - is that because it's the count of talking to the
PSP?

> +	iounmap(layout);
> +
> +	/*
> +	 * The message sequence counter for the SNP guest request is a 64-bit value
> +	 * but the version 2 of GHCB specification defines the 32-bit storage for the
> +	 * it.
> +	 */
> +	if ((count + 1) >= INT_MAX)
> +		return 0;

Is that UINT_MAX?

> +
> +	return count + 1;
> +}
> +EXPORT_SYMBOL_GPL(snp_msg_seqno);
> +
> +static void snp_gen_msg_seqno(void)
> +{
> +	struct snp_secrets_page_layout *layout;
> +	u64 count;
> +
> +	layout = snp_map_secrets_page();
> +	if (layout == NULL)
> +		return;
> +
> +	/* Increment the sequence counter by 2 and save in secrets page. */
> +	count = readl(&layout->os_area.msg_seqno_0);
> +	count += 2;

Why 2 not 1 ?

> +	writel(count, &layout->os_area.msg_seqno_0);
> +	iounmap(layout);
> +}
> +
> +static int get_snp_secrets_resource(struct resource *res)
> +{
> +	struct setup_header *hdr = &boot_params.hdr;
> +	struct cc_blob_sev_info *info;
> +	unsigned long paddr;
> +	int ret = -ENODEV;
> +
> +	/*
> +	 * The secret page contains the VM encryption key used for encrypting the
> +	 * messages between the guest and the PSP. The secrets page location is
> +	 * available either through the setup_data or EFI configuration table.
> +	 */
> +	if (hdr->cc_blob_address) {
> +		paddr = hdr->cc_blob_address;

Can you trust the paddr the host has given you or do you need to do some
form of validation?

Dave
> +	} else if (efi_enabled(EFI_CONFIG_TABLES)) {
> +#ifdef CONFIG_EFI
> +		paddr = cc_blob_phys;
> +#else
> +		return -ENODEV;
> +#endif
> +	} else {
> +		return -ENODEV;
> +	}
> +
> +	info = memremap(paddr, sizeof(*info), MEMREMAP_WB);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	/* Verify the header that its a valid SEV_SNP CC header */
> +	if ((info->magic == CC_BLOB_SEV_HDR_MAGIC) &&
> +	    info->secrets_phys &&
> +	    (info->secrets_len == PAGE_SIZE)) {
> +		res->start = info->secrets_phys;
> +		res->end = info->secrets_phys + info->secrets_len;
> +		res->flags = IORESOURCE_MEM;
> +		snp_secrets_phys = info->secrets_phys;
> +		ret = 0;
> +	}
> +
> +	memunmap(info);
> +	return ret;
> +}
> +
> +static int __init add_snp_guest_request(void)
> +{
> +	if (!sev_feature_enabled(SEV_SNP))
> +		return -ENODEV;
> +
> +	if (get_snp_secrets_resource(&guest_req_res[0]))
> +		return -ENODEV;
> +
> +	platform_device_register(&guest_req_device);
> +	dev_info(&guest_req_device.dev, "registered [secret 0x%llx - 0x%llx]\n",
> +		guest_req_res[0].start, guest_req_res[0].end);
> +
> +	return 0;
> +}
> +device_initcall(add_snp_guest_request);
> +
> +unsigned long snp_issue_guest_request(int type, struct snp_guest_request_data *input)
> +{
> +	struct ghcb_state state;
> +	struct ghcb *ghcb;
> +	unsigned long id;
> +	int ret;
> +
> +	if (!sev_feature_enabled(SEV_SNP))
> +		return -ENODEV;
> +
> +	if (type == GUEST_REQUEST)
> +		id = SVM_VMGEXIT_GUEST_REQUEST;
> +	else
> +		return -EINVAL;
> +
> +	ghcb = sev_es_get_ghcb(&state);
> +	if (!ghcb)
> +		return -ENODEV;
> +
> +	vc_ghcb_invalidate(ghcb);
> +	ghcb_set_rax(ghcb, input->data_gpa);
> +	ghcb_set_rbx(ghcb, input->data_npages);
> +
> +	ret = sev_es_ghcb_hv_call(ghcb, NULL, id, input->req_gpa, input->resp_gpa);
> +	if (ret)
> +		goto e_put;
> +
> +	if (ghcb->save.sw_exit_info_2) {
> +		ret = ghcb->save.sw_exit_info_2;
> +		goto e_put;
> +	}
> +
> +	/* Command was successful, increment the message sequence counter. */
> +	snp_gen_msg_seqno();
> +
> +e_put:
> +	sev_es_put_ghcb(&state);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(snp_issue_guest_request);
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 8a26e705cb06..2cca9ee6e1d4 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -57,6 +57,7 @@ static unsigned long efi_systab_phys __initdata;
>  static unsigned long prop_phys = EFI_INVALID_TABLE_ADDR;
>  static unsigned long uga_phys = EFI_INVALID_TABLE_ADDR;
>  static unsigned long efi_runtime, efi_nr_tables;
> +unsigned long cc_blob_phys;
>  
>  unsigned long efi_fw_vendor, efi_config_table;
>  
> @@ -66,6 +67,7 @@ static const efi_config_table_type_t arch_tables[] __initconst = {
>  #ifdef CONFIG_X86_UV
>  	{UV_SYSTEM_TABLE_GUID,		&uv_systab_phys,	"UVsystab"	},
>  #endif
> +	{EFI_CC_BLOB_GUID,		&cc_blob_phys,		"CC blob"	},
>  	{},
>  };
>  
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 6b5d36babfcc..75aeb2a56888 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -344,6 +344,7 @@ void efi_native_runtime_setup(void);
>  #define EFI_CERT_SHA256_GUID			EFI_GUID(0xc1c41626, 0x504c, 0x4092, 0xac, 0xa9, 0x41, 0xf9, 0x36, 0x93, 0x43, 0x28)
>  #define EFI_CERT_X509_GUID			EFI_GUID(0xa5c059a1, 0x94e4, 0x4aa7, 0x87, 0xb5, 0xab, 0x15, 0x5c, 0x2b, 0xf0, 0x72)
>  #define EFI_CERT_X509_SHA256_GUID		EFI_GUID(0x3bd2a492, 0x96c0, 0x4079, 0xb4, 0x20, 0xfc, 0xf9, 0x8e, 0xf1, 0x03, 0xed)
> +#define EFI_CC_BLOB_GUID			EFI_GUID(0x067b1f5f, 0xcf26, 0x44c5, 0x85, 0x54, 0x93, 0xd7, 0x77, 0x91, 0x2d, 0x42)
>  
>  /*
>   * This GUID is used to pass to the kernel proper the struct screen_info
> diff --git a/include/linux/sev-guest.h b/include/linux/sev-guest.h
> new file mode 100644
> index 000000000000..51277448a108
> --- /dev/null
> +++ b/include/linux/sev-guest.h
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * AMD Secure Encrypted Virtualization (SEV) guest driver interface
> + *
> + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + *
> + * Author: Brijesh Singh <brijesh.singh@....com>
> + *
> + */
> +
> +#ifndef __LINUX_SEV_GUEST_H_
> +#define __LINUX_SEV_GUEST_H_
> +
> +#include <linux/types.h>
> +
> +enum vmgexit_type {
> +	GUEST_REQUEST,
> +
> +	GUEST_REQUEST_MAX
> +};
> +
> +/*
> + * The secrets page contains 96-bytes of reserved field that can be used by
> + * the guest OS. The guest OS uses the area to save the message sequence
> + * number for each VMPL level.
> + *
> + * See the GHCB spec section Secret page layout for the format for this area.
> + */
> +struct secrets_os_area {
> +	u32 msg_seqno_0;
> +	u32 msg_seqno_1;
> +	u32 msg_seqno_2;
> +	u32 msg_seqno_3;
> +	u64 ap_jump_table_pa;
> +	u8 rsvd[40];
> +	u8 guest_usage[32];
> +} __packed;
> +
> +#define VMPCK_KEY_LEN		32
> +
> +/* See the SNP spec secrets page layout section for the structure */
> +struct snp_secrets_page_layout {
> +	u32 version;
> +	u32 imiEn	: 1,
> +	    rsvd1	: 31;
> +	u32 fms;
> +	u32 rsvd2;
> +	u8 gosvw[16];
> +	u8 vmpck0[VMPCK_KEY_LEN];
> +	u8 vmpck1[VMPCK_KEY_LEN];
> +	u8 vmpck2[VMPCK_KEY_LEN];
> +	u8 vmpck3[VMPCK_KEY_LEN];
> +	struct secrets_os_area os_area;
> +	u8 rsvd3[3840];
> +} __packed;
> +
> +struct snp_guest_request_data {
> +	unsigned long req_gpa;
> +	unsigned long resp_gpa;
> +	unsigned long data_gpa;
> +	unsigned int data_npages;
> +};
> +
> +#ifdef CONFIG_AMD_MEM_ENCRYPT
> +unsigned long snp_issue_guest_request(int vmgexit_type, struct snp_guest_request_data *input);
> +u64 snp_msg_seqno(void);
> +#else
> +
> +static inline unsigned long snp_issue_guest_request(int type,
> +						    struct snp_guest_request_data *input)
> +{
> +	return -ENODEV;
> +}
> +static inline u64 snp_msg_seqno(void) { return 0; }
> +#endif /* CONFIG_AMD_MEM_ENCRYPT */
> +#endif /* __LINUX_SEV_GUEST_H__ */
> -- 
> 2.17.1
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@...hat.com / Manchester, UK

Powered by blists - more mailing lists