Skip to content

Commit e10de31

Browse files
ssuthiku-amdPeter Zijlstra
authored andcommitted
x86/events/amd/iommu: Fix invalid Perf result due to IOMMU PMC power-gating
On certain AMD platforms, when the IOMMU performance counter source (csource) field is zero, power-gating for the counter is enabled, which prevents write access and returns zero for read access. This can cause invalid perf result especially when event multiplexing is needed (i.e. more number of events than available counters) since the current logic keeps track of the previously read counter value, and subsequently re-program the counter to continue counting the event. With power-gating enabled, we cannot gurantee successful re-programming of the counter. Workaround this issue by : 1. Modifying the ordering of setting/reading counters and enabing/ disabling csources to only access the counter when the csource is set to non-zero. 2. Since AMD IOMMU PMU does not support interrupt mode, the logic can be simplified to always start counting with value zero, and accumulate the counter value when stopping without the need to keep track and reprogram the counter with the previously read counter value. This has been tested on systems with and without power-gating. Fixes: 994d660 ("iommu/amd: Remove performance counter pre-initialization test") Suggested-by: Alexander Monakov <[email protected]> Signed-off-by: Suravee Suthikulpanit <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Link: https://lkml.kernel.org/r/[email protected]
1 parent 635de95 commit e10de31

File tree

1 file changed

+26
-21
lines changed

1 file changed

+26
-21
lines changed

arch/x86/events/amd/iommu.c

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
#include "../perf_event.h"
1919
#include "iommu.h"
2020

21-
#define COUNTER_SHIFT 16
22-
2321
/* iommu pmu conf masks */
2422
#define GET_CSOURCE(x) ((x)->conf & 0xFFULL)
2523
#define GET_DEVID(x) (((x)->conf >> 8) & 0xFFFFULL)
@@ -285,22 +283,31 @@ static void perf_iommu_start(struct perf_event *event, int flags)
285283
WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
286284
hwc->state = 0;
287285

286+
/*
287+
* To account for power-gating, which prevents write to
288+
* the counter, we need to enable the counter
289+
* before setting up counter register.
290+
*/
291+
perf_iommu_enable_event(event);
292+
288293
if (flags & PERF_EF_RELOAD) {
289-
u64 prev_raw_count = local64_read(&hwc->prev_count);
294+
u64 count = 0;
290295
struct amd_iommu *iommu = perf_event_2_iommu(event);
291296

297+
/*
298+
* Since the IOMMU PMU only support counting mode,
299+
* the counter always start with value zero.
300+
*/
292301
amd_iommu_pc_set_reg(iommu, hwc->iommu_bank, hwc->iommu_cntr,
293-
IOMMU_PC_COUNTER_REG, &prev_raw_count);
302+
IOMMU_PC_COUNTER_REG, &count);
294303
}
295304

296-
perf_iommu_enable_event(event);
297305
perf_event_update_userpage(event);
298-
299306
}
300307

301308
static void perf_iommu_read(struct perf_event *event)
302309
{
303-
u64 count, prev, delta;
310+
u64 count;
304311
struct hw_perf_event *hwc = &event->hw;
305312
struct amd_iommu *iommu = perf_event_2_iommu(event);
306313

@@ -311,14 +318,11 @@ static void perf_iommu_read(struct perf_event *event)
311318
/* IOMMU pc counter register is only 48 bits */
312319
count &= GENMASK_ULL(47, 0);
313320

314-
prev = local64_read(&hwc->prev_count);
315-
if (local64_cmpxchg(&hwc->prev_count, prev, count) != prev)
316-
return;
317-
318-
/* Handle 48-bit counter overflow */
319-
delta = (count << COUNTER_SHIFT) - (prev << COUNTER_SHIFT);
320-
delta >>= COUNTER_SHIFT;
321-
local64_add(delta, &event->count);
321+
/*
322+
* Since the counter always start with value zero,
323+
* simply just accumulate the count for the event.
324+
*/
325+
local64_add(count, &event->count);
322326
}
323327

324328
static void perf_iommu_stop(struct perf_event *event, int flags)
@@ -328,15 +332,16 @@ static void perf_iommu_stop(struct perf_event *event, int flags)
328332
if (hwc->state & PERF_HES_UPTODATE)
329333
return;
330334

335+
/*
336+
* To account for power-gating, in which reading the counter would
337+
* return zero, we need to read the register before disabling.
338+
*/
339+
perf_iommu_read(event);
340+
hwc->state |= PERF_HES_UPTODATE;
341+
331342
perf_iommu_disable_event(event);
332343
WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
333344
hwc->state |= PERF_HES_STOPPED;
334-
335-
if (hwc->state & PERF_HES_UPTODATE)
336-
return;
337-
338-
perf_iommu_read(event);
339-
hwc->state |= PERF_HES_UPTODATE;
340345
}
341346

342347
static int perf_iommu_add(struct perf_event *event, int flags)

0 commit comments

Comments
 (0)