| Message ID | 20230607221651.2454764-9-terry.bowman@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl/pci: Add support for RCH RAS error handling | expand |
Terry Bowman wrote: > From: Robert Richter <rrichter@amd.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@amd.com> > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.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);
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c index 4fd9fe32f830..78a24b2ca923 100644 --- a/drivers/cxl/acpi.c +++ b/drivers/cxl/acpi.c @@ -333,8 +333,8 @@ 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; @@ -362,6 +362,22 @@ static int cxl_get_chbs(union acpi_subtable_headers *header, void *arg, return 0; } +static int cxl_get_chbs(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) + return -ENOENT; + + memset(ctx, 0, sizeof(*ctx)); + ctx->uid = uid; + 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; @@ -377,19 +393,15 @@ 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) { + rc = cxl_get_chbs(hb, &ctx); + if (rc == -ENOENT) dev_err(match, "unable to retrieve _UID\n"); - return -ENODEV; - } + if (rc) + return rc; + uid = ctx.uid; dev_dbg(match, "UID found: %lld\n", uid); - ctx = (struct cxl_chbs_context) { - .uid = uid, - }; - acpi_table_parse_cedt(ACPI_CEDT_TYPE_CHBS, cxl_get_chbs, &ctx); - if (!ctx.base) { dev_warn(match, "No CHBS found for Host Bridge (UID %lld)\n", uid); @@ -405,12 +417,17 @@ static int add_host_bridge_dport(struct device *match, void *arg) 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); } 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, uid, + CXL_RESOURCE_NONE); } if (IS_ERR(dport)) @@ -432,6 +449,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) @@ -450,12 +469,28 @@ static int add_host_bridge_uport(struct device *match, void *arg) return 0; } + rc = cxl_get_chbs(hb, &ctx); + if (rc) + return rc; + + if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) + /* RCH mode, should never happen */ + return 0; + + if (ctx.base) + component_reg_phys = ctx.base; + else + component_reg_phys = CXL_RESOURCE_NONE; + + 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);