On 01.12.22 13:34:16, Dan Williams wrote:
> Unlike a CXL memory expander in a VH topology that has at least one
> intervening 'struct cxl_port' instance between itself and the CXL root
> device, an RCD attaches one-level higher. For example:
>
> VH
> ┌──────────┐
> │ ACPI0017 │
> │ root0 │
> └─────┬────┘
> │
> ┌─────┴────┐
> │ dport0 │
> ┌─────┤ ACPI0016 ├─────┐
> │ │ port1 │ │
> │ └────┬─────┘ │
> │ │ │
> ┌──┴───┐ ┌──┴───┐ ┌───┴──┐
> │dport0│ │dport1│ │dport2│
> │ RP0 │ │ RP1 │ │ RP2 │
> └──────┘ └──┬───┘ └──────┘
> │
> ┌───┴─────┐
> │endpoint0│
> │ port2 │
> └─────────┘
>
> ...vs:
>
> RCH
> ┌──────────┐
> │ ACPI0017 │
> │ root0 │
> └────┬─────┘
> │
> ┌───┴────┐
> │ dport0 │
> │ACPI0016│
> └───┬────┘
> │
> ┌────┴─────┐
> │endpoint0 │
> │ port1 │
> └──────────┘
>
> So arrange for endpoint port in the RCH/RCD case to appear directly
> connected to the host-bridge in its singular role as a dport. Compare
> that to the VH case where the host-bridge serves a dual role as a
> 'cxl_dport' for the CXL root device *and* a 'cxl_port' upstream port for
> the Root Ports in the Root Complex that are modeled as 'cxl_dport'
> instances in the CXL topology.
>
> Another deviation from the VH case is that RCDs may need to look up
> their component registers from the Root Complex Register Block (RCRB).
> That platform firmware specified RCRB area is cached by the cxl_acpi
> driver and conveyed via the host-bridge dport to the cxl_mem driver to
> perform the cxl_rcrb_to_component() lookup for the endpoint port
> (See 9.11.8 CXL Devices Attached to an RCH for the lookup of the
> upstream port component registers).
>
> Tested-by: Robert Richter <rrich...@amd.com>
With the one comment below addressed you can also add my:
Reviewed-by: Robert Richter <rrich...@amd.com>
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
> drivers/cxl/core/port.c | 7 +++++++
> drivers/cxl/cxlmem.h | 2 ++
> drivers/cxl/mem.c | 31 ++++++++++++++++++++++++-------
> drivers/cxl/pci.c | 10 ++++++++++
> 4 files changed, 43 insertions(+), 7 deletions(-)
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> @@ -119,17 +131,22 @@ static int cxl_mem_probe(struct device *dev)
> return -ENXIO;
> }
>
> - device_lock(&parent_port->dev);
> - if (!parent_port->dev.driver) {
> + if (dport->rch)
> + endpoint_parent = parent_port->uport;
> + else
> + endpoint_parent = &parent_port->dev;
> +
> + device_lock(endpoint_parent);
> + if (!endpoint_parent->driver) {
> dev_err(dev, "CXL port topology %s not enabled\n",
> dev_name(&parent_port->dev));
Already reported: dev_name(endpoint_parent)
> rc = -ENXIO;
> goto unlock;
> }
>
> - rc = devm_cxl_add_endpoint(cxlmd, dport);
> + rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport);
> unlock:
> - device_unlock(&parent_port->dev);
> + device_unlock(endpoint_parent);
> put_device(&parent_port->dev);
> if (rc)
> return rc;
