Skip to contentSkip to conversation

cxl/region: Match auto-discovered region decoders by HPA range

PATCH In review [email protected] 2 participants 2 messages Sep 5, 2023 – Sep 13, 2023 CXL

lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

PATCH v3 1/1 cxl/region: Match auto-discovered region decoders by HPA range

From: Alison Schofield <[email protected]>

Currently, when the region driver attaches a region to a port, it selects the ports next available decoder to program.

With the addition of auto-discovered regions, a port decoder has already been programmed so grabbing the next available decoder can be a mismatch when there is more than one region using the port.

The failure appears like this with CXL DEBUG enabled:

[] cxl_core:alloc_region_ref:754: cxl region0: endpoint9: HPA order violation region0:[mem 0x14780000000-0x1478fffffff flags 0x200] vs [mem 0x880000000-0x185fffffff flags 0x200] [] cxl_core:cxl_port_attach_region:972: cxl region0: endpoint9: failed to allocate region reference

When CXL DEBUG is not enabled, there is no failure message. The region just never materializes. Users can suspect this issue if they know their firmware has programmed decoders so that more than one region is using a port. Note that the problem may appear intermittently, ie not on every reboot.

Add a matching method for auto-discovered regions that finds a decoder based on an HPA range. The decoder range must exactly match the region resource parameter.

Fixes a32320b71f08 ("cxl/region: Add region autodiscovery")

---

Changes in v3:
- Update match_auto_decoder() to simply return 1 if found, 0 if not found. Conflict resolution is already done in cxl_rr_alloc_decoder()
(Jonathan, Dan)
- Add Reviewed-by tags (DaveJ, Davidlohr)
- v2: https://lore.kernel.org/linux-cxl/[email protected]/

Changes in v2:
- Use cxlr->params for HPA match rather than requiring cxled (Dan)
- dev_warn() if decoder already assigned to a region (Dan)
- Add failure footprint to commit log (Dan)
- Add Fixes Tag (Dan)
- v1: https://lore.kernel.org/linux-cxl/[email protected]/

 drivers/cxl/core/region.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)
drivers/cxl/core/region.c +23 -1
index e115ba382e04..b4c6a749406f 100644
@@ -717,13 +717,35 @@ static int match_free_decoder(struct device *dev, void *data)
 	return 0;
 }
 
+static int match_auto_decoder(struct device *dev, void *data)
+{
+	struct cxl_region_params *p = data;
+	struct cxl_decoder *cxld;
+	struct range *r;
+
+	if (!is_switch_decoder(dev))
+		return 0;
+
+	cxld = to_cxl_decoder(dev);
+	r = &cxld->hpa_range;
+
+	if (p->res && p->res->start == r->start && p->res->end == r->end)
+		return 1;
+
+	return 0;
+}
+
 static struct cxl_decoder *cxl_region_find_decoder(struct cxl_port *port,
 						   struct cxl_region *cxlr)
 {
 	struct device *dev;
 	int id = 0;
 
-	dev = device_find_child(&port->dev, &id, match_free_decoder);
+	if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags))
+		dev = device_find_child(&port->dev, &cxlr->params,
+					match_auto_decoder);
+	else
+		dev = device_find_child(&port->dev, &id, match_free_decoder);
 	if (!dev)
 		return NULL;
 	/*

base-commit: fe77cc2e5a6a7c85f5c6ef8a39d7694ffc7f41c9

Signature · 1 line
2.37.3
Reviewed-by on v3 1/1
#1 lore raw
Quoted 20 lines · On Tue, 5 Sep 2023 14:10:07 -0700 [email protected] wrote:

From: Alison Schofield <[email protected]>

Currently, when the region driver attaches a region to a port, it selects the ports next available decoder to program.

With the addition of auto-discovered regions, a port decoder has already been programmed so grabbing the next available decoder can be a mismatch when there is more than one region using the port.

The failure appears like this with CXL DEBUG enabled:

[] cxl_core:alloc_region_ref:754: cxl region0: endpoint9: HPA order violation region0:[mem 0x14780000000-0x1478fffffff flags 0x200] vs [mem 0x880000000-0x185fffffff flags 0x200] [] cxl_core:cxl_port_attach_region:972: cxl region0: endpoint9: failed to allocate region reference

When CXL DEBUG is not enabled, there is no failure message. The region just never materializes. Users can suspect this issue if they know their firmware has programmed decoders so that more than one region is using a port. Note that the problem may appear intermittently, ie not on every reboot.

Quoted 8 lines

Add a matching method for auto-discovered regions that finds a decoder based on an HPA range. The decoder range must exactly match the region resource parameter.

Fixes a32320b71f08 ("cxl/region: Add region autodiscovery")

LGTM as addressed only thing I found in v2.

Quoted 63 lines

---

Changes in v3:
- Update match_auto_decoder() to simply return 1 if found, 0 if not found. Conflict resolution is already done in cxl_rr_alloc_decoder()
(Jonathan, Dan)
- Add Reviewed-by tags (DaveJ, Davidlohr)
- v2: https://lore.kernel.org/linux-cxl/[email protected]/

Changes in v2:
- Use cxlr->params for HPA match rather than requiring cxled (Dan)
- dev_warn() if decoder already assigned to a region (Dan)
- Add failure footprint to commit log (Dan)
- Add Fixes Tag (Dan)
- v1: https://lore.kernel.org/linux-cxl/[email protected]/

 drivers/cxl/core/region.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)
drivers/cxl/core/region.c +23 -1
index e115ba382e04..b4c6a749406f 100644
@@ -717,13 +717,35 @@ static int match_free_decoder(struct device *dev, void *data)
 	return 0;
 }
 
+static int match_auto_decoder(struct device *dev, void *data)
+{
+	struct cxl_region_params *p = data;
+	struct cxl_decoder *cxld;
+	struct range *r;
+
+	if (!is_switch_decoder(dev))
+		return 0;
+
+	cxld = to_cxl_decoder(dev);
+	r = &cxld->hpa_range;
+
+	if (p->res && p->res->start == r->start && p->res->end == r->end)
+		return 1;
+
+	return 0;
+}
+
 static struct cxl_decoder *cxl_region_find_decoder(struct cxl_port *port,
 						   struct cxl_region *cxlr)
 {
 	struct device *dev;
 	int id = 0;
 
-	dev = device_find_child(&port->dev, &id, match_free_decoder);
+	if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags))
+		dev = device_find_child(&port->dev, &cxlr->params,
+					match_auto_decoder);
+	else
+		dev = device_find_child(&port->dev, &id, match_free_decoder);
 	if (!dev)
 		return NULL;
 	/*

base-commit: fe77cc2e5a6a7c85f5c6ef8a39d7694ffc7f41c9