Re: [RFC PATCH 00/15] Region driver

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

On Fri, May 20, 2022 at 9:23 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Wed, 13 Apr 2022 11:37:05 -0700
> Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> > Spring cleaning is here and we're starting fresh so I won't be referencing
> > previous postings and I've removed revision history from commit messages.
> >
> > This patch series introduces the CXL region driver as well as associated APIs in
> > CXL core to create and configure regions. Regions are defined by the CXL 2.0
> > specification [1], a summary follows.
> >
> > A region surfaces a swath of RAM (persistent or volatile) that appears as normal
> > memory to the operating system. The memory, unless programmed by BIOS, or a
> > previous Operating System, is inaccessible until the CXL driver creates a region
> > for it.A region may be strided (interleave granularity) across multiple devices
> > (interleave ways). The interleaving may traverse multiple levels of the CXL
> > hierarchy.
> >
> > +-------------------------+      +-------------------------+
> > |                         |      |                         |
> > |   CXL 2.0 Host Bridge   |      |   CXL 2.0 Host Bridge   |
> > |                         |      |                         |
> > |  +------+     +------+  |      |  +------+     +------+  |
> > |  |  RP  |     |  RP  |  |      |  |  RP  |     |  RP  |  |
> > +--+------+-----+------+--+      +--+------+-----+------+--+
> >       |            |                   |               \--
> >       |            |                   |        +-------+-\--+------+
> >    +------+    +-------+            +-------+   |       |USP |      |
> >    |Type 3|    |Type 3 |            |Type 3 |   |       +----+      |
> >    |Device|    |Device |            |Device |   |     CXL Switch    |
> >    +------+    +-------+            +-------+   | +----+     +----+ |
> >                                                 | |DSP |     |DSP | |
> >                                                 +-+-|--+-----+-|--+-+
> >                                                     |          |
> >                                                 +------+    +-------+
> >                                                 |Type 3|    |Type 3 |
> >                                                 |Device|    |Device |
> >                                                 +------+    +-------+
> >
> > Region verification and programming state are owned by the cxl_region driver
> > (implemented in the cxl_region module). Much of the region driver is an
> > implementation of algorithms described in the CXL Type 3 Memory Device Software
> > Guide [2].
> >
> > The region driver is responsible for configuring regions found on persistent
> > capacities in the Label Storage Area (LSA), it will also enumerate regions
> > configured by BIOS, usually volatile capacities, and will allow for dynamic
> > region creation (which can then be stored in the LSA). Only dynamically created
> > regions are implemented thus far.
> >
> > Dan has previously stated that he doesn't want to merge ABI until the whole
> > series is posted and reviewed, to make sure we have no gaps. As such, the goal
> > of posting this series is *not* to discuss the ABI specifically, feedback is of
> > course welcome. In other wordsIt has been discussed previously. The goal is to find
> > architectural flaws in the implementation of the ABI that may pose problematic
> > for cases we haven't yet conceived.
> >
> > Since region creation is done via sysfs, it is left to userspace to prevent
> > racing for resource usage. Here is an overview for creating a x1 256M
> > dynamically created region programming to be used by userspace clients. In this
> > example, the following topology is used (cropped for brevity):
> > /sys/bus/cxl/devices/
> > ├── decoder0.0 -> ../../../devices/platform/ACPI0017:00/root0/decoder0.0
> > ├── decoder0.1 -> ../../../devices/platform/ACPI0017:00/root0/decoder0.1
> > ├── decoder1.0 -> ../../../devices/platform/ACPI0017:00/root0/port1/decoder1.0
> > ├── decoder2.0 -> ../../../devices/platform/ACPI0017:00/root0/port2/decoder2.0
> > ├── decoder3.0 -> ../../../devices/platform/ACPI0017:00/root0/port1/endpoint3/decoder3.0
> > ├── decoder4.0 -> ../../../devices/platform/ACPI0017:00/root0/port2/endpoint4/decoder4.0
> > ├── decoder5.0 -> ../../../devices/platform/ACPI0017:00/root0/port1/endpoint5/decoder5.0
> > ├── decoder6.0 -> ../../../devices/platform/ACPI0017:00/root0/port2/endpoint6/decoder6.0
> > ├── endpoint3 -> ../../../devices/platform/ACPI0017:00/root0/port1/endpoint3
> > ├── endpoint4 -> ../../../devices/platform/ACPI0017:00/root0/port2/endpoint4
> > ├── endpoint5 -> ../../../devices/platform/ACPI0017:00/root0/port1/endpoint5
> > ├── endpoint6 -> ../../../devices/platform/ACPI0017:00/root0/port2/endpoint6
> > ...
> >
> > 1. Select a Root Decoder whose interleave spans the desired interleave config
> >    - devices, IG, IW, Large enough address space.
> >    - ie. pick decoder0.0
> > 2. Program the decoders for the endpoints comprising the interleave set.
> >    - ie. echo $((256 << 20)) > /sys/bus/cxl/devices/decoder3.0
> > 3. Create a region
> >    - ie. echo $(cat create_pmem_region) >| create_pmem_region
> > 4. Configure a region
> >    - ie. echo 256 >| interleave_granularity
> >        echo 1 >| interleave_ways
> >        echo $((256 << 20)) >| size
> >        echo decoder3.0 >| target0
> > 5. Bind the region driver to the region
> >    - ie. echo region0 > /sys/bus/cxl/drivers/cxl_region/bind
> >
> Hi Ben,
>
> I finally got around to actually trying this out on top of Dan's recent fix set
> (I rebased it from the cxl/preview branch on kernel.org).
>
> I'm not having much luck actually bring up a region.
>
> The patch set refers to configuring the end point decoders, but all their
> sysfs attributes are read only.  Am I missing a dependency somewhere or
> is the intent that this series is part of the solution only?
>
> I'm confused!

There's a new series that's being reviewed internally before going to the list:

https://gitlab.com/bwidawsk/linux/-/tree/cxl_region-redux3

Given the proximity to the merge window opening and the need to get
the "mem_enabled" series staged, I asked Ben to hold it back from the
list for now.

There are some changes I am folding into it, but I hope to send it out
in the next few days after "mem_enabled" is finalized.