Re: [PATCH v6 2/2] perf/arm-cmn: Add workarounds for CMN-S3 on Graviton5

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The Graviton5 workaround sets the DTC node's logical ID to the XP's mesh logical ID instead of the DTC domain ID, which can cross-wire PMU event routing.
--

commit d22a8571a1d86b4f3c00bde161d5fb2ce99fe43f
Author: Aviv Bakal <avivb@amazon.com>

perf/arm-cmn: Add workarounds for CMN-S3 on Graviton5

This patch adds workarounds for the CMN-S3 implementation on Graviton5,
which reports zeroed fields in discovery registers. It introduces an ACPI
HID for identification, derives the DTC domain from the XP node ID, and
sets the DTC logical ID from the XP's logical ID.

> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c
[ ... ]
> @@ -2475,6 +2500,10 @@ static int arm_cmn_discover(struct arm_cmn *cmn, unsigned int rgn_offset)
>  
>  			switch (dn->type) {
>  			case CMN_TYPE_DTC:
> +				if (graviton5_workaround) {
> +					/* Node info logical ID is zeroed; use the XP's */
> +					dn->logid = xp->logid;

[Severity: Medium]
Does this code assign the host XP's mesh logical ID to the DTC node instead
of the computed domain ID (xp->dtc)?

If the DTC nodes' mesh logical IDs do not sort into the exact same order
as their domain IDs, will the array indices be cross-wired, potentially
misrouting PMU events?

Event configuration looks up the cmn->dtc[] array using the node's domain
ID (dn->dtc).

Could this instead be dn->logid = xp->dtc; to restore the correct semantic
meaning of a DTC's logical ID as its domain index?

> +				}
>  				cmn->num_dtcs++;
>  				dn++;
>  				break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782824005.git.robin.murphy@arm.com?part=2