On Wed, 19 Apr 2023 13:21:07 -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)
>
> cxl cli will use this QTG ID to match with the _DSM retrieved QTG ID for a
> hot-plugged CXL memory device DPA memory range to make sure that the DPA range
> is under the right CFMWS window.
>
> Reviewed-by: Ira Weiny <ira.weiny@xxxxxxxxx>
> Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
>
Bikeshedding alert:
Why not just call it qtg? What does the _id add?
I don't really care either way...
LGTM
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
One (more) completely trivial comment inline.
Jonathan
> ---
> v4:
> - Change kernel version for documentation to v6.5
> v2:
> - Add explanation commit header (Jonathan)
> ---
> Documentation/ABI/testing/sysfs-bus-cxl | 9 +++++++++
> drivers/cxl/acpi.c | 3 +++
> drivers/cxl/core/port.c | 14 ++++++++++++++
> drivers/cxl/cxl.h | 3 +++
> 4 files changed, 29 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 3acf2f17a73f..bd2b59784979 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -309,6 +309,15 @@ 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.5
> +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. A value of
> + -1 indicates that no QTG ID was retrieved. The QTG ID is used as
> + guidance to match against the QTG ID of a hot-plugged device.
>
> What: /sys/bus/cxl/devices/regionZ/uuid
> Date: May, 2022
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 7e1765b09e04..abc24137c291 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 4d1f9c5b5029..024d4178f557 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -276,6 +276,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);
> +}
> +
No blank line here would be more consistent with local style (based on
a really quick look).
> +static DEVICE_ATTR_RO(qtg_id);
> +
> static struct attribute *cxl_decoder_base_attrs[] = {
> &dev_attr_start.attr,
> &dev_attr_size.attr,
> @@ -295,6 +305,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(create_ram_region)
> SET_CXL_REGION_ATTR(delete_region)
> @@ -1625,6 +1636,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);
> @@ -1662,6 +1674,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);
> @@ -1694,6 +1707,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 044a92d9813e..278ab6952332 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -300,6 +300,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
> @@ -311,6 +312,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
> */
> @@ -323,6 +325,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);
> };
>
>
