* Re: [PATCH] arm64: perf: Fix 64-bit event counter read truncation
2021-03-10 0:44 [PATCH] arm64: perf: Fix 64-bit event counter read truncation Rob Herring
@ 2021-03-10 10:44 ` Mark Rutland
2021-03-10 10:53 ` Alexandru Elisei
2021-03-10 11:39 ` Will Deacon
2 siblings, 0 replies; 4+ messages in thread
From: Mark Rutland @ 2021-03-10 10:44 UTC (permalink / raw)
To: Rob Herring, Will Deacon
Cc: linux-arm-kernel, linux-kernel, Alexandru Elisei, Julien Thierry,
Catalin Marinas, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim
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 <mark.rutland@arm.com>
Will, could you pick this up?
Thanks,
Mark.
> Fixes: 0fdf1bb75953 ("arm64: perf: Avoid PMXEV* indirection")
> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Cc: Julien Thierry <julien.thierry.kdev@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] arm64: perf: Fix 64-bit event counter read truncation
2021-03-10 0:44 [PATCH] arm64: perf: Fix 64-bit event counter read truncation Rob Herring
2021-03-10 10:44 ` Mark Rutland
@ 2021-03-10 10:53 ` Alexandru Elisei
2021-03-10 11:39 ` Will Deacon
2 siblings, 0 replies; 4+ messages in thread
From: Alexandru Elisei @ 2021-03-10 10:53 UTC (permalink / raw)
To: Rob Herring, Will Deacon, Mark Rutland
Cc: linux-arm-kernel, linux-kernel, Julien Thierry, Catalin Marinas,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Namhyung Kim
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 <alexandru.elisei@arm.com>
Thanks,
Alex
>
> Fixes: 0fdf1bb75953 ("arm64: perf: Avoid PMXEV* indirection")
> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Cc: Julien Thierry <julien.thierry.kdev@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> 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);
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] arm64: perf: Fix 64-bit event counter read truncation
2021-03-10 0:44 [PATCH] arm64: perf: Fix 64-bit event counter read truncation Rob Herring
2021-03-10 10:44 ` Mark Rutland
2021-03-10 10:53 ` Alexandru Elisei
@ 2021-03-10 11:39 ` Will Deacon
2 siblings, 0 replies; 4+ messages in thread
From: Will Deacon @ 2021-03-10 11:39 UTC (permalink / raw)
To: Rob Herring, Mark Rutland
Cc: catalin.marinas, kernel-team, Will Deacon, linux-kernel,
Julien Thierry, Arnaldo Carvalho de Melo, Alexander Shishkin,
Namhyung Kim, Peter Zijlstra, linux-arm-kernel, Ingo Molnar,
Jiri Olsa, Alexandru Elisei
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
^ permalink raw reply [flat|nested] 4+ messages in thread