Received-SPF: pass (google.com: domain of linux-kernel+bounces-55616-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 6 Feb 2024 13:04:27 -0800 (PST)
From: Ilkka Koskinen <ilkka@os.amperecomputing.com>
To: Robin Murphy <robin.murphy@arm.com>
cc: Ilkka Koskinen <ilkka@os.amperecomputing.com>, 
    Will Deacon <will@kernel.org>, Mark Rutland <mark.rutland@arm.com>, 
    linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf/arm-cmn: Workaround AmpereOneX errata AC04_MESH_1
 (incorrect child count)
In-Reply-To: <dd862d7a-11bd-482a-b248-60d4616fb6ef@arm.com>
Message-ID: <8bf2275a-dea7-1817-731a-7d47d3b01d13@os.amperecomputing.com>
References: <20240205194655.1567434-1-ilkka@os.amperecomputing.com> <dd862d7a-11bd-482a-b248-60d4616fb6ef@arm.com>
Content-Type: text/plain; charset=US-ASCII; format=flowed
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?t1SCVkeFDNhXwRJObiqH9tXH/4e/wpq31hEBQLXVVR/Jw3qLSbSNwLusKKD8?=
 =?us-ascii?Q?8MCvA4cTP85f2RKZxasXPOvGZyGE/k/fMj8AUPJGGKhd4jLeyThcfyD13zio?=
 =?us-ascii?Q?34U2x1i+La3z+KAZtpq0w5hge0sj5cyVHGaQDk59MO8j2Zvfjz+AIfgoZWJB?=
 =?us-ascii?Q?3KkmDCaejwln2BLFzOWDykSSPIzkle6dauXPZc+TUWO9nkJyhqjLJIB9TyR3?=
 =?us-ascii?Q?JSrKOjQFRdS7TNS5AlxXeBaz9+YUHNXW9DjfkCqnBoVC00NlOrBC1Jy2Ztok?=
 =?us-ascii?Q?zWT+S/cYkZhBXrPBqZc7xtcoUjZIgdN4dsLCJQbeyqtJO69ODW/cthysHi2J?=
 =?us-ascii?Q?PxgoDWDqU65oe1SExW716PgcRhTHDmBahn906Swkclqexk7Iy0T1XFGG0Y1K?=
 =?us-ascii?Q?k+BToXDCR9e75Tm6IpumzT+fVk4rmRypWCs5rKtEJb51YdiWrHgUiS5rVqJX?=
 =?us-ascii?Q?lhWVDRgeXFuTtzL7QpD0vs0hLQCKD2OuYlfBHd08ObvJ0z4+QLdwPlrGh/TO?=
 =?us-ascii?Q?ljZbcH66nLTCF1icFUkGfxlyGBaWUHgjOH4pk1QHDHfbmTQKyzuQH1Uulkd0?=
 =?us-ascii?Q?uASlR2za5bzT7RaEKzBazkJZ+nCGv0G8e/Nm8B8mtqAGN3VtJ74brBF7NpkX?=
 =?us-ascii?Q?IeEoRSwY9WeS6csPtiTpR5ZZew9+HC7n1Wld342R7g3Hzx4jKtKyAeKN5bc5?=
 =?us-ascii?Q?69SsR5FME+JR2spRRjUGM4vmoqHM3Ee7EIR8R74yiIrlLIknFhGaeUIsjxvi?=
 =?us-ascii?Q?1fFvrQDZBC620jvvLuRvYAHsbpTrIPPu6Mnk8zIKz74Ya80De1NTsgJyC0fH?=
 =?us-ascii?Q?9qWAinmcmPXDKSkVFdb7OpfgKmxqs78McgcoK1kx5vXdFa3CWHL2q/y8+hwh?=
 =?us-ascii?Q?KyChjOfqDsY+GOAhrfchwf6vAidfVkByAIW3J2NgH3T6ip6v2wHyNoZprVaN?=
 =?us-ascii?Q?p2IWAxz4hvgBegcafAZPTa+NgGwSLTXxwFTCb5JLDmWHJB7u33Lh05j/Bcuv?=
 =?us-ascii?Q?BTvSz3/sxJqinf/LW5E/z9TTiEKNmDtL6+uutv6IcdyZLiE2HzI1E/rDvIGK?=
 =?us-ascii?Q?toixYD5Mt9WTb5wChEGPkJkFGaz9mDbc072Ervxio0RvmRy3WTPBedIiPeAU?=
 =?us-ascii?Q?m22sbPzxldieBzC6puqWxELqi/pqCSM4ihQUbzbcsiXiTcOnklixNVAA1Ea3?=
 =?us-ascii?Q?6r+UWVQbyExljPpVNB6d8JVjcgqvLbredmLgiV5bDvkHRtvLVuOndthn35uK?=
 =?us-ascii?Q?WQJc0ps6V5KBkhdbHt8RAZGr2yoN44vQ1UInzAURuEnYaHrcp6Mp1mlKqcS/?=
 =?us-ascii?Q?2HyNMZDBxtrQMxgkWqgDIHlrOYB707V3TQPUo424w9YMXMd6Jtc7FCV5PzVP?=
 =?us-ascii?Q?gHK1F5cGsVNmPsYoaDIpWi4hK+k65NjKaY8K1wRXEDwRvcczIALKtl74Ybn4?=
 =?us-ascii?Q?RbVE+GlrvuAOWWy92l5aT1pw9yPeCkAFjD31GX/CV5l1/fAZ1d8JSXis42lZ?=
 =?us-ascii?Q?uS8GFyuTOGcQLeGk6CPRMsAC4XfMgTaPIKxnwf8T/7c1neKd4kXcAuxGRag4?=
 =?us-ascii?Q?7IyNzMRxS3/Y7RUbXzl7I29WYwDU6j7jSxkJlY6EHSuGAOw9uD9jwlG9kX3o?=
 =?us-ascii?Q?34OVzH8rMHzUqWfZY56XTGY=3D?=
X-OriginatorOrg: os.amperecomputing.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 58fb871e-ad79-413c-baa7-08dc27573b36
X-MS-Exchange-CrossTenant-AuthSource: PH0PR01MB8166.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Feb 2024 21:04:39.0398
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: zzgHtSIEfDYX8eSN+43HaMdTXXtKctzMtHIiiDcLZlgXicT97pVzt7BLYCdwiIKUopO/jivK4fBO8gY/wb4bBO5r4k5IDkpF67FxokuxoskCvLAIkCVFywVzIKCdiy94
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL3PR01MB6851


On Tue, 6 Feb 2024, Robin Murphy wrote:
> On 2024-02-05 7:46 pm, Ilkka Koskinen wrote:
>> AmpereOneX mesh implementation has a bug in HN-P nodes that makes them
>> report incorrect child count. The failing crosspoints report 8 children
>> while they only have two.
>
> Ooh, fun :)
>
>> When the driver tries to access the inexistent child nodes, it believes it
>> has reached an invalid node type and probing fails. The workaround is to
>> ignore those incorrect child nodes and continue normally.
>> 
>> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
>> ---
>>   drivers/perf/arm-cmn.c | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>> 
>> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
>> index c584165b13ba..97fed8ec3693 100644
>> --- a/drivers/perf/arm-cmn.c
>> +++ b/drivers/perf/arm-cmn.c
>> @@ -2168,6 +2168,23 @@ static enum cmn_node_type arm_cmn_subtype(enum 
>> cmn_node_type type)
>>   	}
>>   }
>>   +static inline bool arm_cmn_is_ampereonex_bug(const struct arm_cmn *cmn,
>> +					     struct arm_cmn_node *dn,
>> +					     u16 child_count, int child)
>> +{
>> +	/*
>> +	 * The bug occurs only when a crosspoint reports 8 children
>> +	 * while it only has two HN-P child nodes.
>> +	 */
>> +	dn -= 2;
>> +
>> +	if (arm_cmn_model(cmn) == CMN650 && child_count == 8 &&
>> +	    child == 2 && dn->type == CMN_TYPE_HNP)
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>>   static int arm_cmn_discover(struct arm_cmn *cmn, unsigned int rgn_offset)
>>   {
>>   	void __iomem *cfg_region;
>> @@ -2292,6 +2309,14 @@ static int arm_cmn_discover(struct arm_cmn *cmn, 
>> unsigned int rgn_offset)
>>     		for (j = 0; j < child_count; j++) {
>>   			reg = readq_relaxed(xp_region + child_poff + j * 8);
>> +			if (reg == 0)
>> +				if (arm_cmn_is_ampereonex_bug(cmn, dn, 
>> child_count, j))
>> +					/*
>> +					 * We know there are only two real 
>> children and the rest 6
>> +					 * are inexistent. Thus, we can skip 
>> the rest of the loop
>> +					 */
>> +					break;
>> +
>
> TBH I don't see much harm in taking an even simpler approach, so I'd be
> inclined to not bother being all that specific beyond documenting it,
> something like the below:

Sounds good to me.

>
> Cheers,
> Robin.
>
> ----->8-----
>
> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index c584165b13ba..7e3aa7e2345f 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c
> @@ -2305,6 +2305,17 @@ static int arm_cmn_discover(struct arm_cmn *cmn, 
> unsigned int rgn_offset)
> 				dev_dbg(cmn->dev, "ignoring external node 
> %llx\n", reg);
> 				continue;
> 			}
> +			/*
> +			 * AmpereOneX erratum AC04_MESH_1 makes some XPs 
> report a bogus
> +			 * child count larger than the number of valid child 
> pointers.
> +			 * A child offset of 0 can only occur on CMN-600; 
> otherwise it
> +			 * would imply the root node being its own 
> grandchild, which
> +			 * we can safely dismiss in general.
> +			 */
> +			if (reg == 0 && cmn->part != PART_CMN600) {
> +				dev_dbg(cmn->dev, "bogus child pointer?\n");
> +				continue;
> +			}
>  			arm_cmn_init_node_info(cmn, reg & 
> CMN_CHILD_NODE_ADDR, dn);
>

Tested-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>

Cheers, Ilkka