| Message ID | 1434989108-20924-10-git-send-email-den@openvz.org |
|---|---|
| State | New |
| Headers | show |
On 22/06/2015 18:05, Denis V. Lunev wrote: > From: Andrey Smetanin <asmetanin@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 <asmetanin@virtuozzo.com> > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Paolo Bonzini <pbonzini@redhat.com> > CC: Gleb Natapov <gleb@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 into patch 7. The patches looked good so far, so I can do this myself, but I'm dropping a note here in case I find something on a second read and you need to do a v3. Paolo
On Mon, Jun 22, 2015 at 9:05 AM, Denis V. Lunev <den@openvz.org> wrote: > From: Andrey Smetanin <asmetanin@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 <asmetanin@virtuozzo.com> > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Paolo Bonzini <pbonzini@redhat.com> > CC: Gleb Natapov <gleb@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); > + 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
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
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