Skip to content

Commit

Permalink
drivers/perf: apple_m1: Force 63bit counters for M2 CPUs
Browse files Browse the repository at this point in the history
Sidharth reports that on M2, the PMU never generates any interrupt
when using 'perf record', which is a annoying as you get no sample.
I'm temped to say "no sample, no problem", but others may have
a different opinion.

Upon investigation, it appears that the counters on M2 are
significantly different from the ones on M1, as they count on
64 bits instead of 48. Which of course, in the fine M1 tradition,
means that we can only use 63 bits, as the top bit is used to signal
the interrupt...

This results in having to introduce yet another flag to indicate yet
another odd counter width. Who knows what the next crazy implementation
will do...

With this, perf can work out the correct offset, and 'perf record'
works as intended.

Tested on M2 and M2-Pro CPUs.

Cc: Janne Grunau <j@jannau.net>
Cc: Hector Martin <marcan@marcan.st>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>
Fixes: 7d0bfb7 ("drivers/perf: apple_m1: Add Apple M2 support")
Reported-by: Sidharth Kshatriya <sid.kshatriya@gmail.com>
Tested-by: Sidharth Kshatriya <sid.kshatriya@gmail.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20230528080205.288446-1-maz@kernel.org
Signed-off-by: Will Deacon <will@kernel.org>
  • Loading branch information
Marc Zyngier authored and Will Deacon committed Jun 5, 2023
1 parent 71746c9 commit 8be3593
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
30 changes: 24 additions & 6 deletions drivers/perf/apple_m1_cpu_pmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,17 @@ static int m1_pmu_map_event(struct perf_event *event)
return armpmu_map_event(event, &m1_pmu_perf_map, NULL, M1_PMU_CFG_EVENT);
}

static int m2_pmu_map_event(struct perf_event *event)
{
/*
* Same deal as the above, except that M2 has 64bit counters.
* Which, as far as we're concerned, actually means 63 bits.
* Yes, this is getting awkward.
*/
event->hw.flags |= ARMPMU_EVT_63BIT;
return armpmu_map_event(event, &m1_pmu_perf_map, NULL, M1_PMU_CFG_EVENT);
}

static void m1_pmu_reset(void *info)
{
int i;
Expand Down Expand Up @@ -525,7 +536,7 @@ static int m1_pmu_set_event_filter(struct hw_perf_event *event,
return 0;
}

static int m1_pmu_init(struct arm_pmu *cpu_pmu)
static int m1_pmu_init(struct arm_pmu *cpu_pmu, u32 flags)
{
cpu_pmu->handle_irq = m1_pmu_handle_irq;
cpu_pmu->enable = m1_pmu_enable_event;
Expand All @@ -536,7 +547,14 @@ static int m1_pmu_init(struct arm_pmu *cpu_pmu)
cpu_pmu->clear_event_idx = m1_pmu_clear_event_idx;
cpu_pmu->start = m1_pmu_start;
cpu_pmu->stop = m1_pmu_stop;
cpu_pmu->map_event = m1_pmu_map_event;

if (flags & ARMPMU_EVT_47BIT)
cpu_pmu->map_event = m1_pmu_map_event;
else if (flags & ARMPMU_EVT_63BIT)
cpu_pmu->map_event = m2_pmu_map_event;
else
return WARN_ON(-EINVAL);

cpu_pmu->reset = m1_pmu_reset;
cpu_pmu->set_event_filter = m1_pmu_set_event_filter;

Expand All @@ -550,25 +568,25 @@ static int m1_pmu_init(struct arm_pmu *cpu_pmu)
static int m1_pmu_ice_init(struct arm_pmu *cpu_pmu)
{
cpu_pmu->name = "apple_icestorm_pmu";
return m1_pmu_init(cpu_pmu);
return m1_pmu_init(cpu_pmu, ARMPMU_EVT_47BIT);
}

static int m1_pmu_fire_init(struct arm_pmu *cpu_pmu)
{
cpu_pmu->name = "apple_firestorm_pmu";
return m1_pmu_init(cpu_pmu);
return m1_pmu_init(cpu_pmu, ARMPMU_EVT_47BIT);
}

static int m2_pmu_avalanche_init(struct arm_pmu *cpu_pmu)
{
cpu_pmu->name = "apple_avalanche_pmu";
return m1_pmu_init(cpu_pmu);
return m1_pmu_init(cpu_pmu, ARMPMU_EVT_63BIT);
}

static int m2_pmu_blizzard_init(struct arm_pmu *cpu_pmu)
{
cpu_pmu->name = "apple_blizzard_pmu";
return m1_pmu_init(cpu_pmu);
return m1_pmu_init(cpu_pmu, ARMPMU_EVT_63BIT);
}

static const struct of_device_id m1_pmu_of_device_ids[] = {
Expand Down
2 changes: 2 additions & 0 deletions drivers/perf/arm_pmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ static inline u64 arm_pmu_event_max_period(struct perf_event *event)
{
if (event->hw.flags & ARMPMU_EVT_64BIT)
return GENMASK_ULL(63, 0);
else if (event->hw.flags & ARMPMU_EVT_63BIT)
return GENMASK_ULL(62, 0);
else if (event->hw.flags & ARMPMU_EVT_47BIT)
return GENMASK_ULL(46, 0);
else
Expand Down
2 changes: 2 additions & 0 deletions include/linux/perf/arm_pmu.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@
*/
#define ARMPMU_EVT_64BIT 0x00001 /* Event uses a 64bit counter */
#define ARMPMU_EVT_47BIT 0x00002 /* Event uses a 47bit counter */
#define ARMPMU_EVT_63BIT 0x00004 /* Event uses a 63bit counter */

static_assert((PERF_EVENT_FLAG_ARCH & ARMPMU_EVT_64BIT) == ARMPMU_EVT_64BIT);
static_assert((PERF_EVENT_FLAG_ARCH & ARMPMU_EVT_47BIT) == ARMPMU_EVT_47BIT);
static_assert((PERF_EVENT_FLAG_ARCH & ARMPMU_EVT_63BIT) == ARMPMU_EVT_63BIT);

#define HW_OP_UNSUPPORTED 0xFFFF
#define C(_x) PERF_COUNT_HW_CACHE_##_x
Expand Down

0 comments on commit 8be3593

Please sign in to comment.