Re: [PATCH 05/14] cxl/mem: Register CXL memX devices

2021-02-02 Thread Christoph Hellwig
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 25e08e5f40bd..33432a4cbe23 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3179,6 +3179,20 @@ struct device *get_device(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(get_device);
>  
> +/**
> + * get_live_device() - increment reference count for device iff !dead
> + * @dev: device.
> + *
> + * Forward the call to get_device() if the device is still alive. If
> + * this is called with the device_lock() held then the device is
> + * guaranteed to not die until the device_lock() is dropped.
> + */
> +struct device *get_live_device(struct device *dev)
> +{
> + return dev && !dev->p->dead ? get_device(dev) : NULL;
> +}
> +EXPORT_SYMBOL_GPL(get_live_device);

Err, if you want to add new core functionality that needs to be in a
separate well documented prep patch, and also CCed to the relevant
maintainers.

>   mutex_unlock(&cxlm->mbox.mutex);
>  }
>  
> +static int cxl_memdev_open(struct inode *inode, struct file *file)
> +{
> + struct cxl_memdev *cxlmd =
> + container_of(inode->i_cdev, typeof(*cxlmd), cdev);
> +
> + file->private_data = cxlmd;

There is no good reason to ever mirror stuff from the inode into
file->private_data, as you can just trivially get at the original
location using file_inode(file).


Re: [PATCH 05/14] cxl/mem: Register CXL memX devices

2021-02-01 Thread Dan Williams
On Mon, Feb 1, 2021 at 1:53 PM David Rientjes  wrote:
>
> On Mon, 1 Feb 2021, Ben Widawsky wrote:
>
> > > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl 
> > > > b/Documentation/ABI/testing/sysfs-bus-cxl
> > > > new file mode 100644
> > > > index ..fe7b87eba988
> > > > --- /dev/null
> > > > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > > > @@ -0,0 +1,26 @@
> > > > +What:/sys/bus/cxl/devices/memX/firmware_version
> > > > +Date:December, 2020
> > > > +KernelVersion:   v5.12
> > > > +Contact: linux-...@vger.kernel.org
> > > > +Description:
> > > > + (RO) "FW Revision" string as reported by the Identify
> > > > + Memory Device Output Payload in the CXL-2.0
> > > > + specification.
> > > > +
> > > > +What:/sys/bus/cxl/devices/memX/ram/size
> > > > +Date:December, 2020
> > > > +KernelVersion:   v5.12
> > > > +Contact: linux-...@vger.kernel.org
> > > > +Description:
> > > > + (RO) "Volatile Only Capacity" as reported by the
> > > > + Identify Memory Device Output Payload in the CXL-2.0
> > > > + specification.
> > > > +
> > > > +What:/sys/bus/cxl/devices/memX/pmem/size
> > > > +Date:December, 2020
> > > > +KernelVersion:   v5.12
> > > > +Contact: linux-...@vger.kernel.org
> > > > +Description:
> > > > + (RO) "Persistent Only Capacity" as reported by the
> > > > + Identify Memory Device Output Payload in the CXL-2.0
> > > > + specification.
> > >
> > > Aren't volatile and persistent capacities expressed in multiples of 256MB?
> >
> > As of the spec today, volatile and persistent capacities are required to be
> > in multiples of 256MB, however, future specs may not have such a 
> > requirement and
> > I think keeping sysfs ABI easily forward portable makes sense.
> >
>
> Makes sense, can we add that these are expressed in bytes or is that
> already implied?

Makes sense to declare units here.


Re: [PATCH 05/14] cxl/mem: Register CXL memX devices

2021-02-01 Thread David Rientjes
On Mon, 1 Feb 2021, Ben Widawsky wrote:

> > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl 
> > > b/Documentation/ABI/testing/sysfs-bus-cxl
> > > new file mode 100644
> > > index ..fe7b87eba988
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > > @@ -0,0 +1,26 @@
> > > +What:/sys/bus/cxl/devices/memX/firmware_version
> > > +Date:December, 2020
> > > +KernelVersion:   v5.12
> > > +Contact: linux-...@vger.kernel.org
> > > +Description:
> > > + (RO) "FW Revision" string as reported by the Identify
> > > + Memory Device Output Payload in the CXL-2.0
> > > + specification.
> > > +
> > > +What:/sys/bus/cxl/devices/memX/ram/size
> > > +Date:December, 2020
> > > +KernelVersion:   v5.12
> > > +Contact: linux-...@vger.kernel.org
> > > +Description:
> > > + (RO) "Volatile Only Capacity" as reported by the
> > > + Identify Memory Device Output Payload in the CXL-2.0
> > > + specification.
> > > +
> > > +What:/sys/bus/cxl/devices/memX/pmem/size
> > > +Date:December, 2020
> > > +KernelVersion:   v5.12
> > > +Contact: linux-...@vger.kernel.org
> > > +Description:
> > > + (RO) "Persistent Only Capacity" as reported by the
> > > + Identify Memory Device Output Payload in the CXL-2.0
> > > + specification.
> > 
> > Aren't volatile and persistent capacities expressed in multiples of 256MB?
> 
> As of the spec today, volatile and persistent capacities are required to be
> in multiples of 256MB, however, future specs may not have such a requirement 
> and
> I think keeping sysfs ABI easily forward portable makes sense.
> 

Makes sense, can we add that these are expressed in bytes or is that 
already implied?


Re: [PATCH 05/14] cxl/mem: Register CXL memX devices

2021-02-01 Thread Ben Widawsky
On 21-01-30 15:52:01, David Rientjes wrote:
> On Fri, 29 Jan 2021, Ben Widawsky wrote:
> 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl 
> > b/Documentation/ABI/testing/sysfs-bus-cxl
> > new file mode 100644
> > index ..fe7b87eba988
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > @@ -0,0 +1,26 @@
> > +What:  /sys/bus/cxl/devices/memX/firmware_version
> > +Date:  December, 2020
> > +KernelVersion: v5.12
> > +Contact:   linux-...@vger.kernel.org
> > +Description:
> > +   (RO) "FW Revision" string as reported by the Identify
> > +   Memory Device Output Payload in the CXL-2.0
> > +   specification.
> > +
> > +What:  /sys/bus/cxl/devices/memX/ram/size
> > +Date:  December, 2020
> > +KernelVersion: v5.12
> > +Contact:   linux-...@vger.kernel.org
> > +Description:
> > +   (RO) "Volatile Only Capacity" as reported by the
> > +   Identify Memory Device Output Payload in the CXL-2.0
> > +   specification.
> > +
> > +What:  /sys/bus/cxl/devices/memX/pmem/size
> > +Date:  December, 2020
> > +KernelVersion: v5.12
> > +Contact:   linux-...@vger.kernel.org
> > +Description:
> > +   (RO) "Persistent Only Capacity" as reported by the
> > +   Identify Memory Device Output Payload in the CXL-2.0
> > +   specification.
> 
> Aren't volatile and persistent capacities expressed in multiples of 256MB?

As of the spec today, volatile and persistent capacities are required to be
in multiples of 256MB, however, future specs may not have such a requirement and
I think keeping sysfs ABI easily forward portable makes sense.


Re: [PATCH 05/14] cxl/mem: Register CXL memX devices

2021-01-30 Thread David Rientjes
On Fri, 29 Jan 2021, Ben Widawsky wrote:

> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl 
> b/Documentation/ABI/testing/sysfs-bus-cxl
> new file mode 100644
> index ..fe7b87eba988
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -0,0 +1,26 @@
> +What:/sys/bus/cxl/devices/memX/firmware_version
> +Date:December, 2020
> +KernelVersion:   v5.12
> +Contact: linux-...@vger.kernel.org
> +Description:
> + (RO) "FW Revision" string as reported by the Identify
> + Memory Device Output Payload in the CXL-2.0
> + specification.
> +
> +What:/sys/bus/cxl/devices/memX/ram/size
> +Date:December, 2020
> +KernelVersion:   v5.12
> +Contact: linux-...@vger.kernel.org
> +Description:
> + (RO) "Volatile Only Capacity" as reported by the
> + Identify Memory Device Output Payload in the CXL-2.0
> + specification.
> +
> +What:/sys/bus/cxl/devices/memX/pmem/size
> +Date:December, 2020
> +KernelVersion:   v5.12
> +Contact: linux-...@vger.kernel.org
> +Description:
> + (RO) "Persistent Only Capacity" as reported by the
> + Identify Memory Device Output Payload in the CXL-2.0
> + specification.

Aren't volatile and persistent capacities expressed in multiples of 256MB?


[PATCH 05/14] cxl/mem: Register CXL memX devices

2021-01-30 Thread Ben Widawsky
From: Dan Williams 

Create the /sys/bus/cxl hierarchy to enumerate:

* Memory Devices (per-endpoint control devices)

* Memory Address Space Devices (platform address ranges with
  interleaving, performance, and persistence attributes)

* Memory Regions (active provisioned memory from an address space device
  that is in use as System RAM or delegated to libnvdimm as Persistent
  Memory regions).

For now, only the per-endpoint control devices are registered on the
'cxl' bus. However, going forward it will provide a mechanism to
coordinate cross-device interleave.

Signed-off-by: Dan Williams 
Signed-off-by: Ben Widawsky 
---
 Documentation/ABI/testing/sysfs-bus-cxl   |  26 ++
 .../driver-api/cxl/memory-devices.rst |  17 +
 drivers/base/core.c   |  14 +
 drivers/cxl/Makefile  |   3 +
 drivers/cxl/bus.c |  29 ++
 drivers/cxl/cxl.h |   4 +
 drivers/cxl/mem.c | 308 +-
 include/linux/device.h|   1 +
 8 files changed, 400 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-cxl
 create mode 100644 drivers/cxl/bus.c

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl 
b/Documentation/ABI/testing/sysfs-bus-cxl
new file mode 100644
index ..fe7b87eba988
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -0,0 +1,26 @@
+What:  /sys/bus/cxl/devices/memX/firmware_version
+Date:  December, 2020
+KernelVersion: v5.12
+Contact:   linux-...@vger.kernel.org
+Description:
+   (RO) "FW Revision" string as reported by the Identify
+   Memory Device Output Payload in the CXL-2.0
+   specification.
+
+What:  /sys/bus/cxl/devices/memX/ram/size
+Date:  December, 2020
+KernelVersion: v5.12
+Contact:   linux-...@vger.kernel.org
+Description:
+   (RO) "Volatile Only Capacity" as reported by the
+   Identify Memory Device Output Payload in the CXL-2.0
+   specification.
+
+What:  /sys/bus/cxl/devices/memX/pmem/size
+Date:  December, 2020
+KernelVersion: v5.12
+Contact:   linux-...@vger.kernel.org
+Description:
+   (RO) "Persistent Only Capacity" as reported by the
+   Identify Memory Device Output Payload in the CXL-2.0
+   specification.
diff --git a/Documentation/driver-api/cxl/memory-devices.rst 
b/Documentation/driver-api/cxl/memory-devices.rst
index 43177e700d62..1bad466f9167 100644
--- a/Documentation/driver-api/cxl/memory-devices.rst
+++ b/Documentation/driver-api/cxl/memory-devices.rst
@@ -27,3 +27,20 @@ CXL Memory Device
 
 .. kernel-doc:: drivers/cxl/mem.c
:internal:
+
+CXL Bus
+---
+.. kernel-doc:: drivers/cxl/bus.c
+   :doc: cxl bus
+
+External Interfaces
+===
+
+CXL IOCTL Interface
+---
+
+.. kernel-doc:: include/uapi/linux/cxl_mem.h
+   :doc: UAPI
+
+.. kernel-doc:: include/uapi/linux/cxl_mem.h
+   :internal:
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 25e08e5f40bd..33432a4cbe23 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3179,6 +3179,20 @@ struct device *get_device(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(get_device);
 
+/**
+ * get_live_device() - increment reference count for device iff !dead
+ * @dev: device.
+ *
+ * Forward the call to get_device() if the device is still alive. If
+ * this is called with the device_lock() held then the device is
+ * guaranteed to not die until the device_lock() is dropped.
+ */
+struct device *get_live_device(struct device *dev)
+{
+   return dev && !dev->p->dead ? get_device(dev) : NULL;
+}
+EXPORT_SYMBOL_GPL(get_live_device);
+
 /**
  * put_device - decrement reference count.
  * @dev: device in question.
diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
index 4a30f7c3fc4a..a314a1891f4d 100644
--- a/drivers/cxl/Makefile
+++ b/drivers/cxl/Makefile
@@ -1,4 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_CXL_BUS) += cxl_bus.o
 obj-$(CONFIG_CXL_MEM) += cxl_mem.o
 
+ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL
+cxl_bus-y := bus.o
 cxl_mem-y := mem.o
diff --git a/drivers/cxl/bus.c b/drivers/cxl/bus.c
new file mode 100644
index ..58f74796d525
--- /dev/null
+++ b/drivers/cxl/bus.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2020 Intel Corporation. All rights reserved. */
+#include 
+#include 
+
+/**
+ * DOC: cxl bus
+ *
+ * The CXL bus provides namespace for control devices and a rendezvous
+ * point for cross-device interleave coordination.
+ */
+struct bus_type cxl_bus_type = {
+   .name = "cxl",
+};
+EXPORT_SYMBOL_GPL(cxl_bus_type);
+
+static __init int cxl_bus_init(void)
+{
+   return bus_register(&cxl_bus_type);
+}
+
+static void cxl_bus_exit(void)
+{
+   bus_unregister(&cxl_bus_type);
+}
+
+module_init(cxl_bus_init);
+module_exi

Re: [PATCH 05/14] cxl/mem: Register CXL memX devices

2021-01-29 Thread Dan Williams
On Fri, Jan 29, 2021 at 4:25 PM Ben Widawsky  wrote:
>
> From: Dan Williams 
>
> Create the /sys/bus/cxl hierarchy to enumerate:
>
> * Memory Devices (per-endpoint control devices)
>
> * Memory Address Space Devices (platform address ranges with
>   interleaving, performance, and persistence attributes)
>
> * Memory Regions (active provisioned memory from an address space device
>   that is in use as System RAM or delegated to libnvdimm as Persistent
>   Memory regions).
>
> For now, only the per-endpoint control devices are registered on the
> 'cxl' bus. However, going forward it will provide a mechanism to
> coordinate cross-device interleave.
>
> Signed-off-by: Dan Williams 
> Signed-off-by: Ben Widawsky 
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl   |  26 ++
>  .../driver-api/cxl/memory-devices.rst |  17 +
>  drivers/base/core.c   |  14 +
>  drivers/cxl/Makefile  |   3 +
>  drivers/cxl/bus.c |  29 ++
>  drivers/cxl/cxl.h |   4 +
>  drivers/cxl/mem.c | 308 +-
>  include/linux/device.h|   1 +
>  8 files changed, 400 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-cxl
>  create mode 100644 drivers/cxl/bus.c
[..]
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 25e08e5f40bd..33432a4cbe23 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3179,6 +3179,20 @@ struct device *get_device(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(get_device);
>
> +/**
> + * get_live_device() - increment reference count for device iff !dead
> + * @dev: device.
> + *
> + * Forward the call to get_device() if the device is still alive. If
> + * this is called with the device_lock() held then the device is
> + * guaranteed to not die until the device_lock() is dropped.
> + */
> +struct device *get_live_device(struct device *dev)
> +{
> +   return dev && !dev->p->dead ? get_device(dev) : NULL;
> +}
> +EXPORT_SYMBOL_GPL(get_live_device);

Disregard this hunk, it's an abandoned idea that I failed to cleanup.