Re: [PATCH 01/18] cxl: Export QTG ids from CFMWS to sysfs

[Dave Jiang <dave.jiang@xxxxxxxxx>]

On 2/9/23 4:15 AM, Jonathan Cameron wrote:
> On Mon, 06 Feb 2023 13:49:30 -0700
> Dave Jiang <dave.jiang@xxxxxxxxx> wrote:
>
> > Export the QoS Throttling Group ID from the CXL Fixed Memory Window
> > Structure (CFMWS) under the root decoder sysfs attributes.
> > CXL rev3.0 9.17.1.3 CXL Fixed Memory Window Structure (CFMWS)
> >
> > Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
>
> Hi Dave,
>
>
> I've no objection to this, but would good to say why this
> might be of use to userspace.  What tooling needs it?

Will do.

> One comment on docs inline. With those two things tidied up
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
>
> > ---
> >   Documentation/ABI/testing/sysfs-bus-cxl |    7 +++++++
> >   drivers/cxl/acpi.c                      |    3 +++
> >   drivers/cxl/core/port.c                 |   14 ++++++++++++++
> >   drivers/cxl/cxl.h                       |    3 +++
> >   4 files changed, 27 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> > index 8494ef27e8d2..0932c2f6fbf4 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-cxl
> > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > @@ -294,6 +294,13 @@ Description:
> >   		(WO) Write a string in the form 'regionZ' to delete that region,
> >   		provided it is currently idle / not bound to a driver.
> >   
> > +What:		/sys/bus/cxl/devices/decoderX.Y/qtg_id
> > +Date:		Jan, 2023
> > +KernelVersion:	v6.3
> > +Contact:	linux-cxl@xxxxxxxxxxxxxxx
> > +Description:
> > +		(RO) Shows the QoS Throttling Group ID. The QTG ID for a root
> > +		decoder comes from the CFMWS structure of the CEDT.
>
> Document the -1 value for no ID in here. Hopefully people will write
> their userspace against this document and we want them to know about that
> corner case!

Ok I will add.

> >   
> >   What:		/sys/bus/cxl/devices/regionZ/uuid
> >   Date:		May, 2022
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index 13cde44c6086..7a71bb5041c7 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -289,6 +289,9 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
> >   			}
> >   		}
> >   	}
> > +
> > +	cxld->qtg_id = cfmws->qtg_id;
> > +
> >   	rc = cxl_decoder_add(cxld, target_map);
> >   err_xormap:
> >   	if (rc)
> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > index b631a0520456..fe78daf7e7c8 100644
> > --- a/drivers/cxl/core/port.c
> > +++ b/drivers/cxl/core/port.c
> > @@ -284,6 +284,16 @@ static ssize_t interleave_ways_show(struct device *dev,
> >   
> >   static DEVICE_ATTR_RO(interleave_ways);
> >   
> > +static ssize_t qtg_id_show(struct device *dev,
> > +			   struct device_attribute *attr, char *buf)
> > +{
> > +	struct cxl_decoder *cxld = to_cxl_decoder(dev);
> > +
> > +	return sysfs_emit(buf, "%d\n", cxld->qtg_id);
> > +}
> > +
> > +static DEVICE_ATTR_RO(qtg_id);
> > +
> >   static struct attribute *cxl_decoder_base_attrs[] = {
> >   	&dev_attr_start.attr,
> >   	&dev_attr_size.attr,
> > @@ -303,6 +313,7 @@ static struct attribute *cxl_decoder_root_attrs[] = {
> >   	&dev_attr_cap_type2.attr,
> >   	&dev_attr_cap_type3.attr,
> >   	&dev_attr_target_list.attr,
> > +	&dev_attr_qtg_id.attr,
> >   	SET_CXL_REGION_ATTR(create_pmem_region)
> >   	SET_CXL_REGION_ATTR(delete_region)
> >   	NULL,
> > @@ -1606,6 +1617,7 @@ struct cxl_root_decoder *cxl_root_decoder_alloc(struct cxl_port *port,
> >   	}
> >   
> >   	atomic_set(&cxlrd->region_id, rc);
> > +	cxld->qtg_id = CXL_QTG_ID_INVALID;
> >   	return cxlrd;
> >   }
> >   EXPORT_SYMBOL_NS_GPL(cxl_root_decoder_alloc, CXL);
> > @@ -1643,6 +1655,7 @@ struct cxl_switch_decoder *cxl_switch_decoder_alloc(struct cxl_port *port,
> >   
> >   	cxld = &cxlsd->cxld;
> >   	cxld->dev.type = &cxl_decoder_switch_type;
> > +	cxld->qtg_id = CXL_QTG_ID_INVALID;
> >   	return cxlsd;
> >   }
> >   EXPORT_SYMBOL_NS_GPL(cxl_switch_decoder_alloc, CXL);
> > @@ -1675,6 +1688,7 @@ struct cxl_endpoint_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port)
> >   	}
> >   
> >   	cxld->dev.type = &cxl_decoder_endpoint_type;
> > +	cxld->qtg_id = CXL_QTG_ID_INVALID;
> >   	return cxled;
> >   }
> >   EXPORT_SYMBOL_NS_GPL(cxl_endpoint_decoder_alloc, CXL);
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index 1b1cf459ac77..f558bbfc0332 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -279,6 +279,7 @@ enum cxl_decoder_type {
> >    */
> >   #define CXL_DECODER_MAX_INTERLEAVE 16
> >   
> > +#define CXL_QTG_ID_INVALID	-1
> >   
> >   /**
> >    * struct cxl_decoder - Common CXL HDM Decoder Attributes
> > @@ -290,6 +291,7 @@ enum cxl_decoder_type {
> >    * @target_type: accelerator vs expander (type2 vs type3) selector
> >    * @region: currently assigned region for this decoder
> >    * @flags: memory type capabilities and locking
> > + * @qtg_id: QoS Throttling Group ID
> >    * @commit: device/decoder-type specific callback to commit settings to hw
> >    * @reset: device/decoder-type specific callback to reset hw settings
> >   */
> > @@ -302,6 +304,7 @@ struct cxl_decoder {
> >   	enum cxl_decoder_type target_type;
> >   	struct cxl_region *region;
> >   	unsigned long flags;
> > +	int qtg_id;
> >   	int (*commit)(struct cxl_decoder *cxld);
> >   	int (*reset)(struct cxl_decoder *cxld);
> >   };