[PATCH 5/5] cxl/pmem: Register 'pmem' / cxl_nvdimm devices

From: Dan Williams
Date: Thu Jun 10 2021 - 18:26:31 EST


While a memX device on /sys/bus/cxl represents a CXL memory expander
control interface, a pmemX device represents the persistent memory
sub-functionality. It bridges the CXL subystem to the libnvdimm nmemX
control interface.

With this skeleton ndctl can now see persistent memory devices on a
"CXL" bus. Later patches add support for translating libnvdimm native
commands to CXL commands.

# ndctl list -BDiu -b CXL
{
"provider":"CXL",
"dev":"ndbus1",
"dimms":[
{
"dev":"nmem1",
"state":"disabled"
},
{
"dev":"nmem0",
"state":"disabled"
}
]
}

Given nvdimm_bus_unregister() removes all devices on an ndbus0 the
cxl_pmem infrastructure needs to arrange ->remove() to be triggered on
cxl_nvdimm devices to keep their enabled state synchronized with the
registration state of their corresponding device on the nvdimm_bus. In
other words, always arrange for cxl_nvdimm_driver.remove() to unregister
nvdimms from an nvdimm_bus ahead of the bus being unregistered.

Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
drivers/cxl/core.c | 86 ++++++++++++++++++++++++++++++++++++++++
drivers/cxl/cxl.h | 13 ++++++
drivers/cxl/mem.h | 2 +
drivers/cxl/pci.c | 23 ++++++++---
drivers/cxl/pmem.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++-----
5 files changed, 217 insertions(+), 18 deletions(-)

diff --git a/drivers/cxl/core.c b/drivers/cxl/core.c
index f0305c9c91c8..6db660249cea 100644
--- a/drivers/cxl/core.c
+++ b/drivers/cxl/core.c
@@ -7,6 +7,7 @@
#include <linux/slab.h>
#include <linux/idr.h>
#include "cxl.h"
+#include "mem.h"

/**
* DOC: cxl core
@@ -731,6 +732,89 @@ struct cxl_nvdimm_bridge *devm_cxl_add_nvdimm_bridge(struct device *host,
}
EXPORT_SYMBOL_GPL(devm_cxl_add_nvdimm_bridge);

+static void cxl_nvdimm_release(struct device *dev)
+{
+ struct cxl_nvdimm *cxl_nvd = to_cxl_nvdimm(dev);
+
+ kfree(cxl_nvd);
+}
+
+static const struct attribute_group *cxl_nvdimm_attribute_groups[] = {
+ &cxl_base_attribute_group,
+ NULL,
+};
+
+static const struct device_type cxl_nvdimm_type = {
+ .name = "cxl_nvdimm",
+ .release = cxl_nvdimm_release,
+ .groups = cxl_nvdimm_attribute_groups,
+};
+
+bool is_cxl_nvdimm(struct device *dev)
+{
+ return dev->type == &cxl_nvdimm_type;
+}
+EXPORT_SYMBOL_GPL(is_cxl_nvdimm);
+
+struct cxl_nvdimm *to_cxl_nvdimm(struct device *dev)
+{
+ if (dev_WARN_ONCE(dev, !is_cxl_nvdimm(dev),
+ "not a cxl_nvdimm device\n"))
+ return NULL;
+ return container_of(dev, struct cxl_nvdimm, dev);
+}
+EXPORT_SYMBOL_GPL(to_cxl_nvdimm);
+
+static struct cxl_nvdimm *cxl_nvdimm_alloc(struct cxl_memdev *cxlmd)
+{
+ struct cxl_nvdimm *cxl_nvd;
+ struct device *dev;
+
+ cxl_nvd = kzalloc(sizeof(*cxl_nvd), GFP_KERNEL);
+ if (!cxl_nvd)
+ return ERR_PTR(-ENOMEM);
+
+ dev = &cxl_nvd->dev;
+ cxl_nvd->cxlmd = cxlmd;
+ device_initialize(dev);
+ device_set_pm_not_required(dev);
+ dev->parent = &cxlmd->dev;
+ dev->bus = &cxl_bus_type;
+ dev->type = &cxl_nvdimm_type;
+
+ return cxl_nvd;
+}
+
+int devm_cxl_add_nvdimm(struct device *host, struct cxl_memdev *cxlmd)
+{
+ struct cxl_nvdimm *cxl_nvd;
+ struct device *dev;
+ int rc;
+
+ cxl_nvd = cxl_nvdimm_alloc(cxlmd);
+ if (IS_ERR(cxl_nvd))
+ return PTR_ERR(cxl_nvd);
+
+ dev = &cxl_nvd->dev;
+ rc = dev_set_name(dev, "pmem%d", cxlmd->id);
+ if (rc)
+ goto err;
+
+ rc = device_add(dev);
+ if (rc)
+ goto err;
+
+ dev_dbg(host, "%s: register %s\n", dev_name(dev->parent),
+ dev_name(dev));
+
+ return devm_add_action_or_reset(host, unregister_dev, dev);
+
+err:
+ put_device(dev);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(devm_cxl_add_nvdimm);
+
/**
* cxl_probe_device_regs() - Detect CXL Device register blocks
* @dev: Host device of the @base mapping
@@ -930,6 +1014,8 @@ static int cxl_device_id(struct device *dev)
{
if (dev->type == &cxl_nvdimm_bridge_type)
return CXL_DEVICE_NVDIMM_BRIDGE;
+ if (dev->type == &cxl_nvdimm_type)
+ return CXL_DEVICE_NVDIMM;
return 0;
}

diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 47fcb7ad5978..3f9a6f7b05db 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -213,6 +213,13 @@ struct cxl_nvdimm_bridge {
enum cxl_nvdimm_brige_state state;
};

+struct cxl_mem;
+struct cxl_nvdimm {
+ struct device dev;
+ struct cxl_memdev *cxlmd;
+ struct nvdimm *nvdimm;
+};
+
/**
* struct cxl_port - logical collection of upstream port devices and
* downstream port devices to construct a CXL memory
@@ -299,7 +306,8 @@ int __cxl_driver_register(struct cxl_driver *cxl_drv, struct module *owner,
#define cxl_driver_register(x) __cxl_driver_register(x, THIS_MODULE, KBUILD_MODNAME)
void cxl_driver_unregister(struct cxl_driver *cxl_drv);

-#define CXL_DEVICE_NVDIMM_BRIDGE 1
+#define CXL_DEVICE_NVDIMM_BRIDGE 1
+#define CXL_DEVICE_NVDIMM 2

#define MODULE_ALIAS_CXL(type) MODULE_ALIAS("cxl:t" __stringify(type) "*")
#define CXL_MODALIAS_FMT "cxl:t%d"
@@ -307,4 +315,7 @@ void cxl_driver_unregister(struct cxl_driver *cxl_drv);
struct cxl_nvdimm_bridge *to_cxl_nvdimm_bridge(struct device *dev);
struct cxl_nvdimm_bridge *devm_cxl_add_nvdimm_bridge(struct device *host,
struct cxl_port *port);
+struct cxl_nvdimm *to_cxl_nvdimm(struct device *dev);
+bool is_cxl_nvdimm(struct device *dev);
+int devm_cxl_add_nvdimm(struct device *host, struct cxl_memdev *cxlmd);
#endif /* __CXL_H__ */
diff --git a/drivers/cxl/mem.h b/drivers/cxl/mem.h
index 13868ff7cadf..8f02d02b26b4 100644
--- a/drivers/cxl/mem.h
+++ b/drivers/cxl/mem.h
@@ -2,6 +2,8 @@
/* Copyright(c) 2020-2021 Intel Corporation. */
#ifndef __CXL_MEM_H__
#define __CXL_MEM_H__
+#include <linux/cdev.h>
+#include "cxl.h"

/* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */
#define CXLMDEV_STATUS_OFFSET 0x0
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 5a1705b52278..83e81b44c8f5 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1313,7 +1313,8 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm)
return ERR_PTR(rc);
}

-static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
+static struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
+ struct cxl_mem *cxlm)
{
struct cxl_memdev *cxlmd;
struct device *dev;
@@ -1322,7 +1323,7 @@ static int cxl_mem_add_memdev(struct cxl_mem *cxlm)

cxlmd = cxl_memdev_alloc(cxlm);
if (IS_ERR(cxlmd))
- return PTR_ERR(cxlmd);
+ return cxlmd;

dev = &cxlmd->dev;
rc = dev_set_name(dev, "mem%d", cxlmd->id);
@@ -1340,8 +1341,10 @@ static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
if (rc)
goto err;

- return devm_add_action_or_reset(dev->parent, cxl_memdev_unregister,
- cxlmd);
+ rc = devm_add_action_or_reset(host, cxl_memdev_unregister, cxlmd);
+ if (rc)
+ return ERR_PTR(rc);
+ return cxlmd;

err:
/*
@@ -1350,7 +1353,7 @@ static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
*/
cxl_memdev_shutdown(cxlmd);
put_device(dev);
- return rc;
+ return ERR_PTR(rc);
}

static int cxl_xfer_log(struct cxl_mem *cxlm, uuid_t *uuid, u32 size, u8 *out)
@@ -1561,6 +1564,7 @@ static int cxl_mem_identify(struct cxl_mem *cxlm)

static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
+ struct cxl_memdev *cxlmd;
struct cxl_mem *cxlm;
int rc;

@@ -1588,7 +1592,14 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (rc)
return rc;

- return cxl_mem_add_memdev(cxlm);
+ cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlm);
+ if (IS_ERR(cxlmd))
+ return PTR_ERR(cxlmd);
+
+ if (range_len(&cxlm->pmem_range) && IS_ENABLED(CONFIG_CXL_PMEM))
+ rc = devm_cxl_add_nvdimm(&pdev->dev, cxlmd);
+
+ return rc;
}

static const struct pci_device_id cxl_mem_pci_tbl[] = {
diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
index 0067bd734559..1ed19e213157 100644
--- a/drivers/cxl/pmem.c
+++ b/drivers/cxl/pmem.c
@@ -3,7 +3,10 @@
#include <linux/libnvdimm.h>
#include <linux/device.h>
#include <linux/module.h>
+#include <linux/ndctl.h>
+#include <linux/async.h>
#include <linux/slab.h>
+#include "mem.h"
#include "cxl.h"

/*
@@ -13,6 +16,64 @@
*/
static struct workqueue_struct *cxl_pmem_wq;

+static void unregister_nvdimm(void *nvdimm)
+{
+ nvdimm_delete(nvdimm);
+}
+
+static int match_nvdimm_bridge(struct device *dev, const void *data)
+{
+ return strcmp(dev_name(dev), "nvdimm-bridge") == 0;
+}
+
+static struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(void)
+{
+ struct device *dev;
+
+ dev = bus_find_device(&cxl_bus_type, NULL, NULL, match_nvdimm_bridge);
+ if (!dev)
+ return NULL;
+ return to_cxl_nvdimm_bridge(dev);
+}
+
+static int cxl_nvdimm_probe(struct device *dev)
+{
+ struct cxl_nvdimm *cxl_nvd = to_cxl_nvdimm(dev);
+ struct cxl_nvdimm_bridge *cxl_nvb;
+ unsigned long flags = 0;
+ struct nvdimm *nvdimm;
+ int rc = -ENXIO;
+
+ cxl_nvb = cxl_find_nvdimm_bridge();
+ if (!cxl_nvb)
+ return -ENXIO;
+
+ device_lock(&cxl_nvb->dev);
+ if (!cxl_nvb->nvdimm_bus)
+ goto out;
+
+ set_bit(NDD_LABELING, &flags);
+ nvdimm = nvdimm_create(cxl_nvb->nvdimm_bus, cxl_nvd, NULL, flags, 0, 0,
+ NULL);
+ if (!nvdimm)
+ goto out;
+
+ dev_set_drvdata(dev, nvdimm);
+
+ rc = devm_add_action_or_reset(dev, unregister_nvdimm, nvdimm);
+out:
+ device_unlock(&cxl_nvb->dev);
+ put_device(&cxl_nvb->dev);
+
+ return rc;
+}
+
+static struct cxl_driver cxl_nvdimm_driver = {
+ .name = "cxl_nvdimm",
+ .probe = cxl_nvdimm_probe,
+ .id = CXL_DEVICE_NVDIMM,
+};
+
static int cxl_pmem_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm, unsigned int cmd, void *buf,
unsigned int buf_len, int *cmd_rc)
@@ -28,19 +89,31 @@ static void online_nvdimm_bus(struct cxl_nvdimm_bridge *cxl_nvb)
nvdimm_bus_register(&cxl_nvb->dev, &cxl_nvb->nd_desc);
}

-static void offline_nvdimm_bus(struct cxl_nvdimm_bridge *cxl_nvb)
+static int cxl_nvdimm_release_driver(struct device *dev, void *data)
{
- if (!cxl_nvb->nvdimm_bus)
- return;
- nvdimm_bus_unregister(cxl_nvb->nvdimm_bus);
- cxl_nvb->nvdimm_bus = NULL;
+ if (!is_cxl_nvdimm(dev))
+ return 0;
+ device_release_driver(dev);
+ return 0;
+}
+
+static void offline_nvdimm_bus(struct nvdimm_bus *nvdimm_bus)
+{
+ /*
+ * Set the state of cxl_nvdimm devices to unbound / idle before
+ * nvdimm_bus_unregister() rips the nvdimm objects out from
+ * underneath them.
+ */
+ bus_for_each_dev(&cxl_bus_type, NULL, NULL, cxl_nvdimm_release_driver);
+ nvdimm_bus_unregister(nvdimm_bus);
}

static void cxl_nvb_update_state(struct work_struct *work)
{
struct cxl_nvdimm_bridge *cxl_nvb =
container_of(work, typeof(*cxl_nvb), state_work);
- bool release = false;
+ struct nvdimm_bus *nvdimm_bus = NULL;
+ bool release = false, rescan = false;

device_lock(&cxl_nvb->dev);
switch (cxl_nvb->state) {
@@ -50,11 +123,13 @@ static void cxl_nvb_update_state(struct work_struct *work)
dev_err(&cxl_nvb->dev,
"failed to establish nvdimm bus\n");
release = true;
- }
+ } else
+ rescan = true;
break;
case CXL_NVB_OFFLINE:
case CXL_NVB_DEAD:
- offline_nvdimm_bus(cxl_nvb);
+ nvdimm_bus = cxl_nvb->nvdimm_bus;
+ cxl_nvb->nvdimm_bus = NULL;
break;
default:
break;
@@ -63,6 +138,13 @@ static void cxl_nvb_update_state(struct work_struct *work)

if (release)
device_release_driver(&cxl_nvb->dev);
+ if (rescan) {
+ int rc = bus_rescan_devices(&cxl_bus_type);
+
+ dev_dbg(&cxl_nvb->dev, "rescan: %d\n", rc);
+ }
+ if (nvdimm_bus)
+ offline_nvdimm_bus(nvdimm_bus);

put_device(&cxl_nvb->dev);
}
@@ -113,23 +195,29 @@ static __init int cxl_pmem_init(void)
int rc;

cxl_pmem_wq = alloc_ordered_workqueue("cxl_pmem", 0);
-
if (!cxl_pmem_wq)
return -ENXIO;

rc = cxl_driver_register(&cxl_nvdimm_bridge_driver);
if (rc)
- goto err;
+ goto err_bridge;
+
+ rc = cxl_driver_register(&cxl_nvdimm_driver);
+ if (rc)
+ goto err_nvdimm;

return 0;

-err:
+err_nvdimm:
+ cxl_driver_unregister(&cxl_nvdimm_bridge_driver);
+err_bridge:
destroy_workqueue(cxl_pmem_wq);
return rc;
}

static __exit void cxl_pmem_exit(void)
{
+ cxl_driver_unregister(&cxl_nvdimm_driver);
cxl_driver_unregister(&cxl_nvdimm_bridge_driver);
destroy_workqueue(cxl_pmem_wq);
}
@@ -139,3 +227,4 @@ module_init(cxl_pmem_init);
module_exit(cxl_pmem_exit);
MODULE_IMPORT_NS(CXL);
MODULE_ALIAS_CXL(CXL_DEVICE_NVDIMM_BRIDGE);
+MODULE_ALIAS_CXL(CXL_DEVICE_NVDIMM);