| Message ID | 20250207153753.418849-14-rrichter@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: Address translation support, part 1: Cleanups and refactoring | expand |
On Fri, Feb 07, 2025 at 04:37:48PM +0100, Robert Richter wrote: > Factor out code to find the switch decoder of a port for a specific > address range. Reuse the code to search a root decoder, create the > function cxl_port_find_switch_decoder() and rework > match_root_decoder_by_range() to be usable for switch decoders too. > > Signed-off-by: Robert Richter <rrichter@amd.com> Reviewed-by: Gregory Price <gourry@gourry.net>
On Fri, Feb 07, 2025 at 04:37:48PM +0100, Robert Richter wrote: > Factor out code to find the switch decoder of a port for a specific > address range. Reuse the code to search a root decoder, create the > function cxl_port_find_switch_decoder() and rework > match_root_decoder_by_range() to be usable for switch decoders too. > > Signed-off-by: Robert Richter <rrichter@amd.com> > --- > drivers/cxl/core/region.c | 46 +++++++++++++++++++++++---------------- > 1 file changed, 27 insertions(+), 19 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index cfcd235f311e..15286acdf6d1 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c snip > @@ -3218,9 +3234,8 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) > if (!iter) > return NULL; > > - cxlrd_dev = device_find_child(&iter->dev, hpa, > - match_root_decoder_by_range); > - if (!cxlrd_dev) { > + cxld = cxl_port_find_switch_decoder(iter, hpa); > + if (!cxld) { > dev_err(cxlmd->dev.parent, > "%s:%s no CXL window for range %#llx:%#llx\n", > dev_name(&cxlmd->dev), dev_name(&cxld->dev), The dev_err() needs cleanup to align with !cxld. As it is now, fails w NULL ptr dereference. --snip
On 07.02.25 14:22:06, Alison Schofield wrote: > On Fri, Feb 07, 2025 at 04:37:48PM +0100, Robert Richter wrote: > > Factor out code to find the switch decoder of a port for a specific > > address range. Reuse the code to search a root decoder, create the > > function cxl_port_find_switch_decoder() and rework > > match_root_decoder_by_range() to be usable for switch decoders too. > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > --- > > drivers/cxl/core/region.c | 46 +++++++++++++++++++++++---------------- > > 1 file changed, 27 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > > index cfcd235f311e..15286acdf6d1 100644 > > --- a/drivers/cxl/core/region.c > > +++ b/drivers/cxl/core/region.c > > snip > > > > @@ -3218,9 +3234,8 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) > > if (!iter) > > return NULL; > > > > - cxlrd_dev = device_find_child(&iter->dev, hpa, > > - match_root_decoder_by_range); > > - if (!cxlrd_dev) { > > + cxld = cxl_port_find_switch_decoder(iter, hpa); > > + if (!cxld) { > > dev_err(cxlmd->dev.parent, > > "%s:%s no CXL window for range %#llx:%#llx\n", > > dev_name(&cxlmd->dev), dev_name(&cxld->dev), > > The dev_err() needs cleanup to align with !cxld. > As it is now, fails w NULL ptr dereference. Yes, fixed in v3. Thanks for catching this. -Robert
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index cfcd235f311e..15286acdf6d1 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -3189,20 +3189,37 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr) return rc; } -static int match_root_decoder_by_range(struct device *dev, - const void *data) +static int match_decoder_by_range(struct device *dev, const void *data) { const struct range *r1, *r2 = data; - struct cxl_root_decoder *cxlrd; + struct cxl_decoder *cxld; - if (!is_root_decoder(dev)) + if (!is_switch_decoder(dev)) return 0; - cxlrd = to_cxl_root_decoder(dev); - r1 = &cxlrd->cxlsd.cxld.hpa_range; + cxld = to_cxl_decoder(dev); + r1 = &cxld->hpa_range; return range_contains(r1, r2); } +static struct cxl_decoder * +cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa) +{ + /* + * device_find_child() increments the reference count of the + * the switch decoder's parent port to protect the reference + * to its child. The port is already a parent of the endpoint + * decoder's port, at least indirectly in the port hierarchy. + * Thus, the endpoint already holds a reference for the parent + * port of the switch decoder. Free the unnecessary reference + * here. + */ + struct device *cxld_dev __free(put_device) = + device_find_child(&port->dev, hpa, match_decoder_by_range); + + return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL; +} + static struct cxl_root_decoder * cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) { @@ -3210,7 +3227,6 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) struct cxl_port *iter = cxled_to_port(cxled); struct cxl_decoder *cxld = &cxled->cxld; struct range *hpa = &cxld->hpa_range; - struct device *cxlrd_dev; while (iter && !is_cxl_root(iter)) iter = to_cxl_port(iter->dev.parent); @@ -3218,9 +3234,8 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) if (!iter) return NULL; - cxlrd_dev = device_find_child(&iter->dev, hpa, - match_root_decoder_by_range); - if (!cxlrd_dev) { + cxld = cxl_port_find_switch_decoder(iter, hpa); + if (!cxld) { dev_err(cxlmd->dev.parent, "%s:%s no CXL window for range %#llx:%#llx\n", dev_name(&cxlmd->dev), dev_name(&cxld->dev), @@ -3228,16 +3243,9 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) return NULL; } - /* - * device_find_child() created a reference to the root - * decoder. Since the root decoder exists as long as the root - * port exists and the endpoint already holds a reference to - * the root port, this additional reference is not needed. - * Free it here. - */ - put_device(cxlrd_dev); - return to_cxl_root_decoder(cxlrd_dev); + + return to_cxl_root_decoder(&cxld->dev); } static int match_region_by_range(struct device *dev, const void *data)
Factor out code to find the switch decoder of a port for a specific address range. Reuse the code to search a root decoder, create the function cxl_port_find_switch_decoder() and rework match_root_decoder_by_range() to be usable for switch decoders too. Signed-off-by: Robert Richter <rrichter@amd.com> --- drivers/cxl/core/region.c | 46 +++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 19 deletions(-)