Re: [PATCH v3 1/2] drivers/perf: hisi: Support uncore ITS PMU

[Yushan Wang]

On 7/14/2026 8:56 PM, Robin Murphy wrote:
> On 14/07/2026 1:32 pm, Uwe Kleine-König wrote:
>> Hello,
>>
>> On Mon, Jul 13, 2026 at 08:56:46PM +0800, Yushan Wang wrote:
>>> Support uncore ITS PMU, which provides the capability of counting
>>> the number of interrupts routed to ITS by interrupt catagories, and the
>>> latency. It also supports collecting statistics of micro-ops of ITS.
>>>
>>> The driver adapts to HiSilicon uncore PMU framework. It does not support
>>> overflow interruption, which is the same as NoC PMU, so a few dummy
>>> functions or handling interrupts are left empty.
>>>
>>> Signed-off-by: Yushan Wang <wangyushan12@xxxxxxxxxx>
>>> ---
>>>   Documentation/admin-guide/perf/hisi-pmu.rst  |  13 +
>>>   drivers/perf/hisilicon/Makefile              |   2 +-
>>>   drivers/perf/hisilicon/hisi_uncore_its_pmu.c | 393 +++++++++++++++++++
>>>   3 files changed, 407 insertions(+), 1 deletion(-)
>>>   create mode 100644 drivers/perf/hisilicon/hisi_uncore_its_pmu.c
>>>
>>> diff --git a/Documentation/admin-guide/perf/hisi-pmu.rst b/Documentation/admin-guide/perf/hisi-pmu.rst
>>> index d56b2d690709..278bd7e0ae60 100644
>>> --- a/Documentation/admin-guide/perf/hisi-pmu.rst
>>> +++ b/Documentation/admin-guide/perf/hisi-pmu.rst
>>> @@ -128,6 +128,19 @@ channel with this option. The current supported channels are as follows:
>>>   7. tt_en: NoC PMU supports counting only transactions that have tracetag set
>>>   if this option is set. See the 2nd list for more information about tracetag.
>>>   +8. int_id: ITS PMU supports filtering by interrupt id, which is defined by
>>> +hardware. Interrupt id takes up to 32 bits, and can be divided into 2 parts:
>>> +
>>> +- Upper 16 bits: DeviceID if counting LPI, PEID if counting SGI/PPI.
>>> +- Lower 16 bits: EventID if counting LPI, IntID if counting SGI/PPI.
>>> +
>>> +int_id is a global configuration for each PMU instance. If multiple different
>>> +int_id's are specified, the last came in will be effective. And if there are
>>> +already filtered events running, new filtered events came in will be refused.
>>> +
>>> +9. int_en: A one-bit flag to tell if int_id is used to filter the statistics. It
>>> +allows filtering 0 DeviceID and EventID.
>>> +
>>>   For HiSilicon uncore PMU v3 whose identifier is 0x40, some uncore PMUs are
>>>   further divided into parts for finer granularity of tracing, each part has its
>>>   own dedicated PMU, and all such PMUs together cover the monitoring job of events
>>> diff --git a/drivers/perf/hisilicon/Makefile b/drivers/perf/hisilicon/Makefile
>>> index 186be3d02238..5f28cfdb8a72 100644
>>> --- a/drivers/perf/hisilicon/Makefile
>>> +++ b/drivers/perf/hisilicon/Makefile
>>> @@ -2,7 +2,7 @@
>>>   obj-$(CONFIG_HISI_PMU) += hisi_uncore_pmu.o hisi_uncore_l3c_pmu.o \
>>>                 hisi_uncore_hha_pmu.o hisi_uncore_ddrc_pmu.o hisi_uncore_sllc_pmu.o \
>>>                 hisi_uncore_pa_pmu.o hisi_uncore_cpa_pmu.o hisi_uncore_uc_pmu.o \
>>> -              hisi_uncore_noc_pmu.o hisi_uncore_mn_pmu.o
>>> +              hisi_uncore_noc_pmu.o hisi_uncore_mn_pmu.o hisi_uncore_its_pmu.o
>>>     obj-$(CONFIG_HISI_PCIE_PMU) += hisi_pcie_pmu.o
>>>   obj-$(CONFIG_HNS3_PMU) += hns3_pmu.o
>>> diff --git a/drivers/perf/hisilicon/hisi_uncore_its_pmu.c b/drivers/perf/hisilicon/hisi_uncore_its_pmu.c
>>> new file mode 100644
>>> index 000000000000..430e2b06cf4d
>>> --- /dev/null
>>> +++ b/drivers/perf/hisilicon/hisi_uncore_its_pmu.c
>>> @@ -0,0 +1,393 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Driver for HiSilicon Uncore ITS PMU device
>>> + *
>>> + * Copyright (c) 2026 HiSilicon Technologies Co., Ltd.
>>> + * Author: Yushan Wang <wangyushan12@xxxxxxxxxx>
>>> + */
>>> +#include <linux/bitops.h>
>>> +#include <linux/cpuhotplug.h>
>>> +#include <linux/device.h>
>>> +#include <linux/io.h>
>>> +#include <linux/mod_devicetable.h>
>>
>> Please rely on linux/platform_device.h to provice acpi_device_id and
>> drop the include for <linux/mod_devicetable.h>.
>>
>>> +static int __init hisi_its_pmu_module_init(void)
>>> +{
>>> +    int ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
>>> +                      "perf/hisi/its:online",
>>> +                      hisi_uncore_pmu_online_cpu,
>>> +                      hisi_uncore_pmu_offline_cpu);
>>
>> <disclaimer>I don't know anything about the functions involved
>> here</disclaimer>
>>
>> This looks fishy. The module might be loaded on any machine, is it right
>> to call hisilicon specific functions then?
>
> This only registers the state with the hotplug mechanism - the callbacks themselves won't ever be invoked until the driver binds to something and calls cpuhp_state_add_instance() from probe.

Thanks for the explanation.

>
> It is true that the dynamic hotplug states are somewhat limited, so at worst it might consume one unnecessarily if the driver is built-in, but this is the status quo for system PMU drivers, and TBH there are enough *other* issues with this whole perf hotplug mess anyway that I wouldn't see this one as a major concern. (FWIW I have started another side-project to make it all go away, which is beginning to look promising...)
>
> That said, in this particular case, HISI_PMU could seemingly do better than registering *multiple* states with the same exact callbacks - registering a single state from the common hisi_uncore_pmu.c framework, then just having all the subdrivers add their instances to that, would be something of an improvement for now.

It's true that in current implementations, the hotplug callbacks are exactly the
same. hisi_uncore_pmu.c mainly serves to provide public functions for other
actual PMU drivers, I can move this dynamic-state planting code into that module
and export the state to other drivers.

There are also some historic static states for HiSilicon PMUs that could migrate
to dynamic ones for better consistency. I will try to do that refactor in the
next version.

Thanks for the suggestions!
Yushan

>
> Thanks,
> Robin.
>
>>
>>> +    if (ret < 0) {
>>> +        pr_err("hisi_its_pmu: Fail to setup cpuhp callbacks, ret = %d\n", ret);
>>> +        return ret;
>>> +    }
>>> +    hisi_its_pmu_cpuhp_state = ret;
>>> +
>>> +    ret = platform_driver_register(&hisi_its_pmu_driver);
>>> +    if (ret)
>>> +        cpuhp_remove_multi_state(hisi_its_pmu_cpuhp_state);
>>> +
>>> +    return ret;
>>> +}
>>> +module_init(hisi_its_pmu_module_init);
>>
>> Best regards
>> Uwe
>
>