Hi Zhao,
On 3/11/25 6:51 AM, Zhao Liu wrote:
> Hi Dongli,
>
>>>> + /*
>>>> + * 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? 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)?
>
>> It works only when "!cpu->enable_pmu" and KVM_CAP_PMU_CAPABILITY exists.
>>
>>
>> We may still need a static global variable here to indicate where
>> "kvm.enable_pmu=N" (as discussed in PATCH 07).
>>
>>>
>>>> + if (IS_INTEL_CPU(env)) {
>>>
>>> Zhaoxin also supports architectural PerfMon in 0xa.
>>>
>>> I'm not sure if this check should also involve Zhaoxin CPU, so cc
>>> zhaoxin guys for double check.
>>
>> Sure for both here and below 'ditto'. Thank you very much!
>
> Per the Linux commit 3a4ac121c2cac, Zhaoxin mostly follows Intel
> Architectural PerfMon-v2. Afterall, before this patch, these PMU things
> didn't check any vendor, so I suppose vPMU may could work for Zhaoxin as
> well. Therefore, its' better to consider Zhaoxin when you check Intel
> CPU, which can help avoid introducing some regressions.
>
Thank you very much!
zhaoxin_pmu_init() looks self explanatory.
Dongli Zhang
