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. > 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. Thanks, Zhao