[PATCH 0/2] perf: Support Arm Bus Monitor Unit

[PATCH 0/2] perf: Support Arm Bus Monitor Unit

[Robin Murphy]

Hi all,

Here we have the delight of another new PMU driver. While not am Arm
product in its own right, the BMU is found in the latest Neoverse
Compute Subsystems, including the recently-announced AGI CPU.

For now it is expected that BMU users will be ACPI-based (binding in
the latest "ACPI for Arm Components" release[1]), so the token DT
binding is more for completeness and maybe very early bringup hacking,
but there is a chance it may end up being used by more embedded/client-
focused subsystems in future.

And yes, I did initially look at trying to factor out the PMU register
accesses that functionally overlap the existing arm_cspmu code, but it
very quickly became clear that that would end up far bigger and more
complex. This is not a big driver, even with a few dozen lines of
nominally-duplicate (but also simplified since the counter size etc.
is fixed and known) code, so this way it went.

Thanks,
Robin.

[1] https://developer.arm.com/documentation/den0093/latest/


Robin Murphy (2):
  dt-bindings: perf: Add Arm Bus Monitor Unit
  perf: Add Arm Bus Monitor Unit driver

 .../bindings/perf/arm,bus-monitor-unit.yaml   |  33 ++
 drivers/perf/arm_cspmu/Kconfig                |   6 +
 drivers/perf/arm_cspmu/Makefile               |   2 +
 drivers/perf/arm_cspmu/arm-bmu.c              | 538 ++++++++++++++++++
 4 files changed, 579 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/perf/arm,bus-monitor-unit.yaml
 create mode 100644 drivers/perf/arm_cspmu/arm-bmu.c

-- 
2.54.0.dirty

[PATCH 1/2] dt-bindings: perf: Add Arm Bus Monitor Unit

[Robin Murphy]

Arm's Bus Monitor Unit is a low-level performance analysis tool for
matching and counting transactions at interconnect interfaces. Typical
platforms will have multiple BMUs each measuring multiple interfaces in
various parts of the system, hence for the sake of usability we make it
mandatory for the platform to label each instance such that the end user
can correlate with platform documentation to know what's what.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 .../bindings/perf/arm,bus-monitor-unit.yaml   | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/perf/arm,bus-monitor-unit.yaml

diff --git a/Documentation/devicetree/bindings/perf/arm,bus-monitor-unit.yaml b/Documentation/devicetree/bindings/perf/arm,bus-monitor-unit.yaml
new file mode 100644
index 000000000000..3cdc3a05b9a6
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/arm,bus-monitor-unit.yaml
@@ -0,0 +1,33 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/perf/arm,bus-monitor-unit.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Arm Bus Monitor Unit
+
+maintainers:
+  - Robin Murphy <robin.murphy@arm.com>
+
+properties:
+  compatible:
+    const: arm,bus-monitor-unit
+
+  reg:
+    items:
+      - description: Full BMU register space
+
+  interrupts:
+    items:
+      - description: PMU overflow interrupt
+
+  label:
+    description: BMU instance identifier (same as ACPI _UID), per the
+      platform optimisation guide
+
+required:
+  - compatible
+  - reg
+  - label
+
+additionalProperties: false
-- 
2.54.0.dirty

[PATCH 2/2] perf: Add Arm Bus Monitor Unit driver

[Robin Murphy]

Arm's Bus Monitor Unit is a low-level performance analysis tool for
matching and counting transactions at interconnect interfaces. Each
BMU consists of a number of "interface monitoring units", with some
global controls to synchronise them and so permit accurate calculation
of cross-interface metrics. Each IMU contains, among other things, its
own PMU largely based on the CoreSight PMU architecture, but with some
functional incompatibilities due to the nature of how much per-event
filtering needs to be crammed in to categorise AXI/CHI transactions at
the protocol level. As such it gets an honorary home in arm_cspmu/ in
order to share the common register definitions, but for simplicity does
not try to bend the functional abstraction to fit.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/perf/arm_cspmu/Kconfig   |   6 +
 drivers/perf/arm_cspmu/Makefile  |   2 +
 drivers/perf/arm_cspmu/arm-bmu.c | 538 +++++++++++++++++++++++++++++++
 3 files changed, 546 insertions(+)
 create mode 100644 drivers/perf/arm_cspmu/arm-bmu.c

diff --git a/drivers/perf/arm_cspmu/Kconfig b/drivers/perf/arm_cspmu/Kconfig
index 6f4e28fc84a2..8a7e2082b850 100644
--- a/drivers/perf/arm_cspmu/Kconfig
+++ b/drivers/perf/arm_cspmu/Kconfig
@@ -27,3 +27,9 @@ config AMPERE_CORESIGHT_PMU_ARCH_SYSTEM_PMU
 
 	  In the first phase, the driver enables support on MCU PMU used in
 	  AmpereOne SoC family.
+
+config ARM_BMU
+	tristate "ARM Bus Monitor Unit"
+	depends on ARM64 || COMPILE_TEST
+	help
+	  Support perf event counting on Arm Bus Monitor Unit devices.
diff --git a/drivers/perf/arm_cspmu/Makefile b/drivers/perf/arm_cspmu/Makefile
index 220a734efd54..622b8fc3d5dc 100644
--- a/drivers/perf/arm_cspmu/Makefile
+++ b/drivers/perf/arm_cspmu/Makefile
@@ -8,3 +8,5 @@ arm_cspmu_module-y := arm_cspmu.o
 
 obj-$(CONFIG_NVIDIA_CORESIGHT_PMU_ARCH_SYSTEM_PMU) += nvidia_cspmu.o
 obj-$(CONFIG_AMPERE_CORESIGHT_PMU_ARCH_SYSTEM_PMU) += ampere_cspmu.o
+
+obj-$(CONFIG_ARM_BMU) += arm-bmu.o
diff --git a/drivers/perf/arm_cspmu/arm-bmu.c b/drivers/perf/arm_cspmu/arm-bmu.c
new file mode 100644
index 000000000000..0ed9e34857ff
--- /dev/null
+++ b/drivers/perf/arm_cspmu/arm-bmu.c
@@ -0,0 +1,538 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025-2026 Arm Limited
+
+#include <linux/acpi.h>
+#include <linux/bitfield.h>
+#include <linux/interrupt.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/perf_event.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+
+#include "arm_cspmu.h"
+
+#define MCU_CONFIG			0x0000
+#define MCU_PMU_GLOBAL_EN		0x0008
+#define MCU_PMU_INTERRUPT_STATUS	0x0090
+
+#define MCUCFG_NUM_IMU_MONITORS		GENMASK_ULL(63, 60)
+#define MCUCFG_PMU_ELEMENT_SIZE		GENMASK_ULL(11, 8)
+#define MCUCFG_PMU_ELEMENT_START	GENMASK_ULL(7, 0)
+
+#define BMU_PMIMPDEF			0x0200
+#define BMU_PMCR			0x0e10
+
+/* These are reasonable expectations for now */
+#define PMU_MAX_COUNTERS	16
+#define MAX_IMUS		16
+
+/* Event attributes */
+#define BMU_CONFIG_IMU		GENMASK_ULL(63, 56)
+#define BMU_CONFIG_EVENT	GENMASK_ULL(55, 0)
+#define BMU_CONFIG_FILTER	GENMASK_ULL(63, 0)
+#define BMU_CONFIG_MATCH	GENMASK_ULL(63, 0)
+#define BMU_CONFIG_MASK		GENMASK_ULL(63, 0)
+
+#define BMU_EVENT_IMU(e)	FIELD_GET(BMU_CONFIG_IMU, (e)->attr.config)
+#define BMU_EVENT_EVENT(e)	FIELD_GET(BMU_CONFIG_EVENT, (e)->attr.config)
+#define BMU_EVENT_FILTER(e)	FIELD_GET(BMU_CONFIG_FILTER, (e)->attr.config1)
+#define BMU_EVENT_MATCH(e)	FIELD_GET(BMU_CONFIG_MATCH, (e)->attr.config2)
+#define BMU_EVENT_MASK(e)	FIELD_GET(BMU_CONFIG_MASK, (e)->attr.config3)
+
+static unsigned long arm_bmu_cpuhp_state;
+
+struct arm_bmu_pmu {
+	void __iomem *base;
+	struct perf_event *evcnt[PMU_MAX_COUNTERS];
+	int num_counters;
+};
+
+struct arm_bmu {
+	struct device *dev;
+	void __iomem *base;
+	struct pmu pmu;
+	struct hlist_node cpuhp_node;
+	int cpu;
+	int irq;
+	int num_imus;
+	struct arm_bmu_pmu imus[] __counted_by(num_imus);
+};
+
+#define to_bmu(p)	container_of(p, struct arm_bmu, pmu)
+#define event_imu(e)	(e)->hw.event_base
+#define to_bmu_pmu(e)	&to_bmu(e->pmu)->imus[event_imu(e)]
+
+struct arm_bmu_format_attr {
+	struct device_attribute attr;
+	u64 field;
+	int config;
+};
+
+#define BMU_FORMAT_ATTR(_name, _cfg, _fld)				\
+	(&((struct arm_bmu_format_attr[]) {{				\
+		.attr = __ATTR(_name, 0444, arm_bmu_format_show, NULL),	\
+		.field = _fld,						\
+		.config = _cfg,						\
+	}})[0].attr.attr)
+
+static ssize_t arm_bmu_format_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct arm_bmu_format_attr *fmt = container_of(attr, typeof(*fmt), attr);
+
+	if (!fmt->config)
+		return sysfs_emit(buf, "config:%*pbl\n", 64, &fmt->field);
+
+	return sysfs_emit(buf, "config%d:%*pbl\n", fmt->config, 64, &fmt->field);
+}
+
+static struct attribute *arm_bmu_format_attrs[] = {
+	BMU_FORMAT_ATTR(imu, 0, BMU_CONFIG_IMU),
+	BMU_FORMAT_ATTR(event, 0, BMU_CONFIG_EVENT),
+	BMU_FORMAT_ATTR(filter, 1, BMU_CONFIG_FILTER),
+	BMU_FORMAT_ATTR(match, 2, BMU_CONFIG_MATCH),
+	BMU_FORMAT_ATTR(mask, 3, BMU_CONFIG_MASK),
+	NULL
+};
+
+static const struct attribute_group arm_bmu_format_attrs_group = {
+	.name = "format",
+	.attrs = arm_bmu_format_attrs,
+};
+
+static ssize_t arm_bmu_cpumask_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct arm_bmu *bmu = to_bmu(dev_get_drvdata(dev));
+
+	return sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(cpumask_of(bmu->cpu)));
+}
+
+static struct device_attribute arm_bmu_cpumask_attr =
+		__ATTR(cpumask, 0444, arm_bmu_cpumask_show, NULL);
+
+static ssize_t arm_bmu_identifier_show(struct device *dev,
+				       struct device_attribute *attr, char *buf)
+{
+	struct arm_bmu *bmu = to_bmu(dev_get_drvdata(dev));
+	u32 reg = readl_relaxed(bmu->imus[0].base + PMIIDR);
+
+	return sysfs_emit(buf, "%x\n", reg);
+}
+
+static struct device_attribute arm_bmu_identifier_attr =
+		__ATTR(identifier, 0444, arm_bmu_identifier_show, NULL);
+
+static struct attribute *arm_bmu_other_attrs[] = {
+	&arm_bmu_cpumask_attr.attr,
+	&arm_bmu_identifier_attr.attr,
+	NULL
+};
+
+static const struct attribute_group arm_bmu_other_attr_group = {
+	.attrs = arm_bmu_other_attrs,
+};
+
+static const struct attribute_group *arm_bmu_attr_groups[] = {
+	&arm_bmu_format_attrs_group,
+	&arm_bmu_other_attr_group,
+	NULL
+};
+
+static void arm_bmu_enable(struct pmu *pmu)
+{
+	writel_relaxed(1, to_bmu(pmu)->base + MCU_PMU_GLOBAL_EN);
+}
+
+static void arm_bmu_disable(struct pmu *pmu)
+{
+	writel_relaxed(0, to_bmu(pmu)->base + MCU_PMU_GLOBAL_EN);
+}
+
+static int arm_bmu_validate_group(struct perf_event *event)
+{
+	struct arm_bmu *bmu = to_bmu(event->pmu);
+	struct perf_event *sibling, *leader = event->group_leader;
+	int num[MAX_IMUS] = { 0 };
+
+	++num[event_imu(event)];
+	if (leader != event) {
+		if (leader->pmu == &bmu->pmu)
+			++num[event_imu(leader)];
+		for_each_sibling_event(sibling, leader) {
+			if (sibling->pmu == &bmu->pmu)
+				++num[event_imu(sibling)];
+		}
+	}
+	for (int i = 0; i < bmu->num_imus; i++) {
+		if (++num[i] > bmu->imus[i].num_counters)
+			return -EINVAL;
+	}
+	return 0;
+}
+
+static int arm_bmu_event_init(struct perf_event *event)
+{
+	struct arm_bmu *bmu = to_bmu(event->pmu);
+
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	if (is_sampling_event(event))
+		return -EINVAL;
+
+	event->cpu = bmu->cpu;
+	event_imu(event) = BMU_EVENT_IMU(event);
+	if (event_imu(event) >= bmu->num_imus)
+		return -EINVAL;
+
+	return arm_bmu_validate_group(event);
+}
+
+static u64 arm_bmu_read_evcnt(struct perf_event *event)
+{
+	struct arm_bmu_pmu *pmu = to_bmu_pmu(event);
+	void __iomem *reg_base = pmu->base + event->hw.idx * sizeof(u64);
+	u64 lo, hi_old, hi_new;
+	int retries = 3; /* 1st time unlucky, 2nd improbable, 3rd just broken */
+
+	hi_new = readl_relaxed(reg_base + PMEVCNTR_HI);
+	do {
+		hi_old = hi_new;
+		lo = readl_relaxed(reg_base + PMEVCNTR_LO);
+		hi_new = readl_relaxed(reg_base + PMEVCNTR_HI);
+	} while (hi_new != hi_old && --retries);
+	WARN_ON(!retries);
+
+	return (hi_new << 32) | lo;
+}
+
+static void arm_bmu_event_read(struct perf_event *event)
+{
+	struct hw_perf_event *hw = &event->hw;
+	u64 count, prev;
+
+	do {
+		prev = local64_read(&hw->prev_count);
+		count = arm_bmu_read_evcnt(event);
+	} while (local64_cmpxchg(&hw->prev_count, prev, count) != prev);
+
+	count -= prev;
+	local64_add(count, &event->count);
+}
+
+static void arm_bmu_event_start(struct perf_event *event, int flags)
+{
+	struct arm_bmu_pmu *pmu = to_bmu_pmu(event);
+
+	writel_relaxed(1ULL << event->hw.idx, pmu->base + PMCNTENSET);
+}
+
+static void arm_bmu_event_stop(struct perf_event *event, int flags)
+{
+	struct arm_bmu_pmu *pmu = to_bmu_pmu(event);
+
+	writel_relaxed(1ULL << event->hw.idx, pmu->base + PMCNTENCLR);
+	if (flags & PERF_EF_UPDATE)
+		arm_bmu_event_read(event);
+}
+
+static int arm_bmu_event_add(struct perf_event *event, int flags)
+{
+	struct arm_bmu_pmu *pmu = to_bmu_pmu(event);
+	struct hw_perf_event *hw = &event->hw;
+	void __iomem *reg_base;
+
+	hw->idx = 0;
+	while (pmu->evcnt[hw->idx]) {
+		if (++hw->idx == pmu->num_counters)
+			return -ENOSPC;
+	}
+	pmu->evcnt[hw->idx] = event;
+
+	reg_base = pmu->base + hw->idx * sizeof(u64);
+	lo_hi_writeq_relaxed(BMU_EVENT_EVENT(event), reg_base + PMEVTYPER);
+	lo_hi_writeq_relaxed(BMU_EVENT_FILTER(event), reg_base + PMEVFILTR);
+	lo_hi_writeq_relaxed(BMU_EVENT_MATCH(event), reg_base + PMEVFILT2R);
+	lo_hi_writeq_relaxed(BMU_EVENT_MASK(event), reg_base + BMU_PMIMPDEF);
+
+	local64_set(&hw->prev_count, S64_MIN);
+	lo_hi_writeq_relaxed(S64_MIN, reg_base + PMEVCNTR_LO);
+
+	if (flags & PERF_EF_START)
+		arm_bmu_event_start(event, 0);
+	return 0;
+}
+
+static void arm_bmu_event_del(struct perf_event *event, int flags)
+{
+	struct arm_bmu_pmu *pmu = to_bmu_pmu(event);
+
+	arm_bmu_event_stop(event, PERF_EF_UPDATE);
+	pmu->evcnt[event->hw.idx] = NULL;
+}
+
+static void arm_bmu_pmu_irq(struct arm_bmu_pmu *pmu)
+{
+	u32 reg = readl_relaxed(pmu->base + PMOVSCLR);
+	u64 __iomem *pmevcnt = pmu->base + PMEVCNTR_LO;
+
+	for (int i = 0; i < PMU_MAX_COUNTERS; i++) {
+		if (!(reg & (1U << i)))
+			continue;
+		if (WARN_ON(!pmu->evcnt[i]))
+			continue;
+		arm_bmu_event_read(pmu->evcnt[i]);
+		local64_set(&pmu->evcnt[i]->hw.prev_count, S64_MIN);
+		lo_hi_writeq_relaxed(S64_MIN, pmevcnt + i);
+	}
+	writel_relaxed(reg, pmu->base + PMOVSCLR);
+}
+
+static irqreturn_t arm_bmu_handle_irq(int irq, void *dev_id)
+{
+	struct arm_bmu *bmu = dev_id;
+	u32 reg = readl_relaxed(bmu->base + MCU_PMU_INTERRUPT_STATUS);
+	irqreturn_t ret = reg ? IRQ_HANDLED : IRQ_NONE;
+
+	for (int i = 0; i < bmu->num_imus && reg; i++, reg >>= 1) {
+		if (reg & 1)
+			arm_bmu_pmu_irq(bmu->imus + i);
+	}
+	return ret;
+}
+
+static int arm_bmu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct resource *res;
+	struct arm_bmu *bmu;
+	const char *name = NULL;
+	void __iomem *base;
+	static atomic_t n;
+	int err, num, sz, off;
+	u64 cfg;
+	u32 reg;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+
+	/* PMUs and MPAM MSCs are intermingled so we can't claim the whole resource */
+	base = devm_ioremap(dev, res->start, resource_size(res));
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	/* Global and per-PMU-page security all defaults to Root/Secure only */
+	writel_relaxed(1, base + MCU_PMU_GLOBAL_EN);
+	reg = readl_relaxed(base + MCU_PMU_GLOBAL_EN);
+	if (reg != 1)
+		return dev_err_probe(dev, -ENODEV, "Non-Secure access to BMU not enabled?\n");
+	writel_relaxed(0, base + MCU_PMU_GLOBAL_EN);
+
+	cfg = lo_hi_readq_relaxed(base + MCU_CONFIG);
+	num = 1 + FIELD_GET(MCUCFG_NUM_IMU_MONITORS, cfg);
+	/* We don't expect to have dual-page complications to worry about */
+	sz = FIELD_GET(MCUCFG_PMU_ELEMENT_SIZE, cfg);
+	if (sz != 1)
+		return dev_err_probe(dev, -EINVAL, "PMU_ELEMENT_SIZE 0x%x not supported\n", sz);
+
+	/* The PMU pages *are* exclusively ours */
+	off = SZ_4K * FIELD_GET(MCUCFG_PMU_ELEMENT_START, cfg);
+	if (!devm_request_mem_region(dev, res->start + off, num * SZ_4K, dev_name(dev)))
+		return dev_err_probe(dev, err, "Unable to request PMU region\n");
+
+	bmu = devm_kzalloc(dev, struct_size(bmu, imus, num), GFP_KERNEL);
+	if (!bmu)
+		return -ENOMEM;
+
+	bmu->dev = dev;
+	bmu->base = base;
+	bmu->num_imus = num;
+	platform_set_drvdata(pdev, bmu);
+
+	base += off;
+	for (int i = 0; i < bmu->num_imus; i++, base += SZ_4K) {
+		/* At least PMCFGR.SIZE should always be nonzero if visible */
+		reg = readl_relaxed(base + PMCFGR);
+		if (!reg) {
+			dev_warn(dev, "Non-Secure access to PMU %d not enabled?\n", i);
+			num = 0;
+		} else {
+			num = 1 + FIELD_GET(PMCFGR_N, reg);
+		}
+		if (num > PMU_MAX_COUNTERS) {
+			dev_notice(dev, "PMU %d has %d counters, only using %d\n",
+				   i, num, PMU_MAX_COUNTERS);
+			num = PMU_MAX_COUNTERS;
+		}
+		bmu->imus[i].base = base;
+		bmu->imus[i].num_counters = num;
+
+		writel_relaxed(PMCR_P | PMCR_E, base + BMU_PMCR);
+		writel_relaxed(U32_MAX, base + PMCNTENCLR);
+		writel_relaxed(U32_MAX, base + PMOVSCLR);
+		writel_relaxed(U32_MAX, base + PMINTENSET);
+	}
+
+	bmu->cpu = cpumask_local_spread(atomic_fetch_inc(&n), dev_to_node(dev));
+	bmu->irq = platform_get_irq(pdev, 0);
+	if (bmu->irq > 0) {
+		err = devm_request_irq(dev, bmu->irq, arm_bmu_handle_irq,
+				       IRQF_NOBALANCING | IRQF_NO_THREAD,
+				       dev_name(dev), bmu);
+		if (err)
+			bmu->irq = 0;
+		else
+			irq_set_affinity(bmu->irq, cpumask_of(bmu->cpu));
+	}
+	if (!bmu->irq)
+		dev_info(dev, "Continuing without IRQ\n");
+
+	bmu->pmu = (struct pmu) {
+		.module = THIS_MODULE,
+		.parent = dev,
+		.attr_groups = arm_bmu_attr_groups,
+		.capabilities = PERF_PMU_CAP_NO_EXCLUDE,
+		.task_ctx_nr = perf_invalid_context,
+		.pmu_enable = arm_bmu_enable,
+		.pmu_disable = arm_bmu_disable,
+		.event_init = arm_bmu_event_init,
+		.add = arm_bmu_event_add,
+		.del = arm_bmu_event_del,
+		.start = arm_bmu_event_start,
+		.stop = arm_bmu_event_stop,
+		.read = arm_bmu_event_read,
+	};
+
+	if (has_acpi_companion(dev))
+		name = acpi_device_uid(ACPI_COMPANION(dev));
+	else
+		device_property_read_string(dev, "label", &name);
+
+	if (name)
+		name = devm_kasprintf(dev, GFP_KERNEL, "arm_bmu_%s", name);
+	else
+		name = devm_kasprintf(dev, GFP_KERNEL, "arm_bmu_%llx", res->start >> 12);
+	if (!name)
+		return -ENOMEM;
+
+	err = cpuhp_state_add_instance_nocalls(arm_bmu_cpuhp_state, &bmu->cpuhp_node);
+	if (err)
+		return err;
+
+	err = perf_pmu_register(&bmu->pmu, name, -1);
+	if (err)
+		cpuhp_state_remove_instance_nocalls(arm_bmu_cpuhp_state, &bmu->cpuhp_node);
+
+	return err;
+}
+
+static void arm_bmu_remove(struct platform_device *pdev)
+{
+	struct arm_bmu *bmu = platform_get_drvdata(pdev);
+
+	for (int i = 0; i < bmu->num_imus; i++)
+		writel_relaxed(U32_MAX, bmu->imus[i].base + PMINTENCLR);
+
+	perf_pmu_unregister(&bmu->pmu);
+	cpuhp_state_remove_instance_nocalls(arm_bmu_cpuhp_state, &bmu->cpuhp_node);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id arm_bmu_of_match[] = {
+	{ .compatible = "arm,bus-monitor-unit" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, arm_bmu_of_match);
+#endif
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id arm_bmu_acpi_match[] = {
+	{ "ARMHB001" },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, arm_bmu_acpi_match);
+#endif
+
+static struct platform_driver arm_bmu_driver = {
+	.driver = {
+		.name = "arm-bmu",
+		.of_match_table = of_match_ptr(arm_bmu_of_match),
+		.acpi_match_table = ACPI_PTR(arm_bmu_acpi_match),
+		.suppress_bind_attrs = true,
+	},
+	.probe = arm_bmu_probe,
+	.remove = arm_bmu_remove,
+};
+
+static void arm_bmu_migrate(struct arm_bmu *bmu, unsigned int cpu)
+{
+	perf_pmu_migrate_context(&bmu->pmu, bmu->cpu, cpu);
+	if (bmu->irq)
+		irq_set_affinity(bmu->irq, cpumask_of(cpu));
+	bmu->cpu = cpu;
+}
+
+static int arm_bmu_online_cpu(unsigned int cpu, struct hlist_node *cpuhp_node)
+{
+	struct arm_bmu *bmu;
+	int node;
+
+	bmu = hlist_entry_safe(cpuhp_node, struct arm_bmu, cpuhp_node);
+	node = dev_to_node(bmu->dev);
+	if (cpu_to_node(bmu->cpu) != node && cpu_to_node(cpu) == node)
+		arm_bmu_migrate(bmu, cpu);
+	return 0;
+}
+
+static int arm_bmu_offline_cpu(unsigned int cpu, struct hlist_node *cpuhp_node)
+{
+	struct arm_bmu *bmu;
+	unsigned int target;
+	int node;
+
+	bmu = hlist_entry_safe(cpuhp_node, struct arm_bmu, cpuhp_node);
+	if (cpu != bmu->cpu)
+		return 0;
+
+	node = dev_to_node(bmu->dev);
+	target = cpumask_any_and_but(cpumask_of_node(node), cpu_online_mask, cpu);
+	if (target >= nr_cpu_ids)
+		target = cpumask_any_but(cpu_online_mask, cpu);
+
+	if (target < nr_cpu_ids)
+		arm_bmu_migrate(bmu, target);
+	return 0;
+}
+
+static int __init arm_bmu_init(void)
+{
+	int ret;
+
+	ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, "perf/arm/bmu:online",
+				      arm_bmu_online_cpu, arm_bmu_offline_cpu);
+	if (ret < 0)
+		return ret;
+
+	arm_bmu_cpuhp_state = ret;
+
+	ret = platform_driver_register(&arm_bmu_driver);
+	if (ret)
+		cpuhp_remove_multi_state(arm_bmu_cpuhp_state);
+	return ret;
+}
+
+static void __exit arm_bmu_exit(void)
+{
+	platform_driver_unregister(&arm_bmu_driver);
+	cpuhp_remove_multi_state(arm_bmu_cpuhp_state);
+}
+
+module_init(arm_bmu_init);
+module_exit(arm_bmu_exit);
+
+MODULE_DESCRIPTION("ARM Bus Monitor Unit Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.54.0.dirty

Re: [PATCH 1/2] dt-bindings: perf: Add Arm Bus Monitor Unit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The YAML device tree binding schema is missing the mandatory `examples` section.
--

--- Patch [1]: [PATCH 1/2] dt-bindings: perf: Add Arm Bus Monitor Unit ---
commit d43448a74f2d7de7e06899d55b1561b64c21375c
Author: Robin Murphy <robin.murphy@arm.com>

dt-bindings: perf: Add Arm Bus Monitor Unit

This commit adds a new device tree binding schema for the Arm Bus Monitor
Unit. It defines the required properties, including a mandatory label to allow
end users to correlate instances with platform documentation.

> diff --git a/Documentation/devicetree/bindings/perf/arm,bus-monitor-unit.yaml b/Documentation/devicetree/bindings/perf/arm,bus-monitor-unit.yaml
> new file mode 100644
> index 0000000000000..3cdc3a05b9a63
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/perf/arm,bus-monitor-unit.yaml

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - label
> +
> +additionalProperties: false

[Severity: Low]
Does this binding schema omit the mandatory examples section? Standard device
tree bindings typically require an examples block at the end of the file to
provide a reference node implementation and allow full validation by the
dt_binding_check tool.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783439341.git.robin.murphy@arm.com?part=1

Re: [PATCH 2/2] perf: Add Arm Bus Monitor Unit driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
- [High] Uninitialized variable `err` is returned when `devm_request_mem_region` fails.
- [High] Event validation erroneously pre-increments the event count for all IMUs in `arm_bmu_validate_group()`, rejecting valid groups.
- [High] Improper error checking of `devm_ioremap` return value causes NULL pointer dereference on failure.
- [High] Incorrect CPU hotplug registration order causes NULL pointer dereference or Use-After-Free.
- [High] The driver fails to verify that the hardware-provided PMU offsets and sizes fall within the mapped MMIO resource boundary.
- [Medium] Negative IRQ error code from `platform_get_irq()` is stored and later passed to `irq_set_affinity()`.
- [Medium] The PMU IRQ handler resets the running hardware counter, permanently losing events.
- [Medium] Spurious `WARN_ON` triggers if an event is deleted while an overflow is pending.
- [Medium] CPU selection races with CPU hotplug because `cpus_read_lock()` is not held.
- [Low] Format string type mismatch passing 32-bit `resource_size_t` to `%llx` specifier.
- [Low] Using `%*pbl` format specifier with a pointer to `u64` reads incorrectly on 32-bit architectures.
--

--- Patch [2]: [PATCH 2/2] perf: Add Arm Bus Monitor Unit driver ---
commit 2a4c03f8d2ffe5343511aa668d1b1b9f0e2a6b53
Author: Robin Murphy <robin.murphy@arm.com>
Subject: perf: Add Arm Bus Monitor Unit driver

This commit introduces the Arm Bus Monitor Unit (BMU) driver to support 
performance event counting on Arm BMU devices, placing it under arm_cspmu 
to share common register definitions.

> diff --git a/drivers/perf/arm_cspmu/arm-bmu.c b/drivers/perf/arm_cspmu/arm-bmu.c
> new file mode 100644
> index 0000000000000..0ed9e34857ff8
> --- /dev/null
> +++ b/drivers/perf/arm_cspmu/arm-bmu.c
[ ... ]
> +static ssize_t arm_bmu_format_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct arm_bmu_format_attr *fmt = container_of(attr, typeof(*fmt), attr);
> +
> +	if (!fmt->config)
> +		return sysfs_emit(buf, "config:%*pbl\n", 64, &fmt->field);
> +
> +	return sysfs_emit(buf, "config%d:%*pbl\n", fmt->config, 64, &fmt->field);
> +}

[Severity: Low]
Does passing a pointer to a u64 variable into the %*pbl format specifier read
incorrectly on 32-bit architectures? 

The %*pbl specifier expects a pointer to an array of unsigned longs. On 32-bit
big-endian systems under COMPILE_TEST, this might result in reversed words.

[ ... ]
> +static int arm_bmu_validate_group(struct perf_event *event)
> +{
> +	struct arm_bmu *bmu = to_bmu(event->pmu);
> +	struct perf_event *sibling, *leader = event->group_leader;
> +	int num[MAX_IMUS] = { 0 };
[ ... ]
> +	for (int i = 0; i < bmu->num_imus; i++) {
> +		if (++num[i] > bmu->imus[i].num_counters)
> +			return -EINVAL;
> +	}
> +	return 0;
> +}

[Severity: High]
Does this loop unconditionally increment the event count requirement for every
IMU on the device? It appears `++num[i]` will be evaluated for all iterations,
meaning valid groups might be rejected if they don't use all available IMUs.

[ ... ]
> +static void arm_bmu_event_stop(struct perf_event *event, int flags)
> +{
> +	struct arm_bmu_pmu *pmu = to_bmu_pmu(event);
> +
> +	writel_relaxed(1ULL << event->hw.idx, pmu->base + PMCNTENCLR);
> +	if (flags & PERF_EF_UPDATE)
> +		arm_bmu_event_read(event);
> +}
[ ... ]
> +static void arm_bmu_event_del(struct perf_event *event, int flags)
> +{
> +	struct arm_bmu_pmu *pmu = to_bmu_pmu(event);
> +
> +	arm_bmu_event_stop(event, PERF_EF_UPDATE);
> +	pmu->evcnt[event->hw.idx] = NULL;
> +}
[ ... ]
> +static void arm_bmu_pmu_irq(struct arm_bmu_pmu *pmu)
> +{
> +	u32 reg = readl_relaxed(pmu->base + PMOVSCLR);
> +	u64 __iomem *pmevcnt = pmu->base + PMEVCNTR_LO;
> +
> +	for (int i = 0; i < PMU_MAX_COUNTERS; i++) {
> +		if (!(reg & (1U << i)))
> +			continue;
> +		if (WARN_ON(!pmu->evcnt[i]))
> +			continue;

[Severity: Medium]
Could an uncleared overflow interrupt trigger this WARN_ON spuriously?

When an event is stopped, the hardware counter is halted, but pending overflow
interrupts in PMOVSCLR are not cleared. If an overflow fires just before the
counter is stopped and deleted, the IRQ handler might observe a NULL pointer
here and unnecessarily spam the logs.

> +		arm_bmu_event_read(pmu->evcnt[i]);
> +		local64_set(&pmu->evcnt[i]->hw.prev_count, S64_MIN);
> +		lo_hi_writeq_relaxed(S64_MIN, pmevcnt + i);
> +	}
> +	writel_relaxed(reg, pmu->base + PMOVSCLR);
> +}

[Severity: Medium]
Does writing S64_MIN back to the hardware counter permanently lose events?

The hardware counter is still running while it is being read in the handler.
Blindly resetting it without first disabling it via PMCNTENCLR might overwrite
and drop any counts that accumulated between the read and this reset.

[ ... ]
> +static int arm_bmu_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	/* PMUs and MPAM MSCs are intermingled so we can't claim the whole resource */
> +	base = devm_ioremap(dev, res->start, resource_size(res));
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);

[Severity: High]
Will IS_ERR(base) ever evaluate to true here? 

devm_ioremap() returns NULL on failure, not an ERR_PTR. Checking it with
IS_ERR() means allocation failures are silently ignored, leading to a NULL
pointer dereference on the subsequent writel_relaxed().

[ ... ]
> +	cfg = lo_hi_readq_relaxed(base + MCU_CONFIG);
> +	num = 1 + FIELD_GET(MCUCFG_NUM_IMU_MONITORS, cfg);
> +	/* We don't expect to have dual-page complications to worry about */
> +	sz = FIELD_GET(MCUCFG_PMU_ELEMENT_SIZE, cfg);
> +	if (sz != 1)
> +		return dev_err_probe(dev, -EINVAL, "PMU_ELEMENT_SIZE 0x%x not supported\n", sz);
> +
> +	/* The PMU pages *are* exclusively ours */
> +	off = SZ_4K * FIELD_GET(MCUCFG_PMU_ELEMENT_START, cfg);

[Severity: High]
Should the hardware-provided offset be validated against the mapped MMIO
resource boundary? 

Adding `off` to `base` without verifying that the resulting region falls within
resource_size(res) could lead to an out-of-bounds memory read if the hardware
reports an unexpectedly large value.

> +	if (!devm_request_mem_region(dev, res->start + off, num * SZ_4K, dev_name(dev)))
> +		return dev_err_probe(dev, err, "Unable to request PMU region\n");

[Severity: High]
Is `err` uninitialized in this error path? 

If devm_request_mem_region() fails, it returns NULL and doesn't modify `err`.
This might cause the probe function to return random stack garbage, or 0
(success), leaving the driver only partially initialized.

[ ... ]
> +	bmu->cpu = cpumask_local_spread(atomic_fetch_inc(&n), dev_to_node(dev));

[Severity: Medium]
Could the selected CPU go offline before the PMU's hotplug state is registered?

Since cpus_read_lock() is not held around this selection, a race could leave
the driver permanently bound to an offline CPU if a hotplug event occurs right
after this assignment.

> +	bmu->irq = platform_get_irq(pdev, 0);
> +	if (bmu->irq > 0) {
> +		err = devm_request_irq(dev, bmu->irq, arm_bmu_handle_irq,
> +				       IRQF_NOBALANCING | IRQF_NO_THREAD,
> +				       dev_name(dev), bmu);
> +		if (err)
> +			bmu->irq = 0;
> +		else
> +			irq_set_affinity(bmu->irq, cpumask_of(bmu->cpu));
> +	}
> +	if (!bmu->irq)
> +		dev_info(dev, "Continuing without IRQ\n");

[Severity: Medium]
If platform_get_irq() fails and returns a negative error code, will it bypass
the `!bmu->irq` check and retain the negative value?

Later during a CPU hotplug migration, arm_bmu_migrate() checks `if (bmu->irq)`
and might pass this negative error code directly into irq_set_affinity().

[ ... ]
> +	if (name)
> +		name = devm_kasprintf(dev, GFP_KERNEL, "arm_bmu_%s", name);
> +	else
> +		name = devm_kasprintf(dev, GFP_KERNEL, "arm_bmu_%llx", res->start >> 12);

[Severity: Low]
Could using %llx here cause format string corruption on 32-bit builds? 

Under COMPILE_TEST, `res->start` is a resource_size_t, which is a 32-bit
integer on 32-bit architectures. Passing a 32-bit integer to a 64-bit format
specifier will consume extra bits from the variadic arguments.

> +	if (!name)
> +		return -ENOMEM;
> +
> +	err = cpuhp_state_add_instance_nocalls(arm_bmu_cpuhp_state, &bmu->cpuhp_node);
> +	if (err)
> +		return err;
> +
> +	err = perf_pmu_register(&bmu->pmu, name, -1);
> +	if (err)
> +		cpuhp_state_remove_instance_nocalls(arm_bmu_cpuhp_state, &bmu->cpuhp_node);
> +
> +	return err;
> +}

[Severity: High]
Does registering with CPU hotplug before perf_pmu_register() create a race
window? 

If a hotplug event fires between these two calls, it would trigger
perf_pmu_migrate_context() on an uninitialized PMU context, which might result
in a NULL pointer dereference.

> +static void arm_bmu_remove(struct platform_device *pdev)
> +{
> +	struct arm_bmu *bmu = platform_get_drvdata(pdev);
> +
> +	for (int i = 0; i < bmu->num_imus; i++)
> +		writel_relaxed(U32_MAX, bmu->imus[i].base + PMINTENCLR);
> +
> +	perf_pmu_unregister(&bmu->pmu);
> +	cpuhp_state_remove_instance_nocalls(arm_bmu_cpuhp_state, &bmu->cpuhp_node);
> +}

[Severity: High]
Similarly, does unregistering the PMU before removing the hotplug instance
create a use-after-free window if a hotplug event occurs right after
perf_pmu_unregister() returns?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783439341.git.robin.murphy@arm.com?part=2