Re: [PATCH 09/11] kvm/x86: distingiush hyper-v guest crash notification

2015-06-23 Thread Paolo Bonzini 


On 23/06/2015 01:55, Peter Hornyack wrote:
  -static int kvm_hv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u64 data)
  +static int kvm_hv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u64 data, bool 
  host)
   {
  struct kvm_arch_hyperv *hv = vcpu-kvm-arch.hyperv;
 
  hv-hv_crash_ctl = data;
 Should we allow hv_crash_ctl to be set if !host? It's a small detail,
 but it doesn't seem like the guest should be able to disable crash
 actions that userspace has enabled in hv_crash_ctl.

No, the MSR contents shouldn't be writable.

Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/11] kvm/x86: distingiush hyper-v guest crash notification

2015-06-22 Thread Paolo Bonzini 


On 22/06/2015 18:05, Denis V. Lunev wrote:
 From: Andrey Smetanin asmeta...@virtuozzo.com
 
 Previous patches allowes userspace to setup Hyper-V crash ctl msr.
 This msr should expose HV_X64_MSR_CRASH_CTL_NOTIFY value to Hyper-V
 guest to allow to send crash data. Unfortunately Hyper-V guest notifies
 hardware about crash by writing the same HV_X64_MSR_CRASH_CTL_NOTIFY value
 into crash ctl msr. Thus both user space and guest writes inside ctl msr the
 same value and this patch distingiush the moment of actual guest crash
 by checking host initiated value from msr info.
 
 Signed-off-by: Andrey Smetanin asmeta...@virtuozzo.com
 Signed-off-by: Denis V. Lunev d...@openvz.org
 CC: Paolo Bonzini pbonz...@redhat.com
 CC: Gleb Natapov g...@kernel.org
 ---
  arch/x86/kvm/hyperv.c | 17 +
  arch/x86/kvm/hyperv.h |  2 +-
  arch/x86/kvm/x86.c|  3 ++-
  3 files changed, 12 insertions(+), 10 deletions(-)
 
 diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
 index 6b18015..f49502a 100644
 --- a/arch/x86/kvm/hyperv.c
 +++ b/arch/x86/kvm/hyperv.c
 @@ -54,12 +54,12 @@ static int kvm_hv_msr_get_crash_ctl(struct kvm_vcpu 
 *vcpu, u64 *pdata)
   return 0;
  }
  
 -static int kvm_hv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u64 data)
 +static int kvm_hv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u64 data, bool 
 host)
  {
   struct kvm_arch_hyperv *hv = vcpu-kvm-arch.hyperv;
  
   hv-hv_crash_ctl = data;
 - if ((data  HV_X64_MSR_CRASH_CTL_NOTIFY)) {
 + if ((data  HV_X64_MSR_CRASH_CTL_NOTIFY)  !host) {
   vcpu_debug(vcpu, hv crash (0x%llx 0x%llx 0x%llx 0x%llx 
 0x%llx)\n, hv-hv_crash_param[0],
 hv-hv_crash_param[1],
 @@ -99,7 +99,8 @@ static int kvm_hv_msr_get_crash_data(struct kvm_vcpu *vcpu,
   return 0;
  }
  
 -static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 +static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu,
 +  u32 msr, u64 data, bool host)
  {
   struct kvm *kvm = vcpu-kvm;
   struct kvm_arch_hyperv *hv = kvm-arch.hyperv;
 @@ -156,7 +157,7 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 
 msr, u64 data)
msr - HV_X64_MSR_CRASH_P0,
data);
   case HV_X64_MSR_CRASH_CTL:
 - return kvm_hv_msr_set_crash_ctl(vcpu, data);
 + return kvm_hv_msr_set_crash_ctl(vcpu, data, host);
   default:
   vcpu_unimpl(vcpu, Hyper-V unimpl wrmsr: 0x%x data 0x%llx\n,
 msr, data);
 @@ -165,7 +166,7 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 
 msr, u64 data)
   return 0;
  }
  
 -static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 +static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool 
 host)
  {
   struct kvm_vcpu_arch_hyperv *hv = vcpu-arch.hyperv;
  
 @@ -278,17 +279,17 @@ static int kvm_hv_get_msr(struct kvm_vcpu *vcpu, u32 
 msr, u64 *pdata)
   return 0;
  }
  
 -int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 +int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool 
 host)
  {
   if (kvm_hv_msr_partition_wide(msr)) {
   int r;
  
   mutex_lock(vcpu-kvm-lock);
 - r = kvm_hv_set_msr_pw(vcpu, msr, data);
 + r = kvm_hv_set_msr_pw(vcpu, msr, data, host);
   mutex_unlock(vcpu-kvm-lock);
   return r;
   } else
 - return kvm_hv_set_msr(vcpu, msr, data);
 + return kvm_hv_set_msr(vcpu, msr, data, host);
  }
  
  int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
 index 39aee93..dc49527 100644
 --- a/arch/x86/kvm/hyperv.h
 +++ b/arch/x86/kvm/hyperv.h
 @@ -22,7 +22,7 @@
  #ifndef __ARCH_X86_KVM_HYPERV_H__
  #define __ARCH_X86_KVM_HYPERV_H__
  
 -int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data);
 +int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool 
 host);
  int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
  
  bool kvm_hv_hypercall_enabled(struct kvm *kvm);
 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
 index 111fa83..db4eecb 100644
 --- a/arch/x86/kvm/x86.c
 +++ b/arch/x86/kvm/x86.c
 @@ -2210,7 +2210,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct 
 msr_data *msr_info)
   case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
   case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
   case HV_X64_MSR_CRASH_CTL:
 - return kvm_hv_set_msr_common(vcpu, msr, data);
 + return kvm_hv_set_msr_common(vcpu, msr, data,
 +  msr_info-host_initiated);
   break;
   case MSR_IA32_BBL_CR_CTL3:
   /* Drop writes to this legacy MSR -- see rdmsr
 

This has to be squashed

[PATCH 09/11] kvm/x86: distingiush hyper-v guest crash notification

2015-06-22 Thread Denis V. Lunev 
From: Andrey Smetanin asmeta...@virtuozzo.com

Previous patches allowes userspace to setup Hyper-V crash ctl msr.
This msr should expose HV_X64_MSR_CRASH_CTL_NOTIFY value to Hyper-V
guest to allow to send crash data. Unfortunately Hyper-V guest notifies
hardware about crash by writing the same HV_X64_MSR_CRASH_CTL_NOTIFY value
into crash ctl msr. Thus both user space and guest writes inside ctl msr the
same value and this patch distingiush the moment of actual guest crash
by checking host initiated value from msr info.

Signed-off-by: Andrey Smetanin asmeta...@virtuozzo.com
Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Paolo Bonzini pbonz...@redhat.com
CC: Gleb Natapov g...@kernel.org
---
 arch/x86/kvm/hyperv.c | 17 +
 arch/x86/kvm/hyperv.h |  2 +-
 arch/x86/kvm/x86.c|  3 ++-
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 6b18015..f49502a 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -54,12 +54,12 @@ static int kvm_hv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, 
u64 *pdata)
return 0;
 }
 
-static int kvm_hv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u64 data)
+static int kvm_hv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u64 data, bool host)
 {
struct kvm_arch_hyperv *hv = vcpu-kvm-arch.hyperv;
 
hv-hv_crash_ctl = data;
-   if ((data  HV_X64_MSR_CRASH_CTL_NOTIFY)) {
+   if ((data  HV_X64_MSR_CRASH_CTL_NOTIFY)  !host) {
vcpu_debug(vcpu, hv crash (0x%llx 0x%llx 0x%llx 0x%llx 
  0x%llx)\n, hv-hv_crash_param[0],
  hv-hv_crash_param[1],
@@ -99,7 +99,8 @@ static int kvm_hv_msr_get_crash_data(struct kvm_vcpu *vcpu,
return 0;
 }
 
-static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu,
+u32 msr, u64 data, bool host)
 {
struct kvm *kvm = vcpu-kvm;
struct kvm_arch_hyperv *hv = kvm-arch.hyperv;
@@ -156,7 +157,7 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 
msr, u64 data)
 msr - HV_X64_MSR_CRASH_P0,
 data);
case HV_X64_MSR_CRASH_CTL:
-   return kvm_hv_msr_set_crash_ctl(vcpu, data);
+   return kvm_hv_msr_set_crash_ctl(vcpu, data, host);
default:
vcpu_unimpl(vcpu, Hyper-V unimpl wrmsr: 0x%x data 0x%llx\n,
  msr, data);
@@ -165,7 +166,7 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 
msr, u64 data)
return 0;
 }
 
-static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
 {
struct kvm_vcpu_arch_hyperv *hv = vcpu-arch.hyperv;
 
@@ -278,17 +279,17 @@ static int kvm_hv_get_msr(struct kvm_vcpu *vcpu, u32 msr, 
u64 *pdata)
return 0;
 }
 
-int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
 {
if (kvm_hv_msr_partition_wide(msr)) {
int r;
 
mutex_lock(vcpu-kvm-lock);
-   r = kvm_hv_set_msr_pw(vcpu, msr, data);
+   r = kvm_hv_set_msr_pw(vcpu, msr, data, host);
mutex_unlock(vcpu-kvm-lock);
return r;
} else
-   return kvm_hv_set_msr(vcpu, msr, data);
+   return kvm_hv_set_msr(vcpu, msr, data, host);
 }
 
 int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 39aee93..dc49527 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -22,7 +22,7 @@
 #ifndef __ARCH_X86_KVM_HYPERV_H__
 #define __ARCH_X86_KVM_HYPERV_H__
 
-int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data);
+int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
 int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
 
 bool kvm_hv_hypercall_enabled(struct kvm *kvm);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 111fa83..db4eecb 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2210,7 +2210,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct 
msr_data *msr_info)
case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
case HV_X64_MSR_CRASH_CTL:
-   return kvm_hv_set_msr_common(vcpu, msr, data);
+   return kvm_hv_set_msr_common(vcpu, msr, data,
+msr_info-host_initiated);
break;
case MSR_IA32_BBL_CR_CTL3:
/* Drop writes to this legacy MSR -- see rdmsr
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe kvm in

Re: [PATCH 09/11] kvm/x86: distingiush hyper-v guest crash notification

2015-06-22 Thread Peter Hornyack 
On Mon, Jun 22, 2015 at 9:05 AM, Denis V. Lunev d...@openvz.org wrote:
 From: Andrey Smetanin asmeta...@virtuozzo.com

 Previous patches allowes userspace to setup Hyper-V crash ctl msr.
 This msr should expose HV_X64_MSR_CRASH_CTL_NOTIFY value to Hyper-V
 guest to allow to send crash data. Unfortunately Hyper-V guest notifies
 hardware about crash by writing the same HV_X64_MSR_CRASH_CTL_NOTIFY value
 into crash ctl msr. Thus both user space and guest writes inside ctl msr the
 same value and this patch distingiush the moment of actual guest crash
 by checking host initiated value from msr info.

 Signed-off-by: Andrey Smetanin asmeta...@virtuozzo.com
 Signed-off-by: Denis V. Lunev d...@openvz.org
 CC: Paolo Bonzini pbonz...@redhat.com
 CC: Gleb Natapov g...@kernel.org
 ---
  arch/x86/kvm/hyperv.c | 17 +
  arch/x86/kvm/hyperv.h |  2 +-
  arch/x86/kvm/x86.c|  3 ++-
  3 files changed, 12 insertions(+), 10 deletions(-)

 diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
 index 6b18015..f49502a 100644
 --- a/arch/x86/kvm/hyperv.c
 +++ b/arch/x86/kvm/hyperv.c
 @@ -54,12 +54,12 @@ static int kvm_hv_msr_get_crash_ctl(struct kvm_vcpu 
 *vcpu, u64 *pdata)
 return 0;
  }

 -static int kvm_hv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u64 data)
 +static int kvm_hv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u64 data, bool 
 host)
  {
 struct kvm_arch_hyperv *hv = vcpu-kvm-arch.hyperv;

 hv-hv_crash_ctl = data;

Should we allow hv_crash_ctl to be set if !host? It's a small detail,
but it doesn't seem like the guest should be able to disable crash
actions that userspace has enabled in hv_crash_ctl.

 -   if ((data  HV_X64_MSR_CRASH_CTL_NOTIFY)) {
 +   if ((data  HV_X64_MSR_CRASH_CTL_NOTIFY)  !host) {
 vcpu_debug(vcpu, hv crash (0x%llx 0x%llx 0x%llx 0x%llx 
   0x%llx)\n, hv-hv_crash_param[0],
   hv-hv_crash_param[1],
 @@ -99,7 +99,8 @@ static int kvm_hv_msr_get_crash_data(struct kvm_vcpu *vcpu,
 return 0;
  }

 -static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 +static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu,
 +u32 msr, u64 data, bool host)
  {
 struct kvm *kvm = vcpu-kvm;
 struct kvm_arch_hyperv *hv = kvm-arch.hyperv;
 @@ -156,7 +157,7 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 
 msr, u64 data)
  msr - HV_X64_MSR_CRASH_P0,
  data);
 case HV_X64_MSR_CRASH_CTL:
 -   return kvm_hv_msr_set_crash_ctl(vcpu, data);
 +   return kvm_hv_msr_set_crash_ctl(vcpu, data, host);
 default:
 vcpu_unimpl(vcpu, Hyper-V unimpl wrmsr: 0x%x data 0x%llx\n,
   msr, data);
 @@ -165,7 +166,7 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 
 msr, u64 data)
 return 0;
  }

 -static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 +static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool 
 host)
  {
 struct kvm_vcpu_arch_hyperv *hv = vcpu-arch.hyperv;

 @@ -278,17 +279,17 @@ static int kvm_hv_get_msr(struct kvm_vcpu *vcpu, u32 
 msr, u64 *pdata)
 return 0;
  }

 -int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 +int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool 
 host)
  {
 if (kvm_hv_msr_partition_wide(msr)) {
 int r;

 mutex_lock(vcpu-kvm-lock);
 -   r = kvm_hv_set_msr_pw(vcpu, msr, data);
 +   r = kvm_hv_set_msr_pw(vcpu, msr, data, host);
 mutex_unlock(vcpu-kvm-lock);
 return r;
 } else
 -   return kvm_hv_set_msr(vcpu, msr, data);
 +   return kvm_hv_set_msr(vcpu, msr, data, host);
  }

  int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
 index 39aee93..dc49527 100644
 --- a/arch/x86/kvm/hyperv.h
 +++ b/arch/x86/kvm/hyperv.h
 @@ -22,7 +22,7 @@
  #ifndef __ARCH_X86_KVM_HYPERV_H__
  #define __ARCH_X86_KVM_HYPERV_H__

 -int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data);
 +int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool 
 host);
  int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);

  bool kvm_hv_hypercall_enabled(struct kvm *kvm);
 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
 index 111fa83..db4eecb 100644
 --- a/arch/x86/kvm/x86.c
 +++ b/arch/x86/kvm/x86.c
 @@ -2210,7 +2210,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct 
 msr_data *msr_info)
 case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
 case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
 case HV_X64_MSR_CRASH_CTL:
 -   return kvm_hv_set_msr_common(vcpu, msr, data);
 +