* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.