Add support for using AMD Performance Monitoring Version 2
(PerfMonV2) features on upcoming processors. New CPU features
are introduced for PerfMonV2 detection. New MSR definitions
are added to make use of an alternative PMC management scheme
based on the new PMC global control and status registers.
The global control register provides the ability to start and
stop multiple PMCs at the same time. This makes it possible
to enable or disable all counters with a single MSR write
instead of writing to the individual PMC control registers
iteratively under x86_pmu_{enable,disable}(). The effects
can be seen when counting the same events across multiple
PMCs.
E.g.
$ sudo perf stat -e "{cycles,instructions,cycles,instructions}" sleep 1
Before:
Performance counter stats for 'sleep 1':
1013281 cycles
1452859 instructions # 1.43 insn per cycle
1023462 cycles
1461724 instructions # 1.43 insn per cycle
1.001644276 seconds time elapsed
0.001948000 seconds user
0.000000000 seconds sys
After:
Performance counter stats for 'sleep 1':
999165 cycles
1440456 instructions # 1.44 insn per cycle
999165 cycles
1440456 instructions # 1.44 insn per cycle
1.001879504 seconds time elapsed
0.001817000 seconds user
0.000000000 seconds sys
No additional failures are seen upon running the following:
* perf built-in test suite
* perf_event_tests suite
* rr test suite
Previous versions can be found at:
v3: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/
Changes in v4:
- Use ARCH_PERFMON feature bit instead of vendor info to determine the
availability of CPUID leaf 0xA.
Changes in v3:
- Remove unused parameter from amd_pmu_cpu_reset().
- Add Hygon as a vendor that does not support CPUID leaf 0xA.
Changes in v2:
- Sort PerfCntrGlobal* register definitions based on MSR index.
- Use wrmsrl() in cpu_{starting,dead}().
- Add enum to extract bitfields from CPUID leaf 0x80000022.
- Remove static calls for counter management functions.
- Stop counters before inspecting overflow status in NMI handler.
- Save and restore PMU enabled state in NMI handler.
- Remove unused variable in NMI handler.
- Remove redundant write to APIC_LVTPC in NMI handler.
- Add comment on APIC_LVTPC mask bit behaviour during counter overflow.
Sandipan Das (7):
x86/cpufeatures: Add PerfMonV2 feature bit
x86/msr: Add PerfCntrGlobal* registers
perf/x86/amd/core: Detect PerfMonV2 support
perf/x86/amd/core: Detect available counters
perf/x86/amd/core: Add PerfMonV2 counter control
perf/x86/amd/core: Add PerfMonV2 overflow handling
kvm: x86/cpuid: Fix CPUID leaf 0xA
arch/x86/events/amd/core.c | 227 +++++++++++++++++++++++++++--
arch/x86/include/asm/cpufeatures.h | 2 +-
arch/x86/include/asm/msr-index.h | 5 +
arch/x86/include/asm/perf_event.h | 17 +++
arch/x86/kernel/cpu/scattered.c | 1 +
arch/x86/kvm/cpuid.c | 5 +
6 files changed, 240 insertions(+), 17 deletions(-)
--
2.34.1
Add MSR definitions that will be used to enable the new AMD
Performance Monitoring Version 2 (PerfMonV2) features. These
include:
* Performance Counter Global Control (PerfCntrGlobalCtl)
* Performance Counter Global Status (PerfCntrGlobalStatus)
* Performance Counter Global Status Clear (PerfCntrGlobalStatusClr)
The new Performance Counter Global Control and Status MSRs
provide an interface for enabling or disabling multiple
counters at the same time and for testing overflow without
probing the individual registers for each PMC.
The availability of these registers is indicated through the
PerfMonV2 feature bit of CPUID leaf 0x80000022 EAX.
Signed-off-by: Sandipan Das <[email protected]>
---
arch/x86/include/asm/msr-index.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 9e2e7185fc1d..a040f4af93c9 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -527,6 +527,11 @@
#define AMD_CPPC_DES_PERF(x) (((x) & 0xff) << 16)
#define AMD_CPPC_ENERGY_PERF_PREF(x) (((x) & 0xff) << 24)
+/* AMD Performance Counter Global Status and Control MSRs */
+#define MSR_AMD64_PERF_CNTR_GLOBAL_STATUS 0xc0000300
+#define MSR_AMD64_PERF_CNTR_GLOBAL_CTL 0xc0000301
+#define MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR 0xc0000302
+
/* Fam 17h MSRs */
#define MSR_F17H_IRPERF 0xc00000e9
--
2.34.1
AMD Performance Monitoring Version 2 (PerfMonV2) introduces
some new Core PMU features such as detection of the number
of available PMCs and managing PMCs using global registers
namely, PerfCntrGlobalCtl and PerfCntrGlobalStatus.
Clearing PerfCntrGlobalCtl and PerfCntrGlobalStatus ensures
that all PMCs are inactive and have no pending overflows
when CPUs are onlined or offlined.
The PMU version (x86_pmu.version) now indicates PerfMonV2
support and will be used to bypass the new features on
unsupported processors.
Signed-off-by: Sandipan Das <[email protected]>
---
arch/x86/events/amd/core.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 8e1e818f8195..4a61257fc58d 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -19,6 +19,9 @@ static unsigned long perf_nmi_window;
#define AMD_MERGE_EVENT ((0xFULL << 32) | 0xFFULL)
#define AMD_MERGE_EVENT_ENABLE (AMD_MERGE_EVENT | ARCH_PERFMON_EVENTSEL_ENABLE)
+/* PMC Enable and Overflow bits for PerfCntrGlobal* registers */
+static u64 amd_pmu_global_cntr_mask __read_mostly;
+
static __initconst const u64 amd_hw_cache_event_ids
[PERF_COUNT_HW_CACHE_MAX]
[PERF_COUNT_HW_CACHE_OP_MAX]
@@ -578,6 +581,18 @@ static struct amd_nb *amd_alloc_nb(int cpu)
return nb;
}
+static void amd_pmu_cpu_reset(void)
+{
+ if (x86_pmu.version < 2)
+ return;
+
+ /* Clear enable bits i.e. PerfCntrGlobalCtl.PerfCntrEn */
+ wrmsrl(MSR_AMD64_PERF_CNTR_GLOBAL_CTL, 0);
+
+ /* Clear overflow bits i.e. PerfCntrGLobalStatus.PerfCntrOvfl */
+ wrmsrl(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR, amd_pmu_global_cntr_mask);
+}
+
static int amd_pmu_cpu_prepare(int cpu)
{
struct cpu_hw_events *cpuc = &per_cpu(cpu_hw_events, cpu);
@@ -625,6 +640,7 @@ static void amd_pmu_cpu_starting(int cpu)
cpuc->amd_nb->refcnt++;
amd_brs_reset();
+ amd_pmu_cpu_reset();
}
static void amd_pmu_cpu_dead(int cpu)
@@ -644,6 +660,8 @@ static void amd_pmu_cpu_dead(int cpu)
cpuhw->amd_nb = NULL;
}
+
+ amd_pmu_cpu_reset();
}
/*
@@ -1185,6 +1203,15 @@ static int __init amd_core_pmu_init(void)
x86_pmu.eventsel = MSR_F15H_PERF_CTL;
x86_pmu.perfctr = MSR_F15H_PERF_CTR;
x86_pmu.num_counters = AMD64_NUM_COUNTERS_CORE;
+
+ /* Check for Performance Monitoring v2 support */
+ if (boot_cpu_has(X86_FEATURE_PERFMON_V2)) {
+ /* Update PMU version for later usage */
+ x86_pmu.version = 2;
+
+ amd_pmu_global_cntr_mask = (1ULL << x86_pmu.num_counters) - 1;
+ }
+
/*
* AMD Core perfctr has separate MSRs for the NB events, see
* the amd/uncore.c driver.
--
2.34.1
If AMD Performance Monitoring Version 2 (PerfMonV2) is
supported, use CPUID leaf 0x80000022 EBX to detect the
number of Core PMCs. This offers more flexibility if the
counts change in later processor families.
Signed-off-by: Sandipan Das <[email protected]>
---
arch/x86/events/amd/core.c | 6 ++++++
arch/x86/include/asm/perf_event.h | 17 +++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 4a61257fc58d..61a2fce99aa1 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -1186,6 +1186,7 @@ static const struct attribute_group *amd_attr_update[] = {
static int __init amd_core_pmu_init(void)
{
+ union cpuid_0x80000022_ebx ebx;
u64 even_ctr_mask = 0ULL;
int i;
@@ -1206,9 +1207,14 @@ static int __init amd_core_pmu_init(void)
/* Check for Performance Monitoring v2 support */
if (boot_cpu_has(X86_FEATURE_PERFMON_V2)) {
+ ebx.full = cpuid_ebx(EXT_PERFMON_DEBUG_FEATURES);
+
/* Update PMU version for later usage */
x86_pmu.version = 2;
+ /* Find the number of available Core PMCs */
+ x86_pmu.num_counters = ebx.split.num_core_pmc;
+
amd_pmu_global_cntr_mask = (1ULL << x86_pmu.num_counters) - 1;
}
diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index a5dea5da1b52..7aa1d420c779 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -186,6 +186,18 @@ union cpuid28_ecx {
unsigned int full;
};
+/*
+ * AMD "Extended Performance Monitoring and Debug" CPUID
+ * detection/enumeration details:
+ */
+union cpuid_0x80000022_ebx {
+ struct {
+ /* Number of Core Performance Counters */
+ unsigned int num_core_pmc:4;
+ } split;
+ unsigned int full;
+};
+
struct x86_pmu_capability {
int version;
int num_counters_gp;
@@ -372,6 +384,11 @@ struct pebs_xmm {
u64 xmm[16*2]; /* two entries for each register */
};
+/*
+ * AMD Extended Performance Monitoring and Debug cpuid feature detection
+ */
+#define EXT_PERFMON_DEBUG_FEATURES 0x80000022
+
/*
* IBS cpuid feature detection
*/
--
2.34.1
If AMD Performance Monitoring Version 2 (PerfMonV2) is
supported, use a new scheme to manage the Core PMCs using
the new global control and status registers. This will be
bypassed on unsupported hardware (x86_pmu.version < 2).
Currently, all PMCs have dedicated control (PERF_CTL) and
counter (PERF_CTR) registers. For a given PMC, the enable
(En) bit of its PERF_CTL register is used to start or stop
counting.
The Performance Counter Global Control (PerfCntrGlobalCtl)
register has enable (PerfCntrEn) bits for each PMC. For a
PMC to start counting, both PERF_CTL and PerfCntrGlobalCtl
enable bits must be set. If either of those are cleared,
the PMC stops counting.
In x86_pmu_{en,dis}able_all(), the PERF_CTL registers of
all active PMCs are written to in a loop. Ideally, PMCs
counting the same event that were started and stopped at
the same time should record the same counts. Due to delays
in between writes to the PERF_CTL registers across loop
iterations, the PMCs cannot be enabled or disabled at the
same instant and hence, record slightly different counts.
This is fixed by enabling or disabling all active PMCs at
the same time with a single write to the PerfCntrGlobalCtl
register.
Signed-off-by: Sandipan Das <[email protected]>
---
arch/x86/events/amd/core.c | 50 ++++++++++++++++++++++++++++++++++----
1 file changed, 45 insertions(+), 5 deletions(-)
diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 61a2fce99aa1..5b100a5f8489 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -664,6 +664,11 @@ static void amd_pmu_cpu_dead(int cpu)
amd_pmu_cpu_reset();
}
+static inline void amd_pmu_set_global_ctl(u64 ctl)
+{
+ wrmsrl(MSR_AMD64_PERF_CNTR_GLOBAL_CTL, ctl);
+}
+
/*
* When a PMC counter overflows, an NMI is used to process the event and
* reset the counter. NMI latency can result in the counter being updated
@@ -693,15 +698,11 @@ static void amd_pmu_wait_on_overflow(int idx)
}
}
-static void amd_pmu_disable_all(void)
+static void amd_pmu_check_overflow(void)
{
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
int idx;
- amd_brs_disable_all();
-
- x86_pmu_disable_all();
-
/*
* This shouldn't be called from NMI context, but add a safeguard here
* to return, since if we're in NMI context we can't wait for an NMI
@@ -748,6 +749,26 @@ static void amd_pmu_enable_all(int added)
}
}
+static void amd_pmu_v2_enable_event(struct perf_event *event)
+{
+ struct hw_perf_event *hwc = &event->hw;
+
+ /*
+ * Testing cpu_hw_events.enabled should be skipped in this case unlike
+ * in x86_pmu_enable_event().
+ *
+ * Since cpu_hw_events.enabled is set only after returning from
+ * x86_pmu_start(), the PMCs must be programmed and kept ready.
+ * Counting starts only after x86_pmu_enable_all() is called.
+ */
+ __x86_pmu_enable_event(hwc, ARCH_PERFMON_EVENTSEL_ENABLE);
+}
+
+static void amd_pmu_v2_enable_all(int added)
+{
+ amd_pmu_set_global_ctl(amd_pmu_global_cntr_mask);
+}
+
static void amd_pmu_disable_event(struct perf_event *event)
{
x86_pmu_disable_event(event);
@@ -765,6 +786,20 @@ static void amd_pmu_disable_event(struct perf_event *event)
amd_pmu_wait_on_overflow(event->hw.idx);
}
+static void amd_pmu_disable_all(void)
+{
+ amd_brs_disable_all();
+ x86_pmu_disable_all();
+ amd_pmu_check_overflow();
+}
+
+static void amd_pmu_v2_disable_all(void)
+{
+ /* Disable all PMCs */
+ amd_pmu_set_global_ctl(0);
+ amd_pmu_check_overflow();
+}
+
static void amd_pmu_add_event(struct perf_event *event)
{
if (needs_branch_stack(event))
@@ -1216,6 +1251,11 @@ static int __init amd_core_pmu_init(void)
x86_pmu.num_counters = ebx.split.num_core_pmc;
amd_pmu_global_cntr_mask = (1ULL << x86_pmu.num_counters) - 1;
+
+ /* Update PMC handling functions */
+ x86_pmu.enable_all = amd_pmu_v2_enable_all;
+ x86_pmu.disable_all = amd_pmu_v2_disable_all;
+ x86_pmu.enable = amd_pmu_v2_enable_event;
}
/*
--
2.34.1
If AMD Performance Monitoring Version 2 (PerfMonV2) is
supported, use a new scheme to process Core PMC overflows
in the NMI handler using the new global control and status
registers. This will be bypassed on unsupported hardware
(x86_pmu.version < 2).
In x86_pmu_handle_irq(), overflows are detected by testing
the contents of the PERF_CTR register for each active PMC in
a loop. The new scheme instead inspects the overflow bits of
the global status register.
The Performance Counter Global Status (PerfCntrGlobalStatus)
register has overflow (PerfCntrOvfl) bits for each PMC. This
is, however, a read-only MSR. To acknowledge that overflows
have been processed, the NMI handler must clear the bits by
writing to the PerfCntrGlobalStatusClr register.
In x86_pmu_handle_irq(), PMCs counting the same event that
are started and stopped at the same time record slightly
different counts due to delays in between reads from the
PERF_CTR registers. This is fixed by stopping and starting
the PMCs at the same before and with a single write to the
Performance Counter Global Control (PerfCntrGlobalCtl) upon
entering and before exiting the NMI handler.
Signed-off-by: Sandipan Das <[email protected]>
---
arch/x86/events/amd/core.c | 144 ++++++++++++++++++++++++++++++++++---
1 file changed, 133 insertions(+), 11 deletions(-)
diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 5b100a5f8489..5de2c833acf2 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -8,6 +8,7 @@
#include <linux/delay.h>
#include <linux/jiffies.h>
#include <asm/apicdef.h>
+#include <asm/apic.h>
#include <asm/nmi.h>
#include "../perf_event.h"
@@ -669,6 +670,45 @@ static inline void amd_pmu_set_global_ctl(u64 ctl)
wrmsrl(MSR_AMD64_PERF_CNTR_GLOBAL_CTL, ctl);
}
+static inline u64 amd_pmu_get_global_status(void)
+{
+ u64 status;
+
+ /* PerfCntrGlobalStatus is read-only */
+ rdmsrl(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS, status);
+
+ return status & amd_pmu_global_cntr_mask;
+}
+
+static inline void amd_pmu_ack_global_status(u64 status)
+{
+ /*
+ * PerfCntrGlobalStatus is read-only but an overflow acknowledgment
+ * mechanism exists; writing 1 to a bit in PerfCntrGlobalStatusClr
+ * clears the same bit in PerfCntrGlobalStatus
+ */
+
+ /* Only allow modifications to PerfCntrGlobalStatus.PerfCntrOvfl */
+ status &= amd_pmu_global_cntr_mask;
+ wrmsrl(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR, status);
+}
+
+static bool amd_pmu_test_overflow_topbit(int idx)
+{
+ u64 counter;
+
+ rdmsrl(x86_pmu_event_addr(idx), counter);
+
+ return !(counter & BIT_ULL(x86_pmu.cntval_bits - 1));
+}
+
+static bool amd_pmu_test_overflow_status(int idx)
+{
+ return amd_pmu_get_global_status() & BIT_ULL(idx);
+}
+
+DEFINE_STATIC_CALL(amd_pmu_test_overflow, amd_pmu_test_overflow_topbit);
+
/*
* When a PMC counter overflows, an NMI is used to process the event and
* reset the counter. NMI latency can result in the counter being updated
@@ -681,7 +721,6 @@ static inline void amd_pmu_set_global_ctl(u64 ctl)
static void amd_pmu_wait_on_overflow(int idx)
{
unsigned int i;
- u64 counter;
/*
* Wait for the counter to be reset if it has overflowed. This loop
@@ -689,8 +728,7 @@ static void amd_pmu_wait_on_overflow(int idx)
* forever...
*/
for (i = 0; i < OVERFLOW_WAIT_COUNT; i++) {
- rdmsrl(x86_pmu_event_addr(idx), counter);
- if (counter & (1ULL << (x86_pmu.cntval_bits - 1)))
+ if (!static_call(amd_pmu_test_overflow)(idx))
break;
/* Might be in IRQ context, so can't sleep */
@@ -830,6 +868,24 @@ static void amd_pmu_del_event(struct perf_event *event)
* handled a counter. When an un-handled NMI is received, it will be claimed
* only if arriving within that window.
*/
+static inline int amd_pmu_adjust_nmi_window(int handled)
+{
+ /*
+ * If a counter was handled, record a timestamp such that un-handled
+ * NMIs will be claimed if arriving within that window.
+ */
+ if (handled) {
+ this_cpu_write(perf_nmi_tstamp, jiffies + perf_nmi_window);
+
+ return handled;
+ }
+
+ if (time_after(jiffies, this_cpu_read(perf_nmi_tstamp)))
+ return NMI_DONE;
+
+ return NMI_HANDLED;
+}
+
static int amd_pmu_handle_irq(struct pt_regs *regs)
{
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
@@ -857,20 +913,84 @@ static int amd_pmu_handle_irq(struct pt_regs *regs)
if (pmu_enabled)
amd_pmu_enable_all(0);
+ return amd_pmu_adjust_nmi_window(handled);
+}
+
+static int amd_pmu_v2_handle_irq(struct pt_regs *regs)
+{
+ struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+ struct perf_sample_data data;
+ struct hw_perf_event *hwc;
+ struct perf_event *event;
+ int handled = 0, idx;
+ u64 status, mask;
+ bool pmu_enabled;
+
/*
- * If a counter was handled, record a timestamp such that un-handled
- * NMIs will be claimed if arriving within that window.
+ * Save the PMU state as it needs to be restored when leaving the
+ * handler
*/
- if (handled) {
- this_cpu_write(perf_nmi_tstamp, jiffies + perf_nmi_window);
+ pmu_enabled = cpuc->enabled;
+ cpuc->enabled = 0;
- return handled;
+ /* Stop counting */
+ amd_pmu_v2_disable_all();
+
+ status = amd_pmu_get_global_status();
+
+ /* Check if any overflows are pending */
+ if (!status)
+ goto done;
+
+ for (idx = 0; idx < x86_pmu.num_counters; idx++) {
+ if (!test_bit(idx, cpuc->active_mask))
+ continue;
+
+ event = cpuc->events[idx];
+ hwc = &event->hw;
+ x86_perf_event_update(event);
+ mask = BIT_ULL(idx);
+
+ if (!(status & mask))
+ continue;
+
+ /* Event overflow */
+ handled++;
+ perf_sample_data_init(&data, 0, hwc->last_period);
+
+ if (!x86_perf_event_set_period(event))
+ continue;
+
+ if (perf_event_overflow(event, &data, regs))
+ x86_pmu_stop(event, 0);
+
+ status &= ~mask;
}
- if (time_after(jiffies, this_cpu_read(perf_nmi_tstamp)))
- return NMI_DONE;
+ /*
+ * It should never be the case that some overflows are not handled as
+ * the corresponding PMCs are expected to be inactive according to the
+ * active_mask
+ */
+ WARN_ON(status > 0);
- return NMI_HANDLED;
+ /* Clear overflow bits */
+ amd_pmu_ack_global_status(~status);
+
+ /*
+ * Unmasking the LVTPC is not required as the Mask (M) bit of the LVT
+ * PMI entry is not set by the local APIC when a PMC overflow occurs
+ */
+ inc_irq_stat(apic_perf_irqs);
+
+done:
+ cpuc->enabled = pmu_enabled;
+
+ /* Resume counting only if PMU is active */
+ if (pmu_enabled)
+ amd_pmu_v2_enable_all(0);
+
+ return amd_pmu_adjust_nmi_window(handled);
}
static struct event_constraint *
@@ -1256,6 +1376,8 @@ static int __init amd_core_pmu_init(void)
x86_pmu.enable_all = amd_pmu_v2_enable_all;
x86_pmu.disable_all = amd_pmu_v2_disable_all;
x86_pmu.enable = amd_pmu_v2_enable_event;
+ x86_pmu.handle_irq = amd_pmu_v2_handle_irq;
+ static_call_update(amd_pmu_test_overflow, amd_pmu_test_overflow_status);
}
/*
--
2.34.1
On some x86 processors, CPUID leaf 0xA provides information
on Architectural Performance Monitoring features. It
advertises a PMU version which Qemu uses to determine the
availability of additional MSRs to manage the PMCs.
Upon receiving a KVM_GET_SUPPORTED_CPUID ioctl request for
the same, the kernel constructs return values based on the
x86_pmu_capability irrespective of the vendor.
This leaf and the additional MSRs are not supported on AMD
and Hygon processors. If AMD PerfMonV2 is detected, the PMU
version is set to 2 and guest startup breaks because of an
attempt to access a non-existent MSR. Return zeros to avoid
this.
Fixes: a6c06ed1a60a ("KVM: Expose the architectural performance monitoring CPUID leaf")
Reported-by: Vasant Hegde <[email protected]>
Signed-off-by: Sandipan Das <[email protected]>
---
arch/x86/kvm/cpuid.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 4b62d80bb22f..e66ebb747084 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -872,6 +872,11 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
union cpuid10_eax eax;
union cpuid10_edx edx;
+ if (!static_cpu_has(X86_FEATURE_ARCH_PERFMON)) {
+ entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
+ break;
+ }
+
perf_get_x86_pmu_capability(&cap);
/*
--
2.34.1
On Wed, Apr 27, 2022 at 05:01:42PM +0530, Sandipan Das wrote:
> Sandipan Das (7):
> x86/cpufeatures: Add PerfMonV2 feature bit
> x86/msr: Add PerfCntrGlobal* registers
> perf/x86/amd/core: Detect PerfMonV2 support
> perf/x86/amd/core: Detect available counters
> perf/x86/amd/core: Add PerfMonV2 counter control
> perf/x86/amd/core: Add PerfMonV2 overflow handling
> kvm: x86/cpuid: Fix CPUID leaf 0xA
>
> arch/x86/events/amd/core.c | 227 +++++++++++++++++++++++++++--
> arch/x86/include/asm/cpufeatures.h | 2 +-
> arch/x86/include/asm/msr-index.h | 5 +
> arch/x86/include/asm/perf_event.h | 17 +++
> arch/x86/kernel/cpu/scattered.c | 1 +
> arch/x86/kvm/cpuid.c | 5 +
> 6 files changed, 240 insertions(+), 17 deletions(-)
From: Documentation/process/submitting-patches.rst
Don't get discouraged - or impatient
------------------------------------
After you have submitted your change, be patient and wait. Reviewers are
busy people and may not get to your patch right away.
Once upon a time, patches used to disappear into the void without comment,
but the development process works more smoothly than that now. You should
receive comments within a week or so; if that does not happen, make sure
that you have sent your patches to the right place. Wait for a minimum of
^^^^^^^^^^^^^^^^^^^^^
one week before resubmitting or pinging reviewers - possibly longer during
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
busy times like merge windows.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 27/4/2022 7:31 pm, Sandipan Das wrote:
> +static inline void amd_pmu_ack_global_status(u64 status)
> +{
> + /*
> + * PerfCntrGlobalStatus is read-only but an overflow acknowledgment
If wrmsrl PerfCntrGlobalStatus, remain silent or report #GP ?
> + * mechanism exists; writing 1 to a bit in PerfCntrGlobalStatusClr
> + * clears the same bit in PerfCntrGlobalStatus
> + */
> +
> + /* Only allow modifications to PerfCntrGlobalStatus.PerfCntrOvfl */
> + status &= amd_pmu_global_cntr_mask;
> + wrmsrl(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR, status);
If rdmsrl PerfCntrGlobalStatusClr, does it return 0 or the value of
PerfCntrGlobalStatus ?
> +}
On 4/28/2022 7:10 PM, Like Xu wrote:
> On 27/4/2022 7:31 pm, Sandipan Das wrote:
>> +static inline void amd_pmu_ack_global_status(u64 status)
>> +{
>> + /*
>> + * PerfCntrGlobalStatus is read-only but an overflow acknowledgment
>
> If wrmsrl PerfCntrGlobalStatus, remain silent or report #GP ?
>
This is a read-only MSR and writes are silently ignored.
>> + * mechanism exists; writing 1 to a bit in PerfCntrGlobalStatusClr
>> + * clears the same bit in PerfCntrGlobalStatus
>> + */
>> +
>> + /* Only allow modifications to PerfCntrGlobalStatus.PerfCntrOvfl */
>> + status &= amd_pmu_global_cntr_mask;
>> + wrmsrl(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR, status);
>
> If rdmsrl PerfCntrGlobalStatusClr, does it return 0 or the value of PerfCntrGlobalStatus ?
>
This is a write-only MSR and reads are undefined.
The "Field Access Type" section from the AMD Processor Programming
Reference (PPR) has these details.
- Sandipan
On Wed, Apr 27, 2022 at 4:34 AM Sandipan Das <[email protected]> wrote:
>
> On some x86 processors, CPUID leaf 0xA provides information
> on Architectural Performance Monitoring features. It
> advertises a PMU version which Qemu uses to determine the
> availability of additional MSRs to manage the PMCs.
>
> Upon receiving a KVM_GET_SUPPORTED_CPUID ioctl request for
> the same, the kernel constructs return values based on the
> x86_pmu_capability irrespective of the vendor.
>
> This leaf and the additional MSRs are not supported on AMD
> and Hygon processors. If AMD PerfMonV2 is detected, the PMU
> version is set to 2 and guest startup breaks because of an
> attempt to access a non-existent MSR. Return zeros to avoid
> this.
>
> Fixes: a6c06ed1a60a ("KVM: Expose the architectural performance monitoring CPUID leaf")
> Reported-by: Vasant Hegde <[email protected]>
> Signed-off-by: Sandipan Das <[email protected]>
> ---
> arch/x86/kvm/cpuid.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 4b62d80bb22f..e66ebb747084 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -872,6 +872,11 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
> union cpuid10_eax eax;
> union cpuid10_edx edx;
>
> + if (!static_cpu_has(X86_FEATURE_ARCH_PERFMON)) {
Should this be checking kvm_cpu_cap_has(X86_FEATURE_ARCH_PERFMON) instead?
> + entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
> + break;
> + }
> +
> perf_get_x86_pmu_capability(&cap);
>
> /*
> --
> 2.34.1
>
On 5/3/22 01:50, Jim Mattson wrote:
> On Wed, Apr 27, 2022 at 4:34 AM Sandipan Das <[email protected]> wrote:
>>
>> On some x86 processors, CPUID leaf 0xA provides information
>> on Architectural Performance Monitoring features. It
>> advertises a PMU version which Qemu uses to determine the
>> availability of additional MSRs to manage the PMCs.
>>
>> Upon receiving a KVM_GET_SUPPORTED_CPUID ioctl request for
>> the same, the kernel constructs return values based on the
>> x86_pmu_capability irrespective of the vendor.
>>
>> This leaf and the additional MSRs are not supported on AMD
>> and Hygon processors. If AMD PerfMonV2 is detected, the PMU
>> version is set to 2 and guest startup breaks because of an
>> attempt to access a non-existent MSR. Return zeros to avoid
>> this.
>>
>> Fixes: a6c06ed1a60a ("KVM: Expose the architectural performance monitoring CPUID leaf")
>> Reported-by: Vasant Hegde <[email protected]>
>> Signed-off-by: Sandipan Das <[email protected]>
>> ---
>> arch/x86/kvm/cpuid.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index 4b62d80bb22f..e66ebb747084 100644
>> --- a/arch/x86/kvm/cpuid.c
>> +++ b/arch/x86/kvm/cpuid.c
>> @@ -872,6 +872,11 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>> union cpuid10_eax eax;
>> union cpuid10_edx edx;
>>
>> + if (!static_cpu_has(X86_FEATURE_ARCH_PERFMON)) {
>
> Should this be checking kvm_cpu_cap_has(X86_FEATURE_ARCH_PERFMON) instead?
Yes, it should. I have queued this patch for 5.18.
Paolo
On 5/3/22 01:50, Jim Mattson wrote:
> On Wed, Apr 27, 2022 at 4:34 AM Sandipan Das <[email protected]> wrote:
>>
>> On some x86 processors, CPUID leaf 0xA provides information
>> on Architectural Performance Monitoring features. It
>> advertises a PMU version which Qemu uses to determine the
>> availability of additional MSRs to manage the PMCs.
>>
>> Upon receiving a KVM_GET_SUPPORTED_CPUID ioctl request for
>> the same, the kernel constructs return values based on the
>> x86_pmu_capability irrespective of the vendor.
>>
>> This leaf and the additional MSRs are not supported on AMD
>> and Hygon processors. If AMD PerfMonV2 is detected, the PMU
>> version is set to 2 and guest startup breaks because of an
>> attempt to access a non-existent MSR. Return zeros to avoid
>> this.
>>
>> Fixes: a6c06ed1a60a ("KVM: Expose the architectural performance monitoring CPUID leaf")
>> Reported-by: Vasant Hegde <[email protected]>
>> Signed-off-by: Sandipan Das <[email protected]>
>> ---
>> arch/x86/kvm/cpuid.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index 4b62d80bb22f..e66ebb747084 100644
>> --- a/arch/x86/kvm/cpuid.c
>> +++ b/arch/x86/kvm/cpuid.c
>> @@ -872,6 +872,11 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>> union cpuid10_eax eax;
>> union cpuid10_edx edx;
>>
>> + if (!static_cpu_has(X86_FEATURE_ARCH_PERFMON)) {
>
> Should this be checking kvm_cpu_cap_has(X86_FEATURE_ARCH_PERFMON) instead?
Ah, it cannot because X86_FEATURE_ARCH_PERFMON is a synthetic feature.
kvm_cpu_cap_has only works with features that are backed by CPUID bits.
Paolo
On 27/4/2022 7:31 pm, Sandipan Das wrote:
> static void amd_pmu_add_event(struct perf_event *event)
> {
> if (needs_branch_stack(event))
> @@ -1216,6 +1251,11 @@ static int __init amd_core_pmu_init(void)
> x86_pmu.num_counters = ebx.split.num_core_pmc;
>
> amd_pmu_global_cntr_mask = (1ULL << x86_pmu.num_counters) - 1;
> +
> + /* Update PMC handling functions */
> + x86_pmu.enable_all = amd_pmu_v2_enable_all;
> + x86_pmu.disable_all = amd_pmu_v2_disable_all;
> + x86_pmu.enable = amd_pmu_v2_enable_event;
> }
Considering the below part of code also run on the PerfMonV2 host
whatever the guest PMU version, how about updating these two use cases as well
in this patch ?
diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 262e39a85031..2f7c62e46314 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -1469,8 +1469,8 @@ void amd_pmu_enable_virt(void)
cpuc->perf_ctr_virt_mask = 0;
/* Reload all events */
- amd_pmu_disable_all();
- x86_pmu_enable_all(0);
+ amd_pmu.disable_all();
+ amd_pmu.enable_all(0);
}
EXPORT_SYMBOL_GPL(amd_pmu_enable_virt);
@@ -1487,7 +1487,7 @@ void amd_pmu_disable_virt(void)
cpuc->perf_ctr_virt_mask = AMD64_EVENTSEL_HOSTONLY;
/* Reload all events */
- amd_pmu_disable_all();
- x86_pmu_enable_all(0);
+ amd_pmu.disable_all();
+ amd_pmu.enable_all(0);
}
EXPORT_SYMBOL_GPL(amd_pmu_disable_virt);
On 5/9/2022 12:35 PM, Like Xu wrote:
> On 27/4/2022 7:31 pm, Sandipan Das wrote:
>
>> static void amd_pmu_add_event(struct perf_event *event)
>> {
>> if (needs_branch_stack(event))
>> @@ -1216,6 +1251,11 @@ static int __init amd_core_pmu_init(void)
>> x86_pmu.num_counters = ebx.split.num_core_pmc;
>> amd_pmu_global_cntr_mask = (1ULL << x86_pmu.num_counters) - 1;
>> +
>> + /* Update PMC handling functions */
>> + x86_pmu.enable_all = amd_pmu_v2_enable_all;
>> + x86_pmu.disable_all = amd_pmu_v2_disable_all;
>> + x86_pmu.enable = amd_pmu_v2_enable_event;
>> }
>
> Considering the below part of code also run on the PerfMonV2 host
>
> whatever the guest PMU version, how about updating these two use cases as well in this patch ?
>
> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index 262e39a85031..2f7c62e46314 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c
> @@ -1469,8 +1469,8 @@ void amd_pmu_enable_virt(void)
> cpuc->perf_ctr_virt_mask = 0;
>
> /* Reload all events */
> - amd_pmu_disable_all();
> - x86_pmu_enable_all(0);
> + amd_pmu.disable_all();
> + amd_pmu.enable_all(0);
> }
> EXPORT_SYMBOL_GPL(amd_pmu_enable_virt);
>
> @@ -1487,7 +1487,7 @@ void amd_pmu_disable_virt(void)
> cpuc->perf_ctr_virt_mask = AMD64_EVENTSEL_HOSTONLY;
>
> /* Reload all events */
> - amd_pmu_disable_all();
> - x86_pmu_enable_all(0);
> + amd_pmu.disable_all();
> + amd_pmu.enable_all(0);
> }
> EXPORT_SYMBOL_GPL(amd_pmu_disable_virt);
>
Good point. I had made these changes as a part of the guest PerfMonV2
enablement.
Peter,
Since these patches are now in perf/core, should I send out a separate
fix for this?
- Sandipan
On Mon, May 09, 2022 at 03:28:07PM +0530, Sandipan Das wrote:
>
> Since these patches are now in perf/core, should I send out a separate
> fix for this?
Yes please.
On 27/4/2022 7:31 pm, Sandipan Das wrote:
> x86_pmu.num_counters = AMD64_NUM_COUNTERS_CORE;
Thus boot_cpu_has(X86_FEATURE_PERFCTR_CORE) is true.
> +
> + /* Check for Performance Monitoring v2 support */
> + if (boot_cpu_has(X86_FEATURE_PERFMON_V2)) {
> + /* Update PMU version for later usage */
> + x86_pmu.version = 2;
Is it safe to assume that once AMD CPU has the PERFMON_V2 (or further) bit,
it must also have the PERFCTR_CORE bit set ?
On 5/9/2022 6:31 PM, Like Xu wrote:
> On 27/4/2022 7:31 pm, Sandipan Das wrote:
>
>> x86_pmu.num_counters = AMD64_NUM_COUNTERS_CORE;
>
> Thus boot_cpu_has(X86_FEATURE_PERFCTR_CORE) is true.
>
>> +
>> + /* Check for Performance Monitoring v2 support */
>> + if (boot_cpu_has(X86_FEATURE_PERFMON_V2)) {
>> + /* Update PMU version for later usage */
>> + x86_pmu.version = 2;
>
> Is it safe to assume that once AMD CPU has the PERFMON_V2 (or further) bit,
>
> it must also have the PERFCTR_CORE bit set ?
>
Yes, always. There won't be a case where PERFCTR_CORE is absent
but PERFMON_V2 is present.
- Sandipan
On Mon, May 09, 2022 at 06:38:19PM +0530, Sandipan Das wrote:
>
> On 5/9/2022 6:31 PM, Like Xu wrote:
> > On 27/4/2022 7:31 pm, Sandipan Das wrote:
> >
> >> ????? x86_pmu.num_counters??? = AMD64_NUM_COUNTERS_CORE;
> >
> > Thus boot_cpu_has(X86_FEATURE_PERFCTR_CORE) is true.
> >
> >> +
> >> +??? /* Check for Performance Monitoring v2 support */
> >> +??? if (boot_cpu_has(X86_FEATURE_PERFMON_V2)) {
> >> +??????? /* Update PMU version for later usage */
> >> +??????? x86_pmu.version = 2;
> >
> > Is it safe to assume that once AMD CPU has the PERFMON_V2 (or further) bit,
> >
> > it must also have the PERFCTR_CORE bit set ?
> >
>
> Yes, always. There won't be a case where PERFCTR_CORE is absent
> but PERFMON_V2 is present.
Let me introduce you to this dodgy virt stuff :-) Best put a sanity
check on it.
Hi Peter,
On 5/9/2022 6:41 PM, Peter Zijlstra wrote:
> On Mon, May 09, 2022 at 06:38:19PM +0530, Sandipan Das wrote:
>>
>> On 5/9/2022 6:31 PM, Like Xu wrote:
>>> On 27/4/2022 7:31 pm, Sandipan Das wrote:
>>>
>>>> x86_pmu.num_counters = AMD64_NUM_COUNTERS_CORE;
>>>
>>> Thus boot_cpu_has(X86_FEATURE_PERFCTR_CORE) is true.
>>>
>>>> +
>>>> + /* Check for Performance Monitoring v2 support */
>>>> + if (boot_cpu_has(X86_FEATURE_PERFMON_V2)) {
>>>> + /* Update PMU version for later usage */
>>>> + x86_pmu.version = 2;
>>>
>>> Is it safe to assume that once AMD CPU has the PERFMON_V2 (or further) bit,
>>>
>>> it must also have the PERFCTR_CORE bit set ?
>>>
>>
>> Yes, always. There won't be a case where PERFCTR_CORE is absent
>> but PERFMON_V2 is present.
>
> Let me introduce you to this dodgy virt stuff :-) Best put a sanity
> check on it.
I see that amd_core_pmu_init() returns if X86_FEATURE_PERFCTR_CORE is
not found right after entry. Is there anywhere else that you feel should
have an additional sanity check?
I also noticed a bug in the X86_FEATURE_PERFCTR_CORE feature check where
it should have returned something like -ENODEV instead of 0. Will send
out a fix for it.
- Sandipan