From: Robert Richter <[email protected]>
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 <[email protected]>
Signed-off-by: Terry Bowman <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
---
drivers/cxl/acpi.c | 65 +++++++++++++++++++++++++++++++++++-----------
1 file changed, 50 insertions(+), 15 deletions(-)
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);
--
2.34.1
Terry Bowman wrote:
> From: Robert Richter <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Terry Bowman <[email protected]>
> Reviewed-by: Jonathan Cameron <[email protected]>
> ---
> 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);