On 7/15/25 11:04 AM, Smita Koralahalli wrote:
> Reworked from a patch by Alison Schofield <alison.schofi...@intel.com>
>
> Previously, when CXL regions were created through autodiscovery and their
> resources overlapped with SOFT RESERVED ranges, the soft reserved resource
> remained in place after region teardown. This left the HPA range
> unavailable for reuse even after the region was destroyed.
>
> Enhance the logic to reliably remove SOFT RESERVED resources associated
> with a region, regardless of alignment or hierarchy in the iomem tree.
>
> Link:
> https://lore.kernel.org/linux-cxl/29312c0765224ae76862d59a17748c8188fb95f1.1692638817.git.alison.schofi...@intel.com/
> Co-developed-by: Alison Schofield <alison.schofi...@intel.com>
> Signed-off-by: Alison Schofield <alison.schofi...@intel.com>
> Co-developed-by: Terry Bowman <terry.bow...@amd.com>
> Signed-off-by: Terry Bowman <terry.bow...@amd.com>
> Signed-off-by: Smita Koralahalli <smita.koralahallichannabasa...@amd.com>
> ---
> drivers/cxl/acpi.c | 2 +
> drivers/cxl/core/region.c | 124 ++++++++++++++++++++++++++++++++++++++
> drivers/cxl/cxl.h | 2 +
> include/linux/ioport.h | 1 +
> kernel/resource.c | 34 +++++++++++
> 5 files changed, 163 insertions(+)
>
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 3a27289e669b..9eb8a9587dee 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -829,6 +829,8 @@ static void cxl_softreserv_mem_work_fn(struct work_struct
> *work)
> pr_debug("Timeout waiting for cxl_mem probing");
>
> wait_for_device_probe();
> +
> + cxl_region_softreserv_update();
> }
> static DECLARE_WORK(cxl_sr_work, cxl_softreserv_mem_work_fn);
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 6e5e1460068d..95951a1f1cab 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3486,6 +3486,130 @@ int cxl_add_to_region(struct cxl_endpoint_decoder
> *cxled)
> }
> EXPORT_SYMBOL_NS_GPL(cxl_add_to_region, "CXL");
>
> +static int add_soft_reserved(resource_size_t start, resource_size_t len,
> + unsigned long flags)
> +{
> + struct resource *res = kzalloc(sizeof(*res), GFP_KERNEL);
> + int rc;
> +
> + if (!res)
> + return -ENOMEM;
> +
> + *res = DEFINE_RES_NAMED_DESC(start, len, "Soft Reserved",
> + flags | IORESOURCE_MEM,
> + IORES_DESC_SOFT_RESERVED);
> +
> + rc = insert_resource(&iomem_resource, res);
> + if (rc) {
> + kfree(res);
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static void remove_soft_reserved(struct cxl_region *cxlr, struct resource
> *soft,
> + resource_size_t start, resource_size_t end)
> +{
> + struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> + resource_size_t new_start, new_end;
> + int rc;
> +
> + guard(mutex)(&cxlrd->range_lock);
> +
> + if (soft->start == start && soft->end == end) {
> + /*
> + * Exact alignment at both start and end. The entire region is
> + * removed below.
> + */
> +
> + } else if (soft->start == start || soft->end == end) {
> + /* Aligns at either resource start or end */
> + if (soft->start == start) {
> + new_start = end + 1;
> + new_end = soft->end;
> + } else {
> + new_start = soft->start;
> + new_end = start - 1;
> + }
> +
> + /*
> + * Reuse original flags as the trimmed portion retains the same
> + * memory type and access characteristics.
> + */
> + rc = add_soft_reserved(new_start, new_end - new_start + 1,
> + soft->flags);
> + if (rc)
> + dev_warn(&cxlr->dev,
> + "cannot add new soft reserved resource at
> %pa\n",
> + &new_start);
> +
> + } else {
> + /* No alignment - Split into two new soft reserved regions */
> + new_start = soft->start;
> + new_end = soft->end;
> +
> + rc = add_soft_reserved(new_start, start - new_start,
> + soft->flags);
> + if (rc)
> + dev_warn(&cxlr->dev,
> + "cannot add new soft reserved resource at
> %pa\n",
> + &new_start);
> +
> + rc = add_soft_reserved(end + 1, new_end - end, soft->flags);
> + if (rc)
> + dev_warn(&cxlr->dev,
> + "cannot add new soft reserved resource at %pa
> + 1\n",
> + &end);
> + }
> +
> + rc = remove_resource(soft);
> + if (rc)
> + dev_warn(&cxlr->dev, "cannot remove soft reserved resource
> %pr\n",
> + soft);
> +}
> +
> +static int __cxl_region_softreserv_update(struct resource *soft,
> + void *_cxlr)
> +{
> + struct cxl_region *cxlr = _cxlr;
> + struct resource *res = cxlr->params.res;
> +
> + /* Skip non-intersecting soft-reserved regions */
> + if (soft->end < res->start || soft->start > res->end)
> + return 0;
> +
> + soft = normalize_resource(soft);
> + if (!soft)
> + return -EINVAL;
> +
> + remove_soft_reserved(cxlr, soft, res->start, res->end);
> +
> + return 0;
> +}
> +
> +static int cxl_region_softreserv_update_cb(struct device *dev, void *data)
> +{
> + struct cxl_region *cxlr;
> +
> + if (!is_cxl_region(dev))
> + return 0;
> +
> + cxlr = to_cxl_region(dev);
> +
> + walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED, IORESOURCE_MEM, 0, -1,
> + cxlr, __cxl_region_softreserv_update);
No checking return value of walk_iomem_res_desc()?
> +
> + return 0;
> +}
> +
> +void cxl_region_softreserv_update(void)
> +{
> + bus_for_each_dev(&cxl_bus_type, NULL, NULL,
> + cxl_region_softreserv_update_cb);
No checking return value of bus_for_each_dev()? Is it ok to ignore all errors?
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_region_softreserv_update, "CXL");
> +
> u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa)
> {
> struct cxl_region_ref *iter;
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 3117136f0208..9f173467e497 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -862,6 +862,7 @@ struct cxl_pmem_region *to_cxl_pmem_region(struct device
> *dev);
> int cxl_add_to_region(struct cxl_endpoint_decoder *cxled);
> struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
> u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
> +void cxl_region_softreserv_update(void);
> #else
> static inline bool is_cxl_pmem_region(struct device *dev)
> {
> @@ -884,6 +885,7 @@ static inline u64 cxl_port_get_spa_cache_alias(struct
> cxl_port *endpoint,
> {
> return 0;
> }
> +static inline void cxl_region_softreserv_update(void) { }
> #endif
>
> void cxl_endpoint_parse_cdat(struct cxl_port *port);
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index e8b2d6aa4013..8693e095d32b 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -233,6 +233,7 @@ struct resource_constraint {
> extern struct resource ioport_resource;
> extern struct resource iomem_resource;
>
> +extern struct resource *normalize_resource(struct resource *res);
> extern struct resource *request_resource_conflict(struct resource *root,
> struct resource *new);
> extern int request_resource(struct resource *root, struct resource *new);
> extern int release_resource(struct resource *new);
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 8d3e6ed0bdc1..3d8dc2a59cb2 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -50,6 +50,40 @@ EXPORT_SYMBOL(iomem_resource);
>
> static DEFINE_RWLOCK(resource_lock);
>
> +/*
> + * normalize_resource
> + *
> + * The walk_iomem_res_desc() returns a copy of a resource, not a reference
> + * to the actual resource in the iomem_resource tree. As a result,
> + * __release_resource() which relies on pointer equality will fail.
> + *
> + * This helper walks the children of the resource's parent to find and
> + * return the original resource pointer that matches the given resource's
> + * start and end addresses.
> + *
> + * Return: Pointer to the matching original resource in iomem_resource, or
> + * NULL if not found or invalid input.
> + */
> +struct resource *normalize_resource(struct resource *res)
> +{
> + if (!res || !res->parent)
> + return NULL;
> +
> + read_lock(&resource_lock);
May as well go with below for consistency:
guard(read_lock)(&resource_lock);
DJ
> + for (struct resource *res_iter = res->parent->child; res_iter != NULL;
> + res_iter = res_iter->sibling) {
> + if ((res_iter->start == res->start) &&
> + (res_iter->end == res->end)) {
> + read_unlock(&resource_lock);
> + return res_iter;
> + }
> + }
> +
> + read_unlock(&resource_lock);
> + return NULL;
> +}
> +EXPORT_SYMBOL_NS_GPL(normalize_resource, "CXL");
> +
> /*
> * Return the next node of @p in pre-order tree traversal. If
> * @skip_children is true, skip the descendant nodes of @p in
