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

[Yushan Wang]

On 7/14/2026 8: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>.

This is a redundant header file, thanks for pointing out.

linux/platform_device.h is already imported below, I will drop this header
file in the next version.

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

As discussion with Robin, this is correct but not necessary.

I will propose a refactor in the next version to eliminate redundant
cpuhp_setup_state_multi for HISI PMUs.

But to fully achieve a better form of cpuhp callback registration, we can be
counting on Robin's work! :)

Thanks,
Yushan

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