diff mbox series

[v3,3/9] cxl/mem: Adjust cxl_mem_find_port() to find an RCH's port

Message ID 20221109104059.766720-4-rrichter@amd.com
State New, archived
Headers show
Series cxl: Add support for Restricted CXL hosts (RCD mode) | expand

Commit Message

Robert Richter Nov. 9, 2022, 10:40 a.m. UTC
The PCIe software view of an RCH and RCD is different to VH mode. An
RCD is paired with an RCH and shows up as RCiEP with a parent already
pointing to a PCI bridge (struct pci_host_bridge). In contrast, in VH
mode an PCI Express Endpoint is a PCI type 0 device with a PCI type 1
device as parent (struct pci_dev, most of the time a downstream switch
port, but could also be a root port). The following hierarchy applies
in VH mode:

 CXL memory device, cxl_memdev                               endpoint
 └──PCIe Endpoint (type 0), pci_dev                           |
    └──Downstream Port (type 1), pci_dev (Nth switch)        portN
       └──Upstream Port (type 1), pci_dev (Nth switch)        |
          :                                                   :
          └──Downstream Port (type 1), pci_dev (1st switch)  port1
             └──Upstream Port (type 1), pci_dev (1st switch)  |
                └──Root Port (type 1), pci_dev                |
                   └──PCI host bridge, pci_host_bridge       port0
                      :                                       |
                      :..ACPI0017, acpi_dev                  root

 (There can be zero or any other number of switches in between.)

An iterator through the grandparents takes us to the root port which
is registered as dport to the bridge. The next port an endpoint is
connected to can be determined by using the grandparent of the memory
device as a dport_dev in cxl_mem_find_port().

The same does not work in RCD mode where only an RCiEP is connected to
the host bridge:

 CXL memory device, cxl_memdev                               endpoint
 └──PCIe Endpoint (type 0), pci_dev                           |
    └──PCI host bridge, pci_host_bridge                      port0
       :                                                      |
       :..ACPI0017, acpi_dev                                 root

Here, an endpoint is directly connected to the host bridge without a
type 1 PCI device (root or downstream port) in between. To link the
endpoint to the correct port, the endpoint's PCI device (parent of the
memory device) must be taken as dport_dev arg in cxl_mem_find_port().

Change cxl_mem_find_port() to find an RCH's port.

Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/port.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Comments

Dan Williams Nov. 14, 2022, 11:45 p.m. UTC | #1
Robert Richter wrote:
> The PCIe software view of an RCH and RCD is different to VH mode. An
> RCD is paired with an RCH and shows up as RCiEP with a parent already
> pointing to a PCI bridge (struct pci_host_bridge). In contrast, in VH
> mode an PCI Express Endpoint is a PCI type 0 device with a PCI type 1
> device as parent (struct pci_dev, most of the time a downstream switch
> port, but could also be a root port). The following hierarchy applies
> in VH mode:
> 
>  CXL memory device, cxl_memdev                               endpoint
>  └──PCIe Endpoint (type 0), pci_dev                           |
>     └──Downstream Port (type 1), pci_dev (Nth switch)        portN
>        └──Upstream Port (type 1), pci_dev (Nth switch)        |
>           :                                                   :
>           └──Downstream Port (type 1), pci_dev (1st switch)  port1
>              └──Upstream Port (type 1), pci_dev (1st switch)  |
>                 └──Root Port (type 1), pci_dev                |
>                    └──PCI host bridge, pci_host_bridge       port0
>                       :                                       |
>                       :..ACPI0017, acpi_dev                  root
> 
>  (There can be zero or any other number of switches in between.)
> 
> An iterator through the grandparents takes us to the root port which
> is registered as dport to the bridge. The next port an endpoint is
> connected to can be determined by using the grandparent of the memory
> device as a dport_dev in cxl_mem_find_port().
> 
> The same does not work in RCD mode where only an RCiEP is connected to
> the host bridge:
> 
>  CXL memory device, cxl_memdev                               endpoint
>  └──PCIe Endpoint (type 0), pci_dev                           |
>     └──PCI host bridge, pci_host_bridge                      port0
>        :                                                      |
>        :..ACPI0017, acpi_dev                                 root
> 
> Here, an endpoint is directly connected to the host bridge without a
> type 1 PCI device (root or downstream port) in between. To link the
> endpoint to the correct port, the endpoint's PCI device (parent of the
> memory device) must be taken as dport_dev arg in cxl_mem_find_port().
> 
> Change cxl_mem_find_port() to find an RCH's port.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/cxl/core/port.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 0431ed860d8e..d10c3580719b 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1354,6 +1354,14 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>  	return rc;
>  }
>  
> +static inline bool is_cxl_restricted(struct cxl_memdev *cxlmd)
> +{
> +	struct device *parent = cxlmd->dev.parent;
> +	if (!dev_is_pci(parent))
> +		return false;
> +	return pci_pcie_type(to_pci_dev(parent)) == PCI_EXP_TYPE_RC_END;
> +}
> +
>  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>  {
>  	struct device *dev = &cxlmd->dev;
> @@ -1433,9 +1441,39 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_ports, CXL);
>  
> +/*
> + * CXL memory device and port hierarchy:
> + *
> + * VH mode:
> + *
> + * CXL memory device, cxl_memdev                               endpoint
> + * └──PCIe Endpoint (type 0), pci_dev                           |
> + *    └──Downstream Port (type 1), pci_dev (Nth switch)        portN
> + *       └──Upstream Port (type 1), pci_dev (Nth switch)        |
> + *          :                                                   :
> + *          └──Downstream Port (type 1), pci_dev (1st switch)  port1
> + *             └──Upstream Port (type 1), pci_dev (1st switch)  |
> + *                └──Root Port (type 1), pci_dev                |
> + *                   └──PCI host bridge, pci_host_bridge       port0
> + *                      :                                       |
> + *                      :..ACPI0017, acpi_dev                  root
> + *
> + * (There can be zero or any other number of switches in between.)
> + *
> + * RCD mode:
> + *
> + * CXL memory device, cxl_memdev                               endpoint
> + * └──PCIe Endpoint (type 0), pci_dev                           |
> + *    └──PCI host bridge, pci_host_bridge                      port0
> + *       :                                                      |
> + *       :..ACPI0017, acpi_dev                                 root
> + */
>  struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
>  				   struct cxl_dport **dport)
>  {
> +	if (is_cxl_restricted(cxlmd))
> +		return find_cxl_port(cxlmd->dev.parent, dport);
> +
>  	return find_cxl_port(grandparent(&cxlmd->dev), dport);

I do not see why this change is needed. For example:

# readlink -f /sys/bus/cxl/devices/mem0
/sys/devices/pci0000:38/0000:38:00.0/mem0
# cxl list -BT
[
  {
    "bus":"root0",
    "provider":"ACPI.CXL",
    "nr_dports":1,
    "dports":[
      {
        "dport":"pci0000:38",
        "id":49
      }
    ]
  }
]

...so, in this case, the grandparent of "mem0" is "pci0000:38", and
"pci0000:38" is a dport. Unmodified cxl_mem_find_port() will do the
right thing and find that this CXL RCIEP is directly connected to
"root0".
Robert Richter Nov. 15, 2022, 1:12 p.m. UTC | #2
On 14.11.22 15:45:19, Dan Williams wrote:
> Robert Richter wrote:
> > The PCIe software view of an RCH and RCD is different to VH mode. An
> > RCD is paired with an RCH and shows up as RCiEP with a parent already
> > pointing to a PCI bridge (struct pci_host_bridge). In contrast, in VH
> > mode an PCI Express Endpoint is a PCI type 0 device with a PCI type 1
> > device as parent (struct pci_dev, most of the time a downstream switch
> > port, but could also be a root port). The following hierarchy applies
> > in VH mode:
> > 
> >  CXL memory device, cxl_memdev                               endpoint
> >  └──PCIe Endpoint (type 0), pci_dev                           |
> >     └──Downstream Port (type 1), pci_dev (Nth switch)        portN
> >        └──Upstream Port (type 1), pci_dev (Nth switch)        |
> >           :                                                   :
> >           └──Downstream Port (type 1), pci_dev (1st switch)  port1
> >              └──Upstream Port (type 1), pci_dev (1st switch)  |
> >                 └──Root Port (type 1), pci_dev                |
> >                    └──PCI host bridge, pci_host_bridge       port0
> >                       :                                       |
> >                       :..ACPI0017, acpi_dev                  root
> > 
> >  (There can be zero or any other number of switches in between.)
> > 
> > An iterator through the grandparents takes us to the root port which
> > is registered as dport to the bridge. The next port an endpoint is
> > connected to can be determined by using the grandparent of the memory
> > device as a dport_dev in cxl_mem_find_port().
> > 
> > The same does not work in RCD mode where only an RCiEP is connected to
> > the host bridge:
> > 
> >  CXL memory device, cxl_memdev                               endpoint
> >  └──PCIe Endpoint (type 0), pci_dev                           |
> >     └──PCI host bridge, pci_host_bridge                      port0
> >        :                                                      |
> >        :..ACPI0017, acpi_dev                                 root
> > 
> > Here, an endpoint is directly connected to the host bridge without a
> > type 1 PCI device (root or downstream port) in between. To link the
> > endpoint to the correct port, the endpoint's PCI device (parent of the
> > memory device) must be taken as dport_dev arg in cxl_mem_find_port().
> > 
> > Change cxl_mem_find_port() to find an RCH's port.
> > 
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> > ---
> >  drivers/cxl/core/port.c | 38 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> > 
> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > index 0431ed860d8e..d10c3580719b 100644
> > --- a/drivers/cxl/core/port.c
> > +++ b/drivers/cxl/core/port.c
> > @@ -1354,6 +1354,14 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
> >  	return rc;
> >  }
> >  
> > +static inline bool is_cxl_restricted(struct cxl_memdev *cxlmd)
> > +{
> > +	struct device *parent = cxlmd->dev.parent;
> > +	if (!dev_is_pci(parent))
> > +		return false;
> > +	return pci_pcie_type(to_pci_dev(parent)) == PCI_EXP_TYPE_RC_END;
> > +}
> > +
> >  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> >  {
> >  	struct device *dev = &cxlmd->dev;
> > @@ -1433,9 +1441,39 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> >  }
> >  EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_ports, CXL);
> >  
> > +/*
> > + * CXL memory device and port hierarchy:
> > + *
> > + * VH mode:
> > + *
> > + * CXL memory device, cxl_memdev                               endpoint
> > + * └──PCIe Endpoint (type 0), pci_dev                           |
> > + *    └──Downstream Port (type 1), pci_dev (Nth switch)        portN
> > + *       └──Upstream Port (type 1), pci_dev (Nth switch)        |
> > + *          :                                                   :
> > + *          └──Downstream Port (type 1), pci_dev (1st switch)  port1
> > + *             └──Upstream Port (type 1), pci_dev (1st switch)  |
> > + *                └──Root Port (type 1), pci_dev                |
> > + *                   └──PCI host bridge, pci_host_bridge       port0
> > + *                      :                                       |
> > + *                      :..ACPI0017, acpi_dev                  root
> > + *
> > + * (There can be zero or any other number of switches in between.)
> > + *
> > + * RCD mode:
> > + *
> > + * CXL memory device, cxl_memdev                               endpoint
> > + * └──PCIe Endpoint (type 0), pci_dev                           |
> > + *    └──PCI host bridge, pci_host_bridge                      port0
> > + *       :                                                      |
> > + *       :..ACPI0017, acpi_dev                                 root
> > + */
> >  struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
> >  				   struct cxl_dport **dport)
> >  {
> > +	if (is_cxl_restricted(cxlmd))
> > +		return find_cxl_port(cxlmd->dev.parent, dport);
> > +
> >  	return find_cxl_port(grandparent(&cxlmd->dev), dport);
> 
> I do not see why this change is needed. For example:
> 
> # readlink -f /sys/bus/cxl/devices/mem0
> /sys/devices/pci0000:38/0000:38:00.0/mem0
> # cxl list -BT
> [
>   {
>     "bus":"root0",
>     "provider":"ACPI.CXL",
>     "nr_dports":1,
>     "dports":[
>       {
>         "dport":"pci0000:38",
>         "id":49
>       }
>     ]
>   }
> ]
> 
> ...so, in this case, the grandparent of "mem0" is "pci0000:38", and
> "pci0000:38" is a dport. Unmodified cxl_mem_find_port() will do the
> right thing and find that this CXL RCIEP is directly connected to
> "root0".

find_cxl_port() uses the dport_dev, not the uport_dev. A lookup of
pci0000:38 gives the cxl root (ACPI.CXL). Instead, the endpoint's
device (0000:38:00.0) must be used to get to the bridge
("pci0000:38").

There is a parent missing because there is no Root Port in the RCD
hierarchy, simplified example:

VH mode:

 CXL memory device, cxl_memdev                         endpoint <- cxlmd
 └──PCIe Endpoint (type 0), pci_dev                     |
    └──Downstream Port (type 1), pci_dev (1st switch)  port1    <- port1: registered as dport at port0
       └──Upstream Port (type 1), pci_dev (1st switch)  |          port1: grandparent(cxlmd)
          └──Root Port (type 1), pci_dev                |	<- pdev:  registered as dport at port0
             |                                          |          pdev:  grandparent(port1)
             └──PCI host bridge, pci_host_bridge       port0    <- find_cxl_port(grandparent(grandparent(cxlmd)))
                :                                       |          port0 enumerates root ports (pdev) as dports
                :..ACPI0017, acpi_dev                  root

Additional switches add another grandparent() level each (adding an
dport and uport).

RCD mode:

 CXL memory device, cxl_memdev                         endpoint <- cxlmd
 └──PCIe Endpoint (type 0), pci_dev                     |       <- pdev: registered as dport at port0
    |                                                   |          pdev: parent(cxlmd)
    └──PCI host bridge, pci_host_bridge                port0    <- find_cxl_port(parent(endpoint))
       :                                                |          port0 enumerates endpoint (RCiEP) as dports
       :..ACPI0017, acpi_dev                           root

I hope I could shed some light here.

-Robert
Dan Williams Nov. 15, 2022, 6:06 p.m. UTC | #3
Robert Richter wrote:
> On 14.11.22 15:45:19, Dan Williams wrote:
> > Robert Richter wrote:
> > > The PCIe software view of an RCH and RCD is different to VH mode. An
> > > RCD is paired with an RCH and shows up as RCiEP with a parent already
> > > pointing to a PCI bridge (struct pci_host_bridge). In contrast, in VH
> > > mode an PCI Express Endpoint is a PCI type 0 device with a PCI type 1
> > > device as parent (struct pci_dev, most of the time a downstream switch
> > > port, but could also be a root port). The following hierarchy applies
> > > in VH mode:
> > > 
> > >  CXL memory device, cxl_memdev                               endpoint
> > >  └──PCIe Endpoint (type 0), pci_dev                           |
> > >     └──Downstream Port (type 1), pci_dev (Nth switch)        portN
> > >        └──Upstream Port (type 1), pci_dev (Nth switch)        |
> > >           :                                                   :
> > >           └──Downstream Port (type 1), pci_dev (1st switch)  port1
> > >              └──Upstream Port (type 1), pci_dev (1st switch)  |
> > >                 └──Root Port (type 1), pci_dev                |
> > >                    └──PCI host bridge, pci_host_bridge       port0
> > >                       :                                       |
> > >                       :..ACPI0017, acpi_dev                  root
> > > 
> > >  (There can be zero or any other number of switches in between.)
> > > 
> > > An iterator through the grandparents takes us to the root port which
> > > is registered as dport to the bridge. The next port an endpoint is
> > > connected to can be determined by using the grandparent of the memory
> > > device as a dport_dev in cxl_mem_find_port().
> > > 
> > > The same does not work in RCD mode where only an RCiEP is connected to
> > > the host bridge:
> > > 
> > >  CXL memory device, cxl_memdev                               endpoint
> > >  └──PCIe Endpoint (type 0), pci_dev                           |
> > >     └──PCI host bridge, pci_host_bridge                      port0
> > >        :                                                      |
> > >        :..ACPI0017, acpi_dev                                 root
> > > 
> > > Here, an endpoint is directly connected to the host bridge without a
> > > type 1 PCI device (root or downstream port) in between. To link the
> > > endpoint to the correct port, the endpoint's PCI device (parent of the
> > > memory device) must be taken as dport_dev arg in cxl_mem_find_port().
> > > 
> > > Change cxl_mem_find_port() to find an RCH's port.
> > > 
> > > Signed-off-by: Robert Richter <rrichter@amd.com>
> > > ---
> > >  drivers/cxl/core/port.c | 38 ++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 38 insertions(+)
> > > 
> > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > > index 0431ed860d8e..d10c3580719b 100644
> > > --- a/drivers/cxl/core/port.c
> > > +++ b/drivers/cxl/core/port.c
> > > @@ -1354,6 +1354,14 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
> > >  	return rc;
> > >  }
> > >  
> > > +static inline bool is_cxl_restricted(struct cxl_memdev *cxlmd)
> > > +{
> > > +	struct device *parent = cxlmd->dev.parent;
> > > +	if (!dev_is_pci(parent))
> > > +		return false;
> > > +	return pci_pcie_type(to_pci_dev(parent)) == PCI_EXP_TYPE_RC_END;
> > > +}
> > > +
> > >  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> > >  {
> > >  	struct device *dev = &cxlmd->dev;
> > > @@ -1433,9 +1441,39 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> > >  }
> > >  EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_ports, CXL);
> > >  
> > > +/*
> > > + * CXL memory device and port hierarchy:
> > > + *
> > > + * VH mode:
> > > + *
> > > + * CXL memory device, cxl_memdev                               endpoint
> > > + * └──PCIe Endpoint (type 0), pci_dev                           |
> > > + *    └──Downstream Port (type 1), pci_dev (Nth switch)        portN
> > > + *       └──Upstream Port (type 1), pci_dev (Nth switch)        |
> > > + *          :                                                   :
> > > + *          └──Downstream Port (type 1), pci_dev (1st switch)  port1
> > > + *             └──Upstream Port (type 1), pci_dev (1st switch)  |
> > > + *                └──Root Port (type 1), pci_dev                |
> > > + *                   └──PCI host bridge, pci_host_bridge       port0
> > > + *                      :                                       |
> > > + *                      :..ACPI0017, acpi_dev                  root
> > > + *
> > > + * (There can be zero or any other number of switches in between.)
> > > + *
> > > + * RCD mode:
> > > + *
> > > + * CXL memory device, cxl_memdev                               endpoint
> > > + * └──PCIe Endpoint (type 0), pci_dev                           |
> > > + *    └──PCI host bridge, pci_host_bridge                      port0
> > > + *       :                                                      |
> > > + *       :..ACPI0017, acpi_dev                                 root
> > > + */
> > >  struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
> > >  				   struct cxl_dport **dport)
> > >  {
> > > +	if (is_cxl_restricted(cxlmd))
> > > +		return find_cxl_port(cxlmd->dev.parent, dport);
> > > +
> > >  	return find_cxl_port(grandparent(&cxlmd->dev), dport);
> > 
> > I do not see why this change is needed. For example:
> > 
> > # readlink -f /sys/bus/cxl/devices/mem0
> > /sys/devices/pci0000:38/0000:38:00.0/mem0
> > # cxl list -BT
> > [
> >   {
> >     "bus":"root0",
> >     "provider":"ACPI.CXL",
> >     "nr_dports":1,
> >     "dports":[
> >       {
> >         "dport":"pci0000:38",
> >         "id":49
> >       }
> >     ]
> >   }
> > ]
> > 
> > ...so, in this case, the grandparent of "mem0" is "pci0000:38", and
> > "pci0000:38" is a dport. Unmodified cxl_mem_find_port() will do the
> > right thing and find that this CXL RCIEP is directly connected to
> > "root0".
> 
> find_cxl_port() uses the dport_dev, not the uport_dev. A lookup of
> pci0000:38 gives the cxl root (ACPI.CXL).

...but that is what I would expect. I.e. that RCDs appear directly
connected to the cxl root with no intervening cxl_port instance, and RCH
host-bridges only serve the role of dport devices. This also matches the
diagram from 9.11.8 CXL Devices Attached to an RCH where a
downstream-port RCRB in the host bridge is directly connected to the
RCIEP endpoint.
Robert Richter Nov. 17, 2022, 6:13 p.m. UTC | #4
On 15.11.22 10:06:28, Dan Williams wrote:
> Robert Richter wrote:
> > On 14.11.22 15:45:19, Dan Williams wrote:
> > > Robert Richter wrote:
> > > > The PCIe software view of an RCH and RCD is different to VH mode. An
> > > > RCD is paired with an RCH and shows up as RCiEP with a parent already
> > > > pointing to a PCI bridge (struct pci_host_bridge). In contrast, in VH
> > > > mode an PCI Express Endpoint is a PCI type 0 device with a PCI type 1
> > > > device as parent (struct pci_dev, most of the time a downstream switch
> > > > port, but could also be a root port). The following hierarchy applies
> > > > in VH mode:
> > > > 
> > > >  CXL memory device, cxl_memdev                               endpoint
> > > >  └──PCIe Endpoint (type 0), pci_dev                           |
> > > >     └──Downstream Port (type 1), pci_dev (Nth switch)        portN
> > > >        └──Upstream Port (type 1), pci_dev (Nth switch)        |
> > > >           :                                                   :
> > > >           └──Downstream Port (type 1), pci_dev (1st switch)  port1
> > > >              └──Upstream Port (type 1), pci_dev (1st switch)  |
> > > >                 └──Root Port (type 1), pci_dev                |
> > > >                    └──PCI host bridge, pci_host_bridge       port0
> > > >                       :                                       |
> > > >                       :..ACPI0017, acpi_dev                  root
> > > > 
> > > >  (There can be zero or any other number of switches in between.)
> > > > 
> > > > An iterator through the grandparents takes us to the root port which
> > > > is registered as dport to the bridge. The next port an endpoint is
> > > > connected to can be determined by using the grandparent of the memory
> > > > device as a dport_dev in cxl_mem_find_port().
> > > > 
> > > > The same does not work in RCD mode where only an RCiEP is connected to
> > > > the host bridge:
> > > > 
> > > >  CXL memory device, cxl_memdev                               endpoint
> > > >  └──PCIe Endpoint (type 0), pci_dev                           |
> > > >     └──PCI host bridge, pci_host_bridge                      port0
> > > >        :                                                      |
> > > >        :..ACPI0017, acpi_dev                                 root
> > > > 
> > > > Here, an endpoint is directly connected to the host bridge without a
> > > > type 1 PCI device (root or downstream port) in between. To link the
> > > > endpoint to the correct port, the endpoint's PCI device (parent of the
> > > > memory device) must be taken as dport_dev arg in cxl_mem_find_port().
> > > > 
> > > > Change cxl_mem_find_port() to find an RCH's port.
> > > > 
> > > > Signed-off-by: Robert Richter <rrichter@amd.com>
> > > > ---
> > > >  drivers/cxl/core/port.c | 38 ++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 38 insertions(+)
> > > > 
> > > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > > > index 0431ed860d8e..d10c3580719b 100644
> > > > --- a/drivers/cxl/core/port.c
> > > > +++ b/drivers/cxl/core/port.c
> > > > @@ -1354,6 +1354,14 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
> > > >  	return rc;
> > > >  }
> > > >  
> > > > +static inline bool is_cxl_restricted(struct cxl_memdev *cxlmd)
> > > > +{
> > > > +	struct device *parent = cxlmd->dev.parent;
> > > > +	if (!dev_is_pci(parent))
> > > > +		return false;
> > > > +	return pci_pcie_type(to_pci_dev(parent)) == PCI_EXP_TYPE_RC_END;
> > > > +}
> > > > +
> > > >  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> > > >  {
> > > >  	struct device *dev = &cxlmd->dev;
> > > > @@ -1433,9 +1441,39 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
> > > >  }
> > > >  EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_ports, CXL);
> > > >  
> > > > +/*
> > > > + * CXL memory device and port hierarchy:
> > > > + *
> > > > + * VH mode:
> > > > + *
> > > > + * CXL memory device, cxl_memdev                               endpoint
> > > > + * └──PCIe Endpoint (type 0), pci_dev                           |
> > > > + *    └──Downstream Port (type 1), pci_dev (Nth switch)        portN
> > > > + *       └──Upstream Port (type 1), pci_dev (Nth switch)        |
> > > > + *          :                                                   :
> > > > + *          └──Downstream Port (type 1), pci_dev (1st switch)  port1
> > > > + *             └──Upstream Port (type 1), pci_dev (1st switch)  |
> > > > + *                └──Root Port (type 1), pci_dev                |
> > > > + *                   └──PCI host bridge, pci_host_bridge       port0
> > > > + *                      :                                       |
> > > > + *                      :..ACPI0017, acpi_dev                  root
> > > > + *
> > > > + * (There can be zero or any other number of switches in between.)
> > > > + *
> > > > + * RCD mode:
> > > > + *
> > > > + * CXL memory device, cxl_memdev                               endpoint
> > > > + * └──PCIe Endpoint (type 0), pci_dev                           |
> > > > + *    └──PCI host bridge, pci_host_bridge                      port0
> > > > + *       :                                                      |
> > > > + *       :..ACPI0017, acpi_dev                                 root
> > > > + */
> > > >  struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
> > > >  				   struct cxl_dport **dport)
> > > >  {
> > > > +	if (is_cxl_restricted(cxlmd))
> > > > +		return find_cxl_port(cxlmd->dev.parent, dport);
> > > > +
> > > >  	return find_cxl_port(grandparent(&cxlmd->dev), dport);
> > > 
> > > I do not see why this change is needed. For example:
> > > 
> > > # readlink -f /sys/bus/cxl/devices/mem0
> > > /sys/devices/pci0000:38/0000:38:00.0/mem0
> > > # cxl list -BT
> > > [
> > >   {
> > >     "bus":"root0",
> > >     "provider":"ACPI.CXL",
> > >     "nr_dports":1,
> > >     "dports":[
> > >       {
> > >         "dport":"pci0000:38",
> > >         "id":49
> > >       }
> > >     ]
> > >   }
> > > ]
> > > 
> > > ...so, in this case, the grandparent of "mem0" is "pci0000:38", and
> > > "pci0000:38" is a dport. Unmodified cxl_mem_find_port() will do the
> > > right thing and find that this CXL RCIEP is directly connected to
> > > "root0".
> > 
> > find_cxl_port() uses the dport_dev, not the uport_dev. A lookup of
> > pci0000:38 gives the cxl root (ACPI.CXL).
> 
> ...but that is what I would expect. I.e. that RCDs appear directly
> connected to the cxl root with no intervening cxl_port instance, and RCH
> host-bridges only serve the role of dport devices. This also matches the
> diagram from 9.11.8 CXL Devices Attached to an RCH where a
> downstream-port RCRB in the host bridge is directly connected to the
> RCIEP endpoint.

All devices connected to ACPI.CXL? IMO this needs to be bound to the
host bridge. The hierarchy should show a host/device mapping. In fact,
in a VH the root port is also not part of the mapping, instead the
host bridge shows up there. Also, there can be multiple RCHs.

-Robert
diff mbox series

Patch

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 0431ed860d8e..d10c3580719b 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1354,6 +1354,14 @@  static int add_port_attach_ep(struct cxl_memdev *cxlmd,
 	return rc;
 }
 
+static inline bool is_cxl_restricted(struct cxl_memdev *cxlmd)
+{
+	struct device *parent = cxlmd->dev.parent;
+	if (!dev_is_pci(parent))
+		return false;
+	return pci_pcie_type(to_pci_dev(parent)) == PCI_EXP_TYPE_RC_END;
+}
+
 int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
 {
 	struct device *dev = &cxlmd->dev;
@@ -1433,9 +1441,39 @@  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_ports, CXL);
 
+/*
+ * CXL memory device and port hierarchy:
+ *
+ * VH mode:
+ *
+ * CXL memory device, cxl_memdev                               endpoint
+ * └──PCIe Endpoint (type 0), pci_dev                           |
+ *    └──Downstream Port (type 1), pci_dev (Nth switch)        portN
+ *       └──Upstream Port (type 1), pci_dev (Nth switch)        |
+ *          :                                                   :
+ *          └──Downstream Port (type 1), pci_dev (1st switch)  port1
+ *             └──Upstream Port (type 1), pci_dev (1st switch)  |
+ *                └──Root Port (type 1), pci_dev                |
+ *                   └──PCI host bridge, pci_host_bridge       port0
+ *                      :                                       |
+ *                      :..ACPI0017, acpi_dev                  root
+ *
+ * (There can be zero or any other number of switches in between.)
+ *
+ * RCD mode:
+ *
+ * CXL memory device, cxl_memdev                               endpoint
+ * └──PCIe Endpoint (type 0), pci_dev                           |
+ *    └──PCI host bridge, pci_host_bridge                      port0
+ *       :                                                      |
+ *       :..ACPI0017, acpi_dev                                 root
+ */
 struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
 				   struct cxl_dport **dport)
 {
+	if (is_cxl_restricted(cxlmd))
+		return find_cxl_port(cxlmd->dev.parent, dport);
+
 	return find_cxl_port(grandparent(&cxlmd->dev), dport);
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mem_find_port, CXL);