[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6372d30ef4152_12cdff29452@dwillia2-xfh.jf.intel.com.notmuch>
Date: Mon, 14 Nov 2022 15:45:19 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Robert Richter <rrichter@....com>,
Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>,
"Ira Weiny" <ira.weiny@...el.com>,
Ben Widawsky <bwidawsk@...nel.org>,
"Dan Williams" <dan.j.williams@...el.com>
CC: <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
"Davidlohr Bueso" <dave@...olabs.net>,
Dave Jiang <dave.jiang@...el.com>,
Robert Richter <rrichter@....com>
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@....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".
Powered by blists - more mailing lists