| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0a5ee5c3-5850-92b0-dbe1-dded3ec5d0f8@intel.com>
Date: Thu, 22 Jun 2023 16:28:49 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: Terry Bowman <terry.bowman@....com>, <alison.schofield@...el.com>,
<vishal.l.verma@...el.com>, <ira.weiny@...el.com>,
<bwidawsk@...nel.org>, <dan.j.williams@...el.com>,
<Jonathan.Cameron@...wei.com>, <linux-cxl@...r.kernel.org>
CC: <rrichter@....com>, <linux-kernel@...r.kernel.org>,
<bhelgaas@...gle.com>
Subject: Re: [PATCH v6 10/27] cxl/acpi: Directly bind the CEDT detected CHBCR
to the Host Bridge's port
On 6/21/23 20:51, Terry Bowman wrote:
> From: Robert Richter <rrichter@....com>
>
> During a Host Bridge's downstream port enumeration the CHBS entries in
> the CEDT table are parsed, its Component Register base address
> extracted and then stored in struct cxl_dport. The CHBS may contain
> either the RCRB (RCH mode) or the Host Bridge's Component Registers
> (CHBCR, VH mode). The RCRB further contains the CXL downstream port
> register base address, while in VH mode the CXL Downstream Switch
> Ports are visible in the PCI hierarchy and the DP's component regs are
> disovered using the CXL DVSEC register locator capability. The
s/disovered/discovered/
> Component Registers derived from the CHBS for both modes are different
> and thus also must be treated differently. That is, in RCH mode, the
> component regs base should be bound to the dport, but in VH mode to
> the CXL host bridge's port object.
>
> The current implementation stores the CHBCR in addition in struct
> cxl_dport and copies it later from there to struct cxl_port. As a
> result, the dport contains the wrong Component Registers base address
> and, e.g. the RAS capability of a CXL Root Port cannot be detected.
>
> To fix the CHBCR binding, attach it directly to the Host Bridge's
> @cxl_port structure. Do this during port creation of the Host Bridge
> in add_host_bridge_uport(). Factor out CHBS parsing code in
> add_host_bridge_dport() and use it in both functions.
>
> Co-developed-by: Dan Williams <dan.j.williams@...el.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> Signed-off-by: Robert Richter <rrichter@....com>
> Signed-off-by: Terry Bowman <terry.bowman@....com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Reviewed-by: Dave Jiang <dave.jiang@...el.com>
> ---
> drivers/cxl/acpi.c | 91 ++++++++++++++++++++++++++++++++--------------
> 1 file changed, 63 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 0c975ee684b0..89ee01323d43 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -335,13 +335,13 @@ struct cxl_chbs_context {
> u32 cxl_version;
> };
>
> -static int cxl_get_chbs(union acpi_subtable_headers *header, void *arg,
> - const unsigned long end)
> +static int cxl_get_chbs_iter(union acpi_subtable_headers *header, void *arg,
> + const unsigned long end)
> {
> struct cxl_chbs_context *ctx = arg;
> struct acpi_cedt_chbs *chbs;
>
> - if (ctx->base)
> + if (ctx->base != CXL_RESOURCE_NONE)
> return 0;
>
> chbs = (struct acpi_cedt_chbs *) header;
> @@ -350,8 +350,6 @@ static int cxl_get_chbs(union acpi_subtable_headers *header, void *arg,
> return 0;
>
> ctx->cxl_version = chbs->cxl_version;
> - ctx->base = CXL_RESOURCE_NONE;
> -
> if (!chbs->base)
> return 0;
>
> @@ -364,11 +362,35 @@ static int cxl_get_chbs(union acpi_subtable_headers *header, void *arg,
> return 0;
> }
>
> +static int cxl_get_chbs(struct device *dev, struct acpi_device *hb,
> + struct cxl_chbs_context *ctx)
> +{
> + unsigned long long uid;
> + int rc;
> +
> + rc = acpi_evaluate_integer(hb->handle, METHOD_NAME__UID, NULL, &uid);
> + if (rc != AE_OK) {
> + dev_err(dev, "unable to retrieve _UID\n");
> + return -ENOENT;
> + }
> +
> + dev_dbg(dev, "UID found: %lld\n", uid);
> + *ctx = (struct cxl_chbs_context) {
> + .dev = dev,
> + .uid = uid,
> + .base = CXL_RESOURCE_NONE,
> + .cxl_version = UINT_MAX,
> + };
> +
> + acpi_table_parse_cedt(ACPI_CEDT_TYPE_CHBS, cxl_get_chbs_iter, ctx);
> +
> + return 0;
> +}
> +
> static int add_host_bridge_dport(struct device *match, void *arg)
> {
> acpi_status rc;
> struct device *bridge;
> - unsigned long long uid;
> struct cxl_dport *dport;
> struct cxl_chbs_context ctx;
> struct acpi_pci_root *pci_root;
> @@ -379,41 +401,38 @@ static int add_host_bridge_dport(struct device *match, void *arg)
> if (!hb)
> return 0;
>
> - rc = acpi_evaluate_integer(hb->handle, METHOD_NAME__UID, NULL, &uid);
> - if (rc != AE_OK) {
> - dev_err(match, "unable to retrieve _UID\n");
> - return -ENODEV;
> - }
> -
> - dev_dbg(match, "UID found: %lld\n", uid);
> -
> - ctx = (struct cxl_chbs_context) {
> - .dev = match,
> - .uid = uid,
> - };
> - acpi_table_parse_cedt(ACPI_CEDT_TYPE_CHBS, cxl_get_chbs, &ctx);
> + rc = cxl_get_chbs(match, hb, &ctx);
> + if (rc)
> + return rc;
>
> - if (!ctx.base) {
> + if (ctx.cxl_version == UINT_MAX) {
> dev_warn(match, "No CHBS found for Host Bridge (UID %lld)\n",
> - uid);
> + ctx.uid);
> return 0;
> }
>
> if (ctx.base == CXL_RESOURCE_NONE) {
> dev_warn(match, "CHBS invalid for Host Bridge (UID %lld)\n",
> - uid);
> + ctx.uid);
> return 0;
> }
>
> pci_root = acpi_pci_find_root(hb->handle);
> bridge = pci_root->bus->bridge;
>
> + /*
> + * In RCH mode, bind the component regs base to the dport. In
> + * VH mode it will be bound to the CXL host bridge's port
> + * object later in add_host_bridge_uport().
> + */
> if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
> - dev_dbg(match, "RCRB found for UID %lld: %pa\n", uid, &ctx.base);
> - dport = devm_cxl_add_rch_dport(root_port, bridge, uid, ctx.base);
> + dev_dbg(match, "RCRB found for UID %lld: %pa\n", ctx.uid,
> + &ctx.base);
> + dport = devm_cxl_add_rch_dport(root_port, bridge, ctx.uid,
> + ctx.base);
> } else {
> - dev_dbg(match, "CHBCR found for UID %lld: %pa\n", uid, &ctx.base);
> - dport = devm_cxl_add_dport(root_port, bridge, uid, ctx.base);
> + dport = devm_cxl_add_dport(root_port, bridge, ctx.uid,
> + CXL_RESOURCE_NONE);
> }
>
> if (IS_ERR(dport))
> @@ -435,6 +454,8 @@ static int add_host_bridge_uport(struct device *match, void *arg)
> struct cxl_dport *dport;
> struct cxl_port *port;
> struct device *bridge;
> + struct cxl_chbs_context ctx;
> + resource_size_t component_reg_phys;
> int rc;
>
> if (!hb)
> @@ -453,12 +474,26 @@ static int add_host_bridge_uport(struct device *match, void *arg)
> return 0;
> }
>
> + rc = cxl_get_chbs(match, hb, &ctx);
> + if (rc)
> + return rc;
> +
> + if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
> + dev_warn(bridge,
> + "CXL CHBS version mismatch, skip port registration\n");
> + return 0;
> + }
> +
> + component_reg_phys = ctx.base;
> + if (component_reg_phys != CXL_RESOURCE_NONE)
> + dev_dbg(match, "CHBCR found for UID %lld: %pa\n",
> + ctx.uid, &component_reg_phys);
> +
> rc = devm_cxl_register_pci_bus(host, bridge, pci_root->bus);
> if (rc)
> return rc;
>
> - port = devm_cxl_add_port(host, bridge, dport->component_reg_phys,
> - dport);
> + port = devm_cxl_add_port(host, bridge, component_reg_phys, dport);
> if (IS_ERR(port))
> return PTR_ERR(port);
>
Powered by blists - more mailing lists