Hi Zhao,
On 3/12/25 1:30 AM, Zhao Liu wrote:
>>>>>> + /*
>>>>>> + * 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.
>
Sure. That makes coding easier, with less assumptions.
Thank you very much!
Dongli Zhang
