Re: [PATCH v13 10/12] KVM: x86: Introduce new KVM_FEATURE_SEV_LIVE_MIGRATION feature & Custom MSR.

2021-04-20 Thread Paolo Bonzini

On 20/04/21 01:06, Sean Christopherson wrote:

diff --git a/arch/x86/include/uapi/asm/kvm_para.h 
b/arch/x86/include/uapi/asm/kvm_para.h
index 950afebfba88..f6bfa138874f 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_YIELD13
  #define KVM_FEATURE_ASYNC_PF_INT  14
  #define KVM_FEATURE_MSI_EXT_DEST_ID   15
+#define KVM_FEATURE_SEV_LIVE_MIGRATION 16
  
  #define KVM_HINTS_REALTIME  0
  
@@ -54,6 +55,7 @@

  #define MSR_KVM_POLL_CONTROL  0x4b564d05
  #define MSR_KVM_ASYNC_PF_INT  0x4b564d06
  #define MSR_KVM_ASYNC_PF_ACK  0x4b564d07
+#define MSR_KVM_SEV_LIVE_MIGRATION 0x4b564d08
  
  struct kvm_steal_time {

__u64 steal;
@@ -136,4 +138,6 @@ 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 BIT_ULL(0)


Even though the intent is to "force" userspace to intercept the MSR, I think KVM
should at least emulate the legal bits as a nop.  Deferring completely to
userspace is rather bizarre as there's not really anything to justify KVM
getting involved.  It would also force userspace to filter the MSR just to
support the hypercall.


I think this is the intention, the hypercall by itself cannot do much if
you cannot tell userspace that it's up-to-date.

On the other hand it is kind of wrong that KVM_GET_SUPPORTED_CPUID
returns the feature, but the MSR is not supported.


Somewhat of a nit, but I think we should do something like s/ENABLED/READY,


Agreed.  I'll send a patch that puts everything together.

Paolo



Re: [PATCH v13 10/12] KVM: x86: Introduce new KVM_FEATURE_SEV_LIVE_MIGRATION feature & Custom MSR.

2021-04-20 Thread Paolo Bonzini

On 15/04/21 17:58, Ashish Kalra wrote:

From: Ashish Kalra 

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_MIGRATION for guest to enable the SEV live migration
feature.

MSR is handled by userspace using MSR filters.

Signed-off-by: Ashish Kalra 
Reviewed-by: Steve Rutherford 


Let's leave the MSR out for now and rename the feature to 
KVM_FEATURE_HC_PAGE_ENC_STATUS.


Paolo


---
  Documentation/virt/kvm/cpuid.rst |  5 +
  Documentation/virt/kvm/msr.rst   | 12 
  arch/x86/include/uapi/asm/kvm_para.h |  4 
  arch/x86/kvm/cpuid.c |  3 ++-
  4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/Documentation/virt/kvm/cpuid.rst b/Documentation/virt/kvm/cpuid.rst
index cf62162d4be2..0bdb6cdb12d3 100644
--- a/Documentation/virt/kvm/cpuid.rst
+++ b/Documentation/virt/kvm/cpuid.rst
@@ -96,6 +96,11 @@ KVM_FEATURE_MSI_EXT_DEST_ID15  guest checks 
this feature bit
 before using extended 
destination
 ID bits in MSI address bits 
11-5.
  
+KVM_FEATURE_SEV_LIVE_MIGRATION 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/msr.rst b/Documentation/virt/kvm/msr.rst
index e37a14c323d2..020245d16087 100644
--- a/Documentation/virt/kvm/msr.rst
+++ b/Documentation/virt/kvm/msr.rst
@@ -376,3 +376,15 @@ 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_SEV_LIVE_MIGRATION:
+0x4b564d08
+
+   Control SEV Live Migration features.
+
+data:
+Bit 0 enables (1) or disables (0) host-side SEV Live Migration feature,
+in other words, this is guest->host communication that it's properly
+handling the shared pages list.
+
+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 950afebfba88..f6bfa138874f 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_YIELD13
  #define KVM_FEATURE_ASYNC_PF_INT  14
  #define KVM_FEATURE_MSI_EXT_DEST_ID   15
+#define KVM_FEATURE_SEV_LIVE_MIGRATION 16
  
  #define KVM_HINTS_REALTIME  0
  
@@ -54,6 +55,7 @@

  #define MSR_KVM_POLL_CONTROL  0x4b564d05
  #define MSR_KVM_ASYNC_PF_INT  0x4b564d06
  #define MSR_KVM_ASYNC_PF_ACK  0x4b564d07
+#define MSR_KVM_SEV_LIVE_MIGRATION 0x4b564d08
  
  struct kvm_steal_time {

__u64 steal;
@@ -136,4 +138,6 @@ 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 BIT_ULL(0)

+
  #endif /* _UAPI_ASM_X86_KVM_PARA_H */
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 6bd2f8b830e4..4e2e69a692aa 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -812,7 +812,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_SEV_LIVE_MIGRATION);
  
  		if (sched_info_on())

entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);





Re: [PATCH v13 10/12] KVM: x86: Introduce new KVM_FEATURE_SEV_LIVE_MIGRATION feature & Custom MSR.

2021-04-19 Thread Sean Christopherson
On Thu, Apr 15, 2021, Ashish Kalra wrote:
> From: Ashish Kalra 
> 
> 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_MIGRATION for guest to enable the SEV live migration
> feature.
> 
> MSR is handled by userspace using MSR filters.
> 
> Signed-off-by: Ashish Kalra 
> Reviewed-by: Steve Rutherford 
> ---
>  Documentation/virt/kvm/cpuid.rst |  5 +
>  Documentation/virt/kvm/msr.rst   | 12 
>  arch/x86/include/uapi/asm/kvm_para.h |  4 
>  arch/x86/kvm/cpuid.c |  3 ++-
>  4 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/virt/kvm/cpuid.rst 
> b/Documentation/virt/kvm/cpuid.rst
> index cf62162d4be2..0bdb6cdb12d3 100644
> --- a/Documentation/virt/kvm/cpuid.rst
> +++ b/Documentation/virt/kvm/cpuid.rst
> @@ -96,6 +96,11 @@ KVM_FEATURE_MSI_EXT_DEST_ID15  guest 
> checks this feature bit
> before using extended 
> destination
> ID bits in MSI address bits 
> 11-5.
>  
> +KVM_FEATURE_SEV_LIVE_MIGRATION 16  guest checks this feature bit 
> before
> +   using the page encryption 
> state
> +   hypercall to notify the page 
> state
> +   change

Hrm, I think there are two separate things being intertwined: the hypercall to
communicate private/shared pages, and the MSR to control live migration.  More
thoughts below.

>  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..020245d16087 100644
> --- a/Documentation/virt/kvm/msr.rst
> +++ b/Documentation/virt/kvm/msr.rst
> @@ -376,3 +376,15 @@ 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_SEV_LIVE_MIGRATION:
> +0x4b564d08
> +
> + Control SEV Live Migration features.
> +
> +data:
> +Bit 0 enables (1) or disables (0) host-side SEV Live Migration 
> feature,
> +in other words, this is guest->host communication that it's properly
> +handling the shared pages list.
> +
> +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 950afebfba88..f6bfa138874f 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_SEV_LIVE_MIGRATION   16
>  
>  #define KVM_HINTS_REALTIME  0
>  
> @@ -54,6 +55,7 @@
>  #define MSR_KVM_POLL_CONTROL 0x4b564d05
>  #define MSR_KVM_ASYNC_PF_INT 0x4b564d06
>  #define MSR_KVM_ASYNC_PF_ACK 0x4b564d07
> +#define MSR_KVM_SEV_LIVE_MIGRATION   0x4b564d08
>  
>  struct kvm_steal_time {
>   __u64 steal;
> @@ -136,4 +138,6 @@ 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 BIT_ULL(0)

Even though the intent is to "force" userspace to intercept the MSR, I think KVM
should at least emulate the legal bits as a nop.  Deferring completely to
userspace is rather bizarre as there's not really anything to justify KVM
getting involved.  It would also force userspace to filter the MSR just to
support the hypercall.

Somewhat of a nit, but I think we should do something like s/ENABLED/READY,
or maybe s/ENABLED/SAFE, in the bit name so that the semantics are more along
the lines of an announcement from the guest, as opposed to a command.  Treating
the bit as a hint/announcement makes it easier to bundle the hypercall and the
MSR together under a single feature, e.g. it's slightly more obvious that
userspace can ignore the MSR if it knows its use case doesn't need migration or
that it can't migrate its guest at will.

I also think we should drop the "SEV" part, especially since it sounds like the
feature flag also enumerates that the hypercall is available.

E.g. for the WRMSR side

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index eca63625ae..10f90f8491 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3229,6 +3229,13 @@ 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_LIVE_MIGRATION_CONTROL:
+   

[PATCH v13 10/12] KVM: x86: Introduce new KVM_FEATURE_SEV_LIVE_MIGRATION feature & Custom MSR.

2021-04-15 Thread Ashish Kalra
From: Ashish Kalra 

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_MIGRATION for guest to enable the SEV live migration
feature.

MSR is handled by userspace using MSR filters.

Signed-off-by: Ashish Kalra 
Reviewed-by: Steve Rutherford 
---
 Documentation/virt/kvm/cpuid.rst |  5 +
 Documentation/virt/kvm/msr.rst   | 12 
 arch/x86/include/uapi/asm/kvm_para.h |  4 
 arch/x86/kvm/cpuid.c |  3 ++-
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/Documentation/virt/kvm/cpuid.rst b/Documentation/virt/kvm/cpuid.rst
index cf62162d4be2..0bdb6cdb12d3 100644
--- a/Documentation/virt/kvm/cpuid.rst
+++ b/Documentation/virt/kvm/cpuid.rst
@@ -96,6 +96,11 @@ KVM_FEATURE_MSI_EXT_DEST_ID15  guest checks 
this feature bit
before using extended 
destination
ID bits in MSI address bits 
11-5.
 
+KVM_FEATURE_SEV_LIVE_MIGRATION 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/msr.rst b/Documentation/virt/kvm/msr.rst
index e37a14c323d2..020245d16087 100644
--- a/Documentation/virt/kvm/msr.rst
+++ b/Documentation/virt/kvm/msr.rst
@@ -376,3 +376,15 @@ 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_SEV_LIVE_MIGRATION:
+0x4b564d08
+
+   Control SEV Live Migration features.
+
+data:
+Bit 0 enables (1) or disables (0) host-side SEV Live Migration feature,
+in other words, this is guest->host communication that it's properly
+handling the shared pages list.
+
+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 950afebfba88..f6bfa138874f 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_ID15
+#define KVM_FEATURE_SEV_LIVE_MIGRATION 16
 
 #define KVM_HINTS_REALTIME  0
 
@@ -54,6 +55,7 @@
 #define MSR_KVM_POLL_CONTROL   0x4b564d05
 #define MSR_KVM_ASYNC_PF_INT   0x4b564d06
 #define MSR_KVM_ASYNC_PF_ACK   0x4b564d07
+#define MSR_KVM_SEV_LIVE_MIGRATION 0x4b564d08
 
 struct kvm_steal_time {
__u64 steal;
@@ -136,4 +138,6 @@ 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 BIT_ULL(0)
+
 #endif /* _UAPI_ASM_X86_KVM_PARA_H */
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 6bd2f8b830e4..4e2e69a692aa 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -812,7 +812,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_SEV_LIVE_MIGRATION);
 
if (sched_info_on())
entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
-- 
2.17.1