patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/15] CXL Region driver
@ 2022-01-12 23:47 Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 01/15] cxl/core: Rename find_cxl_port Ben Widawsky
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

Major changes since v1:
- bug fixes in certain calculations for region programming
- bug fix in for_each_cxl_decoder_target
- clarify ENIW and IG from ways and granularity
- wait_for_commit bug fix
- use devm management for region removal
- remove trace points
- add basic libnvdimm connection

Original commit message follows with minor updates for correctness.

---

This patch series introduces the CXL region driver as well as associated APIs in
CXL core. The region driver enables the creation of "regions" which is a concept
defined by the CXL 2.0 specification [1]. Region verification and programming
state are owned by the cxl_region driver (implemented in the cxl_region module).
It relies on cxl_mem to determine if devices are CXL routed, and cxl_port to
actually handle the programming of the HDM decoders. Much of the region driver
is an implementation of algorithms described in the CXL Type 3 Memory Device
Software Guide [2].

The region driver will be responsible for configuring regions found on
persistent capacities in the Label Storage Area (LSA), it will also enumerate
regions configured by BIOS, usually volatile capacities, and will allow for
dynamic region creation (which can then be stored in the LSA). It is the primary
consumer of the CXL Port [3] and CXL Mem drivers introduced previously [4]. Dan
has reworked those drivers which is a requirement for this branch. A cached copy
is included in the gitlab for this project [5]. Those patches will be posted
shortly.

The patches for the region driver could be squashed. They're broken out to aid
review and because that's the order they were implemented in. My preference is
to keep those as they are.

Some things are still missing and will be worked on while these are reviewed (in
priority order):
1. Volatile regions/LSA regions (Have a plan)
2. Switch ports (Have a plan)
3. Decoder programming restrictions (No plan). The one know restriction I've
   missed is to disallow programming HDM decoders that aren't in incremental
   system physical address ranges.
4. CXL region teardown -> nd_region teardown
5. Stress testing

Here is an example of output when programming a x2 interleave region

# ./cxlctl create-region -n -a -s $((256<<20)) /sys/bus/cxl/devices/decoder0.0
[   57.564475][  T654] cxl_core:cxl_add_region:478: cxl region0.0:0: Added region0.0:0 to decoder0.0
[   57.608949][  T655] cxl_region:allocate_address_space:170: cxl_region region0.0:0: resource [mem 0x4c00000000-0x4c1fffffff]
[   57.610056][  T655] cxl_core:cxl_commit_decoder:394: cxl_port port1: decoder1.0
[   57.610056][  T655]  Base 0x0000004c00000000
[   57.610056][  T655]  Size 512M
[   57.610056][  T655]  IG 0 (256b)
[   57.610056][  T655]  ENIW 1 (x2)
[   57.610056][  T655]  TargetList: 0 1 -1 -1 -1 -1 -1 -1
[   57.613584][  T655] cxl_core:cxl_commit_decoder:394: cxl_port endpoint2: decoder2.0
[   57.613584][  T655]  Base 0x0000004c00000000
[   57.613584][  T655]  Size 512M
[   57.613584][  T655]  IG 0 (256b)
[   57.613584][  T655]  ENIW 1 (x2)
[   57.613584][  T655]  TargetList: -1 -1 -1 -1 -1 -1 -1 -1
[   57.617051][  T655] cxl_core:cxl_commit_decoder:394: cxl_port endpoint3: decoder3.0
[   57.617051][  T655]  Base 0x0000004c00000000
[   57.617051][  T655]  Size 512M
[   57.617051][  T655]  IG 0 (256b)
[   57.617051][  T655]  ENIW 1 (x2)
[   57.617051][  T655]  TargetList: -1 -1 -1 -1 -1 -1 -1 -1
[   57.619433][  T655] cxl_region region0.0:0: Bound
[   57.621435][  T655] cxl_core:cxl_bus_probe:1384: cxl_region region0.0:0: probe: 0

If you're wondering how I tested this, I've baked it into my cxlctl app [6] and
lib [7]. Eventually this will get absorbed by ndctl/cxl-cli/libcxl.


Branch can be found at gitlab [8].

---

[1]: https://www.computeexpresslink.org/download-the-specification
[2]: https://cdrdv2.intel.com/v1/dl/getContent/643805?wapkw=CXL%20memory%20device%20sw%20guide
[3]: https://lore.kernel.org/linux-cxl/20211022183709.1199701-9-ben.widawsky@intel.com/
[4]: https://lore.kernel.org/linux-cxl/20211022183709.1199701-17-ben.widawsky@intel.com/
[5]: https://gitlab.com/bwidawsk/linux/-/commit/126793e22427f7975a8f2fca373764be78012e88
[6]: https://gitlab.com/bwidawsk-cxl/cxlctl
[7]: https://gitlab.com/bwidawsk-cxl/cxl_rs
[8]: https://gitlab.com/bwidawsk/linux/-/tree/cxl_region-v2

Ben Widawsky (15):
  cxl/core: Rename find_cxl_port
  cxl/core: Track port depth
  cxl/region: Add region creation ABI
  cxl/region: Introduce concept of region configuration
  cxl/mem: Cache port created by the mem dev
  cxl/region: Introduce a cxl_region driver
  cxl/acpi: Handle address space allocation
  cxl/region: Address space allocation
  cxl/region: Implement XHB verification
  cxl/region: HB port config verification
  cxl/region: Add infrastructure for decoder programming
  cxl/region: Collect host bridge decoders
  cxl: Program decoders for regions
  cxl/pmem: Convert nvdimm bridge API to use memdev
  cxl/region: Create an nd_region

 .clang-format                                 |   3 +
 Documentation/ABI/testing/sysfs-bus-cxl       |  63 ++
 .../driver-api/cxl/memory-devices.rst         |  14 +
 drivers/cxl/Makefile                          |   2 +
 drivers/cxl/acpi.c                            |  30 +
 drivers/cxl/core/Makefile                     |   1 +
 drivers/cxl/core/core.h                       |   4 +
 drivers/cxl/core/hdm.c                        | 199 +++++
 drivers/cxl/core/pmem.c                       |  19 +-
 drivers/cxl/core/port.c                       | 132 ++-
 drivers/cxl/core/region.c                     | 525 ++++++++++++
 drivers/cxl/cxl.h                             |  48 +-
 drivers/cxl/cxlmem.h                          |   9 +
 drivers/cxl/mem.c                             |  16 +-
 drivers/cxl/pmem.c                            |   2 +-
 drivers/cxl/port.c                            |  42 +-
 drivers/cxl/region.c                          | 769 ++++++++++++++++++
 drivers/cxl/region.h                          |  47 ++
 tools/testing/cxl/Kbuild                      |   1 +
 19 files changed, 1903 insertions(+), 23 deletions(-)
 create mode 100644 drivers/cxl/core/region.c
 create mode 100644 drivers/cxl/region.c
 create mode 100644 drivers/cxl/region.h

-- 
2.34.1


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

* [PATCH v2 01/15] cxl/core: Rename find_cxl_port
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 02/15] cxl/core: Track port depth Ben Widawsky
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

Needed for other things.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 drivers/cxl/core/port.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 29b0722dc6eb..5a1ffadd5d0d 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -666,7 +666,7 @@ static int match_port_by_dport(struct device *dev, const void *data)
 	return cxl_find_dport_by_dev(port, data) != NULL;
 }
 
-static struct cxl_port *find_cxl_port(struct device *dport_dev)
+static struct cxl_port *dport_find_cxl_port(struct device *dport_dev)
 {
 	struct device *port_dev;
 
@@ -699,7 +699,7 @@ struct cxl_port *find_cxl_root(struct cxl_memdev *cxlmd)
 		if (!dport_dev)
 			break;
 
-		port = find_cxl_port(dport_dev);
+		port = dport_find_cxl_port(dport_dev);
 		if (!port)
 			continue;
 
@@ -728,7 +728,7 @@ static void cxl_remove_ep(void *data)
 		if (!dport_dev)
 			break;
 
-		port = find_cxl_port(dport_dev);
+		port = dport_find_cxl_port(dport_dev);
 		if (!port || is_cxl_root(port))
 			continue;
 
@@ -787,7 +787,7 @@ static int add_port_register_ep(struct cxl_memdev *cxlmd,
 	resource_size_t component_reg_phys;
 	int rc;
 
-	parent_port = find_cxl_port(grandparent(dport_dev));
+	parent_port = dport_find_cxl_port(grandparent(dport_dev));
 	if (!parent_port) {
 		/*
 		 * The root CXL port is added by the CXL platform driver, fail
@@ -811,7 +811,7 @@ static int add_port_register_ep(struct cxl_memdev *cxlmd,
 		goto out;
 	}
 
-	port = find_cxl_port(dport_dev);
+	port = dport_find_cxl_port(dport_dev);
 	if (!port) {
 		component_reg_phys = find_component_registers(uport_dev);
 		port = devm_cxl_add_port(&parent_port->dev, uport_dev,
@@ -876,7 +876,7 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
 			break;
 		}
 
-		port = find_cxl_port(dport_dev);
+		port = dport_find_cxl_port(dport_dev);
 		if (port) {
 			dev_dbg(&cxlmd->dev,
 				"found already registered port %s:%s\n",
@@ -922,7 +922,7 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_ports, CXL);
 
 struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd)
 {
-	return find_cxl_port(grandparent(&cxlmd->dev));
+	return dport_find_cxl_port(grandparent(&cxlmd->dev));
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mem_find_port, CXL);
 
-- 
2.34.1


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

* [PATCH v2 02/15] cxl/core: Track port depth
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 01/15] cxl/core: Rename find_cxl_port Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 03/15] cxl/region: Add region creation ABI Ben Widawsky
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 drivers/cxl/core/port.c | 7 ++++++-
 drivers/cxl/cxl.h       | 2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 5a1ffadd5d0d..ecab7cfa88f0 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -436,13 +436,18 @@ struct cxl_port *devm_cxl_add_port(struct device *host, struct device *uport,
 {
 	struct cxl_port *port;
 	struct device *dev;
-	int rc;
+	int rc, depth = parent_port ? parent_port->depth + 1 : 0;
 
 	port = cxl_port_alloc(uport, component_reg_phys, parent_port);
 	if (IS_ERR(port))
 		return port;
 
+	if (dev_WARN_ONCE(&port->dev, parent_port && !depth,
+			  "Invalid parent port depth\n"))
+		return ERR_PTR(-ENODEV);
+
 	port->host = host;
+	port->depth = depth;
 	dev = &port->dev;
 	if (is_cxl_memdev(uport))
 		rc = dev_set_name(dev, "endpoint%d", port->id);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 276b93316e7f..6eeb82711443 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -272,6 +272,7 @@ struct cxl_walk_context {
  * @decoder_ida: allocator for decoder ids
  * @component_reg_phys: component register capability base address (optional)
  * @dead: last ep has been removed, force port re-creation
+ * @depth: How deep this port is relative to the root. depth 0 is the root.
  */
 struct cxl_port {
 	struct device dev;
@@ -283,6 +284,7 @@ struct cxl_port {
 	struct ida decoder_ida;
 	resource_size_t component_reg_phys;
 	bool dead;
+	unsigned int depth;
 };
 
 /**
-- 
2.34.1


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

* [PATCH v2 03/15] cxl/region: Add region creation ABI
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 01/15] cxl/core: Rename find_cxl_port Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 02/15] cxl/core: Track port depth Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 04/15] cxl/region: Introduce concept of region configuration Ben Widawsky
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

Regions are created as a child of the decoder that encompasses an
address space with constraints. Regions have a number of attributes that
must be configured before the region can be activated.

The ABI is not meant to be secure, but is meant to avoid accidental
races. As a result, a buggy process may create a region by name that was
allocated by a different process. However, multiple processes which are
trying not to race with each other shouldn't need special
synchronization to do so.

// Allocate a new region name
region=$(cat /sys/bus/cxl/devices/decoder0.0/create_region)

// Create a new region by name
echo $region > /sys/bus/cxl/devices/decoder0.0/create_region

// Region now exists in sysfs
stat -t /sys/bus/cxl/devices/decoder0.0/$region

// Delete the region, and name
echo $region > /sys/bus/cxl/devices/decoder0.0/delete_region

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-cxl       |  23 ++
 .../driver-api/cxl/memory-devices.rst         |  11 +
 drivers/cxl/core/Makefile                     |   1 +
 drivers/cxl/core/core.h                       |   3 +
 drivers/cxl/core/port.c                       |  16 ++
 drivers/cxl/core/region.c                     | 209 ++++++++++++++++++
 drivers/cxl/cxl.h                             |   9 +
 drivers/cxl/region.h                          |  38 ++++
 tools/testing/cxl/Kbuild                      |   1 +
 9 files changed, 311 insertions(+)
 create mode 100644 drivers/cxl/core/region.c
 create mode 100644 drivers/cxl/region.h

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 498ae288e143..0fbdd8613654 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -136,3 +136,26 @@ Description:
 		memory (type-3). The 'target_type' attribute indicates the
 		current setting which may dynamically change based on what
 		memory regions are activated in this decode hierarchy.
+
+What:		/sys/bus/cxl/devices/decoderX.Y/create_region
+Date:		November, 2021
+KernelVersion:	v5.17
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		Creates a new CXL region. Writing a value of the form
+		"regionX.Y:Z" will create a new uninitialized region that will
+		be mapped by the CXL decoderX.Y. Reading from this node will
+		return a newly allocated region name. In order to create a
+		region (writing) you must use a value returned from reading the
+		node. Regions must be subsequently configured and bound to a
+		region driver before they can be used.
+
+What:		/sys/bus/cxl/devices/decoderX.Y/delete_region
+Date:		November, 2021
+KernelVersion:	v5.17
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		Deletes the named region. A region must be unbound from the
+		region driver before being deleted. The attributes expects a
+		region in the form "regionX.Y:Z". The region's name, allocated
+		by reading create_region, will also be released.
diff --git a/Documentation/driver-api/cxl/memory-devices.rst b/Documentation/driver-api/cxl/memory-devices.rst
index e101ef02b547..dc756ed23a3a 100644
--- a/Documentation/driver-api/cxl/memory-devices.rst
+++ b/Documentation/driver-api/cxl/memory-devices.rst
@@ -71,6 +71,17 @@ CXL Core
 .. kernel-doc:: drivers/cxl/core/mbox.c
    :doc: cxl mbox
 
+CXL Regions
+-----------
+.. kernel-doc:: drivers/cxl/region.h
+   :identifiers:
+
+.. kernel-doc:: drivers/cxl/core/region.c
+   :doc: cxl core region
+
+.. kernel-doc:: drivers/cxl/core/region.c
+   :identifiers:
+
 External Interfaces
 ===================
 
diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index 6d37cd78b151..39ce8f2f2373 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_CXL_BUS) += cxl_core.o
 ccflags-y += -I$(srctree)/drivers/cxl
 cxl_core-y := port.o
 cxl_core-y += pmem.o
+cxl_core-y += region.o
 cxl_core-y += regs.o
 cxl_core-y += memdev.o
 cxl_core-y += mbox.o
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 19d1f9d8ceba..1d4d1699b479 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -10,6 +10,9 @@ extern const struct device_type cxl_memdev_type;
 
 extern struct attribute_group cxl_base_attribute_group;
 
+extern struct device_attribute dev_attr_create_region;
+extern struct device_attribute dev_attr_delete_region;
+
 struct cxl_send_command;
 struct cxl_mem_query_commands;
 int cxl_query_cmd(struct cxl_memdev *cxlmd,
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index ecab7cfa88f0..ef3840c50e3e 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -196,6 +196,8 @@ static struct attribute_group cxl_decoder_base_attribute_group = {
 };
 
 static struct attribute *cxl_decoder_root_attrs[] = {
+	&dev_attr_create_region.attr,
+	&dev_attr_delete_region.attr,
 	&dev_attr_cap_pmem.attr,
 	&dev_attr_cap_ram.attr,
 	&dev_attr_cap_type2.attr,
@@ -236,11 +238,23 @@ static const struct attribute_group *cxl_decoder_endpoint_attribute_groups[] = {
 	NULL,
 };
 
+static int delete_region(struct device *dev, void *arg)
+{
+	struct cxl_decoder *cxld = to_cxl_decoder(dev->parent);
+
+	return cxl_delete_region(cxld, dev_name(dev));
+}
+
 static void cxl_decoder_release(struct device *dev)
 {
 	struct cxl_decoder *cxld = to_cxl_decoder(dev);
 	struct cxl_port *port = to_cxl_port(dev->parent);
 
+	device_for_each_child(&cxld->dev, cxld, delete_region);
+
+	dev_WARN_ONCE(dev, !ida_is_empty(&cxld->region_ida),
+		      "Lost track of a region");
+
 	ida_free(&port->decoder_ida, cxld->id);
 	kfree(cxld);
 }
@@ -1021,6 +1035,8 @@ static struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port,
 	cxld->target_type = CXL_DECODER_EXPANDER;
 	cxld->platform_res = (struct resource)DEFINE_RES_MEM(0, 0);
 
+	ida_init(&cxld->region_ida);
+
 	return cxld;
 err:
 	kfree(cxld);
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
new file mode 100644
index 000000000000..e3a82f3c118e
--- /dev/null
+++ b/drivers/cxl/core/region.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2021 Intel Corporation. All rights reserved. */
+#include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/idr.h>
+#include <region.h>
+#include <cxl.h>
+#include "core.h"
+
+/**
+ * DOC: cxl core region
+ *
+ * Regions are managed through the Linux device model. Each region instance is a
+ * unique struct device. CXL core provides functionality to create, destroy, and
+ * configure regions. This is all implemented here. Binding a region
+ * (programming the hardware) is handled by a separate region driver.
+ */
+
+static void cxl_region_release(struct device *dev);
+
+static const struct device_type cxl_region_type = {
+	.name = "cxl_region",
+	.release = cxl_region_release,
+};
+
+static ssize_t create_region_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct cxl_port *port = to_cxl_port(dev->parent);
+	struct cxl_decoder *cxld = to_cxl_decoder(dev);
+	int rc;
+
+	if (dev_WARN_ONCE(dev, !is_root_decoder(dev),
+			  "Invalid decoder selected for region.")) {
+		return -ENODEV;
+	}
+
+	rc = ida_alloc(&cxld->region_ida, GFP_KERNEL);
+	if (rc < 0) {
+		dev_err(&cxld->dev, "Couldn't get a new id\n");
+		return rc;
+	}
+
+	return sysfs_emit(buf, "region%d.%d:%d\n", port->id, cxld->id, rc);
+}
+
+static ssize_t create_region_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t len)
+{
+	struct cxl_port *port = to_cxl_port(dev->parent);
+	struct cxl_decoder *cxld = to_cxl_decoder(dev);
+	int decoder_id, port_id, region_id;
+	struct cxl_region *region;
+	ssize_t rc;
+
+	if (sscanf(buf, "region%d.%d:%d", &port_id, &decoder_id, &region_id) != 3)
+		return -EINVAL;
+
+	if (decoder_id != cxld->id)
+		return -EINVAL;
+
+	if (port_id != port->id)
+		return -EINVAL;
+
+	region = cxl_alloc_region(cxld, region_id);
+	if (IS_ERR(region))
+		return PTR_ERR(region);
+
+	rc = cxl_add_region(cxld, region);
+	if (rc) {
+		kfree(region);
+		return rc;
+	}
+
+	return len;
+}
+DEVICE_ATTR_RW(create_region);
+
+static ssize_t delete_region_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t len)
+{
+	struct cxl_decoder *cxld = to_cxl_decoder(dev);
+	int rc;
+
+	rc = cxl_delete_region(cxld, buf);
+	if (rc)
+		return rc;
+
+	return len;
+}
+DEVICE_ATTR_WO(delete_region);
+
+struct cxl_region *to_cxl_region(struct device *dev)
+{
+	if (dev_WARN_ONCE(dev, dev->type != &cxl_region_type,
+			  "not a cxl_region device\n"))
+		return NULL;
+
+	return container_of(dev, struct cxl_region, dev);
+}
+EXPORT_SYMBOL_GPL(to_cxl_region);
+
+static void cxl_region_release(struct device *dev)
+{
+	struct cxl_decoder *cxld = to_cxl_decoder(dev->parent);
+	struct cxl_region *region = to_cxl_region(dev);
+
+	ida_free(&cxld->region_ida, region->id);
+	kfree(region);
+}
+
+struct cxl_region *cxl_alloc_region(struct cxl_decoder *cxld, int id)
+{
+	struct cxl_region *region;
+
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return ERR_PTR(-ENOMEM);
+
+	region->id = id;
+
+	return region;
+}
+
+/**
+ * cxl_add_region - Adds a region to a decoder
+ * @cxld: Parent decoder.
+ * @region: Region to be added to the decoder.
+ *
+ * This is the second step of region initialization. Regions exist within an
+ * address space which is mapped by a @cxld. That @cxld must be a root decoder,
+ * and it enforces constraints upon the region as it is configured.
+ *
+ * Return: 0 if the region was added to the @cxld, else returns negative error
+ * code. The region will be named "regionX.Y.Z" where X is the port, Y is the
+ * decoder id, and Z is the region number.
+ */
+int cxl_add_region(struct cxl_decoder *cxld, struct cxl_region *region)
+{
+	struct cxl_port *port = to_cxl_port(cxld->dev.parent);
+	struct device *dev = &region->dev;
+	int rc;
+
+	device_initialize(dev);
+	dev->parent = &cxld->dev;
+	device_set_pm_not_required(dev);
+	dev->bus = &cxl_bus_type;
+	dev->type = &cxl_region_type;
+	rc = dev_set_name(dev, "region%d.%d:%d", port->id, cxld->id,
+			  region->id);
+	if (rc)
+		goto err;
+
+	rc = device_add(dev);
+	if (rc)
+		goto err;
+
+	dev_dbg(dev, "Added %s to %s\n", dev_name(dev), dev_name(&cxld->dev));
+
+	return 0;
+
+err:
+	put_device(dev);
+	return rc;
+}
+
+static struct cxl_region *cxl_find_region_by_name(struct cxl_decoder *cxld,
+						  const char *name)
+{
+	struct device *region_dev;
+
+	region_dev = device_find_child_by_name(&cxld->dev, name);
+	if (!region_dev)
+		return ERR_PTR(-ENOENT);
+
+	return to_cxl_region(region_dev);
+}
+
+/**
+ * cxl_delete_region - Deletes a region
+ * @cxld: Parent decoder
+ * @region_name: Named region, ie. regionX.Y:Z
+ */
+int cxl_delete_region(struct cxl_decoder *cxld, const char *region_name)
+{
+	struct cxl_region *region;
+
+	device_lock(&cxld->dev);
+
+	region = cxl_find_region_by_name(cxld, region_name);
+	if (IS_ERR(region)) {
+		device_unlock(&cxld->dev);
+		return PTR_ERR(region);
+	}
+
+	dev_dbg(&cxld->dev, "Requested removal of %s from %s\n",
+		dev_name(&region->dev), dev_name(&cxld->dev));
+
+	device_unregister(&region->dev);
+	device_unlock(&cxld->dev);
+
+	put_device(&region->dev);
+
+	return 0;
+}
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 6eeb82711443..79c5781b6173 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -202,6 +202,7 @@ enum cxl_decoder_type {
  * @interleave_granularity: data stride per dport
  * @target_type: accelerator vs expander (type2 vs type3) selector
  * @flags: memory type capabilities and locking
+ * @region_ida: allocator for region ids.
  * @nr_targets: number of elements in @target
  * @target: active ordered target list in current decoder configuration
  */
@@ -216,6 +217,7 @@ struct cxl_decoder {
 	int interleave_granularity;
 	enum cxl_decoder_type target_type;
 	unsigned long flags;
+	struct ida region_ida;
 	const int nr_targets;
 	struct cxl_dport *target[];
 };
@@ -315,6 +317,13 @@ struct cxl_ep {
 	struct list_head list;
 };
 
+bool is_cxl_region(struct device *dev);
+struct cxl_region *to_cxl_region(struct device *dev);
+struct cxl_region *cxl_alloc_region(struct cxl_decoder *cxld,
+				    int interleave_ways);
+int cxl_add_region(struct cxl_decoder *cxld, struct cxl_region *region);
+int cxl_delete_region(struct cxl_decoder *cxld, const char *region);
+
 static inline bool is_cxl_root(struct cxl_port *port)
 {
 	return port->uport == port->dev.parent;
diff --git a/drivers/cxl/region.h b/drivers/cxl/region.h
new file mode 100644
index 000000000000..3e6e5fb35822
--- /dev/null
+++ b/drivers/cxl/region.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright(c) 2021 Intel Corporation. */
+#ifndef __CXL_REGION_H__
+#define __CXL_REGION_H__
+
+#include <linux/uuid.h>
+
+#include "cxl.h"
+
+/**
+ * struct cxl_region - CXL region
+ * @dev: This region's device.
+ * @id: This regions id. Id is globally unique across all regions.
+ * @list: Node in decoder's region list.
+ * @res: Resource this region carves out of the platform decode range.
+ * @config: HDM decoder program config
+ * @config.size: Size of the region determined from LSA or userspace.
+ * @config.uuid: The UUID for this region.
+ * @config.eniw: Number of interleave ways this region is configured for.
+ * @config.ig: Interleave granularity of region
+ * @config.targets: The memory devices comprising the region.
+ */
+struct cxl_region {
+	struct device dev;
+	int id;
+	struct list_head list;
+	struct resource *res;
+
+	struct {
+		u64 size;
+		uuid_t uuid;
+		int eniw;
+		int ig;
+		struct cxl_memdev *targets[CXL_DECODER_MAX_INTERLEAVE];
+	} config;
+};
+
+#endif
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index 8b20e34090f7..73735f561c89 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -43,6 +43,7 @@ cxl_core-y += $(CXL_CORE_SRC)/memdev.o
 cxl_core-y += $(CXL_CORE_SRC)/mbox.o
 cxl_core-y += $(CXL_CORE_SRC)/pci.o
 cxl_core-y += $(CXL_CORE_SRC)/hdm.o
+cxl_core-y += $(CXL_CORE_SRC)/region.o
 cxl_core-y += config_check.o
 
 obj-m += test/
-- 
2.34.1


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

* [PATCH v2 04/15] cxl/region: Introduce concept of region configuration
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (2 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 03/15] cxl/region: Add region creation ABI Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 05/15] cxl/mem: Cache port created by the mem dev Ben Widawsky
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

The region creation APIs create a vacant region. Configuring the region
works in the same way as similar subsystems such as devdax. Sysfs attrs
will be provided to allow userspace to configure the region.  Finally
once all configuration is complete, userspace may activate the region.

Introduced here are the most basic attributes needed to configure a
region. Details of these attribute are described in the ABI
Documentation.

A example is provided below:

/sys/bus/cxl/devices/region0.0:0
├── interleave_granularity
├── interleave_ways
├── offset
├── size
├── subsystem -> ../../../../../../bus/cxl
├── target0
├── uevent
└── uuid

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-cxl |  40 ++++
 drivers/cxl/core/region.c               | 295 ++++++++++++++++++++++++
 2 files changed, 335 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 0fbdd8613654..1a938ad26621 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -159,3 +159,43 @@ Description:
 		region driver before being deleted. The attributes expects a
 		region in the form "regionX.Y:Z". The region's name, allocated
 		by reading create_region, will also be released.
+
+What:		/sys/bus/cxl/devices/decoderX.Y/regionX.Y:Z/offset
+Date:		November, 2021
+KernelVersion:	v5.17
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) A region resides within an address space that is claimed by
+		a decoder. Region space allocation is handled by the driver, but
+		the offset may be read by userspace tooling in order to
+		determine fragmentation, and available size for new regions.
+
+What:
+/sys/bus/cxl/devices/decoderX.Y/regionX.Y:Z/{interleave,size,uuid,target[0-15]}
+Date:		November, 2021
+KernelVersion:	v5.17
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RW) Configuring regions requires a minimal set of parameters in
+		order for the subsequent bind operation to succeed. The
+		following parameters are defined:
+
+		==	========================================================
+		interleave_granularity Mandatory. Number of consecutive bytes
+			each device in the interleave set will claim. The
+			possible interleave granularity values are determined by
+			the CXL spec and the participating devices.
+		interleave_ways Mandatory. Number of devices participating in the
+			region. Each device will provide 1/interleave of storage
+			for the region.
+		size	Manadatory. Phsyical address space the region will
+			consume.
+		target  Mandatory. Memory devices are the backing storage for a
+			region. There will be N targets based on the number of
+			interleave ways that the top level decoder is configured
+			for. Each target must be set with a memdev device ie.
+			'mem1'. This attribute only becomes available after
+			setting the 'interleave' attribute.
+		uuid	Optional. A unique identifier for the region. If none is
+			selected, the kernel will create one.
+		==	========================================================
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index e3a82f3c118e..26b5ad389cd2 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3,9 +3,12 @@
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/sizes.h>
 #include <linux/slab.h>
+#include <linux/uuid.h>
 #include <linux/idr.h>
 #include <region.h>
+#include <cxlmem.h>
 #include <cxl.h>
 #include "core.h"
 
@@ -18,11 +21,300 @@
  * (programming the hardware) is handled by a separate region driver.
  */
 
+struct cxl_region *to_cxl_region(struct device *dev);
+static const struct attribute_group region_interleave_group;
+
+static bool is_region_active(struct cxl_region *region)
+{
+	/* TODO: Regions can't be activated yet. */
+	return false;
+}
+
+static void remove_target(struct cxl_region *region, int target)
+{
+	struct cxl_memdev *cxlmd;
+
+	cxlmd = region->config.targets[target];
+	if (cxlmd)
+		put_device(&cxlmd->dev);
+	region->config.targets[target] = NULL;
+}
+
+static ssize_t interleave_ways_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+
+	return sysfs_emit(buf, "%d\n", region->config.eniw);
+}
+
+static ssize_t interleave_ways_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t len)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+	int ret, prev_eniw;
+	int val;
+
+	prev_eniw = region->config.eniw;
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return ret;
+	if (ret < 0 || ret > CXL_DECODER_MAX_INTERLEAVE)
+		return -EINVAL;
+
+	region->config.eniw = val;
+
+	ret = sysfs_update_group(&dev->kobj, &region_interleave_group);
+	if (ret < 0)
+		goto err;
+
+	sysfs_notify(&dev->kobj, NULL, "target_interleave");
+
+	while (prev_eniw > region->config.eniw)
+		remove_target(region, --prev_eniw);
+
+	return len;
+
+err:
+	region->config.eniw = prev_eniw;
+	return ret;
+}
+static DEVICE_ATTR_RW(interleave_ways);
+
+static ssize_t interleave_granularity_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+
+	return sysfs_emit(buf, "%d\n", region->config.ig);
+}
+
+static ssize_t interleave_granularity_store(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf, size_t len)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+	int val, ret;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return ret;
+	region->config.ig = val;
+
+	return len;
+}
+static DEVICE_ATTR_RW(interleave_granularity);
+
+static ssize_t offset_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+	struct cxl_decoder *cxld = to_cxl_decoder(dev->parent);
+
+	if (!region->res)
+		return sysfs_emit(buf, "\n");
+
+	return sysfs_emit(buf, "%#llx\n", cxld->platform_res.start - region->res->start);
+}
+static DEVICE_ATTR_RO(offset);
+
+static ssize_t size_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+
+	return sysfs_emit(buf, "%llu\n", region->config.size);
+}
+
+static ssize_t size_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t len)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+	unsigned long long val;
+	ssize_t rc;
+
+	rc = kstrtoull(buf, 0, &val);
+	if (rc)
+		return rc;
+
+	device_lock(&region->dev);
+	if (is_region_active(region))
+		rc = -EBUSY;
+	else
+		region->config.size = val;
+	device_unlock(&region->dev);
+
+	return rc ? rc : len;
+}
+static DEVICE_ATTR_RW(size);
+
+static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+
+	return sysfs_emit(buf, "%pUb\n", &region->config.uuid);
+}
+
+static ssize_t uuid_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t len)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+	ssize_t rc;
+
+	if (len != UUID_STRING_LEN + 1)
+		return -EINVAL;
+
+	device_lock(&region->dev);
+	if (is_region_active(region))
+		rc = -EBUSY;
+	else
+		rc = uuid_parse(buf, &region->config.uuid);
+	device_unlock(&region->dev);
+
+	return rc ? rc : len;
+}
+static DEVICE_ATTR_RW(uuid);
+
+static struct attribute *region_attrs[] = {
+	&dev_attr_interleave_ways.attr,
+	&dev_attr_interleave_granularity.attr,
+	&dev_attr_offset.attr,
+	&dev_attr_size.attr,
+	&dev_attr_uuid.attr,
+	NULL,
+};
+
+static const struct attribute_group region_group = {
+	.attrs = region_attrs,
+};
+
+static size_t show_targetN(struct cxl_region *region, char *buf, int n)
+{
+	int ret;
+
+	device_lock(&region->dev);
+	if (!region->config.targets[n])
+		ret = sysfs_emit(buf, "\n");
+	else
+		ret = sysfs_emit(buf, "%s\n",
+				 dev_name(&region->config.targets[n]->dev));
+	device_unlock(&region->dev);
+
+	return ret;
+}
+
+static size_t set_targetN(struct cxl_region *region, const char *buf, int n,
+			  size_t len)
+{
+	struct device *memdev_dev;
+	struct cxl_memdev *cxlmd;
+
+	device_lock(&region->dev);
+
+	if (len == 1 || region->config.targets[n])
+		remove_target(region, n);
+
+	/* Remove target special case */
+	if (len == 1) {
+		device_unlock(&region->dev);
+		return len;
+	}
+
+	memdev_dev = bus_find_device_by_name(&cxl_bus_type, NULL, buf);
+	if (!memdev_dev)
+		return -ENOENT;
+
+	/* reference to memdev held until target is unset or region goes away */
+
+	cxlmd = to_cxl_memdev(memdev_dev);
+	region->config.targets[n] = cxlmd;
+
+	device_unlock(&region->dev);
+
+	return len;
+}
+
+#define TARGET_ATTR_RW(n)                                                      \
+	static ssize_t target##n##_show(                                       \
+		struct device *dev, struct device_attribute *attr, char *buf)  \
+	{                                                                      \
+		return show_targetN(to_cxl_region(dev), buf, (n));             \
+	}                                                                      \
+	static ssize_t target##n##_store(struct device *dev,                   \
+					 struct device_attribute *attr,        \
+					 const char *buf, size_t len)          \
+	{                                                                      \
+		return set_targetN(to_cxl_region(dev), buf, (n), len);         \
+	}                                                                      \
+	static DEVICE_ATTR_RW(target##n)
+
+TARGET_ATTR_RW(0);
+TARGET_ATTR_RW(1);
+TARGET_ATTR_RW(2);
+TARGET_ATTR_RW(3);
+TARGET_ATTR_RW(4);
+TARGET_ATTR_RW(5);
+TARGET_ATTR_RW(6);
+TARGET_ATTR_RW(7);
+TARGET_ATTR_RW(8);
+TARGET_ATTR_RW(9);
+TARGET_ATTR_RW(10);
+TARGET_ATTR_RW(11);
+TARGET_ATTR_RW(12);
+TARGET_ATTR_RW(13);
+TARGET_ATTR_RW(14);
+TARGET_ATTR_RW(15);
+
+static struct attribute *interleave_attrs[] = {
+	&dev_attr_target0.attr,
+	&dev_attr_target1.attr,
+	&dev_attr_target2.attr,
+	&dev_attr_target3.attr,
+	&dev_attr_target4.attr,
+	&dev_attr_target5.attr,
+	&dev_attr_target6.attr,
+	&dev_attr_target7.attr,
+	&dev_attr_target8.attr,
+	&dev_attr_target9.attr,
+	&dev_attr_target10.attr,
+	&dev_attr_target11.attr,
+	&dev_attr_target12.attr,
+	&dev_attr_target13.attr,
+	&dev_attr_target14.attr,
+	&dev_attr_target15.attr,
+	NULL,
+};
+
+static umode_t visible_targets(struct kobject *kobj, struct attribute *a, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct cxl_region *region = to_cxl_region(dev);
+
+	if (n < region->config.eniw)
+		return a->mode;
+	return 0;
+}
+
+static const struct attribute_group region_interleave_group = {
+	.attrs = interleave_attrs,
+	.is_visible = visible_targets,
+};
+
+static const struct attribute_group *region_groups[] = {
+	&region_group,
+	&region_interleave_group,
+	NULL,
+};
+
 static void cxl_region_release(struct device *dev);
 
 static const struct device_type cxl_region_type = {
 	.name = "cxl_region",
 	.release = cxl_region_release,
+	.groups = region_groups
 };
 
 static ssize_t create_region_show(struct device *dev,
@@ -108,8 +400,11 @@ static void cxl_region_release(struct device *dev)
 {
 	struct cxl_decoder *cxld = to_cxl_decoder(dev->parent);
 	struct cxl_region *region = to_cxl_region(dev);
+	int i;
 
 	ida_free(&cxld->region_ida, region->id);
+	for (i = 0; i < region->config.eniw; i++)
+		remove_target(region, i);
 	kfree(region);
 }
 
-- 
2.34.1


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

* [PATCH v2 05/15] cxl/mem: Cache port created by the mem dev
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (3 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 04/15] cxl/region: Introduce concept of region configuration Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 06/15] cxl/region: Introduce a cxl_region driver Ben Widawsky
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

Since region programming sees all components in the topology as a port,
it's required that endpoints are treated equally. The easiest way to go
from endpoint to port is to simply cache it at creation time.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 drivers/cxl/cxlmem.h |  2 ++
 drivers/cxl/mem.c    | 16 ++++++++++------
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 4ea0686e5f84..38d6129499c8 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -36,12 +36,14 @@
  * @cxlds: The device state backing this device
  * @id: id number of this memdev instance.
  * @component_reg_phys: register base of component registers
+ * @port: The port created by this device
  */
 struct cxl_memdev {
 	struct device dev;
 	struct cdev cdev;
 	struct cxl_dev_state *cxlds;
 	int id;
+	struct cxl_port *port;
 };
 
 static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 9e6e98e5ea06..2ed7554155d2 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -45,8 +45,8 @@ static int wait_for_media(struct cxl_memdev *cxlmd)
 	return 0;
 }
 
-static int create_endpoint(struct cxl_memdev *cxlmd,
-			   struct cxl_port *parent_port)
+static struct cxl_port *create_endpoint(struct cxl_memdev *cxlmd,
+					struct cxl_port *parent_port)
 {
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	struct cxl_port *endpoint;
@@ -54,10 +54,10 @@ static int create_endpoint(struct cxl_memdev *cxlmd,
 	endpoint = devm_cxl_add_port(&parent_port->dev, &cxlmd->dev,
 				     cxlds->component_reg_phys, parent_port);
 	if (IS_ERR(endpoint))
-		return PTR_ERR(endpoint);
+		return endpoint;
 
 	dev_dbg(&cxlmd->dev, "add: %s\n", dev_name(&endpoint->dev));
-	return 0;
+	return endpoint;
 }
 
 /**
@@ -123,7 +123,7 @@ static int cxl_mem_probe(struct device *dev)
 {
 	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
-	struct cxl_port *parent_port;
+	struct cxl_port *parent_port, *ep_port;
 	int rc;
 
 	rc = wait_for_media(cxlmd);
@@ -182,7 +182,11 @@ static int cxl_mem_probe(struct device *dev)
 		goto out;
 	}
 
-	rc = create_endpoint(cxlmd, parent_port);
+	ep_port = create_endpoint(cxlmd, parent_port);
+	if (IS_ERR(ep_port))
+		rc = PTR_ERR(ep_port);
+	else
+		cxlmd->port = ep_port;
 out:
 	device_unlock(&parent_port->dev);
 	put_device(&parent_port->dev);
-- 
2.34.1


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

* [PATCH v2 06/15] cxl/region: Introduce a cxl_region driver
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (4 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 05/15] cxl/mem: Cache port created by the mem dev Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 07/15] cxl/acpi: Handle address space allocation Ben Widawsky
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

The cxl_region driver is responsible for managing the HDM decoder
programming in the CXL topology. Once a region is created it must be
configured and bound to the driver in order to activate it.

The following is a sample of how such controls might work:

region=$(cat /sys/bus/cxl/devices/decoder0.0/create_region)
echo $region > /sys/bus/cxl/devices/decoder0.0/create_region
echo 2 > /sys/bus/cxl/devices/decoder0.0/region0.0:0/interleave
echo $((256<<20)) > /sys/bus/cxl/devices/decoder0.0/region0.0:0/size
echo mem0 > /sys/bus/cxl/devices/decoder0.0/region0.0:0/target0
echo mem1 > /sys/bus/cxl/devices/decoder0.0/region0.0:0/target1
echo region0.0:0 > /sys/bus/cxl/drivers/cxl_region/bind

In order to handle the eventual rise in failure modes of binding a
region, a new trace event is created to help track these failures for
debug and reconfiguration paths in userspace.

---
Changes since v1:
- Updated kdoc
- s/eniw/interleave_ways to reflect lack of encoding
- s/ig/interleave_granularity to reflect lack of encoding

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 .../driver-api/cxl/memory-devices.rst         |   3 +
 drivers/cxl/Makefile                          |   2 +
 drivers/cxl/core/core.h                       |   1 +
 drivers/cxl/core/port.c                       |  21 +-
 drivers/cxl/core/region.c                     |  47 ++-
 drivers/cxl/cxl.h                             |   6 +
 drivers/cxl/region.c                          | 331 ++++++++++++++++++
 drivers/cxl/region.h                          |  12 +-
 8 files changed, 403 insertions(+), 20 deletions(-)
 create mode 100644 drivers/cxl/region.c

diff --git a/Documentation/driver-api/cxl/memory-devices.rst b/Documentation/driver-api/cxl/memory-devices.rst
index dc756ed23a3a..6734939b7136 100644
--- a/Documentation/driver-api/cxl/memory-devices.rst
+++ b/Documentation/driver-api/cxl/memory-devices.rst
@@ -73,6 +73,9 @@ CXL Core
 
 CXL Regions
 -----------
+.. kernel-doc:: drivers/cxl/region.c
+   :doc: cxl region
+
 .. kernel-doc:: drivers/cxl/region.h
    :identifiers:
 
diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
index ce267ef11d93..677a04528b22 100644
--- a/drivers/cxl/Makefile
+++ b/drivers/cxl/Makefile
@@ -5,9 +5,11 @@ obj-$(CONFIG_CXL_MEM) += cxl_mem.o
 obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o
 obj-$(CONFIG_CXL_PMEM) += cxl_pmem.o
 obj-$(CONFIG_CXL_PORT) += cxl_port.o
+obj-$(CONFIG_CXL_MEM) += cxl_region.o
 
 cxl_mem-y := mem.o
 cxl_pci-y := pci.o
 cxl_acpi-y := acpi.o
 cxl_pmem-y := pmem.o
 cxl_port-y := port.o
+cxl_region-y := region.o
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 1d4d1699b479..bd47e1b59f8b 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -7,6 +7,7 @@
 extern const struct device_type cxl_nvdimm_bridge_type;
 extern const struct device_type cxl_nvdimm_type;
 extern const struct device_type cxl_memdev_type;
+extern const struct device_type cxl_region_type;
 
 extern struct attribute_group cxl_base_attribute_group;
 
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index ef3840c50e3e..67f3345d44ef 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -8,6 +8,7 @@
 #include <linux/slab.h>
 #include <linux/idr.h>
 #include <cxlmem.h>
+#include <region.h>
 #include <cxl.h>
 #include <pci.h>
 #include "core.h"
@@ -29,6 +30,8 @@
 static DEFINE_IDA(cxl_port_ida);
 static DEFINE_XARRAY(cxl_root_buses);
 
+static void cxl_decoder_release(struct device *dev);
+
 static bool is_cxl_decoder(struct device *dev);
 
 static int decoder_match(struct device *dev, void *data)
@@ -732,6 +735,7 @@ struct cxl_port *find_cxl_root(struct cxl_memdev *cxlmd)
 	}
 	return NULL;
 }
+EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL);
 
 static void cxl_remove_ep(void *data)
 {
@@ -1276,6 +1280,8 @@ static int cxl_device_id(struct device *dev)
 	}
 	if (dev->type == &cxl_memdev_type)
 		return CXL_DEVICE_MEMORY_EXPANDER;
+	if (dev->type == &cxl_region_type)
+		return CXL_DEVICE_REGION;
 	return 0;
 }
 
@@ -1292,10 +1298,21 @@ static int cxl_bus_match(struct device *dev, struct device_driver *drv)
 
 static int cxl_bus_probe(struct device *dev)
 {
-	int rc;
+	int id = cxl_device_id(dev);
+	int rc = -ENODEV;
+
+	if (id == CXL_DEVICE_REGION) {
+		/* Regions cannot bind until parameters are set */
+		struct cxl_region *region = to_cxl_region(dev);
+
+		if (is_cxl_region_configured(region))
+			rc = to_cxl_drv(dev->driver)->probe(dev);
+	} else {
+		rc = to_cxl_drv(dev->driver)->probe(dev);
+	}
 
-	rc = to_cxl_drv(dev->driver)->probe(dev);
 	dev_dbg(dev, "probe: %d\n", rc);
+
 	return rc;
 }
 
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 26b5ad389cd2..051cd32ea628 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -12,6 +12,8 @@
 #include <cxl.h>
 #include "core.h"
 
+#include "core.h"
+
 /**
  * DOC: cxl core region
  *
@@ -26,10 +28,27 @@ static const struct attribute_group region_interleave_group;
 
 static bool is_region_active(struct cxl_region *region)
 {
-	/* TODO: Regions can't be activated yet. */
-	return false;
+	return region->active;
 }
 
+/*
+ * Most sanity checking is left up to region binding. This does the most basic
+ * check to determine whether or not the core should try probing the driver.
+ */
+bool is_cxl_region_configured(const struct cxl_region *region)
+{
+	/* zero sized regions aren't a thing. */
+	if (region->config.size <= 0)
+		return false;
+
+	/* all regions have at least 1 target */
+	if (!region->config.targets[0])
+		return false;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(is_cxl_region_configured);
+
 static void remove_target(struct cxl_region *region, int target)
 {
 	struct cxl_memdev *cxlmd;
@@ -45,7 +64,7 @@ static ssize_t interleave_ways_show(struct device *dev,
 {
 	struct cxl_region *region = to_cxl_region(dev);
 
-	return sysfs_emit(buf, "%d\n", region->config.eniw);
+	return sysfs_emit(buf, "%d\n", region->config.interleave_ways);
 }
 
 static ssize_t interleave_ways_store(struct device *dev,
@@ -53,17 +72,17 @@ static ssize_t interleave_ways_store(struct device *dev,
 				     const char *buf, size_t len)
 {
 	struct cxl_region *region = to_cxl_region(dev);
-	int ret, prev_eniw;
+	int ret, prev_niw;
 	int val;
 
-	prev_eniw = region->config.eniw;
+	prev_niw = region->config.interleave_ways;
 	ret = kstrtoint(buf, 0, &val);
 	if (ret)
 		return ret;
 	if (ret < 0 || ret > CXL_DECODER_MAX_INTERLEAVE)
 		return -EINVAL;
 
-	region->config.eniw = val;
+	region->config.interleave_ways = val;
 
 	ret = sysfs_update_group(&dev->kobj, &region_interleave_group);
 	if (ret < 0)
@@ -71,13 +90,13 @@ static ssize_t interleave_ways_store(struct device *dev,
 
 	sysfs_notify(&dev->kobj, NULL, "target_interleave");
 
-	while (prev_eniw > region->config.eniw)
-		remove_target(region, --prev_eniw);
+	while (prev_niw > region->config.interleave_ways)
+		remove_target(region, --prev_niw);
 
 	return len;
 
 err:
-	region->config.eniw = prev_eniw;
+	region->config.interleave_ways = prev_niw;
 	return ret;
 }
 static DEVICE_ATTR_RW(interleave_ways);
@@ -88,7 +107,7 @@ static ssize_t interleave_granularity_show(struct device *dev,
 {
 	struct cxl_region *region = to_cxl_region(dev);
 
-	return sysfs_emit(buf, "%d\n", region->config.ig);
+	return sysfs_emit(buf, "%d\n", region->config.interleave_granularity);
 }
 
 static ssize_t interleave_granularity_store(struct device *dev,
@@ -101,7 +120,7 @@ static ssize_t interleave_granularity_store(struct device *dev,
 	ret = kstrtoint(buf, 0, &val);
 	if (ret)
 		return ret;
-	region->config.ig = val;
+	region->config.interleave_granularity = val;
 
 	return len;
 }
@@ -293,7 +312,7 @@ static umode_t visible_targets(struct kobject *kobj, struct attribute *a, int n)
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct cxl_region *region = to_cxl_region(dev);
 
-	if (n < region->config.eniw)
+	if (n < region->config.interleave_ways)
 		return a->mode;
 	return 0;
 }
@@ -311,7 +330,7 @@ static const struct attribute_group *region_groups[] = {
 
 static void cxl_region_release(struct device *dev);
 
-static const struct device_type cxl_region_type = {
+const struct device_type cxl_region_type = {
 	.name = "cxl_region",
 	.release = cxl_region_release,
 	.groups = region_groups
@@ -403,7 +422,7 @@ static void cxl_region_release(struct device *dev)
 	int i;
 
 	ida_free(&cxld->region_ida, region->id);
-	for (i = 0; i < region->config.eniw; i++)
+	for (i = 0; i < region->config.interleave_ways; i++)
 		remove_target(region, i);
 	kfree(region);
 }
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 79c5781b6173..b318cabfc4a2 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -181,6 +181,10 @@ void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t addr,
 #define CXL_DECODER_F_ENABLE    BIT(5)
 #define CXL_DECODER_F_MASK  GENMASK(5, 0)
 
+#define cxl_is_pmem_t3(flags)                                                  \
+	(((flags) & (CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM)) ==             \
+	 (CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM))
+
 enum cxl_decoder_type {
        CXL_DECODER_ACCELERATOR = 2,
        CXL_DECODER_EXPANDER = 3,
@@ -348,6 +352,7 @@ int devm_cxl_add_dport(struct cxl_port *port, struct device *dport, int port_id,
 		       resource_size_t component_reg_phys);
 struct cxl_dport *cxl_find_dport_by_dev(struct cxl_port *port,
 					const struct device *dev);
+struct cxl_port *ep_find_cxl_port(struct cxl_memdev *cxlmd, unsigned int depth);
 
 struct cxl_decoder *to_cxl_decoder(struct device *dev);
 bool is_root_decoder(struct device *dev);
@@ -388,6 +393,7 @@ void cxl_driver_unregister(struct cxl_driver *cxl_drv);
 #define CXL_DEVICE_PORT			3
 #define CXL_DEVICE_MEMORY_EXPANDER	4
 #define CXL_DEVICE_ROOT			5
+#define CXL_DEVICE_REGION		6
 
 #define MODULE_ALIAS_CXL(type) MODULE_ALIAS("cxl:t" __stringify(type) "*")
 #define CXL_MODALIAS_FMT "cxl:t%d"
diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
new file mode 100644
index 000000000000..6ab9d640f5e1
--- /dev/null
+++ b/drivers/cxl/region.c
@@ -0,0 +1,331 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2021 Intel Corporation. All rights reserved. */
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include "cxlmem.h"
+#include "region.h"
+#include "cxl.h"
+
+/**
+ * DOC: cxl region
+ *
+ * This module implements a region driver that is capable of programming CXL
+ * hardware to setup regions.
+ *
+ * A CXL region encompasses a chunk of host physical address space that may be
+ * consumed by a single device (x1 interleave aka linear) or across multiple
+ * devices (xN interleaved). The region driver has the following
+ * responsibilities:
+ *
+ * * Walk topology to obtain decoder resources for region configuration.
+ * * Program decoder resources based on region configuration.
+ * * Bridge CXL regions to LIBNVDIMM
+ * * Initiates reading and configuring LSA regions
+ * * Enumerates regions created by BIOS (typically volatile)
+ */
+
+#define region_ways(region) ((region)->config.interleave_ways)
+
+static struct cxl_decoder *rootd_from_region(struct cxl_region *r)
+{
+	struct device *d = r->dev.parent;
+
+	if (WARN_ONCE(!is_root_decoder(d), "Corrupt topology for root region\n"))
+		return NULL;
+
+	return to_cxl_decoder(d);
+}
+
+static struct cxl_port *get_hostbridge(const struct cxl_memdev *ep)
+{
+	struct cxl_port *port = ep->port;
+
+	while (!is_cxl_root(port)) {
+		port = to_cxl_port(port->dev.parent);
+		if (port->depth == 1)
+			return port;
+	}
+
+	BUG();
+	return NULL;
+}
+
+static struct cxl_port *get_root_decoder(const struct cxl_memdev *endpoint)
+{
+	struct cxl_port *hostbridge = get_hostbridge(endpoint);
+
+	if (hostbridge)
+		return to_cxl_port(hostbridge->dev.parent);
+
+	return NULL;
+}
+
+/**
+ * sanitize_region() - Check is region is reasonably configured
+ * @region: The region to check
+ *
+ * Determination as to whether or not a region can possibly be configured is
+ * described in CXL Memory Device SW Guide. In order to implement the algorithms
+ * described there, certain more basic configuration parameters must first need
+ * to be validated. That is accomplished by this function.
+ *
+ * Returns 0 if the region is reasonably configured, else returns a negative
+ * error code.
+ */
+static int sanitize_region(const struct cxl_region *region)
+{
+	int i;
+
+	if (dev_WARN_ONCE(&region->dev, !is_cxl_region_configured(region),
+			  "unconfigured regions can't be probed (race?)\n")) {
+		return -ENXIO;
+	}
+
+	if (region->config.size % (SZ_256M * region_ways(region))) {
+		dev_dbg(&region->dev, "Invalid size. Must be multiple of %uM\n",
+			256 * region_ways(region));
+		return -ENXIO;
+	}
+
+	for (i = 0; i < region_ways(region); i++) {
+		if (!region->config.targets[i]) {
+			dev_dbg(&region->dev, "Missing memory device target%u",
+				i);
+			return -ENXIO;
+		}
+		if (!region->config.targets[i]->dev.driver) {
+			dev_dbg(&region->dev, "%s isn't CXL.mem capable\n",
+				dev_name(&region->config.targets[i]->dev));
+			return -ENODEV;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * allocate_address_space() - Gets address space for the region.
+ * @region: The region that will consume the address space
+ */
+static int allocate_address_space(struct cxl_region *region)
+{
+	/* TODO */
+	return 0;
+}
+
+/**
+ * find_cdat_dsmas() - Find a valid DSMAS for the region
+ * @region: The region
+ */
+static bool find_cdat_dsmas(const struct cxl_region *region)
+{
+	return true;
+}
+
+/**
+ * qtg_match() - Does this CFMWS have desirable QTG for the endpoint
+ * @cfmws: The CFMWS for the region
+ * @endpoint: Endpoint whose QTG is being compared
+ *
+ * Prior to calling this function, the caller should verify that all endpoints
+ * in the region have the same QTG ID.
+ *
+ * Returns true if the QTG ID of the CFMWS matches the endpoint
+ */
+static bool qtg_match(const struct cxl_decoder *cfmws,
+		      const struct cxl_memdev *endpoint)
+{
+	/* TODO: */
+	return true;
+}
+
+/**
+ * region_xhb_config_valid() - determine cross host bridge validity
+ * @cfmws: The CFMWS to check against
+ * @region: The region being programmed
+ *
+ * The algorithm is outlined in 2.13.14 "Verify XHB configuration sequence" of
+ * the CXL Memory Device SW Guide (Rev1p0).
+ *
+ * Returns true if the configuration is valid.
+ */
+static bool region_xhb_config_valid(const struct cxl_region *region,
+				    const struct cxl_decoder *cfmws)
+{
+	/* TODO: */
+	return true;
+}
+
+/**
+ * region_hb_rp_config_valid() - determine root port ordering is correct
+ * @cfmws: CFMWS decoder for this @region
+ * @region: Region to validate
+ *
+ * The algorithm is outlined in 2.13.15 "Verify HB root port configuration
+ * sequence" of the CXL Memory Device SW Guide (Rev1p0).
+ *
+ * Returns true if the configuration is valid.
+ */
+static bool region_hb_rp_config_valid(const struct cxl_region *region,
+				      const struct cxl_decoder *cfmws)
+{
+	/* TODO: */
+	return true;
+}
+
+/**
+ * rootd_contains() - determine if this region can exist in the root decoder
+ * @rootd: CFMWS that potentially decodes to this region
+ * @region: region to be routed by the @rootd
+ */
+static bool rootd_contains(const struct cxl_region *region,
+			   const struct cxl_decoder *rootd)
+{
+	/* TODO: */
+	return true;
+}
+
+static bool rootd_valid(const struct cxl_region *region,
+			const struct cxl_decoder *rootd)
+{
+	const struct cxl_memdev *endpoint = region->config.targets[0];
+
+	if (!qtg_match(rootd, endpoint))
+		return false;
+
+	if (!cxl_is_pmem_t3(rootd->flags))
+		return false;
+
+	if (!region_xhb_config_valid(region, rootd))
+		return false;
+
+	if (!region_hb_rp_config_valid(region, rootd))
+		return false;
+
+	if (!rootd_contains(region, rootd))
+		return false;
+
+	return true;
+}
+
+struct rootd_context {
+	const struct cxl_region *region;
+	struct cxl_port *hbs[CXL_DECODER_MAX_INTERLEAVE];
+	int count;
+};
+
+static int rootd_match(struct device *dev, void *data)
+{
+	struct rootd_context *ctx = (struct rootd_context *)data;
+	const struct cxl_region *region = ctx->region;
+
+	if (!is_root_decoder(dev))
+		return 0;
+
+	return !!rootd_valid(region, to_cxl_decoder(dev));
+}
+
+/*
+ * This is a roughly equivalent implementation to "Figure 45 - High-level
+ * sequence: Finding CFMWS for region" from the CXL Memory Device SW Guide
+ * Rev1p0.
+ */
+static struct cxl_decoder *find_rootd(const struct cxl_region *region,
+				      const struct cxl_port *root)
+{
+	struct rootd_context ctx;
+	struct device *ret;
+
+	ctx.region = region;
+
+	ret = device_find_child((struct device *)&root->dev, &ctx, rootd_match);
+	if (ret)
+		return to_cxl_decoder(ret);
+
+	return NULL;
+}
+
+static int collect_ep_decoders(const struct cxl_region *region)
+{
+	/* TODO: */
+	return 0;
+}
+
+static int bind_region(const struct cxl_region *region)
+{
+	/* TODO: */
+	return 0;
+}
+
+static int cxl_region_probe(struct device *dev)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+	struct cxl_port *root_port;
+	struct cxl_decoder *rootd, *ours;
+	int ret;
+
+	device_lock_assert(&region->dev);
+
+	if (region->active)
+		return 0;
+
+	if (uuid_is_null(&region->config.uuid))
+		uuid_gen(&region->config.uuid);
+
+	/* TODO: What about volatile, and LSA generated regions? */
+
+	ret = sanitize_region(region);
+	if (ret)
+		return ret;
+
+	ret = allocate_address_space(region);
+	if (ret)
+		return ret;
+
+	if (!find_cdat_dsmas(region))
+		return -ENXIO;
+
+	rootd = rootd_from_region(region);
+	if (!rootd) {
+		dev_err(dev, "Couldn't find root decoder\n");
+		return -ENXIO;
+	}
+
+	if (!rootd_valid(region, rootd)) {
+		dev_err(dev, "Picked invalid rootd\n");
+		return -ENXIO;
+	}
+
+	root_port = get_root_decoder(region->config.targets[0]);
+	ours = find_rootd(region, root_port);
+	if (ours != rootd)
+		dev_warn(dev, "Picked different rootd %s %s\n",
+			 dev_name(&rootd->dev), dev_name(&ours->dev));
+	if (ours)
+		put_device(&ours->dev);
+
+	ret = collect_ep_decoders(region);
+	if (ret)
+		return ret;
+
+	ret = bind_region(region);
+	if (!ret) {
+		region->active = true;
+		dev_info(dev, "Bound");
+	}
+
+	return ret;
+}
+
+static struct cxl_driver cxl_region_driver = {
+	.name = "cxl_region",
+	.probe = cxl_region_probe,
+	.id = CXL_DEVICE_REGION,
+};
+module_cxl_driver(cxl_region_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(CXL);
+MODULE_ALIAS_CXL(CXL_DEVICE_REGION);
diff --git a/drivers/cxl/region.h b/drivers/cxl/region.h
index 3e6e5fb35822..9f89f0e8744b 100644
--- a/drivers/cxl/region.h
+++ b/drivers/cxl/region.h
@@ -13,11 +13,12 @@
  * @id: This regions id. Id is globally unique across all regions.
  * @list: Node in decoder's region list.
  * @res: Resource this region carves out of the platform decode range.
+ * @active: If the region has been activated.
  * @config: HDM decoder program config
  * @config.size: Size of the region determined from LSA or userspace.
  * @config.uuid: The UUID for this region.
- * @config.eniw: Number of interleave ways this region is configured for.
- * @config.ig: Interleave granularity of region
+ * @config.interleave_ways: Number of interleave ways this region is configured for.
+ * @config.interleave_granularity: Interleave granularity of region
  * @config.targets: The memory devices comprising the region.
  */
 struct cxl_region {
@@ -25,14 +26,17 @@ struct cxl_region {
 	int id;
 	struct list_head list;
 	struct resource *res;
+	bool active;
 
 	struct {
 		u64 size;
 		uuid_t uuid;
-		int eniw;
-		int ig;
+		int interleave_ways;
+		int interleave_granularity;
 		struct cxl_memdev *targets[CXL_DECODER_MAX_INTERLEAVE];
 	} config;
 };
 
+bool is_cxl_region_configured(const struct cxl_region *region);
+
 #endif
-- 
2.34.1


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

* [PATCH v2 07/15] cxl/acpi: Handle address space allocation
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (5 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 06/15] cxl/region: Introduce a cxl_region driver Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 08/15] cxl/region: Address " Ben Widawsky
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

Regions are carved out of an addresses space which is claimed by top
level decoders, and subsequently their children decoders. Regions are
created with a size and therefore must fit, with proper alignment, in
that address space. The support for doing this fitting is handled by the
driver automatically.

As an example, a platform might configure a top level decoder to claim
1TB of address space @ 0x800000000 -> 0x10800000000; it would be
possible to create M regions with appropriate alignment to occupy that
address space. Each of those regions would have a host physical address
somewhere in the range between 32G and 1.3TB, and the location will be
determined by the logic added here.

The request_region() usage is not strictly mandatory at this point as
the actual handling of the address space is done with genpools. It is
highly likely however that the resource/region APIs will become useful
in the not too distant future.

All decoders manage a host physical address space while active. Only the
root decoder has constraints on location and size. As a result, it makes
most sense for the root decoder to be responsible for managing the
entire address space, and mid-level decoders and endpoints can ask the
root decoder for suballocations.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 drivers/cxl/acpi.c   | 30 ++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h    |  2 ++
 drivers/cxl/region.c | 12 ++++++------
 3 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 4c746a6ef48c..a7ce0d660b34 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright(c) 2021 Intel Corporation. All rights reserved. */
 #include <linux/platform_device.h>
+#include <linux/genalloc.h>
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
@@ -73,6 +74,27 @@ static int cxl_acpi_cfmws_verify(struct device *dev,
 	return 0;
 }
 
+/*
+ * Every decoder while active has an address space that it is decoding. However,
+ * only the root level decoders have fixed host physical address space ranges.
+ */
+static int cxl_create_cfmws_address_space(struct cxl_decoder *cxld,
+					  struct acpi_cedt_cfmws *cfmws)
+{
+	const int order = ilog2(SZ_256M * cxld->interleave_ways);
+	struct device *dev = &cxld->dev;
+	struct gen_pool *pool;
+
+	pool = devm_gen_pool_create(dev, order, NUMA_NO_NODE, dev_name(dev));
+	if (IS_ERR(pool))
+		return PTR_ERR(pool);
+
+	cxld->address_space = pool;
+
+	return gen_pool_add(cxld->address_space, cfmws->base_hpa,
+			    cfmws->window_size, NUMA_NO_NODE);
+}
+
 struct cxl_cfmws_context {
 	struct device *dev;
 	struct cxl_port *root_port;
@@ -113,6 +135,14 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
 	cxld->interleave_ways = CFMWS_INTERLEAVE_WAYS(cfmws);
 	cxld->interleave_granularity = CFMWS_INTERLEAVE_GRANULARITY(cfmws);
 
+	rc = cxl_create_cfmws_address_space(cxld, cfmws);
+	if (rc) {
+		dev_err(dev,
+			"Failed to create CFMWS address space for decoder\n");
+		put_device(&cxld->dev);
+		return 0;
+	}
+
 	rc = cxl_decoder_add(cxld, target_map);
 	if (rc)
 		put_device(&cxld->dev);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index b318cabfc4a2..19e65ed35796 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -207,6 +207,7 @@ enum cxl_decoder_type {
  * @target_type: accelerator vs expander (type2 vs type3) selector
  * @flags: memory type capabilities and locking
  * @region_ida: allocator for region ids.
+ * @address_space: Used/free address space for regions.
  * @nr_targets: number of elements in @target
  * @target: active ordered target list in current decoder configuration
  */
@@ -222,6 +223,7 @@ struct cxl_decoder {
 	enum cxl_decoder_type target_type;
 	unsigned long flags;
 	struct ida region_ida;
+	struct gen_pool *address_space;
 	const int nr_targets;
 	struct cxl_dport *target[];
 };
diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
index 6ab9d640f5e1..53046da2e131 100644
--- a/drivers/cxl/region.c
+++ b/drivers/cxl/region.c
@@ -126,7 +126,7 @@ static bool find_cdat_dsmas(const struct cxl_region *region)
 
 /**
  * qtg_match() - Does this CFMWS have desirable QTG for the endpoint
- * @cfmws: The CFMWS for the region
+ * @rootd: The root decoder for the region
  * @endpoint: Endpoint whose QTG is being compared
  *
  * Prior to calling this function, the caller should verify that all endpoints
@@ -134,7 +134,7 @@ static bool find_cdat_dsmas(const struct cxl_region *region)
  *
  * Returns true if the QTG ID of the CFMWS matches the endpoint
  */
-static bool qtg_match(const struct cxl_decoder *cfmws,
+static bool qtg_match(const struct cxl_decoder *rootd,
 		      const struct cxl_memdev *endpoint)
 {
 	/* TODO: */
@@ -143,7 +143,7 @@ static bool qtg_match(const struct cxl_decoder *cfmws,
 
 /**
  * region_xhb_config_valid() - determine cross host bridge validity
- * @cfmws: The CFMWS to check against
+ * @rootd: The root decoder to check against
  * @region: The region being programmed
  *
  * The algorithm is outlined in 2.13.14 "Verify XHB configuration sequence" of
@@ -152,7 +152,7 @@ static bool qtg_match(const struct cxl_decoder *cfmws,
  * Returns true if the configuration is valid.
  */
 static bool region_xhb_config_valid(const struct cxl_region *region,
-				    const struct cxl_decoder *cfmws)
+				    const struct cxl_decoder *rootd)
 {
 	/* TODO: */
 	return true;
@@ -160,7 +160,7 @@ static bool region_xhb_config_valid(const struct cxl_region *region,
 
 /**
  * region_hb_rp_config_valid() - determine root port ordering is correct
- * @cfmws: CFMWS decoder for this @region
+ * @rootd: root decoder for this @region
  * @region: Region to validate
  *
  * The algorithm is outlined in 2.13.15 "Verify HB root port configuration
@@ -169,7 +169,7 @@ static bool region_xhb_config_valid(const struct cxl_region *region,
  * Returns true if the configuration is valid.
  */
 static bool region_hb_rp_config_valid(const struct cxl_region *region,
-				      const struct cxl_decoder *cfmws)
+				      const struct cxl_decoder *rootd)
 {
 	/* TODO: */
 	return true;
-- 
2.34.1


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

* [PATCH v2 08/15] cxl/region: Address space allocation
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (6 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 07/15] cxl/acpi: Handle address space allocation Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 09/15] cxl/region: Implement XHB verification Ben Widawsky
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

When a region is not assigned a host physical address, one is picked by
the driver. As the address will determine which CFMWS contains the
region, it's usually a better idea to let the driver make this
determination.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 drivers/cxl/region.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
index 53046da2e131..c12d9bd22705 100644
--- a/drivers/cxl/region.c
+++ b/drivers/cxl/region.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright(c) 2021 Intel Corporation. All rights reserved. */
 #include <linux/platform_device.h>
+#include <linux/genalloc.h>
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/pci.h>
@@ -62,6 +63,20 @@ static struct cxl_port *get_root_decoder(const struct cxl_memdev *endpoint)
 	return NULL;
 }
 
+static void release_cxl_region(void *r)
+{
+	struct cxl_region *region = (struct cxl_region *)r;
+	struct cxl_decoder *rootd = rootd_from_region(region);
+	struct resource *res = &rootd->platform_res;
+	resource_size_t start, size;
+
+	start = region->res->start;
+	size = resource_size(region->res);
+
+	__release_region(res, start, size);
+	gen_pool_free(rootd->address_space, start, size);
+}
+
 /**
  * sanitize_region() - Check is region is reasonably configured
  * @region: The region to check
@@ -111,8 +126,29 @@ static int sanitize_region(const struct cxl_region *region)
  */
 static int allocate_address_space(struct cxl_region *region)
 {
-	/* TODO */
-	return 0;
+	struct cxl_decoder *rootd = rootd_from_region(region);
+	unsigned long start;
+
+	start = gen_pool_alloc(rootd->address_space, region->config.size);
+	if (!start) {
+		dev_dbg(&region->dev,
+			"Couldn't allocate %lluM of address space",
+			region->config.size >> 20);
+		return -ENOMEM;
+	}
+
+	region->res = __request_region(&rootd->platform_res, start,
+				       region->config.size,
+				       dev_name(&region->dev), IORESOURCE_MEM);
+	if (!region->res) {
+		dev_dbg(&region->dev, "Couldn't obtain region from %s (%pR)\n",
+			dev_name(&rootd->dev), &rootd->platform_res);
+		gen_pool_free(rootd->address_space, start, region->config.size);
+		return -ENOMEM;
+	}
+
+	return devm_add_action_or_reset(&region->dev, release_cxl_region,
+					region);
 }
 
 /**
-- 
2.34.1


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

* [PATCH v2 09/15] cxl/region: Implement XHB verification
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (7 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 08/15] cxl/region: Address " Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 10/15] cxl/region: HB port config verification Ben Widawsky
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

Cross host bridge verification primarily determines if the requested
interleave ordering can be achieved by the root decoder, which isn't as
programmable as other decoders.

The algorithm implemented here is based on the CXL Type 3 Memory Device
Software Guide, chapter 2.13.14

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>

---
Changes since v1:
- Fix for_each_cxl_decoder_target definition (Jonathan)
- Fix math XHB granularity check (Jonathan)
- Remove bogus xhb check (Jonathan)
- Rename ig/eniw to prevent confusion
---
 .clang-format        |  2 +
 drivers/cxl/cxl.h    | 13 +++++++
 drivers/cxl/region.c | 93 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/.clang-format b/.clang-format
index 15d4eaabc6b5..55f628f21722 100644
--- a/.clang-format
+++ b/.clang-format
@@ -169,6 +169,8 @@ ForEachMacros:
   - 'for_each_cpu_and'
   - 'for_each_cpu_not'
   - 'for_each_cpu_wrap'
+  - 'for_each_cxl_decoder_target'
+  - 'for_each_cxl_endpoint'
   - 'for_each_dapm_widgets'
   - 'for_each_dev_addr'
   - 'for_each_dev_scope'
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 19e65ed35796..c62e93e8a369 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -63,6 +63,19 @@ static inline int cxl_hdm_decoder_count(u32 cap_hdr)
 	return val ? val * 2 : 1;
 }
 
+static inline u8 cxl_to_eniw(u8 ways)
+{
+	if (is_power_of_2(ways))
+		return ilog2(ways);
+
+	return ways / 3 + 8;
+}
+
+static inline u8 cxl_to_ig(u16 g)
+{
+	return 8 - ilog2(g);
+}
+
 /* CXL 2.0 8.2.8.1 Device Capabilities Array Register */
 #define CXLDEV_CAP_ARRAY_OFFSET 0x0
 #define   CXLDEV_CAP_ARRAY_CAP_ID 0
diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
index c12d9bd22705..c01b1ab9f757 100644
--- a/drivers/cxl/region.c
+++ b/drivers/cxl/region.c
@@ -28,6 +28,19 @@
  */
 
 #define region_ways(region) ((region)->config.interleave_ways)
+#define region_eniw(region) (cxl_to_eniw((region)->config.interleave_ways))
+#define region_granularity(region) ((region)->config.interleave_granularity)
+#define region_ig(region) (cxl_to_ig((region)->config.interleave_granularity))
+
+#define for_each_cxl_endpoint(ep, region, idx)                                 \
+	for (idx = 0, ep = (region)->config.targets[idx];                      \
+	     idx < region_ways(region);                                        \
+	     ep = (region)->config.targets[++idx])
+
+#define for_each_cxl_decoder_target(dport, decoder, idx)                      \
+	for (idx = 0, dport = (decoder)->target[idx];                         \
+	     idx < (decoder)->nr_targets;                                     \
+	     dport = (decoder)->target[++idx])
 
 static struct cxl_decoder *rootd_from_region(struct cxl_region *r)
 {
@@ -177,6 +190,30 @@ static bool qtg_match(const struct cxl_decoder *rootd,
 	return true;
 }
 
+static int get_unique_hostbridges(const struct cxl_region *region,
+				  struct cxl_port **hbs)
+{
+	struct cxl_memdev *ep;
+	int i, hb_count = 0;
+
+	for_each_cxl_endpoint(ep, region, i) {
+		struct cxl_port *hb = get_hostbridge(ep);
+		bool found = false;
+		int j;
+
+		BUG_ON(!hb);
+
+		for (j = 0; j < hb_count; j++) {
+			if (hbs[j] == hb)
+				found = true;
+		}
+		if (!found)
+			hbs[hb_count++] = hb;
+	}
+
+	return hb_count;
+}
+
 /**
  * region_xhb_config_valid() - determine cross host bridge validity
  * @rootd: The root decoder to check against
@@ -190,7 +227,61 @@ static bool qtg_match(const struct cxl_decoder *rootd,
 static bool region_xhb_config_valid(const struct cxl_region *region,
 				    const struct cxl_decoder *rootd)
 {
-	/* TODO: */
+	struct cxl_port *hbs[CXL_DECODER_MAX_INTERLEAVE];
+	struct cxl_dport *target;
+	int rootd_ig, i;
+
+	/* Are all devices in this region on the same CXL host bridge */
+	if (get_unique_hostbridges(region, hbs) == 1)
+		return true;
+
+	rootd_ig = cxl_to_ig(rootd->interleave_granularity);
+
+	/* CFMWS.HBIG >= Device.Label.IG */
+	if (rootd_ig < (region_ig(region))) {
+		dev_dbg(&region->dev,
+			"%s HBIG must be greater than region IG (%d < %d)\n",
+			dev_name(&rootd->dev), rootd_ig, region_ig(region));
+		return false;
+	}
+
+	/*
+	 * ((2^(CFMWS.HBIG - Device.RLabel.IG) * (2^CFMWS.ENIW)) > Device.RLabel.NLabel)
+	 *
+	 * Linux notes: 2^CFMWS.ENIW is trying to decode the NIW. Instead we use
+	 * the look up function which supports non power of 2 interleave
+	 * configurations.
+	 */
+	if (((1 << (rootd_ig - region_ig(region))) *
+	     (1 << cxl_to_eniw(rootd->interleave_ways))) >
+	    region_ways(region)) {
+		dev_dbg(&region->dev,
+			"granularity ratio requires a larger number of devices (%d) than currently configured (%d)\n",
+			((1 << (rootd_ig - region_ig(region))) *
+			 (1 << cxl_to_eniw(rootd->interleave_ways))),
+			region_ways(region));
+		return false;
+	}
+
+	/*
+	 * CFMWS.InterleaveTargetList[n] must contain all devices, x where:
+	 *	(Device[x],RegionLabel.Position >> (CFMWS.HBIG -
+	 *	Device[x].RegionLabel.InterleaveGranularity)) &
+	 *	((2^CFMWS.ENIW) - 1) = n
+	 *
+	 * Linux notes: All devices are known to have the same interleave
+	 * granularity at this point.
+	 */
+	for_each_cxl_decoder_target(target, rootd, i) {
+		if (((i >> (rootd_ig - region_granularity(region)))) &
+		    (((1 << cxl_to_eniw(rootd->interleave_ways)) - 1) !=
+		     target->port_id)) {
+			dev_dbg(&region->dev,
+				"One or more devices are not connected to the correct hostbridge.\n");
+			return false;
+		}
+	}
+
 	return true;
 }
 
-- 
2.34.1


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

* [PATCH v2 10/15] cxl/region: HB port config verification
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (8 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 09/15] cxl/region: Implement XHB verification Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 11/15] cxl/region: Add infrastructure for decoder programming Ben Widawsky
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

Host bridge root port verification determines if the device ordering in
an interleave set can be programmed through the host bridges and
switches.

The algorithm implemented here is based on the CXL Type 3 Memory Device
Software Guide, chapter 2.13.15

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 .clang-format           |   1 +
 drivers/cxl/core/port.c |   1 +
 drivers/cxl/cxl.h       |   2 +
 drivers/cxl/region.c    | 122 +++++++++++++++++++++++++++++++++++++++-
 4 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/.clang-format b/.clang-format
index 55f628f21722..96c282b63e7b 100644
--- a/.clang-format
+++ b/.clang-format
@@ -171,6 +171,7 @@ ForEachMacros:
   - 'for_each_cpu_wrap'
   - 'for_each_cxl_decoder_target'
   - 'for_each_cxl_endpoint'
+  - 'for_each_cxl_endpoint_hb'
   - 'for_each_dapm_widgets'
   - 'for_each_dev_addr'
   - 'for_each_dev_scope'
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 67f3345d44ef..99589f23f1ff 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -585,6 +585,7 @@ struct cxl_dport *cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
 		return ERR_PTR(-ENOMEM);
 
 	INIT_LIST_HEAD(&dport->list);
+	INIT_LIST_HEAD(&dport->verify_link);
 	dport->dport = get_device(dport_dev);
 	dport->port_id = port_id;
 	dport->component_reg_phys = component_reg_phys;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index c62e93e8a369..4de4c0ee8eb2 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -316,6 +316,7 @@ struct cxl_port {
  * @port: reference to cxl_port that contains this downstream port
  * @list: node for a cxl_port's list of cxl_dport instances
  * @link_name: the name of the sysfs link from @port to @dport
+ * @verify_link: node used for hb root port verification
  */
 struct cxl_dport {
 	struct device *dport;
@@ -324,6 +325,7 @@ struct cxl_dport {
 	struct cxl_port *port;
 	struct list_head list;
 	char link_name[CXL_TARGET_STRLEN];
+	struct list_head verify_link;
 };
 
 /**
diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
index c01b1ab9f757..1f8919ad8dcc 100644
--- a/drivers/cxl/region.c
+++ b/drivers/cxl/region.c
@@ -4,6 +4,7 @@
 #include <linux/genalloc.h>
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/sort.h>
 #include <linux/pci.h>
 #include "cxlmem.h"
 #include "region.h"
@@ -37,6 +38,12 @@
 	     idx < region_ways(region);                                        \
 	     ep = (region)->config.targets[++idx])
 
+#define for_each_cxl_endpoint_hb(ep, region, hb, idx)                          \
+	for (idx = 0, (ep) = (region)->config.targets[idx];                    \
+	     idx < region_ways(region);                                        \
+	     idx++, (ep) = (region)->config.targets[idx])                      \
+		if (get_hostbridge(ep) == (hb))
+
 #define for_each_cxl_decoder_target(dport, decoder, idx)                      \
 	for (idx = 0, dport = (decoder)->target[idx];                         \
 	     idx < (decoder)->nr_targets;                                     \
@@ -285,6 +292,59 @@ static bool region_xhb_config_valid(const struct cxl_region *region,
 	return true;
 }
 
+static struct cxl_dport *get_rp(struct cxl_memdev *ep)
+{
+	struct cxl_port *port, *parent_port = port = ep->port;
+	struct cxl_dport *dport;
+
+	while (!is_cxl_root(port)) {
+		parent_port = to_cxl_port(port->dev.parent);
+		if (parent_port->depth == 1)
+			list_for_each_entry(dport, &parent_port->dports, list)
+				if (dport->dport == port->uport->parent->parent)
+					return dport;
+		port = parent_port;
+	}
+
+	BUG();
+	return NULL;
+}
+
+static int get_num_root_ports(const struct cxl_region *region)
+{
+	struct cxl_memdev *endpoint;
+	struct cxl_dport *dport, *tmp;
+	int num_root_ports = 0;
+	LIST_HEAD(root_ports);
+	int idx;
+
+	for_each_cxl_endpoint(endpoint, region, idx) {
+		struct cxl_dport *root_port = get_rp(endpoint);
+
+		if (list_empty(&root_port->verify_link)) {
+			list_add_tail(&root_port->verify_link, &root_ports);
+			num_root_ports++;
+		}
+	}
+
+	list_for_each_entry_safe(dport, tmp, &root_ports, verify_link)
+		list_del_init(&dport->verify_link);
+
+	return num_root_ports;
+}
+
+static bool has_switch(const struct cxl_region *region)
+{
+	struct cxl_memdev *ep;
+	int i;
+
+	for_each_cxl_endpoint(ep, region, i)
+		if (ep->port->depth > 2)
+			return true;
+
+	return false;
+}
+
 /**
  * region_hb_rp_config_valid() - determine root port ordering is correct
  * @rootd: root decoder for this @region
@@ -298,7 +358,67 @@ static bool region_xhb_config_valid(const struct cxl_region *region,
 static bool region_hb_rp_config_valid(const struct cxl_region *region,
 				      const struct cxl_decoder *rootd)
 {
-	/* TODO: */
+	const int num_root_ports = get_num_root_ports(region);
+	struct cxl_port *hbs[CXL_DECODER_MAX_INTERLEAVE];
+	int hb_count, i;
+
+	hb_count = get_unique_hostbridges(region, hbs);
+
+	/*
+	 * Are all devices in this region on the same CXL Host Bridge
+	 * Root Port?
+	 */
+	if (num_root_ports == 1 && !has_switch(region))
+		return true;
+
+	for (i = 0; i < hb_count; i++) {
+		int idx, position_mask;
+		struct cxl_dport *rp;
+		struct cxl_port *hb;
+
+		/* Get next CXL Host Bridge this region spans */
+		hb = hbs[i];
+
+		/*
+		 * Calculate the position mask: NumRootPorts = 2^PositionMask
+		 * for this region.
+		 *
+		 * XXX: pos_mask is actually (1 << PositionMask)  - 1
+		 */
+		position_mask = (1 << (ilog2(num_root_ports))) - 1;
+
+		/*
+		 * Calculate the PortGrouping for each device on this CXL Host
+		 * Bridge Root Port:
+		 * PortGrouping = RegionLabel.Position & PositionMask
+		 */
+		list_for_each_entry(rp, &hb->dports, list) {
+			struct cxl_memdev *ep;
+			int port_grouping = -1;
+
+			for_each_cxl_endpoint_hb(ep, region, hb, idx) {
+				/* Only endpoints under the same root port */
+				if (get_rp(ep) != rp)
+					continue;
+
+				if (port_grouping == -1) {
+					port_grouping = idx & position_mask;
+					continue;
+				}
+
+				/*
+				 * Do all devices in the region connected to this CXL
+				 * Host Bridge Root Port have the same PortGrouping?
+				 */
+				if ((idx & position_mask) != port_grouping) {
+					dev_dbg(&region->dev,
+						"One or more devices are not connected to the correct Host Bridge Root Port\n");
+					return false;
+				}
+			}
+		}
+	}
+
 	return true;
 }
 
-- 
2.34.1


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

* [PATCH v2 11/15] cxl/region: Add infrastructure for decoder programming
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (9 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 10/15] cxl/region: HB port config verification Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 12/15] cxl/region: Collect host bridge decoders Ben Widawsky
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

There are 3 steps in handling region programming once it has been
configured by userspace.
1. Sanitize the parameters against the system.
2. Collect decoder resources from the topology
3. Program decoder resources

The infrastructure added here addresses #2. Two new APIs are introduced
to allow collecting and returning decoder resources. Additionally the
infrastructure includes two lists managed by the region driver, a staged
list, and a commit list. The staged list contains those collected in
step #2, and the commit list are all the decoders programmed in step #3.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 drivers/cxl/core/port.c   |  75 +++++++++++++++++++++++++--
 drivers/cxl/core/region.c |   2 +
 drivers/cxl/cxl.h         |   8 +++
 drivers/cxl/cxlmem.h      |   7 +++
 drivers/cxl/port.c        |  42 +++++++++++++++-
 drivers/cxl/region.c      | 103 ++++++++++++++++++++++++++++++++------
 drivers/cxl/region.h      |   5 ++
 7 files changed, 224 insertions(+), 18 deletions(-)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 99589f23f1ff..41a7dccacb49 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -32,8 +32,6 @@ static DEFINE_XARRAY(cxl_root_buses);
 
 static void cxl_decoder_release(struct device *dev);
 
-static bool is_cxl_decoder(struct device *dev);
-
 static int decoder_match(struct device *dev, void *data)
 {
 	struct resource *theirs = (struct resource *)data;
@@ -291,10 +289,11 @@ bool is_root_decoder(struct device *dev)
 }
 EXPORT_SYMBOL_NS_GPL(is_root_decoder, CXL);
 
-static bool is_cxl_decoder(struct device *dev)
+bool is_cxl_decoder(struct device *dev)
 {
 	return dev->type->release == cxl_decoder_release;
 }
+EXPORT_SYMBOL_NS_GPL(is_cxl_decoder, CXL);
 
 struct cxl_decoder *to_cxl_decoder(struct device *dev)
 {
@@ -1040,6 +1039,8 @@ static struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port,
 	cxld->target_type = CXL_DECODER_EXPANDER;
 	cxld->platform_res = (struct resource)DEFINE_RES_MEM(0, 0);
 
+	INIT_LIST_HEAD(&cxld->region_link);
+
 	ida_init(&cxld->region_ida);
 
 	return cxld;
@@ -1200,6 +1201,74 @@ int cxl_decoder_add(struct cxl_decoder *cxld, int *target_map)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_decoder_add, CXL);
 
+/**
+ * cxl_get_decoder() - Get an unused decoder from the port.
+ * @port: The port to obtain a decoder from.
+ *
+ * Region programming requires obtaining decoder resources from all ports that
+ * participate in the interleave set. This function shall be used to pull the
+ * decoder resource out of the list of available.
+ *
+ * Context: Process context. Takes and releases the device lock of the port.
+ *
+ * Return: A cxl_decoder that can be used for programming if successful, else a
+ *	   negative error code.
+ */
+struct cxl_decoder *cxl_get_decoder(struct cxl_port *port)
+{
+	struct cxl_port_state *cxlps;
+	int dec;
+
+	cxlps = dev_get_drvdata(&port->dev);
+	if (dev_WARN_ONCE(&port->dev, !cxlps, "No port drvdata\n"))
+		return ERR_PTR(-ENXIO);
+
+	device_lock(&port->dev);
+	dec = find_first_bit(cxlps->decoders.free_mask, cxlps->decoders.count);
+	if (dec == cxlps->decoders.count) {
+		device_unlock(&port->dev);
+		return ERR_PTR(-ENODEV);
+	}
+
+	clear_bit(dec, cxlps->decoders.free_mask);
+	device_unlock(&port->dev);
+
+	return cxlps->decoders.cxld[dec];
+}
+EXPORT_SYMBOL_NS_GPL(cxl_get_decoder, CXL);
+
+/**
+ * cxl_put_decoder() - Return an inactive decoder to the port.
+ * @cxld: The decoder being returned.
+ */
+void cxl_put_decoder(struct cxl_decoder *cxld)
+{
+	struct cxl_port *port = to_cxl_port(cxld->dev.parent);
+	struct cxl_port_state *cxlps;
+	int i;
+
+	cxlps = dev_get_drvdata(&port->dev);
+	if (dev_WARN_ONCE(&port->dev, !cxlps, "No port drvdata\n"))
+		return;
+
+	device_lock(&port->dev);
+
+	for (i = 0; i < CXL_DECODER_MAX_INSTANCES; i++) {
+		struct cxl_decoder *d = cxlps->decoders.cxld[i];
+
+		if (!d)
+			break;
+
+		if (d == cxld) {
+			set_bit(i, cxlps->decoders.free_mask);
+			break;
+		}
+	}
+
+	device_unlock(&port->dev);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_put_decoder, CXL);
+
 static void cxld_unregister(void *dev)
 {
 	struct cxl_decoder *plat_decoder, *cxld = to_cxl_decoder(dev);
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 051cd32ea628..0ecd17e4dd0c 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -435,6 +435,8 @@ struct cxl_region *cxl_alloc_region(struct cxl_decoder *cxld, int id)
 	if (!region)
 		return ERR_PTR(-ENOMEM);
 
+	INIT_LIST_HEAD(&region->staged_list);
+	INIT_LIST_HEAD(&region->commit_list);
 	region->id = id;
 
 	return region;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 4de4c0ee8eb2..81c35be13416 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -35,6 +35,8 @@
 #define   CXL_CM_CAP_CAP_ID_HDM 0x5
 #define   CXL_CM_CAP_CAP_HDM_VERSION 1
 
+#define CXL_DECODER_MAX_INSTANCES 10
+
 /* HDM decoders CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure */
 #define CXL_HDM_DECODER_CAP_OFFSET 0x0
 #define   CXL_HDM_DECODER_COUNT_MASK GENMASK(3, 0)
@@ -221,6 +223,7 @@ enum cxl_decoder_type {
  * @flags: memory type capabilities and locking
  * @region_ida: allocator for region ids.
  * @address_space: Used/free address space for regions.
+ * @region_link: This decoder's place on either the staged, or commit list.
  * @nr_targets: number of elements in @target
  * @target: active ordered target list in current decoder configuration
  */
@@ -237,6 +240,7 @@ struct cxl_decoder {
 	unsigned long flags;
 	struct ida region_ida;
 	struct gen_pool *address_space;
+	struct list_head region_link;
 	const int nr_targets;
 	struct cxl_dport *target[];
 };
@@ -290,6 +294,7 @@ struct cxl_walk_context {
  * @id: id for port device-name
  * @dports: cxl_dport instances referenced by decoders
  * @endpoints: cxl_ep instances, endpoints that are a descendant of this port
+ * @region_link: this port's node on the region's list of ports
  * @decoder_ida: allocator for decoder ids
  * @component_reg_phys: component register capability base address (optional)
  * @dead: last ep has been removed, force port re-creation
@@ -360,6 +365,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host, struct device *uport,
 				   struct cxl_port *parent_port);
 int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
 struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd);
+struct cxl_decoder *cxl_get_decoder(struct cxl_port *port);
+void cxl_put_decoder(struct cxl_decoder *cxld);
 bool schedule_cxl_rescan(void);
 
 struct cxl_dport *cxl_add_dport(struct cxl_port *port, struct device *dport,
@@ -372,6 +379,7 @@ struct cxl_dport *cxl_find_dport_by_dev(struct cxl_port *port,
 struct cxl_port *ep_find_cxl_port(struct cxl_memdev *cxlmd, unsigned int depth);
 
 struct cxl_decoder *to_cxl_decoder(struct device *dev);
+bool is_cxl_decoder(struct device *dev);
 bool is_root_decoder(struct device *dev);
 struct cxl_decoder *cxl_root_decoder_alloc(struct cxl_port *port,
 					   unsigned int nr_targets);
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 38d6129499c8..e4793e5f25bc 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -301,6 +301,13 @@ struct cxl_port_state {
 		unsigned int interleave11_8;
 		unsigned int interleave14_12;
 	} caps;
+
+	struct port_decoders {
+		unsigned long *free_mask;
+		int count;
+
+		struct cxl_decoder *cxld[CXL_DECODER_MAX_INSTANCES];
+	} decoders;
 };
 
 int devm_cxl_setup_hdm(struct cxl_port *port);
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index c10b462373db..ddf6e78189ee 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -41,10 +41,39 @@ static bool is_cxl_endpoint(struct cxl_port *port)
 	return is_cxl_memdev(port->uport);
 }
 
+static int count_decoders(struct device *dev, void *data)
+{
+	if (is_cxl_decoder(dev))
+		(*(int *)data)++;
+
+	return 0;
+}
+
+static int set_decoders(struct device *dev, void *data)
+{
+	struct cxl_port_state *cxlps;
+	int dec;
+
+	if (!is_cxl_decoder(dev))
+		return 0;
+
+	cxlps = data;
+	dec = find_first_zero_bit(cxlps->decoders.free_mask, cxlps->decoders.count);
+	if (dev_WARN_ONCE(dev, dec == cxlps->decoders.count,
+			  "Impossible decoder bitmap state\n"))
+		return 1;
+
+	set_bit(dec, cxlps->decoders.free_mask);
+	cxlps->decoders.cxld[dec] = to_cxl_decoder(dev);
+
+	return 0;
+}
+
 static int cxl_port_probe(struct device *dev)
 {
 	struct cxl_port *port = to_cxl_port(dev);
-	int rc;
+	struct cxl_port_state *cxlps;
+	int rc, decoder_count = 0;
 
 	if (!is_cxl_endpoint(port)) {
 		rc = cxl_port_enumerate_dports(port);
@@ -59,6 +88,8 @@ static int cxl_port_probe(struct device *dev)
 	if (rc)
 		return rc;
 
+	cxlps = dev_get_drvdata(dev);
+
 	if (is_cxl_endpoint(port))
 		rc = devm_cxl_enumerate_endpoint_decoders(port);
 	else
@@ -68,6 +99,15 @@ static int cxl_port_probe(struct device *dev)
 		return rc;
 	}
 
+	device_for_each_child(&port->dev, &decoder_count, count_decoders);
+
+	cxlps->decoders.free_mask =
+		devm_bitmap_zalloc(&port->dev, decoder_count, GFP_KERNEL);
+	cxlps->decoders.count = decoder_count;
+
+	if (device_for_each_child(&port->dev, cxlps, set_decoders))
+		return -ENXIO;
+
 	schedule_cxl_rescan();
 
 	return 0;
diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
index 1f8919ad8dcc..cb3fc8de4c23 100644
--- a/drivers/cxl/region.c
+++ b/drivers/cxl/region.c
@@ -349,17 +349,20 @@ static bool has_switch(const struct cxl_region *region)
  * region_hb_rp_config_valid() - determine root port ordering is correct
  * @rootd: root decoder for this @region
  * @region: Region to validate
+ * @state_update: Whether or not to update port state
  *
  * The algorithm is outlined in 2.13.15 "Verify HB root port configuration
  * sequence" of the CXL Memory Device SW Guide (Rev1p0).
  *
  * Returns true if the configuration is valid.
  */
-static bool region_hb_rp_config_valid(const struct cxl_region *region,
-				      const struct cxl_decoder *rootd)
+static bool region_hb_rp_config_valid(struct cxl_region *region,
+				      const struct cxl_decoder *rootd,
+				      bool state_update)
 {
 	const int num_root_ports = get_num_root_ports(region);
 	struct cxl_port *hbs[CXL_DECODER_MAX_INTERLEAVE];
+	struct cxl_decoder *cxld, *c;
 	int hb_count, i;
 
 	hb_count = get_unique_hostbridges(region, hbs);
@@ -368,8 +371,25 @@ static bool region_hb_rp_config_valid(const struct cxl_region *region,
 	 * Are all devices in this region on the same CXL Host Bridge
 	 * Root Port?
 	 */
-	if (num_root_ports == 1 && !has_switch(region))
+	if (num_root_ports == 1 && !has_switch(region)) {
+		struct cxl_decoder *cxld;
+
+		if (!state_update)
+			return true;
+
+		cxld = cxl_get_decoder(hbs[0]);
+		if (!cxld) {
+			dev_dbg(&region->dev, "Couldn't get decoder for %s\n",
+				dev_name(&hbs[0]->dev));
+			return false;
+		}
+
+		cxld->interleave_ways = 1;
+		cxld->interleave_granularity = region_granularity(region);
+		cxld->target[0] = get_rp(region->config.targets[0]);
+		list_add_tail(&cxld->region_link, (struct list_head *)&region->staged_list);
 		return true;
+	}
 
 	for (i = 0; i < hb_count; i++) {
 		int idx, position_mask;
@@ -379,6 +399,19 @@ static bool region_hb_rp_config_valid(const struct cxl_region *region,
 		/* Get next CXL Host Bridge this region spans */
 		hb = hbs[i];
 
+		if (state_update) {
+			cxld = cxl_get_decoder(hbs[i]);
+			if (IS_ERR(cxld)) {
+				dev_dbg(&region->dev,
+					"Couldn't get decoder for %s\n",
+					dev_name(&hb->dev));
+				goto err;
+			}
+			cxld->interleave_ways = 0;
+		} else {
+			cxld = NULL;
+		}
+
 		/*
 		 * Calculate the position mask: NumRootPorts = 2^PositionMask
 		 * for this region.
@@ -417,9 +450,18 @@ static bool region_hb_rp_config_valid(const struct cxl_region *region,
 				}
 			}
 		}
+		if (state_update)
+			list_add_tail(&cxld->region_link, &region->staged_list);
 	}
 
 	return true;
+
+err:
+	dev_dbg(&region->dev, "Couldn't get decoder for region\n");
+	list_for_each_entry_safe(cxld, c, &region->staged_list, region_link)
+		cxl_put_decoder(cxld);
+
+	return false;
 }
 
 /**
@@ -435,7 +477,8 @@ static bool rootd_contains(const struct cxl_region *region,
 }
 
 static bool rootd_valid(const struct cxl_region *region,
-			const struct cxl_decoder *rootd)
+			const struct cxl_decoder *rootd,
+			bool state_update)
 {
 	const struct cxl_memdev *endpoint = region->config.targets[0];
 
@@ -448,7 +491,7 @@ static bool rootd_valid(const struct cxl_region *region,
 	if (!region_xhb_config_valid(region, rootd))
 		return false;
 
-	if (!region_hb_rp_config_valid(region, rootd))
+	if (!region_hb_rp_config_valid((struct cxl_region *)region, rootd, state_update))
 		return false;
 
 	if (!rootd_contains(region, rootd))
@@ -471,7 +514,7 @@ static int rootd_match(struct device *dev, void *data)
 	if (!is_root_decoder(dev))
 		return 0;
 
-	return !!rootd_valid(region, to_cxl_decoder(dev));
+	return !!rootd_valid(region, to_cxl_decoder(dev), false);
 }
 
 /*
@@ -494,12 +537,40 @@ static struct cxl_decoder *find_rootd(const struct cxl_region *region,
 	return NULL;
 }
 
-static int collect_ep_decoders(const struct cxl_region *region)
+static int collect_ep_decoders(struct cxl_region *region)
 {
-	/* TODO: */
+	struct cxl_memdev *ep;
+	int i;
+
+	for_each_cxl_endpoint(ep, region, i) {
+		struct cxl_decoder *cxld;
+
+		cxld = cxl_get_decoder(ep->port);
+		if (IS_ERR(cxld))
+			return PTR_ERR(cxld);
+
+		cxld->decoder_range = (struct range) {
+			.start = region->res->start,
+			.end = region->res->end
+		};
+		cxld->interleave_granularity = region_granularity(region);
+		cxld->interleave_ways = region_ways(region);
+		list_add_tail(&cxld->region_link, &region->staged_list);
+	}
+
 	return 0;
 }
 
+static void cleanup_staged_decoders(struct cxl_region *region)
+{
+	struct cxl_decoder *cxld, *d;
+
+	list_for_each_entry_safe(cxld, d, &region->staged_list, region_link) {
+		cxl_put_decoder(cxld);
+		list_del_init(&cxld->region_link);
+	}
+}
+
 static int bind_region(const struct cxl_region *region)
 {
 	/* TODO: */
@@ -540,7 +611,7 @@ static int cxl_region_probe(struct device *dev)
 		return -ENXIO;
 	}
 
-	if (!rootd_valid(region, rootd)) {
+	if (!rootd_valid(region, rootd, true)) {
 		dev_err(dev, "Picked invalid rootd\n");
 		return -ENXIO;
 	}
@@ -555,14 +626,18 @@ static int cxl_region_probe(struct device *dev)
 
 	ret = collect_ep_decoders(region);
 	if (ret)
-		return ret;
+		goto err;
 
 	ret = bind_region(region);
-	if (!ret) {
-		region->active = true;
-		dev_info(dev, "Bound");
-	}
+	if (ret)
+		goto err;
 
+	region->active = true;
+	dev_info(dev, "Bound");
+	return 0;
+
+err:
+	cleanup_staged_decoders(region);
 	return ret;
 }
 
diff --git a/drivers/cxl/region.h b/drivers/cxl/region.h
index 9f89f0e8744b..a7938d5090bd 100644
--- a/drivers/cxl/region.h
+++ b/drivers/cxl/region.h
@@ -14,6 +14,9 @@
  * @list: Node in decoder's region list.
  * @res: Resource this region carves out of the platform decode range.
  * @active: If the region has been activated.
+ * @staged_list: All decoders staged for programming.
+ * @commit_list: All decoders programmed for this region's parameters.
+ *
  * @config: HDM decoder program config
  * @config.size: Size of the region determined from LSA or userspace.
  * @config.uuid: The UUID for this region.
@@ -27,6 +30,8 @@ struct cxl_region {
 	struct list_head list;
 	struct resource *res;
 	bool active;
+	struct list_head staged_list;
+	struct list_head commit_list;
 
 	struct {
 		u64 size;
-- 
2.34.1


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

* [PATCH v2 12/15] cxl/region: Collect host bridge decoders
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (10 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 11/15] cxl/region: Add infrastructure for decoder programming Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 13/15] cxl: Program decoders for regions Ben Widawsky
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

Part of host bridge verification in the CXL Type 3 Memory Device
Software Guide calculates the host bridge interleave target list (6th
step in the flow chart), ie. verification and state update are done in
the same step. Host bridge verification is already in place, so go ahead
and store the decoders with their target lists.

TODO: Needs support for switches (7th step in the flow chart).

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 drivers/cxl/region.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
index cb3fc8de4c23..6d39f71b6dfa 100644
--- a/drivers/cxl/region.c
+++ b/drivers/cxl/region.c
@@ -392,6 +392,7 @@ static bool region_hb_rp_config_valid(struct cxl_region *region,
 	}
 
 	for (i = 0; i < hb_count; i++) {
+		struct cxl_decoder *cxld;
 		int idx, position_mask;
 		struct cxl_dport *rp;
 		struct cxl_port *hb;
@@ -434,10 +435,8 @@ static bool region_hb_rp_config_valid(struct cxl_region *region,
 				if (get_rp(ep) != rp)
 					continue;
 
-				if (port_grouping == -1) {
+				if (port_grouping == -1)
 					port_grouping = idx & position_mask;
-					continue;
-				}
 
 				/*
 				 * Do all devices in the region connected to this CXL
@@ -448,10 +447,32 @@ static bool region_hb_rp_config_valid(struct cxl_region *region,
 						"One or more devices are not connected to the correct Host Bridge Root Port\n");
 					return false;
 				}
+
+				if (!state_update)
+					continue;
+
+				if (dev_WARN_ONCE(&cxld->dev,
+						  port_grouping >= cxld->nr_targets,
+						  "Invalid port grouping %d/%d\n",
+						  port_grouping, cxld->nr_targets))
+					return false;
+
+				cxld->interleave_ways++;
+				cxld->target[port_grouping] = get_rp(ep);
 			}
 		}
-		if (state_update)
+
+		if (state_update) {
+			/* IG doesn't change across host bridges */
+			cxld->interleave_granularity = region_granularity(region);
+
+			cxld->decoder_range = (struct range) {
+				.start = region->res->start,
+				.end = region->res->end
+			};
+
 			list_add_tail(&cxld->region_link, &region->staged_list);
+		}
 	}
 
 	return true;
@@ -476,7 +497,7 @@ static bool rootd_contains(const struct cxl_region *region,
 	return true;
 }
 
-static bool rootd_valid(const struct cxl_region *region,
+static bool rootd_valid(struct cxl_region *region,
 			const struct cxl_decoder *rootd,
 			bool state_update)
 {
@@ -501,20 +522,20 @@ static bool rootd_valid(const struct cxl_region *region,
 }
 
 struct rootd_context {
-	const struct cxl_region *region;
-	struct cxl_port *hbs[CXL_DECODER_MAX_INTERLEAVE];
+	struct cxl_region *region;
+	const struct cxl_port *hbs[CXL_DECODER_MAX_INTERLEAVE];
 	int count;
 };
 
 static int rootd_match(struct device *dev, void *data)
 {
 	struct rootd_context *ctx = (struct rootd_context *)data;
-	const struct cxl_region *region = ctx->region;
+	struct cxl_region *region = ctx->region;
 
 	if (!is_root_decoder(dev))
 		return 0;
 
-	return !!rootd_valid(region, to_cxl_decoder(dev), false);
+	return rootd_valid(region, to_cxl_decoder(dev), false);
 }
 
 /*
@@ -528,7 +549,7 @@ static struct cxl_decoder *find_rootd(const struct cxl_region *region,
 	struct rootd_context ctx;
 	struct device *ret;
 
-	ctx.region = region;
+	ctx.region = (struct cxl_region *)region;
 
 	ret = device_find_child((struct device *)&root->dev, &ctx, rootd_match);
 	if (ret)
-- 
2.34.1


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

* [PATCH v2 13/15] cxl: Program decoders for regions
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (11 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 12/15] cxl/region: Collect host bridge decoders Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 14/15] cxl/pmem: Convert nvdimm bridge API to use memdev Ben Widawsky
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

Configure and commit the HDM decoders for the region. Since the region
driver already was able to walk the topology and build the list of
needed decoders, all that was needed to finish region setup was to
actually write the HDM decoder MMIO.

CXL regions appear as linear addresses in the system's physical address
space. CXL memory devices comprise the storage for the region. In order
for traffic to be properly routed to the memory devices in the region, a
set of Host-manged Device Memory decoders must be present. The decoders
are a piece of hardware defined in the CXL specification.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>

---
Changes since v1:
- Fix wait_for_commit (Jonathan)
- Improved commit message
- Fixed error handling
- Use devm actions for destruction
---
 drivers/cxl/core/hdm.c | 202 +++++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h      |   3 +
 drivers/cxl/region.c   |  72 ++++++++++++---
 3 files changed, 265 insertions(+), 12 deletions(-)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 44e48cea8cd4..9fcd6467f918 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -242,3 +242,205 @@ int devm_cxl_enumerate_switch_decoders(struct cxl_port *port)
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_switch_decoders, CXL);
+
+#define COMMIT_TIMEOUT_MS 10
+static int wait_for_commit(struct cxl_decoder *cxld)
+{
+	const unsigned long end = jiffies + msecs_to_jiffies(COMMIT_TIMEOUT_MS);
+	struct cxl_port *port = to_cxl_port(cxld->dev.parent);
+	struct cxl_port_state *cxlps;
+	void __iomem *hdm_decoder;
+	u32 ctrl;
+
+	cxlps = dev_get_drvdata(&port->dev);
+	hdm_decoder = cxlps->regs.hdm_decoder;
+
+	while (1) {
+		ctrl = readl(hdm_decoder +
+			     CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
+		if (FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl))
+			break;
+
+		if (time_after(jiffies, end)) {
+			dev_err(&cxld->dev, "HDM decoder commit timeout %x\n", ctrl);
+			return -ETIMEDOUT;
+		}
+		if ((ctrl & CXL_HDM_DECODER0_CTRL_COMMIT_ERROR) != 0) {
+			dev_err(&cxld->dev, "HDM decoder commit error %x\n", ctrl);
+			return -ENXIO;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * cxl_commit_decoder() - Program a configured cxl_decoder
+ * @cxld: The preconfigured cxl decoder.
+ *
+ * A cxl decoder that is to be committed should have been earmarked as enabled.
+ * This mechanism acts as a soft reservation on the decoder.
+ *
+ * Returns 0 if commit was successful, negative error code otherwise.
+ */
+int cxl_commit_decoder(struct cxl_decoder *cxld)
+{
+	u32 ctrl, tl_lo, tl_hi, base_lo, base_hi, size_lo, size_hi;
+	struct cxl_port *port = to_cxl_port(cxld->dev.parent);
+	struct cxl_port_state *cxlps;
+	void __iomem *hdm_decoder;
+	int rc;
+
+	/*
+	 * Decoder flags are entirely software controlled and therefore this
+	 * case is purely a driver bug.
+	 */
+	if (dev_WARN_ONCE(&port->dev, (cxld->flags & CXL_DECODER_F_ENABLE) != 0,
+			  "Invalid %s enable state\n", dev_name(&cxld->dev)))
+		return -ENXIO;
+
+	cxlps = dev_get_drvdata(&port->dev);
+	hdm_decoder = cxlps->regs.hdm_decoder;
+	ctrl = readl(hdm_decoder + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
+
+	/*
+	 * A decoder that's currently active cannot be changed without the
+	 * system being quiesced. While the driver should prevent against this,
+	 * for a variety of reasons the hardware might not be in sync with the
+	 * hardware and so, do not splat on error.
+	 */
+	size_hi = readl(hdm_decoder +
+			CXL_HDM_DECODER0_SIZE_HIGH_OFFSET(cxld->id));
+	size_lo =
+		readl(hdm_decoder + CXL_HDM_DECODER0_SIZE_LOW_OFFSET(cxld->id));
+	if (FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl) &&
+	    (size_lo + size_hi)) {
+		dev_err(&port->dev, "Tried to change an active decoder (%s)\n",
+			dev_name(&cxld->dev));
+		return -EBUSY;
+	}
+
+	u32p_replace_bits(&ctrl, cxl_to_ig(cxld->interleave_granularity),
+			  CXL_HDM_DECODER0_CTRL_IG_MASK);
+	u32p_replace_bits(&ctrl, cxl_to_eniw(cxld->interleave_ways),
+			  CXL_HDM_DECODER0_CTRL_IW_MASK);
+	u32p_replace_bits(&ctrl, 1, CXL_HDM_DECODER0_CTRL_COMMIT);
+
+	/* TODO: set based on type */
+	u32p_replace_bits(&ctrl, 1, CXL_HDM_DECODER0_CTRL_TYPE);
+
+	base_lo = FIELD_PREP(GENMASK(31, 28),
+			     (u32)(cxld->decoder_range.start & 0xffffffff));
+	base_hi = FIELD_PREP(~0, (u32)(cxld->decoder_range.start >> 32));
+
+	size_lo = (u32)(range_len(&cxld->decoder_range)) & GENMASK(31, 28);
+	size_hi = (u32)((range_len(&cxld->decoder_range) >> 32));
+
+	if (cxld->nr_targets > 0) {
+		tl_lo |= FIELD_PREP(GENMASK(7, 0), cxld->target[0]->port_id);
+		if (cxld->interleave_ways > 1)
+			tl_lo |= FIELD_PREP(GENMASK(15, 8),
+					    cxld->target[1]->port_id);
+		if (cxld->interleave_ways > 2)
+			tl_lo |= FIELD_PREP(GENMASK(23, 16),
+					    cxld->target[2]->port_id);
+		if (cxld->interleave_ways > 3)
+			tl_lo |= FIELD_PREP(GENMASK(31, 24),
+					    cxld->target[3]->port_id);
+		if (cxld->interleave_ways > 4)
+			tl_hi |= FIELD_PREP(GENMASK(7, 0),
+					    cxld->target[4]->port_id);
+		if (cxld->interleave_ways > 5)
+			tl_hi |= FIELD_PREP(GENMASK(15, 8),
+					    cxld->target[5]->port_id);
+		if (cxld->interleave_ways > 6)
+			tl_hi |= FIELD_PREP(GENMASK(23, 16),
+					    cxld->target[6]->port_id);
+		if (cxld->interleave_ways > 7)
+			tl_hi |= FIELD_PREP(GENMASK(31, 24),
+					    cxld->target[7]->port_id);
+
+		writel(tl_hi, hdm_decoder + CXL_HDM_DECODER0_TL_HIGH(cxld->id));
+		writel(tl_lo, hdm_decoder + CXL_HDM_DECODER0_TL_LOW(cxld->id));
+	}
+
+	writel(size_hi,
+	       hdm_decoder + CXL_HDM_DECODER0_SIZE_HIGH_OFFSET(cxld->id));
+	writel(size_lo,
+	       hdm_decoder + CXL_HDM_DECODER0_SIZE_LOW_OFFSET(cxld->id));
+	writel(base_hi,
+	       hdm_decoder + CXL_HDM_DECODER0_BASE_HIGH_OFFSET(cxld->id));
+	writel(base_lo,
+	       hdm_decoder + CXL_HDM_DECODER0_BASE_LOW_OFFSET(cxld->id));
+	writel(ctrl, hdm_decoder + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
+
+	rc = wait_for_commit(cxld);
+	if (rc)
+		return rc;
+
+	cxld->flags |= CXL_DECODER_F_ENABLE;
+
+#define DPORT_TL_STR "%d %d %d %d %d %d %d %d"
+#define DPORT(i)                                                               \
+	(cxld->nr_targets && cxld->interleave_ways > (i)) ?                    \
+		      cxld->target[(i)]->port_id :                                   \
+		      -1
+#define DPORT_TL                                                               \
+	DPORT(0), DPORT(1), DPORT(2), DPORT(3), DPORT(4), DPORT(5), DPORT(6),  \
+		DPORT(7)
+
+	dev_dbg(&port->dev,
+		"%s\n\tBase %pa\n\tSize %llu\n\tIG %u (%ub)\n\tENIW %u (x%u)\n\tTargetList: \n"
+		DPORT_TL_STR,
+		dev_name(&cxld->dev),
+		&cxld->decoder_range.start,
+		range_len(&cxld->decoder_range),
+		cxl_to_ig(cxld->interleave_granularity),
+		cxld->interleave_granularity,
+		cxl_to_eniw(cxld->interleave_ways),
+		cxld->interleave_ways,
+		DPORT_TL);
+#undef DPORT_TL
+#undef DPORT
+#undef DPORT_TL_STR
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cxl_commit_decoder);
+
+/**
+ * cxl_disable_decoder() - Disables a decoder
+ * @cxld: The active cxl decoder.
+ *
+ * CXL decoders (as of 2.0 spec) have no way to deactivate them other than to
+ * set the size of the HDM to 0. This function will clear all registers, and if
+ * the decoder is active, commit the 0'd out registers.
+ */
+void cxl_disable_decoder(struct cxl_decoder *cxld)
+{
+	struct cxl_port *port = to_cxl_port(cxld->dev.parent);
+	struct cxl_port_state *cxlps;
+	void __iomem *hdm_decoder;
+	u32 ctrl;
+
+	cxlps = dev_get_drvdata(&port->dev);
+	hdm_decoder = cxlps->regs.hdm_decoder;
+	ctrl = readl(hdm_decoder + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
+
+	if (dev_WARN_ONCE(&port->dev, (cxld->flags & CXL_DECODER_F_ENABLE) == 0,
+			  "Invalid decoder enable state\n"))
+		return;
+
+	/* There's no way to "uncommit" a committed decoder, only 0 size it */
+	writel(0, hdm_decoder + CXL_HDM_DECODER0_TL_HIGH(cxld->id));
+	writel(0, hdm_decoder + CXL_HDM_DECODER0_TL_LOW(cxld->id));
+	writel(0, hdm_decoder + CXL_HDM_DECODER0_SIZE_HIGH_OFFSET(cxld->id));
+	writel(0, hdm_decoder + CXL_HDM_DECODER0_SIZE_LOW_OFFSET(cxld->id));
+	writel(0, hdm_decoder + CXL_HDM_DECODER0_BASE_HIGH_OFFSET(cxld->id));
+	writel(0, hdm_decoder + CXL_HDM_DECODER0_BASE_LOW_OFFSET(cxld->id));
+
+	/* If the device isn't actually active, just zero out all the fields */
+	if (FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl))
+		writel(CXL_HDM_DECODER0_CTRL_COMMIT,
+		       hdm_decoder + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
+}
+EXPORT_SYMBOL_GPL(cxl_disable_decoder);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 81c35be13416..1130165dfc8d 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -54,6 +54,7 @@
 #define   CXL_HDM_DECODER0_CTRL_IW_MASK GENMASK(7, 4)
 #define   CXL_HDM_DECODER0_CTRL_COMMIT BIT(9)
 #define   CXL_HDM_DECODER0_CTRL_COMMITTED BIT(10)
+#define   CXL_HDM_DECODER0_CTRL_COMMIT_ERROR BIT(11)
 #define   CXL_HDM_DECODER0_CTRL_TYPE BIT(12)
 #define CXL_HDM_DECODER0_TL_LOW(i) (0x20 * (i) + 0x24)
 #define CXL_HDM_DECODER0_TL_HIGH(i) (0x20 * (i) + 0x28)
@@ -377,6 +378,8 @@ int devm_cxl_add_dport(struct cxl_port *port, struct device *dport, int port_id,
 struct cxl_dport *cxl_find_dport_by_dev(struct cxl_port *port,
 					const struct device *dev);
 struct cxl_port *ep_find_cxl_port(struct cxl_memdev *cxlmd, unsigned int depth);
+int cxl_commit_decoder(struct cxl_decoder *cxld);
+void cxl_disable_decoder(struct cxl_decoder *cxld);
 
 struct cxl_decoder *to_cxl_decoder(struct device *dev);
 bool is_cxl_decoder(struct device *dev);
diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
index 6d39f71b6dfa..d00305655f5a 100644
--- a/drivers/cxl/region.c
+++ b/drivers/cxl/region.c
@@ -167,6 +167,8 @@ static int allocate_address_space(struct cxl_region *region)
 		return -ENOMEM;
 	}
 
+	dev_dbg(&region->dev, "resource %pR", region->res);
+
 	return devm_add_action_or_reset(&region->dev, release_cxl_region,
 					region);
 }
@@ -592,10 +594,49 @@ static void cleanup_staged_decoders(struct cxl_region *region)
 	}
 }
 
-static int bind_region(const struct cxl_region *region)
+static int bind_region(struct cxl_region *region)
 {
-	/* TODO: */
-	return 0;
+	struct cxl_decoder *cxld, *d;
+	int rc;
+
+	list_for_each_entry_safe(cxld, d, &region->staged_list, region_link) {
+		rc = cxl_commit_decoder(cxld);
+		if (!rc) {
+			list_move_tail(&cxld->region_link, &region->commit_list);
+		} else {
+			dev_dbg(&region->dev, "Failed to commit %s\n",
+				dev_name(&cxld->dev));
+			break;
+		}
+	}
+
+	list_for_each_entry_safe(cxld, d, &region->commit_list, region_link) {
+		if (rc)
+			cxl_disable_decoder(cxld);
+		list_del(&cxld->region_link);
+	}
+
+	if (rc)
+		cleanup_staged_decoders((struct cxl_region *)region);
+
+	BUG_ON(!list_empty(&region->staged_list));
+	return rc;
+}
+
+static void region_unregister(void *dev)
+{
+	struct cxl_region *region = to_cxl_region(dev);
+	struct cxl_decoder *cxld, *d;
+
+	if (dev_WARN_ONCE(dev, !list_empty(&region->staged_list),
+			  "Decoders still staged"))
+		cleanup_staged_decoders(region);
+
+	list_for_each_entry_safe(cxld, d, &region->commit_list, region_link) {
+		cxl_disable_decoder(cxld);
+		list_del(&cxld->region_link);
+		cxl_put_decoder(cxld);
+	}
 }
 
 static int cxl_region_probe(struct device *dev)
@@ -646,20 +687,27 @@ static int cxl_region_probe(struct device *dev)
 		put_device(&ours->dev);
 
 	ret = collect_ep_decoders(region);
-	if (ret)
-		goto err;
+	if (ret) {
+		cleanup_staged_decoders(region);
+		return ret;
+	}
 
 	ret = bind_region(region);
-	if (ret)
-		goto err;
+	if (ret) {
+		/* bind_region should cleanup after itself */
+		if (dev_WARN_ONCE(dev, !list_empty(&region->staged_list),
+				  "Region bind failed to cleanup staged decoders\n"))
+			cleanup_staged_decoders(region);
+		if (dev_WARN_ONCE(dev, !list_empty(&region->commit_list),
+				  "Region bind failed to cleanup committed decoders\n"))
+			region_unregister(&region->dev);
+		return ret;
+	}
+
 
 	region->active = true;
 	dev_info(dev, "Bound");
-	return 0;
-
-err:
-	cleanup_staged_decoders(region);
-	return ret;
+	return devm_add_action_or_reset(dev, region_unregister, dev);
 }
 
 static struct cxl_driver cxl_region_driver = {
-- 
2.34.1


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

* [PATCH v2 14/15] cxl/pmem: Convert nvdimm bridge API to use memdev
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (12 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 13/15] cxl: Program decoders for regions Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-12 23:47 ` [PATCH v2 15/15] cxl/region: Create an nd_region Ben Widawsky
  2022-01-15 19:00 ` [PATCH v2 00/15] CXL Region driver Ben Widawsky
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

The cxl_pmem driver specific cxl_nvdimm structure isn't a suitable
parameter for an exported API that can be used by other drivers.
Instead, use a memdev structure, which should be woven into any caller
using this API.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 drivers/cxl/core/pmem.c | 3 +--
 drivers/cxl/cxl.h       | 2 +-
 drivers/cxl/pmem.c      | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/cxl/core/pmem.c b/drivers/cxl/core/pmem.c
index f21e5ce9619a..bfcf51fbda5d 100644
--- a/drivers/cxl/core/pmem.c
+++ b/drivers/cxl/core/pmem.c
@@ -62,9 +62,8 @@ static int match_nvdimm_bridge(struct device *dev, void *data)
 	return is_cxl_nvdimm_bridge(dev);
 }
 
-struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_nvdimm *cxl_nvd)
+struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_memdev *cxlmd)
 {
-	struct cxl_memdev *cxlmd = cxl_nvd->cxlmd;
 	struct cxl_port *port;
 	struct device *dev;
 
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 1130165dfc8d..6f9cabb77c08 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -433,7 +433,7 @@ struct cxl_nvdimm *to_cxl_nvdimm(struct device *dev);
 bool is_cxl_nvdimm(struct device *dev);
 bool is_cxl_nvdimm_bridge(struct device *dev);
 int devm_cxl_add_nvdimm(struct device *host, struct cxl_memdev *cxlmd);
-struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_nvdimm *cxl_nvd);
+struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_memdev *cxlmd);
 
 /*
  * Unit test builds overrides this to __weak, find the 'strong' version
diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
index b65a272a2d6d..420ace433a01 100644
--- a/drivers/cxl/pmem.c
+++ b/drivers/cxl/pmem.c
@@ -39,7 +39,7 @@ static int cxl_nvdimm_probe(struct device *dev)
 	struct nvdimm *nvdimm;
 	int rc;
 
-	cxl_nvb = cxl_find_nvdimm_bridge(cxl_nvd);
+	cxl_nvb = cxl_find_nvdimm_bridge(cxl_nvd->cxlmd);
 	if (!cxl_nvb)
 		return -ENXIO;
 
-- 
2.34.1


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

* [PATCH v2 15/15] cxl/region: Create an nd_region
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (13 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 14/15] cxl/pmem: Convert nvdimm bridge API to use memdev Ben Widawsky
@ 2022-01-12 23:47 ` Ben Widawsky
  2022-01-15 19:00 ` [PATCH v2 00/15] CXL Region driver Ben Widawsky
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-12 23:47 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Ben Widawsky, Alison Schofield,
	Dan Williams, Ira Weiny, Jonathan Cameron, Vishal Verma

LIBNVDIMM supports the creation of regions for both persistent and
volatile memory ranges. The cxl_region driver is capable of handling the
CXL side of region creation but will reuse LIBVDIMM for interfacing with
the rest of the kernel.

TODO: CXL regions can go away. As a result the nd_region must also be
torn down.

TODO2: Handle mappings. LIBNVDIMM is capable of being informed about
which parts of devices contribute to a region and validating whether or
not the region is configured properly. To do this properly requires
tracking allocations per device.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 drivers/cxl/core/pmem.c | 16 +++++++++++++
 drivers/cxl/cxl.h       |  1 +
 drivers/cxl/region.c    | 52 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/drivers/cxl/core/pmem.c b/drivers/cxl/core/pmem.c
index bfcf51fbda5d..762a08c6f073 100644
--- a/drivers/cxl/core/pmem.c
+++ b/drivers/cxl/core/pmem.c
@@ -213,6 +213,22 @@ struct cxl_nvdimm *to_cxl_nvdimm(struct device *dev)
 }
 EXPORT_SYMBOL_NS_GPL(to_cxl_nvdimm, CXL);
 
+static int match_cxl_nvdimm(struct device *dev, void *data)
+{
+	return is_cxl_nvdimm(dev);
+}
+
+struct cxl_nvdimm *cxl_find_nvdimm(struct cxl_memdev *cxlmd)
+{
+	struct device *dev;
+
+	dev = device_find_child(&cxlmd->dev, NULL, match_cxl_nvdimm);
+	if (!dev)
+		return NULL;
+	return to_cxl_nvdimm(dev);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_find_nvdimm, CXL);
+
 static struct cxl_nvdimm *cxl_nvdimm_alloc(struct cxl_memdev *cxlmd)
 {
 	struct cxl_nvdimm *cxl_nvd;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 6f9cabb77c08..a7b90356914d 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -434,6 +434,7 @@ bool is_cxl_nvdimm(struct device *dev);
 bool is_cxl_nvdimm_bridge(struct device *dev);
 int devm_cxl_add_nvdimm(struct device *host, struct cxl_memdev *cxlmd);
 struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_memdev *cxlmd);
+struct cxl_nvdimm *cxl_find_nvdimm(struct cxl_memdev *cxlmd);
 
 /*
  * Unit test builds overrides this to __weak, find the 'strong' version
diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
index d00305655f5a..d4a7e8d47c11 100644
--- a/drivers/cxl/region.c
+++ b/drivers/cxl/region.c
@@ -623,6 +623,52 @@ static int bind_region(struct cxl_region *region)
 	return rc;
 }
 
+static int connect_to_libnvdimm(struct cxl_region *region)
+{
+	struct nd_region_desc ndr_desc;
+	struct cxl_nvdimm_bridge *nvb;
+	struct nd_region *ndr;
+	int rc = 0;
+
+	nvb = cxl_find_nvdimm_bridge(region->config.targets[0]);
+	device_lock(&nvb->dev);
+	if (!nvb->nvdimm_bus) {
+		rc = -ENXIO;
+		goto out;
+	}
+
+	memset(&ndr_desc, 0, sizeof(ndr_desc));
+
+	ndr_desc.res = region->res;
+
+	ndr_desc.numa_node = memory_add_physaddr_to_nid(region->res->start);
+	ndr_desc.target_node = phys_to_target_node(region->res->start);
+	if (ndr_desc.numa_node == NUMA_NO_NODE) {
+		ndr_desc.numa_node =
+			memory_add_physaddr_to_nid(region->res->start);
+		dev_info(&region->dev,
+			 "changing numa node from %d to %d for CXL region %pR",
+			 NUMA_NO_NODE, ndr_desc.numa_node, region->res);
+	}
+	if (ndr_desc.target_node == NUMA_NO_NODE) {
+		ndr_desc.target_node = ndr_desc.numa_node;
+		dev_info(&region->dev,
+			 "changing target node from %d to %d for CXL region %pR",
+			 NUMA_NO_NODE, ndr_desc.target_node, region->res);
+	}
+
+	ndr = nvdimm_pmem_region_create(nvb->nvdimm_bus, &ndr_desc);
+	if (IS_ERR(ndr))
+		rc = PTR_ERR(ndr);
+	else
+		dev_set_drvdata(&region->dev, ndr);
+
+out:
+	device_unlock(&nvb->dev);
+	put_device(&nvb->dev);
+	return rc;
+}
+
 static void region_unregister(void *dev)
 {
 	struct cxl_region *region = to_cxl_region(dev);
@@ -704,6 +750,12 @@ static int cxl_region_probe(struct device *dev)
 		return ret;
 	}
 
+	ret = connect_to_libnvdimm(region);
+	if (ret) {
+		region_unregister(dev);
+		return ret;
+	}
+
 
 	region->active = true;
 	dev_info(dev, "Bound");
-- 
2.34.1


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

* Re: [PATCH v2 00/15] CXL Region driver
  2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
                   ` (14 preceding siblings ...)
  2022-01-12 23:47 ` [PATCH v2 15/15] cxl/region: Create an nd_region Ben Widawsky
@ 2022-01-15 19:00 ` Ben Widawsky
  15 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2022-01-15 19:00 UTC (permalink / raw)
  To: linux-cxl, nvdimm, linux-pci
  Cc: patches, Bjorn Helgaas, Alison Schofield, Dan Williams,
	Ira Weiny, Jonathan Cameron, Vishal Verma

Dang. Responded to the wrong thread...

v3 coming. Ignore this please.

Thanks.
Ben

On 22-01-12 15:47:34, Ben Widawsky wrote:
> Major changes since v1:
> - bug fixes in certain calculations for region programming
> - bug fix in for_each_cxl_decoder_target
> - clarify ENIW and IG from ways and granularity
> - wait_for_commit bug fix
> - use devm management for region removal
> - remove trace points
> - add basic libnvdimm connection
> 
> Original commit message follows with minor updates for correctness.
> 
> ---
> 
> This patch series introduces the CXL region driver as well as associated APIs in
> CXL core. The region driver enables the creation of "regions" which is a concept
> defined by the CXL 2.0 specification [1]. Region verification and programming
> state are owned by the cxl_region driver (implemented in the cxl_region module).
> It relies on cxl_mem to determine if devices are CXL routed, and cxl_port to
> actually handle the programming of the HDM decoders. Much of the region driver
> is an implementation of algorithms described in the CXL Type 3 Memory Device
> Software Guide [2].
> 
> The region driver will be responsible for configuring regions found on
> persistent capacities in the Label Storage Area (LSA), it will also enumerate
> regions configured by BIOS, usually volatile capacities, and will allow for
> dynamic region creation (which can then be stored in the LSA). It is the primary
> consumer of the CXL Port [3] and CXL Mem drivers introduced previously [4]. Dan
> has reworked those drivers which is a requirement for this branch. A cached copy
> is included in the gitlab for this project [5]. Those patches will be posted
> shortly.
> 
> The patches for the region driver could be squashed. They're broken out to aid
> review and because that's the order they were implemented in. My preference is
> to keep those as they are.
> 
> Some things are still missing and will be worked on while these are reviewed (in
> priority order):
> 1. Volatile regions/LSA regions (Have a plan)
> 2. Switch ports (Have a plan)
> 3. Decoder programming restrictions (No plan). The one know restriction I've
>    missed is to disallow programming HDM decoders that aren't in incremental
>    system physical address ranges.
> 4. CXL region teardown -> nd_region teardown
> 5. Stress testing
> 
> Here is an example of output when programming a x2 interleave region
> 
> # ./cxlctl create-region -n -a -s $((256<<20)) /sys/bus/cxl/devices/decoder0.0
> [   57.564475][  T654] cxl_core:cxl_add_region:478: cxl region0.0:0: Added region0.0:0 to decoder0.0
> [   57.608949][  T655] cxl_region:allocate_address_space:170: cxl_region region0.0:0: resource [mem 0x4c00000000-0x4c1fffffff]
> [   57.610056][  T655] cxl_core:cxl_commit_decoder:394: cxl_port port1: decoder1.0
> [   57.610056][  T655]  Base 0x0000004c00000000
> [   57.610056][  T655]  Size 512M
> [   57.610056][  T655]  IG 0 (256b)
> [   57.610056][  T655]  ENIW 1 (x2)
> [   57.610056][  T655]  TargetList: 0 1 -1 -1 -1 -1 -1 -1
> [   57.613584][  T655] cxl_core:cxl_commit_decoder:394: cxl_port endpoint2: decoder2.0
> [   57.613584][  T655]  Base 0x0000004c00000000
> [   57.613584][  T655]  Size 512M
> [   57.613584][  T655]  IG 0 (256b)
> [   57.613584][  T655]  ENIW 1 (x2)
> [   57.613584][  T655]  TargetList: -1 -1 -1 -1 -1 -1 -1 -1
> [   57.617051][  T655] cxl_core:cxl_commit_decoder:394: cxl_port endpoint3: decoder3.0
> [   57.617051][  T655]  Base 0x0000004c00000000
> [   57.617051][  T655]  Size 512M
> [   57.617051][  T655]  IG 0 (256b)
> [   57.617051][  T655]  ENIW 1 (x2)
> [   57.617051][  T655]  TargetList: -1 -1 -1 -1 -1 -1 -1 -1
> [   57.619433][  T655] cxl_region region0.0:0: Bound
> [   57.621435][  T655] cxl_core:cxl_bus_probe:1384: cxl_region region0.0:0: probe: 0
> 
> If you're wondering how I tested this, I've baked it into my cxlctl app [6] and
> lib [7]. Eventually this will get absorbed by ndctl/cxl-cli/libcxl.
> 
> 
> Branch can be found at gitlab [8].
> 
> ---
> 
> [1]: https://www.computeexpresslink.org/download-the-specification
> [2]: https://cdrdv2.intel.com/v1/dl/getContent/643805?wapkw=CXL%20memory%20device%20sw%20guide
> [3]: https://lore.kernel.org/linux-cxl/20211022183709.1199701-9-ben.widawsky@intel.com/
> [4]: https://lore.kernel.org/linux-cxl/20211022183709.1199701-17-ben.widawsky@intel.com/
> [5]: https://gitlab.com/bwidawsk/linux/-/commit/126793e22427f7975a8f2fca373764be78012e88
> [6]: https://gitlab.com/bwidawsk-cxl/cxlctl
> [7]: https://gitlab.com/bwidawsk-cxl/cxl_rs
> [8]: https://gitlab.com/bwidawsk/linux/-/tree/cxl_region-v2
> 
> Ben Widawsky (15):
>   cxl/core: Rename find_cxl_port
>   cxl/core: Track port depth
>   cxl/region: Add region creation ABI
>   cxl/region: Introduce concept of region configuration
>   cxl/mem: Cache port created by the mem dev
>   cxl/region: Introduce a cxl_region driver
>   cxl/acpi: Handle address space allocation
>   cxl/region: Address space allocation
>   cxl/region: Implement XHB verification
>   cxl/region: HB port config verification
>   cxl/region: Add infrastructure for decoder programming
>   cxl/region: Collect host bridge decoders
>   cxl: Program decoders for regions
>   cxl/pmem: Convert nvdimm bridge API to use memdev
>   cxl/region: Create an nd_region
> 
>  .clang-format                                 |   3 +
>  Documentation/ABI/testing/sysfs-bus-cxl       |  63 ++
>  .../driver-api/cxl/memory-devices.rst         |  14 +
>  drivers/cxl/Makefile                          |   2 +
>  drivers/cxl/acpi.c                            |  30 +
>  drivers/cxl/core/Makefile                     |   1 +
>  drivers/cxl/core/core.h                       |   4 +
>  drivers/cxl/core/hdm.c                        | 199 +++++
>  drivers/cxl/core/pmem.c                       |  19 +-
>  drivers/cxl/core/port.c                       | 132 ++-
>  drivers/cxl/core/region.c                     | 525 ++++++++++++
>  drivers/cxl/cxl.h                             |  48 +-
>  drivers/cxl/cxlmem.h                          |   9 +
>  drivers/cxl/mem.c                             |  16 +-
>  drivers/cxl/pmem.c                            |   2 +-
>  drivers/cxl/port.c                            |  42 +-
>  drivers/cxl/region.c                          | 769 ++++++++++++++++++
>  drivers/cxl/region.h                          |  47 ++
>  tools/testing/cxl/Kbuild                      |   1 +
>  19 files changed, 1903 insertions(+), 23 deletions(-)
>  create mode 100644 drivers/cxl/core/region.c
>  create mode 100644 drivers/cxl/region.c
>  create mode 100644 drivers/cxl/region.h
> 
> -- 
> 2.34.1
> 

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

end of thread, other threads:[~2022-01-15 19:01 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-12 23:47 [PATCH v2 00/15] CXL Region driver Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 01/15] cxl/core: Rename find_cxl_port Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 02/15] cxl/core: Track port depth Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 03/15] cxl/region: Add region creation ABI Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 04/15] cxl/region: Introduce concept of region configuration Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 05/15] cxl/mem: Cache port created by the mem dev Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 06/15] cxl/region: Introduce a cxl_region driver Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 07/15] cxl/acpi: Handle address space allocation Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 08/15] cxl/region: Address " Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 09/15] cxl/region: Implement XHB verification Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 10/15] cxl/region: HB port config verification Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 11/15] cxl/region: Add infrastructure for decoder programming Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 12/15] cxl/region: Collect host bridge decoders Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 13/15] cxl: Program decoders for regions Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 14/15] cxl/pmem: Convert nvdimm bridge API to use memdev Ben Widawsky
2022-01-12 23:47 ` [PATCH v2 15/15] cxl/region: Create an nd_region Ben Widawsky
2022-01-15 19:00 ` [PATCH v2 00/15] CXL Region driver Ben Widawsky

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