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 <[email protected]>
---
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
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 <[email protected]>
> ---
...
> @@ -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.
On Tue, Apr 27, 2021 at 3:14 PM Sean Christopherson <[email protected]> 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 <[email protected]>
> > ---
>
> ...
>
> > @@ -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.
On Wed, Apr 28, 2021, Steve Rutherford wrote:
> On Tue, Apr 27, 2021 at 3:14 PM Sean Christopherson <[email protected]> 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 <[email protected]>
> > > ---
> >
> > ...
> >
> > > @@ -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.
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