Re: [PATCH 12/13] cxl/region: Record host bridge target list

[Dan Williams <dan.j.williams@intel.com>]

On Fri, Jan 7, 2022 at 10:15 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Thu,  6 Jan 2022 16:37:55 -0800
> Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> > Part of host bridge verification in the CXL Type 3 Memory Device
> > Software Guide calculates the host bridge interleave target list (6th
> > step in the flow chart). With host bridge verification already done, it
> > is trivial to store away the configuration information.
> >
> > TODO: Needs support for switches (7th step in the flow chart).
> >
> > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> > ---
> >  drivers/cxl/region.c | 41 +++++++++++++++++++++++++++++++----------
> >  1 file changed, 31 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
> > index eafd95419895..3120b65b0bc5 100644
> > --- a/drivers/cxl/region.c
> > +++ b/drivers/cxl/region.c
> > @@ -385,6 +385,7 @@ static bool region_hb_rp_config_valid(struct cxl_region *region,
> >       }
> >
> >       for (i = 0; i < hb_count; i++) {
> > +             struct cxl_decoder *cxld;
> >               int idx, position_mask;
> >               struct cxl_dport *rp;
> >               struct cxl_port *hb;
> > @@ -422,10 +423,8 @@ static bool region_hb_rp_config_valid(struct cxl_region *region,
> >                               if (get_rp(ep) != rp)
> >                                       continue;
> >
> > -                             if (port_grouping == -1) {
> > +                             if (port_grouping == -1)
> >                                       port_grouping = idx & position_mask;
> > -                                     continue;
> > -                             }
> >
> >                               /*
> >                                * Do all devices in the region connected to this CXL
> > @@ -436,10 +435,32 @@ static bool region_hb_rp_config_valid(struct cxl_region *region,
> >                                                         "One or more devices are not connected to the correct Host Bridge Root Port\n");
> >                                       return false;
> >                               }
> > +
> > +                             if (!state_update)
> > +                                     continue;
> > +
> > +                             if (dev_WARN_ONCE(&cxld->dev,
> > +                                               port_grouping >= cxld->nr_targets,
> > +                                               "Invalid port grouping %d/%d\n",
> > +                                               port_grouping, cxld->nr_targets))
> > +                                     return false;
> > +
> > +                             cxld->interleave_ways++;
> > +                             cxld->target[port_grouping] = get_rp(ep);
>
> Hi Ben,
>
> Just one more based on debug rather than review.
>
> The reason is across 2 patches so not necessary obvious from what is visible here,
> but port_grouping here for a 2hb, 2rp on each and 1 ep on each of those
> case goes 0,1,2,3 resulting in us setting one of the host bridges to have
> a decoder with targets 2 and 3 rather than 0 and 1 set.
>
> I haven't figured out a particularly good solution yet...  If everything is nice and symmetric
> and power of 2 then you can simply change the mask on the index to reflect num_root_ports / num_host_bridges
>
> With that change in place my decoders all look good on this particular configuration :)
> Note this is eyeball based testing only and on just one configuration so far.
>
> I'll have to try your tool as it is really annoying that the mem devices change order on every
> boot as my script is dumb currently so have to edit it every run.

I have support pending for cxl-cli that lets memdevs be filtered by
serial number so you don't need to worry about dynamic kernel device
names.

Should be posted later today.