| Message ID | 20240710182357.3701635-1-robh@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perf: arm_pmuv3: Fix chained event check for cycle counter | expand |
On Wed, Jul 10, 2024 at 12:23:56PM -0600, Rob Herring (Arm) wrote: > Since commit b7e89b0f5bd7 ("perf: arm_pmu: Remove event index to > counter remapping"), armv8pmu_event_is_chained() is incorrectly > returning that the cycle counter is chained, but the cycle counter has > always been 64-bit. The result is trying to configure counter #30 which > typically doesn't exist. > > As ARMV8_PMU_MAX_GENERAL_COUNTERS is the number of counters (31), the > comparison to the counter index needs to be '<' rather than '<='. > > Fixes: b7e89b0f5bd7 ("perf: arm_pmu: Remove event index to counter remapping") > Signed-off-by: Rob Herring (Arm) <robh@kernel.org> > --- > drivers/perf/arm_pmuv3.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c > index 3b3a3334cc3f..0e22c68fb804 100644 > --- a/drivers/perf/arm_pmuv3.c > +++ b/drivers/perf/arm_pmuv3.c > @@ -482,7 +482,7 @@ static bool armv8pmu_event_is_chained(struct perf_event *event) > return !armv8pmu_event_has_user_read(event) && > armv8pmu_event_is_64bit(event) && > !armv8pmu_has_long_event(cpu_pmu) && > - (idx <= ARMV8_PMU_MAX_GENERAL_COUNTERS); > + (idx < ARMV8_PMU_MAX_GENERAL_COUNTERS); > } Acked-by: Will Deacon <will@kernel.org> Catalin -- please can you pick this up as a fix? Cheers, Will
On Thu, Aug 1, 2024 at 5:29 AM Will Deacon <will@kernel.org> wrote: > > On Wed, Jul 10, 2024 at 12:23:56PM -0600, Rob Herring (Arm) wrote: > > Since commit b7e89b0f5bd7 ("perf: arm_pmu: Remove event index to > > counter remapping"), armv8pmu_event_is_chained() is incorrectly > > returning that the cycle counter is chained, but the cycle counter has > > always been 64-bit. The result is trying to configure counter #30 which > > typically doesn't exist. > > > > As ARMV8_PMU_MAX_GENERAL_COUNTERS is the number of counters (31), the > > comparison to the counter index needs to be '<' rather than '<='. > > > > Fixes: b7e89b0f5bd7 ("perf: arm_pmu: Remove event index to counter remapping") > > Signed-off-by: Rob Herring (Arm) <robh@kernel.org> > > --- > > drivers/perf/arm_pmuv3.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c > > index 3b3a3334cc3f..0e22c68fb804 100644 > > --- a/drivers/perf/arm_pmuv3.c > > +++ b/drivers/perf/arm_pmuv3.c > > @@ -482,7 +482,7 @@ static bool armv8pmu_event_is_chained(struct perf_event *event) > > return !armv8pmu_event_has_user_read(event) && > > armv8pmu_event_is_64bit(event) && > > !armv8pmu_has_long_event(cpu_pmu) && > > - (idx <= ARMV8_PMU_MAX_GENERAL_COUNTERS); > > + (idx < ARMV8_PMU_MAX_GENERAL_COUNTERS); > > } > > Acked-by: Will Deacon <will@kernel.org> > > Catalin -- please can you pick this up as a fix? No, this is the fix for what Will dropped and said to resend on the v9.4 fixed instruction counter series. Here's the series with the fix in it[1]. Rob [1] https://lore.kernel.org/all/20240731-arm-pmu-3-9-icntr-v3-0-280a8d7ff465@kernel.org/
On Thu, Aug 01, 2024 at 11:19:26AM -0600, Rob Herring wrote: > On Thu, Aug 1, 2024 at 5:29 AM Will Deacon <will@kernel.org> wrote: > > > > On Wed, Jul 10, 2024 at 12:23:56PM -0600, Rob Herring (Arm) wrote: > > > Since commit b7e89b0f5bd7 ("perf: arm_pmu: Remove event index to > > > counter remapping"), armv8pmu_event_is_chained() is incorrectly > > > returning that the cycle counter is chained, but the cycle counter has > > > always been 64-bit. The result is trying to configure counter #30 which > > > typically doesn't exist. > > > > > > As ARMV8_PMU_MAX_GENERAL_COUNTERS is the number of counters (31), the > > > comparison to the counter index needs to be '<' rather than '<='. > > > > > > Fixes: b7e89b0f5bd7 ("perf: arm_pmu: Remove event index to counter remapping") > > > Signed-off-by: Rob Herring (Arm) <robh@kernel.org> > > > --- > > > drivers/perf/arm_pmuv3.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c > > > index 3b3a3334cc3f..0e22c68fb804 100644 > > > --- a/drivers/perf/arm_pmuv3.c > > > +++ b/drivers/perf/arm_pmuv3.c > > > @@ -482,7 +482,7 @@ static bool armv8pmu_event_is_chained(struct perf_event *event) > > > return !armv8pmu_event_has_user_read(event) && > > > armv8pmu_event_is_64bit(event) && > > > !armv8pmu_has_long_event(cpu_pmu) && > > > - (idx <= ARMV8_PMU_MAX_GENERAL_COUNTERS); > > > + (idx < ARMV8_PMU_MAX_GENERAL_COUNTERS); > > > } > > > > Acked-by: Will Deacon <will@kernel.org> > > > > Catalin -- please can you pick this up as a fix? > > No, this is the fix for what Will dropped and said to resend on the > v9.4 fixed instruction counter series. Here's the series with the fix > in it[1]. Urgh, sorry about that, and thanks for pointing it out. I'd initially marked this as read, but then I was pointed at it the other day and thought I'd missed a fix since "git show b7e89b0f5bd7" still shows the patch, despite pruning the remotes. I'll queue up the other series for 6.12. Will
diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c index 3b3a3334cc3f..0e22c68fb804 100644 --- a/drivers/perf/arm_pmuv3.c +++ b/drivers/perf/arm_pmuv3.c @@ -482,7 +482,7 @@ static bool armv8pmu_event_is_chained(struct perf_event *event) return !armv8pmu_event_has_user_read(event) && armv8pmu_event_is_64bit(event) && !armv8pmu_has_long_event(cpu_pmu) && - (idx <= ARMV8_PMU_MAX_GENERAL_COUNTERS); + (idx < ARMV8_PMU_MAX_GENERAL_COUNTERS); } /*
Since commit b7e89b0f5bd7 ("perf: arm_pmu: Remove event index to counter remapping"), armv8pmu_event_is_chained() is incorrectly returning that the cycle counter is chained, but the cycle counter has always been 64-bit. The result is trying to configure counter #30 which typically doesn't exist. As ARMV8_PMU_MAX_GENERAL_COUNTERS is the number of counters (31), the comparison to the counter index needs to be '<' rather than '<='. Fixes: b7e89b0f5bd7 ("perf: arm_pmu: Remove event index to counter remapping") Signed-off-by: Rob Herring (Arm) <robh@kernel.org> --- drivers/perf/arm_pmuv3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)