Re: [PATCH ndctl 2/7] cxl: add a type attribute to region listings

From: Fan Ni <fan.ni@samsung.com>
To: Vishal Verma <vishal.l.verma@intel.com>
Cc: "linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
	Gregory Price <gregory.price@memverge.com>,
	Jonathan Cameron <Jonathan.Cameron@Huawei.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Dan Williams <dan.j.williams@intel.com>,
	"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Subject: Re: [PATCH ndctl 2/7] cxl: add a type attribute to region listings
Date: Fri, 10 Feb 2023 00:47:57 +0000	[thread overview]
Message-ID: <20230210004738.GA883957@bgt-140510-bm03> (raw)
In-Reply-To: <20230120-vv-volatile-regions-v1-2-b42b21ee8d0b@intel.com>

On Tue, Feb 07, 2023 at 12:16:28PM -0700, Vishal Verma wrote:
> In preparation for enumerating and creating 'volatile' or 'ram' type
> regions, add a 'type' attribute to region listings, so these can be
> distinguished from 'pmem' type regions easily. This depends on a new
> 'mode' attribute for region objects in sysfs. For older kernels that
> lack this, region listings will simply omit emitting this attribute,
> but otherwise not treat it as a failure.
> 
> Cc: 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>

> ---
>  Documentation/cxl/lib/libcxl.txt |  1 +
>  cxl/lib/private.h                |  1 +
>  cxl/lib/libcxl.c                 | 11 +++++++++++
>  cxl/libcxl.h                     |  1 +
>  cxl/json.c                       |  5 +++++
>  cxl/lib/libcxl.sym               |  5 +++++
>  6 files changed, 24 insertions(+)
> 
> diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
> index f9af376..dbc4b56 100644
> --- a/Documentation/cxl/lib/libcxl.txt
> +++ b/Documentation/cxl/lib/libcxl.txt
> @@ -550,6 +550,7 @@ int cxl_region_get_id(struct cxl_region *region);
>  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);
> +enum cxl_decoder_mode cxl_region_get_mode(struct cxl_region *region);
>  unsigned long long cxl_region_get_resource(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);
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index f8871bd..306dc3a 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -149,6 +149,7 @@ struct cxl_region {
>  	unsigned int interleave_ways;
>  	unsigned int interleave_granularity;
>  	enum cxl_decode_state decode_state;
> +	enum cxl_decoder_mode mode;
>  	struct kmod_module *module;
>  	struct list_head mappings;
>  };
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 4205a58..83f628b 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -561,6 +561,12 @@ static void *add_cxl_region(void *parent, int id, const char *cxlregion_base)
>  	else
>  		region->decode_state = strtoul(buf, NULL, 0);
>  
> +	sprintf(path, "%s/mode", cxlregion_base);
> +	if (sysfs_read_attr(ctx, path, buf) < 0)
> +		region->mode = CXL_DECODER_MODE_NONE;
> +	else
> +		region->mode = cxl_decoder_mode_from_ident(buf);
> +
>  	sprintf(path, "%s/modalias", cxlregion_base);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		region->module = util_modalias_to_module(ctx, buf);
> @@ -686,6 +692,11 @@ 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)
> +{
> +	return region->mode;
> +}
> +
>  CXL_EXPORT unsigned int
>  cxl_region_get_interleave_ways(struct cxl_region *region)
>  {
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index d699af8..e6cca11 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -273,6 +273,7 @@ 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_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);
>  struct cxl_decoder *cxl_region_get_target_decoder(struct cxl_region *region,
> diff --git a/cxl/json.c b/cxl/json.c
> index 0fc44e4..f625380 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -827,6 +827,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);
>  	const char *devname = cxl_region_get_devname(region);
>  	struct json_object *jregion, *jobj;
>  	u64 val;
> @@ -853,6 +854,10 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
>  			json_object_object_add(jregion, "size", jobj);
>  	}
>  
> +	jobj = json_object_new_string(cxl_decoder_mode_name(mode));
> +	if (jobj)
> +		json_object_object_add(jregion, "type", jobj);
> +
>  	val = cxl_region_get_interleave_ways(region);
>  	if (val < INT_MAX) {
>  		jobj = json_object_new_int(val);
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 6bc0810..9832d09 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -242,3 +242,8 @@ global:
>  	cxl_target_get_firmware_node;
>  	cxl_dport_get_firmware_node;
>  } LIBCXL_3;
> +
> +LIBCXL_5 {
> +global:
> +	cxl_region_get_mode;
> +} LIBCXL_4;
> 
> -- 
> 2.39.1
> 
>