[PATCH v7 8/8] cxl: Allow auto-committed BI hdm decoders

[Davidlohr Bueso <dave@stgolabs.net>]

Allow auto-committed BI hdm decoders on sane platforms, rejecting
only broken paths (ie: one that cannot route BISnp, or BI paired
with a host-only target range type).

The respective region creation is done like any other committed
decoder - with cxlds->bi set by the time an decoder attaches.

A committed BI decoder under a window without the BI restriction is
refused (undefined behavior per the CFMWS Window Restrictions), as
is a committed decoder attaching to a region of a different
coherency model.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/cxl/core/hdm.c    | 24 +++++++++++++++++-------
 drivers/cxl/core/pci.c    | 36 +++++++++++++++++++++++++++++-------
 drivers/cxl/core/region.c | 33 +++++++++++++++++++++++++++++++++
 drivers/cxl/port.c        |  4 ++++
 4 files changed, 83 insertions(+), 14 deletions(-)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index f437fe15c6df..c5be6fe4c77a 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -1061,13 +1061,23 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
 		else
 			cxld->target_type = CXL_DECODER_DEVMEM;
 
-		/*
-		 * Autocommit BI-enabled decoders is not supported.
-		 * At this point cxlds->bi is not yet setup, so there
-		 * are no guarantees that the platform supports BI.
-		 */
-		if (FIELD_GET(CXL_HDM_DECODER0_CTRL_BI, ctrl))
-			return -ENXIO;
+		if (FIELD_GET(CXL_HDM_DECODER0_CTRL_BI, ctrl)) {
+			struct cxl_dev_state *cxlds = cxled ?
+				cxled_to_memdev(cxled)->cxlds : NULL;
+
+			if (cxld->target_type == CXL_DECODER_HOSTONLYMEM) {
+				dev_warn(&port->dev,
+					 "decoder%d.%d: BI with host-only\n",
+					 port->id, cxld->id);
+				return -ENXIO;
+			}
+			if (cxlds && !cxlds->bi_capable) {
+				dev_warn(&port->dev,
+					 "decoder%d.%d: path not BI capable\n",
+					 port->id, cxld->id);
+				return -ENXIO;
+			}
+		}
 
 		guard(rwsem_write)(&cxl_rwsem.region);
 		if (cxld->id != cxl_num_decoders_committed(port)) {
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 554058ccb1e9..8d2651e06a79 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -1083,6 +1083,18 @@ static int __cxl_bi_commit_decoder(struct device *dev, void __iomem *bi)
 				    scale, base);
 }
 
+/* Committed, or no explicit commit required */
+static bool cxl_bi_decoder_committed(void __iomem *bi)
+{
+	u32 caps = readl(bi + CXL_BI_DECODER_CAPS_OFFSET);
+	u32 sts = readl(bi + CXL_BI_DECODER_STATUS_OFFSET);
+
+	if (!FIELD_GET(CXL_BI_DECODER_CAPS_EXPLICIT_COMMIT_REQ, caps))
+		return true;
+
+	return FIELD_GET(CXL_BI_DECODER_STATUS_BI_COMMITTED, sts);
+}
+
 /* Enable or dealloc BI-ID changes in the given level of the topology. */
 static int __cxl_bi_ctrl_dport(struct cxl_dport *dport, bool enable)
 {
@@ -1127,6 +1139,16 @@ static int __cxl_bi_ctrl_dport(struct cxl_dport *dport, bool enable)
 		return 0;
 	case PCI_EXP_TYPE_DOWNSTREAM:
 		if (enable) {
+			/*
+			 * Adopt a dport that was already programmed and
+			 * committed by firmware: nothing new is enabled
+			 * below it, so no commit is due.
+			 */
+			if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl) &&
+			    !FIELD_GET(CXL_BI_DECODER_CTRL_BI_FW, ctrl) &&
+			    cxl_bi_decoder_committed(bi))
+				return 0;
+
 			value = ctrl & ~CXL_BI_DECODER_CTRL_BI_FW;
 			value |= CXL_BI_DECODER_CTRL_BI_ENABLE;
 		} else {
@@ -1173,11 +1195,11 @@ static int __cxl_bi_ctrl_endpoint(struct cxl_dev_state *cxlds, bool enable)
 
 	if (enable) {
 		if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl)) {
-			if (cxlds->bi)
-				return 0;
-			dev_err(cxlds->dev,
-				"BI already enabled in hardware\n");
-			return -EBUSY;
+			/* adopt firmware enabled */
+			if (!cxlds->bi)
+				dev_dbg(cxlds->dev,
+					"adopting firmware-enabled BI\n");
+			goto done;
 		}
 		val = ctrl | CXL_BI_DECODER_CTRL_BI_ENABLE;
 	} else {
@@ -1192,11 +1214,11 @@ static int __cxl_bi_ctrl_endpoint(struct cxl_dev_state *cxlds, bool enable)
 	}
 
 	writel(val, bi + CXL_BI_DECODER_CTRL_OFFSET);
-	cxlds->bi = enable;
 
 	dev_dbg(cxlds->dev, "BI requests %s\n",
 		str_enabled_disabled(enable));
-
+done:
+	cxlds->bi = enable;
 	return 0;
 }
 
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 76c6dc28a407..5578ef68034d 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1827,6 +1827,21 @@ static int cxl_region_attach_position(struct cxl_region *cxlr,
 	return rc;
 }
 
+/* Read back the committed BI bit of an auto-discovered decoder */
+static bool cxled_committed_bi(struct cxl_endpoint_decoder *cxled)
+{
+	struct cxl_port *port = cxled_to_port(cxled);
+	struct cxl_hdm *cxlhdm = dev_get_drvdata(&port->dev);
+	u32 ctrl;
+
+	if (!cxlhdm || !cxlhdm->regs.hdm_decoder)
+		return false;
+
+	ctrl = readl(cxlhdm->regs.hdm_decoder +
+		     CXL_HDM_DECODER0_CTRL_OFFSET(cxled->cxld.id));
+	return FIELD_GET(CXL_HDM_DECODER0_CTRL_BI, ctrl);
+}
+
 static int cxl_region_attach_auto(struct cxl_region *cxlr,
 				  struct cxl_endpoint_decoder *cxled, int pos)
 {
@@ -1839,6 +1854,16 @@ static int cxl_region_attach_auto(struct cxl_region *cxlr,
 		return -EINVAL;
 	}
 
+	/* A committed decoder may only join a region of its own flavor */
+	if (cxled_committed_bi(cxled) !=
+	    (cxlr->type == CXL_DECODER_DEVMEM &&
+	     cxl_root_decoder_is_bi(cxlr->cxlrd))) {
+		dev_err(&cxlr->dev, "%s:%s coherency model mismatch\n",
+			dev_name(&cxled_to_memdev(cxled)->dev),
+			dev_name(&cxled->cxld.dev));
+		return -ENXIO;
+	}
+
 	if (pos >= 0) {
 		dev_dbg(&cxlr->dev, "%s: expected auto position, not %d\n",
 			dev_name(&cxled->cxld.dev), pos);
@@ -3806,6 +3831,14 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 	if (part < 0)
 		return ERR_PTR(-EBUSY);
 
+	/* avoid UB */
+	if (cxled_committed_bi(cxled) && !cxl_root_decoder_is_bi(cxlrd)) {
+		dev_err(cxlmd->dev.parent,
+			"%s:%s BI decoder in a non-BI window\n",
+			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
+		return ERR_PTR(-ENXIO);
+	}
+
 	do {
 		cxlr = __create_region(cxlrd, cxlds->part[part].mode,
 				       atomic_read(&cxlrd->region_id),
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index ab3317fc1388..0c666ce00aa2 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -181,6 +181,10 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
 	if (rc)
 		return rc;
 
+	/*
+	 * Must precede region discovery so that any firmware-committed BI
+	 * decoder is adopted before its region is assembled.
+	 */
 	rc = cxl_bi_setup(port);
 	if (rc)
 		dev_dbg(&port->dev, "BI setup failed rc=%d\n", rc);
-- 
2.39.5