Re: [PATCH v8 2/2] perf: Add Raspberry Pi 5 (BCM2712) AXI PMU support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `.scale` and `.unit` sysfs attributes for legacy VPU events are incorrectly left visible on BCM2712, creating orphaned attributes.
--

--- Patch [2]: [PATCH v8 2/2] perf: Add Raspberry Pi 5 (BCM2712) AXI PMU support ---
commit 9b809ab6d889410949331370e01a80aece4e483f
Author: Ian Rogers <[email protected]>

perf: Add Raspberry Pi 5 (BCM2712) AXI PMU support

This patch introduces AXI PMU support for the Broadcom BCM2712 SoC used in the
Raspberry Pi 5. It expands the driver with BCM2712-specific buses, hardware
filters, and sysfs events, while ensuring compatibility with prior generations.

> diff --git a/drivers/perf/rpi_axi_pmu.c b/drivers/perf/rpi_axi_pmu.c
> index 676c2a3d5962e..25563fa789ca4 100644
> --- a/drivers/perf/rpi_axi_pmu.c
> +++ b/drivers/perf/rpi_axi_pmu.c

[ ... ]

> @@ -1491,12 +1871,62 @@ static struct attribute *rpi_axi_pmu_events_attrs[] = {
>   * @unused: Index (unused)
>   *
>   * Dynamically filters sysfs event aliases based on the detected Broadcom SoC generation:
> + * - On BCM2712 (RPi 5): Hides VideoCore VPU Mailbox IPC events (monitor=1).
> + * - On BCM2835-BCM2711 (RPi 1-4): Hides Cortex-A76 and RP1 PCIe specific events.
>   *
>   * Return: attr->mode (0444) if visible on current SoC, 0 to hide.
>   */
>  static umode_t rpi_axi_pmu_events_is_visible(struct kobject *kobj,
>  					      struct attribute *attr, int unused)
>  {
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct pmu *pmu = dev_get_drvdata(dev);
> +	struct rpi_axi_pmu *rpi_pmu = pmu_to_rpi_axi_pmu(pmu);
> +	struct perf_pmu_events_attr *pmu_attr;
> +
> +	pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr.attr);
> +
> +	if (rpi_pmu->chip == CHIP_BCM2712) {
> +		/*
> +		 * On RPi 5 (BCM2712), hide legacy VPU Mailbox IPC events (monitor=1)
> +		 * and legacy RPi 1-4 System Monitor aliases
> +		 */
> +		if (strstr(pmu_attr->event_str, "monitor=1") ||

[Severity: Medium]
Will this correctly hide the .scale and .unit sysfs attributes for legacy VPU
events on BCM2712?

Since the event_str for those attributes is strictly "32" or "Bytes", the
check for "monitor=1" will fail. This could leave orphaned .scale and .unit
attributes visible in sysfs for events that are otherwise correctly hidden.

Could this be addressed by adding a fallback check on attr->name (for example,
checking for "vpu") to ensure the scale and unit attributes are also caught?

> +		    strstr(attr->name, "cpu0_") ||
> +		    strstr(attr->name, "cpu1_") ||
> +		    strstr(attr->name, "dma0_") ||

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2