Re: [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID

[Dave Jiang <dave.jiang@intel.com>]

On 3/28/23 6:27 PM, Alison Schofield wrote:
> On Mon, Mar 27, 2023 at 02:46:06PM -0700, Dave Jiang wrote:
>> Export qtg_id sysfs attributes for the CXL memory device. The QTG ID
>> should show up as /sys/bus/cxl/devices/memX/qtg_id. The QTG ID is
>> retrieved via _DSM after supplying the caluclated bandwidth and latency
> 
> calculated

Thanks. I'm running codespell with checkpatch.pl and not sure why these 
things aren't being picked up.

> 
>> for the entire CXL path from device to the CPU. This ID is used to match
>> up to the root decoder QTG ID to determine which CFMWS the memory range
>> of a hotplugged CXL mem device should be assigned under.
>>
>> While there may be multiple DSMAS exported by the device CDAT, the driver
>> will only expose the first QTG ID in sysfs for now. In the future when
>> multiple QTG IDs are necessary, they can be exposed. [1]
>>
>> [1]: https://lore.kernel.org/linux-cxl/167571650007.587790.10040913293130712882.stgit@djiang5-mobl3.local/T/#md2a47b1ead3e1ba08f50eab29a4af1aed1d215ab
>>
>> Suggested-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>   Documentation/ABI/testing/sysfs-bus-cxl |   11 +++++++++++
>>   drivers/cxl/core/memdev.c               |   15 +++++++++++++++
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
>> index 471ac9a37078..a018f0a21aca 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-cxl
>> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
>> @@ -58,6 +58,17 @@ Description:
>>   		affinity for this device.
>>   
>>   
>> +What:		/sys/bus/cxl/devices/memX/qtg_id
>> +Date:		March, 2024
>> +KernelVersion:	v6.4
>> +Contact:	linux-cxl@vger.kernel.org
>> +Description:
>> +		(RO) Show the first QoS Throttling Group ID for the device.
>> +		The ID is used to match against the CFMWS (root decoder)
>> +		QTG ID so that the memory range under a hot-plugged device
>> +		is assigned under the appropriate CFMWS.
> 
> Some of the language in the cover letter seemed more descriptive, but
> I guess it's a bit squishy to me. (ie. 'some guidance' and 'appropriate')
> 
> Would a spec link be useful here?

Hmmm...I've not seen anywhere in the CXL spec that provides a complete 
picture of this. The memory device software developer's guide provids 
the most details I suppose. Maybe I can reference that here.

> 
> Alison
> 
>> +
>> +
>>   What:		/sys/bus/cxl/devices/*/devtype
>>   Date:		June, 2021
>>   KernelVersion:	v5.14
>> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
>> index d2605fc39240..974eff833edd 100644
>> --- a/drivers/cxl/core/memdev.c
>> +++ b/drivers/cxl/core/memdev.c
>> @@ -106,12 +106,27 @@ static ssize_t numa_node_show(struct device *dev, struct device_attribute *attr,
>>   }
>>   static DEVICE_ATTR_RO(numa_node);
>>   
>> +static ssize_t qtg_id_show(struct device *dev, struct device_attribute *attr,
>> +			   char *buf)
>> +{
>> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>> +	struct qos_prop_entry *qos;
>> +
>> +	if (list_empty(&cxlmd->qos_list))
>> +		return 0;
>> +
>> +	qos = list_first_entry(&cxlmd->qos_list, struct qos_prop_entry, list);
>> +	return sysfs_emit(buf, "%u\n", qos->qtg_id);
>> +}
>> +static DEVICE_ATTR_RO(qtg_id);
>> +
>>   static struct attribute *cxl_memdev_attributes[] = {
>>   	&dev_attr_serial.attr,
>>   	&dev_attr_firmware_version.attr,
>>   	&dev_attr_payload_max.attr,
>>   	&dev_attr_label_storage_size.attr,
>>   	&dev_attr_numa_node.attr,
>> +	&dev_attr_qtg_id.attr,
>>   	NULL,
>>   };
>>   
>>
>>