From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43962) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V51lz-00036V-HE for qemu-devel@nongnu.org; Thu, 01 Aug 2013 18:55:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V4slC-0004OP-On for qemu-devel@nongnu.org; Thu, 01 Aug 2013 09:17:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53693) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4sXI-0007q3-IU for qemu-devel@nongnu.org; Thu, 01 Aug 2013 09:03:20 -0400 Date: Thu, 1 Aug 2013 15:03:12 +0200 From: Paolo Bonzini Message-ID: <20130801130312.GG5245@mail.corp.redhat.com> References: <1374764722-10685-1-git-send-email-pbonzini@redhat.com> <1374764722-10685-3-git-send-email-pbonzini@redhat.com> <20130728125745.GA27246@redhat.com> <51F521DD.5010308@redhat.com> <20130728135456.GJ11772@redhat.com> <51F525A9.7030503@redhat.com> <20130728142425.GK11772@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20130728142425.GK11772@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 2/2] kvm: migrate vPMU state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gleb Natapov Cc: qemu-devel@nongnu.org, afaerber@suse.de, ehabkost@redhat.com > KVM disabled HW counters when outside of a guest mode (otherwise result > will be useless), so I do not see how the problem you describe can > happen. Yes, you're right. > On the other hand MPU emulation assumes that counter have to be disabled > while MSR_IA32_PERFCTR0 is written since write to MSR_IA32_PERFCTR0 does > not reprogram perf evens, so we need either disable/enable counters to > write MSR_IA32_PERFCTR0 or have this patch in the kernel: > > diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c > index 5c4f631..bf14e42 100644 > --- a/arch/x86/kvm/pmu.c > +++ b/arch/x86/kvm/pmu.c > @@ -412,6 +412,8 @@ int kvm_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > if (!msr_info->host_initiated) > data = (s64)(s32)data; > pmc->counter += data - read_pmc(pmc); > + if (msr_info->host_initiated) > + reprogram_gp_counter(pmc, pmc->eventsel); > return 0; > } else if ((pmc = get_gp_pmc(pmu, index, MSR_P6_EVNTSEL0))) { > if (data == pmc->eventsel) Why do you need "if (msr_info->host_initiated)"? I could not find any hint in the manual that the overflow counter will still use the value of the counter that was programmed first. If we need to do it always, I agree it's better to modify the QEMU patch and not disable/enable the counters. But if we need to restrict it to host-initiated writes, I would rather have the QEMU patch as I posted it. So far we always had less side-effects from host_initiated, not more, and I think it's a good rule of thumb. Paolo