[v4,13/28] cxl/mem: Expose DCD partition capabilities in sysfs

Commit Message

Ira Weiny Oct. 7, 2024, 11:16 p.m. UTC

From: Navneet Singh <[e-mail]>

To properly configure CXL regions on Dynamic Capacity Devices (DCD),
user space will need to know the details of the DC partitions available.

Expose dynamic capacity capabilities through sysfs.

Signed-off-by: Navneet Singh <[e-mail]>
Co-developed-by: Ira Weiny <[e-mail]>
Signed-off-by: Ira Weiny <[e-mail]>

---
Changes:
[iweiny: Change .../memX/dc/* to .../memX/dcY/*]
[iweiny: add read only and shareable attributes from DSMAS]
[djiang: Split sysfs docs]
[iweiny: Adjust sysfs doc dates]
[iweiny: Add qos details]
---
 Documentation/ABI/testing/sysfs-bus-cxl |  45 ++++++++++++
 drivers/cxl/core/memdev.c               | 126 ++++++++++++++++++++++++++++++++
 2 files changed, 171 insertions(+)

Comments

Fan Ni Oct. 9, 2024, 8:46 p.m. UTC | #1

On Mon, Oct 07, 2024 at 06:16:19PM -0500, [e-mail] wrote:
> From: Navneet Singh <[e-mail]>
> 
> To properly configure CXL regions on Dynamic Capacity Devices (DCD),
> user space will need to know the details of the DC partitions available.
> 
> Expose dynamic capacity capabilities through sysfs.
> 
> Signed-off-by: Navneet Singh <[e-mail]>
> Co-developed-by: Ira Weiny <[e-mail]>
> Signed-off-by: Ira Weiny <[e-mail]>
> 
> ---
> Changes:
> [iweiny: Change .../memX/dc/* to .../memX/dcY/*]
> [iweiny: add read only and shareable attributes from DSMAS]
> [djiang: Split sysfs docs]
> [iweiny: Adjust sysfs doc dates]
> [iweiny: Add qos details]
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl |  45 ++++++++++++
>  drivers/cxl/core/memdev.c               | 126 ++++++++++++++++++++++++++++++++
>  2 files changed, 171 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 3f5627a1210a..b865eefdb74c 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -54,6 +54,51 @@ Description:
>  		identically named field in the Identify Memory Device Output
>  		Payload in the CXL-2.0 specification.
>  
> +What:		/sys/bus/cxl/devices/memX/dcY/size
> +Date:		December, 2024
> +KernelVersion:	v6.13
> +Contact:	[e-mail]
> +Description:
> +		(RO) Dynamic Capacity (DC) region information.  Devices only
> +		export dcY if DCD partition Y is supported.
> +		dcY/size is the size of each of those partitions.
> +
> +What:		/sys/bus/cxl/devices/memX/dcY/read_only
> +Date:		December, 2024
> +KernelVersion:	v6.13
> +Contact:	[e-mail]
> +Description:
> +		(RO) Dynamic Capacity (DC) region information.  Devices only
> +		export dcY if DCD partition Y is supported.
> +		dcY/read_only indicates true if the region is exported
> +		read_only from the device.
> +
> +What:		/sys/bus/cxl/devices/memX/dcY/shareable
> +Date:		December, 2024
> +KernelVersion:	v6.13
> +Contact:	[e-mail]
> +Description:
> +		(RO) Dynamic Capacity (DC) region information.  Devices only
> +		export dcY if DCD partition Y is supported.
> +		dcY/shareable indicates true if the region is exported
> +		shareable from the device.
> +
> +What:		/sys/bus/cxl/devices/memX/dcY/qos_class
> +Date:		December, 2024
> +KernelVersion:	v6.13
> +Contact:	[e-mail]
> +Description:
> +		(RO) Dynamic Capacity (DC) region information.  Devices only
> +		export dcY if DCD partition Y is supported.  For CXL host
> +		platforms that support "QoS Telemmetry" this attribute conveys
> +		a comma delimited list of platform specific cookies that
> +		identifies a QoS performance class for the persistent partition
> +		of the CXL mem device. These class-ids can be compared against
> +		a similar "qos_class" published for a root decoder. While it is
> +		not required that the endpoints map their local memory-class to
> +		a matching platform class, mismatches are not recommended and
> +		there are platform specific performance related side-effects
> +		that may result. First class-id is displayed.
>  
>  What:		/sys/bus/cxl/devices/memX/pmem/qos_class
>  Date:		May, 2023
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 84fefb76dafa..2565b10a769c 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -2,6 +2,7 @@
>  /* Copyright(c) 2020 Intel Corporation. */
>  
>  #include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/string_choices.h>
>  #include <linux/firmware.h>
>  #include <linux/device.h>
>  #include <linux/slab.h>
> @@ -449,6 +450,123 @@ static struct attribute *cxl_memdev_security_attributes[] = {
>  	NULL,
>  };
>  
> +static ssize_t show_size_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
> +{
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> +
> +	return sysfs_emit(buf, "%#llx\n", mds->dc_region[pos].decode_len);
> +}
> +
> +static ssize_t show_read_only_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
> +{
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> +
> +	return sysfs_emit(buf, "%s\n",
> +			  str_false_true(mds->dc_region[pos].read_only));

For this function and below, why str_false_true instead of
str_true_false??

Fan
> +}
> +
> +static ssize_t show_shareable_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
> +{
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> +
> +	return sysfs_emit(buf, "%s\n",
> +			  str_false_true(mds->dc_region[pos].shareable));
> +}
> +
> +static ssize_t show_qos_class_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
> +{
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> +
> +	return sysfs_emit(buf, "%d\n", mds->dc_perf[pos].qos_class);
> +}
> +
> +#define CXL_MEMDEV_DC_ATTR_GROUP(n)						\
> +static ssize_t dc##n##_size_show(struct device *dev,				\
> +				 struct device_attribute *attr,			\
> +				 char *buf)					\
> +{										\
> +	return show_size_dcN(to_cxl_memdev(dev), buf, (n));			\
> +}										\
> +struct device_attribute dc##n##_size = {					\
> +	.attr	= { .name = "size", .mode = 0444 },				\
> +	.show	= dc##n##_size_show,						\
> +};										\
> +static ssize_t dc##n##_read_only_show(struct device *dev,			\
> +				      struct device_attribute *attr,		\
> +				      char *buf)				\
> +{										\
> +	return show_read_only_dcN(to_cxl_memdev(dev), buf, (n));		\
> +}										\
> +struct device_attribute dc##n##_read_only = {					\
> +	.attr	= { .name = "read_only", .mode = 0444 },			\
> +	.show	= dc##n##_read_only_show,					\
> +};										\
> +static ssize_t dc##n##_shareable_show(struct device *dev,			\
> +				     struct device_attribute *attr,		\
> +				     char *buf)					\
> +{										\
> +	return show_shareable_dcN(to_cxl_memdev(dev), buf, (n));		\
> +}										\
> +struct device_attribute dc##n##_shareable = {					\
> +	.attr	= { .name = "shareable", .mode = 0444 },			\
> +	.show	= dc##n##_shareable_show,					\
> +};										\
> +static ssize_t dc##n##_qos_class_show(struct device *dev,			\
> +				      struct device_attribute *attr,		\
> +				      char *buf)				\
> +{										\
> +	return show_qos_class_dcN(to_cxl_memdev(dev), buf, (n));		\
> +}										\
> +struct device_attribute dc##n##_qos_class = {					\
> +	.attr	= { .name = "qos_class", .mode = 0444 },			\
> +	.show	= dc##n##_qos_class_show,					\
> +};										\
> +static struct attribute *cxl_memdev_dc##n##_attributes[] = {			\
> +	&dc##n##_size.attr,							\
> +	&dc##n##_read_only.attr,						\
> +	&dc##n##_shareable.attr,						\
> +	&dc##n##_qos_class.attr,						\
> +	NULL,									\
> +};										\
> +static umode_t cxl_memdev_dc##n##_attr_visible(struct kobject *kobj,		\
> +					       struct attribute *a,		\
> +					       int pos)				\
> +{										\
> +	struct device *dev = kobj_to_dev(kobj);					\
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);				\
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);	\
> +										\
> +	/* Not a memory device */						\
> +	if (!mds)								\
> +		return 0;							\
> +	return a->mode;								\
> +}										\
> +static umode_t cxl_memdev_dc##n##_group_visible(struct kobject *kobj)		\
> +{										\
> +	struct device *dev = kobj_to_dev(kobj);					\
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);				\
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);	\
> +										\
> +	/* Not a memory device or partition not supported */			\
> +	if (!mds || n >= mds->nr_dc_region)					\
> +		return false;							\
> +	return true;								\
> +}										\
> +DEFINE_SYSFS_GROUP_VISIBLE(cxl_memdev_dc##n);					\
> +static struct attribute_group cxl_memdev_dc##n##_group = {			\
> +	.name = "dc"#n,								\
> +	.attrs = cxl_memdev_dc##n##_attributes,					\
> +	.is_visible = SYSFS_GROUP_VISIBLE(cxl_memdev_dc##n),			\
> +}
> +CXL_MEMDEV_DC_ATTR_GROUP(0);
> +CXL_MEMDEV_DC_ATTR_GROUP(1);
> +CXL_MEMDEV_DC_ATTR_GROUP(2);
> +CXL_MEMDEV_DC_ATTR_GROUP(3);
> +CXL_MEMDEV_DC_ATTR_GROUP(4);
> +CXL_MEMDEV_DC_ATTR_GROUP(5);
> +CXL_MEMDEV_DC_ATTR_GROUP(6);
> +CXL_MEMDEV_DC_ATTR_GROUP(7);
> +
>  static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
>  				  int n)
>  {
> @@ -525,6 +643,14 @@ static struct attribute_group cxl_memdev_security_attribute_group = {
>  };
>  
>  static const struct attribute_group *cxl_memdev_attribute_groups[] = {
> +	&cxl_memdev_dc0_group,
> +	&cxl_memdev_dc1_group,
> +	&cxl_memdev_dc2_group,
> +	&cxl_memdev_dc3_group,
> +	&cxl_memdev_dc4_group,
> +	&cxl_memdev_dc5_group,
> +	&cxl_memdev_dc6_group,
> +	&cxl_memdev_dc7_group,
>  	&cxl_memdev_attribute_group,
>  	&cxl_memdev_ram_attribute_group,
>  	&cxl_memdev_pmem_attribute_group,
> 
> -- 
> 2.46.0
>

Jonathan Cameron Oct. 10, 2024, 1:04 p.m. UTC | #2

On Mon, 07 Oct 2024 18:16:19 -0500
[e-mail] wrote:

> From: Navneet Singh <[e-mail]>
> 
> To properly configure CXL regions on Dynamic Capacity Devices (DCD),
> user space will need to know the details of the DC partitions available.
> 
> Expose dynamic capacity capabilities through sysfs.
> 
> Signed-off-by: Navneet Singh <[e-mail]>
> Co-developed-by: Ira Weiny <[e-mail]>
> Signed-off-by: Ira Weiny <[e-mail]>
Some trivial stuff inline that I'm not that bothered about either way.

Subject to answering Fan's query
Reviewed-by: Jonathan Cameron <[e-mail]>

> 
> ---
> Changes:
> [iweiny: Change .../memX/dc/* to .../memX/dcY/*]
> [iweiny: add read only and shareable attributes from DSMAS]
> [djiang: Split sysfs docs]
> [iweiny: Adjust sysfs doc dates]
> [iweiny: Add qos details]
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl |  45 ++++++++++++
>  drivers/cxl/core/memdev.c               | 126 ++++++++++++++++++++++++++++++++
>  2 files changed, 171 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 3f5627a1210a..b865eefdb74c 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -54,6 +54,51 @@ Description:
>  		identically named field in the Identify Memory Device Output
>  		Payload in the CXL-2.0 specification.
>  
> +What:		/sys/bus/cxl/devices/memX/dcY/size
> +Date:		December, 2024
> +KernelVersion:	v6.13
> +Contact:	[e-mail]
> +Description:
> +		(RO) Dynamic Capacity (DC) region information.  Devices only
> +		export dcY if DCD partition Y is supported.
> +		dcY/size is the size of each of those partitions.
> +
> +What:		/sys/bus/cxl/devices/memX/dcY/read_only
> +Date:		December, 2024
> +KernelVersion:	v6.13
> +Contact:	[e-mail]
> +Description:
> +		(RO) Dynamic Capacity (DC) region information.  Devices only
> +		export dcY if DCD partition Y is supported.
> +		dcY/read_only indicates true if the region is exported
> +		read_only from the device.
> +
> +What:		/sys/bus/cxl/devices/memX/dcY/shareable
> +Date:		December, 2024
> +KernelVersion:	v6.13
> +Contact:	[e-mail]
> +Description:
> +		(RO) Dynamic Capacity (DC) region information.  Devices only
> +		export dcY if DCD partition Y is supported.
> +		dcY/shareable indicates true if the region is exported
> +		shareable from the device.
> +
> +What:		/sys/bus/cxl/devices/memX/dcY/qos_class
> +Date:		December, 2024
> +KernelVersion:	v6.13
> +Contact:	[e-mail]
> +Description:
> +		(RO) Dynamic Capacity (DC) region information.  Devices only
> +		export dcY if DCD partition Y is supported.  

You can document sysfs directories I think, e.g.
https://elixir.bootlin.com/linux/v6.12-rc2/source/Documentation/ABI/stable/sysfs-devices-node#L32
so maybe

What:			/sys/bus/cxl/device/memX/dcY
Date:		December, 2024
KernelVersion:	v6.13
Contact:	[e-mail]
Description: 
		Directory containing Dynamic Capacity (DC) region information.
                Devices only export dcY if DCD partition Y is supported.

What:		/sys/bus/cxl/devices/memX/dcY/qos_class
Date:		December, 2024
KernelVersion:	v6.13
Contact:	[e-mail]
Description:
		For CXL host...

To avoid the repetition of first bit of docs?

> +		platforms that support "QoS Telemmetry" this attribute conveys
> +		a comma delimited list of platform specific cookies that
> +		identifies a QoS performance class for the persistent partition
> +		of the CXL mem device. These class-ids can be compared against
> +		a similar "qos_class" published for a root decoder. While it is
> +		not required that the endpoints map their local memory-class to
> +		a matching platform class, mismatches are not recommended and
> +		there are platform specific performance related side-effects
> +		that may result. First class-id is displayed.
>  
>  What:		/sys/bus/cxl/devices/memX/pmem/qos_class
>  Date:		May, 2023


> +static ssize_t show_shareable_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
> +{
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> +
> +	return sysfs_emit(buf, "%s\n",
> +			  str_false_true(mds->dc_region[pos].shareable));

Fan has already raised that these seem backwards.

> +}
> +
> +static ssize_t show_qos_class_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
> +{
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> +
> +	return sysfs_emit(buf, "%d\n", mds->dc_perf[pos].qos_class);
> +}
> +
> +#define CXL_MEMDEV_DC_ATTR_GROUP(n)						\
> +static ssize_t dc##n##_size_show(struct device *dev,				\
> +				 struct device_attribute *attr,			\
> +				 char *buf)					\
> +{										\
> +	return show_size_dcN(to_cxl_memdev(dev), buf, (n));			\
> +}										\
> +struct device_attribute dc##n##_size = {					\
> +	.attr	= { .name = "size", .mode = 0444 },				\
> +	.show	= dc##n##_size_show,						\
> +};										\
> +static ssize_t dc##n##_read_only_show(struct device *dev,			\
> +				      struct device_attribute *attr,		\
> +				      char *buf)				\
> +{										\
> +	return show_read_only_dcN(to_cxl_memdev(dev), buf, (n));		\
> +}										\
> +struct device_attribute dc##n##_read_only = {					\
> +	.attr	= { .name = "read_only", .mode = 0444 },			\
> +	.show	= dc##n##_read_only_show,					\
> +};										\
> +static ssize_t dc##n##_shareable_show(struct device *dev,			\
> +				     struct device_attribute *attr,		\
> +				     char *buf)					\
> +{										\
> +	return show_shareable_dcN(to_cxl_memdev(dev), buf, (n));		\
> +}										\
> +struct device_attribute dc##n##_shareable = {					\
> +	.attr	= { .name = "shareable", .mode = 0444 },			\
> +	.show	= dc##n##_shareable_show,					\
> +};										\
> +static ssize_t dc##n##_qos_class_show(struct device *dev,			\
> +				      struct device_attribute *attr,		\
> +				      char *buf)				\
> +{										\
> +	return show_qos_class_dcN(to_cxl_memdev(dev), buf, (n));		\
> +}										\
> +struct device_attribute dc##n##_qos_class = {					\
> +	.attr	= { .name = "qos_class", .mode = 0444 },			\
> +	.show	= dc##n##_qos_class_show,					\
> +};										\
> +static struct attribute *cxl_memdev_dc##n##_attributes[] = {			\
> +	&dc##n##_size.attr,							\
> +	&dc##n##_read_only.attr,						\
> +	&dc##n##_shareable.attr,						\
> +	&dc##n##_qos_class.attr,						\
> +	NULL,									\

No comma needed on terminator.

> +};										\
> +static umode_t cxl_memdev_dc##n##_attr_visible(struct kobject *kobj,		\
> +					       struct attribute *a,		\
> +					       int pos)				\
> +{										\
> +	struct device *dev = kobj_to_dev(kobj);					\
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);				\
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);	\
> +										\
> +	/* Not a memory device */						\
> +	if (!mds)								\
	if (!to_cxl_memdev_state(cxlmd->cxlds))
		return 0;

I dislike long macros so if we can shave them down that is always good!

We do have precedence in hdm.c for just checking the type directly so maybe
	if (cxlmd->cxlds->type != CXL_DEVTYPE_CLASSMEM)

but the above is also fine as compiler should be able to figure out it
doesn't need to do the second half of the inline.


> +		return 0;							\
> +	return a->mode;								\
> +}										\
> +static umode_t cxl_memdev_dc##n##_group_visible(struct kobject *kobj)		\
> +{										\
> +	struct device *dev = kobj_to_dev(kobj);					\
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);				\
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);	\
> +										\
> +	/* Not a memory device or partition not supported */			\
> +	if (!mds || n >= mds->nr_dc_region)					\
> +		return false;							\
> +	return true;								\

	/* Memory device and partition is supported */
	return mds && n < mds->nr_dc_region;

> +}										\
>

Bagas Sanjaya Oct. 11, 2024, 2:15 a.m. UTC | #3

On Mon, Oct 07, 2024 at 06:16:19PM -0500, [e-mail] wrote:
> +What:		/sys/bus/cxl/devices/memX/dcY/qos_class
> +Date:		December, 2024
> +KernelVersion:	v6.13
> +Contact:	[e-mail]
> +Description:
> +		(RO) Dynamic Capacity (DC) region information.  Devices only
> +		export dcY if DCD partition Y is supported.  For CXL host
> +		platforms that support "QoS Telemmetry" this attribute conveys
> +		a comma delimited list of platform specific cookies that
> +		identifies a QoS performance class for the persistent partition
> +		of the CXL mem device. These class-ids can be compared against
> +		a similar "qos_class" published for a root decoder. While it is
> +		not required that the endpoints map their local memory-class to
> +		a matching platform class, mismatches are not recommended and
> +		there are platform specific performance related side-effects
"... mismatches are not recommended as there are ..."
> +		that may result. First class-id is displayed.
>  

Thanks.

Ira Weiny Oct. 14, 2024, 1:34 a.m. UTC | #4

Fan Ni wrote:
> On Mon, Oct 07, 2024 at 06:16:19PM -0500, [e-mail] wrote:
> > From: Navneet Singh <[e-mail]>
> > 

[snip]

> > +
> > +static ssize_t show_read_only_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
> > +{
> > +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> > +
> > +	return sysfs_emit(buf, "%s\n",
> > +			  str_false_true(mds->dc_region[pos].read_only));
> 
> For this function and below, why str_false_true instead of
> str_true_false??
> 

Oh!  I did not realize they were not the same.  That API is tricky.

Yea str_true_false() is the correct call.

Thanks for noticing that.
Ira

Ira Weiny Oct. 16, 2024, 9:34 p.m. UTC | #5

Jonathan Cameron wrote:
> On Mon, 07 Oct 2024 18:16:19 -0500
> [e-mail] wrote:
> 
> > From: Navneet Singh <[e-mail]>
> > 
> > To properly configure CXL regions on Dynamic Capacity Devices (DCD),
> > user space will need to know the details of the DC partitions available.
> > 
> > Expose dynamic capacity capabilities through sysfs.
> > 
> > Signed-off-by: Navneet Singh <[e-mail]>
> > Co-developed-by: Ira Weiny <[e-mail]>
> > Signed-off-by: Ira Weiny <[e-mail]>
> Some trivial stuff inline that I'm not that bothered about either way.
> 
> Subject to answering Fan's query
> Reviewed-by: Jonathan Cameron <[e-mail]>
> 

[snip]

> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> > index 3f5627a1210a..b865eefdb74c 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-cxl
> > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > @@ -54,6 +54,51 @@ Description:
> >  		identically named field in the Identify Memory Device Output
> >  		Payload in the CXL-2.0 specification.
> >  
> > +What:		/sys/bus/cxl/devices/memX/dcY/size
> > +Date:		December, 2024
> > +KernelVersion:	v6.13
> > +Contact:	[e-mail]
> > +Description:
> > +		(RO) Dynamic Capacity (DC) region information.  Devices only
> > +		export dcY if DCD partition Y is supported.
> > +		dcY/size is the size of each of those partitions.
> > +
> > +What:		/sys/bus/cxl/devices/memX/dcY/read_only
> > +Date:		December, 2024
> > +KernelVersion:	v6.13
> > +Contact:	[e-mail]
> > +Description:
> > +		(RO) Dynamic Capacity (DC) region information.  Devices only
> > +		export dcY if DCD partition Y is supported.
> > +		dcY/read_only indicates true if the region is exported
> > +		read_only from the device.
> > +
> > +What:		/sys/bus/cxl/devices/memX/dcY/shareable
> > +Date:		December, 2024
> > +KernelVersion:	v6.13
> > +Contact:	[e-mail]
> > +Description:
> > +		(RO) Dynamic Capacity (DC) region information.  Devices only
> > +		export dcY if DCD partition Y is supported.
> > +		dcY/shareable indicates true if the region is exported
> > +		shareable from the device.
> > +
> > +What:		/sys/bus/cxl/devices/memX/dcY/qos_class
> > +Date:		December, 2024
> > +KernelVersion:	v6.13
> > +Contact:	[e-mail]
> > +Description:
> > +		(RO) Dynamic Capacity (DC) region information.  Devices only
> > +		export dcY if DCD partition Y is supported.  
> 
> You can document sysfs directories I think, e.g.
> https://elixir.bootlin.com/linux/v6.12-rc2/source/Documentation/ABI/stable/sysfs-devices-node#L32
> so maybe
> 
> What:			/sys/bus/cxl/device/memX/dcY
> Date:		December, 2024
> KernelVersion:	v6.13
> Contact:	[e-mail]
> Description: 
> 		Directory containing Dynamic Capacity (DC) region information.
>                 Devices only export dcY if DCD partition Y is supported.
> 
> What:		/sys/bus/cxl/devices/memX/dcY/qos_class
> Date:		December, 2024
> KernelVersion:	v6.13
> Contact:	[e-mail]
> Description:
> 		For CXL host...
> 
> To avoid the repetition of first bit of docs?

The other docs don't do this.  For example:

	/sys/bus/cxl/devices/memX
	/sys/bus/cxl/devices/memX/ram
	/sys/bus/cxl/devices/memX/pmem

Are not documented like that.  I'm inclined to leave it.

> 
> > +		platforms that support "QoS Telemmetry" this attribute conveys
> > +		a comma delimited list of platform specific cookies that
> > +		identifies a QoS performance class for the persistent partition
> > +		of the CXL mem device. These class-ids can be compared against
> > +		a similar "qos_class" published for a root decoder. While it is
> > +		not required that the endpoints map their local memory-class to
> > +		a matching platform class, mismatches are not recommended and
> > +		there are platform specific performance related side-effects
> > +		that may result. First class-id is displayed.
> >  
> >  What:		/sys/bus/cxl/devices/memX/pmem/qos_class
> >  Date:		May, 2023
> 
> 
> > +static ssize_t show_shareable_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
> > +{
> > +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> > +
> > +	return sysfs_emit(buf, "%s\n",
> > +			  str_false_true(mds->dc_region[pos].shareable));
> 
> Fan has already raised that these seem backwards.

Yep fixed.


[snip]

> > +static struct attribute *cxl_memdev_dc##n##_attributes[] = {			\
> > +	&dc##n##_size.attr,							\
> > +	&dc##n##_read_only.attr,						\
> > +	&dc##n##_shareable.attr,						\
> > +	&dc##n##_qos_class.attr,						\
> > +	NULL,									\
> 
> No comma needed on terminator.

Fixed.

> 
> > +};										\
> > +static umode_t cxl_memdev_dc##n##_attr_visible(struct kobject *kobj,		\
> > +					       struct attribute *a,		\
> > +					       int pos)				\
> > +{										\
> > +	struct device *dev = kobj_to_dev(kobj);					\
> > +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);				\
> > +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);	\
> > +										\
> > +	/* Not a memory device */						\
> > +	if (!mds)								\
> 	if (!to_cxl_memdev_state(cxlmd->cxlds))
> 		return 0;
> 
> I dislike long macros so if we can shave them down that is always good!

Agreed but this was the most straight forward way to deal with this.  I
could perhaps break it up by having a 'master macro' which is made of
smaller macros...  But this works.

> 
> We do have precedence in hdm.c

Not in hdm.c directly but all the 'to_XXX()' calls have a type check.  So
it is modeled that way and is called from other places.

>
> for just checking the type directly so maybe
> 	if (cxlmd->cxlds->type != CXL_DEVTYPE_CLASSMEM)
> 
> but the above is also fine as compiler should be able to figure out it
> doesn't need to do the second half of the inline.

I'm going to leave it.

> 
> 
> > +		return 0;							\
> > +	return a->mode;								\
> > +}										\
> > +static umode_t cxl_memdev_dc##n##_group_visible(struct kobject *kobj)		\
> > +{										\
> > +	struct device *dev = kobj_to_dev(kobj);					\
> > +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);				\
> > +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);	\
> > +										\
> > +	/* Not a memory device or partition not supported */			\
> > +	if (!mds || n >= mds->nr_dc_region)					\
> > +		return false;							\
> > +	return true;								\
> 
> 	/* Memory device and partition is supported */
> 	return mds && n < mds->nr_dc_region;

Done.
Ira

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 3f5627a1210a..b865eefdb74c 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -54,6 +54,51 @@  Description:
 		identically named field in the Identify Memory Device Output
 		Payload in the CXL-2.0 specification.
 
+What:		/sys/bus/cxl/devices/memX/dcY/size
+Date:		December, 2024
+KernelVersion:	v6.13
+Contact:	[e-mail]
+Description:
+		(RO) Dynamic Capacity (DC) region information.  Devices only
+		export dcY if DCD partition Y is supported.
+		dcY/size is the size of each of those partitions.
+
+What:		/sys/bus/cxl/devices/memX/dcY/read_only
+Date:		December, 2024
+KernelVersion:	v6.13
+Contact:	[e-mail]
+Description:
+		(RO) Dynamic Capacity (DC) region information.  Devices only
+		export dcY if DCD partition Y is supported.
+		dcY/read_only indicates true if the region is exported
+		read_only from the device.
+
+What:		/sys/bus/cxl/devices/memX/dcY/shareable
+Date:		December, 2024
+KernelVersion:	v6.13
+Contact:	[e-mail]
+Description:
+		(RO) Dynamic Capacity (DC) region information.  Devices only
+		export dcY if DCD partition Y is supported.
+		dcY/shareable indicates true if the region is exported
+		shareable from the device.
+
+What:		/sys/bus/cxl/devices/memX/dcY/qos_class
+Date:		December, 2024
+KernelVersion:	v6.13
+Contact:	[e-mail]
+Description:
+		(RO) Dynamic Capacity (DC) region information.  Devices only
+		export dcY if DCD partition Y is supported.  For CXL host
+		platforms that support "QoS Telemmetry" this attribute conveys
+		a comma delimited list of platform specific cookies that
+		identifies a QoS performance class for the persistent partition
+		of the CXL mem device. These class-ids can be compared against
+		a similar "qos_class" published for a root decoder. While it is
+		not required that the endpoints map their local memory-class to
+		a matching platform class, mismatches are not recommended and
+		there are platform specific performance related side-effects
+		that may result. First class-id is displayed.
 
 What:		/sys/bus/cxl/devices/memX/pmem/qos_class
 Date:		May, 2023
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 84fefb76dafa..2565b10a769c 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -2,6 +2,7 @@ 
 /* Copyright(c) 2020 Intel Corporation. */
 
 #include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/string_choices.h>
 #include <linux/firmware.h>
 #include <linux/device.h>
 #include <linux/slab.h>
@@ -449,6 +450,123 @@  static struct attribute *cxl_memdev_security_attributes[] = {
 	NULL,
 };
 
+static ssize_t show_size_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
+{
+	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
+
+	return sysfs_emit(buf, "%#llx\n", mds->dc_region[pos].decode_len);
+}
+
+static ssize_t show_read_only_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
+{
+	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
+
+	return sysfs_emit(buf, "%s\n",
+			  str_false_true(mds->dc_region[pos].read_only));
+}
+
+static ssize_t show_shareable_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
+{
+	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
+
+	return sysfs_emit(buf, "%s\n",
+			  str_false_true(mds->dc_region[pos].shareable));
+}
+
+static ssize_t show_qos_class_dcN(struct cxl_memdev *cxlmd, char *buf, int pos)
+{
+	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
+
+	return sysfs_emit(buf, "%d\n", mds->dc_perf[pos].qos_class);
+}
+
+#define CXL_MEMDEV_DC_ATTR_GROUP(n)						\
+static ssize_t dc##n##_size_show(struct device *dev,				\
+				 struct device_attribute *attr,			\
+				 char *buf)					\
+{										\
+	return show_size_dcN(to_cxl_memdev(dev), buf, (n));			\
+}										\
+struct device_attribute dc##n##_size = {					\
+	.attr	= { .name = "size", .mode = 0444 },				\
+	.show	= dc##n##_size_show,						\
+};										\
+static ssize_t dc##n##_read_only_show(struct device *dev,			\
+				      struct device_attribute *attr,		\
+				      char *buf)				\
+{										\
+	return show_read_only_dcN(to_cxl_memdev(dev), buf, (n));		\
+}										\
+struct device_attribute dc##n##_read_only = {					\
+	.attr	= { .name = "read_only", .mode = 0444 },			\
+	.show	= dc##n##_read_only_show,					\
+};										\
+static ssize_t dc##n##_shareable_show(struct device *dev,			\
+				     struct device_attribute *attr,		\
+				     char *buf)					\
+{										\
+	return show_shareable_dcN(to_cxl_memdev(dev), buf, (n));		\
+}										\
+struct device_attribute dc##n##_shareable = {					\
+	.attr	= { .name = "shareable", .mode = 0444 },			\
+	.show	= dc##n##_shareable_show,					\
+};										\
+static ssize_t dc##n##_qos_class_show(struct device *dev,			\
+				      struct device_attribute *attr,		\
+				      char *buf)				\
+{										\
+	return show_qos_class_dcN(to_cxl_memdev(dev), buf, (n));		\
+}										\
+struct device_attribute dc##n##_qos_class = {					\
+	.attr	= { .name = "qos_class", .mode = 0444 },			\
+	.show	= dc##n##_qos_class_show,					\
+};										\
+static struct attribute *cxl_memdev_dc##n##_attributes[] = {			\
+	&dc##n##_size.attr,							\
+	&dc##n##_read_only.attr,						\
+	&dc##n##_shareable.attr,						\
+	&dc##n##_qos_class.attr,						\
+	NULL,									\
+};										\
+static umode_t cxl_memdev_dc##n##_attr_visible(struct kobject *kobj,		\
+					       struct attribute *a,		\
+					       int pos)				\
+{										\
+	struct device *dev = kobj_to_dev(kobj);					\
+	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);				\
+	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);	\
+										\
+	/* Not a memory device */						\
+	if (!mds)								\
+		return 0;							\
+	return a->mode;								\
+}										\
+static umode_t cxl_memdev_dc##n##_group_visible(struct kobject *kobj)		\
+{										\
+	struct device *dev = kobj_to_dev(kobj);					\
+	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);				\
+	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);	\
+										\
+	/* Not a memory device or partition not supported */			\
+	if (!mds || n >= mds->nr_dc_region)					\
+		return false;							\
+	return true;								\
+}										\
+DEFINE_SYSFS_GROUP_VISIBLE(cxl_memdev_dc##n);					\
+static struct attribute_group cxl_memdev_dc##n##_group = {			\
+	.name = "dc"#n,								\
+	.attrs = cxl_memdev_dc##n##_attributes,					\
+	.is_visible = SYSFS_GROUP_VISIBLE(cxl_memdev_dc##n),			\
+}
+CXL_MEMDEV_DC_ATTR_GROUP(0);
+CXL_MEMDEV_DC_ATTR_GROUP(1);
+CXL_MEMDEV_DC_ATTR_GROUP(2);
+CXL_MEMDEV_DC_ATTR_GROUP(3);
+CXL_MEMDEV_DC_ATTR_GROUP(4);
+CXL_MEMDEV_DC_ATTR_GROUP(5);
+CXL_MEMDEV_DC_ATTR_GROUP(6);
+CXL_MEMDEV_DC_ATTR_GROUP(7);
+
 static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
 				  int n)
 {
@@ -525,6 +643,14 @@  static struct attribute_group cxl_memdev_security_attribute_group = {
 };
 
 static const struct attribute_group *cxl_memdev_attribute_groups[] = {
+	&cxl_memdev_dc0_group,
+	&cxl_memdev_dc1_group,
+	&cxl_memdev_dc2_group,
+	&cxl_memdev_dc3_group,
+	&cxl_memdev_dc4_group,
+	&cxl_memdev_dc5_group,
+	&cxl_memdev_dc6_group,
+	&cxl_memdev_dc7_group,
 	&cxl_memdev_attribute_group,
 	&cxl_memdev_ram_attribute_group,
 	&cxl_memdev_pmem_attribute_group,