[PATCH 0/5] perf/arm_cspmu: Preparatory patches for NVIDIA T410 PMU

[PATCH 0/5] perf/arm_cspmu: Preparatory patches for NVIDIA T410 PMU

[Besar Wicaksono]

This series contains initial patches before adding support for upcoming
NVIDIA T410 PMUs.

Thanks,
Besar

Besar Wicaksono (5):
  perf/arm_cspmu: Export arm_cspmu_apmt_node
  perf/arm_cspmu: Add callback to reset filter config
  perf/arm_cspmu: Add pmpidr support
  perf/arm_cspmu: nvidia: Add revision id matching
  perf/arm_cspmu: nvidia: Add pmevfiltr2 support

 drivers/perf/arm_cspmu/arm_cspmu.c    |  51 ++++++-
 drivers/perf/arm_cspmu/arm_cspmu.h    |  43 +++++-
 drivers/perf/arm_cspmu/nvidia_cspmu.c | 194 +++++++++++++++++++-------
 3 files changed, 228 insertions(+), 60 deletions(-)

-- 
2.47.0

[PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node

[Besar Wicaksono]

Make arm_cspmu_apmt_node API accessible to vendor driver.

Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 3 ++-
 drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index efa9b229e701..e4b98cfa606c 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -70,12 +70,13 @@ static void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
 static void arm_cspmu_set_cc_filter(struct arm_cspmu *cspmu,
 				    const struct perf_event *event);
 
-static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
+struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
 {
 	struct acpi_apmt_node **ptr = dev_get_platdata(dev);
 
 	return ptr ? *ptr : NULL;
 }
+EXPORT_SYMBOL_GPL(arm_cspmu_apmt_node);
 
 /*
  * In CoreSight PMU architecture, all of the MMIO registers are 32-bit except
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
index 19684b76bd96..36c1dcce33d6 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -8,6 +8,7 @@
 #ifndef __ARM_CSPMU_H__
 #define __ARM_CSPMU_H__
 
+#include <linux/acpi.h>
 #include <linux/bitfield.h>
 #include <linux/cpumask.h>
 #include <linux/device.h>
@@ -222,4 +223,7 @@ int arm_cspmu_impl_register(const struct arm_cspmu_impl_match *impl_match);
 /* Unregister vendor backend. */
 void arm_cspmu_impl_unregister(const struct arm_cspmu_impl_match *impl_match);
 
+/* Get ACPI APMT node. */
+struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev);
+
 #endif /* __ARM_CSPMU_H__ */
-- 
2.47.0

[PATCH 2/5] perf/arm_cspmu: Add callback to reset filter config

[Besar Wicaksono]

Implementer may need to reset a filter config when
stopping a counter, thus adding a callback for this.

Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 4 ++++
 drivers/perf/arm_cspmu/arm_cspmu.h | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index e4b98cfa606c..595e4bdd0b4b 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -816,6 +816,10 @@ static void arm_cspmu_stop(struct perf_event *event, int pmu_flags)
 		return;
 
 	arm_cspmu_disable_counter(cspmu, hwc->idx);
+
+	if (cspmu->impl.ops.reset_ev_filter)
+		cspmu->impl.ops.reset_ev_filter(cspmu, event);
+
 	arm_cspmu_event_update(event);
 
 	hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
index 36c1dcce33d6..e3a4dd068805 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -153,11 +153,13 @@ struct arm_cspmu_impl_ops {
 	bool (*is_cycle_counter_event)(const struct perf_event *event);
 	/* Decode event type/id from configs */
 	u32 (*event_type)(const struct perf_event *event);
-	/* Set event filters */
+	/* Set/reset event filters */
 	void (*set_cc_filter)(struct arm_cspmu *cspmu,
 			      const struct perf_event *event);
 	void (*set_ev_filter)(struct arm_cspmu *cspmu,
 			      const struct perf_event *event);
+	void (*reset_ev_filter)(struct arm_cspmu *cspmu,
+				const struct perf_event *event);
 	/* Implementation specific event validation */
 	int (*validate_event)(struct arm_cspmu *cspmu,
 			      struct perf_event *event);
-- 
2.47.0

[PATCH 3/5] perf/arm_cspmu: Add pmpidr support

[Besar Wicaksono]

The PMIIDR value is composed by the values in PMPIDR registers.
We can use PMPIDR registers as alternative for device
identification for systems that do not implement PMIIDR.

Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c    | 44 +++++++++++++++++++++++++--
 drivers/perf/arm_cspmu/arm_cspmu.h    | 35 +++++++++++++++++++--
 drivers/perf/arm_cspmu/nvidia_cspmu.c |  2 +-
 3 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 595e4bdd0b4b..347aefbd1704 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -323,14 +323,14 @@ static struct arm_cspmu_impl_match impl_match[] = {
 	{
 		.module_name	= "nvidia_cspmu",
 		.pmiidr_val	= ARM_CSPMU_IMPL_ID_NVIDIA,
-		.pmiidr_mask	= ARM_CSPMU_PMIIDR_IMPLEMENTER,
+		.pmiidr_mask	= PMIIDR_IMPLEMENTER,
 		.module		= NULL,
 		.impl_init_ops	= NULL,
 	},
 	{
 		.module_name	= "ampere_cspmu",
 		.pmiidr_val	= ARM_CSPMU_IMPL_ID_AMPERE,
-		.pmiidr_mask	= ARM_CSPMU_PMIIDR_IMPLEMENTER,
+		.pmiidr_mask	= PMIIDR_IMPLEMENTER,
 		.module		= NULL,
 		.impl_init_ops	= NULL,
 	},
@@ -352,6 +352,44 @@ static struct arm_cspmu_impl_match *arm_cspmu_impl_match_get(u32 pmiidr)
 	return NULL;
 }
 
+static u32 arm_cspmu_get_pmiidr(struct arm_cspmu *cspmu)
+{
+	u32 pmiidr, pmpidr;
+
+	pmiidr = readl(cspmu->base0 + PMIIDR);
+
+	if (pmiidr != 0)
+		return pmiidr;
+
+	/* Construct PMIIDR value from PMPIDRs. */
+
+	pmpidr = readl(cspmu->base0 + PMPIDR0);
+	pmiidr |= FIELD_PREP(PMIIDR_PRODUCTID_PART_0,
+				FIELD_GET(PMPIDR0_PART_0, pmpidr));
+
+	pmpidr = readl(cspmu->base0 + PMPIDR1);
+	pmiidr |= FIELD_PREP(PMIIDR_PRODUCTID_PART_1,
+				FIELD_GET(PMPIDR1_PART_1, pmpidr));
+	pmiidr |= FIELD_PREP(PMIIDR_IMPLEMENTER_DES_0,
+				FIELD_GET(PMPIDR1_DES_0, pmpidr));
+
+	pmpidr = readl(cspmu->base0 + PMPIDR2);
+	pmiidr |= FIELD_PREP(PMIIDR_VARIANT,
+				FIELD_GET(PMPIDR2_REVISION, pmpidr));
+	pmiidr |= FIELD_PREP(PMIIDR_IMPLEMENTER_DES_1,
+				FIELD_GET(PMPIDR2_DES_1, pmpidr));
+
+	pmpidr = readl(cspmu->base0 + PMPIDR3);
+	pmiidr |= FIELD_PREP(PMIIDR_REVISION,
+				FIELD_GET(PMPIDR3_REVAND, pmpidr));
+
+	pmpidr = readl(cspmu->base0 + PMPIDR4);
+	pmiidr |= FIELD_PREP(PMIIDR_IMPLEMENTER_DES_2,
+				FIELD_GET(PMPIDR4_DES_2, pmpidr));
+
+	return pmiidr;
+}
+
 #define DEFAULT_IMPL_OP(name)	.name = arm_cspmu_##name
 
 static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
@@ -362,7 +400,7 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 
 	/* Start with a default PMU implementation */
 	cspmu->impl.module = THIS_MODULE;
-	cspmu->impl.pmiidr = readl(cspmu->base0 + PMIIDR);
+	cspmu->impl.pmiidr = arm_cspmu_get_pmiidr(cspmu);
 	cspmu->impl.ops = (struct arm_cspmu_impl_ops) {
 		DEFAULT_IMPL_OP(get_event_attrs),
 		DEFAULT_IMPL_OP(get_format_attrs),
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
index e3a4dd068805..7eb150abc3b7 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -87,6 +87,11 @@
 #define PMCFGR				0xE00
 #define PMCR				0xE04
 #define PMIIDR				0xE08
+#define PMPIDR0				0xFE0
+#define PMPIDR1				0xFE4
+#define PMPIDR2				0xFE8
+#define PMPIDR3				0xFEC
+#define PMPIDR4				0xFD0
 
 /* PMCFGR register field */
 #define PMCFGR_NCG			GENMASK(31, 28)
@@ -116,8 +121,34 @@
 #define PMCR_E				BIT(0)
 
 /* PMIIDR register field */
-#define ARM_CSPMU_PMIIDR_IMPLEMENTER	GENMASK(11, 0)
-#define ARM_CSPMU_PMIIDR_PRODUCTID	GENMASK(31, 20)
+#define PMIIDR_IMPLEMENTER		GENMASK(11, 0)
+#define PMIIDR_IMPLEMENTER_DES_0	GENMASK(3, 0)
+#define PMIIDR_IMPLEMENTER_DES_1	GENMASK(6, 4)
+#define PMIIDR_IMPLEMENTER_DES_2	GENMASK(11, 8)
+#define PMIIDR_REVISION			GENMASK(15, 12)
+#define PMIIDR_VARIANT			GENMASK(19, 16)
+#define PMIIDR_PRODUCTID		GENMASK(31, 20)
+#define PMIIDR_PRODUCTID_PART_0		GENMASK(27, 20)
+#define PMIIDR_PRODUCTID_PART_1		GENMASK(31, 28)
+
+/* PMPIDR0 register field */
+#define PMPIDR0_PART_0			GENMASK(7, 0)
+
+/* PMPIDR1 register field */
+#define PMPIDR1_DES_0			GENMASK(7, 4)
+#define PMPIDR1_PART_1			GENMASK(3, 0)
+
+/* PMPIDR2 register field */
+#define PMPIDR2_REVISION		GENMASK(7, 4)
+#define PMPIDR2_DES_1			GENMASK(2, 0)
+
+/* PMPIDR3 register field */
+#define PMPIDR3_REVAND			GENMASK(7, 4)
+#define PMPIDR3_CMOD			GENMASK(3, 0)
+
+/* PMPIDR4 register field */
+#define PMPIDR4_SIZE			GENMASK(7, 4)
+#define PMPIDR4_DES_2			GENMASK(3, 0)
 
 /* JEDEC-assigned JEP106 identification code */
 #define ARM_CSPMU_IMPL_ID_NVIDIA	0x36B
diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c b/drivers/perf/arm_cspmu/nvidia_cspmu.c
index dc6d4e3e2a1b..b6cec351a142 100644
--- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
+++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
@@ -322,7 +322,7 @@ static int nv_cspmu_init_ops(struct arm_cspmu *cspmu)
 	if (!ctx)
 		return -ENOMEM;
 
-	prodid = FIELD_GET(ARM_CSPMU_PMIIDR_PRODUCTID, cspmu->impl.pmiidr);
+	prodid = FIELD_GET(PMIIDR_PRODUCTID, cspmu->impl.pmiidr);
 
 	/* Find matching PMU. */
 	for (; match->prodid; match++) {
-- 
2.47.0

[PATCH 4/5] perf/arm_cspmu: nvidia: Add revision id matching

[Besar Wicaksono]

Distinguish NVIDIA devices by revision and variant bits
in PMIIDR register in addition to product id.

Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
---
 drivers/perf/arm_cspmu/nvidia_cspmu.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c b/drivers/perf/arm_cspmu/nvidia_cspmu.c
index b6cec351a142..ac91dc46501d 100644
--- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
+++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
@@ -23,7 +23,7 @@
 
 #define NV_GENERIC_FILTER_ID_MASK    GENMASK_ULL(31, 0)
 
-#define NV_PRODID_MASK               GENMASK(31, 0)
+#define NV_PRODID_MASK	(PMIIDR_PRODUCTID | PMIIDR_VARIANT | PMIIDR_REVISION)
 
 #define NV_FORMAT_NAME_GENERIC	0
 
@@ -220,7 +220,7 @@ struct nv_cspmu_match {
 
 static const struct nv_cspmu_match nv_cspmu_match[] = {
 	{
-	  .prodid = 0x103,
+	  .prodid = 0x10300000,
 	  .prodid_mask = NV_PRODID_MASK,
 	  .filter_mask = NV_PCIE_FILTER_ID_MASK,
 	  .filter_default_val = NV_PCIE_FILTER_ID_MASK,
@@ -230,7 +230,7 @@ static const struct nv_cspmu_match nv_cspmu_match[] = {
 	  .format_attr = pcie_pmu_format_attrs
 	},
 	{
-	  .prodid = 0x104,
+	  .prodid = 0x10400000,
 	  .prodid_mask = NV_PRODID_MASK,
 	  .filter_mask = NV_NVL_C2C_FILTER_ID_MASK,
 	  .filter_default_val = NV_NVL_C2C_FILTER_ID_MASK,
@@ -240,7 +240,7 @@ static const struct nv_cspmu_match nv_cspmu_match[] = {
 	  .format_attr = nvlink_c2c_pmu_format_attrs
 	},
 	{
-	  .prodid = 0x105,
+	  .prodid = 0x10500000,
 	  .prodid_mask = NV_PRODID_MASK,
 	  .filter_mask = NV_NVL_C2C_FILTER_ID_MASK,
 	  .filter_default_val = NV_NVL_C2C_FILTER_ID_MASK,
@@ -250,7 +250,7 @@ static const struct nv_cspmu_match nv_cspmu_match[] = {
 	  .format_attr = nvlink_c2c_pmu_format_attrs
 	},
 	{
-	  .prodid = 0x106,
+	  .prodid = 0x10600000,
 	  .prodid_mask = NV_PRODID_MASK,
 	  .filter_mask = NV_CNVL_FILTER_ID_MASK,
 	  .filter_default_val = NV_CNVL_FILTER_ID_MASK,
@@ -260,7 +260,7 @@ static const struct nv_cspmu_match nv_cspmu_match[] = {
 	  .format_attr = cnvlink_pmu_format_attrs
 	},
 	{
-	  .prodid = 0x2CF,
+	  .prodid = 0x2CF00000,
 	  .prodid_mask = NV_PRODID_MASK,
 	  .filter_mask = 0x0,
 	  .filter_default_val = 0x0,
@@ -312,7 +312,6 @@ static char *nv_cspmu_format_name(const struct arm_cspmu *cspmu,
 
 static int nv_cspmu_init_ops(struct arm_cspmu *cspmu)
 {
-	u32 prodid;
 	struct nv_cspmu_ctx *ctx;
 	struct device *dev = cspmu->dev;
 	struct arm_cspmu_impl_ops *impl_ops = &cspmu->impl.ops;
@@ -322,13 +321,12 @@ static int nv_cspmu_init_ops(struct arm_cspmu *cspmu)
 	if (!ctx)
 		return -ENOMEM;
 
-	prodid = FIELD_GET(PMIIDR_PRODUCTID, cspmu->impl.pmiidr);
-
 	/* Find matching PMU. */
 	for (; match->prodid; match++) {
 		const u32 prodid_mask = match->prodid_mask;
 
-		if ((match->prodid & prodid_mask) == (prodid & prodid_mask))
+		if ((match->prodid & prodid_mask) ==
+		    (cspmu->impl.pmiidr & prodid_mask))
 			break;
 	}
 
-- 
2.47.0

[PATCH 5/5] perf/arm_cspmu: nvidia: Add pmevfiltr2 support

[Besar Wicaksono]

Support NVIDIA PMU that utilizes the optional event filter2 register.

Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
---
 drivers/perf/arm_cspmu/nvidia_cspmu.c | 176 +++++++++++++++++++-------
 1 file changed, 133 insertions(+), 43 deletions(-)

diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c b/drivers/perf/arm_cspmu/nvidia_cspmu.c
index ac91dc46501d..e06a06d3407b 100644
--- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
+++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
@@ -40,10 +40,21 @@
 
 struct nv_cspmu_ctx {
 	const char *name;
-	u32 filter_mask;
-	u32 filter_default_val;
+
 	struct attribute **event_attr;
 	struct attribute **format_attr;
+
+	u32 filter_mask;
+	u32 filter_default_val;
+	u32 filter2_mask;
+	u32 filter2_default_val;
+
+	u32 (*get_filter)(const struct perf_event *event);
+	u32 (*get_filter2)(const struct perf_event *event);
+
+	void *data;
+
+	int (*init_data)(struct arm_cspmu *cspmu);
 };
 
 static struct attribute *scf_pmu_event_attrs[] = {
@@ -144,6 +155,7 @@ static struct attribute *cnvlink_pmu_format_attrs[] = {
 static struct attribute *generic_pmu_format_attrs[] = {
 	ARM_CSPMU_FORMAT_EVENT_ATTR,
 	ARM_CSPMU_FORMAT_FILTER_ATTR,
+	ARM_CSPMU_FORMAT_FILTER2_ATTR,
 	NULL,
 };
 
@@ -184,13 +196,36 @@ static u32 nv_cspmu_event_filter(const struct perf_event *event)
 	return filter_val;
 }
 
+static u32 nv_cspmu_event_filter2(const struct perf_event *event)
+{
+	const struct nv_cspmu_ctx *ctx =
+		to_nv_cspmu_ctx(to_arm_cspmu(event->pmu));
+
+	const u32 filter_val = event->attr.config2 & ctx->filter2_mask;
+
+	if (filter_val == 0)
+		return ctx->filter2_default_val;
+
+	return filter_val;
+}
+
 static void nv_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
 				   const struct perf_event *event)
 {
-	u32 filter = nv_cspmu_event_filter(event);
-	u32 offset = PMEVFILTR + (4 * event->hw.idx);
+	u32 filter, offset;
+	const struct nv_cspmu_ctx *ctx =
+		to_nv_cspmu_ctx(to_arm_cspmu(event->pmu));
+	offset = 4 * event->hw.idx;
 
-	writel(filter, cspmu->base0 + offset);
+	if (ctx->get_filter) {
+		filter = ctx->get_filter(event);
+		writel(filter, cspmu->base0 + PMEVFILTR + offset);
+	}
+
+	if (ctx->get_filter2) {
+		filter = ctx->get_filter2(event);
+		writel(filter, cspmu->base0 + PMEVFILT2R + offset);
+	}
 }
 
 static void nv_cspmu_set_cc_filter(struct arm_cspmu *cspmu,
@@ -210,74 +245,120 @@ enum nv_cspmu_name_fmt {
 struct nv_cspmu_match {
 	u32 prodid;
 	u32 prodid_mask;
-	u64 filter_mask;
-	u32 filter_default_val;
 	const char *name_pattern;
 	enum nv_cspmu_name_fmt name_fmt;
-	struct attribute **event_attr;
-	struct attribute **format_attr;
+	struct nv_cspmu_ctx template_ctx;
+	struct arm_cspmu_impl_ops ops;
 };
 
 static const struct nv_cspmu_match nv_cspmu_match[] = {
 	{
 	  .prodid = 0x10300000,
 	  .prodid_mask = NV_PRODID_MASK,
-	  .filter_mask = NV_PCIE_FILTER_ID_MASK,
-	  .filter_default_val = NV_PCIE_FILTER_ID_MASK,
 	  .name_pattern = "nvidia_pcie_pmu_%u",
 	  .name_fmt = NAME_FMT_SOCKET,
-	  .event_attr = mcf_pmu_event_attrs,
-	  .format_attr = pcie_pmu_format_attrs
+	  .template_ctx = {
+		.event_attr = mcf_pmu_event_attrs,
+		.format_attr = pcie_pmu_format_attrs,
+		.filter_mask = NV_PCIE_FILTER_ID_MASK,
+		.filter_default_val = NV_PCIE_FILTER_ID_MASK,
+		.filter2_mask = 0x0,
+		.filter2_default_val = 0x0,
+		.get_filter = nv_cspmu_event_filter,
+		.get_filter2 = NULL,
+		.data = NULL,
+		.init_data = NULL
+	  },
 	},
 	{
 	  .prodid = 0x10400000,
 	  .prodid_mask = NV_PRODID_MASK,
-	  .filter_mask = NV_NVL_C2C_FILTER_ID_MASK,
-	  .filter_default_val = NV_NVL_C2C_FILTER_ID_MASK,
 	  .name_pattern = "nvidia_nvlink_c2c1_pmu_%u",
 	  .name_fmt = NAME_FMT_SOCKET,
-	  .event_attr = mcf_pmu_event_attrs,
-	  .format_attr = nvlink_c2c_pmu_format_attrs
+	  .template_ctx = {
+		.event_attr = mcf_pmu_event_attrs,
+		.format_attr = nvlink_c2c_pmu_format_attrs,
+		.filter_mask = NV_NVL_C2C_FILTER_ID_MASK,
+		.filter_default_val = NV_NVL_C2C_FILTER_ID_MASK,
+		.filter2_mask = 0x0,
+		.filter2_default_val = 0x0,
+		.get_filter = nv_cspmu_event_filter,
+		.get_filter2 = NULL,
+		.data = NULL,
+		.init_data = NULL
+	  },
 	},
 	{
 	  .prodid = 0x10500000,
 	  .prodid_mask = NV_PRODID_MASK,
-	  .filter_mask = NV_NVL_C2C_FILTER_ID_MASK,
-	  .filter_default_val = NV_NVL_C2C_FILTER_ID_MASK,
 	  .name_pattern = "nvidia_nvlink_c2c0_pmu_%u",
 	  .name_fmt = NAME_FMT_SOCKET,
-	  .event_attr = mcf_pmu_event_attrs,
-	  .format_attr = nvlink_c2c_pmu_format_attrs
+	  .template_ctx = {
+		.event_attr = mcf_pmu_event_attrs,
+		.format_attr = nvlink_c2c_pmu_format_attrs,
+		.filter_mask = NV_NVL_C2C_FILTER_ID_MASK,
+		.filter_default_val = NV_NVL_C2C_FILTER_ID_MASK,
+		.filter2_mask = 0x0,
+		.filter2_default_val = 0x0,
+		.get_filter = nv_cspmu_event_filter,
+		.get_filter2 = NULL,
+		.data = NULL,
+		.init_data = NULL
+	  },
 	},
 	{
 	  .prodid = 0x10600000,
 	  .prodid_mask = NV_PRODID_MASK,
-	  .filter_mask = NV_CNVL_FILTER_ID_MASK,
-	  .filter_default_val = NV_CNVL_FILTER_ID_MASK,
 	  .name_pattern = "nvidia_cnvlink_pmu_%u",
 	  .name_fmt = NAME_FMT_SOCKET,
-	  .event_attr = mcf_pmu_event_attrs,
-	  .format_attr = cnvlink_pmu_format_attrs
+	  .template_ctx = {
+		.event_attr = mcf_pmu_event_attrs,
+		.format_attr = cnvlink_pmu_format_attrs,
+		.filter_mask = NV_CNVL_FILTER_ID_MASK,
+		.filter_default_val = NV_CNVL_FILTER_ID_MASK,
+		.filter2_mask = 0x0,
+		.filter2_default_val = 0x0,
+		.get_filter = nv_cspmu_event_filter,
+		.get_filter2 = NULL,
+		.data = NULL,
+		.init_data = NULL
+	  },
 	},
 	{
 	  .prodid = 0x2CF00000,
 	  .prodid_mask = NV_PRODID_MASK,
-	  .filter_mask = 0x0,
-	  .filter_default_val = 0x0,
 	  .name_pattern = "nvidia_scf_pmu_%u",
 	  .name_fmt = NAME_FMT_SOCKET,
-	  .event_attr = scf_pmu_event_attrs,
-	  .format_attr = scf_pmu_format_attrs
+	  .template_ctx = {
+		.event_attr = scf_pmu_event_attrs,
+		.format_attr = scf_pmu_format_attrs,
+		.filter_mask = 0x0,
+		.filter_default_val = 0x0,
+		.filter2_mask = 0x0,
+		.filter2_default_val = 0x0,
+		.get_filter = nv_cspmu_event_filter,
+		.get_filter2 = NULL,
+		.data = NULL,
+		.init_data = NULL
+	  },
 	},
 	{
 	  .prodid = 0,
 	  .prodid_mask = 0,
-	  .filter_mask = NV_GENERIC_FILTER_ID_MASK,
-	  .filter_default_val = NV_GENERIC_FILTER_ID_MASK,
 	  .name_pattern = "nvidia_uncore_pmu_%u",
 	  .name_fmt = NAME_FMT_GENERIC,
-	  .event_attr = generic_pmu_event_attrs,
-	  .format_attr = generic_pmu_format_attrs
+	  .template_ctx = {
+		.event_attr = generic_pmu_event_attrs,
+		.format_attr = generic_pmu_format_attrs,
+		.filter_mask = NV_GENERIC_FILTER_ID_MASK,
+		.filter_default_val = NV_GENERIC_FILTER_ID_MASK,
+		.filter2_mask = NV_GENERIC_FILTER_ID_MASK,
+		.filter2_default_val = NV_GENERIC_FILTER_ID_MASK,
+		.get_filter = nv_cspmu_event_filter,
+		.get_filter2 = nv_cspmu_event_filter2,
+		.data = NULL,
+		.init_data = NULL
+	  },
 	},
 };
 
@@ -310,6 +391,14 @@ static char *nv_cspmu_format_name(const struct arm_cspmu *cspmu,
 	return name;
 }
 
+#define SET_OP(name, impl, match, default_op) \
+	do { \
+		if (match->ops.name) \
+			impl->name = match->ops.name; \
+		else if (default_op != NULL) \
+			impl->name = default_op; \
+	} while (false)
+
 static int nv_cspmu_init_ops(struct arm_cspmu *cspmu)
 {
 	struct nv_cspmu_ctx *ctx;
@@ -330,20 +419,21 @@ static int nv_cspmu_init_ops(struct arm_cspmu *cspmu)
 			break;
 	}
 
-	ctx->name		= nv_cspmu_format_name(cspmu, match);
-	ctx->filter_mask	= match->filter_mask;
-	ctx->filter_default_val = match->filter_default_val;
-	ctx->event_attr		= match->event_attr;
-	ctx->format_attr	= match->format_attr;
+	/* Initialize the context with the matched template. */
+	memcpy(ctx, &match->template_ctx, sizeof(struct nv_cspmu_ctx));
+	ctx->name = nv_cspmu_format_name(cspmu, match);
 
 	cspmu->impl.ctx = ctx;
 
 	/* NVIDIA specific callbacks. */
-	impl_ops->set_cc_filter			= nv_cspmu_set_cc_filter;
-	impl_ops->set_ev_filter			= nv_cspmu_set_ev_filter;
-	impl_ops->get_event_attrs		= nv_cspmu_get_event_attrs;
-	impl_ops->get_format_attrs		= nv_cspmu_get_format_attrs;
-	impl_ops->get_name			= nv_cspmu_get_name;
+	SET_OP(set_cc_filter, impl_ops, match, nv_cspmu_set_cc_filter);
+	SET_OP(set_ev_filter, impl_ops, match, nv_cspmu_set_ev_filter);
+	SET_OP(get_event_attrs, impl_ops, match, nv_cspmu_get_event_attrs);
+	SET_OP(get_format_attrs, impl_ops, match, nv_cspmu_get_format_attrs);
+	SET_OP(get_name, impl_ops, match, nv_cspmu_get_name);
+
+	if (ctx->init_data)
+		return ctx->init_data(cspmu);
 
 	return 0;
 }
-- 
2.47.0

Re: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node

[Ilkka Koskinen]

Hi Ben,

On Tue, 12 Aug 2025, Besar Wicaksono wrote:
> Make arm_cspmu_apmt_node API accessible to vendor driver.

I think I haven't seen the latest version of the spec. So, I'm curious,
what kind of information the table has that the vendor drivers needs to 
have access to it?

>
> Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
> ---
> drivers/perf/arm_cspmu/arm_cspmu.c | 3 ++-
> drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index efa9b229e701..e4b98cfa606c 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -70,12 +70,13 @@ static void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
> static void arm_cspmu_set_cc_filter(struct arm_cspmu *cspmu,
> 				    const struct perf_event *event);
>
> -static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
> +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
> {
> 	struct acpi_apmt_node **ptr = dev_get_platdata(dev);
>
> 	return ptr ? *ptr : NULL;
> }
> +EXPORT_SYMBOL_GPL(arm_cspmu_apmt_node);

Rather than exporting the function, wouldn't it be better to move it to 
arm_cspmu.h instead?

Cheers, Ilkka

>
> /*
>  * In CoreSight PMU architecture, all of the MMIO registers are 32-bit except
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
> index 19684b76bd96..36c1dcce33d6 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> @@ -8,6 +8,7 @@
> #ifndef __ARM_CSPMU_H__
> #define __ARM_CSPMU_H__
>
> +#include <linux/acpi.h>
> #include <linux/bitfield.h>
> #include <linux/cpumask.h>
> #include <linux/device.h>
> @@ -222,4 +223,7 @@ int arm_cspmu_impl_register(const struct arm_cspmu_impl_match *impl_match);
> /* Unregister vendor backend. */
> void arm_cspmu_impl_unregister(const struct arm_cspmu_impl_match *impl_match);
>
> +/* Get ACPI APMT node. */
> +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev);
> +
> #endif /* __ARM_CSPMU_H__ */
> -- 
> 2.47.0
>
>

Re: [PATCH 3/5] perf/arm_cspmu: Add pmpidr support

[Ilkka Koskinen]

On Tue, 12 Aug 2025, Besar Wicaksono wrote:

> The PMIIDR value is composed by the values in PMPIDR registers.
> We can use PMPIDR registers as alternative for device
> identification for systems that do not implement PMIIDR.
>
> Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>

Looks good to me

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>

> ---
> drivers/perf/arm_cspmu/arm_cspmu.c    | 44 +++++++++++++++++++++++++--
> drivers/perf/arm_cspmu/arm_cspmu.h    | 35 +++++++++++++++++++--
> drivers/perf/arm_cspmu/nvidia_cspmu.c |  2 +-
> 3 files changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index 595e4bdd0b4b..347aefbd1704 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -323,14 +323,14 @@ static struct arm_cspmu_impl_match impl_match[] = {
> 	{
> 		.module_name	= "nvidia_cspmu",
> 		.pmiidr_val	= ARM_CSPMU_IMPL_ID_NVIDIA,
> -		.pmiidr_mask	= ARM_CSPMU_PMIIDR_IMPLEMENTER,
> +		.pmiidr_mask	= PMIIDR_IMPLEMENTER,
> 		.module		= NULL,
> 		.impl_init_ops	= NULL,
> 	},
> 	{
> 		.module_name	= "ampere_cspmu",
> 		.pmiidr_val	= ARM_CSPMU_IMPL_ID_AMPERE,
> -		.pmiidr_mask	= ARM_CSPMU_PMIIDR_IMPLEMENTER,
> +		.pmiidr_mask	= PMIIDR_IMPLEMENTER,
> 		.module		= NULL,
> 		.impl_init_ops	= NULL,
> 	},
> @@ -352,6 +352,44 @@ static struct arm_cspmu_impl_match *arm_cspmu_impl_match_get(u32 pmiidr)
> 	return NULL;
> }
>
> +static u32 arm_cspmu_get_pmiidr(struct arm_cspmu *cspmu)
> +{
> +	u32 pmiidr, pmpidr;
> +
> +	pmiidr = readl(cspmu->base0 + PMIIDR);
> +
> +	if (pmiidr != 0)
> +		return pmiidr;
> +
> +	/* Construct PMIIDR value from PMPIDRs. */
> +
> +	pmpidr = readl(cspmu->base0 + PMPIDR0);
> +	pmiidr |= FIELD_PREP(PMIIDR_PRODUCTID_PART_0,
> +				FIELD_GET(PMPIDR0_PART_0, pmpidr));
> +
> +	pmpidr = readl(cspmu->base0 + PMPIDR1);
> +	pmiidr |= FIELD_PREP(PMIIDR_PRODUCTID_PART_1,
> +				FIELD_GET(PMPIDR1_PART_1, pmpidr));
> +	pmiidr |= FIELD_PREP(PMIIDR_IMPLEMENTER_DES_0,
> +				FIELD_GET(PMPIDR1_DES_0, pmpidr));
> +
> +	pmpidr = readl(cspmu->base0 + PMPIDR2);
> +	pmiidr |= FIELD_PREP(PMIIDR_VARIANT,
> +				FIELD_GET(PMPIDR2_REVISION, pmpidr));
> +	pmiidr |= FIELD_PREP(PMIIDR_IMPLEMENTER_DES_1,
> +				FIELD_GET(PMPIDR2_DES_1, pmpidr));
> +
> +	pmpidr = readl(cspmu->base0 + PMPIDR3);
> +	pmiidr |= FIELD_PREP(PMIIDR_REVISION,
> +				FIELD_GET(PMPIDR3_REVAND, pmpidr));
> +
> +	pmpidr = readl(cspmu->base0 + PMPIDR4);
> +	pmiidr |= FIELD_PREP(PMIIDR_IMPLEMENTER_DES_2,
> +				FIELD_GET(PMPIDR4_DES_2, pmpidr));
> +
> +	return pmiidr;
> +}
> +
> #define DEFAULT_IMPL_OP(name)	.name = arm_cspmu_##name
>
> static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
> @@ -362,7 +400,7 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
>
> 	/* Start with a default PMU implementation */
> 	cspmu->impl.module = THIS_MODULE;
> -	cspmu->impl.pmiidr = readl(cspmu->base0 + PMIIDR);
> +	cspmu->impl.pmiidr = arm_cspmu_get_pmiidr(cspmu);
> 	cspmu->impl.ops = (struct arm_cspmu_impl_ops) {
> 		DEFAULT_IMPL_OP(get_event_attrs),
> 		DEFAULT_IMPL_OP(get_format_attrs),
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
> index e3a4dd068805..7eb150abc3b7 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> @@ -87,6 +87,11 @@
> #define PMCFGR				0xE00
> #define PMCR				0xE04
> #define PMIIDR				0xE08
> +#define PMPIDR0				0xFE0
> +#define PMPIDR1				0xFE4
> +#define PMPIDR2				0xFE8
> +#define PMPIDR3				0xFEC
> +#define PMPIDR4				0xFD0
>
> /* PMCFGR register field */
> #define PMCFGR_NCG			GENMASK(31, 28)
> @@ -116,8 +121,34 @@
> #define PMCR_E				BIT(0)
>
> /* PMIIDR register field */
> -#define ARM_CSPMU_PMIIDR_IMPLEMENTER	GENMASK(11, 0)
> -#define ARM_CSPMU_PMIIDR_PRODUCTID	GENMASK(31, 20)
> +#define PMIIDR_IMPLEMENTER		GENMASK(11, 0)
> +#define PMIIDR_IMPLEMENTER_DES_0	GENMASK(3, 0)
> +#define PMIIDR_IMPLEMENTER_DES_1	GENMASK(6, 4)
> +#define PMIIDR_IMPLEMENTER_DES_2	GENMASK(11, 8)
> +#define PMIIDR_REVISION			GENMASK(15, 12)
> +#define PMIIDR_VARIANT			GENMASK(19, 16)
> +#define PMIIDR_PRODUCTID		GENMASK(31, 20)
> +#define PMIIDR_PRODUCTID_PART_0		GENMASK(27, 20)
> +#define PMIIDR_PRODUCTID_PART_1		GENMASK(31, 28)
> +
> +/* PMPIDR0 register field */
> +#define PMPIDR0_PART_0			GENMASK(7, 0)
> +
> +/* PMPIDR1 register field */
> +#define PMPIDR1_DES_0			GENMASK(7, 4)
> +#define PMPIDR1_PART_1			GENMASK(3, 0)
> +
> +/* PMPIDR2 register field */
> +#define PMPIDR2_REVISION		GENMASK(7, 4)
> +#define PMPIDR2_DES_1			GENMASK(2, 0)
> +
> +/* PMPIDR3 register field */
> +#define PMPIDR3_REVAND			GENMASK(7, 4)
> +#define PMPIDR3_CMOD			GENMASK(3, 0)
> +
> +/* PMPIDR4 register field */
> +#define PMPIDR4_SIZE			GENMASK(7, 4)
> +#define PMPIDR4_DES_2			GENMASK(3, 0)
>
> /* JEDEC-assigned JEP106 identification code */
> #define ARM_CSPMU_IMPL_ID_NVIDIA	0x36B
> diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c b/drivers/perf/arm_cspmu/nvidia_cspmu.c
> index dc6d4e3e2a1b..b6cec351a142 100644
> --- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
> +++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
> @@ -322,7 +322,7 @@ static int nv_cspmu_init_ops(struct arm_cspmu *cspmu)
> 	if (!ctx)
> 		return -ENOMEM;
>
> -	prodid = FIELD_GET(ARM_CSPMU_PMIIDR_PRODUCTID, cspmu->impl.pmiidr);
> +	prodid = FIELD_GET(PMIIDR_PRODUCTID, cspmu->impl.pmiidr);
>
> 	/* Find matching PMU. */
> 	for (; match->prodid; match++) {
> -- 
> 2.47.0
>
>

Re: [PATCH 4/5] perf/arm_cspmu: nvidia: Add revision id matching

[Ilkka Koskinen]

On Tue, 12 Aug 2025, Besar Wicaksono wrote:

> Distinguish NVIDIA devices by revision and variant bits
> in PMIIDR register in addition to product id.
>
> Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>

Looks good to me

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>

> ---
> drivers/perf/arm_cspmu/nvidia_cspmu.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c b/drivers/perf/arm_cspmu/nvidia_cspmu.c
> index b6cec351a142..ac91dc46501d 100644
> --- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
> +++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
> @@ -23,7 +23,7 @@
>
> #define NV_GENERIC_FILTER_ID_MASK    GENMASK_ULL(31, 0)
>
> -#define NV_PRODID_MASK               GENMASK(31, 0)
> +#define NV_PRODID_MASK	(PMIIDR_PRODUCTID | PMIIDR_VARIANT | PMIIDR_REVISION)
>
> #define NV_FORMAT_NAME_GENERIC	0
>
> @@ -220,7 +220,7 @@ struct nv_cspmu_match {
>
> static const struct nv_cspmu_match nv_cspmu_match[] = {
> 	{
> -	  .prodid = 0x103,
> +	  .prodid = 0x10300000,
> 	  .prodid_mask = NV_PRODID_MASK,
> 	  .filter_mask = NV_PCIE_FILTER_ID_MASK,
> 	  .filter_default_val = NV_PCIE_FILTER_ID_MASK,
> @@ -230,7 +230,7 @@ static const struct nv_cspmu_match nv_cspmu_match[] = {
> 	  .format_attr = pcie_pmu_format_attrs
> 	},
> 	{
> -	  .prodid = 0x104,
> +	  .prodid = 0x10400000,
> 	  .prodid_mask = NV_PRODID_MASK,
> 	  .filter_mask = NV_NVL_C2C_FILTER_ID_MASK,
> 	  .filter_default_val = NV_NVL_C2C_FILTER_ID_MASK,
> @@ -240,7 +240,7 @@ static const struct nv_cspmu_match nv_cspmu_match[] = {
> 	  .format_attr = nvlink_c2c_pmu_format_attrs
> 	},
> 	{
> -	  .prodid = 0x105,
> +	  .prodid = 0x10500000,
> 	  .prodid_mask = NV_PRODID_MASK,
> 	  .filter_mask = NV_NVL_C2C_FILTER_ID_MASK,
> 	  .filter_default_val = NV_NVL_C2C_FILTER_ID_MASK,
> @@ -250,7 +250,7 @@ static const struct nv_cspmu_match nv_cspmu_match[] = {
> 	  .format_attr = nvlink_c2c_pmu_format_attrs
> 	},
> 	{
> -	  .prodid = 0x106,
> +	  .prodid = 0x10600000,
> 	  .prodid_mask = NV_PRODID_MASK,
> 	  .filter_mask = NV_CNVL_FILTER_ID_MASK,
> 	  .filter_default_val = NV_CNVL_FILTER_ID_MASK,
> @@ -260,7 +260,7 @@ static const struct nv_cspmu_match nv_cspmu_match[] = {
> 	  .format_attr = cnvlink_pmu_format_attrs
> 	},
> 	{
> -	  .prodid = 0x2CF,
> +	  .prodid = 0x2CF00000,
> 	  .prodid_mask = NV_PRODID_MASK,
> 	  .filter_mask = 0x0,
> 	  .filter_default_val = 0x0,
> @@ -312,7 +312,6 @@ static char *nv_cspmu_format_name(const struct arm_cspmu *cspmu,
>
> static int nv_cspmu_init_ops(struct arm_cspmu *cspmu)
> {
> -	u32 prodid;
> 	struct nv_cspmu_ctx *ctx;
> 	struct device *dev = cspmu->dev;
> 	struct arm_cspmu_impl_ops *impl_ops = &cspmu->impl.ops;
> @@ -322,13 +321,12 @@ static int nv_cspmu_init_ops(struct arm_cspmu *cspmu)
> 	if (!ctx)
> 		return -ENOMEM;
>
> -	prodid = FIELD_GET(PMIIDR_PRODUCTID, cspmu->impl.pmiidr);
> -
> 	/* Find matching PMU. */
> 	for (; match->prodid; match++) {
> 		const u32 prodid_mask = match->prodid_mask;
>
> -		if ((match->prodid & prodid_mask) == (prodid & prodid_mask))
> +		if ((match->prodid & prodid_mask) ==
> +		    (cspmu->impl.pmiidr & prodid_mask))
> 			break;
> 	}
>
> -- 
> 2.47.0
>
>

RE: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node

[Besar Wicaksono]

Hi Ilkka,

Thanks for the feedback. Please see my comment inline.

> -----Original Message-----
> From: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> Sent: Tuesday, August 19, 2025 3:16 PM
> To: Besar Wicaksono <bwicaksono@nvidia.com>
> Cc: will@kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-tegra@vger.kernel.org;
> suzuki.poulose@arm.com; robin.murphy@arm.com;
> ilkka@os.amperecomputing.com; mark.rutland@arm.com; Thierry Reding
> <treding@nvidia.com>; Jon Hunter <jonathanh@nvidia.com>; Vikram Sethi
> <vsethi@nvidia.com>; Rich Wiley <rwiley@nvidia.com>; Shanker Donthineni
> <sdonthineni@nvidia.com>
> Subject: Re: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Ben,
> 
> On Tue, 12 Aug 2025, Besar Wicaksono wrote:
> > Make arm_cspmu_apmt_node API accessible to vendor driver.
> 
> I think I haven't seen the latest version of the spec. So, I'm curious,
> what kind of information the table has that the vendor drivers needs to
> have access to it?
> 

The vendor driver may need the node instance primary and secondary
fields to get additional properties of the PMU that is not covered
by the standard properties. For example, the PMU device entry in
APMT can be defined as ACPI node type. The node instance primary
and secondary will contain the HID and UID of an ACPI device object
that is associated with the PMU. This ACPI object can have more info
to supplement the standard props.

> >
> > Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
> > ---
> > drivers/perf/arm_cspmu/arm_cspmu.c | 3 ++-
> > drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
> b/drivers/perf/arm_cspmu/arm_cspmu.c
> > index efa9b229e701..e4b98cfa606c 100644
> > --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> > +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> > @@ -70,12 +70,13 @@ static void arm_cspmu_set_ev_filter(struct
> arm_cspmu *cspmu,
> > static void arm_cspmu_set_cc_filter(struct arm_cspmu *cspmu,
> >                                   const struct perf_event *event);
> >
> > -static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
> > +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
> > {
> >       struct acpi_apmt_node **ptr = dev_get_platdata(dev);
> >
> >       return ptr ? *ptr : NULL;
> > }
> > +EXPORT_SYMBOL_GPL(arm_cspmu_apmt_node);
> 
> Rather than exporting the function, wouldn't it be better to move it to
> arm_cspmu.h instead?

Sounds good to me. I will make the change on the next revision.

Thanks,
Besar

> 
> Cheers, Ilkka
> 
> >
> > /*
> >  * In CoreSight PMU architecture, all of the MMIO registers are 32-bit except
> > diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h
> b/drivers/perf/arm_cspmu/arm_cspmu.h
> > index 19684b76bd96..36c1dcce33d6 100644
> > --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> > +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> > @@ -8,6 +8,7 @@
> > #ifndef __ARM_CSPMU_H__
> > #define __ARM_CSPMU_H__
> >
> > +#include <linux/acpi.h>
> > #include <linux/bitfield.h>
> > #include <linux/cpumask.h>
> > #include <linux/device.h>
> > @@ -222,4 +223,7 @@ int arm_cspmu_impl_register(const struct
> arm_cspmu_impl_match *impl_match);
> > /* Unregister vendor backend. */
> > void arm_cspmu_impl_unregister(const struct arm_cspmu_impl_match
> *impl_match);
> >
> > +/* Get ACPI APMT node. */
> > +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev);
> > +
> > #endif /* __ARM_CSPMU_H__ */
> > --
> > 2.47.0
> >
> >

Re: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node

[Robin Murphy]

On 2025-08-20 8:07 pm, Besar Wicaksono wrote:
> Hi Ilkka,
> 
> Thanks for the feedback. Please see my comment inline.
> 
>> -----Original Message-----
>> From: Ilkka Koskinen <ilkka@os.amperecomputing.com>
>> Sent: Tuesday, August 19, 2025 3:16 PM
>> To: Besar Wicaksono <bwicaksono@nvidia.com>
>> Cc: will@kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; linux-tegra@vger.kernel.org;
>> suzuki.poulose@arm.com; robin.murphy@arm.com;
>> ilkka@os.amperecomputing.com; mark.rutland@arm.com; Thierry Reding
>> <treding@nvidia.com>; Jon Hunter <jonathanh@nvidia.com>; Vikram Sethi
>> <vsethi@nvidia.com>; Rich Wiley <rwiley@nvidia.com>; Shanker Donthineni
>> <sdonthineni@nvidia.com>
>> Subject: Re: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Ben,
>>
>> On Tue, 12 Aug 2025, Besar Wicaksono wrote:
>>> Make arm_cspmu_apmt_node API accessible to vendor driver.
>>
>> I think I haven't seen the latest version of the spec. So, I'm curious,
>> what kind of information the table has that the vendor drivers needs to
>> have access to it?
>>
> 
> The vendor driver may need the node instance primary and secondary
> fields to get additional properties of the PMU that is not covered
> by the standard properties. For example, the PMU device entry in
> APMT can be defined as ACPI node type. The node instance primary
> and secondary will contain the HID and UID of an ACPI device object
> that is associated with the PMU. This ACPI object can have more info
> to supplement the standard props.

Rather than exposing the raw APMT data, maybe then cspmu should just 
encapsulate a method for retrieving the associated ACPI device, if any? 
I guess it could be a generalised "firmware device" notion - even though 
for DT that should mostly be cspmu->dev already - since that could then 
work neatly for generic device properties, but perhaps we don't expect 
many sub-drivers to support both ACPI and DT...

Thanks,
Robin.

>>>
>>> Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
>>> ---
>>> drivers/perf/arm_cspmu/arm_cspmu.c | 3 ++-
>>> drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
>>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
>> b/drivers/perf/arm_cspmu/arm_cspmu.c
>>> index efa9b229e701..e4b98cfa606c 100644
>>> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
>>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
>>> @@ -70,12 +70,13 @@ static void arm_cspmu_set_ev_filter(struct
>> arm_cspmu *cspmu,
>>> static void arm_cspmu_set_cc_filter(struct arm_cspmu *cspmu,
>>>                                    const struct perf_event *event);
>>>
>>> -static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
>>> +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
>>> {
>>>        struct acpi_apmt_node **ptr = dev_get_platdata(dev);
>>>
>>>        return ptr ? *ptr : NULL;
>>> }
>>> +EXPORT_SYMBOL_GPL(arm_cspmu_apmt_node);
>>
>> Rather than exporting the function, wouldn't it be better to move it to
>> arm_cspmu.h instead?
> 
> Sounds good to me. I will make the change on the next revision.
> 
> Thanks,
> Besar
> 
>>
>> Cheers, Ilkka
>>
>>>
>>> /*
>>>   * In CoreSight PMU architecture, all of the MMIO registers are 32-bit except
>>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h
>> b/drivers/perf/arm_cspmu/arm_cspmu.h
>>> index 19684b76bd96..36c1dcce33d6 100644
>>> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
>>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
>>> @@ -8,6 +8,7 @@
>>> #ifndef __ARM_CSPMU_H__
>>> #define __ARM_CSPMU_H__
>>>
>>> +#include <linux/acpi.h>
>>> #include <linux/bitfield.h>
>>> #include <linux/cpumask.h>
>>> #include <linux/device.h>
>>> @@ -222,4 +223,7 @@ int arm_cspmu_impl_register(const struct
>> arm_cspmu_impl_match *impl_match);
>>> /* Unregister vendor backend. */
>>> void arm_cspmu_impl_unregister(const struct arm_cspmu_impl_match
>> *impl_match);
>>>
>>> +/* Get ACPI APMT node. */
>>> +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev);
>>> +
>>> #endif /* __ARM_CSPMU_H__ */
>>> --
>>> 2.47.0
>>>
>>>

RE: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node

[Besar Wicaksono]

Hi Robin,

Please see my comment inline.

> -----Original Message-----
> From: Robin Murphy <robin.murphy@arm.com>
> Sent: Thursday, August 21, 2025 11:05 AM
> To: Besar Wicaksono <bwicaksono@nvidia.com>; Ilkka Koskinen
> <ilkka@os.amperecomputing.com>
> Cc: will@kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-tegra@vger.kernel.org;
> suzuki.poulose@arm.com; mark.rutland@arm.com; Thierry Reding
> <treding@nvidia.com>; Jon Hunter <jonathanh@nvidia.com>; Vikram Sethi
> <vsethi@nvidia.com>; Rich Wiley <rwiley@nvidia.com>; Shanker Donthineni
> <sdonthineni@nvidia.com>
> Subject: Re: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node
> 
> External email: Use caution opening links or attachments
> 
> 
> On 2025-08-20 8:07 pm, Besar Wicaksono wrote:
> > Hi Ilkka,
> >
> > Thanks for the feedback. Please see my comment inline.
> >
> >> -----Original Message-----
> >> From: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> >> Sent: Tuesday, August 19, 2025 3:16 PM
> >> To: Besar Wicaksono <bwicaksono@nvidia.com>
> >> Cc: will@kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> >> kernel@vger.kernel.org; linux-tegra@vger.kernel.org;
> >> suzuki.poulose@arm.com; robin.murphy@arm.com;
> >> ilkka@os.amperecomputing.com; mark.rutland@arm.com; Thierry Reding
> >> <treding@nvidia.com>; Jon Hunter <jonathanh@nvidia.com>; Vikram Sethi
> >> <vsethi@nvidia.com>; Rich Wiley <rwiley@nvidia.com>; Shanker
> Donthineni
> >> <sdonthineni@nvidia.com>
> >> Subject: Re: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> Hi Ben,
> >>
> >> On Tue, 12 Aug 2025, Besar Wicaksono wrote:
> >>> Make arm_cspmu_apmt_node API accessible to vendor driver.
> >>
> >> I think I haven't seen the latest version of the spec. So, I'm curious,
> >> what kind of information the table has that the vendor drivers needs to
> >> have access to it?
> >>
> >
> > The vendor driver may need the node instance primary and secondary
> > fields to get additional properties of the PMU that is not covered
> > by the standard properties. For example, the PMU device entry in
> > APMT can be defined as ACPI node type. The node instance primary
> > and secondary will contain the HID and UID of an ACPI device object
> > that is associated with the PMU. This ACPI object can have more info
> > to supplement the standard props.
> 
> Rather than exposing the raw APMT data, maybe then cspmu should just
> encapsulate a method for retrieving the associated ACPI device, if any?

Thanks for the suggestion, this makes sense.

> I guess it could be a generalised "firmware device" notion - even though
> for DT that should mostly be cspmu->dev already - since that could then
> work neatly for generic device properties, but perhaps we don't expect
> many sub-drivers to support both ACPI and DT...
> 

How about making it explicitly return acpi_device object? i.e returns NULL
if not an ACPI node type. Vendor driver can choose (or not) to fallback to
cspmu->dev in case of DT.

Thanks,
Besar

> Thanks,
> Robin.
> 
> >>>
> >>> Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
> >>> ---
> >>> drivers/perf/arm_cspmu/arm_cspmu.c | 3 ++-
> >>> drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
> >>> 2 files changed, 6 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
> >> b/drivers/perf/arm_cspmu/arm_cspmu.c
> >>> index efa9b229e701..e4b98cfa606c 100644
> >>> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> >>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> >>> @@ -70,12 +70,13 @@ static void arm_cspmu_set_ev_filter(struct
> >> arm_cspmu *cspmu,
> >>> static void arm_cspmu_set_cc_filter(struct arm_cspmu *cspmu,
> >>>                                    const struct perf_event *event);
> >>>
> >>> -static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device
> *dev)
> >>> +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
> >>> {
> >>>        struct acpi_apmt_node **ptr = dev_get_platdata(dev);
> >>>
> >>>        return ptr ? *ptr : NULL;
> >>> }
> >>> +EXPORT_SYMBOL_GPL(arm_cspmu_apmt_node);
> >>
> >> Rather than exporting the function, wouldn't it be better to move it to
> >> arm_cspmu.h instead?
> >
> > Sounds good to me. I will make the change on the next revision.
> >
> > Thanks,
> > Besar
> >
> >>
> >> Cheers, Ilkka
> >>
> >>>
> >>> /*
> >>>   * In CoreSight PMU architecture, all of the MMIO registers are 32-bit
> except
> >>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h
> >> b/drivers/perf/arm_cspmu/arm_cspmu.h
> >>> index 19684b76bd96..36c1dcce33d6 100644
> >>> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> >>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> >>> @@ -8,6 +8,7 @@
> >>> #ifndef __ARM_CSPMU_H__
> >>> #define __ARM_CSPMU_H__
> >>>
> >>> +#include <linux/acpi.h>
> >>> #include <linux/bitfield.h>
> >>> #include <linux/cpumask.h>
> >>> #include <linux/device.h>
> >>> @@ -222,4 +223,7 @@ int arm_cspmu_impl_register(const struct
> >> arm_cspmu_impl_match *impl_match);
> >>> /* Unregister vendor backend. */
> >>> void arm_cspmu_impl_unregister(const struct arm_cspmu_impl_match
> >> *impl_match);
> >>>
> >>> +/* Get ACPI APMT node. */
> >>> +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev);
> >>> +
> >>> #endif /* __ARM_CSPMU_H__ */
> >>> --
> >>> 2.47.0
> >>>
> >>>