From: Ashish Kalra <[email protected]>
The series add support for AMD SEV guest live migration commands. To protect the
confidentiality of an SEV protected guest memory while in transit we need to
use the SEV commands defined in SEV API spec [1].
SEV guest VMs have the concept of private and shared memory. Private memory
is encrypted with the guest-specific key, while shared memory may be encrypted
with hypervisor key. The commands provided by the SEV FW are meant to be used
for the private memory only. The patch series introduces a new hypercall.
The guest OS can use this hypercall to notify the page encryption status.
If the page is encrypted with guest specific-key then we use SEV command during
the migration. If page is not encrypted then fallback to default.
The patch adds new ioctls KVM_{SET,GET}_PAGE_ENC_BITMAP. The ioctl can be used
by the qemu to get the page encrypted bitmap. Qemu can consult this bitmap
during the migration to know whether the page is encrypted.
[1] https://developer.amd.com/wp-content/resources/55766.PDF
Changes since v6:
- Rebasing to mainline and refactoring to the new split SVM
infrastructre.
- Move to static allocation of the unified Page Encryption bitmap
instead of the dynamic resizing of the bitmap, the static allocation
is done implicitly by extending kvm_arch_commit_memory_region() callack
to add svm specific x86_ops which can read the userspace provided memory
region/memslots and calculate the amount of guest RAM managed by the KVM
and grow the bitmap.
- Fixed KVM_SET_PAGE_ENC_BITMAP ioctl to set the whole bitmap instead
of simply clearing specific bits.
- Removed KVM_PAGE_ENC_BITMAP_RESET ioctl, which is now performed using
KVM_SET_PAGE_ENC_BITMAP.
- Extended guest support for enabling Live Migration feature by adding a
check for UEFI environment variable indicating OVMF support for Live
Migration feature and additionally checking for KVM capability for the
same feature. If not booted under EFI, then we simply check for KVM
capability.
- Add hypervisor specific hypercall for SEV live migration by adding
a new paravirt callback as part of x86_hyper_runtime.
(x86 hypervisor specific runtime callbacks)
- Moving MSR handling for MSR_KVM_SEV_LIVE_MIG_EN into svm/sev code
and adding check for SEV live migration enabled by guest in the
KVM_GET_PAGE_ENC_BITMAP ioctl.
- Instead of the complete __bss_decrypted section, only specific variables
such as hv_clock_boot and wall_clock are marked as decrypted in the
page encryption bitmap
Changes since v5:
- Fix build errors as
Reported-by: kbuild test robot <[email protected]>
Changes since v4:
- Host support has been added to extend KVM capabilities/feature bits to
include a new KVM_FEATURE_SEV_LIVE_MIGRATION, which the guest can
query for host-side support for SEV live migration and a new custom MSR
MSR_KVM_SEV_LIVE_MIG_EN is added for guest to enable the SEV live
migration feature.
- Ensure that _bss_decrypted section is marked as decrypted in the
page encryption bitmap.
- Fixing KVM_GET_PAGE_ENC_BITMAP ioctl to return the correct bitmap
as per the number of pages being requested by the user. Ensure that
we only copy bmap->num_pages bytes in the userspace buffer, if
bmap->num_pages is not byte aligned we read the trailing bits
from the userspace and copy those bits as is. This fixes guest
page(s) corruption issues observed after migration completion.
- Add kexec support for SEV Live Migration to reset the host's
page encryption bitmap related to kernel specific page encryption
status settings before we load a new kernel by kexec. We cannot
reset the complete page encryption bitmap here as we need to
retain the UEFI/OVMF firmware specific settings.
Changes since v3:
- Rebasing to mainline and testing.
- Adding a new KVM_PAGE_ENC_BITMAP_RESET ioctl, which resets the
page encryption bitmap on a guest reboot event.
- Adding a more reliable sanity check for GPA range being passed to
the hypercall to ensure that guest MMIO ranges are also marked
in the page encryption bitmap.
Changes since v2:
- reset the page encryption bitmap on vcpu reboot
Changes since v1:
- Add support to share the page encryption between the source and target
machine.
- Fix review feedbacks from Tom Lendacky.
- Add check to limit the session blob length.
- Update KVM_GET_PAGE_ENC_BITMAP icotl to use the base_gfn instead of
the memory slot when querying the bitmap.
Ashish Kalra (7):
x86/paravirt: Add hypervisor specific hypercall for SEV live
migration.
KVM: SVM: Add support for static allocation of unified Page Encryption
Bitmap.
KVM: x86: Introduce new KVM_FEATURE_SEV_LIVE_MIGRATION feature &
Custom MSR.
EFI: Introduce the new AMD Memory Encryption GUID.
KVM: x86: Add guest support for detecting and enabling SEV Live
Migration feature.
KVM: x86: Mark _bss_decrypted section variables as decrypted in page
encryption bitmap.
KVM: x86: Add kexec support for SEV Live Migration.
Brijesh Singh (11):
KVM: SVM: Add KVM_SEV SEND_START command
KVM: SVM: Add KVM_SEND_UPDATE_DATA command
KVM: SVM: Add KVM_SEV_SEND_FINISH command
KVM: SVM: Add support for KVM_SEV_RECEIVE_START command
KVM: SVM: Add KVM_SEV_RECEIVE_UPDATE_DATA command
KVM: SVM: Add KVM_SEV_RECEIVE_FINISH command
KVM: x86: Add AMD SEV specific Hypercall3
KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
KVM: x86: Introduce KVM_GET_PAGE_ENC_BITMAP ioctl
mm: x86: Invoke hypercall when page encryption status is changed
KVM: x86: Introduce KVM_SET_PAGE_ENC_BITMAP ioctl
.../virt/kvm/amd-memory-encryption.rst | 120 +++
Documentation/virt/kvm/api.rst | 71 ++
Documentation/virt/kvm/cpuid.rst | 5 +
Documentation/virt/kvm/hypercalls.rst | 15 +
Documentation/virt/kvm/msr.rst | 10 +
arch/x86/include/asm/kvm_host.h | 7 +
arch/x86/include/asm/kvm_para.h | 12 +
arch/x86/include/asm/mem_encrypt.h | 11 +
arch/x86/include/asm/paravirt.h | 10 +
arch/x86/include/asm/paravirt_types.h | 2 +
arch/x86/include/asm/x86_init.h | 10 +-
arch/x86/include/uapi/asm/kvm_para.h | 5 +
arch/x86/kernel/kvm.c | 102 +++
arch/x86/kernel/kvmclock.c | 12 +
arch/x86/kernel/paravirt.c | 1 +
arch/x86/kvm/svm/sev.c | 723 +++++++++++++++++-
arch/x86/kvm/svm/svm.c | 21 +
arch/x86/kvm/svm/svm.h | 9 +
arch/x86/kvm/vmx/vmx.c | 1 +
arch/x86/kvm/x86.c | 35 +
arch/x86/mm/mem_encrypt.c | 69 +-
arch/x86/mm/pat/set_memory.c | 7 +
include/linux/efi.h | 1 +
include/linux/psp-sev.h | 8 +-
include/uapi/linux/kvm.h | 52 ++
include/uapi/linux/kvm_para.h | 1 +
26 files changed, 1310 insertions(+), 10 deletions(-)
--
2.17.1
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]
Reviewed-by: Steve Rutherford <[email protected]>
Reviewed-by: Venu Busireddy <[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/sev.c | 125 ++++++++++++++++++
include/linux/psp-sev.h | 8 +-
include/uapi/linux/kvm.h | 12 ++
4 files changed, 168 insertions(+), 4 deletions(-)
diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
index c3129b9ba5cb..4fd34fc5c7a7 100644
--- a/Documentation/virt/kvm/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/amd-memory-encryption.rst
@@ -263,6 +263,33 @@ Returns: 0 on success, -negative on error
__u32 trans_len;
};
+10. KVM_SEV_SEND_START
+----------------------
+
+The KVM_SEV_SEND_START command can be used by the hypervisor to create an
+outgoing guest encryption context.
+
+Parameters (in): struct kvm_sev_send_start
+
+Returns: 0 on success, -negative on error
+
+::
+ struct kvm_sev_send_start {
+ __u32 policy; /* guest policy */
+
+ __u64 pdh_cert_uaddr; /* platform Diffie-Hellman certificate */
+ __u32 pdh_cert_len;
+
+ __u64 plat_certs_uadr; /* platform certificate chain */
+ __u32 plat_certs_len;
+
+ __u64 amd_certs_uaddr; /* AMD certificate */
+ __u32 amd_cert_len;
+
+ __u64 session_uaddr; /* Guest session information */
+ __u32 session_len;
+ };
+
References
==========
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index cf912b4aaba8..5a15b43b4349 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -913,6 +913,128 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
+/* Userspace wants to query session length. */
+static int
+__sev_send_start_query_session_length(struct kvm *kvm, struct kvm_sev_cmd *argp,
+ struct kvm_sev_send_start *params)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ struct sev_data_send_start *data;
+ int ret;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+ if (data == NULL)
+ return -ENOMEM;
+
+ data->handle = sev->handle;
+ ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
+
+ params->session_len = data->session_len;
+ if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
+ sizeof(struct kvm_sev_send_start)))
+ ret = -EFAULT;
+
+ kfree(data);
+ return ret;
+}
+
+static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ struct sev_data_send_start *data;
+ struct kvm_sev_send_start params;
+ void *amd_certs, *session_data;
+ void *pdh_cert, *plat_certs;
+ int ret;
+
+ if (!sev_guest(kvm))
+ return -ENOTTY;
+
+ if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
+ sizeof(struct kvm_sev_send_start)))
+ return -EFAULT;
+
+ /* if session_len is zero, userspace wants to query the session length */
+ if (!params.session_len)
+ return __sev_send_start_query_session_length(kvm, argp,
+ ¶ms);
+
+ /* some sanity checks */
+ if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
+ !params.session_uaddr || params.session_len > SEV_FW_BLOB_MAX_SIZE)
+ return -EINVAL;
+
+ /* allocate the memory to hold the session data blob */
+ session_data = kmalloc(params.session_len, GFP_KERNEL_ACCOUNT);
+ if (!session_data)
+ return -ENOMEM;
+
+ /* copy the certificate blobs from userspace */
+ pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr,
+ params.pdh_cert_len);
+ if (IS_ERR(pdh_cert)) {
+ ret = PTR_ERR(pdh_cert);
+ goto e_free_session;
+ }
+
+ plat_certs = psp_copy_user_blob(params.plat_certs_uaddr,
+ params.plat_certs_len);
+ if (IS_ERR(plat_certs)) {
+ ret = PTR_ERR(plat_certs);
+ goto e_free_pdh;
+ }
+
+ amd_certs = psp_copy_user_blob(params.amd_certs_uaddr,
+ params.amd_certs_len);
+ if (IS_ERR(amd_certs)) {
+ ret = PTR_ERR(amd_certs);
+ goto e_free_plat_cert;
+ }
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+ if (data == NULL) {
+ ret = -ENOMEM;
+ goto e_free_amd_cert;
+ }
+
+ /* populate the FW SEND_START field with system physical address */
+ data->pdh_cert_address = __psp_pa(pdh_cert);
+ data->pdh_cert_len = params.pdh_cert_len;
+ data->plat_certs_address = __psp_pa(plat_certs);
+ data->plat_certs_len = params.plat_certs_len;
+ data->amd_certs_address = __psp_pa(amd_certs);
+ 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);
+
+ if (!ret && copy_to_user((void __user *)(uintptr_t)params.session_uaddr,
+ session_data, params.session_len)) {
+ ret = -EFAULT;
+ goto e_free;
+ }
+
+ params.policy = data->policy;
+ params.session_len = data->session_len;
+ if (copy_to_user((void __user *)(uintptr_t)argp->data, ¶ms,
+ sizeof(struct kvm_sev_send_start)))
+ ret = -EFAULT;
+
+e_free:
+ kfree(data);
+e_free_amd_cert:
+ kfree(amd_certs);
+e_free_plat_cert:
+ kfree(plat_certs);
+e_free_pdh:
+ kfree(pdh_cert);
+e_free_session:
+ kfree(session_data);
+ return ret;
+}
+
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
@@ -957,6 +1079,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
case KVM_SEV_LAUNCH_SECRET:
r = sev_launch_secret(kvm, &sev_cmd);
break;
+ case KVM_SEV_SEND_START:
+ r = sev_send_start(kvm, &sev_cmd);
+ break;
default:
r = -EINVAL;
goto out;
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 5167bf2bfc75..9f63b9d48b63 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -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 */
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 428c7dde6b4b..8827d43e2684 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1598,6 +1598,18 @@ struct kvm_sev_dbg {
__u32 len;
};
+struct kvm_sev_send_start {
+ __u32 policy;
+ __u64 pdh_cert_uaddr;
+ __u32 pdh_cert_len;
+ __u64 plat_certs_uaddr;
+ __u32 plat_certs_len;
+ __u64 amd_certs_uaddr;
+ __u32 amd_certs_len;
+ __u64 session_uaddr;
+ __u32 session_len;
+};
+
#define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
#define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
#define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
--
2.17.1
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]
Reviewed-by : Steve Rutherford <[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/sev.c | 135 +++++++++++++++++-
include/uapi/linux/kvm.h | 9 ++
3 files changed, 164 insertions(+), 4 deletions(-)
diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
index 4fd34fc5c7a7..f46817ef7019 100644
--- a/Documentation/virt/kvm/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/amd-memory-encryption.rst
@@ -290,6 +290,30 @@ Returns: 0 on success, -negative on error
__u32 session_len;
};
+11. KVM_SEV_SEND_UPDATE_DATA
+----------------------------
+
+The KVM_SEV_SEND_UPDATE_DATA command can be used by the hypervisor to encrypt the
+outgoing guest memory region with the encryption context creating using
+KVM_SEV_SEND_START.
+
+Parameters (in): struct kvm_sev_send_update_data
+
+Returns: 0 on success, -negative on error
+
+::
+
+ struct kvm_sev_launch_send_update_data {
+ __u64 hdr_uaddr; /* userspace address containing the packet header */
+ __u32 hdr_len;
+
+ __u64 guest_uaddr; /* the source memory region to be encrypted */
+ __u32 guest_len;
+
+ __u64 trans_uaddr; /* the destition memory region */
+ __u32 trans_len;
+ };
+
References
==========
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 5a15b43b4349..0c92c16505ab 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -23,6 +23,7 @@ static DECLARE_RWSEM(sev_deactivate_lock);
static DEFINE_MUTEX(sev_bitmap_lock);
unsigned int max_sev_asid;
static unsigned int min_sev_asid;
+static unsigned long sev_me_mask;
static unsigned long *sev_asid_bitmap;
static unsigned long *sev_reclaim_asid_bitmap;
#define __sme_page_pa(x) __sme_set(page_to_pfn(x) << PAGE_SHIFT)
@@ -1035,6 +1036,123 @@ static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
+/* Userspace wants to query either header or trans length. */
+static int
+__sev_send_update_data_query_lengths(struct kvm *kvm, struct kvm_sev_cmd *argp,
+ struct kvm_sev_send_update_data *params)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ struct sev_data_send_update_data *data;
+ int ret;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+ if (!data)
+ return -ENOMEM;
+
+ data->handle = sev->handle;
+ ret = sev_issue_cmd(kvm, SEV_CMD_SEND_UPDATE_DATA, data, &argp->error);
+
+ params->hdr_len = data->hdr_len;
+ params->trans_len = data->trans_len;
+
+ if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
+ sizeof(struct kvm_sev_send_update_data)))
+ ret = -EFAULT;
+
+ kfree(data);
+ return ret;
+}
+
+static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ struct sev_data_send_update_data *data;
+ struct kvm_sev_send_update_data params;
+ void *hdr, *trans_data;
+ struct page **guest_page;
+ unsigned long n;
+ int ret, offset;
+
+ if (!sev_guest(kvm))
+ return -ENOTTY;
+
+ if (copy_from_user(¶ms, (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, ¶ms);
+
+ 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;
+}
+
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
@@ -1082,6 +1200,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
case KVM_SEV_SEND_START:
r = sev_send_start(kvm, &sev_cmd);
break;
+ case KVM_SEV_SEND_UPDATE_DATA:
+ r = sev_send_update_data(kvm, &sev_cmd);
+ break;
default:
r = -EINVAL;
goto out;
@@ -1238,16 +1359,22 @@ void sev_vm_destroy(struct kvm *kvm)
int __init sev_hardware_setup(void)
{
struct sev_user_data_status *status;
+ u32 eax, ebx;
int rc;
- /* Maximum number of encrypted guests supported simultaneously */
- max_sev_asid = cpuid_ecx(0x8000001F);
+ /*
+ * Query the memory encryption information.
+ * EBX: Bit 0:5 Pagetable bit position used to indicate encryption
+ * (aka Cbit).
+ * ECX: Maximum number of encrypted guests supported simultaneously.
+ * EDX: Minimum ASID value that should be used for SEV guest.
+ */
+ cpuid(0x8000001f, &eax, &ebx, &max_sev_asid, &min_sev_asid);
if (!svm_sev_enabled())
return 1;
- /* Minimum ASID value that should be used for SEV guest */
- min_sev_asid = cpuid_edx(0x8000001F);
+ sev_me_mask = 1UL << (ebx & 0x3f);
/* Initialize SEV ASID bitmaps */
sev_asid_bitmap = bitmap_zalloc(max_sev_asid, GFP_KERNEL);
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 8827d43e2684..7aaed8ee33cf 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1610,6 +1610,15 @@ struct kvm_sev_send_start {
__u32 session_len;
};
+struct kvm_sev_send_update_data {
+ __u64 hdr_uaddr;
+ __u32 hdr_len;
+ __u64 guest_uaddr;
+ __u32 guest_len;
+ __u64 trans_uaddr;
+ __u32 trans_len;
+};
+
#define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
#define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
#define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
--
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]
Reviewed-by: Steve Rutherford <[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/sev.c | 81 +++++++++++++++++++
include/uapi/linux/kvm.h | 9 +++
3 files changed, 119 insertions(+)
diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
index a45dcb5f8687..ef1f1f3a5b40 100644
--- a/Documentation/virt/kvm/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/amd-memory-encryption.rst
@@ -322,6 +322,35 @@ issued by the hypervisor to delete the encryption context.
Returns: 0 on success, -negative on error
+13. KVM_SEV_RECEIVE_START
+------------------------
+
+The KVM_SEV_RECEIVE_START command is used for creating the memory encryption
+context for an incoming SEV guest. To create the encryption context, the user must
+provide a guest policy, the platform public Diffie-Hellman (PDH) key and session
+information.
+
+Parameters: struct kvm_sev_receive_start (in/out)
+
+Returns: 0 on success, -negative on error
+
+::
+
+ struct kvm_sev_receive_start {
+ __u32 handle; /* if zero then firmware creates a new handle */
+ __u32 policy; /* guest's policy */
+
+ __u64 pdh_uaddr; /* userspace address pointing to the PDH key */
+ __u32 dh_len;
+
+ __u64 session_addr; /* userspace address which points to the guest session information */
+ __u32 session_len;
+ };
+
+On success, the 'handle' field contains a new handle and on error, a negative value.
+
+For more details, see SEV spec Section 6.12.
+
References
==========
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 81d661706d31..74a847c9106d 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1173,6 +1173,84 @@ static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
+static int sev_receive_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ struct sev_data_receive_start *start;
+ struct kvm_sev_receive_start params;
+ int *error = &argp->error;
+ void *session_data;
+ void *pdh_data;
+ int ret;
+
+ if (!sev_guest(kvm))
+ return -ENOTTY;
+
+ /* Get parameter from the userspace */
+ if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
+ sizeof(struct kvm_sev_receive_start)))
+ return -EFAULT;
+
+ /* some sanity checks */
+ if (!params.pdh_uaddr || !params.pdh_len ||
+ !params.session_uaddr || !params.session_len)
+ return -EINVAL;
+
+ pdh_data = psp_copy_user_blob(params.pdh_uaddr, params.pdh_len);
+ if (IS_ERR(pdh_data))
+ return PTR_ERR(pdh_data);
+
+ session_data = psp_copy_user_blob(params.session_uaddr,
+ params.session_len);
+ if (IS_ERR(session_data)) {
+ ret = PTR_ERR(session_data);
+ goto e_free_pdh;
+ }
+
+ ret = -ENOMEM;
+ start = kzalloc(sizeof(*start), GFP_KERNEL);
+ if (!start)
+ goto e_free_session;
+
+ start->handle = params.handle;
+ start->policy = params.policy;
+ start->pdh_cert_address = __psp_pa(pdh_data);
+ start->pdh_cert_len = params.pdh_len;
+ start->session_address = __psp_pa(session_data);
+ start->session_len = params.session_len;
+
+ /* create memory encryption context */
+ ret = __sev_issue_cmd(argp->sev_fd, SEV_CMD_RECEIVE_START, start,
+ error);
+ if (ret)
+ goto e_free;
+
+ /* Bind ASID to this guest */
+ ret = sev_bind_asid(kvm, start->handle, error);
+ if (ret)
+ goto e_free;
+
+ params.handle = start->handle;
+ if (copy_to_user((void __user *)(uintptr_t)argp->data,
+ ¶ms, sizeof(struct kvm_sev_receive_start))) {
+ ret = -EFAULT;
+ sev_unbind_asid(kvm, start->handle);
+ goto e_free;
+ }
+
+ sev->handle = start->handle;
+ sev->fd = argp->sev_fd;
+
+e_free:
+ kfree(start);
+e_free_session:
+ kfree(session_data);
+e_free_pdh:
+ kfree(pdh_data);
+
+ return ret;
+}
+
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
@@ -1226,6 +1304,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
case KVM_SEV_SEND_FINISH:
r = sev_send_finish(kvm, &sev_cmd);
break;
+ case KVM_SEV_RECEIVE_START:
+ r = sev_receive_start(kvm, &sev_cmd);
+ break;
default:
r = -EINVAL;
goto out;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 7aaed8ee33cf..24ac57151d53 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1619,6 +1619,15 @@ struct kvm_sev_send_update_data {
__u32 trans_len;
};
+struct kvm_sev_receive_start {
+ __u32 handle;
+ __u32 policy;
+ __u64 pdh_uaddr;
+ __u32 pdh_len;
+ __u64 session_uaddr;
+ __u32 session_len;
+};
+
#define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
#define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
#define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
--
2.17.1
From: Brijesh Singh <[email protected]>
The command is used for copying the incoming buffer 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]
Reviewed-by: Venu Busireddy <[email protected]>
Reviewed-by: Steve Rutherford <[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/sev.c | 79 +++++++++++++++++++
include/uapi/linux/kvm.h | 9 +++
3 files changed, 112 insertions(+)
diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
index ef1f1f3a5b40..554aa33a99cc 100644
--- a/Documentation/virt/kvm/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/amd-memory-encryption.rst
@@ -351,6 +351,30 @@ On success, the 'handle' field contains a new handle and on error, a negative va
For more details, see SEV spec Section 6.12.
+14. KVM_SEV_RECEIVE_UPDATE_DATA
+----------------------------
+
+The KVM_SEV_RECEIVE_UPDATE_DATA command can be used by the hypervisor to copy
+the incoming buffers into the guest memory region with encryption context
+created during the KVM_SEV_RECEIVE_START.
+
+Parameters (in): struct kvm_sev_receive_update_data
+
+Returns: 0 on success, -negative on error
+
+::
+
+ struct kvm_sev_launch_receive_update_data {
+ __u64 hdr_uaddr; /* userspace address containing the packet header */
+ __u32 hdr_len;
+
+ __u64 guest_uaddr; /* the destination guest memory region */
+ __u32 guest_len;
+
+ __u64 trans_uaddr; /* the incoming buffer memory region */
+ __u32 trans_len;
+ };
+
References
==========
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 74a847c9106d..d5dfd0da53b9 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1251,6 +1251,82 @@ static int sev_receive_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
+static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ struct kvm_sev_receive_update_data params;
+ struct sev_data_receive_update_data *data;
+ void *hdr = NULL, *trans = NULL;
+ struct page **guest_page;
+ unsigned long n;
+ int ret, offset;
+
+ if (!sev_guest(kvm))
+ return -EINVAL;
+
+ if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
+ sizeof(struct kvm_sev_receive_update_data)))
+ return -EFAULT;
+
+ if (!params.hdr_uaddr || !params.hdr_len ||
+ !params.guest_uaddr || !params.guest_len ||
+ !params.trans_uaddr || !params.trans_len)
+ 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;
+
+ hdr = psp_copy_user_blob(params.hdr_uaddr, params.hdr_len);
+ if (IS_ERR(hdr))
+ return PTR_ERR(hdr);
+
+ trans = psp_copy_user_blob(params.trans_uaddr, params.trans_len);
+ if (IS_ERR(trans)) {
+ ret = PTR_ERR(trans);
+ goto e_free_hdr;
+ }
+
+ ret = -ENOMEM;
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ goto e_free_trans;
+
+ data->hdr_address = __psp_pa(hdr);
+ data->hdr_len = params.hdr_len;
+ data->trans_address = __psp_pa(trans);
+ data->trans_len = params.trans_len;
+
+ /* Pin guest memory */
+ ret = -EFAULT;
+ guest_page = sev_pin_memory(kvm, params.guest_uaddr & PAGE_MASK,
+ PAGE_SIZE, &n, 0);
+ if (!guest_page)
+ goto e_free;
+
+ /* The RECEIVE_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_RECEIVE_UPDATE_DATA, data,
+ &argp->error);
+
+ sev_unpin_memory(kvm, guest_page, n);
+
+e_free:
+ kfree(data);
+e_free_trans:
+ kfree(trans);
+e_free_hdr:
+ kfree(hdr);
+
+ return ret;
+}
+
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
@@ -1307,6 +1383,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
case KVM_SEV_RECEIVE_START:
r = sev_receive_start(kvm, &sev_cmd);
break;
+ case KVM_SEV_RECEIVE_UPDATE_DATA:
+ r = sev_receive_update_data(kvm, &sev_cmd);
+ break;
default:
r = -EINVAL;
goto out;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 24ac57151d53..0fe1d206d750 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1628,6 +1628,15 @@ struct kvm_sev_receive_start {
__u32 session_len;
};
+struct kvm_sev_receive_update_data {
+ __u64 hdr_uaddr;
+ __u32 hdr_len;
+ __u64 guest_uaddr;
+ __u32 guest_len;
+ __u64 trans_uaddr;
+ __u32 trans_len;
+};
+
#define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
#define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
#define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
--
2.17.1
From: Brijesh Singh <[email protected]>
The command finalize the guest receiving process and make the SEV guest
ready for the execution.
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: "Radim Krčmář" <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Reviewed-by: Steve Rutherford <[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/sev.c | 23 +++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
index 554aa33a99cc..93cd95d9a6c0 100644
--- a/Documentation/virt/kvm/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/amd-memory-encryption.rst
@@ -375,6 +375,14 @@ Returns: 0 on success, -negative on error
__u32 trans_len;
};
+15. KVM_SEV_RECEIVE_FINISH
+------------------------
+
+After completion of the migration flow, the KVM_SEV_RECEIVE_FINISH command can be
+issued by the hypervisor to make the guest ready for execution.
+
+Returns: 0 on success, -negative on error
+
References
==========
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index d5dfd0da53b9..1f9181e37ef0 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1327,6 +1327,26 @@ static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
+static int sev_receive_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ struct sev_data_receive_finish *data;
+ int ret;
+
+ if (!sev_guest(kvm))
+ return -ENOTTY;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->handle = sev->handle;
+ ret = sev_issue_cmd(kvm, SEV_CMD_RECEIVE_FINISH, data, &argp->error);
+
+ kfree(data);
+ return ret;
+}
+
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
@@ -1386,6 +1406,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
case KVM_SEV_RECEIVE_UPDATE_DATA:
r = sev_receive_update_data(kvm, &sev_cmd);
break;
+ case KVM_SEV_RECEIVE_FINISH:
+ r = sev_receive_finish(kvm, &sev_cmd);
+ break;
default:
r = -EINVAL;
goto out;
--
2.17.1
From: Brijesh Singh <[email protected]>
KVM hypercall framework relies on alternative framework to patch the
VMCALL -> VMMCALL on AMD platform. If a hypercall is made before
apply_alternative() is called then it defaults to VMCALL. The approach
works fine on non SEV guest. A VMCALL would causes #UD, and hypervisor
will be able to decode the instruction and do the right things. But
when SEV is active, guest memory is encrypted with guest key and
hypervisor will not be able to decode the instruction bytes.
Add SEV specific hypercall3, it unconditionally uses VMMCALL. The hypercall
will be used by the SEV guest to notify encrypted pages to the hypervisor.
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]
Reviewed-by: Steve Rutherford <[email protected]>
Reviewed-by: Venu Busireddy <[email protected]>
Signed-off-by: Brijesh Singh <[email protected]>
Signed-off-by: Ashish Kalra <[email protected]>
---
arch/x86/include/asm/kvm_para.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 9b4df6eaa11a..6c09255633a4 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -84,6 +84,18 @@ static inline long kvm_hypercall4(unsigned int nr, unsigned long p1,
return ret;
}
+static inline long kvm_sev_hypercall3(unsigned int nr, unsigned long p1,
+ unsigned long p2, unsigned long p3)
+{
+ long ret;
+
+ asm volatile("vmmcall"
+ : "=a"(ret)
+ : "a"(nr), "b"(p1), "c"(p2), "d"(p3)
+ : "memory");
+ return ret;
+}
+
#ifdef CONFIG_KVM_GUEST
bool kvm_para_available(void);
unsigned int kvm_arch_para_features(void);
--
2.17.1
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]
Reviewed-by: Steve Rutherford <[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/sev.c | 23 +++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
index f46817ef7019..a45dcb5f8687 100644
--- a/Documentation/virt/kvm/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/amd-memory-encryption.rst
@@ -314,6 +314,14 @@ Returns: 0 on success, -negative on error
__u32 trans_len;
};
+12. KVM_SEV_SEND_FINISH
+------------------------
+
+After completion of the migration flow, the KVM_SEV_SEND_FINISH command can be
+issued by the hypervisor to delete the encryption context.
+
+Returns: 0 on success, -negative on error
+
References
==========
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 0c92c16505ab..81d661706d31 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1153,6 +1153,26 @@ static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
+static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ struct sev_data_send_finish *data;
+ int ret;
+
+ if (!sev_guest(kvm))
+ return -ENOTTY;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->handle = sev->handle;
+ ret = sev_issue_cmd(kvm, SEV_CMD_SEND_FINISH, data, &argp->error);
+
+ kfree(data);
+ return ret;
+}
+
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
@@ -1203,6 +1223,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
case KVM_SEV_SEND_UPDATE_DATA:
r = sev_send_update_data(kvm, &sev_cmd);
break;
+ case KVM_SEV_SEND_FINISH:
+ r = sev_send_finish(kvm, &sev_cmd);
+ break;
default:
r = -EINVAL;
goto out;
--
2.17.1
From: Brijesh Singh <[email protected]>
This hypercall is used by the SEV guest to notify a change in the page
encryption status to the hypervisor. The hypercall should be invoked
only when the encryption attribute is changed from encrypted -> decrypted
and vice versa. By default all guest pages are considered encrypted.
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: "Radim Krčmář" <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Reviewed-by: Venu Busireddy <[email protected]>
Signed-off-by: Brijesh Singh <[email protected]>
Signed-off-by: Ashish Kalra <[email protected]>
---
Documentation/virt/kvm/hypercalls.rst | 15 +++++
arch/x86/include/asm/kvm_host.h | 2 +
arch/x86/kvm/svm/sev.c | 90 +++++++++++++++++++++++++++
arch/x86/kvm/svm/svm.c | 2 +
arch/x86/kvm/svm/svm.h | 4 ++
arch/x86/kvm/vmx/vmx.c | 1 +
arch/x86/kvm/x86.c | 6 ++
include/uapi/linux/kvm_para.h | 1 +
8 files changed, 121 insertions(+)
diff --git a/Documentation/virt/kvm/hypercalls.rst b/Documentation/virt/kvm/hypercalls.rst
index dbaf207e560d..ff5287e68e81 100644
--- a/Documentation/virt/kvm/hypercalls.rst
+++ b/Documentation/virt/kvm/hypercalls.rst
@@ -169,3 +169,18 @@ a0: destination APIC ID
:Usage example: When sending a call-function IPI-many to vCPUs, yield if
any of the IPI target vCPUs was preempted.
+
+
+8. KVM_HC_PAGE_ENC_STATUS
+-------------------------
+:Architecture: x86
+:Status: active
+:Purpose: Notify the encryption status changes in guest page table (SEV guest)
+
+a0: the guest physical address of the start page
+a1: the number of pages
+a2: encryption attribute
+
+ Where:
+ * 1: Encryption attribute is set
+ * 0: Encryption attribute is cleared
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 42a2d0d3984a..4a8ee22f4f5b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1254,6 +1254,8 @@ struct kvm_x86_ops {
bool (*apic_init_signal_blocked)(struct kvm_vcpu *vcpu);
int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu);
+ int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa,
+ unsigned long sz, unsigned long mode);
};
struct kvm_x86_init_ops {
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 1f9181e37ef0..7dc68db70405 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1347,6 +1347,93 @@ static int sev_receive_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
+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);
+ if (!map) {
+ pr_err_once("Failed to allocate encrypted bitmap size %lx\n",
+ sz);
+ return -ENOMEM;
+ }
+
+ /* mark the page encrypted (by default) */
+ memset(map, 0xff, sz);
+
+ bitmap_copy(map, sev->page_enc_bmap, sev->page_enc_bmap_size);
+ kvfree(sev->page_enc_bmap);
+
+ sev->page_enc_bmap = map;
+ sev->page_enc_bmap_size = new_size;
+
+ return 0;
+}
+
+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;
+ kvm_pfn_t pfn_start, pfn_end;
+ gfn_t gfn_start, gfn_end;
+
+ if (!sev_guest(kvm))
+ return -EINVAL;
+
+ if (!npages)
+ return 0;
+
+ gfn_start = gpa_to_gfn(gpa);
+ gfn_end = gfn_start + npages;
+
+ /* out of bound access error check */
+ if (gfn_end <= gfn_start)
+ return -EINVAL;
+
+ /* lets make sure that gpa exist in our memslot */
+ pfn_start = gfn_to_pfn(kvm, gfn_start);
+ pfn_end = gfn_to_pfn(kvm, gfn_end);
+
+ 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);
+
+ if (sev->page_enc_bmap_size < gfn_end)
+ 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 0;
+}
+
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
@@ -1560,6 +1647,9 @@ void sev_vm_destroy(struct kvm *kvm)
sev_unbind_asid(kvm, sev->handle);
sev_asid_free(sev->asid);
+
+ kvfree(sev->page_enc_bmap);
+ sev->page_enc_bmap = NULL;
}
int __init sev_hardware_setup(void)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 2f379bacbb26..1013ef0f4ce2 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4014,6 +4014,8 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
.apic_init_signal_blocked = svm_apic_init_signal_blocked,
.check_nested_events = svm_check_nested_events,
+
+ .page_enc_status_hc = svm_page_enc_status_hc,
};
static struct kvm_x86_init_ops svm_init_ops __initdata = {
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index df3474f4fb02..6a562f5928a2 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -65,6 +65,8 @@ struct kvm_sev_info {
int fd; /* SEV device fd */
unsigned long pages_locked; /* Number of pages locked */
struct list_head regions_list; /* List of registered regions */
+ unsigned long *page_enc_bmap;
+ unsigned long page_enc_bmap_size;
};
struct kvm_svm {
@@ -400,6 +402,8 @@ int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
bool has_error_code, u32 error_code);
int svm_check_nested_events(struct kvm_vcpu *vcpu);
int nested_svm_exit_special(struct vcpu_svm *svm);
+int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
+ unsigned long npages, unsigned long enc);
/* avic.c */
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index c2c6335a998c..7d01d3aa6461 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7838,6 +7838,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {
.nested_get_evmcs_version = NULL,
.need_emulation_on_page_fault = vmx_need_emulation_on_page_fault,
.apic_init_signal_blocked = vmx_apic_init_signal_blocked,
+ .page_enc_status_hc = NULL,
};
static __init int hardware_setup(void)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c5835f9cb9ad..5f5ddb5765e2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7605,6 +7605,12 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
kvm_sched_yield(vcpu->kvm, a0);
ret = 0;
break;
+ case KVM_HC_PAGE_ENC_STATUS:
+ ret = -KVM_ENOSYS;
+ if (kvm_x86_ops.page_enc_status_hc)
+ ret = kvm_x86_ops.page_enc_status_hc(vcpu->kvm,
+ a0, a1, a2);
+ break;
default:
ret = -KVM_ENOSYS;
break;
diff --git a/include/uapi/linux/kvm_para.h b/include/uapi/linux/kvm_para.h
index 8b86609849b9..847b83b75dc8 100644
--- a/include/uapi/linux/kvm_para.h
+++ b/include/uapi/linux/kvm_para.h
@@ -29,6 +29,7 @@
#define KVM_HC_CLOCK_PAIRING 9
#define KVM_HC_SEND_IPI 10
#define KVM_HC_SCHED_YIELD 11
+#define KVM_HC_PAGE_ENC_STATUS 12
/*
* hypercalls use architecture specific
--
2.17.1
From: Brijesh Singh <[email protected]>
The ioctl can be used to retrieve page encryption bitmap for a given
gfn range.
Return the correct bitmap as per the number of pages being requested
by the user. Ensure that we only copy bmap->num_pages bytes in the
userspace buffer, if bmap->num_pages is not byte aligned we read
the trailing bits from the userspace and copy those bits as is.
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: "Radim Krčmář" <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Reviewed-by: Venu Busireddy <[email protected]>
Signed-off-by: Brijesh Singh <[email protected]>
Signed-off-by: Ashish Kalra <[email protected]>
---
Documentation/virt/kvm/api.rst | 27 +++++++++++++
arch/x86/include/asm/kvm_host.h | 2 +
arch/x86/kvm/svm/sev.c | 70 +++++++++++++++++++++++++++++++++
arch/x86/kvm/svm/svm.c | 1 +
arch/x86/kvm/svm/svm.h | 1 +
arch/x86/kvm/x86.c | 12 ++++++
include/uapi/linux/kvm.h | 12 ++++++
7 files changed, 125 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index efbbe570aa9b..e2f0dd105b5c 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4636,6 +4636,33 @@ This ioctl resets VCPU registers and control structures according to
the clear cpu reset definition in the POP. However, the cpu is not put
into ESA mode. This reset is a superset of the initial reset.
+4.125 KVM_GET_PAGE_ENC_BITMAP (vm ioctl)
+---------------------------------------
+
+:Capability: basic
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: struct kvm_page_enc_bitmap (in/out)
+:Returns: 0 on success, -1 on error
+
+/* for KVM_GET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+ __u64 start_gfn;
+ __u64 num_pages;
+ union {
+ void __user *enc_bitmap; /* one bit per page */
+ __u64 padding2;
+ };
+};
+
+The encrypted VMs have concept of private and shared pages. The private
+page is encrypted with the guest-specific key, while shared page may
+be encrypted with the hypervisor key. The KVM_GET_PAGE_ENC_BITMAP can
+be used to get the bitmap indicating whether the guest page is private
+or shared. The bitmap can be used during the guest migration, if the page
+is private then userspace need to use SEV migration commands to transmit
+the page.
+
4.125 KVM_S390_PV_COMMAND
-------------------------
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4a8ee22f4f5b..9e428befb6a4 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1256,6 +1256,8 @@ struct kvm_x86_ops {
int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu);
int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa,
unsigned long sz, unsigned long mode);
+ int (*get_page_enc_bitmap)(struct kvm *kvm,
+ struct kvm_page_enc_bitmap *bmap);
};
struct kvm_x86_init_ops {
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 7dc68db70405..73bbbffb3487 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1434,6 +1434,76 @@ int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
return 0;
}
+int svm_get_page_enc_bitmap(struct kvm *kvm,
+ struct kvm_page_enc_bitmap *bmap)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ unsigned long gfn_start, gfn_end;
+ unsigned long sz, i, sz_bytes;
+ unsigned long *bitmap;
+ int ret, n;
+
+ if (!sev_guest(kvm))
+ return -ENOTTY;
+
+ gfn_start = bmap->start_gfn;
+ gfn_end = gfn_start + bmap->num_pages;
+
+ sz = ALIGN(bmap->num_pages, BITS_PER_LONG) / BITS_PER_BYTE;
+ bitmap = kmalloc(sz, GFP_KERNEL);
+ if (!bitmap)
+ return -ENOMEM;
+
+ /* by default all pages are marked encrypted */
+ memset(bitmap, 0xff, sz);
+
+ mutex_lock(&kvm->lock);
+ if (sev->page_enc_bmap) {
+ i = gfn_start;
+ for_each_clear_bit_from(i, sev->page_enc_bmap,
+ min(sev->page_enc_bmap_size, gfn_end))
+ clear_bit(i - gfn_start, bitmap);
+ }
+ mutex_unlock(&kvm->lock);
+
+ ret = -EFAULT;
+
+ n = bmap->num_pages % BITS_PER_BYTE;
+ sz_bytes = ALIGN(bmap->num_pages, BITS_PER_BYTE) / BITS_PER_BYTE;
+
+ /*
+ * Return the correct bitmap as per the number of pages being
+ * requested by the user. Ensure that we only copy bmap->num_pages
+ * bytes in the userspace buffer, if bmap->num_pages is not byte
+ * aligned we read the trailing bits from the userspace and copy
+ * those bits as is.
+ */
+
+ if (n) {
+ unsigned char *bitmap_kernel = (unsigned char *)bitmap;
+ unsigned char bitmap_user;
+ unsigned long offset, mask;
+
+ offset = bmap->num_pages / BITS_PER_BYTE;
+ if (copy_from_user(&bitmap_user, bmap->enc_bitmap + offset,
+ sizeof(unsigned char)))
+ goto out;
+
+ mask = GENMASK(n - 1, 0);
+ bitmap_user &= ~mask;
+ bitmap_kernel[offset] &= mask;
+ bitmap_kernel[offset] |= bitmap_user;
+ }
+
+ if (copy_to_user(bmap->enc_bitmap, bitmap, sz_bytes))
+ goto out;
+
+ ret = 0;
+out:
+ kfree(bitmap);
+ return ret;
+}
+
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 1013ef0f4ce2..588709a9f68e 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4016,6 +4016,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
.check_nested_events = svm_check_nested_events,
.page_enc_status_hc = svm_page_enc_status_hc,
+ .get_page_enc_bitmap = svm_get_page_enc_bitmap,
};
static struct kvm_x86_init_ops svm_init_ops __initdata = {
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 6a562f5928a2..f087fa7b380c 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -404,6 +404,7 @@ int svm_check_nested_events(struct kvm_vcpu *vcpu);
int nested_svm_exit_special(struct vcpu_svm *svm);
int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
unsigned long npages, unsigned long enc);
+int svm_get_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
/* avic.c */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5f5ddb5765e2..937797cfaf9a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5208,6 +5208,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
case KVM_SET_PMU_EVENT_FILTER:
r = kvm_vm_ioctl_set_pmu_event_filter(kvm, argp);
break;
+ case KVM_GET_PAGE_ENC_BITMAP: {
+ struct kvm_page_enc_bitmap bitmap;
+
+ r = -EFAULT;
+ if (copy_from_user(&bitmap, argp, sizeof(bitmap)))
+ goto out;
+
+ r = -ENOTTY;
+ if (kvm_x86_ops.get_page_enc_bitmap)
+ r = kvm_x86_ops.get_page_enc_bitmap(kvm, &bitmap);
+ break;
+ }
default:
r = -ENOTTY;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 0fe1d206d750..af62f2afaa5d 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -505,6 +505,16 @@ struct kvm_dirty_log {
};
};
+/* for KVM_GET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+ __u64 start_gfn;
+ __u64 num_pages;
+ union {
+ void __user *enc_bitmap; /* one bit per page */
+ __u64 padding2;
+ };
+};
+
/* for KVM_CLEAR_DIRTY_LOG */
struct kvm_clear_dirty_log {
__u32 slot;
@@ -1518,6 +1528,8 @@ struct kvm_pv_cmd {
/* Available with KVM_CAP_S390_PROTECTED */
#define KVM_S390_PV_COMMAND _IOWR(KVMIO, 0xc5, struct kvm_pv_cmd)
+#define KVM_GET_PAGE_ENC_BITMAP _IOW(KVMIO, 0xc6, struct kvm_page_enc_bitmap)
+
/* Secure Encrypted Virtualization command */
enum sev_cmd_id {
/* Guest initialization commands */
--
2.17.1
From: Ashish Kalra <[email protected]>
Add a new paravirt callback as part of x86_hyper_runtime
(x86 hypervisor specific runtime callbacks) to do hypervisor
specific page encryption status notifications from the SEV
guest.
Signed-off-by: Ashish Kalra <[email protected]>
---
arch/x86/include/asm/x86_init.h | 10 +++++++++-
arch/x86/kernel/kvm.c | 12 ++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 96d9cd208610..888dca30a17a 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -238,10 +238,18 @@ struct x86_legacy_features {
/**
* struct x86_hyper_runtime - x86 hypervisor specific runtime callbacks
*
- * @pin_vcpu: pin current vcpu to specified physical cpu (run rarely)
+ * @pin_vcpu: pin current vcpu to specified physical
+ * cpu (run rarely)
+ * @sev_migration_hcall: this hypercall is used by the SEV guest
+ * to notify a change in the page encryption
+ * status to the hypervisor.
*/
struct x86_hyper_runtime {
void (*pin_vcpu)(int cpu);
+#if defined(CONFIG_AMD_MEM_ENCRYPT)
+ long (*sev_migration_hcall)(unsigned long physaddr,
+ unsigned long npages, bool enc);
+#endif
};
/**
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 6efe0410fb72..928ddb8a8cfc 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -729,6 +729,15 @@ static void __init kvm_init_platform(void)
x86_platform.apic_post_init = kvm_apic_init;
}
+#if defined(CONFIG_AMD_MEM_ENCRYPT)
+long kvm_sev_migration_hcall(unsigned long physaddr, unsigned long npages,
+ bool enc)
+{
+ return kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS, physaddr, npages,
+ enc);
+}
+#endif
+
const __initconst struct hypervisor_x86 x86_hyper_kvm = {
.name = "KVM",
.detect = kvm_detect,
@@ -736,6 +745,9 @@ const __initconst struct hypervisor_x86 x86_hyper_kvm = {
.init.guest_late_init = kvm_guest_init,
.init.x2apic_available = kvm_para_available,
.init.init_platform = kvm_init_platform,
+#if defined(CONFIG_AMD_MEM_ENCRYPT)
+ .runtime.sev_migration_hcall = kvm_sev_migration_hcall,
+#endif
};
static __init int activate_jump_labels(void)
--
2.17.1
From: Brijesh Singh <[email protected]>
Invoke a hypercall when a memory region is changed from encrypted ->
decrypted and vice versa. Hypervisor needs to know the page encryption
status during the guest migration.
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: "Radim Krčmář" <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Reviewed-by: Venu Busireddy <[email protected]>
Signed-off-by: Brijesh Singh <[email protected]>
Signed-off-by: Ashish Kalra <[email protected]>
---
arch/x86/include/asm/paravirt.h | 10 +++++
arch/x86/include/asm/paravirt_types.h | 2 +
arch/x86/kernel/paravirt.c | 1 +
arch/x86/mm/mem_encrypt.c | 58 ++++++++++++++++++++++++++-
arch/x86/mm/pat/set_memory.c | 7 ++++
5 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 694d8daf4983..8127b9c141bf 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -78,6 +78,12 @@ static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
PVOP_VCALL1(mmu.exit_mmap, mm);
}
+static inline void page_encryption_changed(unsigned long vaddr, int npages,
+ bool enc)
+{
+ PVOP_VCALL3(mmu.page_encryption_changed, vaddr, npages, enc);
+}
+
#ifdef CONFIG_PARAVIRT_XXL
static inline void load_sp0(unsigned long sp0)
{
@@ -946,6 +952,10 @@ static inline void paravirt_arch_dup_mmap(struct mm_struct *oldmm,
static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
{
}
+
+static inline void page_encryption_changed(unsigned long vaddr, int npages, bool enc)
+{
+}
#endif
#endif /* __ASSEMBLY__ */
#endif /* _ASM_X86_PARAVIRT_H */
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 732f62e04ddb..03bfd515c59c 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -215,6 +215,8 @@ struct pv_mmu_ops {
/* Hook for intercepting the destruction of an mm_struct. */
void (*exit_mmap)(struct mm_struct *mm);
+ void (*page_encryption_changed)(unsigned long vaddr, int npages,
+ bool enc);
#ifdef CONFIG_PARAVIRT_XXL
struct paravirt_callee_save read_cr2;
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index c131ba4e70ef..840c02b23aeb 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -367,6 +367,7 @@ struct paravirt_patch_template pv_ops = {
(void (*)(struct mmu_gather *, void *))tlb_remove_page,
.mmu.exit_mmap = paravirt_nop,
+ .mmu.page_encryption_changed = paravirt_nop,
#ifdef CONFIG_PARAVIRT_XXL
.mmu.read_cr2 = __PV_IS_CALLEE_SAVE(native_read_cr2),
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index f4bd4b431ba1..603f5abf8a78 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -19,6 +19,7 @@
#include <linux/kernel.h>
#include <linux/bitops.h>
#include <linux/dma-mapping.h>
+#include <linux/kvm_para.h>
#include <asm/tlbflush.h>
#include <asm/fixmap.h>
@@ -29,6 +30,7 @@
#include <asm/processor-flags.h>
#include <asm/msr.h>
#include <asm/cmdline.h>
+#include <asm/kvm_para.h>
#include "mm_internal.h"
@@ -196,6 +198,48 @@ void __init sme_early_init(void)
swiotlb_force = SWIOTLB_FORCE;
}
+static void set_memory_enc_dec_hypercall(unsigned long vaddr, int npages,
+ bool enc)
+{
+ unsigned long sz = npages << PAGE_SHIFT;
+ unsigned long vaddr_end, vaddr_next;
+
+ vaddr_end = vaddr + sz;
+
+ for (; vaddr < vaddr_end; vaddr = vaddr_next) {
+ int psize, pmask, level;
+ unsigned long pfn;
+ pte_t *kpte;
+
+ kpte = lookup_address(vaddr, &level);
+ if (!kpte || pte_none(*kpte))
+ return;
+
+ switch (level) {
+ case PG_LEVEL_4K:
+ pfn = pte_pfn(*kpte);
+ break;
+ case PG_LEVEL_2M:
+ pfn = pmd_pfn(*(pmd_t *)kpte);
+ break;
+ case PG_LEVEL_1G:
+ pfn = pud_pfn(*(pud_t *)kpte);
+ break;
+ default:
+ return;
+ }
+
+ psize = page_level_size(level);
+ pmask = page_level_mask(level);
+
+ if (x86_platform.hyper.sev_migration_hcall)
+ x86_platform.hyper.sev_migration_hcall(pfn << PAGE_SHIFT,
+ psize >> PAGE_SHIFT,
+ enc);
+ vaddr_next = (vaddr & pmask) + psize;
+ }
+}
+
static void __init __set_clr_pte_enc(pte_t *kpte, int level, bool enc)
{
pgprot_t old_prot, new_prot;
@@ -253,12 +297,13 @@ static void __init __set_clr_pte_enc(pte_t *kpte, int level, bool enc)
static int __init early_set_memory_enc_dec(unsigned long vaddr,
unsigned long size, bool enc)
{
- unsigned long vaddr_end, vaddr_next;
+ unsigned long vaddr_end, vaddr_next, start;
unsigned long psize, pmask;
int split_page_size_mask;
int level, ret;
pte_t *kpte;
+ start = vaddr;
vaddr_next = vaddr;
vaddr_end = vaddr + size;
@@ -313,6 +358,8 @@ static int __init early_set_memory_enc_dec(unsigned long vaddr,
ret = 0;
+ set_memory_enc_dec_hypercall(start, PAGE_ALIGN(size) >> PAGE_SHIFT,
+ enc);
out:
__flush_tlb_all();
return ret;
@@ -451,6 +498,15 @@ void __init mem_encrypt_init(void)
if (sev_active())
static_branch_enable(&sev_enable_key);
+#ifdef CONFIG_PARAVIRT
+ /*
+ * With SEV, we need to make a hypercall when page encryption state is
+ * changed.
+ */
+ if (sev_active())
+ pv_ops.mmu.page_encryption_changed = set_memory_enc_dec_hypercall;
+#endif
+
pr_info("AMD %s active\n",
sev_active() ? "Secure Encrypted Virtualization (SEV)"
: "Secure Memory Encryption (SME)");
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 59eca6a94ce7..9aaf1b6f5a1b 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -27,6 +27,7 @@
#include <asm/proto.h>
#include <asm/memtype.h>
#include <asm/set_memory.h>
+#include <asm/paravirt.h>
#include "../mm_internal.h"
@@ -2003,6 +2004,12 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
*/
cpa_flush(&cpa, 0);
+ /* Notify hypervisor that a given memory range is mapped encrypted
+ * or decrypted. The hypervisor will use this information during the
+ * VM migration.
+ */
+ page_encryption_changed(addr, numpages, enc);
+
return ret;
}
--
2.17.1
From: Brijesh Singh <[email protected]>
The ioctl can be used to set page encryption bitmap for an
incoming guest.
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: "Radim Krčmář" <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Reviewed-by: Venu Busireddy <[email protected]>
Signed-off-by: Brijesh Singh <[email protected]>
Signed-off-by: Ashish Kalra <[email protected]>
---
Documentation/virt/kvm/api.rst | 44 ++++++++++++++++++++++++++++++
arch/x86/include/asm/kvm_host.h | 2 ++
arch/x86/kvm/svm/sev.c | 48 +++++++++++++++++++++++++++++++++
arch/x86/kvm/svm/svm.c | 1 +
arch/x86/kvm/svm/svm.h | 1 +
arch/x86/kvm/x86.c | 12 +++++++++
include/uapi/linux/kvm.h | 1 +
7 files changed, 109 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index e2f0dd105b5c..56c934db2a96 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4663,6 +4663,28 @@ or shared. The bitmap can be used during the guest migration, if the page
is private then userspace need to use SEV migration commands to transmit
the page.
+4.126 KVM_SET_PAGE_ENC_BITMAP (vm ioctl)
+---------------------------------------
+
+:Capability: basic
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: struct kvm_page_enc_bitmap (in/out)
+:Returns: 0 on success, -1 on error
+
+/* for KVM_SET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+ __u64 start_gfn;
+ __u64 num_pages;
+ union {
+ void __user *enc_bitmap; /* one bit per page */
+ __u64 padding2;
+ };
+};
+
+During the guest live migration the outgoing guest exports its page encryption
+bitmap, the KVM_SET_PAGE_ENC_BITMAP can be used to build the page encryption
+bitmap for an incoming guest.
4.125 KVM_S390_PV_COMMAND
-------------------------
@@ -4717,6 +4739,28 @@ KVM_PV_VM_VERIFY
Verify the integrity of the unpacked image. Only if this succeeds,
KVM is allowed to start protected VCPUs.
+4.126 KVM_SET_PAGE_ENC_BITMAP (vm ioctl)
+---------------------------------------
+
+:Capability: basic
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: struct kvm_page_enc_bitmap (in/out)
+:Returns: 0 on success, -1 on error
+
+/* for KVM_SET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+ __u64 start_gfn;
+ __u64 num_pages;
+ union {
+ void __user *enc_bitmap; /* one bit per page */
+ __u64 padding2;
+ };
+};
+
+During the guest live migration the outgoing guest exports its page encryption
+bitmap, the KVM_SET_PAGE_ENC_BITMAP can be used to build the page encryption
+bitmap for an incoming guest.
5. The kvm_run structure
========================
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 9e428befb6a4..fc74144d5ab0 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1258,6 +1258,8 @@ struct kvm_x86_ops {
unsigned long sz, unsigned long mode);
int (*get_page_enc_bitmap)(struct kvm *kvm,
struct kvm_page_enc_bitmap *bmap);
+ int (*set_page_enc_bitmap)(struct kvm *kvm,
+ struct kvm_page_enc_bitmap *bmap);
};
struct kvm_x86_init_ops {
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 73bbbffb3487..64ff51ec4933 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1504,6 +1504,54 @@ int svm_get_page_enc_bitmap(struct kvm *kvm,
return ret;
}
+int svm_set_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;
+ int ret;
+
+ if (!sev_guest(kvm))
+ return -ENOTTY;
+ /* special case of resetting the complete bitmap */
+ if (!bmap->enc_bitmap) {
+ mutex_lock(&kvm->lock);
+ /* by default all pages are marked encrypted */
+ if (sev->page_enc_bmap_size)
+ bitmap_fill(sev->page_enc_bmap, sev->page_enc_bmap_size);
+ mutex_unlock(&kvm->lock);
+ return 0;
+ }
+
+ 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;
+
+ ret = -EFAULT;
+ if (copy_from_user(bitmap, bmap->enc_bitmap, sz))
+ goto out;
+
+ mutex_lock(&kvm->lock);
+ ret = sev_resize_page_enc_bitmap(kvm, gfn_end);
+ if (ret)
+ goto unlock;
+
+ bitmap_copy(sev->page_enc_bmap + BIT_WORD(gfn_start), bitmap, (gfn_end - gfn_start));
+
+ ret = 0;
+unlock:
+ mutex_unlock(&kvm->lock);
+out:
+ kfree(bitmap);
+ return ret;
+}
+
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 588709a9f68e..501e82f5593c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4017,6 +4017,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
.page_enc_status_hc = svm_page_enc_status_hc,
.get_page_enc_bitmap = svm_get_page_enc_bitmap,
+ .set_page_enc_bitmap = svm_set_page_enc_bitmap,
};
static struct kvm_x86_init_ops svm_init_ops __initdata = {
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index f087fa7b380c..2ebdcce50312 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -405,6 +405,7 @@ int nested_svm_exit_special(struct vcpu_svm *svm);
int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
unsigned long npages, unsigned long enc);
int svm_get_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
+int svm_set_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
/* avic.c */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 937797cfaf9a..c4166d7a0493 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5220,6 +5220,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = kvm_x86_ops.get_page_enc_bitmap(kvm, &bitmap);
break;
}
+ case KVM_SET_PAGE_ENC_BITMAP: {
+ struct kvm_page_enc_bitmap bitmap;
+
+ r = -EFAULT;
+ if (copy_from_user(&bitmap, argp, sizeof(bitmap)))
+ goto out;
+
+ r = -ENOTTY;
+ if (kvm_x86_ops.set_page_enc_bitmap)
+ r = kvm_x86_ops.set_page_enc_bitmap(kvm, &bitmap);
+ break;
+ }
default:
r = -ENOTTY;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index af62f2afaa5d..2798b17484d0 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1529,6 +1529,7 @@ struct kvm_pv_cmd {
#define KVM_S390_PV_COMMAND _IOWR(KVMIO, 0xc5, struct kvm_pv_cmd)
#define KVM_GET_PAGE_ENC_BITMAP _IOW(KVMIO, 0xc6, struct kvm_page_enc_bitmap)
+#define KVM_SET_PAGE_ENC_BITMAP _IOW(KVMIO, 0xc7, struct kvm_page_enc_bitmap)
/* Secure Encrypted Virtualization command */
enum sev_cmd_id {
--
2.17.1
From: Ashish Kalra <[email protected]>
Add support for static allocation of the unified Page encryption bitmap by
extending kvm_arch_commit_memory_region() callack to add svm specific x86_ops
which can read the userspace provided memory region/memslots and calculate
the amount of guest RAM managed by the KVM and grow the bitmap based
on that information, i.e. the highest guest PA that is mapped by a memslot.
Signed-off-by: Ashish Kalra <[email protected]>
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/svm/sev.c | 35 +++++++++++++++++++++++++++++++++
arch/x86/kvm/svm/svm.c | 1 +
arch/x86/kvm/svm/svm.h | 1 +
arch/x86/kvm/x86.c | 5 +++++
5 files changed, 43 insertions(+)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index fc74144d5ab0..b573ea85b57e 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1254,6 +1254,7 @@ struct kvm_x86_ops {
bool (*apic_init_signal_blocked)(struct kvm_vcpu *vcpu);
int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu);
+ void (*commit_memory_region)(struct kvm *kvm, enum kvm_mr_change change);
int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa,
unsigned long sz, unsigned long mode);
int (*get_page_enc_bitmap)(struct kvm *kvm,
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 64ff51ec4933..ba5ecd1de644 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1377,6 +1377,41 @@ static int sev_resize_page_enc_bitmap(struct kvm *kvm, unsigned long new_size)
return 0;
}
+void svm_commit_memory_region(struct kvm *kvm, enum kvm_mr_change change)
+{
+ struct kvm_memslots *slots;
+ struct kvm_memory_slot *memslot;
+ gfn_t start, end = 0;
+
+ spin_lock(&kvm->mmu_lock);
+ if (change == KVM_MR_CREATE) {
+ slots = kvm_memslots(kvm);
+ kvm_for_each_memslot(memslot, slots) {
+ start = memslot->base_gfn;
+ end = memslot->base_gfn + memslot->npages;
+ /*
+ * KVM memslots is a sorted list, starting with
+ * the highest mapped guest PA, so pick the topmost
+ * valid guest PA.
+ */
+ if (memslot->npages)
+ break;
+ }
+ }
+ spin_unlock(&kvm->mmu_lock);
+
+ if (end) {
+ /*
+ * NORE: This callback is invoked in vm ioctl
+ * set_user_memory_region, hence we can use a
+ * mutex here.
+ */
+ mutex_lock(&kvm->lock);
+ sev_resize_page_enc_bitmap(kvm, end);
+ mutex_unlock(&kvm->lock);
+ }
+}
+
int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
unsigned long npages, unsigned long enc)
{
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 501e82f5593c..442adbbb0641 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4015,6 +4015,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
.check_nested_events = svm_check_nested_events,
+ .commit_memory_region = svm_commit_memory_region,
.page_enc_status_hc = svm_page_enc_status_hc,
.get_page_enc_bitmap = svm_get_page_enc_bitmap,
.set_page_enc_bitmap = svm_set_page_enc_bitmap,
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 2ebdcce50312..fd99e0a5417a 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -406,6 +406,7 @@ int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
unsigned long npages, unsigned long enc);
int svm_get_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
int svm_set_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
+void svm_commit_memory_region(struct kvm *kvm, enum kvm_mr_change change);
/* avic.c */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c4166d7a0493..8938de868d42 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10133,6 +10133,11 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
kvm_mmu_change_mmu_pages(kvm,
kvm_mmu_calculate_default_mmu_pages(kvm));
+ if (change == KVM_MR_CREATE || change == KVM_MR_DELETE) {
+ if (kvm_x86_ops.commit_memory_region)
+ kvm_x86_ops.commit_memory_region(kvm, change);
+ }
+
/*
* Dirty logging tracks sptes in 4k granularity, meaning that large
* sptes have to be split. If live migration is successful, the guest
--
2.17.1
From: Ashish Kalra <[email protected]>
The guest support for detecting and enabling SEV Live migration
feature uses the following logic :
- kvm_init_plaform() checks if its booted under the EFI
- If not EFI,
i) check for the KVM_FEATURE_CPUID
ii) if CPUID reports that migration is support then issue wrmsrl
to enable the SEV migration support
- If EFI,
i) Check the KVM_FEATURE_CPUID.
ii) If CPUID reports that migration is supported, then reads the UEFI enviroment variable which
indicates OVMF support for live migration.
iii) If variable is set then wrmsr to enable the SEV migration support.
The EFI live migration check is done using a late_initcall() callback.
Signed-off-by: Ashish Kalra <[email protected]>
---
arch/x86/include/asm/mem_encrypt.h | 11 ++++++
arch/x86/kernel/kvm.c | 62 ++++++++++++++++++++++++++++++
arch/x86/mm/mem_encrypt.c | 11 ++++++
3 files changed, 84 insertions(+)
diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 848ce43b9040..d10e92ae5ca1 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -20,6 +20,7 @@
extern u64 sme_me_mask;
extern bool sev_enabled;
+extern bool sev_live_mig_enabled;
void sme_encrypt_execute(unsigned long encrypted_kernel_vaddr,
unsigned long decrypted_kernel_vaddr,
@@ -42,6 +43,8 @@ void __init sme_enable(struct boot_params *bp);
int __init early_set_memory_decrypted(unsigned long vaddr, unsigned long size);
int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size);
+void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages,
+ bool enc);
/* Architecture __weak replacement functions */
void __init mem_encrypt_init(void);
@@ -55,6 +58,7 @@ bool sev_active(void);
#else /* !CONFIG_AMD_MEM_ENCRYPT */
#define sme_me_mask 0ULL
+#define sev_live_mig_enabled false
static inline void __init sme_early_encrypt(resource_size_t paddr,
unsigned long size) { }
@@ -76,6 +80,8 @@ static inline int __init
early_set_memory_decrypted(unsigned long vaddr, unsigned long size) { return 0; }
static inline int __init
early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0; }
+static inline void __init
+early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages, bool enc) {}
#define __bss_decrypted
@@ -102,6 +108,11 @@ static inline u64 sme_get_me_mask(void)
return sme_me_mask;
}
+static inline bool sev_live_migration_enabled(void)
+{
+ return sev_live_mig_enabled;
+}
+
#endif /* __ASSEMBLY__ */
#endif /* __X86_MEM_ENCRYPT_H__ */
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 928ddb8a8cfc..8b8cc87a3461 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -24,6 +24,7 @@
#include <linux/debugfs.h>
#include <linux/nmi.h>
#include <linux/swait.h>
+#include <linux/efi.h>
#include <asm/timer.h>
#include <asm/cpu.h>
#include <asm/traps.h>
@@ -403,6 +404,53 @@ static inline void __set_percpu_decrypted(void *ptr, unsigned long size)
early_set_memory_decrypted((unsigned long) ptr, size);
}
+#ifdef CONFIG_EFI
+static bool setup_kvm_sev_migration(void)
+{
+ efi_char16_t efi_Sev_Live_Mig_support_name[] = L"SevLiveMigrationEnabled";
+ efi_guid_t efi_variable_guid = MEM_ENCRYPT_GUID;
+ efi_status_t status;
+ unsigned long size;
+ bool enabled;
+
+ if (!sev_live_migration_enabled())
+ return false;
+
+ size = sizeof(enabled);
+
+ if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+ pr_info("setup_kvm_sev_migration: no efi\n");
+ return false;
+ }
+
+ /* Get variable contents into buffer */
+ status = efi.get_variable(efi_Sev_Live_Mig_support_name,
+ &efi_variable_guid, NULL, &size, &enabled);
+
+ if (status == EFI_NOT_FOUND) {
+ pr_info("setup_kvm_sev_migration: variable not found\n");
+ return false;
+ }
+
+ if (status != EFI_SUCCESS) {
+ pr_info("setup_kvm_sev_migration: get_variable fail\n");
+ return false;
+ }
+
+ if (enabled == 0) {
+ pr_info("setup_kvm_sev_migration: live migration disabled in OVMF\n");
+ return false;
+ }
+
+ pr_info("setup_kvm_sev_migration: live migration enabled in OVMF\n");
+ wrmsrl(MSR_KVM_SEV_LIVE_MIG_EN, KVM_SEV_LIVE_MIGRATION_ENABLED);
+
+ return true;
+}
+
+late_initcall(setup_kvm_sev_migration);
+#endif
+
/*
* Iterate through all possible CPUs and map the memory region pointed
* by apf_reason, steal_time and kvm_apic_eoi as decrypted at once.
@@ -725,6 +773,20 @@ static void __init kvm_apic_init(void)
static void __init kvm_init_platform(void)
{
+ if (sev_active() &&
+ kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION)) {
+
+ printk(KERN_INFO "KVM enable live migration\n");
+ sev_live_mig_enabled = true;
+ /*
+ * If not booted using EFI, enable Live migration support.
+ */
+ if (!efi_enabled(EFI_BOOT))
+ wrmsrl(MSR_KVM_SEV_LIVE_MIG_EN,
+ KVM_SEV_LIVE_MIGRATION_ENABLED);
+ } else
+ printk(KERN_INFO "KVM enable live migration feature unsupported\n");
+
kvmclock_init();
x86_platform.apic_post_init = kvm_apic_init;
}
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 603f5abf8a78..3964f5de058c 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -46,6 +46,8 @@ EXPORT_SYMBOL_GPL(sev_enable_key);
bool sev_enabled __section(.data);
+bool sev_live_mig_enabled __section(.data);
+
/* Buffer used for early in-place encryption by BSP, no locking needed */
static char sme_early_buffer[PAGE_SIZE] __initdata __aligned(PAGE_SIZE);
@@ -204,6 +206,9 @@ static void set_memory_enc_dec_hypercall(unsigned long vaddr, int npages,
unsigned long sz = npages << PAGE_SHIFT;
unsigned long vaddr_end, vaddr_next;
+ if (!sev_live_migration_enabled())
+ return;
+
vaddr_end = vaddr + sz;
for (; vaddr < vaddr_end; vaddr = vaddr_next) {
@@ -375,6 +380,12 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
return early_set_memory_enc_dec(vaddr, size, true);
}
+void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages,
+ bool enc)
+{
+ set_memory_enc_dec_hypercall(vaddr, npages, enc);
+}
+
/*
* SME and SEV are very similar but they are not the same, so there are
* times that the kernel will need to distinguish between SME and SEV. The
--
2.17.1
From: Ashish Kalra <[email protected]>
Add new KVM_FEATURE_SEV_LIVE_MIGRATION feature for guest to check
for host-side support for SEV live migration. Also add a new custom
MSR_KVM_SEV_LIVE_MIG_EN for guest to enable the SEV live migration
feature.
Signed-off-by: Ashish Kalra <[email protected]>
---
Documentation/virt/kvm/cpuid.rst | 5 +++++
Documentation/virt/kvm/msr.rst | 10 ++++++++++
arch/x86/include/uapi/asm/kvm_para.h | 5 +++++
arch/x86/kvm/svm/sev.c | 14 ++++++++++++++
arch/x86/kvm/svm/svm.c | 16 ++++++++++++++++
arch/x86/kvm/svm/svm.h | 2 ++
6 files changed, 52 insertions(+)
diff --git a/Documentation/virt/kvm/cpuid.rst b/Documentation/virt/kvm/cpuid.rst
index 01b081f6e7ea..0514523e00cd 100644
--- a/Documentation/virt/kvm/cpuid.rst
+++ b/Documentation/virt/kvm/cpuid.rst
@@ -86,6 +86,11 @@ KVM_FEATURE_PV_SCHED_YIELD 13 guest checks this feature bit
before using paravirtualized
sched yield.
+KVM_FEATURE_SEV_LIVE_MIGRATION 14 guest checks this feature bit before
+ using the page encryption state
+ hypercall to notify the page state
+ change
+
KVM_FEATURE_CLOCSOURCE_STABLE_BIT 24 host will warn if no guest-side
per-cpu warps are expeced in
kvmclock
diff --git a/Documentation/virt/kvm/msr.rst b/Documentation/virt/kvm/msr.rst
index 33892036672d..7cd7786bbb03 100644
--- a/Documentation/virt/kvm/msr.rst
+++ b/Documentation/virt/kvm/msr.rst
@@ -319,3 +319,13 @@ data:
KVM guests can request the host not to poll on HLT, for example if
they are performing polling themselves.
+
+MSR_KVM_SEV_LIVE_MIG_EN:
+ 0x4b564d06
+
+ Control SEV Live Migration features.
+
+data:
+ Bit 0 enables (1) or disables (0) host-side SEV Live Migration feature.
+ Bit 1 enables (1) or disables (0) support for SEV Live Migration extensions.
+ All other bits are reserved.
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index 2a8e0b6b9805..d9d4953b42ad 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -31,6 +31,7 @@
#define KVM_FEATURE_PV_SEND_IPI 11
#define KVM_FEATURE_POLL_CONTROL 12
#define KVM_FEATURE_PV_SCHED_YIELD 13
+#define KVM_FEATURE_SEV_LIVE_MIGRATION 14
#define KVM_HINTS_REALTIME 0
@@ -50,6 +51,7 @@
#define MSR_KVM_STEAL_TIME 0x4b564d03
#define MSR_KVM_PV_EOI_EN 0x4b564d04
#define MSR_KVM_POLL_CONTROL 0x4b564d05
+#define MSR_KVM_SEV_LIVE_MIG_EN 0x4b564d06
struct kvm_steal_time {
__u64 steal;
@@ -122,4 +124,7 @@ struct kvm_vcpu_pv_apf_data {
#define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
#define KVM_PV_EOI_DISABLED 0x0
+#define KVM_SEV_LIVE_MIGRATION_ENABLED (1 << 0)
+#define KVM_SEV_LIVE_MIGRATION_EXTENSIONS_SUPPORTED (1 << 1)
+
#endif /* _UAPI_ASM_X86_KVM_PARA_H */
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index ba5ecd1de644..0ac82e4aac33 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1469,6 +1469,17 @@ int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
return 0;
}
+void sev_update_migration_flags(struct kvm *kvm, u64 data)
+{
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+
+ if (!sev_guest(kvm))
+ return;
+
+ if (data & KVM_SEV_LIVE_MIGRATION_ENABLED)
+ sev->live_migration_enabled = true;
+}
+
int svm_get_page_enc_bitmap(struct kvm *kvm,
struct kvm_page_enc_bitmap *bmap)
{
@@ -1481,6 +1492,9 @@ int svm_get_page_enc_bitmap(struct kvm *kvm,
if (!sev_guest(kvm))
return -ENOTTY;
+ if (!sev->live_migration_enabled)
+ return -EINVAL;
+
gfn_start = bmap->start_gfn;
gfn_end = gfn_start + bmap->num_pages;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 442adbbb0641..a99f5457f244 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2633,6 +2633,9 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
svm->msr_decfg = data;
break;
}
+ case MSR_KVM_SEV_LIVE_MIG_EN:
+ sev_update_migration_flags(vcpu->kvm, data);
+ break;
case MSR_IA32_APICBASE:
if (kvm_vcpu_apicv_active(vcpu))
avic_update_vapic_bar(to_svm(vcpu), data);
@@ -3493,6 +3496,19 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
svm->nrips_enabled = kvm_cpu_cap_has(X86_FEATURE_NRIPS) &&
guest_cpuid_has(&svm->vcpu, X86_FEATURE_NRIPS);
+ /*
+ * If SEV guest then enable the Live migration feature.
+ */
+ if (sev_guest(vcpu->kvm)) {
+ struct kvm_cpuid_entry2 *best;
+
+ best = kvm_find_cpuid_entry(vcpu, KVM_CPUID_FEATURES, 0);
+ if (!best)
+ return;
+
+ best->eax |= (1 << KVM_FEATURE_SEV_LIVE_MIGRATION);
+ }
+
if (!kvm_vcpu_apicv_active(vcpu))
return;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index fd99e0a5417a..77f132a6fead 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -65,6 +65,7 @@ struct kvm_sev_info {
int fd; /* SEV device fd */
unsigned long pages_locked; /* Number of pages locked */
struct list_head regions_list; /* List of registered regions */
+ bool live_migration_enabled;
unsigned long *page_enc_bmap;
unsigned long page_enc_bmap_size;
};
@@ -494,5 +495,6 @@ int svm_unregister_enc_region(struct kvm *kvm,
void pre_sev_run(struct vcpu_svm *svm, int cpu);
int __init sev_hardware_setup(void);
void sev_hardware_teardown(void);
+void sev_update_migration_flags(struct kvm *kvm, u64 data);
#endif
--
2.17.1
From: Ashish Kalra <[email protected]>
Reset the host's page encryption bitmap related to kernel
specific page encryption status settings before we load a
new kernel by kexec. We cannot reset the complete
page encryption bitmap here as we need to retain the
UEFI/OVMF firmware specific settings.
Signed-off-by: Ashish Kalra <[email protected]>
---
arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 8b8cc87a3461..68f2de4c1e74 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -35,6 +35,7 @@
#include <asm/hypervisor.h>
#include <asm/tlb.h>
#include <asm/cpuidle_haltpoll.h>
+#include <asm/e820/api.h>
static int kvmapf = 1;
@@ -358,6 +359,33 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
*/
if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
wrmsrl(MSR_KVM_PV_EOI_EN, 0);
+ /*
+ * Reset the host's page encryption bitmap related to kernel
+ * specific page encryption status settings before we load a
+ * new kernel by kexec. NOTE: We cannot reset the complete
+ * page encryption bitmap here as we need to retain the
+ * UEFI/OVMF firmware specific settings.
+ */
+ if (sev_live_migration_enabled() & (smp_processor_id() == 0)) {
+ int i;
+ unsigned long nr_pages;
+
+ for (i = 0; i < e820_table->nr_entries; i++) {
+ struct e820_entry *entry = &e820_table->entries[i];
+ unsigned long start_pfn;
+ unsigned long end_pfn;
+
+ if (entry->type != E820_TYPE_RAM)
+ continue;
+
+ start_pfn = entry->addr >> PAGE_SHIFT;
+ end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
+ nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
+
+ kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
+ entry->addr, nr_pages, 1);
+ }
+ }
kvm_pv_disable_apf();
kvm_disable_steal_time();
}
--
2.17.1
From: Ashish Kalra <[email protected]>
Introduce a new AMD Memory Encryption GUID which is currently
used for defining a new UEFI enviroment variable which indicates
UEFI/OVMF support for the SEV live migration feature. This variable
is setup when UEFI/OVMF detects host/hypervisor support for SEV
live migration and later this variable is read by the kernel using
EFI runtime services to verify if OVMF supports the live migration
feature.
Signed-off-by: Ashish Kalra <[email protected]>
---
include/linux/efi.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 251f1f783cdf..2efb42ccf3a8 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -358,6 +358,7 @@ void efi_native_runtime_setup(void);
/* OEM GUIDs */
#define DELLEMC_EFI_RCI2_TABLE_GUID EFI_GUID(0x2d9f28a2, 0xa886, 0x456a, 0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
+#define MEM_ENCRYPT_GUID EFI_GUID(0x0cf29b71, 0x9e51, 0x433a, 0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75)
typedef struct {
efi_guid_t guid;
--
2.17.1
From: Ashish Kalra <[email protected]>
Ensure that _bss_decrypted section variables such as hv_clock_boot and
wall_clock are marked as decrypted in the page encryption bitmap if
sev liv migration is supported.
Signed-off-by: Ashish Kalra <[email protected]>
---
arch/x86/kernel/kvmclock.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 34b18f6eeb2c..65777bf1218d 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -334,6 +334,18 @@ void __init kvmclock_init(void)
pr_info("kvm-clock: Using msrs %x and %x",
msr_kvm_system_time, msr_kvm_wall_clock);
+ if (sev_live_migration_enabled()) {
+ unsigned long nr_pages;
+ /*
+ * sizeof(hv_clock_boot) is already PAGE_SIZE aligned
+ */
+ early_set_mem_enc_dec_hypercall((unsigned long)hv_clock_boot,
+ 1, 0);
+ nr_pages = DIV_ROUND_UP(sizeof(wall_clock), PAGE_SIZE);
+ early_set_mem_enc_dec_hypercall((unsigned long)&wall_clock,
+ nr_pages, 0);
+ }
+
this_cpu_write(hv_clock_per_cpu, &hv_clock_boot[0]);
kvm_register_clock("primary cpu clock");
pvclock_set_pvti_cpu0_va(hv_clock_boot);
--
2.17.1
On 30.04.20 10:45, Ashish Kalra wrote:
> From: Brijesh Singh <[email protected]>
>
> Invoke a hypercall when a memory region is changed from encrypted ->
> decrypted and vice versa. Hypervisor needs to know the page encryption
> status during the guest migration.
>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Cc: "Radim Krčmář" <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Reviewed-by: Venu Busireddy <[email protected]>
> Signed-off-by: Brijesh Singh <[email protected]>
> Signed-off-by: Ashish Kalra <[email protected]>
> ---
> arch/x86/include/asm/paravirt.h | 10 +++++
> arch/x86/include/asm/paravirt_types.h | 2 +
> arch/x86/kernel/paravirt.c | 1 +
> arch/x86/mm/mem_encrypt.c | 58 ++++++++++++++++++++++++++-
> arch/x86/mm/pat/set_memory.c | 7 ++++
> 5 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
> index 694d8daf4983..8127b9c141bf 100644
> --- a/arch/x86/include/asm/paravirt.h
> +++ b/arch/x86/include/asm/paravirt.h
> @@ -78,6 +78,12 @@ static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
> PVOP_VCALL1(mmu.exit_mmap, mm);
> }
>
> +static inline void page_encryption_changed(unsigned long vaddr, int npages,
> + bool enc)
> +{
> + PVOP_VCALL3(mmu.page_encryption_changed, vaddr, npages, enc);
> +}
> +
> #ifdef CONFIG_PARAVIRT_XXL
> static inline void load_sp0(unsigned long sp0)
> {
> @@ -946,6 +952,10 @@ static inline void paravirt_arch_dup_mmap(struct mm_struct *oldmm,
> static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
> {
> }
> +
> +static inline void page_encryption_changed(unsigned long vaddr, int npages, bool enc)
> +{
> +}
> #endif
> #endif /* __ASSEMBLY__ */
> #endif /* _ASM_X86_PARAVIRT_H */
> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> index 732f62e04ddb..03bfd515c59c 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -215,6 +215,8 @@ struct pv_mmu_ops {
>
> /* Hook for intercepting the destruction of an mm_struct. */
> void (*exit_mmap)(struct mm_struct *mm);
> + void (*page_encryption_changed)(unsigned long vaddr, int npages,
> + bool enc);
>
> #ifdef CONFIG_PARAVIRT_XXL
> struct paravirt_callee_save read_cr2;
> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> index c131ba4e70ef..840c02b23aeb 100644
> --- a/arch/x86/kernel/paravirt.c
> +++ b/arch/x86/kernel/paravirt.c
> @@ -367,6 +367,7 @@ struct paravirt_patch_template pv_ops = {
> (void (*)(struct mmu_gather *, void *))tlb_remove_page,
>
> .mmu.exit_mmap = paravirt_nop,
> + .mmu.page_encryption_changed = paravirt_nop,
>
> #ifdef CONFIG_PARAVIRT_XXL
> .mmu.read_cr2 = __PV_IS_CALLEE_SAVE(native_read_cr2),
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index f4bd4b431ba1..603f5abf8a78 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -19,6 +19,7 @@
> #include <linux/kernel.h>
> #include <linux/bitops.h>
> #include <linux/dma-mapping.h>
> +#include <linux/kvm_para.h>
>
> #include <asm/tlbflush.h>
> #include <asm/fixmap.h>
> @@ -29,6 +30,7 @@
> #include <asm/processor-flags.h>
> #include <asm/msr.h>
> #include <asm/cmdline.h>
> +#include <asm/kvm_para.h>
>
> #include "mm_internal.h"
>
> @@ -196,6 +198,48 @@ void __init sme_early_init(void)
> swiotlb_force = SWIOTLB_FORCE;
> }
>
> +static void set_memory_enc_dec_hypercall(unsigned long vaddr, int npages,
> + bool enc)
> +{
> + unsigned long sz = npages << PAGE_SHIFT;
> + unsigned long vaddr_end, vaddr_next;
> +
> + vaddr_end = vaddr + sz;
> +
> + for (; vaddr < vaddr_end; vaddr = vaddr_next) {
> + int psize, pmask, level;
> + unsigned long pfn;
> + pte_t *kpte;
> +
> + kpte = lookup_address(vaddr, &level);
> + if (!kpte || pte_none(*kpte))
> + return;
> +
> + switch (level) {
> + case PG_LEVEL_4K:
> + pfn = pte_pfn(*kpte);
> + break;
> + case PG_LEVEL_2M:
> + pfn = pmd_pfn(*(pmd_t *)kpte);
> + break;
> + case PG_LEVEL_1G:
> + pfn = pud_pfn(*(pud_t *)kpte);
> + break;
> + default:
> + return;
> + }
> +
> + psize = page_level_size(level);
> + pmask = page_level_mask(level);
> +
> + if (x86_platform.hyper.sev_migration_hcall)
> + x86_platform.hyper.sev_migration_hcall(pfn << PAGE_SHIFT,
> + psize >> PAGE_SHIFT,
> + enc);
Why do you need two indirections? One via pv.mmu_ops and then another
via x86_platform.hyper? Isn't one enough?
And if x86_platform.hyper.sev_migration_hcall isn't set the whole loop
is basically a nop.
> + vaddr_next = (vaddr & pmask) + psize;
> + }
> +}
> +
> static void __init __set_clr_pte_enc(pte_t *kpte, int level, bool enc)
> {
> pgprot_t old_prot, new_prot;
> @@ -253,12 +297,13 @@ static void __init __set_clr_pte_enc(pte_t *kpte, int level, bool enc)
> static int __init early_set_memory_enc_dec(unsigned long vaddr,
> unsigned long size, bool enc)
> {
> - unsigned long vaddr_end, vaddr_next;
> + unsigned long vaddr_end, vaddr_next, start;
> unsigned long psize, pmask;
> int split_page_size_mask;
> int level, ret;
> pte_t *kpte;
>
> + start = vaddr;
> vaddr_next = vaddr;
> vaddr_end = vaddr + size;
>
> @@ -313,6 +358,8 @@ static int __init early_set_memory_enc_dec(unsigned long vaddr,
>
> ret = 0;
>
> + set_memory_enc_dec_hypercall(start, PAGE_ALIGN(size) >> PAGE_SHIFT,
> + enc);
> out:
> __flush_tlb_all();
> return ret;
> @@ -451,6 +498,15 @@ void __init mem_encrypt_init(void)
> if (sev_active())
> static_branch_enable(&sev_enable_key);
>
> +#ifdef CONFIG_PARAVIRT
> + /*
> + * With SEV, we need to make a hypercall when page encryption state is
> + * changed.
> + */
> + if (sev_active())
> + pv_ops.mmu.page_encryption_changed = set_memory_enc_dec_hypercall;
> +#endif
> +
> pr_info("AMD %s active\n",
> sev_active() ? "Secure Encrypted Virtualization (SEV)"
> : "Secure Memory Encryption (SME)");
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index 59eca6a94ce7..9aaf1b6f5a1b 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -27,6 +27,7 @@
> #include <asm/proto.h>
> #include <asm/memtype.h>
> #include <asm/set_memory.h>
> +#include <asm/paravirt.h>
>
> #include "../mm_internal.h"
>
> @@ -2003,6 +2004,12 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
> */
> cpa_flush(&cpa, 0);
>
> + /* Notify hypervisor that a given memory range is mapped encrypted
> + * or decrypted. The hypervisor will use this information during the
> + * VM migration.
> + */
> + page_encryption_changed(addr, numpages, enc);
Is this operation really so performance critical that a pv-op is
needed? Wouldn't a static key be sufficient here?
> +
> return ret;
> }
>
>
Juergen
On 4/30/20 4:49 AM, Jürgen Groß wrote:
> On 30.04.20 10:45, Ashish Kalra wrote:
>> From: Brijesh Singh <[email protected]>
>>
>> Invoke a hypercall when a memory region is changed from encrypted ->
>> decrypted and vice versa. Hypervisor needs to know the page encryption
>> status during the guest migration.
>>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: "H. Peter Anvin" <[email protected]>
>> Cc: Paolo Bonzini <[email protected]>
>> Cc: "Radim Krčmář" <[email protected]>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Borislav Petkov <[email protected]>
>> Cc: Tom Lendacky <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Reviewed-by: Venu Busireddy <[email protected]>
>> Signed-off-by: Brijesh Singh <[email protected]>
>> Signed-off-by: Ashish Kalra <[email protected]>
>> ---
>> arch/x86/include/asm/paravirt.h | 10 +++++
>> arch/x86/include/asm/paravirt_types.h | 2 +
>> arch/x86/kernel/paravirt.c | 1 +
>> arch/x86/mm/mem_encrypt.c | 58 ++++++++++++++++++++++++++-
>> arch/x86/mm/pat/set_memory.c | 7 ++++
>> 5 files changed, 77 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/paravirt.h
>> b/arch/x86/include/asm/paravirt.h
>> index 694d8daf4983..8127b9c141bf 100644
>> --- a/arch/x86/include/asm/paravirt.h
>> +++ b/arch/x86/include/asm/paravirt.h
>> @@ -78,6 +78,12 @@ static inline void paravirt_arch_exit_mmap(struct
>> mm_struct *mm)
>> PVOP_VCALL1(mmu.exit_mmap, mm);
>> }
>> +static inline void page_encryption_changed(unsigned long vaddr,
>> int npages,
>> + bool enc)
>> +{
>> + PVOP_VCALL3(mmu.page_encryption_changed, vaddr, npages, enc);
>> +}
>> +
>> #ifdef CONFIG_PARAVIRT_XXL
>> static inline void load_sp0(unsigned long sp0)
>> {
>> @@ -946,6 +952,10 @@ static inline void paravirt_arch_dup_mmap(struct
>> mm_struct *oldmm,
>> static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
>> {
>> }
>> +
>> +static inline void page_encryption_changed(unsigned long vaddr, int
>> npages, bool enc)
>> +{
>> +}
>> #endif
>> #endif /* __ASSEMBLY__ */
>> #endif /* _ASM_X86_PARAVIRT_H */
>> diff --git a/arch/x86/include/asm/paravirt_types.h
>> b/arch/x86/include/asm/paravirt_types.h
>> index 732f62e04ddb..03bfd515c59c 100644
>> --- a/arch/x86/include/asm/paravirt_types.h
>> +++ b/arch/x86/include/asm/paravirt_types.h
>> @@ -215,6 +215,8 @@ struct pv_mmu_ops {
>> /* Hook for intercepting the destruction of an mm_struct. */
>> void (*exit_mmap)(struct mm_struct *mm);
>> + void (*page_encryption_changed)(unsigned long vaddr, int npages,
>> + bool enc);
>> #ifdef CONFIG_PARAVIRT_XXL
>> struct paravirt_callee_save read_cr2;
>> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
>> index c131ba4e70ef..840c02b23aeb 100644
>> --- a/arch/x86/kernel/paravirt.c
>> +++ b/arch/x86/kernel/paravirt.c
>> @@ -367,6 +367,7 @@ struct paravirt_patch_template pv_ops = {
>> (void (*)(struct mmu_gather *, void *))tlb_remove_page,
>> .mmu.exit_mmap = paravirt_nop,
>> + .mmu.page_encryption_changed = paravirt_nop,
>> #ifdef CONFIG_PARAVIRT_XXL
>> .mmu.read_cr2 = __PV_IS_CALLEE_SAVE(native_read_cr2),
>> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
>> index f4bd4b431ba1..603f5abf8a78 100644
>> --- a/arch/x86/mm/mem_encrypt.c
>> +++ b/arch/x86/mm/mem_encrypt.c
>> @@ -19,6 +19,7 @@
>> #include <linux/kernel.h>
>> #include <linux/bitops.h>
>> #include <linux/dma-mapping.h>
>> +#include <linux/kvm_para.h>
>> #include <asm/tlbflush.h>
>> #include <asm/fixmap.h>
>> @@ -29,6 +30,7 @@
>> #include <asm/processor-flags.h>
>> #include <asm/msr.h>
>> #include <asm/cmdline.h>
>> +#include <asm/kvm_para.h>
>> #include "mm_internal.h"
>> @@ -196,6 +198,48 @@ void __init sme_early_init(void)
>> swiotlb_force = SWIOTLB_FORCE;
>> }
>> +static void set_memory_enc_dec_hypercall(unsigned long vaddr, int
>> npages,
>> + bool enc)
>> +{
>> + unsigned long sz = npages << PAGE_SHIFT;
>> + unsigned long vaddr_end, vaddr_next;
>> +
>> + vaddr_end = vaddr + sz;
>> +
>> + for (; vaddr < vaddr_end; vaddr = vaddr_next) {
>> + int psize, pmask, level;
>> + unsigned long pfn;
>> + pte_t *kpte;
>> +
>> + kpte = lookup_address(vaddr, &level);
>> + if (!kpte || pte_none(*kpte))
>> + return;
>> +
>> + switch (level) {
>> + case PG_LEVEL_4K:
>> + pfn = pte_pfn(*kpte);
>> + break;
>> + case PG_LEVEL_2M:
>> + pfn = pmd_pfn(*(pmd_t *)kpte);
>> + break;
>> + case PG_LEVEL_1G:
>> + pfn = pud_pfn(*(pud_t *)kpte);
>> + break;
>> + default:
>> + return;
>> + }
>> +
>> + psize = page_level_size(level);
>> + pmask = page_level_mask(level);
>> +
>> + if (x86_platform.hyper.sev_migration_hcall)
>> + x86_platform.hyper.sev_migration_hcall(pfn << PAGE_SHIFT,
>> + psize >> PAGE_SHIFT,
>> + enc);
>
> Why do you need two indirections? One via pv.mmu_ops and then another
> via x86_platform.hyper? Isn't one enough?
>
Currently, there is no strong reason to have two indirections other than
building a flexibility for the future expansion, e.g when we add SEV
support for the Xen then hypercall invocation may be slightly different
but the code to walk the page table to find the GPA will be same for
both KVM and Xen. The pv.mmu_ops provides a generic indirection which
can be used by set_memory_{decrypted,encrypted}. I will look into
removing the extra indirection in next version. thanks.
> And if x86_platform.hyper.sev_migration_hcall isn't set the whole loop
> is basically a nop.
>
Yes, this double indirection has a draw back that we will be executing
the unnecessary code when x86_platform.hyper.sev_migration_hcall isn't
set. I will look into improving it.
>> + vaddr_next = (vaddr & pmask) + psize;
>> + }
>> +}
>> +
>> static void __init __set_clr_pte_enc(pte_t *kpte, int level, bool enc)
>> {
>> pgprot_t old_prot, new_prot;
>> @@ -253,12 +297,13 @@ static void __init __set_clr_pte_enc(pte_t
>> *kpte, int level, bool enc)
>> static int __init early_set_memory_enc_dec(unsigned long vaddr,
>> unsigned long size, bool enc)
>> {
>> - unsigned long vaddr_end, vaddr_next;
>> + unsigned long vaddr_end, vaddr_next, start;
>> unsigned long psize, pmask;
>> int split_page_size_mask;
>> int level, ret;
>> pte_t *kpte;
>> + start = vaddr;
>> vaddr_next = vaddr;
>> vaddr_end = vaddr + size;
>> @@ -313,6 +358,8 @@ static int __init
>> early_set_memory_enc_dec(unsigned long vaddr,
>> ret = 0;
>> + set_memory_enc_dec_hypercall(start, PAGE_ALIGN(size) >>
>> PAGE_SHIFT,
>> + enc);
>> out:
>> __flush_tlb_all();
>> return ret;
>> @@ -451,6 +498,15 @@ void __init mem_encrypt_init(void)
>> if (sev_active())
>> static_branch_enable(&sev_enable_key);
>> +#ifdef CONFIG_PARAVIRT
>> + /*
>> + * With SEV, we need to make a hypercall when page encryption
>> state is
>> + * changed.
>> + */
>> + if (sev_active())
>> + pv_ops.mmu.page_encryption_changed =
>> set_memory_enc_dec_hypercall;
>> +#endif
>> +
>> pr_info("AMD %s active\n",
>> sev_active() ? "Secure Encrypted Virtualization (SEV)"
>> : "Secure Memory Encryption (SME)");
>> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
>> index 59eca6a94ce7..9aaf1b6f5a1b 100644
>> --- a/arch/x86/mm/pat/set_memory.c
>> +++ b/arch/x86/mm/pat/set_memory.c
>> @@ -27,6 +27,7 @@
>> #include <asm/proto.h>
>> #include <asm/memtype.h>
>> #include <asm/set_memory.h>
>> +#include <asm/paravirt.h>
>> #include "../mm_internal.h"
>> @@ -2003,6 +2004,12 @@ static int __set_memory_enc_dec(unsigned
>> long addr, int numpages, bool enc)
>> */
>> cpa_flush(&cpa, 0);
>> + /* Notify hypervisor that a given memory range is mapped
>> encrypted
>> + * or decrypted. The hypervisor will use this information during
>> the
>> + * VM migration.
>> + */
>> + page_encryption_changed(addr, numpages, enc);
>
> Is this operation really so performance critical that a pv-op is
> needed? Wouldn't a static key be sufficient here?
>
Well, in a typical Linux kernel boot it does not get called so many
times. We noticed that some drivers (mainly nvme) calls it more often
than others. I am open for the suggestions, we went with the pv-op path
based on the previous feedbacks. A static key maybe sufficient as well.
On 30.04.20 17:21, Brijesh Singh wrote:
>
> On 4/30/20 4:49 AM, Jürgen Groß wrote:
>> On 30.04.20 10:45, Ashish Kalra wrote:
>>> From: Brijesh Singh <[email protected]>
>>>
>>> Invoke a hypercall when a memory region is changed from encrypted ->
>>> decrypted and vice versa. Hypervisor needs to know the page encryption
>>> status during the guest migration.
>>>
>>> Cc: Thomas Gleixner <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: "H. Peter Anvin" <[email protected]>
>>> Cc: Paolo Bonzini <[email protected]>
>>> Cc: "Radim Krčmář" <[email protected]>
>>> Cc: Joerg Roedel <[email protected]>
>>> Cc: Borislav Petkov <[email protected]>
>>> Cc: Tom Lendacky <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Reviewed-by: Venu Busireddy <[email protected]>
>>> Signed-off-by: Brijesh Singh <[email protected]>
>>> Signed-off-by: Ashish Kalra <[email protected]>
>>> ---
>>> arch/x86/include/asm/paravirt.h | 10 +++++
>>> arch/x86/include/asm/paravirt_types.h | 2 +
>>> arch/x86/kernel/paravirt.c | 1 +
>>> arch/x86/mm/mem_encrypt.c | 58 ++++++++++++++++++++++++++-
>>> arch/x86/mm/pat/set_memory.c | 7 ++++
>>> 5 files changed, 77 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/include/asm/paravirt.h
>>> b/arch/x86/include/asm/paravirt.h
>>> index 694d8daf4983..8127b9c141bf 100644
>>> --- a/arch/x86/include/asm/paravirt.h
>>> +++ b/arch/x86/include/asm/paravirt.h
>>> @@ -78,6 +78,12 @@ static inline void paravirt_arch_exit_mmap(struct
>>> mm_struct *mm)
>>> PVOP_VCALL1(mmu.exit_mmap, mm);
>>> }
>>> +static inline void page_encryption_changed(unsigned long vaddr,
>>> int npages,
>>> + bool enc)
>>> +{
>>> + PVOP_VCALL3(mmu.page_encryption_changed, vaddr, npages, enc);
>>> +}
>>> +
>>> #ifdef CONFIG_PARAVIRT_XXL
>>> static inline void load_sp0(unsigned long sp0)
>>> {
>>> @@ -946,6 +952,10 @@ static inline void paravirt_arch_dup_mmap(struct
>>> mm_struct *oldmm,
>>> static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
>>> {
>>> }
>>> +
>>> +static inline void page_encryption_changed(unsigned long vaddr, int
>>> npages, bool enc)
>>> +{
>>> +}
>>> #endif
>>> #endif /* __ASSEMBLY__ */
>>> #endif /* _ASM_X86_PARAVIRT_H */
>>> diff --git a/arch/x86/include/asm/paravirt_types.h
>>> b/arch/x86/include/asm/paravirt_types.h
>>> index 732f62e04ddb..03bfd515c59c 100644
>>> --- a/arch/x86/include/asm/paravirt_types.h
>>> +++ b/arch/x86/include/asm/paravirt_types.h
>>> @@ -215,6 +215,8 @@ struct pv_mmu_ops {
>>> /* Hook for intercepting the destruction of an mm_struct. */
>>> void (*exit_mmap)(struct mm_struct *mm);
>>> + void (*page_encryption_changed)(unsigned long vaddr, int npages,
>>> + bool enc);
>>> #ifdef CONFIG_PARAVIRT_XXL
>>> struct paravirt_callee_save read_cr2;
>>> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
>>> index c131ba4e70ef..840c02b23aeb 100644
>>> --- a/arch/x86/kernel/paravirt.c
>>> +++ b/arch/x86/kernel/paravirt.c
>>> @@ -367,6 +367,7 @@ struct paravirt_patch_template pv_ops = {
>>> (void (*)(struct mmu_gather *, void *))tlb_remove_page,
>>> .mmu.exit_mmap = paravirt_nop,
>>> + .mmu.page_encryption_changed = paravirt_nop,
>>> #ifdef CONFIG_PARAVIRT_XXL
>>> .mmu.read_cr2 = __PV_IS_CALLEE_SAVE(native_read_cr2),
>>> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
>>> index f4bd4b431ba1..603f5abf8a78 100644
>>> --- a/arch/x86/mm/mem_encrypt.c
>>> +++ b/arch/x86/mm/mem_encrypt.c
>>> @@ -19,6 +19,7 @@
>>> #include <linux/kernel.h>
>>> #include <linux/bitops.h>
>>> #include <linux/dma-mapping.h>
>>> +#include <linux/kvm_para.h>
>>> #include <asm/tlbflush.h>
>>> #include <asm/fixmap.h>
>>> @@ -29,6 +30,7 @@
>>> #include <asm/processor-flags.h>
>>> #include <asm/msr.h>
>>> #include <asm/cmdline.h>
>>> +#include <asm/kvm_para.h>
>>> #include "mm_internal.h"
>>> @@ -196,6 +198,48 @@ void __init sme_early_init(void)
>>> swiotlb_force = SWIOTLB_FORCE;
>>> }
>>> +static void set_memory_enc_dec_hypercall(unsigned long vaddr, int
>>> npages,
>>> + bool enc)
>>> +{
>>> + unsigned long sz = npages << PAGE_SHIFT;
>>> + unsigned long vaddr_end, vaddr_next;
>>> +
>>> + vaddr_end = vaddr + sz;
>>> +
>>> + for (; vaddr < vaddr_end; vaddr = vaddr_next) {
>>> + int psize, pmask, level;
>>> + unsigned long pfn;
>>> + pte_t *kpte;
>>> +
>>> + kpte = lookup_address(vaddr, &level);
>>> + if (!kpte || pte_none(*kpte))
>>> + return;
>>> +
>>> + switch (level) {
>>> + case PG_LEVEL_4K:
>>> + pfn = pte_pfn(*kpte);
>>> + break;
>>> + case PG_LEVEL_2M:
>>> + pfn = pmd_pfn(*(pmd_t *)kpte);
>>> + break;
>>> + case PG_LEVEL_1G:
>>> + pfn = pud_pfn(*(pud_t *)kpte);
>>> + break;
>>> + default:
>>> + return;
>>> + }
>>> +
>>> + psize = page_level_size(level);
>>> + pmask = page_level_mask(level);
>>> +
>>> + if (x86_platform.hyper.sev_migration_hcall)
>>> + x86_platform.hyper.sev_migration_hcall(pfn << PAGE_SHIFT,
>>> + psize >> PAGE_SHIFT,
>>> + enc);
>>
>> Why do you need two indirections? One via pv.mmu_ops and then another
>> via x86_platform.hyper? Isn't one enough?
>>
> Currently, there is no strong reason to have two indirections other than
> building a flexibility for the future expansion, e.g when we add SEV
> support for the Xen then hypercall invocation may be slightly different
> but the code to walk the page table to find the GPA will be same for
> both KVM and Xen. The pv.mmu_ops provides a generic indirection which
> can be used by set_memory_{decrypted,encrypted}. I will look into
> removing the extra indirection in next version. thanks.
>
>
>> And if x86_platform.hyper.sev_migration_hcall isn't set the whole loop
>> is basically a nop.
>>
> Yes, this double indirection has a draw back that we will be executing
> the unnecessary code when x86_platform.hyper.sev_migration_hcall isn't
> set. I will look into improving it.
>
>
>>> + vaddr_next = (vaddr & pmask) + psize;
>>> + }
>>> +}
>>> +
>>> static void __init __set_clr_pte_enc(pte_t *kpte, int level, bool enc)
>>> {
>>> pgprot_t old_prot, new_prot;
>>> @@ -253,12 +297,13 @@ static void __init __set_clr_pte_enc(pte_t
>>> *kpte, int level, bool enc)
>>> static int __init early_set_memory_enc_dec(unsigned long vaddr,
>>> unsigned long size, bool enc)
>>> {
>>> - unsigned long vaddr_end, vaddr_next;
>>> + unsigned long vaddr_end, vaddr_next, start;
>>> unsigned long psize, pmask;
>>> int split_page_size_mask;
>>> int level, ret;
>>> pte_t *kpte;
>>> + start = vaddr;
>>> vaddr_next = vaddr;
>>> vaddr_end = vaddr + size;
>>> @@ -313,6 +358,8 @@ static int __init
>>> early_set_memory_enc_dec(unsigned long vaddr,
>>> ret = 0;
>>> + set_memory_enc_dec_hypercall(start, PAGE_ALIGN(size) >>
>>> PAGE_SHIFT,
>>> + enc);
>>> out:
>>> __flush_tlb_all();
>>> return ret;
>>> @@ -451,6 +498,15 @@ void __init mem_encrypt_init(void)
>>> if (sev_active())
>>> static_branch_enable(&sev_enable_key);
>>> +#ifdef CONFIG_PARAVIRT
>>> + /*
>>> + * With SEV, we need to make a hypercall when page encryption
>>> state is
>>> + * changed.
>>> + */
>>> + if (sev_active())
>>> + pv_ops.mmu.page_encryption_changed =
>>> set_memory_enc_dec_hypercall;
>>> +#endif
>>> +
>>> pr_info("AMD %s active\n",
>>> sev_active() ? "Secure Encrypted Virtualization (SEV)"
>>> : "Secure Memory Encryption (SME)");
>>> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
>>> index 59eca6a94ce7..9aaf1b6f5a1b 100644
>>> --- a/arch/x86/mm/pat/set_memory.c
>>> +++ b/arch/x86/mm/pat/set_memory.c
>>> @@ -27,6 +27,7 @@
>>> #include <asm/proto.h>
>>> #include <asm/memtype.h>
>>> #include <asm/set_memory.h>
>>> +#include <asm/paravirt.h>
>>> #include "../mm_internal.h"
>>> @@ -2003,6 +2004,12 @@ static int __set_memory_enc_dec(unsigned
>>> long addr, int numpages, bool enc)
>>> */
>>> cpa_flush(&cpa, 0);
>>> + /* Notify hypervisor that a given memory range is mapped
>>> encrypted
>>> + * or decrypted. The hypervisor will use this information during
>>> the
>>> + * VM migration.
>>> + */
>>> + page_encryption_changed(addr, numpages, enc);
>>
>> Is this operation really so performance critical that a pv-op is
>> needed? Wouldn't a static key be sufficient here?
>>
> Well, in a typical Linux kernel boot it does not get called so many
> times. We noticed that some drivers (mainly nvme) calls it more often
> than others. I am open for the suggestions, we went with the pv-op path
> based on the previous feedbacks. A static key maybe sufficient as well.
>
I'm fine with a pv-op if this is seen to be the way to go. But I would
skip the x86_platform.hyper indirection and call the kvm specific
function directly. In case Xen gains SEV capability we can still either
add the indirection again or just copy the KVM function and replace the
hypercall with the Xen variant.
Juergen
Hi Ashish,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v5.7-rc3]
[cannot apply to kvm/linux-next tip/x86/mm tip/x86/core next-20200430]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Ashish-Kalra/Add-AMD-SEV-guest-live-migration-support/20200430-202702
base: 6a8b55ed4056ea5559ebe4f6a4b247f627870d4c
config: i386-randconfig-a003-20200430 (attached as .config)
compiler: gcc-5 (Ubuntu 5.5.0-12ubuntu1) 5.5.0 20171010
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):
arch/x86/kernel/kvm.c: In function 'kvm_init_platform':
>> arch/x86/kernel/kvm.c:780:24: error: lvalue required as left operand of assignment
sev_live_mig_enabled = true;
^
vim +780 arch/x86/kernel/kvm.c
773
774 static void __init kvm_init_platform(void)
775 {
776 if (sev_active() &&
777 kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION)) {
778
779 printk(KERN_INFO "KVM enable live migration\n");
> 780 sev_live_mig_enabled = true;
781 /*
782 * If not booted using EFI, enable Live migration support.
783 */
784 if (!efi_enabled(EFI_BOOT))
785 wrmsrl(MSR_KVM_SEV_LIVE_MIG_EN,
786 KVM_SEV_LIVE_MIGRATION_ENABLED);
787 } else
788 printk(KERN_INFO "KVM enable live migration feature unsupported\n");
789
790 kvmclock_init();
791 x86_platform.apic_post_init = kvm_apic_init;
792 }
793
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Signed-off-by: kbuild test robot <[email protected]>
---
kvm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 928ddb8a8cfc7..884ba9203d3b4 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -730,8 +730,8 @@ static void __init kvm_init_platform(void)
}
#if defined(CONFIG_AMD_MEM_ENCRYPT)
-long kvm_sev_migration_hcall(unsigned long physaddr, unsigned long npages,
- bool enc)
+static long kvm_sev_migration_hcall(unsigned long physaddr, unsigned long npages,
+ bool enc)
{
return kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS, physaddr, npages,
enc);
Hi Ashish,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v5.7-rc3]
[cannot apply to kvm/linux-next tip/x86/mm tip/x86/core next-20200501]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Ashish-Kalra/Add-AMD-SEV-guest-live-migration-support/20200430-202702
base: 6a8b55ed4056ea5559ebe4f6a4b247f627870d4c
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-191-gc51a0382-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
>> arch/x86/kernel/kvm.c:733:6: sparse: sparse: symbol 'kvm_sev_migration_hcall' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On 2020-04-30 08:40:34 +0000, Ashish Kalra wrote:
> 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]
> Reviewed-by: Steve Rutherford <[email protected]>
> Reviewed-by: Venu Busireddy <[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/sev.c | 125 ++++++++++++++++++
> include/linux/psp-sev.h | 8 +-
> include/uapi/linux/kvm.h | 12 ++
> 4 files changed, 168 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
> index c3129b9ba5cb..4fd34fc5c7a7 100644
> --- a/Documentation/virt/kvm/amd-memory-encryption.rst
> +++ b/Documentation/virt/kvm/amd-memory-encryption.rst
> @@ -263,6 +263,33 @@ Returns: 0 on success, -negative on error
> __u32 trans_len;
> };
>
> +10. KVM_SEV_SEND_START
> +----------------------
> +
> +The KVM_SEV_SEND_START command can be used by the hypervisor to create an
> +outgoing guest encryption context.
> +
> +Parameters (in): struct kvm_sev_send_start
> +
> +Returns: 0 on success, -negative on error
> +
> +::
> + struct kvm_sev_send_start {
> + __u32 policy; /* guest policy */
> +
> + __u64 pdh_cert_uaddr; /* platform Diffie-Hellman certificate */
> + __u32 pdh_cert_len;
> +
> + __u64 plat_certs_uadr; /* platform certificate chain */
Can this be changed as mentioned in the previous review
(https://lore.kernel.org/kvm/20200402062726.GA647295@vbusired-dt/)?
> + __u32 plat_certs_len;
> +
> + __u64 amd_certs_uaddr; /* AMD certificate */
> + __u32 amd_cert_len;
Can this be changed as mentioned in the previous review
(https://lore.kernel.org/kvm/20200402062726.GA647295@vbusired-dt/)?
> +
> + __u64 session_uaddr; /* Guest session information */
> + __u32 session_len;
> + };
> +
> References
> ==========
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index cf912b4aaba8..5a15b43b4349 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -913,6 +913,128 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
> return ret;
> }
>
> +/* Userspace wants to query session length. */
> +static int
> +__sev_send_start_query_session_length(struct kvm *kvm, struct kvm_sev_cmd *argp,
> + struct kvm_sev_send_start *params)
> +{
> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> + struct sev_data_send_start *data;
> + int ret;
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
> + if (data == NULL)
> + return -ENOMEM;
> +
> + data->handle = sev->handle;
> + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
> +
> + params->session_len = data->session_len;
> + if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
> + sizeof(struct kvm_sev_send_start)))
> + ret = -EFAULT;
> +
> + kfree(data);
> + return ret;
> +}
> +
> +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> +{
> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> + struct sev_data_send_start *data;
> + struct kvm_sev_send_start params;
> + void *amd_certs, *session_data;
> + void *pdh_cert, *plat_certs;
> + int ret;
> +
> + if (!sev_guest(kvm))
> + return -ENOTTY;
> +
> + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
> + sizeof(struct kvm_sev_send_start)))
> + return -EFAULT;
> +
> + /* if session_len is zero, userspace wants to query the session length */
> + if (!params.session_len)
> + return __sev_send_start_query_session_length(kvm, argp,
> + ¶ms);
> +
> + /* some sanity checks */
> + if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
> + !params.session_uaddr || params.session_len > SEV_FW_BLOB_MAX_SIZE)
> + return -EINVAL;
> +
> + /* allocate the memory to hold the session data blob */
> + session_data = kmalloc(params.session_len, GFP_KERNEL_ACCOUNT);
> + if (!session_data)
> + return -ENOMEM;
> +
> + /* copy the certificate blobs from userspace */
> + pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr,
> + params.pdh_cert_len);
> + if (IS_ERR(pdh_cert)) {
> + ret = PTR_ERR(pdh_cert);
> + goto e_free_session;
> + }
> +
> + plat_certs = psp_copy_user_blob(params.plat_certs_uaddr,
> + params.plat_certs_len);
> + if (IS_ERR(plat_certs)) {
> + ret = PTR_ERR(plat_certs);
> + goto e_free_pdh;
> + }
> +
> + amd_certs = psp_copy_user_blob(params.amd_certs_uaddr,
> + params.amd_certs_len);
> + if (IS_ERR(amd_certs)) {
> + ret = PTR_ERR(amd_certs);
> + goto e_free_plat_cert;
> + }
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
> + if (data == NULL) {
> + ret = -ENOMEM;
> + goto e_free_amd_cert;
> + }
> +
> + /* populate the FW SEND_START field with system physical address */
> + data->pdh_cert_address = __psp_pa(pdh_cert);
> + data->pdh_cert_len = params.pdh_cert_len;
> + data->plat_certs_address = __psp_pa(plat_certs);
> + data->plat_certs_len = params.plat_certs_len;
> + data->amd_certs_address = __psp_pa(amd_certs);
> + data->amd_certs_len = params.amd_certs_len;
> + data->session_address = __psp_pa(session_data);
> + data->session_len = params.session_len;
> + data->handle = sev->handle;
> +
Can the following code be changed as acknowledged in
https://lore.kernel.org/kvm/[email protected]/?
> + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
> +
> + if (!ret && copy_to_user((void __user *)(uintptr_t)params.session_uaddr,
> + session_data, params.session_len)) {
> + ret = -EFAULT;
> + goto e_free;
> + }
> +
> + params.policy = data->policy;
> + params.session_len = data->session_len;
> + if (copy_to_user((void __user *)(uintptr_t)argp->data, ¶ms,
> + sizeof(struct kvm_sev_send_start)))
> + ret = -EFAULT;
> +
> +e_free:
> + kfree(data);
> +e_free_amd_cert:
> + kfree(amd_certs);
> +e_free_plat_cert:
> + kfree(plat_certs);
> +e_free_pdh:
> + kfree(pdh_cert);
> +e_free_session:
> + kfree(session_data);
> + return ret;
> +}
> +
> int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> {
> struct kvm_sev_cmd sev_cmd;
> @@ -957,6 +1079,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> case KVM_SEV_LAUNCH_SECRET:
> r = sev_launch_secret(kvm, &sev_cmd);
> break;
> + case KVM_SEV_SEND_START:
> + r = sev_send_start(kvm, &sev_cmd);
> + break;
> default:
> r = -EINVAL;
> goto out;
> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index 5167bf2bfc75..9f63b9d48b63 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -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 */
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 428c7dde6b4b..8827d43e2684 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1598,6 +1598,18 @@ struct kvm_sev_dbg {
> __u32 len;
> };
>
> +struct kvm_sev_send_start {
> + __u32 policy;
> + __u64 pdh_cert_uaddr;
> + __u32 pdh_cert_len;
> + __u64 plat_certs_uaddr;
> + __u32 plat_certs_len;
> + __u64 amd_certs_uaddr;
> + __u32 amd_certs_len;
> + __u64 session_uaddr;
> + __u32 session_len;
> +};
> +
> #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
> #define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
> --
> 2.17.1
>
On 2020-04-30 08:40:50 +0000, Ashish Kalra wrote:
> 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]
> Reviewed-by : Steve Rutherford <[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/sev.c | 135 +++++++++++++++++-
> include/uapi/linux/kvm.h | 9 ++
> 3 files changed, 164 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
> index 4fd34fc5c7a7..f46817ef7019 100644
> --- a/Documentation/virt/kvm/amd-memory-encryption.rst
> +++ b/Documentation/virt/kvm/amd-memory-encryption.rst
> @@ -290,6 +290,30 @@ Returns: 0 on success, -negative on error
> __u32 session_len;
> };
>
> +11. KVM_SEV_SEND_UPDATE_DATA
> +----------------------------
> +
> +The KVM_SEV_SEND_UPDATE_DATA command can be used by the hypervisor to encrypt the
> +outgoing guest memory region with the encryption context creating using
> +KVM_SEV_SEND_START.
> +
> +Parameters (in): struct kvm_sev_send_update_data
> +
> +Returns: 0 on success, -negative on error
> +
> +::
> +
> + struct kvm_sev_launch_send_update_data {
> + __u64 hdr_uaddr; /* userspace address containing the packet header */
> + __u32 hdr_len;
> +
> + __u64 guest_uaddr; /* the source memory region to be encrypted */
> + __u32 guest_len;
> +
> + __u64 trans_uaddr; /* the destition memory region */
> + __u32 trans_len;
> + };
> +
> References
> ==========
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 5a15b43b4349..0c92c16505ab 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -23,6 +23,7 @@ static DECLARE_RWSEM(sev_deactivate_lock);
> static DEFINE_MUTEX(sev_bitmap_lock);
> unsigned int max_sev_asid;
> static unsigned int min_sev_asid;
> +static unsigned long sev_me_mask;
> static unsigned long *sev_asid_bitmap;
> static unsigned long *sev_reclaim_asid_bitmap;
> #define __sme_page_pa(x) __sme_set(page_to_pfn(x) << PAGE_SHIFT)
> @@ -1035,6 +1036,123 @@ static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> return ret;
> }
>
> +/* Userspace wants to query either header or trans length. */
> +static int
> +__sev_send_update_data_query_lengths(struct kvm *kvm, struct kvm_sev_cmd *argp,
> + struct kvm_sev_send_update_data *params)
> +{
> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> + struct sev_data_send_update_data *data;
> + int ret;
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
> + if (!data)
> + return -ENOMEM;
> +
> + data->handle = sev->handle;
> + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_UPDATE_DATA, data, &argp->error);
> +
> + params->hdr_len = data->hdr_len;
> + params->trans_len = data->trans_len;
> +
> + if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
> + sizeof(struct kvm_sev_send_update_data)))
> + ret = -EFAULT;
> +
> + kfree(data);
> + return ret;
> +}
> +
> +static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
> +{
> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> + struct sev_data_send_update_data *data;
> + struct kvm_sev_send_update_data params;
> + void *hdr, *trans_data;
> + struct page **guest_page;
> + unsigned long n;
> + int ret, offset;
> +
> + if (!sev_guest(kvm))
> + return -ENOTTY;
> +
> + if (copy_from_user(¶ms, (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, ¶ms);
> +
> + 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;
Can you address my comment in v6 about this goto statement?
https://lore.kernel.org/kvm/20200402175529.GA655472@vbusired-dt/
> + }
> +
> + /* Copy packet header to userspace. */
> + ret = copy_to_user((void __user *)(uintptr_t)params.hdr_uaddr, hdr,
> + params.hdr_len);
> +
> +e_free:
> + kfree(data);
> +e_free_trans_data:
> + kfree(trans_data);
> +e_free_hdr:
> + kfree(hdr);
> +e_unpin:
> + sev_unpin_memory(kvm, guest_page, n);
> +
> + return ret;
> +}
> +
> int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> {
> struct kvm_sev_cmd sev_cmd;
> @@ -1082,6 +1200,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> case KVM_SEV_SEND_START:
> r = sev_send_start(kvm, &sev_cmd);
> break;
> + case KVM_SEV_SEND_UPDATE_DATA:
> + r = sev_send_update_data(kvm, &sev_cmd);
> + break;
> default:
> r = -EINVAL;
> goto out;
> @@ -1238,16 +1359,22 @@ void sev_vm_destroy(struct kvm *kvm)
> int __init sev_hardware_setup(void)
> {
> struct sev_user_data_status *status;
> + u32 eax, ebx;
> int rc;
>
> - /* Maximum number of encrypted guests supported simultaneously */
> - max_sev_asid = cpuid_ecx(0x8000001F);
> + /*
> + * Query the memory encryption information.
> + * EBX: Bit 0:5 Pagetable bit position used to indicate encryption
> + * (aka Cbit).
> + * ECX: Maximum number of encrypted guests supported simultaneously.
> + * EDX: Minimum ASID value that should be used for SEV guest.
> + */
> + cpuid(0x8000001f, &eax, &ebx, &max_sev_asid, &min_sev_asid);
>
> if (!svm_sev_enabled())
> return 1;
>
> - /* Minimum ASID value that should be used for SEV guest */
> - min_sev_asid = cpuid_edx(0x8000001F);
> + sev_me_mask = 1UL << (ebx & 0x3f);
>
> /* Initialize SEV ASID bitmaps */
> sev_asid_bitmap = bitmap_zalloc(max_sev_asid, GFP_KERNEL);
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 8827d43e2684..7aaed8ee33cf 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1610,6 +1610,15 @@ struct kvm_sev_send_start {
> __u32 session_len;
> };
>
> +struct kvm_sev_send_update_data {
> + __u64 hdr_uaddr;
> + __u32 hdr_len;
> + __u64 guest_uaddr;
> + __u32 guest_len;
> + __u64 trans_uaddr;
> + __u32 trans_len;
> +};
> +
> #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
> #define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
> --
> 2.17.1
>
On 2020-04-30 08:41:08 +0000, Ashish Kalra wrote:
> 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]
> Reviewed-by: Steve Rutherford <[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/sev.c | 23 +++++++++++++++++++
> 2 files changed, 31 insertions(+)
>
> diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
> index f46817ef7019..a45dcb5f8687 100644
> --- a/Documentation/virt/kvm/amd-memory-encryption.rst
> +++ b/Documentation/virt/kvm/amd-memory-encryption.rst
> @@ -314,6 +314,14 @@ Returns: 0 on success, -negative on error
As I commented in the v6 patches
(see https://lore.kernel.org/kvm/20200402181710.GA655710@vbusired-dt/),
there are multiple occurrences of the word "-negative" in
amd-memory-encryption.rst. Can these be changed to either "negative" or
"less than 0"?
> __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
> ==========
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 0c92c16505ab..81d661706d31 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1153,6 +1153,26 @@ static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
> return ret;
> }
>
> +static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
> +{
> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> + struct sev_data_send_finish *data;
> + int ret;
> +
> + if (!sev_guest(kvm))
> + return -ENOTTY;
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->handle = sev->handle;
> + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_FINISH, data, &argp->error);
> +
> + kfree(data);
> + return ret;
> +}
> +
> int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> {
> struct kvm_sev_cmd sev_cmd;
> @@ -1203,6 +1223,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> case KVM_SEV_SEND_UPDATE_DATA:
> r = sev_send_update_data(kvm, &sev_cmd);
> break;
> + case KVM_SEV_SEND_FINISH:
> + r = sev_send_finish(kvm, &sev_cmd);
> + break;
> default:
> r = -EINVAL;
> goto out;
> --
> 2.17.1
>
On 2020-04-30 08:42:02 +0000, Ashish Kalra wrote:
> 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]
> Reviewed-by: Steve Rutherford <[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/sev.c | 81 +++++++++++++++++++
> include/uapi/linux/kvm.h | 9 +++
> 3 files changed, 119 insertions(+)
>
> diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
> index a45dcb5f8687..ef1f1f3a5b40 100644
> --- a/Documentation/virt/kvm/amd-memory-encryption.rst
> +++ b/Documentation/virt/kvm/amd-memory-encryption.rst
> @@ -322,6 +322,35 @@ issued by the hypervisor to delete the encryption context.
>
> Returns: 0 on success, -negative on error
>
> +13. KVM_SEV_RECEIVE_START
> +------------------------
> +
> +The KVM_SEV_RECEIVE_START command is used for creating the memory encryption
> +context for an incoming SEV guest. To create the encryption context, the user must
> +provide a guest policy, the platform public Diffie-Hellman (PDH) key and session
> +information.
> +
> +Parameters: struct kvm_sev_receive_start (in/out)
> +
> +Returns: 0 on success, -negative on error
> +
> +::
> +
> + struct kvm_sev_receive_start {
> + __u32 handle; /* if zero then firmware creates a new handle */
> + __u32 policy; /* guest's policy */
> +
> + __u64 pdh_uaddr; /* userspace address pointing to the PDH key */
> + __u32 dh_len;
> +
> + __u64 session_addr; /* userspace address which points to the guest session information */
> + __u32 session_len;
Can you address my comments in v6?
(https://lore.kernel.org/kvm/20200402213520.GA658288@vbusired-dt/)
> + };
> +
> +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
> ==========
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 81d661706d31..74a847c9106d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1173,6 +1173,84 @@ static int sev_send_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
> return ret;
> }
>
> +static int sev_receive_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> +{
> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> + struct sev_data_receive_start *start;
> + struct kvm_sev_receive_start params;
> + int *error = &argp->error;
> + void *session_data;
> + void *pdh_data;
> + int ret;
> +
> + if (!sev_guest(kvm))
> + return -ENOTTY;
> +
> + /* Get parameter from the userspace */
> + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
> + sizeof(struct kvm_sev_receive_start)))
> + return -EFAULT;
> +
> + /* some sanity checks */
> + if (!params.pdh_uaddr || !params.pdh_len ||
> + !params.session_uaddr || !params.session_len)
> + return -EINVAL;
> +
> + pdh_data = psp_copy_user_blob(params.pdh_uaddr, params.pdh_len);
> + if (IS_ERR(pdh_data))
> + return PTR_ERR(pdh_data);
> +
> + session_data = psp_copy_user_blob(params.session_uaddr,
> + params.session_len);
> + if (IS_ERR(session_data)) {
> + ret = PTR_ERR(session_data);
> + goto e_free_pdh;
> + }
> +
> + ret = -ENOMEM;
> + start = kzalloc(sizeof(*start), GFP_KERNEL);
> + if (!start)
> + goto e_free_session;
> +
> + start->handle = params.handle;
> + start->policy = params.policy;
> + start->pdh_cert_address = __psp_pa(pdh_data);
> + start->pdh_cert_len = params.pdh_len;
> + start->session_address = __psp_pa(session_data);
> + start->session_len = params.session_len;
> +
> + /* create memory encryption context */
> + ret = __sev_issue_cmd(argp->sev_fd, SEV_CMD_RECEIVE_START, start,
> + error);
> + if (ret)
> + goto e_free;
> +
> + /* Bind ASID to this guest */
> + ret = sev_bind_asid(kvm, start->handle, error);
> + if (ret)
> + goto e_free;
> +
> + params.handle = start->handle;
> + if (copy_to_user((void __user *)(uintptr_t)argp->data,
> + ¶ms, sizeof(struct kvm_sev_receive_start))) {
> + ret = -EFAULT;
> + sev_unbind_asid(kvm, start->handle);
> + goto e_free;
> + }
> +
> + sev->handle = start->handle;
> + sev->fd = argp->sev_fd;
> +
> +e_free:
> + kfree(start);
> +e_free_session:
> + kfree(session_data);
> +e_free_pdh:
> + kfree(pdh_data);
> +
> + return ret;
> +}
> +
> int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> {
> struct kvm_sev_cmd sev_cmd;
> @@ -1226,6 +1304,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> case KVM_SEV_SEND_FINISH:
> r = sev_send_finish(kvm, &sev_cmd);
> break;
> + case KVM_SEV_RECEIVE_START:
> + r = sev_receive_start(kvm, &sev_cmd);
> + break;
> default:
> r = -EINVAL;
> goto out;
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 7aaed8ee33cf..24ac57151d53 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1619,6 +1619,15 @@ struct kvm_sev_send_update_data {
> __u32 trans_len;
> };
>
> +struct kvm_sev_receive_start {
> + __u32 handle;
> + __u32 policy;
> + __u64 pdh_uaddr;
> + __u32 pdh_len;
> + __u64 session_uaddr;
> + __u32 session_len;
> +};
> +
> #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
> #define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
> --
> 2.17.1
>
On 2020-04-30 08:42:37 +0000, Ashish Kalra wrote:
> From: Brijesh Singh <[email protected]>
>
> The command finalize the guest receiving process and make the SEV guest
> ready for the execution.
>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Cc: "Radim Krčmář" <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Reviewed-by: Steve Rutherford <[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/sev.c | 23 +++++++++++++++++++
> 2 files changed, 31 insertions(+)
>
> diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
> index 554aa33a99cc..93cd95d9a6c0 100644
> --- a/Documentation/virt/kvm/amd-memory-encryption.rst
> +++ b/Documentation/virt/kvm/amd-memory-encryption.rst
> @@ -375,6 +375,14 @@ Returns: 0 on success, -negative on error
> __u32 trans_len;
> };
>
> +15. KVM_SEV_RECEIVE_FINISH
> +------------------------
> +
> +After completion of the migration flow, the KVM_SEV_RECEIVE_FINISH command can be
> +issued by the hypervisor to make the guest ready for execution.
> +
> +Returns: 0 on success, -negative on error
> +
> References
> ==========
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index d5dfd0da53b9..1f9181e37ef0 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1327,6 +1327,26 @@ static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
> return ret;
> }
>
> +static int sev_receive_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
> +{
> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> + struct sev_data_receive_finish *data;
> + int ret;
> +
> + if (!sev_guest(kvm))
> + return -ENOTTY;
What is the rationale for using -ENOTTY? Is it the best return
value? Aren't one of -ENXIO, or -ENODEV, or -EINVAL a better choice?
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->handle = sev->handle;
> + ret = sev_issue_cmd(kvm, SEV_CMD_RECEIVE_FINISH, data, &argp->error);
> +
> + kfree(data);
> + return ret;
> +}
> +
> int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> {
> struct kvm_sev_cmd sev_cmd;
> @@ -1386,6 +1406,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> case KVM_SEV_RECEIVE_UPDATE_DATA:
> r = sev_receive_update_data(kvm, &sev_cmd);
> break;
> + case KVM_SEV_RECEIVE_FINISH:
> + r = sev_receive_finish(kvm, &sev_cmd);
> + break;
> default:
> r = -EINVAL;
> goto out;
> --
> 2.17.1
>
On 2020-04-30 08:43:40 +0000, Ashish Kalra wrote:
> From: Brijesh Singh <[email protected]>
>
> The ioctl can be used to retrieve page encryption bitmap for a given
> gfn range.
>
> Return the correct bitmap as per the number of pages being requested
> by the user. Ensure that we only copy bmap->num_pages bytes in the
> userspace buffer, if bmap->num_pages is not byte aligned we read
> the trailing bits from the userspace and copy those bits as is.
>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Cc: "Radim Krčmář" <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Reviewed-by: Venu Busireddy <[email protected]>
> Signed-off-by: Brijesh Singh <[email protected]>
> Signed-off-by: Ashish Kalra <[email protected]>
> ---
> Documentation/virt/kvm/api.rst | 27 +++++++++++++
> arch/x86/include/asm/kvm_host.h | 2 +
> arch/x86/kvm/svm/sev.c | 70 +++++++++++++++++++++++++++++++++
> arch/x86/kvm/svm/svm.c | 1 +
> arch/x86/kvm/svm/svm.h | 1 +
> arch/x86/kvm/x86.c | 12 ++++++
> include/uapi/linux/kvm.h | 12 ++++++
> 7 files changed, 125 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index efbbe570aa9b..e2f0dd105b5c 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -4636,6 +4636,33 @@ This ioctl resets VCPU registers and control structures according to
> the clear cpu reset definition in the POP. However, the cpu is not put
> into ESA mode. This reset is a superset of the initial reset.
>
> +4.125 KVM_GET_PAGE_ENC_BITMAP (vm ioctl)
> +---------------------------------------
> +
> +:Capability: basic
> +:Architectures: x86
> +:Type: vm ioctl
> +:Parameters: struct kvm_page_enc_bitmap (in/out)
> +:Returns: 0 on success, -1 on error
> +
> +/* for KVM_GET_PAGE_ENC_BITMAP */
> +struct kvm_page_enc_bitmap {
> + __u64 start_gfn;
> + __u64 num_pages;
> + union {
> + void __user *enc_bitmap; /* one bit per page */
> + __u64 padding2;
> + };
> +};
> +
> +The encrypted VMs have concept of private and shared pages. The private
> +page is encrypted with the guest-specific key, while shared page may
> +be encrypted with the hypervisor key. The KVM_GET_PAGE_ENC_BITMAP can
> +be used to get the bitmap indicating whether the guest page is private
> +or shared. The bitmap can be used during the guest migration, if the page
> +is private then userspace need to use SEV migration commands to transmit
> +the page.
Can you address the comments in the review of v6 patch?
(https://lore.kernel.org/kvm/20200403183046.GA727000@vbusired-dt/)
> +
>
> 4.125 KVM_S390_PV_COMMAND
> -------------------------
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 4a8ee22f4f5b..9e428befb6a4 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1256,6 +1256,8 @@ struct kvm_x86_ops {
> int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu);
> int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa,
> unsigned long sz, unsigned long mode);
> + int (*get_page_enc_bitmap)(struct kvm *kvm,
> + struct kvm_page_enc_bitmap *bmap);
> };
>
> struct kvm_x86_init_ops {
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 7dc68db70405..73bbbffb3487 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1434,6 +1434,76 @@ int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
> return 0;
> }
>
> +int svm_get_page_enc_bitmap(struct kvm *kvm,
> + struct kvm_page_enc_bitmap *bmap)
> +{
> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> + unsigned long gfn_start, gfn_end;
> + unsigned long sz, i, sz_bytes;
> + unsigned long *bitmap;
> + int ret, n;
> +
> + if (!sev_guest(kvm))
> + return -ENOTTY;
> +
> + gfn_start = bmap->start_gfn;
> + gfn_end = gfn_start + bmap->num_pages;
> +
> + sz = ALIGN(bmap->num_pages, BITS_PER_LONG) / BITS_PER_BYTE;
> + bitmap = kmalloc(sz, GFP_KERNEL);
> + if (!bitmap)
> + return -ENOMEM;
> +
> + /* by default all pages are marked encrypted */
> + memset(bitmap, 0xff, sz);
> +
> + mutex_lock(&kvm->lock);
> + if (sev->page_enc_bmap) {
> + i = gfn_start;
> + for_each_clear_bit_from(i, sev->page_enc_bmap,
> + min(sev->page_enc_bmap_size, gfn_end))
> + clear_bit(i - gfn_start, bitmap);
> + }
> + mutex_unlock(&kvm->lock);
> +
> + ret = -EFAULT;
> +
> + n = bmap->num_pages % BITS_PER_BYTE;
> + sz_bytes = ALIGN(bmap->num_pages, BITS_PER_BYTE) / BITS_PER_BYTE;
> +
> + /*
> + * Return the correct bitmap as per the number of pages being
> + * requested by the user. Ensure that we only copy bmap->num_pages
> + * bytes in the userspace buffer, if bmap->num_pages is not byte
> + * aligned we read the trailing bits from the userspace and copy
> + * those bits as is.
> + */
> +
> + if (n) {
> + unsigned char *bitmap_kernel = (unsigned char *)bitmap;
> + unsigned char bitmap_user;
> + unsigned long offset, mask;
> +
> + offset = bmap->num_pages / BITS_PER_BYTE;
> + if (copy_from_user(&bitmap_user, bmap->enc_bitmap + offset,
> + sizeof(unsigned char)))
> + goto out;
> +
> + mask = GENMASK(n - 1, 0);
> + bitmap_user &= ~mask;
> + bitmap_kernel[offset] &= mask;
> + bitmap_kernel[offset] |= bitmap_user;
> + }
> +
> + if (copy_to_user(bmap->enc_bitmap, bitmap, sz_bytes))
> + goto out;
> +
> + ret = 0;
> +out:
> + kfree(bitmap);
> + return ret;
> +}
> +
> int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> {
> struct kvm_sev_cmd sev_cmd;
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 1013ef0f4ce2..588709a9f68e 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4016,6 +4016,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
> .check_nested_events = svm_check_nested_events,
>
> .page_enc_status_hc = svm_page_enc_status_hc,
> + .get_page_enc_bitmap = svm_get_page_enc_bitmap,
> };
>
> static struct kvm_x86_init_ops svm_init_ops __initdata = {
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 6a562f5928a2..f087fa7b380c 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -404,6 +404,7 @@ int svm_check_nested_events(struct kvm_vcpu *vcpu);
> int nested_svm_exit_special(struct vcpu_svm *svm);
> int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa,
> unsigned long npages, unsigned long enc);
> +int svm_get_page_enc_bitmap(struct kvm *kvm, struct kvm_page_enc_bitmap *bmap);
>
> /* avic.c */
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5f5ddb5765e2..937797cfaf9a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5208,6 +5208,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
> case KVM_SET_PMU_EVENT_FILTER:
> r = kvm_vm_ioctl_set_pmu_event_filter(kvm, argp);
> break;
> + case KVM_GET_PAGE_ENC_BITMAP: {
> + struct kvm_page_enc_bitmap bitmap;
> +
> + r = -EFAULT;
> + if (copy_from_user(&bitmap, argp, sizeof(bitmap)))
> + goto out;
> +
> + r = -ENOTTY;
> + if (kvm_x86_ops.get_page_enc_bitmap)
> + r = kvm_x86_ops.get_page_enc_bitmap(kvm, &bitmap);
> + break;
> + }
> default:
> r = -ENOTTY;
> }
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 0fe1d206d750..af62f2afaa5d 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -505,6 +505,16 @@ struct kvm_dirty_log {
> };
> };
>
> +/* for KVM_GET_PAGE_ENC_BITMAP */
> +struct kvm_page_enc_bitmap {
> + __u64 start_gfn;
> + __u64 num_pages;
> + union {
> + void __user *enc_bitmap; /* one bit per page */
> + __u64 padding2;
> + };
> +};
> +
> /* for KVM_CLEAR_DIRTY_LOG */
> struct kvm_clear_dirty_log {
> __u32 slot;
> @@ -1518,6 +1528,8 @@ struct kvm_pv_cmd {
> /* Available with KVM_CAP_S390_PROTECTED */
> #define KVM_S390_PV_COMMAND _IOWR(KVMIO, 0xc5, struct kvm_pv_cmd)
>
> +#define KVM_GET_PAGE_ENC_BITMAP _IOW(KVMIO, 0xc6, struct kvm_page_enc_bitmap)
> +
> /* Secure Encrypted Virtualization command */
> enum sev_cmd_id {
> /* Guest initialization commands */
> --
> 2.17.1
>
On Mon, May 04, 2020 at 04:07:17PM -0500, Venu Busireddy wrote:
> On 2020-04-30 08:40:34 +0000, Ashish Kalra wrote:
> > 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]
> > Reviewed-by: Steve Rutherford <[email protected]>
> > Reviewed-by: Venu Busireddy <[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/sev.c | 125 ++++++++++++++++++
> > include/linux/psp-sev.h | 8 +-
> > include/uapi/linux/kvm.h | 12 ++
> > 4 files changed, 168 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
> > index c3129b9ba5cb..4fd34fc5c7a7 100644
> > --- a/Documentation/virt/kvm/amd-memory-encryption.rst
> > +++ b/Documentation/virt/kvm/amd-memory-encryption.rst
> > @@ -263,6 +263,33 @@ Returns: 0 on success, -negative on error
> > __u32 trans_len;
> > };
> >
> > +10. KVM_SEV_SEND_START
> > +----------------------
> > +
> > +The KVM_SEV_SEND_START command can be used by the hypervisor to create an
> > +outgoing guest encryption context.
> > +
> > +Parameters (in): struct kvm_sev_send_start
> > +
> > +Returns: 0 on success, -negative on error
> > +
> > +::
> > + struct kvm_sev_send_start {
> > + __u32 policy; /* guest policy */
> > +
> > + __u64 pdh_cert_uaddr; /* platform Diffie-Hellman certificate */
> > + __u32 pdh_cert_len;
> > +
> > + __u64 plat_certs_uadr; /* platform certificate chain */
>
> Can this be changed as mentioned in the previous review
> (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846313842&sdata=qfxRdCY3A1Tox%2FMI%2FQLmUcvIxbfL%2BwoR2fzfQa1FVkA%3D&reserved=0)?
>
> > + __u32 plat_certs_len;
> > +
> > + __u64 amd_certs_uaddr; /* AMD certificate */
> > + __u32 amd_cert_len;
>
> Can this be changed as mentioned in the previous review
> (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846323835&sdata=SMYG1m%2BT2KwNQ4Jed%2BJhsK6TQ7EYTKT16moEoZMTf7c%3D&reserved=0)?
>
> > +
> > + __u64 session_uaddr; /* Guest session information */
> > + __u32 session_len;
> > + };
> > +
> > References
> > ==========
> >
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index cf912b4aaba8..5a15b43b4349 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -913,6 +913,128 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > return ret;
> > }
> >
> > +/* Userspace wants to query session length. */
> > +static int
> > +__sev_send_start_query_session_length(struct kvm *kvm, struct kvm_sev_cmd *argp,
> > + struct kvm_sev_send_start *params)
> > +{
> > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> > + struct sev_data_send_start *data;
> > + int ret;
> > +
> > + data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
> > + if (data == NULL)
> > + return -ENOMEM;
> > +
> > + data->handle = sev->handle;
> > + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
> > +
> > + params->session_len = data->session_len;
> > + if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
> > + sizeof(struct kvm_sev_send_start)))
> > + ret = -EFAULT;
> > +
> > + kfree(data);
> > + return ret;
> > +}
> > +
> > +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > +{
> > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> > + struct sev_data_send_start *data;
> > + struct kvm_sev_send_start params;
> > + void *amd_certs, *session_data;
> > + void *pdh_cert, *plat_certs;
> > + int ret;
> > +
> > + if (!sev_guest(kvm))
> > + return -ENOTTY;
> > +
> > + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
> > + sizeof(struct kvm_sev_send_start)))
> > + return -EFAULT;
> > +
> > + /* if session_len is zero, userspace wants to query the session length */
> > + if (!params.session_len)
> > + return __sev_send_start_query_session_length(kvm, argp,
> > + ¶ms);
> > +
> > + /* some sanity checks */
> > + if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
> > + !params.session_uaddr || params.session_len > SEV_FW_BLOB_MAX_SIZE)
> > + return -EINVAL;
> > +
> > + /* allocate the memory to hold the session data blob */
> > + session_data = kmalloc(params.session_len, GFP_KERNEL_ACCOUNT);
> > + if (!session_data)
> > + return -ENOMEM;
> > +
> > + /* copy the certificate blobs from userspace */
> > + pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr,
> > + params.pdh_cert_len);
> > + if (IS_ERR(pdh_cert)) {
> > + ret = PTR_ERR(pdh_cert);
> > + goto e_free_session;
> > + }
> > +
> > + plat_certs = psp_copy_user_blob(params.plat_certs_uaddr,
> > + params.plat_certs_len);
> > + if (IS_ERR(plat_certs)) {
> > + ret = PTR_ERR(plat_certs);
> > + goto e_free_pdh;
> > + }
> > +
> > + amd_certs = psp_copy_user_blob(params.amd_certs_uaddr,
> > + params.amd_certs_len);
> > + if (IS_ERR(amd_certs)) {
> > + ret = PTR_ERR(amd_certs);
> > + goto e_free_plat_cert;
> > + }
> > +
> > + data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
> > + if (data == NULL) {
> > + ret = -ENOMEM;
> > + goto e_free_amd_cert;
> > + }
> > +
> > + /* populate the FW SEND_START field with system physical address */
> > + data->pdh_cert_address = __psp_pa(pdh_cert);
> > + data->pdh_cert_len = params.pdh_cert_len;
> > + data->plat_certs_address = __psp_pa(plat_certs);
> > + data->plat_certs_len = params.plat_certs_len;
> > + data->amd_certs_address = __psp_pa(amd_certs);
> > + data->amd_certs_len = params.amd_certs_len;
> > + data->session_address = __psp_pa(session_data);
> > + data->session_len = params.session_len;
> > + data->handle = sev->handle;
> > +
>
> Can the following code be changed as acknowledged in
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2Ff715bf99-0158-4d5f-77f3-b27743db3c59%40amd.com%2F&data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846323835&sdata=5hbjsP%2Btxt2rdv2PtIc%2BV8cAwKUNsRdtiRglDupYXzs%3D&reserved=0?
>
I believe that this has been already addressed as discussed :
Ah, so the main issue is we should not be going to e_free on error. If
session_len is less than the expected len then FW will return an error.
In the case of an error we can skip copying the session_data into
userspace buffer but we still need to pass the session_len and policy
back to the userspace.
So this patch is still returning session_len and policy back to user
in case of error : ( as the code below shows )
if (!ret && copy_to_user((void
__user*)(uintptr_t)params.session_uaddr,...
Thanks,
Ashish
> > + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
> > +
> > + if (!ret && copy_to_user((void __user *)(uintptr_t)params.session_uaddr,
> > + session_data, params.session_len)) {
> > + ret = -EFAULT;
> > + goto e_free;
> > + }
> > +
> > + params.policy = data->policy;
> > + params.session_len = data->session_len;
> > + if (copy_to_user((void __user *)(uintptr_t)argp->data, ¶ms,
> > + sizeof(struct kvm_sev_send_start)))
> > + ret = -EFAULT;
> > +
> > +e_free:
> > + kfree(data);
> > +e_free_amd_cert:
> > + kfree(amd_certs);
> > +e_free_plat_cert:
> > + kfree(plat_certs);
> > +e_free_pdh:
> > + kfree(pdh_cert);
> > +e_free_session:
> > + kfree(session_data);
> > + return ret;
> > +}
> > +
> > int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> > {
> > struct kvm_sev_cmd sev_cmd;
> > @@ -957,6 +1079,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> > case KVM_SEV_LAUNCH_SECRET:
> > r = sev_launch_secret(kvm, &sev_cmd);
> > break;
> > + case KVM_SEV_SEND_START:
> > + r = sev_send_start(kvm, &sev_cmd);
> > + break;
> > default:
> > r = -EINVAL;
> > goto out;
> > diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> > index 5167bf2bfc75..9f63b9d48b63 100644
> > --- a/include/linux/psp-sev.h
> > +++ b/include/linux/psp-sev.h
> > @@ -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 */
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 428c7dde6b4b..8827d43e2684 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -1598,6 +1598,18 @@ struct kvm_sev_dbg {
> > __u32 len;
> > };
> >
> > +struct kvm_sev_send_start {
> > + __u32 policy;
> > + __u64 pdh_cert_uaddr;
> > + __u32 pdh_cert_len;
> > + __u64 plat_certs_uaddr;
> > + __u32 plat_certs_len;
> > + __u64 amd_certs_uaddr;
> > + __u32 amd_certs_len;
> > + __u64 session_uaddr;
> > + __u32 session_len;
> > +};
> > +
> > #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> > #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
> > #define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
> > --
> > 2.17.1
> >
On 2020-05-04 22:36:37 +0000, Ashish Kalra wrote:
> On Mon, May 04, 2020 at 04:07:17PM -0500, Venu Busireddy wrote:
> > On 2020-04-30 08:40:34 +0000, Ashish Kalra wrote:
> > > 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]
> > > Reviewed-by: Steve Rutherford <[email protected]>
> > > Reviewed-by: Venu Busireddy <[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/sev.c | 125 ++++++++++++++++++
> > > include/linux/psp-sev.h | 8 +-
> > > include/uapi/linux/kvm.h | 12 ++
> > > 4 files changed, 168 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
> > > index c3129b9ba5cb..4fd34fc5c7a7 100644
> > > --- a/Documentation/virt/kvm/amd-memory-encryption.rst
> > > +++ b/Documentation/virt/kvm/amd-memory-encryption.rst
> > > @@ -263,6 +263,33 @@ Returns: 0 on success, -negative on error
> > > __u32 trans_len;
> > > };
> > >
> > > +10. KVM_SEV_SEND_START
> > > +----------------------
> > > +
> > > +The KVM_SEV_SEND_START command can be used by the hypervisor to create an
> > > +outgoing guest encryption context.
> > > +
> > > +Parameters (in): struct kvm_sev_send_start
> > > +
> > > +Returns: 0 on success, -negative on error
> > > +
> > > +::
> > > + struct kvm_sev_send_start {
> > > + __u32 policy; /* guest policy */
> > > +
> > > + __u64 pdh_cert_uaddr; /* platform Diffie-Hellman certificate */
> > > + __u32 pdh_cert_len;
> > > +
> > > + __u64 plat_certs_uadr; /* platform certificate chain */
> >
> > Can this be changed as mentioned in the previous review
> > (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846313842&sdata=qfxRdCY3A1Tox%2FMI%2FQLmUcvIxbfL%2BwoR2fzfQa1FVkA%3D&reserved=0)?
> >
> > > + __u32 plat_certs_len;
> > > +
> > > + __u64 amd_certs_uaddr; /* AMD certificate */
> > > + __u32 amd_cert_len;
> >
> > Can this be changed as mentioned in the previous review
> > (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846323835&sdata=SMYG1m%2BT2KwNQ4Jed%2BJhsK6TQ7EYTKT16moEoZMTf7c%3D&reserved=0)?
> >
> > > +
> > > + __u64 session_uaddr; /* Guest session information */
> > > + __u32 session_len;
> > > + };
> > > +
> > > References
> > > ==========
> > >
> > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > index cf912b4aaba8..5a15b43b4349 100644
> > > --- a/arch/x86/kvm/svm/sev.c
> > > +++ b/arch/x86/kvm/svm/sev.c
> > > @@ -913,6 +913,128 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > > return ret;
> > > }
> > >
> > > +/* Userspace wants to query session length. */
> > > +static int
> > > +__sev_send_start_query_session_length(struct kvm *kvm, struct kvm_sev_cmd *argp,
> > > + struct kvm_sev_send_start *params)
> > > +{
> > > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> > > + struct sev_data_send_start *data;
> > > + int ret;
> > > +
> > > + data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
> > > + if (data == NULL)
> > > + return -ENOMEM;
> > > +
> > > + data->handle = sev->handle;
> > > + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
> > > +
> > > + params->session_len = data->session_len;
> > > + if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
> > > + sizeof(struct kvm_sev_send_start)))
> > > + ret = -EFAULT;
> > > +
> > > + kfree(data);
> > > + return ret;
> > > +}
> > > +
> > > +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > > +{
> > > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> > > + struct sev_data_send_start *data;
> > > + struct kvm_sev_send_start params;
> > > + void *amd_certs, *session_data;
> > > + void *pdh_cert, *plat_certs;
> > > + int ret;
> > > +
> > > + if (!sev_guest(kvm))
> > > + return -ENOTTY;
> > > +
> > > + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
> > > + sizeof(struct kvm_sev_send_start)))
> > > + return -EFAULT;
> > > +
> > > + /* if session_len is zero, userspace wants to query the session length */
> > > + if (!params.session_len)
> > > + return __sev_send_start_query_session_length(kvm, argp,
> > > + ¶ms);
> > > +
> > > + /* some sanity checks */
> > > + if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
> > > + !params.session_uaddr || params.session_len > SEV_FW_BLOB_MAX_SIZE)
> > > + return -EINVAL;
> > > +
> > > + /* allocate the memory to hold the session data blob */
> > > + session_data = kmalloc(params.session_len, GFP_KERNEL_ACCOUNT);
> > > + if (!session_data)
> > > + return -ENOMEM;
> > > +
> > > + /* copy the certificate blobs from userspace */
> > > + pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr,
> > > + params.pdh_cert_len);
> > > + if (IS_ERR(pdh_cert)) {
> > > + ret = PTR_ERR(pdh_cert);
> > > + goto e_free_session;
> > > + }
> > > +
> > > + plat_certs = psp_copy_user_blob(params.plat_certs_uaddr,
> > > + params.plat_certs_len);
> > > + if (IS_ERR(plat_certs)) {
> > > + ret = PTR_ERR(plat_certs);
> > > + goto e_free_pdh;
> > > + }
> > > +
> > > + amd_certs = psp_copy_user_blob(params.amd_certs_uaddr,
> > > + params.amd_certs_len);
> > > + if (IS_ERR(amd_certs)) {
> > > + ret = PTR_ERR(amd_certs);
> > > + goto e_free_plat_cert;
> > > + }
> > > +
> > > + data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
> > > + if (data == NULL) {
> > > + ret = -ENOMEM;
> > > + goto e_free_amd_cert;
> > > + }
> > > +
> > > + /* populate the FW SEND_START field with system physical address */
> > > + data->pdh_cert_address = __psp_pa(pdh_cert);
> > > + data->pdh_cert_len = params.pdh_cert_len;
> > > + data->plat_certs_address = __psp_pa(plat_certs);
> > > + data->plat_certs_len = params.plat_certs_len;
> > > + data->amd_certs_address = __psp_pa(amd_certs);
> > > + data->amd_certs_len = params.amd_certs_len;
> > > + data->session_address = __psp_pa(session_data);
> > > + data->session_len = params.session_len;
> > > + data->handle = sev->handle;
> > > +
> >
> > Can the following code be changed as acknowledged in
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2Ff715bf99-0158-4d5f-77f3-b27743db3c59%40amd.com%2F&data=02%7C01%7CAshish.Kalra%40amd.com%7C5ea8d5ae78814a01618908d7f06f7667%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242233846323835&sdata=5hbjsP%2Btxt2rdv2PtIc%2BV8cAwKUNsRdtiRglDupYXzs%3D&reserved=0?
> >
>
> I believe that this has been already addressed as discussed :
>
> Ah, so the main issue is we should not be going to e_free on error. If
> session_len is less than the expected len then FW will return an error.
> In the case of an error we can skip copying the session_data into
> userspace buffer but we still need to pass the session_len and policy
> back to the userspace.
>
> So this patch is still returning session_len and policy back to user
> in case of error : ( as the code below shows )
>
> if (!ret && copy_to_user((void
> __user*)(uintptr_t)params.session_uaddr,...
This fix addresses only one part of the problem. I am referring to the
other suggestion about avoiding copying the entire kvm_sev_send_start
structure back to the user. As I was mentioning in the discussion,
the only fields that changed are the policy and session_len fields. So,
why copy back the entire structure? Why not just those two fields?
>
> Thanks,
> Ashish
>
> > > + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
> > > +
> > > + if (!ret && copy_to_user((void __user *)(uintptr_t)params.session_uaddr,
> > > + session_data, params.session_len)) {
> > > + ret = -EFAULT;
> > > + goto e_free;
> > > + }
> > > +
> > > + params.policy = data->policy;
> > > + params.session_len = data->session_len;
> > > + if (copy_to_user((void __user *)(uintptr_t)argp->data, ¶ms,
> > > + sizeof(struct kvm_sev_send_start)))
> > > + ret = -EFAULT;
> > > +
> > > +e_free:
> > > + kfree(data);
> > > +e_free_amd_cert:
> > > + kfree(amd_certs);
> > > +e_free_plat_cert:
> > > + kfree(plat_certs);
> > > +e_free_pdh:
> > > + kfree(pdh_cert);
> > > +e_free_session:
> > > + kfree(session_data);
> > > + return ret;
> > > +}
> > > +
> > > int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> > > {
> > > struct kvm_sev_cmd sev_cmd;
> > > @@ -957,6 +1079,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> > > case KVM_SEV_LAUNCH_SECRET:
> > > r = sev_launch_secret(kvm, &sev_cmd);
> > > break;
> > > + case KVM_SEV_SEND_START:
> > > + r = sev_send_start(kvm, &sev_cmd);
> > > + break;
> > > default:
> > > r = -EINVAL;
> > > goto out;
> > > diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> > > index 5167bf2bfc75..9f63b9d48b63 100644
> > > --- a/include/linux/psp-sev.h
> > > +++ b/include/linux/psp-sev.h
> > > @@ -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 */
> > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > > index 428c7dde6b4b..8827d43e2684 100644
> > > --- a/include/uapi/linux/kvm.h
> > > +++ b/include/uapi/linux/kvm.h
> > > @@ -1598,6 +1598,18 @@ struct kvm_sev_dbg {
> > > __u32 len;
> > > };
> > >
> > > +struct kvm_sev_send_start {
> > > + __u32 policy;
> > > + __u64 pdh_cert_uaddr;
> > > + __u32 pdh_cert_len;
> > > + __u64 plat_certs_uaddr;
> > > + __u32 plat_certs_len;
> > > + __u64 amd_certs_uaddr;
> > > + __u32 amd_certs_len;
> > > + __u64 session_uaddr;
> > > + __u32 session_len;
> > > +};
> > > +
> > > #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> > > #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
> > > #define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
> > > --
> > > 2.17.1
> > >
On Mon, May 04, 2020 at 06:10:50PM -0500, Venu Busireddy wrote:
> On 2020-05-04 22:36:37 +0000, Ashish Kalra wrote:
> > On Mon, May 04, 2020 at 04:07:17PM -0500, Venu Busireddy wrote:
> > > On 2020-04-30 08:40:34 +0000, Ashish Kalra wrote:
> > > > 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]
> > > > Reviewed-by: Steve Rutherford <[email protected]>
> > > > Reviewed-by: Venu Busireddy <[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/sev.c | 125 ++++++++++++++++++
> > > > include/linux/psp-sev.h | 8 +-
> > > > include/uapi/linux/kvm.h | 12 ++
> > > > 4 files changed, 168 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
> > > > index c3129b9ba5cb..4fd34fc5c7a7 100644
> > > > --- a/Documentation/virt/kvm/amd-memory-encryption.rst
> > > > +++ b/Documentation/virt/kvm/amd-memory-encryption.rst
> > > > @@ -263,6 +263,33 @@ Returns: 0 on success, -negative on error
> > > > __u32 trans_len;
> > > > };
> > > >
> > > > +10. KVM_SEV_SEND_START
> > > > +----------------------
> > > > +
> > > > +The KVM_SEV_SEND_START command can be used by the hypervisor to create an
> > > > +outgoing guest encryption context.
> > > > +
> > > > +Parameters (in): struct kvm_sev_send_start
> > > > +
> > > > +Returns: 0 on success, -negative on error
> > > > +
> > > > +::
> > > > + struct kvm_sev_send_start {
> > > > + __u32 policy; /* guest policy */
> > > > +
> > > > + __u64 pdh_cert_uaddr; /* platform Diffie-Hellman certificate */
> > > > + __u32 pdh_cert_len;
> > > > +
> > > > + __u64 plat_certs_uadr; /* platform certificate chain */
> > >
> > > Can this be changed as mentioned in the previous review
> > > (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&data=02%7C01%7Cashish.kalra%40amd.com%7Cf8d037dd37224ed4a33608d7f08067f5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242306610788233&sdata=eu1SgC6ukawOuxTeAoA3mzKiMNWlPCCa5lGEVs9D1s0%3D&reserved=0)?
> > >
> > > > + __u32 plat_certs_len;
> > > > +
> > > > + __u64 amd_certs_uaddr; /* AMD certificate */
> > > > + __u32 amd_cert_len;
> > >
> > > Can this be changed as mentioned in the previous review
> > > (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2F20200402062726.GA647295%40vbusired-dt%2F&data=02%7C01%7Cashish.kalra%40amd.com%7Cf8d037dd37224ed4a33608d7f08067f5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242306610798235&sdata=7xgp4ko8VGokGGQz1X1yPhz1ZXY9R501YginoDcen90%3D&reserved=0)?
> > >
> > > > +
> > > > + __u64 session_uaddr; /* Guest session information */
> > > > + __u32 session_len;
> > > > + };
> > > > +
> > > > References
> > > > ==========
> > > >
> > > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > > index cf912b4aaba8..5a15b43b4349 100644
> > > > --- a/arch/x86/kvm/svm/sev.c
> > > > +++ b/arch/x86/kvm/svm/sev.c
> > > > @@ -913,6 +913,128 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > > > return ret;
> > > > }
> > > >
> > > > +/* Userspace wants to query session length. */
> > > > +static int
> > > > +__sev_send_start_query_session_length(struct kvm *kvm, struct kvm_sev_cmd *argp,
> > > > + struct kvm_sev_send_start *params)
> > > > +{
> > > > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> > > > + struct sev_data_send_start *data;
> > > > + int ret;
> > > > +
> > > > + data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
> > > > + if (data == NULL)
> > > > + return -ENOMEM;
> > > > +
> > > > + data->handle = sev->handle;
> > > > + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
> > > > +
> > > > + params->session_len = data->session_len;
> > > > + if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
> > > > + sizeof(struct kvm_sev_send_start)))
> > > > + ret = -EFAULT;
> > > > +
> > > > + kfree(data);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > > > +{
> > > > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> > > > + struct sev_data_send_start *data;
> > > > + struct kvm_sev_send_start params;
> > > > + void *amd_certs, *session_data;
> > > > + void *pdh_cert, *plat_certs;
> > > > + int ret;
> > > > +
> > > > + if (!sev_guest(kvm))
> > > > + return -ENOTTY;
> > > > +
> > > > + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
> > > > + sizeof(struct kvm_sev_send_start)))
> > > > + return -EFAULT;
> > > > +
> > > > + /* if session_len is zero, userspace wants to query the session length */
> > > > + if (!params.session_len)
> > > > + return __sev_send_start_query_session_length(kvm, argp,
> > > > + ¶ms);
> > > > +
> > > > + /* some sanity checks */
> > > > + if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
> > > > + !params.session_uaddr || params.session_len > SEV_FW_BLOB_MAX_SIZE)
> > > > + return -EINVAL;
> > > > +
> > > > + /* allocate the memory to hold the session data blob */
> > > > + session_data = kmalloc(params.session_len, GFP_KERNEL_ACCOUNT);
> > > > + if (!session_data)
> > > > + return -ENOMEM;
> > > > +
> > > > + /* copy the certificate blobs from userspace */
> > > > + pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr,
> > > > + params.pdh_cert_len);
> > > > + if (IS_ERR(pdh_cert)) {
> > > > + ret = PTR_ERR(pdh_cert);
> > > > + goto e_free_session;
> > > > + }
> > > > +
> > > > + plat_certs = psp_copy_user_blob(params.plat_certs_uaddr,
> > > > + params.plat_certs_len);
> > > > + if (IS_ERR(plat_certs)) {
> > > > + ret = PTR_ERR(plat_certs);
> > > > + goto e_free_pdh;
> > > > + }
> > > > +
> > > > + amd_certs = psp_copy_user_blob(params.amd_certs_uaddr,
> > > > + params.amd_certs_len);
> > > > + if (IS_ERR(amd_certs)) {
> > > > + ret = PTR_ERR(amd_certs);
> > > > + goto e_free_plat_cert;
> > > > + }
> > > > +
> > > > + data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
> > > > + if (data == NULL) {
> > > > + ret = -ENOMEM;
> > > > + goto e_free_amd_cert;
> > > > + }
> > > > +
> > > > + /* populate the FW SEND_START field with system physical address */
> > > > + data->pdh_cert_address = __psp_pa(pdh_cert);
> > > > + data->pdh_cert_len = params.pdh_cert_len;
> > > > + data->plat_certs_address = __psp_pa(plat_certs);
> > > > + data->plat_certs_len = params.plat_certs_len;
> > > > + data->amd_certs_address = __psp_pa(amd_certs);
> > > > + data->amd_certs_len = params.amd_certs_len;
> > > > + data->session_address = __psp_pa(session_data);
> > > > + data->session_len = params.session_len;
> > > > + data->handle = sev->handle;
> > > > +
> > >
> > > Can the following code be changed as acknowledged in
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2Ff715bf99-0158-4d5f-77f3-b27743db3c59%40amd.com%2F&data=02%7C01%7Cashish.kalra%40amd.com%7Cf8d037dd37224ed4a33608d7f08067f5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637242306610798235&sdata=s3ZwfpCm94x2LCNN8I1jjcWcrkj3Nrw5PnOvxdDxSCs%3D&reserved=0?
> > >
> >
> > I believe that this has been already addressed as discussed :
> >
> > Ah, so the main issue is we should not be going to e_free on error. If
> > session_len is less than the expected len then FW will return an error.
> > In the case of an error we can skip copying the session_data into
> > userspace buffer but we still need to pass the session_len and policy
> > back to the userspace.
> >
> > So this patch is still returning session_len and policy back to user
> > in case of error : ( as the code below shows )
> >
> > if (!ret && copy_to_user((void
> > __user*)(uintptr_t)params.session_uaddr,...
>
> This fix addresses only one part of the problem. I am referring to the
> other suggestion about avoiding copying the entire kvm_sev_send_start
> structure back to the user. As I was mentioning in the discussion,
> the only fields that changed are the policy and session_len fields. So,
> why copy back the entire structure? Why not just those two fields?
>
> >
Both policy and session_len are two different fields in the
kvm_sev_send_start structure, why to complicate it by using two
different copy_to_user()'s here, to send two 32-bit words back to user,
it's much simpler to copy the whole structure back in one copy_to_user()
(makes it less complicated with no additional address computations),
also this is just a session start code, so it is invoked once at
the beginning of the migration process and it is not a performance
critical path such that saving few bytes of copy_to_user() wil affect it.
Thanks,
Ashish
> >
> > > > + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
> > > > +
> > > > + if (!ret && copy_to_user((void __user *)(uintptr_t)params.session_uaddr,
> > > > + session_data, params.session_len)) {
> > > > + ret = -EFAULT;
> > > > + goto e_free;
> > > > + }
> > > > +
> > > > + params.policy = data->policy;
> > > > + params.session_len = data->session_len;
> > > > + if (copy_to_user((void __user *)(uintptr_t)argp->data, ¶ms,
> > > > + sizeof(struct kvm_sev_send_start)))
> > > > + ret = -EFAULT;
> > > > +
> > > > +e_free:
> > > > + kfree(data);
> > > > +e_free_amd_cert:
> > > > + kfree(amd_certs);
> > > > +e_free_plat_cert:
> > > > + kfree(plat_certs);
> > > > +e_free_pdh:
> > > > + kfree(pdh_cert);
> > > > +e_free_session:
> > > > + kfree(session_data);
> > > > + return ret;
> > > > +}
> > > > +
> > > > int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> > > > {
> > > > struct kvm_sev_cmd sev_cmd;
> > > > @@ -957,6 +1079,9 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
> > > > case KVM_SEV_LAUNCH_SECRET:
> > > > r = sev_launch_secret(kvm, &sev_cmd);
> > > > break;
> > > > + case KVM_SEV_SEND_START:
> > > > + r = sev_send_start(kvm, &sev_cmd);
> > > > + break;
> > > > default:
> > > > r = -EINVAL;
> > > > goto out;
> > > > diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> > > > index 5167bf2bfc75..9f63b9d48b63 100644
> > > > --- a/include/linux/psp-sev.h
> > > > +++ b/include/linux/psp-sev.h
> > > > @@ -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 */
> > > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > > > index 428c7dde6b4b..8827d43e2684 100644
> > > > --- a/include/uapi/linux/kvm.h
> > > > +++ b/include/uapi/linux/kvm.h
> > > > @@ -1598,6 +1598,18 @@ struct kvm_sev_dbg {
> > > > __u32 len;
> > > > };
> > > >
> > > > +struct kvm_sev_send_start {
> > > > + __u32 policy;
> > > > + __u64 pdh_cert_uaddr;
> > > > + __u32 pdh_cert_len;
> > > > + __u64 plat_certs_uaddr;
> > > > + __u32 plat_certs_len;
> > > > + __u64 amd_certs_uaddr;
> > > > + __u32 amd_certs_len;
> > > > + __u64 session_uaddr;
> > > > + __u32 session_len;
> > > > +};
> > > > +
> > > > #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> > > > #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
> > > > #define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
> > > > --
> > > > 2.17.1
> > > >