Re: [PATCH V4 10/17] cxl/mem: Refactor cxl pmem region auto-assembling
On 19/11/25 01:44PM, Dave Jiang wrote: On 11/19/25 12:52 AM, Neeraj Kumar wrote: In 84ec985944ef3, devm_cxl_add_nvdimm() sequence was changed and called before devm_cxl_add_endpoint(). It's because cxl pmem region auto-assembly used to get called at last in cxl_endpoint_port_probe(), which requires cxl_nvd presence. For cxl region persistency, region creation happens during nvdimm_probe which need the completion of endpoint probe. In order to accommodate both cxl pmem region auto-assembly and cxl region persistency, refactored following 1. Re-Sequence devm_cxl_add_nvdimm() after devm_cxl_add_endpoint(). This will be called only after successful completion of endpoint probe. 2. Create cxl_region_discovery() which performs pmem region auto-assembly and remove cxl pmem region auto-assembly from cxl_endpoint_port_probe() 3. Register cxl_region_discovery() with devm_cxl_add_memdev() which gets called during cxl_pci_probe() in context of cxl_mem_probe() 4. As cxlmd->ops->probe() calls registered cxl_region_discovery(), so move devm_cxl_add_nvdimm() before cxlmd->ops->probe(). It gurantees s/gurantees/guarantees/ Fixed it in V5 both the completion of endpoint probe and cxl_nvd presence before calling cxlmd->ops->probe(). Signed-off-by: Neeraj Kumar Reviewed-by: Dave Jiang Thanks Dave for RB tag Regards, Neeraj
Re: [PATCH V4 10/17] cxl/mem: Refactor cxl pmem region auto-assembling
On 11/19/25 12:52 AM, Neeraj Kumar wrote:
> In 84ec985944ef3, devm_cxl_add_nvdimm() sequence was changed and called
> before devm_cxl_add_endpoint(). It's because cxl pmem region auto-assembly
> used to get called at last in cxl_endpoint_port_probe(), which requires
> cxl_nvd presence.
>
> For cxl region persistency, region creation happens during nvdimm_probe
> which need the completion of endpoint probe.
>
> In order to accommodate both cxl pmem region auto-assembly and cxl region
> persistency, refactored following
>
> 1. Re-Sequence devm_cxl_add_nvdimm() after devm_cxl_add_endpoint(). This
>will be called only after successful completion of endpoint probe.
>
> 2. Create cxl_region_discovery() which performs pmem region
>auto-assembly and remove cxl pmem region auto-assembly from
>cxl_endpoint_port_probe()
>
> 3. Register cxl_region_discovery() with devm_cxl_add_memdev() which gets
>called during cxl_pci_probe() in context of cxl_mem_probe()
>
> 4. As cxlmd->ops->probe() calls registered cxl_region_discovery(), so
>move devm_cxl_add_nvdimm() before cxlmd->ops->probe(). It gurantees
s/gurantees/guarantees/
>both the completion of endpoint probe and cxl_nvd presence before
>calling cxlmd->ops->probe().
>
> Signed-off-by: Neeraj Kumar
Reviewed-by: Dave Jiang
> ---
> drivers/cxl/core/region.c | 37 +
> drivers/cxl/cxl.h | 5 +
> drivers/cxl/mem.c | 18 +-
> drivers/cxl/pci.c | 4 +++-
> drivers/cxl/port.c| 39 +--
> 5 files changed, 55 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 2cf5b29cefd2..3c868c4de4ec 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3724,6 +3724,43 @@ int cxl_add_to_region(struct cxl_endpoint_decoder
> *cxled)
> }
> EXPORT_SYMBOL_NS_GPL(cxl_add_to_region, "CXL");
>
> +static int discover_region(struct device *dev, void *unused)
> +{
> + struct cxl_endpoint_decoder *cxled;
> + int rc;
> +
> + if (!is_endpoint_decoder(dev))
> + return 0;
> +
> + cxled = to_cxl_endpoint_decoder(dev);
> + if ((cxled->cxld.flags & CXL_DECODER_F_ENABLE) == 0)
> + return 0;
> +
> + if (cxled->state != CXL_DECODER_STATE_AUTO)
> + return 0;
> +
> + /*
> + * Region enumeration is opportunistic, if this add-event fails,
> + * continue to the next endpoint decoder.
> + */
> + rc = cxl_add_to_region(cxled);
> + if (rc)
> + dev_dbg(dev, "failed to add to region: %#llx-%#llx\n",
> + cxled->cxld.hpa_range.start, cxled->cxld.hpa_range.end);
> +
> + return 0;
> +}
> +
> +int cxl_region_discovery(struct cxl_memdev *cxlmd)
> +{
> + struct cxl_port *port = cxlmd->endpoint;
> +
> + device_for_each_child(&port->dev, NULL, discover_region);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_region_discovery, "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 ba17fa86d249..684a0d1b441a 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -904,6 +904,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);
> +int cxl_region_discovery(struct cxl_memdev *cxlmd);
> #else
> static inline bool is_cxl_pmem_region(struct device *dev)
> {
> @@ -926,6 +927,10 @@ static inline u64 cxl_port_get_spa_cache_alias(struct
> cxl_port *endpoint,
> {
> return 0;
> }
> +static inline int cxl_region_discovery(struct cxl_memdev *cxlmd)
> +{
> + return 0;
> +}
> #endif
>
> void cxl_endpoint_parse_cdat(struct cxl_port *port);
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 13d9e089ecaf..f5e3e2fca86c 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -115,15 +115,6 @@ static int cxl_mem_probe(struct device *dev)
> return -ENXIO;
> }
>
> - if (cxl_pmem_size(cxlds) && IS_ENABLED(CONFIG_CXL_PMEM)) {
> - rc = devm_cxl_add_nvdimm(parent_port, cxlmd);
> - if (rc) {
> - if (rc == -ENODEV)
> - dev_info(dev, "PMEM disabled by platform\n");
> - return rc;
> - }
> - }
> -
> if (dport->rch)
> endpoint_parent = parent_port->uport_dev;
> else
> @@ -143,6 +134,15 @@ static int cxl_mem_probe(struct device *dev)
> return rc;
> }
>
> + if (cxl_pmem_size(cxlds) && IS_ENABLED(CONFIG_CXL_PMEM)) {
> + rc = devm_cxl_add_nvdimm(parent_port, cxlmd);
> + if (rc)
[PATCH V4 10/17] cxl/mem: Refactor cxl pmem region auto-assembling
In 84ec985944ef3, devm_cxl_add_nvdimm() sequence was changed and called
before devm_cxl_add_endpoint(). It's because cxl pmem region auto-assembly
used to get called at last in cxl_endpoint_port_probe(), which requires
cxl_nvd presence.
For cxl region persistency, region creation happens during nvdimm_probe
which need the completion of endpoint probe.
In order to accommodate both cxl pmem region auto-assembly and cxl region
persistency, refactored following
1. Re-Sequence devm_cxl_add_nvdimm() after devm_cxl_add_endpoint(). This
will be called only after successful completion of endpoint probe.
2. Create cxl_region_discovery() which performs pmem region
auto-assembly and remove cxl pmem region auto-assembly from
cxl_endpoint_port_probe()
3. Register cxl_region_discovery() with devm_cxl_add_memdev() which gets
called during cxl_pci_probe() in context of cxl_mem_probe()
4. As cxlmd->ops->probe() calls registered cxl_region_discovery(), so
move devm_cxl_add_nvdimm() before cxlmd->ops->probe(). It gurantees
both the completion of endpoint probe and cxl_nvd presence before
calling cxlmd->ops->probe().
Signed-off-by: Neeraj Kumar
---
drivers/cxl/core/region.c | 37 +
drivers/cxl/cxl.h | 5 +
drivers/cxl/mem.c | 18 +-
drivers/cxl/pci.c | 4 +++-
drivers/cxl/port.c| 39 +--
5 files changed, 55 insertions(+), 48 deletions(-)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 2cf5b29cefd2..3c868c4de4ec 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3724,6 +3724,43 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
}
EXPORT_SYMBOL_NS_GPL(cxl_add_to_region, "CXL");
+static int discover_region(struct device *dev, void *unused)
+{
+ struct cxl_endpoint_decoder *cxled;
+ int rc;
+
+ if (!is_endpoint_decoder(dev))
+ return 0;
+
+ cxled = to_cxl_endpoint_decoder(dev);
+ if ((cxled->cxld.flags & CXL_DECODER_F_ENABLE) == 0)
+ return 0;
+
+ if (cxled->state != CXL_DECODER_STATE_AUTO)
+ return 0;
+
+ /*
+* Region enumeration is opportunistic, if this add-event fails,
+* continue to the next endpoint decoder.
+*/
+ rc = cxl_add_to_region(cxled);
+ if (rc)
+ dev_dbg(dev, "failed to add to region: %#llx-%#llx\n",
+ cxled->cxld.hpa_range.start, cxled->cxld.hpa_range.end);
+
+ return 0;
+}
+
+int cxl_region_discovery(struct cxl_memdev *cxlmd)
+{
+ struct cxl_port *port = cxlmd->endpoint;
+
+ device_for_each_child(&port->dev, NULL, discover_region);
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_region_discovery, "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 ba17fa86d249..684a0d1b441a 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -904,6 +904,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);
+int cxl_region_discovery(struct cxl_memdev *cxlmd);
#else
static inline bool is_cxl_pmem_region(struct device *dev)
{
@@ -926,6 +927,10 @@ static inline u64 cxl_port_get_spa_cache_alias(struct
cxl_port *endpoint,
{
return 0;
}
+static inline int cxl_region_discovery(struct cxl_memdev *cxlmd)
+{
+ return 0;
+}
#endif
void cxl_endpoint_parse_cdat(struct cxl_port *port);
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 13d9e089ecaf..f5e3e2fca86c 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -115,15 +115,6 @@ static int cxl_mem_probe(struct device *dev)
return -ENXIO;
}
- if (cxl_pmem_size(cxlds) && IS_ENABLED(CONFIG_CXL_PMEM)) {
- rc = devm_cxl_add_nvdimm(parent_port, cxlmd);
- if (rc) {
- if (rc == -ENODEV)
- dev_info(dev, "PMEM disabled by platform\n");
- return rc;
- }
- }
-
if (dport->rch)
endpoint_parent = parent_port->uport_dev;
else
@@ -143,6 +134,15 @@ static int cxl_mem_probe(struct device *dev)
return rc;
}
+ if (cxl_pmem_size(cxlds) && IS_ENABLED(CONFIG_CXL_PMEM)) {
+ rc = devm_cxl_add_nvdimm(parent_port, cxlmd);
+ if (rc) {
+ if (rc == -ENODEV)
+ dev_info(dev, "PMEM disabled by platform\n");
+ return rc;
+ }
+ }
+
if (cxlmd->ops) {
rc = cxlmd->ops->probe(cxlmd);
if (rc)
di
