SEV Live Migration Patchset.

From: Brijesh Singh <[email protected]>

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

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]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Signed-off-by Brijesh Singh <[email protected]>

Signed-off-by Ashish Kalra <[email protected]>

---
 .../virt/kvm/amd-memory-encryption.rst        |  27 ++++
 arch/x86/kvm/svm.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 d18c97b4e140..826911f41f3b 100644
@@ -238,6 +238,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.c +32 -0

index a3e32d61d60c..3a7e2cac51de 100644
@@ -7140,6 +7140,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;

arch/x86/kvm/svm.c +41 -0

+
+       /* 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);

arch/x86/kvm/svm.c +29 -0

+
+       if (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;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
        struct kvm_sev_cmd sev_cmd;

@@ -7181,6 +7303,9 @@ static 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 -1

index 4b95f9a31a2f..17bef4c245e1 100644
@@ -1558,6 +1558,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)
--

2.17.1

Quoted 103 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

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

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]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Signed-off-by Brijesh Singh <[email protected]>

Signed-off-by Ashish Kalra <[email protected]>

---
 .../virt/kvm/amd-memory-encryption.rst        |  27 ++++
 arch/x86/kvm/svm.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 d18c97b4e140..826911f41f3b 100644
@@ -238,6 +238,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.c +32 -0

index a3e32d61d60c..3a7e2cac51de 100644
@@ -7140,6 +7140,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;

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

Quoted 10 lines

arch/x86/kvm/svm.c +10 -0

+       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 t query the session length */

/t/to/

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

Quoted 4 lines

arch/x86/kvm/svm.c +3 -0

+       if (!params.session_len)
+               return __sev_send_start_query_session_length(kvm, argp,
+                               &params);

Document this behavior with the command.

Quoted 41 lines

arch/x86/kvm/svm.c +41 -0

+
+       /* 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);

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

Quoted 10 lines

arch/x86/kvm/svm.c +10 -0

+       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;
+
+       ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);

sev_issue_cmd can fail. I think you want to handle those errors here (e.g. it can return -ebadf or a number of others). Right now they could get clobbered by a later copy_to_user error.

It's also worth documenting what the error argp->error is filled in with. I didn't see anything in the docs mentioning the status codes (may have missed it).

Quoted 6 lines

arch/x86/kvm/svm.c +6 -0

+
+       if (copy_to_user((void __user *)(uintptr_t) params.session_uaddr,
+                       session_data, params.session_len)) {
+               ret = -EFAULT;
+               goto e_free;
+       }

Quoted 77 lines

arch/x86/kvm/svm.c +23 -0

+
+       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;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
        struct kvm_sev_cmd sev_cmd;

@@ -7181,6 +7303,9 @@ static 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 -1

index 4b95f9a31a2f..17bef4c245e1 100644
@@ -1558,6 +1558,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)
--

Quoted 2 lines

2.17.1

Looks pretty reasonable overall.

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]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Signed-off-by Brijesh Singh <[email protected]>

Signed-off-by Ashish Kalra <[email protected]>

---
 .../virt/kvm/amd-memory-encryption.rst        |  24 ++++
 arch/x86/kvm/svm.c                            | 136 +++++++++++++++++-
 include/uapi/linux/kvm.h                      |   9 ++
 3 files changed, 165 insertions(+), 4 deletions(-)

Documentation/virt/kvm/amd-memory-encryption.rst +24 -0

index 826911f41f3b..0f1c3860360f 100644
@@ -265,6 +265,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.c +132 -4

index 3a7e2cac51de..ae97f774e979 100644
@@ -426,6 +426,7 @@ static DECLARE_RWSEM(sev_deactivate_lock);
 static DEFINE_MUTEX(sev_bitmap_lock);
 static 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)

@@ -1231,16 +1232,22 @@ static int avic_ga_log_notifier(u32 ga_tag)
 static __init int sev_hardware_setup(void)
 {
        struct sev_user_data_status *status;
+       int 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 (!max_sev_asid)
                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);

@@ -7262,6 +7269,124 @@ 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;
+
+       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;
+       }
+
+       /* 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;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
        struct kvm_sev_cmd sev_cmd;

@@ -7306,6 +7431,9 @@ static 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;

include/uapi/linux/kvm.h +0 -0

index 17bef4c245e1..d9dc81bb9c55 100644
@@ -1570,6 +1570,15 @@ struct kvm_sev_send_start {
        __u32 session_len;

include/uapi/linux/kvm.h +1 -1

+
 #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)
--

2.17.1

Quoted 8 lines

include/uapi/linux/kvm.h +0 -0

index 17bef4c245e1..d9dc81bb9c55 100644
@@ -1570,6 +1570,15 @@ struct kvm_sev_send_start {
        __u32 session_len;
 };
 

include/uapi/linux/kvm.h struct kvm_sev_send_start { · line 1580

Quoted 8 lines

include/uapi/linux/kvm.h +8 -0

+struct kvm_sev_send_update_data {
+       __u64 hdr_uaddr;
+       __u32 hdr_len;
+       __u64 guest_uaddr;
+       __u32 guest_len;
+       __u64 trans_uaddr;
+       __u32 trans_len;
+};

Input from others is welcome here, but I'd put the padding in intentionally (explicitly fill in the reserved u8s between *_len and *_uaddr). I had to double check that this pattern was intentional and matched the SEV spec.

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]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Signed-off-by Brijesh Singh <[email protected]>

Signed-off-by Ashish Kalra <[email protected]>

---
 .../virt/kvm/amd-memory-encryption.rst        |  8 +++++++
 arch/x86/kvm/svm.c                            | 23 +++++++++++++++++++
 2 files changed, 31 insertions(+)

Documentation/virt/kvm/amd-memory-encryption.rst +8 -0

index 0f1c3860360f..f22f09ad72bd 100644
@@ -289,6 +289,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.c +23 -1

index ae97f774e979..c55c1865f9e0 100644
@@ -7387,6 +7387,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;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
        struct kvm_sev_cmd sev_cmd;

@@ -7434,6 +7454,9 @@ static 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;
--

2.17.1

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]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Signed-off-by Brijesh Singh <[email protected]>

Signed-off-by Ashish Kalra <[email protected]>

---
 .../virt/kvm/amd-memory-encryption.rst        | 29 +++++++
 arch/x86/kvm/svm.c                            | 81 +++++++++++++++++++
 include/uapi/linux/kvm.h                      |  9 +++
 3 files changed, 119 insertions(+)

Documentation/virt/kvm/amd-memory-encryption.rst +29 -0

index f22f09ad72bd..4b882fb681fa 100644
@@ -297,6 +297,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.c +37 -0

index c55c1865f9e0..3b766f386c84 100644
@@ -7407,6 +7407,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)

arch/x86/kvm/svm.c +28 -0

+       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;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
        struct kvm_sev_cmd sev_cmd;

@@ -7457,6 +7535,9 @@ static 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 -1

index d9dc81bb9c55..74764b9db5fa 100644
@@ -1579,6 +1579,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)
--

2.17.1

Quoted 41 lines

arch/x86/kvm/svm.c +41 -0

+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;

arch/x86/kvm/svm.c static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp) · line 7456

Quoted 6 lines

arch/x86/kvm/svm.c +6 -0

+       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 */

Set ret to a different value here, since otherwise this will look like -ENOMEM.

arch/x86/kvm/svm.c static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp) · line 7462

Quoted 7 lines

arch/x86/kvm/svm.c +6 -0

+       ret = __sev_issue_cmd(argp->sev_fd, SEV_CMD_RECEIVE_START, start,
+                               error);
+       if (ret)
+               goto e_free;
+
+       /* Bind ASID to this guest */

Ideally, set ret to another distinct value, since the error spaces for these commands overlap, so you won't be sure which had the problem. You also wouldn't be sure if one succeeded and the other failed vs both failing.

arch/x86/kvm/svm.c static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp) · line 7466

Quoted 5 lines

arch/x86/kvm/svm.c +4 -0

+       ret = sev_bind_asid(kvm, start->handle, error);
+       if (ret)
+               goto e_free;
+

But, ret will be the value returned by __sev_issue_cmd(), so why will it look like -ENOMEM ?

Quoted 9 lines

Quoted 8 lines

arch/x86/kvm/svm.c +6 -0

+       ret = __sev_issue_cmd(argp->sev_fd, SEV_CMD_RECEIVE_START, start,
+                               error);
+       if (ret)
+               goto e_free;
+
+       /* Bind ASID to this guest */

Quoted 4 lines

Ideally, set ret to another distinct value, since the error spaces for these commands overlap, so you won't be sure which had the problem. You also wouldn't be sure if one succeeded and the other failed vs both failing.

Both commands "may" return the same error code as set by sev_do_cmd(), but then we need that very specific error code, sev_do_cmd() can't return different error codes for each command it is issuing ?

Quoted 6 lines

arch/x86/kvm/svm.c static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp) · line 7466

Quoted 6 lines

arch/x86/kvm/svm.c +4 -0

+       ret = sev_bind_asid(kvm, start->handle, error);
+       if (ret)
+               goto e_free;
+

Thanks,
Ashish

arch/x86/kvm/svm.c +12 -0

+static int sev_resize_page_enc_bitmap(struct kvm *kvm, unsigned long new_size)
+{
+       struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+       unsigned long *map;
+       unsigned long sz;
+
+       if (sev->page_enc_bmap_size >= new_size)
+               return 0;
+
+       sz = ALIGN(new_size, BITS_PER_LONG) / 8;
+
+       map = vmalloc(sz);

arch/x86/kvm/svm.c +21 -0

+       kvfree(sev->page_enc_bmap);
+
+       sev->page_enc_bmap = map;
+       sev->page_enc_bmap_size = new_size;
+
+       return 0;
+}
+
+static int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
+                                 unsigned long npages, unsigned long enc)
+{
+       struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+       gfn_t gfn_start, gfn_end;
+       int ret;
+
+       if (!sev_guest(kvm))
+               return -EINVAL;
+
+       if (!npages)
+               return 0;
+

arch/x86/kvm/svm.c +36 -0

+
+       if (is_error_noslot_pfn(pfn_start) && !is_noslot_pfn(pfn_start)) {
+               /*
+                * Allow guest MMIO range(s) to be added
+                * to the page encryption bitmap.
+                */
+               return -EINVAL;
+       }
+
+       if (is_error_noslot_pfn(pfn_end) && !is_noslot_pfn(pfn_end)) {
+               /*
+                * Allow guest MMIO range(s) to be added
+                * to the page encryption bitmap.
+                */
+               return -EINVAL;
+       }
+
+       mutex_lock(&kvm->lock);
+       ret = sev_resize_page_enc_bitmap(kvm, gfn_end);
+       if (ret)
+               goto unlock;
+
+       if (enc)
+               __bitmap_set(sev->page_enc_bmap, gfn_start,
+                               gfn_end - gfn_start);
+       else
+               __bitmap_clear(sev->page_enc_bmap, gfn_start,
+                               gfn_end - gfn_start);
+
+unlock:
+       mutex_unlock(&kvm->lock);
+       return ret;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
        struct kvm_sev_cmd sev_cmd;

@@ -7972,6 +8064,8 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = {
        .need_emulation_on_page_fault = svm_need_emulation_on_page_fault,
 
        .apic_init_signal_blocked = svm_apic_init_signal_blocked,
+
+       .page_enc_status_hc = svm_page_enc_status_hc,
 };
 
 static int __init svm_init(void)

arch/x86/kvm/vmx/vmx.c +1 -0

index 9a6664886f2e..7963f2979fdf 100644
@@ -7879,6 +7879,7 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {
        .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 void vmx_cleanup_l1d_flush(void)

arch/x86/kvm/x86.c +0 -0

index fbabb2f06273..298627fa3d39 100644
@@ -7547,6 +7547,12 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
                kvm_sched_yield(vcpu->kvm, a0);
                ret = 0;

Quoted 17 lines · Ashish Kalra

static void vmx_cleanup_l1d_flush(void)

arch/x86/kvm/x86.c +6 -0

index fbabb2f06273..298627fa3d39 100644
@@ -7547,6 +7547,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:

arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 7558

Quoted 2 lines

arch/x86/kvm/x86.c +0 -0

                ret = -KVM_ENOSYS;
                break;

Add a cap to kvm_vm_ioctl_enable_cap so that the vmm can configure whether or not this hypercall is offered. Moving to an enable cap would also allow the vmm to pass down the expected size of the c-bit tracking buffer, so that you don't need to handle dynamic resizing in response to guest hypercall, otherwise KVM will sporadically start copying around large buffers when working with large VMs.

Stepping back a bit, I'm a little surprised by the fact that you don't treat the c-bit buffers the same way as the dirty tracking buffers and put them alongside the memslots. That's probably more effort, and the strategy of using one large buffer should work fine (assuming you don't need to support non-zero as_ids).

Quoted 18 lines · Steve Rutherford

Quoted 17 lines · Ashish Kalra

static void vmx_cleanup_l1d_flush(void)

arch/x86/kvm/x86.c +6 -0

index fbabb2f06273..298627fa3d39 100644
@@ -7547,6 +7547,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:

Quoted 9 lines

arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 7558

Quoted 2 lines

arch/x86/kvm/x86.c +0 -0

                ret = -KVM_ENOSYS;
                break;

Add a cap to kvm_vm_ioctl_enable_cap so that the vmm can configure whether or not this hypercall is offered. Moving to an enable cap would also allow the vmm to pass down the expected size of the c-bit tracking buffer, so that you don't need to handle dynamic resizing in response to guest hypercall, otherwise KVM will sporadically start copying around large buffers when working with large VMs.

Yes, that is something we have been looking at adding.

But, how will VMM know the expected size of the c-bit tracking buffer ?

The guest kernel and firmware make the hypercall to mark page encryption status and depending on the GPA range being marked, the kernel's page encryption bitmap needs to be dynamically resized as response to the guest hypercall.

From: Brijesh Singh <[email protected]>

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

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]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Signed-off-by Brijesh Singh <[email protected]>

Signed-off-by Ashish Kalra <[email protected]>

---
 Documentation/virt/kvm/api.txt  | 27 +++++++++++++++++++++
 arch/x86/include/asm/kvm_host.h |  2 ++
 arch/x86/kvm/svm.c              | 43 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c              | 12 +++++++++
 include/uapi/linux/kvm.h        | 12 +++++++++
 5 files changed, 96 insertions(+)

Documentation/virt/kvm/api.txt +27 -0

index c6e1ce5d40de..053aecfabe74 100644
@@ -4213,6 +4213,33 @@ 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.120 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.
+
+
 5. The kvm_run structure
 ------------------------
 

arch/x86/include/asm/kvm_host.h +2 -0

index 4ae7293033b2..a6882c5214b4 100644
@@ -1258,6 +1258,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_arch_async_pf {

arch/x86/kvm/svm.c +43 -0

index f09791109075..f1c8806a97c6 100644
@@ -7673,6 +7673,48 @@ static int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
 	return ret;
 }
 
+static 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 *bitmap;
+	unsigned long sz, i;
+	int ret;
+
+	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) / 8;
+	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;
+	if (copy_to_user(bmap->enc_bitmap, bitmap, sz))
+		goto out;
+
+	ret = 0;
+out:
+	kfree(bitmap);
+	return ret;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;

@@ -8066,6 +8108,7 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = {
 	.apic_init_signal_blocked = svm_apic_init_signal_blocked,
 
 	.page_enc_status_hc = svm_page_enc_status_hc,
+	.get_page_enc_bitmap = svm_get_page_enc_bitmap,
 };
 
 static int __init svm_init(void)

arch/x86/kvm/x86.c +12 -0

index 298627fa3d39..e955f886ee17 100644
@@ -5213,6 +5213,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 4e80c57a3182..9377b26c5f4e 100644
@@ -500,6 +500,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;

@@ -1478,6 +1488,8 @@ struct kvm_enc_region {
 #define KVM_S390_NORMAL_RESET	_IO(KVMIO,   0xc3)
 #define KVM_S390_CLEAR_RESET	_IO(KVMIO,   0xc4)
 
+#define KVM_GET_PAGE_ENC_BITMAP	_IOW(KVMIO, 0xc2, struct kvm_page_enc_bitmap)
+
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
 	/* Guest initialization commands */

Signature · 1 line

2.17.1

Quoted 5 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

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

Quoted 13 lines

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]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Signed-off-by Brijesh Singh <[email protected]>

Signed-off-by Ashish Kalra <[email protected]>

This patch does not apply to upstream Linux version 5.5.6.

<snip>
Applying: KVM: x86: Introduce KVM_GET_PAGE_ENC_BITMAP ioctl
error: patch failed: Documentation/virt/kvm/api.txt:4213
error: Documentation/virt/kvm/api.txt: patch does not apply
error: patch failed: include/uapi/linux/kvm.h:1478
error: include/uapi/linux/kvm.h: patch does not apply Patch failed at 0009 KVM: x86: Introduce KVM_GET_PAGE_ENC_BITMAP ioctl <snip>

Which kernel version does this patch series apply to, cleanly?

Thanks,

Venu

Quoted 176 lines

---
 Documentation/virt/kvm/api.txt  | 27 +++++++++++++++++++++
 arch/x86/include/asm/kvm_host.h |  2 ++
 arch/x86/kvm/svm.c              | 43 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c              | 12 +++++++++
 include/uapi/linux/kvm.h        | 12 +++++++++
 5 files changed, 96 insertions(+)

Documentation/virt/kvm/api.txt +27 -0

index c6e1ce5d40de..053aecfabe74 100644
@@ -4213,6 +4213,33 @@ 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.120 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.
+
+
 5. The kvm_run structure
 ------------------------
 

arch/x86/include/asm/kvm_host.h +2 -0

index 4ae7293033b2..a6882c5214b4 100644
@@ -1258,6 +1258,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_arch_async_pf {

arch/x86/kvm/svm.c +43 -0

index f09791109075..f1c8806a97c6 100644
@@ -7673,6 +7673,48 @@ static int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
 	return ret;
 }
 
+static 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 *bitmap;
+	unsigned long sz, i;
+	int ret;
+
+	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) / 8;
+	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;
+	if (copy_to_user(bmap->enc_bitmap, bitmap, sz))
+		goto out;
+
+	ret = 0;
+out:
+	kfree(bitmap);
+	return ret;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;

@@ -8066,6 +8108,7 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = {
 	.apic_init_signal_blocked = svm_apic_init_signal_blocked,
 
 	.page_enc_status_hc = svm_page_enc_status_hc,
+	.get_page_enc_bitmap = svm_get_page_enc_bitmap,
 };
 
 static int __init svm_init(void)

arch/x86/kvm/x86.c +12 -0

index 298627fa3d39..e955f886ee17 100644
@@ -5213,6 +5213,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 4e80c57a3182..9377b26c5f4e 100644
@@ -500,6 +500,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;

@@ -1478,6 +1488,8 @@ struct kvm_enc_region {
 #define KVM_S390_NORMAL_RESET	_IO(KVMIO,   0xc3)
 #define KVM_S390_CLEAR_RESET	_IO(KVMIO,   0xc4)
 
+#define KVM_GET_PAGE_ENC_BITMAP	_IOW(KVMIO, 0xc2, struct kvm_page_enc_bitmap)
+
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
 	/* Guest initialization commands */

Signature · 1 line

2.17.1

Quoted 31 lines · Venu Busireddy

Quoted 5 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

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

Quoted 13 lines

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]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Signed-off-by Brijesh Singh <[email protected]>

Signed-off-by Ashish Kalra <[email protected]>

This patch does not apply to upstream Linux version 5.5.6.

<snip>
Applying: KVM: x86: Introduce KVM_GET_PAGE_ENC_BITMAP ioctl
error: patch failed: Documentation/virt/kvm/api.txt:4213
error: Documentation/virt/kvm/api.txt: patch does not apply
error: patch failed: include/uapi/linux/kvm.h:1478
error: include/uapi/linux/kvm.h: patch does not apply Patch failed at 0009 KVM: x86: Introduce KVM_GET_PAGE_ENC_BITMAP ioctl <snip>

Quoted 1 line

Which kernel version does this patch series apply to, cleanly?

Tried git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git.
With that, patch 08/12 fails with the top of that tree, as well as tag v5.6-rc3.

Quoted 180 lines

Thanks,

Venu

Quoted 176 lines

---
 Documentation/virt/kvm/api.txt  | 27 +++++++++++++++++++++
 arch/x86/include/asm/kvm_host.h |  2 ++
 arch/x86/kvm/svm.c              | 43 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c              | 12 +++++++++
 include/uapi/linux/kvm.h        | 12 +++++++++
 5 files changed, 96 insertions(+)

Documentation/virt/kvm/api.txt +27 -0

index c6e1ce5d40de..053aecfabe74 100644
@@ -4213,6 +4213,33 @@ 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.120 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.
+
+
 5. The kvm_run structure
 ------------------------
 

arch/x86/include/asm/kvm_host.h +2 -0

index 4ae7293033b2..a6882c5214b4 100644
@@ -1258,6 +1258,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_arch_async_pf {

arch/x86/kvm/svm.c +43 -0

index f09791109075..f1c8806a97c6 100644
@@ -7673,6 +7673,48 @@ static int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
 	return ret;
 }
 
+static 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 *bitmap;
+	unsigned long sz, i;
+	int ret;
+
+	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) / 8;
+	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;
+	if (copy_to_user(bmap->enc_bitmap, bitmap, sz))
+		goto out;
+
+	ret = 0;
+out:
+	kfree(bitmap);
+	return ret;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;

@@ -8066,6 +8108,7 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = {
 	.apic_init_signal_blocked = svm_apic_init_signal_blocked,
 
 	.page_enc_status_hc = svm_page_enc_status_hc,
+	.get_page_enc_bitmap = svm_get_page_enc_bitmap,
 };
 
 static int __init svm_init(void)

arch/x86/kvm/x86.c +12 -0

index 298627fa3d39..e955f886ee17 100644
@@ -5213,6 +5213,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 4e80c57a3182..9377b26c5f4e 100644
@@ -500,6 +500,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;

@@ -1478,6 +1488,8 @@ struct kvm_enc_region {
 #define KVM_S390_NORMAL_RESET	_IO(KVMIO,   0xc3)
 #define KVM_S390_CLEAR_RESET	_IO(KVMIO,   0xc4)
 
+#define KVM_GET_PAGE_ENC_BITMAP	_IOW(KVMIO, 0xc2, struct kvm_page_enc_bitmap)
+
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
 	/* Guest initialization commands */

Signature · 1 line

2.17.1

From: Brijesh Singh <[email protected]>

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

Quoted 11 lines · Ashish Kalra

Quoted 2 lines

From: Brijesh Singh <[email protected]>

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

What happens if the guest memory status doesn't match what the hypervisor thinks it is? What happens if the guest gets migrated between the hypercall and the associated flushes?

Quoted 11 lines · Andy Lutomirski

Quoted 11 lines · Ashish Kalra

Quoted 2 lines

From: Brijesh Singh <[email protected]>

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

What happens if the guest memory status doesn't match what the hypervisor thinks it is? What happens if the guest gets migrated between the hypercall and the associated flushes?

This is basically same as the dirty page tracking and logging being done during Live Migration. As with dirty page tracking and logging we maintain a page encryption bitmap in the kernel which keeps tracks of guest's page encrypted/decrypted state changes and this bitmap is sync'ed regularly from kernel to qemu and also during the live migration process, therefore any dirty pages whose encryption status will change during migration, should also have their page status updated when the page encryption bitmap is sync'ed.

Also i think that when the amount of dirty pages reach a low threshold, QEMU stops the source VM and then transfers all the remaining dirty pages, so at that point, there will also be a final sync of the page encryption bitmap, there won't be any hypercalls after this as the source VM has been stopped and the remaining VM state gets transferred.

Quoted 23 lines · Ashish Kalra

Quoted 11 lines · Andy Lutomirski

Quoted 11 lines · Ashish Kalra

Quoted 2 lines

From: Brijesh Singh <[email protected]>

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

What happens if the guest memory status doesn't match what the hypervisor thinks it is? What happens if the guest gets migrated between the hypercall and the associated flushes?

This is basically same as the dirty page tracking and logging being done during Live Migration. As with dirty page tracking and logging we maintain a page encryption bitmap in the kernel which keeps tracks of guest's page encrypted/decrypted state changes and this bitmap is sync'ed regularly from kernel to qemu and also during the live migration process, therefore any dirty pages whose encryption status will change during migration, should also have their page status updated when the page encryption bitmap is sync'ed.

Quoted 5 lines

Also i think that when the amount of dirty pages reach a low threshold, QEMU stops the source VM and then transfers all the remaining dirty pages, so at that point, there will also be a final sync of the page encryption bitmap, there won't be any hypercalls after this as the source VM has been stopped and the remaining VM state gets transferred.

And have you ensured that, in the inevitable race when a guest gets migrated part way through an encryption state change, that no data corruption occurs?

From: Brijesh Singh <[email protected]>

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

Quoted 6 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

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

One messy aspect, which I think is fine in practice, is that this presumes that pages are either treated as encrypted or decrypted. If also done on SEV, the in-place re-encryption supported by SME would break SEV migration. Linux doesn't do this now on SEV, and I don't have an intuition for why Linux might want this, but we will need to ensure it is never done in order to ensure that migration works down the line. I don't believe the AMD manual promises this will work anyway.

Something feels a bit wasteful about having all future kernels universally announce c-bit status when SEV is enabled, even if KVM isn't listening, since it may be too old (or just not want to know). Might be worth eliding the hypercalls if you get ENOSYS back? There might be a better way of passing paravirt config metadata across than just trying and seeing if the hypercall succeeds, but I'm not super familiar with it.

Quoted 22 lines · Steve Rutherford

Quoted 6 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

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

One messy aspect, which I think is fine in practice, is that this presumes that pages are either treated as encrypted or decrypted. If also done on SEV, the in-place re-encryption supported by SME would break SEV migration. Linux doesn't do this now on SEV, and I don't have an intuition for why Linux might want this, but we will need to ensure it is never done in order to ensure that migration works down the line. I don't believe the AMD manual promises this will work anyway.

Quoted 7 lines

Something feels a bit wasteful about having all future kernels universally announce c-bit status when SEV is enabled, even if KVM isn't listening, since it may be too old (or just not want to know). Might be worth eliding the hypercalls if you get ENOSYS back? There might be a better way of passing paravirt config metadata across than just trying and seeing if the hypercall succeeds, but I'm not super familiar with it.

I actually think this should be a hard requirement to merge this. The host needs to tell the guest that it supports this particular migration strategy and the guest needs to tell the host that it is using it. And the guest needs a way to tell the host that it’s *not* using it right now due to kexec, for example.

I’m still uneasy about a guest being migrated in the window where the hypercall tracking and the page encryption bit don’t match. I guess maybe corruption in this window doesn’t matter?

Quoted 21 lines · Steve Rutherford

Quoted 6 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

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

One messy aspect, which I think is fine in practice, is that this presumes that pages are either treated as encrypted or decrypted. If also done on SEV, the in-place re-encryption supported by SME would break SEV migration. Linux doesn't do this now on SEV, and I don't have an intuition for why Linux might want this, but we will need to ensure it is never done in order to ensure that migration works down the line. I don't believe the AMD manual promises this will work anyway.

Quoted 7 lines

Something feels a bit wasteful about having all future kernels universally announce c-bit status when SEV is enabled, even if KVM isn't listening, since it may be too old (or just not want to know). Might be worth eliding the hypercalls if you get ENOSYS back? There might be a better way of passing paravirt config metadata across than just trying and seeing if the hypercall succeeds, but I'm not super familiar with it.

I actually think this should be a hard requirement to merge this. The host needs to tell the guest that it supports this particular migration strategy and the guest needs to tell the host that it is using it. And the guest needs a way to tell the host that it’s *not* using it right now due to kexec, for example.

I’m still uneasy about a guest being migrated in the window where the hypercall tracking and the page encryption bit don’t match. I guess maybe corruption in this window doesn’t matter?

Quoted 29 lines · Andy Lutomirski

Quoted 21 lines · Steve Rutherford

Quoted 6 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

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

One messy aspect, which I think is fine in practice, is that this presumes that pages are either treated as encrypted or decrypted. If also done on SEV, the in-place re-encryption supported by SME would break SEV migration. Linux doesn't do this now on SEV, and I don't have an intuition for why Linux might want this, but we will need to ensure it is never done in order to ensure that migration works down the line. I don't believe the AMD manual promises this will work anyway.

Quoted 7 lines

Something feels a bit wasteful about having all future kernels universally announce c-bit status when SEV is enabled, even if KVM isn't listening, since it may be too old (or just not want to know). Might be worth eliding the hypercalls if you get ENOSYS back? There might be a better way of passing paravirt config metadata across than just trying and seeing if the hypercall succeeds, but I'm not super familiar with it.

Quoted 4 lines

I actually think this should be a hard requirement to merge this. The host needs to tell the guest that it supports this particular migration strategy and the guest needs to tell the host that it is using it. And the guest needs a way to tell the host that it’s *not* using it right now due to kexec, for example.

I’m still uneasy about a guest being migrated in the window where the hypercall tracking and the page encryption bit don’t match. I guess maybe corruption in this window doesn’t matter?

I don't think there is a corruption issue here. Let's consider the below case:

1) A page is transmitted as C=1 (encrypted)

2) During the migration window, the page encryption bit is changed to C=0 (decrypted)

3) #2 will cause a change in page table memory, thus dirty memory the tracker will create retransmission of the page table memory.

4) The page itself will not be re-transmitted because there was no change to the content of the page.

On destination, the read from the page will get the ciphertext.

The encryption bit change in the page table is used on the next access. The user of the page needs to ensure that data is written with the correct encryption bit before reading.

thanks

Quoted 52 lines · Brijesh Singh

Quoted 29 lines · Andy Lutomirski

Quoted 21 lines · Steve Rutherford

Quoted 6 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

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

One messy aspect, which I think is fine in practice, is that this presumes that pages are either treated as encrypted or decrypted. If also done on SEV, the in-place re-encryption supported by SME would break SEV migration. Linux doesn't do this now on SEV, and I don't have an intuition for why Linux might want this, but we will need to ensure it is never done in order to ensure that migration works down the line. I don't believe the AMD manual promises this will work anyway.

Quoted 7 lines

Something feels a bit wasteful about having all future kernels universally announce c-bit status when SEV is enabled, even if KVM isn't listening, since it may be too old (or just not want to know). Might be worth eliding the hypercalls if you get ENOSYS back? There might be a better way of passing paravirt config metadata across than just trying and seeing if the hypercall succeeds, but I'm not super familiar with it.

Quoted 4 lines

I actually think this should be a hard requirement to merge this. The host needs to tell the guest that it supports this particular migration strategy and the guest needs to tell the host that it is using it. And the guest needs a way to tell the host that it’s *not* using it right now due to kexec, for example.

I’m still uneasy about a guest being migrated in the window where the hypercall tracking and the page encryption bit don’t match. I guess maybe corruption in this window doesn’t matter?

I don't think there is a corruption issue here. Let's consider the below case:

1) A page is transmitted as C=1 (encrypted)

2) During the migration window, the page encryption bit is changed to C=0 (decrypted)

3) #2 will cause a change in page table memory, thus dirty memory the tracker will create retransmission of the page table memory.

4) The page itself will not be re-transmitted because there was no change to the content of the page.

Quoted 7 lines

On destination, the read from the page will get the ciphertext.

The encryption bit change in the page table is used on the next access. The user of the page needs to ensure that data is written with the correct encryption bit before reading.

thanks

I think the issue results from a slightly different perspective than the one you are using. I think the situation Andy is interested in is when a c-bit change and a write happen close in time. There are five events, and the ordering matters:
1) Guest dirties the c-bit in the guest
2) Guest dirties the page
3) Host userspace observes the c-bit logs
4) Host userspace observes the page dirty logs
5) Host transmits the page

If these are reordered to:
3) Host userspace observes the c-bit logs
1) Guest dirties the c-bit in the guest
2) Guest dirties the page
4) Host userspace observes the page dirty logs
5) Host transmits the page (from the wrong c-bit perspective!)

Then the host will transmit a page with the wrong c-bit status and clear the dirty bit for that page. If the guest page is not retransmitted incidentally later, then this page will be corrupted.

If you treat pages with dirty c-bits as dirty pages, then you will check the c-bit logs later and observe the dirty c-bit and retransmit. There might be some cleverness around enforcing that you always fetch the c-bit logs after fetching the dirty logs, but I haven't convinced myself that this works yet. I think it might, since then the c-bits are at least as fresh as the dirty bits.

The main uncertainty that comes to mind for that strategy is if, on multi-vCPU VMs, the page dirtying event (from the new c-bit perspective) and the c-bit status change hypercall can themselves race. If a write from the new c-bit perspective can arrive before the c-bit status change arrives in the c-bit logs, we will need to treat pages with dirty c-bits as dirty pages.

Note that I do agree that if the c-bit status flips, and no one writes to the page, it doesn't really matter if you retransmit that page. If a guest wants to read nonsense, it can.

Quoted 83 lines · Steve Rutherford

Quoted 52 lines · Brijesh Singh

Quoted 29 lines · Andy Lutomirski

Quoted 21 lines · Steve Rutherford

Quoted 6 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

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

One messy aspect, which I think is fine in practice, is that this presumes that pages are either treated as encrypted or decrypted. If also done on SEV, the in-place re-encryption supported by SME would break SEV migration. Linux doesn't do this now on SEV, and I don't have an intuition for why Linux might want this, but we will need to ensure it is never done in order to ensure that migration works down the line. I don't believe the AMD manual promises this will work anyway.

Quoted 7 lines

Something feels a bit wasteful about having all future kernels universally announce c-bit status when SEV is enabled, even if KVM isn't listening, since it may be too old (or just not want to know). Might be worth eliding the hypercalls if you get ENOSYS back? There might be a better way of passing paravirt config metadata across than just trying and seeing if the hypercall succeeds, but I'm not super familiar with it.

Quoted 4 lines

I actually think this should be a hard requirement to merge this. The host needs to tell the guest that it supports this particular migration strategy and the guest needs to tell the host that it is using it. And the guest needs a way to tell the host that it’s *not* using it right now due to kexec, for example.

I’m still uneasy about a guest being migrated in the window where the hypercall tracking and the page encryption bit don’t match. I guess maybe corruption in this window doesn’t matter?

I don't think there is a corruption issue here. Let's consider the below case:

1) A page is transmitted as C=1 (encrypted)

2) During the migration window, the page encryption bit is changed to C=0 (decrypted)

3) #2 will cause a change in page table memory, thus dirty memory the tracker will create retransmission of the page table memory.

4) The page itself will not be re-transmitted because there was no change to the content of the page.

Quoted 7 lines

On destination, the read from the page will get the ciphertext.

The encryption bit change in the page table is used on the next access. The user of the page needs to ensure that data is written with the correct encryption bit before reading.

thanks

I think the issue results from a slightly different perspective than the one you are using. I think the situation Andy is interested in is when a c-bit change and a write happen close in time. There are five events, and the ordering matters:
1) Guest dirties the c-bit in the guest
2) Guest dirties the page
3) Host userspace observes the c-bit logs
4) Host userspace observes the page dirty logs
5) Host transmits the page

If these are reordered to:
3) Host userspace observes the c-bit logs
1) Guest dirties the c-bit in the guest
2) Guest dirties the page
4) Host userspace observes the page dirty logs
5) Host transmits the page (from the wrong c-bit perspective!)

Then the host will transmit a page with the wrong c-bit status and clear the dirty bit for that page. If the guest page is not retransmitted incidentally later, then this page will be corrupted.

Quoted 7 lines

If you treat pages with dirty c-bits as dirty pages, then you will check the c-bit logs later and observe the dirty c-bit and retransmit. There might be some cleverness around enforcing that you always fetch the c-bit logs after fetching the dirty logs, but I haven't convinced myself that this works yet. I think it might, since then the c-bits are at least as fresh as the dirty bits.

Unlike the dirty log, the c-bit log maintains the complete state. So, I think it is the Host userspace responsibility to ensure that it either keeps track of any c-bit log changes since it last sync'ed. During the migration, after pausing the guest it can get the recent c-bit log and compare if something has changed since it last sync'ed. If so, then retransmit the page with new c-bit state.

Quoted 7 lines

The main uncertainty that comes to mind for that strategy is if, on multi-vCPU VMs, the page dirtying event (from the new c-bit perspective) and the c-bit status change hypercall can themselves race. If a write from the new c-bit perspective can arrive before the c-bit status change arrives in the c-bit logs, we will need to treat pages with dirty c-bits as dirty pages.

I believe if host userspace tracks the changes in the c-bit log since it last synced then this problem can be avoided. Do you think we should consider tracking the last sync changes in KVM or let the host userspace handle it.

Quoted 110 lines · Brijesh Singh

Quoted 87 lines · Steve Rutherford

Quoted 52 lines · Brijesh Singh

Quoted 29 lines · Andy Lutomirski

Quoted 21 lines · Steve Rutherford

Quoted 6 lines · Ashish Kalra

From: Brijesh Singh <[email protected]>

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

One messy aspect, which I think is fine in practice, is that this presumes that pages are either treated as encrypted or decrypted. If also done on SEV, the in-place re-encryption supported by SME would break SEV migration. Linux doesn't do this now on SEV, and I don't have an intuition for why Linux might want this, but we will need to ensure it is never done in order to ensure that migration works down the line. I don't believe the AMD manual promises this will work anyway.

Quoted 7 lines

Something feels a bit wasteful about having all future kernels universally announce c-bit status when SEV is enabled, even if KVM isn't listening, since it may be too old (or just not want to know). Might be worth eliding the hypercalls if you get ENOSYS back? There might be a better way of passing paravirt config metadata across than just trying and seeing if the hypercall succeeds, but I'm not super familiar with it.

Quoted 4 lines

I actually think this should be a hard requirement to merge this. The host needs to tell the guest that it supports this particular migration strategy and the guest needs to tell the host that it is using it. And the guest needs a way to tell the host that it’s *not* using it right now due to kexec, for example.

I’m still uneasy about a guest being migrated in the window where the hypercall tracking and the page encryption bit don’t match. I guess maybe corruption in this window doesn’t matter?

I don't think there is a corruption issue here. Let's consider the below case:

1) A page is transmitted as C=1 (encrypted)

2) During the migration window, the page encryption bit is changed to C=0 (decrypted)

3) #2 will cause a change in page table memory, thus dirty memory the tracker will create retransmission of the page table memory.

4) The page itself will not be re-transmitted because there was no change to the content of the page.

Quoted 7 lines

On destination, the read from the page will get the ciphertext.

The encryption bit change in the page table is used on the next access. The user of the page needs to ensure that data is written with the correct encryption bit before reading.

thanks

I think the issue results from a slightly different perspective than the one you are using. I think the situation Andy is interested in is when a c-bit change and a write happen close in time. There are five events, and the ordering matters:
1) Guest dirties the c-bit in the guest
2) Guest dirties the page
3) Host userspace observes the c-bit logs
4) Host userspace observes the page dirty logs
5) Host transmits the page

If these are reordered to:
3) Host userspace observes the c-bit logs
1) Guest dirties the c-bit in the guest
2) Guest dirties the page
4) Host userspace observes the page dirty logs
5) Host transmits the page (from the wrong c-bit perspective!)

Then the host will transmit a page with the wrong c-bit status and clear the dirty bit for that page. If the guest page is not retransmitted incidentally later, then this page will be corrupted.

Quoted 7 lines

If you treat pages with dirty c-bits as dirty pages, then you will check the c-bit logs later and observe the dirty c-bit and retransmit. There might be some cleverness around enforcing that you always fetch the c-bit logs after fetching the dirty logs, but I haven't convinced myself that this works yet. I think it might, since then the c-bits are at least as fresh as the dirty bits.

Unlike the dirty log, the c-bit log maintains the complete state. So, I think it is the Host userspace responsibility to ensure that it either keeps track of any c-bit log changes since it last sync'ed. During the migration, after pausing the guest it can get the recent c-bit log and compare if something has changed since it last sync'ed. If so, then retransmit the page with new c-bit state.

Quoted 7 lines

The main uncertainty that comes to mind for that strategy is if, on multi-vCPU VMs, the page dirtying event (from the new c-bit perspective) and the c-bit status change hypercall can themselves race. If a write from the new c-bit perspective can arrive before the c-bit status change arrives in the c-bit logs, we will need to treat pages with dirty c-bits as dirty pages.

Quoted 4 lines

I believe if host userspace tracks the changes in the c-bit log since it last synced then this problem can be avoided. Do you think we should consider tracking the last sync changes in KVM or let the host userspace handle it.

Punting this off to userspace to handle works. If storing the old c-bit statuses in userspace becomes a memory issue (unlikely), we can fix that down the line.

Andy, are your concerns about the raceyness of c-bit tracking resolved?

From: Ashish Kalra <[email protected]>

This patchset adds support for SEV Live Migration on KVM/QEMU.

Quoted 4 lines · Ashish Kalra

From: Ashish Kalra <[email protected]>

This patchset adds support for SEV Live Migration on KVM/QEMU.

I skimmed this all and I don't see any description of how this all works.

Does any of this address the mess in svm_register_enc_region()? Right now, when QEMU (or a QEMU alternative) wants to allocate some memory to be used for guest encrypted pages, it mmap()s some memory and the kernel does get_user_pages_fast() on it. The pages are kept pinned for the lifetime of the mapping. This is not at all okay. Let's see:

- The memory is pinned and it doesn't play well with the Linux memory
management code. You just wrote a big patch set to migrate the pages to a whole different machines, but we apparently can't even migrate them to a different NUMA node or even just a different address. And good luck swapping it out.

- The memory is still mapped in the QEMU process, and that mapping is
incoherent with actual guest access to the memory. It's nice that KVM clflushes it so that, in principle, everything might actually work, but this is gross. We should not be exposing incoherent mappings to userspace.

Perhaps all this fancy infrastructure you're writing for migration and all this new API surface could also teach the kernel how to migrate pages from a guest *to the same guest* so we don't need to pin pages forever. And perhaps you could put some thought into how to improve the API so that it doesn't involve nonsensical incoherent mappings.o

Quoted 26 lines · Andy Lutomirski

Quoted 4 lines · Ashish Kalra

From: Ashish Kalra <[email protected]>

This patchset adds support for SEV Live Migration on KVM/QEMU.

I skimmed this all and I don't see any description of how this all works.

Does any of this address the mess in svm_register_enc_region()? Right now, when QEMU (or a QEMU alternative) wants to allocate some memory to be used for guest encrypted pages, it mmap()s some memory and the kernel does get_user_pages_fast() on it. The pages are kept pinned for the lifetime of the mapping. This is not at all okay. Let's see:

- The memory is pinned and it doesn't play well with the Linux memory
management code. You just wrote a big patch set to migrate the pages to a whole different machines, but we apparently can't even migrate them to a different NUMA node or even just a different address. And good luck swapping it out.

- The memory is still mapped in the QEMU process, and that mapping is
incoherent with actual guest access to the memory. It's nice that KVM clflushes it so that, in principle, everything might actually work, but this is gross. We should not be exposing incoherent mappings to userspace.

Quoted 5 lines

Perhaps all this fancy infrastructure you're writing for migration and all this new API surface could also teach the kernel how to migrate pages from a guest *to the same guest* so we don't need to pin pages forever. And perhaps you could put some thought into how to improve the API so that it doesn't involve nonsensical incoherent mappings.o

As a different key is used to encrypt memory in each VM, the hypervisor can't simply copy the the ciphertext from one VM to another to migrate the VM. Therefore, the AMD SEV Key Management API provides a new sets of function which the hypervisor can use to package a guest page for migration, while maintaining the confidentiality provided by AMD SEV.

There is a new page encryption bitmap created in the kernel which keeps tracks of encrypted/decrypted state of guest's pages and this bitmap is updated by a new hypercall interface provided to the guest kernel and firmware.

KVM_GET_PAGE_ENC_BITMAP ioctl can be used to get the guest page encryption bitmap. The bitmap can be used to check if the given guest page is private or shared.

During the migration flow, the SEND_START is called on the source hypervisor to create an outgoing encryption context. The SEV guest policy dictates whether the certificate passed through the migrate-set-parameters command will be validated. SEND_UPDATE_DATA is called to encrypt the guest private pages. After migration is completed, SEND_FINISH is called to destroy the encryption context and make the VM non-runnable to protect it against cloning.

On the target machine, RECEIVE_START is called first to create an incoming encryption context. The RECEIVE_UPDATE_DATA is called to copy the received encrypted page into guest memory. After migration has completed, RECEIVE_FINISH is called to make the VM runnable.

Quoted 56 lines · Ashish Kalra

Quoted 26 lines · Andy Lutomirski

Quoted 4 lines · Ashish Kalra

From: Ashish Kalra <[email protected]>

This patchset adds support for SEV Live Migration on KVM/QEMU.

I skimmed this all and I don't see any description of how this all works.

Does any of this address the mess in svm_register_enc_region()? Right now, when QEMU (or a QEMU alternative) wants to allocate some memory to be used for guest encrypted pages, it mmap()s some memory and the kernel does get_user_pages_fast() on it. The pages are kept pinned for the lifetime of the mapping. This is not at all okay. Let's see:

- The memory is pinned and it doesn't play well with the Linux memory
management code. You just wrote a big patch set to migrate the pages to a whole different machines, but we apparently can't even migrate them to a different NUMA node or even just a different address. And good luck swapping it out.

- The memory is still mapped in the QEMU process, and that mapping is
incoherent with actual guest access to the memory. It's nice that KVM clflushes it so that, in principle, everything might actually work, but this is gross. We should not be exposing incoherent mappings to userspace.

Quoted 5 lines

Perhaps all this fancy infrastructure you're writing for migration and all this new API surface could also teach the kernel how to migrate pages from a guest *to the same guest* so we don't need to pin pages forever. And perhaps you could put some thought into how to improve the API so that it doesn't involve nonsensical incoherent mappings.o

As a different key is used to encrypt memory in each VM, the hypervisor can't simply copy the the ciphertext from one VM to another to migrate the VM. Therefore, the AMD SEV Key Management API provides a new sets of function which the hypervisor can use to package a guest page for migration, while maintaining the confidentiality provided by AMD SEV.

There is a new page encryption bitmap created in the kernel which keeps tracks of encrypted/decrypted state of guest's pages and this bitmap is updated by a new hypercall interface provided to the guest kernel and firmware.

KVM_GET_PAGE_ENC_BITMAP ioctl can be used to get the guest page encryption bitmap. The bitmap can be used to check if the given guest page is private or shared.

During the migration flow, the SEND_START is called on the source hypervisor to create an outgoing encryption context. The SEV guest policy dictates whether the certificate passed through the migrate-set-parameters command will be validated. SEND_UPDATE_DATA is called to encrypt the guest private pages. After migration is completed, SEND_FINISH is called to destroy the encryption context and make the VM non-runnable to protect it against cloning.

Quoted 5 lines

On the target machine, RECEIVE_START is called first to create an incoming encryption context. The RECEIVE_UPDATE_DATA is called to copy the received encrypted page into guest memory. After migration has completed, RECEIVE_FINISH is called to make the VM runnable.

Thanks! This belongs somewhere in the patch set.

You still haven't answered my questions about the existing coherency issues and whether the same infrastructure can be used to migrate guest pages within the same machine.

Also, you're making guest-side and host-side changes. What ensures that you don't try to migrate a guest that doesn't support the hypercall for encryption state tracking?

Quoted 63 lines · Andy Lutomirski

Quoted 56 lines · Ashish Kalra

Quoted 26 lines · Andy Lutomirski

Quoted 4 lines · Ashish Kalra

From: Ashish Kalra <[email protected]>

This patchset adds support for SEV Live Migration on KVM/QEMU.

I skimmed this all and I don't see any description of how this all works.

Does any of this address the mess in svm_register_enc_region()? Right now, when QEMU (or a QEMU alternative) wants to allocate some memory to be used for guest encrypted pages, it mmap()s some memory and the kernel does get_user_pages_fast() on it. The pages are kept pinned for the lifetime of the mapping. This is not at all okay. Let's see:

- The memory is pinned and it doesn't play well with the Linux memory
management code. You just wrote a big patch set to migrate the pages to a whole different machines, but we apparently can't even migrate them to a different NUMA node or even just a different address. And good luck swapping it out.

- The memory is still mapped in the QEMU process, and that mapping is
incoherent with actual guest access to the memory. It's nice that KVM clflushes it so that, in principle, everything might actually work, but this is gross. We should not be exposing incoherent mappings to userspace.

Quoted 5 lines

Perhaps all this fancy infrastructure you're writing for migration and all this new API surface could also teach the kernel how to migrate pages from a guest *to the same guest* so we don't need to pin pages forever. And perhaps you could put some thought into how to improve the API so that it doesn't involve nonsensical incoherent mappings.o

As a different key is used to encrypt memory in each VM, the hypervisor can't simply copy the the ciphertext from one VM to another to migrate the VM. Therefore, the AMD SEV Key Management API provides a new sets of function which the hypervisor can use to package a guest page for migration, while maintaining the confidentiality provided by AMD SEV.

There is a new page encryption bitmap created in the kernel which keeps tracks of encrypted/decrypted state of guest's pages and this bitmap is updated by a new hypercall interface provided to the guest kernel and firmware.

KVM_GET_PAGE_ENC_BITMAP ioctl can be used to get the guest page encryption bitmap. The bitmap can be used to check if the given guest page is private or shared.

During the migration flow, the SEND_START is called on the source hypervisor to create an outgoing encryption context. The SEV guest policy dictates whether the certificate passed through the migrate-set-parameters command will be validated. SEND_UPDATE_DATA is called to encrypt the guest private pages. After migration is completed, SEND_FINISH is called to destroy the encryption context and make the VM non-runnable to protect it against cloning.

Quoted 5 lines

On the target machine, RECEIVE_START is called first to create an incoming encryption context. The RECEIVE_UPDATE_DATA is called to copy the received encrypted page into guest memory. After migration has completed, RECEIVE_FINISH is called to make the VM runnable.

Quoted 5 lines

Thanks! This belongs somewhere in the patch set.

You still haven't answered my questions about the existing coherency issues and whether the same infrastructure can be used to migrate guest pages within the same machine.

Page Migration and Live Migration are separate features and one of my colleagues is currently working on making page migration possible and removing SEV Guest pinning requirements.

Quoted 4 lines

Also, you're making guest-side and host-side changes. What ensures that you don't try to migrate a guest that doesn't support the hypercall for encryption state tracking?

This is a good question and it is still an open-ended question. There are two possibilities here: guest does not have any unencrypted pages (for e.g booting 32-bit) and so it does not make any hypercalls, and the other possibility is that the guest does not have support for the newer hypercall.

In the first case, all the guest pages are then assumed to be encrypted and live migration happens as such.

For the second case, we have been discussing this internally,
and one option is to extend the KVM capabilites/feature bits to check for this ?

Thanks,
Ashish