diff mbox series

[17/46] cxl/hdm: Track next decoder to allocate

Message ID 165603882752.551046.12620934370518380800.stgit@dwillia2-xfh
State Superseded
Headers show
Series CXL PMEM Region Provisioning | expand

Commit Message

Dan Williams June 24, 2022, 2:47 a.m. UTC
The CXL specification enforces that endpoint decoders are committed in
hw instance id order. In preparation for adding dynamic DPA allocation,
record the hw instance id in endpoint decoders, and enforce allocations
to occur in hw instance id order.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/hdm.c  |   14 ++++++++++++++
 drivers/cxl/core/port.c |    1 +
 drivers/cxl/cxl.h       |    2 ++
 3 files changed, 17 insertions(+)

Comments

Jonathan Cameron June 29, 2022, 3:31 p.m. UTC | #1
On Thu, 23 Jun 2022 19:47:07 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> The CXL specification enforces that endpoint decoders are committed in
> hw instance id order. In preparation for adding dynamic DPA allocation,
> record the hw instance id in endpoint decoders, and enforce allocations
> to occur in hw instance id order.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

dpa_end isn't a good name given the value isn't a Device Physical Address.

Otherwise looks fine,

Jonathan

> ---
>  drivers/cxl/core/hdm.c  |   14 ++++++++++++++
>  drivers/cxl/core/port.c |    1 +
>  drivers/cxl/cxl.h       |    2 ++
>  3 files changed, 17 insertions(+)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 3f929231b822..8805afe63ebf 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -153,6 +153,7 @@ static void __cxl_dpa_release(struct cxl_endpoint_decoder *cxled, bool remove_ac
>  	cxled->skip = 0;
>  	__release_region(&cxlds->dpa_res, res->start, resource_size(res));
>  	cxled->dpa_res = NULL;
> +	port->dpa_end--;
>  }
>  
>  static void cxl_dpa_release(void *cxled)
> @@ -183,6 +184,18 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
>  		return -EBUSY;
>  	}
>  
> +	if (port->dpa_end + 1 != cxled->cxld.id) {
> +		/*
> +		 * Assumes alloc and commit order is always in hardware instance
> +		 * order per expectations from 8.2.5.12.20 Committing Decoder
> +		 * Programming that enforce decoder[m] committed before
> +		 * decoder[m+1] commit start.
> +		 */
> +		dev_dbg(dev, "decoder%d.%d: expected decoder%d.%d\n", port->id,
> +			cxled->cxld.id, port->id, port->dpa_end + 1);
> +		return -EBUSY;
> +	}
> +
>  	if (skip) {
>  		res = __request_region(&cxlds->dpa_res, base - skip, skip,
>  				       dev_name(dev), 0);
> @@ -213,6 +226,7 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
>  			cxled->cxld.id, cxled->dpa_res);
>  		cxled->mode = CXL_DECODER_MIXED;
>  	}
> +	port->dpa_end++;
>  
>  	return 0;
>  }
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 9d632c8c580b..54bf032cbcb7 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -485,6 +485,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport,
>  	port->uport = uport;
>  	port->component_reg_phys = component_reg_phys;
>  	ida_init(&port->decoder_ida);
> +	port->dpa_end = -1;
>  	INIT_LIST_HEAD(&port->dports);
>  	INIT_LIST_HEAD(&port->endpoints);
>  
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index aa223166f7ef..d8edbdaa6208 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -326,6 +326,7 @@ struct cxl_nvdimm {
>   * @dports: cxl_dport instances referenced by decoders
>   * @endpoints: cxl_ep instances, endpoints that are a descendant of this port
>   * @decoder_ida: allocator for decoder ids
> + * @dpa_end: cursor to track highest allocated decoder for allocation ordering

dpa_end not a good name as this isn't a Device Physical Address.

>   * @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.
> @@ -337,6 +338,7 @@ struct cxl_port {
>  	struct list_head dports;
>  	struct list_head endpoints;
>  	struct ida decoder_ida;
> +	int dpa_end;
>  	resource_size_t component_reg_phys;
>  	bool dead;
>  	unsigned int depth;
>
Dan Williams July 10, 2022, 3:55 a.m. UTC | #2
Jonathan Cameron wrote:
> On Thu, 23 Jun 2022 19:47:07 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > The CXL specification enforces that endpoint decoders are committed in
> > hw instance id order. In preparation for adding dynamic DPA allocation,
> > record the hw instance id in endpoint decoders, and enforce allocations
> > to occur in hw instance id order.
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> dpa_end isn't a good name given the value isn't a Device Physical Address.
> 
> Otherwise looks fine,
> 
> Jonathan
> 
> > ---
> >  drivers/cxl/core/hdm.c  |   14 ++++++++++++++
> >  drivers/cxl/core/port.c |    1 +
> >  drivers/cxl/cxl.h       |    2 ++
> >  3 files changed, 17 insertions(+)
> > 
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 3f929231b822..8805afe63ebf 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -153,6 +153,7 @@ static void __cxl_dpa_release(struct cxl_endpoint_decoder *cxled, bool remove_ac
> >  	cxled->skip = 0;
> >  	__release_region(&cxlds->dpa_res, res->start, resource_size(res));
> >  	cxled->dpa_res = NULL;
> > +	port->dpa_end--;
> >  }
> >  
> >  static void cxl_dpa_release(void *cxled)
> > @@ -183,6 +184,18 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> >  		return -EBUSY;
> >  	}
> >  
> > +	if (port->dpa_end + 1 != cxled->cxld.id) {
> > +		/*
> > +		 * Assumes alloc and commit order is always in hardware instance
> > +		 * order per expectations from 8.2.5.12.20 Committing Decoder
> > +		 * Programming that enforce decoder[m] committed before
> > +		 * decoder[m+1] commit start.
> > +		 */
> > +		dev_dbg(dev, "decoder%d.%d: expected decoder%d.%d\n", port->id,
> > +			cxled->cxld.id, port->id, port->dpa_end + 1);
> > +		return -EBUSY;
> > +	}
> > +
> >  	if (skip) {
> >  		res = __request_region(&cxlds->dpa_res, base - skip, skip,
> >  				       dev_name(dev), 0);
> > @@ -213,6 +226,7 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> >  			cxled->cxld.id, cxled->dpa_res);
> >  		cxled->mode = CXL_DECODER_MIXED;
> >  	}
> > +	port->dpa_end++;
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > index 9d632c8c580b..54bf032cbcb7 100644
> > --- a/drivers/cxl/core/port.c
> > +++ b/drivers/cxl/core/port.c
> > @@ -485,6 +485,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport,
> >  	port->uport = uport;
> >  	port->component_reg_phys = component_reg_phys;
> >  	ida_init(&port->decoder_ida);
> > +	port->dpa_end = -1;
> >  	INIT_LIST_HEAD(&port->dports);
> >  	INIT_LIST_HEAD(&port->endpoints);
> >  
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index aa223166f7ef..d8edbdaa6208 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -326,6 +326,7 @@ struct cxl_nvdimm {
> >   * @dports: cxl_dport instances referenced by decoders
> >   * @endpoints: cxl_ep instances, endpoints that are a descendant of this port
> >   * @decoder_ida: allocator for decoder ids
> > + * @dpa_end: cursor to track highest allocated decoder for allocation ordering
> 
> dpa_end not a good name as this isn't a Device Physical Address.

Ok, renamed it to 'hdm_end'. Suitable to add "Reviewed-by" now?
Dan Williams July 10, 2022, 4:34 p.m. UTC | #3
Jonathan Cameron wrote:
> On Thu, 23 Jun 2022 19:47:07 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > The CXL specification enforces that endpoint decoders are committed in
> > hw instance id order. In preparation for adding dynamic DPA allocation,
> > record the hw instance id in endpoint decoders, and enforce allocations
> > to occur in hw instance id order.
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> dpa_end isn't a good name given the value isn't a Device Physical Address.
> 
> Otherwise looks fine,
> 
> Jonathan
> 
> > ---
> >  drivers/cxl/core/hdm.c  |   14 ++++++++++++++
> >  drivers/cxl/core/port.c |    1 +
> >  drivers/cxl/cxl.h       |    2 ++
> >  3 files changed, 17 insertions(+)
> > 
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 3f929231b822..8805afe63ebf 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -153,6 +153,7 @@ static void __cxl_dpa_release(struct cxl_endpoint_decoder *cxled, bool remove_ac
> >  	cxled->skip = 0;
> >  	__release_region(&cxlds->dpa_res, res->start, resource_size(res));
> >  	cxled->dpa_res = NULL;
> > +	port->dpa_end--;
> >  }
> >  
> >  static void cxl_dpa_release(void *cxled)
> > @@ -183,6 +184,18 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> >  		return -EBUSY;
> >  	}
> >  
> > +	if (port->dpa_end + 1 != cxled->cxld.id) {
> > +		/*
> > +		 * Assumes alloc and commit order is always in hardware instance
> > +		 * order per expectations from 8.2.5.12.20 Committing Decoder
> > +		 * Programming that enforce decoder[m] committed before
> > +		 * decoder[m+1] commit start.
> > +		 */
> > +		dev_dbg(dev, "decoder%d.%d: expected decoder%d.%d\n", port->id,
> > +			cxled->cxld.id, port->id, port->dpa_end + 1);
> > +		return -EBUSY;
> > +	}
> > +
> >  	if (skip) {
> >  		res = __request_region(&cxlds->dpa_res, base - skip, skip,
> >  				       dev_name(dev), 0);
> > @@ -213,6 +226,7 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> >  			cxled->cxld.id, cxled->dpa_res);
> >  		cxled->mode = CXL_DECODER_MIXED;
> >  	}
> > +	port->dpa_end++;
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > index 9d632c8c580b..54bf032cbcb7 100644
> > --- a/drivers/cxl/core/port.c
> > +++ b/drivers/cxl/core/port.c
> > @@ -485,6 +485,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport,
> >  	port->uport = uport;
> >  	port->component_reg_phys = component_reg_phys;
> >  	ida_init(&port->decoder_ida);
> > +	port->dpa_end = -1;
> >  	INIT_LIST_HEAD(&port->dports);
> >  	INIT_LIST_HEAD(&port->endpoints);
> >  
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index aa223166f7ef..d8edbdaa6208 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -326,6 +326,7 @@ struct cxl_nvdimm {
> >   * @dports: cxl_dport instances referenced by decoders
> >   * @endpoints: cxl_ep instances, endpoints that are a descendant of this port
> >   * @decoder_ida: allocator for decoder ids
> > + * @dpa_end: cursor to track highest allocated decoder for allocation ordering
> 
> dpa_end not a good name as this isn't a Device Physical Address.

Ok, renamed it like this:

diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 546a022ef17f..22b7fc8ed510 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -333,7 +333,7 @@ struct cxl_nvdimm {
  * @dports: cxl_dport instances referenced by decoders
  * @endpoints: cxl_ep instances, endpoints that are a descendant of this port
  * @decoder_ida: allocator for decoder ids
- * @dpa_end: cursor to track highest allocated decoder for allocation ordering
+ * @hdm_end: track last allocated HDM decoder instance for allocation ordering
  * @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.
@@ -345,7 +345,7 @@ struct cxl_port {
        struct list_head dports;
        struct list_head endpoints;
        struct ida decoder_ida;
-       int dpa_end;
+       int hdm_end;
        resource_size_t component_reg_phys;
        bool dead;
        unsigned int depth;
Jonathan Cameron July 19, 2022, 2:27 p.m. UTC | #4
On Sat, 9 Jul 2022 20:55:17 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Jonathan Cameron wrote:
> > On Thu, 23 Jun 2022 19:47:07 -0700
> > Dan Williams <dan.j.williams@intel.com> wrote:
> >   
> > > The CXL specification enforces that endpoint decoders are committed in
> > > hw instance id order. In preparation for adding dynamic DPA allocation,
> > > record the hw instance id in endpoint decoders, and enforce allocations
> > > to occur in hw instance id order.
> > > 
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>  
> > 
> > dpa_end isn't a good name given the value isn't a Device Physical Address.
> > 
> > Otherwise looks fine,
> > 
> > Jonathan
> >   
> > > ---
> > >  drivers/cxl/core/hdm.c  |   14 ++++++++++++++
> > >  drivers/cxl/core/port.c |    1 +
> > >  drivers/cxl/cxl.h       |    2 ++
> > >  3 files changed, 17 insertions(+)
> > > 
> > > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > > index 3f929231b822..8805afe63ebf 100644
> > > --- a/drivers/cxl/core/hdm.c
> > > +++ b/drivers/cxl/core/hdm.c
> > > @@ -153,6 +153,7 @@ static void __cxl_dpa_release(struct cxl_endpoint_decoder *cxled, bool remove_ac
> > >  	cxled->skip = 0;
> > >  	__release_region(&cxlds->dpa_res, res->start, resource_size(res));
> > >  	cxled->dpa_res = NULL;
> > > +	port->dpa_end--;
> > >  }
> > >  
> > >  static void cxl_dpa_release(void *cxled)
> > > @@ -183,6 +184,18 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> > >  		return -EBUSY;
> > >  	}
> > >  
> > > +	if (port->dpa_end + 1 != cxled->cxld.id) {
> > > +		/*
> > > +		 * Assumes alloc and commit order is always in hardware instance
> > > +		 * order per expectations from 8.2.5.12.20 Committing Decoder
> > > +		 * Programming that enforce decoder[m] committed before
> > > +		 * decoder[m+1] commit start.
> > > +		 */
> > > +		dev_dbg(dev, "decoder%d.%d: expected decoder%d.%d\n", port->id,
> > > +			cxled->cxld.id, port->id, port->dpa_end + 1);
> > > +		return -EBUSY;
> > > +	}
> > > +
> > >  	if (skip) {
> > >  		res = __request_region(&cxlds->dpa_res, base - skip, skip,
> > >  				       dev_name(dev), 0);
> > > @@ -213,6 +226,7 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> > >  			cxled->cxld.id, cxled->dpa_res);
> > >  		cxled->mode = CXL_DECODER_MIXED;
> > >  	}
> > > +	port->dpa_end++;
> > >  
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > > index 9d632c8c580b..54bf032cbcb7 100644
> > > --- a/drivers/cxl/core/port.c
> > > +++ b/drivers/cxl/core/port.c
> > > @@ -485,6 +485,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport,
> > >  	port->uport = uport;
> > >  	port->component_reg_phys = component_reg_phys;
> > >  	ida_init(&port->decoder_ida);
> > > +	port->dpa_end = -1;
> > >  	INIT_LIST_HEAD(&port->dports);
> > >  	INIT_LIST_HEAD(&port->endpoints);
> > >  
> > > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > > index aa223166f7ef..d8edbdaa6208 100644
> > > --- a/drivers/cxl/cxl.h
> > > +++ b/drivers/cxl/cxl.h
> > > @@ -326,6 +326,7 @@ struct cxl_nvdimm {
> > >   * @dports: cxl_dport instances referenced by decoders
> > >   * @endpoints: cxl_ep instances, endpoints that are a descendant of this port
> > >   * @decoder_ida: allocator for decoder ids
> > > + * @dpa_end: cursor to track highest allocated decoder for allocation ordering  
> > 
> > dpa_end not a good name as this isn't a Device Physical Address.  
> 
> Ok, renamed it to 'hdm_end'. Suitable to add "Reviewed-by" now?

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Though I'll probably catch up with a later version when I've gotten through
more of my email and add it there as well so you don't have to add it
manually.

Thanks,

J
diff mbox series

Patch

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 3f929231b822..8805afe63ebf 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -153,6 +153,7 @@  static void __cxl_dpa_release(struct cxl_endpoint_decoder *cxled, bool remove_ac
 	cxled->skip = 0;
 	__release_region(&cxlds->dpa_res, res->start, resource_size(res));
 	cxled->dpa_res = NULL;
+	port->dpa_end--;
 }
 
 static void cxl_dpa_release(void *cxled)
@@ -183,6 +184,18 @@  static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
 		return -EBUSY;
 	}
 
+	if (port->dpa_end + 1 != cxled->cxld.id) {
+		/*
+		 * Assumes alloc and commit order is always in hardware instance
+		 * order per expectations from 8.2.5.12.20 Committing Decoder
+		 * Programming that enforce decoder[m] committed before
+		 * decoder[m+1] commit start.
+		 */
+		dev_dbg(dev, "decoder%d.%d: expected decoder%d.%d\n", port->id,
+			cxled->cxld.id, port->id, port->dpa_end + 1);
+		return -EBUSY;
+	}
+
 	if (skip) {
 		res = __request_region(&cxlds->dpa_res, base - skip, skip,
 				       dev_name(dev), 0);
@@ -213,6 +226,7 @@  static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
 			cxled->cxld.id, cxled->dpa_res);
 		cxled->mode = CXL_DECODER_MIXED;
 	}
+	port->dpa_end++;
 
 	return 0;
 }
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 9d632c8c580b..54bf032cbcb7 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -485,6 +485,7 @@  static struct cxl_port *cxl_port_alloc(struct device *uport,
 	port->uport = uport;
 	port->component_reg_phys = component_reg_phys;
 	ida_init(&port->decoder_ida);
+	port->dpa_end = -1;
 	INIT_LIST_HEAD(&port->dports);
 	INIT_LIST_HEAD(&port->endpoints);
 
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index aa223166f7ef..d8edbdaa6208 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -326,6 +326,7 @@  struct cxl_nvdimm {
  * @dports: cxl_dport instances referenced by decoders
  * @endpoints: cxl_ep instances, endpoints that are a descendant of this port
  * @decoder_ida: allocator for decoder ids
+ * @dpa_end: cursor to track highest allocated decoder for allocation ordering
  * @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.
@@ -337,6 +338,7 @@  struct cxl_port {
 	struct list_head dports;
 	struct list_head endpoints;
 	struct ida decoder_ida;
+	int dpa_end;
 	resource_size_t component_reg_phys;
 	bool dead;
 	unsigned int depth;