Thread (4 messages) 4 messages, 3 authors, 2021-06-01

Re: [PATCH] arm_pmu: Fix write counter incorrect in ARMv7 big-endian mode

From: Yang Jihong <hidden>
Date: 2021-05-20 08:32:00
Also in: lkml

Hello, Mark

On 2021/4/30 20:34, Mark Rutland wrote:
On Fri, Apr 30, 2021 at 09:26:59AM +0800, Yang Jihong wrote:
quoted
Commit 3a95200d3f89a ("arm_pmu: Change API to support 64bit counter values")
changes the input "value" type from 32-bit to 64-bit,
which introduces the following problem:
ARMv7 PMU counters is 32-bit width, in big-endian mode, write counter uses
high 32-bit, which writes an incorrect value.
It might be worth noting that the reason for this is that when a 64-bit
value is split across two 32-bit registers, the high/low halves are
split to match how LDRD would load a 64-bit quantity (and so differs
across big/little endian), but the "r" constraint consistently uses the
first of the two regiseters.

Given that, this patch makes sense to me (and I didn't spot any related
issues), so:

Acked-by: Mark Rutland <mark.rutland@arm.com>
Thank you for your review. :)
Yang.
Will, I assume you'll pick this up.

Mark.
quoted
Before:

  Performance counter stats for 'ls':

               2.22 msec task-clock                #    0.675 CPUs utilized
                  0      context-switches          #    0.000 K/sec
                  0      cpu-migrations            #    0.000 K/sec
                 49      page-faults               #    0.022 M/sec
         2150476593      cycles                    #  966.663 GHz
         2148588788      instructions              #    1.00  insn per cycle
         2147745484      branches                  # 965435.074 M/sec
         2147508540      branch-misses             #   99.99% of all branches

None of the above hw event counters are correct.

Solution:

"value" forcibly converted to 32-bit type before being written to PMU register.

After:

  Performance counter stats for 'ls':

               2.09 msec task-clock                #    0.681 CPUs utilized
                  0      context-switches          #    0.000 K/sec
                  0      cpu-migrations            #    0.000 K/sec
                 46      page-faults               #    0.022 M/sec
            2807301      cycles                    #    1.344 GHz
            1060159      instructions              #    0.38  insn per cycle
             250496      branches                  #  119.914 M/sec
              23192      branch-misses             #    9.26% of all branches

Fixes: 3a95200d3f89a ("arm_pmu: Change API to support 64bit counter values")
Signed-off-by: Yang Jihong <redacted>
---
  arch/arm/kernel/perf_event_v7.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
index 2924d7910b10..eb2190477da1 100644
--- a/arch/arm/kernel/perf_event_v7.c
+++ b/arch/arm/kernel/perf_event_v7.c
@@ -773,10 +773,10 @@ static inline void armv7pmu_write_counter(struct perf_event *event, u64 value)
  		pr_err("CPU%u writing wrong counter %d\n",
  			smp_processor_id(), idx);
  	} else if (idx == ARMV7_IDX_CYCLE_COUNTER) {
-		asm volatile("mcr p15, 0, %0, c9, c13, 0" : : "r" (value));
+		asm volatile("mcr p15, 0, %0, c9, c13, 0" : : "r" ((u32)value));
  	} else {
  		armv7_pmnc_select_counter(idx);
-		asm volatile("mcr p15, 0, %0, c9, c13, 2" : : "r" (value));
+		asm volatile("mcr p15, 0, %0, c9, c13, 2" : : "r" ((u32)value));
  	}
  }
  
-- 
2.30.GIT
.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help