| Message ID | 20230905211007.256385-1-alison.schofield@intel.com |
|---|---|
| State | Accepted |
| Commit | 9e4edf1a2196fa4bea6e8201f166785bd066446a |
| Headers | show |
| Series | [v3] cxl/region: Match auto-discovered region decoders by HPA range | expand |
On Tue, 5 Sep 2023 14:10:07 -0700 alison.schofield@intel.com wrote: > From: Alison Schofield <alison.schofield@intel.com> > > 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") > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > Reviewed-by: Dave Jiang <dave.jiang@intel.com> > Reviewed-by: Davidlohr Bueso <dave@stgolabs.net> LGTM as addressed only thing I found in v2. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > > 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/20230822014303.110509-1-alison.schofield@intel.com/ > > 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/20230804213004.1669658-1-alison.schofield@intel.com/ > > > drivers/cxl/core/region.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index e115ba382e04..b4c6a749406f 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -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
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index e115ba382e04..b4c6a749406f 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -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; /*