* RE: [PATCH] cxl/region: Refactor granularity select in cxl_port_setup_targets()
2023-08-04 23:27 [PATCH] cxl/region: Refactor granularity select in cxl_port_setup_targets() alison.schofield
@ 2023-08-05 2:49 ` Dan Williams
2023-08-15 0:27 ` Alison Schofield
2023-08-07 13:26 ` Jonathan Cameron
2023-08-09 22:28 ` Dave Jiang
2 siblings, 1 reply; 7+ messages in thread
From: Dan Williams @ 2023-08-05 2:49 UTC (permalink / raw)
To: alison.schofield, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
Vishal Verma, Ira Weiny, Dan Williams
Cc: linux-cxl
alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> In cxl_port_setup_targets() the region driver validates the
> configuration of auto-discovered region decoders, as well
> as decoders the driver is preparing to program.
>
> The existing calculations use the encrypted interleave granularity
s/encrypted/encoded/
> value, the eig, to create an interleave granularity that properly
> fans out when routing an x1 interleave to a greater than x1 interleave.
>
> That all worked well, until this config came along:
> Host Bridge: 2 way at 256 granularity
> Switch Decoder_A: 1 way at 512
> Endpoint_X: 2 way at 512
> Switch Decoder_B: 1 way at 512
> Endpoint_Y: 2 way at 512
>
> When the Host Bridge interleave is greater that 1, and the root
s/that/than/
> decoder interleave is exactly 1, the region driver needs to
> consider the number of targets in the region when calculating
> the expected granularity.
>
> While examining the existing logic, and trying to cover the case
> above, a couple of simplifications appeared, hence this proposed
> refactoring.
>
> The first simplicfication is to apply the logic to the unencrypted
s/unencrypted/nominal/
> values and use the existing helper function granularity_to_eig() to
> translate the desired granularity to the encrypted form. This means
> the comment and code regarding setting address bits is discarded.
> Although that logic was not wrong, it adds a level of complexity that
> is not required in the granularity selection. The eig and eiw are
> indeed part of the routing instructions programmed into the decoders.
> Up-level the discussion to plain ways and granularity for clearer
> analysis.
>
> The second simplification reduces the logic to a single granularity
> calculation that works for all cases. The new calculation doesn't
> care if parent_iw => 1, because parent_iw is used as a multiplier.
>
> The refactor cleans up a useless assignment of eiw, made after the iw
> is already calculated.
>
> Regression testing included an examination of all of the ways and
> granularity selections made during a run of the cxl_test unit tests.
> There were no differences in selections before and after this patch.
Fixes: ("27b3f8d13830 cxl/region: Program target lists")
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> drivers/cxl/core/region.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
Fix *and* less code!? Yes, please!
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e115ba382e04..5a1cc59cca99 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1154,16 +1154,15 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> }
>
> /*
> - * If @parent_port is masking address bits, pick the next unused address
> - * bit to route @port's targets.
> + * Interleave granularity is a multiple of @parent_port granularity.
> + * Multiplier is the parent port interleave ways.
> */
> - if (parent_iw > 1 && cxl_rr->nr_targets > 1) {
> - u32 address_bit = max(peig + peiw, eiw + peig);
> -
> - eig = address_bit - eiw + 1;
> - } else {
> - eiw = peiw;
> - eig = peig;
> + rc = granularity_to_eig(parent_ig * parent_iw, &eig);
I was going to say "wait, does this work with x3 parent_iw", and I
believe it does because this:
/*
* For purposes of address bit routing, use power-of-2 math for
* switch ports.
*/
if (!is_power_of_2(parent_iw))
parent_iw /= 3;
...is handled in the is_cxl_root(parent_port) case.
A thing of beauty this patch. Ship it! (modulo those minor nits above).
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] cxl/region: Refactor granularity select in cxl_port_setup_targets()
2023-08-05 2:49 ` Dan Williams
@ 2023-08-15 0:27 ` Alison Schofield
0 siblings, 0 replies; 7+ messages in thread
From: Alison Schofield @ 2023-08-15 0:27 UTC (permalink / raw)
To: Dan Williams
Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Vishal Verma,
Ira Weiny, linux-cxl
On Fri, Aug 04, 2023 at 07:49:14PM -0700, Dan Williams wrote:
> alison.schofield@ wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> >
> > In cxl_port_setup_targets() the region driver validates the
> > configuration of auto-discovered region decoders, as well
> > as decoders the driver is preparing to program.
> >
> > The existing calculations use the encrypted interleave granularity
>
> s/encrypted/encoded/
Got it.
>
> > value, the eig, to create an interleave granularity that properly
> > fans out when routing an x1 interleave to a greater than x1 interleave.
> >
> > That all worked well, until this config came along:
> > Host Bridge: 2 way at 256 granularity
> > Switch Decoder_A: 1 way at 512
> > Endpoint_X: 2 way at 512
> > Switch Decoder_B: 1 way at 512
> > Endpoint_Y: 2 way at 512
> >
> > When the Host Bridge interleave is greater that 1, and the root
>
> s/that/than/
Got it.
> > decoder interleave is exactly 1, the region driver needs to
> > consider the number of targets in the region when calculating
> > the expected granularity.
> >
> > While examining the existing logic, and trying to cover the case
> > above, a couple of simplifications appeared, hence this proposed
> > refactoring.
> >
> > The first simplicfication is to apply the logic to the unencrypted
>
> s/unencrypted/nominal/
Got it.
>
> > values and use the existing helper function granularity_to_eig() to
> > translate the desired granularity to the encrypted form. This means
> > the comment and code regarding setting address bits is discarded.
> > Although that logic was not wrong, it adds a level of complexity that
> > is not required in the granularity selection. The eig and eiw are
> > indeed part of the routing instructions programmed into the decoders.
> > Up-level the discussion to plain ways and granularity for clearer
> > analysis.
> >
> > The second simplification reduces the logic to a single granularity
> > calculation that works for all cases. The new calculation doesn't
> > care if parent_iw => 1, because parent_iw is used as a multiplier.
> >
> > The refactor cleans up a useless assignment of eiw, made after the iw
> > is already calculated.
> >
> > Regression testing included an examination of all of the ways and
> > granularity selections made during a run of the cxl_test unit tests.
> > There were no differences in selections before and after this patch.
>
> Fixes: ("27b3f8d13830 cxl/region: Program target lists")
Got it.
>
> >
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> > drivers/cxl/core/region.c | 17 ++++++++---------
> > 1 file changed, 8 insertions(+), 9 deletions(-)
>
> Fix *and* less code!? Yes, please!
>
:)
> >
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index e115ba382e04..5a1cc59cca99 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1154,16 +1154,15 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> > }
> >
> > /*
> > - * If @parent_port is masking address bits, pick the next unused address
> > - * bit to route @port's targets.
> > + * Interleave granularity is a multiple of @parent_port granularity.
> > + * Multiplier is the parent port interleave ways.
> > */
> > - if (parent_iw > 1 && cxl_rr->nr_targets > 1) {
> > - u32 address_bit = max(peig + peiw, eiw + peig);
> > -
> > - eig = address_bit - eiw + 1;
> > - } else {
> > - eiw = peiw;
> > - eig = peig;
> > + rc = granularity_to_eig(parent_ig * parent_iw, &eig);
>
> I was going to say "wait, does this work with x3 parent_iw", and I
> believe it does because this:
>
> /*
> * For purposes of address bit routing, use power-of-2 math for
> * switch ports.
> */
> if (!is_power_of_2(parent_iw))
> parent_iw /= 3;
>
> ...is handled in the is_cxl_root(parent_port) case.
I think I'll mention the above in the commit log.
Thanks!
>
> A thing of beauty this patch. Ship it! (modulo those minor nits above).
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cxl/region: Refactor granularity select in cxl_port_setup_targets()
2023-08-04 23:27 [PATCH] cxl/region: Refactor granularity select in cxl_port_setup_targets() alison.schofield
2023-08-05 2:49 ` Dan Williams
@ 2023-08-07 13:26 ` Jonathan Cameron
2023-08-15 0:23 ` Alison Schofield
2023-08-09 22:28 ` Dave Jiang
2 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2023-08-07 13:26 UTC (permalink / raw)
To: alison.schofield
Cc: Davidlohr Bueso, Dave Jiang, Vishal Verma, Ira Weiny,
Dan Williams, linux-cxl
On Fri, 4 Aug 2023 16:27:26 -0700
alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> In cxl_port_setup_targets() the region driver validates the
> configuration of auto-discovered region decoders, as well
> as decoders the driver is preparing to program.
>
> The existing calculations use the encrypted interleave granularity
> value, the eig, to create an interleave granularity that properly
> fans out when routing an x1 interleave to a greater than x1 interleave.
>
> That all worked well, until this config came along:
> Host Bridge: 2 way at 256 granularity
> Switch Decoder_A: 1 way at 512
> Endpoint_X: 2 way at 512
> Switch Decoder_B: 1 way at 512
> Endpoint_Y: 2 way at 512
Not sure this is a valid example
Those endpoints need to have 2 way at 256 I think...
Mapping out the result...
0000-0255 RP 0 -> Switch Decoder_A -> End_Point_X DPA 0-255
0256-0511 RP 1 -> Switch Decoder_B -> End_point_Y DPA 256-511 *
0512-0767 RP 0 -> Switch Decoder_A -> End_point_X DPA 0-255 *
0768-1023 RP 1 -> Switch Decoder_B -> End_point_Y DPA 256-511
1024-1279 RP 0 -> Switch Decoder_A -> End_point_X
* address bit for 512 dropped not the one for 256
>
> When the Host Bridge interleave is greater that 1, and the root
> decoder interleave is exactly 1, the region driver needs to
> consider the number of targets in the region when calculating
> the expected granularity.
>
> While examining the existing logic, and trying to cover the case
> above, a couple of simplifications appeared, hence this proposed
> refactoring.
>
> The first simplicfication is to apply the logic to the unencrypted
> values and use the existing helper function granularity_to_eig() to
> translate the desired granularity to the encrypted form. This means
> the comment and code regarding setting address bits is discarded.
> Although that logic was not wrong, it adds a level of complexity that
> is not required in the granularity selection. The eig and eiw are
> indeed part of the routing instructions programmed into the decoders.
> Up-level the discussion to plain ways and granularity for clearer
> analysis.
>
> The second simplification reduces the logic to a single granularity
> calculation that works for all cases. The new calculation doesn't
> care if parent_iw => 1, because parent_iw is used as a multiplier.
>
> The refactor cleans up a useless assignment of eiw, made after the iw
> is already calculated.
>
> Regression testing included an examination of all of the ways and
> granularity selections made during a run of the cxl_test unit tests.
> There were no differences in selections before and after this patch.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> drivers/cxl/core/region.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e115ba382e04..5a1cc59cca99 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1154,16 +1154,15 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> }
>
> /*
> - * If @parent_port is masking address bits, pick the next unused address
> - * bit to route @port's targets.
> + * Interleave granularity is a multiple of @parent_port granularity.
> + * Multiplier is the parent port interleave ways.
> */
> - if (parent_iw > 1 && cxl_rr->nr_targets > 1) {
> - u32 address_bit = max(peig + peiw, eiw + peig);
> -
> - eig = address_bit - eiw + 1;
> - } else {
> - eiw = peiw;
> - eig = peig;
> + rc = granularity_to_eig(parent_ig * parent_iw, &eig);
> + if (rc) {
> + dev_dbg(&cxlr->dev,
> + "%s: invalid granularity calculation (%d * %d)\n",
> + dev_name(&parent_port->dev), parent_ig, parent_iw);
> + return rc;
> }
>
> rc = eig_to_granularity(eig, &ig);
>
> base-commit: fe77cc2e5a6a7c85f5c6ef8a39d7694ffc7f41c9
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] cxl/region: Refactor granularity select in cxl_port_setup_targets()
2023-08-07 13:26 ` Jonathan Cameron
@ 2023-08-15 0:23 ` Alison Schofield
0 siblings, 0 replies; 7+ messages in thread
From: Alison Schofield @ 2023-08-15 0:23 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Davidlohr Bueso, Dave Jiang, Vishal Verma, Ira Weiny,
Dan Williams, linux-cxl
On Mon, Aug 07, 2023 at 02:26:44PM +0100, Jonathan Cameron wrote:
> On Fri, 4 Aug 2023 16:27:26 -0700
> alison.schofield@intel.com wrote:
>
> > From: Alison Schofield <alison.schofield@intel.com>
> >
> > In cxl_port_setup_targets() the region driver validates the
> > configuration of auto-discovered region decoders, as well
> > as decoders the driver is preparing to program.
> >
> > The existing calculations use the encrypted interleave granularity
> > value, the eig, to create an interleave granularity that properly
> > fans out when routing an x1 interleave to a greater than x1 interleave.
> >
> > That all worked well, until this config came along:
> > Host Bridge: 2 way at 256 granularity
> > Switch Decoder_A: 1 way at 512
> > Endpoint_X: 2 way at 512
> > Switch Decoder_B: 1 way at 512
> > Endpoint_Y: 2 way at 512
>
> Not sure this is a valid example
> Those endpoints need to have 2 way at 256 I think...
>
> Mapping out the result...
>
> 0000-0255 RP 0 -> Switch Decoder_A -> End_Point_X DPA 0-255
> 0256-0511 RP 1 -> Switch Decoder_B -> End_point_Y DPA 256-511 *
> 0512-0767 RP 0 -> Switch Decoder_A -> End_point_X DPA 0-255 *
> 0768-1023 RP 1 -> Switch Decoder_B -> End_point_Y DPA 256-511
> 1024-1279 RP 0 -> Switch Decoder_A -> End_point_X
>
> * address bit for 512 dropped not the one for 256
Jonathan,
Sorry for the typo. The endpoints were indeed iw: 2 ig: 256.
I appreciate your mapping.
Thanks,
Alison
> >
> > When the Host Bridge interleave is greater that 1, and the root
> > decoder interleave is exactly 1, the region driver needs to
> > consider the number of targets in the region when calculating
> > the expected granularity.
> >
> > While examining the existing logic, and trying to cover the case
> > above, a couple of simplifications appeared, hence this proposed
> > refactoring.
> >
> > The first simplicfication is to apply the logic to the unencrypted
> > values and use the existing helper function granularity_to_eig() to
> > translate the desired granularity to the encrypted form. This means
> > the comment and code regarding setting address bits is discarded.
> > Although that logic was not wrong, it adds a level of complexity that
> > is not required in the granularity selection. The eig and eiw are
> > indeed part of the routing instructions programmed into the decoders.
> > Up-level the discussion to plain ways and granularity for clearer
> > analysis.
> >
> > The second simplification reduces the logic to a single granularity
> > calculation that works for all cases. The new calculation doesn't
> > care if parent_iw => 1, because parent_iw is used as a multiplier.
> >
> > The refactor cleans up a useless assignment of eiw, made after the iw
> > is already calculated.
> >
> > Regression testing included an examination of all of the ways and
> > granularity selections made during a run of the cxl_test unit tests.
> > There were no differences in selections before and after this patch.
> >
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> > drivers/cxl/core/region.c | 17 ++++++++---------
> > 1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index e115ba382e04..5a1cc59cca99 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1154,16 +1154,15 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> > }
> >
> > /*
> > - * If @parent_port is masking address bits, pick the next unused address
> > - * bit to route @port's targets.
> > + * Interleave granularity is a multiple of @parent_port granularity.
> > + * Multiplier is the parent port interleave ways.
> > */
> > - if (parent_iw > 1 && cxl_rr->nr_targets > 1) {
> > - u32 address_bit = max(peig + peiw, eiw + peig);
> > -
> > - eig = address_bit - eiw + 1;
> > - } else {
> > - eiw = peiw;
> > - eig = peig;
> > + rc = granularity_to_eig(parent_ig * parent_iw, &eig);
> > + if (rc) {
> > + dev_dbg(&cxlr->dev,
> > + "%s: invalid granularity calculation (%d * %d)\n",
> > + dev_name(&parent_port->dev), parent_ig, parent_iw);
> > + return rc;
> > }
> >
> > rc = eig_to_granularity(eig, &ig);
> >
> > base-commit: fe77cc2e5a6a7c85f5c6ef8a39d7694ffc7f41c9
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cxl/region: Refactor granularity select in cxl_port_setup_targets()
2023-08-04 23:27 [PATCH] cxl/region: Refactor granularity select in cxl_port_setup_targets() alison.schofield
2023-08-05 2:49 ` Dan Williams
2023-08-07 13:26 ` Jonathan Cameron
@ 2023-08-09 22:28 ` Dave Jiang
2023-08-15 0:17 ` Alison Schofield
2 siblings, 1 reply; 7+ messages in thread
From: Dave Jiang @ 2023-08-09 22:28 UTC (permalink / raw)
To: alison.schofield, Davidlohr Bueso, Jonathan Cameron, Vishal Verma,
Ira Weiny, Dan Williams
Cc: linux-cxl
On 8/4/23 16:27, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> In cxl_port_setup_targets() the region driver validates the
> configuration of auto-discovered region decoders, as well
> as decoders the driver is preparing to program.
>
> The existing calculations use the encrypted interleave granularity
s/encrypted/encoded/
> value, the eig, to create an interleave granularity that properly
> fans out when routing an x1 interleave to a greater than x1 interleave.
>
> That all worked well, until this config came along:
> Host Bridge: 2 way at 256 granularity
> Switch Decoder_A: 1 way at 512
> Endpoint_X: 2 way at 512
> Switch Decoder_B: 1 way at 512
> Endpoint_Y: 2 way at 512
>
> When the Host Bridge interleave is greater that 1, and the root
> decoder interleave is exactly 1, the region driver needs to
> consider the number of targets in the region when calculating
> the expected granularity.
>
> While examining the existing logic, and trying to cover the case
> above, a couple of simplifications appeared, hence this proposed
> refactoring.
>
> The first simplicfication is to apply the logic to the unencrypted
s/simplicfication/simplification/
s/unencrypted/normal?/
> values and use the existing helper function granularity_to_eig() to
> translate the desired granularity to the encrypted form. This means
s/encrypted/encoded/
> the comment and code regarding setting address bits is discarded.
> Although that logic was not wrong, it adds a level of complexity that
> is not required in the granularity selection. The eig and eiw are
> indeed part of the routing instructions programmed into the decoders.
> Up-level the discussion to plain ways and granularity for clearer
> analysis.
>
> The second simplification reduces the logic to a single granularity
> calculation that works for all cases. The new calculation doesn't
> care if parent_iw => 1, because parent_iw is used as a multiplier.
>
> The refactor cleans up a useless assignment of eiw, made after the iw
> is already calculated.
>
> Regression testing included an examination of all of the ways and
> granularity selections made during a run of the cxl_test unit tests.
> There were no differences in selections before and after this patch.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> drivers/cxl/core/region.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e115ba382e04..5a1cc59cca99 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1154,16 +1154,15 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> }
>
> /*
> - * If @parent_port is masking address bits, pick the next unused address
> - * bit to route @port's targets.
> + * Interleave granularity is a multiple of @parent_port granularity.
> + * Multiplier is the parent port interleave ways.
> */
> - if (parent_iw > 1 && cxl_rr->nr_targets > 1) {
> - u32 address_bit = max(peig + peiw, eiw + peig);
> -
> - eig = address_bit - eiw + 1;
> - } else {
> - eiw = peiw;
> - eig = peig;
> + rc = granularity_to_eig(parent_ig * parent_iw, &eig);
> + if (rc) {
> + dev_dbg(&cxlr->dev,
> + "%s: invalid granularity calculation (%d * %d)\n",
> + dev_name(&parent_port->dev), parent_ig, parent_iw);
> + return rc;
> }
>
> rc = eig_to_granularity(eig, &ig);
>
> base-commit: fe77cc2e5a6a7c85f5c6ef8a39d7694ffc7f41c9
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] cxl/region: Refactor granularity select in cxl_port_setup_targets()
2023-08-09 22:28 ` Dave Jiang
@ 2023-08-15 0:17 ` Alison Schofield
0 siblings, 0 replies; 7+ messages in thread
From: Alison Schofield @ 2023-08-15 0:17 UTC (permalink / raw)
To: Dave Jiang
Cc: Davidlohr Bueso, Jonathan Cameron, Vishal Verma, Ira Weiny,
Dan Williams, linux-cxl
On Wed, Aug 09, 2023 at 03:28:36PM -0700, Dave Jiang wrote:
>
>
> On 8/4/23 16:27, alison.schofield@intel.com wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> >
> > In cxl_port_setup_targets() the region driver validates the
> > configuration of auto-discovered region decoders, as well
> > as decoders the driver is preparing to program.
> >
> > The existing calculations use the encrypted interleave granularity
>
> s/encrypted/encoded/
Thanks. I'll clean up the language around this to use encoded & normal.
>
> > value, the eig, to create an interleave granularity that properly
> > fans out when routing an x1 interleave to a greater than x1 interleave.
> >
> > That all worked well, until this config came along:
> > Host Bridge: 2 way at 256 granularity
> > Switch Decoder_A: 1 way at 512
> > Endpoint_X: 2 way at 512
> > Switch Decoder_B: 1 way at 512
> > Endpoint_Y: 2 way at 512
> >
> > When the Host Bridge interleave is greater that 1, and the root
> > decoder interleave is exactly 1, the region driver needs to
> > consider the number of targets in the region when calculating
> > the expected granularity.
> >
> > While examining the existing logic, and trying to cover the case
> > above, a couple of simplifications appeared, hence this proposed
> > refactoring.
> >
> > The first simplicfication is to apply the logic to the unencrypted
> s/simplicfication/simplification/
> s/unencrypted/normal?/
Got it.
>
>
> > values and use the existing helper function granularity_to_eig() to
> > translate the desired granularity to the encrypted form. This means
>
> s/encrypted/encoded/
Got it.
>
> > the comment and code regarding setting address bits is discarded.
> > Although that logic was not wrong, it adds a level of complexity that
> > is not required in the granularity selection. The eig and eiw are
> > indeed part of the routing instructions programmed into the decoders.
> > Up-level the discussion to plain ways and granularity for clearer
> > analysis.
> >
> > The second simplification reduces the logic to a single granularity
> > calculation that works for all cases. The new calculation doesn't
> > care if parent_iw => 1, because parent_iw is used as a multiplier.
> >
> > The refactor cleans up a useless assignment of eiw, made after the iw
> > is already calculated.
> >
> > Regression testing included an examination of all of the ways and
> > granularity selections made during a run of the cxl_test unit tests.
> > There were no differences in selections before and after this patch.
> >
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> > drivers/cxl/core/region.c | 17 ++++++++---------
> > 1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index e115ba382e04..5a1cc59cca99 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1154,16 +1154,15 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> > }
> > /*
> > - * If @parent_port is masking address bits, pick the next unused address
> > - * bit to route @port's targets.
> > + * Interleave granularity is a multiple of @parent_port granularity.
> > + * Multiplier is the parent port interleave ways.
> > */
> > - if (parent_iw > 1 && cxl_rr->nr_targets > 1) {
> > - u32 address_bit = max(peig + peiw, eiw + peig);
> > -
> > - eig = address_bit - eiw + 1;
> > - } else {
> > - eiw = peiw;
> > - eig = peig;
> > + rc = granularity_to_eig(parent_ig * parent_iw, &eig);
> > + if (rc) {
> > + dev_dbg(&cxlr->dev,
> > + "%s: invalid granularity calculation (%d * %d)\n",
> > + dev_name(&parent_port->dev), parent_ig, parent_iw);
> > + return rc;
> > }
> > rc = eig_to_granularity(eig, &ig);
> >
> > base-commit: fe77cc2e5a6a7c85f5c6ef8a39d7694ffc7f41c9
^ permalink raw reply [flat|nested] 7+ messages in thread