Re: [PATCH 2/4] perf/arm-cmn: Add CMN-650 support

[Ilkka Koskinen <ilkka@os.amperecomputing.com>]

I still have a couple tiny comments. Otherwise the patch set looks good to 
me.

On Mon, 18 Apr 2022, Robin Murphy wrote:
> Add the identifiers and events for CMN-650, which slots into its
> evolutionary position between CMN-600 and the 700-series products.
> Imagine CMN-600 made bigger, and with most of the rough edges smoothed
> off, but that then balanced out by some bonkers PMU functionality for
> the new HN-P enhancement in CMN-650r2.
>
> Most of the CXG events are actually common to newer revisions of CMN-600
> too, so they're arguably a little late; oh well.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
> drivers/perf/arm-cmn.c | 222 ++++++++++++++++++++++++++++++++---------
> 1 file changed, 176 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index 9c1d82be7a2f..cce8516d465c 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c
> @@ -39,7 +39,7 @@
> #define CMN_CHILD_NODE_ADDR		GENMASK(27, 0)
> #define CMN_CHILD_NODE_EXTERNAL		BIT(31)
>
> -#define CMN_MAX_DIMENSION		8
> +#define CMN_MAX_DIMENSION		12
> #define CMN_MAX_XPS			(CMN_MAX_DIMENSION * CMN_MAX_DIMENSION)
> #define CMN_MAX_DTMS			(CMN_MAX_XPS + (CMN_MAX_DIMENSION - 1) * 4)
>
> @@ -65,7 +65,9 @@
>
> /* For most nodes, this is all there is */
> #define CMN_PMU_EVENT_SEL		0x000
> -#define CMN_PMU_EVENTn_ID_SHIFT(n)	((n) * 8)
> +
> +/* HN-Ps are weird... */
> +#define CMN_HNP_PMU_EVENT_SEL		0x008
>
> /* DTMs live in the PMU space of XP registers */
> #define CMN_DTM_WPn(n)			(0x1A0 + (n) * 0x18)
> @@ -177,9 +179,12 @@
>
>
> enum cmn_model {
> -	CMN_ANY = -1,
> 	CMN600 = 1,
> -	CI700 = 2,
> +	CMN650 = 2,
> +	CI700 = 8,
> +	/* ...and then we can use bitmap tricks for commonality */
> +	CMN_ANY = -1,
> +	NOT_CMN600 = -2,

Matter of taste, but I'd probably prefer NOT_CMN600 = ~CMN600

> };
>
> /* CMN-600 r0px shouldn't exist in silicon, thankfully */
> @@ -191,6 +196,11 @@ enum cmn_revision {
> 	CMN600_R2P0,
> 	CMN600_R3P0,
> 	CMN600_R3P1,
> +	CMN650_R0P0 = 0,
> +	CMN650_R1P0,
> +	CMN650_R1P1,
> +	CMN650_R2P0,
> +	CMN650_R1P2,
> 	CI700_R0P0 = 0,
> 	CI700_R1P0,
> 	CI700_R2P0,
> @@ -211,6 +221,7 @@ enum cmn_node_type {
> 	CMN_TYPE_RND = 0xd,
> 	CMN_TYPE_RNSAM = 0xf,
> 	CMN_TYPE_MTSX,
> +	CMN_TYPE_HNP,
> 	CMN_TYPE_CXRA = 0x100,
> 	CMN_TYPE_CXHA = 0x101,
> 	CMN_TYPE_CXLA = 0x102,
> @@ -307,9 +318,7 @@ struct arm_cmn_nodeid {

<snip>

> @@ -580,20 +592,25 @@ static umode_t arm_cmn_event_attr_is_visible(struct kobject *kobj,
> 	struct device *dev = kobj_to_dev(kobj);
> 	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
> 	struct arm_cmn_event_attr *eattr;
> +	enum cmn_node_type type;
> +	u16 eventid;
>
> 	eattr = container_of(attr, typeof(*eattr), attr.attr);
>
> 	if (!(eattr->model & cmn->model))
> 		return 0;
>
> +	type = eattr->type;
> +	eventid = eattr->eventid;
> +
> 	/* Watchpoints aren't nodes, so avoid confusion */
> -	if (eattr->type == CMN_TYPE_WP)
> +	if (type == CMN_TYPE_WP)
> 		return attr->mode;
>
> 	/* Hide XP events for unused interfaces/channels */
> -	if (eattr->type == CMN_TYPE_XP) {
> -		unsigned int intf = (eattr->eventid >> 2) & 7;
> -		unsigned int chan = eattr->eventid >> 5;
> +	if (type == CMN_TYPE_XP) {
> +		unsigned int intf = (eventid >> 2) & 7;
> +		unsigned int chan = eventid >> 5;
>
> 		if ((intf & 4) && !(cmn->ports_used & BIT(intf & 3)))
> 			return 0;
> @@ -607,12 +624,29 @@ static umode_t arm_cmn_event_attr_is_visible(struct kobject *kobj,
> 	}
>
> 	/* Revision-specific differences */
> -	if (cmn->model == CMN600 && cmn->rev < CMN600_R1P2) {
> -		if (eattr->type == CMN_TYPE_HNF && eattr->eventid == 0x1b)
> -			return 0;
> +	if (cmn->model == CMN600) {
> +		if (cmn->rev < CMN600_R1P3) {
> +			if (type == CMN_TYPE_CXRA && eventid > 0x10)
> +				return 0;
> +		}
> +		if (cmn->rev < CMN600_R1P2) {
> +			if (type == CMN_TYPE_HNF && eventid == 0x1b)
> +				return 0;
> +			if (type == CMN_TYPE_CXRA || type == CMN_TYPE_CXHA)
> +				return 0;
> +		}
> +	} else if (cmn->model == CMN650) {
> +		if (cmn->rev < CMN650_R2P0 || cmn->rev == CMN650_R1P2) {
> +			if (type == CMN_TYPE_HNF && eventid > 0x22)
> +				return 0;
> +			if (type == CMN_TYPE_SBSX && eventid == 0x17)
> +				return 0;
> +			if (type == CMN_TYPE_RNI && eventid > 0x10)
> +				return 0;
> +		}

What's the plan with cmn-650 r2p0 event settings? There seem to be a few 
extra ones made visible now. I'm fine with updating this patch or taking 
care of them in a separate one, which ever makes more sense.

Cheers, Ilkka


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