Re: [PATCH v7 1/2] perf: Fujitsu: Add the Uncore MAC PMU driver

Jonathan Cameron
2025-08-15T14:38:01Z

On Fri, 15 Aug 2025 12:47:28 +0900
Koichi Okuno <fj2767dz@fujitsu.com> wrote:

> This adds a new dynamic PMU to the Perf Events framework to program and
> control the Uncore MAC PMUs in Fujitsu chips.
>
> This driver was created with reference to drivers/perf/qcom_l3_pmu.c.
>
> This driver exports formatting and event information to sysfs so it can
> be used by the perf user space tools with the syntaxes:
>
> perf stat -e mac_iod0_mac0_ch0/ea-mac/ ls
> perf stat -e mac_iod0_mac0_ch0/event=0x80/ ls
>
> FUJITSU-MONAKA PMU Events Specification v1.1 URL:
> https://github.com/fujitsu/FUJITSU-MONAKA
>
> Signed-off-by: Koichi Okuno <fj2767dz@fujitsu.com>

Hi, just one slight doubt inline around interrupt affinities.
I can't quite remember what breaks when those don't move with the
perf state and so can't immediately see if relevant here.

Anyhow, if that is resolved (either by you saying it's definitely
not an issue) or adding the irq_set_affinity() calls, then

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Jonathan

> diff --git a/drivers/perf/fujitsu_mac_pmu.c b/drivers/perf/fujitsu_mac_pmu.c
> new file mode 100644
> index 000000000000..1031e0221bb2
> --- /dev/null
> +++ b/drivers/perf/fujitsu_mac_pmu.c
> @@ -0,0 +1,552 @@

> +#define MAC_EVENT_CYCLES 0x000
Maybe drop the lading 0 on all these values? 0x00
etc as we don't go beyond 0xa0

> +#define MAC_EVENT_READ_COUNT 0x010
> +#define MAC_EVENT_READ_COUNT_REQUEST 0x011
> +#define MAC_EVENT_READ_COUNT_RETURN 0x012
> +#define MAC_EVENT_READ_COUNT_REQUEST_PFTGT 0x013
> +#define MAC_EVENT_READ_COUNT_REQUEST_NORMAL 0x014
> +#define MAC_EVENT_READ_COUNT_RETURN_PFTGT_HIT 0x015
> +#define MAC_EVENT_READ_COUNT_RETURN_PFTGT_MISS 0x016
> +#define MAC_EVENT_READ_WAIT 0x017
> +#define MAC_EVENT_WRITE_COUNT 0x020
> +#define MAC_EVENT_WRITE_COUNT_WRITE 0x021
> +#define MAC_EVENT_WRITE_COUNT_PWRITE 0x022
> +#define MAC_EVENT_MEMORY_READ_COUNT 0x040
> +#define MAC_EVENT_MEMORY_WRITE_COUNT 0x050
> +#define MAC_EVENT_MEMORY_PWRITE_COUNT 0x060
> +#define MAC_EVENT_EA_MAC 0x080
> +#define MAC_EVENT_EA_MEMORY 0x090
> +#define MAC_EVENT_EA_MEMORY_MAC_WRITE 0x092
> +#define MAC_EVENT_EA_HA 0x0a0
> +
> +struct mac_pmu {
> + struct pmu pmu;
> + struct hlist_node node;
> + void __iomem *regs;
> + struct perf_event *events[MAC_NUM_COUNTERS];
> + unsigned long used_mask[BITS_TO_LONGS(MAC_NUM_COUNTERS)];
> + cpumask_t cpumask;
> +};

> +static void fujitsu_mac_counter_update(struct perf_event *event)
> +{
> + struct mac_pmu *macpmu = to_mac_pmu(event->pmu);
> + int idx = event->hw.idx;
> + u64 prev, new;
> +
> + do {
> + prev = local64_read(&event->hw.prev_count);
> + new = readq_relaxed(macpmu->regs + MAC_PM_EVCNTR(idx));
> + } while (local64_cmpxchg(&event->hw.prev_count, prev, new) != prev);
> +
> + local64_add(new - prev, &event->count);
> +}

> +static int fujitsu_mac_pmu_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct acpi_device *acpi_dev;
> + struct mac_pmu *macpmu;
> + struct resource *memrc;
> + char *name;
> + int ret;
> + u64 uid;
> +
> + acpi_dev = ACPI_COMPANION(dev);
> + if (!acpi_dev)
> + return -ENODEV;
> +
> + ret = acpi_dev_uid_to_integer(acpi_dev, &uid);
> + if (ret)
> + return dev_err_probe(dev, ret, "unable to read ACPI uid\n");
> +
> + macpmu = devm_kzalloc(dev, sizeof(*macpmu), GFP_KERNEL);
> + if (!macpmu)
> + return -ENOMEM;
> +
> + name = devm_kasprintf(dev, GFP_KERNEL, "mac_iod%llu_mac%llu_ch%llu",
> + (uid >> 8) & 0xF, (uid >> 4) & 0xF, uid & 0xF);
> + if (!name)
> + return -ENOMEM;
> +
> + macpmu->pmu = (struct pmu) {
> + .parent = dev,
> + .task_ctx_nr = perf_invalid_context,
> +
> + .pmu_enable = fujitsu_mac__pmu_enable,
> + .pmu_disable = fujitsu_mac__pmu_disable,
> + .event_init = fujitsu_mac__event_init,
> + .add = fujitsu_mac__event_add,
> + .del = fujitsu_mac__event_del,
> + .start = fujitsu_mac__event_start,
> + .stop = fujitsu_mac__event_stop,
> + .read = fujitsu_mac__event_read,
> +
> + .attr_groups = fujitsu_mac_pmu_attr_grps,
> + .capabilities = PERF_PMU_CAP_NO_EXCLUDE | PERF_PMU_CAP_NO_INTERRUPT,
> + };
> +
> + macpmu->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &memrc);
> + if (IS_ERR(macpmu->regs))
> + return PTR_ERR(macpmu->regs);
> +
> + fujitsu_mac__init(macpmu);
> +
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0)
> + return ret;
> +
> + ret = devm_request_irq(dev, ret, fujitsu_mac__handle_irq, 0,
> + name, macpmu);

It's been a while since I wrote a perf driver, but I'm a bit surprised to not
see irq affinity management in here. I remember running into issues with
per cpu variables deep in perf when we didn't ensure the perf state and
the irq remain on the same cpu core.

There might be something different here though.

> + if (ret)
> + return dev_err_probe(dev, ret, "Request for IRQ failed for slice @%pa\n",
> + &memrc->start);
> +
> + /* Add this instance to the list used by the offline callback */
> + ret = cpuhp_state_add_instance(mac_pmu_cpuhp_state, &macpmu->node);
> + if (ret)
> + return dev_err_probe(dev, ret, "Error registering hotplug");
> +
> + ret = perf_pmu_register(&macpmu->pmu, name, -1);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to register MAC PMU\n");
> +
> + dev_dbg(dev, "Registered %s, type: %d\n", name, macpmu->pmu.type);
> +
> + return 0;
> +}