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
