diff mbox series

[08/26] cxl/mem: Expose device dynamic capacity capabilities

Message ID 20240324-dcd-type2-upstream-v1-8-b7b00d623625@intel.com (mailing list archive)
State New, archived
Headers show
Series DCD: Add support for Dynamic Capacity Devices (DCD) | expand

Commit Message

Ira Weiny March 24, 2024, 11:18 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 Regions available on
a device.

Expose driver dynamic capacity capabilities through sysfs
attributes.

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 for v1:
[iweiny: remove review tags]
[iweiny: mark sysfs for 6.10 kernel]
---
 Documentation/ABI/testing/sysfs-bus-cxl | 17 ++++++++
 drivers/cxl/core/memdev.c               | 76 +++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

Comments

Davidlohr Bueso March 25, 2024, 11:40 p.m. UTC | #1 
On Sun, 24 Mar 2024, ira.weiny@intel.com wrote:

>+What:		/sys/bus/cxl/devices/memX/dc/region_count
>+Date:		June, 2024
>+KernelVersion:	v6.10
>+Contact:	linux-cxl@vger.kernel.org
>+Description:
>+		(RO) Number of Dynamic Capacity (DC) regions supported on the
>+		device.  May be 0 if the device does not support Dynamic
>+		Capacity.

If dcd is not supported then we should not have the dc/ directory
altogether.

Thanks,
Davidlohr
fan March 26, 2024, 6:30 p.m. UTC | #2 
On Mon, Mar 25, 2024 at 04:40:16PM -0700, Davidlohr Bueso wrote:
> On Sun, 24 Mar 2024, ira.weiny@intel.com wrote:
> 
> > +What:		/sys/bus/cxl/devices/memX/dc/region_count
> > +Date:		June, 2024
> > +KernelVersion:	v6.10
> > +Contact:	linux-cxl@vger.kernel.org
> > +Description:
> > +		(RO) Number of Dynamic Capacity (DC) regions supported on the
> > +		device.  May be 0 if the device does not support Dynamic
> > +		Capacity.
> 
> If dcd is not supported then we should not have the dc/ directory
> altogether.
> 
> Thanks,
> Davidlohr 

I also think so. However, I also noticed one thing (not DCD related).
Even for a PMEM device, for example, we have a ram directory under the
device directory.

===================
root@DT:~# cxl list
[
  {
    "memdev":"mem0",
    "pmem_size":536870912,
    "serial":0,
    "host":"0000:0d:00.0"
  }
]
root@DT:~# ls /sys/bus/cxl/devices/mem0/
dc  dev  driver  firmware  firmware_version  label_storage_size  numa_node  payload_max  pmem  pmem0  ram  security  serial  subsystem	trigger_poison_list  uevent
root@DT:~#
===================

Fan
Jonathan Cameron April 4, 2024, 8:44 a.m. UTC | #3 
On Tue, 26 Mar 2024 11:30:38 -0700
fan <nifan.cxl@gmail.com> wrote:

> On Mon, Mar 25, 2024 at 04:40:16PM -0700, Davidlohr Bueso wrote:
> > On Sun, 24 Mar 2024, ira.weiny@intel.com wrote:
> >   
> > > +What:		/sys/bus/cxl/devices/memX/dc/region_count
> > > +Date:		June, 2024
> > > +KernelVersion:	v6.10
> > > +Contact:	linux-cxl@vger.kernel.org
> > > +Description:
> > > +		(RO) Number of Dynamic Capacity (DC) regions supported on the
> > > +		device.  May be 0 if the device does not support Dynamic
> > > +		Capacity.  
> > 
> > If dcd is not supported then we should not have the dc/ directory
> > altogether.
> > 
> > Thanks,
> > Davidlohr   
> 
> I also think so. However, I also noticed one thing (not DCD related).
> Even for a PMEM device, for example, we have a ram directory under the
> device directory.

True, but it's new ABI so we don't have to copy and Dan's patch to
allow for hiding static attribute directories has landed in the meantime.
So I vote for hiding it.

> 
> ===================
> root@DT:~# cxl list
> [
>   {
>     "memdev":"mem0",
>     "pmem_size":536870912,
>     "serial":0,
>     "host":"0000:0d:00.0"
>   }
> ]
> root@DT:~# ls /sys/bus/cxl/devices/mem0/
> dc  dev  driver  firmware  firmware_version  label_storage_size  numa_node  payload_max  pmem  pmem0  ram  security  serial  subsystem	trigger_poison_list  uevent
> root@DT:~#
> ===================
> 
> Fan
>
Jonathan Cameron April 4, 2024, 8:51 a.m. UTC | #4 
On Sun, 24 Mar 2024 16:18:11 -0700
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 Regions available on
> a device.
> 
> Expose driver dynamic capacity capabilities through sysfs
> attributes.
> 
> 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>

Trivial comments inline.
Whilst I'd like the directory hidden as per the other suggestions,
I don't mind that much.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> 
> ---
> Changes for v1:
> [iweiny: remove review tags]
> [iweiny: mark sysfs for 6.10 kernel]
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl | 17 ++++++++
>  drivers/cxl/core/memdev.c               | 76 +++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 8b3efaf6563c..8a4f572c8498 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -54,6 +54,23 @@ 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
> +Date:		June, 2024
> +KernelVersion:	v6.10
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) Number of Dynamic Capacity (DC) regions supported on the
> +		device.  May be 0 if the device does not support Dynamic
> +		Capacity.

That will change if you go ahead and hide the directory as per suggestions.

> +
> +What:		/sys/bus/cxl/devices/memX/dc/regionY_size
> +Date:		June, 2024
> +KernelVersion:	v6.10
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) Size of the Dynamic Capacity (DC) region Y.  Only

Units always good to have in docs even if somewhat obvious.

> +		available on devices which support DC and only for those
> +		region indexes supported by the device.
>  
>  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 d4e259f3a7e9..a7b880e33a7e 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 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_dc_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 */
> +	if (n < mds->nr_dc_region)
> +		return a->mode;

This feels a bit fragile if anyone adds new attrs in future and for whatever reason
does it before these.

Maybe add a comment at top of cxl_memdev_dc_attributes()?  Say they must be first.

> +
> +	return 0;
> +}
> +

>
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 8b3efaf6563c..8a4f572c8498 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -54,6 +54,23 @@  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
+Date:		June, 2024
+KernelVersion:	v6.10
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) Number of Dynamic Capacity (DC) regions supported on the
+		device.  May be 0 if the device does not support Dynamic
+		Capacity.
+
+What:		/sys/bus/cxl/devices/memX/dc/regionY_size
+Date:		June, 2024
+KernelVersion:	v6.10
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) Size of the Dynamic Capacity (DC) region Y.  Only
+		available on devices which support DC and only for those
+		region indexes supported by the device.
 
 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 d4e259f3a7e9..a7b880e33a7e 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)
 {
@@ -492,6 +504,63 @@  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);
+
+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_dc_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 */
+	if (n < mds->nr_dc_region)
+		return a->mode;
+
+	return 0;
+}
+
 static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
 				  int n)
 {
@@ -567,11 +636,18 @@  static struct attribute_group cxl_memdev_security_attribute_group = {
 	.is_visible = cxl_memdev_security_visible,
 };
 
+static struct attribute_group cxl_memdev_dc_attribute_group = {
+	.name = "dc",
+	.attrs = cxl_memdev_dc_attributes,
+	.is_visible = cxl_dc_visible,
+};
+
 static const struct attribute_group *cxl_memdev_attribute_groups[] = {
 	&cxl_memdev_attribute_group,
 	&cxl_memdev_ram_attribute_group,
 	&cxl_memdev_pmem_attribute_group,
 	&cxl_memdev_security_attribute_group,
+	&cxl_memdev_dc_attribute_group,
 	NULL,
 };