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

[PATCH v2 0/3] 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.
I've kept it here for the sake of discussion, but the only requirement
on me for now is to get patch #1 landed.

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.

v1: https://lore.kernel.org/linux-perf-users/cover.1783439341.git.robin.murphy@arm.com/
v2: Fix valid Sashiko nits, separate DT support

Thanks,
Robin.

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

Robin Murphy (3):
  perf: Add Arm Bus Monitor Unit driver
  dt-bindings: perf: Add Arm Bus Monitor Unit
  perf/arm-bmu: Add Devicetree support

 .../bindings/perf/arm,bus-monitor-unit.yaml   |  41 ++
 drivers/perf/arm_cspmu/Kconfig                |   6 +
 drivers/perf/arm_cspmu/Makefile               |   2 +
 drivers/perf/arm_cspmu/arm-bmu.c              | 538 ++++++++++++++++++
 4 files changed, 587 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 v2 1/3] 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>

---
v2:
 - Fix copy-paste typo in group validation logic
 - Fix wonky error returns in probe
 - Make the handling of IRQ errors what it was supposed to be
 - Make format bitmap printing 100% robust for theoretical nitpickers
 - Split out DT support
---
 drivers/perf/arm_cspmu/Kconfig   |   6 +
 drivers/perf/arm_cspmu/Makefile  |   2 +
 drivers/perf/arm_cspmu/arm-bmu.c | 525 +++++++++++++++++++++++++++++++
 3 files changed, 533 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..0cb369f91023 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 && ACPI) || 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..c829a0728e9b
--- /dev/null
+++ b/drivers/perf/arm_cspmu/arm-bmu.c
@@ -0,0 +1,525 @@
+// 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/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;
+	DECLARE_BITMAP(field, 64);
+	int config;
+};
+
+#define BMU_FORMAT_ATTR(_name, _cfg, _fld)				\
+	(&((struct arm_bmu_format_attr[]) {{				\
+		.attr = __ATTR(_name, 0444, arm_bmu_format_show, NULL),	\
+		.field = { BITMAP_FROM_U64(_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 monitors are intermingled so we can't claim the whole resource */
+	base = devm_ioremap(dev, res->start, resource_size(res));
+	if (!base)
+		return -ENOMEM;
+
+	/* 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, -EADDRINUSE, "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 = err;
+		else
+			irq_set_affinity(bmu->irq, cpumask_of(bmu->cpu));
+	}
+	if (bmu->irq < 0)
+		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,
+	};
+
+	name = acpi_device_uid(ACPI_COMPANION(dev));
+
+	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_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",
+		.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 > 0)
+		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

[PATCH v2 2/3] 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>

---
v2: Invent example
---
 .../bindings/perf/arm,bus-monitor-unit.yaml   | 41 +++++++++++++++++++
 1 file changed, 41 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..3da59e5447e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/arm,bus-monitor-unit.yaml
@@ -0,0 +1,41 @@
+# 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
+
+examples:
+  - |
+    pmu@12340000 {
+        compatible = "arm,bus-monitor-unit";
+        reg = <0x12340000 0x40000>;
+        label = "ddr_ch_1_2";
+    };
-- 
2.54.0.dirty

[PATCH v2 3/3] perf/arm-bmu: Add Devicetree support

[Robin Murphy]

Initial production BMU users will be ACPI-based, but DT consumers
will almost certainly come along sooner or later.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: Separate patch as it's not a critical dependency right now
---
 drivers/perf/arm_cspmu/Kconfig   |  2 +-
 drivers/perf/arm_cspmu/arm-bmu.c | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm_cspmu/Kconfig b/drivers/perf/arm_cspmu/Kconfig
index 0cb369f91023..8a7e2082b850 100644
--- a/drivers/perf/arm_cspmu/Kconfig
+++ b/drivers/perf/arm_cspmu/Kconfig
@@ -30,6 +30,6 @@ config AMPERE_CORESIGHT_PMU_ARCH_SYSTEM_PMU
 
 config ARM_BMU
 	tristate "ARM Bus Monitor Unit"
-	depends on (ARM64 && ACPI) || COMPILE_TEST
+	depends on ARM64 || COMPILE_TEST
 	help
 	  Support perf event counting on Arm Bus Monitor Unit devices.
diff --git a/drivers/perf/arm_cspmu/arm-bmu.c b/drivers/perf/arm_cspmu/arm-bmu.c
index c829a0728e9b..0c2442392f7b 100644
--- a/drivers/perf/arm_cspmu/arm-bmu.c
+++ b/drivers/perf/arm_cspmu/arm-bmu.c
@@ -6,6 +6,7 @@
 #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>
@@ -406,7 +407,10 @@ static int arm_bmu_probe(struct platform_device *pdev)
 		.read = arm_bmu_event_read,
 	};
 
-	name = acpi_device_uid(ACPI_COMPANION(dev));
+	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);
@@ -437,6 +441,14 @@ static void arm_bmu_remove(struct platform_device *pdev)
 	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" },
@@ -448,6 +460,7 @@ MODULE_DEVICE_TABLE(acpi, arm_bmu_acpi_match);
 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,
 	},
-- 
2.54.0.dirty

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The driver calculates an offset from hardware configuration registers and accesses memory within an ioremapped region without bounds checking, which can lead to out-of-bounds MMIO accesses.
- [High] The CPU hotplug instance is added before PMU registration completes, exposing a race condition where a CPU hotplug event will cause a NULL pointer dereference.
- [Medium] A pending overflow interrupt will trigger a spurious `WARN_ON` when an event is deleted because the hardware interrupt flag is not cleared.
--

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

perf: Add Arm Bus Monitor Unit driver

This commit introduces the Arm Bus Monitor Unit (BMU) driver, which provides
a low-level performance analysis tool for matching and counting transactions
at interconnect interfaces. It supports multiple Interface Monitoring Units
(IMUs) by exposing them as perf PMU instances.

> diff --git a/drivers/perf/arm_cspmu/arm-bmu.c b/drivers/perf/arm_cspmu/arm-bmu.c
> new file mode 100644
> index 0000000000000..c829a0728e9b2
> --- /dev/null
> +++ b/drivers/perf/arm_cspmu/arm-bmu.c

[ ... ]

> +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;
> +}

[Severity: Medium]
Will an unhandled pending overflow interrupt trigger a spurious warning here?

If a performance counter overflows right before or during arm_bmu_event_del(),
local interrupts are disabled by the perf core, leaving the interrupt pending.
Since arm_bmu_event_del() stops the counter and sets the evcnt pointer to
NULL but doesn't clear the pending overflow interrupt from the PMOVSCLR
register, this pending interrupt will be processed when interrupts are
re-enabled.

This leads to arm_bmu_handle_irq() executing and reading the uncleared bit
from PMOVSCLR, which evaluates to true in the following WARN_ON():

[ ... ]

> +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;

[ ... ]

> +static int arm_bmu_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const struct resource *res;
> +	struct arm_bmu *bmu;

[ ... ]

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -EINVAL;
> +
> +	/* PMUs and MPAM monitors are intermingled so we can't claim the whole resource */
> +	base = devm_ioremap(dev, res->start, resource_size(res));
> +	if (!base)
> +		return -ENOMEM;

[ ... ]

> +	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, -EADDRINUSE, "Unable to request PMU region\n");

[Severity: High]
Does this calculation permit an out-of-bounds MMIO access if the offset from
the hardware configuration register is unexpectedly large?

The off and num values are read from the MCU_CONFIG hardware register and
used to calculate an offset into an ioremapped memory region that was mapped
to resource_size(res). There does not appear to be a check verifying that
off + num * SZ_4K does not exceed resource_size(res). 

If the hardware registers provide an unexpectedly large offset, accessing
memory outside the ioremapped bounds will trigger a synchronous external
abort when the driver attempts to read the PMCFGR register below:

> +
> +	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);

[ ... ]

> +	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);

[Severity: High]
Could a CPU hotplug event during this narrow window cause a NULL pointer
dereference?

The cpuhp_state_add_instance_nocalls() function is invoked before
perf_pmu_register(). If the assigned CPU goes offline during this exact
window, the hotplug state machine invokes arm_bmu_offline_cpu() which in
turn calls arm_bmu_migrate(). 

This leads to perf_pmu_migrate_context() being called. Because
perf_pmu_register() has not yet executed, the core context pointer is still
NULL, resulting in a dereference of an invalid pointer at the start of the
percpu offset region.

> +	if (err)
> +		cpuhp_state_remove_instance_nocalls(arm_bmu_cpuhp_state, &bmu->cpuhp_node);
> +
> +	return err;
> +}

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

Re: [PATCH v2 3/3] perf/arm-bmu: Add Devicetree support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Implicit declaration of function 'acpi_device_uid' causes a build failure when CONFIG_ACPI is disabled.
--

--- Patch [3]: [PATCH v2 3/3] perf/arm-bmu: Add Devicetree support ---
commit b787d1765a1d6aaa8fa04e23d64224ec5f6d4dc3
Author: Robin Murphy <robin.murphy@arm.com>

perf/arm-bmu: Add Devicetree support

This commit introduces Devicetree support for the ARM Bus Monitor Unit. It
adds the necessary OF match tables and relaxes the Kconfig dependencies so
the driver is no longer strictly tied to ACPI.

> diff --git a/drivers/perf/arm_cspmu/Kconfig b/drivers/perf/arm_cspmu/Kconfig
> --- a/drivers/perf/arm_cspmu/Kconfig
> +++ b/drivers/perf/arm_cspmu/Kconfig
> @@ -30,6 +30,6 @@ config AMPERE_CORESIGHT_PMU_ARCH_SYSTEM_PMU
>  
>  config ARM_BMU
>  	tristate "ARM Bus Monitor Unit"
> -	depends on (ARM64 && ACPI) || COMPILE_TEST
> +	depends on ARM64 || COMPILE_TEST
>  	help

[ ... ]

> diff --git a/drivers/perf/arm_cspmu/arm-bmu.c b/drivers/perf/arm_cspmu/arm-bmu.c
> --- a/drivers/perf/arm_cspmu/arm-bmu.c
> +++ b/drivers/perf/arm_cspmu/arm-bmu.c

[ ... ]

> @@ -406,7 +407,10 @@ static int arm_bmu_probe(struct platform_device *pdev)
>  		.read = arm_bmu_event_read,
>  	};
>  
> -	name = acpi_device_uid(ACPI_COMPANION(dev));
> +	if (has_acpi_companion(dev))
> +		name = acpi_device_uid(ACPI_COMPANION(dev));

[Severity: Low]
Does this cause a build failure in arm_bmu_probe() when CONFIG_ACPI is
disabled?

Since the strict ACPI dependency was removed in Kconfig, building for standard
ARM64 without ACPI is now possible. It appears acpi_device_uid() might not be
stubbed out or available when CONFIG_ACPI is turned off, leading to an implicit
function declaration error during compilation.

> +	else
> +		device_property_read_string(dev, "label", &name);
>  
>  	if (name)

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

Re: [PATCH v2 1/3] perf: Add Arm Bus Monitor Unit driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] Unconditional use of `ACPI_COMPANION()` and `acpi_device_uid()` causes a NULL pointer dereference or build failure.
- [High] Missing `continue` when Non-Secure access is disabled leads to writes to inaccessible PMU registers.
- [High] ABA vulnerability and NMI torn state in lockless `prev_count` update causes massive PMU count corruption.
- [High] Missing bounds check on the computed PMU region offset against the actual ioremapped memory resource size.
- [High] Missing cleanup of hardware interrupt state on probe error paths causes an unhandled interrupt storm.
- [Medium] TOCTOU race in PMU CPU assignment during probe can bind the PMU to an offline CPU, leaving it non-functional.
- [Medium] Ignoring `-EPROBE_DEFER` for the interrupt permanently breaks IRQ functionality.
- [Medium] False-positive `WARN_ON` triggered by a race between event deletion and pending PMU interrupts.
- [Low] Format string type mismatch passing a 32-bit `resource_size_t` / `phys_addr_t` to a 64-bit `%llx` specifier.
--

--- Patch [1]: [PATCH v2 1/3] perf: Add Arm Bus Monitor Unit driver ---
commit 46bae7b04d539138ea677d6d9ef5697dcb457c20
Author: Robin Murphy <robin.murphy@arm.com>

perf: Add Arm Bus Monitor Unit driver

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.

> diff --git a/drivers/perf/arm_cspmu/arm-bmu.c b/drivers/perf/arm_cspmu/arm-bmu.c
> new file mode 100644
> index 0000000000000..c829a0728e9b2
> --- /dev/null
> +++ b/drivers/perf/arm_cspmu/arm-bmu.c
[ ... ]
> +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);

[Severity: Medium]
Should the hardware overflow state (PMOVSCLR) be cleared here when stopping
the event?

If an overflow occurs right before stopping, the pending interrupt bit
remains set. When arm_bmu_event_del() completes, it clears the event pointer.
When the interrupt is finally delivered, wouldn't the interrupt handler see
the overflow bit set and incorrectly trigger the WARN_ON for a NULL event
pointer?

> +	if (flags & PERF_EF_UPDATE)
> +		arm_bmu_event_read(event);
> +}
[ ... ]
> +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);

[Severity: High]
Is the lockless compare-and-exchange logic in arm_bmu_event_read() vulnerable
to the ABA problem when resetting prev_count here?

If an NMI interrupts this IRQ handler right after prev_count is set to S64_MIN
but before the hardware register is written, arm_bmu_event_read() could read
the old wrapped hardware value and falsely succeed in the cmpxchg because
prev_count is S64_MIN.

This would save the wrapped value as prev_count. When the IRQ handler resumes
and resets the hardware to S64_MIN, wouldn't subsequent delta calculations
underflow and cause massive permanent counter corruption?

> +	}
> +	writel_relaxed(reg, pmu->base + PMOVSCLR);
> +}
[ ... ]
> +static int arm_bmu_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	/* 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)))

[Severity: High]
Does this compute the PMU region offset without verifying it fits within the
originally mapped memory resource size?

If the hardware provides an offset such that the new region exceeds
resource_size(res), applying this unvalidated offset to the virtual base pointer
and dereferencing it later will access memory outside the mapped virtual
memory area, potentially causing a kernel page fault or panic.

> +		return dev_err_probe(dev, -EADDRINUSE, "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 {

[Severity: High]
Should there be a continue statement when Non-Secure access is disabled?

Without a continue, the code proceeds to unconditionally write to BMU_PMCR and
other registers for this PMU below. Writing to inaccessible secure hardware
from EL1 typically triggers a Synchronous External Abort (SError), resulting
in an immediate kernel panic.

> +			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);
> +	}

[Severity: High]
Are hardware interrupts left enabled if an error occurs later in the probe
function?

If cpuhp_state_add_instance_nocalls() or perf_pmu_register() fails, the error
path exits without clearing PMINTENCLR. If the hardware asserts the IRQ line,
the kernel would receive an interrupt without a registered handler, leading to
an unhandled interrupt storm.

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

[Severity: Medium]
Is there a race in assigning the target CPU without holding the CPU hotplug
read lock?

If the selected CPU goes offline before the hotplug registration, the
arm_bmu_offline_cpu callback will not be invoked retroactively because
cpuhp_state_add_instance_nocalls() is used below. Could this leave the PMU
bound to an offline CPU, rendering it non-functional?

> +	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 = err;
> +		else
> +			irq_set_affinity(bmu->irq, cpumask_of(bmu->cpu));
> +	}
> +	if (bmu->irq < 0)
> +		dev_info(dev, "Continuing without IRQ\n");

[Severity: Medium]
Does this completely ignore -EPROBE_DEFER from platform_get_irq()?

If platform_get_irq() returns -EPROBE_DEFER, the probe simply succeeds and
falls back to polling mode. This permanently breaks deferred probing for the
interrupt, preventing the driver from retrying when the interrupt controller
is ready.

> +
> +	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,
> +	};
> +
> +	name = acpi_device_uid(ACPI_COMPANION(dev));

[Severity: High]
Will this cause a NULL pointer dereference if the device has no ACPI companion
or if CONFIG_ACPI is disabled?

ACPI_COMPANION() returns NULL in these cases, and acpi_device_uid() directly
dereferences it. This would lead to a kernel panic during driver probe on
such configurations.

> +
> +	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]
Is there a format string type mismatch here?

When CONFIG_PHYS_ADDR_T_64BIT is not enabled, resource_size_t is 32-bit.
Passing a 32-bit integer to a %llx specifier causes undefined behavior,
which can read garbage from the stack for the upper 32 bits.

> +	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;
> +}

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