Messages in this thread | | | Date | Tue, 15 Nov 2022 10:06:28 -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: > 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.
| |