linux-cxl.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem
@ 2023-03-27 21:44 Dave Jiang
  2023-03-27 21:44 ` [PATCH v2 01/21] cxl: Export QTG ids from CFMWS to sysfs Dave Jiang
                   ` (21 more replies)
  0 siblings, 22 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:44 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: Dan Williams, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, rafael, lukas

v2:
- Please see specific patches for log entries addressing comments from v1.
- Removed ACPICA code usages.
- Removed PCI subsystem helpers for latency and bandwidth.
- Add CXL switch CDAT parsing support (SSLBIS)
- Add generic port SRAT+HMAT support (ACPI)
- Export a single QTG ID via sysfs per memory device (Dan)
- Provide rest of DSMAS range info in debugfs (Dan)


Hi Rafael, please review the relevant patches to ACPI: 13/18, 14/18. Thank you!
If they are ok, Dan can take them through the CXL tree for upstream merging.
These patches are different than what was posted in v1. I've removed ACPICA usages
from v1. The two new patches adds the SRAT Generic Port Affinity Structure parsing
in order to store the device handle that correlates to the proximity domains
exported by the SRAT table and ties that to the performance data that's exported by
the HMAT.

This series adds the retrieval of QoS Throttling Group (QTG) IDs for the CXL Fixed
Memory Window Structure (CFMWS) and the CXL memory device. It provides the QTG IDs
to user space to provide some guidance with putting the proper DPA range under the
appropriate CFMWS window for a hot-plugged CXL memory device.

The CFMWS structure contains a QTG ID that is associated with the memory window that the
structure exports. On Linux, the CFMWS is represented as a CXL root decoder. The QTG
ID will be attached to the CXL root decoder and exported as a sysfs attribute (qtg_id).

The QTG ID for a device is retrieved via sending a _DSM method to the ACPI0017 device.
The _DSM expects an input package of 4 DWORDS that contains the read latency, write
latency, read bandwidth, and write banwidth. These are the caluclated numbers for the
path between the CXL device and the CXL host bridge (HB). The QTG ID is also exported
as a sysfs attribute under the mem device memory type:
/sys/bus/cxl/devices/memX/qtg_id
Only the first QTG ID is exported. The rest of the information can be found under
/sys/kernel/debug/cxl/memX/qtgmap where all the DPA ranges with the correlated QTG ID
are displayed. Each DSMAS from the device CDAT will provide a DPA range.

The latency numbers are the aggregated latencies for the path between the CXL device and
the CPU. If a CXL device is directly attached to the CXL HB, the latency
would be the aggregated latencies from the device Coherent Device Attribute Table (CDAT),
the caluclated PCIe link latency between the device and the HB, and the generic port data
from ACPI SRAT+HMAT. The bandwidth in this configuration would be the minimum between the
CDAT bandwidth number, link bandwidth between the device and the HB, and the bandwidth data
from the generic port data via ACPI SRAT+HMAT.

If a configuration has a switch in between then the latency would be the aggregated
latencies from the device CDAT, the link latency between device and switch, the
latency from the switch CDAT, the link latency between switch and the HB, and the
generic port latency between the CPU and the CXL HB. The bandwidth calculation would be the
min of device CDAT bandwidth, link bandwith between device and switch, switch CDAT
bandwidth, the link bandwidth between switch and HB, and the generic port bandwidth

There can be 0 or more switches between the CXL device and the CXL HB. There are detailed
examples on calculating bandwidth and latency in the CXL Memory Device Software Guide [4].

The CDAT provides Device Scoped Memory Affinity Structures (DSMAS) that contains the
Device Physical Address (DPA) range and the related Device Scoped Latency and Bandwidth
Informat Stuctures (DSLBIS). Each DSLBIS provides a latency or bandwidth entry that is
tied to a DSMAS entry via a per DSMAS unique DSMAD handle.

This series is based on Lukas's latest DOE changes [5]. Kernel branch with all the code can
be retrieved here [6] for convenience.

Test setup is done with runqemu genport support branch [6]. The setup provides 2 CXL HBs
with one HB having a CXL switch underneath. It also provides generic port support detailed
below.

A hacked up qemu branch is used to support generic port SRAT and HMAT [7].

To create the appropriate HMAT entries for generic port, the following qemu paramters must
be added:

-object genport,id=$X -numa node,genport=genport$X,nodeid=$Y,initiator=$Z
-numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-latency,latency=$latency
-numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-bandwidth,bandwidth=$bandwidthM
for ((i = 0; i < total_nodes; i++)); do
	for ((j = 0; j < cxl_hbs; j++ )); do	# 2 CXL HBs
		-numa dist,src=$i,dst=$X,val=$dist
	done
done

See the genport run_qemu branch for full details.

[1]: https://www.computeexpresslink.org/download-the-specification
[2]: https://uefi.org/sites/default/files/resources/Coherent%20Device%20Attribute%20Table_1.01.pdf
[3]: https://uefi.org/sites/default/files/resources/ACPI_Spec_6_5_Aug29.pdf
[4]: https://cdrdv2-public.intel.com/643805/643805_CXL%20Memory%20Device%20SW%20Guide_Rev1p0.pdf
[5]: https://lore.kernel.org/linux-cxl/20230313195530.GA1532686@bhelgaas/T/#t
[6]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-qtg
[7]: https://github.com/pmem/run_qemu/tree/djiang/genport
[8]: https://github.com/davejiang/qemu/tree/genport

---

Dave Jiang (21):
      cxl: Export QTG ids from CFMWS to sysfs
      cxl: Add checksum verification to CDAT from CXL
      cxl: Add support for reading CXL switch CDAT table
      cxl: Add common helpers for cdat parsing
      cxl: Add callback to parse the DSMAS subtables from CDAT
      cxl: Add callback to parse the DSLBIS subtable from CDAT
      cxl: Add callback to parse the SSLBIS subtable from CDAT
      cxl: Add support for _DSM Function for retrieving QTG ID
      cxl: Add helper function to retrieve ACPI handle of CXL root device
      cxl: Add helpers to calculate pci latency for the CXL device
      cxl: Add helper function that calculates QoS values for switches
      cxl: Add helper function that calculate QoS values for PCI path
      ACPI: NUMA: Add genport target allocation to the HMAT parsing
      ACPI: NUMA: Add helper function to retrieve the performance attributes
      cxl: Add helper function to retrieve generic port QoS
      cxl: Add latency and bandwidth calculations for the CXL path
      cxl: Wait Memory_Info_Valid before access memory related info
      cxl: Move identify and partition query from pci probe to port probe
      cxl: Store QTG IDs and related info to the CXL memory device context
      cxl: Export sysfs attributes for memory device QTG ID
      cxl/mem: Add debugfs output for QTG related data


 Documentation/ABI/testing/sysfs-bus-cxl |  20 ++
 drivers/acpi/numa/hmat.c                |  95 +++++++++
 drivers/cxl/acpi.c                      |   3 +
 drivers/cxl/core/Makefile               |   2 +
 drivers/cxl/core/acpi.c                 | 180 ++++++++++++++++++
 drivers/cxl/core/cdat.c                 | 243 ++++++++++++++++++++++++
 drivers/cxl/core/memdev.c               |  16 ++
 drivers/cxl/core/pci.c                  | 187 ++++++++++++++++--
 drivers/cxl/core/port.c                 | 169 ++++++++++++++++
 drivers/cxl/cxl.h                       |  27 +++
 drivers/cxl/cxlmem.h                    |  14 ++
 drivers/cxl/cxlpci.h                    | 121 ++++++++++++
 drivers/cxl/mem.c                       |  16 ++
 drivers/cxl/pci.c                       |  21 --
 drivers/cxl/port.c                      | 130 ++++++++++++-
 include/acpi/actbl3.h                   |   2 +
 include/linux/acpi.h                    |   6 +
 tools/testing/cxl/Kbuild                |   1 +
 tools/testing/cxl/test/mock.c           |   5 +
 19 files changed, 1221 insertions(+), 37 deletions(-)
 create mode 100644 drivers/cxl/core/acpi.c
 create mode 100644 drivers/cxl/core/cdat.c

--


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v2 01/21] cxl: Export QTG ids from CFMWS to sysfs
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
@ 2023-03-27 21:44 ` Dave Jiang
  2023-03-29 23:57   ` Ira Weiny
  2023-03-27 21:44 ` [PATCH v2 02/21] cxl: Add checksum verification to CDAT from CXL Dave Jiang
                   ` (20 subsequent siblings)
  21 siblings, 1 reply; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:44 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

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.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
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..471ac9a37078 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.4
+Contact:	linux-cxl@vger.kernel.org
+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 8ee6b6e2e2a4..5ec48dddb2f9 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);
+}
+
+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)
@@ -1649,6 +1660,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);
@@ -1686,6 +1698,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);
@@ -1718,6 +1731,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 f2b0962a552d..cc3309794b45 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);
 };



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 02/21] cxl: Add checksum verification to CDAT from CXL
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
  2023-03-27 21:44 ` [PATCH v2 01/21] cxl: Export QTG ids from CFMWS to sysfs Dave Jiang
@ 2023-03-27 21:44 ` Dave Jiang
  2023-03-29  0:03   ` Alison Schofield
  2023-03-30  0:09   ` Ira Weiny
  2023-03-27 21:44 ` [PATCH v2 03/21] cxl: Add support for reading CXL switch CDAT table Dave Jiang
                   ` (19 subsequent siblings)
  21 siblings, 2 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:44 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

A CDAT table is available from a CXL device. The table is read by the
driver and cached in software. With the CXL subsystem needing to parse the
CDAT table, the checksum should be verified. Add checksum verification
after the CDAT table is read from device.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v2:
- Drop ACPI checksum export and just use local verification. (Dan)
---
 drivers/cxl/core/pci.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 25b7e8125d5d..e0d5e6525c0d 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -528,6 +528,16 @@ static int cxl_cdat_read_table(struct device *dev,
 	return 0;
 }
 
+static unsigned char cdat_checksum(void *buf, size_t size)
+{
+	unsigned char sum, *data = buf;
+	size_t i;
+
+	for (sum = 0, i = 0; i < size; i++)
+		sum += data[i];
+	return 0 - sum;
+}
+
 /**
  * read_cdat_data - Read the CDAT data on this port
  * @port: Port to read data from
@@ -573,6 +583,12 @@ void read_cdat_data(struct cxl_port *port)
 	}
 
 	port->cdat.table = cdat_table + sizeof(__le32);
+	if (cdat_checksum(port->cdat.table, cdat_length)) {
+		/* Don't leave table data allocated on error */
+		devm_kfree(dev, cdat_table);
+		dev_err(dev, "CDAT data checksum error\n");
+	}
+
 	port->cdat.length = cdat_length;
 }
 EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 03/21] cxl: Add support for reading CXL switch CDAT table
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
  2023-03-27 21:44 ` [PATCH v2 01/21] cxl: Export QTG ids from CFMWS to sysfs Dave Jiang
  2023-03-27 21:44 ` [PATCH v2 02/21] cxl: Add checksum verification to CDAT from CXL Dave Jiang
@ 2023-03-27 21:44 ` Dave Jiang
  2023-03-30  0:19   ` Ira Weiny
  2023-03-27 21:44 ` [PATCH v2 04/21] cxl: Add common helpers for cdat parsing Dave Jiang
                   ` (18 subsequent siblings)
  21 siblings, 1 reply; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:44 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Move read_cdat_data() from endpoint probe to general port probe to
allow reading of CDAT data for CXL switches as well as CXL device.
Add wrapper support for cxl_test to bypass the cdat reading.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/pci.c        |   20 +++++++++++++++-----
 drivers/cxl/port.c            |    6 +++---
 tools/testing/cxl/Kbuild      |    1 +
 tools/testing/cxl/test/mock.c |    5 +++++
 4 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index e0d5e6525c0d..4241c7b8d5c2 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -546,16 +546,26 @@ static unsigned char cdat_checksum(void *buf, size_t size)
  */
 void read_cdat_data(struct cxl_port *port)
 {
-	struct pci_doe_mb *cdat_doe;
-	struct device *dev = &port->dev;
 	struct device *uport = port->uport;
-	struct cxl_memdev *cxlmd = to_cxl_memdev(uport);
-	struct cxl_dev_state *cxlds = cxlmd->cxlds;
-	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
+	struct device *dev = &port->dev;
+	struct cxl_dev_state *cxlds;
+	struct pci_doe_mb *cdat_doe;
+	struct cxl_memdev *cxlmd;
+	struct pci_dev *pdev;
 	size_t cdat_length;
 	void *cdat_table;
 	int rc;
 
+	if (is_cxl_memdev(uport)) {
+		cxlmd = to_cxl_memdev(uport);
+		cxlds = cxlmd->cxlds;
+		pdev = to_pci_dev(cxlds->dev);
+	} else if (dev_is_pci(uport)) {
+		pdev = to_pci_dev(uport);
+	} else {
+		return;
+	}
+
 	cdat_doe = pci_find_doe_mailbox(pdev, PCI_DVSEC_VENDOR_ID_CXL,
 					CXL_DOE_PROTOCOL_TABLE_ACCESS);
 	if (!cdat_doe) {
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 1049bb5ea496..60a865680e22 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -93,9 +93,6 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
 	if (IS_ERR(cxlhdm))
 		return PTR_ERR(cxlhdm);
 
-	/* Cache the data early to ensure is_visible() works */
-	read_cdat_data(port);
-
 	get_device(&cxlmd->dev);
 	rc = devm_add_action_or_reset(&port->dev, schedule_detach, cxlmd);
 	if (rc)
@@ -135,6 +132,9 @@ static int cxl_port_probe(struct device *dev)
 {
 	struct cxl_port *port = to_cxl_port(dev);
 
+	/* Cache the data early to ensure is_visible() works */
+	read_cdat_data(port);
+
 	if (is_cxl_endpoint(port))
 		return cxl_endpoint_port_probe(port);
 	return cxl_switch_port_probe(port);
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index fba7bec96acd..2637c71f3378 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -12,6 +12,7 @@ ldflags-y += --wrap=cxl_await_media_ready
 ldflags-y += --wrap=cxl_hdm_decode_init
 ldflags-y += --wrap=cxl_dvsec_rr_decode
 ldflags-y += --wrap=cxl_rcrb_to_component
+ldflags-y += --wrap=read_cdat_data
 
 DRIVERS := ../../../drivers
 CXL_SRC := $(DRIVERS)/cxl
diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
index c4e53f22e421..3a75909b2aae 100644
--- a/tools/testing/cxl/test/mock.c
+++ b/tools/testing/cxl/test/mock.c
@@ -263,6 +263,11 @@ resource_size_t __wrap_cxl_rcrb_to_component(struct device *dev,
 }
 EXPORT_SYMBOL_NS_GPL(__wrap_cxl_rcrb_to_component, CXL);
 
+void __wrap_read_cdat_data(struct cxl_port *port)
+{
+}
+EXPORT_SYMBOL_NS_GPL(__wrap_read_cdat_data, CXL);
+
 MODULE_LICENSE("GPL v2");
 MODULE_IMPORT_NS(ACPI);
 MODULE_IMPORT_NS(CXL);



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 04/21] cxl: Add common helpers for cdat parsing
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (2 preceding siblings ...)
  2023-03-27 21:44 ` [PATCH v2 03/21] cxl: Add support for reading CXL switch CDAT table Dave Jiang
@ 2023-03-27 21:44 ` Dave Jiang
  2023-03-27 21:44 ` [PATCH v2 05/21] cxl: Add callback to parse the DSMAS subtables from CDAT Dave Jiang
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:44 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Add helper functions to parse the CDAT table and provide a callback to
parse the sub-table. Helpers are provided for DSMAS and DSLBIS sub-table
parsing. The code is patterned after the ACPI table parsing helpers.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v2:
- Use local headers to handle LE instead of ACPI header
- Reduce complexity of parser function. (Jonathan)
- Directly access header type. (Jonathan)
- Simplify header ptr math. (Jonathan)
- Move parsed counter to the correct location. (Jonathan)
- Add LE to host conversion for entry length
---
 drivers/cxl/core/Makefile |    1 
 drivers/cxl/core/cdat.c   |  100 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxlpci.h      |   29 +++++++++++++
 3 files changed, 130 insertions(+)
 create mode 100644 drivers/cxl/core/cdat.c

diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index ca4ae31d8f57..867a8014b462 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -12,5 +12,6 @@ cxl_core-y += memdev.o
 cxl_core-y += mbox.o
 cxl_core-y += pci.o
 cxl_core-y += hdm.o
+cxl_core-y += cdat.o
 cxl_core-$(CONFIG_TRACING) += trace.o
 cxl_core-$(CONFIG_CXL_REGION) += region.o
diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
new file mode 100644
index 000000000000..210f4499bddb
--- /dev/null
+++ b/drivers/cxl/core/cdat.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2023 Intel Corporation. All rights reserved. */
+#include "cxlpci.h"
+#include "cxl.h"
+
+static bool has_handler(struct cdat_subtable_proc *proc)
+{
+	return proc->handler;
+}
+
+static int call_handler(struct cdat_subtable_proc *proc,
+			struct cdat_subtable_entry *ent)
+{
+	if (has_handler(proc))
+		return proc->handler(ent->hdr, proc->arg);
+	return -EINVAL;
+}
+
+static bool cdat_is_subtable_match(struct cdat_subtable_entry *ent)
+{
+	return ent->hdr->type == ent->type;
+}
+
+static int cdat_table_parse_entries(enum cdat_type type,
+				    struct cdat_header *table_header,
+				    struct cdat_subtable_proc *proc)
+{
+	unsigned long table_end, entry_len;
+	struct cdat_subtable_entry entry;
+	int count = 0;
+	int rc;
+
+	if (!has_handler(proc))
+		return -EINVAL;
+
+	table_end = (unsigned long)table_header + table_header->length;
+
+	if (type >= CDAT_TYPE_RESERVED)
+		return -EINVAL;
+
+	entry.type = type;
+	entry.hdr = (struct cdat_entry_header *)(table_header + 1);
+
+	while ((unsigned long)entry.hdr < table_end) {
+		entry_len = le16_to_cpu(entry.hdr->length);
+
+		if ((unsigned long)entry.hdr + entry_len > table_end)
+			return -EINVAL;
+
+		if (entry_len == 0)
+			return -EINVAL;
+
+		if (cdat_is_subtable_match(&entry)) {
+			rc = call_handler(proc, &entry);
+			if (rc)
+				return rc;
+			count++;
+		}
+
+		entry.hdr = (struct cdat_entry_header *)((unsigned long)entry.hdr + entry_len);
+	}
+
+	return count;
+}
+
+int cdat_table_parse_dsmas(struct cdat_header *table,
+			   cdat_tbl_entry_handler handler, void *arg)
+{
+	struct cdat_subtable_proc proc = {
+		.handler	= handler,
+		.arg		= arg,
+	};
+
+	return cdat_table_parse_entries(CDAT_TYPE_DSMAS, table, &proc);
+}
+EXPORT_SYMBOL_NS_GPL(cdat_table_parse_dsmas, CXL);
+
+int cdat_table_parse_dslbis(struct cdat_header *table,
+			    cdat_tbl_entry_handler handler, void *arg)
+{
+	struct cdat_subtable_proc proc = {
+		.handler	= handler,
+		.arg		= arg,
+	};
+
+	return cdat_table_parse_entries(CDAT_TYPE_DSLBIS, table, &proc);
+}
+EXPORT_SYMBOL_NS_GPL(cdat_table_parse_dslbis, CXL);
+
+int cdat_table_parse_sslbis(struct cdat_header *table,
+			    cdat_tbl_entry_handler handler, void *arg)
+{
+	struct cdat_subtable_proc proc = {
+		.handler	= handler,
+		.arg		= arg,
+	};
+
+	return cdat_table_parse_entries(CDAT_TYPE_SSLBIS, table, &proc);
+}
+EXPORT_SYMBOL_NS_GPL(cdat_table_parse_sslbis, CXL);
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 0465ef963cd6..45e2f2bf5ef8 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -76,12 +76,34 @@ struct cdat_header {
 	__le32 sequence;
 } __packed;
 
+enum cdat_type {
+	CDAT_TYPE_DSMAS = 0,
+	CDAT_TYPE_DSLBIS,
+	CDAT_TYPE_DSMSCIS,
+	CDAT_TYPE_DSIS,
+	CDAT_TYPE_DSEMTS,
+	CDAT_TYPE_SSLBIS,
+	CDAT_TYPE_RESERVED
+};
+
 struct cdat_entry_header {
 	u8 type;
 	u8 reserved;
 	__le16 length;
 } __packed;
 
+typedef int (*cdat_tbl_entry_handler)(struct cdat_entry_header *header, void *arg);
+
+struct cdat_subtable_proc {
+	cdat_tbl_entry_handler handler;
+	void *arg;
+};
+
+struct cdat_subtable_entry {
+	struct cdat_entry_header *hdr;
+	enum cdat_type type;
+};
+
 int devm_cxl_port_enumerate_dports(struct cxl_port *port);
 struct cxl_dev_state;
 int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
@@ -90,4 +112,11 @@ void read_cdat_data(struct cxl_port *port);
 void cxl_cor_error_detected(struct pci_dev *pdev);
 pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 				    pci_channel_state_t state);
+
+#define cdat_table_parse(x) \
+int cdat_table_parse_##x(struct cdat_header *table, \
+			 cdat_tbl_entry_handler handler, void *arg)
+cdat_table_parse(dsmas);
+cdat_table_parse(dslbis);
+cdat_table_parse(sslbis);
 #endif /* __CXL_PCI_H__ */



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 05/21] cxl: Add callback to parse the DSMAS subtables from CDAT
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (3 preceding siblings ...)
  2023-03-27 21:44 ` [PATCH v2 04/21] cxl: Add common helpers for cdat parsing Dave Jiang
@ 2023-03-27 21:44 ` Dave Jiang
  2023-03-29  0:20   ` Alison Schofield
  2023-03-30 15:43   ` Dave Jiang
  2023-03-27 21:44 ` [PATCH v2 06/21] cxl: Add callback to parse the DSLBIS subtable " Dave Jiang
                   ` (16 subsequent siblings)
  21 siblings, 2 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:44 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Provide a callback function to the CDAT parser in order to parse the Device
Scoped Memory Affinity Structure (DSMAS). Each DSMAS structure contains the
DPA range and its associated attributes in each entry. See the CDAT
specification for details.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v2:
- Add DSMAS table size check. (Lukas)
- Use local DSMAS header for LE handling.
- Remove dsmas lock. (Jonathan)
- Fix handle size (Jonathan)
- Add LE to host conversion for DSMAS address and length.
- Make dsmas_list local
---
 drivers/cxl/core/cdat.c |   26 ++++++++++++++++++++++++++
 drivers/cxl/cxl.h       |    1 +
 drivers/cxl/cxlpci.h    |   18 ++++++++++++++++++
 drivers/cxl/port.c      |   24 ++++++++++++++++++++++++
 4 files changed, 69 insertions(+)

diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index 210f4499bddb..d068609fb6f9 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -98,3 +98,29 @@ int cdat_table_parse_sslbis(struct cdat_header *table,
 	return cdat_table_parse_entries(CDAT_TYPE_SSLBIS, table, &proc);
 }
 EXPORT_SYMBOL_NS_GPL(cdat_table_parse_sslbis, CXL);
+
+int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg)
+{
+	struct cdat_dsmas *dsmas = (struct cdat_dsmas *)(header);
+	struct list_head *dsmas_list = (struct list_head *)arg;
+	struct dsmas_entry *dent;
+
+	if (dsmas->hdr.length != sizeof(*dsmas)) {
+		pr_warn("Malformed DSMAS table length: (%lu:%u)\n",
+			 (unsigned long)sizeof(*dsmas), dsmas->hdr.length);
+		return -EINVAL;
+	}
+
+	dent = kzalloc(sizeof(*dent), GFP_KERNEL);
+	if (!dent)
+		return -ENOMEM;
+
+	dent->handle = dsmas->dsmad_handle;
+	dent->dpa_range.start = le64_to_cpu(dsmas->dpa_base_address);
+	dent->dpa_range.end = le64_to_cpu(dsmas->dpa_base_address) +
+			      le64_to_cpu(dsmas->dpa_length) - 1;
+	list_add_tail(&dent->list, dsmas_list);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index cc3309794b45..9d0e22fe72c0 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -8,6 +8,7 @@
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/log2.h>
+#include <linux/list.h>
 #include <linux/io.h>
 
 /**
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 45e2f2bf5ef8..9a2468a93d83 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -104,6 +104,22 @@ struct cdat_subtable_entry {
 	enum cdat_type type;
 };
 
+struct dsmas_entry {
+	struct list_head list;
+	struct range dpa_range;
+	u8 handle;
+};
+
+/* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */
+struct cdat_dsmas {
+	struct cdat_entry_header hdr;
+	u8 dsmad_handle;
+	u8 flags;
+	__u16 reserved;
+	__le64 dpa_base_address;
+	__le64 dpa_length;
+} __packed;
+
 int devm_cxl_port_enumerate_dports(struct cxl_port *port);
 struct cxl_dev_state;
 int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
@@ -119,4 +135,6 @@ int cdat_table_parse_##x(struct cdat_header *table, \
 cdat_table_parse(dsmas);
 cdat_table_parse(dslbis);
 cdat_table_parse(sslbis);
+
+int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 60a865680e22..c8136797d528 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -57,6 +57,16 @@ static int discover_region(struct device *dev, void *root)
 	return 0;
 }
 
+static void dsmas_list_destroy(struct list_head *dsmas_list)
+{
+	struct dsmas_entry *dentry, *n;
+
+	list_for_each_entry_safe(dentry, n, dsmas_list, list) {
+		list_del(&dentry->list);
+		kfree(dentry);
+	}
+}
+
 static int cxl_switch_port_probe(struct cxl_port *port)
 {
 	struct cxl_hdm *cxlhdm;
@@ -131,9 +141,23 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
 static int cxl_port_probe(struct device *dev)
 {
 	struct cxl_port *port = to_cxl_port(dev);
+	int rc;
 
 	/* Cache the data early to ensure is_visible() works */
 	read_cdat_data(port);
+	if (port->cdat.table) {
+		if (is_cxl_endpoint(port)) {
+			LIST_HEAD(dsmas_list);
+
+			rc = cdat_table_parse_dsmas(port->cdat.table,
+						    cxl_dsmas_parse_entry,
+						    (void *)&dsmas_list);
+			if (rc < 0)
+				dev_warn(dev, "Failed to parse DSMAS: %d\n", rc);
+
+			dsmas_list_destroy(&dsmas_list);
+		}
+	}
 
 	if (is_cxl_endpoint(port))
 		return cxl_endpoint_port_probe(port);



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 06/21] cxl: Add callback to parse the DSLBIS subtable from CDAT
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (4 preceding siblings ...)
  2023-03-27 21:44 ` [PATCH v2 05/21] cxl: Add callback to parse the DSMAS subtables from CDAT Dave Jiang
@ 2023-03-27 21:44 ` Dave Jiang
  2023-03-29  0:44   ` Alison Schofield
  2023-03-27 21:44 ` [PATCH v2 07/21] cxl: Add callback to parse the SSLBIS " Dave Jiang
                   ` (15 subsequent siblings)
  21 siblings, 1 reply; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:44 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Provide a callback to parse the Device Scoped Latency and Bandwidth
Information Structure (DSLBIS) in the CDAT structures. The DSLBIS
contains the bandwidth and latency information that's tied to a DSMAS
handle. The driver will retrieve the read and write latency and
bandwidth associated with the DSMAS which is tied to a DPA range.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v2:
- Add size check to DSLIBIS table. (Lukas)
- Remove unnecessary entry type check. (Jonathan)
- Move data_type check to after match. (Jonathan)
- Skip unknown data type. (Jonathan)
- Add overflow check for unit multiply. (Jonathan)
- Use dev_warn() when entries parsing fail. (Jonathan)
---
 drivers/cxl/core/cdat.c |   41 +++++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxlpci.h    |   34 +++++++++++++++++++++++++++++++++-
 drivers/cxl/port.c      |    9 ++++++++-
 3 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index d068609fb6f9..0e88973e9f38 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright(c) 2023 Intel Corporation. All rights reserved. */
+#include <linux/overflow.h>
 #include "cxlpci.h"
 #include "cxl.h"
 
@@ -124,3 +125,43 @@ int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg)
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL);
+
+int cxl_dslbis_parse_entry(struct cdat_entry_header *header, void *arg)
+{
+	struct cdat_dslbis *dslbis = (struct cdat_dslbis *)header;
+	struct list_head *dsmas_list = (struct list_head *)arg;
+	struct dsmas_entry *dent;
+
+	if (dslbis->hdr.length != sizeof(*dslbis)) {
+		pr_warn("Malformed DSLBIS table length: (%lu:%u)\n",
+			(unsigned long)sizeof(*dslbis), dslbis->hdr.length);
+		return -EINVAL;
+	}
+
+	/* Unrecognized data type, we can skip */
+	if (dslbis->data_type >= HMAT_SLLBIS_DATA_TYPE_MAX)
+		return 0;
+
+	list_for_each_entry(dent, dsmas_list, list) {
+		u64 val;
+		int rc;
+
+		if (dslbis->handle != dent->handle)
+			continue;
+
+		/* Not a memory type, skip */
+		if ((dslbis->flags & DSLBIS_MEM_MASK) != DSLBIS_MEM_MEMORY)
+			return 0;
+
+		rc = check_mul_overflow(le64_to_cpu(dslbis->entry_base_unit),
+					le16_to_cpu(dslbis->entry[0]), &val);
+		if (unlikely(rc))
+			pr_warn("DSLBIS value overflowed!\n");
+
+		dent->qos[dslbis->data_type] = val;
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_dslbis_parse_entry, CXL);
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 9a2468a93d83..1429de49e0c4 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -104,10 +104,21 @@ struct cdat_subtable_entry {
 	enum cdat_type type;
 };
 
+enum {
+	HMAT_SLLBIS_ACCESS_LATENCY = 0,
+	HMAT_SLLBIS_READ_LATENCY,
+	HMAT_SLLBIS_WRITE_LATENCY,
+	HMAT_SLLBIS_ACCESS_BANDWIDTH,
+	HMAT_SLLBIS_READ_BANDWIDTH,
+	HMAT_SLLBIS_WRITE_BANDWIDTH,
+	HMAT_SLLBIS_DATA_TYPE_MAX
+};
+
 struct dsmas_entry {
 	struct list_head list;
 	struct range dpa_range;
 	u8 handle;
+	u64 qos[HMAT_SLLBIS_DATA_TYPE_MAX];
 };
 
 /* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */
@@ -120,6 +131,23 @@ struct cdat_dsmas {
 	__le64 dpa_length;
 } __packed;
 
+/* Sub-table 1: Device Scoped Latency and Bandwidth Information Structure (DSLBIS) */
+struct cdat_dslbis {
+	struct cdat_entry_header hdr;
+	u8 handle;
+	u8 flags;
+	u8 data_type;
+	u8 reserved;
+	__le64 entry_base_unit;
+	__le16 entry[3];
+	__le16 reserved2;
+} __packed;
+
+/* Flags for subtable above */
+
+#define DSLBIS_MEM_MASK		GENMASK(3, 0)
+#define DSLBIS_MEM_MEMORY	0
+
 int devm_cxl_port_enumerate_dports(struct cxl_port *port);
 struct cxl_dev_state;
 int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
@@ -136,5 +164,9 @@ cdat_table_parse(dsmas);
 cdat_table_parse(dslbis);
 cdat_table_parse(sslbis);
 
-int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg);
+#define cxl_parse_entry(x) \
+int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg)
+
+cxl_parse_entry(dsmas);
+cxl_parse_entry(dslbis);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index c8136797d528..6f2b327f7128 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -152,8 +152,15 @@ static int cxl_port_probe(struct device *dev)
 			rc = cdat_table_parse_dsmas(port->cdat.table,
 						    cxl_dsmas_parse_entry,
 						    (void *)&dsmas_list);
-			if (rc < 0)
+			if (rc > 0) {
+				rc = cdat_table_parse_dslbis(port->cdat.table,
+							     cxl_dslbis_parse_entry,
+							     (void *)&dsmas_list);
+				if (rc <= 0)
+					dev_warn(dev, "Failed to parse DSLBIS: %d\n", rc);
+			} else {
 				dev_warn(dev, "Failed to parse DSMAS: %d\n", rc);
+			}
 
 			dsmas_list_destroy(&dsmas_list);
 		}



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 07/21] cxl: Add callback to parse the SSLBIS subtable from CDAT
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (5 preceding siblings ...)
  2023-03-27 21:44 ` [PATCH v2 06/21] cxl: Add callback to parse the DSLBIS subtable " Dave Jiang
@ 2023-03-27 21:44 ` Dave Jiang
  2023-03-27 21:44 ` [PATCH v2 08/21] cxl: Add support for _DSM Function for retrieving QTG ID Dave Jiang
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:44 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Provide a callback to parse the Switched Scoped Latency and Bandwidth
Information Structure (DSLBIS) in the CDAT structures. The SSLBIS
contains the bandwidth and latency information that's tied to the
CLX switch that the data table has been read from. The extracted
values are indexed by the downstream port id. It is possible
the downstream port id is 0xffff which is a wildcard value for any
port id.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/cdat.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/cxl/core/port.c |    5 +++
 drivers/cxl/cxl.h       |    1 +
 drivers/cxl/cxlpci.h    |   24 +++++++++++++++
 drivers/cxl/port.c      |    6 ++++
 5 files changed, 112 insertions(+)

diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index 0e88973e9f38..0e7a4f74fcf8 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -165,3 +165,79 @@ int cxl_dslbis_parse_entry(struct cdat_entry_header *header, void *arg)
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_dslbis_parse_entry, CXL);
+
+int cxl_sslbis_parse_entry(struct cdat_entry_header *header, void *arg)
+{
+	struct cdat_sslbis *sslbis = (struct cdat_sslbis *)header;
+	struct xarray *sslbis_xa = (struct xarray *)arg;
+	int remain, entries, i;
+
+	remain = sslbis->hdr.length - sizeof(*sslbis);
+	if (!remain || remain % sizeof(struct sslbis_sslbe)) {
+		pr_warn("Malformed SSLBIS table length: (%u)\n",
+			sslbis->hdr.length);
+		return -EINVAL;
+	}
+
+	/* Unrecognized data type, we can skip */
+	if (sslbis->data_type >= HMAT_SLLBIS_DATA_TYPE_MAX)
+		return 0;
+
+	entries = remain / sizeof(*sslbis);
+
+	for (i = 0; i < entries; i++) {
+		struct sslbis_sslbe *sslbe = &sslbis->sslbe[i];
+		u16 x = le16_to_cpu(sslbe->port_x_id);
+		u16 y = le16_to_cpu(sslbe->port_y_id);
+		struct sslbis_entry *sentry;
+		u16 dsp_id;
+		u64 val;
+		int rc;
+
+		switch (x) {
+		case SSLBIS_US_PORT:
+			dsp_id = y;
+			break;
+		case SSLBIS_ANY_PORT:
+			switch (y) {
+			case SSLBIS_US_PORT:
+				dsp_id = x;
+				break;
+			case SSLBIS_ANY_PORT:
+				dsp_id = SSLBIS_ANY_PORT;
+				break;
+			default:
+				dsp_id = y;
+				break;
+			}
+			break;
+		default:
+			dsp_id = x;
+			break;
+		}
+
+		sentry = xa_load(sslbis_xa, dsp_id);
+		if (xa_is_err(sentry))
+			return xa_err(sentry);
+		if (!sentry) {
+			sentry = kzalloc(sizeof(*sentry), GFP_KERNEL);
+			if (!sentry)
+				return -ENOMEM;
+		}
+
+		rc = check_mul_overflow(le64_to_cpu(sslbis->entry_base_unit),
+					le16_to_cpu(sslbe->value), &val);
+		if (unlikely(rc))
+			pr_warn("SSLBIS value overflowed!\n");
+
+		sentry->qos[sslbis->data_type] = val;
+		rc = xa_insert(sslbis_xa, dsp_id, sentry, GFP_KERNEL);
+		if (rc < 0 && rc != -EBUSY) {
+			kfree(sentry);
+			return rc;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_sslbis_parse_entry, CXL);
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 5ec48dddb2f9..a61f9395a209 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -518,6 +518,7 @@ static void cxl_ep_remove(struct cxl_port *port, struct cxl_ep *ep)
 static void cxl_port_release(struct device *dev)
 {
 	struct cxl_port *port = to_cxl_port(dev);
+	struct sslbis_entry *sentry;
 	unsigned long index;
 	struct cxl_ep *ep;
 
@@ -526,6 +527,9 @@ static void cxl_port_release(struct device *dev)
 	xa_destroy(&port->endpoints);
 	xa_destroy(&port->dports);
 	xa_destroy(&port->regions);
+	xa_for_each(&port->cdat.sslbis_xa, index, sentry)
+		kfree(sentry);
+	xa_destroy(&port->cdat.sslbis_xa);
 	ida_free(&cxl_port_ida, port->id);
 	kfree(port);
 }
@@ -684,6 +688,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport,
 	xa_init(&port->dports);
 	xa_init(&port->endpoints);
 	xa_init(&port->regions);
+	xa_init(&port->cdat.sslbis_xa);
 
 	device_initialize(dev);
 	lockdep_set_class_and_subclass(&dev->mutex, &cxl_port_key, port->depth);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 9d0e22fe72c0..50ac74f66cbd 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -581,6 +581,7 @@ struct cxl_port {
 	struct cxl_cdat {
 		void *table;
 		size_t length;
+		struct xarray sslbis_xa;
 	} cdat;
 	bool cdat_available;
 };
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 1429de49e0c4..1c9e8b078369 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -121,6 +121,10 @@ struct dsmas_entry {
 	u64 qos[HMAT_SLLBIS_DATA_TYPE_MAX];
 };
 
+struct sslbis_entry {
+	u64 qos[HMAT_SLLBIS_DATA_TYPE_MAX];
+};
+
 /* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */
 struct cdat_dsmas {
 	struct cdat_entry_header hdr;
@@ -148,6 +152,25 @@ struct cdat_dslbis {
 #define DSLBIS_MEM_MASK		GENMASK(3, 0)
 #define DSLBIS_MEM_MEMORY	0
 
+struct sslbis_sslbe {
+	__le16 port_x_id;
+	__le16 port_y_id;
+	__le16 value;	/* latency or bandwidth */
+	__le16 reserved;
+} __packed;
+
+/* Sub-table 5: Switch Scoped Latency and Bandwidth Information Structure (SSLBIS) */
+struct cdat_sslbis {
+	struct cdat_entry_header hdr;
+	u8 data_type;
+	u8 reserved[3];
+	__le64 entry_base_unit;
+	struct sslbis_sslbe sslbe[];
+} __packed;
+
+#define SSLBIS_US_PORT		0x0100
+#define SSLBIS_ANY_PORT		0xffff
+
 int devm_cxl_port_enumerate_dports(struct cxl_port *port);
 struct cxl_dev_state;
 int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
@@ -169,4 +192,5 @@ int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg)
 
 cxl_parse_entry(dsmas);
 cxl_parse_entry(dslbis);
+cxl_parse_entry(sslbis);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 6f2b327f7128..7839e0244d0d 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -163,6 +163,12 @@ static int cxl_port_probe(struct device *dev)
 			}
 
 			dsmas_list_destroy(&dsmas_list);
+		} else {
+			rc = cdat_table_parse_sslbis(port->cdat.table,
+						     cxl_sslbis_parse_entry,
+						     (void *)&port->cdat.sslbis_xa);
+			if (rc <= 0)
+				dev_warn(dev, "Failed to parse SSLBIS: %d\n", rc);
 		}
 	}
 



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 08/21] cxl: Add support for _DSM Function for retrieving QTG ID
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (6 preceding siblings ...)
  2023-03-27 21:44 ` [PATCH v2 07/21] cxl: Add callback to parse the SSLBIS " Dave Jiang
@ 2023-03-27 21:44 ` Dave Jiang
  2023-03-27 21:44 ` [PATCH v2 09/21] cxl: Add helper function to retrieve ACPI handle of CXL root device Dave Jiang
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:44 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

CXL spec v3.0 9.17.3 CXL Root Device Specific Methods (_DSM)

Add support to retrieve QTG ID via ACPI _DSM call. The _DSM call requires
an input of an ACPI package with 4 dwords (read latency, write latency,
read bandwidth, write bandwidth). The call returns a package with 1 WORD
that provides the max supported QTG ID and a package that may contain 0 or
more WORDs as the recommended QTG IDs in the recommended order.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v2:
- Reorder var declaration and use C99 style. (Jonathan)
- Allow >2 ACPI objects in package for future expansion. (Jonathan)
- Check QTG IDs against MAX QTG ID provided by output package. (Jonathan)
---
 drivers/cxl/core/Makefile |    1 
 drivers/cxl/core/acpi.c   |  116 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h         |   16 ++++++
 3 files changed, 133 insertions(+)
 create mode 100644 drivers/cxl/core/acpi.c

diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index 867a8014b462..30d61c8cae22 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -13,5 +13,6 @@ cxl_core-y += mbox.o
 cxl_core-y += pci.o
 cxl_core-y += hdm.o
 cxl_core-y += cdat.o
+cxl_core-y += acpi.o
 cxl_core-$(CONFIG_TRACING) += trace.o
 cxl_core-$(CONFIG_CXL_REGION) += region.o
diff --git a/drivers/cxl/core/acpi.c b/drivers/cxl/core/acpi.c
new file mode 100644
index 000000000000..6eda5cad8d59
--- /dev/null
+++ b/drivers/cxl/core/acpi.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/acpi.h>
+#include <linux/pci.h>
+#include <asm/div64.h>
+#include "cxlpci.h"
+#include "cxl.h"
+
+const guid_t acpi_cxl_qtg_id_guid =
+	GUID_INIT(0xF365F9A6, 0xA7DE, 0x4071,
+		  0xA6, 0x6A, 0xB4, 0x0C, 0x0B, 0x4F, 0x8E, 0x52);
+
+/**
+ * cxl_acpi_evaluate_qtg_dsm - Retrieve QTG ids via ACPI _DSM
+ * @handle: ACPI handle
+ * @input: bandwidth and latency data
+ *
+ * Issue QTG _DSM with accompanied bandwidth and latency data in order to get
+ * the QTG IDs that falls within the performance data.
+ */
+struct qtg_dsm_output *cxl_acpi_evaluate_qtg_dsm(acpi_handle handle,
+						 struct qtg_dsm_input *input)
+{
+	union acpi_object *out_obj, *out_buf, *pkg;
+	union acpi_object in_buf = {
+		.buffer = {
+			.type = ACPI_TYPE_BUFFER,
+			.pointer = (u8 *)input,
+			.length = sizeof(u32) * 4,
+		},
+	};
+	union acpi_object in_obj = {
+		.package = {
+			.type = ACPI_TYPE_PACKAGE,
+			.count = 1,
+			.elements = &in_buf
+		},
+	};
+	struct qtg_dsm_output *output = NULL;
+	int len, rc, i;
+	u16 *max_qtg;
+
+	out_obj = acpi_evaluate_dsm(handle, &acpi_cxl_qtg_id_guid, 1, 1, &in_obj);
+	if (!out_obj)
+		return ERR_PTR(-ENXIO);
+
+	if (out_obj->type != ACPI_TYPE_PACKAGE) {
+		rc = -ENXIO;
+		goto err;
+	}
+
+	/* Check Max QTG ID */
+	pkg = &out_obj->package.elements[0];
+	if (pkg->type != ACPI_TYPE_BUFFER) {
+		rc = -ENXIO;
+		goto err;
+	}
+
+	if (pkg->buffer.length != sizeof(u16)) {
+		rc = -ENXIO;
+		goto err;
+	}
+	max_qtg = (u16 *)pkg->buffer.pointer;
+
+	/* Retrieve QTG IDs package */
+	pkg = &out_obj->package.elements[1];
+	if (pkg->type != ACPI_TYPE_PACKAGE) {
+		rc = -ENXIO;
+		goto err;
+	}
+
+	out_buf = &pkg->package.elements[0];
+	if (out_buf->type != ACPI_TYPE_BUFFER) {
+		rc = -ENXIO;
+		goto err;
+	}
+
+	len = out_buf->buffer.length;
+
+	/* It's legal to have 0 QTG entries */
+	if (len == 0)
+		goto out;
+
+	/* Malformed package, not multiple of WORD size */
+	if (len % sizeof(u16)) {
+		rc = -ENXIO;
+		goto out;
+	}
+
+	output = kmalloc(len + sizeof(*output), GFP_KERNEL);
+	if (!output) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	output->nr = len / sizeof(u16);
+	memcpy(output->qtg_ids, out_buf->buffer.pointer, len);
+
+	for (i = 0; i < output->nr; i++) {
+		if (output->qtg_ids[i] > *max_qtg)
+			pr_warn("QTG ID %u greater than MAX %u\n",
+				output->qtg_ids[i], *max_qtg);
+	}
+
+out:
+	ACPI_FREE(out_obj);
+	return output;
+
+err:
+	ACPI_FREE(out_obj);
+	return ERR_PTR(rc);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_acpi_evaluate_qtg_dsm, CXL);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 50ac74f66cbd..04b8a032bd14 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -7,6 +7,7 @@
 #include <linux/libnvdimm.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
+#include <linux/acpi.h>
 #include <linux/log2.h>
 #include <linux/list.h>
 #include <linux/io.h>
@@ -791,6 +792,21 @@ static inline struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
 }
 #endif
 
+struct qtg_dsm_input {
+	u32 rd_lat;
+	u32 wr_lat;
+	u32 rd_bw;
+	u32 wr_bw;
+};
+
+struct qtg_dsm_output {
+	int nr;
+	u16 qtg_ids[];
+};
+
+struct qtg_dsm_output *cxl_acpi_evaluate_qtg_dsm(acpi_handle handle,
+						 struct qtg_dsm_input *input);
+
 /*
  * Unit test builds overrides this to __weak, find the 'strong' version
  * of these symbols in tools/testing/cxl/.



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 09/21] cxl: Add helper function to retrieve ACPI handle of CXL root device
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (7 preceding siblings ...)
  2023-03-27 21:44 ` [PATCH v2 08/21] cxl: Add support for _DSM Function for retrieving QTG ID Dave Jiang
@ 2023-03-27 21:44 ` Dave Jiang
  2023-03-27 21:45 ` [PATCH v2 10/21] cxl: Add helpers to calculate pci latency for the CXL device Dave Jiang
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:44 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Provide a helper to find the ACPI0017 device in order to issue the _DSM.
The helper will take the 'struct device' from a cxl_port and iterate until
the root device is reached. The ACPI handle will be returned from the root
device.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v2:
- Fix commenting style. (Jonathan)
- Fix var declaration aligning. (Jonathan)
---
 drivers/cxl/core/acpi.c |   34 ++++++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h       |    1 +
 2 files changed, 35 insertions(+)

diff --git a/drivers/cxl/core/acpi.c b/drivers/cxl/core/acpi.c
index 6eda5cad8d59..191644d0ca6d 100644
--- a/drivers/cxl/core/acpi.c
+++ b/drivers/cxl/core/acpi.c
@@ -5,6 +5,7 @@
 #include <linux/kernel.h>
 #include <linux/acpi.h>
 #include <linux/pci.h>
+#include <linux/platform_device.h>
 #include <asm/div64.h>
 #include "cxlpci.h"
 #include "cxl.h"
@@ -13,6 +14,39 @@ const guid_t acpi_cxl_qtg_id_guid =
 	GUID_INIT(0xF365F9A6, 0xA7DE, 0x4071,
 		  0xA6, 0x6A, 0xB4, 0x0C, 0x0B, 0x4F, 0x8E, 0x52);
 
+/**
+ * cxl_acpi_get_rootdev_handle - get the ACPI handle of the CXL root device
+ * @dev: 'struct device' to start searching from. Should be from cxl_port->dev.
+ *
+ * Return: acpi_handle on success, errptr of errno on error.
+ *
+ * Looks for the ACPI0017 device and return the ACPI handle
+ **/
+acpi_handle cxl_acpi_get_rootdev_handle(struct device *dev)
+{
+	struct device *itr = dev;
+	struct device *root_dev;
+	acpi_handle handle;
+
+	if (!dev)
+		return ERR_PTR(-EINVAL);
+
+	while (itr->parent) {
+		root_dev = itr;
+		itr = itr->parent;
+	}
+
+	if (!dev_is_platform(root_dev))
+		return ERR_PTR(-ENODEV);
+
+	handle = ACPI_HANDLE(root_dev);
+	if (!handle)
+		return ERR_PTR(-ENODEV);
+
+	return handle;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_acpi_get_rootdev_handle, CXL);
+
 /**
  * cxl_acpi_evaluate_qtg_dsm - Retrieve QTG ids via ACPI _DSM
  * @handle: ACPI handle
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 04b8a032bd14..dc6da641ced0 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -806,6 +806,7 @@ struct qtg_dsm_output {
 
 struct qtg_dsm_output *cxl_acpi_evaluate_qtg_dsm(acpi_handle handle,
 						 struct qtg_dsm_input *input);
+acpi_handle cxl_acpi_get_rootdev_handle(struct device *dev);
 
 /*
  * Unit test builds overrides this to __weak, find the 'strong' version



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 10/21] cxl: Add helpers to calculate pci latency for the CXL device
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (8 preceding siblings ...)
  2023-03-27 21:44 ` [PATCH v2 09/21] cxl: Add helper function to retrieve ACPI handle of CXL root device Dave Jiang
@ 2023-03-27 21:45 ` Dave Jiang
  2023-03-27 21:45 ` [PATCH v2 11/21] cxl: Add helper function that calculates QoS values for switches Dave Jiang
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:45 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

The latency is calculated by dividing the flit size over the bandwidth. Add
support to retrieve the flit size for the CXL device and calculate the
latency of the downstream link.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v2:
- Fix commit log issues. (Jonathan)
- Fix var declaration issues. (Jonathan)
---
 drivers/cxl/core/pci.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxlpci.h   |   15 +++++++++++
 drivers/cxl/pci.c      |   13 ---------
 3 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 4241c7b8d5c2..2f58cc54e108 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -712,3 +712,71 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 	return PCI_ERS_RESULT_NEED_RESET;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_error_detected, CXL);
+
+static int pcie_speed_to_mbps(enum pci_bus_speed speed)
+{
+	switch (speed) {
+	case PCIE_SPEED_2_5GT:
+		return 2500;
+	case PCIE_SPEED_5_0GT:
+		return 5000;
+	case PCIE_SPEED_8_0GT:
+		return 8000;
+	case PCIE_SPEED_16_0GT:
+		return 16000;
+	case PCIE_SPEED_32_0GT:
+		return 32000;
+	case PCIE_SPEED_64_0GT:
+		return 64000;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int cxl_pci_mbits_to_mbytes(struct pci_dev *pdev)
+{
+	int mbits;
+
+	mbits = pcie_speed_to_mbps(pdev->bus->cur_bus_speed);
+	if (mbits < 0)
+		return mbits;
+
+	return mbits >> 3;
+}
+
+static int cxl_flit_size(struct pci_dev *pdev)
+{
+	if (cxl_pci_flit_256(pdev))
+		return 256;
+
+	return 68;
+}
+
+/**
+ * cxl_pci_get_latency - calculate the link latency for the PCIe link
+ * @pdev - PCI device
+ *
+ * return: calculated latency or -errno
+ *
+ * CXL Memory Device SW Guide v1.0 2.11.4 Link latency calculation
+ * Link latency = LinkPropagationLatency + FlitLatency + RetimerLatency
+ * LinkProgationLatency is negligible, so 0 will be used
+ * RetimerLatency is assumed to be negligible and 0 will be used
+ * FlitLatency = FlitSize / LinkBandwidth
+ * FlitSize is defined by spec. CXL rev3.0 4.2.1.
+ * 68B flit is used up to 32GT/s. >32GT/s, 256B flit size is used.
+ * The FlitLatency is converted to picoseconds.
+ */
+long cxl_pci_get_latency(struct pci_dev *pdev)
+{
+	long bw;
+
+	bw = cxl_pci_mbits_to_mbytes(pdev);
+	if (bw < 0)
+		return bw;
+
+	return cxl_flit_size(pdev) * 1000000L / bw;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_pci_get_latency, CXL);
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 1c9e8b078369..815bf843018e 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -171,6 +171,19 @@ struct cdat_sslbis {
 #define SSLBIS_US_PORT		0x0100
 #define SSLBIS_ANY_PORT		0xffff
 
+/*
+ * CXL v3.0 6.2.3 Table 6-4
+ * The table indicates that if PCIe Flit Mode is set, then CXL is in 256B flits
+ * mode, otherwise it's 68B flits mode.
+ */
+static inline bool cxl_pci_flit_256(struct pci_dev *pdev)
+{
+	u16 lnksta2;
+
+	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &lnksta2);
+	return lnksta2 & PCI_EXP_LNKSTA2_FLIT;
+}
+
 int devm_cxl_port_enumerate_dports(struct cxl_port *port);
 struct cxl_dev_state;
 int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
@@ -193,4 +206,6 @@ int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg)
 cxl_parse_entry(dsmas);
 cxl_parse_entry(dslbis);
 cxl_parse_entry(sslbis);
+
+long cxl_pci_get_latency(struct pci_dev *pdev);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index ea38bd49b0cf..ed39d133b70d 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -365,19 +365,6 @@ static bool is_cxl_restricted(struct pci_dev *pdev)
 	return pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END;
 }
 
-/*
- * CXL v3.0 6.2.3 Table 6-4
- * The table indicates that if PCIe Flit Mode is set, then CXL is in 256B flits
- * mode, otherwise it's 68B flits mode.
- */
-static bool cxl_pci_flit_256(struct pci_dev *pdev)
-{
-	u16 lnksta2;
-
-	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &lnksta2);
-	return lnksta2 & PCI_EXP_LNKSTA2_FLIT;
-}
-
 static int cxl_pci_ras_unmask(struct pci_dev *pdev)
 {
 	struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus);



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 11/21] cxl: Add helper function that calculates QoS values for switches
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (9 preceding siblings ...)
  2023-03-27 21:45 ` [PATCH v2 10/21] cxl: Add helpers to calculate pci latency for the CXL device Dave Jiang
@ 2023-03-27 21:45 ` Dave Jiang
  2023-03-27 21:45 ` [PATCH v2 12/21] cxl: Add helper function that calculate QoS values for PCI path Dave Jiang
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:45 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

The CDAT information from the switch, Switch Scoped Latency and Bandwidth
Information Strucutre (SSLBIS), is parsed and stored in an xarray under the
cxl_port. The QoS data are indexed by the downstream port id.  Walk the CXL
ports from endpoint to root and retrieve the relevant QoS information
(bandwidth and latency) that are from the switch CDAT. If read or write QoS
values are not available, then use the access QoS value.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/port.c |   89 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h       |    2 +
 2 files changed, 91 insertions(+)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index a61f9395a209..6e2f8e40757e 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1945,6 +1945,95 @@ bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd)
 }
 EXPORT_SYMBOL_NS_GPL(schedule_cxl_memdev_detach, CXL);
 
+/**
+ * cxl_port_get_switch_qos - retrieve QoS data for CXL switches
+ * @port: endpoint cxl_port
+ * @rd_bw: writeback value for min read bandwidth
+ * @rd_lat: writeback value for total read latency
+ * @wr_bw: writeback value for min write bandwidth
+ * @wr_lat: writeback value for total write latency
+ *
+ * Return: Errno on failure, 0 on success. -ENOENT if no switch device
+ */
+int cxl_port_get_switch_qos(struct cxl_port *port, u64 *rd_bw, u64 *rd_lat,
+			    u64 *wr_bw, u64 *wr_lat)
+{
+	u64 min_rd_bw = ULONG_MAX;
+	u64 min_wr_bw = ULONG_MAX;
+	struct cxl_dport *dport;
+	struct cxl_port *nport;
+	u64 total_rd_lat = 0;
+	u64 total_wr_lat = 0;
+	struct device *next;
+	int switches = 0;
+	int rc = 0;
+
+	if (!is_cxl_endpoint(port))
+		return -EINVAL;
+
+	/* Skip the endpoint */
+	next = port->dev.parent;
+	nport = to_cxl_port(next);
+	dport = port->parent_dport;
+
+	do {
+		struct sslbis_entry *sentry;
+		u64 lat, bw;
+
+		if (!nport->cdat.table)
+			break;
+
+		if (!dev_is_pci(dport->dport))
+			break;
+
+		sentry = xa_load(&nport->cdat.sslbis_xa, dport->port_id);
+		if (xa_is_err(sentry))
+			return xa_err(sentry);
+
+		if (!sentry) {
+			sentry = xa_load(&nport->cdat.sslbis_xa, SSLBIS_ANY_PORT);
+			if (xa_is_err(sentry))
+				return xa_err(sentry);
+			if (!sentry)
+				return -ENXIO;
+		}
+
+		bw = sentry->qos[HMAT_SLLBIS_WRITE_BANDWIDTH];
+		if (!bw)
+			bw = sentry->qos[HMAT_SLLBIS_ACCESS_BANDWIDTH];
+		lat = sentry->qos[HMAT_SLLBIS_WRITE_LATENCY];
+		if (!lat)
+			lat = sentry->qos[HMAT_SLLBIS_ACCESS_LATENCY];
+		min_wr_bw = min_t(u64, min_wr_bw, bw);
+		total_wr_lat += lat;
+
+		bw = sentry->qos[HMAT_SLLBIS_READ_BANDWIDTH];
+		if (!bw)
+			bw = sentry->qos[HMAT_SLLBIS_ACCESS_BANDWIDTH];
+		lat = sentry->qos[HMAT_SLLBIS_READ_LATENCY];
+		if (!lat)
+			lat = sentry->qos[HMAT_SLLBIS_ACCESS_LATENCY];
+		min_rd_bw = min_t(u64, min_rd_bw, bw);
+		total_rd_lat += lat;
+
+		dport = nport->parent_dport;
+		next = next->parent;
+		nport = to_cxl_port(next);
+		switches++;
+	} while (next);
+
+	*wr_bw = min_wr_bw;
+	*wr_lat = total_wr_lat;
+	*rd_bw = min_rd_bw;
+	*rd_lat = total_rd_lat;
+
+	if (!switches)
+		return -ENOENT;
+
+	return rc;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_port_get_switch_qos, CXL);
+
 /* for user tooling to ensure port disable work has completed */
 static ssize_t flush_store(struct bus_type *bus, const char *buf, size_t count)
 {
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index dc6da641ced0..21e7c1f78f1f 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -807,6 +807,8 @@ struct qtg_dsm_output {
 struct qtg_dsm_output *cxl_acpi_evaluate_qtg_dsm(acpi_handle handle,
 						 struct qtg_dsm_input *input);
 acpi_handle cxl_acpi_get_rootdev_handle(struct device *dev);
+int cxl_port_get_switch_qos(struct cxl_port *port, u64 *rd_bw, u64 *rd_lat,
+			    u64 *wr_bw, u64 *wr_lat);
 
 /*
  * Unit test builds overrides this to __weak, find the 'strong' version



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 12/21] cxl: Add helper function that calculate QoS values for PCI path
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (10 preceding siblings ...)
  2023-03-27 21:45 ` [PATCH v2 11/21] cxl: Add helper function that calculates QoS values for switches Dave Jiang
@ 2023-03-27 21:45 ` Dave Jiang
  2023-03-27 21:45 ` [PATCH v2 13/21] ACPI: NUMA: Add genport target allocation to the HMAT parsing Dave Jiang
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:45 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Calculate the link bandwidth and latency for the PCIe path from the device
to the CXL Host Bridge. This does not include the CDAT data from the device
or the switch(es) in the path.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/port.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h       |    2 ++
 2 files changed, 63 insertions(+)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 6e2f8e40757e..f78559edd239 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -2034,6 +2034,67 @@ int cxl_port_get_switch_qos(struct cxl_port *port, u64 *rd_bw, u64 *rd_lat,
 }
 EXPORT_SYMBOL_NS_GPL(cxl_port_get_switch_qos, CXL);
 
+/**
+ * cxl_port_get_downstream_qos - retrieve QoS data for PCIE downstream path
+ * @port: endpoint cxl_port
+ * @bandwidth: writeback value for min bandwidth
+ * @latency: writeback value for total latency
+ *
+ * Return: Errno on failure, 0 on success.
+ */
+int cxl_port_get_downstream_qos(struct cxl_port *port, u64 *bandwidth,
+				u64 *latency)
+{
+	u64 min_bw = ULONG_MAX;
+	struct pci_dev *pdev;
+	struct cxl_port *p;
+	struct device *dev;
+	u64 total_lat = 0;
+	int devices = 0;
+	u64 lat;
+
+	/* Grab the device that is the PCI device for CXL memdev */
+	dev = port->uport->parent;
+	/* Skip if it's not PCI, most likely a cxl_test device */
+	if (!dev_is_pci(dev))
+		return 0;
+
+	pdev = to_pci_dev(dev);
+	min_bw = pcie_bandwidth_available(pdev, NULL, NULL, NULL);
+	if (min_bw == 0)
+		return -ENXIO;
+
+	/* convert to MB/s from Mb/s */
+	min_bw >>= 3;
+
+	p = port;
+	do {
+		struct cxl_dport *dport;
+
+		lat = cxl_pci_get_latency(pdev);
+		if (lat < 0)
+			return lat;
+
+		total_lat += lat;
+		devices++;
+
+		dport = p->parent_dport;
+		if (!dport)
+			break;
+
+		p = dport->port;
+		dev = p->uport;
+		if (!dev_is_pci(dev))
+			break;
+		pdev = to_pci_dev(dev);
+	} while (1);
+
+	*bandwidth = min_bw;
+	*latency = total_lat;
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_port_get_downstream_qos, CXL);
+
 /* for user tooling to ensure port disable work has completed */
 static ssize_t flush_store(struct bus_type *bus, const char *buf, size_t count)
 {
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 21e7c1f78f1f..67e844645ef6 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -809,6 +809,8 @@ struct qtg_dsm_output *cxl_acpi_evaluate_qtg_dsm(acpi_handle handle,
 acpi_handle cxl_acpi_get_rootdev_handle(struct device *dev);
 int cxl_port_get_switch_qos(struct cxl_port *port, u64 *rd_bw, u64 *rd_lat,
 			    u64 *wr_bw, u64 *wr_lat);
+int cxl_port_get_downstream_qos(struct cxl_port *port, u64 *bandwidth,
+				u64 *latency);
 
 /*
  * Unit test builds overrides this to __weak, find the 'strong' version



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 13/21] ACPI: NUMA: Add genport target allocation to the HMAT parsing
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (11 preceding siblings ...)
  2023-03-27 21:45 ` [PATCH v2 12/21] cxl: Add helper function that calculate QoS values for PCI path Dave Jiang
@ 2023-03-27 21:45 ` Dave Jiang
  2023-03-27 21:45 ` [PATCH v2 14/21] ACPI: NUMA: Add helper function to retrieve the performance attributes Dave Jiang
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:45 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Add SRAT parsing for the HMAT init in order to collect the device handle
from the Generic Port Affinity Structure. The devie handle will serve as
the key to search for target data.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/numa/hmat.c |   51 ++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/actbl3.h    |    2 ++
 2 files changed, 53 insertions(+)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index bba268ecd802..8879c4576cf5 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -65,6 +65,7 @@ struct memory_target {
 	struct node_hmem_attrs hmem_attrs[2];
 	struct list_head caches;
 	struct node_cache_attrs cache_attrs;
+	u8 device_handle[ACPI_SRAT_DEVICE_HANDLE_SIZE];
 	bool registered;
 };
 
@@ -151,6 +152,33 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
 				start, start + len, mem_pxm);
 }
 
+static __init void alloc_genport_target(unsigned int mem_pxm, u8 *handle)
+{
+	struct memory_target *target;
+
+	target = find_mem_target(mem_pxm);
+	if (!target) {
+		target = kzalloc(sizeof(*target), GFP_KERNEL);
+		if (!target)
+			return;
+		target->memory_pxm = mem_pxm;
+		target->processor_pxm = PXM_INVAL;
+		target->memregions = (struct resource) {
+			.name	= "ACPI genport",
+			.start	= 0,
+			.end	= -1,
+			.flags	= IORESOURCE_MEM,
+		};
+		memcpy(target->device_handle, handle,
+		       ACPI_SRAT_DEVICE_HANDLE_SIZE);
+		list_add_tail(&target->node, &targets);
+		INIT_LIST_HEAD(&target->caches);
+	} else {
+		memcpy(target->device_handle, handle,
+		       ACPI_SRAT_DEVICE_HANDLE_SIZE);
+	}
+}
+
 static __init const char *hmat_data_type(u8 type)
 {
 	switch (type) {
@@ -490,6 +518,22 @@ static __init int srat_parse_mem_affinity(union acpi_subtable_headers *header,
 	return 0;
 }
 
+static __init int srat_parse_genport_affinity(union acpi_subtable_headers *header,
+					      const unsigned long end)
+{
+	struct acpi_srat_generic_affinity *ga = (void *)header;
+
+	if (!ga)
+		return -EINVAL;
+
+	if (!(ga->flags & ACPI_SRAT_GENERIC_AFFINITY_ENABLED))
+		return 0;
+
+	alloc_genport_target(ga->proximity_domain, (u8 *)ga->device_handle);
+
+	return 0;
+}
+
 static u32 hmat_initiator_perf(struct memory_target *target,
 			       struct memory_initiator *initiator,
 			       struct acpi_hmat_locality *hmat_loc)
@@ -835,6 +879,13 @@ static __init int hmat_init(void)
 				ACPI_SRAT_TYPE_MEMORY_AFFINITY,
 				srat_parse_mem_affinity, 0) < 0)
 		goto out_put;
+
+	if (acpi_table_parse_entries(ACPI_SIG_SRAT,
+				     sizeof(struct acpi_table_srat),
+				     ACPI_SRAT_TYPE_GENERIC_PORT_AFFINITY,
+				     srat_parse_genport_affinity, 0) < 0)
+		goto out_put;
+
 	acpi_put_table(tbl);
 
 	status = acpi_get_table(ACPI_SIG_HMAT, 0, &tbl);
diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
index 832c6464f063..0daf5a94f08a 100644
--- a/include/acpi/actbl3.h
+++ b/include/acpi/actbl3.h
@@ -289,6 +289,8 @@ struct acpi_srat_generic_affinity {
 	u32 reserved1;
 };
 
+#define ACPI_SRAT_DEVICE_HANDLE_SIZE		16
+
 /* Flags for struct acpi_srat_generic_affinity */
 
 #define ACPI_SRAT_GENERIC_AFFINITY_ENABLED     (1)	/* 00: Use affinity structure */



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 14/21] ACPI: NUMA: Add helper function to retrieve the performance attributes
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (12 preceding siblings ...)
  2023-03-27 21:45 ` [PATCH v2 13/21] ACPI: NUMA: Add genport target allocation to the HMAT parsing Dave Jiang
@ 2023-03-27 21:45 ` Dave Jiang
  2023-03-27 21:45 ` [PATCH v2 15/21] cxl: Add helper function to retrieve generic port QoS Dave Jiang
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:45 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Add helper to retrieve the performance attributes based on the device
handle.  The helper function is exported so the CXL driver can use that
to acquire the performance data between the CPU and the CXL host bridge.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/numa/hmat.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h     |    6 ++++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index 8879c4576cf5..9952a9bafe70 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -100,6 +100,50 @@ static struct memory_target *find_mem_target(unsigned int mem_pxm)
 	return NULL;
 }
 
+static struct memory_target *acpi_find_genport_target(u8 *device_handle)
+{
+	struct memory_target *target;
+
+	list_for_each_entry(target, &targets, node) {
+		if (!strncmp(target->device_handle, device_handle,
+			     ACPI_SRAT_DEVICE_HANDLE_SIZE))
+			return target;
+	}
+
+	return NULL;
+}
+
+int acpi_get_genport_attrs(u8 *device_handle, u64 *val, int type)
+{
+	struct memory_target *target;
+
+	target = acpi_find_genport_target(device_handle);
+	if (!target)
+		return -ENOENT;
+
+	switch (type) {
+	case ACPI_HMAT_ACCESS_LATENCY:
+	case ACPI_HMAT_READ_LATENCY:
+		*val = target->hmem_attrs[0].read_latency;
+		break;
+	case ACPI_HMAT_WRITE_LATENCY:
+		*val = target->hmem_attrs[0].write_latency;
+		break;
+	case ACPI_HMAT_ACCESS_BANDWIDTH:
+	case ACPI_HMAT_READ_BANDWIDTH:
+		*val = target->hmem_attrs[0].read_bandwidth;
+		break;
+	case ACPI_HMAT_WRITE_BANDWIDTH:
+		*val = target->hmem_attrs[0].write_bandwidth;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_get_genport_attrs);
+
 static __init void alloc_memory_initiator(unsigned int cpu_pxm)
 {
 	struct memory_initiator *initiator;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index efff750f326d..1a727053fabb 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -451,6 +451,7 @@ extern bool acpi_osi_is_win8(void);
 #ifdef CONFIG_ACPI_NUMA
 int acpi_map_pxm_to_node(int pxm);
 int acpi_get_node(acpi_handle handle);
+int acpi_get_genport_attrs(u8 *device_handle, u64 *val, int type);
 
 /**
  * pxm_to_online_node - Map proximity ID to online node
@@ -485,6 +486,11 @@ static inline int acpi_get_node(acpi_handle handle)
 {
 	return 0;
 }
+
+static inline int acpi_get_genport_attrs(u8 *device_handle, u64 *val, int type);
+{
+	return -EOPNOTSUPP;
+}
 #endif
 extern int acpi_paddr_to_node(u64 start_addr, u64 size);
 



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 15/21] cxl: Add helper function to retrieve generic port QoS
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (13 preceding siblings ...)
  2023-03-27 21:45 ` [PATCH v2 14/21] ACPI: NUMA: Add helper function to retrieve the performance attributes Dave Jiang
@ 2023-03-27 21:45 ` Dave Jiang
  2023-03-27 21:45 ` [PATCH v2 16/21] cxl: Add latency and bandwidth calculations for the CXL path Dave Jiang
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:45 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Add CXL helper function that retrieves the bandwidth and latency data of a
generic port by calling acpi_get_genport_attrs() function. A device handle
is passed in constructed from the ACPI HID and UID of the CXL host bridge
(ACPI0016) device.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/acpi.c |   30 ++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h       |    1 +
 2 files changed, 31 insertions(+)

diff --git a/drivers/cxl/core/acpi.c b/drivers/cxl/core/acpi.c
index 191644d0ca6d..41eeaa8c272e 100644
--- a/drivers/cxl/core/acpi.c
+++ b/drivers/cxl/core/acpi.c
@@ -148,3 +148,33 @@ struct qtg_dsm_output *cxl_acpi_evaluate_qtg_dsm(acpi_handle handle,
 	return ERR_PTR(rc);
 }
 EXPORT_SYMBOL_NS_GPL(cxl_acpi_evaluate_qtg_dsm, CXL);
+
+/**
+ * cxl_acpi_get_hb_qos - retrieve QoS data for generic port
+ * @host: 'struct device' of the CXL host bridge
+ * @latency: genport latency data
+ * @bandwidth: genport bandwidth data
+ *
+ * Return: Errno on failure, 0 on success.
+ */
+int cxl_acpi_get_hb_qos(struct device *host, u64 *latency, u64 *bandwidth)
+{
+	u8 handle[ACPI_SRAT_DEVICE_HANDLE_SIZE] = { 0 };
+	struct acpi_device *adev = ACPI_COMPANION(host);
+	int rc;
+
+	/* ACPI spec 6.5 Table 5.65 */
+	memcpy(handle, acpi_device_hid(adev), 8);
+	memcpy(&handle[8], acpi_device_uid(adev), 4);
+
+	rc = acpi_get_genport_attrs(handle, latency, ACPI_HMAT_ACCESS_LATENCY);
+	if (rc)
+		return rc;
+
+	rc = acpi_get_genport_attrs(handle, bandwidth, ACPI_HMAT_ACCESS_BANDWIDTH);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_acpi_get_hb_qos, CXL);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 67e844645ef6..56bcf144eede 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -811,6 +811,7 @@ int cxl_port_get_switch_qos(struct cxl_port *port, u64 *rd_bw, u64 *rd_lat,
 			    u64 *wr_bw, u64 *wr_lat);
 int cxl_port_get_downstream_qos(struct cxl_port *port, u64 *bandwidth,
 				u64 *latency);
+int cxl_acpi_get_hb_qos(struct device *host, u64 *latency, u64 *bandwidth);
 
 /*
  * Unit test builds overrides this to __weak, find the 'strong' version



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 16/21] cxl: Add latency and bandwidth calculations for the CXL path
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (14 preceding siblings ...)
  2023-03-27 21:45 ` [PATCH v2 15/21] cxl: Add helper function to retrieve generic port QoS Dave Jiang
@ 2023-03-27 21:45 ` Dave Jiang
  2023-03-27 21:45 ` [PATCH v2 17/21] cxl: Wait Memory_Info_Valid before access memory related info Dave Jiang
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:45 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

CXL Memory Device SW Guide rev1.0 2.11.2 provides instruction on how to
caluclate latency and bandwidth for CXL memory device. Calculate minimum
bandwidth and total latency for the path from the CXL device to the root
port. The retrieved QTG ID is stored to the cxl_port of the CXL device.

For example for a device that is directly attached to a host bus:
Total Latency = Device Latency (from CDAT) + Dev to Host Bus (HB) Link
		Latency + Generic Port Latency
Min Bandwidth = Min bandwidth for link bandwidth between HB
		and CXL device, device CDAT bandwidth, and Generic Port
		Bandwidth

For a device that has a switch in between host bus and CXL device:
Total Latency = Device (CDAT) Latency + Dev to Switch Link Latency +
		Switch (CDAT) Latency + Switch to HB Link Latency +
		Generic Port Latency
Min Bandwidth = Min bandwidth for link bandwidth between CXL device
		to CXL switch, CXL device CDAT bandwidth, CXL switch CDAT
		bandwidth, CXL switch to HB bandwidth, and Generic Port
		Bandwidth.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/cxlpci.h |    1 +
 drivers/cxl/port.c   |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 815bf843018e..8ed8dd6903e9 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -119,6 +119,7 @@ struct dsmas_entry {
 	struct range dpa_range;
 	u8 handle;
 	u64 qos[HMAT_SLLBIS_DATA_TYPE_MAX];
+	u16 qtg_id;
 };
 
 struct sslbis_entry {
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 7839e0244d0d..55517f6f5b84 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -67,6 +67,63 @@ static void dsmas_list_destroy(struct list_head *dsmas_list)
 	}
 }
 
+static int cxl_port_qos_calculate(struct cxl_port *port,
+				  struct list_head *dsmas_list)
+{
+	u64 sw_wr_bw, sw_wr_lat, sw_rd_bw, sw_rd_lat;
+	u64 min_rd_bw, total_rd_lat, min_wr_bw, total_wr_lat;
+	struct qtg_dsm_output *output;
+	struct qtg_dsm_input input;
+	struct dsmas_entry *dent;
+	acpi_handle handle;
+	u64 gp_bw, gp_lat;
+	u64 ds_bw, ds_lat;
+	int rc;
+
+	rc = cxl_port_get_downstream_qos(port, &ds_bw, &ds_lat);
+	if (rc)
+		return rc;
+
+	rc = cxl_port_get_switch_qos(port, &sw_rd_bw, &sw_rd_lat,
+				     &sw_wr_bw, &sw_wr_lat);
+	if (rc && rc != -ENOENT)
+		return rc;
+
+	rc = cxl_acpi_get_hb_qos(port->host_bridge, &gp_lat, &gp_bw);
+	if (rc)
+		return rc;
+
+	min_rd_bw = min_t(u64, ds_bw, sw_rd_bw);
+	min_rd_bw = min_t(u64, gp_bw, min_rd_bw);
+	total_rd_lat = ds_lat + gp_lat + sw_rd_lat;
+
+	min_wr_bw = min_t(u64, ds_bw, sw_wr_bw);
+	min_wr_bw = min_t(u64, gp_bw, min_wr_bw);
+	total_wr_lat = ds_lat + gp_lat + sw_wr_lat;
+
+	handle = cxl_acpi_get_rootdev_handle(&port->dev);
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+
+	list_for_each_entry(dent, dsmas_list, list) {
+		input.rd_lat = dent->qos[ACPI_HMAT_READ_LATENCY] + total_rd_lat;
+		input.wr_lat = dent->qos[ACPI_HMAT_WRITE_LATENCY] + total_wr_lat;
+		input.rd_bw = min_t(int, min_rd_bw,
+				    dent->qos[ACPI_HMAT_READ_BANDWIDTH]);
+		input.wr_bw = min_t(int, min_wr_bw,
+				    dent->qos[ACPI_HMAT_WRITE_BANDWIDTH]);
+
+		output = cxl_acpi_evaluate_qtg_dsm(handle, &input);
+		if (IS_ERR(output))
+			continue;
+
+		dent->qtg_id = output->qtg_ids[0];
+		kfree(output);
+	}
+
+	return 0;
+}
+
 static int cxl_switch_port_probe(struct cxl_port *port)
 {
 	struct cxl_hdm *cxlhdm;
@@ -162,6 +219,9 @@ static int cxl_port_probe(struct device *dev)
 				dev_warn(dev, "Failed to parse DSMAS: %d\n", rc);
 			}
 
+			rc = cxl_port_qos_calculate(port, &dsmas_list);
+			if (rc)
+				dev_dbg(dev, "Failed to do QoS calculations\n");
 			dsmas_list_destroy(&dsmas_list);
 		} else {
 			rc = cdat_table_parse_sslbis(port->cdat.table,



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 17/21] cxl: Wait Memory_Info_Valid before access memory related info
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (15 preceding siblings ...)
  2023-03-27 21:45 ` [PATCH v2 16/21] cxl: Add latency and bandwidth calculations for the CXL path Dave Jiang
@ 2023-03-27 21:45 ` Dave Jiang
  2023-03-27 21:45 ` [PATCH v2 18/21] cxl: Move identify and partition query from pci probe to port probe Dave Jiang
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:45 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

CXL rev3.0 8.1.3.8.2 Memory_Info_valid field

The Memory_Info_Valid bit indicates that the CXL Range Size High and Size
Low registers are valid. The bit must be set within 1 second of reset
deassertion to the device. Check valid bit before we check the
Memory_Active bit when waiting for cxl_await_media_ready() to ensure that
the memory info is valid for consumption.

Fixes: 2e4ba0ec9783 ("cxl/pci: Move cxl_await_media_ready() to the core")
Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v2:
- Check both ranges. (Jonathan)
---
 drivers/cxl/core/pci.c |   83 +++++++++++++++++++++++++++++++++++++++++++-----
 drivers/cxl/cxlpci.h   |    2 +
 2 files changed, 77 insertions(+), 8 deletions(-)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 2f58cc54e108..268694d33a34 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -101,21 +101,55 @@ int devm_cxl_port_enumerate_dports(struct cxl_port *port)
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_port_enumerate_dports, CXL);
 
-/*
- * Wait up to @media_ready_timeout for the device to report memory
- * active.
- */
-int cxl_await_media_ready(struct cxl_dev_state *cxlds)
+static int cxl_dvsec_mem_range_valid(struct cxl_dev_state *cxlds, int id)
+{
+	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
+	int d = cxlds->cxl_dvsec;
+	bool valid = false;
+	int rc, i;
+	u32 temp;
+
+	if (id > CXL_DVSEC_RANGE_MAX)
+		return -EINVAL;
+
+	/* Check MEM INFO VALID bit first, give up after 1s */
+	i = 1;
+	do {
+		rc = pci_read_config_dword(pdev,
+					   d + CXL_DVSEC_RANGE_SIZE_LOW(id),
+					   &temp);
+		if (rc)
+			return rc;
+
+		valid = FIELD_GET(CXL_DVSEC_MEM_INFO_VALID, temp);
+		if (valid)
+			break;
+		msleep(1000);
+	} while (i--);
+
+	if (!valid) {
+		dev_err(&pdev->dev,
+			"Timeout awaiting memory range %d valid after 1s.\n",
+			id);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int cxl_dvsec_mem_range_active(struct cxl_dev_state *cxlds, int id)
 {
 	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
 	int d = cxlds->cxl_dvsec;
 	bool active = false;
-	u64 md_status;
 	int rc, i;
+	u32 temp;
 
-	for (i = media_ready_timeout; i; i--) {
-		u32 temp;
+	if (id > CXL_DVSEC_RANGE_MAX)
+		return -EINVAL;
 
+	/* Check MEM ACTIVE bit, up to 60s timeout by default */
+	for (i = media_ready_timeout; i; i--) {
 		rc = pci_read_config_dword(
 			pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &temp);
 		if (rc)
@@ -134,6 +168,39 @@ int cxl_await_media_ready(struct cxl_dev_state *cxlds)
 		return -ETIMEDOUT;
 	}
 
+	return 0;
+}
+
+/*
+ * Wait up to @media_ready_timeout for the device to report memory
+ * active.
+ */
+int cxl_await_media_ready(struct cxl_dev_state *cxlds)
+{
+	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
+	int d = cxlds->cxl_dvsec;
+	int rc, i, hdm_count;
+	u64 md_status;
+	u16 cap;
+
+	rc = pci_read_config_word(pdev,
+				  d + CXL_DVSEC_CAP_OFFSET, &cap);
+	if (rc)
+		return rc;
+
+	hdm_count = FIELD_GET(CXL_DVSEC_HDM_COUNT_MASK, cap);
+	for (i = 0; i < hdm_count; i++) {
+		rc = cxl_dvsec_mem_range_valid(cxlds, i);
+		if (rc)
+			return rc;
+	}
+
+	for (i = 0; i < hdm_count; i++) {
+		rc = cxl_dvsec_mem_range_active(cxlds, i);
+		if (rc)
+			return rc;
+	}
+
 	md_status = readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);
 	if (!CXLMDEV_READY(md_status))
 		return -EIO;
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 8ed8dd6903e9..754bfeab2921 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -31,6 +31,8 @@
 #define   CXL_DVSEC_RANGE_BASE_LOW(i)	(0x24 + (i * 0x10))
 #define     CXL_DVSEC_MEM_BASE_LOW_MASK	GENMASK(31, 28)
 
+#define CXL_DVSEC_RANGE_MAX		2
+
 /* CXL 2.0 8.1.4: Non-CXL Function Map DVSEC */
 #define CXL_DVSEC_FUNCTION_MAP					2
 



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 18/21] cxl: Move identify and partition query from pci probe to port probe
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (16 preceding siblings ...)
  2023-03-27 21:45 ` [PATCH v2 17/21] cxl: Wait Memory_Info_Valid before access memory related info Dave Jiang
@ 2023-03-27 21:45 ` Dave Jiang
  2023-03-27 21:46 ` [PATCH v2 19/21] cxl: Store QTG IDs and related info to the CXL memory device context Dave Jiang
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:45 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Move the enumeration of device capacity to cxl_port_probe() from
cxl_pci_probe(). The size and capacity information should be read
after cxl_await_media_ready() so the data is valid.

Fixes: 5e2411ae8071 ("cxl/memdev: Change cxl_mem to a more descriptive name")
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/pci.c  |    8 --------
 drivers/cxl/port.c |    8 ++++++++
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index ed39d133b70d..06324266eae8 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -707,14 +707,6 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
-	rc = cxl_dev_state_identify(cxlds);
-	if (rc)
-		return rc;
-
-	rc = cxl_mem_create_range_info(cxlds);
-	if (rc)
-		return rc;
-
 	rc = cxl_alloc_irq_vectors(pdev);
 	if (rc)
 		return rc;
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 55517f6f5b84..f6646d91ae26 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -175,6 +175,14 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
 		return rc;
 	}
 
+	rc = cxl_dev_state_identify(cxlds);
+	if (rc)
+		return rc;
+
+	rc = cxl_mem_create_range_info(cxlds);
+	if (rc)
+		return rc;
+
 	rc = devm_cxl_enumerate_decoders(cxlhdm, &info);
 	if (rc)
 		return rc;



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 19/21] cxl: Store QTG IDs and related info to the CXL memory device context
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (17 preceding siblings ...)
  2023-03-27 21:45 ` [PATCH v2 18/21] cxl: Move identify and partition query from pci probe to port probe Dave Jiang
@ 2023-03-27 21:46 ` Dave Jiang
  2023-03-27 21:46 ` [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID Dave Jiang
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:46 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Once the QTG ID _DSM is executed successfully, the QTG ID is retrieved from
the return package. Create a list of entries in the cxl_memdev context and
store the QTG ID and the associated DPA range. This information can be
exposed to user space via sysfs in order to help region setup for
hot-plugged CXL memory devices.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/memdev.c |    1 +
 drivers/cxl/cxlmem.h      |   14 ++++++++++++++
 drivers/cxl/port.c        |   19 +++++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 28a05f2fe32d..d2605fc39240 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -346,6 +346,7 @@ struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
 	 */
 	cxlmd->cxlds = cxlds;
 	cxlds->cxlmd = cxlmd;
+	INIT_LIST_HEAD(&cxlmd->qos_list);
 
 	cdev = &cxlmd->cdev;
 	rc = cdev_device_add(cdev, dev);
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 001dabf0231b..c8b8d4865e49 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -40,6 +40,7 @@
  * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
  * @id: id number of this memdev instance.
  * @depth: endpoint port depth
+ * @qos_list: QTG ID related list of entries
  */
 struct cxl_memdev {
 	struct device dev;
@@ -50,6 +51,7 @@ struct cxl_memdev {
 	struct cxl_nvdimm *cxl_nvd;
 	int id;
 	int depth;
+	struct list_head qos_list;
 };
 
 static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
@@ -215,6 +217,18 @@ struct cxl_event_state {
 	struct mutex log_lock;
 };
 
+/**
+ * struct qos_prop - QoS property entry
+ * @list - list entry
+ * @dpa_range - range for DPA address
+ * @qtg_id - QoS Throttling Group ID
+ */
+struct qos_prop_entry {
+	struct list_head list;
+	struct range dpa_range;
+	u16 qtg_id;
+};
+
 /**
  * struct cxl_dev_state - The driver device state
  *
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index f6646d91ae26..4e7e22c13790 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -124,6 +124,22 @@ static int cxl_port_qos_calculate(struct cxl_port *port,
 	return 0;
 }
 
+static void cxl_memdev_set_qtg(struct cxl_memdev *cxlmd, struct list_head *dsmas_list)
+{
+	struct dsmas_entry *dent;
+	struct qos_prop_entry *qos;
+
+	list_for_each_entry(dent, dsmas_list, list) {
+		qos = devm_kzalloc(&cxlmd->dev, sizeof(*qos), GFP_KERNEL);
+		if (!qos)
+			return;
+
+		qos->dpa_range = dent->dpa_range;
+		qos->qtg_id = dent->qtg_id;
+		list_add_tail(&qos->list, &cxlmd->qos_list);
+	}
+}
+
 static int cxl_switch_port_probe(struct cxl_port *port)
 {
 	struct cxl_hdm *cxlhdm;
@@ -212,6 +228,7 @@ static int cxl_port_probe(struct device *dev)
 	read_cdat_data(port);
 	if (port->cdat.table) {
 		if (is_cxl_endpoint(port)) {
+			struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport);
 			LIST_HEAD(dsmas_list);
 
 			rc = cdat_table_parse_dsmas(port->cdat.table,
@@ -230,6 +247,8 @@ static int cxl_port_probe(struct device *dev)
 			rc = cxl_port_qos_calculate(port, &dsmas_list);
 			if (rc)
 				dev_dbg(dev, "Failed to do QoS calculations\n");
+
+			cxl_memdev_set_qtg(cxlmd, &dsmas_list);
 			dsmas_list_destroy(&dsmas_list);
 		} else {
 			rc = cdat_table_parse_sslbis(port->cdat.table,



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (18 preceding siblings ...)
  2023-03-27 21:46 ` [PATCH v2 19/21] cxl: Store QTG IDs and related info to the CXL memory device context Dave Jiang
@ 2023-03-27 21:46 ` Dave Jiang
  2023-03-29  1:27   ` Alison Schofield
  2023-03-29 21:55   ` Dan Williams
  2023-03-27 21:46 ` [PATCH v2 21/21] cxl/mem: Add debugfs output for QTG related data Dave Jiang
  2023-03-28 17:45 ` [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
  21 siblings, 2 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:46 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: Dan Williams, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, rafael, lukas

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
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.
+
+
 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,
 };
 



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2 21/21] cxl/mem: Add debugfs output for QTG related data
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (19 preceding siblings ...)
  2023-03-27 21:46 ` [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID Dave Jiang
@ 2023-03-27 21:46 ` Dave Jiang
  2023-03-29  1:13   ` Alison Schofield
  2023-03-28 17:45 ` [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
  21 siblings, 1 reply; 40+ messages in thread
From: Dave Jiang @ 2023-03-27 21:46 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: Dan Williams, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, rafael, lukas

Add debugfs output to /sys/kernel/debug/cxl/memX/qtgmap
The debugfs attribute will dump out all the DSMAS ranges and the associated
QTG ID exported by the CXL device CDAT.

Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/mem.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 39c4b54f0715..bf2cb5a54a7f 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -45,6 +45,21 @@ static int cxl_mem_dpa_show(struct seq_file *file, void *data)
 	return 0;
 }
 
+static int cxl_mem_qtg_show(struct seq_file *file, void *data)
+{
+	struct device *dev = file->private;
+	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+	struct qos_prop_entry *qos;
+
+	list_for_each_entry(qos, &cxlmd->qos_list, list) {
+		seq_printf(file, "%08llx-%08llx : QTG ID %u\n",
+			   qos->dpa_range.start, qos->dpa_range.end,
+			   qos->qtg_id);
+	}
+
+	return 0;
+}
+
 static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
 				 struct cxl_dport *parent_dport)
 {
@@ -117,6 +132,7 @@ static int cxl_mem_probe(struct device *dev)
 
 	dentry = cxl_debugfs_create_dir(dev_name(dev));
 	debugfs_create_devm_seqfile(dev, "dpamem", dentry, cxl_mem_dpa_show);
+	debugfs_create_devm_seqfile(dev, "qtgmap", dentry, cxl_mem_qtg_show);
 	rc = devm_add_action_or_reset(dev, remove_debugfs, dentry);
 	if (rc)
 		return rc;



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem
  2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
                   ` (20 preceding siblings ...)
  2023-03-27 21:46 ` [PATCH v2 21/21] cxl/mem: Add debugfs output for QTG related data Dave Jiang
@ 2023-03-28 17:45 ` Dave Jiang
  21 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-28 17:45 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: Dan Williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas, Jonathan Cameron

Missed the cc to you as well Jonathan. Sorry about that.

On 3/27/23 2:44 PM, Dave Jiang wrote:
> v2:
> - Please see specific patches for log entries addressing comments from v1.
> - Removed ACPICA code usages.
> - Removed PCI subsystem helpers for latency and bandwidth.
> - Add CXL switch CDAT parsing support (SSLBIS)
> - Add generic port SRAT+HMAT support (ACPI)
> - Export a single QTG ID via sysfs per memory device (Dan)
> - Provide rest of DSMAS range info in debugfs (Dan)
> 
> 
> Hi Rafael, please review the relevant patches to ACPI: 13/18, 14/18. Thank you!
> If they are ok, Dan can take them through the CXL tree for upstream merging.
> These patches are different than what was posted in v1. I've removed ACPICA usages
> from v1. The two new patches adds the SRAT Generic Port Affinity Structure parsing
> in order to store the device handle that correlates to the proximity domains
> exported by the SRAT table and ties that to the performance data that's exported by
> the HMAT.
> 
> This series adds the retrieval of QoS Throttling Group (QTG) IDs for the CXL Fixed
> Memory Window Structure (CFMWS) and the CXL memory device. It provides the QTG IDs
> to user space to provide some guidance with putting the proper DPA range under the
> appropriate CFMWS window for a hot-plugged CXL memory device.
> 
> The CFMWS structure contains a QTG ID that is associated with the memory window that the
> structure exports. On Linux, the CFMWS is represented as a CXL root decoder. The QTG
> ID will be attached to the CXL root decoder and exported as a sysfs attribute (qtg_id).
> 
> The QTG ID for a device is retrieved via sending a _DSM method to the ACPI0017 device.
> The _DSM expects an input package of 4 DWORDS that contains the read latency, write
> latency, read bandwidth, and write banwidth. These are the caluclated numbers for the
> path between the CXL device and the CXL host bridge (HB). The QTG ID is also exported
> as a sysfs attribute under the mem device memory type:
> /sys/bus/cxl/devices/memX/qtg_id
> Only the first QTG ID is exported. The rest of the information can be found under
> /sys/kernel/debug/cxl/memX/qtgmap where all the DPA ranges with the correlated QTG ID
> are displayed. Each DSMAS from the device CDAT will provide a DPA range.
> 
> The latency numbers are the aggregated latencies for the path between the CXL device and
> the CPU. If a CXL device is directly attached to the CXL HB, the latency
> would be the aggregated latencies from the device Coherent Device Attribute Table (CDAT),
> the caluclated PCIe link latency between the device and the HB, and the generic port data
> from ACPI SRAT+HMAT. The bandwidth in this configuration would be the minimum between the
> CDAT bandwidth number, link bandwidth between the device and the HB, and the bandwidth data
> from the generic port data via ACPI SRAT+HMAT.
> 
> If a configuration has a switch in between then the latency would be the aggregated
> latencies from the device CDAT, the link latency between device and switch, the
> latency from the switch CDAT, the link latency between switch and the HB, and the
> generic port latency between the CPU and the CXL HB. The bandwidth calculation would be the
> min of device CDAT bandwidth, link bandwith between device and switch, switch CDAT
> bandwidth, the link bandwidth between switch and HB, and the generic port bandwidth
> 
> There can be 0 or more switches between the CXL device and the CXL HB. There are detailed
> examples on calculating bandwidth and latency in the CXL Memory Device Software Guide [4].
> 
> The CDAT provides Device Scoped Memory Affinity Structures (DSMAS) that contains the
> Device Physical Address (DPA) range and the related Device Scoped Latency and Bandwidth
> Informat Stuctures (DSLBIS). Each DSLBIS provides a latency or bandwidth entry that is
> tied to a DSMAS entry via a per DSMAS unique DSMAD handle.
> 
> This series is based on Lukas's latest DOE changes [5]. Kernel branch with all the code can
> be retrieved here [6] for convenience.
> 
> Test setup is done with runqemu genport support branch [6]. The setup provides 2 CXL HBs
> with one HB having a CXL switch underneath. It also provides generic port support detailed
> below.
> 
> A hacked up qemu branch is used to support generic port SRAT and HMAT [7].
> 
> To create the appropriate HMAT entries for generic port, the following qemu paramters must
> be added:
> 
> -object genport,id=$X -numa node,genport=genport$X,nodeid=$Y,initiator=$Z
> -numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-latency,latency=$latency
> -numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-bandwidth,bandwidth=$bandwidthM
> for ((i = 0; i < total_nodes; i++)); do
> 	for ((j = 0; j < cxl_hbs; j++ )); do	# 2 CXL HBs
> 		-numa dist,src=$i,dst=$X,val=$dist
> 	done
> done
> 
> See the genport run_qemu branch for full details.
> 
> [1]: https://www.computeexpresslink.org/download-the-specification
> [2]: https://uefi.org/sites/default/files/resources/Coherent%20Device%20Attribute%20Table_1.01.pdf
> [3]: https://uefi.org/sites/default/files/resources/ACPI_Spec_6_5_Aug29.pdf
> [4]: https://cdrdv2-public.intel.com/643805/643805_CXL%20Memory%20Device%20SW%20Guide_Rev1p0.pdf
> [5]: https://lore.kernel.org/linux-cxl/20230313195530.GA1532686@bhelgaas/T/#t
> [6]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-qtg
> [7]: https://github.com/pmem/run_qemu/tree/djiang/genport
> [8]: https://github.com/davejiang/qemu/tree/genport
> 
> ---
> 
> Dave Jiang (21):
>        cxl: Export QTG ids from CFMWS to sysfs
>        cxl: Add checksum verification to CDAT from CXL
>        cxl: Add support for reading CXL switch CDAT table
>        cxl: Add common helpers for cdat parsing
>        cxl: Add callback to parse the DSMAS subtables from CDAT
>        cxl: Add callback to parse the DSLBIS subtable from CDAT
>        cxl: Add callback to parse the SSLBIS subtable from CDAT
>        cxl: Add support for _DSM Function for retrieving QTG ID
>        cxl: Add helper function to retrieve ACPI handle of CXL root device
>        cxl: Add helpers to calculate pci latency for the CXL device
>        cxl: Add helper function that calculates QoS values for switches
>        cxl: Add helper function that calculate QoS values for PCI path
>        ACPI: NUMA: Add genport target allocation to the HMAT parsing
>        ACPI: NUMA: Add helper function to retrieve the performance attributes
>        cxl: Add helper function to retrieve generic port QoS
>        cxl: Add latency and bandwidth calculations for the CXL path
>        cxl: Wait Memory_Info_Valid before access memory related info
>        cxl: Move identify and partition query from pci probe to port probe
>        cxl: Store QTG IDs and related info to the CXL memory device context
>        cxl: Export sysfs attributes for memory device QTG ID
>        cxl/mem: Add debugfs output for QTG related data
> 
> 
>   Documentation/ABI/testing/sysfs-bus-cxl |  20 ++
>   drivers/acpi/numa/hmat.c                |  95 +++++++++
>   drivers/cxl/acpi.c                      |   3 +
>   drivers/cxl/core/Makefile               |   2 +
>   drivers/cxl/core/acpi.c                 | 180 ++++++++++++++++++
>   drivers/cxl/core/cdat.c                 | 243 ++++++++++++++++++++++++
>   drivers/cxl/core/memdev.c               |  16 ++
>   drivers/cxl/core/pci.c                  | 187 ++++++++++++++++--
>   drivers/cxl/core/port.c                 | 169 ++++++++++++++++
>   drivers/cxl/cxl.h                       |  27 +++
>   drivers/cxl/cxlmem.h                    |  14 ++
>   drivers/cxl/cxlpci.h                    | 121 ++++++++++++
>   drivers/cxl/mem.c                       |  16 ++
>   drivers/cxl/pci.c                       |  21 --
>   drivers/cxl/port.c                      | 130 ++++++++++++-
>   include/acpi/actbl3.h                   |   2 +
>   include/linux/acpi.h                    |   6 +
>   tools/testing/cxl/Kbuild                |   1 +
>   tools/testing/cxl/test/mock.c           |   5 +
>   19 files changed, 1221 insertions(+), 37 deletions(-)
>   create mode 100644 drivers/cxl/core/acpi.c
>   create mode 100644 drivers/cxl/core/cdat.c
> 
> --
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 02/21] cxl: Add checksum verification to CDAT from CXL
  2023-03-27 21:44 ` [PATCH v2 02/21] cxl: Add checksum verification to CDAT from CXL Dave Jiang
@ 2023-03-29  0:03   ` Alison Schofield
  2023-03-29  0:21     ` Dave Jiang
  2023-03-30  0:09   ` Ira Weiny
  1 sibling, 1 reply; 40+ messages in thread
From: Alison Schofield @ 2023-03-29  0:03 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, linux-acpi, dan.j.williams, ira.weiny, vishal.l.verma,
	rafael, lukas

On Mon, Mar 27, 2023 at 02:44:13PM -0700, Dave Jiang wrote:
> A CDAT table is available from a CXL device. The table is read by the
> driver and cached in software. With the CXL subsystem needing to parse the
> CDAT table, the checksum should be verified. Add checksum verification
> after the CDAT table is read from device.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> ---
> v2:
> - Drop ACPI checksum export and just use local verification. (Dan)
> ---
>  drivers/cxl/core/pci.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 25b7e8125d5d..e0d5e6525c0d 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -528,6 +528,16 @@ static int cxl_cdat_read_table(struct device *dev,
>  	return 0;
>  }
>  
> +static unsigned char cdat_checksum(void *buf, size_t size)
> +{
> +	unsigned char sum, *data = buf;
> +	size_t i;
> +
> +	for (sum = 0, i = 0; i < size; i++)
> +		sum += data[i];
> +	return 0 - sum;

This return value isn't obvious to me. What's happening here?
Thanks for explaining,

Alison

> +}
> +
>  /**
>   * read_cdat_data - Read the CDAT data on this port
>   * @port: Port to read data from
> @@ -573,6 +583,12 @@ void read_cdat_data(struct cxl_port *port)
>  	}
>  
>  	port->cdat.table = cdat_table + sizeof(__le32);
> +	if (cdat_checksum(port->cdat.table, cdat_length)) {
> +		/* Don't leave table data allocated on error */
> +		devm_kfree(dev, cdat_table);
> +		dev_err(dev, "CDAT data checksum error\n");
> +	}
> +
>  	port->cdat.length = cdat_length;
>  }
>  EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 05/21] cxl: Add callback to parse the DSMAS subtables from CDAT
  2023-03-27 21:44 ` [PATCH v2 05/21] cxl: Add callback to parse the DSMAS subtables from CDAT Dave Jiang
@ 2023-03-29  0:20   ` Alison Schofield
  2023-03-29 20:41     ` Dave Jiang
  2023-03-30 15:43   ` Dave Jiang
  1 sibling, 1 reply; 40+ messages in thread
From: Alison Schofield @ 2023-03-29  0:20 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, linux-acpi, dan.j.williams, ira.weiny, vishal.l.verma,
	rafael, lukas

On Mon, Mar 27, 2023 at 02:44:32PM -0700, Dave Jiang wrote:
> Provide a callback function to the CDAT parser in order to parse the Device
> Scoped Memory Affinity Structure (DSMAS). Each DSMAS structure contains the
> DPA range and its associated attributes in each entry. See the CDAT
> specification for details.

Are we going away from offering spec section numbers or links?

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> ---
> v2:
> - Add DSMAS table size check. (Lukas)
> - Use local DSMAS header for LE handling.
> - Remove dsmas lock. (Jonathan)
> - Fix handle size (Jonathan)
> - Add LE to host conversion for DSMAS address and length.
> - Make dsmas_list local
> ---
>  drivers/cxl/core/cdat.c |   26 ++++++++++++++++++++++++++
>  drivers/cxl/cxl.h       |    1 +
>  drivers/cxl/cxlpci.h    |   18 ++++++++++++++++++
>  drivers/cxl/port.c      |   24 ++++++++++++++++++++++++
>  4 files changed, 69 insertions(+)
> 
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index 210f4499bddb..d068609fb6f9 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -98,3 +98,29 @@ int cdat_table_parse_sslbis(struct cdat_header *table,
>  	return cdat_table_parse_entries(CDAT_TYPE_SSLBIS, table, &proc);
>  }
>  EXPORT_SYMBOL_NS_GPL(cdat_table_parse_sslbis, CXL);
> +
> +int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg)
> +{
> +	struct cdat_dsmas *dsmas = (struct cdat_dsmas *)(header);
> +	struct list_head *dsmas_list = (struct list_head *)arg;

cast from void unnecessary ?

> +	struct dsmas_entry *dent;
> +
> +	if (dsmas->hdr.length != sizeof(*dsmas)) {
> +		pr_warn("Malformed DSMAS table length: (%lu:%u)\n",
> +			 (unsigned long)sizeof(*dsmas), dsmas->hdr.length);
> +		return -EINVAL;
> +	}
> +
> +	dent = kzalloc(sizeof(*dent), GFP_KERNEL);
> +	if (!dent)
> +		return -ENOMEM;
> +
> +	dent->handle = dsmas->dsmad_handle;
> +	dent->dpa_range.start = le64_to_cpu(dsmas->dpa_base_address);
> +	dent->dpa_range.end = le64_to_cpu(dsmas->dpa_base_address) +
> +			      le64_to_cpu(dsmas->dpa_length) - 1;
> +	list_add_tail(&dent->list, dsmas_list);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index cc3309794b45..9d0e22fe72c0 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -8,6 +8,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
>  #include <linux/log2.h>
> +#include <linux/list.h>
>  #include <linux/io.h>
>  
>  /**
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index 45e2f2bf5ef8..9a2468a93d83 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -104,6 +104,22 @@ struct cdat_subtable_entry {
>  	enum cdat_type type;
>  };
>  
> +struct dsmas_entry {
> +	struct list_head list;
> +	struct range dpa_range;
> +	u8 handle;
> +};
> +
> +/* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */
> +struct cdat_dsmas {
> +	struct cdat_entry_header hdr;
> +	u8 dsmad_handle;
> +	u8 flags;
> +	__u16 reserved;
> +	__le64 dpa_base_address;
> +	__le64 dpa_length;
> +} __packed;
> +
>  int devm_cxl_port_enumerate_dports(struct cxl_port *port);
>  struct cxl_dev_state;
>  int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
> @@ -119,4 +135,6 @@ int cdat_table_parse_##x(struct cdat_header *table, \
>  cdat_table_parse(dsmas);
>  cdat_table_parse(dslbis);
>  cdat_table_parse(sslbis);
> +
> +int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg);
>  #endif /* __CXL_PCI_H__ */
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 60a865680e22..c8136797d528 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -57,6 +57,16 @@ static int discover_region(struct device *dev, void *root)
>  	return 0;
>  }
>  
> +static void dsmas_list_destroy(struct list_head *dsmas_list)
> +{
> +	struct dsmas_entry *dentry, *n;
> +
> +	list_for_each_entry_safe(dentry, n, dsmas_list, list) {
> +		list_del(&dentry->list);
> +		kfree(dentry);
> +	}
> +}
> +
>  static int cxl_switch_port_probe(struct cxl_port *port)
>  {
>  	struct cxl_hdm *cxlhdm;
> @@ -131,9 +141,23 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>  static int cxl_port_probe(struct device *dev)
>  {
>  	struct cxl_port *port = to_cxl_port(dev);
> +	int rc;
>  
>  	/* Cache the data early to ensure is_visible() works */
>  	read_cdat_data(port);
> +	if (port->cdat.table) {
> +		if (is_cxl_endpoint(port)) {

How about reducing indentation w:
	if ((port->cdat.table) && (is_cxl_endpoint(port))

> +			LIST_HEAD(dsmas_list);
> +
> +			rc = cdat_table_parse_dsmas(port->cdat.table,
> +						    cxl_dsmas_parse_entry,
> +						    (void *)&dsmas_list);
> +			if (rc < 0)
> +				dev_warn(dev, "Failed to parse DSMAS: %d\n", rc);
> +
> +			dsmas_list_destroy(&dsmas_list);
> +		}
> +	}
>  
>  	if (is_cxl_endpoint(port))
>  		return cxl_endpoint_port_probe(port);
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 02/21] cxl: Add checksum verification to CDAT from CXL
  2023-03-29  0:03   ` Alison Schofield
@ 2023-03-29  0:21     ` Dave Jiang
  0 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-29  0:21 UTC (permalink / raw)
  To: Alison Schofield
  Cc: linux-cxl, linux-acpi, dan.j.williams, ira.weiny, vishal.l.verma,
	rafael, lukas



On 3/28/23 5:03 PM, Alison Schofield wrote:
> On Mon, Mar 27, 2023 at 02:44:13PM -0700, Dave Jiang wrote:
>> A CDAT table is available from a CXL device. The table is read by the
>> driver and cached in software. With the CXL subsystem needing to parse the
>> CDAT table, the checksum should be verified. Add checksum verification
>> after the CDAT table is read from device.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>
>> ---
>> v2:
>> - Drop ACPI checksum export and just use local verification. (Dan)
>> ---
>>   drivers/cxl/core/pci.c |   16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>> index 25b7e8125d5d..e0d5e6525c0d 100644
>> --- a/drivers/cxl/core/pci.c
>> +++ b/drivers/cxl/core/pci.c
>> @@ -528,6 +528,16 @@ static int cxl_cdat_read_table(struct device *dev,
>>   	return 0;
>>   }
>>   
>> +static unsigned char cdat_checksum(void *buf, size_t size)
>> +{
>> +	unsigned char sum, *data = buf;
>> +	size_t i;
>> +
>> +	for (sum = 0, i = 0; i < size; i++)
>> +		sum += data[i];
>> +	return 0 - sum;
> 
> This return value isn't obvious to me. What's happening here?
> Thanks for explaining,

The expectation is that all the bytes add up to be equal 0 for the 
checksum verification. So if we get anything other than 0, the check 
would fail.

DJ

> 
> Alison
> 
>> +}
>> +
>>   /**
>>    * read_cdat_data - Read the CDAT data on this port
>>    * @port: Port to read data from
>> @@ -573,6 +583,12 @@ void read_cdat_data(struct cxl_port *port)
>>   	}
>>   
>>   	port->cdat.table = cdat_table + sizeof(__le32);
>> +	if (cdat_checksum(port->cdat.table, cdat_length)) {
>> +		/* Don't leave table data allocated on error */
>> +		devm_kfree(dev, cdat_table);
>> +		dev_err(dev, "CDAT data checksum error\n");
>> +	}
>> +
>>   	port->cdat.length = cdat_length;
>>   }
>>   EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
>>
>>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 06/21] cxl: Add callback to parse the DSLBIS subtable from CDAT
  2023-03-27 21:44 ` [PATCH v2 06/21] cxl: Add callback to parse the DSLBIS subtable " Dave Jiang
@ 2023-03-29  0:44   ` Alison Schofield
  2023-03-29 20:59     ` Dave Jiang
  0 siblings, 1 reply; 40+ messages in thread
From: Alison Schofield @ 2023-03-29  0:44 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, linux-acpi, dan.j.williams, ira.weiny, vishal.l.verma,
	rafael, lukas

On Mon, Mar 27, 2023 at 02:44:39PM -0700, Dave Jiang wrote:
> Provide a callback to parse the Device Scoped Latency and Bandwidth
> Information Structure (DSLBIS) in the CDAT structures. The DSLBIS
> contains the bandwidth and latency information that's tied to a DSMAS
> handle. The driver will retrieve the read and write latency and
> bandwidth associated with the DSMAS which is tied to a DPA range.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> ---
> v2:
> - Add size check to DSLIBIS table. (Lukas)
> - Remove unnecessary entry type check. (Jonathan)
> - Move data_type check to after match. (Jonathan)
> - Skip unknown data type. (Jonathan)
> - Add overflow check for unit multiply. (Jonathan)
> - Use dev_warn() when entries parsing fail. (Jonathan)
> ---
>  drivers/cxl/core/cdat.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxlpci.h    |   34 +++++++++++++++++++++++++++++++++-
>  drivers/cxl/port.c      |    9 ++++++++-
>  3 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index d068609fb6f9..0e88973e9f38 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright(c) 2023 Intel Corporation. All rights reserved. */
> +#include <linux/overflow.h>
>  #include "cxlpci.h"
>  #include "cxl.h"
>  
> @@ -124,3 +125,43 @@ int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg)
>  	return 0;
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL);
> +
> +int cxl_dslbis_parse_entry(struct cdat_entry_header *header, void *arg)
> +{
> +	struct cdat_dslbis *dslbis = (struct cdat_dslbis *)header;
> +	struct list_head *dsmas_list = (struct list_head *)arg;

cast from void ?

> +	struct dsmas_entry *dent;
> +
> +	if (dslbis->hdr.length != sizeof(*dslbis)) {
> +		pr_warn("Malformed DSLBIS table length: (%lu:%u)\n",
> +			(unsigned long)sizeof(*dslbis), dslbis->hdr.length);
> +		return -EINVAL;
> +	}
> +
> +	/* Unrecognized data type, we can skip */

/* Skip unrecognized data type */

> +	if (dslbis->data_type >= HMAT_SLLBIS_DATA_TYPE_MAX)
> +		return 0;
> +
> +	list_for_each_entry(dent, dsmas_list, list) {
> +		u64 val;
> +		int rc;
> +
> +		if (dslbis->handle != dent->handle)
> +			continue;
> +
> +		/* Not a memory type, skip */
> +		if ((dslbis->flags & DSLBIS_MEM_MASK) != DSLBIS_MEM_MEMORY)
> +			return 0;
> +
> +		rc = check_mul_overflow(le64_to_cpu(dslbis->entry_base_unit),
> +					le16_to_cpu(dslbis->entry[0]), &val);
> +		if (unlikely(rc))
> +			pr_warn("DSLBIS value overflowed!\n");

Must this be shouted !
I have a vague recollection of being told not to use exclamation points
in user visible messages.

> +
> +		dent->qos[dslbis->data_type] = val;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_dslbis_parse_entry, CXL);
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index 9a2468a93d83..1429de49e0c4 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -104,10 +104,21 @@ struct cdat_subtable_entry {
>  	enum cdat_type type;
>  };
>  
> +enum {
> +	HMAT_SLLBIS_ACCESS_LATENCY = 0,
> +	HMAT_SLLBIS_READ_LATENCY,
> +	HMAT_SLLBIS_WRITE_LATENCY,
> +	HMAT_SLLBIS_ACCESS_BANDWIDTH,
> +	HMAT_SLLBIS_READ_BANDWIDTH,
> +	HMAT_SLLBIS_WRITE_BANDWIDTH,
> +	HMAT_SLLBIS_DATA_TYPE_MAX
> +};
> +
>  struct dsmas_entry {
>  	struct list_head list;
>  	struct range dpa_range;
>  	u8 handle;
> +	u64 qos[HMAT_SLLBIS_DATA_TYPE_MAX];
>  };
>  
>  /* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */
> @@ -120,6 +131,23 @@ struct cdat_dsmas {
>  	__le64 dpa_length;
>  } __packed;
>  
> +/* Sub-table 1: Device Scoped Latency and Bandwidth Information Structure (DSLBIS) */
> +struct cdat_dslbis {
> +	struct cdat_entry_header hdr;
> +	u8 handle;
> +	u8 flags;
> +	u8 data_type;
> +	u8 reserved;
> +	__le64 entry_base_unit;
> +	__le16 entry[3];
> +	__le16 reserved2;
> +} __packed;
> +
> +/* Flags for subtable above */

/* Flags for DSLBIS subtable */

> +
> +#define DSLBIS_MEM_MASK		GENMASK(3, 0)
> +#define DSLBIS_MEM_MEMORY	0
> +
>  int devm_cxl_port_enumerate_dports(struct cxl_port *port);
>  struct cxl_dev_state;
>  int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
> @@ -136,5 +164,9 @@ cdat_table_parse(dsmas);
>  cdat_table_parse(dslbis);
>  cdat_table_parse(sslbis);
>  
> -int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg);
> +#define cxl_parse_entry(x) \
> +int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg)
> +
> +cxl_parse_entry(dsmas);
> +cxl_parse_entry(dslbis);
>  #endif /* __CXL_PCI_H__ */
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index c8136797d528..6f2b327f7128 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -152,8 +152,15 @@ static int cxl_port_probe(struct device *dev)
>  			rc = cdat_table_parse_dsmas(port->cdat.table,
>  						    cxl_dsmas_parse_entry,
>  						    (void *)&dsmas_list);
> -			if (rc < 0)
> +			if (rc > 0) {
> +				rc = cdat_table_parse_dslbis(port->cdat.table,
> +							     cxl_dslbis_parse_entry,
> +							     (void *)&dsmas_list);
> +				if (rc <= 0)
> +					dev_warn(dev, "Failed to parse DSLBIS: %d\n", rc);
> +			} else {
>  				dev_warn(dev, "Failed to parse DSMAS: %d\n", rc);
> +			}

I see you touch this func more than once. Maybe some earlier nips and
tucks, makes this more readable.

>  
>  			dsmas_list_destroy(&dsmas_list);
>  		}
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 21/21] cxl/mem: Add debugfs output for QTG related data
  2023-03-27 21:46 ` [PATCH v2 21/21] cxl/mem: Add debugfs output for QTG related data Dave Jiang
@ 2023-03-29  1:13   ` Alison Schofield
  2023-03-29 21:49     ` Dave Jiang
  0 siblings, 1 reply; 40+ messages in thread
From: Alison Schofield @ 2023-03-29  1:13 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, linux-acpi, Dan Williams, ira.weiny, vishal.l.verma,
	rafael, lukas

On Mon, Mar 27, 2023 at 02:46:11PM -0700, Dave Jiang wrote:
> Add debugfs output to /sys/kernel/debug/cxl/memX/qtgmap
> The debugfs attribute will dump out all the DSMAS ranges and the associated
> QTG ID exported by the CXL device CDAT.

Do you want to document these?

The poison inject & clear inflight patchset documents in: 
Documentation/ABI/testing/debugfs-cxl

Alison

> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/mem.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 39c4b54f0715..bf2cb5a54a7f 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -45,6 +45,21 @@ static int cxl_mem_dpa_show(struct seq_file *file, void *data)
>  	return 0;
>  }
>  
> +static int cxl_mem_qtg_show(struct seq_file *file, void *data)
> +{
> +	struct device *dev = file->private;
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +	struct qos_prop_entry *qos;
> +
> +	list_for_each_entry(qos, &cxlmd->qos_list, list) {
> +		seq_printf(file, "%08llx-%08llx : QTG ID %u\n",
> +			   qos->dpa_range.start, qos->dpa_range.end,
> +			   qos->qtg_id);
> +	}
> +
> +	return 0;
> +}
> +
>  static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
>  				 struct cxl_dport *parent_dport)
>  {
> @@ -117,6 +132,7 @@ static int cxl_mem_probe(struct device *dev)
>  
>  	dentry = cxl_debugfs_create_dir(dev_name(dev));
>  	debugfs_create_devm_seqfile(dev, "dpamem", dentry, cxl_mem_dpa_show);
> +	debugfs_create_devm_seqfile(dev, "qtgmap", dentry, cxl_mem_qtg_show);
>  	rc = devm_add_action_or_reset(dev, remove_debugfs, dentry);
>  	if (rc)
>  		return rc;
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID
  2023-03-27 21:46 ` [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID Dave Jiang
@ 2023-03-29  1:27   ` Alison Schofield
  2023-03-29 21:44     ` Dave Jiang
  2023-03-29 21:55   ` Dan Williams
  1 sibling, 1 reply; 40+ messages in thread
From: Alison Schofield @ 2023-03-29  1:27 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, linux-acpi, Dan Williams, ira.weiny, vishal.l.verma,
	rafael, lukas

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

> 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?

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,
>  };
>  
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 05/21] cxl: Add callback to parse the DSMAS subtables from CDAT
  2023-03-29  0:20   ` Alison Schofield
@ 2023-03-29 20:41     ` Dave Jiang
  0 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-29 20:41 UTC (permalink / raw)
  To: Alison Schofield
  Cc: linux-cxl, linux-acpi, dan.j.williams, ira.weiny, vishal.l.verma,
	rafael, lukas



On 3/28/23 5:20 PM, Alison Schofield wrote:
> On Mon, Mar 27, 2023 at 02:44:32PM -0700, Dave Jiang wrote:
>> Provide a callback function to the CDAT parser in order to parse the Device
>> Scoped Memory Affinity Structure (DSMAS). Each DSMAS structure contains the
>> DPA range and its associated attributes in each entry. See the CDAT
>> specification for details.
> 
> Are we going away from offering spec section numbers or links?

Ok I'll add.

> 
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>
>> ---
>> v2:
>> - Add DSMAS table size check. (Lukas)
>> - Use local DSMAS header for LE handling.
>> - Remove dsmas lock. (Jonathan)
>> - Fix handle size (Jonathan)
>> - Add LE to host conversion for DSMAS address and length.
>> - Make dsmas_list local
>> ---
>>   drivers/cxl/core/cdat.c |   26 ++++++++++++++++++++++++++
>>   drivers/cxl/cxl.h       |    1 +
>>   drivers/cxl/cxlpci.h    |   18 ++++++++++++++++++
>>   drivers/cxl/port.c      |   24 ++++++++++++++++++++++++
>>   4 files changed, 69 insertions(+)
>>
>> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
>> index 210f4499bddb..d068609fb6f9 100644
>> --- a/drivers/cxl/core/cdat.c
>> +++ b/drivers/cxl/core/cdat.c
>> @@ -98,3 +98,29 @@ int cdat_table_parse_sslbis(struct cdat_header *table,
>>   	return cdat_table_parse_entries(CDAT_TYPE_SSLBIS, table, &proc);
>>   }
>>   EXPORT_SYMBOL_NS_GPL(cdat_table_parse_sslbis, CXL);
>> +
>> +int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg)
>> +{
>> +	struct cdat_dsmas *dsmas = (struct cdat_dsmas *)(header);
>> +	struct list_head *dsmas_list = (struct list_head *)arg;
> 
> cast from void unnecessary ?

Ok will remove

> 
>> +	struct dsmas_entry *dent;
>> +
>> +	if (dsmas->hdr.length != sizeof(*dsmas)) {
>> +		pr_warn("Malformed DSMAS table length: (%lu:%u)\n",
>> +			 (unsigned long)sizeof(*dsmas), dsmas->hdr.length);
>> +		return -EINVAL;
>> +	}
>> +
>> +	dent = kzalloc(sizeof(*dent), GFP_KERNEL);
>> +	if (!dent)
>> +		return -ENOMEM;
>> +
>> +	dent->handle = dsmas->dsmad_handle;
>> +	dent->dpa_range.start = le64_to_cpu(dsmas->dpa_base_address);
>> +	dent->dpa_range.end = le64_to_cpu(dsmas->dpa_base_address) +
>> +			      le64_to_cpu(dsmas->dpa_length) - 1;
>> +	list_add_tail(&dent->list, dsmas_list);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL);
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index cc3309794b45..9d0e22fe72c0 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -8,6 +8,7 @@
>>   #include <linux/bitfield.h>
>>   #include <linux/bitops.h>
>>   #include <linux/log2.h>
>> +#include <linux/list.h>
>>   #include <linux/io.h>
>>   
>>   /**
>> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
>> index 45e2f2bf5ef8..9a2468a93d83 100644
>> --- a/drivers/cxl/cxlpci.h
>> +++ b/drivers/cxl/cxlpci.h
>> @@ -104,6 +104,22 @@ struct cdat_subtable_entry {
>>   	enum cdat_type type;
>>   };
>>   
>> +struct dsmas_entry {
>> +	struct list_head list;
>> +	struct range dpa_range;
>> +	u8 handle;
>> +};
>> +
>> +/* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */
>> +struct cdat_dsmas {
>> +	struct cdat_entry_header hdr;
>> +	u8 dsmad_handle;
>> +	u8 flags;
>> +	__u16 reserved;
>> +	__le64 dpa_base_address;
>> +	__le64 dpa_length;
>> +} __packed;
>> +
>>   int devm_cxl_port_enumerate_dports(struct cxl_port *port);
>>   struct cxl_dev_state;
>>   int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
>> @@ -119,4 +135,6 @@ int cdat_table_parse_##x(struct cdat_header *table, \
>>   cdat_table_parse(dsmas);
>>   cdat_table_parse(dslbis);
>>   cdat_table_parse(sslbis);
>> +
>> +int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg);
>>   #endif /* __CXL_PCI_H__ */
>> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
>> index 60a865680e22..c8136797d528 100644
>> --- a/drivers/cxl/port.c
>> +++ b/drivers/cxl/port.c
>> @@ -57,6 +57,16 @@ static int discover_region(struct device *dev, void *root)
>>   	return 0;
>>   }
>>   
>> +static void dsmas_list_destroy(struct list_head *dsmas_list)
>> +{
>> +	struct dsmas_entry *dentry, *n;
>> +
>> +	list_for_each_entry_safe(dentry, n, dsmas_list, list) {
>> +		list_del(&dentry->list);
>> +		kfree(dentry);
>> +	}
>> +}
>> +
>>   static int cxl_switch_port_probe(struct cxl_port *port)
>>   {
>>   	struct cxl_hdm *cxlhdm;
>> @@ -131,9 +141,23 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>>   static int cxl_port_probe(struct device *dev)
>>   {
>>   	struct cxl_port *port = to_cxl_port(dev);
>> +	int rc;
>>   
>>   	/* Cache the data early to ensure is_visible() works */
>>   	read_cdat_data(port);
>> +	if (port->cdat.table) {
>> +		if (is_cxl_endpoint(port)) {
> 
> How about reducing indentation w:
> 	if ((port->cdat.table) && (is_cxl_endpoint(port))

Yes. I got a little disoriented around here from the v6.2 to v6.3 rebase 
where this function changed drastically. In fact I think I'll move this 
block under the endpoint check below to avoid another endpoint check.

> 
>> +			LIST_HEAD(dsmas_list);
>> +
>> +			rc = cdat_table_parse_dsmas(port->cdat.table,
>> +						    cxl_dsmas_parse_entry,
>> +						    (void *)&dsmas_list);
>> +			if (rc < 0)
>> +				dev_warn(dev, "Failed to parse DSMAS: %d\n", rc);
>> +
>> +			dsmas_list_destroy(&dsmas_list);
>> +		}
>> +	}
>>   
>>   	if (is_cxl_endpoint(port))
>>   		return cxl_endpoint_port_probe(port);
>>
>>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 06/21] cxl: Add callback to parse the DSLBIS subtable from CDAT
  2023-03-29  0:44   ` Alison Schofield
@ 2023-03-29 20:59     ` Dave Jiang
  2023-03-29 21:59       ` Alison Schofield
  0 siblings, 1 reply; 40+ messages in thread
From: Dave Jiang @ 2023-03-29 20:59 UTC (permalink / raw)
  To: Alison Schofield
  Cc: linux-cxl, linux-acpi, dan.j.williams, ira.weiny, vishal.l.verma,
	rafael, lukas



On 3/28/23 5:44 PM, Alison Schofield wrote:
> On Mon, Mar 27, 2023 at 02:44:39PM -0700, Dave Jiang wrote:
>> Provide a callback to parse the Device Scoped Latency and Bandwidth
>> Information Structure (DSLBIS) in the CDAT structures. The DSLBIS
>> contains the bandwidth and latency information that's tied to a DSMAS
>> handle. The driver will retrieve the read and write latency and
>> bandwidth associated with the DSMAS which is tied to a DPA range.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>
>> ---
>> v2:
>> - Add size check to DSLIBIS table. (Lukas)
>> - Remove unnecessary entry type check. (Jonathan)
>> - Move data_type check to after match. (Jonathan)
>> - Skip unknown data type. (Jonathan)
>> - Add overflow check for unit multiply. (Jonathan)
>> - Use dev_warn() when entries parsing fail. (Jonathan)
>> ---
>>   drivers/cxl/core/cdat.c |   41 +++++++++++++++++++++++++++++++++++++++++
>>   drivers/cxl/cxlpci.h    |   34 +++++++++++++++++++++++++++++++++-
>>   drivers/cxl/port.c      |    9 ++++++++-
>>   3 files changed, 82 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
>> index d068609fb6f9..0e88973e9f38 100644
>> --- a/drivers/cxl/core/cdat.c
>> +++ b/drivers/cxl/core/cdat.c
>> @@ -1,5 +1,6 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /* Copyright(c) 2023 Intel Corporation. All rights reserved. */
>> +#include <linux/overflow.h>
>>   #include "cxlpci.h"
>>   #include "cxl.h"
>>   
>> @@ -124,3 +125,43 @@ int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg)
>>   	return 0;
>>   }
>>   EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL);
>> +
>> +int cxl_dslbis_parse_entry(struct cdat_entry_header *header, void *arg)
>> +{
>> +	struct cdat_dslbis *dslbis = (struct cdat_dslbis *)header;
>> +	struct list_head *dsmas_list = (struct list_head *)arg;
> 
> cast from void ?

ok

> 
>> +	struct dsmas_entry *dent;
>> +
>> +	if (dslbis->hdr.length != sizeof(*dslbis)) {
>> +		pr_warn("Malformed DSLBIS table length: (%lu:%u)\n",
>> +			(unsigned long)sizeof(*dslbis), dslbis->hdr.length);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Unrecognized data type, we can skip */
> 
> /* Skip unrecognized data type */

ok

> 
>> +	if (dslbis->data_type >= HMAT_SLLBIS_DATA_TYPE_MAX)
>> +		return 0;
>> +
>> +	list_for_each_entry(dent, dsmas_list, list) {
>> +		u64 val;
>> +		int rc;
>> +
>> +		if (dslbis->handle != dent->handle)
>> +			continue;
>> +
>> +		/* Not a memory type, skip */
>> +		if ((dslbis->flags & DSLBIS_MEM_MASK) != DSLBIS_MEM_MEMORY)
>> +			return 0;
>> +
>> +		rc = check_mul_overflow(le64_to_cpu(dslbis->entry_base_unit),
>> +					le16_to_cpu(dslbis->entry[0]), &val);
>> +		if (unlikely(rc))
>> +			pr_warn("DSLBIS value overflowed!\n");
> 
> Must this be shouted !
> I have a vague recollection of being told not to use exclamation points
> in user visible messages.

ok

> 
>> +
>> +		dent->qos[dslbis->data_type] = val;
>> +		break;
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_dslbis_parse_entry, CXL);
>> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
>> index 9a2468a93d83..1429de49e0c4 100644
>> --- a/drivers/cxl/cxlpci.h
>> +++ b/drivers/cxl/cxlpci.h
>> @@ -104,10 +104,21 @@ struct cdat_subtable_entry {
>>   	enum cdat_type type;
>>   };
>>   
>> +enum {
>> +	HMAT_SLLBIS_ACCESS_LATENCY = 0,
>> +	HMAT_SLLBIS_READ_LATENCY,
>> +	HMAT_SLLBIS_WRITE_LATENCY,
>> +	HMAT_SLLBIS_ACCESS_BANDWIDTH,
>> +	HMAT_SLLBIS_READ_BANDWIDTH,
>> +	HMAT_SLLBIS_WRITE_BANDWIDTH,
>> +	HMAT_SLLBIS_DATA_TYPE_MAX
>> +};
>> +
>>   struct dsmas_entry {
>>   	struct list_head list;
>>   	struct range dpa_range;
>>   	u8 handle;
>> +	u64 qos[HMAT_SLLBIS_DATA_TYPE_MAX];
>>   };
>>   
>>   /* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */
>> @@ -120,6 +131,23 @@ struct cdat_dsmas {
>>   	__le64 dpa_length;
>>   } __packed;
>>   
>> +/* Sub-table 1: Device Scoped Latency and Bandwidth Information Structure (DSLBIS) */
>> +struct cdat_dslbis {
>> +	struct cdat_entry_header hdr;
>> +	u8 handle;
>> +	u8 flags;
>> +	u8 data_type;
>> +	u8 reserved;
>> +	__le64 entry_base_unit;
>> +	__le16 entry[3];
>> +	__le16 reserved2;
>> +} __packed;
>> +
>> +/* Flags for subtable above */
> 
> /* Flags for DSLBIS subtable */

ok

> 
>> +
>> +#define DSLBIS_MEM_MASK		GENMASK(3, 0)
>> +#define DSLBIS_MEM_MEMORY	0
>> +
>>   int devm_cxl_port_enumerate_dports(struct cxl_port *port);
>>   struct cxl_dev_state;
>>   int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
>> @@ -136,5 +164,9 @@ cdat_table_parse(dsmas);
>>   cdat_table_parse(dslbis);
>>   cdat_table_parse(sslbis);
>>   
>> -int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg);
>> +#define cxl_parse_entry(x) \
>> +int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg)
>> +
>> +cxl_parse_entry(dsmas);
>> +cxl_parse_entry(dslbis);
>>   #endif /* __CXL_PCI_H__ */
>> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
>> index c8136797d528..6f2b327f7128 100644
>> --- a/drivers/cxl/port.c
>> +++ b/drivers/cxl/port.c
>> @@ -152,8 +152,15 @@ static int cxl_port_probe(struct device *dev)
>>   			rc = cdat_table_parse_dsmas(port->cdat.table,
>>   						    cxl_dsmas_parse_entry,
>>   						    (void *)&dsmas_list);
>> -			if (rc < 0)
>> +			if (rc > 0) {
>> +				rc = cdat_table_parse_dslbis(port->cdat.table,
>> +							     cxl_dslbis_parse_entry,
>> +							     (void *)&dsmas_list);
>> +				if (rc <= 0)
>> +					dev_warn(dev, "Failed to parse DSLBIS: %d\n", rc);
>> +			} else {
>>   				dev_warn(dev, "Failed to parse DSMAS: %d\n", rc);
>> +			}
> 
> I see you touch this func more than once. Maybe some earlier nips and
> tucks, makes this more readable.

Not sure what you mean.

> 
>>   
>>   			dsmas_list_destroy(&dsmas_list);
>>   		}
>>
>>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID
  2023-03-29  1:27   ` Alison Schofield
@ 2023-03-29 21:44     ` Dave Jiang
  0 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-29 21:44 UTC (permalink / raw)
  To: Alison Schofield
  Cc: linux-cxl, linux-acpi, Dan Williams, ira.weiny, vishal.l.verma,
	rafael, lukas



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,
>>   };
>>   
>>
>>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 21/21] cxl/mem: Add debugfs output for QTG related data
  2023-03-29  1:13   ` Alison Schofield
@ 2023-03-29 21:49     ` Dave Jiang
  0 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-29 21:49 UTC (permalink / raw)
  To: Alison Schofield
  Cc: linux-cxl, linux-acpi, Dan Williams, ira.weiny, vishal.l.verma,
	rafael, lukas



On 3/28/23 6:13 PM, Alison Schofield wrote:
> On Mon, Mar 27, 2023 at 02:46:11PM -0700, Dave Jiang wrote:
>> Add debugfs output to /sys/kernel/debug/cxl/memX/qtgmap
>> The debugfs attribute will dump out all the DSMAS ranges and the associated
>> QTG ID exported by the CXL device CDAT.
> 
> Do you want to document these?
> 
> The poison inject & clear inflight patchset documents in:
> Documentation/ABI/testing/debugfs-cxl

Sure. I'll take a look at your series to see what it should look like.

> 
> Alison
> 
>>
>> Suggested-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>   drivers/cxl/mem.c |   16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
>> index 39c4b54f0715..bf2cb5a54a7f 100644
>> --- a/drivers/cxl/mem.c
>> +++ b/drivers/cxl/mem.c
>> @@ -45,6 +45,21 @@ static int cxl_mem_dpa_show(struct seq_file *file, void *data)
>>   	return 0;
>>   }
>>   
>> +static int cxl_mem_qtg_show(struct seq_file *file, void *data)
>> +{
>> +	struct device *dev = file->private;
>> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>> +	struct qos_prop_entry *qos;
>> +
>> +	list_for_each_entry(qos, &cxlmd->qos_list, list) {
>> +		seq_printf(file, "%08llx-%08llx : QTG ID %u\n",
>> +			   qos->dpa_range.start, qos->dpa_range.end,
>> +			   qos->qtg_id);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
>>   				 struct cxl_dport *parent_dport)
>>   {
>> @@ -117,6 +132,7 @@ static int cxl_mem_probe(struct device *dev)
>>   
>>   	dentry = cxl_debugfs_create_dir(dev_name(dev));
>>   	debugfs_create_devm_seqfile(dev, "dpamem", dentry, cxl_mem_dpa_show);
>> +	debugfs_create_devm_seqfile(dev, "qtgmap", dentry, cxl_mem_qtg_show);
>>   	rc = devm_add_action_or_reset(dev, remove_debugfs, dentry);
>>   	if (rc)
>>   		return rc;
>>
>>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* RE: [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID
  2023-03-27 21:46 ` [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID Dave Jiang
  2023-03-29  1:27   ` Alison Schofield
@ 2023-03-29 21:55   ` Dan Williams
  2023-03-29 22:02     ` Dave Jiang
  1 sibling, 1 reply; 40+ messages in thread
From: Dan Williams @ 2023-03-29 21:55 UTC (permalink / raw)
  To: Dave Jiang, linux-cxl, linux-acpi
  Cc: Dan Williams, ira.weiny, vishal.l.verma, alison.schofield, rafael, lukas

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
> 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

Oh, I was still thinking there would be a qtg_id per partition type,
just not a multiple qtg_ids per partition type until it is clear that
those are something hardware vendors are actually going to ship, but I
expect a DSMAS per partition type will be common.

So I was expecting:

/sys/bus/cxl/devices/memX/{ram,pmem}/qtg_id

...and when the DCD patches land that expands to:

/sys/bus/cxl/devices/memX/{ram,pmem,dcd[0-7]}/qtg_id

If someone builds a device with multiple performance classes per
partition then it would become:

/sys/bus/cxl/devices/memX/{ram,pmem,dcd[0-7]}/qtg_id
/sys/bus/cxl/devices/memX/{ram,pmem,dcd[0-7]}/qtg_id[1..n]
/sys/bus/cxl/devices/memX/{ram,pmem,dcd[0-7]}/qtg_range/
/sys/bus/cxl/devices/memX/{ram,pmem,dcd[0-7]}/qtg_range[1..n]/

...where I am using CXL 3.0 Figure 9-24 "DCD DPA Space Example" as the
delineation of the possible partition types.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 06/21] cxl: Add callback to parse the DSLBIS subtable from CDAT
  2023-03-29 20:59     ` Dave Jiang
@ 2023-03-29 21:59       ` Alison Schofield
  0 siblings, 0 replies; 40+ messages in thread
From: Alison Schofield @ 2023-03-29 21:59 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, linux-acpi, dan.j.williams, ira.weiny, vishal.l.verma,
	rafael, lukas

On Wed, Mar 29, 2023 at 01:59:12PM -0700, Dave Jiang wrote:
> 
> 
> On 3/28/23 5:44 PM, Alison Schofield wrote:
> > On Mon, Mar 27, 2023 at 02:44:39PM -0700, Dave Jiang wrote:
> > > Provide a callback to parse the Device Scoped Latency and Bandwidth
> > > Information Structure (DSLBIS) in the CDAT structures. The DSLBIS
> > > contains the bandwidth and latency information that's tied to a DSMAS
> > > handle. The driver will retrieve the read and write latency and
> > > bandwidth associated with the DSMAS which is tied to a DPA range.
> > > 
> > > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > > 
> > > ---
> > > v2:
> > > - Add size check to DSLIBIS table. (Lukas)
> > > - Remove unnecessary entry type check. (Jonathan)
> > > - Move data_type check to after match. (Jonathan)
> > > - Skip unknown data type. (Jonathan)
> > > - Add overflow check for unit multiply. (Jonathan)
> > > - Use dev_warn() when entries parsing fail. (Jonathan)
> > > ---
> > >   drivers/cxl/core/cdat.c |   41 +++++++++++++++++++++++++++++++++++++++++
> > >   drivers/cxl/cxlpci.h    |   34 +++++++++++++++++++++++++++++++++-
> > >   drivers/cxl/port.c      |    9 ++++++++-
> > >   3 files changed, 82 insertions(+), 2 deletions(-)
> > > 

snip

> > > --- a/drivers/cxl/port.c
> > > +++ b/drivers/cxl/port.c
> > > @@ -152,8 +152,15 @@ static int cxl_port_probe(struct device *dev)
> > >   			rc = cdat_table_parse_dsmas(port->cdat.table,
> > >   						    cxl_dsmas_parse_entry,
> > >   						    (void *)&dsmas_list);
> > > -			if (rc < 0)
> > > +			if (rc > 0) {
> > > +				rc = cdat_table_parse_dslbis(port->cdat.table,
> > > +							     cxl_dslbis_parse_entry,
> > > +							     (void *)&dsmas_list);
> > > +				if (rc <= 0)
> > > +					dev_warn(dev, "Failed to parse DSLBIS: %d\n", rc);
> > > +			} else {
> > >   				dev_warn(dev, "Failed to parse DSMAS: %d\n", rc);
> > > +			}
> > 
> > I see you touch this func more than once. Maybe some earlier nips and
> > tucks, makes this more readable.
> 
> Not sure what you mean.

I thought this was the same func, cxl_port_probe(), that I commented
on in the previous patch, so maybe it was already getting re-aligned.

> 
> > 
> > >   			dsmas_list_destroy(&dsmas_list);
> > >   		}
> > > 
> > > 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID
  2023-03-29 21:55   ` Dan Williams
@ 2023-03-29 22:02     ` Dave Jiang
  0 siblings, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-29 22:02 UTC (permalink / raw)
  To: Dan Williams, linux-cxl, linux-acpi
  Cc: ira.weiny, vishal.l.verma, alison.schofield, rafael, lukas



On 3/29/23 2:55 PM, Dan Williams wrote:
> 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
>> 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
> 
> Oh, I was still thinking there would be a qtg_id per partition type,
> just not a multiple qtg_ids per partition type until it is clear that
> those are something hardware vendors are actually going to ship, but I
> expect a DSMAS per partition type will be common.

Oh ok. I guess I really need to save previous changes. Time to revert. I 
hope I still have the old cxl cli changes as well. :(

> 
> So I was expecting:
> 
> /sys/bus/cxl/devices/memX/{ram,pmem}/qtg_id
> 
> ...and when the DCD patches land that expands to:
> 
> /sys/bus/cxl/devices/memX/{ram,pmem,dcd[0-7]}/qtg_id
> 
> If someone builds a device with multiple performance classes per
> partition then it would become:
> 
> /sys/bus/cxl/devices/memX/{ram,pmem,dcd[0-7]}/qtg_id
> /sys/bus/cxl/devices/memX/{ram,pmem,dcd[0-7]}/qtg_id[1..n]
> /sys/bus/cxl/devices/memX/{ram,pmem,dcd[0-7]}/qtg_range/
> /sys/bus/cxl/devices/memX/{ram,pmem,dcd[0-7]}/qtg_range[1..n]/
> 
> ...where I am using CXL 3.0 Figure 9-24 "DCD DPA Space Example" as the
> delineation of the possible partition types.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 01/21] cxl: Export QTG ids from CFMWS to sysfs
  2023-03-27 21:44 ` [PATCH v2 01/21] cxl: Export QTG ids from CFMWS to sysfs Dave Jiang
@ 2023-03-29 23:57   ` Ira Weiny
  0 siblings, 0 replies; 40+ messages in thread
From: Ira Weiny @ 2023-03-29 23:57 UTC (permalink / raw)
  To: Dave Jiang, linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Dave Jiang 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.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 02/21] cxl: Add checksum verification to CDAT from CXL
  2023-03-27 21:44 ` [PATCH v2 02/21] cxl: Add checksum verification to CDAT from CXL Dave Jiang
  2023-03-29  0:03   ` Alison Schofield
@ 2023-03-30  0:09   ` Ira Weiny
  1 sibling, 0 replies; 40+ messages in thread
From: Ira Weiny @ 2023-03-30  0:09 UTC (permalink / raw)
  To: Dave Jiang, linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Dave Jiang wrote:
> A CDAT table is available from a CXL device. The table is read by the
> driver and cached in software. With the CXL subsystem needing to parse the
> CDAT table, the checksum should be verified. Add checksum verification
> after the CDAT table is read from device.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 03/21] cxl: Add support for reading CXL switch CDAT table
  2023-03-27 21:44 ` [PATCH v2 03/21] cxl: Add support for reading CXL switch CDAT table Dave Jiang
@ 2023-03-30  0:19   ` Ira Weiny
  0 siblings, 0 replies; 40+ messages in thread
From: Ira Weiny @ 2023-03-30  0:19 UTC (permalink / raw)
  To: Dave Jiang, linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas

Dave Jiang wrote:
> Move read_cdat_data() from endpoint probe to general port probe to
> allow reading of CDAT data for CXL switches as well as CXL device.
> Add wrapper support for cxl_test to bypass the cdat reading.

I'm not sure why you needed to add this wrapper.  Overall I don't have an
issue with it though.

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/pci.c        |   20 +++++++++++++++-----
>  drivers/cxl/port.c            |    6 +++---
>  tools/testing/cxl/Kbuild      |    1 +
>  tools/testing/cxl/test/mock.c |    5 +++++
>  4 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index e0d5e6525c0d..4241c7b8d5c2 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -546,16 +546,26 @@ static unsigned char cdat_checksum(void *buf, size_t size)
>   */
>  void read_cdat_data(struct cxl_port *port)
>  {
> -	struct pci_doe_mb *cdat_doe;
> -	struct device *dev = &port->dev;
>  	struct device *uport = port->uport;
> -	struct cxl_memdev *cxlmd = to_cxl_memdev(uport);
> -	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> -	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
> +	struct device *dev = &port->dev;
> +	struct cxl_dev_state *cxlds;
> +	struct pci_doe_mb *cdat_doe;
> +	struct cxl_memdev *cxlmd;
> +	struct pci_dev *pdev;
>  	size_t cdat_length;
>  	void *cdat_table;
>  	int rc;
>  
> +	if (is_cxl_memdev(uport)) {
> +		cxlmd = to_cxl_memdev(uport);
> +		cxlds = cxlmd->cxlds;
> +		pdev = to_pci_dev(cxlds->dev);
> +	} else if (dev_is_pci(uport)) {
> +		pdev = to_pci_dev(uport);
> +	} else {
> +		return;
> +	}
> +
>  	cdat_doe = pci_find_doe_mailbox(pdev, PCI_DVSEC_VENDOR_ID_CXL,
>  					CXL_DOE_PROTOCOL_TABLE_ACCESS);
>  	if (!cdat_doe) {
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 1049bb5ea496..60a865680e22 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -93,9 +93,6 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>  	if (IS_ERR(cxlhdm))
>  		return PTR_ERR(cxlhdm);
>  
> -	/* Cache the data early to ensure is_visible() works */
> -	read_cdat_data(port);
> -
>  	get_device(&cxlmd->dev);
>  	rc = devm_add_action_or_reset(&port->dev, schedule_detach, cxlmd);
>  	if (rc)
> @@ -135,6 +132,9 @@ static int cxl_port_probe(struct device *dev)
>  {
>  	struct cxl_port *port = to_cxl_port(dev);
>  
> +	/* Cache the data early to ensure is_visible() works */
> +	read_cdat_data(port);
> +
>  	if (is_cxl_endpoint(port))
>  		return cxl_endpoint_port_probe(port);
>  	return cxl_switch_port_probe(port);
> diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
> index fba7bec96acd..2637c71f3378 100644
> --- a/tools/testing/cxl/Kbuild
> +++ b/tools/testing/cxl/Kbuild
> @@ -12,6 +12,7 @@ ldflags-y += --wrap=cxl_await_media_ready
>  ldflags-y += --wrap=cxl_hdm_decode_init
>  ldflags-y += --wrap=cxl_dvsec_rr_decode
>  ldflags-y += --wrap=cxl_rcrb_to_component
> +ldflags-y += --wrap=read_cdat_data
>  
>  DRIVERS := ../../../drivers
>  CXL_SRC := $(DRIVERS)/cxl
> diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
> index c4e53f22e421..3a75909b2aae 100644
> --- a/tools/testing/cxl/test/mock.c
> +++ b/tools/testing/cxl/test/mock.c
> @@ -263,6 +263,11 @@ resource_size_t __wrap_cxl_rcrb_to_component(struct device *dev,
>  }
>  EXPORT_SYMBOL_NS_GPL(__wrap_cxl_rcrb_to_component, CXL);
>  
> +void __wrap_read_cdat_data(struct cxl_port *port)
> +{
> +}
> +EXPORT_SYMBOL_NS_GPL(__wrap_read_cdat_data, CXL);
> +
>  MODULE_LICENSE("GPL v2");
>  MODULE_IMPORT_NS(ACPI);
>  MODULE_IMPORT_NS(CXL);
> 
> 



^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 05/21] cxl: Add callback to parse the DSMAS subtables from CDAT
  2023-03-27 21:44 ` [PATCH v2 05/21] cxl: Add callback to parse the DSMAS subtables from CDAT Dave Jiang
  2023-03-29  0:20   ` Alison Schofield
@ 2023-03-30 15:43   ` Dave Jiang
  1 sibling, 0 replies; 40+ messages in thread
From: Dave Jiang @ 2023-03-30 15:43 UTC (permalink / raw)
  To: linux-cxl, linux-acpi
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	rafael, lukas



On 3/27/23 2:44 PM, Dave Jiang wrote:
> Provide a callback function to the CDAT parser in order to parse the Device
> Scoped Memory Affinity Structure (DSMAS). Each DSMAS structure contains the
> DPA range and its associated attributes in each entry. See the CDAT
> specification for details.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> ---
> v2:
> - Add DSMAS table size check. (Lukas)
> - Use local DSMAS header for LE handling.
> - Remove dsmas lock. (Jonathan)
> - Fix handle size (Jonathan)
> - Add LE to host conversion for DSMAS address and length.
> - Make dsmas_list local
> ---
>   drivers/cxl/core/cdat.c |   26 ++++++++++++++++++++++++++
>   drivers/cxl/cxl.h       |    1 +
>   drivers/cxl/cxlpci.h    |   18 ++++++++++++++++++
>   drivers/cxl/port.c      |   24 ++++++++++++++++++++++++
>   4 files changed, 69 insertions(+)
> 
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index 210f4499bddb..d068609fb6f9 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -98,3 +98,29 @@ int cdat_table_parse_sslbis(struct cdat_header *table,
>   	return cdat_table_parse_entries(CDAT_TYPE_SSLBIS, table, &proc);
>   }
>   EXPORT_SYMBOL_NS_GPL(cdat_table_parse_sslbis, CXL);
> +
> +int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg)
> +{
> +	struct cdat_dsmas *dsmas = (struct cdat_dsmas *)(header);
> +	struct list_head *dsmas_list = (struct list_head *)arg;
> +	struct dsmas_entry *dent;
> +
> +	if (dsmas->hdr.length != sizeof(*dsmas)) {
> +		pr_warn("Malformed DSMAS table length: (%lu:%u)\n",
> +			 (unsigned long)sizeof(*dsmas), dsmas->hdr.length);
> +		return -EINVAL;
> +	}
> +
> +	dent = kzalloc(sizeof(*dent), GFP_KERNEL);
> +	if (!dent)
> +		return -ENOMEM;
> +
> +	dent->handle = dsmas->dsmad_handle;
> +	dent->dpa_range.start = le64_to_cpu(dsmas->dpa_base_address);
> +	dent->dpa_range.end = le64_to_cpu(dsmas->dpa_base_address) +
> +			      le64_to_cpu(dsmas->dpa_length) - 1;
> +	list_add_tail(&dent->list, dsmas_list);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index cc3309794b45..9d0e22fe72c0 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -8,6 +8,7 @@
>   #include <linux/bitfield.h>
>   #include <linux/bitops.h>
>   #include <linux/log2.h>
> +#include <linux/list.h>
>   #include <linux/io.h>
>   
>   /**
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index 45e2f2bf5ef8..9a2468a93d83 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -104,6 +104,22 @@ struct cdat_subtable_entry {
>   	enum cdat_type type;
>   };
>   
> +struct dsmas_entry {
> +	struct list_head list;
> +	struct range dpa_range;
> +	u8 handle;
> +};
> +
> +/* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */
> +struct cdat_dsmas {
> +	struct cdat_entry_header hdr;
> +	u8 dsmad_handle;
> +	u8 flags;
> +	__u16 reserved;
> +	__le64 dpa_base_address;
> +	__le64 dpa_length;
> +} __packed;
> +
>   int devm_cxl_port_enumerate_dports(struct cxl_port *port);
>   struct cxl_dev_state;
>   int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
> @@ -119,4 +135,6 @@ int cdat_table_parse_##x(struct cdat_header *table, \
>   cdat_table_parse(dsmas);
>   cdat_table_parse(dslbis);
>   cdat_table_parse(sslbis);
> +
> +int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg);
>   #endif /* __CXL_PCI_H__ */
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 60a865680e22..c8136797d528 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -57,6 +57,16 @@ static int discover_region(struct device *dev, void *root)
>   	return 0;
>   }
>   
> +static void dsmas_list_destroy(struct list_head *dsmas_list)
> +{
> +	struct dsmas_entry *dentry, *n;
> +
> +	list_for_each_entry_safe(dentry, n, dsmas_list, list) {
> +		list_del(&dentry->list);
> +		kfree(dentry);
> +	}
> +}
> +
>   static int cxl_switch_port_probe(struct cxl_port *port)
>   {
>   	struct cxl_hdm *cxlhdm;
> @@ -131,9 +141,23 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>   static int cxl_port_probe(struct device *dev)
>   {
>   	struct cxl_port *port = to_cxl_port(dev);
> +	int rc;
>   
>   	/* Cache the data early to ensure is_visible() works */
>   	read_cdat_data(port);
> +	if (port->cdat.table) {
> +		if (is_cxl_endpoint(port)) {
> +			LIST_HEAD(dsmas_list);
> +
> +			rc = cdat_table_parse_dsmas(port->cdat.table,
> +						    cxl_dsmas_parse_entry,
> +						    (void *)&dsmas_list);
> +			if (rc < 0)
> +				dev_warn(dev, "Failed to parse DSMAS: %d\n", rc);
> +
> +			dsmas_list_destroy(&dsmas_list);
> +		}
> +	}

This block needs to be moved to cxl_endpoint_port_probe() after media is 
ready.

>   
>   	if (is_cxl_endpoint(port))
>   		return cxl_endpoint_port_probe(port);
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2023-03-30 15:46 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-27 21:44 [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang
2023-03-27 21:44 ` [PATCH v2 01/21] cxl: Export QTG ids from CFMWS to sysfs Dave Jiang
2023-03-29 23:57   ` Ira Weiny
2023-03-27 21:44 ` [PATCH v2 02/21] cxl: Add checksum verification to CDAT from CXL Dave Jiang
2023-03-29  0:03   ` Alison Schofield
2023-03-29  0:21     ` Dave Jiang
2023-03-30  0:09   ` Ira Weiny
2023-03-27 21:44 ` [PATCH v2 03/21] cxl: Add support for reading CXL switch CDAT table Dave Jiang
2023-03-30  0:19   ` Ira Weiny
2023-03-27 21:44 ` [PATCH v2 04/21] cxl: Add common helpers for cdat parsing Dave Jiang
2023-03-27 21:44 ` [PATCH v2 05/21] cxl: Add callback to parse the DSMAS subtables from CDAT Dave Jiang
2023-03-29  0:20   ` Alison Schofield
2023-03-29 20:41     ` Dave Jiang
2023-03-30 15:43   ` Dave Jiang
2023-03-27 21:44 ` [PATCH v2 06/21] cxl: Add callback to parse the DSLBIS subtable " Dave Jiang
2023-03-29  0:44   ` Alison Schofield
2023-03-29 20:59     ` Dave Jiang
2023-03-29 21:59       ` Alison Schofield
2023-03-27 21:44 ` [PATCH v2 07/21] cxl: Add callback to parse the SSLBIS " Dave Jiang
2023-03-27 21:44 ` [PATCH v2 08/21] cxl: Add support for _DSM Function for retrieving QTG ID Dave Jiang
2023-03-27 21:44 ` [PATCH v2 09/21] cxl: Add helper function to retrieve ACPI handle of CXL root device Dave Jiang
2023-03-27 21:45 ` [PATCH v2 10/21] cxl: Add helpers to calculate pci latency for the CXL device Dave Jiang
2023-03-27 21:45 ` [PATCH v2 11/21] cxl: Add helper function that calculates QoS values for switches Dave Jiang
2023-03-27 21:45 ` [PATCH v2 12/21] cxl: Add helper function that calculate QoS values for PCI path Dave Jiang
2023-03-27 21:45 ` [PATCH v2 13/21] ACPI: NUMA: Add genport target allocation to the HMAT parsing Dave Jiang
2023-03-27 21:45 ` [PATCH v2 14/21] ACPI: NUMA: Add helper function to retrieve the performance attributes Dave Jiang
2023-03-27 21:45 ` [PATCH v2 15/21] cxl: Add helper function to retrieve generic port QoS Dave Jiang
2023-03-27 21:45 ` [PATCH v2 16/21] cxl: Add latency and bandwidth calculations for the CXL path Dave Jiang
2023-03-27 21:45 ` [PATCH v2 17/21] cxl: Wait Memory_Info_Valid before access memory related info Dave Jiang
2023-03-27 21:45 ` [PATCH v2 18/21] cxl: Move identify and partition query from pci probe to port probe Dave Jiang
2023-03-27 21:46 ` [PATCH v2 19/21] cxl: Store QTG IDs and related info to the CXL memory device context Dave Jiang
2023-03-27 21:46 ` [PATCH v2 20/21] cxl: Export sysfs attributes for memory device QTG ID Dave Jiang
2023-03-29  1:27   ` Alison Schofield
2023-03-29 21:44     ` Dave Jiang
2023-03-29 21:55   ` Dan Williams
2023-03-29 22:02     ` Dave Jiang
2023-03-27 21:46 ` [PATCH v2 21/21] cxl/mem: Add debugfs output for QTG related data Dave Jiang
2023-03-29  1:13   ` Alison Schofield
2023-03-29 21:49     ` Dave Jiang
2023-03-28 17:45 ` [PATCH v2 00/21] cxl: Add support for QTG ID retrieval for CXL subsystem Dave Jiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).