Jonathan Cameron wrote:
> On Thu, 23 Jun 2022 21:19:41 -0700
> Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
>
> > After a region's interleave parameters (ways and granularity) are set,
> > add a way for regions to allocate HPA from the free capacity in their
> > decoder. The allocator for this capacity reuses the 'struct resource'
> > based allocator used for CONFIG_DEVICE_PRIVATE.
> >
> > Once the tuple of "ways, granularity, and size" is set the
> > region configuration transitions to the CXL_CONFIG_INTERLEAVE_ACTIVE
> > state which is a precursor to allowing endpoint decoders to be added to
> > a region.
> >
> > Co-developed-by: Ben Widawsky <bwidawsk@xxxxxxxxxx>
> > Signed-off-by: Ben Widawsky <bwidawsk@xxxxxxxxxx>
> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>
> A few comments on the interface inline.
>
> Thanks,
>
> Jonathan
>
> > ---
> > Documentation/ABI/testing/sysfs-bus-cxl | 25 ++++
> > drivers/cxl/Kconfig | 3 +
> > drivers/cxl/core/region.c | 148 +++++++++++++++++++++++-
> > drivers/cxl/cxl.h | 2 +
> > 4 files changed, 177 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> > index 46d5295c1149..3658facc9944 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-cxl
> > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > @@ -294,3 +294,28 @@ Description:
> > (RW) Configures the number of devices participating in the
> > region is set by writing this value. Each device will provide
> > 1/interleave_ways of storage for the region.
> > +
> > +
> > +What: /sys/bus/cxl/devices/regionZ/size
> > +Date: May, 2022
> > +KernelVersion: v5.20
> > +Contact: linux-cxl@xxxxxxxxxxxxxxx
> > +Description:
> > + (RW) System physical address space to be consumed by the region.
> > + When written to, this attribute will allocate space out of the
> > + CXL root decoder's address space. When read the size of the
> > + address space is reported and should match the span of the
> > + region's resource attribute. Size shall be set after the
> > + interleave configuration parameters.
>
> There seem to be constraints that say you have to set this to 0 and then something
> else later to force a resize. That should be mentioned here or gotten rid of.
Yes, a constraint that precludes questions about what happens to
existing data during a resize. The force trip through a zero allocation
is to help codify that the kernel makes no guarantees about the state of
data over a resize.
Updated the text to:
(RW) System physical address space to be consumed by the region.
When written trigger the driver to allocate space out of the
parent root decoder's address space. When read the size of the
address space is reported and should match the span of the
region's resource attribute. Size shall be set after the
interleave configuration parameters. Once set it cannot be
changed, only freed by writing 0. The kernel makes no guarantees
that data is maintained over an address space freeing event, and
there is no guarantee that a free followed by an allocate
results in the same address being allocated.
>
>
> > +
> > +
> > +What: /sys/bus/cxl/devices/regionZ/resource
> > +Date: May, 2022
> > +KernelVersion: v5.20
> > +Contact: linux-cxl@xxxxxxxxxxxxxxx
> > +Description:
> > + (RO) A region is a contiguous partition of a CXL root decoder
> > + address space. Region capacity is allocated by writing to the
> > + size attribute, the resulting physical address space determined
> > + by the driver is reflected here. It is therefore not useful to
> > + read this before writing a value to the size attribute.
>
> I don't much like naming a "base address" resource. I'd expect resource to contain
> both base and size whereas this only has the base address of the region.
I think there is too much precedent to rename at this point.
