diff mbox series

[30/46] cxl/hdm: Add sysfs attributes for interleave ways + granularity

Message ID 20220624041950.559155-5-dan.j.williams@intel.com (mailing list archive)
State Superseded
Headers show
Series CXL PMEM Region Provisioning | expand

Commit Message

Dan Williams June 24, 2022, 4:19 a.m. UTC
From: Ben Widawsky <bwidawsk@kernel.org>

The region provisioning flow involves selecting interleave ways +
granularity settings for a region, and then programming the decoder
topology to meet those constraints, if possible. For example, root
decoders set the minimum interleave ways + granularity for any hosted
regions.

Given decoder programming is not atomic and collisions can occur between
multiple requesting regions userpace will be resonsible for conflict
resolution and it needs these attributes to make those decisions.

Signed-off-by: Ben Widawsky <bwidawsk@kernel.org>
[djbw: reword changelog, make read-only, add sysfs ABI documentaion]
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-cxl | 23 +++++++++++++++++++++++
 drivers/cxl/core/port.c                 | 23 +++++++++++++++++++++++
 2 files changed, 46 insertions(+)

Comments

Jonathan Cameron June 30, 2022, 9:26 a.m. UTC | #1
On Thu, 23 Jun 2022 21:19:34 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> From: Ben Widawsky <bwidawsk@kernel.org>
> 
> The region provisioning flow involves selecting interleave ways +
> granularity settings for a region, and then programming the decoder
> topology to meet those constraints, if possible. For example, root
> decoders set the minimum interleave ways + granularity for any hosted
> regions.
> 
> Given decoder programming is not atomic and collisions can occur between
> multiple requesting regions userpace will be resonsible for conflict
> resolution and it needs these attributes to make those decisions.
> 
> Signed-off-by: Ben Widawsky <bwidawsk@kernel.org>
> [djbw: reword changelog, make read-only, add sysfs ABI documentaion]
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
some comments on docs.

> ---
>  Documentation/ABI/testing/sysfs-bus-cxl | 23 +++++++++++++++++++++++
>  drivers/cxl/core/port.c                 | 23 +++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 85844f9bc00b..2a4e4163879f 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -215,3 +215,26 @@ Description:
>  		allocations are enforced to occur in increasing 'decoderX.Y/id'
>  		order and frees are enforced to occur in decreasing
>  		'decoderX.Y/id' order.
> +
> +
> +What:		/sys/bus/cxl/devices/decoderX.Y/interleave_ways
> +Date:		May, 2022
> +KernelVersion:	v5.20
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) The number of targets across which this decoder's host
> +		physical address (HPA) memory range is interleaved. The device
> +		maps every Nth block of HPA (of size ==
> +		'interleave_granularity') to consecutive DPA addresses. The
> +		decoder's position in the interleave is determined by the
> +		device's (endpoint or switch) switch ancestry.

Perhaps make it clear what happens for host bridges (i.e. decoder position
in interleave defined by fixed memory window.

> +
> +
> +What:		/sys/bus/cxl/devices/decoderX.Y/interleave_granularity
> +Date:		May, 2022
> +KernelVersion:	v5.20
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) The number of consecutive bytes of host physical address
> +		space this decoder claims at address N before awaint the next

awaint?

> +		address (N + interleave_granularity * intereleave_ways).

interleave_ways

Even knowing exactly what this is, I don't understand the docs so
perhaps reword this :)

> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index c48f217e689a..08a380d20cf1 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -260,10 +260,33 @@ static ssize_t dpa_size_store(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RW(dpa_size);
>  
> +static ssize_t interleave_granularity_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf)
> +{
> +	struct cxl_decoder *cxld = to_cxl_decoder(dev);
> +
> +	return sysfs_emit(buf, "%d\n", cxld->interleave_granularity);
> +}
> +
> +static DEVICE_ATTR_RO(interleave_granularity);
> +
> +static ssize_t interleave_ways_show(struct device *dev,
> +				    struct device_attribute *attr, char *buf)
> +{
> +	struct cxl_decoder *cxld = to_cxl_decoder(dev);
> +
> +	return sysfs_emit(buf, "%d\n", cxld->interleave_ways);
> +}
> +
> +static DEVICE_ATTR_RO(interleave_ways);
> +
>  static struct attribute *cxl_decoder_base_attrs[] = {
>  	&dev_attr_start.attr,
>  	&dev_attr_size.attr,
>  	&dev_attr_locked.attr,
> +	&dev_attr_interleave_granularity.attr,
> +	&dev_attr_interleave_ways.attr,
>  	NULL,
>  };
>
Dan Williams July 10, 2022, 8:40 p.m. UTC | #2
Jonathan Cameron wrote:
> On Thu, 23 Jun 2022 21:19:34 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > From: Ben Widawsky <bwidawsk@kernel.org>
> > 
> > The region provisioning flow involves selecting interleave ways +
> > granularity settings for a region, and then programming the decoder
> > topology to meet those constraints, if possible. For example, root
> > decoders set the minimum interleave ways + granularity for any hosted
> > regions.
> > 
> > Given decoder programming is not atomic and collisions can occur between
> > multiple requesting regions userpace will be resonsible for conflict
> > resolution and it needs these attributes to make those decisions.
> > 
> > Signed-off-by: Ben Widawsky <bwidawsk@kernel.org>
> > [djbw: reword changelog, make read-only, add sysfs ABI documentaion]
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> some comments on docs.
> 
> > ---
> >  Documentation/ABI/testing/sysfs-bus-cxl | 23 +++++++++++++++++++++++
> >  drivers/cxl/core/port.c                 | 23 +++++++++++++++++++++++
> >  2 files changed, 46 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> > index 85844f9bc00b..2a4e4163879f 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-cxl
> > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > @@ -215,3 +215,26 @@ Description:
> >  		allocations are enforced to occur in increasing 'decoderX.Y/id'
> >  		order and frees are enforced to occur in decreasing
> >  		'decoderX.Y/id' order.
> > +
> > +
> > +What:		/sys/bus/cxl/devices/decoderX.Y/interleave_ways
> > +Date:		May, 2022
> > +KernelVersion:	v5.20
> > +Contact:	linux-cxl@vger.kernel.org
> > +Description:
> > +		(RO) The number of targets across which this decoder's host
> > +		physical address (HPA) memory range is interleaved. The device
> > +		maps every Nth block of HPA (of size ==
> > +		'interleave_granularity') to consecutive DPA addresses. The
> > +		decoder's position in the interleave is determined by the
> > +		device's (endpoint or switch) switch ancestry.
> 
> Perhaps make it clear what happens for host bridges (i.e. decoder position
> in interleave defined by fixed memory window.

Added: "For root decoders their interleave is specified by platform
firmware and they only specify a downstream target order for host
bridges".

> 
> > +
> > +
> > +What:		/sys/bus/cxl/devices/decoderX.Y/interleave_granularity
> > +Date:		May, 2022
> > +KernelVersion:	v5.20
> > +Contact:	linux-cxl@vger.kernel.org
> > +Description:
> > +		(RO) The number of consecutive bytes of host physical address
> > +		space this decoder claims at address N before awaint the next
> 
> awaint?

Surprised checkpatch did not flag this, or that I missed the checkpatch
flag.

> 
> > +		address (N + interleave_granularity * intereleave_ways).
> 
> interleave_ways
> 
> Even knowing exactly what this is, I don't understand the docs so
> perhaps reword this :)

Reworded to:

(RO) The number of consecutive bytes of host physical address space this
decoder claims at address N before the decode rotates to the next target
in the interleave at address N + interleave_granularity (assuming N is
aligned to interleave_granularity).
Jonathan Cameron July 19, 2022, 2:32 p.m. UTC | #3
> >   
> > > +		address (N + interleave_granularity * intereleave_ways).  
> > 
> > interleave_ways
> > 
> > Even knowing exactly what this is, I don't understand the docs so
> > perhaps reword this :)  
> 
> Reworded to:
> 
> (RO) The number of consecutive bytes of host physical address space this
> decoder claims at address N before the decode rotates to the next target
> in the interleave at address N + interleave_granularity (assuming N is
> aligned to interleave_granularity).

LGTM
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 85844f9bc00b..2a4e4163879f 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -215,3 +215,26 @@  Description:
 		allocations are enforced to occur in increasing 'decoderX.Y/id'
 		order and frees are enforced to occur in decreasing
 		'decoderX.Y/id' order.
+
+
+What:		/sys/bus/cxl/devices/decoderX.Y/interleave_ways
+Date:		May, 2022
+KernelVersion:	v5.20
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) The number of targets across which this decoder's host
+		physical address (HPA) memory range is interleaved. The device
+		maps every Nth block of HPA (of size ==
+		'interleave_granularity') to consecutive DPA addresses. The
+		decoder's position in the interleave is determined by the
+		device's (endpoint or switch) switch ancestry.
+
+
+What:		/sys/bus/cxl/devices/decoderX.Y/interleave_granularity
+Date:		May, 2022
+KernelVersion:	v5.20
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) The number of consecutive bytes of host physical address
+		space this decoder claims at address N before awaint the next
+		address (N + interleave_granularity * intereleave_ways).
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index c48f217e689a..08a380d20cf1 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -260,10 +260,33 @@  static ssize_t dpa_size_store(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RW(dpa_size);
 
+static ssize_t interleave_granularity_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct cxl_decoder *cxld = to_cxl_decoder(dev);
+
+	return sysfs_emit(buf, "%d\n", cxld->interleave_granularity);
+}
+
+static DEVICE_ATTR_RO(interleave_granularity);
+
+static ssize_t interleave_ways_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct cxl_decoder *cxld = to_cxl_decoder(dev);
+
+	return sysfs_emit(buf, "%d\n", cxld->interleave_ways);
+}
+
+static DEVICE_ATTR_RO(interleave_ways);
+
 static struct attribute *cxl_decoder_base_attrs[] = {
 	&dev_attr_start.attr,
 	&dev_attr_size.attr,
 	&dev_attr_locked.attr,
+	&dev_attr_interleave_granularity.attr,
+	&dev_attr_interleave_ways.attr,
 	NULL,
 };