Re: [PATCH 6/7] cxl/region: Fix 'distance' calculation with passthrough ports

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Thu, 2022-11-03 at 17:30 -0700, Dan Williams wrote:
> When programming port decode targets, the algorithm wants to ensure that
> two devices are compatible to be programmed as peers beneath a given
> port. A compatible peer is a target that shares the same dport, and
> where that target's interleave position also routes it to the same
> dport. Compatibility is determined by the device's interleave position
> being >= to distance. For example, if a given dport can only map every
> Nth position then positions less than N away from the last target
> programmed are incompatible.
> 
> The @distance for the host-bridge-cxl_port a simple dual-ported
                                   ^
Is this meant to be "the distance for the host-bridge to a cxl_port"?

Also missing '/in/ a simple dual-ported..'?

> host-bridge configuration with 2 direct-attached devices is 1. An x2
> region divded by 2 dports to reach 2 region targets.

The second sentence seems slightly incomprehensible too.
> 
> An x4 region under an x2 host-bridge would need 2 intervening switches
> where the @distance at the host bridge level is 2 (x4 region divided by
> 2 switches to reach 4 devices).
> 
> However, the distance between peers underneath a single ported
> host-bridge is always zero because there is no limit to the number of
> devices that can be mapped. In other words, there are no decoders to
> program in a passthrough, all descendants are mapped and distance only
> starts matters for the intervening descendant ports of the passthrough

starts to matter?

> port.
> 
> Add tracking for the number of dports mapped to a port, and use that to
> detect the passthrough case for calculating @distance.
> 
> Cc: <stable@vger.kernel.org>
> Reported-by: Bobo WL <lmw.bobo@gmail.com>
> Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Link: http://lore.kernel.org/r/20221010172057.00001559@huawei.com
> Fixes: 27b3f8d13830 ("cxl/region: Program target lists")
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/port.c   |   11 +++++++++--
>  drivers/cxl/core/region.c |    9 ++++++++-
>  drivers/cxl/cxl.h         |    2 ++
>  3 files changed, 19 insertions(+), 3 deletions(-)

Other than the above, looks good,

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

> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index bffde862de0b..e7556864ea80 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -811,6 +811,7 @@ static struct cxl_dport *find_dport(struct cxl_port *port, int id)
>  static int add_dport(struct cxl_port *port, struct cxl_dport *new)
>  {
>         struct cxl_dport *dup;
> +       int rc;
>  
>         device_lock_assert(&port->dev);
>         dup = find_dport(port, new->port_id);
> @@ -821,8 +822,14 @@ static int add_dport(struct cxl_port *port, struct cxl_dport *new)
>                         dev_name(dup->dport));
>                 return -EBUSY;
>         }
> -       return xa_insert(&port->dports, (unsigned long)new->dport, new,
> -                        GFP_KERNEL);
> +
> +       rc = xa_insert(&port->dports, (unsigned long)new->dport, new,
> +                      GFP_KERNEL);
> +       if (rc)
> +               return rc;
> +
> +       port->nr_dports++;
> +       return 0;
>  }
>  
>  /*
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index c52465e09f26..c0253de74945 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -990,7 +990,14 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>         if (cxl_rr->nr_targets_set) {
>                 int i, distance;
>  
> -               distance = p->nr_targets / cxl_rr->nr_targets;
> +               /*
> +                * Passthrough ports impose no distance requirements between
> +                * peers
> +                */
> +               if (port->nr_dports == 1)
> +                       distance = 0;
> +               else
> +                       distance = p->nr_targets / cxl_rr->nr_targets;
>                 for (i = 0; i < cxl_rr->nr_targets_set; i++)
>                         if (ep->dport == cxlsd->target[i]) {
>                                 rc = check_last_peer(cxled, ep, cxl_rr,
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 1164ad49f3d3..ac75554b5d76 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -457,6 +457,7 @@ struct cxl_pmem_region {
>   * @regions: cxl_region_ref instances, regions mapped by this port
>   * @parent_dport: dport that points to this port in the parent
>   * @decoder_ida: allocator for decoder ids
> + * @nr_dports: number of entries in @dports
>   * @hdm_end: track last allocated HDM decoder instance for allocation ordering
>   * @commit_end: cursor to track highest committed decoder for commit ordering
>   * @component_reg_phys: component register capability base address (optional)
> @@ -475,6 +476,7 @@ struct cxl_port {
>         struct xarray regions;
>         struct cxl_dport *parent_dport;
>         struct ida decoder_ida;
> +       int nr_dports;
>         int hdm_end;
>         int commit_end;
>         resource_size_t component_reg_phys;
>