| Message ID | 1623050343-62397-1-git-send-email-zhangshaokun@hisilicon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] arm64: perf: Fix hw_event_id check in __armv8_pmuv3_map_event | expand |
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 >
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 >> > . >
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; }
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(-)