Messages in this thread | | | Date | Tue, 15 Nov 2022 14:12:48 +0100 | From | Robert Richter <> | Subject | Re: [PATCH v3 3/9] cxl/mem: Adjust cxl_mem_find_port() to find an RCH's port |
| |
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
| |