Re: [PATCH v2 1/3] KVM: x86/pmu: Make part of the Intel v2 PMU MSRs handling x86 generic

From: Like Xu <like.xu.linux@gmail.com>
To: Jim Mattson <jmattson@google.com>
Cc: Sean Christopherson <seanjc@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] KVM: x86/pmu: Make part of the Intel v2 PMU MSRs handling x86 generic
Date: Thu, 22 Sep 2022 14:20:33 +0800	[thread overview]
Message-ID: <6525670b-eb27-d0a5-2d03-cfa2006d0579@gmail.com> (raw)
In-Reply-To: <856e3332-9f6b-a5f7-c3ec-afe89003cb84@gmail.com>

On 22/9/2022 1:47 pm, Like Xu wrote:
>> Why this new request? It's not in the Intel-specific version of these
>> function that you elide below.
>>
>> Perhaps you could split up the semantic changes from the simple renamings?
> 
> The creation of perf_event is delayed to the last step,
> https://lore.kernel.org/kvm/20220831085328.45489-5-likexu@tencent.com/
> 
> Based on the new code base, no semantic changes I assume.

Sorry, I think we do need one more clear commit like this:

---

 From e08b2d03a652e5ec226d8907c7648bff57f31d3b Mon Sep 17 00:00:00 2001
From: Like Xu <likexu@tencent.com>
Date: Thu, 22 Sep 2022 14:15:18 +0800
Subject: [PATCH] KVM: x86/pmu: Rewrite reprogram_counters() to improve
  performance

Before using pmu->reprogram_pmi, the test for a valid pmc is always
applied. This part of the redundancy could be removed by setting the
counters' bitmask directly, and furthermore triggering KVM_REQ_PMU
only once to save more cycles.

Signed-off-by: Like Xu <likexu@tencent.com>
---
  arch/x86/kvm/pmu.h | 12 ++++--------
  1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
index fb8040854f4d..9b8e74ccd18a 100644
--- a/arch/x86/kvm/pmu.h
+++ b/arch/x86/kvm/pmu.h
@@ -204,15 +204,11 @@ static inline bool pmc_is_enabled_globally(struct kvm_pmc 
*pmc)
  	return test_bit(pmc->idx, (unsigned long *)&pmu->global_ctrl);
  }

-static void reprogram_counters(struct kvm_pmu *pmu, u64 diff)
+static inline void reprogram_counters(struct kvm_pmu *pmu, u64 diff)
  {
-	int bit;
-	struct kvm_pmc *pmc;
-
-	for_each_set_bit(bit, (unsigned long *)&diff, X86_PMC_IDX_MAX) {
-		pmc = intel_pmc_idx_to_pmc(pmu, bit);
-		if (pmc)
-			kvm_pmu_request_counter_reprogam(pmc);
+	if (diff) {
+		pmu->reprogram_pmi |= diff;
+		kvm_make_request(KVM_REQ_PMU, pmu_to_vcpu(pmu));
  	}
  }

-- 
2.37.3

