Re: [RFC PATCH v2 08/28] cxl/port: Introduce a port driver

From: Dan Williams <dan.j.williams@intel.com>
To: Ben Widawsky <ben.widawsky@intel.com>
Cc: linux-cxl@vger.kernel.org,
	Chet Douglas <chet.r.douglas@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Vishal Verma <vishal.l.verma@intel.com>
Subject: Re: [RFC PATCH v2 08/28] cxl/port: Introduce a port driver
Date: Sun, 31 Oct 2021 11:10:23 -0700	[thread overview]
Message-ID: <CAPcyv4gpA=DH0SQvRdmF6dY01mZ1S-gGEWTSDbb+0ajYtyNv0A@mail.gmail.com> (raw)
In-Reply-To: <CAPcyv4jUDXuV-R8HFzJb1K5HK6xaGMor3JH-4AFKJqPJk9FtdA@mail.gmail.com>

On Sun, Oct 31, 2021 at 10:53 AM Dan Williams <dan.j.williams@intel.com> wrote:
>
> On Fri, Oct 29, 2021 at 6:37 PM Dan Williams <dan.j.williams@intel.com> wrote:
> [..]
> >
> > Proposed infrastructure for a new cxl_port_decoder_autoremove():
> >
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index 3163167ecc3a..78f8313a1069 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -374,6 +374,11 @@ static int add_root_nvdimm_bridge(struct device
> > *match, void *data)
> >         return 1;
> >  }
> >
> > +static void clear_cxl_topology_host(void *data)
> > +{
> > +       set_cxl_topology_host(NULL);
> > +}
> > +
> >  static int cxl_acpi_probe(struct platform_device *pdev)
> >  {
> >         int rc;
> > @@ -382,6 +387,11 @@ static int cxl_acpi_probe(struct platform_device *pdev)
> >         struct acpi_device *adev = ACPI_COMPANION(host);
> >         struct cxl_cfmws_context ctx;
> >
> > +       set_cxl_topology_host(host);
> > +       rc = devm_add_action_or_reset(host, clear_cxl_topology_host, host);
> > +       if (rc)
> > +               return rc;
> > +
> >         root_port = devm_cxl_add_port(host, host, CXL_RESOURCE_NONE, NULL);
> >         if (IS_ERR(root_port))
> >                 return PTR_ERR(root_port);
> > diff --git a/drivers/cxl/core/bus.c b/drivers/cxl/core/bus.c
> > index 17a4fff029f8..3146b6aa0a2f 100644
> > --- a/drivers/cxl/core/bus.c
> > +++ b/drivers/cxl/core/bus.c
> > @@ -3,6 +3,7 @@
> >  #include <linux/io-64-nonatomic-lo-hi.h>
> >  #include <linux/device.h>
> >  #include <linux/module.h>
> > +#include <linux/rwsem.h>
> >  #include <linux/pci.h>
> >  #include <linux/slab.h>
> >  #include <linux/idr.h>
> > @@ -563,6 +564,84 @@ int cxl_decoder_autoremove(struct device *host,
> > struct cxl_decoder *cxld)
> >  }
> >  EXPORT_SYMBOL_NS_GPL(cxl_decoder_autoremove, CXL);
> >
> > +void trigger_decoder_autoremove(void *data)
> > +{
> > +       struct cxl_decoder *cxld = data;
> > +       struct device *host = get_cxl_topology_host();
> > +
> > +       /* The topology host driver beat us to the punch */
> > +       if (!host)
> > +               return;
>
> This can be dropped or turned into a WARN_ON() because something is
> wrong if the port removal is being triggered after the topology host
> has already went away.

It further occurs to me that if all ports are guaranteed to be torn
down by topology host removal, then we might not even need to play
these games at all. cxl_acpi is already unregistering all of its child
ports, we just need the property that if a port adds a child port it
remove it. Then I don't think we even need cxl_topology_host games.