On Tue, May 30, 2023, Like Xu wrote: > From: Like Xu <likexu@xxxxxxxxxxx> > > AMD PerfMonV2 defines three registers similar to part of the Intel > v2 PMU registers, including the GLOBAL_CTRL, GLOBAL_STATUS and > GLOBAL_OVF_CTRL MSRs. For better code reuse, this specific part of > the handling can be extracted to make it generic for X86 as a straight > code movement. > > Specifically, the kvm_pmu_set/get_msr() handlers of GLOBAL_STATUS, > GLOBAL_CTRL, GLOBAL_OVF_CTRL defined for Intel are moved to generic > pmu.c and the callback function .pmc_is_globally_enabled is removed, > which is very helpful to introduce the AMD PerfMonV2 code later. Yeah, except this patch doesn't actually move anything. *Some* of the common bits show up in pmu.c, but the same bits in pmu_intel.c get left behind. > The new eponymous pmc_is_globally_enabled() works well as legacy AMD > vPMU version is indexed as 1. Note that the specific *_is_valid_msr will > continue to be used to avoid cross-vendor MSR access. This should be two patches. Moving the GLOBAL_CTRL stuff is logically separate from moving the pmc_is_enabled() code. > Signed-off-by: Like Xu <likexu@xxxxxxxxxxx> > --- > @@ -213,6 +212,22 @@ static inline void reprogram_counters(struct kvm_pmu *pmu, u64 diff) > kvm_make_request(KVM_REQ_PMU, pmu_to_vcpu(pmu)); > } > > +/* > + * Check if a PMC is enabled by comparing it against global_ctrl bits. > + * > + * If the current version of vPMU doesn't have global_ctrl MSR, > + * all vPMCs are enabled (return TRUE). > + */ > +static inline bool pmc_is_globally_enabled(struct kvm_pmc *pmc) > +{ > + struct kvm_pmu *pmu = pmc_to_pmu(pmc); > + > + if (pmu->version < 2) Nah, we're not open coding this check, not after putting in the effort to squash the mess of open coding on Intel. Moving intel_pmu_has_perf_global_ctrl() is trivial, and also allows moving the existence checks into kvm_pmu_is_valid_msr().