Skip to contentSkip to conversation

Add AMD SEV guest live migration support

PATCH Superseded Ashish Kalra 5 participants 34 messages Apr 30, 2020 – May 4, 2020

Subsystems (10)
Documentation x86 KVM KVM x86 +6 more subsystems

v7 of 13 Superseded Latest is v13 →

lore
Share & tools
lore
#0 lore raw

v7 cover Add AMD SEV guest live migration support

From: Ashish Kalra <[email protected]>

The series add support for AMD SEV guest live migration commands. To protect the confidentiality of an SEV protected guest memory while in transit we need to use the SEV commands defined in SEV API spec [1].

SEV guest VMs have the concept of private and shared memory. Private memory is encrypted with the guest-specific key, while shared memory may be encrypted with hypervisor key. The commands provided by the SEV FW are meant to be used for the private memory only. The patch series introduces a new hypercall. The guest OS can use this hypercall to notify the page encryption status. If the page is encrypted with guest specific-key then we use SEV command during the migration. If page is not encrypted then fallback to default.

The patch adds new ioctls KVM_{SET,GET}_PAGE_ENC_BITMAP. The ioctl can be used by the qemu to get the page encrypted bitmap. Qemu can consult this bitmap during the migration to know whether the page is encrypted.

[1] https://developer.amd.com/wp-content/resources/55766.PDF

Changes since v6:
- Rebasing to mainline and refactoring to the new split SVM infrastructre.
- Move to static allocation of the unified Page Encryption bitmap instead of the dynamic resizing of the bitmap, the static allocation is done implicitly by extending kvm_arch_commit_memory_region() callack to add svm specific x86_ops which can read the userspace provided memory region/memslots and calculate the amount of guest RAM managed by the KVM and grow the bitmap.
- Fixed KVM_SET_PAGE_ENC_BITMAP ioctl to set the whole bitmap instead of simply clearing specific bits.
- Removed KVM_PAGE_ENC_BITMAP_RESET ioctl, which is now performed using KVM_SET_PAGE_ENC_BITMAP.
- Extended guest support for enabling Live Migration feature by adding a check for UEFI environment variable indicating OVMF support for Live Migration feature and additionally checking for KVM capability for the same feature. If not booted under EFI, then we simply check for KVM capability.
- Add hypervisor specific hypercall for SEV live migration by adding a new paravirt callback as part of x86_hyper_runtime.
(x86 hypervisor specific runtime callbacks)
- Moving MSR handling for MSR_KVM_SEV_LIVE_MIG_EN into svm/sev code and adding check for SEV live migration enabled by guest in the KVM_GET_PAGE_ENC_BITMAP ioctl.
- Instead of the complete __bss_decrypted section, only specific variables such as hv_clock_boot and wall_clock are marked as decrypted in the page encryption bitmap

Changes since v5:
- Fix build errors as

Changes since v4:
- Host support has been added to extend KVM capabilities/feature bits to include a new KVM_FEATURE_SEV_LIVE_MIGRATION, which the guest can query for host-side support for SEV live migration and a new custom MSR MSR_KVM_SEV_LIVE_MIG_EN is added for guest to enable the SEV live migration feature.
- Ensure that _bss_decrypted section is marked as decrypted in the page encryption bitmap.
- Fixing KVM_GET_PAGE_ENC_BITMAP ioctl to return the correct bitmap as per the number of pages being requested by the user. Ensure that we only copy bmap->num_pages bytes in the userspace buffer, if bmap->num_pages is not byte aligned we read the trailing bits from the userspace and copy those bits as is. This fixes guest page(s) corruption issues observed after migration completion.
- Add kexec support for SEV Live Migration to reset the host's page encryption bitmap related to kernel specific page encryption status settings before we load a new kernel by kexec. We cannot reset the complete page encryption bitmap here as we need to retain the UEFI/OVMF firmware specific settings.

Changes since v3:
- Rebasing to mainline and testing.
- Adding a new KVM_PAGE_ENC_BITMAP_RESET ioctl, which resets the page encryption bitmap on a guest reboot event.
- Adding a more reliable sanity check for GPA range being passed to the hypercall to ensure that guest MMIO ranges are also marked in the page encryption bitmap.

Changes since v2:
- reset the page encryption bitmap on vcpu reboot

Changes since v1:
- Add support to share the page encryption between the source and target machine.
- Fix review feedbacks from Tom Lendacky.
- Add check to limit the session blob length.
- Update KVM_GET_PAGE_ENC_BITMAP icotl to use the base_gfn instead of the memory slot when querying the bitmap.

Ashish Kalra (7):
x86/paravirt: Add hypervisor specific hypercall for SEV live
migration.
KVM: SVM: Add support for static allocation of unified Page Encryption
Bitmap.
KVM: x86: Introduce new KVM_FEATURE_SEV_LIVE_MIGRATION feature &
Custom MSR.
EFI: Introduce the new AMD Memory Encryption GUID.
KVM: x86: Add guest support for detecting and enabling SEV Live
Migration feature.
KVM: x86: Mark _bss_decrypted section variables as decrypted in page
encryption bitmap.
KVM: x86: Add kexec support for SEV Live Migration.

Brijesh Singh (11):
KVM: SVM: Add KVM_SEV SEND_START command
KVM: SVM: Add KVM_SEND_UPDATE_DATA command
KVM: SVM: Add KVM_SEV_SEND_FINISH command
KVM: SVM: Add support for KVM_SEV_RECEIVE_START command
KVM: SVM: Add KVM_SEV_RECEIVE_UPDATE_DATA command
KVM: SVM: Add KVM_SEV_RECEIVE_FINISH command
KVM: x86: Add AMD SEV specific Hypercall3
KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
KVM: x86: Introduce KVM_GET_PAGE_ENC_BITMAP ioctl
mm: x86: Invoke hypercall when page encryption status is changed
KVM: x86: Introduce KVM_SET_PAGE_ENC_BITMAP ioctl

 .../virt/kvm/amd-memory-encryption.rst        | 120 +++
 Documentation/virt/kvm/api.rst                |  71 ++
 Documentation/virt/kvm/cpuid.rst              |   5 +
 Documentation/virt/kvm/hypercalls.rst         |  15 +
 Documentation/virt/kvm/msr.rst                |  10 +
 arch/x86/include/asm/kvm_host.h               |   7 +
 arch/x86/include/asm/kvm_para.h               |  12 +
 arch/x86/include/asm/mem_encrypt.h            |  11 +
 arch/x86/include/asm/paravirt.h               |  10 +
 arch/x86/include/asm/paravirt_types.h         |   2 +
 arch/x86/include/asm/x86_init.h               |  10 +-
 arch/x86/include/uapi/asm/kvm_para.h          |   5 +
 arch/x86/kernel/kvm.c                         | 102 +++
 arch/x86/kernel/kvmclock.c                    |  12 +
 arch/x86/kernel/paravirt.c                    |   1 +
 arch/x86/kvm/svm/sev.c                        | 723 +++++++++++++++++-
 arch/x86/kvm/svm/svm.c                        |  21 +
 arch/x86/kvm/svm/svm.h                        |   9 +
 arch/x86/kvm/vmx/vmx.c                        |   1 +
 arch/x86/kvm/x86.c                            |  35 +
 arch/x86/mm/mem_encrypt.c                     |  69 +-
 arch/x86/mm/pat/set_memory.c                  |   7 +
 include/linux/efi.h                           |   1 +
 include/linux/psp-sev.h                       |   8 +-
 include/uapi/linux/kvm.h                      |  52 ++
 include/uapi/linux/kvm_para.h                 |   1 +
 26 files changed, 1310 insertions(+), 10 deletions(-)
Signature · 1 line
2.17.1
#1 lore raw

PATCH v7 1/18 KVM: SVM: Add KVM_SEV SEND_START command

From: Brijesh Singh <[email protected]>

The command is used to create an outgoing SEV guest encryption context.

---
 .../virt/kvm/amd-memory-encryption.rst        |  27 ++++
 arch/x86/kvm/svm/sev.c                        | 125 ++++++++++++++++++
 include/linux/psp-sev.h                       |   8 +-
 include/uapi/linux/kvm.h                      |  12 ++
 4 files changed, 168 insertions(+), 4 deletions(-)
Documentation/virt/kvm/amd-memory-encryption.rst +27 -0
index c3129b9ba5cb..4fd34fc5c7a7 100644
@@ -263,6 +263,33 @@ Returns: 0 on success, -negative on error
                 __u32 trans_len;
         };
 
+10. KVM_SEV_SEND_START
+----------------------
+
+The KVM_SEV_SEND_START command can be used by the hypervisor to create an
+outgoing guest encryption context.
+
+Parameters (in): struct kvm_sev_send_start
+
+Returns: 0 on success, -negative on error
+
+::
+        struct kvm_sev_send_start {
+                __u32 policy;                 /* guest policy */
+
+                __u64 pdh_cert_uaddr;         /* platform Diffie-Hellman certificate */
+                __u32 pdh_cert_len;
+
+                __u64 plat_certs_uadr;        /* platform certificate chain */
+                __u32 plat_certs_len;
+
+                __u64 amd_certs_uaddr;        /* AMD certificate */
+                __u32 amd_cert_len;
+
+                __u64 session_uaddr;          /* Guest session information */
+                __u32 session_len;
+        };
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +125 -0

137 lines not on this page · raw message

include/linux/psp-sev.h +4 -4
index 5167bf2bfc75..9f63b9d48b63 100644
@@ -323,11 +323,11 @@ struct sev_data_send_start {
 	u64 pdh_cert_address;			/* In */
 	u32 pdh_cert_len;			/* In */
 	u32 reserved1;
-	u64 plat_cert_address;			/* In */
-	u32 plat_cert_len;			/* In */
+	u64 plat_certs_address;			/* In */
+	u32 plat_certs_len;			/* In */
 	u32 reserved2;
-	u64 amd_cert_address;			/* In */
-	u32 amd_cert_len;			/* In */
+	u64 amd_certs_address;			/* In */
+	u32 amd_certs_len;			/* In */
 	u32 reserved3;
 	u64 session_address;			/* In */
 	u32 session_len;			/* In/Out */
include/uapi/linux/kvm.h +12 -0
index 428c7dde6b4b..8827d43e2684 100644
@@ -1598,6 +1598,18 @@ struct kvm_sev_dbg {
 	__u32 len;
 };
 
+struct kvm_sev_send_start {
+	__u32 policy;
+	__u64 pdh_cert_uaddr;
+	__u32 pdh_cert_len;
+	__u64 plat_certs_uaddr;
+	__u32 plat_certs_len;
+	__u64 amd_certs_uaddr;
+	__u32 amd_certs_len;
+	__u64 session_uaddr;
+	__u32 session_len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
Signature · 1 line
2.17.1
#2 lore raw

Re: [PATCH v7 01/18] KVM: SVM: Add KVM_SEV SEND_START command

Quoted 42 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used to create an outgoing SEV guest encryption context.

---
 .../virt/kvm/amd-memory-encryption.rst        |  27 ++++
 arch/x86/kvm/svm/sev.c                        | 125 ++++++++++++++++++
 include/linux/psp-sev.h                       |   8 +-
 include/uapi/linux/kvm.h                      |  12 ++
 4 files changed, 168 insertions(+), 4 deletions(-)
Documentation/virt/kvm/amd-memory-encryption.rst +8 -0
index c3129b9ba5cb..4fd34fc5c7a7 100644
@@ -263,6 +263,33 @@ Returns: 0 on success, -negative on error
                 __u32 trans_len;
         };
 
+10. KVM_SEV_SEND_START
+----------------------
+
+The KVM_SEV_SEND_START command can be used by the hypervisor to create an
+outgoing guest encryption context.
+
+Parameters (in): struct kvm_sev_send_start
+

Documentation/virt/kvm/amd-memory-encryption.rst Returns: 0 on success, -negative on error · line 283

Quoted 10 lines
Documentation/virt/kvm/amd-memory-encryption.rst +10 -0
+Returns: 0 on success, -negative on error
+
+::
+        struct kvm_sev_send_start {
+                __u32 policy;                 /* guest policy */
+
+                __u64 pdh_cert_uaddr;         /* platform Diffie-Hellman certificate */
+                __u32 pdh_cert_len;
+
+                __u64 plat_certs_uadr;        /* platform certificate chain */

Can this be changed as mentioned in the previous review
(https://lore.kernel.org/kvm/20200402062726.GA647295@vbusired-dt/)?

Documentation/virt/kvm/amd-memory-encryption.rst Returns: 0 on success, -negative on error · line 287

Quoted 4 lines
Documentation/virt/kvm/amd-memory-encryption.rst +4 -0
+                __u32 plat_certs_len;
+
+                __u64 amd_certs_uaddr;        /* AMD certificate */
+                __u32 amd_cert_len;

Can this be changed as mentioned in the previous review
(https://lore.kernel.org/kvm/20200402062726.GA647295@vbusired-dt/)?

Quoted 101 lines
Documentation/virt/kvm/amd-memory-encryption.rst +5 -0
+
+                __u64 session_uaddr;          /* Guest session information */
+                __u32 session_len;
+        };
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +85 -0
index cf912b4aaba8..5a15b43b4349 100644
@@ -913,6 +913,128 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+/* Userspace wants to query session length. */
+static int
+__sev_send_start_query_session_length(struct kvm *kvm, struct kvm_sev_cmd *argp,
+				      struct kvm_sev_send_start *params)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_start *data;
+	int ret;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
+
+	params->session_len = data->session_len;
+	if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
+				sizeof(struct kvm_sev_send_start)))
+		ret = -EFAULT;
+
+	kfree(data);
+	return ret;
+}
+
+static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_start *data;
+	struct kvm_sev_send_start params;
+	void *amd_certs, *session_data;
+	void *pdh_cert, *plat_certs;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	if (copy_from_user(&params, (void __user *)(uintptr_t)argp->data,
+				sizeof(struct kvm_sev_send_start)))
+		return -EFAULT;
+
+	/* if session_len is zero, userspace wants to query the session length */
+	if (!params.session_len)
+		return __sev_send_start_query_session_length(kvm, argp,
+				&params);
+
+	/* some sanity checks */
+	if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
+	    !params.session_uaddr || params.session_len > SEV_FW_BLOB_MAX_SIZE)
+		return -EINVAL;
+
+	/* allocate the memory to hold the session data blob */
+	session_data = kmalloc(params.session_len, GFP_KERNEL_ACCOUNT);
+	if (!session_data)
+		return -ENOMEM;
+
+	/* copy the certificate blobs from userspace */
+	pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr,
+				params.pdh_cert_len);
+	if (IS_ERR(pdh_cert)) {
+		ret = PTR_ERR(pdh_cert);
+		goto e_free_session;
+	}
+
+	plat_certs = psp_copy_user_blob(params.plat_certs_uaddr,
+				params.plat_certs_len);
+	if (IS_ERR(plat_certs)) {
+		ret = PTR_ERR(plat_certs);
+		goto e_free_pdh;
+	}
+
+	amd_certs = psp_copy_user_blob(params.amd_certs_uaddr,
+				params.amd_certs_len);
+	if (IS_ERR(amd_certs)) {
+		ret = PTR_ERR(amd_certs);
+		goto e_free_plat_cert;
+	}
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+	if (data == NULL) {
+		ret = -ENOMEM;
+		goto e_free_amd_cert;
+	}
+
+	/* populate the FW SEND_START field with system physical address */

arch/x86/kvm/svm/sev.c static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp) · line 1010

Quoted 10 lines
arch/x86/kvm/svm/sev.c +10 -0
+	data->pdh_cert_address = __psp_pa(pdh_cert);
+	data->pdh_cert_len = params.pdh_cert_len;
+	data->plat_certs_address = __psp_pa(plat_certs);
+	data->plat_certs_len = params.plat_certs_len;
+	data->amd_certs_address = __psp_pa(amd_certs);
+	data->amd_certs_len = params.amd_certs_len;
+	data->session_address = __psp_pa(session_data);
+	data->session_len = params.session_len;
+	data->handle = sev->handle;
+

Can the following code be changed as acknowledged in
https://lore.kernel.org/kvm/[email protected]/?

Quoted 86 lines
arch/x86/kvm/svm/sev.c +30 -0
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
+
+	if (!ret && copy_to_user((void __user *)(uintptr_t)params.session_uaddr,
+			session_data, params.session_len)) {
+		ret = -EFAULT;
+		goto e_free;
+	}
+
+	params.policy = data->policy;
+	params.session_len = data->session_len;
+	if (copy_to_user((void __user *)(uintptr_t)argp->data, &params,
+				sizeof(struct kvm_sev_send_start)))
+		ret = -EFAULT;
+
+e_free:
+	kfree(data);
+e_free_amd_cert:
+	kfree(amd_certs);
+e_free_plat_cert:
+	kfree(plat_certs);
+e_free_pdh:
+	kfree(pdh_cert);
+e_free_session:
+	kfree(session_data);
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -957,6 +1079,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_LAUNCH_SECRET:
 		r = sev_launch_secret(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_SEND_START:
+		r = sev_send_start(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
include/linux/psp-sev.h +4 -4
index 5167bf2bfc75..9f63b9d48b63 100644
@@ -323,11 +323,11 @@ struct sev_data_send_start {
 	u64 pdh_cert_address;			/* In */
 	u32 pdh_cert_len;			/* In */
 	u32 reserved1;
-	u64 plat_cert_address;			/* In */
-	u32 plat_cert_len;			/* In */
+	u64 plat_certs_address;			/* In */
+	u32 plat_certs_len;			/* In */
 	u32 reserved2;
-	u64 amd_cert_address;			/* In */
-	u32 amd_cert_len;			/* In */
+	u64 amd_certs_address;			/* In */
+	u32 amd_certs_len;			/* In */
 	u32 reserved3;
 	u64 session_address;			/* In */
 	u32 session_len;			/* In/Out */
include/uapi/linux/kvm.h +12 -0
index 428c7dde6b4b..8827d43e2684 100644
@@ -1598,6 +1598,18 @@ struct kvm_sev_dbg {
 	__u32 len;
 };
 
+struct kvm_sev_send_start {
+	__u32 policy;
+	__u64 pdh_cert_uaddr;
+	__u32 pdh_cert_len;
+	__u64 plat_certs_uaddr;
+	__u32 plat_certs_len;
+	__u64 amd_certs_uaddr;
+	__u32 amd_certs_len;
+	__u64 session_uaddr;
+	__u32 session_len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
Signature · 1 line
2.17.1
#3 lore raw

Re: [PATCH v7 01/18] KVM: SVM: Add KVM_SEV SEND_START command

Quoted 171 lines · Venu Busireddy
Quoted 42 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used to create an outgoing SEV guest encryption context.

---
 .../virt/kvm/amd-memory-encryption.rst        |  27 ++++
 arch/x86/kvm/svm/sev.c                        | 125 ++++++++++++++++++
 include/linux/psp-sev.h                       |   8 +-
 include/uapi/linux/kvm.h                      |  12 ++
 4 files changed, 168 insertions(+), 4 deletions(-)
Documentation/virt/kvm/amd-memory-encryption.rst +8 -0
index c3129b9ba5cb..4fd34fc5c7a7 100644
@@ -263,6 +263,33 @@ Returns: 0 on success, -negative on error
                 __u32 trans_len;
         };
 
+10. KVM_SEV_SEND_START
+----------------------
+
+The KVM_SEV_SEND_START command can be used by the hypervisor to create an
+outgoing guest encryption context.
+
+Parameters (in): struct kvm_sev_send_start
+

Documentation/virt/kvm/amd-memory-encryption.rst Returns: 0 on success, -negative on error · line 283

Quoted 10 lines
Documentation/virt/kvm/amd-memory-encryption.rst +10 -0
+Returns: 0 on success, -negative on error
+
+::
+        struct kvm_sev_send_start {
+                __u32 policy;                 /* guest policy */
+
+                __u64 pdh_cert_uaddr;         /* platform Diffie-Hellman certificate */
+                __u32 pdh_cert_len;
+
+                __u64 plat_certs_uadr;        /* platform certificate chain */

Can this be changed as mentioned in the previous review
(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&amp;data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846313842&amp;sdata=qfxRdCY3A1Tox%2FMI%2FQLmUcvIxbfL%2BwoR2fzfQa1FVkA%3D&amp;reserved=0)?

Documentation/virt/kvm/amd-memory-encryption.rst Returns: 0 on success, -negative on error · line 287

Quoted 4 lines
Documentation/virt/kvm/amd-memory-encryption.rst +4 -0
+                __u32 plat_certs_len;
+
+                __u64 amd_certs_uaddr;        /* AMD certificate */
+                __u32 amd_cert_len;

Can this be changed as mentioned in the previous review
(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&amp;data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846323835&amp;sdata=SMYG1m%2BT2KwNQ4Jed%2BJhsK6TQ7EYTKT16moEoZMTf7c%3D&amp;reserved=0)?

Quoted 106 lines
Documentation/virt/kvm/amd-memory-encryption.rst +5 -0
+
+                __u64 session_uaddr;          /* Guest session information */
+                __u32 session_len;
+        };
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +90 -0
index cf912b4aaba8..5a15b43b4349 100644
@@ -913,6 +913,128 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+/* Userspace wants to query session length. */
+static int
+__sev_send_start_query_session_length(struct kvm *kvm, struct kvm_sev_cmd *argp,
+				      struct kvm_sev_send_start *params)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_start *data;
+	int ret;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
+
+	params->session_len = data->session_len;
+	if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
+				sizeof(struct kvm_sev_send_start)))
+		ret = -EFAULT;
+
+	kfree(data);
+	return ret;
+}
+
+static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_start *data;
+	struct kvm_sev_send_start params;
+	void *amd_certs, *session_data;
+	void *pdh_cert, *plat_certs;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	if (copy_from_user(&params, (void __user *)(uintptr_t)argp->data,
+				sizeof(struct kvm_sev_send_start)))
+		return -EFAULT;
+
+	/* if session_len is zero, userspace wants to query the session length */
+	if (!params.session_len)
+		return __sev_send_start_query_session_length(kvm, argp,
+				&params);
+
+	/* some sanity checks */
+	if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
+	    !params.session_uaddr || params.session_len > SEV_FW_BLOB_MAX_SIZE)
+		return -EINVAL;
+
+	/* allocate the memory to hold the session data blob */
+	session_data = kmalloc(params.session_len, GFP_KERNEL_ACCOUNT);
+	if (!session_data)
+		return -ENOMEM;
+
+	/* copy the certificate blobs from userspace */
+	pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr,
+				params.pdh_cert_len);
+	if (IS_ERR(pdh_cert)) {
+		ret = PTR_ERR(pdh_cert);
+		goto e_free_session;
+	}
+
+	plat_certs = psp_copy_user_blob(params.plat_certs_uaddr,
+				params.plat_certs_len);
+	if (IS_ERR(plat_certs)) {
+		ret = PTR_ERR(plat_certs);
+		goto e_free_pdh;
+	}
+
+	amd_certs = psp_copy_user_blob(params.amd_certs_uaddr,
+				params.amd_certs_len);
+	if (IS_ERR(amd_certs)) {
+		ret = PTR_ERR(amd_certs);
+		goto e_free_plat_cert;
+	}
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+	if (data == NULL) {
+		ret = -ENOMEM;
+		goto e_free_amd_cert;
+	}
+
+	/* populate the FW SEND_START field with system physical address */
+	data->pdh_cert_address = __psp_pa(pdh_cert);
+	data->pdh_cert_len = params.pdh_cert_len;
+	data->plat_certs_address = __psp_pa(plat_certs);
+	data->plat_certs_len = params.plat_certs_len;
+	data->amd_certs_address = __psp_pa(amd_certs);
Quoted 9 lines

arch/x86/kvm/svm/sev.c static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp) · line 1010

Quoted 5 lines
arch/x86/kvm/svm/sev.c +5 -0
+	data->amd_certs_len = params.amd_certs_len;
+	data->session_address = __psp_pa(session_data);
+	data->session_len = params.session_len;
+	data->handle = sev->handle;
+

Can the following code be changed as acknowledged in
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2Ff715bf99-0158-4d5f-77f3-b27743db3c59%40amd.com%2F&amp;data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846323835&amp;sdata=5hbjsP%2Btxt2rdv2PtIc%2BV8cAwKUNsRdtiRglDupYXzs%3D&amp;reserved=0?

I believe that this has been already addressed as discussed :

Ah, so the main issue is we should not be going to e_free on error. If session_len is less than the expected len then FW will return an error. In the case of an error we can skip copying the session_data into userspace buffer but we still need to pass the session_len and policy back to the userspace.

So this patch is still returning session_len and policy back to user in case of error : ( as the code below shows )

if (!ret && copy_to_user((void
__user*)(uintptr_t)params.session_uaddr,...

Thanks,
Ashish

Quoted 86 lines
Quoted 86 lines
arch/x86/kvm/svm/sev.c +30 -0
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
+
+	if (!ret && copy_to_user((void __user *)(uintptr_t)params.session_uaddr,
+			session_data, params.session_len)) {
+		ret = -EFAULT;
+		goto e_free;
+	}
+
+	params.policy = data->policy;
+	params.session_len = data->session_len;
+	if (copy_to_user((void __user *)(uintptr_t)argp->data, &params,
+				sizeof(struct kvm_sev_send_start)))
+		ret = -EFAULT;
+
+e_free:
+	kfree(data);
+e_free_amd_cert:
+	kfree(amd_certs);
+e_free_plat_cert:
+	kfree(plat_certs);
+e_free_pdh:
+	kfree(pdh_cert);
+e_free_session:
+	kfree(session_data);
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -957,6 +1079,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_LAUNCH_SECRET:
 		r = sev_launch_secret(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_SEND_START:
+		r = sev_send_start(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
include/linux/psp-sev.h +4 -4
index 5167bf2bfc75..9f63b9d48b63 100644
@@ -323,11 +323,11 @@ struct sev_data_send_start {
 	u64 pdh_cert_address;			/* In */
 	u32 pdh_cert_len;			/* In */
 	u32 reserved1;
-	u64 plat_cert_address;			/* In */
-	u32 plat_cert_len;			/* In */
+	u64 plat_certs_address;			/* In */
+	u32 plat_certs_len;			/* In */
 	u32 reserved2;
-	u64 amd_cert_address;			/* In */
-	u32 amd_cert_len;			/* In */
+	u64 amd_certs_address;			/* In */
+	u32 amd_certs_len;			/* In */
 	u32 reserved3;
 	u64 session_address;			/* In */
 	u32 session_len;			/* In/Out */
include/uapi/linux/kvm.h +12 -0
index 428c7dde6b4b..8827d43e2684 100644
@@ -1598,6 +1598,18 @@ struct kvm_sev_dbg {
 	__u32 len;
 };
 
+struct kvm_sev_send_start {
+	__u32 policy;
+	__u64 pdh_cert_uaddr;
+	__u32 pdh_cert_len;
+	__u64 plat_certs_uaddr;
+	__u32 plat_certs_len;
+	__u64 amd_certs_uaddr;
+	__u32 amd_certs_len;
+	__u64 session_uaddr;
+	__u32 session_len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
Signature · 1 line
2.17.1
#4 lore raw

Re: [PATCH v7 01/18] KVM: SVM: Add KVM_SEV SEND_START command

Quoted 190 lines · Ashish Kalra
Quoted 171 lines · Venu Busireddy
Quoted 42 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used to create an outgoing SEV guest encryption context.

---
 .../virt/kvm/amd-memory-encryption.rst        |  27 ++++
 arch/x86/kvm/svm/sev.c                        | 125 ++++++++++++++++++
 include/linux/psp-sev.h                       |   8 +-
 include/uapi/linux/kvm.h                      |  12 ++
 4 files changed, 168 insertions(+), 4 deletions(-)
Documentation/virt/kvm/amd-memory-encryption.rst +8 -0
index c3129b9ba5cb..4fd34fc5c7a7 100644
@@ -263,6 +263,33 @@ Returns: 0 on success, -negative on error
                 __u32 trans_len;
         };
 
+10. KVM_SEV_SEND_START
+----------------------
+
+The KVM_SEV_SEND_START command can be used by the hypervisor to create an
+outgoing guest encryption context.
+
+Parameters (in): struct kvm_sev_send_start
+

Documentation/virt/kvm/amd-memory-encryption.rst Returns: 0 on success, -negative on error · line 283

Quoted 10 lines
Documentation/virt/kvm/amd-memory-encryption.rst +10 -0
+Returns: 0 on success, -negative on error
+
+::
+        struct kvm_sev_send_start {
+                __u32 policy;                 /* guest policy */
+
+                __u64 pdh_cert_uaddr;         /* platform Diffie-Hellman certificate */
+                __u32 pdh_cert_len;
+
+                __u64 plat_certs_uadr;        /* platform certificate chain */

Can this be changed as mentioned in the previous review
(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&amp;data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846313842&amp;sdata=qfxRdCY3A1Tox%2FMI%2FQLmUcvIxbfL%2BwoR2fzfQa1FVkA%3D&amp;reserved=0)?

Documentation/virt/kvm/amd-memory-encryption.rst Returns: 0 on success, -negative on error · line 287

Quoted 4 lines
Documentation/virt/kvm/amd-memory-encryption.rst +4 -0
+                __u32 plat_certs_len;
+
+                __u64 amd_certs_uaddr;        /* AMD certificate */
+                __u32 amd_cert_len;

Can this be changed as mentioned in the previous review
(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&amp;data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846323835&amp;sdata=SMYG1m%2BT2KwNQ4Jed%2BJhsK6TQ7EYTKT16moEoZMTf7c%3D&amp;reserved=0)?

Quoted 106 lines
Documentation/virt/kvm/amd-memory-encryption.rst +5 -0
+
+                __u64 session_uaddr;          /* Guest session information */
+                __u32 session_len;
+        };
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +90 -0
index cf912b4aaba8..5a15b43b4349 100644
@@ -913,6 +913,128 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+/* Userspace wants to query session length. */
+static int
+__sev_send_start_query_session_length(struct kvm *kvm, struct kvm_sev_cmd *argp,
+				      struct kvm_sev_send_start *params)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_start *data;
+	int ret;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
+
+	params->session_len = data->session_len;
+	if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
+				sizeof(struct kvm_sev_send_start)))
+		ret = -EFAULT;
+
+	kfree(data);
+	return ret;
+}
+
+static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_start *data;
+	struct kvm_sev_send_start params;
+	void *amd_certs, *session_data;
+	void *pdh_cert, *plat_certs;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	if (copy_from_user(&params, (void __user *)(uintptr_t)argp->data,
+				sizeof(struct kvm_sev_send_start)))
+		return -EFAULT;
+
+	/* if session_len is zero, userspace wants to query the session length */
+	if (!params.session_len)
+		return __sev_send_start_query_session_length(kvm, argp,
+				&params);
+
+	/* some sanity checks */
+	if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
+	    !params.session_uaddr || params.session_len > SEV_FW_BLOB_MAX_SIZE)
+		return -EINVAL;
+
+	/* allocate the memory to hold the session data blob */
+	session_data = kmalloc(params.session_len, GFP_KERNEL_ACCOUNT);
+	if (!session_data)
+		return -ENOMEM;
+
+	/* copy the certificate blobs from userspace */
+	pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr,
+				params.pdh_cert_len);
+	if (IS_ERR(pdh_cert)) {
+		ret = PTR_ERR(pdh_cert);
+		goto e_free_session;
+	}
+
+	plat_certs = psp_copy_user_blob(params.plat_certs_uaddr,
+				params.plat_certs_len);
+	if (IS_ERR(plat_certs)) {
+		ret = PTR_ERR(plat_certs);
+		goto e_free_pdh;
+	}
+
+	amd_certs = psp_copy_user_blob(params.amd_certs_uaddr,
+				params.amd_certs_len);
+	if (IS_ERR(amd_certs)) {
+		ret = PTR_ERR(amd_certs);
+		goto e_free_plat_cert;
+	}
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+	if (data == NULL) {
+		ret = -ENOMEM;
+		goto e_free_amd_cert;
+	}
+
+	/* populate the FW SEND_START field with system physical address */
+	data->pdh_cert_address = __psp_pa(pdh_cert);
+	data->pdh_cert_len = params.pdh_cert_len;
+	data->plat_certs_address = __psp_pa(plat_certs);
+	data->plat_certs_len = params.plat_certs_len;
+	data->amd_certs_address = __psp_pa(amd_certs);
Quoted 9 lines

arch/x86/kvm/svm/sev.c static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp) · line 1010

Quoted 5 lines
arch/x86/kvm/svm/sev.c +5 -0
+	data->amd_certs_len = params.amd_certs_len;
+	data->session_address = __psp_pa(session_data);
+	data->session_len = params.session_len;
+	data->handle = sev->handle;
+

Can the following code be changed as acknowledged in
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2Ff715bf99-0158-4d5f-77f3-b27743db3c59%40amd.com%2F&amp;data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846323835&amp;sdata=5hbjsP%2Btxt2rdv2PtIc%2BV8cAwKUNsRdtiRglDupYXzs%3D&amp;reserved=0?

I believe that this has been already addressed as discussed :

Ah, so the main issue is we should not be going to e_free on error. If session_len is less than the expected len then FW will return an error. In the case of an error we can skip copying the session_data into userspace buffer but we still need to pass the session_len and policy back to the userspace.

Quoted 5 lines

So this patch is still returning session_len and policy back to user in case of error : ( as the code below shows )

if (!ret && copy_to_user((void
__user*)(uintptr_t)params.session_uaddr,...

This fix addresses only one part of the problem. I am referring to the other suggestion about avoiding copying the entire kvm_sev_send_start structure back to the user. As I was mentioning in the discussion, the only fields that changed are the policy and session_len fields. So, why copy back the entire structure? Why not just those two fields?

Quoted 90 lines

Thanks,
Ashish

Quoted 86 lines
Quoted 86 lines
arch/x86/kvm/svm/sev.c +30 -0
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
+
+	if (!ret && copy_to_user((void __user *)(uintptr_t)params.session_uaddr,
+			session_data, params.session_len)) {
+		ret = -EFAULT;
+		goto e_free;
+	}
+
+	params.policy = data->policy;
+	params.session_len = data->session_len;
+	if (copy_to_user((void __user *)(uintptr_t)argp->data, &params,
+				sizeof(struct kvm_sev_send_start)))
+		ret = -EFAULT;
+
+e_free:
+	kfree(data);
+e_free_amd_cert:
+	kfree(amd_certs);
+e_free_plat_cert:
+	kfree(plat_certs);
+e_free_pdh:
+	kfree(pdh_cert);
+e_free_session:
+	kfree(session_data);
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -957,6 +1079,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_LAUNCH_SECRET:
 		r = sev_launch_secret(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_SEND_START:
+		r = sev_send_start(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
include/linux/psp-sev.h +4 -4
index 5167bf2bfc75..9f63b9d48b63 100644
@@ -323,11 +323,11 @@ struct sev_data_send_start {
 	u64 pdh_cert_address;			/* In */
 	u32 pdh_cert_len;			/* In */
 	u32 reserved1;
-	u64 plat_cert_address;			/* In */
-	u32 plat_cert_len;			/* In */
+	u64 plat_certs_address;			/* In */
+	u32 plat_certs_len;			/* In */
 	u32 reserved2;
-	u64 amd_cert_address;			/* In */
-	u32 amd_cert_len;			/* In */
+	u64 amd_certs_address;			/* In */
+	u32 amd_certs_len;			/* In */
 	u32 reserved3;
 	u64 session_address;			/* In */
 	u32 session_len;			/* In/Out */
include/uapi/linux/kvm.h +12 -0
index 428c7dde6b4b..8827d43e2684 100644
@@ -1598,6 +1598,18 @@ struct kvm_sev_dbg {
 	__u32 len;
 };
 
+struct kvm_sev_send_start {
+	__u32 policy;
+	__u64 pdh_cert_uaddr;
+	__u32 pdh_cert_len;
+	__u64 plat_certs_uaddr;
+	__u32 plat_certs_len;
+	__u64 amd_certs_uaddr;
+	__u32 amd_certs_len;
+	__u64 session_uaddr;
+	__u32 session_len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
Signature · 1 line
2.17.1
#5 lore raw

Re: [PATCH v7 01/18] KVM: SVM: Add KVM_SEV SEND_START command

Quoted 194 lines · Venu Busireddy
Quoted 193 lines · Ashish Kalra
Quoted 171 lines · Venu Busireddy
Quoted 42 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used to create an outgoing SEV guest encryption context.

---
 .../virt/kvm/amd-memory-encryption.rst        |  27 ++++
 arch/x86/kvm/svm/sev.c                        | 125 ++++++++++++++++++
 include/linux/psp-sev.h                       |   8 +-
 include/uapi/linux/kvm.h                      |  12 ++
 4 files changed, 168 insertions(+), 4 deletions(-)
Documentation/virt/kvm/amd-memory-encryption.rst +8 -0
index c3129b9ba5cb..4fd34fc5c7a7 100644
@@ -263,6 +263,33 @@ Returns: 0 on success, -negative on error
                 __u32 trans_len;
         };
 
+10. KVM_SEV_SEND_START
+----------------------
+
+The KVM_SEV_SEND_START command can be used by the hypervisor to create an
+outgoing guest encryption context.
+
+Parameters (in): struct kvm_sev_send_start
+

Documentation/virt/kvm/amd-memory-encryption.rst Returns: 0 on success, -negative on error · line 283

Quoted 10 lines
Documentation/virt/kvm/amd-memory-encryption.rst +10 -0
+Returns: 0 on success, -negative on error
+
+::
+        struct kvm_sev_send_start {
+                __u32 policy;                 /* guest policy */
+
+                __u64 pdh_cert_uaddr;         /* platform Diffie-Hellman certificate */
+                __u32 pdh_cert_len;
+
+                __u64 plat_certs_uadr;        /* platform certificate chain */

Can this be changed as mentioned in the previous review
(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&amp;data=02%7C01%7Cashish.kalra%40amd.com%7Cf8d037dd37224ed4a33608d7f08067f5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242306610788233&amp;sdata=eu1SgC6ukawOuxTeAoA3mzKiMNWlPCCa5lGEVs9D1s0%3D&amp;reserved=0)?

Documentation/virt/kvm/amd-memory-encryption.rst Returns: 0 on success, -negative on error · line 287

Quoted 4 lines
Documentation/virt/kvm/amd-memory-encryption.rst +4 -0
+                __u32 plat_certs_len;
+
+                __u64 amd_certs_uaddr;        /* AMD certificate */
+                __u32 amd_cert_len;

Can this be changed as mentioned in the previous review
(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&amp;data=02%7C01%7Cashish.kalra%40amd.com%7Cf8d037dd37224ed4a33608d7f08067f5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242306610798235&amp;sdata=7xgp4ko8VGokGGQz1X1yPhz1ZXY9R501YginoDcen90%3D&amp;reserved=0)?

Quoted 106 lines
Documentation/virt/kvm/amd-memory-encryption.rst +5 -0
+
+                __u64 session_uaddr;          /* Guest session information */
+                __u32 session_len;
+        };
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +90 -0
index cf912b4aaba8..5a15b43b4349 100644
@@ -913,6 +913,128 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+/* Userspace wants to query session length. */
+static int
+__sev_send_start_query_session_length(struct kvm *kvm, struct kvm_sev_cmd *argp,
+				      struct kvm_sev_send_start *params)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_start *data;
+	int ret;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
+
+	params->session_len = data->session_len;
+	if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
+				sizeof(struct kvm_sev_send_start)))
+		ret = -EFAULT;
+
+	kfree(data);
+	return ret;
+}
+
+static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_start *data;
+	struct kvm_sev_send_start params;
+	void *amd_certs, *session_data;
+	void *pdh_cert, *plat_certs;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	if (copy_from_user(&params, (void __user *)(uintptr_t)argp->data,
+				sizeof(struct kvm_sev_send_start)))
+		return -EFAULT;
+
+	/* if session_len is zero, userspace wants to query the session length */
+	if (!params.session_len)
+		return __sev_send_start_query_session_length(kvm, argp,
+				&params);
+
+	/* some sanity checks */
+	if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
+	    !params.session_uaddr || params.session_len > SEV_FW_BLOB_MAX_SIZE)
+		return -EINVAL;
+
+	/* allocate the memory to hold the session data blob */
+	session_data = kmalloc(params.session_len, GFP_KERNEL_ACCOUNT);
+	if (!session_data)
+		return -ENOMEM;
+
+	/* copy the certificate blobs from userspace */
+	pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr,
+				params.pdh_cert_len);
+	if (IS_ERR(pdh_cert)) {
+		ret = PTR_ERR(pdh_cert);
+		goto e_free_session;
+	}
+
+	plat_certs = psp_copy_user_blob(params.plat_certs_uaddr,
+				params.plat_certs_len);
+	if (IS_ERR(plat_certs)) {
+		ret = PTR_ERR(plat_certs);
+		goto e_free_pdh;
+	}
+
+	amd_certs = psp_copy_user_blob(params.amd_certs_uaddr,
+				params.amd_certs_len);
+	if (IS_ERR(amd_certs)) {
+		ret = PTR_ERR(amd_certs);
+		goto e_free_plat_cert;
+	}
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+	if (data == NULL) {
+		ret = -ENOMEM;
+		goto e_free_amd_cert;
+	}
+
+	/* populate the FW SEND_START field with system physical address */
+	data->pdh_cert_address = __psp_pa(pdh_cert);
+	data->pdh_cert_len = params.pdh_cert_len;
+	data->plat_certs_address = __psp_pa(plat_certs);
+	data->plat_certs_len = params.plat_certs_len;
+	data->amd_certs_address = __psp_pa(amd_certs);
Quoted 9 lines

arch/x86/kvm/svm/sev.c static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp) · line 1010

Quoted 5 lines
arch/x86/kvm/svm/sev.c +5 -0
+	data->amd_certs_len = params.amd_certs_len;
+	data->session_address = __psp_pa(session_data);
+	data->session_len = params.session_len;
+	data->handle = sev->handle;
+

Can the following code be changed as acknowledged in
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2Ff715bf99-0158-4d5f-77f3-b27743db3c59%40amd.com%2F&amp;data=02%7C01%7Cashish.kalra%40amd.com%7Cf8d037dd37224ed4a33608d7f08067f5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242306610798235&amp;sdata=s3ZwfpCm94x2LCNN8I1jjcWcrkj3Nrw5PnOvxdDxSCs%3D&amp;reserved=0?

I believe that this has been already addressed as discussed :

Ah, so the main issue is we should not be going to e_free on error. If session_len is less than the expected len then FW will return an error. In the case of an error we can skip copying the session_data into userspace buffer but we still need to pass the session_len and policy back to the userspace.

So this patch is still returning session_len and policy back to user in case of error : ( as the code below shows )

Quoted 10 lines
Quoted 2 lines

if (!ret && copy_to_user((void
__user*)(uintptr_t)params.session_uaddr,...

This fix addresses only one part of the problem. I am referring to the other suggestion about avoiding copying the entire kvm_sev_send_start structure back to the user. As I was mentioning in the discussion, the only fields that changed are the policy and session_len fields. So, why copy back the entire structure? Why not just those two fields?

Quoted 1 line

Both policy and session_len are two different fields in the kvm_sev_send_start structure, why to complicate it by using two different copy_to_user()'s here, to send two 32-bit words back to user, it's much simpler to copy the whole structure back in one copy_to_user() (makes it less complicated with no additional address computations), also this is just a session start code, so it is invoked once at the beginning of the migration process and it is not a performance critical path such that saving few bytes of copy_to_user() wil affect it.

Thanks,
Ashish

Quoted 87 lines
Quoted 87 lines
Quoted 87 lines
Quoted 87 lines
arch/x86/kvm/svm/sev.c +30 -0
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
+
+	if (!ret && copy_to_user((void __user *)(uintptr_t)params.session_uaddr,
+			session_data, params.session_len)) {
+		ret = -EFAULT;
+		goto e_free;
+	}
+
+	params.policy = data->policy;
+	params.session_len = data->session_len;
+	if (copy_to_user((void __user *)(uintptr_t)argp->data, &params,
+				sizeof(struct kvm_sev_send_start)))
+		ret = -EFAULT;
+
+e_free:
+	kfree(data);
+e_free_amd_cert:
+	kfree(amd_certs);
+e_free_plat_cert:
+	kfree(plat_certs);
+e_free_pdh:
+	kfree(pdh_cert);
+e_free_session:
+	kfree(session_data);
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -957,6 +1079,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_LAUNCH_SECRET:
 		r = sev_launch_secret(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_SEND_START:
+		r = sev_send_start(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
include/linux/psp-sev.h +4 -4
index 5167bf2bfc75..9f63b9d48b63 100644
@@ -323,11 +323,11 @@ struct sev_data_send_start {
 	u64 pdh_cert_address;			/* In */
 	u32 pdh_cert_len;			/* In */
 	u32 reserved1;
-	u64 plat_cert_address;			/* In */
-	u32 plat_cert_len;			/* In */
+	u64 plat_certs_address;			/* In */
+	u32 plat_certs_len;			/* In */
 	u32 reserved2;
-	u64 amd_cert_address;			/* In */
-	u32 amd_cert_len;			/* In */
+	u64 amd_certs_address;			/* In */
+	u32 amd_certs_len;			/* In */
 	u32 reserved3;
 	u64 session_address;			/* In */
 	u32 session_len;			/* In/Out */
include/uapi/linux/kvm.h +12 -0
index 428c7dde6b4b..8827d43e2684 100644
@@ -1598,6 +1598,18 @@ struct kvm_sev_dbg {
 	__u32 len;
 };
 
+struct kvm_sev_send_start {
+	__u32 policy;
+	__u64 pdh_cert_uaddr;
+	__u32 pdh_cert_len;
+	__u64 plat_certs_uaddr;
+	__u32 plat_certs_len;
+	__u64 amd_certs_uaddr;
+	__u32 amd_certs_len;
+	__u64 session_uaddr;
+	__u32 session_len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
Signature · 1 line
2.17.1
#6 lore raw

PATCH v7 2/18 KVM: SVM: Add KVM_SEND_UPDATE_DATA command

In reply to Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used for encrypting the guest memory region using the encryption context created with KVM_SEV_SEND_START.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 .../virt/kvm/amd-memory-encryption.rst        |  24 ++++
 arch/x86/kvm/svm/sev.c                        | 135 +++++++++++++++++-
 include/uapi/linux/kvm.h                      |   9 ++
 3 files changed, 164 insertions(+), 4 deletions(-)
Documentation/virt/kvm/amd-memory-encryption.rst +24 -0
index 4fd34fc5c7a7..f46817ef7019 100644
@@ -290,6 +290,30 @@ Returns: 0 on success, -negative on error
                 __u32 session_len;
         };
 
+11. KVM_SEV_SEND_UPDATE_DATA
+----------------------------
+
+The KVM_SEV_SEND_UPDATE_DATA command can be used by the hypervisor to encrypt the
+outgoing guest memory region with the encryption context creating using
+KVM_SEV_SEND_START.
+
+Parameters (in): struct kvm_sev_send_update_data
+
+Returns: 0 on success, -negative on error
+
+::
+
+        struct kvm_sev_launch_send_update_data {
+                __u64 hdr_uaddr;        /* userspace address containing the packet header */
+                __u32 hdr_len;
+
+                __u64 guest_uaddr;      /* the source memory region to be encrypted */
+                __u32 guest_len;
+
+                __u64 trans_uaddr;      /* the destition memory region  */
+                __u32 trans_len;
+        };
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +131 -4

165 lines not on this page · raw message

include/uapi/linux/kvm.h +9 -0
index 8827d43e2684..7aaed8ee33cf 100644
@@ -1610,6 +1610,15 @@ struct kvm_sev_send_start {
 	__u32 session_len;
 };
 
+struct kvm_sev_send_update_data {
+	__u64 hdr_uaddr;
+	__u32 hdr_len;
+	__u64 guest_uaddr;
+	__u32 guest_len;
+	__u64 trans_uaddr;
+	__u32 trans_len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
Signature · 1 line
2.17.1
#7 lore raw

Re: [PATCH v7 02/18] KVM: SVM: Add KVM_SEND_UPDATE_DATA command

Quoted 165 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used for encrypting the guest memory region using the encryption context created with KVM_SEV_SEND_START.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 .../virt/kvm/amd-memory-encryption.rst        |  24 ++++
 arch/x86/kvm/svm/sev.c                        | 135 +++++++++++++++++-
 include/uapi/linux/kvm.h                      |   9 ++
 3 files changed, 164 insertions(+), 4 deletions(-)
Documentation/virt/kvm/amd-memory-encryption.rst +24 -0
index 4fd34fc5c7a7..f46817ef7019 100644
@@ -290,6 +290,30 @@ Returns: 0 on success, -negative on error
                 __u32 session_len;
         };
 
+11. KVM_SEV_SEND_UPDATE_DATA
+----------------------------
+
+The KVM_SEV_SEND_UPDATE_DATA command can be used by the hypervisor to encrypt the
+outgoing guest memory region with the encryption context creating using
+KVM_SEV_SEND_START.
+
+Parameters (in): struct kvm_sev_send_update_data
+
+Returns: 0 on success, -negative on error
+
+::
+
+        struct kvm_sev_launch_send_update_data {
+                __u64 hdr_uaddr;        /* userspace address containing the packet header */
+                __u32 hdr_len;
+
+                __u64 guest_uaddr;      /* the source memory region to be encrypted */
+                __u32 guest_len;
+
+                __u64 trans_uaddr;      /* the destition memory region  */
+                __u32 trans_len;
+        };
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +90 -0
index 5a15b43b4349..0c92c16505ab 100644
@@ -23,6 +23,7 @@ static DECLARE_RWSEM(sev_deactivate_lock);
 static DEFINE_MUTEX(sev_bitmap_lock);
 unsigned int max_sev_asid;
 static unsigned int min_sev_asid;
+static unsigned long sev_me_mask;
 static unsigned long *sev_asid_bitmap;
 static unsigned long *sev_reclaim_asid_bitmap;
 #define __sme_page_pa(x) __sme_set(page_to_pfn(x) << PAGE_SHIFT)
@@ -1035,6 +1036,123 @@ static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+/* Userspace wants to query either header or trans length. */
+static int
+__sev_send_update_data_query_lengths(struct kvm *kvm, struct kvm_sev_cmd *argp,
+				     struct kvm_sev_send_update_data *params)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_update_data *data;
+	int ret;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+	if (!data)
+		return -ENOMEM;
+
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_UPDATE_DATA, data, &argp->error);
+
+	params->hdr_len = data->hdr_len;
+	params->trans_len = data->trans_len;
+
+	if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
+			 sizeof(struct kvm_sev_send_update_data)))
+		ret = -EFAULT;
+
+	kfree(data);
+	return ret;
+}
+
+static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_update_data *data;
+	struct kvm_sev_send_update_data params;
+	void *hdr, *trans_data;
+	struct page **guest_page;
+	unsigned long n;
+	int ret, offset;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	if (copy_from_user(&params, (void __user *)(uintptr_t)argp->data,
+			sizeof(struct kvm_sev_send_update_data)))
+		return -EFAULT;
+
+	/* userspace wants to query either header or trans length */
+	if (!params.trans_len || !params.hdr_len)
+		return __sev_send_update_data_query_lengths(kvm, argp, &params);
+
+	if (!params.trans_uaddr || !params.guest_uaddr ||
+	    !params.guest_len || !params.hdr_uaddr)
+		return -EINVAL;
+
+	/* Check if we are crossing the page boundary */
+	offset = params.guest_uaddr & (PAGE_SIZE - 1);
+	if ((params.guest_len + offset > PAGE_SIZE))
+		return -EINVAL;
+
+	/* Pin guest memory */
+	guest_page = sev_pin_memory(kvm, params.guest_uaddr & PAGE_MASK,
+				    PAGE_SIZE, &n, 0);
+	if (!guest_page)
+		return -EFAULT;
+
+	/* allocate memory for header and transport buffer */
+	ret = -ENOMEM;
+	hdr = kmalloc(params.hdr_len, GFP_KERNEL_ACCOUNT);
+	if (!hdr)
+		goto e_unpin;
+
+	trans_data = kmalloc(params.trans_len, GFP_KERNEL_ACCOUNT);
+	if (!trans_data)
+		goto e_free_hdr;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		goto e_free_trans_data;
+
+	data->hdr_address = __psp_pa(hdr);
+	data->hdr_len = params.hdr_len;
+	data->trans_address = __psp_pa(trans_data);
+	data->trans_len = params.trans_len;
+
+	/* The SEND_UPDATE_DATA command requires C-bit to be always set. */
+	data->guest_address = (page_to_pfn(guest_page[0]) << PAGE_SHIFT) +
+				offset;
+	data->guest_address |= sev_me_mask;
+	data->guest_len = params.guest_len;
+	data->handle = sev->handle;
+

arch/x86/kvm/svm/sev.c static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp) · line 1137

Quoted 10 lines
arch/x86/kvm/svm/sev.c +10 -0
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_UPDATE_DATA, data, &argp->error);
+
+	if (ret)
+		goto e_free;
+
+	/* copy transport buffer to user space */
+	if (copy_to_user((void __user *)(uintptr_t)params.trans_uaddr,
+			 trans_data, params.trans_len)) {
+		ret = -EFAULT;
+		goto e_unpin;

Can you address my comment in v6 about this goto statement?
https://lore.kernel.org/kvm/20200402175529.GA655472@vbusired-dt/

Quoted 81 lines
arch/x86/kvm/svm/sev.c +31 -4
+	}
+
+	/* Copy packet header to userspace. */
+	ret = copy_to_user((void __user *)(uintptr_t)params.hdr_uaddr, hdr,
+				params.hdr_len);
+
+e_free:
+	kfree(data);
+e_free_trans_data:
+	kfree(trans_data);
+e_free_hdr:
+	kfree(hdr);
+e_unpin:
+	sev_unpin_memory(kvm, guest_page, n);
+
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -1082,6 +1200,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_SEND_START:
 		r = sev_send_start(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_SEND_UPDATE_DATA:
+		r = sev_send_update_data(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
@@ -1238,16 +1359,22 @@ void sev_vm_destroy(struct kvm *kvm)
 int __init sev_hardware_setup(void)
 {
 	struct sev_user_data_status *status;
+	u32 eax, ebx;
 	int rc;
 
-	/* Maximum number of encrypted guests supported simultaneously */
-	max_sev_asid = cpuid_ecx(0x8000001F);
+	/*
+	 * Query the memory encryption information.
+	 *  EBX:  Bit 0:5 Pagetable bit position used to indicate encryption
+	 *  (aka Cbit).
+	 *  ECX:  Maximum number of encrypted guests supported simultaneously.
+	 *  EDX:  Minimum ASID value that should be used for SEV guest.
+	 */
+	cpuid(0x8000001f, &eax, &ebx, &max_sev_asid, &min_sev_asid);
 
 	if (!svm_sev_enabled())
 		return 1;
 
-	/* Minimum ASID value that should be used for SEV guest */
-	min_sev_asid = cpuid_edx(0x8000001F);
+	sev_me_mask = 1UL << (ebx & 0x3f);
 
 	/* Initialize SEV ASID bitmaps */
 	sev_asid_bitmap = bitmap_zalloc(max_sev_asid, GFP_KERNEL);
include/uapi/linux/kvm.h +9 -0
index 8827d43e2684..7aaed8ee33cf 100644
@@ -1610,6 +1610,15 @@ struct kvm_sev_send_start {
 	__u32 session_len;
 };
 
+struct kvm_sev_send_update_data {
+	__u64 hdr_uaddr;
+	__u32 hdr_len;
+	__u64 guest_uaddr;
+	__u32 guest_len;
+	__u64 trans_uaddr;
+	__u32 trans_len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
Signature · 1 line
2.17.1
#8 lore raw

PATCH v7 3/18 KVM: SVM: Add KVM_SEV_SEND_FINISH command

In reply to Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used to finailize the encryption context created with KVM_SEV_SEND_START command.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 .../virt/kvm/amd-memory-encryption.rst        |  8 +++++++
 arch/x86/kvm/svm/sev.c                        | 23 +++++++++++++++++++
 2 files changed, 31 insertions(+)
Documentation/virt/kvm/amd-memory-encryption.rst +8 -0
index f46817ef7019..a45dcb5f8687 100644
@@ -314,6 +314,14 @@ Returns: 0 on success, -negative on error
                 __u32 trans_len;
         };
 
+12. KVM_SEV_SEND_FINISH
+------------------------
+
+After completion of the migration flow, the KVM_SEV_SEND_FINISH command can be
+issued by the hypervisor to delete the encryption context.
+
+Returns: 0 on success, -negative on error
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +23 -0
index 0c92c16505ab..81d661706d31 100644
@@ -1153,6 +1153,26 @@ static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_finish *data;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_FINISH, data, &argp->error);
+
+	kfree(data);
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -1203,6 +1223,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_SEND_UPDATE_DATA:
 		r = sev_send_update_data(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_SEND_FINISH:
+		r = sev_send_finish(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
Signature · 1 line
2.17.1
#9 lore raw

Re: [PATCH v7 03/18] KVM: SVM: Add KVM_SEV_SEND_FINISH command

Documentation/virt/kvm/amd-memory-encryption.rst Returns: 0 on success, -negative on error

Quoted 29 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used to finailize the encryption context created with KVM_SEV_SEND_START command.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 .../virt/kvm/amd-memory-encryption.rst        |  8 +++++++
 arch/x86/kvm/svm/sev.c                        | 23 +++++++++++++++++++
 2 files changed, 31 insertions(+)
Documentation/virt/kvm/amd-memory-encryption.rst +0 -0
index f46817ef7019..a45dcb5f8687 100644
@@ -314,6 +314,14 @@ Returns: 0 on success, -negative on error

As I commented in the v6 patches
(see https://lore.kernel.org/kvm/20200402181710.GA655710@vbusired-dt/),
there are multiple occurrences of the word "-negative" in amd-memory-encryption.rst. Can these be changed to either "negative" or "less than 0"?

Quoted 58 lines
Documentation/virt/kvm/amd-memory-encryption.rst +8 -0
                 __u32 trans_len;
         };
 
+12. KVM_SEV_SEND_FINISH
+------------------------
+
+After completion of the migration flow, the KVM_SEV_SEND_FINISH command can be
+issued by the hypervisor to delete the encryption context.
+
+Returns: 0 on success, -negative on error
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +23 -0
index 0c92c16505ab..81d661706d31 100644
@@ -1153,6 +1153,26 @@ static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_send_finish *data;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_FINISH, data, &argp->error);
+
+	kfree(data);
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -1203,6 +1223,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_SEND_UPDATE_DATA:
 		r = sev_send_update_data(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_SEND_FINISH:
+		r = sev_send_finish(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
Signature · 1 line
2.17.1
#10 lore raw

PATCH v7 4/18 KVM: SVM: Add support for KVM_SEV_RECEIVE_START command

In reply to Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used to create the encryption context for an incoming SEV guest. The encryption context can be later used by the hypervisor to import the incoming data into the SEV guest memory space.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 .../virt/kvm/amd-memory-encryption.rst        | 29 +++++++
 arch/x86/kvm/svm/sev.c                        | 81 +++++++++++++++++++
 include/uapi/linux/kvm.h                      |  9 +++
 3 files changed, 119 insertions(+)
Documentation/virt/kvm/amd-memory-encryption.rst +29 -0
index a45dcb5f8687..ef1f1f3a5b40 100644
@@ -322,6 +322,35 @@ issued by the hypervisor to delete the encryption context.
 
 Returns: 0 on success, -negative on error
 
+13. KVM_SEV_RECEIVE_START
+------------------------
+
+The KVM_SEV_RECEIVE_START command is used for creating the memory encryption
+context for an incoming SEV guest. To create the encryption context, the user must
+provide a guest policy, the platform public Diffie-Hellman (PDH) key and session
+information.
+
+Parameters: struct  kvm_sev_receive_start (in/out)
+
+Returns: 0 on success, -negative on error
+
+::
+
+        struct kvm_sev_receive_start {
+                __u32 handle;           /* if zero then firmware creates a new handle */
+                __u32 policy;           /* guest's policy */
+
+                __u64 pdh_uaddr;         /* userspace address pointing to the PDH key */
+                __u32 dh_len;
+
+                __u64 session_addr;     /* userspace address which points to the guest session information */
+                __u32 session_len;
+        };
+
+On success, the 'handle' field contains a new handle and on error, a negative value.
+
+For more details, see SEV spec Section 6.12.
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +81 -0

93 lines not on this page · raw message

include/uapi/linux/kvm.h +9 -0
index 7aaed8ee33cf..24ac57151d53 100644
@@ -1619,6 +1619,15 @@ struct kvm_sev_send_update_data {
 	__u32 trans_len;
 };
 
+struct kvm_sev_receive_start {
+	__u32 handle;
+	__u32 policy;
+	__u64 pdh_uaddr;
+	__u32 pdh_len;
+	__u64 session_uaddr;
+	__u32 session_len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
Signature · 1 line
2.17.1
#11 lore raw

Re: [PATCH v7 04/18] KVM: SVM: Add support for KVM_SEV_RECEIVE_START command

Quoted 47 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used to create the encryption context for an incoming SEV guest. The encryption context can be later used by the hypervisor to import the incoming data into the SEV guest memory space.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 .../virt/kvm/amd-memory-encryption.rst        | 29 +++++++
 arch/x86/kvm/svm/sev.c                        | 81 +++++++++++++++++++
 include/uapi/linux/kvm.h                      |  9 +++
 3 files changed, 119 insertions(+)
Documentation/virt/kvm/amd-memory-encryption.rst +13 -0
index a45dcb5f8687..ef1f1f3a5b40 100644
@@ -322,6 +322,35 @@ issued by the hypervisor to delete the encryption context.
 
 Returns: 0 on success, -negative on error
 
+13. KVM_SEV_RECEIVE_START
+------------------------
+
+The KVM_SEV_RECEIVE_START command is used for creating the memory encryption
+context for an incoming SEV guest. To create the encryption context, the user must
+provide a guest policy, the platform public Diffie-Hellman (PDH) key and session
+information.
+
+Parameters: struct  kvm_sev_receive_start (in/out)
+
+Returns: 0 on success, -negative on error
+
+::

Documentation/virt/kvm/amd-memory-encryption.rst issued by the hypervisor to delete the encryption context. · line 347

Quoted 10 lines
Documentation/virt/kvm/amd-memory-encryption.rst +10 -0
+
+        struct kvm_sev_receive_start {
+                __u32 handle;           /* if zero then firmware creates a new handle */
+                __u32 policy;           /* guest's policy */
+
+                __u64 pdh_uaddr;         /* userspace address pointing to the PDH key */
+                __u32 dh_len;
+
+                __u64 session_addr;     /* userspace address which points to the guest session information */
+                __u32 session_len;

Can you address my comments in v6?
(https://lore.kernel.org/kvm/20200402213520.GA658288@vbusired-dt/)

Quoted 131 lines
Documentation/virt/kvm/amd-memory-encryption.rst +6 -0
+        };
+
+On success, the 'handle' field contains a new handle and on error, a negative value.
+
+For more details, see SEV spec Section 6.12.
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +81 -0
index 81d661706d31..74a847c9106d 100644
@@ -1173,6 +1173,84 @@ static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+static int sev_receive_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_receive_start *start;
+	struct kvm_sev_receive_start params;
+	int *error = &argp->error;
+	void *session_data;
+	void *pdh_data;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	/* Get parameter from the userspace */
+	if (copy_from_user(&params, (void __user *)(uintptr_t)argp->data,
+			sizeof(struct kvm_sev_receive_start)))
+		return -EFAULT;
+
+	/* some sanity checks */
+	if (!params.pdh_uaddr || !params.pdh_len ||
+	    !params.session_uaddr || !params.session_len)
+		return -EINVAL;
+
+	pdh_data = psp_copy_user_blob(params.pdh_uaddr, params.pdh_len);
+	if (IS_ERR(pdh_data))
+		return PTR_ERR(pdh_data);
+
+	session_data = psp_copy_user_blob(params.session_uaddr,
+			params.session_len);
+	if (IS_ERR(session_data)) {
+		ret = PTR_ERR(session_data);
+		goto e_free_pdh;
+	}
+
+	ret = -ENOMEM;
+	start = kzalloc(sizeof(*start), GFP_KERNEL);
+	if (!start)
+		goto e_free_session;
+
+	start->handle = params.handle;
+	start->policy = params.policy;
+	start->pdh_cert_address = __psp_pa(pdh_data);
+	start->pdh_cert_len = params.pdh_len;
+	start->session_address = __psp_pa(session_data);
+	start->session_len = params.session_len;
+
+	/* create memory encryption context */
+	ret = __sev_issue_cmd(argp->sev_fd, SEV_CMD_RECEIVE_START, start,
+				error);
+	if (ret)
+		goto e_free;
+
+	/* Bind ASID to this guest */
+	ret = sev_bind_asid(kvm, start->handle, error);
+	if (ret)
+		goto e_free;
+
+	params.handle = start->handle;
+	if (copy_to_user((void __user *)(uintptr_t)argp->data,
+			 &params, sizeof(struct kvm_sev_receive_start))) {
+		ret = -EFAULT;
+		sev_unbind_asid(kvm, start->handle);
+		goto e_free;
+	}
+
+	sev->handle = start->handle;
+	sev->fd = argp->sev_fd;
+
+e_free:
+	kfree(start);
+e_free_session:
+	kfree(session_data);
+e_free_pdh:
+	kfree(pdh_data);
+
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -1226,6 +1304,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_SEND_FINISH:
 		r = sev_send_finish(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_RECEIVE_START:
+		r = sev_receive_start(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
include/uapi/linux/kvm.h +9 -0
index 7aaed8ee33cf..24ac57151d53 100644
@@ -1619,6 +1619,15 @@ struct kvm_sev_send_update_data {
 	__u32 trans_len;
 };
 
+struct kvm_sev_receive_start {
+	__u32 handle;
+	__u32 policy;
+	__u64 pdh_uaddr;
+	__u32 pdh_len;
+	__u64 session_uaddr;
+	__u32 session_len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
Signature · 1 line
2.17.1
#12 lore raw

PATCH v7 5/18 KVM: SVM: Add KVM_SEV_RECEIVE_UPDATE_DATA command

In reply to Ashish Kalra

From: Brijesh Singh <[email protected]>

The command is used for copying the incoming buffer into the SEV guest memory space.

---
 .../virt/kvm/amd-memory-encryption.rst        | 24 ++++++
 arch/x86/kvm/svm/sev.c                        | 79 +++++++++++++++++++
 include/uapi/linux/kvm.h                      |  9 +++
 3 files changed, 112 insertions(+)
Documentation/virt/kvm/amd-memory-encryption.rst +24 -0
index ef1f1f3a5b40..554aa33a99cc 100644
@@ -351,6 +351,30 @@ On success, the 'handle' field contains a new handle and on error, a negative va
 
 For more details, see SEV spec Section 6.12.
 
+14. KVM_SEV_RECEIVE_UPDATE_DATA
+----------------------------
+
+The KVM_SEV_RECEIVE_UPDATE_DATA command can be used by the hypervisor to copy
+the incoming buffers into the guest memory region with encryption context
+created during the KVM_SEV_RECEIVE_START.
+
+Parameters (in): struct kvm_sev_receive_update_data
+
+Returns: 0 on success, -negative on error
+
+::
+
+        struct kvm_sev_launch_receive_update_data {
+                __u64 hdr_uaddr;        /* userspace address containing the packet header */
+                __u32 hdr_len;
+
+                __u64 guest_uaddr;      /* the destination guest memory region */
+                __u32 guest_len;
+
+                __u64 trans_uaddr;      /* the incoming buffer memory region  */
+                __u32 trans_len;
+        };
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +79 -0

91 lines not on this page · raw message

include/uapi/linux/kvm.h +9 -0
index 24ac57151d53..0fe1d206d750 100644
@@ -1628,6 +1628,15 @@ struct kvm_sev_receive_start {
 	__u32 session_len;
 };
 
+struct kvm_sev_receive_update_data {
+	__u64 hdr_uaddr;
+	__u32 hdr_len;
+	__u64 guest_uaddr;
+	__u32 guest_len;
+	__u64 trans_uaddr;
+	__u32 trans_len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
Signature · 1 line
2.17.1
#13 lore raw

PATCH v7 6/18 KVM: SVM: Add KVM_SEV_RECEIVE_FINISH command

In reply to Ashish Kalra

From: Brijesh Singh <[email protected]>

The command finalize the guest receiving process and make the SEV guest ready for the execution.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 .../virt/kvm/amd-memory-encryption.rst        |  8 +++++++
 arch/x86/kvm/svm/sev.c                        | 23 +++++++++++++++++++
 2 files changed, 31 insertions(+)
Documentation/virt/kvm/amd-memory-encryption.rst +8 -0
index 554aa33a99cc..93cd95d9a6c0 100644
@@ -375,6 +375,14 @@ Returns: 0 on success, -negative on error
                 __u32 trans_len;
         };
 
+15. KVM_SEV_RECEIVE_FINISH
+------------------------
+
+After completion of the migration flow, the KVM_SEV_RECEIVE_FINISH command can be
+issued by the hypervisor to make the guest ready for execution.
+
+Returns: 0 on success, -negative on error
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +23 -0
index d5dfd0da53b9..1f9181e37ef0 100644
@@ -1327,6 +1327,26 @@ static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+static int sev_receive_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_receive_finish *data;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_RECEIVE_FINISH, data, &argp->error);
+
+	kfree(data);
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -1386,6 +1406,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_RECEIVE_UPDATE_DATA:
 		r = sev_receive_update_data(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_RECEIVE_FINISH:
+		r = sev_receive_finish(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
Signature · 1 line
2.17.1
#14 lore raw

Re: [PATCH v7 06/18] KVM: SVM: Add KVM_SEV_RECEIVE_FINISH command

Quoted 49 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

The command finalize the guest receiving process and make the SEV guest ready for the execution.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 .../virt/kvm/amd-memory-encryption.rst        |  8 +++++++
 arch/x86/kvm/svm/sev.c                        | 23 +++++++++++++++++++
 2 files changed, 31 insertions(+)
Documentation/virt/kvm/amd-memory-encryption.rst +8 -0
index 554aa33a99cc..93cd95d9a6c0 100644
@@ -375,6 +375,14 @@ Returns: 0 on success, -negative on error
                 __u32 trans_len;
         };
 
+15. KVM_SEV_RECEIVE_FINISH
+------------------------
+
+After completion of the migration flow, the KVM_SEV_RECEIVE_FINISH command can be
+issued by the hypervisor to make the guest ready for execution.
+
+Returns: 0 on success, -negative on error
+
 References
 ==========
 
arch/x86/kvm/svm/sev.c +0 -0
index d5dfd0da53b9..1f9181e37ef0 100644
@@ -1327,6 +1327,26 @@ static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;

arch/x86/kvm/svm/sev.c static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp) · line 1337

Quoted 10 lines
arch/x86/kvm/svm/sev.c +8 -0
 }
 
+static int sev_receive_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_receive_finish *data;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;

What is the rationale for using -ENOTTY? Is it the best return value? Aren't one of -ENXIO, or -ENODEV, or -EINVAL a better choice?

Quoted 28 lines
arch/x86/kvm/svm/sev.c +15 -0
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_RECEIVE_FINISH, data, &argp->error);
+
+	kfree(data);
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -1386,6 +1406,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_RECEIVE_UPDATE_DATA:
 		r = sev_receive_update_data(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_RECEIVE_FINISH:
+		r = sev_receive_finish(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
Signature · 1 line
2.17.1
#15 lore raw

PATCH v7 7/18 KVM: x86: Add AMD SEV specific Hypercall3

In reply to Ashish Kalra

From: Brijesh Singh <[email protected]>

KVM hypercall framework relies on alternative framework to patch the VMCALL -> VMMCALL on AMD platform. If a hypercall is made before apply_alternative() is called then it defaults to VMCALL. The approach works fine on non SEV guest. A VMCALL would causes #UD, and hypervisor will be able to decode the instruction and do the right things. But when SEV is active, guest memory is encrypted with guest key and hypervisor will not be able to decode the instruction bytes.

Add SEV specific hypercall3, it unconditionally uses VMMCALL. The hypercall will be used by the SEV guest to notify encrypted pages to the hypervisor.

---
 arch/x86/include/asm/kvm_para.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)
arch/x86/include/asm/kvm_para.h +12 -0
index 9b4df6eaa11a..6c09255633a4 100644
@@ -84,6 +84,18 @@ static inline long kvm_hypercall4(unsigned int nr, unsigned long p1,
 	return ret;
 }
 
+static inline long kvm_sev_hypercall3(unsigned int nr, unsigned long p1,
+				      unsigned long p2, unsigned long p3)
+{
+	long ret;
+
+	asm volatile("vmmcall"
+		     : "=a"(ret)
+		     : "a"(nr), "b"(p1), "c"(p2), "d"(p3)
+		     : "memory");
+	return ret;
+}
+
 #ifdef CONFIG_KVM_GUEST
 bool kvm_para_available(void);
 unsigned int kvm_arch_para_features(void);
Signature · 1 line
2.17.1
#16 lore raw

PATCH v7 8/18 KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall

In reply to Ashish Kalra

From: Brijesh Singh <[email protected]>

This hypercall is used by the SEV guest to notify a change in the page encryption status to the hypervisor. The hypercall should be invoked only when the encryption attribute is changed from encrypted -> decrypted and vice versa. By default all guest pages are considered encrypted.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 Documentation/virt/kvm/hypercalls.rst | 15 +++++
 arch/x86/include/asm/kvm_host.h       |  2 +
 arch/x86/kvm/svm/sev.c                | 90 +++++++++++++++++++++++++++
 arch/x86/kvm/svm/svm.c                |  2 +
 arch/x86/kvm/svm/svm.h                |  4 ++
 arch/x86/kvm/vmx/vmx.c                |  1 +
 arch/x86/kvm/x86.c                    |  6 ++
 include/uapi/linux/kvm_para.h         |  1 +
 8 files changed, 121 insertions(+)
Documentation/virt/kvm/hypercalls.rst +15 -0
index dbaf207e560d..ff5287e68e81 100644
@@ -169,3 +169,18 @@ a0: destination APIC ID
 
 :Usage example: When sending a call-function IPI-many to vCPUs, yield if
 	        any of the IPI target vCPUs was preempted.
+
+
+8. KVM_HC_PAGE_ENC_STATUS
+-------------------------
+:Architecture: x86
+:Status: active
+:Purpose: Notify the encryption status changes in guest page table (SEV guest)
+
+a0: the guest physical address of the start page
+a1: the number of pages
+a2: encryption attribute
+
+   Where:
+	* 1: Encryption attribute is set
+	* 0: Encryption attribute is cleared
arch/x86/include/asm/kvm_host.h +2 -0
index 42a2d0d3984a..4a8ee22f4f5b 100644
@@ -1254,6 +1254,8 @@ struct kvm_x86_ops {
 
 	bool (*apic_init_signal_blocked)(struct kvm_vcpu *vcpu);
 	int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu);
+	int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa,
+				  unsigned long sz, unsigned long mode);
 };
 
 struct kvm_x86_init_ops {
arch/x86/kvm/svm/sev.c +90 -0

102 lines not on this page · raw message

arch/x86/kvm/svm/svm.c +2 -0
index 2f379bacbb26..1013ef0f4ce2 100644
@@ -4014,6 +4014,8 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
 	.apic_init_signal_blocked = svm_apic_init_signal_blocked,
 
 	.check_nested_events = svm_check_nested_events,
+
+	.page_enc_status_hc = svm_page_enc_status_hc,
 };
 
 static struct kvm_x86_init_ops svm_init_ops __initdata = {
arch/x86/kvm/svm/svm.h +4 -0
index df3474f4fb02..6a562f5928a2 100644
@@ -65,6 +65,8 @@ struct kvm_sev_info {
 	int fd;			/* SEV device fd */
 	unsigned long pages_locked; /* Number of pages locked */
 	struct list_head regions_list;  /* List of registered regions */
+	unsigned long *page_enc_bmap;
+	unsigned long page_enc_bmap_size;
 };
 
 struct kvm_svm {
@@ -400,6 +402,8 @@ int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
 			       bool has_error_code, u32 error_code);
 int svm_check_nested_events(struct kvm_vcpu *vcpu);
 int nested_svm_exit_special(struct vcpu_svm *svm);
+int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
+				  unsigned long npages, unsigned long enc);
 
 /* avic.c */
 
arch/x86/kvm/vmx/vmx.c +1 -0
index c2c6335a998c..7d01d3aa6461 100644
@@ -7838,6 +7838,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {
 	.nested_get_evmcs_version = NULL,
 	.need_emulation_on_page_fault = vmx_need_emulation_on_page_fault,
 	.apic_init_signal_blocked = vmx_apic_init_signal_blocked,
+	.page_enc_status_hc = NULL,
 };
 
 static __init int hardware_setup(void)
arch/x86/kvm/x86.c +6 -0
index c5835f9cb9ad..5f5ddb5765e2 100644
@@ -7605,6 +7605,12 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 		kvm_sched_yield(vcpu->kvm, a0);
 		ret = 0;
 		break;
+	case KVM_HC_PAGE_ENC_STATUS:
+		ret = -KVM_ENOSYS;
+		if (kvm_x86_ops.page_enc_status_hc)
+			ret = kvm_x86_ops.page_enc_status_hc(vcpu->kvm,
+					a0, a1, a2);
+		break;
 	default:
 		ret = -KVM_ENOSYS;
 		break;
include/uapi/linux/kvm_para.h +1 -0
index 8b86609849b9..847b83b75dc8 100644
@@ -29,6 +29,7 @@
 #define KVM_HC_CLOCK_PAIRING		9
 #define KVM_HC_SEND_IPI		10
 #define KVM_HC_SCHED_YIELD		11
+#define KVM_HC_PAGE_ENC_STATUS		12
 
 /*
  * hypercalls use architecture specific
Signature · 1 line
2.17.1
#17 lore raw

PATCH v7 9/18 KVM: x86: Introduce KVM_GET_PAGE_ENC_BITMAP ioctl

In reply to Ashish Kalra

From: Brijesh Singh <[email protected]>

The ioctl can be used to retrieve page encryption bitmap for a given gfn range.

Return the correct bitmap as per the number of pages being requested by the user. Ensure that we only copy bmap->num_pages bytes in the userspace buffer, if bmap->num_pages is not byte aligned we read the trailing bits from the userspace and copy those bits as is.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 Documentation/virt/kvm/api.rst  | 27 +++++++++++++
 arch/x86/include/asm/kvm_host.h |  2 +
 arch/x86/kvm/svm/sev.c          | 70 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/svm/svm.c          |  1 +
 arch/x86/kvm/svm/svm.h          |  1 +
 arch/x86/kvm/x86.c              | 12 ++++++
 include/uapi/linux/kvm.h        | 12 ++++++
 7 files changed, 125 insertions(+)
Documentation/virt/kvm/api.rst +27 -0
index efbbe570aa9b..e2f0dd105b5c 100644
@@ -4636,6 +4636,33 @@ This ioctl resets VCPU registers and control structures according to
 the clear cpu reset definition in the POP. However, the cpu is not put
 into ESA mode. This reset is a superset of the initial reset.
 
+4.125 KVM_GET_PAGE_ENC_BITMAP (vm ioctl)
+---------------------------------------
+
+:Capability: basic
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: struct kvm_page_enc_bitmap (in/out)
+:Returns: 0 on success, -1 on error
+
+/* for KVM_GET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+	__u64 start_gfn;
+	__u64 num_pages;
+	union {
+		void __user *enc_bitmap; /* one bit per page */
+		__u64 padding2;
+	};
+};
+
+The encrypted VMs have concept of private and shared pages. The private
+page is encrypted with the guest-specific key, while shared page may
+be encrypted with the hypervisor key. The KVM_GET_PAGE_ENC_BITMAP can
+be used to get the bitmap indicating whether the guest page is private
+or shared. The bitmap can be used during the guest migration, if the page
+is private then userspace need to use SEV migration commands to transmit
+the page.
+
 
 4.125 KVM_S390_PV_COMMAND
 -------------------------
arch/x86/include/asm/kvm_host.h +2 -0
index 4a8ee22f4f5b..9e428befb6a4 100644
@@ -1256,6 +1256,8 @@ struct kvm_x86_ops {
 	int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu);
 	int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa,
 				  unsigned long sz, unsigned long mode);
+	int (*get_page_enc_bitmap)(struct kvm *kvm,
+				struct kvm_page_enc_bitmap *bmap);
 };
 
 struct kvm_x86_init_ops {
arch/x86/kvm/svm/sev.c +70 -0

76 lines not on this page · raw message

arch/x86/kvm/svm/svm.c +1 -0
index 1013ef0f4ce2..588709a9f68e 100644
@@ -4016,6 +4016,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
 	.check_nested_events = svm_check_nested_events,
 
 	.page_enc_status_hc = svm_page_enc_status_hc,
+	.get_page_enc_bitmap = svm_get_page_enc_bitmap,
 };
 
 static struct kvm_x86_init_ops svm_init_ops __initdata = {
arch/x86/kvm/svm/svm.h +1 -0
index 6a562f5928a2..f087fa7b380c 100644
@@ -404,6 +404,7 @@ int svm_check_nested_events(struct kvm_vcpu *vcpu);
 int nested_svm_exit_special(struct vcpu_svm *svm);
 int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
 				  unsigned long npages, unsigned long enc);
+int svm_get_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
 
 /* avic.c */
 
arch/x86/kvm/x86.c +12 -0
index 5f5ddb5765e2..937797cfaf9a 100644
@@ -5208,6 +5208,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	case KVM_SET_PMU_EVENT_FILTER:
 		r = kvm_vm_ioctl_set_pmu_event_filter(kvm, argp);
 		break;
+	case KVM_GET_PAGE_ENC_BITMAP: {
+		struct kvm_page_enc_bitmap bitmap;
+
+		r = -EFAULT;
+		if (copy_from_user(&bitmap, argp, sizeof(bitmap)))
+			goto out;
+
+		r = -ENOTTY;
+		if (kvm_x86_ops.get_page_enc_bitmap)
+			r = kvm_x86_ops.get_page_enc_bitmap(kvm, &bitmap);
+		break;
+	}
 	default:
 		r = -ENOTTY;
 	}
include/uapi/linux/kvm.h +12 -0
index 0fe1d206d750..af62f2afaa5d 100644
@@ -505,6 +505,16 @@ struct kvm_dirty_log {
 	};
 };
 
+/* for KVM_GET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+	__u64 start_gfn;
+	__u64 num_pages;
+	union {
+		void __user *enc_bitmap; /* one bit per page */
+		__u64 padding2;
+	};
+};
+
 /* for KVM_CLEAR_DIRTY_LOG */
 struct kvm_clear_dirty_log {
 	__u32 slot;
@@ -1518,6 +1528,8 @@ struct kvm_pv_cmd {
 /* Available with KVM_CAP_S390_PROTECTED */
 #define KVM_S390_PV_COMMAND		_IOWR(KVMIO, 0xc5, struct kvm_pv_cmd)
 
+#define KVM_GET_PAGE_ENC_BITMAP	_IOW(KVMIO, 0xc6, struct kvm_page_enc_bitmap)
+
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
 	/* Guest initialization commands */
Signature · 1 line
2.17.1
#18 lore raw

Re: [PATCH v7 09/18] KVM: x86: Introduce KVM_GET_PAGE_ENC_BITMAP ioctl

Quoted 58 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

The ioctl can be used to retrieve page encryption bitmap for a given gfn range.

Return the correct bitmap as per the number of pages being requested by the user. Ensure that we only copy bmap->num_pages bytes in the userspace buffer, if bmap->num_pages is not byte aligned we read the trailing bits from the userspace and copy those bits as is.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 Documentation/virt/kvm/api.rst  | 27 +++++++++++++
 arch/x86/include/asm/kvm_host.h |  2 +
 arch/x86/kvm/svm/sev.c          | 70 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/svm/svm.c          |  1 +
 arch/x86/kvm/svm/svm.h          |  1 +
 arch/x86/kvm/x86.c              | 12 ++++++
 include/uapi/linux/kvm.h        | 12 ++++++
 7 files changed, 125 insertions(+)
Documentation/virt/kvm/api.rst +16 -0
index efbbe570aa9b..e2f0dd105b5c 100644
@@ -4636,6 +4636,33 @@ This ioctl resets VCPU registers and control structures according to
 the clear cpu reset definition in the POP. However, the cpu is not put
 into ESA mode. This reset is a superset of the initial reset.
 
+4.125 KVM_GET_PAGE_ENC_BITMAP (vm ioctl)
+---------------------------------------
+
+:Capability: basic
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: struct kvm_page_enc_bitmap (in/out)
+:Returns: 0 on success, -1 on error
+
+/* for KVM_GET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+	__u64 start_gfn;
+	__u64 num_pages;
+	union {
+		void __user *enc_bitmap; /* one bit per page */
+		__u64 padding2;

Documentation/virt/kvm/api.rst This ioctl resets VCPU registers and control structures according to · line 4664

Quoted 10 lines
Documentation/virt/kvm/api.rst +10 -0
+	};
+};
+
+The encrypted VMs have concept of private and shared pages. The private
+page is encrypted with the guest-specific key, while shared page may
+be encrypted with the hypervisor key. The KVM_GET_PAGE_ENC_BITMAP can
+be used to get the bitmap indicating whether the guest page is private
+or shared. The bitmap can be used during the guest migration, if the page
+is private then userspace need to use SEV migration commands to transmit
+the page.

Can you address the comments in the review of v6 patch?
(https://lore.kernel.org/kvm/20200403183046.GA727000@vbusired-dt/)

Quoted 178 lines
Documentation/virt/kvm/api.rst +1 -0
+
 
 4.125 KVM_S390_PV_COMMAND
 -------------------------
arch/x86/include/asm/kvm_host.h +2 -0
index 4a8ee22f4f5b..9e428befb6a4 100644
@@ -1256,6 +1256,8 @@ struct kvm_x86_ops {
 	int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu);
 	int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa,
 				  unsigned long sz, unsigned long mode);
+	int (*get_page_enc_bitmap)(struct kvm *kvm,
+				struct kvm_page_enc_bitmap *bmap);
 };
 
 struct kvm_x86_init_ops {
arch/x86/kvm/svm/sev.c +70 -0
index 7dc68db70405..73bbbffb3487 100644
@@ -1434,6 +1434,76 @@ int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
 	return 0;
 }
 
+int svm_get_page_enc_bitmap(struct kvm *kvm,
+				   struct kvm_page_enc_bitmap *bmap)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	unsigned long gfn_start, gfn_end;
+	unsigned long sz, i, sz_bytes;
+	unsigned long *bitmap;
+	int ret, n;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	gfn_start = bmap->start_gfn;
+	gfn_end = gfn_start + bmap->num_pages;
+
+	sz = ALIGN(bmap->num_pages, BITS_PER_LONG) / BITS_PER_BYTE;
+	bitmap = kmalloc(sz, GFP_KERNEL);
+	if (!bitmap)
+		return -ENOMEM;
+
+	/* by default all pages are marked encrypted */
+	memset(bitmap, 0xff, sz);
+
+	mutex_lock(&kvm->lock);
+	if (sev->page_enc_bmap) {
+		i = gfn_start;
+		for_each_clear_bit_from(i, sev->page_enc_bmap,
+				      min(sev->page_enc_bmap_size, gfn_end))
+			clear_bit(i - gfn_start, bitmap);
+	}
+	mutex_unlock(&kvm->lock);
+
+	ret = -EFAULT;
+
+	n = bmap->num_pages % BITS_PER_BYTE;
+	sz_bytes = ALIGN(bmap->num_pages, BITS_PER_BYTE) / BITS_PER_BYTE;
+
+	/*
+	 * Return the correct bitmap as per the number of pages being
+	 * requested by the user. Ensure that we only copy bmap->num_pages
+	 * bytes in the userspace buffer, if bmap->num_pages is not byte
+	 * aligned we read the trailing bits from the userspace and copy
+	 * those bits as is.
+	 */
+
+	if (n) {
+		unsigned char *bitmap_kernel = (unsigned char *)bitmap;
+		unsigned char bitmap_user;
+		unsigned long offset, mask;
+
+		offset = bmap->num_pages / BITS_PER_BYTE;
+		if (copy_from_user(&bitmap_user, bmap->enc_bitmap + offset,
+				sizeof(unsigned char)))
+			goto out;
+
+		mask = GENMASK(n - 1, 0);
+		bitmap_user &= ~mask;
+		bitmap_kernel[offset] &= mask;
+		bitmap_kernel[offset] |= bitmap_user;
+	}
+
+	if (copy_to_user(bmap->enc_bitmap, bitmap, sz_bytes))
+		goto out;
+
+	ret = 0;
+out:
+	kfree(bitmap);
+	return ret;
+}
+
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
arch/x86/kvm/svm/svm.c +1 -0
index 1013ef0f4ce2..588709a9f68e 100644
@@ -4016,6 +4016,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
 	.check_nested_events = svm_check_nested_events,
 
 	.page_enc_status_hc = svm_page_enc_status_hc,
+	.get_page_enc_bitmap = svm_get_page_enc_bitmap,
 };
 
 static struct kvm_x86_init_ops svm_init_ops __initdata = {
arch/x86/kvm/svm/svm.h +1 -0
index 6a562f5928a2..f087fa7b380c 100644
@@ -404,6 +404,7 @@ int svm_check_nested_events(struct kvm_vcpu *vcpu);
 int nested_svm_exit_special(struct vcpu_svm *svm);
 int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
 				  unsigned long npages, unsigned long enc);
+int svm_get_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
 
 /* avic.c */
 
arch/x86/kvm/x86.c +12 -0
index 5f5ddb5765e2..937797cfaf9a 100644
@@ -5208,6 +5208,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	case KVM_SET_PMU_EVENT_FILTER:
 		r = kvm_vm_ioctl_set_pmu_event_filter(kvm, argp);
 		break;
+	case KVM_GET_PAGE_ENC_BITMAP: {
+		struct kvm_page_enc_bitmap bitmap;
+
+		r = -EFAULT;
+		if (copy_from_user(&bitmap, argp, sizeof(bitmap)))
+			goto out;
+
+		r = -ENOTTY;
+		if (kvm_x86_ops.get_page_enc_bitmap)
+			r = kvm_x86_ops.get_page_enc_bitmap(kvm, &bitmap);
+		break;
+	}
 	default:
 		r = -ENOTTY;
 	}
include/uapi/linux/kvm.h +12 -0
index 0fe1d206d750..af62f2afaa5d 100644
@@ -505,6 +505,16 @@ struct kvm_dirty_log {
 	};
 };
 
+/* for KVM_GET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+	__u64 start_gfn;
+	__u64 num_pages;
+	union {
+		void __user *enc_bitmap; /* one bit per page */
+		__u64 padding2;
+	};
+};
+
 /* for KVM_CLEAR_DIRTY_LOG */
 struct kvm_clear_dirty_log {
 	__u32 slot;
@@ -1518,6 +1528,8 @@ struct kvm_pv_cmd {
 /* Available with KVM_CAP_S390_PROTECTED */
 #define KVM_S390_PV_COMMAND		_IOWR(KVMIO, 0xc5, struct kvm_pv_cmd)
 
+#define KVM_GET_PAGE_ENC_BITMAP	_IOW(KVMIO, 0xc6, struct kvm_page_enc_bitmap)
+
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
 	/* Guest initialization commands */
Signature · 1 line
2.17.1
#19 lore raw

PATCH v7 10/18 x86/paravirt: Add hypervisor specific hypercall for SEV live migration.

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

Add a new paravirt callback as part of x86_hyper_runtime (x86 hypervisor specific runtime callbacks) to do hypervisor specific page encryption status notifications from the SEV guest.

---
 arch/x86/include/asm/x86_init.h | 10 +++++++++-
 arch/x86/kernel/kvm.c           | 12 ++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)
arch/x86/include/asm/x86_init.h +9 -1
index 96d9cd208610..888dca30a17a 100644
@@ -238,10 +238,18 @@ struct x86_legacy_features {
 /**
  * struct x86_hyper_runtime - x86 hypervisor specific runtime callbacks
  *
- * @pin_vcpu:		pin current vcpu to specified physical cpu (run rarely)
+ * @pin_vcpu:			pin current vcpu to specified physical
+ * 				cpu (run rarely)
+ * @sev_migration_hcall:	this hypercall is used by the SEV guest
+ * 				to notify a change in the page encryption
+ * 				status to the hypervisor.
  */
 struct x86_hyper_runtime {
 	void (*pin_vcpu)(int cpu);
+#if defined(CONFIG_AMD_MEM_ENCRYPT)
+	long (*sev_migration_hcall)(unsigned long physaddr,
+				    unsigned long npages, bool enc);
+#endif
 };
 
 /**
arch/x86/kernel/kvm.c +12 -0
index 6efe0410fb72..928ddb8a8cfc 100644
@@ -729,6 +729,15 @@ static void __init kvm_init_platform(void)
 	x86_platform.apic_post_init = kvm_apic_init;
 }
 
+#if defined(CONFIG_AMD_MEM_ENCRYPT)
+long kvm_sev_migration_hcall(unsigned long physaddr, unsigned long npages,
+			     bool enc)
+{
+	return kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS, physaddr, npages,
+				  enc);
+}
+#endif
+
 const __initconst struct hypervisor_x86 x86_hyper_kvm = {
 	.name			= "KVM",
 	.detect			= kvm_detect,
@@ -736,6 +745,9 @@ const __initconst struct hypervisor_x86 x86_hyper_kvm = {
 	.init.guest_late_init	= kvm_guest_init,
 	.init.x2apic_available	= kvm_para_available,
 	.init.init_platform	= kvm_init_platform,
+#if defined(CONFIG_AMD_MEM_ENCRYPT)
+	.runtime.sev_migration_hcall = kvm_sev_migration_hcall,
+#endif
 };
 
 static __init int activate_jump_labels(void)
Signature · 1 line
2.17.1
#20 lore raw

Re: [PATCH v7 10/18] x86/paravirt: Add hypervisor specific hypercall for SEV live migration.

Thank you for the patch! Perhaps something to improve:

Hi Ashish,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v5.7-rc3]
[cannot apply to kvm/linux-next tip/x86/mm tip/x86/core next-20200501] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Ashish-Kalra/Add-AMD-SEV-guest-live-migration-support/20200430-202702
base:    6a8b55ed4056ea5559ebe4f6a4b247f627870d4c
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-191-gc51a0382-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag as appropriate

sparse warnings: (new ones prefixed by >>)

>> arch/x86/kernel/kvm.c:733:6: sparse: sparse: symbol 'kvm_sev_migration_hcall' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

#21 lore raw

[RFC PATCH] x86/paravirt: kvm_sev_migration_hcall() can be static

In reply to Ashish Kalra
Signed-off-by: kbuild test robot <[email protected]>
---
 kvm.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/kvm.c +2 -2
index 928ddb8a8cfc7..884ba9203d3b4 100644
@@ -730,8 +730,8 @@ static void __init kvm_init_platform(void)
 }
 
 #if defined(CONFIG_AMD_MEM_ENCRYPT)
-long kvm_sev_migration_hcall(unsigned long physaddr, unsigned long npages,
-			     bool enc)
+static long kvm_sev_migration_hcall(unsigned long physaddr, unsigned long npages,
+				    bool enc)
 {
 	return kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS, physaddr, npages,
 				  enc);
#22 lore raw

PATCH v7 11/18 mm: x86: Invoke hypercall when page encryption status is changed

In reply to Ashish Kalra

From: Brijesh Singh <[email protected]>

Invoke a hypercall when a memory region is changed from encrypted -> decrypted and vice versa. Hypervisor needs to know the page encryption status during the guest migration.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 arch/x86/include/asm/paravirt.h       | 10 +++++
 arch/x86/include/asm/paravirt_types.h |  2 +
 arch/x86/kernel/paravirt.c            |  1 +
 arch/x86/mm/mem_encrypt.c             | 58 ++++++++++++++++++++++++++-
 arch/x86/mm/pat/set_memory.c          |  7 ++++
 5 files changed, 77 insertions(+), 1 deletion(-)
arch/x86/include/asm/paravirt.h +10 -0
index 694d8daf4983..8127b9c141bf 100644
@@ -78,6 +78,12 @@ static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
 	PVOP_VCALL1(mmu.exit_mmap, mm);
 }
 
+static inline void page_encryption_changed(unsigned long vaddr, int npages,
+						bool enc)
+{
+	PVOP_VCALL3(mmu.page_encryption_changed, vaddr, npages, enc);
+}
+
 #ifdef CONFIG_PARAVIRT_XXL
 static inline void load_sp0(unsigned long sp0)
 {
@@ -946,6 +952,10 @@ static inline void paravirt_arch_dup_mmap(struct mm_struct *oldmm,
 static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
 {
 }
+
+static inline void page_encryption_changed(unsigned long vaddr, int npages, bool enc)
+{
+}
 #endif
 #endif /* __ASSEMBLY__ */
 #endif /* _ASM_X86_PARAVIRT_H */
arch/x86/include/asm/paravirt_types.h +2 -0
index 732f62e04ddb..03bfd515c59c 100644
@@ -215,6 +215,8 @@ struct pv_mmu_ops {
 
 	/* Hook for intercepting the destruction of an mm_struct. */
 	void (*exit_mmap)(struct mm_struct *mm);
+	void (*page_encryption_changed)(unsigned long vaddr, int npages,
+					bool enc);
 
 #ifdef CONFIG_PARAVIRT_XXL
 	struct paravirt_callee_save read_cr2;
arch/x86/kernel/paravirt.c +1 -0
index c131ba4e70ef..840c02b23aeb 100644
@@ -367,6 +367,7 @@ struct paravirt_patch_template pv_ops = {
 			(void (*)(struct mmu_gather *, void *))tlb_remove_page,
 
 	.mmu.exit_mmap		= paravirt_nop,
+	.mmu.page_encryption_changed	= paravirt_nop,
 
 #ifdef CONFIG_PARAVIRT_XXL
 	.mmu.read_cr2		= __PV_IS_CALLEE_SAVE(native_read_cr2),
arch/x86/mm/mem_encrypt.c +57 -1

99 lines not on this page · raw message

arch/x86/mm/pat/set_memory.c +7 -0
index 59eca6a94ce7..9aaf1b6f5a1b 100644
@@ -27,6 +27,7 @@
 #include <asm/proto.h>
 #include <asm/memtype.h>
 #include <asm/set_memory.h>
+#include <asm/paravirt.h>
 
 #include "../mm_internal.h"
 
@@ -2003,6 +2004,12 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
 	 */
 	cpa_flush(&cpa, 0);
 
+	/* Notify hypervisor that a given memory range is mapped encrypted
+	 * or decrypted. The hypervisor will use this information during the
+	 * VM migration.
+	 */
+	page_encryption_changed(addr, numpages, enc);
+
 	return ret;
 }
 
Signature · 1 line
2.17.1
maintainer
#23 lore raw

Re: [PATCH v7 11/18] mm: x86: Invoke hypercall when page encryption status is changed

Quoted 133 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

Invoke a hypercall when a memory region is changed from encrypted -> decrypted and vice versa. Hypervisor needs to know the page encryption status during the guest migration.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
  arch/x86/include/asm/paravirt.h       | 10 +++++
  arch/x86/include/asm/paravirt_types.h |  2 +
  arch/x86/kernel/paravirt.c            |  1 +
  arch/x86/mm/mem_encrypt.c             | 58 ++++++++++++++++++++++++++-
  arch/x86/mm/pat/set_memory.c          |  7 ++++
  5 files changed, 77 insertions(+), 1 deletion(-)
arch/x86/include/asm/paravirt.h +10 -0
index 694d8daf4983..8127b9c141bf 100644
@@ -78,6 +78,12 @@ static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
  	PVOP_VCALL1(mmu.exit_mmap, mm);
  }
 
+static inline void page_encryption_changed(unsigned long vaddr, int npages,
+						bool enc)
+{
+	PVOP_VCALL3(mmu.page_encryption_changed, vaddr, npages, enc);
+}
+
  #ifdef CONFIG_PARAVIRT_XXL
  static inline void load_sp0(unsigned long sp0)
  {
@@ -946,6 +952,10 @@ static inline void paravirt_arch_dup_mmap(struct mm_struct *oldmm,
  static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
  {
  }
+
+static inline void page_encryption_changed(unsigned long vaddr, int npages, bool enc)
+{
+}
  #endif
  #endif /* __ASSEMBLY__ */
  #endif /* _ASM_X86_PARAVIRT_H */
arch/x86/include/asm/paravirt_types.h +2 -0
index 732f62e04ddb..03bfd515c59c 100644
@@ -215,6 +215,8 @@ struct pv_mmu_ops {
 
  	/* Hook for intercepting the destruction of an mm_struct. */
  	void (*exit_mmap)(struct mm_struct *mm);
+	void (*page_encryption_changed)(unsigned long vaddr, int npages,
+					bool enc);
 
  #ifdef CONFIG_PARAVIRT_XXL
  	struct paravirt_callee_save read_cr2;
arch/x86/kernel/paravirt.c +1 -0
index c131ba4e70ef..840c02b23aeb 100644
@@ -367,6 +367,7 @@ struct paravirt_patch_template pv_ops = {
  			(void (*)(struct mmu_gather *, void *))tlb_remove_page,
 
  	.mmu.exit_mmap		= paravirt_nop,
+	.mmu.page_encryption_changed	= paravirt_nop,
 
  #ifdef CONFIG_PARAVIRT_XXL
  	.mmu.read_cr2		= __PV_IS_CALLEE_SAVE(native_read_cr2),
arch/x86/mm/mem_encrypt.c +30 -0
index f4bd4b431ba1..603f5abf8a78 100644
@@ -19,6 +19,7 @@
  #include <linux/kernel.h>
  #include <linux/bitops.h>
  #include <linux/dma-mapping.h>
+#include <linux/kvm_para.h>
 
  #include <asm/tlbflush.h>
  #include <asm/fixmap.h>
@@ -29,6 +30,7 @@
  #include <asm/processor-flags.h>
  #include <asm/msr.h>
  #include <asm/cmdline.h>
+#include <asm/kvm_para.h>
 
  #include "mm_internal.h"
 
@@ -196,6 +198,48 @@ void __init sme_early_init(void)
  		swiotlb_force = SWIOTLB_FORCE;
  }
 
+static void set_memory_enc_dec_hypercall(unsigned long vaddr, int npages,
+					bool enc)
+{
+	unsigned long sz = npages << PAGE_SHIFT;
+	unsigned long vaddr_end, vaddr_next;
+
+	vaddr_end = vaddr + sz;
+
+	for (; vaddr < vaddr_end; vaddr = vaddr_next) {
+		int psize, pmask, level;
+		unsigned long pfn;
+		pte_t *kpte;
+
+		kpte = lookup_address(vaddr, &level);
+		if (!kpte || pte_none(*kpte))
+			return;
+
+		switch (level) {
+		case PG_LEVEL_4K:
+			pfn = pte_pfn(*kpte);
+			break;
+		case PG_LEVEL_2M:
+			pfn = pmd_pfn(*(pmd_t *)kpte);
+			break;
+		case PG_LEVEL_1G:
+			pfn = pud_pfn(*(pud_t *)kpte);
+			break;
+		default:

arch/x86/mm/mem_encrypt.c void __init sme_early_init(void) · line 238

Quoted 10 lines
arch/x86/mm/mem_encrypt.c +10 -0
+			return;
+		}
+
+		psize = page_level_size(level);
+		pmask = page_level_mask(level);
+
+		if (x86_platform.hyper.sev_migration_hcall)
+			x86_platform.hyper.sev_migration_hcall(pfn << PAGE_SHIFT,
+							       psize >> PAGE_SHIFT,
+							       enc);

Why do you need two indirections? One via pv.mmu_ops and then another via x86_platform.hyper? Isn't one enough?

And if x86_platform.hyper.sev_migration_hcall isn't set the whole loop is basically a nop.

Quoted 60 lines
arch/x86/mm/mem_encrypt.c +17 -1
+		vaddr_next = (vaddr & pmask) + psize;
+	}
+}
+
  static void __init __set_clr_pte_enc(pte_t *kpte, int level, bool enc)
  {
  	pgprot_t old_prot, new_prot;
@@ -253,12 +297,13 @@ static void __init __set_clr_pte_enc(pte_t *kpte, int level, bool enc)
  static int __init early_set_memory_enc_dec(unsigned long vaddr,
  					   unsigned long size, bool enc)
  {
-	unsigned long vaddr_end, vaddr_next;
+	unsigned long vaddr_end, vaddr_next, start;
  	unsigned long psize, pmask;
  	int split_page_size_mask;
  	int level, ret;
  	pte_t *kpte;
 
+	start = vaddr;
  	vaddr_next = vaddr;
  	vaddr_end = vaddr + size;
 
@@ -313,6 +358,8 @@ static int __init early_set_memory_enc_dec(unsigned long vaddr,
 
  	ret = 0;
 
+	set_memory_enc_dec_hypercall(start, PAGE_ALIGN(size) >> PAGE_SHIFT,
+					enc);
  out:
  	__flush_tlb_all();
  	return ret;
@@ -451,6 +498,15 @@ void __init mem_encrypt_init(void)
  	if (sev_active())
  		static_branch_enable(&sev_enable_key);
 
+#ifdef CONFIG_PARAVIRT
+	/*
+	 * With SEV, we need to make a hypercall when page encryption state is
+	 * changed.
+	 */
+	if (sev_active())
+		pv_ops.mmu.page_encryption_changed = set_memory_enc_dec_hypercall;
+#endif
+
  	pr_info("AMD %s active\n",
  		sev_active() ? "Secure Encrypted Virtualization (SEV)"
  			     : "Secure Memory Encryption (SME)");
arch/x86/mm/pat/set_memory.c +1 -0
index 59eca6a94ce7..9aaf1b6f5a1b 100644
@@ -27,6 +27,7 @@
  #include <asm/proto.h>
  #include <asm/memtype.h>
  #include <asm/set_memory.h>
+#include <asm/paravirt.h>
 
  #include "../mm_internal.h"
 

arch/x86/mm/pat/set_memory.c static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc) · line 2011

Quoted 8 lines
arch/x86/mm/pat/set_memory.c +5 -0
@@ -2003,6 +2004,12 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
  	 */
  	cpa_flush(&cpa, 0);
 
+	/* Notify hypervisor that a given memory range is mapped encrypted
+	 * or decrypted. The hypervisor will use this information during the
+	 * VM migration.
+	 */
+	page_encryption_changed(addr, numpages, enc);

Is this operation really so performance critical that a pv-op is needed? Wouldn't a static key be sufficient here?

arch/x86/mm/pat/set_memory.c static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc) · line 2015

Quoted 5 lines
arch/x86/mm/pat/set_memory.c +1 -0
+
  	return ret;
  }
 

Juergen

#24 lore raw

Re: [PATCH v7 11/18] mm: x86: Invoke hypercall when page encryption status is changed

Quoted 137 lines · Jürgen Groß
Quoted 136 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

Invoke a hypercall when a memory region is changed from encrypted -> decrypted and vice versa. Hypervisor needs to know the page encryption status during the guest migration.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>

---
  arch/x86/include/asm/paravirt.h       | 10 +++++ arch/x86/include/asm/paravirt_types.h |  2 + arch/x86/kernel/paravirt.c            |  1 + arch/x86/mm/mem_encrypt.c             | 58 ++++++++++++++++++++++++++- arch/x86/mm/pat/set_memory.c          |  7 ++++

  5 files changed, 77 insertions(+), 1 deletion(-)
arch/x86/include/asm/paravirt.h +5 -0
index 694d8daf4983..8127b9c141bf 100644
@@ -78,6 +78,12 @@ static inline void paravirt_arch_exit_mmap(struct
 mm_struct *mm)
       PVOP_VCALL1(mmu.exit_mmap, mm);
   }
   +static inline void page_encryption_changed(unsigned long vaddr,
 int npages,
+                        bool enc)
+{
+    PVOP_VCALL3(mmu.page_encryption_changed, vaddr, npages, enc);
+}
+
   #ifdef CONFIG_PARAVIRT_XXL

  static inline void load_sp0(unsigned long sp0)
  {

arch/x86/include/asm/paravirt.h +4 -0
@@ -946,6 +952,10 @@ static inline void paravirt_arch_dup_mmap(struct
 mm_struct *oldmm,
   static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
   {
   }
+
+static inline void page_encryption_changed(unsigned long vaddr, int
 npages, bool enc)
+{
+}
   #endif

  #endif /* __ASSEMBLY__ */
  #endif /* _ASM_X86_PARAVIRT_H */

arch/x86/include/asm/paravirt_types.h +2 -0
index 732f62e04ddb..03bfd515c59c 100644
@@ -215,6 +215,8 @@ struct pv_mmu_ops {
         /* Hook for intercepting the destruction of an mm_struct. */
       void (*exit_mmap)(struct mm_struct *mm);
+    void (*page_encryption_changed)(unsigned long vaddr, int npages,
+                    bool enc);
     #ifdef CONFIG_PARAVIRT_XXL
       struct paravirt_callee_save read_cr2;
arch/x86/kernel/paravirt.c +1 -0
index c131ba4e70ef..840c02b23aeb 100644
@@ -367,6 +367,7 @@ struct paravirt_patch_template pv_ops = {
               (void (*)(struct mmu_gather *, void *))tlb_remove_page,
         .mmu.exit_mmap        = paravirt_nop,
+    .mmu.page_encryption_changed    = paravirt_nop,
     #ifdef CONFIG_PARAVIRT_XXL
       .mmu.read_cr2        = __PV_IS_CALLEE_SAVE(native_read_cr2),
arch/x86/mm/mem_encrypt.c +2 -0
index f4bd4b431ba1..603f5abf8a78 100644
@@ -19,6 +19,7 @@
   #include <linux/kernel.h>
   #include <linux/bitops.h>
   #include <linux/dma-mapping.h>
+#include <linux/kvm_para.h>
     #include <asm/tlbflush.h>
   #include <asm/fixmap.h>
@@ -29,6 +30,7 @@
   #include <asm/processor-flags.h>
   #include <asm/msr.h>
   #include <asm/cmdline.h>
+#include <asm/kvm_para.h>
     #include "mm_internal.h"
   @@ -196,6 +198,48 @@ void __init sme_early_init(void)
           swiotlb_force = SWIOTLB_FORCE;

  }
  +static void set_memory_enc_dec_hypercall(unsigned long vaddr, int npages,
+                    bool enc)

arch/x86/mm/mem_encrypt.c +31 -0
+{
+    unsigned long sz = npages << PAGE_SHIFT;
+    unsigned long vaddr_end, vaddr_next;
+
+    vaddr_end = vaddr + sz;
+
+    for (; vaddr < vaddr_end; vaddr = vaddr_next) {
+        int psize, pmask, level;
+        unsigned long pfn;
+        pte_t *kpte;
+
+        kpte = lookup_address(vaddr, &level);
+        if (!kpte || pte_none(*kpte))
+            return;
+
+        switch (level) {
+        case PG_LEVEL_4K:
+            pfn = pte_pfn(*kpte);
+            break;
+        case PG_LEVEL_2M:
+            pfn = pmd_pfn(*(pmd_t *)kpte);
+            break;
+        case PG_LEVEL_1G:
+            pfn = pud_pfn(*(pud_t *)kpte);
+            break;
+        default:
+            return;
+        }
+
+        psize = page_level_size(level);
+        pmask = page_level_mask(level);
Quoted 9 lines
Quoted 5 lines

+
+        if (x86_platform.hyper.sev_migration_hcall)
+            x86_platform.hyper.sev_migration_hcall(pfn << PAGE_SHIFT, +                                   psize >> PAGE_SHIFT,
+                                   enc);

Why do you need two indirections? One via pv.mmu_ops and then another via x86_platform.hyper? Isn't one enough?

Currently, there is no strong reason to have two indirections other than building a flexibility for the future expansion, e.g when we add SEV support for the Xen then hypercall invocation may be slightly different but the code to walk the page table to find the GPA will be same for both KVM and Xen. The pv.mmu_ops provides a generic indirection which can be used by set_memory_{decrypted,encrypted}. I will look into removing the extra indirection in next version. thanks.

Quoted 3 lines

And if x86_platform.hyper.sev_migration_hcall isn't set the whole loop is basically a nop.

Yes, this double indirection has a draw back that we will be executing the unnecessary code  when x86_platform.hyper.sev_migration_hcall isn't set. I will look into improving it.

Quoted 63 lines
Quoted 63 lines

+        vaddr_next = (vaddr & pmask) + psize;
+    }

arch/x86/mm/mem_encrypt.c +2 -0
+}
+

  static void __init __set_clr_pte_enc(pte_t *kpte, int level, bool enc) {
      pgprot_t old_prot, new_prot;

arch/x86/mm/mem_encrypt.c +1 -1
@@ -253,12 +297,13 @@ static void __init __set_clr_pte_enc(pte_t
 *kpte, int level, bool enc)
   static int __init early_set_memory_enc_dec(unsigned long vaddr,
                          unsigned long size, bool enc)
   {
-    unsigned long vaddr_end, vaddr_next;
+    unsigned long vaddr_end, vaddr_next, start;
       unsigned long psize, pmask;
       int split_page_size_mask;
       int level, ret;
       pte_t *kpte;
   +    start = vaddr;
       vaddr_next = vaddr;
       vaddr_end = vaddr + size;

  @@ -313,6 +358,8 @@ static int __init early_set_memory_enc_dec(unsigned long vaddr,
        ret = 0;
  +    set_memory_enc_dec_hypercall(start, PAGE_ALIGN(size) >> PAGE_SHIFT,
+                    enc);
  out:
      __flush_tlb_all();
      return ret;

arch/x86/mm/mem_encrypt.c +0 -0
@@ -451,6 +498,15 @@ void __init mem_encrypt_init(void)
       if (sev_active())
           static_branch_enable(&sev_enable_key);

  +#ifdef CONFIG_PARAVIRT
+    /*
+     * With SEV, we need to make a hypercall when page encryption state is
+     * changed.
+     */
+    if (sev_active())
+        pv_ops.mmu.page_encryption_changed = set_memory_enc_dec_hypercall;

arch/x86/mm/mem_encrypt.c +2 -0
+#endif
+

      pr_info("AMD %s active\n",
          sev_active() ? "Secure Encrypted Virtualization (SEV)" : "Secure Memory Encryption (SME)");

arch/x86/mm/pat/set_memory.c +1 -0
index 59eca6a94ce7..9aaf1b6f5a1b 100644
@@ -27,6 +27,7 @@
   #include <asm/proto.h>
   #include <asm/memtype.h>
   #include <asm/set_memory.h>
+#include <asm/paravirt.h>
     #include "../mm_internal.h"

  @@ -2003,6 +2004,12 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
       */
      cpa_flush(&cpa, 0);
  +    /* Notify hypervisor that a given memory range is mapped encrypted

Quoted 9 lines
Quoted 5 lines

+     * or decrypted. The hypervisor will use this information during the
+     * VM migration.
+     */
+    page_encryption_changed(addr, numpages, enc);

Is this operation really so performance critical that a pv-op is needed? Wouldn't a static key be sufficient here?

Well, in a typical Linux kernel boot it does not get called so many times. We noticed that some drivers (mainly nvme) calls it more often than others. I am open for the suggestions, we went with the pv-op path based on the previous feedbacks. A static key maybe sufficient as well.

maintainer
#25 lore raw

Re: [PATCH v7 11/18] mm: x86: Invoke hypercall when page encryption status is changed

Quoted 237 lines · Brijesh Singh
Quoted 137 lines · Jürgen Groß
Quoted 136 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

Invoke a hypercall when a memory region is changed from encrypted -> decrypted and vice versa. Hypervisor needs to know the page encryption status during the guest migration.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
   arch/x86/include/asm/paravirt.h       | 10 +++++
   arch/x86/include/asm/paravirt_types.h |  2 +
   arch/x86/kernel/paravirt.c            |  1 +
   arch/x86/mm/mem_encrypt.c             | 58 ++++++++++++++++++++++++++-
   arch/x86/mm/pat/set_memory.c          |  7 ++++
   5 files changed, 77 insertions(+), 1 deletion(-)
arch/x86/include/asm/paravirt.h +9 -0
index 694d8daf4983..8127b9c141bf 100644
@@ -78,6 +78,12 @@ static inline void paravirt_arch_exit_mmap(struct
 mm_struct *mm)
       PVOP_VCALL1(mmu.exit_mmap, mm);
   }
   +static inline void page_encryption_changed(unsigned long vaddr,
 int npages,
+                        bool enc)
+{
+    PVOP_VCALL3(mmu.page_encryption_changed, vaddr, npages, enc);
+}
+
   #ifdef CONFIG_PARAVIRT_XXL
   static inline void load_sp0(unsigned long sp0)
   {
@@ -946,6 +952,10 @@ static inline void paravirt_arch_dup_mmap(struct
 mm_struct *oldmm,
   static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
   {
   }
+
+static inline void page_encryption_changed(unsigned long vaddr, int
 npages, bool enc)
+{
+}
   #endif

  #endif /* __ASSEMBLY__ */
  #endif /* _ASM_X86_PARAVIRT_H */

arch/x86/include/asm/paravirt_types.h +2 -0
index 732f62e04ddb..03bfd515c59c 100644
@@ -215,6 +215,8 @@ struct pv_mmu_ops {
         /* Hook for intercepting the destruction of an mm_struct. */
       void (*exit_mmap)(struct mm_struct *mm);
+    void (*page_encryption_changed)(unsigned long vaddr, int npages,
+                    bool enc);
     #ifdef CONFIG_PARAVIRT_XXL
       struct paravirt_callee_save read_cr2;
arch/x86/kernel/paravirt.c +1 -0
index c131ba4e70ef..840c02b23aeb 100644
@@ -367,6 +367,7 @@ struct paravirt_patch_template pv_ops = {
               (void (*)(struct mmu_gather *, void *))tlb_remove_page,
         .mmu.exit_mmap        = paravirt_nop,
+    .mmu.page_encryption_changed    = paravirt_nop,
     #ifdef CONFIG_PARAVIRT_XXL
       .mmu.read_cr2        = __PV_IS_CALLEE_SAVE(native_read_cr2),
arch/x86/mm/mem_encrypt.c +2 -0
index f4bd4b431ba1..603f5abf8a78 100644
@@ -19,6 +19,7 @@
   #include <linux/kernel.h>
   #include <linux/bitops.h>
   #include <linux/dma-mapping.h>
+#include <linux/kvm_para.h>
     #include <asm/tlbflush.h>
   #include <asm/fixmap.h>
@@ -29,6 +30,7 @@
   #include <asm/processor-flags.h>
   #include <asm/msr.h>
   #include <asm/cmdline.h>
+#include <asm/kvm_para.h>
     #include "mm_internal.h"
   @@ -196,6 +198,48 @@ void __init sme_early_init(void)
           swiotlb_force = SWIOTLB_FORCE;

  }
  +static void set_memory_enc_dec_hypercall(unsigned long vaddr, int
npages,
+                    bool enc)

arch/x86/mm/mem_encrypt.c +31 -0
+{
+    unsigned long sz = npages << PAGE_SHIFT;
+    unsigned long vaddr_end, vaddr_next;
+
+    vaddr_end = vaddr + sz;
+
+    for (; vaddr < vaddr_end; vaddr = vaddr_next) {
+        int psize, pmask, level;
+        unsigned long pfn;
+        pte_t *kpte;
+
+        kpte = lookup_address(vaddr, &level);
+        if (!kpte || pte_none(*kpte))
+            return;
+
+        switch (level) {
+        case PG_LEVEL_4K:
+            pfn = pte_pfn(*kpte);
+            break;
+        case PG_LEVEL_2M:
+            pfn = pmd_pfn(*(pmd_t *)kpte);
+            break;
+        case PG_LEVEL_1G:
+            pfn = pud_pfn(*(pud_t *)kpte);
+            break;
+        default:
+            return;
+        }
+
+        psize = page_level_size(level);
+        pmask = page_level_mask(level);
Quoted 9 lines
Quoted 5 lines

+
+        if (x86_platform.hyper.sev_migration_hcall)
+            x86_platform.hyper.sev_migration_hcall(pfn << PAGE_SHIFT, +                                   psize >> PAGE_SHIFT,
+                                   enc);

Why do you need two indirections? One via pv.mmu_ops and then another via x86_platform.hyper? Isn't one enough?

Currently, there is no strong reason to have two indirections other than building a flexibility for the future expansion, e.g when we add SEV support for the Xen then hypercall invocation may be slightly different but the code to walk the page table to find the GPA will be same for both KVM and Xen. The pv.mmu_ops provides a generic indirection which can be used by set_memory_{decrypted,encrypted}. I will look into removing the extra indirection in next version. thanks.

Quoted 3 lines

And if x86_platform.hyper.sev_migration_hcall isn't set the whole loop is basically a nop.

Yes, this double indirection has a draw back that we will be executing the unnecessary code  when x86_platform.hyper.sev_migration_hcall isn't set. I will look into improving it.

Quoted 63 lines
Quoted 63 lines

+        vaddr_next = (vaddr & pmask) + psize;
+    }

arch/x86/mm/mem_encrypt.c +3 -1
+}
+
   static void __init __set_clr_pte_enc(pte_t *kpte, int level, bool enc)
   {
       pgprot_t old_prot, new_prot;
@@ -253,12 +297,13 @@ static void __init __set_clr_pte_enc(pte_t
 *kpte, int level, bool enc)
   static int __init early_set_memory_enc_dec(unsigned long vaddr,
                          unsigned long size, bool enc)
   {
-    unsigned long vaddr_end, vaddr_next;
+    unsigned long vaddr_end, vaddr_next, start;
       unsigned long psize, pmask;
       int split_page_size_mask;
       int level, ret;
       pte_t *kpte;
   +    start = vaddr;
       vaddr_next = vaddr;
       vaddr_end = vaddr + size;
   @@ -313,6 +358,8 @@ static int __init

early_set_memory_enc_dec(unsigned long vaddr,
        ret = 0;
  +    set_memory_enc_dec_hypercall(start, PAGE_ALIGN(size) >>
PAGE_SHIFT,
+                    enc);
  out:
      __flush_tlb_all();
      return ret;

arch/x86/mm/mem_encrypt.c +2 -0
@@ -451,6 +498,15 @@ void __init mem_encrypt_init(void)
       if (sev_active())
           static_branch_enable(&sev_enable_key);
   +#ifdef CONFIG_PARAVIRT
+    /*
+     * With SEV, we need to make a hypercall when page encryption

state is
+     * changed.
+     */
+    if (sev_active())
+        pv_ops.mmu.page_encryption_changed =
set_memory_enc_dec_hypercall;

arch/x86/mm/mem_encrypt.c +2 -0
+#endif
+
       pr_info("AMD %s active\n",
           sev_active() ? "Secure Encrypted Virtualization (SEV)"
                    : "Secure Memory Encryption (SME)");
arch/x86/mm/pat/set_memory.c +1 -0
index 59eca6a94ce7..9aaf1b6f5a1b 100644
@@ -27,6 +27,7 @@
   #include <asm/proto.h>
   #include <asm/memtype.h>
   #include <asm/set_memory.h>
+#include <asm/paravirt.h>
     #include "../mm_internal.h"
   @@ -2003,6 +2004,12 @@ static int __set_memory_enc_dec(unsigned
 long addr, int numpages, bool enc)

       */
      cpa_flush(&cpa, 0);
  +    /* Notify hypervisor that a given memory range is mapped
encrypted

Quoted 9 lines
Quoted 5 lines

+     * or decrypted. The hypervisor will use this information during the
+     * VM migration.
+     */
+    page_encryption_changed(addr, numpages, enc);

Is this operation really so performance critical that a pv-op is needed? Wouldn't a static key be sufficient here?

Quoted 5 lines

Well, in a typical Linux kernel boot it does not get called so many times. We noticed that some drivers (mainly nvme) calls it more often than others. I am open for the suggestions, we went with the pv-op path based on the previous feedbacks. A static key maybe sufficient as well.

I'm fine with a pv-op if this is seen to be the way to go. But I would skip the x86_platform.hyper indirection and call the kvm specific function directly. In case Xen gains SEV capability we can still either add the indirection again or just copy the KVM function and replace the hypercall with the Xen variant.

Juergen

#26 lore raw

PATCH v7 12/18 KVM: x86: Introduce KVM_SET_PAGE_ENC_BITMAP ioctl

In reply to Ashish Kalra

From: Brijesh Singh <[email protected]>

The ioctl can be used to set page encryption bitmap for an incoming guest.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc "Radim Krčmář" <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Tom Lendacky <[email protected]>
---
 Documentation/virt/kvm/api.rst  | 44 ++++++++++++++++++++++++++++++
 arch/x86/include/asm/kvm_host.h |  2 ++
 arch/x86/kvm/svm/sev.c          | 48 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/svm/svm.c          |  1 +
 arch/x86/kvm/svm/svm.h          |  1 +
 arch/x86/kvm/x86.c              | 12 +++++++++
 include/uapi/linux/kvm.h        |  1 +
 7 files changed, 109 insertions(+)
Documentation/virt/kvm/api.rst +44 -0
index e2f0dd105b5c..56c934db2a96 100644
@@ -4663,6 +4663,28 @@ or shared. The bitmap can be used during the guest migration, if the page
 is private then userspace need to use SEV migration commands to transmit
 the page.
 
+4.126 KVM_SET_PAGE_ENC_BITMAP (vm ioctl)
+---------------------------------------
+
+:Capability: basic
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: struct kvm_page_enc_bitmap (in/out)
+:Returns: 0 on success, -1 on error
+
+/* for KVM_SET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+	__u64 start_gfn;
+	__u64 num_pages;
+	union {
+		void __user *enc_bitmap; /* one bit per page */
+		__u64 padding2;
+	};
+};
+
+During the guest live migration the outgoing guest exports its page encryption
+bitmap, the KVM_SET_PAGE_ENC_BITMAP can be used to build the page encryption
+bitmap for an incoming guest.
 
 4.125 KVM_S390_PV_COMMAND
 -------------------------
@@ -4717,6 +4739,28 @@ KVM_PV_VM_VERIFY
   Verify the integrity of the unpacked image. Only if this succeeds,
   KVM is allowed to start protected VCPUs.
 
+4.126 KVM_SET_PAGE_ENC_BITMAP (vm ioctl)
+---------------------------------------
+
+:Capability: basic
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: struct kvm_page_enc_bitmap (in/out)
+:Returns: 0 on success, -1 on error
+
+/* for KVM_SET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+	__u64 start_gfn;
+	__u64 num_pages;
+	union {
+		void __user *enc_bitmap; /* one bit per page */
+		__u64 padding2;
+	};
+};
+
+During the guest live migration the outgoing guest exports its page encryption
+bitmap, the KVM_SET_PAGE_ENC_BITMAP can be used to build the page encryption
+bitmap for an incoming guest.
 
 5. The kvm_run structure
 ========================
arch/x86/include/asm/kvm_host.h +2 -0
index 9e428befb6a4..fc74144d5ab0 100644
@@ -1258,6 +1258,8 @@ struct kvm_x86_ops {
 				  unsigned long sz, unsigned long mode);
 	int (*get_page_enc_bitmap)(struct kvm *kvm,
 				struct kvm_page_enc_bitmap *bmap);
+	int (*set_page_enc_bitmap)(struct kvm *kvm,
+				struct kvm_page_enc_bitmap *bmap);
 };
 
 struct kvm_x86_init_ops {
arch/x86/kvm/svm/sev.c +48 -0

54 lines not on this page · raw message

arch/x86/kvm/svm/svm.c +1 -0
index 588709a9f68e..501e82f5593c 100644
@@ -4017,6 +4017,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
 
 	.page_enc_status_hc = svm_page_enc_status_hc,
 	.get_page_enc_bitmap = svm_get_page_enc_bitmap,
+	.set_page_enc_bitmap = svm_set_page_enc_bitmap,
 };
 
 static struct kvm_x86_init_ops svm_init_ops __initdata = {
arch/x86/kvm/svm/svm.h +1 -0
index f087fa7b380c..2ebdcce50312 100644
@@ -405,6 +405,7 @@ int nested_svm_exit_special(struct vcpu_svm *svm);
 int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
 				  unsigned long npages, unsigned long enc);
 int svm_get_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
+int svm_set_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
 
 /* avic.c */
 
arch/x86/kvm/x86.c +12 -0
index 937797cfaf9a..c4166d7a0493 100644
@@ -5220,6 +5220,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			r = kvm_x86_ops.get_page_enc_bitmap(kvm, &bitmap);
 		break;
 	}
+	case KVM_SET_PAGE_ENC_BITMAP: {
+		struct kvm_page_enc_bitmap bitmap;
+
+		r = -EFAULT;
+		if (copy_from_user(&bitmap, argp, sizeof(bitmap)))
+			goto out;
+
+		r = -ENOTTY;
+		if (kvm_x86_ops.set_page_enc_bitmap)
+			r = kvm_x86_ops.set_page_enc_bitmap(kvm, &bitmap);
+		break;
+	}
 	default:
 		r = -ENOTTY;
 	}
include/uapi/linux/kvm.h +1 -0
index af62f2afaa5d..2798b17484d0 100644
@@ -1529,6 +1529,7 @@ struct kvm_pv_cmd {
 #define KVM_S390_PV_COMMAND		_IOWR(KVMIO, 0xc5, struct kvm_pv_cmd)
 
 #define KVM_GET_PAGE_ENC_BITMAP	_IOW(KVMIO, 0xc6, struct kvm_page_enc_bitmap)
+#define KVM_SET_PAGE_ENC_BITMAP	_IOW(KVMIO, 0xc7, struct kvm_page_enc_bitmap)
 
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
Signature · 1 line
2.17.1
#27 lore raw

PATCH v7 13/18 KVM: SVM: Add support for static allocation of unified Page Encryption Bitmap.

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

Add support for static allocation of the unified Page encryption bitmap by extending kvm_arch_commit_memory_region() callack to add svm specific x86_ops which can read the userspace provided memory region/memslots and calculate the amount of guest RAM managed by the KVM and grow the bitmap based on that information, i.e. the highest guest PA that is mapped by a memslot.

---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/svm/sev.c          | 35 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/svm/svm.c          |  1 +
 arch/x86/kvm/svm/svm.h          |  1 +
 arch/x86/kvm/x86.c              |  5 +++++
 5 files changed, 43 insertions(+)
arch/x86/include/asm/kvm_host.h +1 -0
index fc74144d5ab0..b573ea85b57e 100644
@@ -1254,6 +1254,7 @@ struct kvm_x86_ops {
 
 	bool (*apic_init_signal_blocked)(struct kvm_vcpu *vcpu);
 	int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu);
+	void (*commit_memory_region)(struct kvm *kvm, enum kvm_mr_change change);
 	int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa,
 				  unsigned long sz, unsigned long mode);
 	int (*get_page_enc_bitmap)(struct kvm *kvm,
arch/x86/kvm/svm/sev.c +35 -0
index 64ff51ec4933..ba5ecd1de644 100644
@@ -1377,6 +1377,41 @@ static int sev_resize_page_enc_bitmap(struct kvm *kvm, unsigned long new_size)
 	return 0;
 }
 
+void svm_commit_memory_region(struct kvm *kvm, enum kvm_mr_change change)
+{
+	struct kvm_memslots *slots;
+	struct kvm_memory_slot *memslot;
+	gfn_t start, end = 0;
+
+	spin_lock(&kvm->mmu_lock);
+	if (change == KVM_MR_CREATE) {
+		slots = kvm_memslots(kvm);
+		kvm_for_each_memslot(memslot, slots) {
+			start = memslot->base_gfn;
+			end = memslot->base_gfn + memslot->npages;
+			/*
+			 * KVM memslots is a sorted list, starting with
+			 * the highest mapped guest PA, so pick the topmost
+			 * valid guest PA.
+			 */
+			if (memslot->npages)
+				break;
+		}
+	}
+	spin_unlock(&kvm->mmu_lock);
+
+	if (end) {
+		/*
+		 * NORE: This callback is invoked in vm ioctl
+		 * set_user_memory_region, hence we can use a
+		 * mutex here.
+		 */
+		mutex_lock(&kvm->lock);
+		sev_resize_page_enc_bitmap(kvm, end);
+		mutex_unlock(&kvm->lock);
+	}
+}
+
 int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
 				  unsigned long npages, unsigned long enc)
 {
arch/x86/kvm/svm/svm.c +1 -0
index 501e82f5593c..442adbbb0641 100644
@@ -4015,6 +4015,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
 
 	.check_nested_events = svm_check_nested_events,
 
+	.commit_memory_region = svm_commit_memory_region,
 	.page_enc_status_hc = svm_page_enc_status_hc,
 	.get_page_enc_bitmap = svm_get_page_enc_bitmap,
 	.set_page_enc_bitmap = svm_set_page_enc_bitmap,
arch/x86/kvm/svm/svm.h +1 -0
index 2ebdcce50312..fd99e0a5417a 100644
@@ -406,6 +406,7 @@ int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
 				  unsigned long npages, unsigned long enc);
 int svm_get_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
 int svm_set_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
+void svm_commit_memory_region(struct kvm *kvm, enum kvm_mr_change change);
 
 /* avic.c */
 
arch/x86/kvm/x86.c +5 -0
index c4166d7a0493..8938de868d42 100644
@@ -10133,6 +10133,11 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 		kvm_mmu_change_mmu_pages(kvm,
 				kvm_mmu_calculate_default_mmu_pages(kvm));
 
+	if (change == KVM_MR_CREATE || change == KVM_MR_DELETE) {
+		if (kvm_x86_ops.commit_memory_region)
+			kvm_x86_ops.commit_memory_region(kvm, change);
+	}
+
 	/*
 	 * Dirty logging tracks sptes in 4k granularity, meaning that large
 	 * sptes have to be split.  If live migration is successful, the guest
Signature · 1 line
2.17.1
#28 lore raw

PATCH v7 14/18 KVM: x86: Introduce new KVM_FEATURE_SEV_LIVE_MIGRATION feature & Custom MSR.

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

Add new KVM_FEATURE_SEV_LIVE_MIGRATION feature for guest to check for host-side support for SEV live migration. Also add a new custom MSR_KVM_SEV_LIVE_MIG_EN for guest to enable the SEV live migration feature.

---
 Documentation/virt/kvm/cpuid.rst     |  5 +++++
 Documentation/virt/kvm/msr.rst       | 10 ++++++++++
 arch/x86/include/uapi/asm/kvm_para.h |  5 +++++
 arch/x86/kvm/svm/sev.c               | 14 ++++++++++++++
 arch/x86/kvm/svm/svm.c               | 16 ++++++++++++++++
 arch/x86/kvm/svm/svm.h               |  2 ++
 6 files changed, 52 insertions(+)
Documentation/virt/kvm/cpuid.rst +5 -0
index 01b081f6e7ea..0514523e00cd 100644
@@ -86,6 +86,11 @@ KVM_FEATURE_PV_SCHED_YIELD        13          guest checks this feature bit
                                               before using paravirtualized
                                               sched yield.
 
+KVM_FEATURE_SEV_LIVE_MIGRATION    14          guest checks this feature bit before
+                                              using the page encryption state
+                                              hypercall to notify the page state
+                                              change
+
 KVM_FEATURE_CLOCSOURCE_STABLE_BIT 24          host will warn if no guest-side
                                               per-cpu warps are expeced in
                                               kvmclock
Documentation/virt/kvm/msr.rst +10 -0
index 33892036672d..7cd7786bbb03 100644
@@ -319,3 +319,13 @@ data:
 
 	KVM guests can request the host not to poll on HLT, for example if
 	they are performing polling themselves.
+
+MSR_KVM_SEV_LIVE_MIG_EN:
+        0x4b564d06
+
+	Control SEV Live Migration features.
+
+data:
+        Bit 0 enables (1) or disables (0) host-side SEV Live Migration feature.
+        Bit 1 enables (1) or disables (0) support for SEV Live Migration extensions.
+        All other bits are reserved.
arch/x86/include/uapi/asm/kvm_para.h +5 -0
index 2a8e0b6b9805..d9d4953b42ad 100644
@@ -31,6 +31,7 @@
 #define KVM_FEATURE_PV_SEND_IPI	11
 #define KVM_FEATURE_POLL_CONTROL	12
 #define KVM_FEATURE_PV_SCHED_YIELD	13
+#define KVM_FEATURE_SEV_LIVE_MIGRATION	14
 
 #define KVM_HINTS_REALTIME      0
 
@@ -50,6 +51,7 @@
 #define MSR_KVM_STEAL_TIME  0x4b564d03
 #define MSR_KVM_PV_EOI_EN      0x4b564d04
 #define MSR_KVM_POLL_CONTROL	0x4b564d05
+#define MSR_KVM_SEV_LIVE_MIG_EN	0x4b564d06
 
 struct kvm_steal_time {
 	__u64 steal;
@@ -122,4 +124,7 @@ struct kvm_vcpu_pv_apf_data {
 #define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
 #define KVM_PV_EOI_DISABLED 0x0
 
+#define KVM_SEV_LIVE_MIGRATION_ENABLED			(1 << 0)
+#define KVM_SEV_LIVE_MIGRATION_EXTENSIONS_SUPPORTED	(1 << 1)
+
 #endif /* _UAPI_ASM_X86_KVM_PARA_H */
arch/x86/kvm/svm/sev.c +14 -0
index ba5ecd1de644..0ac82e4aac33 100644
@@ -1469,6 +1469,17 @@ int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
 	return 0;
 }
 
+void sev_update_migration_flags(struct kvm *kvm, u64 data)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+
+	if (!sev_guest(kvm))
+		return;
+
+	if (data & KVM_SEV_LIVE_MIGRATION_ENABLED)
+		sev->live_migration_enabled = true;
+}
+
 int svm_get_page_enc_bitmap(struct kvm *kvm,
 				   struct kvm_page_enc_bitmap *bmap)
 {
@@ -1481,6 +1492,9 @@ int svm_get_page_enc_bitmap(struct kvm *kvm,
 	if (!sev_guest(kvm))
 		return -ENOTTY;
 
+	if (!sev->live_migration_enabled)
+		return -EINVAL;
+
 	gfn_start = bmap->start_gfn;
 	gfn_end = gfn_start + bmap->num_pages;
 
arch/x86/kvm/svm/svm.c +16 -0
index 442adbbb0641..a99f5457f244 100644
@@ -2633,6 +2633,9 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 		svm->msr_decfg = data;
 		break;
 	}
+	case MSR_KVM_SEV_LIVE_MIG_EN:
+		sev_update_migration_flags(vcpu->kvm, data);
+		break;
 	case MSR_IA32_APICBASE:
 		if (kvm_vcpu_apicv_active(vcpu))
 			avic_update_vapic_bar(to_svm(vcpu), data);
@@ -3493,6 +3496,19 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
 	svm->nrips_enabled = kvm_cpu_cap_has(X86_FEATURE_NRIPS) &&
 			     guest_cpuid_has(&svm->vcpu, X86_FEATURE_NRIPS);
 
+        /*
+         * If SEV guest then enable the Live migration feature.
+         */
+        if (sev_guest(vcpu->kvm)) {
+              struct kvm_cpuid_entry2 *best;
+
+              best = kvm_find_cpuid_entry(vcpu, KVM_CPUID_FEATURES, 0);
+              if (!best)
+                      return;
+
+              best->eax |= (1 << KVM_FEATURE_SEV_LIVE_MIGRATION);
+        }
+
 	if (!kvm_vcpu_apicv_active(vcpu))
 		return;
 
arch/x86/kvm/svm/svm.h +2 -0
index fd99e0a5417a..77f132a6fead 100644
@@ -65,6 +65,7 @@ struct kvm_sev_info {
 	int fd;			/* SEV device fd */
 	unsigned long pages_locked; /* Number of pages locked */
 	struct list_head regions_list;  /* List of registered regions */
+	bool live_migration_enabled;
 	unsigned long *page_enc_bmap;
 	unsigned long page_enc_bmap_size;
 };
@@ -494,5 +495,6 @@ int svm_unregister_enc_region(struct kvm *kvm,
 void pre_sev_run(struct vcpu_svm *svm, int cpu);
 int __init sev_hardware_setup(void);
 void sev_hardware_teardown(void);
+void sev_update_migration_flags(struct kvm *kvm, u64 data);
 
 #endif
Signature · 1 line
2.17.1
#29 lore raw

PATCH v7 15/18 EFI: Introduce the new AMD Memory Encryption GUID.

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

Introduce a new AMD Memory Encryption GUID which is currently used for defining a new UEFI enviroment variable which indicates UEFI/OVMF support for the SEV live migration feature. This variable is setup when UEFI/OVMF detects host/hypervisor support for SEV live migration and later this variable is read by the kernel using EFI runtime services to verify if OVMF supports the live migration feature.

---
 include/linux/efi.h | 1 +
 1 file changed, 1 insertion(+)
include/linux/efi.h +1 -0
index 251f1f783cdf..2efb42ccf3a8 100644
@@ -358,6 +358,7 @@ void efi_native_runtime_setup(void);
 
 /* OEM GUIDs */
 #define DELLEMC_EFI_RCI2_TABLE_GUID		EFI_GUID(0x2d9f28a2, 0xa886, 0x456a,  0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
+#define MEM_ENCRYPT_GUID			EFI_GUID(0x0cf29b71, 0x9e51, 0x433a,  0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75)
 
 typedef struct {
 	efi_guid_t guid;
Signature · 1 line
2.17.1
#30 lore raw

PATCH v7 16/18 KVM: x86: Add guest support for detecting and enabling SEV Live Migration feature.

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

The guest support for detecting and enabling SEV Live migration feature uses the following logic :

- kvm_init_plaform() checks if its booted under the EFI

- If not EFI,

i) check for the KVM_FEATURE_CPUID

ii) if CPUID reports that migration is support then issue wrmsrl
to enable the SEV migration support

- If EFI,

i) Check the KVM_FEATURE_CPUID.

ii) If CPUID reports that migration is supported, then reads the UEFI enviroment variable which
indicates OVMF support for live migration.

iii) If variable is set then wrmsr to enable the SEV migration support.

The EFI live migration check is done using a late_initcall() callback.

---
 arch/x86/include/asm/mem_encrypt.h | 11 ++++++
 arch/x86/kernel/kvm.c              | 62 ++++++++++++++++++++++++++++++
 arch/x86/mm/mem_encrypt.c          | 11 ++++++
 3 files changed, 84 insertions(+)
arch/x86/include/asm/mem_encrypt.h +11 -0
index 848ce43b9040..d10e92ae5ca1 100644
@@ -20,6 +20,7 @@
 
 extern u64 sme_me_mask;
 extern bool sev_enabled;
+extern bool sev_live_mig_enabled;
 
 void sme_encrypt_execute(unsigned long encrypted_kernel_vaddr,
 			 unsigned long decrypted_kernel_vaddr,
@@ -42,6 +43,8 @@ void __init sme_enable(struct boot_params *bp);
 
 int __init early_set_memory_decrypted(unsigned long vaddr, unsigned long size);
 int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size);
+void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages,
+					    bool enc);
 
 /* Architecture __weak replacement functions */
 void __init mem_encrypt_init(void);
@@ -55,6 +58,7 @@ bool sev_active(void);
 #else	/* !CONFIG_AMD_MEM_ENCRYPT */
 
 #define sme_me_mask	0ULL
+#define sev_live_mig_enabled	false
 
 static inline void __init sme_early_encrypt(resource_size_t paddr,
 					    unsigned long size) { }
@@ -76,6 +80,8 @@ static inline int __init
 early_set_memory_decrypted(unsigned long vaddr, unsigned long size) { return 0; }
 static inline int __init
 early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0; }
+static inline void __init
+early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages, bool enc) {}
 
 #define __bss_decrypted
 
@@ -102,6 +108,11 @@ static inline u64 sme_get_me_mask(void)
 	return sme_me_mask;
 }
 
+static inline bool sev_live_migration_enabled(void)
+{
+	return sev_live_mig_enabled;
+}
+
 #endif	/* __ASSEMBLY__ */
 
 #endif	/* __X86_MEM_ENCRYPT_H__ */
arch/x86/kernel/kvm.c +62 -0

80 lines not on this page · raw message

arch/x86/mm/mem_encrypt.c +11 -0
index 603f5abf8a78..3964f5de058c 100644
@@ -46,6 +46,8 @@ EXPORT_SYMBOL_GPL(sev_enable_key);
 
 bool sev_enabled __section(.data);
 
+bool sev_live_mig_enabled __section(.data);
+
 /* Buffer used for early in-place encryption by BSP, no locking needed */
 static char sme_early_buffer[PAGE_SIZE] __initdata __aligned(PAGE_SIZE);
 
@@ -204,6 +206,9 @@ static void set_memory_enc_dec_hypercall(unsigned long vaddr, int npages,
 	unsigned long sz = npages << PAGE_SHIFT;
 	unsigned long vaddr_end, vaddr_next;
 
+	if (!sev_live_migration_enabled())
+		return;
+
 	vaddr_end = vaddr + sz;
 
 	for (; vaddr < vaddr_end; vaddr = vaddr_next) {
@@ -375,6 +380,12 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
 	return early_set_memory_enc_dec(vaddr, size, true);
 }
 
+void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages,
+					bool enc)
+{
+	set_memory_enc_dec_hypercall(vaddr, npages, enc);
+}
+
 /*
  * SME and SEV are very similar but they are not the same, so there are
  * times that the kernel will need to distinguish between SME and SEV. The
Signature · 1 line
2.17.1
#31 lore raw

Re: [PATCH v7 16/18] KVM: x86: Add guest support for detecting and enabling SEV Live Migration feature.

Thank you for the patch! Yet something to improve:

Hi Ashish,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v5.7-rc3]
[cannot apply to kvm/linux-next tip/x86/mm tip/x86/core next-20200430] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Ashish-Kalra/Add-AMD-SEV-guest-live-migration-support/20200430-202702
base:    6a8b55ed4056ea5559ebe4f6a4b247f627870d4c
config: i386-randconfig-a003-20200430 (attached as .config)
compiler: gcc-5 (Ubuntu 5.5.0-12ubuntu1) 5.5.0 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag as appropriate

All errors (new ones prefixed by >>):

arch/x86/kernel/kvm.c: In function 'kvm_init_platform':

>> arch/x86/kernel/kvm.c:780:24: error: lvalue required as left operand of assignment
      sev_live_mig_enabled = true;
                           ^

vim +780 arch/x86/kernel/kvm.c

773
774 static void __init kvm_init_platform(void)
775 {
776 if (sev_active() &&
777 kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION)) {
778
779 printk(KERN_INFO "KVM enable live migration\n");

 > 780			sev_live_mig_enabled = true;

781 /*
782 * If not booted using EFI, enable Live migration support.
783 */
784 if (!efi_enabled(EFI_BOOT))
785 wrmsrl(MSR_KVM_SEV_LIVE_MIG_EN,
786 KVM_SEV_LIVE_MIGRATION_ENABLED);
787 } else
788 printk(KERN_INFO "KVM enable live migration feature unsupported\n");
789
790 kvmclock_init();
791 x86_platform.apic_post_init = kvm_apic_init;
792 }
793

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

#32 lore raw

PATCH v7 17/18 KVM: x86: Mark _bss_decrypted section variables as decrypted in page encryption bitmap.

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

Ensure that _bss_decrypted section variables such as hv_clock_boot and wall_clock are marked as decrypted in the page encryption bitmap if sev liv migration is supported.

---
 arch/x86/kernel/kvmclock.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
arch/x86/kernel/kvmclock.c +12 -0
index 34b18f6eeb2c..65777bf1218d 100644
@@ -334,6 +334,18 @@ void __init kvmclock_init(void)
 	pr_info("kvm-clock: Using msrs %x and %x",
 		msr_kvm_system_time, msr_kvm_wall_clock);
 
+	if (sev_live_migration_enabled()) {
+		unsigned long nr_pages;
+		/*
+		 * sizeof(hv_clock_boot) is already PAGE_SIZE aligned
+		 */
+		early_set_mem_enc_dec_hypercall((unsigned long)hv_clock_boot,
+						1, 0);
+		nr_pages = DIV_ROUND_UP(sizeof(wall_clock), PAGE_SIZE);
+		early_set_mem_enc_dec_hypercall((unsigned long)&wall_clock,
+						nr_pages, 0);
+	}
+
 	this_cpu_write(hv_clock_per_cpu, &hv_clock_boot[0]);
 	kvm_register_clock("primary cpu clock");
 	pvclock_set_pvti_cpu0_va(hv_clock_boot);
Signature · 1 line
2.17.1
#33 lore raw

PATCH v7 18/18 KVM: x86: Add kexec support for SEV Live Migration.

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

Reset the host's page encryption bitmap related to kernel specific page encryption status settings before we load a new kernel by kexec. We cannot reset the complete page encryption bitmap here as we need to retain the UEFI/OVMF firmware specific settings.

---
 arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
arch/x86/kernel/kvm.c +28 -0
index 8b8cc87a3461..68f2de4c1e74 100644
@@ -35,6 +35,7 @@
 #include <asm/hypervisor.h>
 #include <asm/tlb.h>
 #include <asm/cpuidle_haltpoll.h>
+#include <asm/e820/api.h>
 
 static int kvmapf = 1;
 
@@ -358,6 +359,33 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
 	 */
 	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
 		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
+	/*
+	 * Reset the host's page encryption bitmap related to kernel
+	 * specific page encryption status settings before we load a
+	 * new kernel by kexec. NOTE: We cannot reset the complete
+	 * page encryption bitmap here as we need to retain the
+	 * UEFI/OVMF firmware specific settings.
+	 */
+	if (sev_live_migration_enabled() & (smp_processor_id() == 0)) {
+		int i;
+		unsigned long nr_pages;
+
+		for (i = 0; i < e820_table->nr_entries; i++) {
+			struct e820_entry *entry = &e820_table->entries[i];
+			unsigned long start_pfn;
+			unsigned long end_pfn;
+
+			if (entry->type != E820_TYPE_RAM)
+				continue;
+
+			start_pfn = entry->addr >> PAGE_SHIFT;
+			end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
+			nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
+
+			kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
+					   entry->addr, nr_pages, 1);
+		}
+	}
 	kvm_pv_disable_apf();
 	kvm_disable_steal_time();
 }
Signature · 1 line
2.17.1