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

[Uwe Kleine-König]

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?

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

Attachment: signature.asc
Description: PGP signature