[PATCH v3 0/3] perf/cxlpmu: Misc updates

[PATCH v3 0/3] perf/cxlpmu: Misc updates

[Davidlohr Bueso]

Hello,

Changes from v2 (https://lore.kernel.org/all/20260630235002.253297-1-dave@stgolabs.net/):
 - Split the 64-bit HDM filter write fix into its own patch 1. (Richard).
 - Allow CRB filtering for the Queue Occupancy, Queue Residency and
   Retry event groups, not just DDR Interface in patch 3 (Richard).
 - Reject CRB filtering when counting multiple events (multiple
   mask bits set), undefined behavior in patch 3.
 - Reordered the series so the events patch comes first, providing
   the event group ID definitions the filter validation needs.
 - Add the RD/WR merged queue occupancy and power down events,
   and the Throttle Events group in patch 2.

Three patches for the CXL PMU driver.

Patch 1 is a standalone fix.

Patch 2 adds the CXL 4.0 events that hardware exposes but the
driver did not. It now precedes the filter work as the latter
refers to the new event group IDs.

Patch 3 implements Channel/Rank/Bank (CRB) filtering, now permitted
for all event groups that CXL 4.0 Table 13-5 allows Filter ID 1 on.

v2 was tested on real hardware. The v3 changes have additionally
been exercised under qemu CPMU emulation (jic23 tree) + changes for
the new events/Filter=1.

Thanks!

Davidlohr Bueso (1):
  perf/cxlpmu: Fix 64-bit write to 32-bit HDM filter register

Harshal Thakkar (2):
  perf/cxlpmu: Add missing CXL 4.0 events
  perf/cxlpmu: Support Channel/Rank/Bank filter

 drivers/perf/cxl_pmu.c | 115 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 112 insertions(+), 3 deletions(-)

-- 
2.39.5

[PATCH v3 1/3] perf/cxlpmu: Fix 64-bit write to 32-bit HDM filter register

[Davidlohr Bueso]

The HDM decoder filter configuration register is 32 bits wide, but the
driver programs it with a 64-bit writeq(). The upper half of the write
is always zero and lands in the adjacent Filter ID 1 (Channel/Rank/Bank)
configuration register at offset+4.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/perf/cxl_pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 68a54d97d2a8..39b46550a510 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -635,7 +635,7 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
 			cfg = cxl_pmu_config2_get_hdm_decoder(event);
 		else
 			cfg = GENMASK(31, 0); /* No filtering if 0xFFFF_FFFF */
-		writeq(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 0));
+		writel(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 0));
 	}
 
 	cfg = readq(base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
-- 
2.39.5

[PATCH v3 2/3] perf/cxlpmu: Add missing CXL 4.0 events

[Davidlohr Bueso]

From: Harshal Thakkar <harshal.t@samsung.com>

Add support for CXL 4.0 events that are exposed by the CPMU hardware
but not present in the driver. Such events are defined in Table 13-5
of the spec.

Signed-off-by: Harshal Thakkar <harshal.t@samsung.com>
[davidlohr: add missing throttle and queue occupancy events]
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/perf/cxl_pmu.c | 50 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 39b46550a510..1fc83858f653 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -77,6 +77,10 @@
 #define CXL_PMU_GID_S2M_NDR		0x0024
 #define CXL_PMU_GID_S2M_DRS		0x0025
 #define CXL_PMU_GID_DDR			0x8000
+#define CXL_PMU_GID_QUEUE_OCC           0x8001
+#define CXL_PMU_GID_QUEUE_RESID         0x8002
+#define CXL_PMU_GID_RETRY_EVENTS        0x8003
+#define CXL_PMU_GID_THROTTLE            0x8004
 
 static int cxl_pmu_cpuhp_state_num;
 
@@ -385,13 +389,23 @@ static struct attribute *cxl_pmu_event_attrs[] = {
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memwrfwd,		CXL_PMU_GID_M2S_REQ, BIT(4)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrdtee,		CXL_PMU_GID_M2S_REQ, BIT(5)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrddatatee,		CXL_PMU_GID_M2S_REQ, BIT(6)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_req_meminvtee,               CXL_PMU_GID_M2S_REQ, BIT(7)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memspecrd,		CXL_PMU_GID_M2S_REQ, BIT(8)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_meminvnt,		CXL_PMU_GID_M2S_REQ, BIT(9)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memcleanevict,		CXL_PMU_GID_M2S_REQ, BIT(10)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_req_meminvptee,		CXL_PMU_GID_M2S_REQ, BIT(11)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memspecrdtee,		CXL_PMU_GID_M2S_REQ, BIT(12)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_req_teupdate,		CXL_PMU_GID_M2S_REQ, BIT(13)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memclnevcttee,		CXL_PMU_GID_M2S_REQ, BIT(14)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memclnevctu,		CXL_PMU_GID_M2S_REQ, BIT(15)),
 	/* CXL rev 3.0 Table 3-35 M2S RwD Memory Opcodes */
 	CXL_PMU_EVENT_CXL_ATTR(m2s_rwd_memwr,			CXL_PMU_GID_M2S_RWD, BIT(1)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_rwd_memwrptl,		CXL_PMU_GID_M2S_RWD, BIT(2)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_rwd_biconflict,		CXL_PMU_GID_M2S_RWD, BIT(4)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_rwd_memrdfill,		CXL_PMU_GID_M2S_RWD, BIT(5)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_rwd_memwrtee,		CXL_PMU_GID_M2S_RWD, BIT(9)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_rwd_memwrptltee,		CXL_PMU_GID_M2S_RWD, BIT(10)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_rwd_memrdfilltee,		CXL_PMU_GID_M2S_RWD, BIT(13)),
 	/* CXL rev 3.0 Table 3-38 M2S BIRsp Memory Opcodes */
 	CXL_PMU_EVENT_CXL_ATTR(m2s_birsp_i,			CXL_PMU_GID_M2S_BIRSP, BIT(0)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_birsp_s,			CXL_PMU_GID_M2S_BIRSP, BIT(1)),
@@ -406,15 +420,25 @@ static struct attribute *cxl_pmu_event_attrs[] = {
 	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_curblk,		CXL_PMU_GID_S2M_BISNP, BIT(4)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_datblk,		CXL_PMU_GID_S2M_BISNP, BIT(5)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_invblk,		CXL_PMU_GID_S2M_BISNP, BIT(6)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_curtee,		CXL_PMU_GID_S2M_BISNP, BIT(8)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_datatee,		CXL_PMU_GID_S2M_BISNP, BIT(9)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_invtee,		CXL_PMU_GID_S2M_BISNP, BIT(10)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_curblktee,		CXL_PMU_GID_S2M_BISNP, BIT(12)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_datablktee,		CXL_PMU_GID_S2M_BISNP, BIT(13)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_invblktee,		CXL_PMU_GID_S2M_BISNP, BIT(14)),
 	/* CXL rev 3.1 Table 3-50 S2M NDR Opcodes */
 	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmp,			CXL_PMU_GID_S2M_NDR, BIT(0)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmps,			CXL_PMU_GID_S2M_NDR, BIT(1)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpe,			CXL_PMU_GID_S2M_NDR, BIT(2)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpm,			CXL_PMU_GID_S2M_NDR, BIT(3)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_biconflictack,		CXL_PMU_GID_S2M_NDR, BIT(4)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmptee,			CXL_PMU_GID_S2M_NDR, BIT(5)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmptee_s,		CXL_PMU_GID_S2M_NDR, BIT(6)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmptee_e,		CXL_PMU_GID_S2M_NDR, BIT(7)),
 	/* CXL rev 3.0 Table 3-46 S2M DRS opcodes */
 	CXL_PMU_EVENT_CXL_ATTR(s2m_drs_memdata,			CXL_PMU_GID_S2M_DRS, BIT(0)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_drs_memdatanxm,		CXL_PMU_GID_S2M_DRS, BIT(1)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_drs_memdatatee,		CXL_PMU_GID_S2M_DRS, BIT(2)),
 	/* CXL rev 3.0 Table 13-5 directly lists these */
 	CXL_PMU_EVENT_CXL_ATTR(ddr_act,				CXL_PMU_GID_DDR, BIT(0)),
 	CXL_PMU_EVENT_CXL_ATTR(ddr_pre,				CXL_PMU_GID_DDR, BIT(1)),
@@ -423,6 +447,32 @@ static struct attribute *cxl_pmu_event_attrs[] = {
 	CXL_PMU_EVENT_CXL_ATTR(ddr_refresh,			CXL_PMU_GID_DDR, BIT(4)),
 	CXL_PMU_EVENT_CXL_ATTR(ddr_selfrefreshent,		CXL_PMU_GID_DDR, BIT(5)),
 	CXL_PMU_EVENT_CXL_ATTR(ddr_rfm,				CXL_PMU_GID_DDR, BIT(6)),
+	/* CXL 4.0 Table 13-5 DDR add-on events opcodes */
+	CXL_PMU_EVENT_CXL_ATTR(ddr_cas_rd_ap,			CXL_PMU_GID_DDR, BIT(7)),
+	CXL_PMU_EVENT_CXL_ATTR(ddr_cas_wr_ap,			CXL_PMU_GID_DDR, BIT(8)),
+	CXL_PMU_EVENT_CXL_ATTR(ddr_refresh_all_banks,		CXL_PMU_GID_DDR, BIT(9)),
+	CXL_PMU_EVENT_CXL_ATTR(ddr_refresh_same_bank,		CXL_PMU_GID_DDR, BIT(10)),
+	CXL_PMU_EVENT_CXL_ATTR(ddr_pwrdn_entry,			CXL_PMU_GID_DDR, BIT(11)),
+	CXL_PMU_EVENT_CXL_ATTR(ddr_pwrdn_exit,			CXL_PMU_GID_DDR, BIT(12)),
+	CXL_PMU_EVENT_CXL_ATTR(ddr_rd_wr_ddr_bus_switching,	CXL_PMU_GID_DDR, BIT(13)),
+	CXL_PMU_EVENT_CXL_ATTR(ddr_incoming_rd_req,		CXL_PMU_GID_DDR, BIT(14)),
+	CXL_PMU_EVENT_CXL_ATTR(ddr_incoming_wr_req,		CXL_PMU_GID_DDR, BIT(15)),
+	/* CXL 4.0 Table 13-5 QUEUE OCCUPANCY events opcodes */
+	CXL_PMU_EVENT_CXL_ATTR(rd_queue_occ,			CXL_PMU_GID_QUEUE_OCC, BIT(0)),
+	CXL_PMU_EVENT_CXL_ATTR(wr_queue_occ,			CXL_PMU_GID_QUEUE_OCC, BIT(1)),
+	CXL_PMU_EVENT_CXL_ATTR(rd_wr_merged_queue_occ,		CXL_PMU_GID_QUEUE_OCC, BIT(2)),
+	CXL_PMU_EVENT_CXL_ATTR(pwrdn_event,			CXL_PMU_GID_QUEUE_OCC, BIT(3)),
+	/* CXL 4.0 Table 13-5 QUEUE RESIDENCY events opcodes */
+	CXL_PMU_EVENT_CXL_ATTR(mc_rd_resid_cnt,			CXL_PMU_GID_QUEUE_RESID, BIT(0)),
+	CXL_PMU_EVENT_CXL_ATTR(mc_wr_resid_cnt,			CXL_PMU_GID_QUEUE_RESID, BIT(1)),
+	/* CXL 4.0 Table 13-5 RETRY events opcodes */
+	CXL_PMU_EVENT_CXL_ATTR(retry_event_trig_by_rd_crc,      CXL_PMU_GID_RETRY_EVENTS, BIT(0)),
+	CXL_PMU_EVENT_CXL_ATTR(retry_event_trig_by_wr_crc,      CXL_PMU_GID_RETRY_EVENTS, BIT(1)),
+	CXL_PMU_EVENT_CXL_ATTR(retry_event_trig_by_ca_parity,   CXL_PMU_GID_RETRY_EVENTS, BIT(2)),
+	CXL_PMU_EVENT_CXL_ATTR(retry_event_trig_by_ecc,         CXL_PMU_GID_RETRY_EVENTS, BIT(3)),
+	/* CXL 4.0 Table 13-5 THROTTLE events opcodes */
+	CXL_PMU_EVENT_CXL_ATTR(thermal_throttle_event,		CXL_PMU_GID_THROTTLE, BIT(0)),
+	CXL_PMU_EVENT_CXL_ATTR(power_throttle_event,		CXL_PMU_GID_THROTTLE, BIT(1)),
 	NULL
 };
 
-- 
2.39.5

[PATCH v3 3/3] perf/cxlpmu: Support Channel/Rank/Bank filter

[Davidlohr Bueso]

From: Harshal Thakkar <harshal.t@samsung.com>

Implement CRB filtering per CXL 4.0 8.2.7.2.2, and extend the
current filtering support beyond HDM. CRB filtering is only
permitted for the DDR Interface, Queue Occupancy, Queue Residency
and Retry event groups (CXL 4.0 Table 13-5), and only when counting
a single event (a single mask bit). For example, to count DDR
activates on channel 2 only:

  perf stat -a -e cxl_pmu_mem0.0/ddr_act,crb_filter_en=1,crb=0x02FFFFFF/

Placing the 32-bit CRB value at config2:32-63 leaves the
existing HDM value at config2:0-15 untouched and avoids needing
a new config3.

Signed-off-by: Harshal Thakkar <harshal.t@samsung.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/perf/cxl_pmu.c | 63 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 1fc83858f653..ca3361eb832d 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -110,6 +110,7 @@ struct cxl_pmu_info {
 	int on_cpu;
 	struct hlist_node node;
 	bool filter_hdm;
+	bool filter_crb;
 	int irq;
 };
 
@@ -146,6 +147,8 @@ static int cxl_pmu_parse_caps(struct device *dev, struct cxl_pmu_info *info)
 	info->num_event_capabilities = FIELD_GET(CXL_PMU_CAP_NUM_EVN_CAP_REG_SUP_MSK, val) + 1;
 
 	info->filter_hdm = FIELD_GET(CXL_PMU_CAP_FILTERS_SUP_MSK, val) & CXL_PMU_FILTER_HDM;
+	info->filter_crb = FIELD_GET(CXL_PMU_CAP_FILTERS_SUP_MSK, val) &
+		CXL_PMU_FILTER_CHAN_RANK_BANK;
 	if (FIELD_GET(CXL_PMU_CAP_INT, val))
 		info->irq = FIELD_GET(CXL_PMU_CAP_MSI_N_MSK, val);
 	else
@@ -229,6 +232,8 @@ enum {
 	cxl_pmu_edge_attr,
 	cxl_pmu_hdm_filter_en_attr,
 	cxl_pmu_hdm_attr,
+	cxl_pmu_crb_filter_en_attr,
+	cxl_pmu_crb_attr,
 };
 
 static struct attribute *cxl_pmu_format_attr[] = {
@@ -240,6 +245,8 @@ static struct attribute *cxl_pmu_format_attr[] = {
 	[cxl_pmu_edge_attr] = CXL_PMU_FORMAT_ATTR(edge, "config1:17"),
 	[cxl_pmu_hdm_filter_en_attr] = CXL_PMU_FORMAT_ATTR(hdm_filter_en, "config1:18"),
 	[cxl_pmu_hdm_attr] = CXL_PMU_FORMAT_ATTR(hdm, "config2:0-15"),
+	[cxl_pmu_crb_filter_en_attr] = CXL_PMU_FORMAT_ATTR(crb_filter_en, "config1:19"),
+	[cxl_pmu_crb_attr] = CXL_PMU_FORMAT_ATTR(crb, "config2:32-63"),
 	NULL
 };
 
@@ -250,7 +257,9 @@ static struct attribute *cxl_pmu_format_attr[] = {
 #define CXL_PMU_ATTR_CONFIG1_INVERT_MSK		BIT(16)
 #define CXL_PMU_ATTR_CONFIG1_EDGE_MSK		BIT(17)
 #define CXL_PMU_ATTR_CONFIG1_FILTER_EN_MSK	BIT(18)
+#define CXL_PMU_ATTR_CONFIG1_CRB_FILTER_EN_MSK	BIT(19)
 #define CXL_PMU_ATTR_CONFIG2_HDM_MSK		GENMASK(15, 0)
+#define CXL_PMU_ATTR_CONFIG2_CRB_MSK		GENMASK_ULL(63, 32)
 
 static umode_t cxl_pmu_format_is_visible(struct kobject *kobj,
 					 struct attribute *attr, int a)
@@ -267,6 +276,11 @@ static umode_t cxl_pmu_format_is_visible(struct kobject *kobj,
 	     attr == cxl_pmu_format_attr[cxl_pmu_hdm_attr]))
 		return 0;
 
+	if (!info->filter_crb &&
+	    (attr == cxl_pmu_format_attr[cxl_pmu_crb_filter_en_attr] ||
+	     attr == cxl_pmu_format_attr[cxl_pmu_crb_attr]))
+		return 0;
+
 	return attr->mode;
 }
 
@@ -323,6 +337,17 @@ static u16 cxl_pmu_config2_get_hdm_decoder(struct perf_event *event)
 	return FIELD_GET(CXL_PMU_ATTR_CONFIG2_HDM_MSK, event->attr.config2);
 }
 
+static u16 cxl_pmu_config1_crb_filter_en(struct perf_event *event)
+{
+	return FIELD_GET(CXL_PMU_ATTR_CONFIG1_CRB_FILTER_EN_MSK,
+			 event->attr.config1);
+}
+
+static u32 cxl_pmu_config2_get_crb(struct perf_event *event)
+{
+	return FIELD_GET(CXL_PMU_ATTR_CONFIG2_CRB_MSK, event->attr.config2);
+}
+
 static ssize_t cxl_pmu_event_sysfs_show(struct device *dev,
 					struct device_attribute *attr, char *buf)
 {
@@ -621,6 +646,32 @@ static int cxl_pmu_event_init(struct perf_event *event)
 		return -EOPNOTSUPP;
 	/* TODO: Validation of any filter */
 
+	if (cxl_pmu_config1_crb_filter_en(event)) {
+		if (!info->filter_crb)
+			return -EINVAL;
+		/*
+		 * CRB filtering (Filter ID 1) is only valid for the DDR
+		 * Interface, Queue Occupancy, Queue Residency and Retry
+		 * event groups (CXL 4.0 Table 13-5).
+		 */
+		switch (cxl_pmu_config_get_gid(event)) {
+		case CXL_PMU_GID_DDR:
+		case CXL_PMU_GID_QUEUE_OCC:
+		case CXL_PMU_GID_QUEUE_RESID:
+		case CXL_PMU_GID_RETRY_EVENTS:
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		/*
+		 * Filtering while counting multiple events is
+		 * undefined behavior.
+		 */
+		if (hweight32(cxl_pmu_config_get_mask(event)) > 1)
+			return -EINVAL;
+	}
+
 	/*
 	 * Verify that it is possible to count what was requested. Either must
 	 * be a fixed counter that is a precise match or a configurable counter
@@ -677,8 +728,8 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
 	hwc->state = 0;
 
 	/*
-	 * Currently only hdm filter control is implemented, this code will
-	 * want generalizing when more filters are added.
+	 * Filter ID=0: HDM decoder filter
+	 * Filter ID=1: Channel/Rank/Bank (CRB) filter
 	 */
 	if (info->filter_hdm) {
 		if (cxl_pmu_config1_hdm_filter_en(event))
@@ -688,6 +739,14 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
 		writel(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 0));
 	}
 
+	if (info->filter_crb) {
+		if (cxl_pmu_config1_crb_filter_en(event))
+			cfg = cxl_pmu_config2_get_crb(event);
+		else
+			cfg = GENMASK(31, 0); /* no filtering if 0xFFFF_FFFF */
+		writel(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 1));
+	}
+
 	cfg = readq(base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
 	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_INT_ON_OVRFLW, 1);
 	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_FREEZE_ON_OVRFLW, 1);
-- 
2.39.5

Re: [PATCH v3 3/3] perf/cxlpmu: Support Channel/Rank/Bank filter

[Richard Cheng]

On Sun, Jul 12, 2026 at 11:11:12PM +0800, Davidlohr Bueso wrote:
> From: Harshal Thakkar <harshal.t@samsung.com>
> 
> Implement CRB filtering per CXL 4.0 8.2.7.2.2, and extend the
> current filtering support beyond HDM. CRB filtering is only
> permitted for the DDR Interface, Queue Occupancy, Queue Residency
> and Retry event groups (CXL 4.0 Table 13-5), and only when counting
> a single event (a single mask bit). For example, to count DDR
> activates on channel 2 only:
> 
>   perf stat -a -e cxl_pmu_mem0.0/ddr_act,crb_filter_en=1,crb=0x02FFFFFF/
> 
> Placing the 32-bit CRB value at config2:32-63 leaves the
> existing HDM value at config2:0-15 untouched and avoids needing
> a new config3.
> 
> Signed-off-by: Harshal Thakkar <harshal.t@samsung.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  drivers/perf/cxl_pmu.c | 63 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 61 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 1fc83858f653..ca3361eb832d 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -110,6 +110,7 @@ struct cxl_pmu_info {
>  	int on_cpu;
>  	struct hlist_node node;
>  	bool filter_hdm;
> +	bool filter_crb;
>  	int irq;
>  };
>  
> @@ -146,6 +147,8 @@ static int cxl_pmu_parse_caps(struct device *dev, struct cxl_pmu_info *info)
>  	info->num_event_capabilities = FIELD_GET(CXL_PMU_CAP_NUM_EVN_CAP_REG_SUP_MSK, val) + 1;
>  
>  	info->filter_hdm = FIELD_GET(CXL_PMU_CAP_FILTERS_SUP_MSK, val) & CXL_PMU_FILTER_HDM;
> +	info->filter_crb = FIELD_GET(CXL_PMU_CAP_FILTERS_SUP_MSK, val) &
> +		CXL_PMU_FILTER_CHAN_RANK_BANK;
>  	if (FIELD_GET(CXL_PMU_CAP_INT, val))
>  		info->irq = FIELD_GET(CXL_PMU_CAP_MSI_N_MSK, val);
>  	else
> @@ -229,6 +232,8 @@ enum {
>  	cxl_pmu_edge_attr,
>  	cxl_pmu_hdm_filter_en_attr,
>  	cxl_pmu_hdm_attr,
> +	cxl_pmu_crb_filter_en_attr,
> +	cxl_pmu_crb_attr,
>  };
>  
>  static struct attribute *cxl_pmu_format_attr[] = {
> @@ -240,6 +245,8 @@ static struct attribute *cxl_pmu_format_attr[] = {
>  	[cxl_pmu_edge_attr] = CXL_PMU_FORMAT_ATTR(edge, "config1:17"),
>  	[cxl_pmu_hdm_filter_en_attr] = CXL_PMU_FORMAT_ATTR(hdm_filter_en, "config1:18"),
>  	[cxl_pmu_hdm_attr] = CXL_PMU_FORMAT_ATTR(hdm, "config2:0-15"),
> +	[cxl_pmu_crb_filter_en_attr] = CXL_PMU_FORMAT_ATTR(crb_filter_en, "config1:19"),
> +	[cxl_pmu_crb_attr] = CXL_PMU_FORMAT_ATTR(crb, "config2:32-63"),
>  	NULL
>  };
>  
> @@ -250,7 +257,9 @@ static struct attribute *cxl_pmu_format_attr[] = {
>  #define CXL_PMU_ATTR_CONFIG1_INVERT_MSK		BIT(16)
>  #define CXL_PMU_ATTR_CONFIG1_EDGE_MSK		BIT(17)
>  #define CXL_PMU_ATTR_CONFIG1_FILTER_EN_MSK	BIT(18)
> +#define CXL_PMU_ATTR_CONFIG1_CRB_FILTER_EN_MSK	BIT(19)
>  #define CXL_PMU_ATTR_CONFIG2_HDM_MSK		GENMASK(15, 0)
> +#define CXL_PMU_ATTR_CONFIG2_CRB_MSK		GENMASK_ULL(63, 32)
>  
>  static umode_t cxl_pmu_format_is_visible(struct kobject *kobj,
>  					 struct attribute *attr, int a)
> @@ -267,6 +276,11 @@ static umode_t cxl_pmu_format_is_visible(struct kobject *kobj,
>  	     attr == cxl_pmu_format_attr[cxl_pmu_hdm_attr]))
>  		return 0;
>  
> +	if (!info->filter_crb &&
> +	    (attr == cxl_pmu_format_attr[cxl_pmu_crb_filter_en_attr] ||
> +	     attr == cxl_pmu_format_attr[cxl_pmu_crb_attr]))
> +		return 0;
> +
>  	return attr->mode;
>  }
>  
> @@ -323,6 +337,17 @@ static u16 cxl_pmu_config2_get_hdm_decoder(struct perf_event *event)
>  	return FIELD_GET(CXL_PMU_ATTR_CONFIG2_HDM_MSK, event->attr.config2);
>  }
>  
> +static u16 cxl_pmu_config1_crb_filter_en(struct perf_event *event)
> +{
> +	return FIELD_GET(CXL_PMU_ATTR_CONFIG1_CRB_FILTER_EN_MSK,
> +			 event->attr.config1);
> +}
> +
> +static u32 cxl_pmu_config2_get_crb(struct perf_event *event)
> +{
> +	return FIELD_GET(CXL_PMU_ATTR_CONFIG2_CRB_MSK, event->attr.config2);
> +}
> +
>  static ssize_t cxl_pmu_event_sysfs_show(struct device *dev,
>  					struct device_attribute *attr, char *buf)
>  {
> @@ -621,6 +646,32 @@ static int cxl_pmu_event_init(struct perf_event *event)
>  		return -EOPNOTSUPP;
>  	/* TODO: Validation of any filter */
>  
> +	if (cxl_pmu_config1_crb_filter_en(event)) {
> +		if (!info->filter_crb)
> +			return -EINVAL;
> +		/*
> +		 * CRB filtering (Filter ID 1) is only valid for the DDR
> +		 * Interface, Queue Occupancy, Queue Residency and Retry
> +		 * event groups (CXL 4.0 Table 13-5).
> +		 */
> +		switch (cxl_pmu_config_get_gid(event)) {
> +		case CXL_PMU_GID_DDR:
> +		case CXL_PMU_GID_QUEUE_OCC:

Should we also validate the Event Vendor ID?

GID namespace is scoped by the Vendor ID, while the groups listed
in CXL 4.0 Table 13-5 are under CXL vendor ID.
A vendor-specific event could reuse one of these numeric GID and pass
this switch even though the CXL CRB rules don't apply to it.

I would suggest to reject non-CXL events before the switch
"""
if (cxl_pmu_config_get_vid(event) != PCI_VENDOR_ID_CXL)
    return -EINVAL;
"""

Do you think this make sense to you?

Best regards,
Richard Cheng.

> +		case CXL_PMU_GID_QUEUE_RESID:
> +		case CXL_PMU_GID_RETRY_EVENTS:
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		/*
> +		 * Filtering while counting multiple events is
> +		 * undefined behavior.
> +		 */
> +		if (hweight32(cxl_pmu_config_get_mask(event)) > 1)
> +			return -EINVAL;
> +	}
> +
>  	/*
>  	 * Verify that it is possible to count what was requested. Either must
>  	 * be a fixed counter that is a precise match or a configurable counter
> @@ -677,8 +728,8 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
>  	hwc->state = 0;
>  
>  	/*
> -	 * Currently only hdm filter control is implemented, this code will
> -	 * want generalizing when more filters are added.
> +	 * Filter ID=0: HDM decoder filter
> +	 * Filter ID=1: Channel/Rank/Bank (CRB) filter
>  	 */
>  	if (info->filter_hdm) {
>  		if (cxl_pmu_config1_hdm_filter_en(event))
> @@ -688,6 +739,14 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
>  		writel(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 0));
>  	}
>  
> +	if (info->filter_crb) {
> +		if (cxl_pmu_config1_crb_filter_en(event))
> +			cfg = cxl_pmu_config2_get_crb(event);
> +		else
> +			cfg = GENMASK(31, 0); /* no filtering if 0xFFFF_FFFF */
> +		writel(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 1));
> +	}
> +
>  	cfg = readq(base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
>  	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_INT_ON_OVRFLW, 1);
>  	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_FREEZE_ON_OVRFLW, 1);
> -- 
> 2.39.5
> 

Re: [PATCH v3 1/3] perf/cxlpmu: Fix 64-bit write to 32-bit HDM filter register

[Richard Cheng]

On Sun, Jul 12, 2026 at 11:11:10PM +0800, Davidlohr Bueso wrote:
> The HDM decoder filter configuration register is 32 bits wide, but the
> driver programs it with a 64-bit writeq(). The upper half of the write
> is always zero and lands in the adjacent Filter ID 1 (Channel/Rank/Bank)
> configuration register at offset+4.
> 
> Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  drivers/perf/cxl_pmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 68a54d97d2a8..39b46550a510 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -635,7 +635,7 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
>  			cfg = cxl_pmu_config2_get_hdm_decoder(event);
>  		else
>  			cfg = GENMASK(31, 0); /* No filtering if 0xFFFF_FFFF */
> -		writeq(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 0));
> +		writel(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 0));
>  	}
>  
>  	cfg = readq(base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
> -- 
> 2.39.5
>

Reviewed-by: Richard Cheng <icheng@nvidia.com> 

Re: [PATCH v3 3/3] perf/cxlpmu: Support Channel/Rank/Bank filter

[Davidlohr Bueso]

On Tue, 14 Jul 2026, Richard Cheng wrote:

>On Sun, Jul 12, 2026 at 11:11:12PM +0800, Davidlohr Bueso wrote:
>> @@ -621,6 +646,32 @@ static int cxl_pmu_event_init(struct perf_event *event)
>>		return -EOPNOTSUPP;
>>	/* TODO: Validation of any filter */
>>
>> +	if (cxl_pmu_config1_crb_filter_en(event)) {
>> +		if (!info->filter_crb)
>> +			return -EINVAL;
>> +		/*
>> +		 * CRB filtering (Filter ID 1) is only valid for the DDR
>> +		 * Interface, Queue Occupancy, Queue Residency and Retry
>> +		 * event groups (CXL 4.0 Table 13-5).
>> +		 */
>> +		switch (cxl_pmu_config_get_gid(event)) {
>> +		case CXL_PMU_GID_DDR:
>> +		case CXL_PMU_GID_QUEUE_OCC:
>
>Should we also validate the Event Vendor ID?
>
>GID namespace is scoped by the Vendor ID, while the groups listed
>in CXL 4.0 Table 13-5 are under CXL vendor ID.
>A vendor-specific event could reuse one of these numeric GID and pass
>this switch even though the CXL CRB rules don't apply to it.
>
>I would suggest to reject non-CXL events before the switch
>"""
>if (cxl_pmu_config_get_vid(event) != PCI_VENDOR_ID_CXL)
>    return -EINVAL;
>"""
>
>Do you think this make sense to you?

Good catch, yes this makes sense.

Thanks,
Davidlohr