[RFC 0/2] type2 cxl initialization

[RFC 0/2] type2 cxl initialization

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Avoiding to send the full type2 support patchset until there is an
agreement about how to do the cxl initialization from accel drivers.

Using the idea from fwctl for embedding cxl_dev_state in a private
accel driver struct and a macro helping for the allocation and
initialization of such struct.

The main problem is the amount of internal cxl structs which need to be
public for accel drivers which we tried to avoid since v1 of the type2
patchset history.

Alejandro Lucero (2):
  cxl: add type2 device basic support
  sfc: add cxl support

 drivers/cxl/core/mbox.c               |   3 +-
 drivers/cxl/core/memdev.c             |  25 +++
 drivers/cxl/core/pci.c                |   1 +
 drivers/cxl/core/regs.c               |   1 +
 drivers/cxl/cxl.h                     |  98 +----------
 drivers/cxl/cxlmem.h                  | 108 +------------
 drivers/cxl/cxlpci.h                  |  21 ---
 drivers/cxl/pci.c                     |  17 +-
 drivers/net/ethernet/sfc/Kconfig      |   6 +
 drivers/net/ethernet/sfc/Makefile     |   1 +
 drivers/net/ethernet/sfc/efx.c        |  15 +-
 drivers/net/ethernet/sfc/efx_cxl.c    |  53 ++++++
 drivers/net/ethernet/sfc/efx_cxl.h    |  40 +++++
 drivers/net/ethernet/sfc/net_driver.h |  10 ++
 include/cxl/cxl.h                     | 225 ++++++++++++++++++++++++++
 include/cxl/pci.h                     |  23 +++
 16 files changed, 417 insertions(+), 230 deletions(-)
 create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c
 create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h
 create mode 100644 include/cxl/cxl.h
 create mode 100644 include/cxl/pci.h


base-commit: d5d2106e2118c4e09fef131d9889f79559b95bfc
-- 
2.34.1

[RFC type cxl initialization 1/2] cxl: add type2 device basic support

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Differentiate CXL memory expanders (type 3) from CXL device accelerators
(type 2) with a new function for initializing cxl_dev_state and a macro
for helping accel drivers to embed cxl_dev_state inside a private
struct.

Move structs to include/cxl as the size of the accel driver private
struct embedding cxl_dev_state needs to know the size of this struct.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
---
 drivers/cxl/core/mbox.c   |   3 +-
 drivers/cxl/core/memdev.c |  25 +++++
 drivers/cxl/core/pci.c    |   1 +
 drivers/cxl/core/regs.c   |   1 +
 drivers/cxl/cxl.h         |  98 +----------------
 drivers/cxl/cxlmem.h      | 108 +-----------------
 drivers/cxl/cxlpci.h      |  21 ----
 drivers/cxl/pci.c         |  17 +--
 include/cxl/cxl.h         | 225 ++++++++++++++++++++++++++++++++++++++
 include/cxl/pci.h         |  23 ++++
 10 files changed, 293 insertions(+), 229 deletions(-)
 create mode 100644 include/cxl/cxl.h
 create mode 100644 include/cxl/pci.h

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index c5eedcae3b02..ae92db38a921 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1426,7 +1426,8 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL");
 
-struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev)
+struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
+						 u16 dvsec)
 {
 	struct cxl_memdev_state *mds;
 
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 2914e3ff104b..14bd74823467 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -636,6 +636,31 @@ static void detach_memdev(struct work_struct *work)
 
 static struct lock_class_key cxl_memdev_key;
 
+void cxl_dev_state_init(struct cxl_dev_state *cxlds, struct device *dev,
+			enum cxl_devtype type, u64 serial, u16 dvsec)
+{
+	cxlds->dev = dev;
+	cxlds->type = type;
+	cxlds->serial = serial;
+	cxlds->cxl_dvsec = dvsec;
+	cxlds->reg_map.host = dev;
+	cxlds->reg_map.resource = CXL_RESOURCE_NONE;
+}
+
+struct cxl_dev_state *_cxl_dev_state_create(struct device *dev,
+					   u64 serial, u16 dvsec,
+					   size_t size)
+{
+	struct cxl_dev_state *cxlds __free(kfree) = kzalloc(size, GFP_KERNEL);
+
+	if (!cxlds)
+		return NULL;
+
+	cxl_dev_state_init(cxlds, dev, CXL_DEVTYPE_DEVMEM, serial, dvsec);
+	return_ptr(cxlds);
+}
+EXPORT_SYMBOL_NS_GPL(_cxl_dev_state_create, "CXL");
+
 static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
 					   const struct file_operations *fops)
 {
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index a5c65f79db18..36a1cf4e59fb 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright(c) 2021 Intel Corporation. All rights reserved. */
+#include <cxl/pci.h>
 #include <linux/units.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/device.h>
diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
index 117c2e94c761..58a942a4946c 100644
--- a/drivers/cxl/core/regs.c
+++ b/drivers/cxl/core/regs.c
@@ -4,6 +4,7 @@
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
+#include <cxl/pci.h>
 #include <cxlmem.h>
 #include <cxlpci.h>
 #include <pmu.h>
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 6baec4ba9141..556b1ed24f0e 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -9,8 +9,8 @@
 #include <linux/notifier.h>
 #include <linux/bitops.h>
 #include <linux/log2.h>
-#include <linux/node.h>
 #include <linux/io.h>
+#include <cxl/cxl.h>
 
 extern const struct nvdimm_security_ops *cxl_security_ops;
 
@@ -200,97 +200,6 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)
 #define   CXLDEV_MBOX_BG_CMD_COMMAND_VENDOR_MASK GENMASK_ULL(63, 48)
 #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20
 
-/*
- * Using struct_group() allows for per register-block-type helper routines,
- * without requiring block-type agnostic code to include the prefix.
- */
-struct cxl_regs {
-	/*
-	 * Common set of CXL Component register block base pointers
-	 * @hdm_decoder: CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure
-	 * @ras: CXL 2.0 8.2.5.9 CXL RAS Capability Structure
-	 */
-	struct_group_tagged(cxl_component_regs, component,
-		void __iomem *hdm_decoder;
-		void __iomem *ras;
-	);
-	/*
-	 * Common set of CXL Device register block base pointers
-	 * @status: CXL 2.0 8.2.8.3 Device Status Registers
-	 * @mbox: CXL 2.0 8.2.8.4 Mailbox Registers
-	 * @memdev: CXL 2.0 8.2.8.5 Memory Device Registers
-	 */
-	struct_group_tagged(cxl_device_regs, device_regs,
-		void __iomem *status, *mbox, *memdev;
-	);
-
-	struct_group_tagged(cxl_pmu_regs, pmu_regs,
-		void __iomem *pmu;
-	);
-
-	/*
-	 * RCH downstream port specific RAS register
-	 * @aer: CXL 3.0 8.2.1.1 RCH Downstream Port RCRB
-	 */
-	struct_group_tagged(cxl_rch_regs, rch_regs,
-		void __iomem *dport_aer;
-	);
-
-	/*
-	 * RCD upstream port specific PCIe cap register
-	 * @pcie_cap: CXL 3.0 8.2.1.2 RCD Upstream Port RCRB
-	 */
-	struct_group_tagged(cxl_rcd_regs, rcd_regs,
-		void __iomem *rcd_pcie_cap;
-	);
-};
-
-struct cxl_reg_map {
-	bool valid;
-	int id;
-	unsigned long offset;
-	unsigned long size;
-};
-
-struct cxl_component_reg_map {
-	struct cxl_reg_map hdm_decoder;
-	struct cxl_reg_map ras;
-};
-
-struct cxl_device_reg_map {
-	struct cxl_reg_map status;
-	struct cxl_reg_map mbox;
-	struct cxl_reg_map memdev;
-};
-
-struct cxl_pmu_reg_map {
-	struct cxl_reg_map pmu;
-};
-
-/**
- * struct cxl_register_map - DVSEC harvested register block mapping parameters
- * @host: device for devm operations and logging
- * @base: virtual base of the register-block-BAR + @block_offset
- * @resource: physical resource base of the register block
- * @max_size: maximum mapping size to perform register search
- * @reg_type: see enum cxl_regloc_type
- * @component_map: cxl_reg_map for component registers
- * @device_map: cxl_reg_maps for device registers
- * @pmu_map: cxl_reg_maps for CXL Performance Monitoring Units
- */
-struct cxl_register_map {
-	struct device *host;
-	void __iomem *base;
-	resource_size_t resource;
-	resource_size_t max_size;
-	u8 reg_type;
-	union {
-		struct cxl_component_reg_map component_map;
-		struct cxl_device_reg_map device_map;
-		struct cxl_pmu_reg_map pmu_map;
-	};
-};
-
 void cxl_probe_component_regs(struct device *dev, void __iomem *base,
 			      struct cxl_component_reg_map *map);
 void cxl_probe_device_regs(struct device *dev, void __iomem *base,
@@ -480,11 +389,6 @@ struct cxl_region_params {
 	int nr_targets;
 };
 
-enum cxl_partition_mode {
-	CXL_PARTMODE_RAM,
-	CXL_PARTMODE_PMEM,
-};
-
 /*
  * Indicate whether this region has been assembled by autodetection or
  * userspace assembly. Prevent endpoint decoders outside of automatic
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 8e1e46c348f5..0bdf581044da 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -4,9 +4,9 @@
 #define __CXL_MEM_H__
 #include <uapi/linux/cxl_mem.h>
 #include <linux/pci.h>
-#include <linux/cdev.h>
 #include <linux/uuid.h>
 #include <linux/node.h>
+#include <cxl/cxl.h>
 #include <cxl/event.h>
 #include <cxl/mailbox.h>
 #include "cxl.h"
@@ -34,30 +34,6 @@
 	(FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) !=                       \
 	 CXLMDEV_RESET_NEEDED_NOT)
 
-/**
- * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device
- * @dev: driver core device object
- * @cdev: char dev core object for ioctl operations
- * @cxlds: The device state backing this device
- * @detach_work: active memdev lost a port in its ancestry
- * @cxl_nvb: coordinate removal of @cxl_nvd if present
- * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
- * @endpoint: connection to the CXL port topology for this memory device
- * @id: id number of this memdev instance.
- * @depth: endpoint port depth
- */
-struct cxl_memdev {
-	struct device dev;
-	struct cdev cdev;
-	struct cxl_dev_state *cxlds;
-	struct work_struct detach_work;
-	struct cxl_nvdimm_bridge *cxl_nvb;
-	struct cxl_nvdimm *cxl_nvd;
-	struct cxl_port *endpoint;
-	int id;
-	int depth;
-};
-
 static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
 {
 	return container_of(dev, struct cxl_memdev, dev);
@@ -357,83 +333,6 @@ struct cxl_security_state {
 	struct kernfs_node *sanitize_node;
 };
 
-/*
- * enum cxl_devtype - delineate type-2 from a generic type-3 device
- * @CXL_DEVTYPE_DEVMEM - Vendor specific CXL Type-2 device implementing HDM-D or
- *			 HDM-DB, no requirement that this device implements a
- *			 mailbox, or other memory-device-standard manageability
- *			 flows.
- * @CXL_DEVTYPE_CLASSMEM - Common class definition of a CXL Type-3 device with
- *			   HDM-H and class-mandatory memory device registers
- */
-enum cxl_devtype {
-	CXL_DEVTYPE_DEVMEM,
-	CXL_DEVTYPE_CLASSMEM,
-};
-
-/**
- * struct cxl_dpa_perf - DPA performance property entry
- * @dpa_range: range for DPA address
- * @coord: QoS performance data (i.e. latency, bandwidth)
- * @cdat_coord: raw QoS performance data from CDAT
- * @qos_class: QoS Class cookies
- */
-struct cxl_dpa_perf {
-	struct range dpa_range;
-	struct access_coordinate coord[ACCESS_COORDINATE_MAX];
-	struct access_coordinate cdat_coord[ACCESS_COORDINATE_MAX];
-	int qos_class;
-};
-
-/**
- * struct cxl_dpa_partition - DPA partition descriptor
- * @res: shortcut to the partition in the DPA resource tree (cxlds->dpa_res)
- * @perf: performance attributes of the partition from CDAT
- * @mode: operation mode for the DPA capacity, e.g. ram, pmem, dynamic...
- */
-struct cxl_dpa_partition {
-	struct resource res;
-	struct cxl_dpa_perf perf;
-	enum cxl_partition_mode mode;
-};
-
-/**
- * struct cxl_dev_state - The driver device state
- *
- * cxl_dev_state represents the CXL driver/device state.  It provides an
- * interface to mailbox commands as well as some cached data about the device.
- * Currently only memory devices are represented.
- *
- * @dev: The device associated with this CXL state
- * @cxlmd: The device representing the CXL.mem capabilities of @dev
- * @reg_map: component and ras register mapping parameters
- * @regs: Parsed register blocks
- * @cxl_dvsec: Offset to the PCIe device DVSEC
- * @rcd: operating in RCD mode (CXL 3.0 9.11.8 CXL Devices Attached to an RCH)
- * @media_ready: Indicate whether the device media is usable
- * @dpa_res: Overall DPA resource tree for the device
- * @part: DPA partition array
- * @nr_partitions: Number of DPA partitions
- * @serial: PCIe Device Serial Number
- * @type: Generic Memory Class device or Vendor Specific Memory device
- * @cxl_mbox: CXL mailbox context
- */
-struct cxl_dev_state {
-	struct device *dev;
-	struct cxl_memdev *cxlmd;
-	struct cxl_register_map reg_map;
-	struct cxl_regs regs;
-	int cxl_dvsec;
-	bool rcd;
-	bool media_ready;
-	struct resource dpa_res;
-	struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX];
-	unsigned int nr_partitions;
-	u64 serial;
-	enum cxl_devtype type;
-	struct cxl_mailbox cxl_mbox;
-};
-
 static inline resource_size_t cxl_pmem_size(struct cxl_dev_state *cxlds)
 {
 	/*
@@ -812,7 +711,10 @@ int cxl_dev_state_identify(struct cxl_memdev_state *mds);
 int cxl_await_media_ready(struct cxl_dev_state *cxlds);
 int cxl_enumerate_cmds(struct cxl_memdev_state *mds);
 int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info);
-struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev);
+struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
+						 u16 dvsec);
+void cxl_dev_state_init(struct cxl_dev_state *cxlds, struct device *dev,
+			enum cxl_devtype type, u64 serial, u16 dvsec);
 void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
 				unsigned long *cmds);
 void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 54e219b0049e..570e53e26f11 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -7,29 +7,8 @@
 
 #define CXL_MEMORY_PROGIF	0x10
 
-/*
- * See section 8.1 Configuration Space Registers in the CXL 2.0
- * Specification. Names are taken straight from the specification with "CXL" and
- * "DVSEC" redundancies removed. When obvious, abbreviations may be used.
- */
 #define PCI_DVSEC_HEADER1_LENGTH_MASK	GENMASK(31, 20)
 
-/* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */
-#define CXL_DVSEC_PCIE_DEVICE					0
-#define   CXL_DVSEC_CAP_OFFSET		0xA
-#define     CXL_DVSEC_MEM_CAPABLE	BIT(2)
-#define     CXL_DVSEC_HDM_COUNT_MASK	GENMASK(5, 4)
-#define   CXL_DVSEC_CTRL_OFFSET		0xC
-#define     CXL_DVSEC_MEM_ENABLE	BIT(2)
-#define   CXL_DVSEC_RANGE_SIZE_HIGH(i)	(0x18 + (i * 0x10))
-#define   CXL_DVSEC_RANGE_SIZE_LOW(i)	(0x1C + (i * 0x10))
-#define     CXL_DVSEC_MEM_INFO_VALID	BIT(0)
-#define     CXL_DVSEC_MEM_ACTIVE	BIT(1)
-#define     CXL_DVSEC_MEM_SIZE_LOW_MASK	GENMASK(31, 28)
-#define   CXL_DVSEC_RANGE_BASE_HIGH(i)	(0x20 + (i * 0x10))
-#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 */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index b2c943a4de0a..006a0036e779 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright(c) 2020 Intel Corporation. All rights reserved. */
+#include <cxl/cxl.h>
+#include <cxl/pci.h>
 #include <linux/unaligned.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/moduleparam.h>
@@ -911,6 +913,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	int rc, pmu_count;
 	unsigned int i;
 	bool irq_avail;
+	u16 dvsec;
 
 	/*
 	 * Double check the anonymous union trickery in struct cxl_regs
@@ -924,19 +927,19 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return rc;
 	pci_set_master(pdev);
 
-	mds = cxl_memdev_state_create(&pdev->dev);
+	dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
+					  CXL_DVSEC_PCIE_DEVICE);
+	if (!dvsec)
+		dev_warn(&pdev->dev,
+			 "Device DVSEC not present, skip CXL.mem init\n");
+
+	mds = cxl_memdev_state_create(&pdev->dev, pci_get_dsn(pdev), dvsec);
 	if (IS_ERR(mds))
 		return PTR_ERR(mds);
 	cxlds = &mds->cxlds;
 	pci_set_drvdata(pdev, cxlds);
 
 	cxlds->rcd = is_cxl_restricted(pdev);
-	cxlds->serial = pci_get_dsn(pdev);
-	cxlds->cxl_dvsec = pci_find_dvsec_capability(
-		pdev, PCI_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE);
-	if (!cxlds->cxl_dvsec)
-		dev_warn(&pdev->dev,
-			 "Device DVSEC not present, skip CXL.mem init\n");
 
 	rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
 	if (rc)
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
new file mode 100644
index 000000000000..dc8a68104bfa
--- /dev/null
+++ b/include/cxl/cxl.h
@@ -0,0 +1,225 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2025 Advanced Micro Devices, Inc. */
+
+#ifndef __CXL_H
+#define __CXL_H
+
+#include <linux/cdev.h>
+#include <linux/node.h>
+#include <linux/ioport.h>
+#include <cxl/mailbox.h>
+
+/*
+ * enum cxl_devtype - delineate type-2 from a generic type-3 device
+ * @CXL_DEVTYPE_DEVMEM - Vendor specific CXL Type-2 device implementing HDM-D or
+ *			 HDM-DB, no requirement that this device implements a
+ *			 mailbox, or other memory-device-standard manageability
+ *			 flows.
+ * @CXL_DEVTYPE_CLASSMEM - Common class definition of a CXL Type-3 device with
+ *			   HDM-H and class-mandatory memory device registers
+ */
+enum cxl_devtype {
+	CXL_DEVTYPE_DEVMEM,
+	CXL_DEVTYPE_CLASSMEM,
+};
+
+struct device;
+
+/*
+ * Using struct_group() allows for per register-block-type helper routines,
+ * without requiring block-type agnostic code to include the prefix.
+ */
+struct cxl_regs {
+	/*
+	 * Common set of CXL Component register block base pointers
+	 * @hdm_decoder: CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure
+	 * @ras: CXL 2.0 8.2.5.9 CXL RAS Capability Structure
+	 */
+	struct_group_tagged(cxl_component_regs, component,
+		void __iomem *hdm_decoder;
+		void __iomem *ras;
+	);
+	/*
+	 * Common set of CXL Device register block base pointers
+	 * @status: CXL 2.0 8.2.8.3 Device Status Registers
+	 * @mbox: CXL 2.0 8.2.8.4 Mailbox Registers
+	 * @memdev: CXL 2.0 8.2.8.5 Memory Device Registers
+	 */
+	struct_group_tagged(cxl_device_regs, device_regs,
+		void __iomem *status, *mbox, *memdev;
+	);
+
+	struct_group_tagged(cxl_pmu_regs, pmu_regs,
+		void __iomem *pmu;
+	);
+
+	/*
+	 * RCH downstream port specific RAS register
+	 * @aer: CXL 3.0 8.2.1.1 RCH Downstream Port RCRB
+	 */
+	struct_group_tagged(cxl_rch_regs, rch_regs,
+		void __iomem *dport_aer;
+	);
+
+	/*
+	 * RCD upstream port specific PCIe cap register
+	 * @pcie_cap: CXL 3.0 8.2.1.2 RCD Upstream Port RCRB
+	 */
+	struct_group_tagged(cxl_rcd_regs, rcd_regs,
+		void __iomem *rcd_pcie_cap;
+	);
+};
+
+struct cxl_reg_map {
+	bool valid;
+	int id;
+	unsigned long offset;
+	unsigned long size;
+};
+
+struct cxl_component_reg_map {
+	struct cxl_reg_map hdm_decoder;
+	struct cxl_reg_map ras;
+};
+
+struct cxl_device_reg_map {
+	struct cxl_reg_map status;
+	struct cxl_reg_map mbox;
+	struct cxl_reg_map memdev;
+};
+
+struct cxl_pmu_reg_map {
+	struct cxl_reg_map pmu;
+};
+
+/**
+ * struct cxl_register_map - DVSEC harvested register block mapping parameters
+ * @host: device for devm operations and logging
+ * @base: virtual base of the register-block-BAR + @block_offset
+ * @resource: physical resource base of the register block
+ * @max_size: maximum mapping size to perform register search
+ * @reg_type: see enum cxl_regloc_type
+ * @component_map: cxl_reg_map for component registers
+ * @device_map: cxl_reg_maps for device registers
+ * @pmu_map: cxl_reg_maps for CXL Performance Monitoring Units
+ */
+struct cxl_register_map {
+	struct device *host;
+	void __iomem *base;
+	resource_size_t resource;
+	resource_size_t max_size;
+	u8 reg_type;
+	union {
+		struct cxl_component_reg_map component_map;
+		struct cxl_device_reg_map device_map;
+		struct cxl_pmu_reg_map pmu_map;
+	};
+};
+
+/**
+ * struct cxl_dpa_perf - DPA performance property entry
+ * @dpa_range: range for DPA address
+ * @coord: QoS performance data (i.e. latency, bandwidth)
+ * @cdat_coord: raw QoS performance data from CDAT
+ * @qos_class: QoS Class cookies
+ */
+struct cxl_dpa_perf {
+	struct range dpa_range;
+	struct access_coordinate coord[ACCESS_COORDINATE_MAX];
+	struct access_coordinate cdat_coord[ACCESS_COORDINATE_MAX];
+	int qos_class;
+};
+
+enum cxl_partition_mode {
+	CXL_PARTMODE_RAM,
+	CXL_PARTMODE_PMEM,
+};
+
+/**
+ * struct cxl_dpa_partition - DPA partition descriptor
+ * @res: shortcut to the partition in the DPA resource tree (cxlds->dpa_res)
+ * @perf: performance attributes of the partition from CDAT
+ * @mode: operation mode for the DPA capacity, e.g. ram, pmem, dynamic...
+ */
+struct cxl_dpa_partition {
+	struct resource res;
+	struct cxl_dpa_perf perf;
+	enum cxl_partition_mode mode;
+};
+
+/**
+ * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device
+ * @dev: driver core device object
+ * @cdev: char dev core object for ioctl operations
+ * @cxlds: The device state backing this device
+ * @detach_work: active memdev lost a port in its ancestry
+ * @cxl_nvb: coordinate removal of @cxl_nvd if present
+ * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
+ * @endpoint: connection to the CXL port topology for this memory device
+ * @id: id number of this memdev instance.
+ * @depth: endpoint port depth
+ */
+struct cxl_memdev {
+	struct device dev;
+	struct cdev cdev;
+	struct cxl_dev_state *cxlds;
+	struct work_struct detach_work;
+	struct cxl_nvdimm_bridge *cxl_nvb;
+	struct cxl_nvdimm *cxl_nvd;
+	struct cxl_port *endpoint;
+	int id;
+	int depth;
+};
+
+#define CXL_NR_PARTITIONS_MAX 2
+
+/**
+ * struct cxl_dev_state - The driver device state
+ *
+ * cxl_dev_state represents the CXL driver/device state.  It provides an
+ * interface to mailbox commands as well as some cached data about the device.
+ * Currently only memory devices are represented.
+ *
+ * @dev: The device associated with this CXL state
+ * @cxlmd: The device representing the CXL.mem capabilities of @dev
+ * @reg_map: component and ras register mapping parameters
+ * @regs: Parsed register blocks
+ * @cxl_dvsec: Offset to the PCIe device DVSEC
+ * @rcd: operating in RCD mode (CXL 3.0 9.11.8 CXL Devices Attached to an RCH)
+ * @media_ready: Indicate whether the device media is usable
+ * @dpa_res: Overall DPA resource tree for the device
+ * @part: DPA partition array
+ * @nr_partitions: Number of DPA partitions
+ * @serial: PCIe Device Serial Number
+ * @type: Generic Memory Class device or Vendor Specific Memory device
+ * @cxl_mbox: CXL mailbox context
+ */
+struct cxl_dev_state {
+	struct device *dev;
+	struct cxl_memdev *cxlmd;
+	struct cxl_register_map reg_map;
+	struct cxl_regs regs;
+	int cxl_dvsec;
+	bool rcd;
+	bool media_ready;
+	struct resource dpa_res;
+	struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX];
+	unsigned int nr_partitions;
+	u64 serial;
+	enum cxl_devtype type;
+	struct cxl_mailbox cxl_mbox;
+};
+
+struct cxl_dev_state *_cxl_dev_state_create(struct device *dev,
+					   u64 serial, u16 dvsec,
+					   size_t size);
+
+#define cxl_dev_state_create(parent, serial, dvsec, drv_struct, member) \
+	({								      \
+	 	static_assert(__same_type(struct cxl_dev_state, 	      \
+					((drv_struct *)NULL)->member));	      \
+		static_assert(offsetof(drv_struct, member) == 0);	      \
+		(drv_struct *)_cxl_dev_state_create(parent, serial, dvsec,    \
+						    sizeof(drv_struct));\
+	})
+#endif
diff --git a/include/cxl/pci.h b/include/cxl/pci.h
new file mode 100644
index 000000000000..ad63560caa2c
--- /dev/null
+++ b/include/cxl/pci.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright(c) 2020 Intel Corporation. All rights reserved. */
+
+#ifndef __CXL_ACCEL_PCI_H
+#define __CXL_ACCEL_PCI_H
+
+/* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */
+#define CXL_DVSEC_PCIE_DEVICE					0
+#define   CXL_DVSEC_CAP_OFFSET		0xA
+#define     CXL_DVSEC_MEM_CAPABLE	BIT(2)
+#define     CXL_DVSEC_HDM_COUNT_MASK	GENMASK(5, 4)
+#define   CXL_DVSEC_CTRL_OFFSET		0xC
+#define     CXL_DVSEC_MEM_ENABLE	BIT(2)
+#define   CXL_DVSEC_RANGE_SIZE_HIGH(i)	(0x18 + ((i) * 0x10))
+#define   CXL_DVSEC_RANGE_SIZE_LOW(i)	(0x1C + ((i) * 0x10))
+#define     CXL_DVSEC_MEM_INFO_VALID	BIT(0)
+#define     CXL_DVSEC_MEM_ACTIVE	BIT(1)
+#define     CXL_DVSEC_MEM_SIZE_LOW_MASK	GENMASK(31, 28)
+#define   CXL_DVSEC_RANGE_BASE_HIGH(i)	(0x20 + ((i) * 0x10))
+#define   CXL_DVSEC_RANGE_BASE_LOW(i)	(0x24 + ((i) * 0x10))
+#define     CXL_DVSEC_MEM_BASE_LOW_MASK	GENMASK(31, 28)
+
+#endif
-- 
2.34.1

Re: [RFC type cxl initialization 1/2] cxl: add type2 device basic support

[Dan Williams]

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Differentiate CXL memory expanders (type 3) from CXL device accelerators
> (type 2) with a new function for initializing cxl_dev_state and a macro
> for helping accel drivers to embed cxl_dev_state inside a private
> struct.
> 
> Move structs to include/cxl as the size of the accel driver private
> struct embedding cxl_dev_state needs to know the size of this struct.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> ---
>  drivers/cxl/core/mbox.c   |   3 +-
>  drivers/cxl/core/memdev.c |  25 +++++
>  drivers/cxl/core/pci.c    |   1 +
>  drivers/cxl/core/regs.c   |   1 +
>  drivers/cxl/cxl.h         |  98 +----------------
>  drivers/cxl/cxlmem.h      | 108 +-----------------
>  drivers/cxl/cxlpci.h      |  21 ----
>  drivers/cxl/pci.c         |  17 +--
>  include/cxl/cxl.h         | 225 ++++++++++++++++++++++++++++++++++++++
>  include/cxl/pci.h         |  23 ++++
>  10 files changed, 293 insertions(+), 229 deletions(-)
>  create mode 100644 include/cxl/cxl.h
>  create mode 100644 include/cxl/pci.h
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index c5eedcae3b02..ae92db38a921 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1426,7 +1426,8 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host)
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL");
>  
> -struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev)
> +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
> +						 u16 dvsec)
>  {
>  	struct cxl_memdev_state *mds;

I would ultimately expect that this starts to use the
cxl_dev_state_create() internally as well to keep those internals
common.

>  
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 2914e3ff104b..14bd74823467 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -636,6 +636,31 @@ static void detach_memdev(struct work_struct *work)
>  
>  static struct lock_class_key cxl_memdev_key;
>  
> +void cxl_dev_state_init(struct cxl_dev_state *cxlds, struct device *dev,
> +			enum cxl_devtype type, u64 serial, u16 dvsec)
> +{
> +	cxlds->dev = dev;
> +	cxlds->type = type;
> +	cxlds->serial = serial;
> +	cxlds->cxl_dvsec = dvsec;
> +	cxlds->reg_map.host = dev;
> +	cxlds->reg_map.resource = CXL_RESOURCE_NONE;

Let's take the opportunity to now use compound literal syntax which
immediately answers the question of whether any new or not included
fields are initialized. I.e.

    *cxlds = (struct cxl_dev_state) {
	.dev = dev,
	...
    };

> +}
> +
> +struct cxl_dev_state *_cxl_dev_state_create(struct device *dev,
> +					   u64 serial, u16 dvsec,
> +					   size_t size)
> +{
> +	struct cxl_dev_state *cxlds __free(kfree) = kzalloc(size, GFP_KERNEL);
> +
> +	if (!cxlds)
> +		return NULL;
> +
> +	cxl_dev_state_init(cxlds, dev, CXL_DEVTYPE_DEVMEM, serial, dvsec);

I expect the devtype will need to be passed in as _cxl_dev_state_create
can not assume memory expander vs an accelerator.

> +	return_ptr(cxlds);
> +}
> +EXPORT_SYMBOL_NS_GPL(_cxl_dev_state_create, "CXL");
> +
>  static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
>  					   const struct file_operations *fops)
>  {
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index a5c65f79db18..36a1cf4e59fb 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright(c) 2021 Intel Corporation. All rights reserved. */
> +#include <cxl/pci.h>
>  #include <linux/units.h>
>  #include <linux/io-64-nonatomic-lo-hi.h>
>  #include <linux/device.h>
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index 117c2e94c761..58a942a4946c 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -4,6 +4,7 @@
>  #include <linux/device.h>
>  #include <linux/slab.h>
>  #include <linux/pci.h>
> +#include <cxl/pci.h>
>  #include <cxlmem.h>
>  #include <cxlpci.h>
>  #include <pmu.h>
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 6baec4ba9141..556b1ed24f0e 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -9,8 +9,8 @@
>  #include <linux/notifier.h>
>  #include <linux/bitops.h>
>  #include <linux/log2.h>
> -#include <linux/node.h>
>  #include <linux/io.h>
> +#include <cxl/cxl.h>
>  
>  extern const struct nvdimm_security_ops *cxl_security_ops;
>  
> @@ -200,97 +200,6 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)
>  #define   CXLDEV_MBOX_BG_CMD_COMMAND_VENDOR_MASK GENMASK_ULL(63, 48)
>  #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20
>  
> -/*
> - * Using struct_group() allows for per register-block-type helper routines,
> - * without requiring block-type agnostic code to include the prefix.
> - */
> -struct cxl_regs {
> -	/*
> -	 * Common set of CXL Component register block base pointers
> -	 * @hdm_decoder: CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure
> -	 * @ras: CXL 2.0 8.2.5.9 CXL RAS Capability Structure
> -	 */
> -	struct_group_tagged(cxl_component_regs, component,
> -		void __iomem *hdm_decoder;
> -		void __iomem *ras;
> -	);
> -	/*
> -	 * Common set of CXL Device register block base pointers
> -	 * @status: CXL 2.0 8.2.8.3 Device Status Registers
> -	 * @mbox: CXL 2.0 8.2.8.4 Mailbox Registers
> -	 * @memdev: CXL 2.0 8.2.8.5 Memory Device Registers
> -	 */
> -	struct_group_tagged(cxl_device_regs, device_regs,
> -		void __iomem *status, *mbox, *memdev;
> -	);
> -
> -	struct_group_tagged(cxl_pmu_regs, pmu_regs,
> -		void __iomem *pmu;
> -	);
> -
> -	/*
> -	 * RCH downstream port specific RAS register
> -	 * @aer: CXL 3.0 8.2.1.1 RCH Downstream Port RCRB
> -	 */
> -	struct_group_tagged(cxl_rch_regs, rch_regs,
> -		void __iomem *dport_aer;
> -	);
> -
> -	/*
> -	 * RCD upstream port specific PCIe cap register
> -	 * @pcie_cap: CXL 3.0 8.2.1.2 RCD Upstream Port RCRB
> -	 */
> -	struct_group_tagged(cxl_rcd_regs, rcd_regs,
> -		void __iomem *rcd_pcie_cap;
> -	);
> -};
> -
> -struct cxl_reg_map {
> -	bool valid;
> -	int id;
> -	unsigned long offset;
> -	unsigned long size;
> -};
> -
> -struct cxl_component_reg_map {
> -	struct cxl_reg_map hdm_decoder;
> -	struct cxl_reg_map ras;
> -};
> -
> -struct cxl_device_reg_map {
> -	struct cxl_reg_map status;
> -	struct cxl_reg_map mbox;
> -	struct cxl_reg_map memdev;
> -};
> -
> -struct cxl_pmu_reg_map {
> -	struct cxl_reg_map pmu;
> -};
> -
> -/**
> - * struct cxl_register_map - DVSEC harvested register block mapping parameters
> - * @host: device for devm operations and logging
> - * @base: virtual base of the register-block-BAR + @block_offset
> - * @resource: physical resource base of the register block
> - * @max_size: maximum mapping size to perform register search
> - * @reg_type: see enum cxl_regloc_type
> - * @component_map: cxl_reg_map for component registers
> - * @device_map: cxl_reg_maps for device registers
> - * @pmu_map: cxl_reg_maps for CXL Performance Monitoring Units
> - */
> -struct cxl_register_map {
> -	struct device *host;
> -	void __iomem *base;
> -	resource_size_t resource;
> -	resource_size_t max_size;
> -	u8 reg_type;
> -	union {
> -		struct cxl_component_reg_map component_map;
> -		struct cxl_device_reg_map device_map;
> -		struct cxl_pmu_reg_map pmu_map;
> -	};
> -};

Ok, all of the above make sense to move because they are needed to be
able to export the size. It would be nice for the follow on patches to
delineate which fields of 'struct cxl_dev_state' are expected to be
private to the CXL core.

> -
>  void cxl_probe_component_regs(struct device *dev, void __iomem *base,
>  			      struct cxl_component_reg_map *map);
>  void cxl_probe_device_regs(struct device *dev, void __iomem *base,
> @@ -480,11 +389,6 @@ struct cxl_region_params {
>  	int nr_targets;
>  };
>  
> -enum cxl_partition_mode {
> -	CXL_PARTMODE_RAM,
> -	CXL_PARTMODE_PMEM,
> -};

Makes sense, this is exported as part of struct cxl_dpa_info.

> -
>  /*
>   * Indicate whether this region has been assembled by autodetection or
>   * userspace assembly. Prevent endpoint decoders outside of automatic
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 8e1e46c348f5..0bdf581044da 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -4,9 +4,9 @@
>  #define __CXL_MEM_H__
>  #include <uapi/linux/cxl_mem.h>
>  #include <linux/pci.h>
> -#include <linux/cdev.h>
>  #include <linux/uuid.h>
>  #include <linux/node.h>
> +#include <cxl/cxl.h>
>  #include <cxl/event.h>
>  #include <cxl/mailbox.h>
>  #include "cxl.h"
> @@ -34,30 +34,6 @@
>  	(FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) !=                       \
>  	 CXLMDEV_RESET_NEEDED_NOT)
>  
> -/**
> - * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device
> - * @dev: driver core device object
> - * @cdev: char dev core object for ioctl operations
> - * @cxlds: The device state backing this device
> - * @detach_work: active memdev lost a port in its ancestry
> - * @cxl_nvb: coordinate removal of @cxl_nvd if present
> - * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
> - * @endpoint: connection to the CXL port topology for this memory device
> - * @id: id number of this memdev instance.
> - * @depth: endpoint port depth
> - */
> -struct cxl_memdev {
> -	struct device dev;
> -	struct cdev cdev;
> -	struct cxl_dev_state *cxlds;
> -	struct work_struct detach_work;
> -	struct cxl_nvdimm_bridge *cxl_nvb;
> -	struct cxl_nvdimm *cxl_nvd;
> -	struct cxl_port *endpoint;
> -	int id;
> -	int depth;
> -};

Why does this need to be public?

> -
>  static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
>  {
>  	return container_of(dev, struct cxl_memdev, dev);
> @@ -357,83 +333,6 @@ struct cxl_security_state {
>  	struct kernfs_node *sanitize_node;
>  };
>  
> -/*
> - * enum cxl_devtype - delineate type-2 from a generic type-3 device
> - * @CXL_DEVTYPE_DEVMEM - Vendor specific CXL Type-2 device implementing HDM-D or
> - *			 HDM-DB, no requirement that this device implements a
> - *			 mailbox, or other memory-device-standard manageability
> - *			 flows.
> - * @CXL_DEVTYPE_CLASSMEM - Common class definition of a CXL Type-3 device with
> - *			   HDM-H and class-mandatory memory device registers
> - */
> -enum cxl_devtype {
> -	CXL_DEVTYPE_DEVMEM,
> -	CXL_DEVTYPE_CLASSMEM,
> -};
> -
> -/**
> - * struct cxl_dpa_perf - DPA performance property entry
> - * @dpa_range: range for DPA address
> - * @coord: QoS performance data (i.e. latency, bandwidth)
> - * @cdat_coord: raw QoS performance data from CDAT
> - * @qos_class: QoS Class cookies
> - */
> -struct cxl_dpa_perf {
> -	struct range dpa_range;
> -	struct access_coordinate coord[ACCESS_COORDINATE_MAX];
> -	struct access_coordinate cdat_coord[ACCESS_COORDINATE_MAX];
> -	int qos_class;
> -};
> -
> -/**
> - * struct cxl_dpa_partition - DPA partition descriptor
> - * @res: shortcut to the partition in the DPA resource tree (cxlds->dpa_res)
> - * @perf: performance attributes of the partition from CDAT
> - * @mode: operation mode for the DPA capacity, e.g. ram, pmem, dynamic...
> - */
> -struct cxl_dpa_partition {
> -	struct resource res;
> -	struct cxl_dpa_perf perf;
> -	enum cxl_partition_mode mode;
> -};

Makes sense, this needs to go public with 'struct cxl_dev_state'.

> -
> -/**
> - * struct cxl_dev_state - The driver device state
> - *
> - * cxl_dev_state represents the CXL driver/device state.  It provides an
> - * interface to mailbox commands as well as some cached data about the device.
> - * Currently only memory devices are represented.
> - *
> - * @dev: The device associated with this CXL state
> - * @cxlmd: The device representing the CXL.mem capabilities of @dev
> - * @reg_map: component and ras register mapping parameters
> - * @regs: Parsed register blocks
> - * @cxl_dvsec: Offset to the PCIe device DVSEC
> - * @rcd: operating in RCD mode (CXL 3.0 9.11.8 CXL Devices Attached to an RCH)
> - * @media_ready: Indicate whether the device media is usable
> - * @dpa_res: Overall DPA resource tree for the device
> - * @part: DPA partition array
> - * @nr_partitions: Number of DPA partitions
> - * @serial: PCIe Device Serial Number
> - * @type: Generic Memory Class device or Vendor Specific Memory device
> - * @cxl_mbox: CXL mailbox context
> - */
> -struct cxl_dev_state {
> -	struct device *dev;
> -	struct cxl_memdev *cxlmd;
> -	struct cxl_register_map reg_map;
> -	struct cxl_regs regs;
> -	int cxl_dvsec;
> -	bool rcd;
> -	bool media_ready;
> -	struct resource dpa_res;
> -	struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX];
> -	unsigned int nr_partitions;
> -	u64 serial;
> -	enum cxl_devtype type;
> -	struct cxl_mailbox cxl_mbox;
> -};

Ack.

> -
>  static inline resource_size_t cxl_pmem_size(struct cxl_dev_state *cxlds)
>  {
>  	/*
> @@ -812,7 +711,10 @@ int cxl_dev_state_identify(struct cxl_memdev_state *mds);
>  int cxl_await_media_ready(struct cxl_dev_state *cxlds);
>  int cxl_enumerate_cmds(struct cxl_memdev_state *mds);
>  int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info);
> -struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev);
> +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
> +						 u16 dvsec);
> +void cxl_dev_state_init(struct cxl_dev_state *cxlds, struct device *dev,
> +			enum cxl_devtype type, u64 serial, u16 dvsec);
>  void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
>  				unsigned long *cmds);
>  void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index 54e219b0049e..570e53e26f11 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -7,29 +7,8 @@
>  
>  #define CXL_MEMORY_PROGIF	0x10
>  
> -/*
> - * See section 8.1 Configuration Space Registers in the CXL 2.0
> - * Specification. Names are taken straight from the specification with "CXL" and
> - * "DVSEC" redundancies removed. When obvious, abbreviations may be used.
> - */
>  #define PCI_DVSEC_HEADER1_LENGTH_MASK	GENMASK(31, 20)
>  
> -/* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */
> -#define CXL_DVSEC_PCIE_DEVICE					0
> -#define   CXL_DVSEC_CAP_OFFSET		0xA
> -#define     CXL_DVSEC_MEM_CAPABLE	BIT(2)
> -#define     CXL_DVSEC_HDM_COUNT_MASK	GENMASK(5, 4)
> -#define   CXL_DVSEC_CTRL_OFFSET		0xC
> -#define     CXL_DVSEC_MEM_ENABLE	BIT(2)
> -#define   CXL_DVSEC_RANGE_SIZE_HIGH(i)	(0x18 + (i * 0x10))
> -#define   CXL_DVSEC_RANGE_SIZE_LOW(i)	(0x1C + (i * 0x10))
> -#define     CXL_DVSEC_MEM_INFO_VALID	BIT(0)
> -#define     CXL_DVSEC_MEM_ACTIVE	BIT(1)
> -#define     CXL_DVSEC_MEM_SIZE_LOW_MASK	GENMASK(31, 28)
> -#define   CXL_DVSEC_RANGE_BASE_HIGH(i)	(0x20 + (i * 0x10))
> -#define   CXL_DVSEC_RANGE_BASE_LOW(i)	(0x24 + (i * 0x10))
> -#define     CXL_DVSEC_MEM_BASE_LOW_MASK	GENMASK(31, 28)

Makes sense.

> -
>  #define CXL_DVSEC_RANGE_MAX		2
>  
>  /* CXL 2.0 8.1.4: Non-CXL Function Map DVSEC */
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index b2c943a4de0a..006a0036e779 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1,5 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright(c) 2020 Intel Corporation. All rights reserved. */
> +#include <cxl/cxl.h>
> +#include <cxl/pci.h>
>  #include <linux/unaligned.h>
>  #include <linux/io-64-nonatomic-lo-hi.h>
>  #include <linux/moduleparam.h>
> @@ -911,6 +913,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	int rc, pmu_count;
>  	unsigned int i;
>  	bool irq_avail;
> +	u16 dvsec;
>  
>  	/*
>  	 * Double check the anonymous union trickery in struct cxl_regs
> @@ -924,19 +927,19 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		return rc;
>  	pci_set_master(pdev);
>  
> -	mds = cxl_memdev_state_create(&pdev->dev);
> +	dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
> +					  CXL_DVSEC_PCIE_DEVICE);
> +	if (!dvsec)
> +		dev_warn(&pdev->dev,
> +			 "Device DVSEC not present, skip CXL.mem init\n");
> +
> +	mds = cxl_memdev_state_create(&pdev->dev, pci_get_dsn(pdev), dvsec);
>  	if (IS_ERR(mds))
>  		return PTR_ERR(mds);
>  	cxlds = &mds->cxlds;
>  	pci_set_drvdata(pdev, cxlds);
>  
>  	cxlds->rcd = is_cxl_restricted(pdev);
> -	cxlds->serial = pci_get_dsn(pdev);
> -	cxlds->cxl_dvsec = pci_find_dvsec_capability(
> -		pdev, PCI_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE);
> -	if (!cxlds->cxl_dvsec)
> -		dev_warn(&pdev->dev,
> -			 "Device DVSEC not present, skip CXL.mem init\n");
>  
>  	rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
>  	if (rc)

Looks good.

Re: [RFC type cxl initialization 1/2] cxl: add type2 device basic support

[Alejandro Lucero Palau]

Hi Dan,


Thanks for the review. My comments below.


On 3/3/25 20:15, Dan Williams wrote:
> alejandro.lucero-palau@ wrote:
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> Differentiate CXL memory expanders (type 3) from CXL device accelerators
>> (type 2) with a new function for initializing cxl_dev_state and a macro
>> for helping accel drivers to embed cxl_dev_state inside a private
>> struct.
>>
>> Move structs to include/cxl as the size of the accel driver private
>> struct embedding cxl_dev_state needs to know the size of this struct.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> ---
>>   drivers/cxl/core/mbox.c   |   3 +-
>>   drivers/cxl/core/memdev.c |  25 +++++
>>   drivers/cxl/core/pci.c    |   1 +
>>   drivers/cxl/core/regs.c   |   1 +
>>   drivers/cxl/cxl.h         |  98 +----------------
>>   drivers/cxl/cxlmem.h      | 108 +-----------------
>>   drivers/cxl/cxlpci.h      |  21 ----
>>   drivers/cxl/pci.c         |  17 +--
>>   include/cxl/cxl.h         | 225 ++++++++++++++++++++++++++++++++++++++
>>   include/cxl/pci.h         |  23 ++++
>>   10 files changed, 293 insertions(+), 229 deletions(-)
>>   create mode 100644 include/cxl/cxl.h
>>   create mode 100644 include/cxl/pci.h
>>
>> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
>> index c5eedcae3b02..ae92db38a921 100644
>> --- a/drivers/cxl/core/mbox.c
>> +++ b/drivers/cxl/core/mbox.c
>> @@ -1426,7 +1426,8 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host)
>>   }
>>   EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL");
>>   
>> -struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev)
>> +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
>> +						 u16 dvsec)
>>   {
>>   	struct cxl_memdev_state *mds;
> I would ultimately expect that this starts to use the
> cxl_dev_state_create() internally as well to keep those internals
> common.
>

OK. I think I can do this for the impending v11.


>>   
>> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
>> index 2914e3ff104b..14bd74823467 100644
>> --- a/drivers/cxl/core/memdev.c
>> +++ b/drivers/cxl/core/memdev.c
>> @@ -636,6 +636,31 @@ static void detach_memdev(struct work_struct *work)
>>   
>>   static struct lock_class_key cxl_memdev_key;
>>   
>> +void cxl_dev_state_init(struct cxl_dev_state *cxlds, struct device *dev,
>> +			enum cxl_devtype type, u64 serial, u16 dvsec)
>> +{
>> +	cxlds->dev = dev;
>> +	cxlds->type = type;
>> +	cxlds->serial = serial;
>> +	cxlds->cxl_dvsec = dvsec;
>> +	cxlds->reg_map.host = dev;
>> +	cxlds->reg_map.resource = CXL_RESOURCE_NONE;
> Let's take the opportunity to now use compound literal syntax which
> immediately answers the question of whether any new or not included
> fields are initialized. I.e.
>
>      *cxlds = (struct cxl_dev_state) {
> 	.dev = dev,
> 	...
>      };
>

OK.


>> +}
>> +
>> +struct cxl_dev_state *_cxl_dev_state_create(struct device *dev,
>> +					   u64 serial, u16 dvsec,
>> +					   size_t size)
>> +{
>> +	struct cxl_dev_state *cxlds __free(kfree) = kzalloc(size, GFP_KERNEL);
>> +
>> +	if (!cxlds)
>> +		return NULL;
>> +
>> +	cxl_dev_state_init(cxlds, dev, CXL_DEVTYPE_DEVMEM, serial, dvsec);
> I expect the devtype will need to be passed in as _cxl_dev_state_create
> can not assume memory expander vs an accelerator.


Yes, this is needed for the work to do related to the first comment. 
I'll do that.


<snip>


> Ok, all of the above make sense to move because they are needed to be
> able to export the size. It would be nice for the follow on patches to
> delineate which fields of 'struct cxl_dev_state' are expected to be
> private to the CXL core.
>

I'll do so.


<snip>

>>   
>> -/**
>> - * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device
>> - * @dev: driver core device object
>> - * @cdev: char dev core object for ioctl operations
>> - * @cxlds: The device state backing this device
>> - * @detach_work: active memdev lost a port in its ancestry
>> - * @cxl_nvb: coordinate removal of @cxl_nvd if present
>> - * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
>> - * @endpoint: connection to the CXL port topology for this memory device
>> - * @id: id number of this memdev instance.
>> - * @depth: endpoint port depth
>> - */
>> -struct cxl_memdev {
>> -	struct device dev;
>> -	struct cdev cdev;
>> -	struct cxl_dev_state *cxlds;
>> -	struct work_struct detach_work;
>> -	struct cxl_nvdimm_bridge *cxl_nvb;
>> -	struct cxl_nvdimm *cxl_nvd;
>> -	struct cxl_port *endpoint;
>> -	int id;
>> -	int depth;
>> -};
> Why does this need to be public?


Good catch. This comes from the previous try for using this in v10. It 
is not needed for what the RFC tries to do.

<snip>

> Looks good.


I think I got what I need now for v11.

Thanks!

Re: [RFC type cxl initialization 1/2] cxl: add type2 device basic support

[Ben Cheatham]

On 2/20/25 2:00 PM, alejandro.lucero-palau@amd.com wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Differentiate CXL memory expanders (type 3) from CXL device accelerators
> (type 2) with a new function for initializing cxl_dev_state and a macro
> for helping accel drivers to embed cxl_dev_state inside a private
> struct.
> 
> Move structs to include/cxl as the size of the accel driver private
> struct embedding cxl_dev_state needs to know the size of this struct.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> ---
>  drivers/cxl/core/mbox.c   |   3 +-
>  drivers/cxl/core/memdev.c |  25 +++++
>  drivers/cxl/core/pci.c    |   1 +
>  drivers/cxl/core/regs.c   |   1 +
>  drivers/cxl/cxl.h         |  98 +----------------
>  drivers/cxl/cxlmem.h      | 108 +-----------------
>  drivers/cxl/cxlpci.h      |  21 ----
>  drivers/cxl/pci.c         |  17 +--
>  include/cxl/cxl.h         | 225 ++++++++++++++++++++++++++++++++++++++
>  include/cxl/pci.h         |  23 ++++
>  10 files changed, 293 insertions(+), 229 deletions(-)

[snip]

> +/**
> + * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device
> + * @dev: driver core device object
> + * @cdev: char dev core object for ioctl operations
> + * @cxlds: The device state backing this device
> + * @detach_work: active memdev lost a port in its ancestry
> + * @cxl_nvb: coordinate removal of @cxl_nvd if present
> + * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
> + * @endpoint: connection to the CXL port topology for this memory device
> + * @id: id number of this memdev instance.
> + * @depth: endpoint port depth
> + */
> +struct cxl_memdev {
> +	struct device dev;
> +	struct cdev cdev;
> +	struct cxl_dev_state *cxlds;
> +	struct work_struct detach_work;
> +	struct cxl_nvdimm_bridge *cxl_nvb;
> +	struct cxl_nvdimm *cxl_nvd;
> +	struct cxl_port *endpoint;
> +	int id;
> +	int depth;
> +};

This can stay in cxlmem.h, only a pointer is being used in cxl_dev_state.

> +
> +#define CXL_NR_PARTITIONS_MAX 2
> +
> +/**
> + * struct cxl_dev_state - The driver device state
> + *
> + * cxl_dev_state represents the CXL driver/device state.  It provides an
> + * interface to mailbox commands as well as some cached data about the device.
> + * Currently only memory devices are represented.
> + *
> + * @dev: The device associated with this CXL state
> + * @cxlmd: The device representing the CXL.mem capabilities of @dev
> + * @reg_map: component and ras register mapping parameters
> + * @regs: Parsed register blocks
> + * @cxl_dvsec: Offset to the PCIe device DVSEC
> + * @rcd: operating in RCD mode (CXL 3.0 9.11.8 CXL Devices Attached to an RCH)
> + * @media_ready: Indicate whether the device media is usable
> + * @dpa_res: Overall DPA resource tree for the device
> + * @part: DPA partition array
> + * @nr_partitions: Number of DPA partitions
> + * @serial: PCIe Device Serial Number
> + * @type: Generic Memory Class device or Vendor Specific Memory device
> + * @cxl_mbox: CXL mailbox context
> + */
> +struct cxl_dev_state {
> +	struct device *dev;
> +	struct cxl_memdev *cxlmd;
> +	struct cxl_register_map reg_map;
> +	struct cxl_regs regs;
> +	int cxl_dvsec;
> +	bool rcd;
> +	bool media_ready;
> +	struct resource dpa_res;
> +	struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX];
> +	unsigned int nr_partitions;
> +	u64 serial;
> +	enum cxl_devtype type;
> +	struct cxl_mailbox cxl_mbox;
> +};
> +
> +struct cxl_dev_state *_cxl_dev_state_create(struct device *dev,
> +					   u64 serial, u16 dvsec,
> +					   size_t size);
> +
> +#define cxl_dev_state_create(parent, serial, dvsec, drv_struct, member) \
> +	({								      \
> +	 	static_assert(__same_type(struct cxl_dev_state, 	      \
> +					((drv_struct *)NULL)->member));	      \
> +		static_assert(offsetof(drv_struct, member) == 0);	      \
> +		(drv_struct *)_cxl_dev_state_create(parent, serial, dvsec,    \
> +						    sizeof(drv_struct));\
> +	})

This should take a CXL device type parameter as well, then you can also use it in
cxl_memdev_state_create() like this:

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 2c49e33851b2..14d7f93e4584 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1484,24 +1484,31 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL");

+static void cxl_memdev_state_destroy(void *cxlmds)
+{
+       kfree(cxlmds);
+}
+
 struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
                                                 u16 dvsec)
 {
        struct cxl_memdev_state *mds;
        int rc;

-       mds = devm_kzalloc(dev, sizeof(*mds), GFP_KERNEL);
+       mds = cxl_dev_state_create(dev, serial, dvsec,
+                                  CXL_DEVTYPE_CLASSMEM,
+                                  struct cxl_memdev_state,
+                                  cxlds);
        if (!mds) {
                dev_err(dev, "No memory available\n");
                return ERR_PTR(-ENOMEM);
        }

+       rc = devm_add_action_or_reset(dev, cxl_memdev_state_destroy, mds);
+       if (rc)
+               return ERR_PTR(rc);
+
        mutex_init(&mds->event.log_lock);
-       mds->cxlds.dev = dev;
-       mds->cxlds.reg_map.host = dev;
-       mds->cxlds.cxl_mbox.host = dev;
-       mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE;
-       mds->cxlds.type = CXL_DEVTYPE_CLASSMEM;


Looking at the above, it occurs to me that it may be good to either:

a) Have a devm version of cxl_dev_state_create() or,
b) Have an init macro (in addition) that takes a pointer so that you can use an
allocator of your choice

Neither are necessary, but would be more ergonomic IMO.

Thanks,
Ben

Re: [RFC type cxl initialization 1/2] cxl: add type2 device basic support

[Dan Williams]

Ben Cheatham wrote:
> On 2/20/25 2:00 PM, alejandro.lucero-palau@amd.com wrote:
> > From: Alejandro Lucero <alucerop@amd.com>
> > 
> > Differentiate CXL memory expanders (type 3) from CXL device accelerators
> > (type 2) with a new function for initializing cxl_dev_state and a macro
> > for helping accel drivers to embed cxl_dev_state inside a private
> > struct.
> > 
> > Move structs to include/cxl as the size of the accel driver private
> > struct embedding cxl_dev_state needs to know the size of this struct.
> > 
> > Signed-off-by: Alejandro Lucero <alucerop@amd.com>
[..]
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 2c49e33851b2..14d7f93e4584 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1484,24 +1484,31 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host)
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL");
> 
> +static void cxl_memdev_state_destroy(void *cxlmds)
> +{
> +       kfree(cxlmds);
> +}
> +
>  struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
>                                                  u16 dvsec)
>  {
>         struct cxl_memdev_state *mds;
>         int rc;
> 
> -       mds = devm_kzalloc(dev, sizeof(*mds), GFP_KERNEL);
> +       mds = cxl_dev_state_create(dev, serial, dvsec,
> +                                  CXL_DEVTYPE_CLASSMEM,
> +                                  struct cxl_memdev_state,
> +                                  cxlds);
>         if (!mds) {
>                 dev_err(dev, "No memory available\n");
>                 return ERR_PTR(-ENOMEM);
>         }
> 
> +       rc = devm_add_action_or_reset(dev, cxl_memdev_state_destroy, mds);
> +       if (rc)
> +               return ERR_PTR(rc);
> +
>         mutex_init(&mds->event.log_lock);
> -       mds->cxlds.dev = dev;
> -       mds->cxlds.reg_map.host = dev;
> -       mds->cxlds.cxl_mbox.host = dev;
> -       mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE;
> -       mds->cxlds.type = CXL_DEVTYPE_CLASSMEM;
> 
> 
> Looking at the above, it occurs to me that it may be good to either:
> 
> a) Have a devm version of cxl_dev_state_create() or,
> b) Have an init macro (in addition) that takes a pointer so that you can use an
> allocator of your choice

The caller can always wrap the result with devm_add_action_or_reset().
No need to force comsumer drivers into an allocation regime.

Re: [RFC type cxl initialization 1/2] cxl: add type2 device basic support

[Ben Cheatham]

On 3/3/25 2:51 PM, Dan Williams wrote:
> Ben Cheatham wrote:
>> On 2/20/25 2:00 PM, alejandro.lucero-palau@amd.com wrote:
>>> From: Alejandro Lucero <alucerop@amd.com>
>>>
>>> Differentiate CXL memory expanders (type 3) from CXL device accelerators
>>> (type 2) with a new function for initializing cxl_dev_state and a macro
>>> for helping accel drivers to embed cxl_dev_state inside a private
>>> struct.
>>>
>>> Move structs to include/cxl as the size of the accel driver private
>>> struct embedding cxl_dev_state needs to know the size of this struct.
>>>
>>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> [..]
>> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
>> index 2c49e33851b2..14d7f93e4584 100644
>> --- a/drivers/cxl/core/mbox.c
>> +++ b/drivers/cxl/core/mbox.c
>> @@ -1484,24 +1484,31 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host)
>>  }
>>  EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL");
>>
>> +static void cxl_memdev_state_destroy(void *cxlmds)
>> +{
>> +       kfree(cxlmds);
>> +}
>> +
>>  struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial,
>>                                                  u16 dvsec)
>>  {
>>         struct cxl_memdev_state *mds;
>>         int rc;
>>
>> -       mds = devm_kzalloc(dev, sizeof(*mds), GFP_KERNEL);
>> +       mds = cxl_dev_state_create(dev, serial, dvsec,
>> +                                  CXL_DEVTYPE_CLASSMEM,
>> +                                  struct cxl_memdev_state,
>> +                                  cxlds);
>>         if (!mds) {
>>                 dev_err(dev, "No memory available\n");
>>                 return ERR_PTR(-ENOMEM);
>>         }
>>
>> +       rc = devm_add_action_or_reset(dev, cxl_memdev_state_destroy, mds);
>> +       if (rc)
>> +               return ERR_PTR(rc);
>> +
>>         mutex_init(&mds->event.log_lock);
>> -       mds->cxlds.dev = dev;
>> -       mds->cxlds.reg_map.host = dev;
>> -       mds->cxlds.cxl_mbox.host = dev;
>> -       mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE;
>> -       mds->cxlds.type = CXL_DEVTYPE_CLASSMEM;
>>
>>
>> Looking at the above, it occurs to me that it may be good to either:
>>
>> a) Have a devm version of cxl_dev_state_create() or,
>> b) Have an init macro (in addition) that takes a pointer so that you can use an
>> allocator of your choice
> 
> The caller can always wrap the result with devm_add_action_or_reset().
> No need to force comsumer drivers into an allocation regime.
Sounds good to me, figured I would suggest it more as a convenience option
than anything.

Thanks,
Ben

[RFC type cxl initialization 2/2] sfc: add cxl support

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Add CXL initialization based on new CXL API for accel drivers and make
it dependent on kernel CXL configuration.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
---
 drivers/net/ethernet/sfc/Kconfig      |  6 +++
 drivers/net/ethernet/sfc/Makefile     |  1 +
 drivers/net/ethernet/sfc/efx.c        | 15 +++++++-
 drivers/net/ethernet/sfc/efx_cxl.c    | 53 +++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/efx_cxl.h    | 40 ++++++++++++++++++++
 drivers/net/ethernet/sfc/net_driver.h | 10 +++++
 6 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c
 create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h

diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig
index 3eb55dcfa8a6..459db14e5ef9 100644
--- a/drivers/net/ethernet/sfc/Kconfig
+++ b/drivers/net/ethernet/sfc/Kconfig
@@ -65,6 +65,12 @@ config SFC_MCDI_LOGGING
 	  Driver-Interface) commands and responses, allowing debugging of
 	  driver/firmware interaction.  The tracing is actually enabled by
 	  a sysfs file 'mcdi_logging' under the PCI device.
+config SFC_CXL
+	bool "Solarflare SFC9100-family CXL support"
+	depends on SFC && CXL_BUS && !(SFC=y && CXL_BUS=m)
+	depends on CXL_BUS >= SFC
+	default SFC
+
 
 source "drivers/net/ethernet/sfc/falcon/Kconfig"
 source "drivers/net/ethernet/sfc/siena/Kconfig"
diff --git a/drivers/net/ethernet/sfc/Makefile b/drivers/net/ethernet/sfc/Makefile
index 8f446b9bd5ee..e909cafd5908 100644
--- a/drivers/net/ethernet/sfc/Makefile
+++ b/drivers/net/ethernet/sfc/Makefile
@@ -13,6 +13,7 @@ sfc-$(CONFIG_SFC_SRIOV)	+= sriov.o ef10_sriov.o ef100_sriov.o ef100_rep.o \
                            mae.o tc.o tc_bindings.o tc_counters.o \
                            tc_encap_actions.o tc_conntrack.o
 
+sfc-$(CONFIG_SFC_CXL)	+= efx_cxl.o
 obj-$(CONFIG_SFC)	+= sfc.o
 
 obj-$(CONFIG_SFC_FALCON) += falcon/
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 650136dfc642..f60f9153f018 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -34,6 +34,7 @@
 #include "selftest.h"
 #include "sriov.h"
 #include "efx_devlink.h"
+#include "efx_cxl.h"
 
 #include "mcdi_port_common.h"
 #include "mcdi_pcol.h"
@@ -1004,12 +1005,15 @@ static void efx_pci_remove(struct pci_dev *pci_dev)
 	efx_pci_remove_main(efx);
 
 	efx_fini_io(efx);
+
+	probe_data = container_of(efx, struct efx_probe_data, efx);
+	efx_cxl_exit(probe_data);
+
 	pci_dbg(efx->pci_dev, "shutdown successful\n");
 
 	efx_fini_devlink_and_unlock(efx);
 	efx_fini_struct(efx);
 	free_netdev(efx->net_dev);
-	probe_data = container_of(efx, struct efx_probe_data, efx);
 	kfree(probe_data);
 };
 
@@ -1214,6 +1218,15 @@ static int efx_pci_probe(struct pci_dev *pci_dev,
 	if (rc)
 		goto fail2;
 
+	/* A successful cxl initialization implies a CXL region created to be
+	 * used for PIO buffers. If there is no CXL support, or initialization
+	 * fails, efx_cxl_pio_initialised wll be false and legacy PIO buffers
+	 * defined at specific PCI BAR regions will be used.
+	 */
+	rc = efx_cxl_init(probe_data);
+	if (rc)
+		pci_err(pci_dev, "CXL initialization failed with error %d\n", rc);
+
 	rc = efx_pci_probe_post_io(efx);
 	if (rc) {
 		/* On failure, retry once immediately.
diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
new file mode 100644
index 000000000000..d34aedbcaddf
--- /dev/null
+++ b/drivers/net/ethernet/sfc/efx_cxl.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/****************************************************************************
+ *
+ * Driver for AMD network controllers and boards
+ * Copyright (C) 2024, Advanced Micro Devices, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation, incorporated herein by reference.
+ */
+
+#include <cxl/pci.h>
+#include <linux/pci.h>
+
+#include "net_driver.h"
+#include "efx_cxl.h"
+
+#define EFX_CTPIO_BUFFER_SIZE	SZ_256M
+
+int efx_cxl_init(struct efx_probe_data *probe_data)
+{
+	struct efx_nic *efx = &probe_data->efx;
+	struct pci_dev *pci_dev = efx->pci_dev;
+	struct efx_cxl *cxl;
+	u16 dvsec;
+
+	probe_data->cxl_pio_initialised = false;
+
+	dvsec = pci_find_dvsec_capability(pci_dev, PCI_VENDOR_ID_CXL,
+					  CXL_DVSEC_PCIE_DEVICE);
+	if (!dvsec)
+		return 0;
+
+	pci_dbg(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability found\n");
+
+	cxl = cxl_dev_state_create(&pci_dev->dev, pci_dev->dev.id,
+					  dvsec, struct efx_cxl, cxlds);
+
+	if (!cxl)
+		return -ENOMEM;
+
+	probe_data->cxl = cxl;
+
+	return 0;
+}
+
+void efx_cxl_exit(struct efx_probe_data *probe_data)
+{
+	if (probe_data->cxl)
+		kfree(probe_data->cxl);
+}
+
+MODULE_IMPORT_NS("CXL");
diff --git a/drivers/net/ethernet/sfc/efx_cxl.h b/drivers/net/ethernet/sfc/efx_cxl.h
new file mode 100644
index 000000000000..67be714b57d1
--- /dev/null
+++ b/drivers/net/ethernet/sfc/efx_cxl.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/****************************************************************************
+ * Driver for AMD network controllers and boards
+ * Copyright (C) 2024, Advanced Micro Devices, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation, incorporated herein by reference.
+ */
+
+#ifndef EFX_CXL_H
+#define EFX_CXL_H
+
+#ifdef CONFIG_SFC_CXL
+
+#include <cxl/cxl.h>
+
+struct cxl_root_decoder;
+struct cxl_port;
+struct cxl_endpoint_decoder;
+struct cxl_region;
+struct efx_probe_data;
+
+struct efx_cxl {
+	struct cxl_dev_state cxlds;
+	struct cxl_memdev *cxlmd;
+	struct cxl_root_decoder *cxlrd;
+	struct cxl_port *endpoint;
+	struct cxl_endpoint_decoder *cxled;
+	struct cxl_region *efx_region;
+	void __iomem *ctpio_cxl;
+};
+
+int efx_cxl_init(struct efx_probe_data *probe_data);
+void efx_cxl_exit(struct efx_probe_data *probe_data);
+#else
+inline int efx_cxl_init(struct efx_probe_data *probe_data) { return 0; }
+inline void efx_cxl_exit(struct efx_probe_data *probe_data) {}
+#endif
+#endif
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index f70a7b7d6345..a2626bcd6a41 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -1201,14 +1201,24 @@ struct efx_nic {
 	atomic_t n_rx_noskb_drops;
 };
 
+#ifdef CONFIG_SFC_CXL
+struct efx_cxl;
+#endif
+
 /**
  * struct efx_probe_data - State after hardware probe
  * @pci_dev: The PCI device
  * @efx: Efx NIC details
+ * @cxl: details of related cxl objects
+ * @cxl_pio_initialised: cxl initialization outcome.
  */
 struct efx_probe_data {
 	struct pci_dev *pci_dev;
 	struct efx_nic efx;
+#ifdef CONFIG_SFC_CXL
+	struct efx_cxl *cxl;
+	bool cxl_pio_initialised;
+#endif
 };
 
 static inline struct efx_nic *efx_netdev_priv(struct net_device *dev)
-- 
2.34.1

Re: [RFC type cxl initialization 2/2] sfc: add cxl support

[Dan Williams]

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Add CXL initialization based on new CXL API for accel drivers and make
> it dependent on kernel CXL configuration.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> ---
>  drivers/net/ethernet/sfc/Kconfig      |  6 +++
>  drivers/net/ethernet/sfc/Makefile     |  1 +
>  drivers/net/ethernet/sfc/efx.c        | 15 +++++++-
>  drivers/net/ethernet/sfc/efx_cxl.c    | 53 +++++++++++++++++++++++++++
>  drivers/net/ethernet/sfc/efx_cxl.h    | 40 ++++++++++++++++++++
>  drivers/net/ethernet/sfc/net_driver.h | 10 +++++
>  6 files changed, 124 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c
>  create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h
> 
> diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig
> index 3eb55dcfa8a6..459db14e5ef9 100644
> --- a/drivers/net/ethernet/sfc/Kconfig
> +++ b/drivers/net/ethernet/sfc/Kconfig
> @@ -65,6 +65,12 @@ config SFC_MCDI_LOGGING
>  	  Driver-Interface) commands and responses, allowing debugging of
>  	  driver/firmware interaction.  The tracing is actually enabled by
>  	  a sysfs file 'mcdi_logging' under the PCI device.
> +config SFC_CXL
> +	bool "Solarflare SFC9100-family CXL support"
> +	depends on SFC && CXL_BUS && !(SFC=y && CXL_BUS=m)
> +	depends on CXL_BUS >= SFC

You only need one statement checking to make sure that SFC is not
compiled in with CXL_BUS is a module. The second syntax of:

    "$dependency >= $consumer"

...appears to be more idiomatic to me, or at least more concise.

Otherwise, this patch looks good to me.

Re: [RFC 0/2] type2 cxl initialization

[Ben Cheatham]

On 2/20/25 2:00 PM, alejandro.lucero-palau@amd.com wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Avoiding to send the full type2 support patchset until there is an
> agreement about how to do the cxl initialization from accel drivers.
> 
> Using the idea from fwctl for embedding cxl_dev_state in a private
> accel driver struct and a macro helping for the allocation and
> initialization of such struct.
> 
> The main problem is the amount of internal cxl structs which need to be
> public for accel drivers which we tried to avoid since v1 of the type2
> patchset history.

I have a suggestion or two in the next patch, but in my opinion the only
potentially problematic exports are the cxl register structs. I'm assuming
allowing accelerator drivers access to those may cause stale values in the
CXL driver, but I haven't looked. The only suggestion I have there is to
use pointers to the struct instead, but I imagine achieving could be a
non-trivial amount of effort.

The only other member that may be a problem is the DPA partition info,
but I don't know enough about that to comment.

> 
> Alejandro Lucero (2):
>   cxl: add type2 device basic support
>   sfc: add cxl support
> 
>  drivers/cxl/core/mbox.c               |   3 +-
>  drivers/cxl/core/memdev.c             |  25 +++
>  drivers/cxl/core/pci.c                |   1 +
>  drivers/cxl/core/regs.c               |   1 +
>  drivers/cxl/cxl.h                     |  98 +----------
>  drivers/cxl/cxlmem.h                  | 108 +------------
>  drivers/cxl/cxlpci.h                  |  21 ---
>  drivers/cxl/pci.c                     |  17 +-
>  drivers/net/ethernet/sfc/Kconfig      |   6 +
>  drivers/net/ethernet/sfc/Makefile     |   1 +
>  drivers/net/ethernet/sfc/efx.c        |  15 +-
>  drivers/net/ethernet/sfc/efx_cxl.c    |  53 ++++++
>  drivers/net/ethernet/sfc/efx_cxl.h    |  40 +++++
>  drivers/net/ethernet/sfc/net_driver.h |  10 ++
>  include/cxl/cxl.h                     | 225 ++++++++++++++++++++++++++
>  include/cxl/pci.h                     |  23 +++
>  16 files changed, 417 insertions(+), 230 deletions(-)
>  create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c
>  create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h
>  create mode 100644 include/cxl/cxl.h
>  create mode 100644 include/cxl/pci.h
> 
> 
> base-commit: d5d2106e2118c4e09fef131d9889f79559b95bfc

Re: [RFC 0/2] type2 cxl initialization

[Dan Williams]

Ben Cheatham wrote:
> On 2/20/25 2:00 PM, alejandro.lucero-palau@amd.com wrote:
> > From: Alejandro Lucero <alucerop@amd.com>
> > 
> > Avoiding to send the full type2 support patchset until there is an
> > agreement about how to do the cxl initialization from accel drivers.
> > 
> > Using the idea from fwctl for embedding cxl_dev_state in a private
> > accel driver struct and a macro helping for the allocation and
> > initialization of such struct.
> > 
> > The main problem is the amount of internal cxl structs which need to be
> > public for accel drivers which we tried to avoid since v1 of the type2
> > patchset history.
> 
> I have a suggestion or two in the next patch, but in my opinion the only
> potentially problematic exports are the cxl register structs. I'm assuming
> allowing accelerator drivers access to those may cause stale values in the
> CXL driver, but I haven't looked. The only suggestion I have there is to
> use pointers to the struct instead, but I imagine achieving could be a
> non-trivial amount of effort.

Hmm, say more about why you think those are a problem. Those are
"write-once at init" things, why would an accelerator driver have any
reason to play with those post init?

> The only other member that may be a problem is the DPA partition info,
> but I don't know enough about that to comment.

I think it is sufficient to mark the bulk of 'struct cxl_dev_state' as
private and then move things into the public space as needed, something
like:

struct cxl_dev_state {
/* public */
        struct device *dev;
        struct cxl_memdev *cxlmd;
/* private */
        struct cxl_register_map reg_map;
        struct cxl_regs regs;
        int cxl_dvsec;
        bool rcd;
        bool media_ready;
        struct resource dpa_res;
        struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX];
        unsigned int nr_partitions;
        u64 serial;
        enum cxl_devtype type;
        struct cxl_mailbox cxl_mbox;
};

...but otherwise let accelerator drivers see the full definition of that
struct for constainer_of() operations.

Re: [RFC 0/2] type2 cxl initialization

[Ben Cheatham]

On 3/3/25 2:49 PM, Dan Williams wrote:
> Ben Cheatham wrote:
>> On 2/20/25 2:00 PM, alejandro.lucero-palau@amd.com wrote:
>>> From: Alejandro Lucero <alucerop@amd.com>
>>>
>>> Avoiding to send the full type2 support patchset until there is an
>>> agreement about how to do the cxl initialization from accel drivers.
>>>
>>> Using the idea from fwctl for embedding cxl_dev_state in a private
>>> accel driver struct and a macro helping for the allocation and
>>> initialization of such struct.
>>>
>>> The main problem is the amount of internal cxl structs which need to be
>>> public for accel drivers which we tried to avoid since v1 of the type2
>>> patchset history.
>>
>> I have a suggestion or two in the next patch, but in my opinion the only
>> potentially problematic exports are the cxl register structs. I'm assuming
>> allowing accelerator drivers access to those may cause stale values in the
>> CXL driver, but I haven't looked. The only suggestion I have there is to
>> use pointers to the struct instead, but I imagine achieving could be a
>> non-trivial amount of effort.
> 
> Hmm, say more about why you think those are a problem. Those are
> "write-once at init" things, why would an accelerator driver have any
> reason to play with those post init?
> 

I could see a scenario where an accelerator driver tries to do some concurrent
setup with the CXL driver and may cause an issue. For example if a type 2 driver
tried to do something with HDM during setup, but I think a driver touching
any of these registers when the CXL driver already does this stuff is pretty
unlikely.

So that leads me to think it's better to not expose this at all, but I think
that would require more work than it's worth.

>> The only other member that may be a problem is the DPA partition info,
>> but I don't know enough about that to comment.
> 
> I think it is sufficient to mark the bulk of 'struct cxl_dev_state' as
> private and then move things into the public space as needed, something
> like:
> 
> struct cxl_dev_state {
> /* public */
>         struct device *dev;
>         struct cxl_memdev *cxlmd;
> /* private */
>         struct cxl_register_map reg_map;
>         struct cxl_regs regs;
>         int cxl_dvsec;
>         bool rcd;
>         bool media_ready;
>         struct resource dpa_res;
>         struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX];
>         unsigned int nr_partitions;
>         u64 serial;
>         enum cxl_devtype type;
>         struct cxl_mailbox cxl_mbox;
> };
> 
> ...but otherwise let accelerator drivers see the full definition of that
> struct for constainer_of() operations.
That's fine by me.