[PATCH -next] arm64: perf: Fix hw_event_id check in __armv8_pmuv3_map_event

Raw event SW_INCR (0x0000) shall be included in this check, so fix it.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Shaokun Zhang <redacted>
---
 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 a661010308c0..fa2f60d09e15 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c

@@ -1000,7 +1000,7 @@ static int __armv8_pmuv3_map_event(struct perf_event *event,
 		event->hw.flags |= ARMPMU_EVT_64BIT;
 
 	/* Only expose micro/arch events supported by this PMU */
-	if ((hw_event_id > 0) && (hw_event_id < ARMV8_PMUV3_MAX_COMMON_EVENTS)
+	if ((hw_event_id >= 0) && (hw_event_id < ARMV8_PMUV3_MAX_COMMON_EVENTS)
 	    && test_bit(hw_event_id, armpmu->pmceid_bitmap)) {
 		return hw_event_id;
 	}

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Mon, Jun 07, 2021 at 03:19:03PM +0800, Shaokun Zhang wrote:

Raw event SW_INCR (0x0000) shall be included in this check, so fix it.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Shaokun Zhang <redacted>

Given that SW_INCR depends on the specific counter used (and userspace
has no control over this), I don't think that it makes sense to expose
to userspace, much like CHAIN, so I suspect it's better to update the
commentary instead.

Do you have a need for SW_INCR, or was this found by inspection?

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Mark,

On 2021/6/7 16:37, Mark Rutland wrote:

On Mon, Jun 07, 2021 at 03:19:03PM +0800, Shaokun Zhang wrote:

quoted

Raw event SW_INCR (0x0000) shall be included in this check, so fix it.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Shaokun Zhang <redacted>

Given that SW_INCR depends on the specific counter used (and userspace
has no control over this), I don't think that it makes sense to expose
to userspace, much like CHAIN, so I suspect it's better to update the
commentary instead.

Indeed, no harm on perf use.

Do you have a need for SW_INCR, or was this found by inspection?

By inspection, when I traced some other thing and noticed that the
event 0x0000 was exclusive in the if condition by accident.

Thanks,
Shaokun

Thanks,
Mark.

quoted

---
 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 a661010308c0..fa2f60d09e15 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c

@@ -1000,7 +1000,7 @@ static int __armv8_pmuv3_map_event(struct perf_event *event,
 		event->hw.flags |= ARMPMU_EVT_64BIT;
 
 	/* Only expose micro/arch events supported by this PMU */
-	if ((hw_event_id > 0) && (hw_event_id < ARMV8_PMUV3_MAX_COMMON_EVENTS)
+	if ((hw_event_id >= 0) && (hw_event_id < ARMV8_PMUV3_MAX_COMMON_EVENTS)
 	    && test_bit(hw_event_id, armpmu->pmceid_bitmap)) {
 		return hw_event_id;
 	}

-- 
2.7.4

.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel