Re: [PATCH v4 35/40] cxl/core/port: Add endpoint decoders

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

On Mon, Feb 14, 2022 at 9:47 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Wed, 02 Feb 2022 20:02:06 -0800
> Dan Williams <dan.j.williams@intel.com> wrote:
>
> > From: Ben Widawsky <ben.widawsky@intel.com>
> >
> > Recall that a CXL Port is any object that publishes a CXL HDM Decoder
> > Capability structure. That is Host Bridge and Switches that have been
> > enabled so far. Now, add decoder support to the 'endpoint' CXL Ports
> > registered by the cxl_mem driver. They mostly share the same enumeration
> > as Bridges and Switches, but witout a target list. The target of
> > endpoint decode is device-internal DPA space, not another downstream
> > port.
> >
> > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > [djbw: clarify changelog, hookup enumeration in the port driver]
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> ...
>
> > index f5e5b4ac8228..990b6670222e 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -346,6 +346,7 @@ struct cxl_decoder *cxl_root_decoder_alloc(struct cxl_port *port,
> >  struct cxl_decoder *cxl_switch_decoder_alloc(struct cxl_port *port,
> >                                            unsigned int nr_targets);
> >  int cxl_decoder_add(struct cxl_decoder *cxld, int *target_map);
> > +struct cxl_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port);
> >  int cxl_decoder_add_locked(struct cxl_decoder *cxld, int *target_map);
> >  int cxl_decoder_autoremove(struct device *host, struct cxl_decoder *cxld);
> >  int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint);
> > diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> > index 4d4e23b9adff..d420da5fc39c 100644
> > --- a/drivers/cxl/port.c
> > +++ b/drivers/cxl/port.c
> > @@ -40,16 +40,17 @@ static int cxl_port_probe(struct device *dev)
> >               struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport);
> >
> >               get_device(&cxlmd->dev);
> > -             return devm_add_action_or_reset(dev, schedule_detach, cxlmd);
> > +             rc = devm_add_action_or_reset(dev, schedule_detach, cxlmd);
> > +             if (rc)
> > +                     return rc;
> > +     } else {
> > +             rc = devm_cxl_port_enumerate_dports(port);
> > +             if (rc < 0)
> > +                     return rc;
> > +             if (rc == 1)
> > +                     return devm_cxl_add_passthrough_decoder(port);
>
> This is just a convenient place to ask a question rather that really being
> connected to this patch.
>
> 8.2.5.12 in CXL r2.0
>
> "A CXL Host Bridge is identified as an ACPI device with Host Interface ID (HID) of
> “ACPI0016” and is associated with one or more CXL Root ports. Any CXL 2.0 Host
> Bridge that is associated with more than one CXL Root Port must contain one instance
> of this capability structure in the CHBCR. This capability structure resolves the target
> CXL Root Ports for a given memory address."
>
> Suggests to me that there may be an HDM decoder in the one port case and it may need
> programming.
>
> Hitting this in QEMU but I suspect it'll occur in real systems as well.

It seems reasonable to wait for a real system like that to arrive. If
someone is cheap enough to build a one-port host bridge would they
spend the silicon gates on these unnecessary registers?