[PATCH V4 16/17] cxl/pmem_region: Create pmem region using information parsed from LSA

Neeraj Kumar posted 17 patches 1 week, 5 days ago
[PATCH V4 16/17] cxl/pmem_region: Create pmem region using information parsed from LSA
Posted by Neeraj Kumar 1 week, 5 days ago
create_pmem_region() creates cxl region based on region information
parsed from LSA. This routine required cxl root decoder and endpoint
decoder. Therefore added cxl_find_root_decoder_by_port() and
cxl_find_free_ep_decoder(). These routines find cxl root decoder and
free endpoint decoder on cxl bus using cxl port

Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
---
 drivers/cxl/core/core.h        |  4 ++
 drivers/cxl/core/pmem_region.c | 97 ++++++++++++++++++++++++++++++++++
 drivers/cxl/core/region.c      | 13 +++--
 drivers/cxl/cxl.h              |  5 ++
 4 files changed, 115 insertions(+), 4 deletions(-)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index beeb9b7527b8..dd2efd3deb5e 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -35,6 +35,7 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
 #define CXL_REGION_TYPE(x) (&cxl_region_type)
 #define SET_CXL_REGION_ATTR(x) (&dev_attr_##x.attr),
 #define CXL_DAX_REGION_TYPE(x) (&cxl_dax_region_type)
+int verify_free_decoder(struct device *dev);
 int cxl_region_init(void);
 void cxl_region_exit(void);
 int cxl_get_poison_by_endpoint(struct cxl_port *port);
@@ -88,6 +89,9 @@ static inline struct cxl_region *to_cxl_region(struct device *dev)
 {
 	return NULL;
 }
+static inline int verify_free_decoder(struct device *dev)
+{
+}
 #define CXL_REGION_ATTR(x) NULL
 #define CXL_REGION_TYPE(x) NULL
 #define SET_CXL_REGION_ATTR(x)
diff --git a/drivers/cxl/core/pmem_region.c b/drivers/cxl/core/pmem_region.c
index be4feb73aafc..06665937c180 100644
--- a/drivers/cxl/core/pmem_region.c
+++ b/drivers/cxl/core/pmem_region.c
@@ -291,3 +291,100 @@ int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
 	cxlr->cxl_nvb = NULL;
 	return rc;
 }
+
+static int match_root_decoder(struct device *dev, const void *data)
+{
+	return is_root_decoder(dev);
+}
+
+/**
+ * cxl_find_root_decoder_by_port() - find a cxl root decoder on cxl bus
+ * @port: any descendant port in CXL port topology
+ *
+ * Caller of this function must call put_device() when done as a device ref
+ * is taken via device_find_child()
+ */
+static struct cxl_root_decoder *cxl_find_root_decoder_by_port(struct cxl_port *port)
+{
+	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
+	struct device *dev;
+
+	if (!cxl_root)
+		return NULL;
+
+	dev = device_find_child(&cxl_root->port.dev, NULL, match_root_decoder);
+	if (!dev)
+		return NULL;
+
+	return to_cxl_root_decoder(dev);
+}
+
+static int match_free_ep_decoder(struct device *dev, const void *data)
+{
+	if (!is_endpoint_decoder(dev))
+		return 0;
+
+	return verify_free_decoder(dev);
+}
+
+/**
+ * cxl_find_endpoint_decoder_by_port() - find a cxl root decoder on cxl bus
+ * @port: any descendant port in CXL port topology
+ *
+ * Caller of this function must call put_device() when done as a device ref
+ * is taken via device_find_child()
+ */
+static struct cxl_decoder *cxl_find_free_ep_decoder(struct cxl_port *port)
+{
+	struct device *dev;
+
+	dev = device_find_child(&port->dev, NULL, match_free_ep_decoder);
+	if (!dev)
+		return NULL;
+
+	return to_cxl_decoder(dev);
+}
+
+void create_pmem_region(struct nvdimm *nvdimm)
+{
+	struct cxl_nvdimm *cxl_nvd;
+	struct cxl_memdev *cxlmd;
+	struct cxl_pmem_region_params *params;
+	struct cxl_region *cxlr;
+
+	if (!nvdimm_has_cxl_region(nvdimm))
+		return;
+
+	lockdep_assert_held(&cxl_rwsem.region);
+	cxl_nvd = nvdimm_provider_data(nvdimm);
+	params = nvdimm_get_cxl_region_param(nvdimm);
+	cxlmd = cxl_nvd->cxlmd;
+
+	/* TODO: Region creation support only for interleave way == 1 */
+	if (!(params->nlabel == 1)) {
+		dev_dbg(&cxlmd->dev,
+				"Region Creation is not supported with iw > 1\n");
+		return;
+	}
+
+	struct cxl_root_decoder *cxlrd __free(put_cxl_root_decoder) =
+		cxl_find_root_decoder_by_port(cxlmd->endpoint);
+	if (!cxlrd) {
+		dev_err(&cxlmd->dev, "CXL root decoder not found\n");
+		return;
+	}
+
+	struct cxl_decoder *cxld __free(put_cxl_decoder) =
+		cxl_find_free_ep_decoder(cxlmd->endpoint);
+	if (!cxlrd) {
+		dev_err(&cxlmd->dev, "CXL endpoint decoder not found\n");
+		return;
+	}
+
+	cxlr = cxl_create_region(cxlrd, CXL_PARTMODE_PMEM,
+			atomic_read(&cxlrd->region_id),
+			params, cxld);
+	if (IS_ERR(cxlr))
+		dev_warn(&cxlmd->dev, "Region Creation failed\n");
+}
+EXPORT_SYMBOL_NS_GPL(create_pmem_region, "CXL");
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 408e139718f1..96f3cf4143b8 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -835,15 +835,12 @@ static int check_commit_order(struct device *dev, void *data)
 	return 0;
 }
 
-static int match_free_decoder(struct device *dev, const void *data)
+int verify_free_decoder(struct device *dev)
 {
 	struct cxl_port *port = to_cxl_port(dev->parent);
 	struct cxl_decoder *cxld;
 	int rc;
 
-	if (!is_switch_decoder(dev))
-		return 0;
-
 	cxld = to_cxl_decoder(dev);
 
 	if (cxld->id != port->commit_end + 1)
@@ -867,6 +864,14 @@ static int match_free_decoder(struct device *dev, const void *data)
 	return 1;
 }
 
+static int match_free_decoder(struct device *dev, const void *data)
+{
+	if (!is_switch_decoder(dev))
+		return 0;
+
+	return verify_free_decoder(dev);
+}
+
 static bool spa_maps_hpa(const struct cxl_region_params *p,
 			 const struct range *range)
 {
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 8c76c4a981bf..088841a3e238 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -792,6 +792,7 @@ struct cxl_root *find_cxl_root(struct cxl_port *port);
 DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_device(&_T->port.dev))
 DEFINE_FREE(put_cxl_port, struct cxl_port *, if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev))
 DEFINE_FREE(put_cxl_root_decoder, struct cxl_root_decoder *, if (!IS_ERR_OR_NULL(_T)) put_device(&_T->cxlsd.cxld.dev))
+DEFINE_FREE(put_cxl_decoder, struct cxl_decoder *, if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev))
 DEFINE_FREE(put_cxl_region, struct cxl_region *, if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev))
 
 int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
@@ -933,6 +934,7 @@ static inline int cxl_region_discovery(struct cxl_memdev *cxlmd)
 #ifdef CONFIG_CXL_PMEM_REGION
 bool is_cxl_pmem_region(struct device *dev);
 struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
+void create_pmem_region(struct nvdimm *nvdimm);
 #else
 static inline bool is_cxl_pmem_region(struct device *dev)
 {
@@ -942,6 +944,9 @@ static inline struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev)
 {
 	return NULL;
 }
+static inline void create_pmem_region(struct nvdimm *nvdimm)
+{
+}
 #endif
 
 void cxl_endpoint_parse_cdat(struct cxl_port *port);
-- 
2.34.1
Re: [PATCH V4 16/17] cxl/pmem_region: Create pmem region using information parsed from LSA
Posted by Dave Jiang 1 week, 5 days ago

On 11/19/25 12:52 AM, Neeraj Kumar wrote:
> create_pmem_region() creates cxl region based on region information
> parsed from LSA. This routine required cxl root decoder and endpoint
> decoder. Therefore added cxl_find_root_decoder_by_port() and
> cxl_find_free_ep_decoder(). These routines find cxl root decoder and
> free endpoint decoder on cxl bus using cxl port

Please consider:
create_pmem_region() creates CXL region based on region information
parsed from the Label Storage Area (LSA). This routine requires cxl root
decoder and endpoint decoder. Add cxl_find_root_decoder_by_port()
and cxl_find_free_ep_decoder() to find the root decoder and a free
endpoint decoder respectively.

> 
> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
> ---
>  drivers/cxl/core/core.h        |  4 ++
>  drivers/cxl/core/pmem_region.c | 97 ++++++++++++++++++++++++++++++++++
>  drivers/cxl/core/region.c      | 13 +++--
>  drivers/cxl/cxl.h              |  5 ++
>  4 files changed, 115 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index beeb9b7527b8..dd2efd3deb5e 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -35,6 +35,7 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
>  #define CXL_REGION_TYPE(x) (&cxl_region_type)
>  #define SET_CXL_REGION_ATTR(x) (&dev_attr_##x.attr),
>  #define CXL_DAX_REGION_TYPE(x) (&cxl_dax_region_type)
> +int verify_free_decoder(struct device *dev);
>  int cxl_region_init(void);
>  void cxl_region_exit(void);
>  int cxl_get_poison_by_endpoint(struct cxl_port *port);
> @@ -88,6 +89,9 @@ static inline struct cxl_region *to_cxl_region(struct device *dev)
>  {
>  	return NULL;
>  }
> +static inline int verify_free_decoder(struct device *dev)
> +{

this function needs to return something

> +}
>  #define CXL_REGION_ATTR(x) NULL
>  #define CXL_REGION_TYPE(x) NULL
>  #define SET_CXL_REGION_ATTR(x)
> diff --git a/drivers/cxl/core/pmem_region.c b/drivers/cxl/core/pmem_region.c
> index be4feb73aafc..06665937c180 100644
> --- a/drivers/cxl/core/pmem_region.c
> +++ b/drivers/cxl/core/pmem_region.c
> @@ -291,3 +291,100 @@ int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
>  	cxlr->cxl_nvb = NULL;
>  	return rc;
>  }
> +
> +static int match_root_decoder(struct device *dev, const void *data)
> +{
> +	return is_root_decoder(dev);

Is it suppose to just grab the first root decoder? If so the function should be match_first_root_decoder(). However, should the root decoder cover the region it's trying to match to? Should there be some checks to see if the region fits under the root decoder range? Also, should it not check the root decoder flags to see if it has CXL_DECODER_F_PMEM set so the CFMWS can cover PMEM?

> +}
> +
> +/**
> + * cxl_find_root_decoder_by_port() - find a cxl root decoder on cxl bus
> + * @port: any descendant port in CXL port topology
> + *
> + * Caller of this function must call put_device() when done as a device ref
> + * is taken via device_find_child()
> + */
> +static struct cxl_root_decoder *cxl_find_root_decoder_by_port(struct cxl_port *port)
> +{
> +	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
> +	struct device *dev;
> +
> +	if (!cxl_root)
> +		return NULL;
> +
> +	dev = device_find_child(&cxl_root->port.dev, NULL, match_root_decoder);
> +	if (!dev)
> +		return NULL;
> +
> +	return to_cxl_root_decoder(dev);
> +}
> +
> +static int match_free_ep_decoder(struct device *dev, const void *data)
> +{
> +	if (!is_endpoint_decoder(dev))
> +		return 0;
> +
> +	return verify_free_decoder(dev);
> +}
> +
> +/**
> + * cxl_find_endpoint_decoder_by_port() - find a cxl root decoder on cxl bus
> + * @port: any descendant port in CXL port topology
> + *
> + * Caller of this function must call put_device() when done as a device ref
> + * is taken via device_find_child()
> + */
> +static struct cxl_decoder *cxl_find_free_ep_decoder(struct cxl_port *port)
> +{
> +	struct device *dev;
> +
> +	dev = device_find_child(&port->dev, NULL, match_free_ep_decoder);
> +	if (!dev)
> +		return NULL;
> +
> +	return to_cxl_decoder(dev);
> +}
> +
> +void create_pmem_region(struct nvdimm *nvdimm)
> +{
> +	struct cxl_nvdimm *cxl_nvd;
> +	struct cxl_memdev *cxlmd;
> +	struct cxl_pmem_region_params *params;
> +	struct cxl_region *cxlr;
> +
> +	if (!nvdimm_has_cxl_region(nvdimm))
> +		return;
> +
> +	lockdep_assert_held(&cxl_rwsem.region);
> +	cxl_nvd = nvdimm_provider_data(nvdimm);
> +	params = nvdimm_get_cxl_region_param(nvdimm);
> +	cxlmd = cxl_nvd->cxlmd;
> +
> +	/* TODO: Region creation support only for interleave way == 1 */
> +	if (!(params->nlabel == 1)) {
> +		dev_dbg(&cxlmd->dev,
> +				"Region Creation is not supported with iw > 1\n");
> +		return;
> +	}
> +
> +	struct cxl_root_decoder *cxlrd __free(put_cxl_root_decoder) =
> +		cxl_find_root_decoder_by_port(cxlmd->endpoint);
> +	if (!cxlrd) {
> +		dev_err(&cxlmd->dev, "CXL root decoder not found\n");
> +		return;
> +	}
> +
> +	struct cxl_decoder *cxld __free(put_cxl_decoder) =
> +		cxl_find_free_ep_decoder(cxlmd->endpoint);
> +	if (!cxlrd) {
> +		dev_err(&cxlmd->dev, "CXL endpoint decoder not found\n");
> +		return;
> +	}
> +
> +	cxlr = cxl_create_region(cxlrd, CXL_PARTMODE_PMEM,
> +			atomic_read(&cxlrd->region_id),
> +			params, cxld);
> +	if (IS_ERR(cxlr))
> +		dev_warn(&cxlmd->dev, "Region Creation failed\n");
> +}
> +EXPORT_SYMBOL_NS_GPL(create_pmem_region, "CXL");
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 408e139718f1..96f3cf4143b8 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -835,15 +835,12 @@ static int check_commit_order(struct device *dev, void *data)
>  	return 0;
>  }
>  
> -static int match_free_decoder(struct device *dev, const void *data)
> +int verify_free_decoder(struct device *dev)

I would call it is_free_decoder() instead. Probably ok to return bool instead of int.

DJ

>  {
>  	struct cxl_port *port = to_cxl_port(dev->parent);
>  	struct cxl_decoder *cxld;
>  	int rc;
>  
> -	if (!is_switch_decoder(dev))
> -		return 0;
> -
>  	cxld = to_cxl_decoder(dev);
>  
>  	if (cxld->id != port->commit_end + 1)
> @@ -867,6 +864,14 @@ static int match_free_decoder(struct device *dev, const void *data)
>  	return 1;
>  }
>  
> +static int match_free_decoder(struct device *dev, const void *data)
> +{
> +	if (!is_switch_decoder(dev))
> +		return 0;
> +
> +	return verify_free_decoder(dev);
> +}
> +
>  static bool spa_maps_hpa(const struct cxl_region_params *p,
>  			 const struct range *range)
>  {
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 8c76c4a981bf..088841a3e238 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -792,6 +792,7 @@ struct cxl_root *find_cxl_root(struct cxl_port *port);
>  DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_device(&_T->port.dev))
>  DEFINE_FREE(put_cxl_port, struct cxl_port *, if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev))
>  DEFINE_FREE(put_cxl_root_decoder, struct cxl_root_decoder *, if (!IS_ERR_OR_NULL(_T)) put_device(&_T->cxlsd.cxld.dev))
> +DEFINE_FREE(put_cxl_decoder, struct cxl_decoder *, if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev))
>  DEFINE_FREE(put_cxl_region, struct cxl_region *, if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev))
>  
>  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
> @@ -933,6 +934,7 @@ static inline int cxl_region_discovery(struct cxl_memdev *cxlmd)
>  #ifdef CONFIG_CXL_PMEM_REGION
>  bool is_cxl_pmem_region(struct device *dev);
>  struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
> +void create_pmem_region(struct nvdimm *nvdimm);
>  #else
>  static inline bool is_cxl_pmem_region(struct device *dev)
>  {
> @@ -942,6 +944,9 @@ static inline struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev)
>  {
>  	return NULL;
>  }
> +static inline void create_pmem_region(struct nvdimm *nvdimm)
> +{
> +}
>  #endif
>  
>  void cxl_endpoint_parse_cdat(struct cxl_port *port);