[PATCH 6/9] drivers/perf: hisi: Relax the event number check of v2 PMUs

Yicong Yang posted 9 patches 10 months ago
There is a newer version of this series
[PATCH 6/9] drivers/perf: hisi: Relax the event number check of v2 PMUs
Posted by Yicong Yang 10 months ago
From: Junhao He <hejunhao3@huawei.com>

The supported event number range of each Uncore PMUs is provided by
each driver in hisi_pmu::check_event and out of range events
will be rejected. A later version with expanded event number range
needs to register the PMU with updated hisi_pmu::check_event
even if it's the only update, which means the expanded events
cannot be used unless the driver's updated. However the unsupported
events won't be counted by the hardware so we can relax the event
number check to allow the use the expanded events.

Signed-off-by: Junhao He <hejunhao3@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 2 +-
 drivers/perf/hisilicon/hisi_uncore_hha_pmu.c  | 7 +++----
 drivers/perf/hisilicon/hisi_uncore_pa_pmu.c   | 2 +-
 drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 3 +--
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
index 26eaa6d20c00..21c494881ca0 100644
--- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
@@ -53,7 +53,7 @@
 #define DDRC_V1_PERF_CTRL_EN	0x2
 #define DDRC_V2_PERF_CTRL_EN	0x1
 #define DDRC_V1_NR_EVENTS	0x7
-#define DDRC_V2_NR_EVENTS	0x90
+#define DDRC_V2_NR_EVENTS	0xFF
 
 #define DDRC_EVENT_CNTn(base, n)	((base) + (n) * 8)
 #define DDRC_EVENT_TYPEn(base, n)	((base) + (n) * 4)
diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
index ca609db86046..78cd6d67f209 100644
--- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
@@ -47,9 +47,8 @@
 #define HHA_SRCID_CMD		GENMASK(16, 6)
 #define HHA_SRCID_MSK		GENMASK(30, 20)
 #define HHA_DATSRC_SKT_EN	BIT(23)
-#define HHA_EVTYPE_NONE		0xff
+#define HHA_EVTYPE_MASK		GENMASK(7, 0)
 #define HHA_V1_NR_EVENT		0x65
-#define HHA_V2_NR_EVENT		0xCE
 
 HISI_PMU_EVENT_ATTR_EXTRACTOR(srcid_cmd, config1, 10, 0);
 HISI_PMU_EVENT_ATTR_EXTRACTOR(srcid_msk, config1, 21, 11);
@@ -197,7 +196,7 @@ static void hisi_hha_pmu_write_evtype(struct hisi_pmu *hha_pmu, int idx,
 
 	/* Write event code to HHA_EVENT_TYPEx register */
 	val = readl(hha_pmu->base + reg);
-	val &= ~(HHA_EVTYPE_NONE << shift);
+	val &= ~(HHA_EVTYPE_MASK << shift);
 	val |= (type << shift);
 	writel(val, hha_pmu->base + reg);
 }
@@ -453,7 +452,7 @@ static int hisi_hha_pmu_dev_probe(struct platform_device *pdev,
 
 	if (hha_pmu->identifier >= HISI_PMU_V2) {
 		hha_pmu->counter_bits = 64;
-		hha_pmu->check_event = HHA_V2_NR_EVENT;
+		hha_pmu->check_event = HHA_EVTYPE_MASK;
 		hha_pmu->pmu_events.attr_groups = hisi_hha_pmu_v2_attr_groups;
 		hha_pmu->num_counters = HHA_V2_NR_COUNTERS;
 	} else {
diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
index a0142684e379..80108c63cb60 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
@@ -440,7 +440,7 @@ static int hisi_pa_pmu_dev_probe(struct platform_device *pdev,
 	pa_pmu->pmu_events.attr_groups = pa_pmu->dev_info->attr_groups;
 	pa_pmu->num_counters = PA_NR_COUNTERS;
 	pa_pmu->ops = &hisi_uncore_pa_ops;
-	pa_pmu->check_event = 0xB0;
+	pa_pmu->check_event = PA_EVTYPE_MASK;
 	pa_pmu->counter_bits = 64;
 	pa_pmu->dev = &pdev->dev;
 	pa_pmu->on_cpu = -1;
diff --git a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
index dce7195320f2..ea31b64e05e2 100644
--- a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
@@ -58,7 +58,6 @@
 #define SLLC_V3_SRCID_CMD_MSK		GENMASK(9, 1)
 #define SLLC_V3_SRCID_MSK_MSK		GENMASK(18, 10)
 
-#define SLLC_NR_EVENTS			0x80
 #define SLLC_EVENT_CNTn(cnt0, n)	((cnt0) + (n) * 8)
 #define SLLC_FIRST_BIT(_mask)		(find_first_bit((const unsigned long *)&(_mask), 32))
 #define SLLC_FIELD_PREP(_mask, _val)	(_mask & (_val << SLLC_FIRST_BIT(_mask)))
@@ -476,7 +475,7 @@ static int hisi_sllc_pmu_dev_probe(struct platform_device *pdev,
 
 	sllc_pmu->pmu_events.attr_groups = hisi_sllc_pmu_v2_attr_groups;
 	sllc_pmu->ops = &hisi_uncore_sllc_ops;
-	sllc_pmu->check_event = SLLC_NR_EVENTS;
+	sllc_pmu->check_event = SLLC_EVTYPE_MASK;
 	sllc_pmu->counter_bits = 64;
 	sllc_pmu->num_counters = 8;
 	sllc_pmu->dev = &pdev->dev;
-- 
2.24.0
Re: [PATCH 6/9] drivers/perf: hisi: Relax the event number check of v2 PMUs
Posted by Jonathan Cameron 9 months, 2 weeks ago
On Tue, 18 Feb 2025 17:19:57 +0800
Yicong Yang <yangyicong@huawei.com> wrote:

> From: Junhao He <hejunhao3@huawei.com>
> 
> The supported event number range of each Uncore PMUs is provided by
> each driver in hisi_pmu::check_event and out of range events
> will be rejected. A later version with expanded event number range
> needs to register the PMU with updated hisi_pmu::check_event
> even if it's the only update, which means the expanded events
> cannot be used unless the driver's updated. However the unsupported
> events won't be counted by the hardware so we can relax the event
> number check to allow the use the expanded events.
> 
> Signed-off-by: Junhao He <hejunhao3@huawei.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
>  drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 2 +-
>  drivers/perf/hisilicon/hisi_uncore_hha_pmu.c  | 7 +++----
>  drivers/perf/hisilicon/hisi_uncore_pa_pmu.c   | 2 +-
>  drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 3 +--
>  4 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> index 26eaa6d20c00..21c494881ca0 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> @@ -53,7 +53,7 @@
>  #define DDRC_V1_PERF_CTRL_EN	0x2
>  #define DDRC_V2_PERF_CTRL_EN	0x1
>  #define DDRC_V1_NR_EVENTS	0x7
> -#define DDRC_V2_NR_EVENTS	0x90
> +#define DDRC_V2_NR_EVENTS	0xFF
>  
>  #define DDRC_EVENT_CNTn(base, n)	((base) + (n) * 8)
>  #define DDRC_EVENT_TYPEn(base, n)	((base) + (n) * 4)
> diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> index ca609db86046..78cd6d67f209 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> @@ -47,9 +47,8 @@
>  #define HHA_SRCID_CMD		GENMASK(16, 6)
>  #define HHA_SRCID_MSK		GENMASK(30, 20)
>  #define HHA_DATSRC_SKT_EN	BIT(23)
> -#define HHA_EVTYPE_NONE		0xff
> +#define HHA_EVTYPE_MASK		GENMASK(7, 0)
Using something called mask in places where we previously
had something called nr_events seems a little odd.

renaming EVTYPE_NONE to EVTYPE_MASK seems valid given the
useage but I'd have a different define for the number
of events and not make both changes in one patch.

>  #define HHA_V1_NR_EVENT		0x65
> -#define HHA_V2_NR_EVENT		0xCE
>  
>  HISI_PMU_EVENT_ATTR_EXTRACTOR(srcid_cmd, config1, 10, 0);
>  HISI_PMU_EVENT_ATTR_EXTRACTOR(srcid_msk, config1, 21, 11);
> @@ -197,7 +196,7 @@ static void hisi_hha_pmu_write_evtype(struct hisi_pmu *hha_pmu, int idx,
>  
>  	/* Write event code to HHA_EVENT_TYPEx register */
>  	val = readl(hha_pmu->base + reg);
> -	val &= ~(HHA_EVTYPE_NONE << shift);
> +	val &= ~(HHA_EVTYPE_MASK << shift);
>  	val |= (type << shift);
>  	writel(val, hha_pmu->base + reg);
>  }
> @@ -453,7 +452,7 @@ static int hisi_hha_pmu_dev_probe(struct platform_device *pdev,
>  
>  	if (hha_pmu->identifier >= HISI_PMU_V2) {
>  		hha_pmu->counter_bits = 64;
> -		hha_pmu->check_event = HHA_V2_NR_EVENT;
> +		hha_pmu->check_event = HHA_EVTYPE_MASK;
To me this makes little sense.  Should be HHA_MAX_NR_EVENT
or something like that.

>  		hha_pmu->pmu_events.attr_groups = hisi_hha_pmu_v2_attr_groups;
>  		hha_pmu->num_counters = HHA_V2_NR_COUNTERS;
>  	} else {

Jonathan