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

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

[Shaokun Zhang]

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 <zhangshaokun@hisilicon.com>
---
 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

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

[Mark Rutland]

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 <zhangshaokun@hisilicon.com>

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.

> ---
>  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

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

[Shaokun Zhang]

Hi Mark,

On 2021/6/7 16:37, Mark Rutland wrote:
> 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 <zhangshaokun@hisilicon.com>
> 
> 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.
> 
>> ---
>>  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