* [PATCH] perf/arm-cmn: Move overlapping wp_combine field
@ 2023-03-01 17:55 Ilkka Koskinen
2023-03-01 18:39 ` Robin Murphy
2023-03-27 15:01 ` Will Deacon
0 siblings, 2 replies; 5+ messages in thread
From: Ilkka Koskinen @ 2023-03-01 17:55 UTC (permalink / raw)
To: Robin Murphy, Will Deacon
Cc: Mark Rutland, linux-arm-kernel, Ilkka Koskinen, patches
As eventid field was expanded to support new mesh versions, it started to
overlap with wp_combine field. Move wp_combine to fix the issue.
Fixes: 23760a014417 ("perf/arm-cmn: Add CMN-700 support")
Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
---
drivers/perf/arm-cmn.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
index c9689861be3f..9f2edc28d16e 100644
--- a/drivers/perf/arm-cmn.c
+++ b/drivers/perf/arm-cmn.c
@@ -166,7 +166,7 @@
#define CMN_EVENT_BYNODEID(event) FIELD_GET(CMN_CONFIG_BYNODEID, (event)->attr.config)
#define CMN_EVENT_NODEID(event) FIELD_GET(CMN_CONFIG_NODEID, (event)->attr.config)
-#define CMN_CONFIG_WP_COMBINE GENMASK_ULL(27, 24)
+#define CMN_CONFIG_WP_COMBINE GENMASK_ULL(30, 27)
#define CMN_CONFIG_WP_DEV_SEL GENMASK_ULL(50, 48)
#define CMN_CONFIG_WP_CHN_SEL GENMASK_ULL(55, 51)
/* Note that we don't yet support the tertiary match group on newer IPs */
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] perf/arm-cmn: Move overlapping wp_combine field 2023-03-01 17:55 [PATCH] perf/arm-cmn: Move overlapping wp_combine field Ilkka Koskinen @ 2023-03-01 18:39 ` Robin Murphy 2023-03-02 0:55 ` Ilkka Koskinen 2023-03-27 15:01 ` Will Deacon 1 sibling, 1 reply; 5+ messages in thread From: Robin Murphy @ 2023-03-01 18:39 UTC (permalink / raw) To: Ilkka Koskinen, Will Deacon; +Cc: Mark Rutland, linux-arm-kernel, patches On 2023-03-01 17:55, Ilkka Koskinen wrote: > As eventid field was expanded to support new mesh versions, it started to > overlap with wp_combine field. Move wp_combine to fix the issue. Watchpoint events still only strictly need 2 bits of eventid, though. Could you clarify whether userspace is getting confused by this, or whether it's just arm_cmn_event_init() falling over itself (FWIW I can see how I broke things there...) Thanks, Robin. > Fixes: 23760a014417 ("perf/arm-cmn: Add CMN-700 support") > Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> > --- > drivers/perf/arm-cmn.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c > index c9689861be3f..9f2edc28d16e 100644 > --- a/drivers/perf/arm-cmn.c > +++ b/drivers/perf/arm-cmn.c > @@ -166,7 +166,7 @@ > #define CMN_EVENT_BYNODEID(event) FIELD_GET(CMN_CONFIG_BYNODEID, (event)->attr.config) > #define CMN_EVENT_NODEID(event) FIELD_GET(CMN_CONFIG_NODEID, (event)->attr.config) > > -#define CMN_CONFIG_WP_COMBINE GENMASK_ULL(27, 24) > +#define CMN_CONFIG_WP_COMBINE GENMASK_ULL(30, 27) > #define CMN_CONFIG_WP_DEV_SEL GENMASK_ULL(50, 48) > #define CMN_CONFIG_WP_CHN_SEL GENMASK_ULL(55, 51) > /* Note that we don't yet support the tertiary match group on newer IPs */ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf/arm-cmn: Move overlapping wp_combine field 2023-03-01 18:39 ` Robin Murphy @ 2023-03-02 0:55 ` Ilkka Koskinen 2023-03-20 23:47 ` Ilkka Koskinen 0 siblings, 1 reply; 5+ messages in thread From: Ilkka Koskinen @ 2023-03-02 0:55 UTC (permalink / raw) To: Robin Murphy Cc: Ilkka Koskinen, Will Deacon, Mark Rutland, linux-arm-kernel, patches Hi Robin, On Wed, 1 Mar 2023, Robin Murphy wrote: > On 2023-03-01 17:55, Ilkka Koskinen wrote: >> As eventid field was expanded to support new mesh versions, it started to >> overlap with wp_combine field. Move wp_combine to fix the issue. > > Watchpoint events still only strictly need 2 bits of eventid, though. Could > you clarify whether userspace is getting confused by this, or whether it's > just arm_cmn_event_init() falling over itself (FWIW I can see how I broke > things there...) Basically, perf seems to set eventid at first and then wp_combine field into the config argument. The problem is when arm_cmn_event_init() is doing: eventid = CMN_EVENT_EVENTID(event); .... if (eventid != CMN_WP_UP && eventid != CMN_WP_DOWN) return -EINVAL; If wp_combine has any of the overlapping bits set, eventid doesn't match with either up or down event. Cheers, Ilkka > > Thanks, > Robin. > >> Fixes: 23760a014417 ("perf/arm-cmn: Add CMN-700 support") >> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> >> --- >> drivers/perf/arm-cmn.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c >> index c9689861be3f..9f2edc28d16e 100644 >> --- a/drivers/perf/arm-cmn.c >> +++ b/drivers/perf/arm-cmn.c >> @@ -166,7 +166,7 @@ >> #define CMN_EVENT_BYNODEID(event) FIELD_GET(CMN_CONFIG_BYNODEID, >> (event)->attr.config) >> #define CMN_EVENT_NODEID(event) FIELD_GET(CMN_CONFIG_NODEID, >> (event)->attr.config) >> -#define CMN_CONFIG_WP_COMBINE GENMASK_ULL(27, 24) >> +#define CMN_CONFIG_WP_COMBINE GENMASK_ULL(30, 27) >> #define CMN_CONFIG_WP_DEV_SEL GENMASK_ULL(50, 48) >> #define CMN_CONFIG_WP_CHN_SEL GENMASK_ULL(55, 51) >> /* Note that we don't yet support the tertiary match group on newer IPs >> */ > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf/arm-cmn: Move overlapping wp_combine field 2023-03-02 0:55 ` Ilkka Koskinen @ 2023-03-20 23:47 ` Ilkka Koskinen 0 siblings, 0 replies; 5+ messages in thread From: Ilkka Koskinen @ 2023-03-20 23:47 UTC (permalink / raw) To: Robin Murphy; +Cc: Will Deacon, Mark Rutland, linux-arm-kernel, patches Hi Robin, On Wed, 1 Mar 2023, Ilkka Koskinen wrote: > > Hi Robin, > > On Wed, 1 Mar 2023, Robin Murphy wrote: >> On 2023-03-01 17:55, Ilkka Koskinen wrote: >>> As eventid field was expanded to support new mesh versions, it started to >>> overlap with wp_combine field. Move wp_combine to fix the issue. >> >> Watchpoint events still only strictly need 2 bits of eventid, though. Could >> you clarify whether userspace is getting confused by this, or whether it's >> just arm_cmn_event_init() falling over itself (FWIW I can see how I broke >> things there...) > > Basically, perf seems to set eventid at first and then wp_combine field into > the config argument. The problem is when arm_cmn_event_init() is doing: > > eventid = CMN_EVENT_EVENTID(event); > .... > if (eventid != CMN_WP_UP && eventid != CMN_WP_DOWN) > return -EINVAL; > > If wp_combine has any of the overlapping bits set, eventid doesn't match with > either up or down event. I probably should have also copy pasted what the format files say: $ cat format/eventid format/wp_combine config:16-26 config:24-27 So, here you can see that those two fields overlap. If wp_combine has any of the bits 24-26 set, arm_cmn_event_init() returns -EINVAL as shown in the code above. Would you need some more clarification from me? Br, Ilkka > > Cheers, Ilkka > >> >> Thanks, >> Robin. >> >>> Fixes: 23760a014417 ("perf/arm-cmn: Add CMN-700 support") >>> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> >>> --- >>> drivers/perf/arm-cmn.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c >>> index c9689861be3f..9f2edc28d16e 100644 >>> --- a/drivers/perf/arm-cmn.c >>> +++ b/drivers/perf/arm-cmn.c >>> @@ -166,7 +166,7 @@ >>> #define CMN_EVENT_BYNODEID(event) FIELD_GET(CMN_CONFIG_BYNODEID, >>> (event)->attr.config) >>> #define CMN_EVENT_NODEID(event) FIELD_GET(CMN_CONFIG_NODEID, >>> (event)->attr.config) >>> -#define CMN_CONFIG_WP_COMBINE GENMASK_ULL(27, 24) >>> +#define CMN_CONFIG_WP_COMBINE GENMASK_ULL(30, 27) >>> #define CMN_CONFIG_WP_DEV_SEL GENMASK_ULL(50, 48) >>> #define CMN_CONFIG_WP_CHN_SEL GENMASK_ULL(55, 51) >>> /* Note that we don't yet support the tertiary match group on newer IPs >>> */ >> > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf/arm-cmn: Move overlapping wp_combine field 2023-03-01 17:55 [PATCH] perf/arm-cmn: Move overlapping wp_combine field Ilkka Koskinen 2023-03-01 18:39 ` Robin Murphy @ 2023-03-27 15:01 ` Will Deacon 1 sibling, 0 replies; 5+ messages in thread From: Will Deacon @ 2023-03-27 15:01 UTC (permalink / raw) To: Ilkka Koskinen, Robin Murphy Cc: catalin.marinas, kernel-team, Will Deacon, Mark Rutland, patches, linux-arm-kernel On Wed, 1 Mar 2023 09:55:40 -0800, Ilkka Koskinen wrote: > As eventid field was expanded to support new mesh versions, it started to > overlap with wp_combine field. Move wp_combine to fix the issue. > > Applied to will (for-next/perf), thanks! [1/1] perf/arm-cmn: Move overlapping wp_combine field https://git.kernel.org/will/c/f87e9114b5e5 Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-27 15:02 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-01 17:55 [PATCH] perf/arm-cmn: Move overlapping wp_combine field Ilkka Koskinen 2023-03-01 18:39 ` Robin Murphy 2023-03-02 0:55 ` Ilkka Koskinen 2023-03-20 23:47 ` Ilkka Koskinen 2023-03-27 15:01 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).