On Sun, Jul 28, 2013 at 04:07:37PM +0200, Paolo Bonzini wrote:
> Il 28/07/2013 15:54, Gleb Natapov ha scritto:
> > On Sun, Jul 28, 2013 at 03:51:25PM +0200, Paolo Bonzini wrote:
> >> Il 28/07/2013 14:57, Gleb Natapov ha scritto:
> >>>> @@ -1114,6 +1135,33 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> >>>> kvm_msr_entry_set(&msrs[n++], MSR_KVM_STEAL_TIME,
> >>>> env->steal_time_msr);
> >>>> }
> >>>> + if (has_msr_architectural_pmu) {
> >>>> + /* Stop the counter. */
> >>>> + kvm_msr_entry_set(&msrs[n++], MSR_CORE_PERF_FIXED_CTR_CTRL,
> >>>> 0);
> >>>> + kvm_msr_entry_set(&msrs[n++], MSR_CORE_PERF_GLOBAL_CTRL, 0);
> >>>> +
> >>> Why is this needed?
> >>
> >> In v1 it was in the commit message. I'll fix it up before applying:
> >>
> >>> Second, to avoid any possible side effects during the setting of MSRs
> >>> I stop the PMU while setting the counters and event selector MSRs.
> >>> Stopping the PMU snapshots the counters and ensures that no strange
> >>> races can happen if the counters were saved close to their overflow
> >>> value.
> >>
> > Since vcpu is not running counters should not count anyway.
>
> Does the perf event distinguish KVM_RUN from any other activity in the
> vCPU thread (in which this code runs)? It seemed unsafe to me to change
> the overflow status and the performance counter value while the counter
> could be running, since the counter value could affect the overflow
> status. Maybe I was being paranoid?
>
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. 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)
--
Gleb.
