Re: perf: Add Raspberry Pi BCM2835 AXI PMU driver

[Florian Fainelli <florian.fainelli@broadcom.com>]

On 8/13/26 00:30, Ian Rogers wrote:
> This commit adds a new performance monitoring driver for the Raspberry Pi
> AXI bus (BCM2835/2711), exposing system-level and VideoCore PMU hardware to
> the Linux perf subsystem.
> 
> Note on out-of-tree macro compatibility:
> The inclusion of #if LINUX_VERSION_CODE < KERNEL_VERSION(6, 13, 0)
> surrounding hrtimer_setup is intentionally maintained alongside
> this patch to guarantee seamless out-of-tree compilation fallback
> compatibility for Raspberry Pi Long Term Support (LTS) kernel variants.

Upstream does not care about that, I appreciate the thought and mention, 
but that's a backporting job to ensure it works.

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>   drivers/perf/rpi_axi_pmu.c | 2456 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 2456 insertions(+)
>   create mode 100644 drivers/perf/rpi_axi_pmu.c
> 
> diff --git a/drivers/perf/rpi_axi_pmu.c b/drivers/perf/rpi_axi_pmu.c
> new file mode 100644
> index 000000000000..d8efc8bb205f
> --- /dev/null
> +++ b/drivers/perf/rpi_axi_pmu.c
> @@ -0,0 +1,2456 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/**
> + * DOC: Raspberry Pi AXI Bus Performance Monitoring Unit (PMU) Driver
> + *
> + * This driver exposes the performance monitoring hardware on Raspberry Pi
> + * System-on-Chips to the Linux perf subsystem:
> + * - Raspberry Pi 1, 2, 3, 4, Compute Modules 1-4, Zero, Zero W (SoCs BCM2835/2836/2837/2711).
> + *
> + * Architecture Overview:
> + * ----------------------
> + * The Broadcom AXI performance hardware provides up to two independent monitors:
> + * 1. System Monitor (MON__SYSTEM = 0):
> + *    Monitors system-level AXI traffic (ARM CPU L2/UC, DMA, V3D, ISP, HVS, PCIe/RP1).
> + *    Directly memory-mapped via ARM physical IO memory space (MMIO).
> + *    Read latency: ~10-20 nanoseconds (fast, atomic-safe, non-blocking).
> + *
> + * 2. VPU Monitor (MON__VPU = 1):
> + *    Monitors VideoCore VPU buses (VPU0/1 Data/Instruction L2/UC, SDRAM, etc.).
> + *    Accessible through VideoCore firmware mailbox IPC (RPI_FIRMWARE_SET/GET_PERIPH_REG).
> + *    Read latency: ~10-100 microseconds (IPC over VPU mailbox).
> + *
> + * Synchronization & Concurrency Model:
> + * ------------------------------------
> + * - Spinlock (pmu->lock):
> + *   Protects active event array (events[]), event generation sequence counters (event_gen[]),
> + *   bus watcher allocation/refcounting, active_vpu_events counter, and MMIO register updates
> + *   (MON__SYSTEM) against SMP race conditions and ABA pointer recycling races.
> + *
> + * - Mutex (pmu->vpu_mutex):
> + *   Serializes VideoCore Mailbox IPC transactions (MON__VPU) in process context,
> + *   preventing concurrent mailbox buffer corruption across multiple CPUs.
> + *
> + * - Cached Async VPU Reads & Multiplexing (MON__VPU):
> + *   Polled periodically in process context by vpu_work when active_vpu_events > 0.
> + *   Uses PERF_HES_UPTODATE state flag to safely establish counter baselines during
> + *   event rotation / multiplexing. User read() syscalls return cached cumulative event counter
> + *   instantly without blocking.
> + */

The patch description needs to go into details as to why both the MMIO 
and firmware interfaces are supported. The firmware interface requires 
you to play games with sleeping/non-sleeping context, I would really 
want to avoid that and just support the MMIO path exclusively, is that 
practical? If that means ditching support for Pi 1&2, that would be 
reasonable IMHO.

> +
> +#include <linux/cpuhotplug.h>
> +#include <linux/cpumask.h>
> +#include <linux/hrtimer.h>
> +#include <linux/io.h>
> +#include <linux/version.h>
> +
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(6, 13, 0)
> +static inline void rpi_hrtimer_setup(struct hrtimer *timer,
> +				     enum hrtimer_restart (*function)(struct hrtimer *),
> +				     clockid_t clock_id, enum hrtimer_mode mode)
> +{
> +	hrtimer_init(timer, clock_id, mode);
> +	timer->function = function;
> +}
> +#define hrtimer_setup rpi_hrtimer_setup
 > +#endif> +#include <linux/lockdep.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/perf_event.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <linux/sysfs.h>
> +#include <linux/vmalloc.h>
> +#include <linux/workqueue.h>
> +
> +#include <soc/bcm2835/raspberrypi-firmware.h>
> +
> +/* --- PLATFORM CONSTANTS & ENUMERATIONS --------------------------- */
> +
> +/**
> + * enum rpi_axi_chip - Supported Broadcom SoC generations
> + * @CHIP_BCM2835: BCM2835 / BCM2836 / BCM2837 / BCM2711 (RPi 1-4, CM 1-4, Zero/W)
> + */
> +enum rpi_axi_chip {
> +	CHIP_BCM2835 = 0,
> +};
> +
> +enum monitor {
> +	MON__SYSTEM = 0,
> +	MON__VPU,
> +	MON__MAX
> +};
> +
> +/* Number of hardware bus watcher units per monitor */
> +#define NUM_BUS_WATCHERS_PER_MONITOR 3
> +
> +/**
> + * enum bcm2835_system_bus - AXI buses monitored by System Monitor on BCM2835-BCM2711 (RPi 1-4)
> + * @BCM2835_SB__DMA_L2: DMA engine L2 cache interconnect bus

Why the double underscore in the naming convention?

[snip]

> +
> +/* Hardware register offsets & control bitwise constants */
> +#define GEN_CTRL			0x00
> +#define GEN_CTL_ENABLE_BIT		BIT(0)
> +#define GEN_CTL_RESET_BIT		BIT(1)
> +#define GEN_CTL_WATCH_BIT		BIT(2)
> +
> +#define BW_PITCH			0x40

Rather than pitch, maybe stride?

> +#define BW0_CTRL			0x40
> +#define BW1_CTRL			0x80
> +#define BW2_CTRL			0xc0
> +
> +#define BW_ATRANS_OFFSET		0x04

That's confusing, so you are offsetting from the base of BW0_CTRL, that 
needs a comment to explain that offset choice.

> +#define BW_ATWAIT_OFFSET		0x08
> +#define BW_AMAX_OFFSET			0x0c
> +#define BW_WTRANS_OFFSET		0x10
> +#define BW_WTWAIT_OFFSET		0x14
> +#define BW_WMAX_OFFSET			0x18
> +#define BW_RTRANS_OFFSET		0x1c
> +#define BW_RTWAIT_OFFSET		0x20
> +#define BW_RMAX_OFFSET			0x24
> +
> +#define BW_CTRL_RESET_BIT		BIT(31)
> +#define BW_CTRL_ENABLE_BIT		BIT(30)
> +#define BW_CTRL_ENABLE_ID_FILTER_BIT	BIT(29)
> +#define BW_CTRL_LIMIT_HALT_BIT		BIT(28)
> +
> +#define BW_CTRL_BUS_WATCH_SHIFT		0
> +#define BW_CTRL_BUS_WATCH_MASK		GENMASK(5, 0)
> +#define BW_CTRL_BUS_FILTER_SHIFT	8
> +#define BW_CTRL_BUS_FILTER_MASK		GENMASK(12, 8)
> +
> +/*
> + * RPI_AXI_PMU_TIMER_INTERVAL determines the background polling frequency
> + * for VideoCore VPU Mailbox IPC counters.
> + *
> + * A balance is required:
> + * - IPC Overhead: Polling overly fast (e.g., 10ms) generates excessive CPU
> + *   wakeups and VideoCore IPC interrupts on older CPUs (Pi 1/2).
> + * - Accuracy: Polling overly slow (e.g., 2000ms) causes short time-multiplexed
> + *   profiling sessions (under the interval) to mathematically strand residual
> + *   counts since the mailbox cannot be queried synchronously inside pmu->read().
> + *
> + * 100ms (10 Hz) provides reasonably accurate profiling without heavy overhead.
> + */
> +#define RPI_AXI_PMU_TIMER_INTERVAL ms_to_ktime(100)
> +
> +static enum cpuhp_state rpi_axi_pmu_cpuhp_state;
> +
> +/* --- PMU API & CONFIG DECODING ---------------------------------- */
> +
> +#define PMU_NAME "rpi_axi_pmu"
> +
> +/**
> + * config_to_filter() - Extracts AXI filter ID from perf event config
> + * @config: 64-bit config value from struct perf_event_attr
> + *
> + * Return: Filter ID value (bits 10-14).
> + */
> +static int config_to_filter(__u64 config)
> +{
> +	return (config >> 10) & 0x1F;

Can we have a definition for the shift and mask here?

> +}
> +
> +/**
> + * config_to_monitor() - Extracts Monitor ID from perf event config
> + * @config: 64-bit config value from struct perf_event_attr
> + *
> + * Return: Monitor enum (bit 9: 0 = System, 1 = VPU).
> + */
> +static enum monitor config_to_monitor(__u64 config)
> +{
> +	return (config >> 9) & 1;

Likewise.

> +}
> +
> +/**
> + * config_to_bus() - Extracts bus index from perf event config
> + * @config: 64-bit config value from struct perf_event_attr
> + *
> + * Return: Bus index (bits 4-8).
> + */
> +static int config_to_bus(__u64 config)
> +{
> +	return (config >> 4) & 0x1F;

Likewise

> +}
> +
> +/**
> + * config_to_counter() - Extracts metric counter type from perf event config
> + * @config: 64-bit config value from struct perf_event_attr
> + *
> + * Return: Counter enum (bits 0-3).
> + */
> +static enum counter config_to_counter(__u64 config)
> +{
> +	return config & 0xF;

And here as well.

[snip]

> +	for (int i = 0; i < MON__MAX; i++) {
> +		rpi_axi_hw_events__init(&pmu->monitor[i].hw_events);
> +
> +		if (pmu->monitor[i].use_mailbox_interface) {
> +			struct resource *resource = platform_get_resource(pdev, IORESOURCE_MEM, i);
> +
> +			if (!resource) {
> +				dev_err(dev, "Error reading mailbox resource %d\n", i);
> +				ret = -EINVAL;
> +				goto err_firmware_put;
> +			}
> +			pmu->monitor[i].mailbox = (u32)resource->start;
> +		} else {
> +			struct resource *resource = platform_get_resource(pdev, IORESOURCE_MEM, i);

Well you are fetching MMIO resources here, so you need a Device Tree 
description and you need to submit the Device Tree changes that describe 
these register ranges. Is it fair to assume only the RPi firmware path 
has been tested or did you also test with MMIO?

[snip]
> +
> +MODULE_LICENSE("GPL");
Missing MODULE_AUTHOR() and MODULE_DESCRIPTION().
-- 
Florian