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(-)
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 7d2318f80955..4658fcf88c2b 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -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);
--
2.27.0
On Tue, Mar 09, 2021 at 05:44:12PM -0700, Rob Herring wrote:
> 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.
> 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(-)
>
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 7d2318f80955..4658fcf88c2b 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -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);
>
> --
> 2.27.0
>
Hi Rob,
On 3/10/21 12:44 AM, Rob Herring wrote:
> 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
>
> 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(-)
>
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 7d2318f80955..4658fcf88c2b 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -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);
>
On Tue, 9 Mar 2021 17:44:12 -0700, Rob Herring wrote:
> 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,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev