| 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
| ||
|
Date: Thu, 8 Jun 2023 21:24:44 -0700
From: Dan Williams <dan.j.williams@...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>,
<dave.jiang@...el.com>, <Jonathan.Cameron@...wei.com>,
<linux-cxl@...r.kernel.org>
CC: <terry.bowman@....com>, <rrichter@....com>,
<linux-kernel@...r.kernel.org>, <bhelgaas@...gle.com>
Subject: RE: [PATCH v5 08/26] cxl/acpi: Directly bind the CEDT detected CHBCR
to the Host Bridge's port
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
> 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.
>
> Signed-off-by: Robert Richter <rrichter@....com>
> Signed-off-by: Terry Bowman <terry.bowman@....com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> ---
> drivers/cxl/acpi.c | 65 +++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 50 insertions(+), 15 deletions(-)
I like where this patch is going. I needed to rebase and fix it up to
get cxl_test working again, and spotted some additional cleanups along
the way. I notice that after this patch dport->component_reg_phys is
totally unused. So I would like to see that removal patch comes next in
the series before going into the port->component_reg_phys reworks.
-- >8 --
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index eb64dc7a3d8d..7df6c6f5e902 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -341,7 +341,7 @@ static int cxl_get_chbs_iter(union acpi_subtable_headers *header, void *arg,
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_iter(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,17 +362,26 @@ static int cxl_get_chbs_iter(union acpi_subtable_headers *header, void *arg,
return 0;
}
-static int cxl_get_chbs(struct acpi_device *hb, struct cxl_chbs_context *ctx)
+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)
+ 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,
+ };
- memset(ctx, 0, sizeof(*ctx));
- ctx->uid = uid;
acpi_table_parse_cedt(ACPI_CEDT_TYPE_CHBS, cxl_get_chbs_iter, ctx);
return 0;
@@ -384,7 +391,6 @@ 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;
@@ -395,24 +401,19 @@ static int add_host_bridge_dport(struct device *match, void *arg)
if (!hb)
return 0;
- rc = cxl_get_chbs(hb, &ctx);
- if (rc == -ENOENT)
- dev_err(match, "unable to retrieve _UID\n");
+ rc = cxl_get_chbs(match, hb, &ctx);
if (rc)
return rc;
- uid = ctx.uid;
- dev_dbg(match, "UID found: %lld\n", uid);
-
- 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;
}
@@ -425,10 +426,12 @@ static int add_host_bridge_dport(struct device *match, void *arg)
* 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 {
- dport = devm_cxl_add_dport(root_port, bridge, uid,
+ dport = devm_cxl_add_dport(root_port, bridge, ctx.uid,
CXL_RESOURCE_NONE);
}
@@ -471,19 +474,17 @@ static int add_host_bridge_uport(struct device *match, void *arg)
return 0;
}
- rc = cxl_get_chbs(hb, &ctx);
+ rc = cxl_get_chbs(match, hb, &ctx);
if (rc)
return rc;
- if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11)
- /* RCH mode, should never happen */
+ if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
+ dev_warn(bridge,
+ "CXL CHBS version mismatch, skip port registration\n");
return 0;
+ }
- if (ctx.base)
- component_reg_phys = ctx.base;
- else
- component_reg_phys = CXL_RESOURCE_NONE;
-
+ 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);
Powered by blists - more mailing lists