Re: [PATCH v3 5/9] drivers/perf: hisi: Extend the field of tt_core

[Will Deacon <will@kernel.org>]

On Fri, Aug 29, 2025 at 06:14:23PM +0800, Yushan Wang wrote:
> From: Yicong Yang <yangyicong@hisilicon.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@huawei.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> Signed-off-by: Yushan Wang <wangyushan12@huawei.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