| Message ID | 162295949351.1109360.10329014558746500142.stgit@dwillia2-desk3.amr.corp.intel.com |
|---|---|
| Headers | show |
| Series | CXL port and decoder enumeration | expand |
On Sun, Jun 6, 2021 at 8:05 AM Dan Williams <dan.j.williams@intel.com> wrote: > > The recently released CXL specification change (ECN) for the CXL Fixed > Memory Window Structure (CFMWS) extension to the CXL Early Discovery > Table (CEDT) enables a large amount of functionality. It defines the > root of a CXL memory topology and is needed for all OS flows for CXL > provisioning CXL memory expanders. For ease of merging and tree > management add the new ACPI definition locally (drivers/cxl/acpi.h) in > such a way that they will not collide with the eventual arrival of the > definitions through the ACPICA project to their final location > (drivers/acpi/actbl1.h). I've just applied the ACPICA series including this change which can be made available as a forward-only branch in my tree, if that helps. > The definitions in drivers/cxl/acpi.h can be dropped post -rc1. > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > Co-developed-by: Alison Schofield <alison.schofield@intel.com> > Co-developed-by: Erik Kaneda <erik.kaneda@intel.com> > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > Signed-off-by: Erik Kaneda <erik.kaneda@intel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/cxl/acpi.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 drivers/cxl/acpi.h > > diff --git a/drivers/cxl/acpi.h b/drivers/cxl/acpi.h > new file mode 100644 > index 000000000000..1482c19e7227 > --- /dev/null > +++ b/drivers/cxl/acpi.h > @@ -0,0 +1,48 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* Copyright(c) 2021 Intel Corporation. */ > +#ifndef __CXL_ACPI_H__ > +#define __CXL_ACPI_H__ > + > +#ifndef ACPI_CEDT_CHBS_VERSION_CXL20 > +/* > + * NOTE: These definitions are temporary and to be deleted in v5.14-rc1 > + * when the identical definitions become available from > + * include/acpi/actbl1.h. > + */ > + > +#define ACPI_CEDT_TYPE_CFMWS 1 > +#define ACPI_CEDT_TYPE_RESERVED 2 > + > +#define ACPI_CEDT_CHBS_VERSION_CXL11 (0) > +#define ACPI_CEDT_CHBS_VERSION_CXL20 (1) > + > +#define ACPI_CEDT_CHBS_LENGTH_CXL11 (0x2000) > +#define ACPI_CEDT_CHBS_LENGTH_CXL20 (0x10000) > + > +struct acpi_cedt_cfmws { > + struct acpi_cedt_header header; > + u32 reserved1; > + u64 base_hpa; > + u64 window_size; > + u8 interleave_ways; > + u8 interleave_arithmetic; > + u16 reserved2; > + u32 granularity; > + u16 restrictions; > + u16 qtg_id; > + u32 interleave_targets[]; > +}; > + > +/* Values for Interleave Arithmetic field above */ > + > +#define ACPI_CEDT_CFMWS_ARITHMETIC_MODULO (0) > + > +/* Values for Restrictions field above */ > + > +#define ACPI_CEDT_CFMWS_RESTRICT_TYPE2 (1) > +#define ACPI_CEDT_CFMWS_RESTRICT_TYPE3 (1 << 1) > +#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE (1 << 2) > +#define ACPI_CEDT_CFMWS_RESTRICT_PMEM (1 << 3) > +#define ACPI_CEDT_CFMWS_RESTRICT_FIXED (1 << 4) > +#endif /* ACPI_CEDT_CHBS_VERSION_CXL20 */ > +#endif /* __CXL_ACPI_H__ */ >
On Mon, Jun 7, 2021 at 5:26 AM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Sun, Jun 6, 2021 at 8:05 AM Dan Williams <dan.j.williams@intel.com> wrote: > > > > The recently released CXL specification change (ECN) for the CXL Fixed > > Memory Window Structure (CFMWS) extension to the CXL Early Discovery > > Table (CEDT) enables a large amount of functionality. It defines the > > root of a CXL memory topology and is needed for all OS flows for CXL > > provisioning CXL memory expanders. For ease of merging and tree > > management add the new ACPI definition locally (drivers/cxl/acpi.h) in > > such a way that they will not collide with the eventual arrival of the > > definitions through the ACPICA project to their final location > > (drivers/acpi/actbl1.h). > > I've just applied the ACPICA series including this change which can be > made available as a forward-only branch in my tree, if that helps. Yes, please, that would be my preference. When I created this patch the concern was that a stable branch was possibly weeks away.
On Sat, 5 Jun 2021 23:05:22 -0700 Dan Williams <dan.j.williams@intel.com> wrote: > While the resources enumerated by the CEDT.CFMWS identify a cxl_port > with host bridges as downstream ports, host bridges themselves are > upstream ports that decode to downstream ports represented by PCIe Root > Ports. Walk the PCIe Root Ports connected to a CXL Host Bridge, > identified by the ACPI0016 _HID, and add each one as a cxl_dport of the > host bridge cxl_port. > > For now, component registers are not enumerated, only the first order > uport / dport relationships. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> LGTM Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cxl/acpi.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 95 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index 0ae7464b603d..ec324bf063b8 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > @@ -8,6 +8,48 @@ > #include <linux/pci.h> > #include "cxl.h" > > +struct cxl_walk_context { > + struct device *dev; > + struct pci_bus *root; > + struct cxl_port *port; > + int error; > + int count; > +}; > + > +static int match_add_root_ports(struct pci_dev *pdev, void *data) > +{ > + struct cxl_walk_context *ctx = data; > + struct pci_bus *root_bus = ctx->root; > + struct cxl_port *port = ctx->port; > + int type = pci_pcie_type(pdev); > + struct device *dev = ctx->dev; > + u32 lnkcap, port_num; > + int rc; > + > + if (pdev->bus != root_bus) > + return 0; > + if (!pci_is_pcie(pdev)) > + return 0; > + if (type != PCI_EXP_TYPE_ROOT_PORT) > + return 0; > + if (pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCAP, > + &lnkcap) != PCIBIOS_SUCCESSFUL) > + return 0; > + > + /* TODO walk DVSEC to find component register base */ > + port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap); > + rc = cxl_add_dport(port, &pdev->dev, port_num, CXL_RESOURCE_NONE); > + if (rc) { > + ctx->error = rc; > + return rc; > + } > + ctx->count++; > + > + dev_dbg(dev, "add dport%d: %s\n", port_num, dev_name(&pdev->dev)); > + > + return 0; > +} > + > static struct acpi_device *to_cxl_host_bridge(struct device *dev) > { > struct acpi_device *adev = to_acpi_device(dev); > @@ -17,6 +59,44 @@ static struct acpi_device *to_cxl_host_bridge(struct device *dev) > return NULL; > } > > +/* > + * A host bridge is a dport to a CFMWS decode and it is a uport to the > + * dport (PCIe Root Ports) in the host bridge. > + */ > +static int add_host_bridge_uport(struct device *match, void *arg) > +{ > + struct acpi_device *bridge = to_cxl_host_bridge(match); > + struct cxl_port *root_port = arg; > + struct device *host = root_port->dev.parent; > + struct acpi_pci_root *pci_root; > + struct cxl_walk_context ctx; > + struct cxl_port *port; > + > + if (!bridge) > + return 0; > + > + pci_root = acpi_pci_find_root(bridge->handle); > + if (!pci_root) > + return -ENXIO; > + > + /* TODO: fold in CEDT.CHBS retrieval */ > + port = devm_cxl_add_port(host, match, CXL_RESOURCE_NONE, root_port); > + if (IS_ERR(port)) > + return PTR_ERR(port); > + dev_dbg(host, "%s: add: %s\n", dev_name(match), dev_name(&port->dev)); > + > + ctx = (struct cxl_walk_context){ > + .dev = host, > + .root = pci_root->bus, > + .port = port, > + }; > + pci_walk_bus(pci_root->bus, match_add_root_ports, &ctx); > + > + if (ctx.count == 0) > + return -ENODEV; > + return ctx.error; > +} > + > static int add_host_bridge_dport(struct device *match, void *arg) > { > int rc; > @@ -49,6 +129,7 @@ static int add_host_bridge_dport(struct device *match, void *arg) > > static int cxl_acpi_probe(struct platform_device *pdev) > { > + int rc; > struct cxl_port *root_port; > struct device *host = &pdev->dev; > struct acpi_device *adev = ACPI_COMPANION(host); > @@ -58,8 +139,20 @@ static int cxl_acpi_probe(struct platform_device *pdev) > return PTR_ERR(root_port); > dev_dbg(host, "add: %s\n", dev_name(&root_port->dev)); > > - return bus_for_each_dev(adev->dev.bus, NULL, root_port, > - add_host_bridge_dport); > + rc = bus_for_each_dev(adev->dev.bus, NULL, root_port, > + add_host_bridge_dport); > + if (rc) > + return rc; > + > + /* > + * Root level scanned with host-bridge as dports, now scan host-bridges > + * for their role as CXL uports to their CXL-capable PCIe Root Ports. > + */ > + rc = bus_for_each_dev(adev->dev.bus, NULL, root_port, > + add_host_bridge_uport); > + if (rc) > + dev_err(host, "failed to scan host bridges\n"); > + return 0; > } > > static const struct acpi_device_id cxl_acpi_ids[] = { >
On Mon, Jun 7, 2021 at 10:03 AM Dan Williams <dan.j.williams@intel.com> wrote: > > On Mon, Jun 7, 2021 at 5:26 AM Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > On Sun, Jun 6, 2021 at 8:05 AM Dan Williams <dan.j.williams@intel.com> wrote: > > > > > > The recently released CXL specification change (ECN) for the CXL Fixed > > > Memory Window Structure (CFMWS) extension to the CXL Early Discovery > > > Table (CEDT) enables a large amount of functionality. It defines the > > > root of a CXL memory topology and is needed for all OS flows for CXL > > > provisioning CXL memory expanders. For ease of merging and tree > > > management add the new ACPI definition locally (drivers/cxl/acpi.h) in > > > such a way that they will not collide with the eventual arrival of the > > > definitions through the ACPICA project to their final location > > > (drivers/acpi/actbl1.h). > > > > I've just applied the ACPICA series including this change which can be > > made available as a forward-only branch in my tree, if that helps. > > Yes, please, that would be my preference. When I created this patch > the concern was that a stable branch was possibly weeks away. Rafael, I see "4a2c1dcfaf59 ACPICA: Add the CFMWS structure definition to the CEDT table" in your tree, I can safely assume that commit will not rebase at this point? I'll likely rewind your acpica branch to that point and merge there to avoid carrying any unrelated follow-on commits.
On Tue, Jun 8, 2021 at 8:13 PM Dan Williams <dan.j.williams@intel.com> wrote: > > On Mon, Jun 7, 2021 at 10:03 AM Dan Williams <dan.j.williams@intel.com> wrote: > > > > On Mon, Jun 7, 2021 at 5:26 AM Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > > > On Sun, Jun 6, 2021 at 8:05 AM Dan Williams <dan.j.williams@intel.com> wrote: > > > > > > > > The recently released CXL specification change (ECN) for the CXL Fixed > > > > Memory Window Structure (CFMWS) extension to the CXL Early Discovery > > > > Table (CEDT) enables a large amount of functionality. It defines the > > > > root of a CXL memory topology and is needed for all OS flows for CXL > > > > provisioning CXL memory expanders. For ease of merging and tree > > > > management add the new ACPI definition locally (drivers/cxl/acpi.h) in > > > > such a way that they will not collide with the eventual arrival of the > > > > definitions through the ACPICA project to their final location > > > > (drivers/acpi/actbl1.h). > > > > > > I've just applied the ACPICA series including this change which can be > > > made available as a forward-only branch in my tree, if that helps. > > > > Yes, please, that would be my preference. When I created this patch > > the concern was that a stable branch was possibly weeks away. > > Rafael, I see "4a2c1dcfaf59 ACPICA: Add the CFMWS structure definition > to the CEDT table" in your tree, I can safely assume that commit will > not rebase at this point? Yes, please. > I'll likely rewind your acpica branch to > that point and merge there to avoid carrying any unrelated follow-on > commits. Sure.
Changes since v4 [1]: - Rework the object model to better account for downstream ports and enumerate decode capabilities. - Include the definition of CFMWS - Reference CFMWS in the changelogs and implementation - Drop mention of pcie_portdriver for CXL switch enumeration. This will be handled by CXL path validation when an endpoint wants to contribute to a memory region. [1]: http://lore.kernel.org/r/162096970332.1865304.10280028741091576940.stgit@dwillia2-desk3.amr.corp.intel.com --- The recently published CXL Fixed Memory Window Structure (CFMWS) extension to the CXL Early Discovery Table (CEDT) provides a platform firmware mechanism to enumerate CXL memory resources. The table data indicates which CXL memory ranges were configured by platform BIOS, and which address ranges are available to support hot plug and dynamic provisioning of CXL memory regions. CXL Port Topology: The enumeration starts with the ACPI0017 driver registering a 'struct cxl_port' object to establish the top of a port topology. It then scans the ACPI bus looking for CXL Host Bridges (ACPI0016 instances). A cxl_port represents one or more decoder resources between a 'uport' (upstream port) and one or more 'dport' (downstream port) instances. System software must not assume that 'struct cxl_port' device names will be static from one boot to the next. It will generally be the case that the root cxl_port starts at id '0' and the host bridges are enumerated in the same order starting at id '1', but that is not guaranteed. A 'uport' is a device that implements a decode. It can either be a platform firmware device like ACPI0017 where the decode is described by an ACPI data-structure, or a PCIe switch where the upstream port of the switch implements a CXL DVSEC pointing to component registers with the HDM decoder capability (see CXL 2.0 section 8.2.5.12 CXL HDM Decoder Capability Structure). Once a uport and its corresponding dport instances are collected into a cxl_port the actual decode resources are then modeled as cxl_decode objects that are children of their parent cxl_port. The 'decode' object has a 1:1 relationship with ether CFMWS entries at the root level, or hardware HDM decoder register instances in a PCIe device's CXL component register space at any level of a CXL switch hierarchy. In addition to the interleave geometry and address range a decode object conveys the target list (targeted dports) in interleave order. The dport id in a target list is either its ACPI _UID for Host Bridge targets, or the "port number" field from the link capabilities register in the PCIe "Express" capability [2]. Here is a tree(1) topology of QEMU emulating a single-ported host-bridge: /sys/bus/cxl/devices/root0 ├── devtype ├── dport0 -> ../LNXSYSTM:00/LNXSYBUS:00/ACPI0016:00 ├── port1 │ ├── decoder1.0 │ │ ├── devtype │ │ ├── end │ │ ├── locked │ │ ├── start │ │ ├── subsystem -> ../../../../bus/cxl │ │ ├── target_list │ │ ├── target_type │ │ └── uevent │ ├── devtype │ ├── dport0 -> ../../pci0000:34/0000:34:00.0 │ ├── subsystem -> ../../../bus/cxl │ ├── uevent │ └── uport -> ../../LNXSYSTM:00/LNXSYBUS:00/ACPI0016:00 ├── subsystem -> ../../bus/cxl ├── uevent └── uport -> ../platform/ACPI0017:00 * The root port is singleton only by convention. A given uport device like ACPI0017 could create a root level port per CFMWS entry. This patch set chooses to implement 1 port at the root level and list all CFMWS decode entries under that port regardless of which dport host bridges are targeted. [2]: CXL 2.0 8.2.5.12.8 CXL HDM Decoder 0 Target List Low Register (Offset 24h) ...The Target Port Identifier for a given Downstream Port is reported via Port Number field in Link Capabilities Register. (See PCI Express Base Specification). --- Dan Williams (6): cxl/acpi: Local definition of ACPICA infrastructure cxl/acpi: Introduce cxl_root, the root of a cxl_port topology cxl/Kconfig: Default drivers to CONFIG_CXL_BUS cxl/acpi: Add downstream port data to cxl_port instances cxl/acpi: Enumerate host bridge root ports cxl/acpi: Introduce cxl_decoder objects Documentation/ABI/testing/sysfs-bus-cxl | 94 ++++++ drivers/cxl/Kconfig | 17 + drivers/cxl/Makefile | 2 drivers/cxl/acpi.c | 193 ++++++++++++ drivers/cxl/acpi.h | 48 +++ drivers/cxl/core.c | 514 +++++++++++++++++++++++++++++++ drivers/cxl/cxl.h | 92 ++++++ 7 files changed, 960 insertions(+) create mode 100644 drivers/cxl/acpi.c create mode 100644 drivers/cxl/acpi.h base-commit: 605a5e41db7d8c930fb80115686991c4c1d08ee4