Skip to contentSkip to conversation

arm64: perf: Fix 64-bit event counter read truncation

PATCH Applied Rob Herring 4 participants 4 messages Mar 10, 2021

Subsystems (3)
perf events arm64 Arm PMU profiling and debugging
lore
Share & tools
lore

Rob Herring

author
#0 lore raw

PATCH 1/1 arm64: perf: Fix 64-bit event counter read truncation

Commit 0fdf1bb75953 ("arm64: perf: Avoid PMXEV* indirection") changed armv8pmu_read_evcntr() to return a u32 instead of u64. The result is silent truncation of the event counter when using 64-bit counters. Given the offending commit appears to have passed thru several folks, it seems likely this was a bad rebase after v8.5 PMU 64-bit counters landed.

Fixes 0fdf1bb75953 ("arm64: perf: Avoid PMXEV* indirection")
Cc Alexandru Elisei <[email protected]>
Cc Julien Thierry <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Alexander Shishkin <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Signed-off-by Rob Herring <[email protected]>
---
 arch/arm64/kernel/perf_event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/perf_event.c +1 -1
index 7d2318f80955..4658fcf88c2b 100644
@@ -460,7 +460,7 @@ static inline int armv8pmu_counter_has_overflowed(u32 pmnc, int idx)
 	return pmnc & BIT(ARMV8_IDX_TO_COUNTER(idx));
 }
 
-static inline u32 armv8pmu_read_evcntr(int idx)
+static inline u64 armv8pmu_read_evcntr(int idx)
 {
 	u32 counter = ARMV8_IDX_TO_COUNTER(idx);
Signature · 1 line
2.27.0
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 5 lines · Rob Herring

Commit 0fdf1bb75953 ("arm64: perf: Avoid PMXEV* indirection") changed armv8pmu_read_evcntr() to return a u32 instead of u64. The result is silent truncation of the event counter when using 64-bit counters. Given the offending commit appears to have passed thru several folks, it seems likely this was a bad rebase after v8.5 PMU 64-bit counters landed.

IIRC I wrote the indirection patch first, so this does sound like an oversight when rebasing or reworking the patch.

Comparing against commit 0fdf1bb75953, this does appear to be the only point of truncation given read_pmevcntrn() directly returns the result of read_sysreg(), so:

Acked-by Mark Rutland <[email protected]>

Will, could you pick this up?

Thanks,
Mark.

Quoted 33 lines
Fixes 0fdf1bb75953 ("arm64: perf: Avoid PMXEV* indirection")
Cc Alexandru Elisei <[email protected]>
Cc Julien Thierry <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Alexander Shishkin <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Signed-off-by Rob Herring <[email protected]>
---
 arch/arm64/kernel/perf_event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/perf_event.c +1 -1
index 7d2318f80955..4658fcf88c2b 100644
@@ -460,7 +460,7 @@ static inline int armv8pmu_counter_has_overflowed(u32 pmnc, int idx)
 	return pmnc & BIT(ARMV8_IDX_TO_COUNTER(idx));
 }
 
-static inline u32 armv8pmu_read_evcntr(int idx)
+static inline u64 armv8pmu_read_evcntr(int idx)
 {
 	u32 counter = ARMV8_IDX_TO_COUNTER(idx);
Signature · 1 line
2.27.0
Reviewed-by on v1 1/1
#2 lore raw
In reply to Rob Herring

Hi Rob,

Quoted 5 lines · Rob Herring

Commit 0fdf1bb75953 ("arm64: perf: Avoid PMXEV* indirection") changed armv8pmu_read_evcntr() to return a u32 instead of u64. The result is silent truncation of the event counter when using 64-bit counters. Given the offending commit appears to have passed thru several folks, it seems likely this was a bad rebase after v8.5 PMU 64-bit counters landed.

Thank you for the fix, it does seem that I made a mistake when rebasing the series. Version v4 of the PMU NMI series was sent in 2019, then patch 8673e02e5841 ("arm64: perf: Add support for ARMv8.5-PMU 64-bit counters") from March 2020 changed the read of PMEVCNTR_EL0 to return an u64, then version v5 from June 2020 changed it back to returning an u32.

The result of read_pmvevcntr() is returned by armv8pmu_read_evcntr(), and it is an unsigned long which is 64bits for arm64, so the patch looks good to me:

Reviewed-by Alexandru Elisei <[email protected]>

Thanks,

Alex

Quoted 31 lines
Fixes 0fdf1bb75953 ("arm64: perf: Avoid PMXEV* indirection")
Cc Alexandru Elisei <[email protected]>
Cc Julien Thierry <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Alexander Shishkin <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Signed-off-by Rob Herring <[email protected]>
---
 arch/arm64/kernel/perf_event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/perf_event.c +1 -1
index 7d2318f80955..4658fcf88c2b 100644
@@ -460,7 +460,7 @@ static inline int armv8pmu_counter_has_overflowed(u32 pmnc, int idx)
 	return pmnc & BIT(ARMV8_IDX_TO_COUNTER(idx));
 }
 
-static inline u32 armv8pmu_read_evcntr(int idx)
+static inline u64 armv8pmu_read_evcntr(int idx)
 {
 	u32 counter = ARMV8_IDX_TO_COUNTER(idx);
maintainer Applied
#3 lore raw
In reply to Rob Herring
Quoted 5 lines · Rob Herring

Commit 0fdf1bb75953 ("arm64: perf: Avoid PMXEV* indirection") changed armv8pmu_read_evcntr() to return a u32 instead of u64. The result is silent truncation of the event counter when using 64-bit counters. Given the offending commit appears to have passed thru several folks, it seems likely this was a bad rebase after v8.5 PMU 64-bit counters landed.

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm64: perf: Fix 64-bit event counter read truncation

      https://git.kernel.org/arm64/c/7bb8bc6eb550

Cheers,

Signature · 5 lines
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev