Re: [PATCH v2 08/10] target/i386/kvm: reset AMD PMU registers during VM reset

[Zhao Liu <zhao1.liu@intel.com>]

> >>>> +    /*
> >>>> +     * If KVM_CAP_PMU_CAPABILITY is not supported, there is no way to
> >>>> +     * disable the AMD pmu virtualization.
> >>>> +     *
> >>>> +     * If KVM_CAP_PMU_CAPABILITY is supported !cpu->enable_pmu
> >>>> +     * indicates the KVM has already disabled the PMU virtualization.
> >>>> +     */
> >>>> +    if (has_pmu_cap && !cpu->enable_pmu) {
> >>>> +        return;
> >>>> +    }
> >>>
> >>> Could we only check "cpu->enable_pmu" at the beginning of this function?
> >>> then if pmu is already disabled, we don't need to initialize the pmu info.
> >>
> >> I don't think so. There is a case:
> >>
> >> - cpu->enable_pmu = false. (That is, "-cpu host,-pmu").
> >> - But for KVM prior v5.18 that KVM_CAP_PMU_CAPABILITY doesn't exist.
> >>
> >> There is no way to disable vPMU. To determine based on only
> >> "!cpu->enable_pmu" doesn't work.
> > 
> > Ah, I didn't get your point here. When QEMU user has already disabled
> > PMU, why we still need to continue initialize PMU info and save/load PMU
> > MSRs? In this case, user won't expect vPMU could work.
> 
> Yes, "In this case, user won't expect vPMU could work.".
> 
> But in reality vPMU is still active, although that doesn't match user's
> expectation.
> 
> User doesn't expect PMU to work. However, "perf stat" still works in VM
> (when KVM_CAP_PMU_CAPABILITY isn't available).
> 
> Would you suggest we only follow user's expectation?

Yes, for this case, many PMU related CPUIDs have already been disabled
because of "!enable_pmu", so IMO it's not necessary to handle other PMU
MSRs.

> That is, once user
> configure "-pmu", we are going to always assume vPMU is disabled, even it
> is still available (on KVM without KVM_CAP_PMU_CAPABILITY and prior v5.18)?

Strictly speaking, only the earlier AMD PMUs are still AVAILABLE at this
point, as the other platforms, have CPUIDs to indicate PMU enablement.
So for the latter (which I understand is most of the cases nowadays),
there's no reason to assume that the PMUs are still working when the CPUIDs
are corrupted...

There is no perfect solution for pre-v5.18 kernel... But while not breaking
compatibility, again IMO, we need the logic to be self-consistent, i.e.
any time the user does not enable vPMU (enable_pmu = false), it should be
assumed that vPMU does not work.

Thanks,
Zhao