Re: [PATCH v2 1/2] perf: coresight_pmu: Add support for ARM CoreSight PMU driver

From: Suzuki K Poulose
Date: Thu May 19 2022 - 04:52:15 EST

On 15/05/2022 17:30, Besar Wicaksono wrote:
Add support for ARM CoreSight PMU driver framework and interfaces.
The driver provides generic implementation to operate uncore PMU based
on ARM CoreSight PMU architecture. The driver also provides interface
to get vendor/implementation specific information, for example event
attributes and formating.

The specification used in this implementation can be found below:
* ACPI Arm Performance Monitoring Unit table:
https://developer.arm.com/documentation/den0117/latest
* ARM Coresight PMU architecture:
https://developer.arm.com/documentation/ihi0091/latest

Signed-off-by: Besar Wicaksono <bwicaksono@xxxxxxxxxx>
---
arch/arm64/configs/defconfig | 1 +
drivers/perf/Kconfig | 2 +
drivers/perf/Makefile | 1 +
drivers/perf/coresight_pmu/Kconfig | 11 +
drivers/perf/coresight_pmu/Makefile | 6 +
.../perf/coresight_pmu/arm_coresight_pmu.c | 1267 +++++++++++++++++
.../perf/coresight_pmu/arm_coresight_pmu.h | 171 +++
7 files changed, 1459 insertions(+)
create mode 100644 drivers/perf/coresight_pmu/Kconfig
create mode 100644 drivers/perf/coresight_pmu/Makefile
create mode 100644 drivers/perf/coresight_pmu/arm_coresight_pmu.c
create mode 100644 drivers/perf/coresight_pmu/arm_coresight_pmu.h

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 2ca8b1b336d2..8f2120182b25 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1196,6 +1196,7 @@ CONFIG_PHY_UNIPHIER_USB3=y
CONFIG_PHY_TEGRA_XUSB=y
CONFIG_PHY_AM654_SERDES=m
CONFIG_PHY_J721E_WIZ=m
+CONFIG_ARM_CORESIGHT_PMU=y
CONFIG_ARM_SMMU_V3_PMU=m
CONFIG_FSL_IMX8_DDR_PMU=m
CONFIG_QCOM_L2_PMU=y
diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index 1e2d69453771..c4e7cd5b4162 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -192,4 +192,6 @@ config MARVELL_CN10K_DDR_PMU
Enable perf support for Marvell DDR Performance monitoring
event on CN10K platform.
+source "drivers/perf/coresight_pmu/Kconfig"
+
endmenu
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index 57a279c61df5..4126a04b5583 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_ARM_DMC620_PMU) += arm_dmc620_pmu.o
obj-$(CONFIG_MARVELL_CN10K_TAD_PMU) += marvell_cn10k_tad_pmu.o
obj-$(CONFIG_MARVELL_CN10K_DDR_PMU) += marvell_cn10k_ddr_pmu.o
obj-$(CONFIG_APPLE_M1_CPU_PMU) += apple_m1_cpu_pmu.o
+obj-$(CONFIG_ARM_CORESIGHT_PMU) += coresight_pmu/
diff --git a/drivers/perf/coresight_pmu/Kconfig b/drivers/perf/coresight_pmu/Kconfig
new file mode 100644
index 000000000000..89174f54c7be
--- /dev/null
+++ b/drivers/perf/coresight_pmu/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES.
+
+config ARM_CORESIGHT_PMU
+ tristate "ARM Coresight PMU"
+ depends on ACPI
+ depends on ACPI_APMT || COMPILE_TEST
+ help
+ Provides support for Performance Monitoring Unit (PMU) events based on
+ ARM CoreSight PMU architecture.
\ No newline at end of file
diff --git a/drivers/perf/coresight_pmu/Makefile b/drivers/perf/coresight_pmu/Makefile
new file mode 100644
index 000000000000..a2a7a5fbbc16
--- /dev/null
+++ b/drivers/perf/coresight_pmu/Makefile
@@ -0,0 +1,6 @@
+# Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES.
+#
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_ARM_CORESIGHT_PMU) += \
+ arm_coresight_pmu.o
diff --git a/drivers/perf/coresight_pmu/arm_coresight_pmu.c b/drivers/perf/coresight_pmu/arm_coresight_pmu.c
new file mode 100644
index 000000000000..36ac77ab85cd
--- /dev/null
+++ b/drivers/perf/coresight_pmu/arm_coresight_pmu.c
@@ -0,0 +1,1267 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM CoreSight PMU driver.
+ *
+ * This driver adds support for uncore PMU based on ARM CoreSight Performance
+ * Monitoring Unit Architecture. The PMU is accessible via MMIO registers and
+ * like other uncore PMUs, it does not support process specific events and
+ * cannot be used in sampling mode.
+ *
+ * This code is based on other uncore PMUs like ARM DSU PMU. It provides a
+ * generic implementation to operate the PMU according to CoreSight PMU
+ * architecture and ACPI ARM PMU table (APMT) documents below:
+ * - ARM CoreSight PMU architecture document number: ARM IHI 0091 A.a-00bet0.
+ * - APMT document number: ARM DEN0117.
+ * The description of the PMU, like the PMU device identification, available
+ * events, and configuration options, is vendor specific. The driver provides
+ * interface for vendor specific code to get this information. This allows the
+ * driver to be shared with PMU from different vendors.
+ *
+ * CoreSight PMU devices are named as arm_coresight_pmu<node_id> where <node_id>
+ * is APMT node id. The description of the device, like the identifier,

Please see my comment below, near coresight_pmu_alloc().

+ * supported events, and formats can be found in sysfs
+ * /sys/bus/event_source/devices/arm_coresight_pmu<node_id>
+ *
+ * The user should refer to the vendor technical documentation to get details
+ * about the supported events.
+ *
+ * Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES.
+ *
+ */

...

+static int coresight_pmu_alloc(struct platform_device *pdev,
+ struct coresight_pmu **coresight_pmu)
+{
+ struct acpi_apmt_node *apmt_node;
+ struct device *dev;
+ struct coresight_pmu *pmu;
+
+ dev = &pdev->dev;
+ apmt_node = *(struct acpi_apmt_node **)dev_get_platdata(dev);
+ if (!apmt_node) {
+ dev_err(dev, "failed to get APMT node\n");
+ return -ENOMEM;
+ }
+
+ pmu = devm_kzalloc(dev, sizeof(*pmu), GFP_KERNEL);
+ if (!pmu)
+ return -ENOMEM;
+
+ *coresight_pmu = pmu;
+
+ pmu->dev = dev;
+ pmu->apmt_node = apmt_node;
+ pmu->name =
+ devm_kasprintf(dev, GFP_KERNEL, PMUNAME "%u", apmt_node->id);

Could we not name this "<vendor>_<ipname>_pmu" ? Or even let the "implementor" name it ? After all, for a *normal user*, all it matters
is how to find a PMU device for my xyz IP.
The coresight_pmu architecture is there to make the life easier for the
software driver and the hardware implementation. We don't need to necessarily pass on this abstraction to an end-user and learn how to figure out which "arm_*_pmu<N>" is my PCIe, or SMMU or xyz instance.

Suzuki