diff mbox series

[ndctl,v2,3/7] cxl: add core plumbing for creation of ram regions

Message ID 20230120-vv-volatile-regions-v2-3-4ea6253000e5@intel.com
State Accepted
Commit aa8ae068752a9a3b01c012259b9210e14d7245a4
Headers show
Series cxl: add support for listing and creating volatile regions | expand

Commit Message

Verma, Vishal L Feb. 8, 2023, 8 p.m. UTC 
Add support in libcxl to create ram regions through a new
cxl_decoder_create_ram_region() API, which works similarly to its pmem
sibling.

Enable ram region creation in cxl-cli, with the only differences from
the pmem flow being:
  1/ Use the above create_ram_region API, and
  2/ Elide setting the UUID, since ram regions don't have one

Cc: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 Documentation/cxl/cxl-create-region.txt |  3 ++-
 cxl/lib/libcxl.c                        | 22 +++++++++++++++++++---
 cxl/libcxl.h                            |  1 +
 cxl/region.c                            | 28 ++++++++++++++++++++++++----
 cxl/lib/libcxl.sym                      |  1 +
 5 files changed, 47 insertions(+), 8 deletions(-)

Comments

Ira Weiny Feb. 9, 2023, 4:24 p.m. UTC | #1 
Vishal Verma wrote:
> Add support in libcxl to create ram regions through a new
> cxl_decoder_create_ram_region() API, which works similarly to its pmem
> sibling.
> 
> Enable ram region creation in cxl-cli, with the only differences from
> the pmem flow being:
>   1/ Use the above create_ram_region API, and
>   2/ Elide setting the UUID, since ram regions don't have one
> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  Documentation/cxl/cxl-create-region.txt |  3 ++-
>  cxl/lib/libcxl.c                        | 22 +++++++++++++++++++---
>  cxl/libcxl.h                            |  1 +
>  cxl/region.c                            | 28 ++++++++++++++++++++++++----
>  cxl/lib/libcxl.sym                      |  1 +
>  5 files changed, 47 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
> index 286779e..ada0e52 100644
> --- a/Documentation/cxl/cxl-create-region.txt
> +++ b/Documentation/cxl/cxl-create-region.txt
> @@ -80,7 +80,8 @@ include::bus-option.txt[]
>  -U::
>  --uuid=::
>  	Specify a UUID for the new region. This shouldn't usually need to be
> -	specified, as one will be generated by default.
> +	specified, as one will be generated by default. Only applicable to
> +	pmem regions.
>  
>  -w::
>  --ways=::
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 83f628b..c5b9b18 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -2234,8 +2234,8 @@ cxl_decoder_get_region(struct cxl_decoder *decoder)
>  	return NULL;
>  }
>  
> -CXL_EXPORT struct cxl_region *
> -cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
> +static struct cxl_region *cxl_decoder_create_region(struct cxl_decoder *decoder,
> +						    enum cxl_decoder_mode mode)
>  {
>  	struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
>  	char *path = decoder->dev_buf;
> @@ -2243,7 +2243,11 @@ cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
>  	struct cxl_region *region;
>  	int rc;
>  
> -	sprintf(path, "%s/create_pmem_region", decoder->dev_path);
> +	if (mode == CXL_DECODER_MODE_PMEM)
> +		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);
> +
>  	rc = sysfs_read_attr(ctx, path, buf);
>  	if (rc < 0) {
>  		err(ctx, "failed to read new region name: %s\n",
> @@ -2282,6 +2286,18 @@ cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
>  	return region;
>  }
>  
> +CXL_EXPORT struct cxl_region *
> +cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
> +{
> +	return cxl_decoder_create_region(decoder, CXL_DECODER_MODE_PMEM);
> +}
> +
> +CXL_EXPORT struct cxl_region *
> +cxl_decoder_create_ram_region(struct cxl_decoder *decoder)
> +{
> +	return cxl_decoder_create_region(decoder, CXL_DECODER_MODE_RAM);
> +}
> +
>  CXL_EXPORT int cxl_decoder_get_nr_targets(struct cxl_decoder *decoder)
>  {
>  	return decoder->nr_targets;
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index e6cca11..904156c 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -213,6 +213,7 @@ cxl_decoder_get_interleave_granularity(struct cxl_decoder *decoder);
>  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_decoder *cxl_decoder_get_by_name(struct cxl_ctx *ctx,
>  					    const char *ident);
>  struct cxl_memdev *cxl_decoder_get_memdev(struct cxl_decoder *decoder);
> diff --git a/cxl/region.c b/cxl/region.c
> index 38aa142..c69cb9a 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -380,7 +380,18 @@ static void collect_minsize(struct cxl_ctx *ctx, struct parsed_params *p)
>  		struct json_object *jobj =
>  			json_object_array_get_idx(p->memdevs, i);
>  		struct cxl_memdev *memdev = json_object_get_userdata(jobj);
> -		u64 size = cxl_memdev_get_pmem_size(memdev);
> +		u64 size = 0;
> +
> +		switch(p->mode) {
> +		case CXL_DECODER_MODE_RAM:
> +			size = cxl_memdev_get_ram_size(memdev);
> +			break;
> +		case CXL_DECODER_MODE_PMEM:
> +			size = cxl_memdev_get_pmem_size(memdev);
> +			break;
> +		default:
> +			/* Shouldn't ever get here */ ;
> +		}
>  
>  		if (!p->ep_min_size)
>  			p->ep_min_size = size;
> @@ -589,8 +600,15 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  				param.root_decoder);
>  			return -ENXIO;
>  		}
> +	} else if (p->mode == CXL_DECODER_MODE_RAM) {
> +		region = cxl_decoder_create_ram_region(p->root_decoder);
> +		if (!region) {
> +			log_err(&rl, "failed to create region under %s\n",
> +				param.root_decoder);
> +			return -ENXIO;
> +		}
>  	} else {
> -		log_err(&rl, "region type '%s' not supported yet\n",
> +		log_err(&rl, "region type '%s' is not supported\n",
>  			param.type);
>  		return -EOPNOTSUPP;
>  	}
> @@ -602,10 +620,12 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  		goto out;
>  	granularity = rc;
>  
> -	uuid_generate(uuid);
>  	try(cxl_region, set_interleave_granularity, region, granularity);
>  	try(cxl_region, set_interleave_ways, region, p->ways);
> -	try(cxl_region, set_uuid, region, uuid);
> +	if (p->mode == CXL_DECODER_MODE_PMEM) {
> +		uuid_generate(uuid);
> +		try(cxl_region, set_uuid, region, uuid);
> +	}
>  	try(cxl_region, set_size, region, size);
>  
>  	for (i = 0; i < p->ways; i++) {
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 9832d09..84f60ad 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -246,4 +246,5 @@ global:
>  LIBCXL_5 {
>  global:
>  	cxl_region_get_mode;
> +	cxl_decoder_create_ram_region;
>  } LIBCXL_4;
> 
> -- 
> 2.39.1
>
Fan Ni Feb. 10, 2023, 1:18 a.m. UTC | #2 
On Wed, Feb 08, 2023 at 01:00:31PM -0700, Vishal Verma wrote:

> Add support in libcxl to create ram regions through a new
> cxl_decoder_create_ram_region() API, which works similarly to its pmem
> sibling.
> 
> Enable ram region creation in cxl-cli, with the only differences from
> the pmem flow being:
>   1/ Use the above create_ram_region API, and
>   2/ Elide setting the UUID, since ram regions don't have one
> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

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

One minor thing, there exists some code format inconsistency in
cxl/region.c file (not introduced by the patch). For exmaple, the
"switch" sometimes is followed with a space but sometime not.


> ---
>  Documentation/cxl/cxl-create-region.txt |  3 ++-
>  cxl/lib/libcxl.c                        | 22 +++++++++++++++++++---
>  cxl/libcxl.h                            |  1 +
>  cxl/region.c                            | 28 ++++++++++++++++++++++++----
>  cxl/lib/libcxl.sym                      |  1 +
>  5 files changed, 47 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
> index 286779e..ada0e52 100644
> --- a/Documentation/cxl/cxl-create-region.txt
> +++ b/Documentation/cxl/cxl-create-region.txt
> @@ -80,7 +80,8 @@ include::bus-option.txt[]
>  -U::
>  --uuid=::
>  	Specify a UUID for the new region. This shouldn't usually need to be
> -	specified, as one will be generated by default.
> +	specified, as one will be generated by default. Only applicable to
> +	pmem regions.
>  
>  -w::
>  --ways=::
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 83f628b..c5b9b18 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -2234,8 +2234,8 @@ cxl_decoder_get_region(struct cxl_decoder *decoder)
>  	return NULL;
>  }
>  
> -CXL_EXPORT struct cxl_region *
> -cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
> +static struct cxl_region *cxl_decoder_create_region(struct cxl_decoder *decoder,
> +						    enum cxl_decoder_mode mode)
>  {
>  	struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
>  	char *path = decoder->dev_buf;
> @@ -2243,7 +2243,11 @@ cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
>  	struct cxl_region *region;
>  	int rc;
>  
> -	sprintf(path, "%s/create_pmem_region", decoder->dev_path);
> +	if (mode == CXL_DECODER_MODE_PMEM)
> +		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);
> +
>  	rc = sysfs_read_attr(ctx, path, buf);
>  	if (rc < 0) {
>  		err(ctx, "failed to read new region name: %s\n",
> @@ -2282,6 +2286,18 @@ cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
>  	return region;
>  }
>  
> +CXL_EXPORT struct cxl_region *
> +cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
> +{
> +	return cxl_decoder_create_region(decoder, CXL_DECODER_MODE_PMEM);
> +}
> +
> +CXL_EXPORT struct cxl_region *
> +cxl_decoder_create_ram_region(struct cxl_decoder *decoder)
> +{
> +	return cxl_decoder_create_region(decoder, CXL_DECODER_MODE_RAM);
> +}
> +
>  CXL_EXPORT int cxl_decoder_get_nr_targets(struct cxl_decoder *decoder)
>  {
>  	return decoder->nr_targets;
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index e6cca11..904156c 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -213,6 +213,7 @@ cxl_decoder_get_interleave_granularity(struct cxl_decoder *decoder);
>  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_decoder *cxl_decoder_get_by_name(struct cxl_ctx *ctx,
>  					    const char *ident);
>  struct cxl_memdev *cxl_decoder_get_memdev(struct cxl_decoder *decoder);
> diff --git a/cxl/region.c b/cxl/region.c
> index 38aa142..c69cb9a 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -380,7 +380,18 @@ static void collect_minsize(struct cxl_ctx *ctx, struct parsed_params *p)
>  		struct json_object *jobj =
>  			json_object_array_get_idx(p->memdevs, i);
>  		struct cxl_memdev *memdev = json_object_get_userdata(jobj);
> -		u64 size = cxl_memdev_get_pmem_size(memdev);
> +		u64 size = 0;
> +
> +		switch(p->mode) {
> +		case CXL_DECODER_MODE_RAM:
> +			size = cxl_memdev_get_ram_size(memdev);
> +			break;
> +		case CXL_DECODER_MODE_PMEM:
> +			size = cxl_memdev_get_pmem_size(memdev);
> +			break;
> +		default:
> +			/* Shouldn't ever get here */ ;
> +		}
>  
>  		if (!p->ep_min_size)
>  			p->ep_min_size = size;
> @@ -589,8 +600,15 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  				param.root_decoder);
>  			return -ENXIO;
>  		}
> +	} else if (p->mode == CXL_DECODER_MODE_RAM) {
> +		region = cxl_decoder_create_ram_region(p->root_decoder);
> +		if (!region) {
> +			log_err(&rl, "failed to create region under %s\n",
> +				param.root_decoder);
> +			return -ENXIO;
> +		}
>  	} else {
> -		log_err(&rl, "region type '%s' not supported yet\n",
> +		log_err(&rl, "region type '%s' is not supported\n",
>  			param.type);
>  		return -EOPNOTSUPP;
>  	}
> @@ -602,10 +620,12 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  		goto out;
>  	granularity = rc;
>  
> -	uuid_generate(uuid);
>  	try(cxl_region, set_interleave_granularity, region, granularity);
>  	try(cxl_region, set_interleave_ways, region, p->ways);
> -	try(cxl_region, set_uuid, region, uuid);
> +	if (p->mode == CXL_DECODER_MODE_PMEM) {
> +		uuid_generate(uuid);
> +		try(cxl_region, set_uuid, region, uuid);
> +	}
>  	try(cxl_region, set_size, region, size);
>  
>  	for (i = 0; i < p->ways; i++) {
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 9832d09..84f60ad 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -246,4 +246,5 @@ global:
>  LIBCXL_5 {
>  global:
>  	cxl_region_get_mode;
> +	cxl_decoder_create_ram_region;
>  } LIBCXL_4;
> 
> -- 
> 2.39.1
> 
>
diff mbox series

Patch

diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
index 286779e..ada0e52 100644
--- a/Documentation/cxl/cxl-create-region.txt
+++ b/Documentation/cxl/cxl-create-region.txt
@@ -80,7 +80,8 @@  include::bus-option.txt[]
 -U::
 --uuid=::
 	Specify a UUID for the new region. This shouldn't usually need to be
-	specified, as one will be generated by default.
+	specified, as one will be generated by default. Only applicable to
+	pmem regions.
 
 -w::
 --ways=::
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 83f628b..c5b9b18 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -2234,8 +2234,8 @@  cxl_decoder_get_region(struct cxl_decoder *decoder)
 	return NULL;
 }
 
-CXL_EXPORT struct cxl_region *
-cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
+static struct cxl_region *cxl_decoder_create_region(struct cxl_decoder *decoder,
+						    enum cxl_decoder_mode mode)
 {
 	struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
 	char *path = decoder->dev_buf;
@@ -2243,7 +2243,11 @@  cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
 	struct cxl_region *region;
 	int rc;
 
-	sprintf(path, "%s/create_pmem_region", decoder->dev_path);
+	if (mode == CXL_DECODER_MODE_PMEM)
+		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);
+
 	rc = sysfs_read_attr(ctx, path, buf);
 	if (rc < 0) {
 		err(ctx, "failed to read new region name: %s\n",
@@ -2282,6 +2286,18 @@  cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
 	return region;
 }
 
+CXL_EXPORT struct cxl_region *
+cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
+{
+	return cxl_decoder_create_region(decoder, CXL_DECODER_MODE_PMEM);
+}
+
+CXL_EXPORT struct cxl_region *
+cxl_decoder_create_ram_region(struct cxl_decoder *decoder)
+{
+	return cxl_decoder_create_region(decoder, CXL_DECODER_MODE_RAM);
+}
+
 CXL_EXPORT int cxl_decoder_get_nr_targets(struct cxl_decoder *decoder)
 {
 	return decoder->nr_targets;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index e6cca11..904156c 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -213,6 +213,7 @@  cxl_decoder_get_interleave_granularity(struct cxl_decoder *decoder);
 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_decoder *cxl_decoder_get_by_name(struct cxl_ctx *ctx,
 					    const char *ident);
 struct cxl_memdev *cxl_decoder_get_memdev(struct cxl_decoder *decoder);
diff --git a/cxl/region.c b/cxl/region.c
index 38aa142..c69cb9a 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -380,7 +380,18 @@  static void collect_minsize(struct cxl_ctx *ctx, struct parsed_params *p)
 		struct json_object *jobj =
 			json_object_array_get_idx(p->memdevs, i);
 		struct cxl_memdev *memdev = json_object_get_userdata(jobj);
-		u64 size = cxl_memdev_get_pmem_size(memdev);
+		u64 size = 0;
+
+		switch(p->mode) {
+		case CXL_DECODER_MODE_RAM:
+			size = cxl_memdev_get_ram_size(memdev);
+			break;
+		case CXL_DECODER_MODE_PMEM:
+			size = cxl_memdev_get_pmem_size(memdev);
+			break;
+		default:
+			/* Shouldn't ever get here */ ;
+		}
 
 		if (!p->ep_min_size)
 			p->ep_min_size = size;
@@ -589,8 +600,15 @@  static int create_region(struct cxl_ctx *ctx, int *count,
 				param.root_decoder);
 			return -ENXIO;
 		}
+	} else if (p->mode == CXL_DECODER_MODE_RAM) {
+		region = cxl_decoder_create_ram_region(p->root_decoder);
+		if (!region) {
+			log_err(&rl, "failed to create region under %s\n",
+				param.root_decoder);
+			return -ENXIO;
+		}
 	} else {
-		log_err(&rl, "region type '%s' not supported yet\n",
+		log_err(&rl, "region type '%s' is not supported\n",
 			param.type);
 		return -EOPNOTSUPP;
 	}
@@ -602,10 +620,12 @@  static int create_region(struct cxl_ctx *ctx, int *count,
 		goto out;
 	granularity = rc;
 
-	uuid_generate(uuid);
 	try(cxl_region, set_interleave_granularity, region, granularity);
 	try(cxl_region, set_interleave_ways, region, p->ways);
-	try(cxl_region, set_uuid, region, uuid);
+	if (p->mode == CXL_DECODER_MODE_PMEM) {
+		uuid_generate(uuid);
+		try(cxl_region, set_uuid, region, uuid);
+	}
 	try(cxl_region, set_size, region, size);
 
 	for (i = 0; i < p->ways; i++) {
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 9832d09..84f60ad 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -246,4 +246,5 @@  global:
 LIBCXL_5 {
 global:
 	cxl_region_get_mode;
+	cxl_decoder_create_ram_region;
 } LIBCXL_4;