Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups
@ 2024-12-03 12:50 Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 01/10] drivers/perf: hisi: Define a symbol namespace for HiSilicon Uncore PMUs Yicong Yang
                   ` (10 more replies)
  0 siblings, 11 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Yicong Yang <yangyicong@hisilicon.com>

This series mainly contains the refactor of the HiSilicon Uncore PMU by
extracting the common parts from each drivers into the core:
- the retrieving of the PMU topology ID
- the common sysfs attributes like cpumask and identifier

In order to achieve this, we need to do below preparation:
- refactor the detection of associated CPUs for PMUs locates on a SICL
- maintain the topology information in a dedicated data structure
- provides a generic implementation of cpumask/identifier attributes

Besides also include below changes/cleanups in this patchset
- add one new sysfs attributes for better describing the topology information
  of the associated CPUs of the PMU.
- define a symbol namespace for HiSilicon Uncore PMUs to avoid pollute the
  common ones
- two minor nonfunctional cleanups of DDRC PMU

Change since v3:
- Split the associated_cpus refactor patches according to the functions, hope
  will be easier to review
- Drop the use of cpu_online_mask out of cpuhp
Link: https://lore.kernel.org/linux-arm-kernel/20241026072424.29887-1-yangyicong@huawei.com/

Change since v2:
- remove one redundant newline and add Jonathan's tag for Patch 1 and 5
Link: https://lore.kernel.org/linux-arm-kernel/20241022145305.47056-1-yangyicong@huawei.com/

Changes since v1:
Address Jonathan's comments and add tags for some patches
- Provide a generic cpumask/identifier attribute group
- refine the comments/commit as suggested
Link: https://lore.kernel.org/linux-arm-kernel/20241018095745.57057-1-yangyicong@huawei.com/

Junhao He (2):
  drivers/perf: hisi: Fix incorrect variable name "hha_pmu" in DDRC PMU
    driver
  drivers/perf: hisi: Delete redundant blank line of DDRC PMU

Yicong Yang (8):
  drivers/perf: hisi: Define a symbol namespace for HiSilicon Uncore
    PMUs
  drivers/perf: hisi: Don't update the associated_cpus on CPU offline
  drivers/perf: hisi: Migrate to one online CPU if no associated one
    online
  drivers/perf: hisi: Refactor the detection of associated CPUs
  drivers/perf: hisi: Extract topology information to a separate
    structure
  drivers/perf: hisi: Add a common function to retrieve topology from
    firmware
  drivers/perf: hisi: Provide a generic implementation of
    cpumask/identifier
  drivers/perf: hisi: Export associated CPUs of each PMU through sysfs

 Documentation/admin-guide/perf/hisi-pmu.rst   |   5 +-
 drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c  |  42 +----
 drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c |  61 ++-----
 drivers/perf/hisilicon/hisi_uncore_hha_pmu.c  |  48 ++----
 drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c  |  44 ++---
 drivers/perf/hisilicon/hisi_uncore_pa_pmu.c   |  53 ++----
 drivers/perf/hisilicon/hisi_uncore_pmu.c      | 160 +++++++++++++-----
 drivers/perf/hisilicon/hisi_uncore_pmu.h      |  49 +++++-
 drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c |  43 +----
 drivers/perf/hisilicon/hisi_uncore_uc_pmu.c   |  45 ++---
 10 files changed, 248 insertions(+), 302 deletions(-)

-- 
2.24.0



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4 01/10] drivers/perf: hisi: Define a symbol namespace for HiSilicon Uncore PMUs
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
@ 2024-12-03 12:50 ` Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 02/10] drivers/perf: hisi: Don't update the associated_cpus on CPU offline Yicong Yang
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Yicong Yang <yangyicong@hisilicon.com>

The HiSilicon Uncore PMU framework implements some common functions
and exported them to the drivers. Use a specific HISI_PMU namespace
for the exported symbols to avoid pollute the generic ones.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c  |  1 +
 drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c |  1 +
 drivers/perf/hisilicon/hisi_uncore_hha_pmu.c  |  1 +
 drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c  |  1 +
 drivers/perf/hisilicon/hisi_uncore_pa_pmu.c   |  1 +
 drivers/perf/hisilicon/hisi_uncore_pmu.c      | 36 +++++++++----------
 drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c |  1 +
 drivers/perf/hisilicon/hisi_uncore_uc_pmu.c   |  1 +
 8 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
index 3f3fb1de11f5..d2dc848956af 100644
--- a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
@@ -389,6 +389,7 @@ static void __exit hisi_cpa_pmu_module_exit(void)
 }
 module_exit(hisi_cpa_pmu_module_exit);
 
+MODULE_IMPORT_NS(HISI_PMU);
 MODULE_DESCRIPTION("HiSilicon SoC CPA PMU driver");
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Qi Liu <liuqi115@huawei.com>");
diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
index a6ebf2ec99d3..e323f0c339ec 100644
--- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
@@ -579,6 +579,7 @@ static void __exit hisi_ddrc_pmu_module_exit(void)
 }
 module_exit(hisi_ddrc_pmu_module_exit);
 
+MODULE_IMPORT_NS(HISI_PMU);
 MODULE_DESCRIPTION("HiSilicon SoC DDRC uncore PMU driver");
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Shaokun Zhang <zhangshaokun@hisilicon.com>");
diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
index 32624872596f..d5c2eab8023e 100644
--- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
@@ -581,6 +581,7 @@ static void __exit hisi_hha_pmu_module_exit(void)
 }
 module_exit(hisi_hha_pmu_module_exit);
 
+MODULE_IMPORT_NS(HISI_PMU);
 MODULE_DESCRIPTION("HiSilicon SoC HHA uncore PMU driver");
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Shaokun Zhang <zhangshaokun@hisilicon.com>");
diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
index c235b46ce873..0d1b5d17ba26 100644
--- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
@@ -615,6 +615,7 @@ static void __exit hisi_l3c_pmu_module_exit(void)
 }
 module_exit(hisi_l3c_pmu_module_exit);
 
+MODULE_IMPORT_NS(HISI_PMU);
 MODULE_DESCRIPTION("HiSilicon SoC L3C uncore PMU driver");
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Anurup M <anurup.m@huawei.com>");
diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
index c0f5d7c73e06..eaaad28871a3 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
@@ -569,6 +569,7 @@ static void __exit hisi_pa_pmu_module_exit(void)
 }
 module_exit(hisi_pa_pmu_module_exit);
 
+MODULE_IMPORT_NS(HISI_PMU);
 MODULE_DESCRIPTION("HiSilicon Protocol Adapter uncore PMU driver");
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Shaokun Zhang <zhangshaokun@hisilicon.com>");
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index 918cdc31de57..416f72a813fc 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -34,7 +34,7 @@ ssize_t hisi_event_sysfs_show(struct device *dev,
 
 	return sysfs_emit(page, "config=0x%lx\n", (unsigned long)eattr->var);
 }
-EXPORT_SYMBOL_GPL(hisi_event_sysfs_show);
+EXPORT_SYMBOL_NS_GPL(hisi_event_sysfs_show, HISI_PMU);
 
 /*
  * sysfs cpumask attributes. For uncore PMU, we only have a single CPU to show
@@ -46,7 +46,7 @@ ssize_t hisi_cpumask_sysfs_show(struct device *dev,
 
 	return sysfs_emit(buf, "%d\n", hisi_pmu->on_cpu);
 }
-EXPORT_SYMBOL_GPL(hisi_cpumask_sysfs_show);
+EXPORT_SYMBOL_NS_GPL(hisi_cpumask_sysfs_show, HISI_PMU);
 
 static bool hisi_validate_event_group(struct perf_event *event)
 {
@@ -96,7 +96,7 @@ int hisi_uncore_pmu_get_event_idx(struct perf_event *event)
 
 	return idx;
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_get_event_idx);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_get_event_idx, HISI_PMU);
 
 ssize_t hisi_uncore_pmu_identifier_attr_show(struct device *dev,
 					     struct device_attribute *attr,
@@ -106,7 +106,7 @@ ssize_t hisi_uncore_pmu_identifier_attr_show(struct device *dev,
 
 	return sysfs_emit(page, "0x%08x\n", hisi_pmu->identifier);
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_identifier_attr_show);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_identifier_attr_show, HISI_PMU);
 
 static void hisi_uncore_pmu_clear_event_idx(struct hisi_pmu *hisi_pmu, int idx)
 {
@@ -165,7 +165,7 @@ int hisi_uncore_pmu_init_irq(struct hisi_pmu *hisi_pmu,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_init_irq);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_init_irq, HISI_PMU);
 
 int hisi_uncore_pmu_event_init(struct perf_event *event)
 {
@@ -219,7 +219,7 @@ int hisi_uncore_pmu_event_init(struct perf_event *event)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_event_init);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_event_init, HISI_PMU);
 
 /*
  * Set the counter to count the event that we're interested in,
@@ -273,7 +273,7 @@ void hisi_uncore_pmu_set_event_period(struct perf_event *event)
 	/* Write start value to the hardware event counter */
 	hisi_pmu->ops->write_counter(hisi_pmu, hwc, val);
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_set_event_period);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_set_event_period, HISI_PMU);
 
 void hisi_uncore_pmu_event_update(struct perf_event *event)
 {
@@ -294,7 +294,7 @@ void hisi_uncore_pmu_event_update(struct perf_event *event)
 		HISI_MAX_PERIOD(hisi_pmu->counter_bits);
 	local64_add(delta, &event->count);
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_event_update);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_event_update, HISI_PMU);
 
 void hisi_uncore_pmu_start(struct perf_event *event, int flags)
 {
@@ -317,7 +317,7 @@ void hisi_uncore_pmu_start(struct perf_event *event, int flags)
 	hisi_uncore_pmu_enable_event(event);
 	perf_event_update_userpage(event);
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_start);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_start, HISI_PMU);
 
 void hisi_uncore_pmu_stop(struct perf_event *event, int flags)
 {
@@ -334,7 +334,7 @@ void hisi_uncore_pmu_stop(struct perf_event *event, int flags)
 	hisi_uncore_pmu_event_update(event);
 	hwc->state |= PERF_HES_UPTODATE;
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_stop);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_stop, HISI_PMU);
 
 int hisi_uncore_pmu_add(struct perf_event *event, int flags)
 {
@@ -357,7 +357,7 @@ int hisi_uncore_pmu_add(struct perf_event *event, int flags)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_add);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_add, HISI_PMU);
 
 void hisi_uncore_pmu_del(struct perf_event *event, int flags)
 {
@@ -369,14 +369,14 @@ void hisi_uncore_pmu_del(struct perf_event *event, int flags)
 	perf_event_update_userpage(event);
 	hisi_pmu->pmu_events.hw_events[hwc->idx] = NULL;
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_del);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_del, HISI_PMU);
 
 void hisi_uncore_pmu_read(struct perf_event *event)
 {
 	/* Read hardware counter and update the perf counter statistics */
 	hisi_uncore_pmu_event_update(event);
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_read);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_read, HISI_PMU);
 
 void hisi_uncore_pmu_enable(struct pmu *pmu)
 {
@@ -389,7 +389,7 @@ void hisi_uncore_pmu_enable(struct pmu *pmu)
 
 	hisi_pmu->ops->start_counters(hisi_pmu);
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_enable);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_enable, HISI_PMU);
 
 void hisi_uncore_pmu_disable(struct pmu *pmu)
 {
@@ -397,7 +397,7 @@ void hisi_uncore_pmu_disable(struct pmu *pmu)
 
 	hisi_pmu->ops->stop_counters(hisi_pmu);
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_disable);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_disable, HISI_PMU);
 
 
 /*
@@ -484,7 +484,7 @@ int hisi_uncore_pmu_online_cpu(unsigned int cpu, struct hlist_node *node)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_online_cpu);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_online_cpu, HISI_PMU);
 
 int hisi_uncore_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
 {
@@ -515,7 +515,7 @@ int hisi_uncore_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(hisi_uncore_pmu_offline_cpu);
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_offline_cpu, HISI_PMU);
 
 void hisi_pmu_init(struct hisi_pmu *hisi_pmu, struct module *module)
 {
@@ -535,7 +535,7 @@ void hisi_pmu_init(struct hisi_pmu *hisi_pmu, struct module *module)
 	pmu->attr_groups        = hisi_pmu->pmu_events.attr_groups;
 	pmu->capabilities       = PERF_PMU_CAP_NO_EXCLUDE;
 }
-EXPORT_SYMBOL_GPL(hisi_pmu_init);
+EXPORT_SYMBOL_NS_GPL(hisi_pmu_init, HISI_PMU);
 
 MODULE_DESCRIPTION("HiSilicon SoC uncore Performance Monitor driver framework");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
index c5f4764ee888..0ef2f6797109 100644
--- a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
@@ -507,6 +507,7 @@ static void __exit hisi_sllc_pmu_module_exit(void)
 }
 module_exit(hisi_sllc_pmu_module_exit);
 
+MODULE_IMPORT_NS(HISI_PMU);
 MODULE_DESCRIPTION("HiSilicon SLLC uncore PMU driver");
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Shaokun Zhang <zhangshaokun@hisilicon.com>");
diff --git a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
index 481dcc9e8fbf..9c724dab6b64 100644
--- a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
@@ -613,6 +613,7 @@ static void __exit hisi_uc_pmu_module_exit(void)
 }
 module_exit(hisi_uc_pmu_module_exit);
 
+MODULE_IMPORT_NS(HISI_PMU);
 MODULE_DESCRIPTION("HiSilicon SoC UC uncore PMU driver");
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Junhao He <hejunhao3@huawei.com>");
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 02/10] drivers/perf: hisi: Don't update the associated_cpus on CPU offline
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 01/10] drivers/perf: hisi: Define a symbol namespace for HiSilicon Uncore PMUs Yicong Yang
@ 2024-12-03 12:50 ` Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 03/10] drivers/perf: hisi: Migrate to one online CPU if no associated one online Yicong Yang
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Yicong Yang <yangyicong@hisilicon.com>

Event will be scheduled on CPU of hisi_pmu::on_cpu which is selected
from the intersection of hisi_pmu::associated_cpus and online CPUs.
So the associated_cpus don't need to be maintained with online CPUs.
This will save one update operation if one associated CPU is offlined.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_pmu.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index 416f72a813fc..dbe7299f0f05 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -492,9 +492,6 @@ int hisi_uncore_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
 						     node);
 	unsigned int target;
 
-	if (!cpumask_test_and_clear_cpu(cpu, &hisi_pmu->associated_cpus))
-		return 0;
-
 	/* Nothing to do if this CPU doesn't own the PMU */
 	if (hisi_pmu->on_cpu != cpu)
 		return 0;
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 03/10] drivers/perf: hisi: Migrate to one online CPU if no associated one online
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 01/10] drivers/perf: hisi: Define a symbol namespace for HiSilicon Uncore PMUs Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 02/10] drivers/perf: hisi: Don't update the associated_cpus on CPU offline Yicong Yang
@ 2024-12-03 12:50 ` Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 04/10] drivers/perf: hisi: Refactor the detection of associated CPUs Yicong Yang
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Yicong Yang <yangyicong@hisilicon.com>

If the selected CPU hisi_pmu::on_cpu goes offline, driver will select
a new online CPU from hisi_pmu::associated_cpus, or if no online CPU
found the PMU context won't be migrated. However for uncore PMUs the
associated CPUs are just a peference and it also works to schedule
the events on any online CPUs. So add a fallback to choose an online
CPU if no associated CPUs found.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_pmu.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index dbe7299f0f05..3a4db5c5d70f 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -499,9 +499,16 @@ int hisi_uncore_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
 	/* Give up ownership of the PMU */
 	hisi_pmu->on_cpu = -1;
 
-	/* Choose a new CPU to migrate ownership of the PMU to */
+	/*
+	 * Migrate ownership of the PMU to a new CPU chosen from PMU's online
+	 * associated CPUs if possible, if no associated CPU online then
+	 * migrate to one online CPU.
+	 */
 	target = cpumask_any_and_but(&hisi_pmu->associated_cpus,
 				     cpu_online_mask, cpu);
+	if (target >= nr_cpu_ids)
+		target = cpumask_any_but(cpu_online_mask, cpu);
+
 	if (target >= nr_cpu_ids)
 		return 0;
 
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 04/10] drivers/perf: hisi: Refactor the detection of associated CPUs
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
                   ` (2 preceding siblings ...)
  2024-12-03 12:50 ` [PATCH v4 03/10] drivers/perf: hisi: Migrate to one online CPU if no associated one online Yicong Yang
@ 2024-12-03 12:50 ` Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 05/10] drivers/perf: hisi: Extract topology information to a separate structure Yicong Yang
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Yicong Yang <yangyicong@hisilicon.com>

There are two type of PMUs supported currently:
1) PMUs locate on SCCL (Super CPU Cluster [1]), associated with certain
   CCL (CPU cluster [1])(e.g. L3C PMU) or not (e.g. DDRC PMU)
2) PMUs locate on the SICL (Super IO Cluster [1]), which has no
   association with certain CPU topology (e.g. CPA PMU)

Currently the associated CPUs of the PMU is detected in the cpuhp online
callback as below:
- for type 1) the CPUs match PMU's sccl_id and ccl_id
- for type 2) PMU's sccl_id is -1 and all online CPUs will be associated

Since uncore PMUs are not bound to certain CPU context and event could be
counting started by any online CPU, the associated CPUs are just a
preference. Below disadvantages are observed in current implementation:
- the PMU cannot be used if its associated CPUs are offline
- SICL PMUs are associated to all the online CPUs implicitly without
  the consideration of locality

So refactor the way we detect the associated CPUs in below aspects:
- add a clear definition of hisi_pmu::associated_cpus
- initialize hisi_pmu::on_cpu based on locality if no associated CPU
  found, otherwise update it from associated CPUs
- drop the detection with a sccl_id of -1 for SICL PMUs

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/admin-guide/perf/hisi-pmu.rst?h=v6.12-rc1
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_pmu.c | 22 +++++++++++++++-------
 drivers/perf/hisilicon/hisi_uncore_pmu.h |  6 +++++-
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index 3a4db5c5d70f..dda8cff4baf4 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -446,10 +446,6 @@ static bool hisi_pmu_cpu_is_associated_pmu(struct hisi_pmu *hisi_pmu)
 {
 	int sccl_id, ccl_id;
 
-	/* If SCCL_ID is -1, the PMU is in a SICL and has no CPU affinity */
-	if (hisi_pmu->sccl_id == -1)
-		return true;
-
 	if (hisi_pmu->ccl_id == -1) {
 		/* If CCL_ID is -1, the PMU only shares the same SCCL */
 		hisi_read_sccl_and_ccl_id(&sccl_id, NULL);
@@ -467,13 +463,25 @@ int hisi_uncore_pmu_online_cpu(unsigned int cpu, struct hlist_node *node)
 	struct hisi_pmu *hisi_pmu = hlist_entry_safe(node, struct hisi_pmu,
 						     node);
 
-	if (!hisi_pmu_cpu_is_associated_pmu(hisi_pmu))
+	/*
+	 * If the CPU is not associated to PMU, initialize the hisi_pmu->on_cpu
+	 * based on the locality if it hasn't been initialized yet. For PMUs
+	 * do have associated CPUs, it'll be updated later.
+	 */
+	if (!hisi_pmu_cpu_is_associated_pmu(hisi_pmu)) {
+		if (hisi_pmu->on_cpu != -1)
+			return 0;
+
+		hisi_pmu->on_cpu = cpumask_local_spread(0, dev_to_node(hisi_pmu->dev));
+		WARN_ON(irq_set_affinity(hisi_pmu->irq, cpumask_of(cpu)));
 		return 0;
+	}
 
 	cpumask_set_cpu(cpu, &hisi_pmu->associated_cpus);
 
-	/* If another CPU is already managing this PMU, simply return. */
-	if (hisi_pmu->on_cpu != -1)
+	/* If another associated CPU is already managing this PMU, simply return. */
+	if (hisi_pmu->on_cpu != -1 &&
+	    cpumask_test_cpu(hisi_pmu->on_cpu, &hisi_pmu->associated_cpus))
 		return 0;
 
 	/* Use this CPU in cpumask for event counting */
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
index 25b2d43b72bf..bd48338ce937 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -87,7 +87,11 @@ struct hisi_pmu {
 	const struct hisi_uncore_ops *ops;
 	const struct hisi_pmu_dev_info *dev_info;
 	struct hisi_pmu_hwevents pmu_events;
-	/* associated_cpus: All CPUs associated with the PMU */
+	/*
+	 * CPUs associated to the PMU and are preferred to use for counting.
+	 * Could be empty if PMU has no association (e.g. PMU on SICL), in
+	 * which case any online CPU will be used.
+	 */
 	cpumask_t associated_cpus;
 	/* CPU used for counting */
 	int on_cpu;
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 05/10] drivers/perf: hisi: Extract topology information to a separate structure
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
                   ` (3 preceding siblings ...)
  2024-12-03 12:50 ` [PATCH v4 04/10] drivers/perf: hisi: Refactor the detection of associated CPUs Yicong Yang
@ 2024-12-03 12:50 ` Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 06/10] drivers/perf: hisi: Add a common function to retrieve topology from firmware Yicong Yang
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Yicong Yang <yangyicong@hisilicon.com>

HiSilicon Uncore PMUs are identified by the IDs of the topology element
on which the PMUs are located. Add a new separate struct hisi_pmu_toplogy
to encapsulate this information. Add additional documentation on the
meaning of each ID.

- make sccl_id and sicl_id into a union since they're exclusive. It can
  also be accessed by scl_id if the SICL/SCCL distinction is not
  relevant
- make index_id and sub_id signed so -1 may be used to indicate the PMU
  doesn't have this topology element or it could not be retrieved.

This patch should have no functional changes.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c  | 12 +++---
 drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 18 ++++-----
 drivers/perf/hisilicon/hisi_uncore_hha_pmu.c  | 12 +++---
 drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c  |  8 ++--
 drivers/perf/hisilicon/hisi_uncore_pa_pmu.c   | 14 +++----
 drivers/perf/hisilicon/hisi_uncore_pmu.c      |  7 ++--
 drivers/perf/hisilicon/hisi_uncore_pmu.h      | 38 +++++++++++++++----
 drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 10 ++---
 drivers/perf/hisilicon/hisi_uncore_uc_pmu.c   | 11 +++---
 9 files changed, 78 insertions(+), 52 deletions(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
index d2dc848956af..826e25c14cec 100644
--- a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
@@ -181,19 +181,19 @@ static int hisi_cpa_pmu_init_data(struct platform_device *pdev,
 				  struct hisi_pmu *cpa_pmu)
 {
 	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &cpa_pmu->sicl_id)) {
+				     &cpa_pmu->topo.sicl_id)) {
 		dev_err(&pdev->dev, "Can not read sicl-id\n");
 		return -EINVAL;
 	}
 
 	if (device_property_read_u32(&pdev->dev, "hisilicon,idx-id",
-				     &cpa_pmu->index_id)) {
+				     &cpa_pmu->topo.index_id)) {
 		dev_err(&pdev->dev, "Cannot read idx-id\n");
 		return -EINVAL;
 	}
 
-	cpa_pmu->ccl_id = -1;
-	cpa_pmu->sccl_id = -1;
+	cpa_pmu->topo.ccl_id = -1;
+	cpa_pmu->topo.sccl_id = -1;
 	cpa_pmu->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(cpa_pmu->base))
 		return PTR_ERR(cpa_pmu->base);
@@ -311,8 +311,8 @@ static int hisi_cpa_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sicl%d_cpa%u",
-			      cpa_pmu->sicl_id, cpa_pmu->index_id);
+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sicl%d_cpa%d",
+			      cpa_pmu->topo.sicl_id, cpa_pmu->topo.index_id);
 	if (!name)
 		return -ENOMEM;
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
index e323f0c339ec..f7bc86644ebe 100644
--- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
@@ -302,18 +302,18 @@ static int hisi_ddrc_pmu_init_data(struct platform_device *pdev,
 	 * DDRC PMU, while SCCL_ID is in MPIDR[aff2].
 	 */
 	if (device_property_read_u32(&pdev->dev, "hisilicon,ch-id",
-				     &ddrc_pmu->index_id)) {
+				     &ddrc_pmu->topo.index_id)) {
 		dev_err(&pdev->dev, "Can not read ddrc channel-id!\n");
 		return -EINVAL;
 	}
 
 	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &ddrc_pmu->sccl_id)) {
+				     &ddrc_pmu->topo.sccl_id)) {
 		dev_err(&pdev->dev, "Can not read ddrc sccl-id!\n");
 		return -EINVAL;
 	}
 	/* DDRC PMUs only share the same SCCL */
-	ddrc_pmu->ccl_id = -1;
+	ddrc_pmu->topo.ccl_id = -1;
 
 	ddrc_pmu->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(ddrc_pmu->base)) {
@@ -324,7 +324,7 @@ static int hisi_ddrc_pmu_init_data(struct platform_device *pdev,
 	ddrc_pmu->identifier = readl(ddrc_pmu->base + DDRC_VERSION);
 	if (ddrc_pmu->identifier >= HISI_PMU_V2) {
 		if (device_property_read_u32(&pdev->dev, "hisilicon,sub-id",
-					     &ddrc_pmu->sub_id)) {
+					     &ddrc_pmu->topo.sub_id)) {
 			dev_err(&pdev->dev, "Can not read sub-id!\n");
 			return -EINVAL;
 		}
@@ -501,13 +501,13 @@ static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
 
 	if (ddrc_pmu->identifier >= HISI_PMU_V2)
 		name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
-				      "hisi_sccl%u_ddrc%u_%u",
-				      ddrc_pmu->sccl_id, ddrc_pmu->index_id,
-				      ddrc_pmu->sub_id);
+				      "hisi_sccl%d_ddrc%d_%d",
+				      ddrc_pmu->topo.sccl_id, ddrc_pmu->topo.index_id,
+				      ddrc_pmu->topo.sub_id);
 	else
 		name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
-				      "hisi_sccl%u_ddrc%u", ddrc_pmu->sccl_id,
-				      ddrc_pmu->index_id);
+				      "hisi_sccl%d_ddrc%d", ddrc_pmu->topo.sccl_id,
+				      ddrc_pmu->topo.index_id);
 
 	if (!name)
 		return -ENOMEM;
diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
index d5c2eab8023e..c5a8f61b5ad5 100644
--- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
@@ -300,7 +300,7 @@ static int hisi_hha_pmu_init_data(struct platform_device *pdev,
 	 * SCCL_ID is in MPIDR[aff2].
 	 */
 	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &hha_pmu->sccl_id)) {
+				     &hha_pmu->topo.sccl_id)) {
 		dev_err(&pdev->dev, "Can not read hha sccl-id!\n");
 		return -EINVAL;
 	}
@@ -310,7 +310,7 @@ static int hisi_hha_pmu_init_data(struct platform_device *pdev,
 	 * both "hisilicon, idx-id" as preference, if available.
 	 */
 	if (device_property_read_u32(&pdev->dev, "hisilicon,idx-id",
-				     &hha_pmu->index_id)) {
+				     &hha_pmu->topo.index_id)) {
 		status = acpi_evaluate_integer(ACPI_HANDLE(&pdev->dev),
 					       "_UID", NULL, &id);
 		if (ACPI_FAILURE(status)) {
@@ -318,10 +318,10 @@ static int hisi_hha_pmu_init_data(struct platform_device *pdev,
 			return -EINVAL;
 		}
 
-		hha_pmu->index_id = id;
+		hha_pmu->topo.index_id = id;
 	}
 	/* HHA PMUs only share the same SCCL */
-	hha_pmu->ccl_id = -1;
+	hha_pmu->topo.ccl_id = -1;
 
 	hha_pmu->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(hha_pmu->base)) {
@@ -510,8 +510,8 @@ static int hisi_hha_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
-			      hha_pmu->sccl_id, hha_pmu->index_id);
+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%d_hha%d",
+			      hha_pmu->topo.sccl_id, hha_pmu->topo.index_id);
 	if (!name)
 		return -ENOMEM;
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
index 0d1b5d17ba26..94546738f6ba 100644
--- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
@@ -360,13 +360,13 @@ static int hisi_l3c_pmu_init_data(struct platform_device *pdev,
 	 * SCCL_ID is in MPIDR[aff2] and CCL_ID is in MPIDR[aff1].
 	 */
 	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &l3c_pmu->sccl_id)) {
+				     &l3c_pmu->topo.sccl_id)) {
 		dev_err(&pdev->dev, "Can not read l3c sccl-id!\n");
 		return -EINVAL;
 	}
 
 	if (device_property_read_u32(&pdev->dev, "hisilicon,ccl-id",
-				     &l3c_pmu->ccl_id)) {
+				     &l3c_pmu->topo.ccl_id)) {
 		dev_err(&pdev->dev, "Can not read l3c ccl-id!\n");
 		return -EINVAL;
 	}
@@ -544,8 +544,8 @@ static int hisi_l3c_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
-			      l3c_pmu->sccl_id, l3c_pmu->ccl_id);
+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%d_l3c%d",
+			      l3c_pmu->topo.sccl_id, l3c_pmu->topo.ccl_id);
 	if (!name)
 		return -ENOMEM;
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
index eaaad28871a3..3130b19e8afc 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
@@ -274,19 +274,19 @@ static int hisi_pa_pmu_init_data(struct platform_device *pdev,
 	 * to identify the PA PMU.
 	 */
 	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &pa_pmu->sicl_id)) {
+				     &pa_pmu->topo.sicl_id)) {
 		dev_err(&pdev->dev, "Cannot read sicl-id!\n");
 		return -EINVAL;
 	}
 
 	if (device_property_read_u32(&pdev->dev, "hisilicon,idx-id",
-				     &pa_pmu->index_id)) {
+				     &pa_pmu->topo.index_id)) {
 		dev_err(&pdev->dev, "Cannot read idx-id!\n");
 		return -EINVAL;
 	}
 
-	pa_pmu->ccl_id = -1;
-	pa_pmu->sccl_id = -1;
+	pa_pmu->topo.ccl_id = -1;
+	pa_pmu->topo.sccl_id = -1;
 
 	pa_pmu->dev_info = device_get_match_data(&pdev->dev);
 	if (!pa_pmu->dev_info)
@@ -488,9 +488,9 @@ static int hisi_pa_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sicl%d_%s%u",
-			      pa_pmu->sicl_id, pa_pmu->dev_info->name,
-			      pa_pmu->index_id);
+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sicl%d_%s%d",
+			      pa_pmu->topo.sicl_id, pa_pmu->dev_info->name,
+			      pa_pmu->topo.index_id);
 	if (!name)
 		return -ENOMEM;
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index dda8cff4baf4..5d4de5f27819 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -444,18 +444,19 @@ static void hisi_read_sccl_and_ccl_id(int *scclp, int *cclp)
  */
 static bool hisi_pmu_cpu_is_associated_pmu(struct hisi_pmu *hisi_pmu)
 {
+	struct hisi_pmu_topology *topo = &hisi_pmu->topo;
 	int sccl_id, ccl_id;
 
-	if (hisi_pmu->ccl_id == -1) {
+	if (topo->ccl_id == -1) {
 		/* If CCL_ID is -1, the PMU only shares the same SCCL */
 		hisi_read_sccl_and_ccl_id(&sccl_id, NULL);
 
-		return sccl_id == hisi_pmu->sccl_id;
+		return sccl_id == topo->sccl_id;
 	}
 
 	hisi_read_sccl_and_ccl_id(&sccl_id, &ccl_id);
 
-	return sccl_id == hisi_pmu->sccl_id && ccl_id == hisi_pmu->ccl_id;
+	return sccl_id == topo->sccl_id && ccl_id == topo->ccl_id;
 }
 
 int hisi_uncore_pmu_online_cpu(unsigned int cpu, struct hlist_node *node)
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
index bd48338ce937..f162a1b6c6d1 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -81,12 +81,43 @@ struct hisi_pmu_hwevents {
 	const struct attribute_group **attr_groups;
 };
 
+/**
+ * struct hisi_pmu_topology - Describe the topology hierarchy on which the PMU
+ *                            is located.
+ * @sccl_id: ID of the SCCL on which the PMU locate is located.
+ * @sicl_id: ID of the SICL on which the PMU locate is located.
+ * @scl_id:  ID used by the core which is unaware of the SCCL/SICL.
+ * @ccl_id: ID of the CCL (CPU cluster) on which the PMU is located.
+ * @index_id: the ID of the PMU module if there're several PMUs at a
+ *            particularly location in the topology.
+ * @sub_id: submodule ID of the PMU. For example we use this for DDRC PMU v2
+ *          since each DDRC has more than one DMC
+ *
+ * The ID will be -1 if the PMU isn't located on a certain topology.
+ */
+struct hisi_pmu_topology {
+	/*
+	 * SCCL (Super CPU CLuster) and SICL (Super I/O Cluster) are parallel
+	 * so a PMU cannot locate on a SCCL and a SICL. If the SCCL/SICL
+	 * distinction is not relevant, use scl_id instead.
+	 */
+	union {
+		int sccl_id;
+		int sicl_id;
+		int scl_id;
+	};
+	int ccl_id;
+	int index_id;
+	int sub_id;
+};
+
 /* Generic pmu struct for different pmu types */
 struct hisi_pmu {
 	struct pmu pmu;
 	const struct hisi_uncore_ops *ops;
 	const struct hisi_pmu_dev_info *dev_info;
 	struct hisi_pmu_hwevents pmu_events;
+	struct hisi_pmu_topology topo;
 	/*
 	 * CPUs associated to the PMU and are preferred to use for counting.
 	 * Could be empty if PMU has no association (e.g. PMU on SICL), in
@@ -98,14 +129,7 @@ struct hisi_pmu {
 	int irq;
 	struct device *dev;
 	struct hlist_node node;
-	int sccl_id;
-	int sicl_id;
-	int ccl_id;
 	void __iomem *base;
-	/* the ID of the PMU modules */
-	u32 index_id;
-	/* For DDRC PMU v2: each DDRC has more than one DMC */
-	u32 sub_id;
 	int num_counters;
 	int counter_bits;
 	/* check event code range */
diff --git a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
index 0ef2f6797109..276534c12fa4 100644
--- a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
@@ -293,19 +293,19 @@ static int hisi_sllc_pmu_init_data(struct platform_device *pdev,
 	 * while SCCL_ID is from MPIDR_EL1 by CPU.
 	 */
 	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &sllc_pmu->sccl_id)) {
+				     &sllc_pmu->topo.sccl_id)) {
 		dev_err(&pdev->dev, "Cannot read sccl-id!\n");
 		return -EINVAL;
 	}
 
 	if (device_property_read_u32(&pdev->dev, "hisilicon,idx-id",
-				     &sllc_pmu->index_id)) {
+				     &sllc_pmu->topo.index_id)) {
 		dev_err(&pdev->dev, "Cannot read idx-id!\n");
 		return -EINVAL;
 	}
 
 	/* SLLC PMUs only share the same SCCL */
-	sllc_pmu->ccl_id = -1;
+	sllc_pmu->topo.ccl_id = -1;
 
 	sllc_pmu->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(sllc_pmu->base)) {
@@ -433,8 +433,8 @@ static int hisi_sllc_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_sllc%u",
-			      sllc_pmu->sccl_id, sllc_pmu->index_id);
+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%d_sllc%d",
+			      sllc_pmu->topo.sccl_id, sllc_pmu->topo.index_id);
 	if (!name)
 		return -ENOMEM;
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
index 9c724dab6b64..19a44498758f 100644
--- a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
@@ -372,19 +372,19 @@ static int hisi_uc_pmu_init_data(struct platform_device *pdev,
 	 * They have some CCLs per SCCL and then 4 UC PMU per CCL.
 	 */
 	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &uc_pmu->sccl_id)) {
+				     &uc_pmu->topo.sccl_id)) {
 		dev_err(&pdev->dev, "Can not read uc sccl-id!\n");
 		return -EINVAL;
 	}
 
 	if (device_property_read_u32(&pdev->dev, "hisilicon,ccl-id",
-				     &uc_pmu->ccl_id)) {
+				     &uc_pmu->topo.ccl_id)) {
 		dev_err(&pdev->dev, "Can not read uc ccl-id!\n");
 		return -EINVAL;
 	}
 
 	if (device_property_read_u32(&pdev->dev, "hisilicon,sub-id",
-				     &uc_pmu->sub_id)) {
+				     &uc_pmu->topo.sub_id)) {
 		dev_err(&pdev->dev, "Can not read sub-id!\n");
 		return -EINVAL;
 	}
@@ -538,8 +538,9 @@ static int hisi_uc_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%d_uc%d_%u",
-			      uc_pmu->sccl_id, uc_pmu->ccl_id, uc_pmu->sub_id);
+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%d_uc%d_%d",
+			      uc_pmu->topo.sccl_id, uc_pmu->topo.ccl_id,
+			      uc_pmu->topo.sub_id);
 	if (!name)
 		return -ENOMEM;
 
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 06/10] drivers/perf: hisi: Add a common function to retrieve topology from firmware
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
                   ` (4 preceding siblings ...)
  2024-12-03 12:50 ` [PATCH v4 05/10] drivers/perf: hisi: Extract topology information to a separate structure Yicong Yang
@ 2024-12-03 12:50 ` Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 07/10] drivers/perf: hisi: Provide a generic implementation of cpumask/identifier Yicong Yang
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Yicong Yang <yangyicong@hisilicon.com>

Currently each type of uncore PMU driver uses almost the same routine
and the same firmware interface (or properties) to retrieve the topology
information, then reset the unused IDs to -1. Extract the common parts
to the framework in hisi_uncore_pmu_init_topology().

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c  | 10 +++----
 drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 10 +++----
 drivers/perf/hisilicon/hisi_uncore_hha_pmu.c  | 10 +++----
 drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c  |  8 ++---
 drivers/perf/hisilicon/hisi_uncore_pa_pmu.c   | 11 +++----
 drivers/perf/hisilicon/hisi_uncore_pmu.c      | 30 +++++++++++++++++++
 drivers/perf/hisilicon/hisi_uncore_pmu.h      |  1 +
 drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 11 +++----
 drivers/perf/hisilicon/hisi_uncore_uc_pmu.c   | 12 ++++----
 9 files changed, 60 insertions(+), 43 deletions(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
index 826e25c14cec..c9b5016fafdb 100644
--- a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
@@ -180,20 +180,18 @@ MODULE_DEVICE_TABLE(acpi, hisi_cpa_pmu_acpi_match);
 static int hisi_cpa_pmu_init_data(struct platform_device *pdev,
 				  struct hisi_pmu *cpa_pmu)
 {
-	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &cpa_pmu->topo.sicl_id)) {
+	hisi_uncore_pmu_init_topology(cpa_pmu, &pdev->dev);
+
+	if (cpa_pmu->topo.sicl_id < 0) {
 		dev_err(&pdev->dev, "Can not read sicl-id\n");
 		return -EINVAL;
 	}
 
-	if (device_property_read_u32(&pdev->dev, "hisilicon,idx-id",
-				     &cpa_pmu->topo.index_id)) {
+	if (cpa_pmu->topo.index_id < 0) {
 		dev_err(&pdev->dev, "Cannot read idx-id\n");
 		return -EINVAL;
 	}
 
-	cpa_pmu->topo.ccl_id = -1;
-	cpa_pmu->topo.sccl_id = -1;
 	cpa_pmu->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(cpa_pmu->base))
 		return PTR_ERR(cpa_pmu->base);
diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
index f7bc86644ebe..56a4068d6568 100644
--- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
@@ -297,6 +297,8 @@ MODULE_DEVICE_TABLE(acpi, hisi_ddrc_pmu_acpi_match);
 static int hisi_ddrc_pmu_init_data(struct platform_device *pdev,
 				   struct hisi_pmu *ddrc_pmu)
 {
+	hisi_uncore_pmu_init_topology(ddrc_pmu, &pdev->dev);
+
 	/*
 	 * Use the SCCL_ID and DDRC channel ID to identify the
 	 * DDRC PMU, while SCCL_ID is in MPIDR[aff2].
@@ -307,13 +309,10 @@ static int hisi_ddrc_pmu_init_data(struct platform_device *pdev,
 		return -EINVAL;
 	}
 
-	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &ddrc_pmu->topo.sccl_id)) {
+	if (ddrc_pmu->topo.sccl_id < 0) {
 		dev_err(&pdev->dev, "Can not read ddrc sccl-id!\n");
 		return -EINVAL;
 	}
-	/* DDRC PMUs only share the same SCCL */
-	ddrc_pmu->topo.ccl_id = -1;
 
 	ddrc_pmu->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(ddrc_pmu->base)) {
@@ -323,8 +322,7 @@ static int hisi_ddrc_pmu_init_data(struct platform_device *pdev,
 
 	ddrc_pmu->identifier = readl(ddrc_pmu->base + DDRC_VERSION);
 	if (ddrc_pmu->identifier >= HISI_PMU_V2) {
-		if (device_property_read_u32(&pdev->dev, "hisilicon,sub-id",
-					     &ddrc_pmu->topo.sub_id)) {
+		if (ddrc_pmu->topo.sub_id < 0) {
 			dev_err(&pdev->dev, "Can not read sub-id!\n");
 			return -EINVAL;
 		}
diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
index c5a8f61b5ad5..e59b31fdc4e4 100644
--- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
@@ -295,12 +295,13 @@ static int hisi_hha_pmu_init_data(struct platform_device *pdev,
 	unsigned long long id;
 	acpi_status status;
 
+	hisi_uncore_pmu_init_topology(hha_pmu, &pdev->dev);
+
 	/*
 	 * Use SCCL_ID and UID to identify the HHA PMU, while
 	 * SCCL_ID is in MPIDR[aff2].
 	 */
-	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &hha_pmu->topo.sccl_id)) {
+	if (hha_pmu->topo.sccl_id < 0) {
 		dev_err(&pdev->dev, "Can not read hha sccl-id!\n");
 		return -EINVAL;
 	}
@@ -309,8 +310,7 @@ static int hisi_hha_pmu_init_data(struct platform_device *pdev,
 	 * Early versions of BIOS support _UID by mistake, so we support
 	 * both "hisilicon, idx-id" as preference, if available.
 	 */
-	if (device_property_read_u32(&pdev->dev, "hisilicon,idx-id",
-				     &hha_pmu->topo.index_id)) {
+	if (hha_pmu->topo.index_id < 0) {
 		status = acpi_evaluate_integer(ACPI_HANDLE(&pdev->dev),
 					       "_UID", NULL, &id);
 		if (ACPI_FAILURE(status)) {
@@ -320,8 +320,6 @@ static int hisi_hha_pmu_init_data(struct platform_device *pdev,
 
 		hha_pmu->topo.index_id = id;
 	}
-	/* HHA PMUs only share the same SCCL */
-	hha_pmu->topo.ccl_id = -1;
 
 	hha_pmu->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(hha_pmu->base)) {
diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
index 94546738f6ba..290a9e311f48 100644
--- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
@@ -355,18 +355,18 @@ MODULE_DEVICE_TABLE(acpi, hisi_l3c_pmu_acpi_match);
 static int hisi_l3c_pmu_init_data(struct platform_device *pdev,
 				  struct hisi_pmu *l3c_pmu)
 {
+	hisi_uncore_pmu_init_topology(l3c_pmu, &pdev->dev);
+
 	/*
 	 * Use the SCCL_ID and CCL_ID to identify the L3C PMU, while
 	 * SCCL_ID is in MPIDR[aff2] and CCL_ID is in MPIDR[aff1].
 	 */
-	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &l3c_pmu->topo.sccl_id)) {
+	if (l3c_pmu->topo.sccl_id < 0) {
 		dev_err(&pdev->dev, "Can not read l3c sccl-id!\n");
 		return -EINVAL;
 	}
 
-	if (device_property_read_u32(&pdev->dev, "hisilicon,ccl-id",
-				     &l3c_pmu->topo.ccl_id)) {
+	if (l3c_pmu->topo.ccl_id < 0) {
 		dev_err(&pdev->dev, "Can not read l3c ccl-id!\n");
 		return -EINVAL;
 	}
diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
index 3130b19e8afc..d7b525f634b9 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
@@ -269,25 +269,22 @@ static void hisi_pa_pmu_clear_int_status(struct hisi_pmu *pa_pmu, int idx)
 static int hisi_pa_pmu_init_data(struct platform_device *pdev,
 				   struct hisi_pmu *pa_pmu)
 {
+	hisi_uncore_pmu_init_topology(pa_pmu, &pdev->dev);
+
 	/*
 	 * As PA PMU is in a SICL, use the SICL_ID and the index ID
 	 * to identify the PA PMU.
 	 */
-	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &pa_pmu->topo.sicl_id)) {
+	if (pa_pmu->topo.sicl_id < 0) {
 		dev_err(&pdev->dev, "Cannot read sicl-id!\n");
 		return -EINVAL;
 	}
 
-	if (device_property_read_u32(&pdev->dev, "hisilicon,idx-id",
-				     &pa_pmu->topo.index_id)) {
+	if (pa_pmu->topo.index_id < 0) {
 		dev_err(&pdev->dev, "Cannot read idx-id!\n");
 		return -EINVAL;
 	}
 
-	pa_pmu->topo.ccl_id = -1;
-	pa_pmu->topo.sccl_id = -1;
-
 	pa_pmu->dev_info = device_get_match_data(&pdev->dev);
 	if (!pa_pmu->dev_info)
 		return -ENODEV;
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index 5d4de5f27819..1132822ff97a 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -14,6 +14,7 @@
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
+#include <linux/property.h>
 
 #include <asm/cputype.h>
 #include <asm/local64.h>
@@ -530,6 +531,35 @@ int hisi_uncore_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
 }
 EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_offline_cpu, HISI_PMU);
 
+/*
+ * Retrieve the topology information from the firmware for the hisi_pmu device.
+ * The topology ID will be -1 if we cannot initialize it, it may either due to
+ * the PMU doesn't locate on this certain topology or the firmware needs to be
+ * fixed.
+ */
+void hisi_uncore_pmu_init_topology(struct hisi_pmu *hisi_pmu, struct device *dev)
+{
+	struct hisi_pmu_topology *topo = &hisi_pmu->topo;
+
+	topo->sccl_id = -1;
+	topo->ccl_id = -1;
+	topo->index_id = -1;
+	topo->sub_id = -1;
+
+	if (device_property_read_u32(dev, "hisilicon,scl-id", &topo->sccl_id))
+		dev_dbg(dev, "no scl-id present\n");
+
+	if (device_property_read_u32(dev, "hisilicon,ccl-id", &topo->ccl_id))
+		dev_dbg(dev, "no ccl-id present\n");
+
+	if (device_property_read_u32(dev, "hisilicon,idx-id", &topo->index_id))
+		dev_dbg(dev, "no idx-id present\n");
+
+	if (device_property_read_u32(dev, "hisilicon,sub-id", &topo->sub_id))
+		dev_dbg(dev, "no sub-id present\n");
+}
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_init_topology, HISI_PMU);
+
 void hisi_pmu_init(struct hisi_pmu *hisi_pmu, struct module *module)
 {
 	struct pmu *pmu = &hisi_pmu->pmu;
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
index f162a1b6c6d1..53b1331b8565 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -160,6 +160,7 @@ ssize_t hisi_uncore_pmu_identifier_attr_show(struct device *dev,
 					     char *page);
 int hisi_uncore_pmu_init_irq(struct hisi_pmu *hisi_pmu,
 			     struct platform_device *pdev);
+void hisi_uncore_pmu_init_topology(struct hisi_pmu *hisi_pmu, struct device *dev);
 
 void hisi_pmu_init(struct hisi_pmu *hisi_pmu, struct module *module);
 #endif /* __HISI_UNCORE_PMU_H__ */
diff --git a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
index 276534c12fa4..a342ef3d9916 100644
--- a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
@@ -288,25 +288,22 @@ MODULE_DEVICE_TABLE(acpi, hisi_sllc_pmu_acpi_match);
 static int hisi_sllc_pmu_init_data(struct platform_device *pdev,
 				   struct hisi_pmu *sllc_pmu)
 {
+	hisi_uncore_pmu_init_topology(sllc_pmu, &pdev->dev);
+
 	/*
 	 * Use the SCCL_ID and the index ID to identify the SLLC PMU,
 	 * while SCCL_ID is from MPIDR_EL1 by CPU.
 	 */
-	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &sllc_pmu->topo.sccl_id)) {
+	if (sllc_pmu->topo.sccl_id < 0) {
 		dev_err(&pdev->dev, "Cannot read sccl-id!\n");
 		return -EINVAL;
 	}
 
-	if (device_property_read_u32(&pdev->dev, "hisilicon,idx-id",
-				     &sllc_pmu->topo.index_id)) {
+	if (sllc_pmu->topo.index_id < 0) {
 		dev_err(&pdev->dev, "Cannot read idx-id!\n");
 		return -EINVAL;
 	}
 
-	/* SLLC PMUs only share the same SCCL */
-	sllc_pmu->topo.ccl_id = -1;
-
 	sllc_pmu->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(sllc_pmu->base)) {
 		dev_err(&pdev->dev, "ioremap failed for sllc_pmu resource.\n");
diff --git a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
index 19a44498758f..2df8976ab4be 100644
--- a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
@@ -11,7 +11,6 @@
 #include <linux/irq.h>
 #include <linux/list.h>
 #include <linux/mod_devicetable.h>
-#include <linux/property.h>
 
 #include "hisi_uncore_pmu.h"
 
@@ -366,25 +365,24 @@ static void hisi_uc_pmu_clear_int_status(struct hisi_pmu *uc_pmu, int idx)
 static int hisi_uc_pmu_init_data(struct platform_device *pdev,
 				 struct hisi_pmu *uc_pmu)
 {
+	hisi_uncore_pmu_init_topology(uc_pmu, &pdev->dev);
+
 	/*
 	 * Use SCCL (Super CPU Cluster) ID and CCL (CPU Cluster) ID to
 	 * identify the topology information of UC PMU devices in the chip.
 	 * They have some CCLs per SCCL and then 4 UC PMU per CCL.
 	 */
-	if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
-				     &uc_pmu->topo.sccl_id)) {
+	if (uc_pmu->topo.sccl_id < 0) {
 		dev_err(&pdev->dev, "Can not read uc sccl-id!\n");
 		return -EINVAL;
 	}
 
-	if (device_property_read_u32(&pdev->dev, "hisilicon,ccl-id",
-				     &uc_pmu->topo.ccl_id)) {
+	if (uc_pmu->topo.ccl_id < 0) {
 		dev_err(&pdev->dev, "Can not read uc ccl-id!\n");
 		return -EINVAL;
 	}
 
-	if (device_property_read_u32(&pdev->dev, "hisilicon,sub-id",
-				     &uc_pmu->topo.sub_id)) {
+	if (uc_pmu->topo.sub_id < 0) {
 		dev_err(&pdev->dev, "Can not read sub-id!\n");
 		return -EINVAL;
 	}
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 07/10] drivers/perf: hisi: Provide a generic implementation of cpumask/identifier
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
                   ` (5 preceding siblings ...)
  2024-12-03 12:50 ` [PATCH v4 06/10] drivers/perf: hisi: Add a common function to retrieve topology from firmware Yicong Yang
@ 2024-12-03 12:50 ` Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 08/10] drivers/perf: hisi: Export associated CPUs of each PMU through sysfs Yicong Yang
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Yicong Yang <yangyicong@hisilicon.com>

Each type of HiSilicon Uncore PMU has the following sysfs attributes:

- format: bitmask in perf_event_attr::config[012] of corresponding
  attribute
- event: events name and corresponding event code
- cpumask: range of CPUs the events can be opened on
- identifier: the version of this PMU

Different types of PMU have different implementations of the "format"
and "event" but all share the same implementation of the "cpumask"
and "identifier". Thus we can move cpumask and identifier to the
hisi_uncore_pmu framework and drivers can use the generic
implementation.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c  | 27 +----------
 drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 31 ++-----------
 drivers/perf/hisilicon/hisi_uncore_hha_pmu.c  | 31 ++-----------
 drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c  | 31 ++-----------
 drivers/perf/hisilicon/hisi_uncore_pa_pmu.c   | 35 +++------------
 drivers/perf/hisilicon/hisi_uncore_pmu.c      | 45 ++++++++++++++-----
 drivers/perf/hisilicon/hisi_uncore_pmu.h      |  4 ++
 drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 27 +----------
 drivers/perf/hisilicon/hisi_uncore_uc_pmu.c   | 27 +----------
 9 files changed, 63 insertions(+), 195 deletions(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
index c9b5016fafdb..0d6a1683892a 100644
--- a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
@@ -225,34 +225,11 @@ static const struct attribute_group hisi_cpa_pmu_events_group = {
 	.attrs = hisi_cpa_pmu_events_attr,
 };
 
-static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
-
-static struct attribute *hisi_cpa_pmu_cpumask_attrs[] = {
-	&dev_attr_cpumask.attr,
-	NULL
-};
-
-static const struct attribute_group hisi_cpa_pmu_cpumask_attr_group = {
-	.attrs = hisi_cpa_pmu_cpumask_attrs,
-};
-
-static struct device_attribute hisi_cpa_pmu_identifier_attr =
-	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
-
-static struct attribute *hisi_cpa_pmu_identifier_attrs[] = {
-	&hisi_cpa_pmu_identifier_attr.attr,
-	NULL
-};
-
-static const struct attribute_group hisi_cpa_pmu_identifier_group = {
-	.attrs = hisi_cpa_pmu_identifier_attrs,
-};
-
 static const struct attribute_group *hisi_cpa_pmu_attr_groups[] = {
 	&hisi_cpa_pmu_format_group,
 	&hisi_cpa_pmu_events_group,
-	&hisi_cpa_pmu_cpumask_attr_group,
-	&hisi_cpa_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL
 };
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
index 56a4068d6568..fb596f43ef9a 100644
--- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
@@ -380,42 +380,19 @@ static const struct attribute_group hisi_ddrc_pmu_v2_events_group = {
 	.attrs = hisi_ddrc_pmu_v2_events_attr,
 };
 
-static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
-
-static struct attribute *hisi_ddrc_pmu_cpumask_attrs[] = {
-	&dev_attr_cpumask.attr,
-	NULL,
-};
-
-static const struct attribute_group hisi_ddrc_pmu_cpumask_attr_group = {
-	.attrs = hisi_ddrc_pmu_cpumask_attrs,
-};
-
-static struct device_attribute hisi_ddrc_pmu_identifier_attr =
-	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
-
-static struct attribute *hisi_ddrc_pmu_identifier_attrs[] = {
-	&hisi_ddrc_pmu_identifier_attr.attr,
-	NULL
-};
-
-static const struct attribute_group hisi_ddrc_pmu_identifier_group = {
-	.attrs = hisi_ddrc_pmu_identifier_attrs,
-};
-
 static const struct attribute_group *hisi_ddrc_pmu_v1_attr_groups[] = {
 	&hisi_ddrc_pmu_v1_format_group,
 	&hisi_ddrc_pmu_v1_events_group,
-	&hisi_ddrc_pmu_cpumask_attr_group,
-	&hisi_ddrc_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL,
 };
 
 static const struct attribute_group *hisi_ddrc_pmu_v2_attr_groups[] = {
 	&hisi_ddrc_pmu_v2_format_group,
 	&hisi_ddrc_pmu_v2_events_group,
-	&hisi_ddrc_pmu_cpumask_attr_group,
-	&hisi_ddrc_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL
 };
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
index e59b31fdc4e4..0a7c44f72ecc 100644
--- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
@@ -405,42 +405,19 @@ static const struct attribute_group hisi_hha_pmu_v2_events_group = {
 	.attrs = hisi_hha_pmu_v2_events_attr,
 };
 
-static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
-
-static struct attribute *hisi_hha_pmu_cpumask_attrs[] = {
-	&dev_attr_cpumask.attr,
-	NULL,
-};
-
-static const struct attribute_group hisi_hha_pmu_cpumask_attr_group = {
-	.attrs = hisi_hha_pmu_cpumask_attrs,
-};
-
-static struct device_attribute hisi_hha_pmu_identifier_attr =
-	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
-
-static struct attribute *hisi_hha_pmu_identifier_attrs[] = {
-	&hisi_hha_pmu_identifier_attr.attr,
-	NULL
-};
-
-static const struct attribute_group hisi_hha_pmu_identifier_group = {
-	.attrs = hisi_hha_pmu_identifier_attrs,
-};
-
 static const struct attribute_group *hisi_hha_pmu_v1_attr_groups[] = {
 	&hisi_hha_pmu_v1_format_group,
 	&hisi_hha_pmu_v1_events_group,
-	&hisi_hha_pmu_cpumask_attr_group,
-	&hisi_hha_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL,
 };
 
 static const struct attribute_group *hisi_hha_pmu_v2_attr_groups[] = {
 	&hisi_hha_pmu_v2_format_group,
 	&hisi_hha_pmu_v2_events_group,
-	&hisi_hha_pmu_cpumask_attr_group,
-	&hisi_hha_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL
 };
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
index 290a9e311f48..602d8d0b72f7 100644
--- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
@@ -441,42 +441,19 @@ static const struct attribute_group hisi_l3c_pmu_v2_events_group = {
 	.attrs = hisi_l3c_pmu_v2_events_attr,
 };
 
-static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
-
-static struct attribute *hisi_l3c_pmu_cpumask_attrs[] = {
-	&dev_attr_cpumask.attr,
-	NULL,
-};
-
-static const struct attribute_group hisi_l3c_pmu_cpumask_attr_group = {
-	.attrs = hisi_l3c_pmu_cpumask_attrs,
-};
-
-static struct device_attribute hisi_l3c_pmu_identifier_attr =
-	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
-
-static struct attribute *hisi_l3c_pmu_identifier_attrs[] = {
-	&hisi_l3c_pmu_identifier_attr.attr,
-	NULL
-};
-
-static const struct attribute_group hisi_l3c_pmu_identifier_group = {
-	.attrs = hisi_l3c_pmu_identifier_attrs,
-};
-
 static const struct attribute_group *hisi_l3c_pmu_v1_attr_groups[] = {
 	&hisi_l3c_pmu_v1_format_group,
 	&hisi_l3c_pmu_v1_events_group,
-	&hisi_l3c_pmu_cpumask_attr_group,
-	&hisi_l3c_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL,
 };
 
 static const struct attribute_group *hisi_l3c_pmu_v2_attr_groups[] = {
 	&hisi_l3c_pmu_v2_format_group,
 	&hisi_l3c_pmu_v2_events_group,
-	&hisi_l3c_pmu_cpumask_attr_group,
-	&hisi_l3c_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL
 };
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
index d7b525f634b9..bd7640516d17 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
@@ -353,29 +353,6 @@ static const struct attribute_group hisi_h60pa_pmu_events_group = {
 	.attrs = hisi_h60pa_pmu_events_attr,
 };
 
-static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
-
-static struct attribute *hisi_pa_pmu_cpumask_attrs[] = {
-	&dev_attr_cpumask.attr,
-	NULL
-};
-
-static const struct attribute_group hisi_pa_pmu_cpumask_attr_group = {
-	.attrs = hisi_pa_pmu_cpumask_attrs,
-};
-
-static struct device_attribute hisi_pa_pmu_identifier_attr =
-	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
-
-static struct attribute *hisi_pa_pmu_identifier_attrs[] = {
-	&hisi_pa_pmu_identifier_attr.attr,
-	NULL
-};
-
-static const struct attribute_group hisi_pa_pmu_identifier_group = {
-	.attrs = hisi_pa_pmu_identifier_attrs,
-};
-
 static struct hisi_pa_pmu_int_regs hisi_pa_pmu_regs = {
 	.mask_offset = PA_INT_MASK,
 	.clear_offset = PA_INT_CLEAR,
@@ -385,8 +362,8 @@ static struct hisi_pa_pmu_int_regs hisi_pa_pmu_regs = {
 static const struct attribute_group *hisi_pa_pmu_v2_attr_groups[] = {
 	&hisi_pa_pmu_v2_format_group,
 	&hisi_pa_pmu_v2_events_group,
-	&hisi_pa_pmu_cpumask_attr_group,
-	&hisi_pa_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL
 };
 
@@ -399,8 +376,8 @@ static const struct hisi_pmu_dev_info hisi_h32pa_v2 = {
 static const struct attribute_group *hisi_pa_pmu_v3_attr_groups[] = {
 	&hisi_pa_pmu_v2_format_group,
 	&hisi_pa_pmu_v3_events_group,
-	&hisi_pa_pmu_cpumask_attr_group,
-	&hisi_pa_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL
 };
 
@@ -419,8 +396,8 @@ static struct hisi_pa_pmu_int_regs hisi_h60pa_pmu_regs = {
 static const struct attribute_group *hisi_h60pa_pmu_attr_groups[] = {
 	&hisi_pa_pmu_v2_format_group,
 	&hisi_h60pa_pmu_events_group,
-	&hisi_pa_pmu_cpumask_attr_group,
-	&hisi_pa_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL
 };
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index 1132822ff97a..339e346e94ee 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -49,6 +49,41 @@ ssize_t hisi_cpumask_sysfs_show(struct device *dev,
 }
 EXPORT_SYMBOL_NS_GPL(hisi_cpumask_sysfs_show, HISI_PMU);
 
+static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
+
+static struct attribute *hisi_pmu_cpumask_attrs[] = {
+	&dev_attr_cpumask.attr,
+	NULL
+};
+
+const struct attribute_group hisi_pmu_cpumask_attr_group = {
+	.attrs = hisi_pmu_cpumask_attrs,
+};
+EXPORT_SYMBOL_NS_GPL(hisi_pmu_cpumask_attr_group, HISI_PMU);
+
+ssize_t hisi_uncore_pmu_identifier_attr_show(struct device *dev,
+					     struct device_attribute *attr,
+					     char *page)
+{
+	struct hisi_pmu *hisi_pmu = to_hisi_pmu(dev_get_drvdata(dev));
+
+	return sysfs_emit(page, "0x%08x\n", hisi_pmu->identifier);
+}
+EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_identifier_attr_show, HISI_PMU);
+
+static struct device_attribute hisi_pmu_identifier_attr =
+	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
+
+static struct attribute *hisi_pmu_identifier_attrs[] = {
+	&hisi_pmu_identifier_attr.attr,
+	NULL
+};
+
+const struct attribute_group hisi_pmu_identifier_group = {
+	.attrs = hisi_pmu_identifier_attrs,
+};
+EXPORT_SYMBOL_NS_GPL(hisi_pmu_identifier_group, HISI_PMU);
+
 static bool hisi_validate_event_group(struct perf_event *event)
 {
 	struct perf_event *sibling, *leader = event->group_leader;
@@ -99,16 +134,6 @@ int hisi_uncore_pmu_get_event_idx(struct perf_event *event)
 }
 EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_get_event_idx, HISI_PMU);
 
-ssize_t hisi_uncore_pmu_identifier_attr_show(struct device *dev,
-					     struct device_attribute *attr,
-					     char *page)
-{
-	struct hisi_pmu *hisi_pmu = to_hisi_pmu(dev_get_drvdata(dev));
-
-	return sysfs_emit(page, "0x%08x\n", hisi_pmu->identifier);
-}
-EXPORT_SYMBOL_NS_GPL(hisi_uncore_pmu_identifier_attr_show, HISI_PMU);
-
 static void hisi_uncore_pmu_clear_event_idx(struct hisi_pmu *hisi_pmu, int idx)
 {
 	clear_bit(idx, hisi_pmu->pmu_events.used_mask);
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
index 53b1331b8565..f4fed2544877 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -137,6 +137,10 @@ struct hisi_pmu {
 	u32 identifier;
 };
 
+/* Generic implementation of cpumask/identifier group */
+extern const struct attribute_group hisi_pmu_cpumask_attr_group;
+extern const struct attribute_group hisi_pmu_identifier_group;
+
 int hisi_uncore_pmu_get_event_idx(struct perf_event *event);
 void hisi_uncore_pmu_read(struct perf_event *event);
 int hisi_uncore_pmu_add(struct perf_event *event, int flags);
diff --git a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
index a342ef3d9916..b55a06f351aa 100644
--- a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
@@ -344,34 +344,11 @@ static const struct attribute_group hisi_sllc_pmu_v2_events_group = {
 	.attrs = hisi_sllc_pmu_v2_events_attr,
 };
 
-static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
-
-static struct attribute *hisi_sllc_pmu_cpumask_attrs[] = {
-	&dev_attr_cpumask.attr,
-	NULL
-};
-
-static const struct attribute_group hisi_sllc_pmu_cpumask_attr_group = {
-	.attrs = hisi_sllc_pmu_cpumask_attrs,
-};
-
-static struct device_attribute hisi_sllc_pmu_identifier_attr =
-	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
-
-static struct attribute *hisi_sllc_pmu_identifier_attrs[] = {
-	&hisi_sllc_pmu_identifier_attr.attr,
-	NULL
-};
-
-static const struct attribute_group hisi_sllc_pmu_identifier_group = {
-	.attrs = hisi_sllc_pmu_identifier_attrs,
-};
-
 static const struct attribute_group *hisi_sllc_pmu_v2_attr_groups[] = {
 	&hisi_sllc_pmu_v2_format_group,
 	&hisi_sllc_pmu_v2_events_group,
-	&hisi_sllc_pmu_cpumask_attr_group,
-	&hisi_sllc_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL
 };
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
index 2df8976ab4be..f90f752f32dd 100644
--- a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
@@ -437,34 +437,11 @@ static const struct attribute_group hisi_uc_pmu_events_group = {
 	.attrs = hisi_uc_pmu_events_attr,
 };
 
-static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
-
-static struct attribute *hisi_uc_pmu_cpumask_attrs[] = {
-	&dev_attr_cpumask.attr,
-	NULL,
-};
-
-static const struct attribute_group hisi_uc_pmu_cpumask_attr_group = {
-	.attrs = hisi_uc_pmu_cpumask_attrs,
-};
-
-static struct device_attribute hisi_uc_pmu_identifier_attr =
-	__ATTR(identifier, 0444, hisi_uncore_pmu_identifier_attr_show, NULL);
-
-static struct attribute *hisi_uc_pmu_identifier_attrs[] = {
-	&hisi_uc_pmu_identifier_attr.attr,
-	NULL
-};
-
-static const struct attribute_group hisi_uc_pmu_identifier_group = {
-	.attrs = hisi_uc_pmu_identifier_attrs,
-};
-
 static const struct attribute_group *hisi_uc_pmu_attr_groups[] = {
 	&hisi_uc_pmu_format_group,
 	&hisi_uc_pmu_events_group,
-	&hisi_uc_pmu_cpumask_attr_group,
-	&hisi_uc_pmu_identifier_group,
+	&hisi_pmu_cpumask_attr_group,
+	&hisi_pmu_identifier_group,
 	NULL
 };
 
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 08/10] drivers/perf: hisi: Export associated CPUs of each PMU through sysfs
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
                   ` (6 preceding siblings ...)
  2024-12-03 12:50 ` [PATCH v4 07/10] drivers/perf: hisi: Provide a generic implementation of cpumask/identifier Yicong Yang
@ 2024-12-03 12:50 ` Yicong Yang
  2024-12-09 15:48   ` Will Deacon
  2024-12-03 12:50 ` [PATCH v4 09/10] drivers/perf: hisi: Fix incorrect variable name "hha_pmu" in DDRC PMU driver Yicong Yang
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Yicong Yang <yangyicong@hisilicon.com>

Although the event of the uncore PMU can only be opened on a single
CPU, some PMU does have the affinity on a range of CPUs. For example
the L3C PMU is associated to the CPUs sharing the L3T it monitors.
Users may infer this affinity by the PMU name which may have SCCL ID
and CCL ID encoded (for L3C etc), but it's not that straightforward.
So export this information by adding an "associated_cpus" sysfs
attribute then user can get this directly.

Reviewed-by: Jonathan Cameron <Joanthan.Cameron@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 Documentation/admin-guide/perf/hisi-pmu.rst |  5 ++++-
 drivers/perf/hisilicon/hisi_uncore_pmu.c    | 10 ++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/perf/hisi-pmu.rst b/Documentation/admin-guide/perf/hisi-pmu.rst
index 5cc248d18c63..48992a0b8e94 100644
--- a/Documentation/admin-guide/perf/hisi-pmu.rst
+++ b/Documentation/admin-guide/perf/hisi-pmu.rst
@@ -35,7 +35,10 @@ e.g. hisi_sccl1_hha0/rx_operations is RX_OPERATIONS event of HHA index #0 in
 SCCL ID #1.
 
 The driver also provides a "cpumask" sysfs attribute, which shows the CPU core
-ID used to count the uncore PMU event.
+ID used to count the uncore PMU event. An "associated_cpus" sysfs attribute is
+also provided to show the CPUs associated with this PMU. The "cpumask" indicates
+the CPUs to open the events, usually as a hint for userspaces tools like perf.
+It only contains one associated CPU from the "associated_cpus".
 
 Example usage of perf::
 
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index 339e346e94ee..f86aba567224 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -51,8 +51,18 @@ EXPORT_SYMBOL_NS_GPL(hisi_cpumask_sysfs_show, HISI_PMU);
 
 static DEVICE_ATTR(cpumask, 0444, hisi_cpumask_sysfs_show, NULL);
 
+static ssize_t hisi_associated_cpus_sysfs_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct hisi_pmu *hisi_pmu = to_hisi_pmu(dev_get_drvdata(dev));
+
+	return cpumap_print_to_pagebuf(true, buf, &hisi_pmu->associated_cpus);
+}
+static DEVICE_ATTR(associated_cpus, 0444, hisi_associated_cpus_sysfs_show, NULL);
+
 static struct attribute *hisi_pmu_cpumask_attrs[] = {
 	&dev_attr_cpumask.attr,
+	&dev_attr_associated_cpus.attr,
 	NULL
 };
 
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 09/10] drivers/perf: hisi: Fix incorrect variable name "hha_pmu" in DDRC PMU driver
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
                   ` (7 preceding siblings ...)
  2024-12-03 12:50 ` [PATCH v4 08/10] drivers/perf: hisi: Export associated CPUs of each PMU through sysfs Yicong Yang
@ 2024-12-03 12:50 ` Yicong Yang
  2024-12-03 12:50 ` [PATCH v4 10/10] drivers/perf: hisi: Delete redundant blank line of DDRC PMU Yicong Yang
  2024-12-10 12:05 ` [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Will Deacon
  10 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Junhao He <hejunhao3@huawei.com>

In the callback function write_evtype(), the variable name of struct
hisi_pmu should be "ddrc_pmu" instead of "hha_pmu".

Signed-off-by: Junhao He <hejunhao3@huawei.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
index fb596f43ef9a..0caa15aa836f 100644
--- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
@@ -111,14 +111,14 @@ static void hisi_ddrc_pmu_v2_write_counter(struct hisi_pmu *ddrc_pmu,
  * so there is no need to write event type, while it is programmable counter in
  * PMU v2.
  */
-static void hisi_ddrc_pmu_write_evtype(struct hisi_pmu *hha_pmu, int idx,
+static void hisi_ddrc_pmu_write_evtype(struct hisi_pmu *ddrc_pmu, int idx,
 				       u32 type)
 {
 	u32 offset;
 
-	if (hha_pmu->identifier >= HISI_PMU_V2) {
+	if (ddrc_pmu->identifier >= HISI_PMU_V2) {
 		offset = DDRC_V2_EVENT_TYPE + 4 * idx;
-		writel(type, hha_pmu->base + offset);
+		writel(type, ddrc_pmu->base + offset);
 	}
 }
 
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 10/10] drivers/perf: hisi: Delete redundant blank line of DDRC PMU
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
                   ` (8 preceding siblings ...)
  2024-12-03 12:50 ` [PATCH v4 09/10] drivers/perf: hisi: Fix incorrect variable name "hha_pmu" in DDRC PMU driver Yicong Yang
@ 2024-12-03 12:50 ` Yicong Yang
  2024-12-10 12:05 ` [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Will Deacon
  10 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-03 12:50 UTC (permalink / raw)
  To: jonathan.cameron, will, mark.rutland, linux-arm-kernel
  Cc: yangyicong, hejunhao3, linuxarm, wangyushan12, prime.zeng

From: Junhao He <hejunhao3@huawei.com>

Do not add blank line at the end of a code block defined by braces.

Signed-off-by: Junhao He <hejunhao3@huawei.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
index 0caa15aa836f..6af84807ac3d 100644
--- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
@@ -550,7 +550,6 @@ static void __exit hisi_ddrc_pmu_module_exit(void)
 {
 	platform_driver_unregister(&hisi_ddrc_pmu_driver);
 	cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE);
-
 }
 module_exit(hisi_ddrc_pmu_module_exit);
 
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 08/10] drivers/perf: hisi: Export associated CPUs of each PMU through sysfs
  2024-12-03 12:50 ` [PATCH v4 08/10] drivers/perf: hisi: Export associated CPUs of each PMU through sysfs Yicong Yang
@ 2024-12-09 15:48   ` Will Deacon
  2024-12-10 10:49     ` Yicong Yang
  0 siblings, 1 reply; 17+ messages in thread
From: Will Deacon @ 2024-12-09 15:48 UTC (permalink / raw)
  To: Yicong Yang
  Cc: jonathan.cameron, mark.rutland, linux-arm-kernel, yangyicong,
	hejunhao3, linuxarm, wangyushan12, prime.zeng

On Tue, Dec 03, 2024 at 08:50:47PM +0800, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> Although the event of the uncore PMU can only be opened on a single
> CPU, some PMU does have the affinity on a range of CPUs. For example
> the L3C PMU is associated to the CPUs sharing the L3T it monitors.
> Users may infer this affinity by the PMU name which may have SCCL ID
> and CCL ID encoded (for L3C etc), but it's not that straightforward.
> So export this information by adding an "associated_cpus" sysfs
> attribute then user can get this directly.
> 
> Reviewed-by: Jonathan Cameron <Joanthan.Cameron@huawei.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
>  Documentation/admin-guide/perf/hisi-pmu.rst |  5 ++++-
>  drivers/perf/hisilicon/hisi_uncore_pmu.c    | 10 ++++++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/perf/hisi-pmu.rst b/Documentation/admin-guide/perf/hisi-pmu.rst
> index 5cc248d18c63..48992a0b8e94 100644
> --- a/Documentation/admin-guide/perf/hisi-pmu.rst
> +++ b/Documentation/admin-guide/perf/hisi-pmu.rst
> @@ -35,7 +35,10 @@ e.g. hisi_sccl1_hha0/rx_operations is RX_OPERATIONS event of HHA index #0 in
>  SCCL ID #1.
>  
>  The driver also provides a "cpumask" sysfs attribute, which shows the CPU core
> -ID used to count the uncore PMU event.
> +ID used to count the uncore PMU event. An "associated_cpus" sysfs attribute is
> +also provided to show the CPUs associated with this PMU. The "cpumask" indicates
> +the CPUs to open the events, usually as a hint for userspaces tools like perf.
> +It only contains one associated CPU from the "associated_cpus".

What is userspace expected to do with this information? Can you point me
to patches that add the corresponding support to the 'perf' tool, please?

Will


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 08/10] drivers/perf: hisi: Export associated CPUs of each PMU through sysfs
  2024-12-09 15:48   ` Will Deacon
@ 2024-12-10 10:49     ` Yicong Yang
  2024-12-10 11:39       ` Will Deacon
  0 siblings, 1 reply; 17+ messages in thread
From: Yicong Yang @ 2024-12-10 10:49 UTC (permalink / raw)
  To: Will Deacon
  Cc: yangyicong, jonathan.cameron, mark.rutland, linux-arm-kernel,
	hejunhao3, linuxarm, wangyushan12, prime.zeng

On 2024/12/9 23:48, Will Deacon wrote:
> On Tue, Dec 03, 2024 at 08:50:47PM +0800, Yicong Yang wrote:
>> From: Yicong Yang <yangyicong@hisilicon.com>
>>
>> Although the event of the uncore PMU can only be opened on a single
>> CPU, some PMU does have the affinity on a range of CPUs. For example
>> the L3C PMU is associated to the CPUs sharing the L3T it monitors.
>> Users may infer this affinity by the PMU name which may have SCCL ID
>> and CCL ID encoded (for L3C etc), but it's not that straightforward.
>> So export this information by adding an "associated_cpus" sysfs
>> attribute then user can get this directly.
>>
>> Reviewed-by: Jonathan Cameron <Joanthan.Cameron@huawei.com>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>> ---
>>  Documentation/admin-guide/perf/hisi-pmu.rst |  5 ++++-
>>  drivers/perf/hisilicon/hisi_uncore_pmu.c    | 10 ++++++++++
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/admin-guide/perf/hisi-pmu.rst b/Documentation/admin-guide/perf/hisi-pmu.rst
>> index 5cc248d18c63..48992a0b8e94 100644
>> --- a/Documentation/admin-guide/perf/hisi-pmu.rst
>> +++ b/Documentation/admin-guide/perf/hisi-pmu.rst
>> @@ -35,7 +35,10 @@ e.g. hisi_sccl1_hha0/rx_operations is RX_OPERATIONS event of HHA index #0 in
>>  SCCL ID #1.
>>  
>>  The driver also provides a "cpumask" sysfs attribute, which shows the CPU core
>> -ID used to count the uncore PMU event.
>> +ID used to count the uncore PMU event. An "associated_cpus" sysfs attribute is
>> +also provided to show the CPUs associated with this PMU. The "cpumask" indicates
>> +the CPUs to open the events, usually as a hint for userspaces tools like perf.
>> +It only contains one associated CPU from the "associated_cpus".
> 
> What is userspace expected to do with this information? Can you point me
> to patches that add the corresponding support to the 'perf' tool, please?
> 

two attributes here, "cpumask" and "associated_cpus". For "cpumask" it's used in
perf [1], this patch does no change to this attribute but only update the
description in the doc.

I think you're questioning about the "associated_cpus" introduced by this patch,
which is not intended to be used by the perf tools but provides a hint for the
user to know the CPUs monitored by which uncore PMU, for example which uncore
L3C PMU's monitoring the events by the CPU(s) the application's running on, or
if the counts is imbalanced among the L3C PMUs user can figure out what's running
on the CPUs provided by the "associated_cpus". As described in the commit this
can also inferred by the PMU name to map SICL ID and CCL ID to certain CPU cluster,
but these ID's aren't readable enough and it'll be more straightforward and
friendly to provide an "associated_cpus" information, considering we've already
maintain it in the drvier.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/pmu.c?h=v6.13-rc2#n748

Thanks.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 08/10] drivers/perf: hisi: Export associated CPUs of each PMU through sysfs
  2024-12-10 10:49     ` Yicong Yang
@ 2024-12-10 11:39       ` Will Deacon
  2024-12-10 13:02         ` Yicong Yang
  0 siblings, 1 reply; 17+ messages in thread
From: Will Deacon @ 2024-12-10 11:39 UTC (permalink / raw)
  To: Yicong Yang
  Cc: yangyicong, jonathan.cameron, mark.rutland, linux-arm-kernel,
	hejunhao3, linuxarm, wangyushan12, prime.zeng

On Tue, Dec 10, 2024 at 06:49:15PM +0800, Yicong Yang wrote:
> On 2024/12/9 23:48, Will Deacon wrote:
> > On Tue, Dec 03, 2024 at 08:50:47PM +0800, Yicong Yang wrote:
> >> From: Yicong Yang <yangyicong@hisilicon.com>
> >>
> >> Although the event of the uncore PMU can only be opened on a single
> >> CPU, some PMU does have the affinity on a range of CPUs. For example
> >> the L3C PMU is associated to the CPUs sharing the L3T it monitors.
> >> Users may infer this affinity by the PMU name which may have SCCL ID
> >> and CCL ID encoded (for L3C etc), but it's not that straightforward.
> >> So export this information by adding an "associated_cpus" sysfs
> >> attribute then user can get this directly.
> >>
> >> Reviewed-by: Jonathan Cameron <Joanthan.Cameron@huawei.com>
> >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> >> ---
> >>  Documentation/admin-guide/perf/hisi-pmu.rst |  5 ++++-
> >>  drivers/perf/hisilicon/hisi_uncore_pmu.c    | 10 ++++++++++
> >>  2 files changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/admin-guide/perf/hisi-pmu.rst b/Documentation/admin-guide/perf/hisi-pmu.rst
> >> index 5cc248d18c63..48992a0b8e94 100644
> >> --- a/Documentation/admin-guide/perf/hisi-pmu.rst
> >> +++ b/Documentation/admin-guide/perf/hisi-pmu.rst
> >> @@ -35,7 +35,10 @@ e.g. hisi_sccl1_hha0/rx_operations is RX_OPERATIONS event of HHA index #0 in
> >>  SCCL ID #1.
> >>  
> >>  The driver also provides a "cpumask" sysfs attribute, which shows the CPU core
> >> -ID used to count the uncore PMU event.
> >> +ID used to count the uncore PMU event. An "associated_cpus" sysfs attribute is
> >> +also provided to show the CPUs associated with this PMU. The "cpumask" indicates
> >> +the CPUs to open the events, usually as a hint for userspaces tools like perf.
> >> +It only contains one associated CPU from the "associated_cpus".
> > 
> > What is userspace expected to do with this information? Can you point me
> > to patches that add the corresponding support to the 'perf' tool, please?
> > 
> 
> two attributes here, "cpumask" and "associated_cpus". For "cpumask" it's used in
> perf [1], this patch does no change to this attribute but only update the
> description in the doc.
> 
> I think you're questioning about the "associated_cpus" introduced by this patch,
> which is not intended to be used by the perf tools but provides a hint for the
> user to know the CPUs monitored by which uncore PMU, for example which uncore
> L3C PMU's monitoring the events by the CPU(s) the application's running on, or
> if the counts is imbalanced among the L3C PMUs user can figure out what's running
> on the CPUs provided by the "associated_cpus". As described in the commit this
> can also inferred by the PMU name to map SICL ID and CCL ID to certain CPU cluster,
> but these ID's aren't readable enough and it'll be more straightforward and
> friendly to provide an "associated_cpus" information, considering we've already
> maintain it in the drvier.

Bah. I just noticed that there's precedent for this in the Arm CSPMU driver,
so I don't really have a leg to stand on despite my objections to funneling
firmware information through to userspace without a confirmed user.

Will


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups
  2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
                   ` (9 preceding siblings ...)
  2024-12-03 12:50 ` [PATCH v4 10/10] drivers/perf: hisi: Delete redundant blank line of DDRC PMU Yicong Yang
@ 2024-12-10 12:05 ` Will Deacon
  2024-12-10 13:04   ` Yicong Yang
  10 siblings, 1 reply; 17+ messages in thread
From: Will Deacon @ 2024-12-10 12:05 UTC (permalink / raw)
  To: Yicong Yang
  Cc: jonathan.cameron, mark.rutland, linux-arm-kernel, yangyicong,
	hejunhao3, linuxarm, wangyushan12, prime.zeng

On Tue, Dec 03, 2024 at 08:50:39PM +0800, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> This series mainly contains the refactor of the HiSilicon Uncore PMU by
> extracting the common parts from each drivers into the core:
> - the retrieving of the PMU topology ID
> - the common sysfs attributes like cpumask and identifier
> 
> In order to achieve this, we need to do below preparation:
> - refactor the detection of associated CPUs for PMUs locates on a SICL
> - maintain the topology information in a dedicated data structure
> - provides a generic implementation of cpumask/identifier attributes
> 
> Besides also include below changes/cleanups in this patchset
> - add one new sysfs attributes for better describing the topology information
>   of the associated CPUs of the PMU.
> - define a symbol namespace for HiSilicon Uncore PMUs to avoid pollute the
>   common ones
> - two minor nonfunctional cleanups of DDRC PMU
> 
> Change since v3:
> - Split the associated_cpus refactor patches according to the functions, hope
>   will be easier to review
> - Drop the use of cpu_online_mask out of cpuhp
> Link: https://lore.kernel.org/linux-arm-kernel/20241026072424.29887-1-yangyicong@huawei.com/

This doesn't build on top of -rc2 (see log below). I suspect this is because
of the recent MODULE_IMPORT_NS() changes to use string literals [1].

Please can you send a v5?

Will

[1] cdd30ebb1b9f ("module: Convert symbol namespace to string literal")

--->8

In file included from ./include/linux/module.h:22,
                 from ./include/linux/device/driver.h:21,
                 from ./include/linux/device.h:32,
                 from ./include/linux/acpi.h:14,
                 from drivers/perf/hisilicon/hisi_uncore_hha_pmu.c:11:
drivers/perf/hisilicon/hisi_uncore_hha_pmu.c:559:18: error: expected ‘,’ or ‘;’ before ‘HISI_PMU’
  559 | MODULE_IMPORT_NS(HISI_PMU);
      |                  ^~~~~~~~
./include/linux/moduleparam.h:26:47: note: in definition of macro ‘__MODULE_INFO’
   26 |   = __MODULE_INFO_PREFIX __stringify(tag) "=" info
      |                                               ^~~~
./include/linux/module.h:299:30: note: in expansion of macro ‘MODULE_INFO’
  299 | #define MODULE_IMPORT_NS(ns) MODULE_INFO(import_ns, ns)
      |                              ^~~~~~~~~~~
drivers/perf/hisilicon/hisi_uncore_hha_pmu.c:559:1: note: in expansion of macro ‘MODULE_IMPORT_NS’
  559 | MODULE_IMPORT_NS(HISI_PMU);
      | ^~~~~~~~~~~~~~~~
make[5]: *** [scripts/Makefile.build:194: drivers/perf/hisilicon/hisi_uncore_hha_pmu.o] Error 1
make[5]: *** Waiting for unfinished jobs....
In file included from ./include/linux/module.h:22,
                 from ./include/linux/device/driver.h:21,
                 from ./include/linux/device.h:32,
                 from ./include/linux/acpi.h:14,
                 from drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c:11:
drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c:595:18: error: expected ‘,’ or ‘;’ before ‘HISI_PMU’
  595 | MODULE_IMPORT_NS(HISI_PMU);
      |                  ^~~~~~~~
./include/linux/moduleparam.h:26:47: note: in definition of macro ‘__MODULE_INFO’
   26 |   = __MODULE_INFO_PREFIX __stringify(tag) "=" info
      |                                               ^~~~
./include/linux/module.h:299:30: note: in expansion of macro ‘MODULE_INFO’
  299 | #define MODULE_IMPORT_NS(ns) MODULE_INFO(import_ns, ns)
      |                              ^~~~~~~~~~~
drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c:595:1: note: in expansion of macro ‘MODULE_IMPORT_NS’
  595 | MODULE_IMPORT_NS(HISI_PMU);
      | ^~~~~~~~~~~~~~~~
make[5]: *** [scripts/Makefile.build:194: drivers/perf/hisilicon/hisi_uncore_l3c_pmu.o] Error 1
/tmp/ccKFrDMm.s: Assembler messages:
/tmp/ccKFrDMm.s:7: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:8: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:9: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:10: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:11: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:12: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:13: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:14: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:15: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:16: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:17: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:18: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:19: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:20: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:21: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:22: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:23: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:24: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:25: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:26: Error: junk at end of line, first unrecognized character is `H'
/tmp/ccKFrDMm.s:27: Error: junk at end of line, first unrecognized character is `H'
make[5]: *** [scripts/Makefile.build:194: drivers/perf/hisilicon/hisi_uncore_pmu.o] Error 1
In file included from ./include/linux/module.h:22,
                 from ./include/linux/device/driver.h:21,
                 from ./include/linux/device.h:32,
                 from ./include/linux/acpi.h:14,
                 from drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c:11:
drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c:556:18: error: expected ‘,’ or ‘;’ before ‘HISI_PMU’
  556 | MODULE_IMPORT_NS(HISI_PMU);
      |                  ^~~~~~~~
./include/linux/moduleparam.h:26:47: note: in definition of macro ‘__MODULE_INFO’
   26 |   = __MODULE_INFO_PREFIX __stringify(tag) "=" info
      |                                               ^~~~
./include/linux/module.h:299:30: note: in expansion of macro ‘MODULE_INFO’
  299 | #define MODULE_IMPORT_NS(ns) MODULE_INFO(import_ns, ns)
      |                              ^~~~~~~~~~~
drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c:556:1: note: in expansion of macro ‘MODULE_IMPORT_NS’
  556 | MODULE_IMPORT_NS(HISI_PMU);
      | ^~~~~~~~~~~~~~~~
make[5]: *** [scripts/Makefile.build:194: drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.o] Error 1
make[4]: *** [scripts/Makefile.build:440: drivers/perf/hisilicon] Error 2
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:440: drivers/perf] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:440: drivers] Error 2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 08/10] drivers/perf: hisi: Export associated CPUs of each PMU through sysfs
  2024-12-10 11:39       ` Will Deacon
@ 2024-12-10 13:02         ` Yicong Yang
  0 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-10 13:02 UTC (permalink / raw)
  To: Will Deacon
  Cc: yangyicong, jonathan.cameron, mark.rutland, linux-arm-kernel,
	hejunhao3, linuxarm, wangyushan12, prime.zeng

On 2024/12/10 19:39, Will Deacon wrote:
> On Tue, Dec 10, 2024 at 06:49:15PM +0800, Yicong Yang wrote:
>> On 2024/12/9 23:48, Will Deacon wrote:
>>> On Tue, Dec 03, 2024 at 08:50:47PM +0800, Yicong Yang wrote:
>>>> From: Yicong Yang <yangyicong@hisilicon.com>
>>>>
>>>> Although the event of the uncore PMU can only be opened on a single
>>>> CPU, some PMU does have the affinity on a range of CPUs. For example
>>>> the L3C PMU is associated to the CPUs sharing the L3T it monitors.
>>>> Users may infer this affinity by the PMU name which may have SCCL ID
>>>> and CCL ID encoded (for L3C etc), but it's not that straightforward.
>>>> So export this information by adding an "associated_cpus" sysfs
>>>> attribute then user can get this directly.
>>>>
>>>> Reviewed-by: Jonathan Cameron <Joanthan.Cameron@huawei.com>
>>>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>>>> ---
>>>>  Documentation/admin-guide/perf/hisi-pmu.rst |  5 ++++-
>>>>  drivers/perf/hisilicon/hisi_uncore_pmu.c    | 10 ++++++++++
>>>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/admin-guide/perf/hisi-pmu.rst b/Documentation/admin-guide/perf/hisi-pmu.rst
>>>> index 5cc248d18c63..48992a0b8e94 100644
>>>> --- a/Documentation/admin-guide/perf/hisi-pmu.rst
>>>> +++ b/Documentation/admin-guide/perf/hisi-pmu.rst
>>>> @@ -35,7 +35,10 @@ e.g. hisi_sccl1_hha0/rx_operations is RX_OPERATIONS event of HHA index #0 in
>>>>  SCCL ID #1.
>>>>  
>>>>  The driver also provides a "cpumask" sysfs attribute, which shows the CPU core
>>>> -ID used to count the uncore PMU event.
>>>> +ID used to count the uncore PMU event. An "associated_cpus" sysfs attribute is
>>>> +also provided to show the CPUs associated with this PMU. The "cpumask" indicates
>>>> +the CPUs to open the events, usually as a hint for userspaces tools like perf.
>>>> +It only contains one associated CPU from the "associated_cpus".
>>>
>>> What is userspace expected to do with this information? Can you point me
>>> to patches that add the corresponding support to the 'perf' tool, please?
>>>
>>
>> two attributes here, "cpumask" and "associated_cpus". For "cpumask" it's used in
>> perf [1], this patch does no change to this attribute but only update the
>> description in the doc.
>>
>> I think you're questioning about the "associated_cpus" introduced by this patch,
>> which is not intended to be used by the perf tools but provides a hint for the
>> user to know the CPUs monitored by which uncore PMU, for example which uncore
>> L3C PMU's monitoring the events by the CPU(s) the application's running on, or
>> if the counts is imbalanced among the L3C PMUs user can figure out what's running
>> on the CPUs provided by the "associated_cpus". As described in the commit this
>> can also inferred by the PMU name to map SICL ID and CCL ID to certain CPU cluster,
>> but these ID's aren't readable enough and it'll be more straightforward and
>> friendly to provide an "associated_cpus" information, considering we've already
>> maintain it in the drvier.
> 
> Bah. I just noticed that there's precedent for this in the Arm CSPMU driver,
> so I don't really have a leg to stand on despite my objections to funneling
> firmware information through to userspace without a confirmed user.
> 

it's bit complex to identify the uncore PMU in the system for better usage
of monitoring or performance analyzing since there are many instances like L3C
and DDRC PMU in the system. So this maybe an easier and cheaper way for the
user to map the PMU to the CPUs where interested tasks' running on without
much knowledge/documentation of the platform specific information like ID
mapping which may varies from different products.

Thanks.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups
  2024-12-10 12:05 ` [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Will Deacon
@ 2024-12-10 13:04   ` Yicong Yang
  0 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2024-12-10 13:04 UTC (permalink / raw)
  To: Will Deacon, Yicong Yang
  Cc: jonathan.cameron, mark.rutland, linux-arm-kernel, yangyicong,
	hejunhao3, wangyushan12, prime.zeng

On 2024/12/10 20:05, Will Deacon wrote:
> On Tue, Dec 03, 2024 at 08:50:39PM +0800, Yicong Yang wrote:
>> From: Yicong Yang <yangyicong@hisilicon.com>
>>
>> This series mainly contains the refactor of the HiSilicon Uncore PMU by
>> extracting the common parts from each drivers into the core:
>> - the retrieving of the PMU topology ID
>> - the common sysfs attributes like cpumask and identifier
>>
>> In order to achieve this, we need to do below preparation:
>> - refactor the detection of associated CPUs for PMUs locates on a SICL
>> - maintain the topology information in a dedicated data structure
>> - provides a generic implementation of cpumask/identifier attributes
>>
>> Besides also include below changes/cleanups in this patchset
>> - add one new sysfs attributes for better describing the topology information
>>   of the associated CPUs of the PMU.
>> - define a symbol namespace for HiSilicon Uncore PMUs to avoid pollute the
>>   common ones
>> - two minor nonfunctional cleanups of DDRC PMU
>>
>> Change since v3:
>> - Split the associated_cpus refactor patches according to the functions, hope
>>   will be easier to review
>> - Drop the use of cpu_online_mask out of cpuhp
>> Link: https://lore.kernel.org/linux-arm-kernel/20241026072424.29887-1-yangyicong@huawei.com/
> 
> This doesn't build on top of -rc2 (see log below). I suspect this is because
> of the recent MODULE_IMPORT_NS() changes to use string literals [1].
> 
> Please can you send a v5?

sure, will rebase on -rc2 and address this.

Thanks.

> 
> Will
> 
> [1] cdd30ebb1b9f ("module: Convert symbol namespace to string literal")
> 
> --->8
> 
> In file included from ./include/linux/module.h:22,
>                  from ./include/linux/device/driver.h:21,
>                  from ./include/linux/device.h:32,
>                  from ./include/linux/acpi.h:14,
>                  from drivers/perf/hisilicon/hisi_uncore_hha_pmu.c:11:
> drivers/perf/hisilicon/hisi_uncore_hha_pmu.c:559:18: error: expected ‘,’ or ‘;’ before ‘HISI_PMU’
>   559 | MODULE_IMPORT_NS(HISI_PMU);
>       |                  ^~~~~~~~
> ./include/linux/moduleparam.h:26:47: note: in definition of macro ‘__MODULE_INFO’
>    26 |   = __MODULE_INFO_PREFIX __stringify(tag) "=" info
>       |                                               ^~~~
> ./include/linux/module.h:299:30: note: in expansion of macro ‘MODULE_INFO’
>   299 | #define MODULE_IMPORT_NS(ns) MODULE_INFO(import_ns, ns)
>       |                              ^~~~~~~~~~~
> drivers/perf/hisilicon/hisi_uncore_hha_pmu.c:559:1: note: in expansion of macro ‘MODULE_IMPORT_NS’
>   559 | MODULE_IMPORT_NS(HISI_PMU);
>       | ^~~~~~~~~~~~~~~~
> make[5]: *** [scripts/Makefile.build:194: drivers/perf/hisilicon/hisi_uncore_hha_pmu.o] Error 1
> make[5]: *** Waiting for unfinished jobs....
> In file included from ./include/linux/module.h:22,
>                  from ./include/linux/device/driver.h:21,
>                  from ./include/linux/device.h:32,
>                  from ./include/linux/acpi.h:14,
>                  from drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c:11:
> drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c:595:18: error: expected ‘,’ or ‘;’ before ‘HISI_PMU’
>   595 | MODULE_IMPORT_NS(HISI_PMU);
>       |                  ^~~~~~~~
> ./include/linux/moduleparam.h:26:47: note: in definition of macro ‘__MODULE_INFO’
>    26 |   = __MODULE_INFO_PREFIX __stringify(tag) "=" info
>       |                                               ^~~~
> ./include/linux/module.h:299:30: note: in expansion of macro ‘MODULE_INFO’
>   299 | #define MODULE_IMPORT_NS(ns) MODULE_INFO(import_ns, ns)
>       |                              ^~~~~~~~~~~
> drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c:595:1: note: in expansion of macro ‘MODULE_IMPORT_NS’
>   595 | MODULE_IMPORT_NS(HISI_PMU);
>       | ^~~~~~~~~~~~~~~~
> make[5]: *** [scripts/Makefile.build:194: drivers/perf/hisilicon/hisi_uncore_l3c_pmu.o] Error 1
> /tmp/ccKFrDMm.s: Assembler messages:
> /tmp/ccKFrDMm.s:7: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:8: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:9: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:10: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:11: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:12: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:13: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:14: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:15: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:16: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:17: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:18: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:19: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:20: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:21: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:22: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:23: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:24: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:25: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:26: Error: junk at end of line, first unrecognized character is `H'
> /tmp/ccKFrDMm.s:27: Error: junk at end of line, first unrecognized character is `H'
> make[5]: *** [scripts/Makefile.build:194: drivers/perf/hisilicon/hisi_uncore_pmu.o] Error 1
> In file included from ./include/linux/module.h:22,
>                  from ./include/linux/device/driver.h:21,
>                  from ./include/linux/device.h:32,
>                  from ./include/linux/acpi.h:14,
>                  from drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c:11:
> drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c:556:18: error: expected ‘,’ or ‘;’ before ‘HISI_PMU’
>   556 | MODULE_IMPORT_NS(HISI_PMU);
>       |                  ^~~~~~~~
> ./include/linux/moduleparam.h:26:47: note: in definition of macro ‘__MODULE_INFO’
>    26 |   = __MODULE_INFO_PREFIX __stringify(tag) "=" info
>       |                                               ^~~~
> ./include/linux/module.h:299:30: note: in expansion of macro ‘MODULE_INFO’
>   299 | #define MODULE_IMPORT_NS(ns) MODULE_INFO(import_ns, ns)
>       |                              ^~~~~~~~~~~
> drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c:556:1: note: in expansion of macro ‘MODULE_IMPORT_NS’
>   556 | MODULE_IMPORT_NS(HISI_PMU);
>       | ^~~~~~~~~~~~~~~~
> make[5]: *** [scripts/Makefile.build:194: drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.o] Error 1
> make[4]: *** [scripts/Makefile.build:440: drivers/perf/hisilicon] Error 2
> make[4]: *** Waiting for unfinished jobs....
> make[3]: *** [scripts/Makefile.build:440: drivers/perf] Error 2
> make[3]: *** Waiting for unfinished jobs....
> make[2]: *** [scripts/Makefile.build:440: drivers] Error 2
> .
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-12-10 13:13 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-03 12:50 [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Yicong Yang
2024-12-03 12:50 ` [PATCH v4 01/10] drivers/perf: hisi: Define a symbol namespace for HiSilicon Uncore PMUs Yicong Yang
2024-12-03 12:50 ` [PATCH v4 02/10] drivers/perf: hisi: Don't update the associated_cpus on CPU offline Yicong Yang
2024-12-03 12:50 ` [PATCH v4 03/10] drivers/perf: hisi: Migrate to one online CPU if no associated one online Yicong Yang
2024-12-03 12:50 ` [PATCH v4 04/10] drivers/perf: hisi: Refactor the detection of associated CPUs Yicong Yang
2024-12-03 12:50 ` [PATCH v4 05/10] drivers/perf: hisi: Extract topology information to a separate structure Yicong Yang
2024-12-03 12:50 ` [PATCH v4 06/10] drivers/perf: hisi: Add a common function to retrieve topology from firmware Yicong Yang
2024-12-03 12:50 ` [PATCH v4 07/10] drivers/perf: hisi: Provide a generic implementation of cpumask/identifier Yicong Yang
2024-12-03 12:50 ` [PATCH v4 08/10] drivers/perf: hisi: Export associated CPUs of each PMU through sysfs Yicong Yang
2024-12-09 15:48   ` Will Deacon
2024-12-10 10:49     ` Yicong Yang
2024-12-10 11:39       ` Will Deacon
2024-12-10 13:02         ` Yicong Yang
2024-12-03 12:50 ` [PATCH v4 09/10] drivers/perf: hisi: Fix incorrect variable name "hha_pmu" in DDRC PMU driver Yicong Yang
2024-12-03 12:50 ` [PATCH v4 10/10] drivers/perf: hisi: Delete redundant blank line of DDRC PMU Yicong Yang
2024-12-10 12:05 ` [PATCH v4 00/10] Refactor the common parts to the HiSilicon Uncore PMU core and cleanups Will Deacon
2024-12-10 13:04   ` Yicong Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox