linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] KVM: x86: guest interface for SEV live migration
@ 2021-04-21 17:37 Paolo Bonzini
  2021-04-21 17:37 ` [PATCH v2 1/2] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall Paolo Bonzini
  2021-04-21 17:37 ` [PATCH v2 2/2] KVM: x86: add MSR_KVM_MIGRATION_CONTROL Paolo Bonzini
  0 siblings, 2 replies; 8+ messages in thread
From: Paolo Bonzini @ 2021-04-21 17:37 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: seanjc, srutherford, joro, brijesh.singh, thomas.lendacky,
	venu.busireddy

This is a reviewed version of the guest interface (hypercall+MSR)
for SEV live migration.  The differences lie mostly in the API
for userspace.  In particular:

- the CPUID feature is not exposed in KVM_GET_SUPPORTED_CPUID

- the hypercall must be enabled manually with KVM_ENABLE_CAP

- the MSR has sensible behavior if not filtered (reads as zero,
  writes must leave undefined bits as zero or they #GP)

v1->v2: reviewed KVM_CAP_EXIT_HYPERCALL semantics,
	split CPUID bits so that the migration control MSR
	can be used for TDX

Ashish Kalra (1):
  KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall

Paolo Bonzini (1):
  KVM: x86: add MSR_KVM_MIGRATION_CONTROL

 Documentation/virt/kvm/api.rst        | 19 ++++++++++
 Documentation/virt/kvm/cpuid.rst      |  9 +++++
 Documentation/virt/kvm/hypercalls.rst | 15 ++++++++
 Documentation/virt/kvm/msr.rst        | 10 ++++++
 arch/x86/include/asm/kvm_host.h       |  2 ++
 arch/x86/include/uapi/asm/kvm_para.h  |  5 +++
 arch/x86/kvm/cpuid.c                  |  3 +-
 arch/x86/kvm/x86.c                    | 50 +++++++++++++++++++++++++++
 include/uapi/linux/kvm.h              |  1 +
 include/uapi/linux/kvm_para.h         |  1 +
 10 files changed, 114 insertions(+), 1 deletion(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
  2021-04-21 17:37 [PATCH v2 0/2] KVM: x86: guest interface for SEV live migration Paolo Bonzini
@ 2021-04-21 17:37 ` Paolo Bonzini
  2021-04-27 22:04   ` Sean Christopherson
  2021-04-21 17:37 ` [PATCH v2 2/2] KVM: x86: add MSR_KVM_MIGRATION_CONTROL Paolo Bonzini
  1 sibling, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2021-04-21 17:37 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: seanjc, srutherford, joro, brijesh.singh, thomas.lendacky,
	venu.busireddy, Ashish Kalra, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Borislav Petkov, x86

From: Ashish Kalra <ashish.kalra@amd.com>

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.

The hypercall exits to userspace to manage the guest shared regions and
integrate with the userspace VMM's migration code.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: x86@kernel.org
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Steve Rutherford <srutherford@google.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-Id: <93d7f2c2888315adc48905722574d89699edde33.1618498113.git.ashish.kalra@amd.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Documentation/virt/kvm/api.rst        | 19 ++++++++++++++
 Documentation/virt/kvm/cpuid.rst      |  5 ++++
 Documentation/virt/kvm/hypercalls.rst | 15 +++++++++++
 arch/x86/include/asm/kvm_host.h       |  2 ++
 arch/x86/include/uapi/asm/kvm_para.h  |  1 +
 arch/x86/kvm/x86.c                    | 36 +++++++++++++++++++++++++++
 include/uapi/linux/kvm.h              |  1 +
 include/uapi/linux/kvm_para.h         |  1 +
 8 files changed, 80 insertions(+)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index fd4a84911355..413d5ebbf49c 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6766,3 +6766,22 @@ they will get passed on to user space. So user space still has to have
 an implementation for these despite the in kernel acceleration.
 
 This capability is always enabled.
+
+8.32 KVM_CAP_EXIT_HYPERCALL
+---------------------------
+
+:Capability: KVM_CAP_EXIT_HYPERCALL
+:Architectures: x86
+:Type: vm
+
+This capability, if enabled, will cause KVM to exit to userspace
+with KVM_EXIT_HYPERCALL exit reason to process some hypercalls.
+
+The arguments to KVM_ENABLE_CAP are a bitmask of hypercalls that
+will exit to userspace.  The bits that can be set are returned by
+KVM_CHECK_EXTENSION.  The behavior of hypercalls that are not enabled
+depends on the individual hypercall.
+
+Right now, the only bit that can be set is bit 12, corresponding to
+KVM_HC_PAGE_ENC_STATUS.  The hypercall returns ENOSYS if bit 12 is not
+set or KVM_CAP_EXIT_HYPERCALL is not enabled.
diff --git a/Documentation/virt/kvm/cpuid.rst b/Documentation/virt/kvm/cpuid.rst
index cf62162d4be2..c9378d163b5a 100644
--- a/Documentation/virt/kvm/cpuid.rst
+++ b/Documentation/virt/kvm/cpuid.rst
@@ -96,6 +96,11 @@ KVM_FEATURE_MSI_EXT_DEST_ID        15          guest checks this feature bit
                                                before using extended destination
                                                ID bits in MSI address bits 11-5.
 
+KVM_FEATURE_HC_PAGE_ENC_STATUS     16          guest checks this feature bit before
+                                               using the page encryption state
+                                               hypercall to notify the page state
+                                               change
+
 KVM_FEATURE_CLOCKSOURCE_STABLE_BIT 24          host will warn if no guest-side
                                                per-cpu warps are expected in
                                                kvmclock
diff --git a/Documentation/virt/kvm/hypercalls.rst b/Documentation/virt/kvm/hypercalls.rst
index ed4fddd364ea..234ed5188aef 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: page encryption status
+
+   Where:
+	* 1: Page is encrypted
+	* 0: Page is decrypted
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 6e195f7df4f0..b3c40f0bc80e 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1050,6 +1050,8 @@ struct kvm_arch {
 	/* Guest can access the SGX PROVISIONKEY. */
 	bool sgx_provisioning_allowed;
 
+	u64 hypercall_exit_enabled;
+
 	struct kvm_pmu_event_filter __rcu *pmu_event_filter;
 	struct task_struct *nx_lpage_recovery_thread;
 
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index 950afebfba88..be49956b603f 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -33,6 +33,7 @@
 #define KVM_FEATURE_PV_SCHED_YIELD	13
 #define KVM_FEATURE_ASYNC_PF_INT	14
 #define KVM_FEATURE_MSI_EXT_DEST_ID	15
+#define KVM_FEATURE_HC_PAGE_ENC_STATUS	16
 
 #define KVM_HINTS_REALTIME      0
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c9ba6f2d9bcd..c2babf70a587 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3809,6 +3809,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_SGX_ATTRIBUTE:
 #endif
 	case KVM_CAP_VM_COPY_ENC_CONTEXT_FROM:
+	case KVM_CAP_EXIT_HYPERCALL:
 		r = 1;
 		break;
 	case KVM_CAP_SET_GUEST_DEBUG2:
@@ -5413,6 +5414,14 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 		break;
 	}
 #endif
+	case KVM_CAP_EXIT_HYPERCALL:
+		r = -EINVAL;
+		if ((cap->args[0] & ~(1 << KVM_HC_PAGE_ENC_STATUS)) ||
+		    cap->args[1] || cap->args[2] || cap->args[3])
+			break;
+		r = 0;
+		kvm->arch.hypercall_exit_enabled = cap->args[0];
+		break;
 	case KVM_CAP_VM_COPY_ENC_CONTEXT_FROM:
 		r = -EINVAL;
 		if (kvm_x86_ops.vm_copy_enc_context_from)
@@ -8269,6 +8278,13 @@ static void kvm_sched_yield(struct kvm_vcpu *vcpu, unsigned long dest_id)
 	return;
 }
 
+static int complete_hypercall_exit(struct kvm_vcpu *vcpu)
+{
+	kvm_rax_write(vcpu, vcpu->run->hypercall.ret);
+	++vcpu->stat.hypercalls;
+	return kvm_skip_emulated_instruction(vcpu);
+}
+
 int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 {
 	unsigned long nr, a0, a1, a2, a3, ret;
@@ -8334,6 +8350,26 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 		kvm_sched_yield(vcpu, a0);
 		ret = 0;
 		break;
+	case KVM_HC_PAGE_ENC_STATUS: {
+		u64 gpa = a0, npages = a1, enc = a2;
+		if (!guest_pv_has(vcpu, KVM_FEATURE_HC_PAGE_ENC_STATUS)
+		    || !(vcpu->kvm->arch.hypercall_exit_enabled & (1 << KVM_HC_PAGE_ENC_STATUS)))
+			break;
+
+		ret = -KVM_EINVAL;
+		if (!PAGE_ALIGNED(gpa) || !npages ||
+		    gpa_to_gfn(gpa) + npages <= gpa_to_gfn(gpa))
+			break;
+
+		vcpu->run->exit_reason        = KVM_EXIT_HYPERCALL;
+		vcpu->run->hypercall.nr       = KVM_HC_PAGE_ENC_STATUS;
+		vcpu->run->hypercall.args[0]  = gpa;
+		vcpu->run->hypercall.args[1]  = npages;
+		vcpu->run->hypercall.args[2]  = enc;
+		vcpu->run->hypercall.longmode = op_64_bit;
+		vcpu->arch.complete_userspace_io = complete_hypercall_exit;
+		return 0;
+	}
 	default:
 		ret = -KVM_ENOSYS;
 		break;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index d76533498543..7dc1c217704f 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1081,6 +1081,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_SET_GUEST_DEBUG2 195
 #define KVM_CAP_SGX_ATTRIBUTE 196
 #define KVM_CAP_VM_COPY_ENC_CONTEXT_FROM 197
+#define KVM_CAP_EXIT_HYPERCALL 198
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
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.26.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] KVM: x86: add MSR_KVM_MIGRATION_CONTROL
  2021-04-21 17:37 [PATCH v2 0/2] KVM: x86: guest interface for SEV live migration Paolo Bonzini
  2021-04-21 17:37 ` [PATCH v2 1/2] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall Paolo Bonzini
@ 2021-04-21 17:37 ` Paolo Bonzini
  2021-04-27 22:14   ` Sean Christopherson
  1 sibling, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2021-04-21 17:37 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: seanjc, srutherford, joro, brijesh.singh, thomas.lendacky,
	venu.busireddy

Add a new MSR that can be used to communicate whether the page
encryption status bitmap is up to date and therefore whether live
migration of an encrypted guest is possible.

The MSR should be processed by userspace if it is going to live
migrate the guest; the default implementation does nothing.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Documentation/virt/kvm/cpuid.rst     |  4 ++++
 Documentation/virt/kvm/msr.rst       | 10 ++++++++++
 arch/x86/include/uapi/asm/kvm_para.h |  4 ++++
 arch/x86/kvm/cpuid.c                 |  3 ++-
 arch/x86/kvm/x86.c                   | 14 ++++++++++++++
 5 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/virt/kvm/cpuid.rst b/Documentation/virt/kvm/cpuid.rst
index c9378d163b5a..906d32812ab1 100644
--- a/Documentation/virt/kvm/cpuid.rst
+++ b/Documentation/virt/kvm/cpuid.rst
@@ -101,6 +101,10 @@ KVM_FEATURE_HC_PAGE_ENC_STATUS     16          guest checks this feature bit bef
                                                hypercall to notify the page state
                                                change
 
+KVM_FEATURE_MIGRATION_CONTROL      17          guest checks this feature bit
+                                               before setting bit 0 of
+                                               MSR_KVM_MIGRATION_CONTROL
+
 KVM_FEATURE_CLOCKSOURCE_STABLE_BIT 24          host will warn if no guest-side
                                                per-cpu warps are expected in
                                                kvmclock
diff --git a/Documentation/virt/kvm/msr.rst b/Documentation/virt/kvm/msr.rst
index e37a14c323d2..3917fd57314e 100644
--- a/Documentation/virt/kvm/msr.rst
+++ b/Documentation/virt/kvm/msr.rst
@@ -376,3 +376,13 @@ data:
 	write '1' to bit 0 of the MSR, this causes the host to re-scan its queue
 	and check if there are more notifications pending. The MSR is available
 	if KVM_FEATURE_ASYNC_PF_INT is present in CPUID.
+
+MSR_KVM_MIGRATION_CONTROL:
+        0x4b564d08
+
+data:
+        If the guest is running with encrypted memory and it is communicating
+        page encryption status to the host using the ``KVM_HC_PAGE_ENC_STATUS``
+        hypercall, it can set bit 0 in this MSR to allow live migration of
+        the guest.  The bit remains set to 0 (but WRMSR does not fail) if
+        the host is not interested in page encryption status.
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index be49956b603f..f66b3ad35f97 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -34,6 +34,7 @@
 #define KVM_FEATURE_ASYNC_PF_INT	14
 #define KVM_FEATURE_MSI_EXT_DEST_ID	15
 #define KVM_FEATURE_HC_PAGE_ENC_STATUS	16
+#define KVM_FEATURE_MIGRATION_CONTROL	17
 
 #define KVM_HINTS_REALTIME      0
 
@@ -55,6 +56,7 @@
 #define MSR_KVM_POLL_CONTROL	0x4b564d05
 #define MSR_KVM_ASYNC_PF_INT	0x4b564d06
 #define MSR_KVM_ASYNC_PF_ACK	0x4b564d07
+#define MSR_KVM_MIGRATION_CONTROL	0x4b564d08
 
 struct kvm_steal_time {
 	__u64 steal;
@@ -91,6 +93,8 @@ struct kvm_clock_pairing {
 /* MSR_KVM_ASYNC_PF_INT */
 #define KVM_ASYNC_PF_VEC_MASK			GENMASK(7, 0)
 
+/* MSR_KVM_MIGRATION_CONTROL */
+#define KVM_PAGE_ENC_STATUS_UPTODATE		(1 << 0)
 
 /* Operations for KVM_HC_MMU_OP */
 #define KVM_MMU_OP_WRITE_PTE            1
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 2ae061586677..b488c77bd429 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -887,7 +887,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 			     (1 << KVM_FEATURE_PV_SEND_IPI) |
 			     (1 << KVM_FEATURE_POLL_CONTROL) |
 			     (1 << KVM_FEATURE_PV_SCHED_YIELD) |
-			     (1 << KVM_FEATURE_ASYNC_PF_INT);
+			     (1 << KVM_FEATURE_ASYNC_PF_INT) |
+			     (1 << KVM_FEATURE_MIGRATION_CONTROL);
 
 		if (sched_info_on())
 			entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c2babf70a587..4cc849f2a048 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3258,6 +3258,14 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		vcpu->arch.msr_kvm_poll_control = data;
 		break;
 
+	case MSR_KVM_MIGRATION_CONTROL:
+		if (!guest_pv_has(vcpu, KVM_FEATURE_MIGRATION_CONTROL))
+			return 1;
+
+		if (data & ~KVM_PAGE_ENC_STATUS_UPTODATE)
+			return 1;
+		break;
+
 	case MSR_IA32_MCG_CTL:
 	case MSR_IA32_MCG_STATUS:
 	case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1:
@@ -3549,6 +3557,12 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF))
 			return 1;
 
+		msr_info->data = 0;
+		break;
+	case MSR_KVM_MIGRATION_CONTROL:
+		if (!guest_pv_has(vcpu, KVM_FEATURE_MIGRATION_CONTROL))
+			return 1;
+
 		msr_info->data = 0;
 		break;
 	case MSR_KVM_STEAL_TIME:
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
  2021-04-21 17:37 ` [PATCH v2 1/2] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall Paolo Bonzini
@ 2021-04-27 22:04   ` Sean Christopherson
  0 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2021-04-27 22:04 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: linux-kernel, kvm, srutherford, joro, brijesh.singh,
	thomas.lendacky, venu.busireddy, Ashish Kalra, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin, Borislav Petkov, x86

On Wed, Apr 21, 2021, Paolo Bonzini wrote:
> @@ -8334,6 +8350,26 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
>  		kvm_sched_yield(vcpu, a0);
>  		ret = 0;
>  		break;
> +	case KVM_HC_PAGE_ENC_STATUS: {
> +		u64 gpa = a0, npages = a1, enc = a2;

newline

> +		if (!guest_pv_has(vcpu, KVM_FEATURE_HC_PAGE_ENC_STATUS)
> +		    || !(vcpu->kvm->arch.hypercall_exit_enabled & (1 << KVM_HC_PAGE_ENC_STATUS)))

|| on previous line, pretty please :-)

> +			break;
> +
> +		ret = -KVM_EINVAL;
> +		if (!PAGE_ALIGNED(gpa) || !npages ||
> +		    gpa_to_gfn(gpa) + npages <= gpa_to_gfn(gpa))
> +			break;
> +
> +		vcpu->run->exit_reason        = KVM_EXIT_HYPERCALL;
> +		vcpu->run->hypercall.nr       = KVM_HC_PAGE_ENC_STATUS;
> +		vcpu->run->hypercall.args[0]  = gpa;
> +		vcpu->run->hypercall.args[1]  = npages;
> +		vcpu->run->hypercall.args[2]  = enc;
> +		vcpu->run->hypercall.longmode = op_64_bit;
> +		vcpu->arch.complete_userspace_io = complete_hypercall_exit;
> +		return 0;
> +	}
>  	default:
>  		ret = -KVM_ENOSYS;
>  		break;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: add MSR_KVM_MIGRATION_CONTROL
  2021-04-21 17:37 ` [PATCH v2 2/2] KVM: x86: add MSR_KVM_MIGRATION_CONTROL Paolo Bonzini
@ 2021-04-27 22:14   ` Sean Christopherson
  2021-04-28 19:56     ` Steve Rutherford
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2021-04-27 22:14 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: linux-kernel, kvm, srutherford, joro, brijesh.singh,
	thomas.lendacky, venu.busireddy

On Wed, Apr 21, 2021, Paolo Bonzini wrote:
> Add a new MSR that can be used to communicate whether the page
> encryption status bitmap is up to date and therefore whether live
> migration of an encrypted guest is possible.
> 
> The MSR should be processed by userspace if it is going to live
> migrate the guest; the default implementation does nothing.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

...

> @@ -91,6 +93,8 @@ struct kvm_clock_pairing {
>  /* MSR_KVM_ASYNC_PF_INT */
>  #define KVM_ASYNC_PF_VEC_MASK			GENMASK(7, 0)
>  
> +/* MSR_KVM_MIGRATION_CONTROL */
> +#define KVM_PAGE_ENC_STATUS_UPTODATE		(1 << 0)

Why explicitly tie this to encryption status?  AFAICT, doing so serves no real
purpose and can only hurt us in the long run.  E.g. if a new use case for
"disabling" migration comes along and it has nothing to do with encryption, then
it has the choice of either using a different bit or bastardizing the existing
control.

I've no idea if such a use case is remotely likely to pop up, but allowing for
such a possibility costs us nothing.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: add MSR_KVM_MIGRATION_CONTROL
  2021-04-27 22:14   ` Sean Christopherson
@ 2021-04-28 19:56     ` Steve Rutherford
  2021-04-28 20:06       ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Steve Rutherford @ 2021-04-28 19:56 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, LKML, KVM list, Joerg Roedel, Brijesh Singh,
	Tom Lendacky, Venu Busireddy

On Tue, Apr 27, 2021 at 3:14 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Wed, Apr 21, 2021, Paolo Bonzini wrote:
> > Add a new MSR that can be used to communicate whether the page
> > encryption status bitmap is up to date and therefore whether live
> > migration of an encrypted guest is possible.
> >
> > The MSR should be processed by userspace if it is going to live
> > migrate the guest; the default implementation does nothing.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
>
> ...
>
> > @@ -91,6 +93,8 @@ struct kvm_clock_pairing {
> >  /* MSR_KVM_ASYNC_PF_INT */
> >  #define KVM_ASYNC_PF_VEC_MASK                        GENMASK(7, 0)
> >
> > +/* MSR_KVM_MIGRATION_CONTROL */
> > +#define KVM_PAGE_ENC_STATUS_UPTODATE         (1 << 0)
>
> Why explicitly tie this to encryption status?  AFAICT, doing so serves no real
> purpose and can only hurt us in the long run.  E.g. if a new use case for
> "disabling" migration comes along and it has nothing to do with encryption, then
> it has the choice of either using a different bit or bastardizing the existing
> control.
>
> I've no idea if such a use case is remotely likely to pop up, but allowing for
> such a possibility costs us nothing.
Using a different bit sounds fine to me. It would allow us to avoid
stuffing multiple meanings into a single bit, which would still happen
even if we had a better name.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: add MSR_KVM_MIGRATION_CONTROL
  2021-04-28 19:56     ` Steve Rutherford
@ 2021-04-28 20:06       ` Sean Christopherson
  2021-04-29 10:09         ` Paolo Bonzini
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2021-04-28 20:06 UTC (permalink / raw)
  To: Steve Rutherford
  Cc: Paolo Bonzini, LKML, KVM list, Joerg Roedel, Brijesh Singh,
	Tom Lendacky, Venu Busireddy

On Wed, Apr 28, 2021, Steve Rutherford wrote:
> On Tue, Apr 27, 2021 at 3:14 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Wed, Apr 21, 2021, Paolo Bonzini wrote:
> > > Add a new MSR that can be used to communicate whether the page
> > > encryption status bitmap is up to date and therefore whether live
> > > migration of an encrypted guest is possible.
> > >
> > > The MSR should be processed by userspace if it is going to live
> > > migrate the guest; the default implementation does nothing.
> > >
> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > > ---
> >
> > ...
> >
> > > @@ -91,6 +93,8 @@ struct kvm_clock_pairing {
> > >  /* MSR_KVM_ASYNC_PF_INT */
> > >  #define KVM_ASYNC_PF_VEC_MASK                        GENMASK(7, 0)
> > >
> > > +/* MSR_KVM_MIGRATION_CONTROL */
> > > +#define KVM_PAGE_ENC_STATUS_UPTODATE         (1 << 0)
> >
> > Why explicitly tie this to encryption status?  AFAICT, doing so serves no real
> > purpose and can only hurt us in the long run.  E.g. if a new use case for
> > "disabling" migration comes along and it has nothing to do with encryption, then
> > it has the choice of either using a different bit or bastardizing the existing
> > control.
> >
> > I've no idea if such a use case is remotely likely to pop up, but allowing for
> > such a possibility costs us nothing.
>
> Using a different bit sounds fine to me. It would allow us to avoid
> stuffing multiple meanings into a single bit, which would still happen
> even if we had a better name.

But there's only multiple meanings if we define the bit to be specific to
page encryption.  E.g. if the bit is KVM_READY_FOR_MIGRATION, then its meaning
(when cleared) is simply "please don't migrate me, I will die".  KVM doesn't
care _why_ the guest is telling userspace that it's not ready for migration, nor
does KVM care if userspace honors the indicator.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: add MSR_KVM_MIGRATION_CONTROL
  2021-04-28 20:06       ` Sean Christopherson
@ 2021-04-29 10:09         ` Paolo Bonzini
  0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2021-04-29 10:09 UTC (permalink / raw)
  To: Sean Christopherson, Steve Rutherford
  Cc: LKML, KVM list, Joerg Roedel, Brijesh Singh, Tom Lendacky,
	Venu Busireddy

On 28/04/21 22:06, Sean Christopherson wrote:
> But there's only multiple meanings if we define the bit to be specific to
> page encryption.  E.g. if the bit is KVM_READY_FOR_MIGRATION, then its meaning
> (when cleared) is simply "please don't migrate me, I will die".  KVM doesn't
> care_why_  the guest is telling userspace that it's not ready for migration, nor
> does KVM care if userspace honors the indicator.

Makes sense, I'll change that.

Paolo


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-04-29 10:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21 17:37 [PATCH v2 0/2] KVM: x86: guest interface for SEV live migration Paolo Bonzini
2021-04-21 17:37 ` [PATCH v2 1/2] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall Paolo Bonzini
2021-04-27 22:04   ` Sean Christopherson
2021-04-21 17:37 ` [PATCH v2 2/2] KVM: x86: add MSR_KVM_MIGRATION_CONTROL Paolo Bonzini
2021-04-27 22:14   ` Sean Christopherson
2021-04-28 19:56     ` Steve Rutherford
2021-04-28 20:06       ` Sean Christopherson
2021-04-29 10:09         ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).