| Message ID | 4cf018843ee9846c5b9907b8fc7140ab66b929b6.1692638817.git.alison.schofield@intel.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cxl/region: Improve Soft Reserved resource handling | expand |
On 8/21/23 11:14, alison.schofield@intel.com wrote: > From: Alison Schofield <alison.schofield@intel.com> > > During region autodiscovery, the region driver always inserts the > region resource as a child of the root decoder, a CXL WINDOW. It > has the effect of making a soft reserved resource, with an exactly > matching address range, a child of the region resource. > > It looks like this in /proc/iomem: > > 2080000000-29dbfffffff : CXL Window 0 > 2080000000-247fffffff : region0 > 2080000000-247fffffff : Soft Reserved > > Search for soft reserved resources that include the region resource > and add the new region resource as a child of that found resource. > If a soft reserved resource is not found, insert to the root decoder > as usual. > > With this change, it looks like this: > > 2080000000-29dbfffffff : CXL Window 0 > 2080000000-247fffffff : Soft Reserved > 2080000000-247fffffff : region0 > > This odd parenting only occurs when the resources are an exact match. > When the region resource only uses part of a soft reserved resource, > the parenting appears more logical like this: > > 2080000000-29dbfffffff : CXL Window 0 > 2080000000-287fffffff : Soft Reserved > 2080000000-247fffffff : region0 > > Aside from the more logical appearance, this change is in preparation > for further cleanup in region teardown. A follow-on patch intends to > find and free soft reserved resources upon region teardown. > > Signed-off-by: Alison Schofield <alison.schofield@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/cxl/core/region.c | 57 ++++++++++++++++++++++++++++++++------- > 1 file changed, 47 insertions(+), 10 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index e115ba382e04..5c487aab15ad 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -2709,6 +2709,31 @@ static int match_region_by_range(struct device *dev, void *data) > return rc; > } > > +static int insert_resource_soft_reserved(struct resource *soft_res, void *arg) > +{ > + struct resource *parent, *new, *res = arg; > + bool found = false; > + int rc = 0; > + > + parent = soft_res->parent; > + if (!parent) > + return 0; > + > + /* Caller provides a copy of soft_res. Find the actual resource. */ > + for (new = parent->child; new; new = new->sibling) { > + if (resource_contains(new, soft_res)) { > + rc = insert_resource(new, res); > + found = true; > + break; > + } > + } > + /* Caller handles failure to find or insert resource */ > + if (!found || rc) > + return rc; > + > + return 1; > +} > + > /* Establish an empty region covering the given HPA range */ > static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd, > struct cxl_endpoint_decoder *cxled) > @@ -2755,16 +2780,28 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd, > > *res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa), > dev_name(&cxlr->dev)); > - rc = insert_resource(cxlrd->res, res); > - if (rc) { > - /* > - * Platform-firmware may not have split resources like "System > - * RAM" on CXL window boundaries see cxl_region_iomem_release() > - */ > - dev_warn(cxlmd->dev.parent, > - "%s:%s: %s %s cannot insert resource\n", > - dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev), > - __func__, dev_name(&cxlr->dev)); > + > + /* Try inserting to a Soft Reserved parent. Fallback to root decoder */ > + rc = walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED, 0, res->start, > + res->end, res, insert_resource_soft_reserved); > + if (!rc || rc == -EBUSY) > + dev_dbg(&cxlmd->dev, > + "insert %pr to soft reserved parent failed rc:%d\n", > + res, rc); > + if (rc != 1) { > + rc = insert_resource(cxlrd->res, res); > + if (rc) { > + /* > + * Platform-firmware may not have split resources > + * like "System RAM" on CXL window boundaries see > + * cxl_region_iomem_release() > + */ > + dev_warn(cxlmd->dev.parent, > + "%s:%s: %s %s cannot insert resource\n", > + dev_name(&cxlmd->dev), > + dev_name(&cxled->cxld.dev), __func__, > + dev_name(&cxlr->dev)); > + } > } > > p->res = res;
On Mon, 21 Aug 2023 11:14:36 -0700 alison.schofield@intel.com wrote: > From: Alison Schofield <alison.schofield@intel.com> > > During region autodiscovery, the region driver always inserts the > region resource as a child of the root decoder, a CXL WINDOW. It > has the effect of making a soft reserved resource, with an exactly > matching address range, a child of the region resource. > > It looks like this in /proc/iomem: > > 2080000000-29dbfffffff : CXL Window 0 > 2080000000-247fffffff : region0 > 2080000000-247fffffff : Soft Reserved > > Search for soft reserved resources that include the region resource > and add the new region resource as a child of that found resource. > If a soft reserved resource is not found, insert to the root decoder > as usual. > > With this change, it looks like this: > > 2080000000-29dbfffffff : CXL Window 0 > 2080000000-247fffffff : Soft Reserved > 2080000000-247fffffff : region0 > > This odd parenting only occurs when the resources are an exact match. > When the region resource only uses part of a soft reserved resource, > the parenting appears more logical like this: > > 2080000000-29dbfffffff : CXL Window 0 > 2080000000-287fffffff : Soft Reserved > 2080000000-247fffffff : region0 > > Aside from the more logical appearance, this change is in preparation > for further cleanup in region teardown. A follow-on patch intends to > find and free soft reserved resources upon region teardown. > > Signed-off-by: Alison Schofield <alison.schofield@intel.com> One trivial comment inline. Feel free to ignore. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cxl/core/region.c | 57 ++++++++++++++++++++++++++++++++------- > 1 file changed, 47 insertions(+), 10 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index e115ba382e04..5c487aab15ad 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -2709,6 +2709,31 @@ static int match_region_by_range(struct device *dev, void *data) > return rc; > } > > +static int insert_resource_soft_reserved(struct resource *soft_res, void *arg) > +{ > + struct resource *parent, *new, *res = arg; > + bool found = false; > + int rc = 0; > + > + parent = soft_res->parent; > + if (!parent) > + return 0; > + > + /* Caller provides a copy of soft_res. Find the actual resource. */ > + for (new = parent->child; new; new = new->sibling) { > + if (resource_contains(new, soft_res)) { > + rc = insert_resource(new, res); > + found = true; > + break; > + } > + } > + /* Caller handles failure to find or insert resource */ > + if (!found || rc) > + return rc; rc is only set in the found path, so could move this check up there to simplify flow a tiny bit. Not that important... > + > + return 1; > +} > + > /* Establish an empty region covering the given HPA range */ > static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd, > struct cxl_endpoint_decoder *cxled) > @@ -2755,16 +2780,28 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd, > > *res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa), > dev_name(&cxlr->dev)); > - rc = insert_resource(cxlrd->res, res); > - if (rc) { > - /* > - * Platform-firmware may not have split resources like "System > - * RAM" on CXL window boundaries see cxl_region_iomem_release() > - */ > - dev_warn(cxlmd->dev.parent, > - "%s:%s: %s %s cannot insert resource\n", > - dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev), > - __func__, dev_name(&cxlr->dev)); > + > + /* Try inserting to a Soft Reserved parent. Fallback to root decoder */ > + rc = walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED, 0, res->start, > + res->end, res, insert_resource_soft_reserved); > + if (!rc || rc == -EBUSY) > + dev_dbg(&cxlmd->dev, > + "insert %pr to soft reserved parent failed rc:%d\n", > + res, rc); > + if (rc != 1) { > + rc = insert_resource(cxlrd->res, res); > + if (rc) { > + /* > + * Platform-firmware may not have split resources > + * like "System RAM" on CXL window boundaries see > + * cxl_region_iomem_release() > + */ > + dev_warn(cxlmd->dev.parent, > + "%s:%s: %s %s cannot insert resource\n", > + dev_name(&cxlmd->dev), > + dev_name(&cxled->cxld.dev), __func__, > + dev_name(&cxlr->dev)); > + } > } > > p->res = res;
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index e115ba382e04..5c487aab15ad 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -2709,6 +2709,31 @@ static int match_region_by_range(struct device *dev, void *data) return rc; } +static int insert_resource_soft_reserved(struct resource *soft_res, void *arg) +{ + struct resource *parent, *new, *res = arg; + bool found = false; + int rc = 0; + + parent = soft_res->parent; + if (!parent) + return 0; + + /* Caller provides a copy of soft_res. Find the actual resource. */ + for (new = parent->child; new; new = new->sibling) { + if (resource_contains(new, soft_res)) { + rc = insert_resource(new, res); + found = true; + break; + } + } + /* Caller handles failure to find or insert resource */ + if (!found || rc) + return rc; + + return 1; +} + /* Establish an empty region covering the given HPA range */ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd, struct cxl_endpoint_decoder *cxled) @@ -2755,16 +2780,28 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd, *res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa), dev_name(&cxlr->dev)); - rc = insert_resource(cxlrd->res, res); - if (rc) { - /* - * Platform-firmware may not have split resources like "System - * RAM" on CXL window boundaries see cxl_region_iomem_release() - */ - dev_warn(cxlmd->dev.parent, - "%s:%s: %s %s cannot insert resource\n", - dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev), - __func__, dev_name(&cxlr->dev)); + + /* Try inserting to a Soft Reserved parent. Fallback to root decoder */ + rc = walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED, 0, res->start, + res->end, res, insert_resource_soft_reserved); + if (!rc || rc == -EBUSY) + dev_dbg(&cxlmd->dev, + "insert %pr to soft reserved parent failed rc:%d\n", + res, rc); + if (rc != 1) { + rc = insert_resource(cxlrd->res, res); + if (rc) { + /* + * Platform-firmware may not have split resources + * like "System RAM" on CXL window boundaries see + * cxl_region_iomem_release() + */ + dev_warn(cxlmd->dev.parent, + "%s:%s: %s %s cannot insert resource\n", + dev_name(&cxlmd->dev), + dev_name(&cxled->cxld.dev), __func__, + dev_name(&cxlr->dev)); + } } p->res = res;