From: Ilkka Koskinen <ilkka@os.amperecomputing.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: will@kernel.org, mark.rutland@arm.com,
linux-arm-kernel@lists.infradead.org,
linux-perf-users@vger.kernel.org, ilkka@os.amperecomputing.com,
leo.yan@arm.com
Subject: Re: [PATCH v2 3/5] perf/arm-cmn: Refactor event filter data
Date: Mon, 3 Aug 2026 19:18:35 -0700 (PDT) [thread overview]
Message-ID: <a2ceb76d-e234-24b3-9813-1f3bf95f2be2@os.amperecomputing.com> (raw)
In-Reply-To: <2f1dee73fe016249e13fe74d6137a044c900c9e4.1785252273.git.robin.murphy@arm.com>
On Tue, 28 Jul 2026, Robin Murphy wrote:
> The ABI hole I have dug myself into requires the driver to know which
> event encodings are associated with which particular filter control.
> Since we will soon have a notion of multiple filters per event, refactor
> the event data to encapsulate filters in an explicit structure, which
> can then more easily scale as an array in future.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
LGTM:
Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Cheers, Ilkka
> ---
>
> v2: Avoid transient bug with filter.val in _CMN_EVENT_ATTR()
>
> drivers/perf/arm-cmn.c | 95 ++++++++++++++++++++++--------------------
> 1 file changed, 49 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index 14c267d2e2f9..74bc2aae4a06 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c
> @@ -717,13 +717,17 @@ static void arm_cmn_clear_idx(struct arm_cmn_hw_event *hw)
> bitmap_zero(hw->wp_idx, CMN_MAX_XPS);
> }
>
> +struct arm_cmn_filter_attr {
> + enum cmn_filter_select sel;
> + u8 val;
> +};
> +
> struct arm_cmn_event_attr {
> struct device_attribute attr;
> enum cmn_model model;
> enum cmn_node_type type;
> - enum cmn_filter_select fsel;
> u16 eventid;
> - u8 filter;
> + struct arm_cmn_filter_attr filter[1];
> };
>
> struct arm_cmn_format_attr {
> @@ -732,24 +736,25 @@ struct arm_cmn_format_attr {
> int config;
> };
>
> -#define _CMN_EVENT_ATTR(_model, _name, _type, _eventid, _filter, _fsel)\
> +#define _CMN_EVENT_ATTR(_model, _name, _type, _eventid, _fa, _fb, ...) \
> (&((struct arm_cmn_event_attr[]) {{ \
> .attr = __ATTR(_name, 0444, arm_cmn_event_show, NULL), \
> .model = _model, \
> .type = _type, \
> .eventid = _eventid, \
> - .filter = _filter, \
> - .fsel = _fsel, \
> + .filter = {{_fa, _fb}}, \
> }})[0].attr.attr)
> -#define CMN_EVENT_ATTR(_model, _name, _type, _eventid) \
> - _CMN_EVENT_ATTR(_model, _name, _type, _eventid, 0, SEL_NONE)
> +#define CMN_EVENT_ATTR(_model, _name, _type, _eventid, _filter...) \
> + _CMN_EVENT_ATTR(_model, _name, _type, _eventid, ##_filter, 0, 0)
>
> static ssize_t arm_cmn_event_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> struct arm_cmn_event_attr *eattr;
> + struct arm_cmn_filter_attr *filter;
>
> eattr = container_of(attr, typeof(*eattr), attr);
> + filter = eattr->filter;
>
> if (eattr->type == CMN_TYPE_DTC)
> return sysfs_emit(buf, "type=0x%x\n", eattr->type);
> @@ -759,9 +764,9 @@ static ssize_t arm_cmn_event_show(struct device *dev,
> "type=0x%x,eventid=0x%x,wp_dev_sel=?,wp_chn_sel=?,wp_grp=?,wp_val=?,wp_mask=?\n",
> eattr->type, eattr->eventid);
>
> - if (eattr->fsel)
> + if (filter[0].sel)
> return sysfs_emit(buf, "type=0x%x,eventid=0x%x,filter=0x%x\n",
> - eattr->type, eattr->eventid, eattr->filter);
> + eattr->type, eattr->eventid, filter[0].val);
>
> return sysfs_emit(buf, "type=0x%x,eventid=0x%x\n", eattr->type,
> eattr->eventid);
> @@ -849,8 +854,8 @@ static umode_t arm_cmn_event_attr_is_visible(struct kobject *kobj,
> return attr->mode;
> }
>
> -#define _CMN_EVENT_DVM(_model, _name, _event, _occup, _fsel) \
> - _CMN_EVENT_ATTR(_model, dn_##_name, CMN_TYPE_DVM, _event, _occup, _fsel)
> +#define CMN_EVENT_DVM(_model, _name, _event, _filter...) \
> + CMN_EVENT_ATTR(_model, dn_##_name, CMN_TYPE_DVM, _event, ##_filter)
> #define CMN_EVENT_DTC(_name) \
> CMN_EVENT_ATTR(CMN_ANY, dtc_##_name, CMN_TYPE_DTC, 0)
> #define CMN_EVENT_HNF(_model, _name, _event) \
> @@ -880,32 +885,30 @@ static umode_t arm_cmn_event_attr_is_visible(struct kobject *kobj,
> #define CMN_EVENT_HNS(_name, _event) \
> CMN_EVENT_ATTR(CMN_ANY, hns_##_name, CMN_TYPE_HNS, _event)
>
> -#define CMN_EVENT_DVM(_model, _name, _event) \
> - _CMN_EVENT_DVM(_model, _name, _event, 0, SEL_NONE)
> #define CMN_EVENT_DVM_OCC(_model, _name, _event) \
> - _CMN_EVENT_DVM(_model, _name##_all, _event, 0, SEL_OCCUP1_ID), \
> - _CMN_EVENT_DVM(_model, _name##_dvmop, _event, 1, SEL_OCCUP1_ID), \
> - _CMN_EVENT_DVM(_model, _name##_dvmsync, _event, 2, SEL_OCCUP1_ID)
> + CMN_EVENT_DVM(_model, _name##_all, _event, SEL_OCCUP1_ID, 0), \
> + CMN_EVENT_DVM(_model, _name##_dvmop, _event, SEL_OCCUP1_ID, 1), \
> + CMN_EVENT_DVM(_model, _name##_dvmsync, _event, SEL_OCCUP1_ID, 2)
>
> #define CMN_EVENT_HN_OCC(_model, _name, _type, _event) \
> - _CMN_EVENT_ATTR(_model, _name##_all, _type, _event, 0, SEL_OCCUP1_ID), \
> - _CMN_EVENT_ATTR(_model, _name##_read, _type, _event, 1, SEL_OCCUP1_ID), \
> - _CMN_EVENT_ATTR(_model, _name##_write, _type, _event, 2, SEL_OCCUP1_ID), \
> - _CMN_EVENT_ATTR(_model, _name##_atomic, _type, _event, 3, SEL_OCCUP1_ID), \
> - _CMN_EVENT_ATTR(_model, _name##_stash, _type, _event, 4, SEL_OCCUP1_ID)
> + CMN_EVENT_ATTR(_model, _name##_all, _type, _event, SEL_OCCUP1_ID, 0), \
> + CMN_EVENT_ATTR(_model, _name##_read, _type, _event, SEL_OCCUP1_ID, 1), \
> + CMN_EVENT_ATTR(_model, _name##_write, _type, _event, SEL_OCCUP1_ID, 2), \
> + CMN_EVENT_ATTR(_model, _name##_atomic, _type, _event, SEL_OCCUP1_ID, 3), \
> + CMN_EVENT_ATTR(_model, _name##_stash, _type, _event, SEL_OCCUP1_ID, 4)
> #define CMN_EVENT_HN_CLS(_model, _name, _type, _event) \
> - _CMN_EVENT_ATTR(_model, _name##_class0, _type, _event, 0, SEL_CLASS_OCCUP_ID), \
> - _CMN_EVENT_ATTR(_model, _name##_class1, _type, _event, 1, SEL_CLASS_OCCUP_ID), \
> - _CMN_EVENT_ATTR(_model, _name##_class2, _type, _event, 2, SEL_CLASS_OCCUP_ID), \
> - _CMN_EVENT_ATTR(_model, _name##_class3, _type, _event, 3, SEL_CLASS_OCCUP_ID)
> + CMN_EVENT_ATTR(_model, _name##_class0, _type, _event, SEL_CLASS_OCCUP_ID, 0), \
> + CMN_EVENT_ATTR(_model, _name##_class1, _type, _event, SEL_CLASS_OCCUP_ID, 1), \
> + CMN_EVENT_ATTR(_model, _name##_class2, _type, _event, SEL_CLASS_OCCUP_ID, 2), \
> + CMN_EVENT_ATTR(_model, _name##_class3, _type, _event, SEL_CLASS_OCCUP_ID, 3)
> #define CMN_EVENT_HN_SNT(_model, _name, _type, _event) \
> - _CMN_EVENT_ATTR(_model, _name##_all, _type, _event, 0, SEL_CBUSY_SNTHROTTLE_SEL), \
> - _CMN_EVENT_ATTR(_model, _name##_group0_read, _type, _event, 1, SEL_CBUSY_SNTHROTTLE_SEL), \
> - _CMN_EVENT_ATTR(_model, _name##_group0_write, _type, _event, 2, SEL_CBUSY_SNTHROTTLE_SEL), \
> - _CMN_EVENT_ATTR(_model, _name##_group1_read, _type, _event, 3, SEL_CBUSY_SNTHROTTLE_SEL), \
> - _CMN_EVENT_ATTR(_model, _name##_group1_write, _type, _event, 4, SEL_CBUSY_SNTHROTTLE_SEL), \
> - _CMN_EVENT_ATTR(_model, _name##_read, _type, _event, 5, SEL_CBUSY_SNTHROTTLE_SEL), \
> - _CMN_EVENT_ATTR(_model, _name##_write, _type, _event, 6, SEL_CBUSY_SNTHROTTLE_SEL)
> + CMN_EVENT_ATTR(_model, _name##_all, _type, _event, SEL_CBUSY_SNTHROTTLE_SEL, 0), \
> + CMN_EVENT_ATTR(_model, _name##_group0_read, _type, _event, SEL_CBUSY_SNTHROTTLE_SEL, 1), \
> + CMN_EVENT_ATTR(_model, _name##_group0_write, _type, _event, SEL_CBUSY_SNTHROTTLE_SEL, 2), \
> + CMN_EVENT_ATTR(_model, _name##_group1_read, _type, _event, SEL_CBUSY_SNTHROTTLE_SEL, 3), \
> + CMN_EVENT_ATTR(_model, _name##_group1_write, _type, _event, SEL_CBUSY_SNTHROTTLE_SEL, 4), \
> + CMN_EVENT_ATTR(_model, _name##_read, _type, _event, SEL_CBUSY_SNTHROTTLE_SEL, 5), \
> + CMN_EVENT_ATTR(_model, _name##_write, _type, _event, SEL_CBUSY_SNTHROTTLE_SEL, 6)
>
> #define CMN_EVENT_HNF_OCC(_model, _name, _event) \
> CMN_EVENT_HN_OCC(_model, hnf_##_name, CMN_TYPE_HNF, _event)
> @@ -916,21 +919,21 @@ static umode_t arm_cmn_event_attr_is_visible(struct kobject *kobj,
>
> #define CMN_EVENT_HNS_OCC(_name, _event) \
> CMN_EVENT_HN_OCC(CMN_ANY, hns_##_name, CMN_TYPE_HNS, _event), \
> - _CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_rxsnp, CMN_TYPE_HNS, _event, 5, SEL_OCCUP1_ID), \
> - _CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_lbt, CMN_TYPE_HNS, _event, 6, SEL_OCCUP1_ID), \
> - _CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_hbt, CMN_TYPE_HNS, _event, 7, SEL_OCCUP1_ID)
> + CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_rxsnp, CMN_TYPE_HNS, _event, SEL_OCCUP1_ID, 5), \
> + CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_lbt, CMN_TYPE_HNS, _event, SEL_OCCUP1_ID, 6), \
> + CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_hbt, CMN_TYPE_HNS, _event, SEL_OCCUP1_ID, 7)
> #define CMN_EVENT_HNS_CLS( _name, _event) \
> CMN_EVENT_HN_CLS(CMN_ANY, hns_##_name, CMN_TYPE_HNS, _event)
> #define CMN_EVENT_HNS_SNT(_name, _event) \
> CMN_EVENT_HN_SNT(CMN_ANY, hns_##_name, CMN_TYPE_HNS, _event)
> #define CMN_EVENT_HNS_HBT(_name, _event) \
> - _CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_all, CMN_TYPE_HNS, _event, 0, SEL_HBT_LBT_SEL), \
> - _CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_hbt, CMN_TYPE_HNS, _event, 1, SEL_HBT_LBT_SEL), \
> - _CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_lbt, CMN_TYPE_HNS, _event, 2, SEL_HBT_LBT_SEL)
> + CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_all, CMN_TYPE_HNS, _event, SEL_HBT_LBT_SEL, 0), \
> + CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_hbt, CMN_TYPE_HNS, _event, SEL_HBT_LBT_SEL, 1), \
> + CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_lbt, CMN_TYPE_HNS, _event, SEL_HBT_LBT_SEL, 2)
> #define CMN_EVENT_HNS_SNH(_name, _event) \
> - _CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_all, CMN_TYPE_HNS, _event, 0, SEL_SN_HOME_SEL), \
> - _CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_sn, CMN_TYPE_HNS, _event, 1, SEL_SN_HOME_SEL), \
> - _CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_home, CMN_TYPE_HNS, _event, 2, SEL_SN_HOME_SEL)
> + CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_all, CMN_TYPE_HNS, _event, SEL_SN_HOME_SEL, 0), \
> + CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_sn, CMN_TYPE_HNS, _event, SEL_SN_HOME_SEL, 1), \
> + CMN_EVENT_ATTR(CMN_ANY, hns_##_name##_home, CMN_TYPE_HNS, _event, SEL_SN_HOME_SEL, 2)
>
> #define _CMN_EVENT_XP_MESH(_name, _event) \
> __CMN_EVENT_XP(e_##_name, (_event) | (0 << 2)), \
> @@ -1814,9 +1817,9 @@ static int arm_cmn_validate_group(struct arm_cmn *cmn, struct perf_event *event)
> return ret;
> }
>
> -static enum cmn_filter_select arm_cmn_filter_sel(const struct arm_cmn *cmn,
> - enum cmn_node_type type,
> - unsigned int eventid)
> +static enum cmn_filter_select arm_cmn_event_filter(const struct arm_cmn *cmn,
> + enum cmn_node_type type,
> + unsigned int eventid)
> {
> struct arm_cmn_event_attr *e;
> enum cmn_model model = arm_cmn_model(cmn);
> @@ -1824,7 +1827,7 @@ static enum cmn_filter_select arm_cmn_filter_sel(const struct arm_cmn *cmn,
> for (int i = 0; i < ARRAY_SIZE(arm_cmn_event_attrs) - 1; i++) {
> e = container_of(arm_cmn_event_attrs[i], typeof(*e), attr.attr);
> if (e->model & model && e->type == type && e->eventid == eventid)
> - return e->fsel;
> + return e->filter[0].sel;
> }
> return SEL_NONE;
> }
> @@ -1889,7 +1892,7 @@ static int arm_cmn_event_init(struct perf_event *event)
> }
>
> /* This is sufficiently annoying to recalculate, so cache it */
> - hw->filter_sel = arm_cmn_filter_sel(cmn, type, eventid);
> + hw->filter_sel = arm_cmn_event_filter(cmn, type, eventid);
>
> bynodeid = CMN_EVENT_BYNODEID(event);
> nodeid = CMN_EVENT_NODEID(event);
> --
> 2.54.0.dirty
>
>
next prev parent reply other threads:[~2026-08-04 2:18 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-28 15:59 [PATCH v2 0/5] perf/arm-cmn: Support CMN S3 r2 Robin Murphy
2026-07-28 15:59 ` [PATCH v2 1/5] perf/arm-cmn: Rename filter variables for clarity Robin Murphy
2026-07-29 17:20 ` Jonathan Cameron
2026-07-29 18:39 ` Robin Murphy
2026-07-28 15:59 ` [PATCH v2 2/5] perf/arm-cmn: Refactor event filter programming Robin Murphy
2026-07-28 15:59 ` [PATCH v2 3/5] perf/arm-cmn: Refactor event filter data Robin Murphy
2026-08-04 2:18 ` Ilkka Koskinen [this message]
2026-07-28 15:59 ` [PATCH v2 4/5] perf/arm-cmn: Plumb in new filter types Robin Murphy
2026-07-28 16:43 ` sashiko-bot
2026-08-04 2:22 ` Ilkka Koskinen
2026-07-28 15:59 ` [PATCH v2 5/5] perf/arm-cmn: Support CMN S3 r2 Robin Murphy
2026-07-28 16:57 ` sashiko-bot
2026-07-28 17:16 ` Robin Murphy
2026-08-04 2:33 ` Ilkka Koskinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a2ceb76d-e234-24b3-9813-1f3bf95f2be2@os.amperecomputing.com \
--to=ilkka@os.amperecomputing.com \
--cc=leo.yan@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=robin.murphy@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox