linux-kernel - Re: [PATCH v3 5/9] drivers/perf: hisi: Extend the field of tt

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aNFMUdPJeJo9XU1e@willie-the-truck>
Date: Mon, 22 Sep 2025 14:17:05 +0100
From: Will Deacon <will@...nel.org>
To: Yushan Wang <wangyushan12@...wei.com>
Cc: mark.rutland@....com, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, robin.murphy@....com,
	yangyicong@...wei.com, Jonathan.Cameron@...wei.com,
	liuyonglong@...wei.com, wanghuiqiang@...wei.com,
	prime.zeng@...ilicon.com, hejunhao3@...artners.com,
	linuxarm@...wei.com, fanghao11@...wei.com
Subject: Re: [PATCH v3 5/9] drivers/perf: hisi: Extend the field of tt_core

On Fri, Aug 29, 2025 at 06:14:23PM +0800, Yushan Wang wrote:
> From: Yicong Yang <yangyicong@...ilicon.com>
> 
> Currently the tt_core's using config1's bit [7, 0] and can not be
> extended. For some platforms there's more the 8 CPUs sharing the
> L3 cache. So make tt_core use config2's bit [15, 0] and the remaining
> bits in config2 is reserved for extension.
> 
> Acked-by: Jonathan Cameron <jonathan.cameron@...wei.com>
> Signed-off-by: Yicong Yang <yangyicong@...ilicon.com>
> Signed-off-by: Yushan Wang <wangyushan12@...wei.com>
> ---
>  drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> index a372dd2c07b5..39444f11cbad 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> @@ -55,10 +55,10 @@
>  #define L3C_V1_NR_EVENTS	0x59
>  #define L3C_V2_NR_EVENTS	0xFF
>  
> -HISI_PMU_EVENT_ATTR_EXTRACTOR(tt_core, config1, 7, 0);
>  HISI_PMU_EVENT_ATTR_EXTRACTOR(tt_req, config1, 10, 8);
>  HISI_PMU_EVENT_ATTR_EXTRACTOR(datasrc_cfg, config1, 15, 11);
>  HISI_PMU_EVENT_ATTR_EXTRACTOR(datasrc_skt, config1, 16, 16);
> +HISI_PMU_EVENT_ATTR_EXTRACTOR(tt_core, config2, 15, 0);
>  
>  static void hisi_l3c_pmu_config_req_tracetag(struct perf_event *event)
>  {
> @@ -397,7 +397,7 @@ static const struct attribute_group hisi_l3c_pmu_v1_format_group = {
>  
>  static struct attribute *hisi_l3c_pmu_v2_format_attr[] = {
>  	HISI_PMU_FORMAT_ATTR(event, "config:0-7"),
> -	HISI_PMU_FORMAT_ATTR(tt_core, "config1:0-7"),
> +	HISI_PMU_FORMAT_ATTR(tt_core, "config2:0-15"),
>  	HISI_PMU_FORMAT_ATTR(tt_req, "config1:8-10"),
>  	HISI_PMU_FORMAT_ATTR(datasrc_cfg, "config1:11-15"),
>  	HISI_PMU_FORMAT_ATTR(datasrc_skt, "config1:16"),

I'm a _tiny_ bit worried about this change in that it has the potential
to break any users who've hardcoded the 'tt_core' encoding in 'config1'.
Granted, they should be parsing this out of sysfs, but you never know.

If we were going to avoid the possibility of a regression entirely, I
think we'd either need to (a) split the field so that the upper 8 bits
of 'tt_core' live in 'config2' but the bottom 8 bits stay where they
are or (b) leave 'config1:0-7' as an alias of 'config2:0-7'.

The latter is still do-able, as you haven't re-allocated the config1
bits yet.

Will