[ndctl PATCH v2 0/6] ndctl: DCD additions

[ndctl PATCH v2 0/6] ndctl: DCD additions

[Ira Weiny]

https://github.com/weiny2/ndctl/tree/dcd-region2-2024-11-03

CXL Dynamic Capacity Device (DCD) support is close to landing in the
upstream kernel.  cxl cli requires some modifications to best interact
with those devices.  This includes creating and viewing details of DCD
regions.  cxl-testing is also valuable in regression testing the kernel
interfaces.

Add preliminary patches with some fixes.  Update cxl cli with DCD
support and add cxl-testing.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Changes in v2:
- Fan: properly init index.
- Link to v1: https://patch.msgid.link/20241030-dcd-region2-v1-0-04600ba2b48e@intel.com

---
Ira Weiny (5):
      ndctl/cxl-events: Don't fail test until event counts are reported
      ndctl/cxl/region: Report max size for region creation
      ndctl: Separate region mode from decoder mode
      ndctl/cxl: Add extent output to region query
      ndctl/cxl/test: Add Dynamic Capacity tests

Navneet Singh (1):
      cxl/region: Add creation of Dynamic capacity regions

 Documentation/cxl/cxl-list.txt |   4 +
 cxl/filter.h                   |   3 +
 cxl/json.c                     |  79 ++++-
 cxl/json.h                     |   3 +
 cxl/lib/libcxl.c               | 248 +++++++++++++++-
 cxl/lib/libcxl.sym             |   9 +
 cxl/lib/private.h              |  19 +-
 cxl/libcxl.h                   |  99 ++++++-
 cxl/list.c                     |   3 +
 cxl/memdev.c                   |   7 +-
 cxl/region.c                   |  53 +++-
 test/cxl-dcd.sh                | 656 +++++++++++++++++++++++++++++++++++++++++
 test/cxl-events.sh             |   8 +-
 test/meson.build               |   2 +
 util/json.h                    |   1 +
 15 files changed, 1173 insertions(+), 21 deletions(-)
---
base-commit: 04815e5f8b87e02a4fb5a61aeebaa5cad25a15c3
change-id: 20241030-dcd-region2-2d0149eb8efd

Best regards,
-- 
Ira Weiny <ira.weiny@intel.com>

[ndctl PATCH v2 1/6] ndctl/cxl-events: Don't fail test until event counts are reported

[Ira Weiny]

In testing DCD event modifications a failed cxl-event test lacked
details on the event counts.  This was because the greps were failing
the test rather than the check against the counts.

Suppress the grep failure and rely on event count checks for pass/fail
of the test.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 test/cxl-events.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/cxl-events.sh b/test/cxl-events.sh
index ff4f3fdff1d8f6fd80f093126a27bf14b52d167f..c216d6aa9148c938a649cb22656127b3df440039 100644
--- a/test/cxl-events.sh
+++ b/test/cxl-events.sh
@@ -71,10 +71,10 @@ echo 0 > /sys/kernel/tracing/tracing_on
 echo "TEST: Events seen"
 trace_out=$(cat /sys/kernel/tracing/trace)
 
-num_overflow=$(grep -c "cxl_overflow" <<< "${trace_out}")
-num_fatal=$(grep -c "log=Fatal" <<< "${trace_out}")
-num_failure=$(grep -c "log=Failure" <<< "${trace_out}")
-num_info=$(grep -c "log=Informational" <<< "${trace_out}")
+num_overflow=$(grep -c "cxl_overflow" <<< "${trace_out}" || true)
+num_fatal=$(grep -c "log=Fatal" <<< "${trace_out}" || true)
+num_failure=$(grep -c "log=Failure" <<< "${trace_out}" || true)
+num_info=$(grep -c "log=Informational" <<< "${trace_out}" || true)
 echo "     LOG     (Expected) : (Found)"
 echo "     overflow      ($num_overflow_expected) : $num_overflow"
 echo "     Fatal         ($num_fatal_expected) : $num_fatal"

-- 
2.47.0

Re: [ndctl PATCH v2 1/6] ndctl/cxl-events: Don't fail test until event counts are reported

[Dave Jiang]

On 11/4/24 7:10 PM, Ira Weiny wrote:
> In testing DCD event modifications a failed cxl-event test lacked
> details on the event counts.  This was because the greps were failing
> the test rather than the check against the counts.
> 
> Suppress the grep failure and rely on event count checks for pass/fail
> of the test.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  test/cxl-events.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/test/cxl-events.sh b/test/cxl-events.sh
> index ff4f3fdff1d8f6fd80f093126a27bf14b52d167f..c216d6aa9148c938a649cb22656127b3df440039 100644
> --- a/test/cxl-events.sh
> +++ b/test/cxl-events.sh
> @@ -71,10 +71,10 @@ echo 0 > /sys/kernel/tracing/tracing_on
>  echo "TEST: Events seen"
>  trace_out=$(cat /sys/kernel/tracing/trace)
>  
> -num_overflow=$(grep -c "cxl_overflow" <<< "${trace_out}")
> -num_fatal=$(grep -c "log=Fatal" <<< "${trace_out}")
> -num_failure=$(grep -c "log=Failure" <<< "${trace_out}")
> -num_info=$(grep -c "log=Informational" <<< "${trace_out}")
> +num_overflow=$(grep -c "cxl_overflow" <<< "${trace_out}" || true)
> +num_fatal=$(grep -c "log=Fatal" <<< "${trace_out}" || true)
> +num_failure=$(grep -c "log=Failure" <<< "${trace_out}" || true)
> +num_info=$(grep -c "log=Informational" <<< "${trace_out}" || true)
>  echo "     LOG     (Expected) : (Found)"
>  echo "     overflow      ($num_overflow_expected) : $num_overflow"
>  echo "     Fatal         ($num_fatal_expected) : $num_fatal"
> 

Re: [ndctl PATCH v2 1/6] ndctl/cxl-events: Don't fail test until event counts are reported

[Alison Schofield]

On Mon, Nov 04, 2024 at 08:10:45PM -0600, Ira Weiny wrote:
> In testing DCD event modifications a failed cxl-event test lacked
> details on the event counts.  This was because the greps were failing
> the test rather than the check against the counts.
> 
> Suppress the grep failure and rely on event count checks for pass/fail
> of the test.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---

Thanks!  Applied to https://github.com/pmem/ndctl/commits/pending/
with [ alison: rm unnecessary reference to DCD in commit log ]

snip

[ndctl PATCH v2 2/6] ndctl/cxl/region: Report max size for region creation

[Ira Weiny]

When creating a region if the size exceeds the max an error is printed.
However, the max available space is not reported which makes it harder
to determine what is wrong.

Add the max size available to the output error.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 cxl/region.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cxl/region.c b/cxl/region.c
index 96aa5931d2281c7577679b7f6165218964fa0425..207cf2d003148992255c715f286bc0f38de2ca84 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -677,8 +677,8 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 	}
 	if (!default_size && size > max_extent) {
 		log_err(&rl,
-			"%s: region size %#lx exceeds max available space\n",
-			cxl_decoder_get_devname(p->root_decoder), size);
+			"%s: region size %#lx exceeds max available space (%#lx)\n",
+			cxl_decoder_get_devname(p->root_decoder), size, max_extent);
 		return -ENOSPC;
 	}
 

-- 
2.47.0

Re: [ndctl PATCH v2 2/6] ndctl/cxl/region: Report max size for region creation

[Dave Jiang]

On 11/4/24 7:10 PM, Ira Weiny wrote:
> When creating a region if the size exceeds the max an error is printed.
> However, the max available space is not reported which makes it harder
> to determine what is wrong.
> 
> Add the max size available to the output error.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  cxl/region.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/cxl/region.c b/cxl/region.c
> index 96aa5931d2281c7577679b7f6165218964fa0425..207cf2d003148992255c715f286bc0f38de2ca84 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -677,8 +677,8 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  	}
>  	if (!default_size && size > max_extent) {
>  		log_err(&rl,
> -			"%s: region size %#lx exceeds max available space\n",
> -			cxl_decoder_get_devname(p->root_decoder), size);
> +			"%s: region size %#lx exceeds max available space (%#lx)\n",
> +			cxl_decoder_get_devname(p->root_decoder), size, max_extent);
>  		return -ENOSPC;
>  	}
>  
> 

Re: [ndctl PATCH v2 2/6] ndctl/cxl/region: Report max size for region creation

[Fan Ni]

On Mon, Nov 04, 2024 at 08:10:46PM -0600, Ira Weiny wrote:
> When creating a region if the size exceeds the max an error is printed.
> However, the max available space is not reported which makes it harder
> to determine what is wrong.
> 
> Add the max size available to the output error.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Fan Ni <fan.ni@samsung.com>

> ---
>  cxl/region.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/cxl/region.c b/cxl/region.c
> index 96aa5931d2281c7577679b7f6165218964fa0425..207cf2d003148992255c715f286bc0f38de2ca84 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -677,8 +677,8 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  	}
>  	if (!default_size && size > max_extent) {
>  		log_err(&rl,
> -			"%s: region size %#lx exceeds max available space\n",
> -			cxl_decoder_get_devname(p->root_decoder), size);
> +			"%s: region size %#lx exceeds max available space (%#lx)\n",
> +			cxl_decoder_get_devname(p->root_decoder), size, max_extent);
>  		return -ENOSPC;
>  	}
>  
> 
> -- 
> 2.47.0
> 

-- 
Fan Ni

Re: [ndctl PATCH v2 2/6] ndctl/cxl/region: Report max size for region creation

[Alison Schofield]

On Mon, Nov 04, 2024 at 08:10:46PM -0600, Ira Weiny wrote:
> When creating a region if the size exceeds the max an error is printed.
> However, the max available space is not reported which makes it harder
> to determine what is wrong.
> 
> Add the max size available to the output error.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Thanks! Applied to https://github.com/pmem/ndctl/commits/pending/

snip

[ndctl PATCH v2 3/6] ndctl: Separate region mode from decoder mode

[Ira Weiny]

With the introduction of DCD, region mode and decoder mode no longer
remain a 1:1 relation.  An interleaved region may be made up of Dynamic
Capacity partitions with different indexes on each of the target
devices.

Introduce a new region mode enumeration and access function.

To maintain compatibility with existing software the region mode values
are defined the same as the current decoder mode.  In addition
cxl_region_get_mode() is retained.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 cxl/json.c         |  6 +++---
 cxl/lib/libcxl.c   | 15 ++++++++++-----
 cxl/lib/libcxl.sym |  1 +
 cxl/lib/private.h  |  2 +-
 cxl/libcxl.h       | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/cxl/json.c b/cxl/json.c
index 5066d3bed13f8fcc36ab8f0ea127685c246d94d7..dcd3cc28393faf7e8adf299a857531ecdeaac50a 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -1147,7 +1147,7 @@ void util_cxl_mappings_append_json(struct json_object *jregion,
 struct json_object *util_cxl_region_to_json(struct cxl_region *region,
 					     unsigned long flags)
 {
-	enum cxl_decoder_mode mode = cxl_region_get_mode(region);
+	enum cxl_region_mode mode = cxl_region_get_region_mode(region);
 	const char *devname = cxl_region_get_devname(region);
 	struct json_object *jregion, *jobj;
 	u64 val;
@@ -1174,8 +1174,8 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
 			json_object_object_add(jregion, "size", jobj);
 	}
 
-	if (mode != CXL_DECODER_MODE_NONE) {
-		jobj = json_object_new_string(cxl_decoder_mode_name(mode));
+	if (mode != CXL_REGION_MODE_NONE) {
+		jobj = json_object_new_string(cxl_region_mode_name(mode));
 		if (jobj)
 			json_object_object_add(jregion, "type", jobj);
 	}
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 63aa4ef3acdc2fb3c4ec6c13be5feb802e817d0d..5cbfb3e7d466b491ef87ea285f7e50d3bac230db 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -431,10 +431,10 @@ CXL_EXPORT bool cxl_region_qos_class_mismatch(struct cxl_region *region)
 		if (!memdev)
 			continue;
 
-		if (region->mode == CXL_DECODER_MODE_RAM) {
+		if (region->mode == CXL_REGION_MODE_RAM) {
 			if (root_decoder->qos_class != memdev->ram_qos_class)
 				return true;
-		} else if (region->mode == CXL_DECODER_MODE_PMEM) {
+		} else if (region->mode == CXL_REGION_MODE_PMEM) {
 			if (root_decoder->qos_class != memdev->pmem_qos_class)
 				return true;
 		}
@@ -619,9 +619,9 @@ static void *add_cxl_region(void *parent, int id, const char *cxlregion_base)
 
 	sprintf(path, "%s/mode", cxlregion_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
-		region->mode = CXL_DECODER_MODE_NONE;
+		region->mode = CXL_REGION_MODE_NONE;
 	else
-		region->mode = cxl_decoder_mode_from_ident(buf);
+		region->mode = cxl_region_mode_from_ident(buf);
 
 	sprintf(path, "%s/modalias", cxlregion_base);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
@@ -748,11 +748,16 @@ CXL_EXPORT unsigned long long cxl_region_get_resource(struct cxl_region *region)
 	return region->start;
 }
 
-CXL_EXPORT enum cxl_decoder_mode cxl_region_get_mode(struct cxl_region *region)
+CXL_EXPORT enum cxl_region_mode cxl_region_get_region_mode(struct cxl_region *region)
 {
 	return region->mode;
 }
 
+CXL_EXPORT enum cxl_decoder_mode cxl_region_get_mode(struct cxl_region *region)
+{
+	return (enum cxl_decoder_mode)cxl_region_get_region_mode(region);
+}
+
 CXL_EXPORT unsigned int
 cxl_region_get_interleave_ways(struct cxl_region *region)
 {
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 0c155a40ad4765106f0eab1745281d462af782fe..b5d9bdcc38e09812f26afc1cb0e804f86784b8e6 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -287,4 +287,5 @@ LIBECXL_8 {
 global:
 	cxl_memdev_trigger_poison_list;
 	cxl_region_trigger_poison_list;
+	cxl_region_get_region_mode;
 } LIBCXL_7;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index b6cd910e93359b53cac34427acfe84c7abcb78b0..0f45be89b6a00477d13fb6d7f1906213a3073c48 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -171,7 +171,7 @@ struct cxl_region {
 	unsigned int interleave_ways;
 	unsigned int interleave_granularity;
 	enum cxl_decode_state decode_state;
-	enum cxl_decoder_mode mode;
+	enum cxl_region_mode mode;
 	struct daxctl_region *dax_region;
 	struct kmod_module *module;
 	struct list_head mappings;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 0a5fd0e13cc24e0032d4a83d780278fbe0038d32..06b87a0924faafec6c80eca83ea7551d4e117256 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -303,6 +303,39 @@ int cxl_memdev_is_enabled(struct cxl_memdev *memdev);
 	for (endpoint = cxl_endpoint_get_first(port); endpoint != NULL;        \
 	     endpoint = cxl_endpoint_get_next(endpoint))
 
+enum cxl_region_mode {
+	CXL_REGION_MODE_NONE = CXL_DECODER_MODE_NONE,
+	CXL_REGION_MODE_MIXED = CXL_DECODER_MODE_MIXED,
+	CXL_REGION_MODE_PMEM = CXL_DECODER_MODE_PMEM,
+	CXL_REGION_MODE_RAM = CXL_DECODER_MODE_RAM,
+};
+
+static inline const char *cxl_region_mode_name(enum cxl_region_mode mode)
+{
+	static const char *names[] = {
+		[CXL_REGION_MODE_NONE] = "none",
+		[CXL_REGION_MODE_MIXED] = "mixed",
+		[CXL_REGION_MODE_PMEM] = "pmem",
+		[CXL_REGION_MODE_RAM] = "ram",
+	};
+
+	if (mode < CXL_REGION_MODE_NONE || mode > CXL_REGION_MODE_RAM)
+		mode = CXL_REGION_MODE_NONE;
+	return names[mode];
+}
+
+static inline enum cxl_region_mode
+cxl_region_mode_from_ident(const char *ident)
+{
+	if (strcmp(ident, "ram") == 0)
+		return CXL_REGION_MODE_RAM;
+	else if (strcmp(ident, "volatile") == 0)
+		return CXL_REGION_MODE_RAM;
+	else if (strcmp(ident, "pmem") == 0)
+		return CXL_REGION_MODE_PMEM;
+	return CXL_REGION_MODE_NONE;
+}
+
 struct cxl_region;
 struct cxl_region *cxl_region_get_first(struct cxl_decoder *decoder);
 struct cxl_region *cxl_region_get_next(struct cxl_region *region);
@@ -318,6 +351,8 @@ const char *cxl_region_get_devname(struct cxl_region *region);
 void cxl_region_get_uuid(struct cxl_region *region, uuid_t uu);
 unsigned long long cxl_region_get_size(struct cxl_region *region);
 unsigned long long cxl_region_get_resource(struct cxl_region *region);
+enum cxl_region_mode cxl_region_get_region_mode(struct cxl_region *region);
+/* Deprecated: use cxl_region_get_region_mode() */
 enum cxl_decoder_mode cxl_region_get_mode(struct cxl_region *region);
 unsigned int cxl_region_get_interleave_ways(struct cxl_region *region);
 unsigned int cxl_region_get_interleave_granularity(struct cxl_region *region);

-- 
2.47.0

Re: [ndctl PATCH v2 3/6] ndctl: Separate region mode from decoder mode

[Dave Jiang]

On 11/4/24 7:10 PM, Ira Weiny wrote:
> With the introduction of DCD, region mode and decoder mode no longer
> remain a 1:1 relation.  An interleaved region may be made up of Dynamic
> Capacity partitions with different indexes on each of the target
> devices.
> 
> Introduce a new region mode enumeration and access function.
> 
> To maintain compatibility with existing software the region mode values
> are defined the same as the current decoder mode.  In addition
> cxl_region_get_mode() is retained.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  cxl/json.c         |  6 +++---
>  cxl/lib/libcxl.c   | 15 ++++++++++-----
>  cxl/lib/libcxl.sym |  1 +
>  cxl/lib/private.h  |  2 +-
>  cxl/libcxl.h       | 35 +++++++++++++++++++++++++++++++++++
>  5 files changed, 50 insertions(+), 9 deletions(-)
> 
> diff --git a/cxl/json.c b/cxl/json.c
> index 5066d3bed13f8fcc36ab8f0ea127685c246d94d7..dcd3cc28393faf7e8adf299a857531ecdeaac50a 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -1147,7 +1147,7 @@ void util_cxl_mappings_append_json(struct json_object *jregion,
>  struct json_object *util_cxl_region_to_json(struct cxl_region *region,
>  					     unsigned long flags)
>  {
> -	enum cxl_decoder_mode mode = cxl_region_get_mode(region);
> +	enum cxl_region_mode mode = cxl_region_get_region_mode(region);
>  	const char *devname = cxl_region_get_devname(region);
>  	struct json_object *jregion, *jobj;
>  	u64 val;
> @@ -1174,8 +1174,8 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
>  			json_object_object_add(jregion, "size", jobj);
>  	}
>  
> -	if (mode != CXL_DECODER_MODE_NONE) {
> -		jobj = json_object_new_string(cxl_decoder_mode_name(mode));
> +	if (mode != CXL_REGION_MODE_NONE) {
> +		jobj = json_object_new_string(cxl_region_mode_name(mode));
>  		if (jobj)
>  			json_object_object_add(jregion, "type", jobj);
>  	}
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 63aa4ef3acdc2fb3c4ec6c13be5feb802e817d0d..5cbfb3e7d466b491ef87ea285f7e50d3bac230db 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -431,10 +431,10 @@ CXL_EXPORT bool cxl_region_qos_class_mismatch(struct cxl_region *region)
>  		if (!memdev)
>  			continue;
>  
> -		if (region->mode == CXL_DECODER_MODE_RAM) {
> +		if (region->mode == CXL_REGION_MODE_RAM) {
>  			if (root_decoder->qos_class != memdev->ram_qos_class)
>  				return true;
> -		} else if (region->mode == CXL_DECODER_MODE_PMEM) {
> +		} else if (region->mode == CXL_REGION_MODE_PMEM) {
>  			if (root_decoder->qos_class != memdev->pmem_qos_class)
>  				return true;
>  		}
> @@ -619,9 +619,9 @@ static void *add_cxl_region(void *parent, int id, const char *cxlregion_base)
>  
>  	sprintf(path, "%s/mode", cxlregion_base);
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
> -		region->mode = CXL_DECODER_MODE_NONE;
> +		region->mode = CXL_REGION_MODE_NONE;
>  	else
> -		region->mode = cxl_decoder_mode_from_ident(buf);
> +		region->mode = cxl_region_mode_from_ident(buf);
>  
>  	sprintf(path, "%s/modalias", cxlregion_base);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
> @@ -748,11 +748,16 @@ CXL_EXPORT unsigned long long cxl_region_get_resource(struct cxl_region *region)
>  	return region->start;
>  }
>  
> -CXL_EXPORT enum cxl_decoder_mode cxl_region_get_mode(struct cxl_region *region)
> +CXL_EXPORT enum cxl_region_mode cxl_region_get_region_mode(struct cxl_region *region)
>  {
>  	return region->mode;
>  }
>  
> +CXL_EXPORT enum cxl_decoder_mode cxl_region_get_mode(struct cxl_region *region)
> +{
> +	return (enum cxl_decoder_mode)cxl_region_get_region_mode(region);
> +}
> +
>  CXL_EXPORT unsigned int
>  cxl_region_get_interleave_ways(struct cxl_region *region)
>  {
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 0c155a40ad4765106f0eab1745281d462af782fe..b5d9bdcc38e09812f26afc1cb0e804f86784b8e6 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -287,4 +287,5 @@ LIBECXL_8 {
>  global:
>  	cxl_memdev_trigger_poison_list;
>  	cxl_region_trigger_poison_list;
> +	cxl_region_get_region_mode;
>  } LIBCXL_7;
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index b6cd910e93359b53cac34427acfe84c7abcb78b0..0f45be89b6a00477d13fb6d7f1906213a3073c48 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -171,7 +171,7 @@ struct cxl_region {
>  	unsigned int interleave_ways;
>  	unsigned int interleave_granularity;
>  	enum cxl_decode_state decode_state;
> -	enum cxl_decoder_mode mode;
> +	enum cxl_region_mode mode;
>  	struct daxctl_region *dax_region;
>  	struct kmod_module *module;
>  	struct list_head mappings;
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index 0a5fd0e13cc24e0032d4a83d780278fbe0038d32..06b87a0924faafec6c80eca83ea7551d4e117256 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -303,6 +303,39 @@ int cxl_memdev_is_enabled(struct cxl_memdev *memdev);
>  	for (endpoint = cxl_endpoint_get_first(port); endpoint != NULL;        \
>  	     endpoint = cxl_endpoint_get_next(endpoint))
>  
> +enum cxl_region_mode {
> +	CXL_REGION_MODE_NONE = CXL_DECODER_MODE_NONE,
> +	CXL_REGION_MODE_MIXED = CXL_DECODER_MODE_MIXED,
> +	CXL_REGION_MODE_PMEM = CXL_DECODER_MODE_PMEM,
> +	CXL_REGION_MODE_RAM = CXL_DECODER_MODE_RAM,
> +};
> +
> +static inline const char *cxl_region_mode_name(enum cxl_region_mode mode)
> +{
> +	static const char *names[] = {
> +		[CXL_REGION_MODE_NONE] = "none",
> +		[CXL_REGION_MODE_MIXED] = "mixed",
> +		[CXL_REGION_MODE_PMEM] = "pmem",
> +		[CXL_REGION_MODE_RAM] = "ram",
> +	};
> +
> +	if (mode < CXL_REGION_MODE_NONE || mode > CXL_REGION_MODE_RAM)
> +		mode = CXL_REGION_MODE_NONE;
> +	return names[mode];
> +}
> +
> +static inline enum cxl_region_mode
> +cxl_region_mode_from_ident(const char *ident)
> +{
> +	if (strcmp(ident, "ram") == 0)
> +		return CXL_REGION_MODE_RAM;
> +	else if (strcmp(ident, "volatile") == 0)
> +		return CXL_REGION_MODE_RAM;
> +	else if (strcmp(ident, "pmem") == 0)
> +		return CXL_REGION_MODE_PMEM;
> +	return CXL_REGION_MODE_NONE;
> +}
> +
>  struct cxl_region;
>  struct cxl_region *cxl_region_get_first(struct cxl_decoder *decoder);
>  struct cxl_region *cxl_region_get_next(struct cxl_region *region);
> @@ -318,6 +351,8 @@ const char *cxl_region_get_devname(struct cxl_region *region);
>  void cxl_region_get_uuid(struct cxl_region *region, uuid_t uu);
>  unsigned long long cxl_region_get_size(struct cxl_region *region);
>  unsigned long long cxl_region_get_resource(struct cxl_region *region);
> +enum cxl_region_mode cxl_region_get_region_mode(struct cxl_region *region);
> +/* Deprecated: use cxl_region_get_region_mode() */
>  enum cxl_decoder_mode cxl_region_get_mode(struct cxl_region *region);
>  unsigned int cxl_region_get_interleave_ways(struct cxl_region *region);
>  unsigned int cxl_region_get_interleave_granularity(struct cxl_region *region);
> 

Re: [ndctl PATCH v2 3/6] ndctl: Separate region mode from decoder mode

[Fan Ni]

On Mon, Nov 04, 2024 at 08:10:47PM -0600, Ira Weiny wrote:
> With the introduction of DCD, region mode and decoder mode no longer
> remain a 1:1 relation.  An interleaved region may be made up of Dynamic
> Capacity partitions with different indexes on each of the target
> devices.
> 
> Introduce a new region mode enumeration and access function.
> 
> To maintain compatibility with existing software the region mode values
> are defined the same as the current decoder mode.  In addition
> cxl_region_get_mode() is retained.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Fan Ni <fan.ni@samsung.com>

> ---
>  cxl/json.c         |  6 +++---
>  cxl/lib/libcxl.c   | 15 ++++++++++-----
>  cxl/lib/libcxl.sym |  1 +
>  cxl/lib/private.h  |  2 +-
>  cxl/libcxl.h       | 35 +++++++++++++++++++++++++++++++++++
>  5 files changed, 50 insertions(+), 9 deletions(-)
> 
> diff --git a/cxl/json.c b/cxl/json.c
> index 5066d3bed13f8fcc36ab8f0ea127685c246d94d7..dcd3cc28393faf7e8adf299a857531ecdeaac50a 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -1147,7 +1147,7 @@ void util_cxl_mappings_append_json(struct json_object *jregion,
>  struct json_object *util_cxl_region_to_json(struct cxl_region *region,
>  					     unsigned long flags)
>  {
> -	enum cxl_decoder_mode mode = cxl_region_get_mode(region);
> +	enum cxl_region_mode mode = cxl_region_get_region_mode(region);
>  	const char *devname = cxl_region_get_devname(region);
>  	struct json_object *jregion, *jobj;
>  	u64 val;
> @@ -1174,8 +1174,8 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
>  			json_object_object_add(jregion, "size", jobj);
>  	}
>  
> -	if (mode != CXL_DECODER_MODE_NONE) {
> -		jobj = json_object_new_string(cxl_decoder_mode_name(mode));
> +	if (mode != CXL_REGION_MODE_NONE) {
> +		jobj = json_object_new_string(cxl_region_mode_name(mode));
>  		if (jobj)
>  			json_object_object_add(jregion, "type", jobj);
>  	}
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 63aa4ef3acdc2fb3c4ec6c13be5feb802e817d0d..5cbfb3e7d466b491ef87ea285f7e50d3bac230db 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -431,10 +431,10 @@ CXL_EXPORT bool cxl_region_qos_class_mismatch(struct cxl_region *region)
>  		if (!memdev)
>  			continue;
>  
> -		if (region->mode == CXL_DECODER_MODE_RAM) {
> +		if (region->mode == CXL_REGION_MODE_RAM) {
>  			if (root_decoder->qos_class != memdev->ram_qos_class)
>  				return true;
> -		} else if (region->mode == CXL_DECODER_MODE_PMEM) {
> +		} else if (region->mode == CXL_REGION_MODE_PMEM) {
>  			if (root_decoder->qos_class != memdev->pmem_qos_class)
>  				return true;
>  		}
> @@ -619,9 +619,9 @@ static void *add_cxl_region(void *parent, int id, const char *cxlregion_base)
>  
>  	sprintf(path, "%s/mode", cxlregion_base);
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
> -		region->mode = CXL_DECODER_MODE_NONE;
> +		region->mode = CXL_REGION_MODE_NONE;
>  	else
> -		region->mode = cxl_decoder_mode_from_ident(buf);
> +		region->mode = cxl_region_mode_from_ident(buf);
>  
>  	sprintf(path, "%s/modalias", cxlregion_base);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
> @@ -748,11 +748,16 @@ CXL_EXPORT unsigned long long cxl_region_get_resource(struct cxl_region *region)
>  	return region->start;
>  }
>  
> -CXL_EXPORT enum cxl_decoder_mode cxl_region_get_mode(struct cxl_region *region)
> +CXL_EXPORT enum cxl_region_mode cxl_region_get_region_mode(struct cxl_region *region)
>  {
>  	return region->mode;
>  }
>  
> +CXL_EXPORT enum cxl_decoder_mode cxl_region_get_mode(struct cxl_region *region)
> +{
> +	return (enum cxl_decoder_mode)cxl_region_get_region_mode(region);
> +}
> +
>  CXL_EXPORT unsigned int
>  cxl_region_get_interleave_ways(struct cxl_region *region)
>  {
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 0c155a40ad4765106f0eab1745281d462af782fe..b5d9bdcc38e09812f26afc1cb0e804f86784b8e6 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -287,4 +287,5 @@ LIBECXL_8 {
>  global:
>  	cxl_memdev_trigger_poison_list;
>  	cxl_region_trigger_poison_list;
> +	cxl_region_get_region_mode;
>  } LIBCXL_7;
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index b6cd910e93359b53cac34427acfe84c7abcb78b0..0f45be89b6a00477d13fb6d7f1906213a3073c48 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -171,7 +171,7 @@ struct cxl_region {
>  	unsigned int interleave_ways;
>  	unsigned int interleave_granularity;
>  	enum cxl_decode_state decode_state;
> -	enum cxl_decoder_mode mode;
> +	enum cxl_region_mode mode;
>  	struct daxctl_region *dax_region;
>  	struct kmod_module *module;
>  	struct list_head mappings;
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index 0a5fd0e13cc24e0032d4a83d780278fbe0038d32..06b87a0924faafec6c80eca83ea7551d4e117256 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -303,6 +303,39 @@ int cxl_memdev_is_enabled(struct cxl_memdev *memdev);
>  	for (endpoint = cxl_endpoint_get_first(port); endpoint != NULL;        \
>  	     endpoint = cxl_endpoint_get_next(endpoint))
>  
> +enum cxl_region_mode {
> +	CXL_REGION_MODE_NONE = CXL_DECODER_MODE_NONE,
> +	CXL_REGION_MODE_MIXED = CXL_DECODER_MODE_MIXED,
> +	CXL_REGION_MODE_PMEM = CXL_DECODER_MODE_PMEM,
> +	CXL_REGION_MODE_RAM = CXL_DECODER_MODE_RAM,
> +};
> +
> +static inline const char *cxl_region_mode_name(enum cxl_region_mode mode)
> +{
> +	static const char *names[] = {
> +		[CXL_REGION_MODE_NONE] = "none",
> +		[CXL_REGION_MODE_MIXED] = "mixed",
> +		[CXL_REGION_MODE_PMEM] = "pmem",
> +		[CXL_REGION_MODE_RAM] = "ram",
> +	};
> +
> +	if (mode < CXL_REGION_MODE_NONE || mode > CXL_REGION_MODE_RAM)
> +		mode = CXL_REGION_MODE_NONE;
> +	return names[mode];
> +}
> +
> +static inline enum cxl_region_mode
> +cxl_region_mode_from_ident(const char *ident)
> +{
> +	if (strcmp(ident, "ram") == 0)
> +		return CXL_REGION_MODE_RAM;
> +	else if (strcmp(ident, "volatile") == 0)
> +		return CXL_REGION_MODE_RAM;
> +	else if (strcmp(ident, "pmem") == 0)
> +		return CXL_REGION_MODE_PMEM;
> +	return CXL_REGION_MODE_NONE;
> +}
> +
>  struct cxl_region;
>  struct cxl_region *cxl_region_get_first(struct cxl_decoder *decoder);
>  struct cxl_region *cxl_region_get_next(struct cxl_region *region);
> @@ -318,6 +351,8 @@ const char *cxl_region_get_devname(struct cxl_region *region);
>  void cxl_region_get_uuid(struct cxl_region *region, uuid_t uu);
>  unsigned long long cxl_region_get_size(struct cxl_region *region);
>  unsigned long long cxl_region_get_resource(struct cxl_region *region);
> +enum cxl_region_mode cxl_region_get_region_mode(struct cxl_region *region);
> +/* Deprecated: use cxl_region_get_region_mode() */
>  enum cxl_decoder_mode cxl_region_get_mode(struct cxl_region *region);
>  unsigned int cxl_region_get_interleave_ways(struct cxl_region *region);
>  unsigned int cxl_region_get_interleave_granularity(struct cxl_region *region);
> 
> -- 
> 2.47.0
> 

-- 
Fan Ni

[ndctl PATCH v2 4/6] cxl/region: Add creation of Dynamic capacity regions

[ira.weiny]

From: Navneet Singh <navneet.singh@intel.com>

CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
spare and defined as dynamic capacity (dc).

Add support for DCD devices.  Query for DCD capabilities.  Add the
ability to add DC partitions to a CXL DC region.

Signed-off-by: Navneet Singh <navneet.singh@intel.com>
Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
Co-authored-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes:
[Fan: Properly initialize index]
---
 cxl/json.c         | 26 +++++++++++++++
 cxl/lib/libcxl.c   | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 cxl/lib/libcxl.sym |  3 ++
 cxl/lib/private.h  |  6 +++-
 cxl/libcxl.h       | 55 +++++++++++++++++++++++++++++--
 cxl/memdev.c       |  7 +++-
 cxl/region.c       | 49 ++++++++++++++++++++++++++--
 7 files changed, 234 insertions(+), 7 deletions(-)

diff --git a/cxl/json.c b/cxl/json.c
index dcd3cc28393faf7e8adf299a857531ecdeaac50a..915b2716a524fa8929ed34b01a7cb6590b61d4b7 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -754,10 +754,12 @@ err_free:
 	return jpoison;
 }
 
+#define DC_SIZE_NAME_LEN 64
 struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
 		unsigned long flags)
 {
 	const char *devname = cxl_memdev_get_devname(memdev);
+	char size_name[DC_SIZE_NAME_LEN];
 	struct json_object *jdev, *jobj;
 	unsigned long long serial, size;
 	const char *fw_version;
@@ -800,6 +802,17 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
 		}
 	}
 
+	for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
+		size = cxl_memdev_get_dc_size(memdev, index);
+		if (size) {
+			jobj = util_json_object_size(size, flags);
+			if (jobj) {
+				sprintf(size_name, "dc%d_size", index);
+				json_object_object_add(jdev,
+						       size_name, jobj);
+			}
+		}
+	}
 	if (flags & UTIL_JSON_HEALTH) {
 		jobj = util_cxl_memdev_health_to_json(memdev, flags);
 		if (jobj)
@@ -948,11 +961,13 @@ struct json_object *util_cxl_bus_to_json(struct cxl_bus *bus,
 	return jbus;
 }
 
+#define DC_CAPABILITY_NAME_LEN 16
 struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 					     unsigned long flags)
 {
 	const char *devname = cxl_decoder_get_devname(decoder);
 	struct cxl_port *port = cxl_decoder_get_port(decoder);
+	char dc_capable_name[DC_CAPABILITY_NAME_LEN];
 	struct json_object *jdecoder, *jobj;
 	struct cxl_region *region;
 	u64 val, size;
@@ -1059,6 +1074,17 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 				json_object_object_add(
 					jdecoder, "volatile_capable", jobj);
 		}
+		for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
+			if (cxl_decoder_is_dc_capable(decoder, index)) {
+				jobj = json_object_new_boolean(true);
+				if (jobj) {
+					sprintf(dc_capable_name, "dc%d_capable", index);
+					json_object_object_add(jdecoder,
+							       dc_capable_name,
+							       jobj);
+				}
+			}
+		}
 	}
 
 	if (cxl_port_is_root(port) &&
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 5cbfb3e7d466b491ef87ea285f7e50d3bac230db..4caa2d02313bf71960971c4eaa67fa42cea08d55 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1267,7 +1267,6 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
 	char buf[SYSFS_ATTR_SIZE];
 	struct stat st;
 	char *host;
-
 	if (!path)
 		return NULL;
 	dbg(ctx, "%s: base: \'%s\'\n", devname, cxlmem_base);
@@ -1304,6 +1303,19 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
 	else
 		memdev->ram_qos_class = atoi(buf);
 
+	memdev->dc_partition_count = 0;
+	for (int partition = 0; partition < MAX_NUM_DC_REGIONS; partition++) {
+		sprintf(path, "%s/dc%d/size", cxlmem_base, partition);
+		if (sysfs_read_attr(ctx, path, buf) < 0)
+			continue;
+		memdev->dc_size[partition] = strtoull(buf, NULL, 0);
+		memdev->dc_partition_count++;
+		sprintf(path, "%s/dc%d/qos_class", cxlmem_base, partition);
+		if (sysfs_read_attr(ctx, path, buf) < 0)
+			continue;
+		memdev->dc_qos_class[partition] = strtoull(buf, NULL, 0);
+	}
+
 	sprintf(path, "%s/payload_max", cxlmem_base);
 	if (sysfs_read_attr(ctx, path, buf) == 0) {
 		memdev->payload_max = strtoull(buf, NULL, 0);
@@ -1540,6 +1552,14 @@ CXL_EXPORT int cxl_memdev_get_ram_qos_class(struct cxl_memdev *memdev)
 	return memdev->ram_qos_class;
 }
 
+CXL_EXPORT unsigned long long cxl_memdev_get_dc_size(struct cxl_memdev *memdev, int index)
+{
+	if (index >= 0 && index < MAX_NUM_DC_REGIONS)
+		return memdev->dc_size[index];
+
+	return 0;
+}
+
 CXL_EXPORT const char *cxl_memdev_get_firmware_verison(struct cxl_memdev *memdev)
 {
 	return memdev->firmware_version;
@@ -2275,6 +2295,22 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
 			decoder->mode = CXL_DECODER_MODE_RAM;
 		else if (strcmp(buf, "pmem") == 0)
 			decoder->mode = CXL_DECODER_MODE_PMEM;
+		else if (strcmp(buf, "dc0") == 0)
+			decoder->mode = CXL_DECODER_MODE_DC0;
+		else if (strcmp(buf, "dc1") == 0)
+			decoder->mode = CXL_DECODER_MODE_DC1;
+		else if (strcmp(buf, "dc2") == 0)
+			decoder->mode = CXL_DECODER_MODE_DC2;
+		else if (strcmp(buf, "dc3") == 0)
+			decoder->mode = CXL_DECODER_MODE_DC3;
+		else if (strcmp(buf, "dc4") == 0)
+			decoder->mode = CXL_DECODER_MODE_DC4;
+		else if (strcmp(buf, "dc5") == 0)
+			decoder->mode = CXL_DECODER_MODE_DC5;
+		else if (strcmp(buf, "dc6") == 0)
+			decoder->mode = CXL_DECODER_MODE_DC6;
+		else if (strcmp(buf, "dc7") == 0)
+			decoder->mode = CXL_DECODER_MODE_DC7;
 		else if (strcmp(buf, "mixed") == 0)
 			decoder->mode = CXL_DECODER_MODE_MIXED;
 		else if (strcmp(buf, "none") == 0)
@@ -2318,6 +2354,14 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
 	case CXL_PORT_SWITCH:
 		decoder->pmem_capable = true;
 		decoder->volatile_capable = true;
+		decoder->dc_capable[0] = true;
+		decoder->dc_capable[1] = true;
+		decoder->dc_capable[2] = true;
+		decoder->dc_capable[3] = true;
+		decoder->dc_capable[4] = true;
+		decoder->dc_capable[5] = true;
+		decoder->dc_capable[6] = true;
+		decoder->dc_capable[7] = true;
 		decoder->mem_capable = true;
 		decoder->accelmem_capable = true;
 		sprintf(path, "%s/locked", cxldecoder_base);
@@ -2341,6 +2385,14 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
 			{ "cap_type2", &decoder->accelmem_capable },
 			{ "cap_type3", &decoder->mem_capable },
 			{ "cap_ram", &decoder->volatile_capable },
+			{ "cap_ram", &decoder->dc_capable[0] },
+			{ "cap_ram", &decoder->dc_capable[1] },
+			{ "cap_ram", &decoder->dc_capable[2] },
+			{ "cap_ram", &decoder->dc_capable[3] },
+			{ "cap_ram", &decoder->dc_capable[4] },
+			{ "cap_ram", &decoder->dc_capable[5] },
+			{ "cap_ram", &decoder->dc_capable[6] },
+			{ "cap_ram", &decoder->dc_capable[7] },
 			{ "cap_pmem", &decoder->pmem_capable },
 			{ "locked", &decoder->locked },
 		};
@@ -2592,6 +2644,30 @@ CXL_EXPORT int cxl_decoder_set_mode(struct cxl_decoder *decoder,
 	case CXL_DECODER_MODE_RAM:
 		sprintf(buf, "ram");
 		break;
+	case CXL_DECODER_MODE_DC0:
+		sprintf(buf, "dc0");
+		break;
+	case CXL_DECODER_MODE_DC1:
+		sprintf(buf, "dc1");
+		break;
+	case CXL_DECODER_MODE_DC2:
+		sprintf(buf, "dc2");
+		break;
+	case CXL_DECODER_MODE_DC3:
+		sprintf(buf, "dc3");
+		break;
+	case CXL_DECODER_MODE_DC4:
+		sprintf(buf, "dc4");
+		break;
+	case CXL_DECODER_MODE_DC5:
+		sprintf(buf, "dc5");
+		break;
+	case CXL_DECODER_MODE_DC6:
+		sprintf(buf, "dc6");
+		break;
+	case CXL_DECODER_MODE_DC7:
+		sprintf(buf, "dc7");
+		break;
 	default:
 		err(ctx, "%s: unsupported mode: %d\n",
 		    cxl_decoder_get_devname(decoder), mode);
@@ -2648,6 +2724,14 @@ CXL_EXPORT bool cxl_decoder_is_mem_capable(struct cxl_decoder *decoder)
 	return decoder->mem_capable;
 }
 
+CXL_EXPORT bool cxl_decoder_is_dc_capable(struct cxl_decoder *decoder, int index)
+{
+	if (index >= 0 && index < MAX_NUM_DC_REGIONS)
+		return decoder->dc_capable[index];
+
+	return 0;
+}
+
 CXL_EXPORT bool cxl_decoder_is_accelmem_capable(struct cxl_decoder *decoder)
 {
 	return decoder->accelmem_capable;
@@ -2717,6 +2801,8 @@ static struct cxl_region *cxl_decoder_create_region(struct cxl_decoder *decoder,
 		sprintf(path, "%s/create_pmem_region", decoder->dev_path);
 	else if (mode == CXL_DECODER_MODE_RAM)
 		sprintf(path, "%s/create_ram_region", decoder->dev_path);
+	else if (cxl_decoder_mode_is_dc(mode))
+		sprintf(path, "%s/create_dc_region", decoder->dev_path);
 
 	rc = sysfs_read_attr(ctx, path, buf);
 	if (rc < 0) {
@@ -2768,6 +2854,13 @@ cxl_decoder_create_ram_region(struct cxl_decoder *decoder)
 	return cxl_decoder_create_region(decoder, CXL_DECODER_MODE_RAM);
 }
 
+CXL_EXPORT struct cxl_region *
+cxl_decoder_create_dc_region(struct cxl_decoder *decoder,
+			     enum cxl_decoder_mode mode)
+{
+	return cxl_decoder_create_region(decoder, mode);
+}
+
 CXL_EXPORT int cxl_decoder_get_nr_targets(struct cxl_decoder *decoder)
 {
 	return decoder->nr_targets;
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index b5d9bdcc38e09812f26afc1cb0e804f86784b8e6..351da7512e05080d847fd87740488d613462dbc9 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -19,6 +19,7 @@ global:
 	cxl_memdev_get_ctx;
 	cxl_memdev_get_pmem_size;
 	cxl_memdev_get_ram_size;
+	cxl_memdev_get_dc_size;
 	cxl_memdev_get_firmware_verison;
 	cxl_cmd_get_devname;
 	cxl_cmd_new_raw;
@@ -247,6 +248,8 @@ LIBCXL_5 {
 global:
 	cxl_region_get_mode;
 	cxl_decoder_create_ram_region;
+	cxl_decoder_is_dc_capable;
+	cxl_decoder_create_dc_region;
 	cxl_region_get_daxctl_region;
 	cxl_port_get_parent_dport;
 } LIBCXL_4;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index 0f45be89b6a00477d13fb6d7f1906213a3073c48..10abfa63dfc759b1589f9f039da1b920f8eb605e 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -12,7 +12,6 @@
 #include <util/bitmap.h>
 
 #define CXL_EXPORT __attribute__ ((visibility("default")))
-
 struct cxl_pmem {
 	int id;
 	void *dev_buf;
@@ -47,6 +46,9 @@ struct cxl_memdev {
 	struct list_node list;
 	unsigned long long pmem_size;
 	unsigned long long ram_size;
+	unsigned long long dc_size[MAX_NUM_DC_REGIONS];
+	unsigned long long dc_qos_class[MAX_NUM_DC_REGIONS];
+	int dc_partition_count;
 	int ram_qos_class;
 	int pmem_qos_class;
 	int payload_max;
@@ -111,6 +113,7 @@ struct cxl_endpoint {
 	struct cxl_memdev *memdev;
 };
 
+
 struct cxl_target {
 	struct list_node list;
 	struct cxl_decoder *decoder;
@@ -140,6 +143,7 @@ struct cxl_decoder {
 	bool pmem_capable;
 	bool volatile_capable;
 	bool mem_capable;
+	bool dc_capable[MAX_NUM_DC_REGIONS];
 	bool accelmem_capable;
 	bool locked;
 	enum cxl_decoder_target_type target_type;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 06b87a0924faafec6c80eca83ea7551d4e117256..17ed682548b970d57f016942badc76dce61bdeaf 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -72,6 +72,7 @@ int cxl_memdev_get_minor(struct cxl_memdev *memdev);
 struct cxl_ctx *cxl_memdev_get_ctx(struct cxl_memdev *memdev);
 unsigned long long cxl_memdev_get_pmem_size(struct cxl_memdev *memdev);
 unsigned long long cxl_memdev_get_ram_size(struct cxl_memdev *memdev);
+unsigned long long cxl_memdev_get_dc_size(struct cxl_memdev *memdev, int index);
 int cxl_memdev_get_pmem_qos_class(struct cxl_memdev *memdev);
 int cxl_memdev_get_ram_qos_class(struct cxl_memdev *memdev);
 const char *cxl_memdev_get_firmware_verison(struct cxl_memdev *memdev);
@@ -191,11 +192,20 @@ unsigned long long
 cxl_decoder_get_max_available_extent(struct cxl_decoder *decoder);
 int cxl_root_decoder_get_qos_class(struct cxl_decoder *decoder);
 
+#define MAX_NUM_DC_REGIONS 8
 enum cxl_decoder_mode {
 	CXL_DECODER_MODE_NONE,
 	CXL_DECODER_MODE_MIXED,
 	CXL_DECODER_MODE_PMEM,
 	CXL_DECODER_MODE_RAM,
+	CXL_DECODER_MODE_DC0,
+	CXL_DECODER_MODE_DC1,
+	CXL_DECODER_MODE_DC2,
+	CXL_DECODER_MODE_DC3,
+	CXL_DECODER_MODE_DC4,
+	CXL_DECODER_MODE_DC5,
+	CXL_DECODER_MODE_DC6,
+	CXL_DECODER_MODE_DC7,
 };
 
 static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
@@ -205,9 +215,17 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
 		[CXL_DECODER_MODE_MIXED] = "mixed",
 		[CXL_DECODER_MODE_PMEM] = "pmem",
 		[CXL_DECODER_MODE_RAM] = "ram",
+		[CXL_DECODER_MODE_DC0] = "dc0",
+		[CXL_DECODER_MODE_DC1] = "dc1",
+		[CXL_DECODER_MODE_DC2] = "dc2",
+		[CXL_DECODER_MODE_DC3] = "dc3",
+		[CXL_DECODER_MODE_DC4] = "dc4",
+		[CXL_DECODER_MODE_DC5] = "dc5",
+		[CXL_DECODER_MODE_DC6] = "dc6",
+		[CXL_DECODER_MODE_DC7] = "dc7",
 	};
 
-	if (mode < CXL_DECODER_MODE_NONE || mode > CXL_DECODER_MODE_RAM)
+	if (mode < CXL_DECODER_MODE_NONE || mode > CXL_DECODER_MODE_DC7)
 		mode = CXL_DECODER_MODE_NONE;
 	return names[mode];
 }
@@ -221,9 +239,35 @@ cxl_decoder_mode_from_ident(const char *ident)
 		return CXL_DECODER_MODE_RAM;
 	else if (strcmp(ident, "pmem") == 0)
 		return CXL_DECODER_MODE_PMEM;
+	else if (strcmp(ident, "dc0") == 0)
+		return CXL_DECODER_MODE_DC0;
+	else if (strcmp(ident, "dc1") == 0)
+		return CXL_DECODER_MODE_DC1;
+	else if (strcmp(ident, "dc2") == 0)
+		return CXL_DECODER_MODE_DC2;
+	else if (strcmp(ident, "dc3") == 0)
+		return CXL_DECODER_MODE_DC3;
+	else if (strcmp(ident, "dc4") == 0)
+		return CXL_DECODER_MODE_DC4;
+	else if (strcmp(ident, "dc5") == 0)
+		return CXL_DECODER_MODE_DC5;
+	else if (strcmp(ident, "dc6") == 0)
+		return CXL_DECODER_MODE_DC6;
+	else if (strcmp(ident, "dc7") == 0)
+		return CXL_DECODER_MODE_DC7;
 	return CXL_DECODER_MODE_NONE;
 }
 
+static inline bool cxl_decoder_mode_is_dc(enum cxl_decoder_mode mode)
+{
+	return (mode >= CXL_DECODER_MODE_DC0 && mode <= CXL_DECODER_MODE_DC7);
+}
+
+static inline int cxl_decoder_dc_mode_to_index(enum cxl_decoder_mode mode)
+{
+	return mode - CXL_DECODER_MODE_DC0;
+}
+
 enum cxl_decoder_mode cxl_decoder_get_mode(struct cxl_decoder *decoder);
 int cxl_decoder_set_mode(struct cxl_decoder *decoder,
 			 enum cxl_decoder_mode mode);
@@ -248,6 +292,7 @@ enum cxl_decoder_target_type {
 enum cxl_decoder_target_type
 cxl_decoder_get_target_type(struct cxl_decoder *decoder);
 bool cxl_decoder_is_pmem_capable(struct cxl_decoder *decoder);
+bool cxl_decoder_is_dc_capable(struct cxl_decoder *decoder, int index);
 bool cxl_decoder_is_volatile_capable(struct cxl_decoder *decoder);
 bool cxl_decoder_is_mem_capable(struct cxl_decoder *decoder);
 bool cxl_decoder_is_accelmem_capable(struct cxl_decoder *decoder);
@@ -258,6 +303,8 @@ unsigned int cxl_decoder_get_interleave_ways(struct cxl_decoder *decoder);
 struct cxl_region *cxl_decoder_get_region(struct cxl_decoder *decoder);
 struct cxl_region *cxl_decoder_create_pmem_region(struct cxl_decoder *decoder);
 struct cxl_region *cxl_decoder_create_ram_region(struct cxl_decoder *decoder);
+struct cxl_region *cxl_decoder_create_dc_region(struct cxl_decoder *decoder,
+						enum cxl_decoder_mode mode);
 struct cxl_decoder *cxl_decoder_get_by_name(struct cxl_ctx *ctx,
 					    const char *ident);
 struct cxl_memdev *cxl_decoder_get_memdev(struct cxl_decoder *decoder);
@@ -308,6 +355,7 @@ enum cxl_region_mode {
 	CXL_REGION_MODE_MIXED = CXL_DECODER_MODE_MIXED,
 	CXL_REGION_MODE_PMEM = CXL_DECODER_MODE_PMEM,
 	CXL_REGION_MODE_RAM = CXL_DECODER_MODE_RAM,
+	CXL_REGION_MODE_DC,
 };
 
 static inline const char *cxl_region_mode_name(enum cxl_region_mode mode)
@@ -317,9 +365,10 @@ static inline const char *cxl_region_mode_name(enum cxl_region_mode mode)
 		[CXL_REGION_MODE_MIXED] = "mixed",
 		[CXL_REGION_MODE_PMEM] = "pmem",
 		[CXL_REGION_MODE_RAM] = "ram",
+		[CXL_REGION_MODE_DC] = "dc",
 	};
 
-	if (mode < CXL_REGION_MODE_NONE || mode > CXL_REGION_MODE_RAM)
+	if (mode < CXL_REGION_MODE_NONE || mode > CXL_REGION_MODE_DC)
 		mode = CXL_REGION_MODE_NONE;
 	return names[mode];
 }
@@ -333,6 +382,8 @@ cxl_region_mode_from_ident(const char *ident)
 		return CXL_REGION_MODE_RAM;
 	else if (strcmp(ident, "pmem") == 0)
 		return CXL_REGION_MODE_PMEM;
+	else if (strcmp(ident, "dc") == 0)
+		return CXL_REGION_MODE_DC;
 	return CXL_REGION_MODE_NONE;
 }
 
diff --git a/cxl/memdev.c b/cxl/memdev.c
index 6e44d1578d03b6af998502e54714635b8f31b556..0a7d350efe9e612cd67d32328cca286dcdcb2991 100644
--- a/cxl/memdev.c
+++ b/cxl/memdev.c
@@ -269,8 +269,13 @@ static int __reserve_dpa(struct cxl_memdev *memdev,
 
 	if (mode == CXL_DECODER_MODE_RAM)
 		avail_dpa = cxl_memdev_get_ram_size(memdev);
-	else
+	else if (mode == CXL_DECODER_MODE_PMEM)
 		avail_dpa = cxl_memdev_get_pmem_size(memdev);
+	else if (cxl_decoder_mode_is_dc(mode)) {
+		int i = cxl_decoder_dc_mode_to_index(mode);
+
+		avail_dpa = cxl_memdev_get_dc_size(memdev, i);
+	}
 
 	cxl_decoder_foreach(port, decoder) {
 		size = cxl_decoder_get_dpa_size(decoder);
diff --git a/cxl/region.c b/cxl/region.c
index 207cf2d003148992255c715f286bc0f38de2ca84..310694ae07fae25f13d032a30c130bf7d3394388 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -78,7 +78,7 @@ OPT_INTEGER('w', "ways", &param.ways, \
 OPT_INTEGER('g', "granularity", &param.granularity,  \
 	    "granularity of the interleave set"), \
 OPT_STRING('t', "type", &param.type, \
-	   "region type", "region type - 'pmem' or 'ram'"), \
+	   "region type", "region type - 'pmem', 'ram', 'dcX'"), \
 OPT_STRING('U', "uuid", &param.uuid, \
 	   "region uuid", "uuid for the new region (default: autogenerate)"), \
 OPT_BOOLEAN('m', "memdevs", &param.memdevs, \
@@ -400,9 +400,22 @@ static int parse_region_options(int argc, const char **argv,
 	}
 }
 
+static int dc_mode_to_region_index(enum cxl_decoder_mode mode)
+{
+	int index = 0;
+
+	for (unsigned int i = CXL_DECODER_MODE_DC0; i <= CXL_DECODER_MODE_DC7; i++) {
+		if (mode == i)
+			return index;
+		index++;
+	}
+
+	return -EINVAL;
+}
+
 static void collect_minsize(struct cxl_ctx *ctx, struct parsed_params *p)
 {
-	int i;
+	int i, index;
 
 	for (i = 0; i < p->ways; i++) {
 		struct json_object *jobj =
@@ -417,6 +430,10 @@ static void collect_minsize(struct cxl_ctx *ctx, struct parsed_params *p)
 		case CXL_DECODER_MODE_PMEM:
 			size = cxl_memdev_get_pmem_size(memdev);
 			break;
+		case CXL_DECODER_MODE_DC0 ... CXL_DECODER_MODE_DC7:
+			index =  dc_mode_to_region_index(p->mode);
+			size = cxl_memdev_get_dc_size(memdev, index);
+			break;
 		default:
 			/* Shouldn't ever get here */ ;
 		}
@@ -473,6 +490,7 @@ static int validate_decoder(struct cxl_decoder *decoder,
 			    struct parsed_params *p)
 {
 	const char *devname = cxl_decoder_get_devname(decoder);
+	int index;
 	int rc;
 
 	switch(p->mode) {
@@ -488,6 +506,13 @@ static int validate_decoder(struct cxl_decoder *decoder,
 			return -EINVAL;
 		}
 		break;
+	case CXL_DECODER_MODE_DC0 ... CXL_DECODER_MODE_DC7:
+		index =  dc_mode_to_region_index(p->mode);
+		if (!cxl_decoder_is_dc_capable(decoder, index)) {
+			log_err(&rl, "%s is not dc%d capable\n", devname, index);
+			return -EINVAL;
+		}
+		break;
 	default:
 		log_err(&rl, "unknown type: %s\n", param.type);
 		return -EINVAL;
@@ -502,12 +527,25 @@ static int validate_decoder(struct cxl_decoder *decoder,
 	return 0;
 }
 
+static enum cxl_decoder_mode dc_region_index_to_mode(int index)
+{
+	return (CXL_DECODER_MODE_DC0 + index);
+}
+
 static void set_type_from_decoder(struct cxl_ctx *ctx, struct parsed_params *p)
 {
 	/* if param.type was explicitly specified, nothing to do here */
 	if (param.type)
 		return;
 
+	/* Only chose DC if it is the only type available */
+	for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
+		if (cxl_decoder_is_dc_capable(p->root_decoder, index)) {
+			p->mode = dc_region_index_to_mode(index);
+			break;
+		}
+	}
+
 	/*
 	 * default to pmem if both types are set, otherwise the single
 	 * capability dominates.
@@ -699,6 +737,13 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 				param.root_decoder);
 			return -ENXIO;
 		}
+	} else if (cxl_decoder_mode_is_dc(p->mode)) {
+		region = cxl_decoder_create_dc_region(p->root_decoder, p->mode);
+		if (!region) {
+			log_err(&rl, "failed to create region under %s\n",
+				param.root_decoder);
+			return -ENXIO;
+		}
 	} else {
 		log_err(&rl, "region type '%s' is not supported\n",
 			param.type);

-- 
2.47.0

Re: [ndctl PATCH v2 4/6] cxl/region: Add creation of Dynamic capacity regions

[Dave Jiang]

On 11/4/24 7:10 PM, ira.weiny@intel.com wrote:
> From: Navneet Singh <navneet.singh@intel.com>
> 
> CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> spare and defined as dynamic capacity (dc).
> 
> Add support for DCD devices.  Query for DCD capabilities.  Add the
> ability to add DC partitions to a CXL DC region.
> 
> Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Co-authored-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

A few small things below.
> 
> ---
> Changes:
> [Fan: Properly initialize index]
> ---
>  cxl/json.c         | 26 +++++++++++++++
>  cxl/lib/libcxl.c   | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  cxl/lib/libcxl.sym |  3 ++
>  cxl/lib/private.h  |  6 +++-
>  cxl/libcxl.h       | 55 +++++++++++++++++++++++++++++--
>  cxl/memdev.c       |  7 +++-
>  cxl/region.c       | 49 ++++++++++++++++++++++++++--
>  7 files changed, 234 insertions(+), 7 deletions(-)
> 
> diff --git a/cxl/json.c b/cxl/json.c
> index dcd3cc28393faf7e8adf299a857531ecdeaac50a..915b2716a524fa8929ed34b01a7cb6590b61d4b7 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -754,10 +754,12 @@ err_free:
>  	return jpoison;
>  }
>  
> +#define DC_SIZE_NAME_LEN 64
>  struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  		unsigned long flags)
>  {
>  	const char *devname = cxl_memdev_get_devname(memdev);
> +	char size_name[DC_SIZE_NAME_LEN];
>  	struct json_object *jdev, *jobj;
>  	unsigned long long serial, size;
>  	const char *fw_version;
> @@ -800,6 +802,17 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  		}
>  	}
>  
> +	for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
> +		size = cxl_memdev_get_dc_size(memdev, index);
> +		if (size) {
> +			jobj = util_json_object_size(size, flags);
> +			if (jobj) {
> +				sprintf(size_name, "dc%d_size", index);
> +				json_object_object_add(jdev,
> +						       size_name, jobj);
> +			}
> +		}
> +	}
>  	if (flags & UTIL_JSON_HEALTH) {
>  		jobj = util_cxl_memdev_health_to_json(memdev, flags);
>  		if (jobj)
> @@ -948,11 +961,13 @@ struct json_object *util_cxl_bus_to_json(struct cxl_bus *bus,
>  	return jbus;
>  }
>  
> +#define DC_CAPABILITY_NAME_LEN 16
>  struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
>  					     unsigned long flags)
>  {
>  	const char *devname = cxl_decoder_get_devname(decoder);
>  	struct cxl_port *port = cxl_decoder_get_port(decoder);
> +	char dc_capable_name[DC_CAPABILITY_NAME_LEN];
>  	struct json_object *jdecoder, *jobj;
>  	struct cxl_region *region;
>  	u64 val, size;
> @@ -1059,6 +1074,17 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
>  				json_object_object_add(
>  					jdecoder, "volatile_capable", jobj);
>  		}
> +		for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
> +			if (cxl_decoder_is_dc_capable(decoder, index)) {
> +				jobj = json_object_new_boolean(true);
> +				if (jobj) {
> +					sprintf(dc_capable_name, "dc%d_capable", index);
> +					json_object_object_add(jdecoder,
> +							       dc_capable_name,
> +							       jobj);
> +				}
> +			}
> +		}
>  	}
>  
>  	if (cxl_port_is_root(port) &&
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 5cbfb3e7d466b491ef87ea285f7e50d3bac230db..4caa2d02313bf71960971c4eaa67fa42cea08d55 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1267,7 +1267,6 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
>  	char buf[SYSFS_ATTR_SIZE];
>  	struct stat st;
>  	char *host;
> -

Stray line delete
>  	if (!path)
>  		return NULL;
>  	dbg(ctx, "%s: base: \'%s\'\n", devname, cxlmem_base);
> @@ -1304,6 +1303,19 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
>  	else
>  		memdev->ram_qos_class = atoi(buf);
>  
> +	memdev->dc_partition_count = 0;
> +	for (int partition = 0; partition < MAX_NUM_DC_REGIONS; partition++) {
> +		sprintf(path, "%s/dc%d/size", cxlmem_base, partition);
> +		if (sysfs_read_attr(ctx, path, buf) < 0)
> +			continue;
> +		memdev->dc_size[partition] = strtoull(buf, NULL, 0);
> +		memdev->dc_partition_count++;
> +		sprintf(path, "%s/dc%d/qos_class", cxlmem_base, partition);
> +		if (sysfs_read_attr(ctx, path, buf) < 0)
> +			continue;
> +		memdev->dc_qos_class[partition] = strtoull(buf, NULL, 0);
> +	}
> +
>  	sprintf(path, "%s/payload_max", cxlmem_base);
>  	if (sysfs_read_attr(ctx, path, buf) == 0) {
>  		memdev->payload_max = strtoull(buf, NULL, 0);
> @@ -1540,6 +1552,14 @@ CXL_EXPORT int cxl_memdev_get_ram_qos_class(struct cxl_memdev *memdev)
>  	return memdev->ram_qos_class;
>  }
>  
> +CXL_EXPORT unsigned long long cxl_memdev_get_dc_size(struct cxl_memdev *memdev, int index)

If you make index 'unsigned int', you can skip the index >= 0 check below right?

> +{
> +	if (index >= 0 && index < MAX_NUM_DC_REGIONS)
> +		return memdev->dc_size[index];
> +
> +	return 0;
> +}
> +
>  CXL_EXPORT const char *cxl_memdev_get_firmware_verison(struct cxl_memdev *memdev)
>  {
>  	return memdev->firmware_version;
> @@ -2275,6 +2295,22 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
>  			decoder->mode = CXL_DECODER_MODE_RAM;
>  		else if (strcmp(buf, "pmem") == 0)
>  			decoder->mode = CXL_DECODER_MODE_PMEM;
> +		else if (strcmp(buf, "dc0") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC0;
> +		else if (strcmp(buf, "dc1") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC1;
> +		else if (strcmp(buf, "dc2") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC2;
> +		else if (strcmp(buf, "dc3") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC3;
> +		else if (strcmp(buf, "dc4") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC4;
> +		else if (strcmp(buf, "dc5") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC5;
> +		else if (strcmp(buf, "dc6") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC6;
> +		else if (strcmp(buf, "dc7") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC7;
>  		else if (strcmp(buf, "mixed") == 0)
>  			decoder->mode = CXL_DECODER_MODE_MIXED;
>  		else if (strcmp(buf, "none") == 0)
> @@ -2318,6 +2354,14 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
>  	case CXL_PORT_SWITCH:
>  		decoder->pmem_capable = true;
>  		decoder->volatile_capable = true;
> +		decoder->dc_capable[0] = true;
> +		decoder->dc_capable[1] = true;
> +		decoder->dc_capable[2] = true;
> +		decoder->dc_capable[3] = true;
> +		decoder->dc_capable[4] = true;
> +		decoder->dc_capable[5] = true;
> +		decoder->dc_capable[6] = true;
> +		decoder->dc_capable[7] = true;

for loop?

>  		decoder->mem_capable = true;
>  		decoder->accelmem_capable = true;
>  		sprintf(path, "%s/locked", cxldecoder_base);
> @@ -2341,6 +2385,14 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
>  			{ "cap_type2", &decoder->accelmem_capable },
>  			{ "cap_type3", &decoder->mem_capable },
>  			{ "cap_ram", &decoder->volatile_capable },
> +			{ "cap_ram", &decoder->dc_capable[0] },
> +			{ "cap_ram", &decoder->dc_capable[1] },
> +			{ "cap_ram", &decoder->dc_capable[2] },
> +			{ "cap_ram", &decoder->dc_capable[3] },
> +			{ "cap_ram", &decoder->dc_capable[4] },
> +			{ "cap_ram", &decoder->dc_capable[5] },
> +			{ "cap_ram", &decoder->dc_capable[6] },
> +			{ "cap_ram", &decoder->dc_capable[7] },
>  			{ "cap_pmem", &decoder->pmem_capable },
>  			{ "locked", &decoder->locked },
>  		};
> @@ -2592,6 +2644,30 @@ CXL_EXPORT int cxl_decoder_set_mode(struct cxl_decoder *decoder,
>  	case CXL_DECODER_MODE_RAM:
>  		sprintf(buf, "ram");
>  		break;
> +	case CXL_DECODER_MODE_DC0:
> +		sprintf(buf, "dc0");
> +		break;
> +	case CXL_DECODER_MODE_DC1:
> +		sprintf(buf, "dc1");
> +		break;
> +	case CXL_DECODER_MODE_DC2:
> +		sprintf(buf, "dc2");
> +		break;
> +	case CXL_DECODER_MODE_DC3:
> +		sprintf(buf, "dc3");
> +		break;
> +	case CXL_DECODER_MODE_DC4:
> +		sprintf(buf, "dc4");
> +		break;
> +	case CXL_DECODER_MODE_DC5:
> +		sprintf(buf, "dc5");
> +		break;
> +	case CXL_DECODER_MODE_DC6:
> +		sprintf(buf, "dc6");
> +		break;
> +	case CXL_DECODER_MODE_DC7:
> +		sprintf(buf, "dc7");
> +		break;
>  	default:
>  		err(ctx, "%s: unsupported mode: %d\n",
>  		    cxl_decoder_get_devname(decoder), mode);
> @@ -2648,6 +2724,14 @@ CXL_EXPORT bool cxl_decoder_is_mem_capable(struct cxl_decoder *decoder)
>  	return decoder->mem_capable;
>  }
>  
> +CXL_EXPORT bool cxl_decoder_is_dc_capable(struct cxl_decoder *decoder, int index)

Same comment as above WRT index.

> +{
> +	if (index >= 0 && index < MAX_NUM_DC_REGIONS)
> +		return decoder->dc_capable[index];
> +
> +	return 0;
> +}
> +
>  CXL_EXPORT bool cxl_decoder_is_accelmem_capable(struct cxl_decoder *decoder)
>  {
>  	return decoder->accelmem_capable;
> @@ -2717,6 +2801,8 @@ static struct cxl_region *cxl_decoder_create_region(struct cxl_decoder *decoder,
>  		sprintf(path, "%s/create_pmem_region", decoder->dev_path);
>  	else if (mode == CXL_DECODER_MODE_RAM)
>  		sprintf(path, "%s/create_ram_region", decoder->dev_path);
> +	else if (cxl_decoder_mode_is_dc(mode))
> +		sprintf(path, "%s/create_dc_region", decoder->dev_path);
>  
>  	rc = sysfs_read_attr(ctx, path, buf);
>  	if (rc < 0) {
> @@ -2768,6 +2854,13 @@ cxl_decoder_create_ram_region(struct cxl_decoder *decoder)
>  	return cxl_decoder_create_region(decoder, CXL_DECODER_MODE_RAM);
>  }
>  
> +CXL_EXPORT struct cxl_region *
> +cxl_decoder_create_dc_region(struct cxl_decoder *decoder,
> +			     enum cxl_decoder_mode mode)
> +{
> +	return cxl_decoder_create_region(decoder, mode);
> +}
> +
>  CXL_EXPORT int cxl_decoder_get_nr_targets(struct cxl_decoder *decoder)
>  {
>  	return decoder->nr_targets;
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index b5d9bdcc38e09812f26afc1cb0e804f86784b8e6..351da7512e05080d847fd87740488d613462dbc9 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -19,6 +19,7 @@ global:
>  	cxl_memdev_get_ctx;
>  	cxl_memdev_get_pmem_size;
>  	cxl_memdev_get_ram_size;
> +	cxl_memdev_get_dc_size;
>  	cxl_memdev_get_firmware_verison;
>  	cxl_cmd_get_devname;
>  	cxl_cmd_new_raw;
> @@ -247,6 +248,8 @@ LIBCXL_5 {
>  global:
>  	cxl_region_get_mode;
>  	cxl_decoder_create_ram_region;
> +	cxl_decoder_is_dc_capable;
> +	cxl_decoder_create_dc_region;
>  	cxl_region_get_daxctl_region;
>  	cxl_port_get_parent_dport;
>  } LIBCXL_4;
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index 0f45be89b6a00477d13fb6d7f1906213a3073c48..10abfa63dfc759b1589f9f039da1b920f8eb605e 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -12,7 +12,6 @@
>  #include <util/bitmap.h>
>  
>  #define CXL_EXPORT __attribute__ ((visibility("default")))
> -

Errant line deletion

>  struct cxl_pmem {
>  	int id;
>  	void *dev_buf;
> @@ -47,6 +46,9 @@ struct cxl_memdev {
>  	struct list_node list;
>  	unsigned long long pmem_size;
>  	unsigned long long ram_size;
> +	unsigned long long dc_size[MAX_NUM_DC_REGIONS];
> +	unsigned long long dc_qos_class[MAX_NUM_DC_REGIONS];
> +	int dc_partition_count;
>  	int ram_qos_class;
>  	int pmem_qos_class;
>  	int payload_max;
> @@ -111,6 +113,7 @@ struct cxl_endpoint {
>  	struct cxl_memdev *memdev;
>  };
>  
> +

Errant line addition

>  struct cxl_target {
>  	struct list_node list;
>  	struct cxl_decoder *decoder;
> @@ -140,6 +143,7 @@ struct cxl_decoder {
>  	bool pmem_capable;
>  	bool volatile_capable;
>  	bool mem_capable;
> +	bool dc_capable[MAX_NUM_DC_REGIONS];
>  	bool accelmem_capable;
>  	bool locked;
>  	enum cxl_decoder_target_type target_type;
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index 06b87a0924faafec6c80eca83ea7551d4e117256..17ed682548b970d57f016942badc76dce61bdeaf 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -72,6 +72,7 @@ int cxl_memdev_get_minor(struct cxl_memdev *memdev);
>  struct cxl_ctx *cxl_memdev_get_ctx(struct cxl_memdev *memdev);
>  unsigned long long cxl_memdev_get_pmem_size(struct cxl_memdev *memdev);
>  unsigned long long cxl_memdev_get_ram_size(struct cxl_memdev *memdev);
> +unsigned long long cxl_memdev_get_dc_size(struct cxl_memdev *memdev, int index);
>  int cxl_memdev_get_pmem_qos_class(struct cxl_memdev *memdev);
>  int cxl_memdev_get_ram_qos_class(struct cxl_memdev *memdev);
>  const char *cxl_memdev_get_firmware_verison(struct cxl_memdev *memdev);
> @@ -191,11 +192,20 @@ unsigned long long
>  cxl_decoder_get_max_available_extent(struct cxl_decoder *decoder);
>  int cxl_root_decoder_get_qos_class(struct cxl_decoder *decoder);
>  
> +#define MAX_NUM_DC_REGIONS 8
>  enum cxl_decoder_mode {
>  	CXL_DECODER_MODE_NONE,
>  	CXL_DECODER_MODE_MIXED,
>  	CXL_DECODER_MODE_PMEM,
>  	CXL_DECODER_MODE_RAM,
> +	CXL_DECODER_MODE_DC0,
> +	CXL_DECODER_MODE_DC1,
> +	CXL_DECODER_MODE_DC2,
> +	CXL_DECODER_MODE_DC3,
> +	CXL_DECODER_MODE_DC4,
> +	CXL_DECODER_MODE_DC5,
> +	CXL_DECODER_MODE_DC6,
> +	CXL_DECODER_MODE_DC7,
>  };
>  
>  static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
> @@ -205,9 +215,17 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
>  		[CXL_DECODER_MODE_MIXED] = "mixed",
>  		[CXL_DECODER_MODE_PMEM] = "pmem",
>  		[CXL_DECODER_MODE_RAM] = "ram",
> +		[CXL_DECODER_MODE_DC0] = "dc0",
> +		[CXL_DECODER_MODE_DC1] = "dc1",
> +		[CXL_DECODER_MODE_DC2] = "dc2",
> +		[CXL_DECODER_MODE_DC3] = "dc3",
> +		[CXL_DECODER_MODE_DC4] = "dc4",
> +		[CXL_DECODER_MODE_DC5] = "dc5",
> +		[CXL_DECODER_MODE_DC6] = "dc6",
> +		[CXL_DECODER_MODE_DC7] = "dc7",
>  	};
>  
> -	if (mode < CXL_DECODER_MODE_NONE || mode > CXL_DECODER_MODE_RAM)
> +	if (mode < CXL_DECODER_MODE_NONE || mode > CXL_DECODER_MODE_DC7)
>  		mode = CXL_DECODER_MODE_NONE;
>  	return names[mode];
>  }
> @@ -221,9 +239,35 @@ cxl_decoder_mode_from_ident(const char *ident)
>  		return CXL_DECODER_MODE_RAM;
>  	else if (strcmp(ident, "pmem") == 0)
>  		return CXL_DECODER_MODE_PMEM;
> +	else if (strcmp(ident, "dc0") == 0)
> +		return CXL_DECODER_MODE_DC0;
> +	else if (strcmp(ident, "dc1") == 0)
> +		return CXL_DECODER_MODE_DC1;
> +	else if (strcmp(ident, "dc2") == 0)
> +		return CXL_DECODER_MODE_DC2;
> +	else if (strcmp(ident, "dc3") == 0)
> +		return CXL_DECODER_MODE_DC3;
> +	else if (strcmp(ident, "dc4") == 0)
> +		return CXL_DECODER_MODE_DC4;
> +	else if (strcmp(ident, "dc5") == 0)
> +		return CXL_DECODER_MODE_DC5;
> +	else if (strcmp(ident, "dc6") == 0)
> +		return CXL_DECODER_MODE_DC6;
> +	else if (strcmp(ident, "dc7") == 0)
> +		return CXL_DECODER_MODE_DC7;
>  	return CXL_DECODER_MODE_NONE;
>  }
>  
> +static inline bool cxl_decoder_mode_is_dc(enum cxl_decoder_mode mode)
> +{
> +	return (mode >= CXL_DECODER_MODE_DC0 && mode <= CXL_DECODER_MODE_DC7);
> +}
> +
> +static inline int cxl_decoder_dc_mode_to_index(enum cxl_decoder_mode mode)
> +{
> +	return mode - CXL_DECODER_MODE_DC0;
> +}
> +
>  enum cxl_decoder_mode cxl_decoder_get_mode(struct cxl_decoder *decoder);
>  int cxl_decoder_set_mode(struct cxl_decoder *decoder,
>  			 enum cxl_decoder_mode mode);
> @@ -248,6 +292,7 @@ enum cxl_decoder_target_type {
>  enum cxl_decoder_target_type
>  cxl_decoder_get_target_type(struct cxl_decoder *decoder);
>  bool cxl_decoder_is_pmem_capable(struct cxl_decoder *decoder);
> +bool cxl_decoder_is_dc_capable(struct cxl_decoder *decoder, int index);
>  bool cxl_decoder_is_volatile_capable(struct cxl_decoder *decoder);
>  bool cxl_decoder_is_mem_capable(struct cxl_decoder *decoder);
>  bool cxl_decoder_is_accelmem_capable(struct cxl_decoder *decoder);
> @@ -258,6 +303,8 @@ unsigned int cxl_decoder_get_interleave_ways(struct cxl_decoder *decoder);
>  struct cxl_region *cxl_decoder_get_region(struct cxl_decoder *decoder);
>  struct cxl_region *cxl_decoder_create_pmem_region(struct cxl_decoder *decoder);
>  struct cxl_region *cxl_decoder_create_ram_region(struct cxl_decoder *decoder);
> +struct cxl_region *cxl_decoder_create_dc_region(struct cxl_decoder *decoder,
> +						enum cxl_decoder_mode mode);
>  struct cxl_decoder *cxl_decoder_get_by_name(struct cxl_ctx *ctx,
>  					    const char *ident);
>  struct cxl_memdev *cxl_decoder_get_memdev(struct cxl_decoder *decoder);
> @@ -308,6 +355,7 @@ enum cxl_region_mode {
>  	CXL_REGION_MODE_MIXED = CXL_DECODER_MODE_MIXED,
>  	CXL_REGION_MODE_PMEM = CXL_DECODER_MODE_PMEM,
>  	CXL_REGION_MODE_RAM = CXL_DECODER_MODE_RAM,
> +	CXL_REGION_MODE_DC,
>  };
>  
>  static inline const char *cxl_region_mode_name(enum cxl_region_mode mode)
> @@ -317,9 +365,10 @@ static inline const char *cxl_region_mode_name(enum cxl_region_mode mode)
>  		[CXL_REGION_MODE_MIXED] = "mixed",
>  		[CXL_REGION_MODE_PMEM] = "pmem",
>  		[CXL_REGION_MODE_RAM] = "ram",
> +		[CXL_REGION_MODE_DC] = "dc",
>  	};
>  
> -	if (mode < CXL_REGION_MODE_NONE || mode > CXL_REGION_MODE_RAM)
> +	if (mode < CXL_REGION_MODE_NONE || mode > CXL_REGION_MODE_DC)
>  		mode = CXL_REGION_MODE_NONE;
>  	return names[mode];
>  }
> @@ -333,6 +382,8 @@ cxl_region_mode_from_ident(const char *ident)
>  		return CXL_REGION_MODE_RAM;
>  	else if (strcmp(ident, "pmem") == 0)
>  		return CXL_REGION_MODE_PMEM;
> +	else if (strcmp(ident, "dc") == 0)
> +		return CXL_REGION_MODE_DC;
>  	return CXL_REGION_MODE_NONE;
>  }
>  
> diff --git a/cxl/memdev.c b/cxl/memdev.c
> index 6e44d1578d03b6af998502e54714635b8f31b556..0a7d350efe9e612cd67d32328cca286dcdcb2991 100644
> --- a/cxl/memdev.c
> +++ b/cxl/memdev.c
> @@ -269,8 +269,13 @@ static int __reserve_dpa(struct cxl_memdev *memdev,
>  
>  	if (mode == CXL_DECODER_MODE_RAM)
>  		avail_dpa = cxl_memdev_get_ram_size(memdev);
> -	else
> +	else if (mode == CXL_DECODER_MODE_PMEM)
>  		avail_dpa = cxl_memdev_get_pmem_size(memdev);
> +	else if (cxl_decoder_mode_is_dc(mode)) {
> +		int i = cxl_decoder_dc_mode_to_index(mode);
> +
> +		avail_dpa = cxl_memdev_get_dc_size(memdev, i);
> +	}
>  
>  	cxl_decoder_foreach(port, decoder) {
>  		size = cxl_decoder_get_dpa_size(decoder);
> diff --git a/cxl/region.c b/cxl/region.c
> index 207cf2d003148992255c715f286bc0f38de2ca84..310694ae07fae25f13d032a30c130bf7d3394388 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -78,7 +78,7 @@ OPT_INTEGER('w', "ways", &param.ways, \
>  OPT_INTEGER('g', "granularity", &param.granularity,  \
>  	    "granularity of the interleave set"), \
>  OPT_STRING('t', "type", &param.type, \
> -	   "region type", "region type - 'pmem' or 'ram'"), \
> +	   "region type", "region type - 'pmem', 'ram', 'dcX'"), \
>  OPT_STRING('U', "uuid", &param.uuid, \
>  	   "region uuid", "uuid for the new region (default: autogenerate)"), \
>  OPT_BOOLEAN('m', "memdevs", &param.memdevs, \
> @@ -400,9 +400,22 @@ static int parse_region_options(int argc, const char **argv,
>  	}
>  }
>  
> +static int dc_mode_to_region_index(enum cxl_decoder_mode mode)
> +{
> +	int index = 0;
> +
> +	for (unsigned int i = CXL_DECODER_MODE_DC0; i <= CXL_DECODER_MODE_DC7; i++) {
> +		if (mode == i)
> +			return index;
> +		index++;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static void collect_minsize(struct cxl_ctx *ctx, struct parsed_params *p)
>  {
> -	int i;
> +	int i, index;
>  
>  	for (i = 0; i < p->ways; i++) {
>  		struct json_object *jobj =
> @@ -417,6 +430,10 @@ static void collect_minsize(struct cxl_ctx *ctx, struct parsed_params *p)
>  		case CXL_DECODER_MODE_PMEM:
>  			size = cxl_memdev_get_pmem_size(memdev);
>  			break;
> +		case CXL_DECODER_MODE_DC0 ... CXL_DECODER_MODE_DC7:
> +			index =  dc_mode_to_region_index(p->mode);
Extra space after =
> +			size = cxl_memdev_get_dc_size(memdev, index);
> +			break;
>  		default:
>  			/* Shouldn't ever get here */ ;
>  		}
> @@ -473,6 +490,7 @@ static int validate_decoder(struct cxl_decoder *decoder,
>  			    struct parsed_params *p)
>  {
>  	const char *devname = cxl_decoder_get_devname(decoder);
> +	int index;
>  	int rc;
>  
>  	switch(p->mode) {
> @@ -488,6 +506,13 @@ static int validate_decoder(struct cxl_decoder *decoder,
>  			return -EINVAL;
>  		}
>  		break;
> +	case CXL_DECODER_MODE_DC0 ... CXL_DECODER_MODE_DC7:
> +		index =  dc_mode_to_region_index(p->mode);

Extra space after =

DJ

> +		if (!cxl_decoder_is_dc_capable(decoder, index)) {
> +			log_err(&rl, "%s is not dc%d capable\n", devname, index);
> +			return -EINVAL;
> +		}
> +		break;
>  	default:
>  		log_err(&rl, "unknown type: %s\n", param.type);
>  		return -EINVAL;
> @@ -502,12 +527,25 @@ static int validate_decoder(struct cxl_decoder *decoder,
>  	return 0;
>  }
>  
> +static enum cxl_decoder_mode dc_region_index_to_mode(int index)
> +{
> +	return (CXL_DECODER_MODE_DC0 + index);
> +}
> +
>  static void set_type_from_decoder(struct cxl_ctx *ctx, struct parsed_params *p)
>  {
>  	/* if param.type was explicitly specified, nothing to do here */
>  	if (param.type)
>  		return;
>  
> +	/* Only chose DC if it is the only type available */
> +	for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
> +		if (cxl_decoder_is_dc_capable(p->root_decoder, index)) {
> +			p->mode = dc_region_index_to_mode(index);
> +			break;
> +		}
> +	}
> +
>  	/*
>  	 * default to pmem if both types are set, otherwise the single
>  	 * capability dominates.
> @@ -699,6 +737,13 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  				param.root_decoder);
>  			return -ENXIO;
>  		}
> +	} else if (cxl_decoder_mode_is_dc(p->mode)) {
> +		region = cxl_decoder_create_dc_region(p->root_decoder, p->mode);
> +		if (!region) {
> +			log_err(&rl, "failed to create region under %s\n",
> +				param.root_decoder);
> +			return -ENXIO;
> +		}
>  	} else {
>  		log_err(&rl, "region type '%s' is not supported\n",
>  			param.type);
> 

Re: [ndctl PATCH v2 4/6] cxl/region: Add creation of Dynamic capacity regions

[Ira Weiny]

Dave Jiang wrote:
> 
> 
> On 11/4/24 7:10 PM, ira.weiny@intel.com wrote:
> > From: Navneet Singh <navneet.singh@intel.com>
> > 
> > CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> > with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> > spare and defined as dynamic capacity (dc).
> > 
> > Add support for DCD devices.  Query for DCD capabilities.  Add the
> > ability to add DC partitions to a CXL DC region.
> > 
> > Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> > Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> > Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> > Co-authored-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> A few small things below.

[snip]

> > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> > index 5cbfb3e7d466b491ef87ea285f7e50d3bac230db..4caa2d02313bf71960971c4eaa67fa42cea08d55 100644
> > --- a/cxl/lib/libcxl.c
> > +++ b/cxl/lib/libcxl.c
> > @@ -1267,7 +1267,6 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
> >  	char buf[SYSFS_ATTR_SIZE];
> >  	struct stat st;
> >  	char *host;
> > -
> 
> Stray line delete

Oh...  I thought that was adding a space here when I reviewed the diff.
Yes fixed.

> >  	if (!path)
> >  		return NULL;
> >  	dbg(ctx, "%s: base: \'%s\'\n", devname, cxlmem_base);
> > @@ -1304,6 +1303,19 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)

[snip]

> > @@ -1540,6 +1552,14 @@ CXL_EXPORT int cxl_memdev_get_ram_qos_class(struct cxl_memdev *memdev)
> >  	return memdev->ram_qos_class;
> >  }
> >  
> > +CXL_EXPORT unsigned long long cxl_memdev_get_dc_size(struct cxl_memdev *memdev, int index)
> 
> If you make index 'unsigned int', you can skip the index >= 0 check below right?

True.  To be safe the following needs to be done then.

diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 17ed682548b9..e0584cd658ec 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -263,8 +263,10 @@ static inline bool cxl_decoder_mode_is_dc(enum cxl_decoder_mode mode)
        return (mode >= CXL_DECODER_MODE_DC0 && mode <= CXL_DECODER_MODE_DC7);
 }
 
-static inline int cxl_decoder_dc_mode_to_index(enum cxl_decoder_mode mode)
+static inline unsigned cxl_decoder_dc_mode_to_index(enum cxl_decoder_mode mode)
 {
+       if (mode < CXL_DECODER_MODE_DC0)
+               return 0;
        return mode - CXL_DECODER_MODE_DC0;
 }
 
I'm not sure which is best.  Right now cxl_decoder_dc_mode_to_index() is only
called if the mode has been checked to be DC.  So the above check is a noop.
But in general returning an errno would be better for
cxl_decoder_dc_mode_to_index() is in error.

I'll clean this up to match what is in dc_mode_to_region_index().  See below.

[snip]

> > @@ -2318,6 +2354,14 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
> >  	case CXL_PORT_SWITCH:
> >  		decoder->pmem_capable = true;
> >  		decoder->volatile_capable = true;
> > +		decoder->dc_capable[0] = true;
> > +		decoder->dc_capable[1] = true;
> > +		decoder->dc_capable[2] = true;
> > +		decoder->dc_capable[3] = true;
> > +		decoder->dc_capable[4] = true;
> > +		decoder->dc_capable[5] = true;
> > +		decoder->dc_capable[6] = true;
> > +		decoder->dc_capable[7] = true;
> 
> for loop?

Sure.  Changed.

> 
> >  		decoder->mem_capable = true;
> >  		decoder->accelmem_capable = true;
> >  		sprintf(path, "%s/locked", cxldecoder_base);

[snip]

> > @@ -2648,6 +2724,14 @@ CXL_EXPORT bool cxl_decoder_is_mem_capable(struct cxl_decoder *decoder)
> >  	return decoder->mem_capable;
> >  }
> >  
> > +CXL_EXPORT bool cxl_decoder_is_dc_capable(struct cxl_decoder *decoder, int index)
> 
> Same comment as above WRT index.

This forces changes outside of cxl_decoder_is_dc_capable().  Specifically with
dc_mode_to_region_index().  But I actually like checking for the error from
dc_mode_to_region_index() outside of cxl_decoder_is_dc_capable().

I've made the change.


[snip]

> > diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> > index 0f45be89b6a00477d13fb6d7f1906213a3073c48..10abfa63dfc759b1589f9f039da1b920f8eb605e 100644
> > --- a/cxl/lib/private.h
> > +++ b/cxl/lib/private.h
> > @@ -12,7 +12,6 @@
> >  #include <util/bitmap.h>
> >  
> >  #define CXL_EXPORT __attribute__ ((visibility("default")))
> > -
> 
> Errant line deletion

Fixed.

> 
> >  struct cxl_pmem {
> >  	int id;
> >  	void *dev_buf;
> > @@ -47,6 +46,9 @@ struct cxl_memdev {
> >  	struct list_node list;
> >  	unsigned long long pmem_size;
> >  	unsigned long long ram_size;
> > +	unsigned long long dc_size[MAX_NUM_DC_REGIONS];
> > +	unsigned long long dc_qos_class[MAX_NUM_DC_REGIONS];
> > +	int dc_partition_count;
> >  	int ram_qos_class;
> >  	int pmem_qos_class;
> >  	int payload_max;
> > @@ -111,6 +113,7 @@ struct cxl_endpoint {
> >  	struct cxl_memdev *memdev;
> >  };
> >  
> > +
> 
> Errant line addition

Fixed.


[snip]

> > @@ -488,6 +506,13 @@ static int validate_decoder(struct cxl_decoder *decoder,
> >  			return -EINVAL;
> >  		}
> >  		break;
> > +	case CXL_DECODER_MODE_DC0 ... CXL_DECODER_MODE_DC7:
> > +		index =  dc_mode_to_region_index(p->mode);
> 
> Extra space after =

Fixed.

Thanks for the review!

Ira
> 
> DJ

[snip]

Re: [ndctl PATCH v2 4/6] cxl/region: Add creation of Dynamic capacity regions

[Alison Schofield]

On Mon, Nov 04, 2024 at 08:10:48PM -0600, Ira Weiny wrote:
> From: Navneet Singh <navneet.singh@intel.com>
> 
> CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> spare and defined as dynamic capacity (dc).
> 
> Add support for DCD devices.  Query for DCD capabilities.  Add the
> ability to add DC partitions to a CXL DC region.
> 
> Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Co-authored-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> ---
> Changes:
> [Fan: Properly initialize index]
> ---
>  cxl/json.c         | 26 +++++++++++++++
>  cxl/lib/libcxl.c   | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  cxl/lib/libcxl.sym |  3 ++
>  cxl/lib/private.h  |  6 +++-
>  cxl/libcxl.h       | 55 +++++++++++++++++++++++++++++--

Hi Ira,

This is 'do as I say, not as I do' feedback, because I realize while
reviewing this, that I've been remiss in keeping the libcxl man page
updated.

Please document the new libcxl helpers here:
Documentation/cxl/lib/libcxl.txt  (it emits in 'man libcxl'

Sometimes it's just a quick add of the new helper. You can see what is
there and decide if additional verbage about DCD regions is needed.

Thanks and I'll do some catchup in that file after you ;)

-- Alison

snip

Re: [ndctl PATCH v2 4/6] cxl/region: Add creation of Dynamic capacity regions

[Alison Schofield]

On Mon, Nov 04, 2024 at 08:10:48PM -0600, Ira Weiny wrote:
> From: Navneet Singh <navneet.singh@intel.com>
> 
> CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> spare and defined as dynamic capacity (dc).
> 
> Add support for DCD devices.  Query for DCD capabilities.  Add the
> ability to add DC partitions to a CXL DC region.
> 
> Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Co-authored-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> ---
> Changes:
> [Fan: Properly initialize index]
> ---
>  cxl/json.c         | 26 +++++++++++++++
>  cxl/lib/libcxl.c   | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  cxl/lib/libcxl.sym |  3 ++
>  cxl/lib/private.h  |  6 +++-
>  cxl/libcxl.h       | 55 +++++++++++++++++++++++++++++--
>  cxl/memdev.c       |  7 +++-
>  cxl/region.c       | 49 ++++++++++++++++++++++++++--
>  7 files changed, 234 insertions(+), 7 deletions(-)
> 
> diff --git a/cxl/json.c b/cxl/json.c
> index dcd3cc28393faf7e8adf299a857531ecdeaac50a..915b2716a524fa8929ed34b01a7cb6590b61d4b7 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -754,10 +754,12 @@ err_free:
>  	return jpoison;
>  }
>  
> +#define DC_SIZE_NAME_LEN 64
>  struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  		unsigned long flags)
>  {
>  	const char *devname = cxl_memdev_get_devname(memdev);
> +	char size_name[DC_SIZE_NAME_LEN];
>  	struct json_object *jdev, *jobj;
>  	unsigned long long serial, size;
>  	const char *fw_version;
> @@ -800,6 +802,17 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  		}
>  	}
>  
> +	for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
> +		size = cxl_memdev_get_dc_size(memdev, index);
> +		if (size) {
> +			jobj = util_json_object_size(size, flags);
> +			if (jobj) {
> +				sprintf(size_name, "dc%d_size", index);
> +				json_object_object_add(jdev,
> +						       size_name, jobj);
> +			}
> +		}
> +	}

how about reducing above indentation -

		if (!size)
			continue;
		jobj = util_json_object_size(size, flags);
		if (!jobj)
			continue;
		sprintf(size_name, "dc%d_size", index);
		json_object_object_add(jdev, size_name, jobj);




>  	if (flags & UTIL_JSON_HEALTH) {
>  		jobj = util_cxl_memdev_health_to_json(memdev, flags);
>  		if (jobj)
> @@ -948,11 +961,13 @@ struct json_object *util_cxl_bus_to_json(struct cxl_bus *bus,
>  	return jbus;
>  }
>  
> +#define DC_CAPABILITY_NAME_LEN 16
>  struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
>  					     unsigned long flags)
>  {
>  	const char *devname = cxl_decoder_get_devname(decoder);
>  	struct cxl_port *port = cxl_decoder_get_port(decoder);
> +	char dc_capable_name[DC_CAPABILITY_NAME_LEN];
>  	struct json_object *jdecoder, *jobj;
>  	struct cxl_region *region;
>  	u64 val, size;
> @@ -1059,6 +1074,17 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
>  				json_object_object_add(
>  					jdecoder, "volatile_capable", jobj);
>  		}
> +		for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
> +			if (cxl_decoder_is_dc_capable(decoder, index)) {
> +				jobj = json_object_new_boolean(true);
> +				if (jobj) {
> +					sprintf(dc_capable_name, "dc%d_capable", index);
> +					json_object_object_add(jdecoder,
> +							       dc_capable_name,
> +							       jobj);
> +				}
> +			}

and similar above.


snip

Re: [ndctl PATCH v2 4/6] cxl/region: Add creation of Dynamic capacity regions

[Ira Weiny]

Alison Schofield wrote:
> On Mon, Nov 04, 2024 at 08:10:48PM -0600, Ira Weiny wrote:
> > From: Navneet Singh <navneet.singh@intel.com>
> > 
> > CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> > with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> > spare and defined as dynamic capacity (dc).
> > 
> > Add support for DCD devices.  Query for DCD capabilities.  Add the
> > ability to add DC partitions to a CXL DC region.
> > 
> > Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> > Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> > Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> > Co-authored-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > 
> > ---
> > Changes:
> > [Fan: Properly initialize index]
> > ---
> >  cxl/json.c         | 26 +++++++++++++++
> >  cxl/lib/libcxl.c   | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  cxl/lib/libcxl.sym |  3 ++
> >  cxl/lib/private.h  |  6 +++-
> >  cxl/libcxl.h       | 55 +++++++++++++++++++++++++++++--
> >  cxl/memdev.c       |  7 +++-
> >  cxl/region.c       | 49 ++++++++++++++++++++++++++--
> >  7 files changed, 234 insertions(+), 7 deletions(-)
> > 
> > diff --git a/cxl/json.c b/cxl/json.c
> > index dcd3cc28393faf7e8adf299a857531ecdeaac50a..915b2716a524fa8929ed34b01a7cb6590b61d4b7 100644
> > --- a/cxl/json.c
> > +++ b/cxl/json.c
> > @@ -754,10 +754,12 @@ err_free:
> >  	return jpoison;
> >  }
> >  
> > +#define DC_SIZE_NAME_LEN 64
> >  struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
> >  		unsigned long flags)
> >  {
> >  	const char *devname = cxl_memdev_get_devname(memdev);
> > +	char size_name[DC_SIZE_NAME_LEN];
> >  	struct json_object *jdev, *jobj;
> >  	unsigned long long serial, size;
> >  	const char *fw_version;
> > @@ -800,6 +802,17 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
> >  		}
> >  	}
> >  
> > +	for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
> > +		size = cxl_memdev_get_dc_size(memdev, index);
> > +		if (size) {
> > +			jobj = util_json_object_size(size, flags);
> > +			if (jobj) {
> > +				sprintf(size_name, "dc%d_size", index);
> > +				json_object_object_add(jdev,
> > +						       size_name, jobj);
> > +			}
> > +		}
> > +	}
> 
> how about reducing above indentation -
> 
> 		if (!size)
> 			continue;
> 		jobj = util_json_object_size(size, flags);
> 		if (!jobj)
> 			continue;
> 		sprintf(size_name, "dc%d_size", index);
> 		json_object_object_add(jdev, size_name, jobj);
> 

ok yea that works.


> 
> 
> 
> >  	if (flags & UTIL_JSON_HEALTH) {
> >  		jobj = util_cxl_memdev_health_to_json(memdev, flags);
> >  		if (jobj)
> > @@ -948,11 +961,13 @@ struct json_object *util_cxl_bus_to_json(struct cxl_bus *bus,
> >  	return jbus;
> >  }
> >  
> > +#define DC_CAPABILITY_NAME_LEN 16
> >  struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
> >  					     unsigned long flags)
> >  {
> >  	const char *devname = cxl_decoder_get_devname(decoder);
> >  	struct cxl_port *port = cxl_decoder_get_port(decoder);
> > +	char dc_capable_name[DC_CAPABILITY_NAME_LEN];
> >  	struct json_object *jdecoder, *jobj;
> >  	struct cxl_region *region;
> >  	u64 val, size;
> > @@ -1059,6 +1074,17 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
> >  				json_object_object_add(
> >  					jdecoder, "volatile_capable", jobj);
> >  		}
> > +		for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
> > +			if (cxl_decoder_is_dc_capable(decoder, index)) {
> > +				jobj = json_object_new_boolean(true);
> > +				if (jobj) {
> > +					sprintf(dc_capable_name, "dc%d_capable", index);
> > +					json_object_object_add(jdecoder,
> > +							       dc_capable_name,
> > +							       jobj);
> > +				}
> > +			}
> 
> and similar above.

done.

Thanks,
Ira

> 
> 
> snip
> 
> 

Re: [ndctl PATCH v2 4/6] cxl/region: Add creation of Dynamic capacity regions

[Alison Schofield]

On Mon, Nov 04, 2024 at 08:10:48PM -0600, Ira Weiny wrote:
> From: Navneet Singh <navneet.singh@intel.com>
> 
> CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> spare and defined as dynamic capacity (dc).
> 
> Add support for DCD devices.  Query for DCD capabilities.  Add the
> ability to add DC partitions to a CXL DC region.
> 
> Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Co-authored-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> ---
> Changes:
> [Fan: Properly initialize index]
> ---
>  cxl/json.c         | 26 +++++++++++++++
>  cxl/lib/libcxl.c   | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  cxl/lib/libcxl.sym |  3 ++
>  cxl/lib/private.h  |  6 +++-
>  cxl/libcxl.h       | 55 +++++++++++++++++++++++++++++--
>  cxl/memdev.c       |  7 +++-
>  cxl/region.c       | 49 ++++++++++++++++++++++++++--
>  7 files changed, 234 insertions(+), 7 deletions(-)


As I send this third snippet of patch review and wonder if I'm being
odd, I've decided no, it's not me - this patch is doing too many
things in one patch.

If you look at the history in ndctl, a new feature add like this is
typically added in mulitple patches like:
- add the libcxl support
- enable the creation of DCD regions
- add DCD info to cxl list

Can you split this up, along the lines I suggest or however it makes
sense for this feature.

and one more bit below...

> 
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index b5d9bdcc38e09812f26afc1cb0e804f86784b8e6..351da7512e05080d847fd87740488d613462dbc9 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -19,6 +19,7 @@ global:
>  	cxl_memdev_get_ctx;
>  	cxl_memdev_get_pmem_size;
>  	cxl_memdev_get_ram_size;
> +	cxl_memdev_get_dc_size;
>  	cxl_memdev_get_firmware_verison;
>  	cxl_cmd_get_devname;
>  	cxl_cmd_new_raw;
> @@ -247,6 +248,8 @@ LIBCXL_5 {
>  global:
>  	cxl_region_get_mode;
>  	cxl_decoder_create_ram_region;
> +	cxl_decoder_is_dc_capable;
> +	cxl_decoder_create_dc_region;
>  	cxl_region_get_daxctl_region;
>  	cxl_port_get_parent_dport;
>  } LIBCXL_4;

New symbols need to be appended in a new section, not inserted.
See predecessors.


snip


> 

Re: [ndctl PATCH v2 4/6] cxl/region: Add creation of Dynamic capacity regions

[Alison Schofield]

On Mon, Nov 04, 2024 at 08:10:48PM -0600, Ira Weiny wrote:
> From: Navneet Singh <navneet.singh@intel.com>
> 
> CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> spare and defined as dynamic capacity (dc).
> 
> Add support for DCD devices.  Query for DCD capabilities.  Add the
> ability to add DC partitions to a CXL DC region.
> 
> Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Co-authored-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
 
Since you are submitting as a Co-developer, according to this:

https://docs.kernel.org/process/submitting-patches.html

it should look like this: 

Co-developed-by: Sushant1 Kumar <sushant1.kumar@intel.com>
Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
Signed-off-by: Navneet Singh <navneet.singh@intel.com>
Co-developed-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

I recognize the Co-authored-by Tag as commonly used w github,
while ndctl follows the kernel customs 

Re: [ndctl PATCH v2 4/6] cxl/region: Add creation of Dynamic capacity regions

[Ira Weiny]

Alison Schofield wrote:
> On Mon, Nov 04, 2024 at 08:10:48PM -0600, Ira Weiny wrote:
> > From: Navneet Singh <navneet.singh@intel.com>
> > 
> > CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> > with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> > spare and defined as dynamic capacity (dc).
> > 
> > Add support for DCD devices.  Query for DCD capabilities.  Add the
> > ability to add DC partitions to a CXL DC region.
> > 
> > Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> > Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> > Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> > Co-authored-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>  
> Since you are submitting as a Co-developer, according to this:
> 
> https://docs.kernel.org/process/submitting-patches.html
> 
> it should look like this: 
> 
> Co-developed-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> Co-developed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> I recognize the Co-authored-by Tag as commonly used w github,
> while ndctl follows the kernel customs 
> 

I've changed it.  And have carried that with the split of the patch

Ira

[ndctl PATCH v2 5/6] ndctl/cxl: Add extent output to region query

[Ira Weiny]

DCD regions have 0 or more extents.  The ability to list those and their
properties is useful to end users.

Add extent output to region queries.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 Documentation/cxl/cxl-list.txt |   4 ++
 cxl/filter.h                   |   3 +
 cxl/json.c                     |  47 ++++++++++++++
 cxl/json.h                     |   3 +
 cxl/lib/libcxl.c               | 138 +++++++++++++++++++++++++++++++++++++++++
 cxl/lib/libcxl.sym             |   5 ++
 cxl/lib/private.h              |  11 ++++
 cxl/libcxl.h                   |  11 ++++
 cxl/list.c                     |   3 +
 util/json.h                    |   1 +
 10 files changed, 226 insertions(+)

diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
index 9a9911e7dd9bba561c6202784017db1bb4b9f4bd..71fd313cfec2509c79f8ad1e0f64857d0d804c13 100644
--- a/Documentation/cxl/cxl-list.txt
+++ b/Documentation/cxl/cxl-list.txt
@@ -411,6 +411,10 @@ OPTIONS
 }
 ----
 
+-N::
+--extents::
+	Extend Dynamic Capacity region listings extent information.
+
 -r::
 --region::
 	Specify CXL region device name(s), or device id(s), to filter the listing.
diff --git a/cxl/filter.h b/cxl/filter.h
index 956a46e0c7a9f05abf696cce97a365164e95e50d..a31b80c87ccac407bd4ff98b302a23b33cbe413c 100644
--- a/cxl/filter.h
+++ b/cxl/filter.h
@@ -31,6 +31,7 @@ struct cxl_filter_params {
 	bool alert_config;
 	bool dax;
 	bool media_errors;
+	bool extents;
 	int verbose;
 	struct log_ctx ctx;
 };
@@ -91,6 +92,8 @@ static inline unsigned long cxl_filter_to_flags(struct cxl_filter_params *param)
 		flags |= UTIL_JSON_DAX | UTIL_JSON_DAX_DEVS;
 	if (param->media_errors)
 		flags |= UTIL_JSON_MEDIA_ERRORS;
+	if (param->extents)
+		flags |= UTIL_JSON_EXTENTS;
 	return flags;
 }
 
diff --git a/cxl/json.c b/cxl/json.c
index 915b2716a524fa8929ed34b01a7cb6590b61d4b7..0d6644916b8dd2cdee39ac9bf7310b5e318f0e2d 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -1170,6 +1170,50 @@ void util_cxl_mappings_append_json(struct json_object *jregion,
 	json_object_object_add(jregion, "mappings", jmappings);
 }
 
+void util_cxl_extents_append_json(struct json_object *jregion,
+				  struct cxl_region *region,
+				  unsigned long flags)
+{
+	struct json_object *jextents;
+	struct cxl_region_extent *extent;
+
+	jextents = json_object_new_array();
+	if (!jextents)
+		return;
+
+	cxl_extent_foreach(region, extent) {
+		struct json_object *jextent, *jobj;
+		unsigned long long val;
+		char tag_str[40];
+		uuid_t tag;
+
+		jextent = json_object_new_object();
+		if (!jextent)
+			continue;
+
+		val = cxl_extent_get_offset(extent);
+		jobj = util_json_object_hex(val, flags);
+		if (jobj)
+			json_object_object_add(jextent, "offset", jobj);
+
+		val = cxl_extent_get_length(extent);
+		jobj = util_json_object_size(val, flags);
+		if (jobj)
+			json_object_object_add(jextent, "length", jobj);
+
+		cxl_extent_get_tag(extent, tag);
+		uuid_unparse(tag, tag_str);
+		jobj = json_object_new_string(tag_str);
+		if (jobj)
+			json_object_object_add(jextent, "tag", jobj);
+
+		json_object_array_add(jextents, jextent);
+		json_object_set_userdata(jextent, extent, NULL);
+	}
+
+	json_object_object_add(jregion, "extents", jextents);
+}
+
 struct json_object *util_cxl_region_to_json(struct cxl_region *region,
 					     unsigned long flags)
 {
@@ -1256,6 +1300,9 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
 		}
 	}
 
+	if (flags & UTIL_JSON_EXTENTS)
+		util_cxl_extents_append_json(jregion, region, flags);
+
 	if (cxl_region_qos_class_mismatch(region)) {
 		jobj = json_object_new_boolean(true);
 		if (jobj)
diff --git a/cxl/json.h b/cxl/json.h
index eb7572be4106baf0469ba9243a9a767d07df8882..f9c07ab41a337838b75ffee4486f6c48ddc99863 100644
--- a/cxl/json.h
+++ b/cxl/json.h
@@ -20,6 +20,9 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
 void util_cxl_mappings_append_json(struct json_object *jregion,
 				  struct cxl_region *region,
 				  unsigned long flags);
+void util_cxl_extents_append_json(struct json_object *jregion,
+				  struct cxl_region *region,
+				  unsigned long flags);
 void util_cxl_targets_append_json(struct json_object *jdecoder,
 				  struct cxl_decoder *decoder,
 				  const char *ident, const char *serial,
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 4caa2d02313bf71960971c4eaa67fa42cea08d55..8ebb100df0c6078630bbe45fbed270709dfb4a5f 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -568,6 +568,7 @@ static void *add_cxl_region(void *parent, int id, const char *cxlregion_base)
 	region->ctx = ctx;
 	region->decoder = decoder;
 	list_head_init(&region->mappings);
+	list_head_init(&region->extents);
 
 	region->dev_path = strdup(cxlregion_base);
 	if (!region->dev_path)
@@ -1178,6 +1179,143 @@ cxl_mapping_get_next(struct cxl_memdev_mapping *mapping)
 	return list_next(&region->mappings, mapping, list);
 }
 
+static void cxl_extents_init(struct cxl_region *region)
+{
+	const char *devname = cxl_region_get_devname(region);
+	struct cxl_ctx *ctx = cxl_region_get_ctx(region);
+	char *extent_path, *dax_region_path;
+	struct dirent *de;
+	DIR *dir = NULL;
+
+	if (region->extents_init)
+		return;
+	region->extents_init = 1;
+
+	dbg(ctx, "Checking extents: %s\n", region->dev_path);
+
+	dax_region_path = calloc(1, strlen(region->dev_path) + 64);
+	if (!dax_region_path) {
+		err(ctx, "%s: allocation failure\n", devname);
+		return;
+	}
+
+	extent_path = calloc(1, strlen(region->dev_path) + 100);
+	if (!extent_path) {
+		err(ctx, "%s: allocation failure\n", devname);
+		free(dax_region_path);
+		return;
+	}
+
+	sprintf(dax_region_path, "%s/dax_region%d",
+		region->dev_path, region->id);
+	dir = opendir(dax_region_path);
+	if (!dir) {
+		err(ctx, "no extents found: %s\n", dax_region_path);
+		free(extent_path);
+		free(dax_region_path);
+		return;
+	}
+
+	while ((de = readdir(dir)) != NULL) {
+		struct cxl_region_extent *extent;
+		char buf[SYSFS_ATTR_SIZE];
+		u64 offset, length;
+		int id, region_id;
+
+		if (sscanf(de->d_name, "extent%d.%d", &region_id, &id) != 2)
+			continue;
+
+		sprintf(extent_path, "%s/extent%d.%d/offset",
+			dax_region_path, region_id, id);
+		if (sysfs_read_attr(ctx, extent_path, buf) < 0) {
+			err(ctx, "%s: failed to read extent%d.%d/offset\n",
+				devname, region_id, id);
+			continue;
+		}
+
+		offset = strtoull(buf, NULL, 0);
+		if (offset == ERANGE) {
+			err(ctx, "%s extent%d.%d: failed to read offset\n",
+				devname, region_id, id);
+			continue;
+		}
+
+		sprintf(extent_path, "%s/extent%d.%d/length",
+			dax_region_path, region_id, id);
+		if (sysfs_read_attr(ctx, extent_path, buf) < 0) {
+			err(ctx, "%s: failed to read extent%d.%d/length\n",
+				devname, region_id, id);
+			continue;
+		}
+
+		length = strtoull(buf, NULL, 0);
+		if (length == ERANGE) {
+			err(ctx, "%s extent%d.%d: failed to read length\n",
+				devname, region_id, id);
+			continue;
+		}
+
+		sprintf(extent_path, "%s/extent%d.%d/tag",
+			dax_region_path, region_id, id);
+		buf[0] = '\0';
+		if (sysfs_read_attr(ctx, extent_path, buf) != 0)
+			dbg(ctx, "%s extent%d.%d: failed to read tag\n",
+				devname, region_id, id);
+
+		extent = calloc(1, sizeof(*extent));
+		if (!extent) {
+			err(ctx, "%s extent%d.%d: allocation failure\n",
+				devname, region_id, id);
+			continue;
+		}
+		if (strlen(buf) && uuid_parse(buf, extent->tag) < 0)
+			err(ctx, "%s:%s\n", extent_path, buf);
+		extent->region = region;
+		extent->offset = offset;
+		extent->length = length;
+
+		list_node_init(&extent->list);
+		list_add(&region->extents, &extent->list);
+	}
+	free(dax_region_path);
+	free(extent_path);
+	closedir(dir);
+}
+
+CXL_EXPORT struct cxl_region_extent *
+cxl_extent_get_first(struct cxl_region *region)
+{
+	cxl_extents_init(region);
+
+	return list_top(&region->extents, struct cxl_region_extent, list);
+}
+
+CXL_EXPORT struct cxl_region_extent *
+cxl_extent_get_next(struct cxl_region_extent *extent)
+{
+	struct cxl_region *region = extent->region;
+
+	return list_next(&region->extents, extent, list);
+}
+
+CXL_EXPORT unsigned long long
+cxl_extent_get_offset(struct cxl_region_extent *extent)
+{
+	return extent->offset;
+}
+
+CXL_EXPORT unsigned long long
+cxl_extent_get_length(struct cxl_region_extent *extent)
+{
+	return extent->length;
+}
+
+CXL_EXPORT void
+cxl_extent_get_tag(struct cxl_region_extent *extent, uuid_t tag)
+{
+	memcpy(tag, extent->tag, sizeof(uuid_t));
+}
+
 CXL_EXPORT struct cxl_decoder *
 cxl_mapping_get_decoder(struct cxl_memdev_mapping *mapping)
 {
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 351da7512e05080d847fd87740488d613462dbc9..37c3531115c73cdb69b96fa47bc88bbbb901f085 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -291,4 +291,9 @@ global:
 	cxl_memdev_trigger_poison_list;
 	cxl_region_trigger_poison_list;
 	cxl_region_get_region_mode;
+	cxl_extent_get_first;
+	cxl_extent_get_next;
+	cxl_extent_get_offset;
+	cxl_extent_get_length;
+	cxl_extent_get_tag;
 } LIBCXL_7;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index 10abfa63dfc759b1589f9f039da1b920f8eb605e..5b50b3f778a66a2266d6d5ee69e2a72cdad54a70 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -164,6 +164,7 @@ struct cxl_region {
 	struct cxl_decoder *decoder;
 	struct list_node list;
 	int mappings_init;
+	int extents_init;
 	struct cxl_ctx *ctx;
 	void *dev_buf;
 	size_t buf_len;
@@ -179,6 +180,7 @@ struct cxl_region {
 	struct daxctl_region *dax_region;
 	struct kmod_module *module;
 	struct list_head mappings;
+	struct list_head extents;
 };
 
 struct cxl_memdev_mapping {
@@ -188,6 +190,15 @@ struct cxl_memdev_mapping {
 	struct list_node list;
 };
 
+#define CXL_REGION_EXTENT_TAG 0x10
+struct cxl_region_extent {
+	struct cxl_region *region;
+	u64 offset;
+	u64 length;
+	uuid_t tag;
+	struct list_node list;
+};
+
 enum cxl_cmd_query_status {
 	CXL_CMD_QUERY_NOT_RUN = 0,
 	CXL_CMD_QUERY_OK,
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 17ed682548b970d57f016942badc76dce61bdeaf..b7c85a67224c86d17a41376c147364e1f88db080 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -448,6 +448,17 @@ unsigned int cxl_mapping_get_position(struct cxl_memdev_mapping *mapping);
              mapping != NULL; \
              mapping = cxl_mapping_get_next(mapping))
 
+struct cxl_region_extent;
+struct cxl_region_extent *cxl_extent_get_first(struct cxl_region *region);
+struct cxl_region_extent *cxl_extent_get_next(struct cxl_region_extent *extent);
+#define cxl_extent_foreach(region, extent) \
+        for (extent = cxl_extent_get_first(region); \
+             extent != NULL; \
+             extent = cxl_extent_get_next(extent))
+unsigned long long cxl_extent_get_offset(struct cxl_region_extent *extent);
+unsigned long long cxl_extent_get_length(struct cxl_region_extent *extent);
+void cxl_extent_get_tag(struct cxl_region_extent *extent, uuid_t tag);
+
 struct cxl_cmd;
 const char *cxl_cmd_get_devname(struct cxl_cmd *cmd);
 struct cxl_cmd *cxl_cmd_new_raw(struct cxl_memdev *memdev, int opcode);
diff --git a/cxl/list.c b/cxl/list.c
index 0b25d78248d5f4f529fd2c2e073e43895c722568..47d135166212b87449f960e94ee75657f7040ca9 100644
--- a/cxl/list.c
+++ b/cxl/list.c
@@ -59,6 +59,8 @@ static const struct option options[] = {
 		    "include alert configuration information"),
 	OPT_BOOLEAN('L', "media-errors", &param.media_errors,
 		    "include media-error information "),
+	OPT_BOOLEAN('N', "extents", &param.extents,
+		    "include extent information (Dynamic Capacity regions only)"),
 	OPT_INCR('v', "verbose", &param.verbose, "increase output detail"),
 #ifdef ENABLE_DEBUG
 	OPT_BOOLEAN(0, "debug", &debug, "debug list walk"),
@@ -135,6 +137,7 @@ int cmd_list(int argc, const char **argv, struct cxl_ctx *ctx)
 		param.decoders = true;
 		param.targets = true;
 		param.regions = true;
+		param.extents = true;
 		/*fallthrough*/
 	case 0:
 		break;
diff --git a/util/json.h b/util/json.h
index 560f845c6753ee176f7c64b4310fe1f9b1ce6d39..79ae3240e7ce151be75f6666fcaba0ba90aba7fc 100644
--- a/util/json.h
+++ b/util/json.h
@@ -21,6 +21,7 @@ enum util_json_flags {
 	UTIL_JSON_TARGETS	= (1 << 11),
 	UTIL_JSON_PARTITION	= (1 << 12),
 	UTIL_JSON_ALERT_CONFIG	= (1 << 13),
+	UTIL_JSON_EXTENTS	= (1 << 14),
 };
 
 void util_display_json_array(FILE *f_out, struct json_object *jarray,

-- 
2.47.0

Re: [ndctl PATCH v2 5/6] ndctl/cxl: Add extent output to region query

[Dave Jiang]

On 11/4/24 7:10 PM, Ira Weiny wrote:
> DCD regions have 0 or more extents.  The ability to list those and their
> properties is useful to end users.
> 
> Add extent output to region queries.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
>  Documentation/cxl/cxl-list.txt |   4 ++
>  cxl/filter.h                   |   3 +
>  cxl/json.c                     |  47 ++++++++++++++
>  cxl/json.h                     |   3 +
>  cxl/lib/libcxl.c               | 138 +++++++++++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym             |   5 ++
>  cxl/lib/private.h              |  11 ++++
>  cxl/libcxl.h                   |  11 ++++
>  cxl/list.c                     |   3 +
>  util/json.h                    |   1 +
>  10 files changed, 226 insertions(+)
> 
> diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
> index 9a9911e7dd9bba561c6202784017db1bb4b9f4bd..71fd313cfec2509c79f8ad1e0f64857d0d804c13 100644
> --- a/Documentation/cxl/cxl-list.txt
> +++ b/Documentation/cxl/cxl-list.txt
> @@ -411,6 +411,10 @@ OPTIONS
>  }
>  ----
>  
> +-N::
> +--extents::
> +	Extend Dynamic Capacity region listings extent information.
> +
>  -r::
>  --region::
>  	Specify CXL region device name(s), or device id(s), to filter the listing.
> diff --git a/cxl/filter.h b/cxl/filter.h
> index 956a46e0c7a9f05abf696cce97a365164e95e50d..a31b80c87ccac407bd4ff98b302a23b33cbe413c 100644
> --- a/cxl/filter.h
> +++ b/cxl/filter.h
> @@ -31,6 +31,7 @@ struct cxl_filter_params {
>  	bool alert_config;
>  	bool dax;
>  	bool media_errors;
> +	bool extents;
>  	int verbose;
>  	struct log_ctx ctx;
>  };
> @@ -91,6 +92,8 @@ static inline unsigned long cxl_filter_to_flags(struct cxl_filter_params *param)
>  		flags |= UTIL_JSON_DAX | UTIL_JSON_DAX_DEVS;
>  	if (param->media_errors)
>  		flags |= UTIL_JSON_MEDIA_ERRORS;
> +	if (param->extents)
> +		flags |= UTIL_JSON_EXTENTS;
>  	return flags;
>  }
>  
> diff --git a/cxl/json.c b/cxl/json.c
> index 915b2716a524fa8929ed34b01a7cb6590b61d4b7..0d6644916b8dd2cdee39ac9bf7310b5e318f0e2d 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -1170,6 +1170,50 @@ void util_cxl_mappings_append_json(struct json_object *jregion,
>  	json_object_object_add(jregion, "mappings", jmappings);
>  }
>  
> +void util_cxl_extents_append_json(struct json_object *jregion,
> +				  struct cxl_region *region,
> +				  unsigned long flags)
> +{
> +	struct json_object *jextents;
> +	struct cxl_region_extent *extent;
> +
> +	jextents = json_object_new_array();
> +	if (!jextents)
> +		return;
> +
> +	cxl_extent_foreach(region, extent) {
> +		struct json_object *jextent, *jobj;
> +		unsigned long long val;
> +		char tag_str[40];
> +		uuid_t tag;
> +
> +		jextent = json_object_new_object();
> +		if (!jextent)
> +			continue;
> +
> +		val = cxl_extent_get_offset(extent);
> +		jobj = util_json_object_hex(val, flags);
> +		if (jobj)
> +			json_object_object_add(jextent, "offset", jobj);
> +
> +		val = cxl_extent_get_length(extent);
> +		jobj = util_json_object_size(val, flags);
> +		if (jobj)
> +			json_object_object_add(jextent, "length", jobj);
> +
> +		cxl_extent_get_tag(extent, tag);
> +		uuid_unparse(tag, tag_str);
> +		jobj = json_object_new_string(tag_str);
> +		if (jobj)
> +			json_object_object_add(jextent, "tag", jobj);
> +
> +		json_object_array_add(jextents, jextent);
> +		json_object_set_userdata(jextent, extent, NULL);
> +	}
> +
> +	json_object_object_add(jregion, "extents", jextents);
> +}
> +
>  struct json_object *util_cxl_region_to_json(struct cxl_region *region,
>  					     unsigned long flags)
>  {
> @@ -1256,6 +1300,9 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
>  		}
>  	}
>  
> +	if (flags & UTIL_JSON_EXTENTS)
> +		util_cxl_extents_append_json(jregion, region, flags);
> +
>  	if (cxl_region_qos_class_mismatch(region)) {
>  		jobj = json_object_new_boolean(true);
>  		if (jobj)
> diff --git a/cxl/json.h b/cxl/json.h
> index eb7572be4106baf0469ba9243a9a767d07df8882..f9c07ab41a337838b75ffee4486f6c48ddc99863 100644
> --- a/cxl/json.h
> +++ b/cxl/json.h
> @@ -20,6 +20,9 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
>  void util_cxl_mappings_append_json(struct json_object *jregion,
>  				  struct cxl_region *region,
>  				  unsigned long flags);
> +void util_cxl_extents_append_json(struct json_object *jregion,
> +				  struct cxl_region *region,
> +				  unsigned long flags);
>  void util_cxl_targets_append_json(struct json_object *jdecoder,
>  				  struct cxl_decoder *decoder,
>  				  const char *ident, const char *serial,
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 4caa2d02313bf71960971c4eaa67fa42cea08d55..8ebb100df0c6078630bbe45fbed270709dfb4a5f 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -568,6 +568,7 @@ static void *add_cxl_region(void *parent, int id, const char *cxlregion_base)
>  	region->ctx = ctx;
>  	region->decoder = decoder;
>  	list_head_init(&region->mappings);
> +	list_head_init(&region->extents);
>  
>  	region->dev_path = strdup(cxlregion_base);
>  	if (!region->dev_path)
> @@ -1178,6 +1179,143 @@ cxl_mapping_get_next(struct cxl_memdev_mapping *mapping)
>  	return list_next(&region->mappings, mapping, list);
>  }
>  
> +static void cxl_extents_init(struct cxl_region *region)
> +{
> +	const char *devname = cxl_region_get_devname(region);
> +	struct cxl_ctx *ctx = cxl_region_get_ctx(region);
> +	char *extent_path, *dax_region_path;
> +	struct dirent *de;
> +	DIR *dir = NULL;
> +
> +	if (region->extents_init)
> +		return;
> +	region->extents_init = 1;
> +
> +	dbg(ctx, "Checking extents: %s\n", region->dev_path);
> +
> +	dax_region_path = calloc(1, strlen(region->dev_path) + 64);
> +	if (!dax_region_path) {
> +		err(ctx, "%s: allocation failure\n", devname);
> +		return;
> +	}
> +
> +	extent_path = calloc(1, strlen(region->dev_path) + 100);
> +	if (!extent_path) {
> +		err(ctx, "%s: allocation failure\n", devname);
> +		free(dax_region_path);
> +		return;
> +	}
> +
> +	sprintf(dax_region_path, "%s/dax_region%d",
> +		region->dev_path, region->id);
> +	dir = opendir(dax_region_path);
> +	if (!dir) {
> +		err(ctx, "no extents found: %s\n", dax_region_path);

Also printing the errno may be helpful

> +		free(extent_path);
> +		free(dax_region_path);
> +		return;
> +	}
> +
> +	while ((de = readdir(dir)) != NULL) {
> +		struct cxl_region_extent *extent;
> +		char buf[SYSFS_ATTR_SIZE];
> +		u64 offset, length;
> +		int id, region_id;
> +
> +		if (sscanf(de->d_name, "extent%d.%d", &region_id, &id) != 2)
> +			continue;
> +
> +		sprintf(extent_path, "%s/extent%d.%d/offset",
> +			dax_region_path, region_id, id);
> +		if (sysfs_read_attr(ctx, extent_path, buf) < 0) {
> +			err(ctx, "%s: failed to read extent%d.%d/offset\n",
> +				devname, region_id, id);
> +			continue;
> +		}
> +
> +		offset = strtoull(buf, NULL, 0);
> +		if (offset == ERANGE) {

I think it needs to be coded like:
if (offset == ULLONG_MAX) {

Not sure why specifically checking for ERANGE, but should be checking against errno for that if needed.

DJ

> +			err(ctx, "%s extent%d.%d: failed to read offset\n",
> +				devname, region_id, id);
> +			continue;
> +		}
> +
> +		sprintf(extent_path, "%s/extent%d.%d/length",
> +			dax_region_path, region_id, id);
> +		if (sysfs_read_attr(ctx, extent_path, buf) < 0) {
> +			err(ctx, "%s: failed to read extent%d.%d/length\n",
> +				devname, region_id, id);
> +			continue;
> +		}
> +
> +		length = strtoull(buf, NULL, 0);
> +		if (length == ERANGE) {
> +			err(ctx, "%s extent%d.%d: failed to read length\n",
> +				devname, region_id, id);
> +			continue;
> +		}
> +
> +		sprintf(extent_path, "%s/extent%d.%d/tag",
> +			dax_region_path, region_id, id);
> +		buf[0] = '\0';
> +		if (sysfs_read_attr(ctx, extent_path, buf) != 0)
> +			dbg(ctx, "%s extent%d.%d: failed to read tag\n",
> +				devname, region_id, id);
> +> +		extent = calloc(1, sizeof(*extent));
> +		if (!extent) {
> +			err(ctx, "%s extent%d.%d: allocation failure\n",
> +				devname, region_id, id);
> +			continue;
> +		}
> +		if (strlen(buf) && uuid_parse(buf, extent->tag) < 0)
> +			err(ctx, "%s:%s\n", extent_path, buf);
> +		extent->region = region;
> +		extent->offset = offset;
> +		extent->length = length;
> +
> +		list_node_init(&extent->list);
> +		list_add(&region->extents, &extent->list);
> +	}
> +	free(dax_region_path);
> +	free(extent_path);
> +	closedir(dir);
> +}
> +
> +CXL_EXPORT struct cxl_region_extent *
> +cxl_extent_get_first(struct cxl_region *region)
> +{
> +	cxl_extents_init(region);
> +
> +	return list_top(&region->extents, struct cxl_region_extent, list);
> +}
> +
> +CXL_EXPORT struct cxl_region_extent *
> +cxl_extent_get_next(struct cxl_region_extent *extent)
> +{
> +	struct cxl_region *region = extent->region;
> +
> +	return list_next(&region->extents, extent, list);
> +}
> +
> +CXL_EXPORT unsigned long long
> +cxl_extent_get_offset(struct cxl_region_extent *extent)
> +{
> +	return extent->offset;
> +}
> +
> +CXL_EXPORT unsigned long long
> +cxl_extent_get_length(struct cxl_region_extent *extent)
> +{
> +	return extent->length;
> +}
> +
> +CXL_EXPORT void
> +cxl_extent_get_tag(struct cxl_region_extent *extent, uuid_t tag)
> +{
> +	memcpy(tag, extent->tag, sizeof(uuid_t));
> +}
> +
>  CXL_EXPORT struct cxl_decoder *
>  cxl_mapping_get_decoder(struct cxl_memdev_mapping *mapping)
>  {
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 351da7512e05080d847fd87740488d613462dbc9..37c3531115c73cdb69b96fa47bc88bbbb901f085 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -291,4 +291,9 @@ global:
>  	cxl_memdev_trigger_poison_list;
>  	cxl_region_trigger_poison_list;
>  	cxl_region_get_region_mode;
> +	cxl_extent_get_first;
> +	cxl_extent_get_next;
> +	cxl_extent_get_offset;
> +	cxl_extent_get_length;
> +	cxl_extent_get_tag;
>  } LIBCXL_7;
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index 10abfa63dfc759b1589f9f039da1b920f8eb605e..5b50b3f778a66a2266d6d5ee69e2a72cdad54a70 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -164,6 +164,7 @@ struct cxl_region {
>  	struct cxl_decoder *decoder;
>  	struct list_node list;
>  	int mappings_init;
> +	int extents_init;
>  	struct cxl_ctx *ctx;
>  	void *dev_buf;
>  	size_t buf_len;
> @@ -179,6 +180,7 @@ struct cxl_region {
>  	struct daxctl_region *dax_region;
>  	struct kmod_module *module;
>  	struct list_head mappings;
> +	struct list_head extents;
>  };
>  
>  struct cxl_memdev_mapping {
> @@ -188,6 +190,15 @@ struct cxl_memdev_mapping {
>  	struct list_node list;
>  };
>  
> +#define CXL_REGION_EXTENT_TAG 0x10
> +struct cxl_region_extent {
> +	struct cxl_region *region;
> +	u64 offset;
> +	u64 length;
> +	uuid_t tag;
> +	struct list_node list;
> +};
> +
>  enum cxl_cmd_query_status {
>  	CXL_CMD_QUERY_NOT_RUN = 0,
>  	CXL_CMD_QUERY_OK,
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index 17ed682548b970d57f016942badc76dce61bdeaf..b7c85a67224c86d17a41376c147364e1f88db080 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -448,6 +448,17 @@ unsigned int cxl_mapping_get_position(struct cxl_memdev_mapping *mapping);
>               mapping != NULL; \
>               mapping = cxl_mapping_get_next(mapping))
>  
> +struct cxl_region_extent;
> +struct cxl_region_extent *cxl_extent_get_first(struct cxl_region *region);
> +struct cxl_region_extent *cxl_extent_get_next(struct cxl_region_extent *extent);
> +#define cxl_extent_foreach(region, extent) \
> +        for (extent = cxl_extent_get_first(region); \
> +             extent != NULL; \
> +             extent = cxl_extent_get_next(extent))
> +unsigned long long cxl_extent_get_offset(struct cxl_region_extent *extent);
> +unsigned long long cxl_extent_get_length(struct cxl_region_extent *extent);
> +void cxl_extent_get_tag(struct cxl_region_extent *extent, uuid_t tag);
> +
>  struct cxl_cmd;
>  const char *cxl_cmd_get_devname(struct cxl_cmd *cmd);
>  struct cxl_cmd *cxl_cmd_new_raw(struct cxl_memdev *memdev, int opcode);
> diff --git a/cxl/list.c b/cxl/list.c
> index 0b25d78248d5f4f529fd2c2e073e43895c722568..47d135166212b87449f960e94ee75657f7040ca9 100644
> --- a/cxl/list.c
> +++ b/cxl/list.c
> @@ -59,6 +59,8 @@ static const struct option options[] = {
>  		    "include alert configuration information"),
>  	OPT_BOOLEAN('L', "media-errors", &param.media_errors,
>  		    "include media-error information "),
> +	OPT_BOOLEAN('N', "extents", &param.extents,
> +		    "include extent information (Dynamic Capacity regions only)"),
>  	OPT_INCR('v', "verbose", &param.verbose, "increase output detail"),
>  #ifdef ENABLE_DEBUG
>  	OPT_BOOLEAN(0, "debug", &debug, "debug list walk"),
> @@ -135,6 +137,7 @@ int cmd_list(int argc, const char **argv, struct cxl_ctx *ctx)
>  		param.decoders = true;
>  		param.targets = true;
>  		param.regions = true;
> +		param.extents = true;
>  		/*fallthrough*/
>  	case 0:
>  		break;
> diff --git a/util/json.h b/util/json.h
> index 560f845c6753ee176f7c64b4310fe1f9b1ce6d39..79ae3240e7ce151be75f6666fcaba0ba90aba7fc 100644
> --- a/util/json.h
> +++ b/util/json.h
> @@ -21,6 +21,7 @@ enum util_json_flags {
>  	UTIL_JSON_TARGETS	= (1 << 11),
>  	UTIL_JSON_PARTITION	= (1 << 12),
>  	UTIL_JSON_ALERT_CONFIG	= (1 << 13),
> +	UTIL_JSON_EXTENTS	= (1 << 14),
>  };
>  
>  void util_display_json_array(FILE *f_out, struct json_object *jarray,
> 

Re: [ndctl PATCH v2 5/6] ndctl/cxl: Add extent output to region query

[Ira Weiny]

Dave Jiang wrote:

[snip]

> > +static void cxl_extents_init(struct cxl_region *region)
> > +{
> > +	const char *devname = cxl_region_get_devname(region);
> > +	struct cxl_ctx *ctx = cxl_region_get_ctx(region);
> > +	char *extent_path, *dax_region_path;
> > +	struct dirent *de;
> > +	DIR *dir = NULL;
> > +
> > +	if (region->extents_init)
> > +		return;
> > +	region->extents_init = 1;
> > +
> > +	dbg(ctx, "Checking extents: %s\n", region->dev_path);
> > +
> > +	dax_region_path = calloc(1, strlen(region->dev_path) + 64);
> > +	if (!dax_region_path) {
> > +		err(ctx, "%s: allocation failure\n", devname);
> > +		return;
> > +	}
> > +
> > +	extent_path = calloc(1, strlen(region->dev_path) + 100);
> > +	if (!extent_path) {
> > +		err(ctx, "%s: allocation failure\n", devname);
> > +		free(dax_region_path);
> > +		return;
> > +	}
> > +
> > +	sprintf(dax_region_path, "%s/dax_region%d",
> > +		region->dev_path, region->id);
> > +	dir = opendir(dax_region_path);
> > +	if (!dir) {
> > +		err(ctx, "no extents found: %s\n", dax_region_path);
> 
> Also printing the errno may be helpful

Yea. strerror()

> 
> > +		free(extent_path);
> > +		free(dax_region_path);
> > +		return;
> > +	}
> > +
> > +	while ((de = readdir(dir)) != NULL) {
> > +		struct cxl_region_extent *extent;
> > +		char buf[SYSFS_ATTR_SIZE];
> > +		u64 offset, length;
> > +		int id, region_id;
> > +
> > +		if (sscanf(de->d_name, "extent%d.%d", &region_id, &id) != 2)
> > +			continue;
> > +
> > +		sprintf(extent_path, "%s/extent%d.%d/offset",
> > +			dax_region_path, region_id, id);
> > +		if (sysfs_read_attr(ctx, extent_path, buf) < 0) {
> > +			err(ctx, "%s: failed to read extent%d.%d/offset\n",
> > +				devname, region_id, id);
> > +			continue;
> > +		}
> > +
> > +		offset = strtoull(buf, NULL, 0);
> > +		if (offset == ERANGE) {
> 
> I think it needs to be coded like:
> if (offset == ULLONG_MAX) {

Yep...  I fell in the habit of the error being returned...

> 
> Not sure why specifically checking for ERANGE, but should be checking against errno for that if needed.

Because I was thinking about strtoull() having odd error handling and was
stupid and did not look at the man page.  If you look closely at the man page
full error coverage needs to be.

	errno = 0;
	offset = strtoull(...);
	if ((offset == ULLONG_MAX) || (offset == 0 && errno == EINVAL)) {
		...
	}

But sysfs is not going to return an invalid value...  So ULLONG_MAX is all we
need to check for.

Thanks!
Ira

> 
> DJ

Re: [ndctl PATCH v2 5/6] ndctl/cxl: Add extent output to region query

[Alison Schofield]

On Mon, Nov 04, 2024 at 08:10:49PM -0600, Ira Weiny wrote:
> DCD regions have 0 or more extents.  The ability to list those and their
> properties is useful to end users.
> 

Should we describe those new useful properties in the man page...see
below...

> Add extent output to region queries.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
>  Documentation/cxl/cxl-list.txt |   4 ++
>  cxl/filter.h                   |   3 +
>  cxl/json.c                     |  47 ++++++++++++++
>  cxl/json.h                     |   3 +
>  cxl/lib/libcxl.c               | 138 +++++++++++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym             |   5 ++
>  cxl/lib/private.h              |  11 ++++
>  cxl/libcxl.h                   |  11 ++++
>  cxl/list.c                     |   3 +
>  util/json.h                    |   1 +
>  10 files changed, 226 insertions(+)
> 
> diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
> index 9a9911e7dd9bba561c6202784017db1bb4b9f4bd..71fd313cfec2509c79f8ad1e0f64857d0d804c13 100644
> --- a/Documentation/cxl/cxl-list.txt
> +++ b/Documentation/cxl/cxl-list.txt
> @@ -411,6 +411,10 @@ OPTIONS
>  }
>  ----
>  
> +-N::
> +--extents::
> +	Extend Dynamic Capacity region listings extent information.
> +

a sample perhaps?  or some verbage on  what to expect.


snip

>  
> +static void cxl_extents_init(struct cxl_region *region)
> +{
> +	const char *devname = cxl_region_get_devname(region);
> +	struct cxl_ctx *ctx = cxl_region_get_ctx(region);
> +	char *extent_path, *dax_region_path;
> +	struct dirent *de;
> +	DIR *dir = NULL;
> +
> +	if (region->extents_init)
> +		return;
> +	region->extents_init = 1;
> +
> +	dbg(ctx, "Checking extents: %s\n", region->dev_path);

Rather than emit the above which makes me assume success if
no err message follows, how about emitting the success debug
msg when all is done below.

> +
> +	dax_region_path = calloc(1, strlen(region->dev_path) + 64);
> +	if (!dax_region_path) {
> +		err(ctx, "%s: allocation failure\n", devname);
> +		return;
> +	}
> +
> +	extent_path = calloc(1, strlen(region->dev_path) + 100);
> +	if (!extent_path) {
> +		err(ctx, "%s: allocation failure\n", devname);
> +		free(dax_region_path);
> +		return;
> +	}
> +
> +	sprintf(dax_region_path, "%s/dax_region%d",
> +		region->dev_path, region->id);
> +	dir = opendir(dax_region_path);
> +	if (!dir) {
> +		err(ctx, "no extents found: %s\n", dax_region_path);
> +		free(extent_path);
> +		free(dax_region_path);
> +		return;
> +	}
> +
> +	while ((de = readdir(dir)) != NULL) {
> +		struct cxl_region_extent *extent;
> +		char buf[SYSFS_ATTR_SIZE];
> +		u64 offset, length;
> +		int id, region_id;
> +
> +		if (sscanf(de->d_name, "extent%d.%d", &region_id, &id) != 2)
> +			continue;
> +
> +		sprintf(extent_path, "%s/extent%d.%d/offset",
> +			dax_region_path, region_id, id);
> +		if (sysfs_read_attr(ctx, extent_path, buf) < 0) {
> +			err(ctx, "%s: failed to read extent%d.%d/offset\n",
> +				devname, region_id, id);
> +			continue;
> +		}
> +
> +		offset = strtoull(buf, NULL, 0);
> +		if (offset == ERANGE) {
> +			err(ctx, "%s extent%d.%d: failed to read offset\n",
> +				devname, region_id, id);
> +			continue;
> +		}
> +
> +		sprintf(extent_path, "%s/extent%d.%d/length",
> +			dax_region_path, region_id, id);
> +		if (sysfs_read_attr(ctx, extent_path, buf) < 0) {
> +			err(ctx, "%s: failed to read extent%d.%d/length\n",
> +				devname, region_id, id);
> +			continue;
> +		}
> +
> +		length = strtoull(buf, NULL, 0);
> +		if (length == ERANGE) {
> +			err(ctx, "%s extent%d.%d: failed to read length\n",
> +				devname, region_id, id);
> +			continue;
> +		}
> +
> +		sprintf(extent_path, "%s/extent%d.%d/tag",
> +			dax_region_path, region_id, id);
> +		buf[0] = '\0';
> +		if (sysfs_read_attr(ctx, extent_path, buf) != 0)
> +			dbg(ctx, "%s extent%d.%d: failed to read tag\n",
> +				devname, region_id, id);
> +
> +		extent = calloc(1, sizeof(*extent));
> +		if (!extent) {
> +			err(ctx, "%s extent%d.%d: allocation failure\n",
> +				devname, region_id, id);
> +			continue;
> +		}
> +		if (strlen(buf) && uuid_parse(buf, extent->tag) < 0)
> +			err(ctx, "%s:%s\n", extent_path, buf);
> +		extent->region = region;
> +		extent->offset = offset;
> +		extent->length = length;
> +
> +		list_node_init(&extent->list);
> +		list_add(&region->extents, &extent->list);
> +	}

	Here - dbg the success message

> +	free(dax_region_path);
> +	free(extent_path);
> +	closedir(dir);
> +}
> 

Re: [ndctl PATCH v2 5/6] ndctl/cxl: Add extent output to region query

[Ira Weiny]

Alison Schofield wrote:
> On Mon, Nov 04, 2024 at 08:10:49PM -0600, Ira Weiny wrote:
> > DCD regions have 0 or more extents.  The ability to list those and their
> > properties is useful to end users.
> > 
> 
> Should we describe those new useful properties in the man page...see
> below...
> 
> > Add extent output to region queries.
> > 
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > ---
> >  Documentation/cxl/cxl-list.txt |   4 ++
> >  cxl/filter.h                   |   3 +
> >  cxl/json.c                     |  47 ++++++++++++++
> >  cxl/json.h                     |   3 +
> >  cxl/lib/libcxl.c               | 138 +++++++++++++++++++++++++++++++++++++++++
> >  cxl/lib/libcxl.sym             |   5 ++
> >  cxl/lib/private.h              |  11 ++++
> >  cxl/libcxl.h                   |  11 ++++
> >  cxl/list.c                     |   3 +
> >  util/json.h                    |   1 +
> >  10 files changed, 226 insertions(+)
> > 
> > diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
> > index 9a9911e7dd9bba561c6202784017db1bb4b9f4bd..71fd313cfec2509c79f8ad1e0f64857d0d804c13 100644
> > --- a/Documentation/cxl/cxl-list.txt
> > +++ b/Documentation/cxl/cxl-list.txt
> > @@ -411,6 +411,10 @@ OPTIONS
> >  }
> >  ----
> >  
> > +-N::
> > +--extents::
> > +	Extend Dynamic Capacity region listings extent information.
> > +
> 
> a sample perhaps?  or some verbage on  what to expect.

Good idea I've added an example.

> 
> 
> snip
> 
> >  
> > +static void cxl_extents_init(struct cxl_region *region)
> > +{
> > +	const char *devname = cxl_region_get_devname(region);
> > +	struct cxl_ctx *ctx = cxl_region_get_ctx(region);
> > +	char *extent_path, *dax_region_path;
> > +	struct dirent *de;
> > +	DIR *dir = NULL;
> > +
> > +	if (region->extents_init)
> > +		return;
> > +	region->extents_init = 1;
> > +
> > +	dbg(ctx, "Checking extents: %s\n", region->dev_path);
> 
> Rather than emit the above which makes me assume success if
> no err message follows, how about emitting the success debug
> msg when all is done below.

Fair enough.


[snip]

> > +
> > +	while ((de = readdir(dir)) != NULL) {
> > +		struct cxl_region_extent *extent;

[snip]

> > +
> > +		list_node_init(&extent->list);
> > +		list_add(&region->extents, &extent->list);
> > +	}
> 
> 	Here - dbg the success message

Actually after moving the simple message here I feel it is not really
helping much.  I think it best to report via debug each extent as it is
added.  So I've changed the above to:

	while (...) {
	...
		dbg(ctx, "%s added extent%d.%d\n", devname, region_id, id);
	}

This gives more information on what might be going wrong if debug is
required.

Ira

[snip]

[ndctl PATCH v2 6/6] ndctl/cxl/test: Add Dynamic Capacity tests

[Ira Weiny]

cxl_test provides a good way to ensure quick smoke and regression
testing.  The complexity of DCD and the new sparse DAX regions required
to use them benefits greatly with a series of smoke tests.

The only part of the kernel stack which must be bypassed is the actual
irq of DCD events.  However, the event processing itself can be tested
via cxl_test calling directly the event processing function directly.

In this way the rest of the stack; management of sparse regions, the
extent device lifetimes, and the dax device operations can be tested.

Add Dynamic Capacity Device tests for kernels which have DCD support in
cxl_test.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 test/cxl-dcd.sh  | 656 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/meson.build |   2 +
 2 files changed, 658 insertions(+)

diff --git a/test/cxl-dcd.sh b/test/cxl-dcd.sh
new file mode 100644
index 0000000000000000000000000000000000000000..f5248fce4f3899acdf646ace4f0eb531ea2286d3
--- /dev/null
+++ b/test/cxl-dcd.sh
@@ -0,0 +1,656 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2024 Intel Corporation. All rights reserved.
+
+. "$(dirname "$0")/common"
+
+rc=77
+set -ex
+
+trap 'err $LINENO' ERR
+
+check_prereq "jq"
+
+modprobe -r cxl_test
+modprobe cxl_test
+rc=1
+
+dev_path="/sys/bus/platform/devices"
+cxl_path="/sys/bus/cxl/devices"
+
+# a test extent tag
+test_tag=dc-test-tag
+
+#
+# The test devices have 2G of non DC capacity.  A single DC reagion of 1G is
+# added beyond that.
+#
+# The testing centers around 3 extents.  Two are pre-existing on test load
+# called pre-existing.  The other is created within this script alone called
+# base.
+
+#
+# | 2G non- |      DC region (1G)                                   |
+# |  DC cap |                                                       |
+# |  ...    |-------------------------------------------------------|
+# |         |--------|       |----------|      |----------|         |
+# |         | (base) |       | (pre)-   |      | (pre2)-  |         |
+# |         |        |       | existing |      | existing |         |
+
+dcsize=""
+
+base_dpa=0x80000000
+
+# base extent at dpa 2G - 64M long
+base_ext_offset=0x0
+base_ext_dpa=$(($base_dpa + $base_ext_offset))
+base_ext_length=0x4000000
+
+# pre existing extent base + 128M, 64M length
+# 0x00000088000000-0x0000008bffffff
+pre_ext_offset=0x8000000
+pre_ext_dpa=$(($base_dpa + $pre_ext_offset))
+pre_ext_length=0x4000000
+
+# pre2 existing extent base + 256M, 64M length
+# 0x00000090000000-0x00000093ffffff
+pre2_ext_offset=0x10000000
+pre2_ext_dpa=$(($base_dpa + $pre2_ext_offset))
+pre2_ext_length=0x4000000
+
+mem=""
+bus=""
+device=""
+decoder=""
+
+create_dcd_region()
+{
+	mem="$1"
+	decoder="$2"
+	reg_size_string=""
+	if [ "$3" != "" ]; then
+		reg_size_string="-s $3"
+	fi
+	dcd_partition="dc0"
+	if [ "$4" != "" ]; then
+		dcd_partition="$4"
+	fi
+
+	# create region
+	rc=$($CXL create-region -t ${dcd_partition} -d "$decoder" -m "$mem" ${reg_size_string} | jq -r ".region")
+
+	if [[ ! $rc ]]; then
+		echo "create-region failed for $decoder / $mem"
+		err "$LINENO"
+	fi
+
+	echo ${rc}
+}
+
+check_region()
+{
+	search=$1
+	region_size=$2
+
+	result=$($CXL list -r "$search" | jq -r ".[].region")
+	if [ "$result" != "$search" ]; then
+		echo "check region failed to find $search"
+		err "$LINENO"
+	fi
+
+	result=$($CXL list -r "$search" | jq -r ".[].size")
+	if [ "$result" != "$region_size" ]; then
+		echo "check region failed invalid size $result != $region_size"
+		err "$LINENO"
+	fi
+}
+
+check_not_region()
+{
+	search=$1
+
+	result=$($CXL list -r "$search" | jq -r ".[].region")
+	if [ "$result" == "$search" ]; then
+		echo "check not region failed; $search found"
+		err "$LINENO"
+	fi
+}
+
+destroy_region()
+{
+	local region=$1
+	$CXL disable-region $region
+	$CXL destroy-region $region
+}
+
+inject_extent()
+{
+	device="$1"
+	dpa="$2"
+	length="$3"
+	tag="$4"
+
+	more="0"
+	if [ "$5" != "" ]; then
+		more="1"
+	fi
+
+	echo ${dpa}:${length}:${tag}:${more} > "${dev_path}/${device}/dc_inject_extent"
+}
+
+remove_extent()
+{
+	device="$1"
+	dpa="$2"
+	length="$3"
+
+	echo ${dpa}:${length} > "${dev_path}/${device}/dc_del_extent"
+}
+
+create_dax_dev()
+{
+	reg="$1"
+
+	dax_dev=$($DAXCTL create-device -r $reg | jq -er '.[].chardev')
+
+	echo ${dax_dev}
+}
+
+fail_create_dax_dev()
+{
+	reg="$1"
+
+	set +e
+	result=$($DAXCTL create-device -r $reg)
+	set -e
+	if [ "$result" == "0" ]; then
+		echo "FAIL device created"
+		err "$LINENO"
+	fi
+}
+
+shrink_dax_dev()
+{
+	dev="$1"
+	new_size="$2"
+
+	$DAXCTL disable-device $dev
+	$DAXCTL reconfigure-device $dev -s $new_size
+	$DAXCTL enable-device $dev
+}
+
+destroy_dax_dev()
+{
+	dev="$1"
+
+	$DAXCTL disable-device $dev
+	$DAXCTL destroy-device $dev
+}
+
+check_dax_dev()
+{
+	search="$1"
+	size="$2"
+
+	result=$($DAXCTL list -d $search | jq -er '.[].chardev')
+	if [ "$result" != "$search" ]; then
+		echo "check dax device failed to find $search"
+		err "$LINENO"
+	fi
+	result=$($DAXCTL list -d $search | jq -er '.[].size')
+	if [ "$result" -ne "$size" ]; then
+		echo "check dax device failed incorrect size $result; exp $size"
+		err "$LINENO"
+	fi
+}
+
+# check that the dax device is not there.
+check_not_dax_dev()
+{
+	reg="$1"
+	search="$2"
+	result=$($DAXCTL list -r $reg -D | jq -er '.[].chardev')
+	if [ "$result" == "$search" ]; then
+		echo "FAIL found $search"
+		err "$LINENO"
+	fi
+}
+
+check_extent()
+{
+	region=$1
+	offset=$(($2))
+	length=$(($3))
+
+	result=$($CXL list -r "$region" -N | jq -r ".[].extents[] | select(.offset == ${offset}) | .length")
+	if [[ $result != $length ]]; then
+		echo "FAIL region $1 could not find extent @ $offset ($length)"
+		err "$LINENO"
+	fi
+}
+
+check_extent_cnt()
+{
+	region=$1
+	count=$(($2))
+
+	result=$($CXL list -r $region -N | jq -r '.[].extents[].offset' | wc -l)
+	if [[ $result != $count ]]; then
+		echo "FAIL region $1: found wrong number of extents $result; expect $count"
+		err "$LINENO"
+	fi
+}
+
+readarray -t memdevs < <("$CXL" list -b cxl_test -Mi | jq -r '.[].memdev')
+
+for mem in ${memdevs[@]}; do
+	dcsize=$($CXL list -m $mem | jq -r '.[].dc0_size')
+	if [ "$dcsize" == "null" ]; then
+		continue
+	fi
+	decoder=$($CXL list -b cxl_test -D -d root -m "$mem" |
+		  jq -r ".[] |
+		  select(.dc0_capable == true) |
+		  select(.nr_targets == 1) |
+		  select(.max_available_extent >= ${dcsize}) |
+		  .decoder")
+	if [[ $decoder ]]; then
+		bus=`"$CXL" list -b cxl_test -m ${mem} | jq -r '.[].bus'`
+		device=$($CXL list -m $mem | jq -r '.[].host')
+		break
+	fi
+done
+
+echo "TEST: DCD test device bus:${bus} decoder:${decoder} mem:${mem} device:${device} size:${dcsize}"
+
+if [ "$decoder" == "" ] || [ "$device" == "" ] || [ "$dcsize" == "" ]; then
+	echo "No mem device/decoder found with DCD support"
+	exit 77
+fi
+
+echo ""
+echo "Test: pre-existing extent"
+echo ""
+region=$(create_dcd_region ${mem} ${decoder})
+check_region ${region} ${dcsize}
+# should contain pre-created extents
+check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
+check_extent ${region} ${pre2_ext_offset} ${pre2_ext_length}
+
+
+# | 2G non- |      DC region                                        |
+# |  DC cap |                                                       |
+# |  ...    |-------------------------------------------------------|
+# |         |                   |----------|         |----------|   |
+# |         |                   | (pre)-   |         | (pre2)-  |   |
+# |         |                   | existing |         | existing |   |
+
+# Remove the pre-created test extent out from under dax device
+# stack should hold ref until dax device deleted
+echo ""
+echo "Test: Remove extent from under DAX dev"
+echo ""
+dax_dev=$(create_dax_dev ${region})
+check_extent_cnt ${region} 2
+remove_extent ${device} $pre_ext_dpa $pre_ext_length
+length="$(($pre_ext_length + $pre2_ext_length))"
+check_dax_dev ${dax_dev} $length
+check_extent_cnt ${region} 2
+destroy_dax_dev ${dax_dev}
+check_not_dax_dev ${region} ${dax_dev}
+
+# In-use extents are not released.  Remove after use.
+check_extent_cnt ${region} 2
+remove_extent ${device} $pre_ext_dpa $pre_ext_length
+remove_extent ${device} $pre2_ext_dpa $pre2_ext_length
+check_extent_cnt ${region} 0
+
+echo ""
+echo "Test: Create dax device spanning 2 extents"
+echo ""
+inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
+check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
+inject_extent ${device} $base_ext_dpa $base_ext_length ""
+check_extent ${region} ${base_ext_offset} ${base_ext_length}
+
+# | 2G non- |      DC region                                        |
+# |  DC cap |                                                       |
+# |  ...    |-------------------------------------------------------|
+# |         |--------|          |----------|                        |
+# |         | (base) |          | (pre)-   |                        |
+# |         |        |          | existing |                        |
+
+check_extent_cnt ${region} 2
+dax_dev=$(create_dax_dev ${region})
+
+echo ""
+echo "Test: dev dax is spanning sparse extents"
+echo ""
+ext_sum_length="$(($base_ext_length + $pre_ext_length))"
+check_dax_dev ${dax_dev} $ext_sum_length
+
+
+echo ""
+echo "Test: Remove extents under sparse dax device"
+echo ""
+remove_extent ${device} $base_ext_dpa $base_ext_length
+check_extent_cnt ${region} 2
+remove_extent ${device} $pre_ext_dpa $pre_ext_length
+check_extent_cnt ${region} 2
+destroy_dax_dev ${dax_dev}
+check_not_dax_dev ${region} ${dax_dev}
+
+# In-use extents are not released.  Remove after use.
+check_extent_cnt ${region} 2
+remove_extent ${device} $base_ext_dpa $base_ext_length
+remove_extent ${device} $pre_ext_dpa $pre_ext_length
+check_extent_cnt ${region} 0
+
+echo ""
+echo "Test: inject without/with tag"
+echo ""
+inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
+check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
+inject_extent ${device} $base_ext_dpa $base_ext_length ""
+check_extent ${region} ${base_ext_offset} ${base_ext_length}
+remove_extent ${device} $base_ext_dpa $base_ext_length
+remove_extent ${device} $pre_ext_dpa $pre_ext_length
+check_extent_cnt ${region} 0
+
+
+echo ""
+echo "Test: partial extent remove"
+echo ""
+inject_extent ${device} $base_ext_dpa $base_ext_length ""
+dax_dev=$(create_dax_dev ${region})
+
+# | 2G non- |      DC region                                        |
+# |  DC cap |                                                       |
+# |  ...    |-------------------------------------------------------|
+# |         |--------|                                              |
+# |         | (base) |                                              |
+# |         |    |---|                                              |
+#                  Partial
+
+partial_ext_dpa="$(($base_ext_dpa + ($base_ext_length / 2)))"
+partial_ext_length="$(($base_ext_length / 2))"
+echo "Removing Partial : $partial_ext_dpa $partial_ext_length"
+remove_extent ${device} $partial_ext_dpa $partial_ext_length
+check_extent_cnt ${region} 1
+destroy_dax_dev ${dax_dev}
+check_not_dax_dev ${region} ${dax_dev}
+
+# In-use extents are not released.  Remove after use.
+check_extent_cnt ${region} 1
+remove_extent ${device} $partial_ext_dpa $partial_ext_length
+check_extent_cnt ${region} 0
+
+# Test multiple extent remove
+echo ""
+echo "Test: multiple extent remove with single extent remove command"
+echo ""
+inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
+inject_extent ${device} $base_ext_dpa $base_ext_length ""
+check_extent_cnt ${region} 2
+dax_dev=$(create_dax_dev ${region})
+
+# | 2G non- |      DC region                                        |
+# |  DC cap |                                                       |
+# |  ...    |-------------------------------------------------------|
+# |         |--------|          |-------------------|               |
+# |         | (base) |          | (pre)-existing    |               |
+#                |------------------|
+#                  Partial
+
+partial_ext_dpa="$(($base_ext_dpa + ($base_ext_length / 2)))"
+partial_ext_length="$(($pre_ext_dpa - $base_ext_dpa))"
+echo "Removing multiple in span : $partial_ext_dpa $partial_ext_length"
+remove_extent ${device} $partial_ext_dpa $partial_ext_length
+check_extent_cnt ${region} 2
+destroy_dax_dev ${dax_dev}
+check_not_dax_dev ${region} ${dax_dev}
+
+
+echo ""
+echo "Test: Destroy region without extent removal"
+echo ""
+
+# In-use extents are not released.
+check_extent_cnt ${region} 2
+destroy_region ${region}
+check_not_region ${region}
+
+
+echo ""
+echo "Test: Destroy region with extents and dax devices"
+echo ""
+region=$(create_dcd_region ${mem} ${decoder})
+check_region ${region} ${dcsize}
+check_extent_cnt ${region} 0
+inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
+
+# | 2G non- |      DC region                                        |
+# |  DC cap |                                                       |
+# |  ...    |-------------------------------------------------------|
+# |         |                   |----------|                        |
+# |         |                   | (pre)-   |                        |
+# |         |                   | existing |                        |
+
+check_extent_cnt ${region} 1
+dax_dev=$(create_dax_dev ${region})
+destroy_region ${region}
+check_not_region ${region}
+
+echo ""
+echo "Test: Fail sparse dax dev creation without space"
+echo ""
+region=$(create_dcd_region ${mem} ${decoder})
+check_region ${region} ${dcsize}
+inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
+
+# | 2G non- |      DC region                                        |
+# |  DC cap |                                                       |
+# |  ...    |-------------------------------------------------------|
+# |         |                   |-------------------|               |
+# |         |                   | (pre)-existing    |               |
+
+check_extent_cnt ${region} 1
+
+# |         |                   | dax0.1            |               |
+
+dax_dev=$(create_dax_dev ${region})
+check_dax_dev ${dax_dev} $pre_ext_length
+fail_create_dax_dev ${region}
+
+echo ""
+echo "Test: Resize sparse dax device"
+echo ""
+
+# Shrink
+# |         |                   | dax0.1  |                         |
+resize_ext_length=$(($pre_ext_length / 2))
+shrink_dax_dev ${dax_dev} $resize_ext_length
+check_dax_dev ${dax_dev} $resize_ext_length
+
+# Fill
+# |         |                   | dax0.1  | dax0.2  |               |
+dax_dev=$(create_dax_dev ${region})
+check_dax_dev ${dax_dev} $resize_ext_length
+destroy_region ${region}
+check_not_region ${region}
+
+
+# 2 extent
+# create dax dev
+# resize into 1st extent
+# create dev on rest of 1st and all of second
+# Ensure both devices are correct
+
+echo ""
+echo "Test: Resize sparse dax device across extents"
+echo ""
+region=$(create_dcd_region ${mem} ${decoder})
+check_region ${region} ${dcsize}
+inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
+inject_extent ${device} $base_ext_dpa $base_ext_length ""
+
+# | 2G non- |      DC region                                        |
+# |  DC cap |                                                       |
+# |  ...    |-------------------------------------------------------|
+# |         |--------|          |-------------------|               |
+# |         | (base) |          | (pre)-existing    |               |
+
+check_extent_cnt ${region} 2
+dax_dev=$(create_dax_dev ${region})
+ext_sum_length="$(($base_ext_length + $pre_ext_length))"
+
+# |         | dax0.1 |          |  dax0.1           |               |
+
+check_dax_dev ${dax_dev} $ext_sum_length
+resize_ext_length=33554432 # 32MB
+
+# |         | D1 |                                                  |
+
+shrink_dax_dev ${dax_dev} $resize_ext_length
+check_dax_dev ${dax_dev} $resize_ext_length
+
+# |         | D1 | D2|          | dax0.2            |               |
+
+dax_dev=$(create_dax_dev ${region})
+remainder_length=$((ext_sum_length - $resize_ext_length))
+check_dax_dev ${dax_dev} $remainder_length
+
+# |         | D1 | D2|          | dax0.2 |                          |
+
+remainder_length=$((remainder_length / 2))
+shrink_dax_dev ${dax_dev} $remainder_length
+check_dax_dev ${dax_dev} $remainder_length
+
+# |         | D1 | D2|          | dax0.2 |  dax0.3  |               |
+
+dax_dev=$(create_dax_dev ${region})
+check_dax_dev ${dax_dev} $remainder_length
+destroy_region ${region}
+check_not_region ${region}
+
+
+echo ""
+echo "Test: Rejecting overlapping extents"
+echo ""
+
+region=$(create_dcd_region ${mem} ${decoder})
+check_region ${region} ${dcsize}
+inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
+
+# | 2G non- |      DC region                                        |
+# |  DC cap |                                                       |
+# |  ...    |-------------------------------------------------------|
+# |         |                   |-------------------|               |
+# |         |                   | (pre)-existing    |               |
+
+check_extent_cnt ${region} 1
+
+# Attempt overlapping extent
+#
+# |         |          |-----------------|                          |
+# |         |          | overlapping     |                          |
+
+partial_ext_dpa="$(($base_ext_dpa + ($pre_ext_dpa / 2)))"
+partial_ext_length=$pre_ext_length
+inject_extent ${device} $partial_ext_dpa $partial_ext_length ""
+
+# Should only see the original ext
+check_extent_cnt ${region} 1
+destroy_region ${region}
+check_not_region ${region}
+
+
+echo ""
+echo "Test: create 2 regions in the same DC partition"
+echo ""
+region_size=$(($dcsize / 2))
+region=$(create_dcd_region ${mem} ${decoder} ${region_size} dc1)
+check_region ${region} ${region_size}
+
+region_two=$(create_dcd_region ${mem} ${decoder} ${region_size} dc1)
+check_region ${region_two} ${region_size}
+
+destroy_region ${region_two}
+check_not_region ${region_two}
+destroy_region ${region}
+check_not_region ${region}
+
+
+echo ""
+echo "Test: More bit"
+echo ""
+region=$(create_dcd_region ${mem} ${decoder})
+check_region ${region} ${dcsize}
+inject_extent ${device} $pre_ext_dpa $pre_ext_length "" 1
+# More bit should hold off surfacing extent until the more bit is 0
+check_extent_cnt ${region} 0
+inject_extent ${device} $base_ext_dpa $base_ext_length ""
+check_extent_cnt ${region} 2
+destroy_region ${region}
+check_not_region ${region}
+
+
+# Create a new region for driver tests
+region=$(create_dcd_region ${mem} ${decoder})
+
+echo ""
+echo "Test: driver remove tear down"
+echo ""
+check_region ${region} ${dcsize}
+inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
+check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
+dax_dev=$(create_dax_dev ${region})
+# remove driver releases extents
+modprobe -r dax_cxl
+check_extent_cnt ${region} 0
+
+# leave region up, driver removed.
+echo ""
+echo "Test: no driver inject ok"
+echo ""
+check_region ${region} ${dcsize}
+inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
+check_extent_cnt ${region} 1
+modprobe dax_cxl
+check_extent_cnt ${region} 1
+
+destroy_region ${region}
+check_not_region ${region}
+
+
+# Test event reporting
+# results expected
+num_dcd_events_expected=2
+
+echo "Test: Prep event trace"
+echo "" > /sys/kernel/tracing/trace
+echo 1 > /sys/kernel/tracing/events/cxl/enable
+echo 1 > /sys/kernel/tracing/tracing_on
+
+inject_extent ${device} $base_ext_dpa $base_ext_length ""
+remove_extent ${device} $base_ext_dpa $base_ext_length
+
+echo 0 > /sys/kernel/tracing/tracing_on
+
+echo "Test: Events seen"
+trace_out=$(cat /sys/kernel/tracing/trace)
+
+# Look for DCD events
+num_dcd_events=$(grep -c "cxl_dynamic_capacity" <<< "${trace_out}")
+echo "     LOG     (Expected) : (Found)"
+echo "     DCD events    ($num_dcd_events_expected) : $num_dcd_events"
+
+if [ "$num_dcd_events" -ne $num_dcd_events_expected ]; then
+	err "$LINENO"
+fi
+
+modprobe -r cxl_test
+
+check_dmesg "$LINENO"
+
+exit 0
diff --git a/test/meson.build b/test/meson.build
index d871e28e17ce512cd1e7b43f3ec081729fe5e03a..1cfcb60d16e05272893ae1c67820aa8614281505 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -161,6 +161,7 @@ cxl_sanitize = find_program('cxl-sanitize.sh')
 cxl_destroy_region = find_program('cxl-destroy-region.sh')
 cxl_qos_class = find_program('cxl-qos-class.sh')
 cxl_poison = find_program('cxl-poison.sh')
+cxl_dcd = find_program('cxl-dcd.sh')
 
 tests = [
   [ 'libndctl',               libndctl,		  'ndctl' ],
@@ -194,6 +195,7 @@ tests = [
   [ 'cxl-destroy-region.sh',  cxl_destroy_region, 'cxl'   ],
   [ 'cxl-qos-class.sh',       cxl_qos_class,      'cxl'   ],
   [ 'cxl-poison.sh',          cxl_poison,         'cxl'   ],
+  [ 'cxl-dcd.sh',             cxl_dcd,            'cxl'   ],
 ]
 
 if get_option('destructive').enabled()

-- 
2.47.0

Re: [ndctl PATCH v2 6/6] ndctl/cxl/test: Add Dynamic Capacity tests

[Dave Jiang]

On 11/4/24 7:10 PM, Ira Weiny wrote:
> cxl_test provides a good way to ensure quick smoke and regression
> testing.  The complexity of DCD and the new sparse DAX regions required
> to use them benefits greatly with a series of smoke tests.
> 
> The only part of the kernel stack which must be bypassed is the actual
> irq of DCD events.  However, the event processing itself can be tested
> via cxl_test calling directly the event processing function directly.
> 
> In this way the rest of the stack; management of sparse regions, the
> extent device lifetimes, and the dax device operations can be tested.
> 
> Add Dynamic Capacity Device tests for kernels which have DCD support in
> cxl_test.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Would things look cleaner if the multiple test cases are organized into individual bash functions?

> ---
>  test/cxl-dcd.sh  | 656 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  test/meson.build |   2 +
>  2 files changed, 658 insertions(+)
> 
> diff --git a/test/cxl-dcd.sh b/test/cxl-dcd.sh
> new file mode 100644
> index 0000000000000000000000000000000000000000..f5248fce4f3899acdf646ace4f0eb531ea2286d3
> --- /dev/null
> +++ b/test/cxl-dcd.sh
> @@ -0,0 +1,656 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2024 Intel Corporation. All rights reserved.
> +
> +. "$(dirname "$0")/common"
> +
> +rc=77
> +set -ex
> +
> +trap 'err $LINENO' ERR
> +
> +check_prereq "jq"
> +
> +modprobe -r cxl_test
> +modprobe cxl_test
> +rc=1
> +
> +dev_path="/sys/bus/platform/devices"
> +cxl_path="/sys/bus/cxl/devices"
> +
> +# a test extent tag
> +test_tag=dc-test-tag
> +
> +#
> +# The test devices have 2G of non DC capacity.  A single DC reagion of 1G is
> +# added beyond that.
> +#
> +# The testing centers around 3 extents.  Two are pre-existing on test load
> +# called pre-existing.  The other is created within this script alone called
> +# base.
> +
> +#
> +# | 2G non- |      DC region (1G)                                   |
> +# |  DC cap |                                                       |
> +# |  ...    |-------------------------------------------------------|
> +# |         |--------|       |----------|      |----------|         |
> +# |         | (base) |       | (pre)-   |      | (pre2)-  |         |
> +# |         |        |       | existing |      | existing |         |
> +
> +dcsize=""
> +
> +base_dpa=0x80000000
> +
> +# base extent at dpa 2G - 64M long
> +base_ext_offset=0x0
> +base_ext_dpa=$(($base_dpa + $base_ext_offset))
> +base_ext_length=0x4000000
> +
> +# pre existing extent base + 128M, 64M length
> +# 0x00000088000000-0x0000008bffffff
> +pre_ext_offset=0x8000000
> +pre_ext_dpa=$(($base_dpa + $pre_ext_offset))
> +pre_ext_length=0x4000000
> +
> +# pre2 existing extent base + 256M, 64M length
> +# 0x00000090000000-0x00000093ffffff
> +pre2_ext_offset=0x10000000
> +pre2_ext_dpa=$(($base_dpa + $pre2_ext_offset))
> +pre2_ext_length=0x4000000
> +
> +mem=""
> +bus=""
> +device=""
> +decoder=""
> +
> +create_dcd_region()
> +{
> +	mem="$1"
> +	decoder="$2"
> +	reg_size_string=""
> +	if [ "$3" != "" ]; then
> +		reg_size_string="-s $3"
> +	fi
> +	dcd_partition="dc0"
> +	if [ "$4" != "" ]; then
> +		dcd_partition="$4"
> +	fi
> +
> +	# create region
> +	rc=$($CXL create-region -t ${dcd_partition} -d "$decoder" -m "$mem" ${reg_size_string} | jq -r ".region")
> +
> +	if [[ ! $rc ]]; then
> +		echo "create-region failed for $decoder / $mem"
> +		err "$LINENO"
> +	fi
> +
> +	echo ${rc}
> +}
> +
> +check_region()
> +{
> +	search=$1
> +	region_size=$2
> +
> +	result=$($CXL list -r "$search" | jq -r ".[].region")
> +	if [ "$result" != "$search" ]; then
> +		echo "check region failed to find $search"
> +		err "$LINENO"
> +	fi
> +
> +	result=$($CXL list -r "$search" | jq -r ".[].size")
> +	if [ "$result" != "$region_size" ]; then
> +		echo "check region failed invalid size $result != $region_size"
> +		err "$LINENO"
> +	fi
> +}
> +
> +check_not_region()
> +{
> +	search=$1
> +
> +	result=$($CXL list -r "$search" | jq -r ".[].region")
> +	if [ "$result" == "$search" ]; then
> +		echo "check not region failed; $search found"
> +		err "$LINENO"
> +	fi
> +}
> +
> +destroy_region()
> +{
> +	local region=$1
> +	$CXL disable-region $region
> +	$CXL destroy-region $region
> +}
> +
> +inject_extent()
> +{
> +	device="$1"
> +	dpa="$2"
> +	length="$3"
> +	tag="$4"
> +
> +	more="0"
> +	if [ "$5" != "" ]; then
> +		more="1"
> +	fi
> +
> +	echo ${dpa}:${length}:${tag}:${more} > "${dev_path}/${device}/dc_inject_extent"
> +}
> +
> +remove_extent()
> +{
> +	device="$1"
> +	dpa="$2"
> +	length="$3"
> +
> +	echo ${dpa}:${length} > "${dev_path}/${device}/dc_del_extent"
> +}
> +
> +create_dax_dev()
> +{
> +	reg="$1"
> +
> +	dax_dev=$($DAXCTL create-device -r $reg | jq -er '.[].chardev')
> +
> +	echo ${dax_dev}
> +}
> +
> +fail_create_dax_dev()
> +{
> +	reg="$1"
> +
> +	set +e
> +	result=$($DAXCTL create-device -r $reg)
> +	set -e
> +	if [ "$result" == "0" ]; then
> +		echo "FAIL device created"
> +		err "$LINENO"
> +	fi
> +}
> +
> +shrink_dax_dev()
> +{
> +	dev="$1"
> +	new_size="$2"
> +
> +	$DAXCTL disable-device $dev
> +	$DAXCTL reconfigure-device $dev -s $new_size
> +	$DAXCTL enable-device $dev
> +}
> +
> +destroy_dax_dev()
> +{
> +	dev="$1"
> +
> +	$DAXCTL disable-device $dev
> +	$DAXCTL destroy-device $dev
> +}
> +
> +check_dax_dev()
> +{
> +	search="$1"
> +	size="$2"
> +
> +	result=$($DAXCTL list -d $search | jq -er '.[].chardev')
> +	if [ "$result" != "$search" ]; then
> +		echo "check dax device failed to find $search"
> +		err "$LINENO"
> +	fi
> +	result=$($DAXCTL list -d $search | jq -er '.[].size')
> +	if [ "$result" -ne "$size" ]; then
> +		echo "check dax device failed incorrect size $result; exp $size"
> +		err "$LINENO"
> +	fi
> +}
> +
> +# check that the dax device is not there.
> +check_not_dax_dev()
> +{
> +	reg="$1"
> +	search="$2"
> +	result=$($DAXCTL list -r $reg -D | jq -er '.[].chardev')
> +	if [ "$result" == "$search" ]; then
> +		echo "FAIL found $search"
> +		err "$LINENO"
> +	fi
> +}
> +
> +check_extent()
> +{
> +	region=$1
> +	offset=$(($2))
> +	length=$(($3))
> +
> +	result=$($CXL list -r "$region" -N | jq -r ".[].extents[] | select(.offset == ${offset}) | .length")
> +	if [[ $result != $length ]]; then
> +		echo "FAIL region $1 could not find extent @ $offset ($length)"
> +		err "$LINENO"
> +	fi
> +}
> +
> +check_extent_cnt()
> +{
> +	region=$1
> +	count=$(($2))
> +
> +	result=$($CXL list -r $region -N | jq -r '.[].extents[].offset' | wc -l)
> +	if [[ $result != $count ]]; then
> +		echo "FAIL region $1: found wrong number of extents $result; expect $count"
> +		err "$LINENO"
> +	fi
> +}
> +
> +readarray -t memdevs < <("$CXL" list -b cxl_test -Mi | jq -r '.[].memdev')
> +
> +for mem in ${memdevs[@]}; do
> +	dcsize=$($CXL list -m $mem | jq -r '.[].dc0_size')
> +	if [ "$dcsize" == "null" ]; then
> +		continue
> +	fi
> +	decoder=$($CXL list -b cxl_test -D -d root -m "$mem" |
> +		  jq -r ".[] |
> +		  select(.dc0_capable == true) |
> +		  select(.nr_targets == 1) |
> +		  select(.max_available_extent >= ${dcsize}) |
> +		  .decoder")
> +	if [[ $decoder ]]; then
> +		bus=`"$CXL" list -b cxl_test -m ${mem} | jq -r '.[].bus'`
> +		device=$($CXL list -m $mem | jq -r '.[].host')
> +		break
> +	fi
> +done
> +
> +echo "TEST: DCD test device bus:${bus} decoder:${decoder} mem:${mem} device:${device} size:${dcsize}"
> +
> +if [ "$decoder" == "" ] || [ "$device" == "" ] || [ "$dcsize" == "" ]; then
> +	echo "No mem device/decoder found with DCD support"
> +	exit 77
> +fi
> +
> +echo ""
> +echo "Test: pre-existing extent"
> +echo ""
> +region=$(create_dcd_region ${mem} ${decoder})
> +check_region ${region} ${dcsize}
> +# should contain pre-created extents
> +check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
> +check_extent ${region} ${pre2_ext_offset} ${pre2_ext_length}
> +
> +
> +# | 2G non- |      DC region                                        |
> +# |  DC cap |                                                       |
> +# |  ...    |-------------------------------------------------------|
> +# |         |                   |----------|         |----------|   |
> +# |         |                   | (pre)-   |         | (pre2)-  |   |
> +# |         |                   | existing |         | existing |   |
> +
> +# Remove the pre-created test extent out from under dax device
> +# stack should hold ref until dax device deleted
> +echo ""
> +echo "Test: Remove extent from under DAX dev"
> +echo ""
> +dax_dev=$(create_dax_dev ${region})
> +check_extent_cnt ${region} 2
> +remove_extent ${device} $pre_ext_dpa $pre_ext_length
> +length="$(($pre_ext_length + $pre2_ext_length))"
> +check_dax_dev ${dax_dev} $length
> +check_extent_cnt ${region} 2
> +destroy_dax_dev ${dax_dev}
> +check_not_dax_dev ${region} ${dax_dev}
> +
> +# In-use extents are not released.  Remove after use.
> +check_extent_cnt ${region} 2
> +remove_extent ${device} $pre_ext_dpa $pre_ext_length
> +remove_extent ${device} $pre2_ext_dpa $pre2_ext_length
> +check_extent_cnt ${region} 0
> +
> +echo ""
> +echo "Test: Create dax device spanning 2 extents"
> +echo ""
> +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> +check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
> +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> +check_extent ${region} ${base_ext_offset} ${base_ext_length}
> +
> +# | 2G non- |      DC region                                        |
> +# |  DC cap |                                                       |
> +# |  ...    |-------------------------------------------------------|
> +# |         |--------|          |----------|                        |
> +# |         | (base) |          | (pre)-   |                        |
> +# |         |        |          | existing |                        |
> +
> +check_extent_cnt ${region} 2
> +dax_dev=$(create_dax_dev ${region})
> +
> +echo ""
> +echo "Test: dev dax is spanning sparse extents"
> +echo ""
> +ext_sum_length="$(($base_ext_length + $pre_ext_length))"
> +check_dax_dev ${dax_dev} $ext_sum_length
> +
> +
> +echo ""
> +echo "Test: Remove extents under sparse dax device"
> +echo ""
> +remove_extent ${device} $base_ext_dpa $base_ext_length
> +check_extent_cnt ${region} 2
> +remove_extent ${device} $pre_ext_dpa $pre_ext_length
> +check_extent_cnt ${region} 2
> +destroy_dax_dev ${dax_dev}
> +check_not_dax_dev ${region} ${dax_dev}
> +
> +# In-use extents are not released.  Remove after use.
> +check_extent_cnt ${region} 2
> +remove_extent ${device} $base_ext_dpa $base_ext_length
> +remove_extent ${device} $pre_ext_dpa $pre_ext_length
> +check_extent_cnt ${region} 0
> +
> +echo ""
> +echo "Test: inject without/with tag"
> +echo ""
> +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> +check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
> +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> +check_extent ${region} ${base_ext_offset} ${base_ext_length}
> +remove_extent ${device} $base_ext_dpa $base_ext_length
> +remove_extent ${device} $pre_ext_dpa $pre_ext_length
> +check_extent_cnt ${region} 0
> +
> +
> +echo ""
> +echo "Test: partial extent remove"
> +echo ""
> +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> +dax_dev=$(create_dax_dev ${region})
> +
> +# | 2G non- |      DC region                                        |
> +# |  DC cap |                                                       |
> +# |  ...    |-------------------------------------------------------|
> +# |         |--------|                                              |
> +# |         | (base) |                                              |
> +# |         |    |---|                                              |
> +#                  Partial
> +
> +partial_ext_dpa="$(($base_ext_dpa + ($base_ext_length / 2)))"
> +partial_ext_length="$(($base_ext_length / 2))"
> +echo "Removing Partial : $partial_ext_dpa $partial_ext_length"
> +remove_extent ${device} $partial_ext_dpa $partial_ext_length
> +check_extent_cnt ${region} 1
> +destroy_dax_dev ${dax_dev}
> +check_not_dax_dev ${region} ${dax_dev}
> +
> +# In-use extents are not released.  Remove after use.
> +check_extent_cnt ${region} 1
> +remove_extent ${device} $partial_ext_dpa $partial_ext_length
> +check_extent_cnt ${region} 0
> +
> +# Test multiple extent remove
> +echo ""
> +echo "Test: multiple extent remove with single extent remove command"
> +echo ""
> +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> +check_extent_cnt ${region} 2
> +dax_dev=$(create_dax_dev ${region})
> +
> +# | 2G non- |      DC region                                        |
> +# |  DC cap |                                                       |
> +# |  ...    |-------------------------------------------------------|
> +# |         |--------|          |-------------------|               |
> +# |         | (base) |          | (pre)-existing    |               |
> +#                |------------------|
> +#                  Partial
> +
> +partial_ext_dpa="$(($base_ext_dpa + ($base_ext_length / 2)))"
> +partial_ext_length="$(($pre_ext_dpa - $base_ext_dpa))"
> +echo "Removing multiple in span : $partial_ext_dpa $partial_ext_length"
> +remove_extent ${device} $partial_ext_dpa $partial_ext_length
> +check_extent_cnt ${region} 2
> +destroy_dax_dev ${dax_dev}
> +check_not_dax_dev ${region} ${dax_dev}
> +
> +
> +echo ""
> +echo "Test: Destroy region without extent removal"
> +echo ""
> +
> +# In-use extents are not released.
> +check_extent_cnt ${region} 2
> +destroy_region ${region}
> +check_not_region ${region}
> +
> +
> +echo ""
> +echo "Test: Destroy region with extents and dax devices"
> +echo ""
> +region=$(create_dcd_region ${mem} ${decoder})
> +check_region ${region} ${dcsize}
> +check_extent_cnt ${region} 0
> +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> +
> +# | 2G non- |      DC region                                        |
> +# |  DC cap |                                                       |
> +# |  ...    |-------------------------------------------------------|
> +# |         |                   |----------|                        |
> +# |         |                   | (pre)-   |                        |
> +# |         |                   | existing |                        |
> +
> +check_extent_cnt ${region} 1
> +dax_dev=$(create_dax_dev ${region})
> +destroy_region ${region}
> +check_not_region ${region}
> +
> +echo ""
> +echo "Test: Fail sparse dax dev creation without space"
> +echo ""
> +region=$(create_dcd_region ${mem} ${decoder})
> +check_region ${region} ${dcsize}
> +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> +
> +# | 2G non- |      DC region                                        |
> +# |  DC cap |                                                       |
> +# |  ...    |-------------------------------------------------------|
> +# |         |                   |-------------------|               |
> +# |         |                   | (pre)-existing    |               |
> +
> +check_extent_cnt ${region} 1
> +
> +# |         |                   | dax0.1            |               |
> +
> +dax_dev=$(create_dax_dev ${region})
> +check_dax_dev ${dax_dev} $pre_ext_length
> +fail_create_dax_dev ${region}
> +
> +echo ""
> +echo "Test: Resize sparse dax device"
> +echo ""
> +
> +# Shrink
> +# |         |                   | dax0.1  |                         |
> +resize_ext_length=$(($pre_ext_length / 2))
> +shrink_dax_dev ${dax_dev} $resize_ext_length
> +check_dax_dev ${dax_dev} $resize_ext_length
> +
> +# Fill
> +# |         |                   | dax0.1  | dax0.2  |               |
> +dax_dev=$(create_dax_dev ${region})
> +check_dax_dev ${dax_dev} $resize_ext_length
> +destroy_region ${region}
> +check_not_region ${region}
> +
> +
> +# 2 extent
> +# create dax dev
> +# resize into 1st extent
> +# create dev on rest of 1st and all of second
> +# Ensure both devices are correct
> +
> +echo ""
> +echo "Test: Resize sparse dax device across extents"
> +echo ""
> +region=$(create_dcd_region ${mem} ${decoder})
> +check_region ${region} ${dcsize}
> +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> +
> +# | 2G non- |      DC region                                        |
> +# |  DC cap |                                                       |
> +# |  ...    |-------------------------------------------------------|
> +# |         |--------|          |-------------------|               |
> +# |         | (base) |          | (pre)-existing    |               |
> +
> +check_extent_cnt ${region} 2
> +dax_dev=$(create_dax_dev ${region})
> +ext_sum_length="$(($base_ext_length + $pre_ext_length))"
> +
> +# |         | dax0.1 |          |  dax0.1           |               |
> +
> +check_dax_dev ${dax_dev} $ext_sum_length
> +resize_ext_length=33554432 # 32MB
> +
> +# |         | D1 |                                                  |
> +
> +shrink_dax_dev ${dax_dev} $resize_ext_length
> +check_dax_dev ${dax_dev} $resize_ext_length
> +
> +# |         | D1 | D2|          | dax0.2            |               |
> +
> +dax_dev=$(create_dax_dev ${region})
> +remainder_length=$((ext_sum_length - $resize_ext_length))
> +check_dax_dev ${dax_dev} $remainder_length
> +
> +# |         | D1 | D2|          | dax0.2 |                          |
> +
> +remainder_length=$((remainder_length / 2))
> +shrink_dax_dev ${dax_dev} $remainder_length
> +check_dax_dev ${dax_dev} $remainder_length
> +
> +# |         | D1 | D2|          | dax0.2 |  dax0.3  |               |
> +
> +dax_dev=$(create_dax_dev ${region})
> +check_dax_dev ${dax_dev} $remainder_length
> +destroy_region ${region}
> +check_not_region ${region}
> +
> +
> +echo ""
> +echo "Test: Rejecting overlapping extents"
> +echo ""
> +
> +region=$(create_dcd_region ${mem} ${decoder})
> +check_region ${region} ${dcsize}
> +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> +
> +# | 2G non- |      DC region                                        |
> +# |  DC cap |                                                       |
> +# |  ...    |-------------------------------------------------------|
> +# |         |                   |-------------------|               |
> +# |         |                   | (pre)-existing    |               |
> +
> +check_extent_cnt ${region} 1
> +
> +# Attempt overlapping extent
> +#
> +# |         |          |-----------------|                          |
> +# |         |          | overlapping     |                          |
> +
> +partial_ext_dpa="$(($base_ext_dpa + ($pre_ext_dpa / 2)))"
> +partial_ext_length=$pre_ext_length
> +inject_extent ${device} $partial_ext_dpa $partial_ext_length ""
> +
> +# Should only see the original ext
> +check_extent_cnt ${region} 1
> +destroy_region ${region}
> +check_not_region ${region}
> +
> +
> +echo ""
> +echo "Test: create 2 regions in the same DC partition"
> +echo ""
> +region_size=$(($dcsize / 2))
> +region=$(create_dcd_region ${mem} ${decoder} ${region_size} dc1)
> +check_region ${region} ${region_size}
> +
> +region_two=$(create_dcd_region ${mem} ${decoder} ${region_size} dc1)
> +check_region ${region_two} ${region_size}
> +
> +destroy_region ${region_two}
> +check_not_region ${region_two}
> +destroy_region ${region}
> +check_not_region ${region}
> +
> +
> +echo ""
> +echo "Test: More bit"
> +echo ""
> +region=$(create_dcd_region ${mem} ${decoder})
> +check_region ${region} ${dcsize}
> +inject_extent ${device} $pre_ext_dpa $pre_ext_length "" 1
> +# More bit should hold off surfacing extent until the more bit is 0
> +check_extent_cnt ${region} 0
> +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> +check_extent_cnt ${region} 2
> +destroy_region ${region}
> +check_not_region ${region}
> +
> +
> +# Create a new region for driver tests
> +region=$(create_dcd_region ${mem} ${decoder})
> +
> +echo ""
> +echo "Test: driver remove tear down"
> +echo ""
> +check_region ${region} ${dcsize}
> +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> +check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
> +dax_dev=$(create_dax_dev ${region})
> +# remove driver releases extents
> +modprobe -r dax_cxl
> +check_extent_cnt ${region} 0
> +
> +# leave region up, driver removed.
> +echo ""
> +echo "Test: no driver inject ok"
> +echo ""
> +check_region ${region} ${dcsize}
> +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> +check_extent_cnt ${region} 1
> +modprobe dax_cxl
> +check_extent_cnt ${region} 1
> +
> +destroy_region ${region}
> +check_not_region ${region}
> +
> +
> +# Test event reporting
> +# results expected
> +num_dcd_events_expected=2
> +
> +echo "Test: Prep event trace"
> +echo "" > /sys/kernel/tracing/trace
> +echo 1 > /sys/kernel/tracing/events/cxl/enable
> +echo 1 > /sys/kernel/tracing/tracing_on
> +
> +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> +remove_extent ${device} $base_ext_dpa $base_ext_length
> +
> +echo 0 > /sys/kernel/tracing/tracing_on
> +
> +echo "Test: Events seen"
> +trace_out=$(cat /sys/kernel/tracing/trace)
> +
> +# Look for DCD events
> +num_dcd_events=$(grep -c "cxl_dynamic_capacity" <<< "${trace_out}")
> +echo "     LOG     (Expected) : (Found)"
> +echo "     DCD events    ($num_dcd_events_expected) : $num_dcd_events"
> +
> +if [ "$num_dcd_events" -ne $num_dcd_events_expected ]; then
> +	err "$LINENO"
> +fi
> +
> +modprobe -r cxl_test
> +
> +check_dmesg "$LINENO"
> +
> +exit 0
> diff --git a/test/meson.build b/test/meson.build
> index d871e28e17ce512cd1e7b43f3ec081729fe5e03a..1cfcb60d16e05272893ae1c67820aa8614281505 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -161,6 +161,7 @@ cxl_sanitize = find_program('cxl-sanitize.sh')
>  cxl_destroy_region = find_program('cxl-destroy-region.sh')
>  cxl_qos_class = find_program('cxl-qos-class.sh')
>  cxl_poison = find_program('cxl-poison.sh')
> +cxl_dcd = find_program('cxl-dcd.sh')
>  
>  tests = [
>    [ 'libndctl',               libndctl,		  'ndctl' ],
> @@ -194,6 +195,7 @@ tests = [
>    [ 'cxl-destroy-region.sh',  cxl_destroy_region, 'cxl'   ],
>    [ 'cxl-qos-class.sh',       cxl_qos_class,      'cxl'   ],
>    [ 'cxl-poison.sh',          cxl_poison,         'cxl'   ],
> +  [ 'cxl-dcd.sh',             cxl_dcd,            'cxl'   ],
>  ]
>  
>  if get_option('destructive').enabled()
> 

Re: [ndctl PATCH v2 6/6] ndctl/cxl/test: Add Dynamic Capacity tests

[Ira Weiny]

Dave Jiang wrote:
> 
> 
> On 11/4/24 7:10 PM, Ira Weiny wrote:
> > cxl_test provides a good way to ensure quick smoke and regression
> > testing.  The complexity of DCD and the new sparse DAX regions required
> > to use them benefits greatly with a series of smoke tests.
> > 
> > The only part of the kernel stack which must be bypassed is the actual
> > irq of DCD events.  However, the event processing itself can be tested
> > via cxl_test calling directly the event processing function directly.
> > 
> > In this way the rest of the stack; management of sparse regions, the
> > extent device lifetimes, and the dax device operations can be tested.
> > 
> > Add Dynamic Capacity Device tests for kernels which have DCD support in
> > cxl_test.
> > 
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> Would things look cleaner if the multiple test cases are organized into individual bash functions?

Perhaps.  But many of the initial tests borrow state from the previous
test.  For example the cxl-test pre-existing extents are used to test
extent removal.  Later on those dependencies lessoned.

Let me see if I can clean up the testing now that things are settling out.

Ira

> 
> > ---
> >  test/cxl-dcd.sh  | 656 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  test/meson.build |   2 +
> >  2 files changed, 658 insertions(+)
> > 
> > diff --git a/test/cxl-dcd.sh b/test/cxl-dcd.sh
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..f5248fce4f3899acdf646ace4f0eb531ea2286d3
> > --- /dev/null
> > +++ b/test/cxl-dcd.sh
> > @@ -0,0 +1,656 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2024 Intel Corporation. All rights reserved.
> > +
> > +. "$(dirname "$0")/common"
> > +
> > +rc=77
> > +set -ex
> > +
> > +trap 'err $LINENO' ERR
> > +
> > +check_prereq "jq"
> > +
> > +modprobe -r cxl_test
> > +modprobe cxl_test
> > +rc=1
> > +
> > +dev_path="/sys/bus/platform/devices"
> > +cxl_path="/sys/bus/cxl/devices"
> > +
> > +# a test extent tag
> > +test_tag=dc-test-tag
> > +
> > +#
> > +# The test devices have 2G of non DC capacity.  A single DC reagion of 1G is
> > +# added beyond that.
> > +#
> > +# The testing centers around 3 extents.  Two are pre-existing on test load
> > +# called pre-existing.  The other is created within this script alone called
> > +# base.
> > +
> > +#
> > +# | 2G non- |      DC region (1G)                                   |
> > +# |  DC cap |                                                       |
> > +# |  ...    |-------------------------------------------------------|
> > +# |         |--------|       |----------|      |----------|         |
> > +# |         | (base) |       | (pre)-   |      | (pre2)-  |         |
> > +# |         |        |       | existing |      | existing |         |
> > +
> > +dcsize=""
> > +
> > +base_dpa=0x80000000
> > +
> > +# base extent at dpa 2G - 64M long
> > +base_ext_offset=0x0
> > +base_ext_dpa=$(($base_dpa + $base_ext_offset))
> > +base_ext_length=0x4000000
> > +
> > +# pre existing extent base + 128M, 64M length
> > +# 0x00000088000000-0x0000008bffffff
> > +pre_ext_offset=0x8000000
> > +pre_ext_dpa=$(($base_dpa + $pre_ext_offset))
> > +pre_ext_length=0x4000000
> > +
> > +# pre2 existing extent base + 256M, 64M length
> > +# 0x00000090000000-0x00000093ffffff
> > +pre2_ext_offset=0x10000000
> > +pre2_ext_dpa=$(($base_dpa + $pre2_ext_offset))
> > +pre2_ext_length=0x4000000
> > +
> > +mem=""
> > +bus=""
> > +device=""
> > +decoder=""
> > +
> > +create_dcd_region()
> > +{
> > +	mem="$1"
> > +	decoder="$2"
> > +	reg_size_string=""
> > +	if [ "$3" != "" ]; then
> > +		reg_size_string="-s $3"
> > +	fi
> > +	dcd_partition="dc0"
> > +	if [ "$4" != "" ]; then
> > +		dcd_partition="$4"
> > +	fi
> > +
> > +	# create region
> > +	rc=$($CXL create-region -t ${dcd_partition} -d "$decoder" -m "$mem" ${reg_size_string} | jq -r ".region")
> > +
> > +	if [[ ! $rc ]]; then
> > +		echo "create-region failed for $decoder / $mem"
> > +		err "$LINENO"
> > +	fi
> > +
> > +	echo ${rc}
> > +}
> > +
> > +check_region()
> > +{
> > +	search=$1
> > +	region_size=$2
> > +
> > +	result=$($CXL list -r "$search" | jq -r ".[].region")
> > +	if [ "$result" != "$search" ]; then
> > +		echo "check region failed to find $search"
> > +		err "$LINENO"
> > +	fi
> > +
> > +	result=$($CXL list -r "$search" | jq -r ".[].size")
> > +	if [ "$result" != "$region_size" ]; then
> > +		echo "check region failed invalid size $result != $region_size"
> > +		err "$LINENO"
> > +	fi
> > +}
> > +
> > +check_not_region()
> > +{
> > +	search=$1
> > +
> > +	result=$($CXL list -r "$search" | jq -r ".[].region")
> > +	if [ "$result" == "$search" ]; then
> > +		echo "check not region failed; $search found"
> > +		err "$LINENO"
> > +	fi
> > +}
> > +
> > +destroy_region()
> > +{
> > +	local region=$1
> > +	$CXL disable-region $region
> > +	$CXL destroy-region $region
> > +}
> > +
> > +inject_extent()
> > +{
> > +	device="$1"
> > +	dpa="$2"
> > +	length="$3"
> > +	tag="$4"
> > +
> > +	more="0"
> > +	if [ "$5" != "" ]; then
> > +		more="1"
> > +	fi
> > +
> > +	echo ${dpa}:${length}:${tag}:${more} > "${dev_path}/${device}/dc_inject_extent"
> > +}
> > +
> > +remove_extent()
> > +{
> > +	device="$1"
> > +	dpa="$2"
> > +	length="$3"
> > +
> > +	echo ${dpa}:${length} > "${dev_path}/${device}/dc_del_extent"
> > +}
> > +
> > +create_dax_dev()
> > +{
> > +	reg="$1"
> > +
> > +	dax_dev=$($DAXCTL create-device -r $reg | jq -er '.[].chardev')
> > +
> > +	echo ${dax_dev}
> > +}
> > +
> > +fail_create_dax_dev()
> > +{
> > +	reg="$1"
> > +
> > +	set +e
> > +	result=$($DAXCTL create-device -r $reg)
> > +	set -e
> > +	if [ "$result" == "0" ]; then
> > +		echo "FAIL device created"
> > +		err "$LINENO"
> > +	fi
> > +}
> > +
> > +shrink_dax_dev()
> > +{
> > +	dev="$1"
> > +	new_size="$2"
> > +
> > +	$DAXCTL disable-device $dev
> > +	$DAXCTL reconfigure-device $dev -s $new_size
> > +	$DAXCTL enable-device $dev
> > +}
> > +
> > +destroy_dax_dev()
> > +{
> > +	dev="$1"
> > +
> > +	$DAXCTL disable-device $dev
> > +	$DAXCTL destroy-device $dev
> > +}
> > +
> > +check_dax_dev()
> > +{
> > +	search="$1"
> > +	size="$2"
> > +
> > +	result=$($DAXCTL list -d $search | jq -er '.[].chardev')
> > +	if [ "$result" != "$search" ]; then
> > +		echo "check dax device failed to find $search"
> > +		err "$LINENO"
> > +	fi
> > +	result=$($DAXCTL list -d $search | jq -er '.[].size')
> > +	if [ "$result" -ne "$size" ]; then
> > +		echo "check dax device failed incorrect size $result; exp $size"
> > +		err "$LINENO"
> > +	fi
> > +}
> > +
> > +# check that the dax device is not there.
> > +check_not_dax_dev()
> > +{
> > +	reg="$1"
> > +	search="$2"
> > +	result=$($DAXCTL list -r $reg -D | jq -er '.[].chardev')
> > +	if [ "$result" == "$search" ]; then
> > +		echo "FAIL found $search"
> > +		err "$LINENO"
> > +	fi
> > +}
> > +
> > +check_extent()
> > +{
> > +	region=$1
> > +	offset=$(($2))
> > +	length=$(($3))
> > +
> > +	result=$($CXL list -r "$region" -N | jq -r ".[].extents[] | select(.offset == ${offset}) | .length")
> > +	if [[ $result != $length ]]; then
> > +		echo "FAIL region $1 could not find extent @ $offset ($length)"
> > +		err "$LINENO"
> > +	fi
> > +}
> > +
> > +check_extent_cnt()
> > +{
> > +	region=$1
> > +	count=$(($2))
> > +
> > +	result=$($CXL list -r $region -N | jq -r '.[].extents[].offset' | wc -l)
> > +	if [[ $result != $count ]]; then
> > +		echo "FAIL region $1: found wrong number of extents $result; expect $count"
> > +		err "$LINENO"
> > +	fi
> > +}
> > +
> > +readarray -t memdevs < <("$CXL" list -b cxl_test -Mi | jq -r '.[].memdev')
> > +
> > +for mem in ${memdevs[@]}; do
> > +	dcsize=$($CXL list -m $mem | jq -r '.[].dc0_size')
> > +	if [ "$dcsize" == "null" ]; then
> > +		continue
> > +	fi
> > +	decoder=$($CXL list -b cxl_test -D -d root -m "$mem" |
> > +		  jq -r ".[] |
> > +		  select(.dc0_capable == true) |
> > +		  select(.nr_targets == 1) |
> > +		  select(.max_available_extent >= ${dcsize}) |
> > +		  .decoder")
> > +	if [[ $decoder ]]; then
> > +		bus=`"$CXL" list -b cxl_test -m ${mem} | jq -r '.[].bus'`
> > +		device=$($CXL list -m $mem | jq -r '.[].host')
> > +		break
> > +	fi
> > +done
> > +
> > +echo "TEST: DCD test device bus:${bus} decoder:${decoder} mem:${mem} device:${device} size:${dcsize}"
> > +
> > +if [ "$decoder" == "" ] || [ "$device" == "" ] || [ "$dcsize" == "" ]; then
> > +	echo "No mem device/decoder found with DCD support"
> > +	exit 77
> > +fi
> > +
> > +echo ""
> > +echo "Test: pre-existing extent"
> > +echo ""
> > +region=$(create_dcd_region ${mem} ${decoder})
> > +check_region ${region} ${dcsize}
> > +# should contain pre-created extents
> > +check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
> > +check_extent ${region} ${pre2_ext_offset} ${pre2_ext_length}
> > +
> > +
> > +# | 2G non- |      DC region                                        |
> > +# |  DC cap |                                                       |
> > +# |  ...    |-------------------------------------------------------|
> > +# |         |                   |----------|         |----------|   |
> > +# |         |                   | (pre)-   |         | (pre2)-  |   |
> > +# |         |                   | existing |         | existing |   |
> > +
> > +# Remove the pre-created test extent out from under dax device
> > +# stack should hold ref until dax device deleted
> > +echo ""
> > +echo "Test: Remove extent from under DAX dev"
> > +echo ""
> > +dax_dev=$(create_dax_dev ${region})
> > +check_extent_cnt ${region} 2
> > +remove_extent ${device} $pre_ext_dpa $pre_ext_length
> > +length="$(($pre_ext_length + $pre2_ext_length))"
> > +check_dax_dev ${dax_dev} $length
> > +check_extent_cnt ${region} 2
> > +destroy_dax_dev ${dax_dev}
> > +check_not_dax_dev ${region} ${dax_dev}
> > +
> > +# In-use extents are not released.  Remove after use.
> > +check_extent_cnt ${region} 2
> > +remove_extent ${device} $pre_ext_dpa $pre_ext_length
> > +remove_extent ${device} $pre2_ext_dpa $pre2_ext_length
> > +check_extent_cnt ${region} 0
> > +
> > +echo ""
> > +echo "Test: Create dax device spanning 2 extents"
> > +echo ""
> > +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> > +check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
> > +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> > +check_extent ${region} ${base_ext_offset} ${base_ext_length}
> > +
> > +# | 2G non- |      DC region                                        |
> > +# |  DC cap |                                                       |
> > +# |  ...    |-------------------------------------------------------|
> > +# |         |--------|          |----------|                        |
> > +# |         | (base) |          | (pre)-   |                        |
> > +# |         |        |          | existing |                        |
> > +
> > +check_extent_cnt ${region} 2
> > +dax_dev=$(create_dax_dev ${region})
> > +
> > +echo ""
> > +echo "Test: dev dax is spanning sparse extents"
> > +echo ""
> > +ext_sum_length="$(($base_ext_length + $pre_ext_length))"
> > +check_dax_dev ${dax_dev} $ext_sum_length
> > +
> > +
> > +echo ""
> > +echo "Test: Remove extents under sparse dax device"
> > +echo ""
> > +remove_extent ${device} $base_ext_dpa $base_ext_length
> > +check_extent_cnt ${region} 2
> > +remove_extent ${device} $pre_ext_dpa $pre_ext_length
> > +check_extent_cnt ${region} 2
> > +destroy_dax_dev ${dax_dev}
> > +check_not_dax_dev ${region} ${dax_dev}
> > +
> > +# In-use extents are not released.  Remove after use.
> > +check_extent_cnt ${region} 2
> > +remove_extent ${device} $base_ext_dpa $base_ext_length
> > +remove_extent ${device} $pre_ext_dpa $pre_ext_length
> > +check_extent_cnt ${region} 0
> > +
> > +echo ""
> > +echo "Test: inject without/with tag"
> > +echo ""
> > +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> > +check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
> > +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> > +check_extent ${region} ${base_ext_offset} ${base_ext_length}
> > +remove_extent ${device} $base_ext_dpa $base_ext_length
> > +remove_extent ${device} $pre_ext_dpa $pre_ext_length
> > +check_extent_cnt ${region} 0
> > +
> > +
> > +echo ""
> > +echo "Test: partial extent remove"
> > +echo ""
> > +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> > +dax_dev=$(create_dax_dev ${region})
> > +
> > +# | 2G non- |      DC region                                        |
> > +# |  DC cap |                                                       |
> > +# |  ...    |-------------------------------------------------------|
> > +# |         |--------|                                              |
> > +# |         | (base) |                                              |
> > +# |         |    |---|                                              |
> > +#                  Partial
> > +
> > +partial_ext_dpa="$(($base_ext_dpa + ($base_ext_length / 2)))"
> > +partial_ext_length="$(($base_ext_length / 2))"
> > +echo "Removing Partial : $partial_ext_dpa $partial_ext_length"
> > +remove_extent ${device} $partial_ext_dpa $partial_ext_length
> > +check_extent_cnt ${region} 1
> > +destroy_dax_dev ${dax_dev}
> > +check_not_dax_dev ${region} ${dax_dev}
> > +
> > +# In-use extents are not released.  Remove after use.
> > +check_extent_cnt ${region} 1
> > +remove_extent ${device} $partial_ext_dpa $partial_ext_length
> > +check_extent_cnt ${region} 0
> > +
> > +# Test multiple extent remove
> > +echo ""
> > +echo "Test: multiple extent remove with single extent remove command"
> > +echo ""
> > +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> > +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> > +check_extent_cnt ${region} 2
> > +dax_dev=$(create_dax_dev ${region})
> > +
> > +# | 2G non- |      DC region                                        |
> > +# |  DC cap |                                                       |
> > +# |  ...    |-------------------------------------------------------|
> > +# |         |--------|          |-------------------|               |
> > +# |         | (base) |          | (pre)-existing    |               |
> > +#                |------------------|
> > +#                  Partial
> > +
> > +partial_ext_dpa="$(($base_ext_dpa + ($base_ext_length / 2)))"
> > +partial_ext_length="$(($pre_ext_dpa - $base_ext_dpa))"
> > +echo "Removing multiple in span : $partial_ext_dpa $partial_ext_length"
> > +remove_extent ${device} $partial_ext_dpa $partial_ext_length
> > +check_extent_cnt ${region} 2
> > +destroy_dax_dev ${dax_dev}
> > +check_not_dax_dev ${region} ${dax_dev}
> > +
> > +
> > +echo ""
> > +echo "Test: Destroy region without extent removal"
> > +echo ""
> > +
> > +# In-use extents are not released.
> > +check_extent_cnt ${region} 2
> > +destroy_region ${region}
> > +check_not_region ${region}
> > +
> > +
> > +echo ""
> > +echo "Test: Destroy region with extents and dax devices"
> > +echo ""
> > +region=$(create_dcd_region ${mem} ${decoder})
> > +check_region ${region} ${dcsize}
> > +check_extent_cnt ${region} 0
> > +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> > +
> > +# | 2G non- |      DC region                                        |
> > +# |  DC cap |                                                       |
> > +# |  ...    |-------------------------------------------------------|
> > +# |         |                   |----------|                        |
> > +# |         |                   | (pre)-   |                        |
> > +# |         |                   | existing |                        |
> > +
> > +check_extent_cnt ${region} 1
> > +dax_dev=$(create_dax_dev ${region})
> > +destroy_region ${region}
> > +check_not_region ${region}
> > +
> > +echo ""
> > +echo "Test: Fail sparse dax dev creation without space"
> > +echo ""
> > +region=$(create_dcd_region ${mem} ${decoder})
> > +check_region ${region} ${dcsize}
> > +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> > +
> > +# | 2G non- |      DC region                                        |
> > +# |  DC cap |                                                       |
> > +# |  ...    |-------------------------------------------------------|
> > +# |         |                   |-------------------|               |
> > +# |         |                   | (pre)-existing    |               |
> > +
> > +check_extent_cnt ${region} 1
> > +
> > +# |         |                   | dax0.1            |               |
> > +
> > +dax_dev=$(create_dax_dev ${region})
> > +check_dax_dev ${dax_dev} $pre_ext_length
> > +fail_create_dax_dev ${region}
> > +
> > +echo ""
> > +echo "Test: Resize sparse dax device"
> > +echo ""
> > +
> > +# Shrink
> > +# |         |                   | dax0.1  |                         |
> > +resize_ext_length=$(($pre_ext_length / 2))
> > +shrink_dax_dev ${dax_dev} $resize_ext_length
> > +check_dax_dev ${dax_dev} $resize_ext_length
> > +
> > +# Fill
> > +# |         |                   | dax0.1  | dax0.2  |               |
> > +dax_dev=$(create_dax_dev ${region})
> > +check_dax_dev ${dax_dev} $resize_ext_length
> > +destroy_region ${region}
> > +check_not_region ${region}
> > +
> > +
> > +# 2 extent
> > +# create dax dev
> > +# resize into 1st extent
> > +# create dev on rest of 1st and all of second
> > +# Ensure both devices are correct
> > +
> > +echo ""
> > +echo "Test: Resize sparse dax device across extents"
> > +echo ""
> > +region=$(create_dcd_region ${mem} ${decoder})
> > +check_region ${region} ${dcsize}
> > +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> > +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> > +
> > +# | 2G non- |      DC region                                        |
> > +# |  DC cap |                                                       |
> > +# |  ...    |-------------------------------------------------------|
> > +# |         |--------|          |-------------------|               |
> > +# |         | (base) |          | (pre)-existing    |               |
> > +
> > +check_extent_cnt ${region} 2
> > +dax_dev=$(create_dax_dev ${region})
> > +ext_sum_length="$(($base_ext_length + $pre_ext_length))"
> > +
> > +# |         | dax0.1 |          |  dax0.1           |               |
> > +
> > +check_dax_dev ${dax_dev} $ext_sum_length
> > +resize_ext_length=33554432 # 32MB
> > +
> > +# |         | D1 |                                                  |
> > +
> > +shrink_dax_dev ${dax_dev} $resize_ext_length
> > +check_dax_dev ${dax_dev} $resize_ext_length
> > +
> > +# |         | D1 | D2|          | dax0.2            |               |
> > +
> > +dax_dev=$(create_dax_dev ${region})
> > +remainder_length=$((ext_sum_length - $resize_ext_length))
> > +check_dax_dev ${dax_dev} $remainder_length
> > +
> > +# |         | D1 | D2|          | dax0.2 |                          |
> > +
> > +remainder_length=$((remainder_length / 2))
> > +shrink_dax_dev ${dax_dev} $remainder_length
> > +check_dax_dev ${dax_dev} $remainder_length
> > +
> > +# |         | D1 | D2|          | dax0.2 |  dax0.3  |               |
> > +
> > +dax_dev=$(create_dax_dev ${region})
> > +check_dax_dev ${dax_dev} $remainder_length
> > +destroy_region ${region}
> > +check_not_region ${region}
> > +
> > +
> > +echo ""
> > +echo "Test: Rejecting overlapping extents"
> > +echo ""
> > +
> > +region=$(create_dcd_region ${mem} ${decoder})
> > +check_region ${region} ${dcsize}
> > +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> > +
> > +# | 2G non- |      DC region                                        |
> > +# |  DC cap |                                                       |
> > +# |  ...    |-------------------------------------------------------|
> > +# |         |                   |-------------------|               |
> > +# |         |                   | (pre)-existing    |               |
> > +
> > +check_extent_cnt ${region} 1
> > +
> > +# Attempt overlapping extent
> > +#
> > +# |         |          |-----------------|                          |
> > +# |         |          | overlapping     |                          |
> > +
> > +partial_ext_dpa="$(($base_ext_dpa + ($pre_ext_dpa / 2)))"
> > +partial_ext_length=$pre_ext_length
> > +inject_extent ${device} $partial_ext_dpa $partial_ext_length ""
> > +
> > +# Should only see the original ext
> > +check_extent_cnt ${region} 1
> > +destroy_region ${region}
> > +check_not_region ${region}
> > +
> > +
> > +echo ""
> > +echo "Test: create 2 regions in the same DC partition"
> > +echo ""
> > +region_size=$(($dcsize / 2))
> > +region=$(create_dcd_region ${mem} ${decoder} ${region_size} dc1)
> > +check_region ${region} ${region_size}
> > +
> > +region_two=$(create_dcd_region ${mem} ${decoder} ${region_size} dc1)
> > +check_region ${region_two} ${region_size}
> > +
> > +destroy_region ${region_two}
> > +check_not_region ${region_two}
> > +destroy_region ${region}
> > +check_not_region ${region}
> > +
> > +
> > +echo ""
> > +echo "Test: More bit"
> > +echo ""
> > +region=$(create_dcd_region ${mem} ${decoder})
> > +check_region ${region} ${dcsize}
> > +inject_extent ${device} $pre_ext_dpa $pre_ext_length "" 1
> > +# More bit should hold off surfacing extent until the more bit is 0
> > +check_extent_cnt ${region} 0
> > +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> > +check_extent_cnt ${region} 2
> > +destroy_region ${region}
> > +check_not_region ${region}
> > +
> > +
> > +# Create a new region for driver tests
> > +region=$(create_dcd_region ${mem} ${decoder})
> > +
> > +echo ""
> > +echo "Test: driver remove tear down"
> > +echo ""
> > +check_region ${region} ${dcsize}
> > +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> > +check_extent ${region} ${pre_ext_offset} ${pre_ext_length}
> > +dax_dev=$(create_dax_dev ${region})
> > +# remove driver releases extents
> > +modprobe -r dax_cxl
> > +check_extent_cnt ${region} 0
> > +
> > +# leave region up, driver removed.
> > +echo ""
> > +echo "Test: no driver inject ok"
> > +echo ""
> > +check_region ${region} ${dcsize}
> > +inject_extent ${device} $pre_ext_dpa $pre_ext_length ""
> > +check_extent_cnt ${region} 1
> > +modprobe dax_cxl
> > +check_extent_cnt ${region} 1
> > +
> > +destroy_region ${region}
> > +check_not_region ${region}
> > +
> > +
> > +# Test event reporting
> > +# results expected
> > +num_dcd_events_expected=2
> > +
> > +echo "Test: Prep event trace"
> > +echo "" > /sys/kernel/tracing/trace
> > +echo 1 > /sys/kernel/tracing/events/cxl/enable
> > +echo 1 > /sys/kernel/tracing/tracing_on
> > +
> > +inject_extent ${device} $base_ext_dpa $base_ext_length ""
> > +remove_extent ${device} $base_ext_dpa $base_ext_length
> > +
> > +echo 0 > /sys/kernel/tracing/tracing_on
> > +
> > +echo "Test: Events seen"
> > +trace_out=$(cat /sys/kernel/tracing/trace)
> > +
> > +# Look for DCD events
> > +num_dcd_events=$(grep -c "cxl_dynamic_capacity" <<< "${trace_out}")
> > +echo "     LOG     (Expected) : (Found)"
> > +echo "     DCD events    ($num_dcd_events_expected) : $num_dcd_events"
> > +
> > +if [ "$num_dcd_events" -ne $num_dcd_events_expected ]; then
> > +	err "$LINENO"
> > +fi
> > +
> > +modprobe -r cxl_test
> > +
> > +check_dmesg "$LINENO"
> > +
> > +exit 0
> > diff --git a/test/meson.build b/test/meson.build
> > index d871e28e17ce512cd1e7b43f3ec081729fe5e03a..1cfcb60d16e05272893ae1c67820aa8614281505 100644
> > --- a/test/meson.build
> > +++ b/test/meson.build
> > @@ -161,6 +161,7 @@ cxl_sanitize = find_program('cxl-sanitize.sh')
> >  cxl_destroy_region = find_program('cxl-destroy-region.sh')
> >  cxl_qos_class = find_program('cxl-qos-class.sh')
> >  cxl_poison = find_program('cxl-poison.sh')
> > +cxl_dcd = find_program('cxl-dcd.sh')
> >  
> >  tests = [
> >    [ 'libndctl',               libndctl,		  'ndctl' ],
> > @@ -194,6 +195,7 @@ tests = [
> >    [ 'cxl-destroy-region.sh',  cxl_destroy_region, 'cxl'   ],
> >    [ 'cxl-qos-class.sh',       cxl_qos_class,      'cxl'   ],
> >    [ 'cxl-poison.sh',          cxl_poison,         'cxl'   ],
> > +  [ 'cxl-dcd.sh',             cxl_dcd,            'cxl'   ],
> >  ]
> >  
> >  if get_option('destructive').enabled()
> > 
>