[RFC PATCH v2 0/4] CXL: Standalone switch CCI driver

[RFC PATCH v2 0/4] CXL: Standalone switch CCI driver

[Jonathan Cameron]

Changes since v1: (thanks Slava!)
- 3 precusor patches added
    PCI Class ID added to pci_ids.h
    Refactoring to change parameters of cxl_send_cmd() to make
    it suitable for use from the new switch CCI code.
- Various fixes around error paths and tidying up (see patch 4)

CXL rev 3.0 introduced the option for a PCI function, intended to sit on an
upstream port of a CXL switch.  This function provides a mailbox
interface similar to that seen on CXL type 3 devices. However, the
command set is mostly different and intended for Fabric management.
Note however that as we add support for multi headed devices (MHDs)
a subset of commands will be available on selected MHD type 3 mailboxes.
(tunnelling DCD commands for example)

See: CXL rev 3.0
7.2.9 Switch Mailbox CCI
8.1.13 Switch Malibox CCI Configuration Space Layout
8.2.8.6 Switch Mailbox CCI capability 

It is probably relatively unusual that a typical host of CXL devices
will have access to the one of these devices, in many cases they will
be on a port connected to a BMC or similar. There are a few use cases
where the host might be in charge of the configuration.

These are very convenient for testing in conjunction with the QEMU
emulation though so far CXL switch and type 3 emulation is in QEMU
is not complex enough to make these particular interesting.

This initial support provides only a few commands but I'm sending it
out as an RFC to get some input on how we should refactor the CXL core
code to support these devices that use some of the provide functionality.

Test wise, there is emulation support on
http://gitlab.com/jic23/qemu cxl-2022-10-24 branch

Assumption for now is that the switch CCI function sits alongside
a CXL USP. A config blob along the lines of:

 -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \
 -device cxl-rp,port=0,bus=cxl.1,id=root_port0,chassis=0,slot=2 \
 -device cxl-rp,port=1,bus=cxl.1,id=root_port2,chassis=0,slot=3 \
 -device cxl-upstream,port=33,bus=root_port0,id=us0,multifunction=on,addr=0.0,sn=12345678 \
 -device cxl-switch-mailbox-cci,bus=root_port0,addr=0.1 \
 -device cxl-downstream,port=0,bus=us0,id=swport0,chassis=0,slot=4 \
 -device cxl-downstream,port=1,bus=us0,id=swport1,chassis=0,slot=5 \
 -device cxl-downstream,port=2,bus=us0,id=swport2,chassis=0,slot=6 \
 -device cxl-downstream,port=3,bus=us0,id=swport3,chassis=0,slot=7

should create a switch with a suitable function 1 alongside the switch CCI.

Test wise, an example using the user space ioctls is given below.
Longer term this will want to be done with a suitable open source fabric
manager library / program.

#include <linux/types.h>
#include <stdint.h>
#include <sys/ioctl.h>
#include <stdio.h>
#include <fcntl.h>
#include <stdlib.h>
#include "cxl_mem.h"

/* Move to appropriate header later */
struct cxl_cmd_infostat_identify_rsp {
  uint16_t pcie_vid;
  uint16_t pcie_did;
  uint16_t pcie_subsys_vid;
  uint16_t pcie_subsys_id;
  uint64_t sn;
  uint8_t max_message_size;
  uint8_t component_type;
};

struct cxl_cmd_infostat_get_bg_cmd_sts_rsp { 
  uint8_t status;
  uint8_t rsvd;
  uint16_t opcode;
  uint16_t returncode;
  uint16_t vendor_ext_status;
};

struct cxl_cmd_identify_switch_device_rsp {
  uint8_t ingress_port_id;
  uint8_t rsvd;
  uint8_t num_physical_ports;
  uint8_t num_vcs;
  uint8_t active_port_bm[0x20];
  uint8_t vcs_bm[0x20];
  uint16_t total_num_vPPBs;
  uint16_t num_bound_vPPBs;
  uint8_t num_hdm_decoders;
} __attribute__((packed));

int main()
{
  struct cxl_send_command cmd = {};
  struct cxl_cmd_infostat_identify_rsp is_identify;
  struct cxl_cmd_identify_switch_device_rsp switch_identify;
  struct cxl_cmd_infostat_get_bg_cmd_sts_rsp bg_cmd_status;
  int fd;
  int rc, i;

  fd = open("/dev/cxl/swcci0", O_RDWR);
  if (fd < 0) {
    printf("could not open file\n");
    return 0;
  }
  cmd.id = CXL_MEM_COMMAND_ID_RAW;
  cmd.id = CXL_MEM_COMMAND_ID_INFO_STAT_IDENT;
  cmd.out.size = sizeof(is_identify);
  cmd.out.payload = (__u64)&is_identify;

  rc = ioctl(fd, CXL_MEM_SEND_COMMAND, &cmd);
  if (rc) {
    printf("rc %d\n", rc);
    if (rc < 0)
      return rc;
  }

  printf("Identify on switch:\n");
  printf("VID:0x%04x DID:0x%04x\n", is_identify.pcie_vid, is_identify.pcie_did);
  printf("Subsys: VID:0x%04x DID:0x%04x\n", is_identify.pcie_subsys_vid, is_identify.pcie_subsys_id);

  cmd.id = CXL_MEM_COMMAND_ID_GET_BG_CMD_STATUS;
  cmd.out.size = sizeof(bg_cmd_status);
  cmd.out.payload = (__u64)&bg_cmd_status;

  rc = ioctl(fd, CXL_MEM_SEND_COMMAND, &cmd);
  if (rc) {
    printf("rc %d\n", rc);
    if (rc < 0)
      return rc;
  }

  cmd.id = CXL_MEM_COMMAND_ID_IDENTIFY_SWITCH_DEVICE;
  cmd.out.size = sizeof(switch_identify);
  cmd.out.payload = (__u64)&switch_identify;

  rc = ioctl(fd, CXL_MEM_SEND_COMMAND, &cmd);
  if (rc) {
    printf("rc %d\n", rc);
    if (rc < 0)
      return rc;
  }

  printf("Switch indent ingress=%#x #ports=%d\n",
	 switch_identify.ingress_port_id,
	 switch_identify.num_physical_ports);
  for (i = 0; i < sizeof(switch_identify.active_port_bm); i++) {
    int j;
    for (j = 0; j < 8; j++) {
      if (switch_identify.active_port_bm[i] & 1 << j) {
	printf("Port %x active\n", i * 8 + j);
      }
    }
  }
  
  return 0;	 
}

Jonathan Cameron (4):
  cxl/mbox: Use local cxl_device_state variable
  cxl/mbox: Change paramaters to cxl_send_cmd() to not assume a cxl
    memory device.
  PCI: Add PCI_CLASS_SERIAL_CXL_SWITCH_CCI class ID to pci_ids.h
  cxl/pci: Add support for stand alone CXL Switch mailbox CCI

 drivers/cxl/core/Makefile     |   1 +
 drivers/cxl/core/core.h       |   6 +-
 drivers/cxl/core/mbox.c       |  12 ++-
 drivers/cxl/core/memdev.c     |   4 +-
 drivers/cxl/core/port.c       |   4 +
 drivers/cxl/core/switch-cci.c | 149 ++++++++++++++++++++++++++++++++++
 drivers/cxl/cxlmem.h          |   3 +
 drivers/cxl/cxlpci.h          |   3 +
 drivers/cxl/cxlswitch.h       |  18 ++++
 drivers/cxl/pci.c             |  95 +++++++++++++++++++++-
 include/linux/pci_ids.h       |   1 +
 include/uapi/linux/cxl_mem.h  |   3 +
 12 files changed, 292 insertions(+), 7 deletions(-)
 create mode 100644 drivers/cxl/core/switch-cci.c
 create mode 100644 drivers/cxl/cxlswitch.h

-- 
2.37.2

[RFC PATCH v2 1/4] cxl/mbox: Use local cxl_device_state variable

[Jonathan Cameron]

Given we have cxlds as a local variable, use that rather than
getting it again from cxlmd.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/cxl/core/mbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 0c90f13870a4..dd9e0aac37ae 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -539,7 +539,7 @@ int cxl_send_cmd(struct cxl_memdev *cxlmd, struct cxl_send_command __user *s)
 	if (copy_from_user(&send, s, sizeof(send)))
 		return -EFAULT;
 
-	rc = cxl_validate_cmd_from_user(&mbox_cmd, cxlmd->cxlds, &send);
+	rc = cxl_validate_cmd_from_user(&mbox_cmd, cxlds, &send);
 	if (rc)
 		return rc;
 
-- 
2.37.2

[RFC PATCH v2 2/4] cxl/mbox: Change parameters to cxl_send_cmd() to not assume a cxl memory device.

[Jonathan Cameron]

Intent here is to enable CXL switch CCIs in the following patch.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/cxl/core/core.h   | 5 ++++-
 drivers/cxl/core/mbox.c   | 5 ++---
 drivers/cxl/core/memdev.c | 4 +++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 1d8f87be283f..0835942bcea6 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -41,9 +41,12 @@ static inline void cxl_region_exit(void)
 
 struct cxl_send_command;
 struct cxl_mem_query_commands;
+struct cxl_dev_state;
+struct device;
 int cxl_query_cmd(struct cxl_memdev *cxlmd,
 		  struct cxl_mem_query_commands __user *q);
-int cxl_send_cmd(struct cxl_memdev *cxlmd, struct cxl_send_command __user *s);
+int cxl_send_cmd(struct cxl_dev_state *cxlds, struct device *dev,
+		 struct cxl_send_command __user *s);
 void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t addr,
 				   resource_size_t length);
 
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index dd9e0aac37ae..3823d450fdd2 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -526,10 +526,9 @@ static int handle_mailbox_cmd_from_user(struct cxl_dev_state *cxlds,
 	return rc;
 }
 
-int cxl_send_cmd(struct cxl_memdev *cxlmd, struct cxl_send_command __user *s)
+int cxl_send_cmd(struct cxl_dev_state *cxlds, struct device *dev,
+		 struct cxl_send_command __user *s)
 {
-	struct cxl_dev_state *cxlds = cxlmd->cxlds;
-	struct device *dev = &cxlmd->dev;
 	struct cxl_send_command send;
 	struct cxl_mbox_cmd mbox_cmd;
 	int rc;
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 20ce488a7754..62840a6da140 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -269,11 +269,13 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
 static long __cxl_memdev_ioctl(struct cxl_memdev *cxlmd, unsigned int cmd,
 			       unsigned long arg)
 {
+	struct cxl_dev_state *cxlds = cxlmd->cxlds;
+	struct device *dev = &cxlmd->dev;
 	switch (cmd) {
 	case CXL_MEM_QUERY_COMMANDS:
 		return cxl_query_cmd(cxlmd, (void __user *)arg);
 	case CXL_MEM_SEND_COMMAND:
-		return cxl_send_cmd(cxlmd, (void __user *)arg);
+		return cxl_send_cmd(cxlds, dev, (void __user *)arg);
 	default:
 		return -ENOTTY;
 	}
-- 
2.37.2

[RFC PATCH v2 3/4] PCI: Add PCI_CLASS_SERIAL_CXL_SWITCH_CCI class ID to pci_ids.h

[Jonathan Cameron]

This ID is used for CXL Switch Mailbox CCIs.
Incorporate the Programming Interface in the definition in common
with similar entries.

See CXL rev3.0 81.1.3.1 PCI Header Class Code Register

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 include/linux/pci_ids.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index b362d90eb9b0..a2b30ef94876 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -128,6 +128,7 @@
 #define PCI_CLASS_SERIAL_IPMI_SMIC	0x0c0700
 #define PCI_CLASS_SERIAL_IPMI_KCS	0x0c0701
 #define PCI_CLASS_SERIAL_IPMI_BT	0x0c0702
+#define PCI_CLASS_SERIAL_CXL_SWITCH_CCI	0x0c0b00
 
 #define PCI_BASE_CLASS_WIRELESS			0x0d
 #define PCI_CLASS_WIRELESS_RF_CONTROLLER	0x0d10
-- 
2.37.2

[RFC PATCH v2 4/4] cxl/pci: Add support for stand alone CXL Switch mailbox CCI

[Jonathan Cameron]

CXL 3.0 defines a mailbox PCI function independent of any other CXL
components. The intent is that instances of this mailbox will be found
as additional PCI functions of upstream CXL switch ports.

RFC: Including this directly in cxl/pci.c as a second pci_driver, is a
bit hacky.  The alternative is to factor out all the common
infrastructure to a library module shared by the Type 3 PCI driver
and the Switch CCI driver.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
V2: Thanks to Viacheslav Dubeyko for review.
 - Drop double free of IDA in error path.
 - Switch to cxl_send_cmd()
 - Hex constant for size.
 - Shuffle code to put the release down near alloc.
 - Correctly handle shutdown in unregister path by setting the
   cxlswd->cxlds = NULL.
 - Use new define in pci_ids.h instead of opencoding the class code.

Options to consider:
1 - Factor out all the shared code and have a separate module for
    switch CCI driver. Messy!
2 - Is sharing the allow lists between type 3 devices and switch CCI
    an issue? Not a whole lot of overlap...
---
 drivers/cxl/core/Makefile     |   1 +
 drivers/cxl/core/core.h       |   1 +
 drivers/cxl/core/mbox.c       |   5 ++
 drivers/cxl/core/port.c       |   4 +
 drivers/cxl/core/switch-cci.c | 149 ++++++++++++++++++++++++++++++++++
 drivers/cxl/cxlmem.h          |   3 +
 drivers/cxl/cxlpci.h          |   3 +
 drivers/cxl/cxlswitch.h       |  18 ++++
 drivers/cxl/pci.c             |  95 +++++++++++++++++++++-
 include/uapi/linux/cxl_mem.h  |   3 +
 10 files changed, 281 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index 79c7257f4107..18275e153437 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -10,4 +10,5 @@ cxl_core-y += memdev.o
 cxl_core-y += mbox.o
 cxl_core-y += pci.o
 cxl_core-y += hdm.o
+cxl_core-y += switch-cci.o
 cxl_core-$(CONFIG_CXL_REGION) += region.o
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 0835942bcea6..f6a35e7f980c 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -73,5 +73,6 @@ static inline struct cxl_ep *cxl_ep_load(struct cxl_port *port,
 int cxl_memdev_init(void);
 void cxl_memdev_exit(void);
 void cxl_mbox_init(void);
+int cxl_switch_cci_init(void);
 
 #endif /* __CXL_CORE_H__ */
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 3823d450fdd2..76987f2e30e2 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -5,6 +5,7 @@
 #include <linux/debugfs.h>
 #include <linux/mutex.h>
 #include <cxlmem.h>
+#include <cxlpci.h>
 #include <cxl.h>
 
 #include "core.h"
@@ -43,6 +44,8 @@ static bool cxl_raw_allow_all;
  * 0, and the user passed in 1, it is an error.
  */
 static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
+	CXL_CMD(INFO_STAT_IDENTIFY, 0, 0x12, 0),
+	CXL_CMD(GET_BG_CMD_STATUS, 0, 8, 0),
 	CXL_CMD(IDENTIFY, 0, 0x43, CXL_CMD_FLAG_FORCE_ENABLE),
 #ifdef CONFIG_CXL_MEM_RAW_COMMANDS
 	CXL_CMD(RAW, CXL_VARIABLE_PAYLOAD, CXL_VARIABLE_PAYLOAD, 0),
@@ -65,6 +68,7 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
 	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
 	CXL_CMD(SCAN_MEDIA, 0x11, 0, 0),
 	CXL_CMD(GET_SCAN_MEDIA, 0, CXL_VARIABLE_PAYLOAD, 0),
+	CXL_CMD(IDENTIFY_SWITCH_DEVICE, 0, 0x49, 0),
 };
 
 /*
@@ -552,6 +556,7 @@ int cxl_send_cmd(struct cxl_dev_state *cxlds, struct device *dev,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(cxl_send_cmd);
 
 static int cxl_xfer_log(struct cxl_dev_state *cxlds, uuid_t *uuid, u32 size, u8 *out)
 {
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index bffde862de0b..3b2e93bc4684 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1859,6 +1859,10 @@ static __init int cxl_core_init(void)
 	if (rc)
 		return rc;
 
+	rc = cxl_switch_cci_init();
+	if (rc)
+		return rc;
+
 	cxl_bus_wq = alloc_ordered_workqueue("cxl_port", 0);
 	if (!cxl_bus_wq) {
 		rc = -ENOMEM;
diff --git a/drivers/cxl/core/switch-cci.c b/drivers/cxl/core/switch-cci.c
new file mode 100644
index 000000000000..8c3f2b8a3b53
--- /dev/null
+++ b/drivers/cxl/core/switch-cci.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <cxlswitch.h>
+#include "cxlmem.h" /* For now to get the cxl_device_state */
+#include "cxlpci.h"
+#include "core.h"
+
+
+static int cxl_sw_major;
+static DEFINE_IDA(cxl_swdev_ida);
+static DECLARE_RWSEM(cxl_swdev_rwsem);
+
+static inline struct cxl_swdev *to_cxl_swdev(struct device *dev)
+{
+	return container_of(dev, struct cxl_swdev, dev);
+}
+
+static char *cxl_swdev_devnode(struct device *dev, umode_t *mode, kuid_t *uid,
+			kgid_t *gid)
+{
+	return kasprintf(GFP_KERNEL, "cxl/%s", dev_name(dev));
+}
+
+static long __cxl_swdev_ioctl(struct cxl_swdev *cxlswd, unsigned int cmd,
+			       unsigned long arg)
+{
+	switch (cmd) {
+	case CXL_MEM_SEND_COMMAND:
+		return cxl_send_cmd(cxlswd->cxlds, &cxlswd->dev, (void __user *)arg);
+	default:
+		return -ENOTTY;
+	}
+}
+
+static long cxl_swdev_ioctl(struct file *file, unsigned int cmd,
+			    unsigned long arg)
+{
+	struct cxl_swdev *cxlswd = file->private_data;
+	int rc = -ENXIO;
+
+	down_read(&cxl_swdev_rwsem);
+	if (cxlswd->cxlds)
+		rc = __cxl_swdev_ioctl(cxlswd, cmd, arg);
+	up_read(&cxl_swdev_rwsem);
+
+	return rc;
+}
+
+static int cxl_swdev_open(struct inode *inode, struct file *file)
+{
+	struct cxl_memdev *cxlswd =
+		container_of(inode->i_cdev, typeof(*cxlswd), cdev);
+
+	get_device(&cxlswd->dev);
+	file->private_data = cxlswd;
+
+	return 0;
+}
+
+static int cxl_swdev_release_file(struct inode *inode, struct file *file)
+{
+	struct cxl_swdev *cxlswd =
+		container_of(inode->i_cdev, typeof(*cxlswd), cdev);
+
+	put_device(&cxlswd->dev);
+
+	return 0;
+}
+
+static const struct file_operations cxl_swdev_fops = {
+	.owner = THIS_MODULE,
+	.unlocked_ioctl = cxl_swdev_ioctl,
+	.open = cxl_swdev_open,
+	.release = cxl_swdev_release_file,
+	.compat_ioctl = compat_ptr_ioctl,
+	.llseek = noop_llseek,
+};
+
+void cxl_swdev_shutdown(struct cxl_swdev *cxlswd)
+{
+	down_write(&cxl_swdev_rwsem);
+	cxlswd->cxlds = NULL;
+	up_write(&cxl_swdev_rwsem);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_swdev_shutdown, CXL);
+
+static void cxl_swdev_release(struct device *dev)
+{
+	struct cxl_swdev *cxlswd = to_cxl_swdev(dev);
+
+	ida_free(&cxl_swdev_ida, cxlswd->id);
+	kfree(cxlswd);
+}
+
+static const struct device_type cxl_swdev_type = {
+	.name = "cxl_swdev",
+	.release = cxl_swdev_release,
+	.devnode = cxl_swdev_devnode,
+};
+
+struct cxl_swdev *cxl_swdev_alloc(struct cxl_dev_state *cxlds)
+{
+	struct cxl_swdev *cxlswd;
+	struct device *dev;
+	struct cdev *cdev;
+	int rc;
+
+	cxlswd = kzalloc(sizeof(*cxlswd), GFP_KERNEL);
+	if (!cxlswd)
+		return ERR_PTR(-ENOMEM);
+
+	rc = ida_alloc_range(&cxl_swdev_ida, 0, 10, GFP_KERNEL);
+	if (rc < 0) {
+		kfree(cxlswd);
+		return ERR_PTR(rc);
+	}
+
+	cxlswd->id = rc;
+	cxlswd->cxlds = cxlds;
+	dev = &cxlswd->dev;
+	device_initialize(dev);
+	dev->parent = cxlds->dev;
+	dev->bus = &cxl_bus_type;
+	dev->devt = MKDEV(cxl_sw_major, cxlswd->id);
+	dev->type = &cxl_swdev_type;
+	device_set_pm_not_required(dev);
+	cdev = &cxlswd->cdev;
+	cdev_init(cdev, &cxl_swdev_fops);
+	rc = dev_set_name(dev, "swcci%d", cxlswd->id);
+	if (rc) {
+		put_device(dev);
+		return ERR_PTR(rc);
+	}
+
+	return cxlswd;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_swdev_alloc, CXL);
+
+__init int cxl_switch_cci_init(void)
+{
+	dev_t devt;
+	int rc;
+
+	rc = alloc_chrdev_region(&devt, 0, 10, "cxlsw");
+	if (rc)
+		return rc;
+	cxl_sw_major = MAJOR(devt);
+
+	return 0;
+}
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 88e3a8e54b6a..ef9c0c347daf 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -252,6 +252,8 @@ struct cxl_dev_state {
 
 enum cxl_opcode {
 	CXL_MBOX_OP_INVALID		= 0x0000,
+	CXL_MBOX_OP_INFO_STAT_IDENTIFY	= 0x0001,
+	CXL_MBOX_OP_GET_BG_CMD_STATUS	= 0x0002,
 	CXL_MBOX_OP_RAW			= CXL_MBOX_OP_INVALID,
 	CXL_MBOX_OP_GET_FW_INFO		= 0x0200,
 	CXL_MBOX_OP_ACTIVATE_FW		= 0x0202,
@@ -273,6 +275,7 @@ enum cxl_opcode {
 	CXL_MBOX_OP_GET_SCAN_MEDIA_CAPS	= 0x4303,
 	CXL_MBOX_OP_SCAN_MEDIA		= 0x4304,
 	CXL_MBOX_OP_GET_SCAN_MEDIA	= 0x4305,
+	CXL_MBOX_OP_IDENTIFY_SWITCH_DEVICE	= 0x5100,
 	CXL_MBOX_OP_MAX			= 0x10000
 };
 
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index eec597dbe763..7f53b601ad7c 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -75,4 +75,7 @@ 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);
 void read_cdat_data(struct cxl_port *port);
+struct cxl_send_command;
+int __cxl_send_cmd(struct cxl_dev_state *cxlds, struct device *dev,
+		   struct cxl_send_command __user *s);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/cxlswitch.h b/drivers/cxl/cxlswitch.h
new file mode 100644
index 000000000000..d823d2cc159d
--- /dev/null
+++ b/drivers/cxl/cxlswitch.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef CXLSWITCH_H
+#define CXLSWITCH_H
+#include <linux/cdev.h>
+#include <linux/device.h>
+#include "cxl.h"
+
+struct cxl_dev_state;
+struct cxl_swdev {
+	struct device dev;
+	struct cdev cdev;
+	struct cxl_dev_state *cxlds;
+	int id;
+};
+
+struct cxl_swdev *cxl_swdev_alloc(struct cxl_dev_state *cxlds);
+void cxl_swdev_shutdown(struct cxl_swdev *cxlswd);
+#endif
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index faeb5d9d7a7a..5f79742fb266 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -12,6 +12,7 @@
 #include <linux/io.h>
 #include "cxlmem.h"
 #include "cxlpci.h"
+#include "cxlswitch.h"
 #include "cxl.h"
 
 /**
@@ -520,6 +521,98 @@ static struct pci_driver cxl_pci_driver = {
 	},
 };
 
+static int cxl_swmbcci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct cxl_dev_state *cxlds;
+	struct cxl_register_map map;
+	struct cxl_swdev *cxlswd;
+	int rc;
+
+	rc = pcim_enable_device(pdev);
+	if (rc)
+		return rc;
+
+	cxlds = cxl_dev_state_create(&pdev->dev);
+	if (IS_ERR(cxlds))
+		return PTR_ERR(cxlds);
+
+	rc = cxl_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
+	if (rc)
+		return rc;
+
+	rc = cxl_map_regs(cxlds, &map);
+	if (rc)
+		return rc;
+
+	rc = cxl_pci_setup_mailbox(cxlds);
+	if (rc)
+		return rc;
+
+	cxlswd = cxl_swdev_alloc(cxlds);
+	if (IS_ERR(cxlswd))
+		return PTR_ERR(cxlswd);
+
+	pci_set_drvdata(pdev, cxlswd);
+
+	rc = cxl_enumerate_cmds(cxlds);
+	if (rc)
+		goto error_put_device;
+
+	rc = cdev_device_add(&cxlswd->cdev, &cxlswd->dev);
+	if (rc)
+		goto error_put_device;
+
+	return 0;
+
+error_put_device:
+	cxl_swdev_shutdown(cxlswd);
+	put_device(&cxlswd->dev);
+	return rc;
+}
+
+static void cxl_swbmcci_remove(struct pci_dev *pdev)
+{
+	struct cxl_swdev *cxlswd = pci_get_drvdata(pdev);
+	struct device *dev = &cxlswd->dev;
+
+	cxl_swdev_shutdown(cxlswd);
+	cdev_device_del(&cxlswd->cdev, dev);
+	put_device(&cxlswd->dev);
+}
+
+static const struct pci_device_id cxl_swmbcci_pci_tbl[] = {
+	{ PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_CXL_SWITCH_CCI, ~0) },
+	{}
+};
+
+static struct pci_driver cxl_swmbcci_driver = {
+	.name = "cxl_swmbcci",
+	.id_table = cxl_swmbcci_pci_tbl,
+	.probe = cxl_swmbcci_probe,
+	.remove = cxl_swbmcci_remove,
+};
+
+static int __init cxl_pci_init(void)
+{
+	int rc;
+
+	rc = pci_register_driver(&cxl_pci_driver);
+	if (rc)
+		return rc;
+
+	rc = pci_register_driver(&cxl_swmbcci_driver);
+	if (rc) {
+		pci_unregister_driver(&cxl_pci_driver);
+		return rc;
+	}
+	return 0;
+}
+module_init(cxl_pci_init);
+static void __exit cxl_pci_exit(void)
+{
+	pci_unregister_driver(&cxl_swmbcci_driver);
+	pci_unregister_driver(&cxl_pci_driver);
+}
+module_exit(cxl_pci_exit);
 MODULE_LICENSE("GPL v2");
-module_pci_driver(cxl_pci_driver);
 MODULE_IMPORT_NS(CXL);
diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
index c71021a2a9ed..ea03a289e56f 100644
--- a/include/uapi/linux/cxl_mem.h
+++ b/include/uapi/linux/cxl_mem.h
@@ -41,6 +41,9 @@
 	___C(GET_SCAN_MEDIA_CAPS, "Get Scan Media Capabilities"),         \
 	___C(SCAN_MEDIA, "Scan Media"),                                   \
 	___C(GET_SCAN_MEDIA, "Get Scan Media Results"),                   \
+	___C(INFO_STAT_IDENTIFY, "Get Information"),			  \
+	___C(GET_BG_CMD_STATUS, "Background Command Status"),             \
+	___C(IDENTIFY_SWITCH_DEVICE, "Identify Switch Device"),           \
 	___C(MAX, "invalid / last command")
 
 #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
-- 
2.37.2

Re: [RFC PATCH v2 1/4] cxl/mbox: Use local cxl_device_state variable

[Dave Jiang]

On 10/25/2022 3:42 AM, Jonathan Cameron wrote:
> Given we have cxlds as a local variable, use that rather than
> getting it again from cxlmd.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

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

This patch can be split out and go upstream as a cleanup now right?

> ---
>   drivers/cxl/core/mbox.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 0c90f13870a4..dd9e0aac37ae 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -539,7 +539,7 @@ int cxl_send_cmd(struct cxl_memdev *cxlmd, struct cxl_send_command __user *s)
>   	if (copy_from_user(&send, s, sizeof(send)))
>   		return -EFAULT;
>   
> -	rc = cxl_validate_cmd_from_user(&mbox_cmd, cxlmd->cxlds, &send);
> +	rc = cxl_validate_cmd_from_user(&mbox_cmd, cxlds, &send);
>   	if (rc)
>   		return rc;
>   

RE: [RFC PATCH v2 0/4] CXL: Standalone switch CCI driver

[Dan Williams]

Jonathan Cameron wrote:
> Changes since v1: (thanks Slava!)
> - 3 precusor patches added
>     PCI Class ID added to pci_ids.h
>     Refactoring to change parameters of cxl_send_cmd() to make
>     it suitable for use from the new switch CCI code.
> - Various fixes around error paths and tidying up (see patch 4)
> 
> CXL rev 3.0 introduced the option for a PCI function, intended to sit on an
> upstream port of a CXL switch.  This function provides a mailbox
> interface similar to that seen on CXL type 3 devices. However, the
> command set is mostly different and intended for Fabric management.
> Note however that as we add support for multi headed devices (MHDs)
> a subset of commands will be available on selected MHD type 3 mailboxes.
> (tunnelling DCD commands for example)
> 
> See: CXL rev 3.0
> 7.2.9 Switch Mailbox CCI
> 8.1.13 Switch Malibox CCI Configuration Space Layout
> 8.2.8.6 Switch Mailbox CCI capability 
> 
> It is probably relatively unusual that a typical host of CXL devices
> will have access to the one of these devices, in many cases they will
> be on a port connected to a BMC or similar. There are a few use cases
> where the host might be in charge of the configuration.
> 
> These are very convenient for testing in conjunction with the QEMU
> emulation though so far CXL switch and type 3 emulation is in QEMU
> is not complex enough to make these particular interesting.
> 
> This initial support provides only a few commands but I'm sending it
> out as an RFC to get some input on how we should refactor the CXL core
> code to support these devices that use some of the provide functionality.
> 
> Test wise, there is emulation support on
> http://gitlab.com/jic23/qemu cxl-2022-10-24 branch
> 
> Assumption for now is that the switch CCI function sits alongside
> a CXL USP. A config blob along the lines of:
> 
>  -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \
>  -device cxl-rp,port=0,bus=cxl.1,id=root_port0,chassis=0,slot=2 \
>  -device cxl-rp,port=1,bus=cxl.1,id=root_port2,chassis=0,slot=3 \
>  -device cxl-upstream,port=33,bus=root_port0,id=us0,multifunction=on,addr=0.0,sn=12345678 \
>  -device cxl-switch-mailbox-cci,bus=root_port0,addr=0.1 \
>  -device cxl-downstream,port=0,bus=us0,id=swport0,chassis=0,slot=4 \
>  -device cxl-downstream,port=1,bus=us0,id=swport1,chassis=0,slot=5 \
>  -device cxl-downstream,port=2,bus=us0,id=swport2,chassis=0,slot=6 \
>  -device cxl-downstream,port=3,bus=us0,id=swport3,chassis=0,slot=7
> 
> should create a switch with a suitable function 1 alongside the switch CCI.
> 
> Test wise, an example using the user space ioctls is given below.
> Longer term this will want to be done with a suitable open source fabric
> manager library / program.
> 
> #include <linux/types.h>
> #include <stdint.h>
> #include <sys/ioctl.h>
> #include <stdio.h>
> #include <fcntl.h>
> #include <stdlib.h>
> #include "cxl_mem.h"
> 
> /* Move to appropriate header later */
> struct cxl_cmd_infostat_identify_rsp {
>   uint16_t pcie_vid;
>   uint16_t pcie_did;
>   uint16_t pcie_subsys_vid;
>   uint16_t pcie_subsys_id;
>   uint64_t sn;
>   uint8_t max_message_size;
>   uint8_t component_type;
> };
> 
> struct cxl_cmd_infostat_get_bg_cmd_sts_rsp { 
>   uint8_t status;
>   uint8_t rsvd;
>   uint16_t opcode;
>   uint16_t returncode;
>   uint16_t vendor_ext_status;
> };
> 
> struct cxl_cmd_identify_switch_device_rsp {
>   uint8_t ingress_port_id;
>   uint8_t rsvd;
>   uint8_t num_physical_ports;
>   uint8_t num_vcs;
>   uint8_t active_port_bm[0x20];
>   uint8_t vcs_bm[0x20];
>   uint16_t total_num_vPPBs;
>   uint16_t num_bound_vPPBs;
>   uint8_t num_hdm_decoders;
> } __attribute__((packed));
> 
> int main()
> {
>   struct cxl_send_command cmd = {};
>   struct cxl_cmd_infostat_identify_rsp is_identify;
>   struct cxl_cmd_identify_switch_device_rsp switch_identify;
>   struct cxl_cmd_infostat_get_bg_cmd_sts_rsp bg_cmd_status;
>   int fd;
>   int rc, i;
> 
>   fd = open("/dev/cxl/swcci0", O_RDWR);
>   if (fd < 0) {
>     printf("could not open file\n");
>     return 0;
>   }
>   cmd.id = CXL_MEM_COMMAND_ID_RAW;

Is the expectation that only "raw" mode operation is needed for this?
That reduces some of the woory since one would need to be running a
debug kernel and a debug utility.

However, if this is going to be a more formal capability then maybe it
wants a sysfs model and the full ABI vetting.


>   cmd.id = CXL_MEM_COMMAND_ID_INFO_STAT_IDENT;
>   cmd.out.size = sizeof(is_identify);
>   cmd.out.payload = (__u64)&is_identify;
> 
>   rc = ioctl(fd, CXL_MEM_SEND_COMMAND, &cmd);
>   if (rc) {
>     printf("rc %d\n", rc);
>     if (rc < 0)
>       return rc;
>   }
> 
>   printf("Identify on switch:\n");
>   printf("VID:0x%04x DID:0x%04x\n", is_identify.pcie_vid, is_identify.pcie_did);
>   printf("Subsys: VID:0x%04x DID:0x%04x\n", is_identify.pcie_subsys_vid, is_identify.pcie_subsys_id);
> 
>   cmd.id = CXL_MEM_COMMAND_ID_GET_BG_CMD_STATUS;
>   cmd.out.size = sizeof(bg_cmd_status);
>   cmd.out.payload = (__u64)&bg_cmd_status;
> 
>   rc = ioctl(fd, CXL_MEM_SEND_COMMAND, &cmd);
>   if (rc) {
>     printf("rc %d\n", rc);
>     if (rc < 0)
>       return rc;
>   }
> 
>   cmd.id = CXL_MEM_COMMAND_ID_IDENTIFY_SWITCH_DEVICE;
>   cmd.out.size = sizeof(switch_identify);
>   cmd.out.payload = (__u64)&switch_identify;
> 
>   rc = ioctl(fd, CXL_MEM_SEND_COMMAND, &cmd);
>   if (rc) {
>     printf("rc %d\n", rc);
>     if (rc < 0)
>       return rc;
>   }
> 
>   printf("Switch indent ingress=%#x #ports=%d\n",
> 	 switch_identify.ingress_port_id,
> 	 switch_identify.num_physical_ports);
>   for (i = 0; i < sizeof(switch_identify.active_port_bm); i++) {
>     int j;
>     for (j = 0; j < 8; j++) {
>       if (switch_identify.active_port_bm[i] & 1 << j) {
> 	printf("Port %x active\n", i * 8 + j);
>       }
>     }
>   }
>   
>   return 0;	 
> }
> 
> Jonathan Cameron (4):
>   cxl/mbox: Use local cxl_device_state variable
>   cxl/mbox: Change paramaters to cxl_send_cmd() to not assume a cxl
>     memory device.
>   PCI: Add PCI_CLASS_SERIAL_CXL_SWITCH_CCI class ID to pci_ids.h
>   cxl/pci: Add support for stand alone CXL Switch mailbox CCI
> 
>  drivers/cxl/core/Makefile     |   1 +
>  drivers/cxl/core/core.h       |   6 +-
>  drivers/cxl/core/mbox.c       |  12 ++-
>  drivers/cxl/core/memdev.c     |   4 +-
>  drivers/cxl/core/port.c       |   4 +
>  drivers/cxl/core/switch-cci.c | 149 ++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxlmem.h          |   3 +
>  drivers/cxl/cxlpci.h          |   3 +
>  drivers/cxl/cxlswitch.h       |  18 ++++
>  drivers/cxl/pci.c             |  95 +++++++++++++++++++++-
>  include/linux/pci_ids.h       |   1 +
>  include/uapi/linux/cxl_mem.h  |   3 +
>  12 files changed, 292 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/cxl/core/switch-cci.c
>  create mode 100644 drivers/cxl/cxlswitch.h
> 
> -- 
> 2.37.2
> 

RE: [RFC PATCH v2 2/4] cxl/mbox: Change parameters to cxl_send_cmd() to not assume a cxl memory device.

[Dan Williams]

Jonathan Cameron wrote:
> Intent here is to enable CXL switch CCIs in the following patch.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/cxl/core/core.h   | 5 ++++-
>  drivers/cxl/core/mbox.c   | 5 ++---
>  drivers/cxl/core/memdev.c | 4 +++-
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 1d8f87be283f..0835942bcea6 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -41,9 +41,12 @@ static inline void cxl_region_exit(void)
>  
>  struct cxl_send_command;
>  struct cxl_mem_query_commands;
> +struct cxl_dev_state;
> +struct device;
>  int cxl_query_cmd(struct cxl_memdev *cxlmd,
>  		  struct cxl_mem_query_commands __user *q);
> -int cxl_send_cmd(struct cxl_memdev *cxlmd, struct cxl_send_command __user *s);
> +int cxl_send_cmd(struct cxl_dev_state *cxlds, struct device *dev,
> +		 struct cxl_send_command __user *s);

Me thinks this wants another level of indirection to move the generic
mailbox bits out of 'struct cxl_dev_state' into a common structure that
endpoint and switch drivers can include in their local driver states.

>  void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t addr,
>  				   resource_size_t length);
>  
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index dd9e0aac37ae..3823d450fdd2 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -526,10 +526,9 @@ static int handle_mailbox_cmd_from_user(struct cxl_dev_state *cxlds,
>  	return rc;
>  }
>  
> -int cxl_send_cmd(struct cxl_memdev *cxlmd, struct cxl_send_command __user *s)
> +int cxl_send_cmd(struct cxl_dev_state *cxlds, struct device *dev,
> +		 struct cxl_send_command __user *s)
>  {
> -	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> -	struct device *dev = &cxlmd->dev;
>  	struct cxl_send_command send;
>  	struct cxl_mbox_cmd mbox_cmd;
>  	int rc;
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 20ce488a7754..62840a6da140 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -269,11 +269,13 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
>  static long __cxl_memdev_ioctl(struct cxl_memdev *cxlmd, unsigned int cmd,
>  			       unsigned long arg)
>  {
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +	struct device *dev = &cxlmd->dev;
>  	switch (cmd) {
>  	case CXL_MEM_QUERY_COMMANDS:
>  		return cxl_query_cmd(cxlmd, (void __user *)arg);
>  	case CXL_MEM_SEND_COMMAND:
> -		return cxl_send_cmd(cxlmd, (void __user *)arg);
> +		return cxl_send_cmd(cxlds, dev, (void __user *)arg);
>  	default:
>  		return -ENOTTY;
>  	}
> -- 
> 2.37.2
> 

RE: [RFC PATCH v2 4/4] cxl/pci: Add support for stand alone CXL Switch mailbox CCI

[Dan Williams]

Jonathan Cameron wrote:
> CXL 3.0 defines a mailbox PCI function independent of any other CXL
> components. The intent is that instances of this mailbox will be found
> as additional PCI functions of upstream CXL switch ports.
> 
> RFC: Including this directly in cxl/pci.c as a second pci_driver, is a
> bit hacky.  The alternative is to factor out all the common
> infrastructure to a library module shared by the Type 3 PCI driver
> and the Switch CCI driver.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> ---
> V2: Thanks to Viacheslav Dubeyko for review.
>  - Drop double free of IDA in error path.
>  - Switch to cxl_send_cmd()
>  - Hex constant for size.
>  - Shuffle code to put the release down near alloc.
>  - Correctly handle shutdown in unregister path by setting the
>    cxlswd->cxlds = NULL.
>  - Use new define in pci_ids.h instead of opencoding the class code.
> 
> Options to consider:
> 1 - Factor out all the shared code and have a separate module for
>     switch CCI driver. Messy!
> 2 - Is sharing the allow lists between type 3 devices and switch CCI
>     an issue? Not a whole lot of overlap...
> ---
>  drivers/cxl/core/Makefile     |   1 +
>  drivers/cxl/core/core.h       |   1 +
>  drivers/cxl/core/mbox.c       |   5 ++
>  drivers/cxl/core/port.c       |   4 +
>  drivers/cxl/core/switch-cci.c | 149 ++++++++++++++++++++++++++++++++++

Warning, incoming bikeshed comment: How about dropping the CCI term?
It is really only used in the specification to indicate the class of
transports that can convey CXL commands, but as far as Linux terminology
it's just a mailbox. As for the filename, perhaps switchdev.c?


>  drivers/cxl/cxlmem.h          |   3 +
>  drivers/cxl/cxlpci.h          |   3 +
>  drivers/cxl/cxlswitch.h       |  18 ++++

The only reason the "cxl" prefix is there for cxlmem.h and cxlpci.h is
to not collide with "mem.h" and "pci.h" for usermode-linux builds and
the like that confuse the core that includes those headers by include
search path. So this one can just be switch.h unless and until a build
robot says otherwise.

>  drivers/cxl/pci.c             |  95 +++++++++++++++++++++-
>  include/uapi/linux/cxl_mem.h  |   3 +
>  10 files changed, 281 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index 79c7257f4107..18275e153437 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -10,4 +10,5 @@ cxl_core-y += memdev.o
>  cxl_core-y += mbox.o
>  cxl_core-y += pci.o
>  cxl_core-y += hdm.o
> +cxl_core-y += switch-cci.o
>  cxl_core-$(CONFIG_CXL_REGION) += region.o
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 0835942bcea6..f6a35e7f980c 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -73,5 +73,6 @@ static inline struct cxl_ep *cxl_ep_load(struct cxl_port *port,
>  int cxl_memdev_init(void);
>  void cxl_memdev_exit(void);
>  void cxl_mbox_init(void);
> +int cxl_switch_cci_init(void);
>  
>  #endif /* __CXL_CORE_H__ */
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 3823d450fdd2..76987f2e30e2 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -5,6 +5,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/mutex.h>
>  #include <cxlmem.h>
> +#include <cxlpci.h>
>  #include <cxl.h>
>  
>  #include "core.h"
> @@ -43,6 +44,8 @@ static bool cxl_raw_allow_all;
>   * 0, and the user passed in 1, it is an error.
>   */
>  static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
> +	CXL_CMD(INFO_STAT_IDENTIFY, 0, 0x12, 0),

Ah, guess this answers the earlier question about raw only or not.
However if there is a non-zero possibility of this being used in
production then it likely does want to be formalized.

> +	CXL_CMD(GET_BG_CMD_STATUS, 0, 8, 0),

This is only for "Bind vPPB", right? In that case the result of the Bind
comes via an event, correct? Perhaps this command does not need to be
exposed. At least there is no use case to expose it even for endpoint
background commands since userspace has no mechanism to reliably
associate to which command that status refers.

>  	CXL_CMD(IDENTIFY, 0, 0x43, CXL_CMD_FLAG_FORCE_ENABLE),
>  #ifdef CONFIG_CXL_MEM_RAW_COMMANDS
>  	CXL_CMD(RAW, CXL_VARIABLE_PAYLOAD, CXL_VARIABLE_PAYLOAD, 0),
> @@ -65,6 +68,7 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
>  	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
>  	CXL_CMD(SCAN_MEDIA, 0x11, 0, 0),
>  	CXL_CMD(GET_SCAN_MEDIA, 0, CXL_VARIABLE_PAYLOAD, 0),
> +	CXL_CMD(IDENTIFY_SWITCH_DEVICE, 0, 0x49, 0),

>  };
>  
>  /*
> @@ -552,6 +556,7 @@ int cxl_send_cmd(struct cxl_dev_state *cxlds, struct device *dev,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(cxl_send_cmd);

Why?

>  
>  static int cxl_xfer_log(struct cxl_dev_state *cxlds, uuid_t *uuid, u32 size, u8 *out)
>  {
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index bffde862de0b..3b2e93bc4684 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1859,6 +1859,10 @@ static __init int cxl_core_init(void)
>  	if (rc)
>  		return rc;
>  
> +	rc = cxl_switch_cci_init();
> +	if (rc)
> +		return rc;
> +
>  	cxl_bus_wq = alloc_ordered_workqueue("cxl_port", 0);
>  	if (!cxl_bus_wq) {
>  		rc = -ENOMEM;
> diff --git a/drivers/cxl/core/switch-cci.c b/drivers/cxl/core/switch-cci.c
> new file mode 100644
> index 000000000000..8c3f2b8a3b53
> --- /dev/null
> +++ b/drivers/cxl/core/switch-cci.c
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <cxlswitch.h>
> +#include "cxlmem.h" /* For now to get the cxl_device_state */
> +#include "cxlpci.h"
> +#include "core.h"
> +
> +
> +static int cxl_sw_major;

I would expect to just reuse (and rename) cxl_mem_major for any CXL
mailbox like character devices.

> +static DEFINE_IDA(cxl_swdev_ida);
> +static DECLARE_RWSEM(cxl_swdev_rwsem);

Not sure this needs its own vs share a common one across endpoints and
switches.

> +
> +static inline struct cxl_swdev *to_cxl_swdev(struct device *dev)
> +{
> +	return container_of(dev, struct cxl_swdev, dev);
> +}
> +
> +static char *cxl_swdev_devnode(struct device *dev, umode_t *mode, kuid_t *uid,
> +			kgid_t *gid)
> +{
> +	return kasprintf(GFP_KERNEL, "cxl/%s", dev_name(dev));
> +}
> +
> +static long __cxl_swdev_ioctl(struct cxl_swdev *cxlswd, unsigned int cmd,
> +			       unsigned long arg)
> +{
> +	switch (cmd) {
> +	case CXL_MEM_SEND_COMMAND:
> +		return cxl_send_cmd(cxlswd->cxlds, &cxlswd->dev, (void __user *)arg);
> +	default:
> +		return -ENOTTY;
> +	}
> +}
> +
> +static long cxl_swdev_ioctl(struct file *file, unsigned int cmd,
> +			    unsigned long arg)
> +{
> +	struct cxl_swdev *cxlswd = file->private_data;
> +	int rc = -ENXIO;
> +
> +	down_read(&cxl_swdev_rwsem);
> +	if (cxlswd->cxlds)
> +		rc = __cxl_swdev_ioctl(cxlswd, cmd, arg);
> +	up_read(&cxl_swdev_rwsem);
> +
> +	return rc;
> +}
> +
> +static int cxl_swdev_open(struct inode *inode, struct file *file)
> +{
> +	struct cxl_memdev *cxlswd =

I do think a common cxl_dev is warranted, but this looks like
s/cxl_memdev/cxl_swdev/ typo.

> +		container_of(inode->i_cdev, typeof(*cxlswd), cdev);
> +
> +	get_device(&cxlswd->dev);
> +	file->private_data = cxlswd;
> +
> +	return 0;
> +}
> +
> +static int cxl_swdev_release_file(struct inode *inode, struct file *file)
> +{
> +	struct cxl_swdev *cxlswd =
> +		container_of(inode->i_cdev, typeof(*cxlswd), cdev);
> +
> +	put_device(&cxlswd->dev);
> +
> +	return 0;
> +}
> +
> +static const struct file_operations cxl_swdev_fops = {
> +	.owner = THIS_MODULE,
> +	.unlocked_ioctl = cxl_swdev_ioctl,
> +	.open = cxl_swdev_open,
> +	.release = cxl_swdev_release_file,
> +	.compat_ioctl = compat_ptr_ioctl,
> +	.llseek = noop_llseek,
> +};
> +
> +void cxl_swdev_shutdown(struct cxl_swdev *cxlswd)
> +{
> +	down_write(&cxl_swdev_rwsem);
> +	cxlswd->cxlds = NULL;
> +	up_write(&cxl_swdev_rwsem);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_swdev_shutdown, CXL);
> +
> +static void cxl_swdev_release(struct device *dev)
> +{
> +	struct cxl_swdev *cxlswd = to_cxl_swdev(dev);
> +
> +	ida_free(&cxl_swdev_ida, cxlswd->id);
> +	kfree(cxlswd);
> +}
> +
> +static const struct device_type cxl_swdev_type = {
> +	.name = "cxl_swdev",
> +	.release = cxl_swdev_release,
> +	.devnode = cxl_swdev_devnode,
> +};
> +
> +struct cxl_swdev *cxl_swdev_alloc(struct cxl_dev_state *cxlds)
> +{
> +	struct cxl_swdev *cxlswd;
> +	struct device *dev;
> +	struct cdev *cdev;
> +	int rc;
> +
> +	cxlswd = kzalloc(sizeof(*cxlswd), GFP_KERNEL);
> +	if (!cxlswd)
> +		return ERR_PTR(-ENOMEM);
> +
> +	rc = ida_alloc_range(&cxl_swdev_ida, 0, 10, GFP_KERNEL);
> +	if (rc < 0) {
> +		kfree(cxlswd);
> +		return ERR_PTR(rc);
> +	}
> +
> +	cxlswd->id = rc;
> +	cxlswd->cxlds = cxlds;
> +	dev = &cxlswd->dev;
> +	device_initialize(dev);
> +	dev->parent = cxlds->dev;
> +	dev->bus = &cxl_bus_type;
> +	dev->devt = MKDEV(cxl_sw_major, cxlswd->id);
> +	dev->type = &cxl_swdev_type;
> +	device_set_pm_not_required(dev);
> +	cdev = &cxlswd->cdev;
> +	cdev_init(cdev, &cxl_swdev_fops);
> +	rc = dev_set_name(dev, "swcci%d", cxlswd->id);

How about just "switch%d"? Likely this will also want some late binding
linkage to the eventual cxl_port that gets registered for this switch.

> +	if (rc) {
> +		put_device(dev);
> +		return ERR_PTR(rc);
> +	}
> +
> +	return cxlswd;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_swdev_alloc, CXL);

I think a few exports can be saved if this is changed to alloc+add like
other cxl core apis.

> +
> +__init int cxl_switch_cci_init(void)
> +{
> +	dev_t devt;
> +	int rc;
> +
> +	rc = alloc_chrdev_region(&devt, 0, 10, "cxlsw");
> +	if (rc)
> +		return rc;
> +	cxl_sw_major = MAJOR(devt);
> +
> +	return 0;
> +}
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 88e3a8e54b6a..ef9c0c347daf 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -252,6 +252,8 @@ struct cxl_dev_state {
>  
>  enum cxl_opcode {
>  	CXL_MBOX_OP_INVALID		= 0x0000,
> +	CXL_MBOX_OP_INFO_STAT_IDENTIFY	= 0x0001,
> +	CXL_MBOX_OP_GET_BG_CMD_STATUS	= 0x0002,
>  	CXL_MBOX_OP_RAW			= CXL_MBOX_OP_INVALID,
>  	CXL_MBOX_OP_GET_FW_INFO		= 0x0200,
>  	CXL_MBOX_OP_ACTIVATE_FW		= 0x0202,
> @@ -273,6 +275,7 @@ enum cxl_opcode {
>  	CXL_MBOX_OP_GET_SCAN_MEDIA_CAPS	= 0x4303,
>  	CXL_MBOX_OP_SCAN_MEDIA		= 0x4304,
>  	CXL_MBOX_OP_GET_SCAN_MEDIA	= 0x4305,
> +	CXL_MBOX_OP_IDENTIFY_SWITCH_DEVICE	= 0x5100,
>  	CXL_MBOX_OP_MAX			= 0x10000
>  };
>  
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index eec597dbe763..7f53b601ad7c 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -75,4 +75,7 @@ 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);
>  void read_cdat_data(struct cxl_port *port);
> +struct cxl_send_command;
> +int __cxl_send_cmd(struct cxl_dev_state *cxlds, struct device *dev,
> +		   struct cxl_send_command __user *s);
>  #endif /* __CXL_PCI_H__ */
> diff --git a/drivers/cxl/cxlswitch.h b/drivers/cxl/cxlswitch.h
> new file mode 100644
> index 000000000000..d823d2cc159d
> --- /dev/null
> +++ b/drivers/cxl/cxlswitch.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef CXLSWITCH_H
> +#define CXLSWITCH_H
> +#include <linux/cdev.h>
> +#include <linux/device.h>
> +#include "cxl.h"
> +
> +struct cxl_dev_state;
> +struct cxl_swdev {
> +	struct device dev;
> +	struct cdev cdev;
> +	struct cxl_dev_state *cxlds;
> +	int id;
> +};
> +
> +struct cxl_swdev *cxl_swdev_alloc(struct cxl_dev_state *cxlds);
> +void cxl_swdev_shutdown(struct cxl_swdev *cxlswd);
> +#endif
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index faeb5d9d7a7a..5f79742fb266 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c

Yeah, these changes want to be in their own cxl_switch.ko module.

> @@ -12,6 +12,7 @@
>  #include <linux/io.h>
>  #include "cxlmem.h"
>  #include "cxlpci.h"
> +#include "cxlswitch.h"
>  #include "cxl.h"
>  
>  /**
> @@ -520,6 +521,98 @@ static struct pci_driver cxl_pci_driver = {
>  	},
>  };
>  
> +static int cxl_swmbcci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct cxl_dev_state *cxlds;
> +	struct cxl_register_map map;
> +	struct cxl_swdev *cxlswd;
> +	int rc;
> +
> +	rc = pcim_enable_device(pdev);
> +	if (rc)
> +		return rc;
> +
> +	cxlds = cxl_dev_state_create(&pdev->dev);
> +	if (IS_ERR(cxlds))
> +		return PTR_ERR(cxlds);
> +
> +	rc = cxl_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
> +	if (rc)
> +		return rc;
> +
> +	rc = cxl_map_regs(cxlds, &map);
> +	if (rc)
> +		return rc;
> +
> +	rc = cxl_pci_setup_mailbox(cxlds);
> +	if (rc)
> +		return rc;
> +
> +	cxlswd = cxl_swdev_alloc(cxlds);
> +	if (IS_ERR(cxlswd))
> +		return PTR_ERR(cxlswd);
> +
> +	pci_set_drvdata(pdev, cxlswd);
> +
> +	rc = cxl_enumerate_cmds(cxlds);
> +	if (rc)
> +		goto error_put_device;
> +
> +	rc = cdev_device_add(&cxlswd->cdev, &cxlswd->dev);
> +	if (rc)
> +		goto error_put_device;
> +
> +	return 0;
> +
> +error_put_device:
> +	cxl_swdev_shutdown(cxlswd);
> +	put_device(&cxlswd->dev);
> +	return rc;
> +}
> +
> +static void cxl_swbmcci_remove(struct pci_dev *pdev)
> +{
> +	struct cxl_swdev *cxlswd = pci_get_drvdata(pdev);
> +	struct device *dev = &cxlswd->dev;
> +
> +	cxl_swdev_shutdown(cxlswd);
> +	cdev_device_del(&cxlswd->cdev, dev);
> +	put_device(&cxlswd->dev);
> +}
> +
> +static const struct pci_device_id cxl_swmbcci_pci_tbl[] = {
> +	{ PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_CXL_SWITCH_CCI, ~0) },
> +	{}
> +};
> +
> +static struct pci_driver cxl_swmbcci_driver = {
> +	.name = "cxl_swmbcci",
> +	.id_table = cxl_swmbcci_pci_tbl,
> +	.probe = cxl_swmbcci_probe,
> +	.remove = cxl_swbmcci_remove,
> +};
> +
> +static int __init cxl_pci_init(void)
> +{
> +	int rc;
> +
> +	rc = pci_register_driver(&cxl_pci_driver);
> +	if (rc)
> +		return rc;
> +
> +	rc = pci_register_driver(&cxl_swmbcci_driver);
> +	if (rc) {
> +		pci_unregister_driver(&cxl_pci_driver);
> +		return rc;
> +	}
> +	return 0;
> +}
> +module_init(cxl_pci_init);
> +static void __exit cxl_pci_exit(void)
> +{
> +	pci_unregister_driver(&cxl_swmbcci_driver);
> +	pci_unregister_driver(&cxl_pci_driver);
> +}
> +module_exit(cxl_pci_exit);
>  MODULE_LICENSE("GPL v2");
> -module_pci_driver(cxl_pci_driver);
>  MODULE_IMPORT_NS(CXL);
> diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
> index c71021a2a9ed..ea03a289e56f 100644
> --- a/include/uapi/linux/cxl_mem.h
> +++ b/include/uapi/linux/cxl_mem.h
> @@ -41,6 +41,9 @@
>  	___C(GET_SCAN_MEDIA_CAPS, "Get Scan Media Capabilities"),         \
>  	___C(SCAN_MEDIA, "Scan Media"),                                   \
>  	___C(GET_SCAN_MEDIA, "Get Scan Media Results"),                   \
> +	___C(INFO_STAT_IDENTIFY, "Get Information"),			  \
> +	___C(GET_BG_CMD_STATUS, "Background Command Status"),             \
> +	___C(IDENTIFY_SWITCH_DEVICE, "Identify Switch Device"),           \
>  	___C(MAX, "invalid / last command")
>  
>  #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
> -- 
> 2.37.2
> 

Re: [RFC PATCH v2 1/4] cxl/mbox: Use local cxl_device_state variable

[Ira Weiny]

Jonathan Cameron wrote:
> Given we have cxlds as a local variable, use that rather than
> getting it again from cxlmd.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Not sure if this ever got sent for real inclusion but it does not look
like it landed.  So FWIW:

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

> ---
>  drivers/cxl/core/mbox.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 0c90f13870a4..dd9e0aac37ae 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -539,7 +539,7 @@ int cxl_send_cmd(struct cxl_memdev *cxlmd, struct cxl_send_command __user *s)
>  	if (copy_from_user(&send, s, sizeof(send)))
>  		return -EFAULT;
>  
> -	rc = cxl_validate_cmd_from_user(&mbox_cmd, cxlmd->cxlds, &send);
> +	rc = cxl_validate_cmd_from_user(&mbox_cmd, cxlds, &send);
>  	if (rc)
>  		return rc;
>  
> -- 
> 2.37.2
> 

Re: [RFC PATCH v2 4/4] cxl/pci: Add support for stand alone CXL Switch mailbox CCI

[Jonathan Cameron]

On Mon, 12 Dec 2022 17:13:54 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> Jonathan Cameron wrote:
> > CXL 3.0 defines a mailbox PCI function independent of any other CXL
> > components. The intent is that instances of this mailbox will be found
> > as additional PCI functions of upstream CXL switch ports.
> > 
> > RFC: Including this directly in cxl/pci.c as a second pci_driver, is a
> > bit hacky.  The alternative is to factor out all the common
> > infrastructure to a library module shared by the Type 3 PCI driver
> > and the Switch CCI driver.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > ---
> > V2: Thanks to Viacheslav Dubeyko for review.
> >  - Drop double free of IDA in error path.
> >  - Switch to cxl_send_cmd()
> >  - Hex constant for size.
> >  - Shuffle code to put the release down near alloc.
> >  - Correctly handle shutdown in unregister path by setting the
> >    cxlswd->cxlds = NULL.
> >  - Use new define in pci_ids.h instead of opencoding the class code.
> > 
> > Options to consider:
> > 1 - Factor out all the shared code and have a separate module for
> >     switch CCI driver. Messy!
> > 2 - Is sharing the allow lists between type 3 devices and switch CCI
> >     an issue? Not a whole lot of overlap...
> > ---
> >  drivers/cxl/core/Makefile     |   1 +
> >  drivers/cxl/core/core.h       |   1 +
> >  drivers/cxl/core/mbox.c       |   5 ++
> >  drivers/cxl/core/port.c       |   4 +
> >  drivers/cxl/core/switch-cci.c | 149 ++++++++++++++++++++++++++++++++++  
> 
> Warning, incoming bikeshed comment: How about dropping the CCI term?
> It is really only used in the specification to indicate the class of
> transports that can convey CXL commands, but as far as Linux terminology
> it's just a mailbox. As for the filename, perhaps switchdev.c?
> 
I'd forgotten to actually incorporate this in previous versions.

There are far too many mailboxes kicking around, but sure switchdev.c sounds
safe enough and we are distant enough from other uses of that term in the
kernel that it shouldn't be a problem.

I have kept one CCI reference in the module description mostly so there
is something for grep to hit if anyone is searching for the spec term.

> 
> >  drivers/cxl/cxlmem.h          |   3 +
> >  drivers/cxl/cxlpci.h          |   3 +
> >  drivers/cxl/cxlswitch.h       |  18 ++++  
> 
> The only reason the "cxl" prefix is there for cxlmem.h and cxlpci.h is
> to not collide with "mem.h" and "pci.h" for usermode-linux builds and
> the like that confuse the core that includes those headers by include
> search path. So this one can just be switch.h unless and until a build
> robot says otherwise.
Done.

> 
> >  drivers/cxl/pci.c             |  95 +++++++++++++++++++++-
> >  include/uapi/linux/cxl_mem.h  |   3 +
> >  10 files changed, 281 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> > index 79c7257f4107..18275e153437 100644
> > --- a/drivers/cxl/core/Makefile
> > +++ b/drivers/cxl/core/Makefile
> > @@ -10,4 +10,5 @@ cxl_core-y += memdev.o
> >  cxl_core-y += mbox.o
> >  cxl_core-y += pci.o
> >  cxl_core-y += hdm.o
> > +cxl_core-y += switch-cci.o
> >  cxl_core-$(CONFIG_CXL_REGION) += region.o
> > diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> > index 0835942bcea6..f6a35e7f980c 100644
> > --- a/drivers/cxl/core/core.h
> > +++ b/drivers/cxl/core/core.h
> > @@ -73,5 +73,6 @@ static inline struct cxl_ep *cxl_ep_load(struct cxl_port *port,
> >  int cxl_memdev_init(void);
> >  void cxl_memdev_exit(void);
> >  void cxl_mbox_init(void);
> > +int cxl_switch_cci_init(void);
> >  
> >  #endif /* __CXL_CORE_H__ */
> > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> > index 3823d450fdd2..76987f2e30e2 100644
> > --- a/drivers/cxl/core/mbox.c
> > +++ b/drivers/cxl/core/mbox.c
> > @@ -5,6 +5,7 @@
> >  #include <linux/debugfs.h>
> >  #include <linux/mutex.h>
> >  #include <cxlmem.h>
> > +#include <cxlpci.h>
> >  #include <cxl.h>
> >  
> >  #include "core.h"
> > @@ -43,6 +44,8 @@ static bool cxl_raw_allow_all;
> >   * 0, and the user passed in 1, it is an error.
> >   */
> >  static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
> > +	CXL_CMD(INFO_STAT_IDENTIFY, 0, 0x12, 0),  
> 
> Ah, guess this answers the earlier question about raw only or not.
> However if there is a non-zero possibility of this being used in
> production then it likely does want to be formalized.
> 
> > +	CXL_CMD(GET_BG_CMD_STATUS, 0, 8, 0),  
> 
> This is only for "Bind vPPB", right? In that case the result of the Bind
> comes via an event, correct? Perhaps this command does not need to be
> exposed. At least there is no use case to expose it even for endpoint
> background commands since userspace has no mechanism to reliably
> associate to which command that status refers.
Ok. Dropped.

> 
> >  	CXL_CMD(IDENTIFY, 0, 0x43, CXL_CMD_FLAG_FORCE_ENABLE),
> >  #ifdef CONFIG_CXL_MEM_RAW_COMMANDS
> >  	CXL_CMD(RAW, CXL_VARIABLE_PAYLOAD, CXL_VARIABLE_PAYLOAD, 0),
> > @@ -65,6 +68,7 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
> >  	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
> >  	CXL_CMD(SCAN_MEDIA, 0x11, 0, 0),
> >  	CXL_CMD(GET_SCAN_MEDIA, 0, CXL_VARIABLE_PAYLOAD, 0),
> > +	CXL_CMD(IDENTIFY_SWITCH_DEVICE, 0, 0x49, 0),  
> 
> >  };
> >  
> >  /*
> > @@ -552,6 +556,7 @@ int cxl_send_cmd(struct cxl_dev_state *cxlds, struct device *dev,
> >  
> >  	return 0;
> >  }
> > +EXPORT_SYMBOL_GPL(cxl_send_cmd);  
> 
> Why?
No idea - probably cruft from the way this code evolved. Gone.

> 
> >  
> >  static int cxl_xfer_log(struct cxl_dev_state *cxlds, uuid_t *uuid, u32 size, u8 *out)
> >  {
> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > index bffde862de0b..3b2e93bc4684 100644
> > --- a/drivers/cxl/core/port.c
> > +++ b/drivers/cxl/core/port.c
> > @@ -1859,6 +1859,10 @@ static __init int cxl_core_init(void)
> >  	if (rc)
> >  		return rc;
> >  
> > +	rc = cxl_switch_cci_init();
> > +	if (rc)
> > +		return rc;
> > +
> >  	cxl_bus_wq = alloc_ordered_workqueue("cxl_port", 0);
> >  	if (!cxl_bus_wq) {
> >  		rc = -ENOMEM;
> > diff --git a/drivers/cxl/core/switch-cci.c b/drivers/cxl/core/switch-cci.c
> > new file mode 100644
> > index 000000000000..8c3f2b8a3b53
> > --- /dev/null
> > +++ b/drivers/cxl/core/switch-cci.c


> > +static int cxl_swdev_open(struct inode *inode, struct file *file)
> > +{
> > +	struct cxl_memdev *cxlswd =  
> 
> I do think a common cxl_dev is warranted, but this looks like
> s/cxl_memdev/cxl_swdev/ typo.

Indeed a typo. I'm not convinced a common cxl_dev makes sense. Not much to
it and adds another layer + I think adds unneeded complexity.

> 
> > +		container_of(inode->i_cdev, typeof(*cxlswd), cdev);
> > +
> > +	get_device(&cxlswd->dev);
> > +	file->private_data = cxlswd;
> > +
> > +	return 0;
> > +}
> > +
> > +static int cxl_swdev_release_file(struct inode *inode, struct file *file)
> > +{
> > +	struct cxl_swdev *cxlswd =
> > +		container_of(inode->i_cdev, typeof(*cxlswd), cdev);
> > +
> > +	put_device(&cxlswd->dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct file_operations cxl_swdev_fops = {
> > +	.owner = THIS_MODULE,
> > +	.unlocked_ioctl = cxl_swdev_ioctl,
> > +	.open = cxl_swdev_open,
> > +	.release = cxl_swdev_release_file,
> > +	.compat_ioctl = compat_ptr_ioctl,
> > +	.llseek = noop_llseek,
> > +};
> > +
> > +void cxl_swdev_shutdown(struct cxl_swdev *cxlswd)
> > +{
> > +	down_write(&cxl_swdev_rwsem);
> > +	cxlswd->cxlds = NULL;
> > +	up_write(&cxl_swdev_rwsem);
> > +}
> > +EXPORT_SYMBOL_NS_GPL(cxl_swdev_shutdown, CXL);
> > +
> > +static void cxl_swdev_release(struct device *dev)
> > +{
> > +	struct cxl_swdev *cxlswd = to_cxl_swdev(dev);
> > +
> > +	ida_free(&cxl_swdev_ida, cxlswd->id);
> > +	kfree(cxlswd);
> > +}
> > +
> > +static const struct device_type cxl_swdev_type = {
> > +	.name = "cxl_swdev",
> > +	.release = cxl_swdev_release,
> > +	.devnode = cxl_swdev_devnode,
> > +};
> > +
> > +struct cxl_swdev *cxl_swdev_alloc(struct cxl_dev_state *cxlds)
> > +{
> > +	struct cxl_swdev *cxlswd;
> > +	struct device *dev;
> > +	struct cdev *cdev;
> > +	int rc;
> > +
> > +	cxlswd = kzalloc(sizeof(*cxlswd), GFP_KERNEL);
> > +	if (!cxlswd)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	rc = ida_alloc_range(&cxl_swdev_ida, 0, 10, GFP_KERNEL);
> > +	if (rc < 0) {
> > +		kfree(cxlswd);
> > +		return ERR_PTR(rc);
> > +	}
> > +
> > +	cxlswd->id = rc;
> > +	cxlswd->cxlds = cxlds;
> > +	dev = &cxlswd->dev;
> > +	device_initialize(dev);
> > +	dev->parent = cxlds->dev;
> > +	dev->bus = &cxl_bus_type;
> > +	dev->devt = MKDEV(cxl_sw_major, cxlswd->id);
> > +	dev->type = &cxl_swdev_type;
> > +	device_set_pm_not_required(dev);
> > +	cdev = &cxlswd->cdev;
> > +	cdev_init(cdev, &cxl_swdev_fops);
> > +	rc = dev_set_name(dev, "swcci%d", cxlswd->id);  
> 
> How about just "switch%d"? Likely this will also want some late binding
> linkage to the eventual cxl_port that gets registered for this switch.

So far I'm not seeing much point in that linkage. This functionality may
well be exposed on a USP with no CXL capabilities at all. 

> 
> > +	if (rc) {
> > +		put_device(dev);
> > +		return ERR_PTR(rc);
> > +	}
> > +
> > +	return cxlswd;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(cxl_swdev_alloc, CXL);  
> 
> I think a few exports can be saved if this is changed to alloc+add like
> other cxl core apis.

Sorry, I don't follow. 

> 
> > +
> > +__init int cxl_switch_cci_init(void)
> > +{
> > +	dev_t devt;
> > +	int rc;
> > +
> > +	rc = alloc_chrdev_region(&devt, 0, 10, "cxlsw");
> > +	if (rc)
> > +		return rc;
> > +	cxl_sw_major = MAJOR(devt);
> > +
> > +	return 0;
> > +}