From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59160) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4tFE-0001KB-Dh for qemu-devel@nongnu.org; Thu, 01 Aug 2013 09:48:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V4tF7-00070c-FS for qemu-devel@nongnu.org; Thu, 01 Aug 2013 09:48:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5751) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4tF7-00070E-5c for qemu-devel@nongnu.org; Thu, 01 Aug 2013 09:48:37 -0400 Date: Thu, 1 Aug 2013 15:48:29 +0200 From: Paolo Bonzini Message-ID: <20130801134829.GK5245@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> <20130801130312.GG5245@mail.corp.redhat.com> <20130801131212.GG6042@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20130801131212.GG6042@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 On Aug 01 2013, Gleb Natapov wrote: > On Thu, Aug 01, 2013 at 03:03:12PM +0200, Paolo Bonzini wrote: > > > 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. > > > Not sure I understand. What "overflow counter will still use the value > of the counter that was programmed first" means? > > spec says in no vague terms that counter should be disabled before > writing into the MSR and it means that reprogram_gp_counter() will be > called again when guest will enable counter later, Yeah, I found it now. > I am OK with your patch, it is a little bit unfortunate that userspase > should care about such low level details though. Is it a Reviewed-by? Paolo