[18/18] cxl: Export sysfs attributes for device QTG IDs

Export qtg_id sysfs attributes for the respective ram and pmem DPA range of
a CXL device. The QTG ID should show up as
/sys/bus/cxl/devices/memX/pmem/qtg_id for pmem or as
/sys/bus/cxl/devices/memX/ram/qtg_id for ram.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-cxl |   15 +++++++++++++++
 drivers/cxl/core/memdev.c               |   26 ++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

On Mon, 06 Feb 2023 13:52:05 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Export qtg_id sysfs attributes for the respective ram and pmem DPA range of
> a CXL device. The QTG ID should show up as
> /sys/bus/cxl/devices/memX/pmem/qtg_id for pmem or as
> /sys/bus/cxl/devices/memX/ram/qtg_id for ram.

This doesn't extend to devices with say multiple DSMAS regions
for RAM with different access characteristics.  Think of a device
with HBM and DDR for example, or a mix of DDR4 and DDR5.

Once we are dealing with memory pools of significant size there
are very likely to be DPA regions with different characteristics.

So minimum I'd suggest is leave space for an ABI that might look like.

mem/range0_qtg_id
mem/range1_qtg_id
mem/range0_base
mem/range0_length
mem/range1_base
mem/range1_length
etc but with the flexibility to not present the rangeX_base/length stuff if there
is only one presented.  For now just present the range0_qtg_id

I'm fine if you want to implement multiple ranges from the start though.

As with previous ABI patch, I'd like to see a little description in the patch
header of what this stuff is for as well.  Obvious to some of us perhaps, but
better to call it out for anyone who is wondering why userspace needs to know.

I'm guessing you have a nice QEMU patch adding the DSM etc?

Thanks,

Jonathan


> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl |   15 +++++++++++++++
>  drivers/cxl/core/memdev.c               |   26 ++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 0932c2f6fbf4..8133a13e118d 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -27,6 +27,14 @@ Description:
>  		identically named field in the Identify Memory Device Output
>  		Payload in the CXL-2.0 specification.
>  
> +What:		/sys/bus/cxl/devices/memX/ram/qtg_id
> +Date:		January, 2023
> +KernelVersion:	v6.3
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) Shows calculated QoS Throttling Group ID for the
> +		"Volatile Only Capacity" DPA range.
> +
>  
>  What:		/sys/bus/cxl/devices/memX/pmem/size
>  Date:		December, 2020
> @@ -37,6 +45,13 @@ Description:
>  		identically named field in the Identify Memory Device Output
>  		Payload in the CXL-2.0 specification.
>  
> +What:		/sys/bus/cxl/devices/memX/pmem/qtg_id
> +Date:		January, 2023
> +KernelVersion:	v6.3
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) Shows calculated QoS Throttling Group ID for the
> +		"Persistent Only Capacity" DPA range.
>  
>  What:		/sys/bus/cxl/devices/memX/serial
>  Date:		January, 2022
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index a74a93310d26..06f9ac929ef4 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -76,6 +76,18 @@ static ssize_t ram_size_show(struct device *dev, struct device_attribute *attr,
>  static struct device_attribute dev_attr_ram_size =
>  	__ATTR(size, 0444, ram_size_show, NULL);
>  
> +static ssize_t ram_qtg_id_show(struct device *dev, struct device_attribute *attr,
> +			       char *buf)
> +{
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> +	return sysfs_emit(buf, "%d\n", cxlds->ram_qtg_id);
> +}
> +
> +static struct device_attribute dev_attr_ram_qtg_id =
> +	__ATTR(qtg_id, 0444, ram_qtg_id_show, NULL);
> +
>  static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr,
>  			      char *buf)
>  {
> @@ -89,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 pmem_qtg_id_show(struct device *dev, struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> +	return sysfs_emit(buf, "%d\n", cxlds->pmem_qtg_id);
> +}
> +
> +static struct device_attribute dev_attr_pmem_qtg_id =
> +	__ATTR(qtg_id, 0444, pmem_qtg_id_show, NULL);
> +
>  static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
>  			   char *buf)
>  {
> @@ -117,11 +141,13 @@ static struct attribute *cxl_memdev_attributes[] = {
>  
>  static struct attribute *cxl_memdev_pmem_attributes[] = {
>  	&dev_attr_pmem_size.attr,
> +	&dev_attr_pmem_qtg_id.attr,
>  	NULL,
>  };
>  
>  static struct attribute *cxl_memdev_ram_attributes[] = {
>  	&dev_attr_ram_size.attr,
> +	&dev_attr_ram_qtg_id.attr,
>  	NULL,
>  };
>  
> 
>

Jonathan Cameron wrote:
> On Mon, 06 Feb 2023 13:52:05 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
> > Export qtg_id sysfs attributes for the respective ram and pmem DPA range of
> > a CXL device. The QTG ID should show up as
> > /sys/bus/cxl/devices/memX/pmem/qtg_id for pmem or as
> > /sys/bus/cxl/devices/memX/ram/qtg_id for ram.
> 
> This doesn't extend to devices with say multiple DSMAS regions
> for RAM with different access characteristics.  Think of a device
> with HBM and DDR for example, or a mix of DDR4 and DDR5.
> 
> Once we are dealing with memory pools of significant size there
> are very likely to be DPA regions with different characteristics.
> 
> So minimum I'd suggest is leave space for an ABI that might look like.
> 
> mem/range0_qtg_id
> mem/range1_qtg_id
> mem/range0_base
> mem/range0_length
> mem/range1_base
> mem/range1_length
> etc but with the flexibility to not present the rangeX_base/length stuff if there
> is only one presented.  For now just present the range0_qtg_id

I do agree that there should be some mechanism to dump this information,
I am just not yet sure the should prioritize for the case where someone
builds multiple performance classes per partition type. There would seem
to be design pressure against that given you can not allocate regions
out of DPA order otherwise capacity gets stranded.

So I am thinking something like a debugfs interface to dump all the
ranges but otherwise leave memX/{ram,pmem,dcd[0-7]} with a single
qtg-id each.

If it turns out later that devices really call for multiple qtg-ids
per-partition as a first-class ABI then there's the option of something
like:

memX/ram/qtg_id
memX/ram/qtg_id1
memX/ram/qtg_id2

memX/ram/qtg_range/
memX/ram/qtg1_range/
memX/ram/qtg2_range/

...but I hope the primary use case for devices with multiple performance
ranges is due to having 'pmem' or 'dcd' in addition to 'ram'.