linux-kernel - Re: [PATCH v4 1/2] drivers/perf: hisi: Add support for HiSilicon NoC PMU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250717113725.000075fb@huawei.com>
Date: Thu, 17 Jul 2025 11:37:25 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Yicong Yang <yangyicong@...wei.com>
CC: <will@...nel.org>, <mark.rutland@....com>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<hejunhao3@...wei.com>, <prime.zeng@...ilicon.com>, <linuxarm@...wei.com>,
	<yangyicong@...ilicon.com>, <wangyushan12@...wei.com>
Subject: Re: [PATCH v4 1/2] drivers/perf: hisi: Add support for HiSilicon
 NoC PMU

On Thu, 17 Jul 2025 15:41:37 +0800
Yicong Yang <yangyicong@...wei.com> wrote:

> From: Yicong Yang <yangyicong@...ilicon.com>
> 
> Adds the support for HiSilicon NoC (Network on Chip) PMU which
> will be used to monitor the events on the system bus. The PMU
> device will be named after the SCL ID (either Super CPU cluster
> or Super IO cluster) and the index ID, just similar to other
> HiSilicon Uncore PMUs. Below PMU formats are provided besides
> the event:
> 
> - ch: the transaction channel (data, request, response, etc) which
>   can be used to filter the counting.
> - tt_en: tracetag filtering enable. Just as other HiSilicon Uncore
>   PMUs the NoC PMU supports only counting the transactions with
>   tracetag.
> 
> The NoC PMU doesn't have an interrupt to indicate the overflow.
> However we have a 64 bit counter which is large enough and it's
> nearly impossible to overflow.
> 
> Signed-off-by: Yicong Yang <yangyicong@...ilicon.com>

Hi.

Just one trivial thing that I missed in internal review yesterday.
Not worth a respin though!

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

> diff --git a/drivers/perf/hisilicon/hisi_uncore_noc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_noc_pmu.c
> new file mode 100644
> index 000000000000..2e3eb7881caa
> --- /dev/null
> +++ b/drivers/perf/hisilicon/hisi_uncore_noc_pmu.c

> +
> +static struct hisi_noc_pmu_regs hisi_noc_v1_pmu_regs = {
> +	.version = NOC_PMU_VERSION,
> +	.pmu_ctrl = NOC_PMU_GLOBAL_CTRL,
> +	.event_ctrl0 = NOC_PMU_EVENT_CTRL0,
> +	.event_cntr0 = NOC_PMU_EVENT_COUNTER0,
> +	.ovflow_status = NOC_PMU_CNT_INFO,

Trivial but if you are spinning a v5 for some I'd just spell out
as .overflow_status.

I've never seen any consistency in abbreviations for overflow!

> +};
> +
> +static const struct hisi_pmu_dev_info hisi_noc_v1 = {
> +	.attr_groups = hisi_noc_pmu_attr_groups,
> +	.counter_bits = 64,
> +	.check_event = NOC_PMU_EVENT_CTRL_TYPE,
> +	.private = &hisi_noc_v1_pmu_regs,
> +};