diff mbox series

[v3,11/25] cxl/mem: Expose DCD partition capabilities in sysfs

Message ID 20240816-dcd-type2-upstream-v3-11-7c9b96cba6d7@intel.com (mailing list archive)
State New, archived
Headers show
Series DCD: Add support for Dynamic Capacity Devices (DCD) | expand

Commit Message

Ira Weiny Aug. 16, 2024, 2:44 p.m. UTC
From: Navneet Singh <navneet.singh@intel.com>

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 <navneet.singh@intel.com>
Co-developed-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes:
[iweiny: remove review tags]
[Davidlohr/Fan/Jonathan: omit 'dc' attribute directory if device is not DC]
[Jonathan: update documentation for dc visibility]
[Jonathan: Add a comment to DC region X attributes to ensure visibility checks work]
[iweiny: push sysfs version to 6.12]
---
 Documentation/ABI/testing/sysfs-bus-cxl | 12 ++++
 drivers/cxl/core/memdev.c               | 97 +++++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+)

Comments

Dave Jiang Aug. 16, 2024, 11:42 p.m. UTC | #1
On 8/16/24 7:44 AM, ira.weiny@intel.com wrote:
> From: Navneet Singh <navneet.singh@intel.com>
> 
> 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 <navneet.singh@intel.com>
> Co-developed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> ---
> Changes:
> [iweiny: remove review tags]
> [Davidlohr/Fan/Jonathan: omit 'dc' attribute directory if device is not DC]
> [Jonathan: update documentation for dc visibility]
> [Jonathan: Add a comment to DC region X attributes to ensure visibility checks work]
> [iweiny: push sysfs version to 6.12]
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl | 12 ++++
>  drivers/cxl/core/memdev.c               | 97 +++++++++++++++++++++++++++++++++
>  2 files changed, 109 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 957717264709..6227ae0ab3fc 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -54,6 +54,18 @@ Description:
>  		identically named field in the Identify Memory Device Output
>  		Payload in the CXL-2.0 specification.
>  
> +What:		/sys/bus/cxl/devices/memX/dc/region_count
> +		/sys/bus/cxl/devices/memX/dc/regionY_size

Just make it into 2 separate entries?

DJ
> +Date:		August, 2024
> +KernelVersion:	v6.12
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) Dynamic Capacity (DC) region information.  The dc
> +		directory is only visible on devices which support Dynamic
> +		Capacity.
> +		The region_count is the number of Dynamic Capacity (DC)
> +		partitions (regions) supported on the device.
> +		regionY_size is the size of each of those partitions.
>  
>  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 0277726afd04..7da1f0f5711a 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -101,6 +101,18 @@ static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr,
>  static struct device_attribute dev_attr_pmem_size =
>  	__ATTR(size, 0444, pmem_size_show, NULL);
>  
> +static ssize_t region_count_show(struct device *dev, struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> +
> +	return sysfs_emit(buf, "%d\n", mds->nr_dc_region);
> +}
> +
> +static struct device_attribute dev_attr_region_count =
> +	__ATTR(region_count, 0444, region_count_show, NULL);
> +
>  static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
>  			   char *buf)
>  {
> @@ -448,6 +460,90 @@ static struct attribute *cxl_memdev_security_attributes[] = {
>  	NULL,
>  };
>  
> +static ssize_t show_size_regionN(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);
> +}
> +
> +#define REGION_SIZE_ATTR_RO(n)						\
> +static ssize_t region##n##_size_show(struct device *dev,		\
> +				     struct device_attribute *attr,	\
> +				     char *buf)				\
> +{									\
> +	return show_size_regionN(to_cxl_memdev(dev), buf, (n));		\
> +}									\
> +static DEVICE_ATTR_RO(region##n##_size)
> +REGION_SIZE_ATTR_RO(0);
> +REGION_SIZE_ATTR_RO(1);
> +REGION_SIZE_ATTR_RO(2);
> +REGION_SIZE_ATTR_RO(3);
> +REGION_SIZE_ATTR_RO(4);
> +REGION_SIZE_ATTR_RO(5);
> +REGION_SIZE_ATTR_RO(6);
> +REGION_SIZE_ATTR_RO(7);
> +
> +/*
> + * RegionX attributes must be listed in order and first in this array to
> + * support the visbility checks.
> + */
> +static struct attribute *cxl_memdev_dc_attributes[] = {
> +	&dev_attr_region0_size.attr,
> +	&dev_attr_region1_size.attr,
> +	&dev_attr_region2_size.attr,
> +	&dev_attr_region3_size.attr,
> +	&dev_attr_region4_size.attr,
> +	&dev_attr_region5_size.attr,
> +	&dev_attr_region6_size.attr,
> +	&dev_attr_region7_size.attr,
> +	&dev_attr_region_count.attr,
> +	NULL,
> +};
> +
> +static umode_t cxl_memdev_dc_attr_visible(struct kobject *kobj, struct attribute *a, int n)
> +{
> +	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;
> +
> +	if (a == &dev_attr_region_count.attr)
> +		return a->mode;
> +
> +	/*
> +	 * Show only the regions supported, regionX attributes are first in the
> +	 * list
> +	 */
> +	if (n < mds->nr_dc_region)
> +		return a->mode;
> +
> +	return 0;
> +}
> +
> +static bool cxl_memdev_dc_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);
> +
> +	/* No DC regions */
> +	if (!mds || mds->nr_dc_region == 0)
> +		return false;
> +	return true;
> +}
> +
> +DEFINE_SYSFS_GROUP_VISIBLE(cxl_memdev_dc);
> +
> +static struct attribute_group cxl_memdev_dc_group = {
> +	.name = "dc",
> +	.attrs = cxl_memdev_dc_attributes,
> +	.is_visible = SYSFS_GROUP_VISIBLE(cxl_memdev_dc),
> +};
> +
>  static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
>  				  int n)
>  {
> @@ -528,6 +624,7 @@ static const struct attribute_group *cxl_memdev_attribute_groups[] = {
>  	&cxl_memdev_ram_attribute_group,
>  	&cxl_memdev_pmem_attribute_group,
>  	&cxl_memdev_security_attribute_group,
> +	&cxl_memdev_dc_group,
>  	NULL,
>  };
>  
>
Ira Weiny Aug. 23, 2024, 2:28 a.m. UTC | #2
Dave Jiang wrote:
> 
> 
> On 8/16/24 7:44 AM, ira.weiny@intel.com wrote:
> > From: Navneet Singh <navneet.singh@intel.com>
> > 
> > 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 <navneet.singh@intel.com>
> > Co-developed-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > 
> > ---
> > Changes:
> > [iweiny: remove review tags]
> > [Davidlohr/Fan/Jonathan: omit 'dc' attribute directory if device is not DC]
> > [Jonathan: update documentation for dc visibility]
> > [Jonathan: Add a comment to DC region X attributes to ensure visibility checks work]
> > [iweiny: push sysfs version to 6.12]
> > ---
> >  Documentation/ABI/testing/sysfs-bus-cxl | 12 ++++
> >  drivers/cxl/core/memdev.c               | 97 +++++++++++++++++++++++++++++++++
> >  2 files changed, 109 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> > index 957717264709..6227ae0ab3fc 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-cxl
> > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > @@ -54,6 +54,18 @@ Description:
> >  		identically named field in the Identify Memory Device Output
> >  		Payload in the CXL-2.0 specification.
> >  
> > +What:		/sys/bus/cxl/devices/memX/dc/region_count
> > +		/sys/bus/cxl/devices/memX/dc/regionY_size
> 
> Just make it into 2 separate entries?

Do you mean in the docs?

Ira

> 
> DJ
> > +Date:		August, 2024
> > +KernelVersion:	v6.12
> > +Contact:	linux-cxl@vger.kernel.org
> > +Description:
> > +		(RO) Dynamic Capacity (DC) region information.  The dc
> > +		directory is only visible on devices which support Dynamic
> > +		Capacity.
> > +		The region_count is the number of Dynamic Capacity (DC)
> > +		partitions (regions) supported on the device.
> > +		regionY_size is the size of each of those partitions.
> >  
> >  What:		/sys/bus/cxl/devices/memX/pmem/qos_class
> >  Date:		May, 2023

[snip]
Dave Jiang Aug. 23, 2024, 2:58 p.m. UTC | #3
On 8/22/24 7:28 PM, Ira Weiny wrote:
> Dave Jiang wrote:
>>
>>
>> On 8/16/24 7:44 AM, ira.weiny@intel.com wrote:
>>> From: Navneet Singh <navneet.singh@intel.com>
>>>
>>> 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 <navneet.singh@intel.com>
>>> Co-developed-by: Ira Weiny <ira.weiny@intel.com>
>>> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>>>
>>> ---
>>> Changes:
>>> [iweiny: remove review tags]
>>> [Davidlohr/Fan/Jonathan: omit 'dc' attribute directory if device is not DC]
>>> [Jonathan: update documentation for dc visibility]
>>> [Jonathan: Add a comment to DC region X attributes to ensure visibility checks work]
>>> [iweiny: push sysfs version to 6.12]
>>> ---
>>>  Documentation/ABI/testing/sysfs-bus-cxl | 12 ++++
>>>  drivers/cxl/core/memdev.c               | 97 +++++++++++++++++++++++++++++++++
>>>  2 files changed, 109 insertions(+)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
>>> index 957717264709..6227ae0ab3fc 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-cxl
>>> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
>>> @@ -54,6 +54,18 @@ Description:
>>>  		identically named field in the Identify Memory Device Output
>>>  		Payload in the CXL-2.0 specification.
>>>  
>>> +What:		/sys/bus/cxl/devices/memX/dc/region_count
>>> +		/sys/bus/cxl/devices/memX/dc/regionY_size
>>
>> Just make it into 2 separate entries?
> 
> Do you mean in the docs?

Yes. Here you are combining all the sysfs entries into 1. I'm suggesting unique block per each sysfs entry with their own description. 
> 
> Ira
> 
>>
>> DJ
>>> +Date:		August, 2024
>>> +KernelVersion:	v6.12
>>> +Contact:	linux-cxl@vger.kernel.org
>>> +Description:
>>> +		(RO) Dynamic Capacity (DC) region information.  The dc
>>> +		directory is only visible on devices which support Dynamic
>>> +		Capacity.
>>> +		The region_count is the number of Dynamic Capacity (DC)
>>> +		partitions (regions) supported on the device.
>>> +		regionY_size is the size of each of those partitions.
>>>  
>>>  What:		/sys/bus/cxl/devices/memX/pmem/qos_class
>>>  Date:		May, 2023
> 
> [snip]
Jonathan Cameron Aug. 23, 2024, 4:14 p.m. UTC | #4
On Thu, 22 Aug 2024 21:28:41 -0500
Ira Weiny <ira.weiny@intel.com> wrote:

> Dave Jiang wrote:
> > 
> > 
> > On 8/16/24 7:44 AM, ira.weiny@intel.com wrote:  
> > > From: Navneet Singh <navneet.singh@intel.com>
> > > 
> > > 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 <navneet.singh@intel.com>
> > > Co-developed-by: Ira Weiny <ira.weiny@intel.com>
> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > > 
> > > ---
> > > Changes:
> > > [iweiny: remove review tags]
> > > [Davidlohr/Fan/Jonathan: omit 'dc' attribute directory if device is not DC]
> > > [Jonathan: update documentation for dc visibility]
> > > [Jonathan: Add a comment to DC region X attributes to ensure visibility checks work]
> > > [iweiny: push sysfs version to 6.12]
> > > ---
> > >  Documentation/ABI/testing/sysfs-bus-cxl | 12 ++++
> > >  drivers/cxl/core/memdev.c               | 97 +++++++++++++++++++++++++++++++++
> > >  2 files changed, 109 insertions(+)
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> > > index 957717264709..6227ae0ab3fc 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-cxl
> > > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > > @@ -54,6 +54,18 @@ Description:
> > >  		identically named field in the Identify Memory Device Output
> > >  		Payload in the CXL-2.0 specification.
> > >  
> > > +What:		/sys/bus/cxl/devices/memX/dc/region_count
> > > +		/sys/bus/cxl/devices/memX/dc/regionY_size  
> > 
> > Just make it into 2 separate entries?  
> 
> Do you mean in the docs?

Assuming yes, then I think it would be cleaner as two separate entries
+ Maybe even one for the directory which can then have
the visibility statement.

> 
> Ira
> 
> > 
> > DJ  
> > > +Date:		August, 2024
> > > +KernelVersion:	v6.12
> > > +Contact:	linux-cxl@vger.kernel.org
> > > +Description:
> > > +		(RO) Dynamic Capacity (DC) region information.  The dc
> > > +		directory is only visible on devices which support Dynamic
> > > +		Capacity.
> > > +		The region_count is the number of Dynamic Capacity (DC)
> > > +		partitions (regions) supported on the device.
> > > +		regionY_size is the size of each of those partitions.
> > >  
> > >  What:		/sys/bus/cxl/devices/memX/pmem/qos_class
> > >  Date:		May, 2023  
> 
> [snip]
>
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 957717264709..6227ae0ab3fc 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -54,6 +54,18 @@  Description:
 		identically named field in the Identify Memory Device Output
 		Payload in the CXL-2.0 specification.
 
+What:		/sys/bus/cxl/devices/memX/dc/region_count
+		/sys/bus/cxl/devices/memX/dc/regionY_size
+Date:		August, 2024
+KernelVersion:	v6.12
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) Dynamic Capacity (DC) region information.  The dc
+		directory is only visible on devices which support Dynamic
+		Capacity.
+		The region_count is the number of Dynamic Capacity (DC)
+		partitions (regions) supported on the device.
+		regionY_size is the size of each of those partitions.
 
 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 0277726afd04..7da1f0f5711a 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -101,6 +101,18 @@  static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr,
 static struct device_attribute dev_attr_pmem_size =
 	__ATTR(size, 0444, pmem_size_show, NULL);
 
+static ssize_t region_count_show(struct device *dev, struct device_attribute *attr,
+				 char *buf)
+{
+	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
+
+	return sysfs_emit(buf, "%d\n", mds->nr_dc_region);
+}
+
+static struct device_attribute dev_attr_region_count =
+	__ATTR(region_count, 0444, region_count_show, NULL);
+
 static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
@@ -448,6 +460,90 @@  static struct attribute *cxl_memdev_security_attributes[] = {
 	NULL,
 };
 
+static ssize_t show_size_regionN(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);
+}
+
+#define REGION_SIZE_ATTR_RO(n)						\
+static ssize_t region##n##_size_show(struct device *dev,		\
+				     struct device_attribute *attr,	\
+				     char *buf)				\
+{									\
+	return show_size_regionN(to_cxl_memdev(dev), buf, (n));		\
+}									\
+static DEVICE_ATTR_RO(region##n##_size)
+REGION_SIZE_ATTR_RO(0);
+REGION_SIZE_ATTR_RO(1);
+REGION_SIZE_ATTR_RO(2);
+REGION_SIZE_ATTR_RO(3);
+REGION_SIZE_ATTR_RO(4);
+REGION_SIZE_ATTR_RO(5);
+REGION_SIZE_ATTR_RO(6);
+REGION_SIZE_ATTR_RO(7);
+
+/*
+ * RegionX attributes must be listed in order and first in this array to
+ * support the visbility checks.
+ */
+static struct attribute *cxl_memdev_dc_attributes[] = {
+	&dev_attr_region0_size.attr,
+	&dev_attr_region1_size.attr,
+	&dev_attr_region2_size.attr,
+	&dev_attr_region3_size.attr,
+	&dev_attr_region4_size.attr,
+	&dev_attr_region5_size.attr,
+	&dev_attr_region6_size.attr,
+	&dev_attr_region7_size.attr,
+	&dev_attr_region_count.attr,
+	NULL,
+};
+
+static umode_t cxl_memdev_dc_attr_visible(struct kobject *kobj, struct attribute *a, int n)
+{
+	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;
+
+	if (a == &dev_attr_region_count.attr)
+		return a->mode;
+
+	/*
+	 * Show only the regions supported, regionX attributes are first in the
+	 * list
+	 */
+	if (n < mds->nr_dc_region)
+		return a->mode;
+
+	return 0;
+}
+
+static bool cxl_memdev_dc_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);
+
+	/* No DC regions */
+	if (!mds || mds->nr_dc_region == 0)
+		return false;
+	return true;
+}
+
+DEFINE_SYSFS_GROUP_VISIBLE(cxl_memdev_dc);
+
+static struct attribute_group cxl_memdev_dc_group = {
+	.name = "dc",
+	.attrs = cxl_memdev_dc_attributes,
+	.is_visible = SYSFS_GROUP_VISIBLE(cxl_memdev_dc),
+};
+
 static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
 				  int n)
 {
@@ -528,6 +624,7 @@  static const struct attribute_group *cxl_memdev_attribute_groups[] = {
 	&cxl_memdev_ram_attribute_group,
 	&cxl_memdev_pmem_attribute_group,
 	&cxl_memdev_security_attribute_group,
+	&cxl_memdev_dc_group,
 	NULL,
 };