Messages in this thread | | | Date | Mon, 14 Nov 2022 15:45:19 -0800 | From | Dan Williams <> | Subject | RE: [PATCH v3 3/9] cxl/mem: Adjust cxl_mem_find_port() to find an RCH's port |
| |
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".
| |