diff mbox series

[v2,2/3] cxl/region: Fix x1 interleave to greater than x1 interleave routing

Message ID 165973126583.1526540.657948655360009242.stgit@dwillia2-xfh.jf.intel.com
State Accepted
Commit 298d44d04b2ba97824c3dadd1dbf7c154a2a86e2
Headers show
Series CXL Region Provisioning Fixes | expand

Commit Message

Dan Williams Aug. 5, 2022, 8:27 p.m. UTC
In cases where the decode fans out as it traverses downstream, the
interleave granularity needs to increment to identify the port selector
bits out of the remaining address bits. For example, recall that with an
x2 parent port intereleave (IW == 1), the downstream decode for children
of those ports will either see address bit IG+8 always set, or address
bit IG+8 always clear. So if the child port needs to select a downstream
port it can only use address bits starting at IG+9 (where IG and IW are
the CXL encoded values for interleave granularity (ilog2(ig) - 8) and
ways (ilog2(iw))).

When the parent port interleave is x1 no such masking occurs and the
child port can maintain the granularity that was routed to the parent
port.

Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/region.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Verma, Vishal L Aug. 5, 2022, 9:54 p.m. UTC | #1
On Fri, 2022-08-05 at 13:27 -0700, Dan Williams wrote:
> In cases where the decode fans out as it traverses downstream, the
> interleave granularity needs to increment to identify the port selector
> bits out of the remaining address bits. For example, recall that with an
> x2 parent port intereleave (IW == 1), the downstream decode for children
> of those ports will either see address bit IG+8 always set, or address
> bit IG+8 always clear. So if the child port needs to select a downstream
> port it can only use address bits starting at IG+9 (where IG and IW are
> the CXL encoded values for interleave granularity (ilog2(ig) - 8) and
> ways (ilog2(iw))).
> 
> When the parent port interleave is x1 no such masking occurs and the
> child port can maintain the granularity that was routed to the parent
> port.
> 
> Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/region.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Makes sense to me,

Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>

> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e71077beb021..641bc6344a4a 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1025,7 +1025,11 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>                 return rc;
>         }
>  
> -       if (cxl_rr->nr_targets > 1) {
> +       /*
> +        * If @parent_port is masking address bits, pick the next unused address
> +        * bit to route @port's targets.
> +        */
> +       if (parent_iw > 1 && cxl_rr->nr_targets > 1) {
>                 u32 address_bit = max(peig + peiw, eiw + peig);
>  
>                 eig = address_bit - eiw + 1;
>
Ira Weiny Aug. 5, 2022, 10:50 p.m. UTC | #2
On Fri, Aug 05, 2022 at 01:27:45PM -0700, Dan Williams wrote:
> In cases where the decode fans out as it traverses downstream, the
> interleave granularity needs to increment to identify the port selector
> bits out of the remaining address bits. For example, recall that with an
> x2 parent port intereleave (IW == 1), the downstream decode for children
> of those ports will either see address bit IG+8 always set, or address
> bit IG+8 always clear. So if the child port needs to select a downstream
> port it can only use address bits starting at IG+9 (where IG and IW are
> the CXL encoded values for interleave granularity (ilog2(ig) - 8) and
> ways (ilog2(iw))).
> 
> When the parent port interleave is x1 no such masking occurs and the
> child port can maintain the granularity that was routed to the parent
> port.
> 
> Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
>  drivers/cxl/core/region.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e71077beb021..641bc6344a4a 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1025,7 +1025,11 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  		return rc;
>  	}
>  
> -	if (cxl_rr->nr_targets > 1) {
> +	/*
> +	 * If @parent_port is masking address bits, pick the next unused address
> +	 * bit to route @port's targets.
> +	 */
> +	if (parent_iw > 1 && cxl_rr->nr_targets > 1) {
>  		u32 address_bit = max(peig + peiw, eiw + peig);
>  
>  		eig = address_bit - eiw + 1;
>
Jonathan Cameron Aug. 8, 2022, 11:03 a.m. UTC | #3
On Fri, 05 Aug 2022 13:27:45 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> In cases where the decode fans out as it traverses downstream, the
> interleave granularity needs to increment to identify the port selector
> bits out of the remaining address bits. For example, recall that with an
> x2 parent port intereleave (IW == 1), the downstream decode for children
> of those ports will either see address bit IG+8 always set, or address
> bit IG+8 always clear. So if the child port needs to select a downstream
> port it can only use address bits starting at IG+9 (where IG and IW are
> the CXL encoded values for interleave granularity (ilog2(ig) - 8) and
> ways (ilog2(iw))).
> 
> When the parent port interleave is x1 no such masking occurs and the
> child port can maintain the granularity that was routed to the parent
> port.
> 
> Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Tested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> #via qemu

Now this is resolved, I'll get qemu fix sent out (hopefully later today).

Thanks,

Jonathan

> ---
>  drivers/cxl/core/region.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e71077beb021..641bc6344a4a 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1025,7 +1025,11 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  		return rc;
>  	}
>  
> -	if (cxl_rr->nr_targets > 1) {
> +	/*
> +	 * If @parent_port is masking address bits, pick the next unused address
> +	 * bit to route @port's targets.
> +	 */
> +	if (parent_iw > 1 && cxl_rr->nr_targets > 1) {
>  		u32 address_bit = max(peig + peiw, eiw + peig);
>  
>  		eig = address_bit - eiw + 1;
>
Dan Williams Aug. 8, 2022, 7:28 p.m. UTC | #4
Jonathan Cameron wrote:
> On Fri, 05 Aug 2022 13:27:45 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > In cases where the decode fans out as it traverses downstream, the
> > interleave granularity needs to increment to identify the port selector
> > bits out of the remaining address bits. For example, recall that with an
> > x2 parent port intereleave (IW == 1), the downstream decode for children
> > of those ports will either see address bit IG+8 always set, or address
> > bit IG+8 always clear. So if the child port needs to select a downstream
> > port it can only use address bits starting at IG+9 (where IG and IW are
> > the CXL encoded values for interleave granularity (ilog2(ig) - 8) and
> > ways (ilog2(iw))).
> > 
> > When the parent port interleave is x1 no such masking occurs and the
> > child port can maintain the granularity that was routed to the parent
> > port.
> > 
> > Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Tested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> #via qemu

Thanks Jonathan.

In the interests of being able to make a:

"these commits have appeared in Linux next with no known outstanding
issues"

...I'd like to note these review and test tags in the merge message.
They will still be in the history just not in the commits directly.
Similar to what I did here:

https://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/tag/?h=libnvdimm-for-5.17

The current state of cxl/next hit linux-next last night. Let me know if
you have any heartburn about that.

> Now this is resolved, I'll get qemu fix sent out (hopefully later today).

Good to hear, thanks.
Jonathan Cameron Aug. 9, 2022, 10:18 a.m. UTC | #5
On Mon, 8 Aug 2022 12:28:36 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Jonathan Cameron wrote:
> > On Fri, 05 Aug 2022 13:27:45 -0700
> > Dan Williams <dan.j.williams@intel.com> wrote:
> >   
> > > In cases where the decode fans out as it traverses downstream, the
> > > interleave granularity needs to increment to identify the port selector
> > > bits out of the remaining address bits. For example, recall that with an
> > > x2 parent port intereleave (IW == 1), the downstream decode for children
> > > of those ports will either see address bit IG+8 always set, or address
> > > bit IG+8 always clear. So if the child port needs to select a downstream
> > > port it can only use address bits starting at IG+9 (where IG and IW are
> > > the CXL encoded values for interleave granularity (ilog2(ig) - 8) and
> > > ways (ilog2(iw))).
> > > 
> > > When the parent port interleave is x1 no such masking occurs and the
> > > child port can maintain the granularity that was routed to the parent
> > > port.
> > > 
> > > Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>  
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Tested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> #via qemu  
> 
> Thanks Jonathan.
> 
> In the interests of being able to make a:
> 
> "these commits have appeared in Linux next with no known outstanding
> issues"
> 
> ...I'd like to note these review and test tags in the merge message.
> They will still be in the history just not in the commits directly.
> Similar to what I did here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/tag/?h=libnvdimm-for-5.17
> 
> The current state of cxl/next hit linux-next last night. Let me know if
> you have any heartburn about that.

Fine by me.

I normally don't even do that if I'm rushing something in. Just rely on
the link to the thread in the actual patch description for anyone who cares.
Maybe I'll start doing something like you are, though can be a pain to gather them
up on a large pull request.

Jonathan

> 
> > Now this is resolved, I'll get qemu fix sent out (hopefully later today).  
> 
> Good to hear, thanks.
diff mbox series

Patch

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index e71077beb021..641bc6344a4a 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1025,7 +1025,11 @@  static int cxl_port_setup_targets(struct cxl_port *port,
 		return rc;
 	}
 
-	if (cxl_rr->nr_targets > 1) {
+	/*
+	 * If @parent_port is masking address bits, pick the next unused address
+	 * bit to route @port's targets.
+	 */
+	if (parent_iw > 1 && cxl_rr->nr_targets > 1) {
 		u32 address_bit = max(peig + peiw, eiw + peig);
 
 		eig = address_bit - eiw + 1;