Re: [PATCH 00/13] Enumerate midlevel and endpoint decoders

[Dan Williams <dan.j.williams@intel.com>]

On Thu, Sep 2, 2021 at 12:50 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> Every CXL component may implement component registers as defined by the CXL 2.0
> specification. In preparation for creating and enumerating regions it's
> important to at least enumerate all HDM decoders which are a subset of the
> component registers. To do this, a new cxl_mem driver is introduced which is
> responsible for binding to a CXL.mem enabled device. In order to determine
> whether or not an endpoint is CXL enabled, the relevant subhierarchy must be
> enumerated.
>
> This serves as the stepping stone toward enabling regions because regions must
> be able to determine if the devices selected for the region are CXL.mem capable
> and enabled.
>
> There's two issues that need to be resolved but I'm going to propose we fix
> them next time we need to touch this code...
> 1. cxl_pci now relinquishes its component register mappings. This may be
>    undesirable as cxl_pci may need to use those mappings.
> 2a. some amount of component register enumeration is duplicated in cxl_pci and
>     cxl_mem
> 2b. awkwardness in cxl_mem where memdevs get their component registers from
>    cxl_pci, and ports that enumerate their own component registers

I don't immediately see this as a technical debt problem. The CXL
component registers belong to the corresponding CXL functionality
driver. One agent should be in charge of all of them and any other
agent that has a request must work through the owner. The sub-device
design pattern like registering cxl_memdev on the cxl-bus, or the
generic mechanism to register auxliary-devices on the auxiliary-bus is
specfically targeting this case of parent-driver registering resources
for a child-driver to operate.

>
> The obvious fix for both of these is to move component register mapping to
> cxl_core, and let cxl_core arbitrate the mappings for the "client" drivers.
> Since the code needed to enable cxl_mem was small and subset of the existing
> code (and fairly error resistent vs creating a cxl_core API) I'm hoping to kick
> the can down the road.

The core contains common enumeration and mapping code, but ownership
belongs to one and only one agent, no runtime arbitration. ...or so
I'd like to assert unless there is a exceedingly good reason to add
arbitration complexity.

>
> NOTE: I do not have a way at present to test switches. For this reason and for
> patch readability, the switch enumeration is left as a separate patch.
>
> NOTE2: Some of these patches were introduced in an RFC for region creation. Upon
> further inspection, it made a lot of sense to land these before region creation
> so long as it's understood upfront why the new driver is needed.
>
> I've pushed this to my gitlab here:
> https://gitlab.com/bwidawsk/linux/-/tree/decoders
>
> Ben Widawsky (13):
>   Documentation/cxl: Add bus internal docs
>   cxl/core/bus: Add kernel docs for decoder ops
>   cxl/core: Ignore interleave when adding decoders
>   cxl: Introduce endpoint decoders
>   cxl/pci: Disambiguate cxl_pci further from cxl_mem
>   cxl/mem: Introduce cxl_mem driver
>   cxl/memdev: Determine CXL.mem capability
>   cxl/mem: Add memdev as a port
>   cxl/pci: Retain map information in cxl_mem_probe
>   cxl/core: Map component registers for ports
>   cxl/core: Convert decoder range to resource
>   cxl/core/bus: Enumerate all HDM decoders
>   cxl/mem: Enumerate switch decoders
>
>  .../driver-api/cxl/memory-devices.rst         |   6 +
>  drivers/cxl/Makefile                          |   3 +-
>  drivers/cxl/acpi.c                            |  39 +--
>  drivers/cxl/core/bus.c                        | 326 +++++++++++++++++-
>  drivers/cxl/core/core.h                       |   1 +
>  drivers/cxl/core/memdev.c                     |  19 +-
>  drivers/cxl/core/regs.c                       |   6 +-
>  drivers/cxl/cxl.h                             |  65 +++-
>  drivers/cxl/cxlmem.h                          |   6 +-
>  drivers/cxl/mem.c                             | 237 +++++++++++++
>  drivers/cxl/pci.c                             | 124 +++----
>  drivers/cxl/pci.h                             |  15 +-
>  12 files changed, 727 insertions(+), 120 deletions(-)
>  create mode 100644 drivers/cxl/mem.c
>
> --
> 2.33.0
>