[PATCH] perf/arm-cmn: fix regitster offset of CMN_MXP__CONNECT_INFO_P2-5

[PATCH] perf/arm-cmn: fix regitster offset of CMN_MXP__CONNECT_INFO_P2-5

[Jing Zhang]

In the CMN700 specification, it is described that the address offset
of the mxp_device_connect_info_p0-5 register is 16'h8 + #{8*index}.
Therefore, the address offset of the CMN_MXP__CONNECT_INFO_P2-5 macro
defined in the code is wrong, which causes the cmn700 topology map
incorrect printed in debugfs.

So correct the address offset value to make the cmn700 topology map
correct in debugfs.

Fixes: 60d1504070c2 ("perf/arm-cmn: Support new IP features")
Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
---
 drivers/perf/arm-cmn.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
index 1deb61b..e9f844b 100644
--- a/drivers/perf/arm-cmn.c
+++ b/drivers/perf/arm-cmn.c
@@ -59,10 +59,10 @@
 /* XPs also have some local topology info which has uses too */
 #define CMN_MXP__CONNECT_INFO_P0	0x0008
 #define CMN_MXP__CONNECT_INFO_P1	0x0010
-#define CMN_MXP__CONNECT_INFO_P2	0x0028
-#define CMN_MXP__CONNECT_INFO_P3	0x0030
-#define CMN_MXP__CONNECT_INFO_P4	0x0038
-#define CMN_MXP__CONNECT_INFO_P5	0x0040
+#define CMN_MXP__CONNECT_INFO_P2	0x0018
+#define CMN_MXP__CONNECT_INFO_P3	0x0020
+#define CMN_MXP__CONNECT_INFO_P4	0x0028
+#define CMN_MXP__CONNECT_INFO_P5	0x0030
 #define CMN__CONNECT_INFO_DEVICE_TYPE	GENMASK_ULL(4, 0)
 
 /* PMU registers occupy the 3rd 4KB page of each node's region */
-- 
1.8.3.1

Re: [PATCH] perf/arm-cmn: fix regitster offset of CMN_MXP__CONNECT_INFO_P2-5

[Robin Murphy]

On 2023-03-30 10:47, Jing Zhang wrote:
> In the CMN700 specification, it is described that the address offset
> of the mxp_device_connect_info_p0-5 register is 16'h8 + #{8*index}.
> Therefore, the address offset of the CMN_MXP__CONNECT_INFO_P2-5 macro
> defined in the code is wrong, which causes the cmn700 topology map
> incorrect printed in debugfs.
> 
> So correct the address offset value to make the cmn700 topology map
> correct in debugfs.
> 
> Fixes: 60d1504070c2 ("perf/arm-cmn: Support new IP features")

Ugh, these offsets are correct for CI-700, so strictly that commit is 
fine. What I failed to notice is that CMN-700 shuffled the 
mesh_port_connect_info registers out of the way, so it's commit 
23760a014417 which should have added more handling for this difference.

Thanks,
Robin.

> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
> Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
> ---
>   drivers/perf/arm-cmn.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index 1deb61b..e9f844b 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c
> @@ -59,10 +59,10 @@
>   /* XPs also have some local topology info which has uses too */
>   #define CMN_MXP__CONNECT_INFO_P0	0x0008
>   #define CMN_MXP__CONNECT_INFO_P1	0x0010
> -#define CMN_MXP__CONNECT_INFO_P2	0x0028
> -#define CMN_MXP__CONNECT_INFO_P3	0x0030
> -#define CMN_MXP__CONNECT_INFO_P4	0x0038
> -#define CMN_MXP__CONNECT_INFO_P5	0x0040
> +#define CMN_MXP__CONNECT_INFO_P2	0x0018
> +#define CMN_MXP__CONNECT_INFO_P3	0x0020
> +#define CMN_MXP__CONNECT_INFO_P4	0x0028
> +#define CMN_MXP__CONNECT_INFO_P5	0x0030
>   #define CMN__CONNECT_INFO_DEVICE_TYPE	GENMASK_ULL(4, 0)
>   
>   /* PMU registers occupy the 3rd 4KB page of each node's region */

Re: [PATCH] perf/arm-cmn: fix regitster offset of CMN_MXP__CONNECT_INFO_P2-5

[Mark Rutland]

On Thu, Mar 30, 2023 at 11:34:22AM +0100, Robin Murphy wrote:
> On 2023-03-30 10:47, Jing Zhang wrote:
> > In the CMN700 specification, it is described that the address offset
> > of the mxp_device_connect_info_p0-5 register is 16'h8 + #{8*index}.
> > Therefore, the address offset of the CMN_MXP__CONNECT_INFO_P2-5 macro
> > defined in the code is wrong, which causes the cmn700 topology map
> > incorrect printed in debugfs.
> > 
> > So correct the address offset value to make the cmn700 topology map
> > correct in debugfs.
> > 
> > Fixes: 60d1504070c2 ("perf/arm-cmn: Support new IP features")
> 
> Ugh, these offsets are correct for CI-700, so strictly that commit is fine.
> What I failed to notice is that CMN-700 shuffled the mesh_port_connect_info
> registers out of the way, so it's commit 23760a014417 which should have
> added more handling for this difference.

I'm assuming that means that this patch breaks !CMN-700 ?

i.e. a more substantial fix is necessary, and it's not just a matter of
changing the Fixes tag.

Thanks,
Mark.

> 
> Thanks,
> Robin.
> 
> > Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
> > Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
> > ---
> >   drivers/perf/arm-cmn.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> > index 1deb61b..e9f844b 100644
> > --- a/drivers/perf/arm-cmn.c
> > +++ b/drivers/perf/arm-cmn.c
> > @@ -59,10 +59,10 @@
> >   /* XPs also have some local topology info which has uses too */
> >   #define CMN_MXP__CONNECT_INFO_P0	0x0008
> >   #define CMN_MXP__CONNECT_INFO_P1	0x0010
> > -#define CMN_MXP__CONNECT_INFO_P2	0x0028
> > -#define CMN_MXP__CONNECT_INFO_P3	0x0030
> > -#define CMN_MXP__CONNECT_INFO_P4	0x0038
> > -#define CMN_MXP__CONNECT_INFO_P5	0x0040
> > +#define CMN_MXP__CONNECT_INFO_P2	0x0018
> > +#define CMN_MXP__CONNECT_INFO_P3	0x0020
> > +#define CMN_MXP__CONNECT_INFO_P4	0x0028
> > +#define CMN_MXP__CONNECT_INFO_P5	0x0030
> >   #define CMN__CONNECT_INFO_DEVICE_TYPE	GENMASK_ULL(4, 0)
> >   /* PMU registers occupy the 3rd 4KB page of each node's region */

Re: [PATCH] perf/arm-cmn: fix regitster offset of CMN_MXP__CONNECT_INFO_P2-5

[Robin Murphy]

On 2023-04-03 08:45, Mark Rutland wrote:
> On Thu, Mar 30, 2023 at 11:34:22AM +0100, Robin Murphy wrote:
>> On 2023-03-30 10:47, Jing Zhang wrote:
>>> In the CMN700 specification, it is described that the address offset
>>> of the mxp_device_connect_info_p0-5 register is 16'h8 + #{8*index}.
>>> Therefore, the address offset of the CMN_MXP__CONNECT_INFO_P2-5 macro
>>> defined in the code is wrong, which causes the cmn700 topology map
>>> incorrect printed in debugfs.
>>>
>>> So correct the address offset value to make the cmn700 topology map
>>> correct in debugfs.
>>>
>>> Fixes: 60d1504070c2 ("perf/arm-cmn: Support new IP features")
>>
>> Ugh, these offsets are correct for CI-700, so strictly that commit is fine.
>> What I failed to notice is that CMN-700 shuffled the mesh_port_connect_info
>> registers out of the way, so it's commit 23760a014417 which should have
>> added more handling for this difference.
> 
> I'm assuming that means that this patch breaks !CMN-700 ?
> 
> i.e. a more substantial fix is necessary, and it's not just a matter of
> changing the Fixes tag.

Yes, indeed. I have half an idea, and the plan for this week was to work 
on other CMN stuff anyway, so I'll see what I can cook up.

Cheers,
Robin.

Re: [PATCH] perf/arm-cmn: fix regitster offset of CMN_MXP__CONNECT_INFO_P2-5

[Jing Zhang]

在 2023/4/3 下午5:40, Robin Murphy 写道:
> On 2023-04-03 08:45, Mark Rutland wrote:
>> On Thu, Mar 30, 2023 at 11:34:22AM +0100, Robin Murphy wrote:
>>> On 2023-03-30 10:47, Jing Zhang wrote:
>>>> In the CMN700 specification, it is described that the address offset
>>>> of the mxp_device_connect_info_p0-5 register is 16'h8 + #{8*index}.
>>>> Therefore, the address offset of the CMN_MXP__CONNECT_INFO_P2-5 macro
>>>> defined in the code is wrong, which causes the cmn700 topology map
>>>> incorrect printed in debugfs.
>>>>
>>>> So correct the address offset value to make the cmn700 topology map
>>>> correct in debugfs.
>>>>
>>>> Fixes: 60d1504070c2 ("perf/arm-cmn: Support new IP features")
>>>
>>> Ugh, these offsets are correct for CI-700, so strictly that commit is fine.
>>> What I failed to notice is that CMN-700 shuffled the mesh_port_connect_info
>>> registers out of the way, so it's commit 23760a014417 which should have
>>> added more handling for this difference.
>>
>> I'm assuming that means that this patch breaks !CMN-700 ?
>>
>> i.e. a more substantial fix is necessary, and it's not just a matter of
>> changing the Fixes tag.
> 
> Yes, indeed. I have half an idea, and the plan for this week was to work on other CMN stuff anyway, so I'll see what I can cook up.
> 
> Cheers,
> Robin.

Yes, I did ignore the CI700 situation, I don't know much about !CMN700.
Robin, can you please help to fix this problem? Thank you so much.

Thanks,
Jing