Re: [PATCH] drivers/perf: hisi: update the sccl_id/ccl_id for certain HiSilicon platform

From: Will Deacon <will@kernel.org>
To: Shaokun Zhang <zhangshaokun@hisilicon.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	John Garry <john.garry@huawei.com>,
	linux-arm-kernel@lists.infradead.org,
	Hanjun Guo <guohanjun@huawei.com>
Subject: Re: [PATCH] drivers/perf: hisi: update the sccl_id/ccl_id for certain HiSilicon platform
Date: Thu, 7 Nov 2019 11:40:42 +0000	[thread overview]
Message-ID: <20191107114041.GB11587@willie-the-truck> (raw)
In-Reply-To: <1573113364-32531-1-git-send-email-zhangshaokun@hisilicon.com>

Hi,

On Thu, Nov 07, 2019 at 03:56:04PM +0800, Shaokun Zhang wrote:
> @@ -338,8 +339,10 @@ void hisi_uncore_pmu_disable(struct pmu *pmu)
>  
>  /*
>   * Read Super CPU cluster and CPU cluster ID from MPIDR_EL1.
> - * If multi-threading is supported, CCL_ID is the low 3-bits in MPIDR[Aff2]
> - * and SCCL_ID is the upper 5-bits of Aff2 field; if not, SCCL_ID
> + * If multi-threading is supported, On Huawei Kunpeng 920 SoC whose cpu
> + * core is tsv110, CCL_ID is the low 3-bits in MPIDR[Aff2] and SCCL_ID
> + * is the upper 5-bits of Aff2 field; while for other cpu types, SCCL_ID
> + * is in MPIDR[Aff3] and CCL_ID is in MPIDR[Aff2], if not, SCCL_ID
>   * is in MPIDR[Aff2] and CCL_ID is in MPIDR[Aff1].
>   */
>  static void hisi_read_sccl_and_ccl_id(int *sccl_id, int *ccl_id)
> @@ -347,12 +350,19 @@ static void hisi_read_sccl_and_ccl_id(int *sccl_id, int *ccl_id)
>  	u64 mpidr = read_cpuid_mpidr();
>  
>  	if (mpidr & MPIDR_MT_BITMASK) {
> -		int aff2 = MPIDR_AFFINITY_LEVEL(mpidr, 2);
> -
> -		if (sccl_id)
> -			*sccl_id = aff2 >> 3;
> -		if (ccl_id)
> -			*ccl_id = aff2 & 0x7;
> +		if (read_cpuid_part_number() == HISI_CPU_PART_TSV110) {
> +			int aff2 = MPIDR_AFFINITY_LEVEL(mpidr, 2);
> +
> +			if (sccl_id)
> +				*sccl_id = aff2 >> 3;
> +			if (ccl_id)
> +				*ccl_id = aff2 & 0x7;
> +		} else {
> +			if (sccl_id)
> +				*sccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 3);
> +			if (ccl_id)
> +				*ccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 2);
> +		}

[I prefer Mark's version, so please reply to indicate whether or not it
 works for you]

So I'll take this, but the lesson here seems to be that it's a terrible idea
to infer system topology from CPU ID registers. In future, I'm going to
insist that this comes from firmware tables because hacks like the above are
not sustainable.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel