From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60184) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7KoQ-0000xz-Pk for qemu-devel@nongnu.org; Tue, 23 Jun 2015 05:48:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7KoN-00046O-Jo for qemu-devel@nongnu.org; Tue, 23 Jun 2015 05:48:14 -0400 Received: from mx2.parallels.com ([199.115.105.18]:44421) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7KoN-00045U-BF for qemu-devel@nongnu.org; Tue, 23 Jun 2015 05:48:11 -0400 Message-ID: <55892B49.3040507@openvz.org> Date: Tue, 23 Jun 2015 12:47:53 +0300 From: "Denis V. Lunev" MIME-Version: 1.0 References: <1434989108-20924-1-git-send-email-den@openvz.org> <1434989108-20924-8-git-send-email-den@openvz.org> In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 07/11] kvm/x86: added hyper-v crash data and ctl msr's get/set'ers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Andrey Smetanin , Gleb Natapov , qemu-devel@nongnu.org, kvm@vger.kernel.org, Peter Hornyack On 23/06/15 02:52, Peter Hornyack wrote: > On Mon, Jun 22, 2015 at 9:05 AM, Denis V. Lunev wrote: >> From: Andrey Smetanin >> >> Added hyper-v crash msr's(HV_X64_MSR_CRASH*) data and control >> geters and setters. >> >> Signed-off-by: Andrey Smetanin >> Signed-off-by: Denis V. Lunev >> CC: Paolo Bonzini >> CC: Gleb Natapov >> --- >> arch/x86/kvm/hyperv.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> arch/x86/kvm/x86.c | 4 ++++ >> 2 files changed, 70 insertions(+) >> >> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c >> index f65fb622..0a7d373 100644 >> --- a/arch/x86/kvm/hyperv.c >> +++ b/arch/x86/kvm/hyperv.c >> @@ -46,6 +46,59 @@ static bool kvm_hv_msr_partition_wide(u32 msr) >> return r; >> } >> >> +static int kvm_hv_msr_get_crash_ctl(struct kvm_vcpu *vcpu, u64 *pdata) >> +{ >> + struct kvm_arch_hyperv *hv = &vcpu->kvm->arch.hyperv; >> + >> + *pdata = hv->hv_crash_ctl; > I see that this is implemented so that userspace controls the Hyper-V > crash capabilities that are enabled - userspace must set > HV_X64_MSR_CRASH_CTL_NOTIFY before the guest reads > HV_X64_MSR_CRASH_CTL. I just want to check that you considered an > alternative: a simpler implementation would have the crash > capabilities statically defined by kvm, and > HV_X64_MSR_CRASH_CTL_CONTENTS could just be returned here (and > hv_crash_ctl could be removed from struct kvm_arch_hyperv). > > The current implementation is potentially more flexible but makes the > MSR handling a little more awkward since the host_initiated bool needs > to be passed around (patch 09). I guess either approach seems ok to > me. > > Also, if this patchset is used then it looks like > HV_X64_MSR_CRASH_CTL_CONTENTS can be removed. Paolo, do you have any opinion on this? I prefer configurable way but there is no problem to enable it by default dropping 'hv_panic' property. Regards, Den >> + return 0; >> +} >> + >> +static int kvm_hv_msr_set_crash_ctl(struct kvm_vcpu *vcpu, u64 data) >> +{ >> + struct kvm_arch_hyperv *hv = &vcpu->kvm->arch.hyperv; >> + >> + hv->hv_crash_ctl = data; >> + if ((data & HV_X64_MSR_CRASH_CTL_NOTIFY)) { >> + 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], >> + hv->hv_crash_param[2], >> + hv->hv_crash_param[3], >> + hv->hv_crash_param[4]); >> + >> + /* Send notification about crash to user space */ >> + kvm_make_request(KVM_REQ_HV_CRASH, vcpu); >> + return 0; > Returning from here seems unnecessary - if more crash capabilities are > added in the future, the guest might want to invoke multiple > capabilities at once, so we'd want to check for those here too. > >> + } >> + >> + return 0; >> +} >> + >> +static int kvm_hv_msr_set_crash_data(struct kvm_vcpu *vcpu, >> + u32 index, u64 data) >> +{ >> + struct kvm_arch_hyperv *hv = &vcpu->kvm->arch.hyperv; >> + >> + if (WARN_ON_ONCE(index >= ARRAY_SIZE(hv->hv_crash_param))) >> + return -EINVAL; >> + >> + hv->hv_crash_param[index] = data; >> + return 0; >> +} >> + >> +static int kvm_hv_msr_get_crash_data(struct kvm_vcpu *vcpu, >> + u32 index, u64 *pdata) >> +{ >> + struct kvm_arch_hyperv *hv = &vcpu->kvm->arch.hyperv; >> + >> + if (WARN_ON_ONCE(index >= ARRAY_SIZE(hv->hv_crash_param))) >> + return -EINVAL; >> + >> + *pdata = hv->hv_crash_param[index]; >> + return 0; >> +} >> + >> static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data) >> { >> struct kvm *kvm = vcpu->kvm; >> @@ -98,6 +152,12 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data) >> mark_page_dirty(kvm, gfn); >> break; >> } >> + case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4: >> + return kvm_hv_msr_set_crash_data(vcpu, >> + msr - HV_X64_MSR_CRASH_P0, >> + data); >> + case HV_X64_MSR_CRASH_CTL: >> + return kvm_hv_msr_set_crash_ctl(vcpu, data); >> default: >> vcpu_unimpl(vcpu, "Hyper-V unimpl wrmsr: 0x%x data 0x%llx\n", >> msr, data); >> @@ -170,6 +230,12 @@ static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata) >> case HV_X64_MSR_REFERENCE_TSC: >> data = hv->hv_tsc_page; >> break; >> + case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4: >> + return kvm_hv_msr_get_crash_data(vcpu, >> + msr - HV_X64_MSR_CRASH_P0, >> + pdata); >> + case HV_X64_MSR_CRASH_CTL: >> + return kvm_hv_msr_get_crash_ctl(vcpu, pdata); >> default: >> vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr); >> return 1; >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 2755c37..2046b78 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -2208,6 +2208,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> */ >> break; >> 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); >> break; >> case MSR_IA32_BBL_CR_CTL3: >> @@ -2451,6 +2453,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata) >> data = 0x20000000; >> break; >> 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_get_msr_common(vcpu, msr, pdata); >> break; >> case MSR_IA32_BBL_CR_CTL3: >> -- >> 1.9.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in