[PATCH drivers/perf: hisi: 0/2] There are some bugfix for the hisi perf driver

[PATCH drivers/perf: hisi: 0/2] There are some bugfix for the hisi perf driver

[Jijie Shao]

There are some bugfix for the hisi perf driver

Hao Chen (2):
  drivers/perf: hisi: hns3: default use hardware event 0 as group leader
    event.
  drivers/perf: hisi: fix set wrong filter mode for running events issue

 drivers/perf/hisilicon/hns3_pmu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH drivers/perf: hisi: 1/2] drivers/perf: hisi: hns3: default use hardware event 0 as group leader event.

[Jijie Shao]

From: Hao Chen <chenhao418@huawei.com>

For hns3 pmu events, we use command as below before:
perf stat -g -e hns3_pmu_sicl_0/config=0x00105,global=1/
-e hns3_pmu_sicl_0/config=0x10105,global=1/ -I 1000

We want to use -g parameter to make 0x00105 event and 0x10105 event
share a hardware event, but for kernel 6.2, 'commit 5f8f95673f68
("perf evlist: Remove group option.")' remove -g parameter.

So add this patch to set default related event idx as 0 to share
the first hardware event.

The new command shows as below:
perf stat -e hns3_pmu_sicl_0/config=0x00105,global=1/
-e hns3_pmu_sicl_0/config=0x10105,global=1/ -I 1000

Fixes: 5f8f95673f68 ("perf evlist: Remove group option.")
Signed-off-by: Hao Chen <chenhao418@huawei.com>
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 drivers/perf/hisilicon/hns3_pmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/hisilicon/hns3_pmu.c b/drivers/perf/hisilicon/hns3_pmu.c
index e0457d84af6b..6a4d04cbae91 100644
--- a/drivers/perf/hisilicon/hns3_pmu.c
+++ b/drivers/perf/hisilicon/hns3_pmu.c
@@ -796,9 +796,11 @@ static int hns3_pmu_find_related_event_idx(struct hns3_pmu *hns3_pmu,
 		if (!hns3_pmu_cmp_event(sibling, event))
 			continue;
 
-		/* Related events is used in group */
+		/* Related events is used in group, else we use index 0 event as related event */
 		if (sibling->group_leader == event->group_leader)
 			return idx;
+		else
+			return 0;
 	}
 
 	/* No related event and all hardware events are used up */
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH drivers/perf: hisi: 1/2] drivers/perf: hisi: hns3: default use hardware event 0 as group leader event.

[Will Deacon]

On Wed, Aug 16, 2023 at 05:46:18PM +0800, Jijie Shao wrote:
> From: Hao Chen <chenhao418@huawei.com>
> 
> For hns3 pmu events, we use command as below before:
> perf stat -g -e hns3_pmu_sicl_0/config=0x00105,global=1/
> -e hns3_pmu_sicl_0/config=0x10105,global=1/ -I 1000
> 
> We want to use -g parameter to make 0x00105 event and 0x10105 event
> share a hardware event, but for kernel 6.2, 'commit 5f8f95673f68
> ("perf evlist: Remove group option.")' remove -g parameter.
> 
> So add this patch to set default related event idx as 0 to share
> the first hardware event.

Hmm, but the change cited above is a userspace change so I don't think
we should be making driver-side changes for that. Furthermore, the commit
message there suggests that you should be using a different syntax,
introduced by 89efb029502d ("perf tools: Add support to parse event
group syntax") (which describes the grammer and has some examples too).

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH drivers/perf: hisi: 1/2] drivers/perf: hisi: hns3: default use hardware event 0 as group leader event.

[Jijie Shao]

on 2023/8/21 19:58, Will Deacon wrote:
> On Wed, Aug 16, 2023 at 05:46:18PM +0800, Jijie Shao wrote:
>> From: Hao Chen <chenhao418@huawei.com>
>>
>> For hns3 pmu events, we use command as below before:
>> perf stat -g -e hns3_pmu_sicl_0/config=0x00105,global=1/
>> -e hns3_pmu_sicl_0/config=0x10105,global=1/ -I 1000
>>
>> We want to use -g parameter to make 0x00105 event and 0x10105 event
>> share a hardware event, but for kernel 6.2, 'commit 5f8f95673f68
>> ("perf evlist: Remove group option.")' remove -g parameter.
>>
>> So add this patch to set default related event idx as 0 to share
>> the first hardware event.
> Hmm, but the change cited above is a userspace change so I don't think
> we should be making driver-side changes for that. Furthermore, the commit
> message there suggests that you should be using a different syntax,
> introduced by 89efb029502d ("perf tools: Add support to parse event
> group syntax") (which describes the grammer and has some examples too).
>
> Will

Thanks for your introduction, we are retesting this scenario as

89efb029502d("perf tools: Add support to parse eventgroup syntax")

Jijie Shao


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH drivers/perf: hisi: 2/2] drivers/perf: hisi: fix set wrong filter mode for running events issue

[Jijie Shao]

From: Hao Chen <chenhao418@huawei.com>

hns3_pmu_select_filter_mode() includes a series of mode judgments such
as global mode ,function mode, function-queue mode, port mode, port-tc
mode.

For a special scenario, command use parameter "bdf=0x3700,config=0x3,
queue=0x0", it is expected to enter function-queue mode, but event of
config 0x3 doesn't support func-queue mode, then it enter port-tc mode.
it's not up to expectations.

It shouldn't enter any modes but return -ENOENT.

So, add judgement of bdf parameter to fix it.

Fixes: 66637ab137b4 ("drivers/perf: hisi: add driver for HNS3 PMU")
Signed-off-by: Hao Chen <chenhao418@huawei.com>
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 drivers/perf/hisilicon/hns3_pmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/hisilicon/hns3_pmu.c b/drivers/perf/hisilicon/hns3_pmu.c
index 6a4d04cbae91..7f38a9e489d4 100644
--- a/drivers/perf/hisilicon/hns3_pmu.c
+++ b/drivers/perf/hisilicon/hns3_pmu.c
@@ -1000,12 +1000,13 @@ static bool
 hns3_pmu_is_enabled_port_tc_mode(struct perf_event *event,
 				 struct hns3_pmu_event_attr *pmu_event)
 {
+	u16 bdf = hns3_pmu_get_bdf(event);
 	u8 tc_id = hns3_pmu_get_tc(event);
 
 	if (!(pmu_event->filter_support & HNS3_PMU_FILTER_SUPPORT_PORT_TC))
 		return false;
 
-	return tc_id != HNS3_PMU_FILTER_ALL_TC;
+	return (tc_id != HNS3_PMU_FILTER_ALL_TC) && (!bdf);
 }
 
 static bool
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH drivers/perf: hisi: 2/2] drivers/perf: hisi: fix set wrong filter mode for running events issue

[Will Deacon]

[+Guangbin Huang and Shaokun Zhang]

On Wed, Aug 16, 2023 at 05:46:19PM +0800, Jijie Shao wrote:
> From: Hao Chen <chenhao418@huawei.com>
> 
> hns3_pmu_select_filter_mode() includes a series of mode judgments such
> as global mode ,function mode, function-queue mode, port mode, port-tc
> mode.
> 
> For a special scenario, command use parameter "bdf=0x3700,config=0x3,
> queue=0x0", it is expected to enter function-queue mode, but event of
> config 0x3 doesn't support func-queue mode, then it enter port-tc mode.
> it's not up to expectations.
> 
> It shouldn't enter any modes but return -ENOENT.
> 
> So, add judgement of bdf parameter to fix it.
> 
> Fixes: 66637ab137b4 ("drivers/perf: hisi: add driver for HNS3 PMU")
> Signed-off-by: Hao Chen <chenhao418@huawei.com>
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
>  drivers/perf/hisilicon/hns3_pmu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/hisilicon/hns3_pmu.c b/drivers/perf/hisilicon/hns3_pmu.c
> index 6a4d04cbae91..7f38a9e489d4 100644
> --- a/drivers/perf/hisilicon/hns3_pmu.c
> +++ b/drivers/perf/hisilicon/hns3_pmu.c
> @@ -1000,12 +1000,13 @@ static bool
>  hns3_pmu_is_enabled_port_tc_mode(struct perf_event *event,
>  				 struct hns3_pmu_event_attr *pmu_event)
>  {
> +	u16 bdf = hns3_pmu_get_bdf(event);
>  	u8 tc_id = hns3_pmu_get_tc(event);
>  
>  	if (!(pmu_event->filter_support & HNS3_PMU_FILTER_SUPPORT_PORT_TC))
>  		return false;
>  
> -	return tc_id != HNS3_PMU_FILTER_ALL_TC;
> +	return (tc_id != HNS3_PMU_FILTER_ALL_TC) && (!bdf);
>  }

Sorry, but I'm struggling to see how this correlates with your example
in the commit message, which implies that it's the config of 0x3 causing
the problem rather than the bdf.

Please can you explain the problem in more detail?

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH drivers/perf: hisi: 2/2] drivers/perf: hisi: fix set wrong filter mode for running events issue

[Will Deacon]

On Mon, Aug 21, 2023 at 01:04:40PM +0100, Will Deacon wrote:
> [+Guangbin Huang and Shaokun Zhang]

                   The mail system

<zhangshaokun@hisilicon.com>: host mx5.hisilicon.com[124.71.93.234] said: 551
    5.1.1 <zhangshaokun@hisilicon.com>: Recipient address rejected: Failed
    recipient validation check.: host 127.0.0.1[127.0.0.1] said: 554 5.7.1
    recipient verify from ldap failed (in reply to RCPT TO command) (in reply
    to RCPT TO command)

<huangguangbin2@huawei.com>: host mx5.huawei.com[124.71.93.234] said: 551 5.1.1
    <huangguangbin2@huawei.com>: Recipient address rejected: Failed recipient
    validation check.: host 127.0.0.1[127.0.0.1] said: 554 5.7.1 recipient
    verify from ldap failed (in reply to RCPT TO command) (in reply to RCPT TO
    command)


So it's only Jonathan left in MAINTAINERS for "HISILICON HNS3 PMU DRIVER".
Lucky him!

Please can you update MAINTAINERS?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH drivers/perf: hisi: 2/2] drivers/perf: hisi: fix set wrong filter mode for running events issue

[Jijie Shao]

on 2023/8/21 20:08, Will Deacon wrote:
> <zhangshaokun@hisilicon.com>: host mx5.hisilicon.com[124.71.93.234] said: 551
>      5.1.1 <zhangshaokun@hisilicon.com>: Recipient address rejected: Failed
>      recipient validation check.: host 127.0.0.1[127.0.0.1] said: 554 5.7.1
>      recipient verify from ldap failed (in reply to RCPT TO command) (in reply
>      to RCPT TO command)
>
> <huangguangbin2@huawei.com>: host mx5.huawei.com[124.71.93.234] said: 551 5.1.1
>      <huangguangbin2@huawei.com>: Recipient address rejected: Failed recipient
>      validation check.: host 127.0.0.1[127.0.0.1] said: 554 5.7.1 recipient
>      verify from ldap failed (in reply to RCPT TO command) (in reply to RCPT TO
>      command)
>
>
> So it's only Jonathan left in MAINTAINERS for "HISILICON HNS3 PMU DRIVER".
> Lucky him!
>
> Please can you update MAINTAINERS?
>
> Will


We will update MAINTAINERS shortly

Jijie Shao


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH drivers/perf: hisi: 2/2] drivers/perf: hisi: fix set wrong filter mode for running events issue

[Jijie Shao]

on 2023/8/21 20:04, Will Deacon wrote:
>> -	return tc_id != HNS3_PMU_FILTER_ALL_TC;
>> +	return (tc_id != HNS3_PMU_FILTER_ALL_TC) && (!bdf);
>>   }
> Sorry, but I'm struggling to see how this correlates with your example
> in the commit message, which implies that it's the config of 0x3 causing
> the problem rather than the bdf.
>
> Please can you explain the problem in more detail?
>
> Thanks,
>
> Will


We'll send the v2 patch to add more details.

Jijie Shao


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel