Hi,
This is a follow up to [0]. By keeping the same semantics of eventsel
for gp and fixed counters, the reprogram code could be made more
symmetrical, simpler and even faster [1], and it also fixes the
obsolescence amd_event_mapping issue [2].
One of the notable changes is that we ended up removing the
reprogram_{gp, fixed}_counter() functions and replacing it with the
merged reprogram_counter(), where KVM programs pmc->perf_event
with only the PERF_TYPE_RAW type for any type of counter
(suggested by Jim as well). PeterZ confirmed the idea, "think so;
the HARDWARE is just a convenience wrapper over RAW IIRC".
Practically, this change drops the guest pmu support on the hosts without
X86_FEATURE_ARCH_PERFMON (the oldest Pentium 4), where the
PERF_TYPE_HARDWAR is intentionally introduced so that hosts can
map the architectural guest PMU events to their own.
Some code refactoring helps to review key changes more easily.
Patches are based on top of kvm/master (ec756e40e271).
The last patch removes the call trace in the commit message while we still
think that kvm->arch.pmu_event_filter requires SRCU protection in terms
of pmu_event_filter functionality, similar to "kvm->arch.msr_filter".
Please check more details in each commit and feel free to comment.
[0] https://lore.kernel.org/kvm/[email protected]/
[1] https://lore.kernel.org/kvm/[email protected]/
[2] https://lore.kernel.org/kvm/[email protected]/
Like Xu (11):
KVM: x86/pmu: Update comments for AMD gp counters
KVM: x86/pmu: Extract check_pmu_event_filter() from the same semantics
KVM: x86/pmu: Pass only "struct kvm_pmc *pmc" to reprogram_counter()
KVM: x86/pmu: Drop "u64 eventsel" for reprogram_gp_counter()
KVM: x86/pmu: Drop "u8 ctrl, int idx" for reprogram_fixed_counter()
KVM: x86/pmu: Use only the uniformly exported interface
reprogram_counter()
KVM: x86/pmu: Use PERF_TYPE_RAW to merge reprogram_{gp,
fixed}counter()
perf: x86/core: Add interface to query perfmon_event_map[] directly
KVM: x86/pmu: Replace pmc_perf_hw_id() with perf_get_hw_event_config()
KVM: x86/pmu: Drop amd_event_mapping[] in the KVM context
KVM: x86/pmu: Protect kvm->arch.pmu_event_filter with SRCU
arch/x86/events/core.c | 11 ++
arch/x86/include/asm/perf_event.h | 6 +
arch/x86/kvm/pmu.c | 188 ++++++++++++------------------
arch/x86/kvm/pmu.h | 6 +-
arch/x86/kvm/svm/pmu.c | 37 +-----
arch/x86/kvm/vmx/pmu_intel.c | 62 +++++-----
6 files changed, 130 insertions(+), 180 deletions(-)
--
2.35.0
From: Like Xu <[email protected]>
The obsolete comment could more accurately state that AMD platforms
have two base MSR addresses and two different maximum numbers
for gp counters, depending on the X86_FEATURE_PERFCTR_CORE feature.
Signed-off-by: Like Xu <[email protected]>
---
arch/x86/kvm/pmu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index b1a02993782b..c4692f0ff87e 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -34,7 +34,7 @@
* However AMD doesn't support fixed-counters;
* - There are three types of index to access perf counters (PMC):
* 1. MSR (named msr): For example Intel has MSR_IA32_PERFCTRn and AMD
- * has MSR_K7_PERFCTRn.
+ * has MSR_F15H_PERF_CTRn or MSR_K7_PERFCTRn.
* 2. MSR Index (named idx): This normally is used by RDPMC instruction.
* For instance AMD RDPMC instruction uses 0000_0003h in ECX to access
* C001_0007h (MSR_K7_PERCTR3). Intel has a similar mechanism, except
@@ -46,7 +46,8 @@
* between pmc and perf counters is as the following:
* * Intel: [0 .. INTEL_PMC_MAX_GENERIC-1] <=> gp counters
* [INTEL_PMC_IDX_FIXED .. INTEL_PMC_IDX_FIXED + 2] <=> fixed
- * * AMD: [0 .. AMD64_NUM_COUNTERS-1] <=> gp counters
+ * * AMD: [0 .. AMD64_NUM_COUNTERS-1] or
+ * [0 .. AMD64_NUM_COUNTERS_CORE-1] <=> gp counters
*/
static void kvm_pmi_trigger_fn(struct irq_work *irq_work)
--
2.35.0
From: Like Xu <[email protected]>
Since afrer reprogram_fixed_counter() is called, it's bound to assign
the requested fixed_ctr_ctrl to pmu->fixed_ctr_ctrl, this assignment step
can be moved forward (the stale value for diff is saved extra early),
thus simplifying the passing of parameters.
No functional change intended.
Signed-off-by: Like Xu <[email protected]>
---
arch/x86/kvm/pmu.c | 13 ++++++-------
arch/x86/kvm/pmu.h | 2 +-
arch/x86/kvm/vmx/pmu_intel.c | 16 ++++++++--------
3 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 482a78956dd0..7c90d5d196a4 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -258,8 +258,11 @@ void reprogram_gp_counter(struct kvm_pmc *pmc)
}
EXPORT_SYMBOL_GPL(reprogram_gp_counter);
-void reprogram_fixed_counter(struct kvm_pmc *pmc, u8 ctrl, int idx)
+void reprogram_fixed_counter(struct kvm_pmc *pmc)
{
+ struct kvm_pmu *pmu = pmc_to_pmu(pmc);
+ int idx = pmc->idx - INTEL_PMC_IDX_FIXED;
+ u8 ctrl = fixed_ctrl_field(pmu->fixed_ctr_ctrl, idx);
unsigned en_field = ctrl & 0x3;
bool pmi = ctrl & 0x8;
@@ -289,12 +292,8 @@ void reprogram_counter(struct kvm_pmc *pmc)
{
if (pmc_is_gp(pmc))
reprogram_gp_counter(pmc);
- else {
- int idx = pmc->idx - INTEL_PMC_IDX_FIXED;
- u8 ctrl = fixed_ctrl_field(pmc_to_pmu(pmc)->fixed_ctr_ctrl, idx);
-
- reprogram_fixed_counter(pmc, ctrl, idx);
- }
+ else
+ reprogram_fixed_counter(pmc);
}
EXPORT_SYMBOL_GPL(reprogram_counter);
diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
index 4db50c290c62..70a982c3cdad 100644
--- a/arch/x86/kvm/pmu.h
+++ b/arch/x86/kvm/pmu.h
@@ -141,7 +141,7 @@ static inline u64 get_sample_period(struct kvm_pmc *pmc, u64 counter_value)
}
void reprogram_gp_counter(struct kvm_pmc *pmc);
-void reprogram_fixed_counter(struct kvm_pmc *pmc, u8 ctrl, int fixed_idx);
+void reprogram_fixed_counter(struct kvm_pmc *pmc);
void reprogram_counter(struct kvm_pmc *pmc);
void kvm_pmu_deliver_pmi(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 1ed7d23d6738..cc4a092f0d67 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -37,23 +37,23 @@ static int fixed_pmc_events[] = {1, 0, 7};
static void reprogram_fixed_counters(struct kvm_pmu *pmu, u64 data)
{
+ struct kvm_pmc *pmc;
+ u8 old_fixed_ctr_ctrl = pmu->fixed_ctr_ctrl;
int i;
+ pmu->fixed_ctr_ctrl = data;
for (i = 0; i < pmu->nr_arch_fixed_counters; i++) {
u8 new_ctrl = fixed_ctrl_field(data, i);
- u8 old_ctrl = fixed_ctrl_field(pmu->fixed_ctr_ctrl, i);
- struct kvm_pmc *pmc;
-
- pmc = get_fixed_pmc(pmu, MSR_CORE_PERF_FIXED_CTR0 + i);
+ u8 old_ctrl = fixed_ctrl_field(old_fixed_ctr_ctrl, i);
if (old_ctrl == new_ctrl)
continue;
- __set_bit(INTEL_PMC_IDX_FIXED + i, pmu->pmc_in_use);
- reprogram_fixed_counter(pmc, new_ctrl, i);
- }
+ pmc = get_fixed_pmc(pmu, MSR_CORE_PERF_FIXED_CTR0 + i);
- pmu->fixed_ctr_ctrl = data;
+ __set_bit(INTEL_PMC_IDX_FIXED + i, pmu->pmc_in_use);
+ reprogram_fixed_counter(pmc);
+ }
}
static struct kvm_pmc *intel_pmc_idx_to_pmc(struct kvm_pmu *pmu, int pmc_idx)
--
2.35.0
On Mon, Feb 21, 2022 at 3:52 AM Like Xu <[email protected]> wrote:
>
> From: Like Xu <[email protected]>
>
> The obsolete comment could more accurately state that AMD platforms
> have two base MSR addresses and two different maximum numbers
> for gp counters, depending on the X86_FEATURE_PERFCTR_CORE feature.
>
> Signed-off-by: Like Xu <[email protected]>
> ---
> arch/x86/kvm/pmu.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index b1a02993782b..c4692f0ff87e 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -34,7 +34,7 @@
> * However AMD doesn't support fixed-counters;
> * - There are three types of index to access perf counters (PMC):
> * 1. MSR (named msr): For example Intel has MSR_IA32_PERFCTRn and AMD
> - * has MSR_K7_PERFCTRn.
> + * has MSR_F15H_PERF_CTRn or MSR_K7_PERFCTRn.
"Or" sounds exclusive. Maybe it would be better to say:
has MSR_K7_PERFCTRn and, for families 15H and later,
MSR_F15H_PERF_CTRn, where MSR_F15H_PERF_CTR[0-3] are aliased to
MSR_K7_PERFCTRn.