On Tue, Jun 8, 2021 at 6:06 AM Jonathan Cameron
<Jonathan.Cameron@xxxxxxxxxx> wrote:
>
> On Sat, 5 Jun 2021 23:05:27 -0700
> Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
>
> > A cxl_decoder is a child of a cxl_port. It represents a hardware
> > decoder configuration of an upstream port to one or more of its
> > downstream ports. The decoder is either represented in standard HDM
> > decoder registers (see CXL 2.0 section 8.2.5.12 CXL HDM Decoder
> > Capability Structure), or it is a static decode configuration
> > communicated by platform firmware (see the CXL Early Discovery Table:
> > Fixed Memory Window Structure).
> >
> > The firmware described and hardware described decoders differ slightly
> > leading to 2 different sub-types of decoders, cxl_decoder_root and
> > cxl_decoder_switch. At the root level the decode capabilities restrict
> > what can be mapped beneath them. Mid-level switch decoders are
> > configured for either acclerator (type-2) or memory-expander (type-3)
> > operation, but they are otherwise agnostic to the type of memory
> > (volatile vs persistent) being mapped.
> >
> > Here is an example topology from a single-ported host-bridge environment
> > without CFMWS decodes enumerated.
> >
> > /sys/bus/cxl/devices/root0
> > ├── devtype
> > ├── dport0 -> ../LNXSYSTM:00/LNXSYBUS:00/ACPI0016:00
> > ├── port1
> > │ ├── decoder1.0
> > │ │ ├── devtype
> > │ │ ├── end
> > │ │ ├── locked
> > │ │ ├── start
> > │ │ ├── subsystem -> ../../../../bus/cxl
> > │ │ ├── target_list
> > │ │ ├── target_type
> > │ │ └── uevent
> > │ ├── devtype
> > │ ├── dport0 -> ../../pci0000:34/0000:34:00.0
> > │ ├── subsystem -> ../../../bus/cxl
> > │ ├── uevent
> > │ └── uport -> ../../LNXSYSTM:00/LNXSYBUS:00/ACPI0016:00
> > ├── subsystem -> ../../bus/cxl
> > ├── uevent
> > └── uport -> ../platform/ACPI0017:00
> >
> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>
> One trivial docs issue and a suggestion that -2 is a bit too magic.
> Otherwise looks good to me.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
> > ---
> > Documentation/ABI/testing/sysfs-bus-cxl | 70 ++++++++
> > drivers/cxl/acpi.c | 21 ++
> > drivers/cxl/core.c | 265 +++++++++++++++++++++++++++++++
> > drivers/cxl/cxl.h | 48 ++++++
> > 4 files changed, 403 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> > index 0cb31b7ad17b..f16f18e77578 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-cxl
> > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > @@ -48,3 +48,73 @@ Description:
> > decode of CXL memory resources. The 'Y' integer reflects the
> > hardware port unique-id used in the hardware decoder target
> > list.
> > +
> > +What: /sys/bus/cxl/devices/decoderX.Y
> > +Date: June, 2021
> > +KernelVersion: v5.14
> > +Contact: linux-cxl@xxxxxxxxxxxxxxx
> > +Description:
> > + CXL decoder objects are enumerated from either a platform
> > + firmware description, or a CXL HDM decoder register set in a
> > + PCIe device (see CXL 2.0 section 8.2.5.12 CXL HDM Decoder
> > + Capability Structure). The 'X' in decoderX.Y represents the
> > + cxl_port container of this decoder, and 'Y' represents the
> > + instance id of a given decoder resource.
> > +
> > +What: /sys/bus/cxl/devices/decoderX.Y/{start,end}
> > +Date: June, 2021
> > +KernelVersion: v5.14
> > +Contact: linux-cxl@xxxxxxxxxxxxxxx
> > +Description:
> > + The 'start' and 'end' attributes together convey the physical
> > + address base and last addressable byte of the decoder's decode
> > + window. For decoders of devtype "cxl_decoder_root" the address
> > + range is fixed. For decoders of devtype "cxl_decoder_switch" the
> > + address is bounded by the decode range of the cxl_port ancestor
> > + of the decoder's cxl_port, and dynamically updates based on the
> > + active memory regions in that address space.
> > +
> > +What: /sys/bus/cxl/devices/decoderX.Y/locked
> > +Date: June, 2021
> > +KernelVersion: v5.14
> > +Contact: linux-cxl@xxxxxxxxxxxxxxx
> > +Description:
> > + CXL HDM decoders have the capability to lock the configuration
> > + until the next device reset. For decoders of devtype
> > + "cxl_decoder_root" there is no standard facility to unlock them.
> > + For decoders of devtype "cxl_decoder_switch" a secondary bus
> > + reset, of the PCIe bridge that provides the bus for this
> > + decoders uport, unlocks / resets the decoder.
> > +
> > +What: /sys/bus/cxl/devices/decoderX.Y/target_list
> > +Date: June, 2021
> > +KernelVersion: v5.14
> > +Contact: linux-cxl@xxxxxxxxxxxxxxx
> > +Description:
> > + Display a comma separated list of the current decoder target
> > + configuration. The list is ordered by the current configured
> > + interleave order of the decoder's dport instances. Each entry in
> > + the list is a dport id.
> > +
> > +What: /sys/bus/cxl/devices/decoderX.Y/cap_{pmem,ram,type2,type3}
> > +Date: June, 2021
> > +KernelVersion: v5.14
> > +Contact: linux-cxl@xxxxxxxxxxxxxxx
> > +Description:
> > + When a CXL decoder is of devtype "cxl_decoder_root", it
> > + represents a fixed memory window identified by platform
> > + firmware. A fixed window may only support a subset of memory
> > + types. The 'cap_*' attributes indicate whether persistent
> > + memory, volatile memory, accelerator memory, and / or expander
> > + memory may be mapped behind this decoder's memory window.
> > +
> > +What: /sys/bus/cxl/devices/decoderX.Y/target_type
> > +Date: June, 2021
> > +KernelVersion: v5.14
> > +Contact: linux-cxl@xxxxxxxxxxxxxxx
> > +Description:
> > + When a CXL decoder is of devtype "cxl_decoder_switch", it can
> > + optionally decode either accelerator memory (type-2) or expander
> > + memory (type-3). The 'target_type' attribute indicates the
> > + current setting which may dynamically change based on what
> > + memory regions are activated in this decode hierarchy.
>
> Nice docs.
>
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index ec324bf063b8..6f203ba7fc1d 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -70,6 +70,7 @@ static int add_host_bridge_uport(struct device *match, void *arg)
> > struct device *host = root_port->dev.parent;
> > struct acpi_pci_root *pci_root;
> > struct cxl_walk_context ctx;
> > + struct cxl_decoder *cxld;
> > struct cxl_port *port;
> >
> > if (!bridge)
> > @@ -94,7 +95,25 @@ static int add_host_bridge_uport(struct device *match, void *arg)
> >
> > if (ctx.count == 0)
> > return -ENODEV;
> > - return ctx.error;
> > + if (ctx.error)
> > + return ctx.error;
> > +
> > + /* TODO: Scan CHBCR for HDM Decoder resources */
> > +
> > + /*
> > + * In the single-port host-bridge case there are no HDM decoders
> > + * in the CHBCR and a 1:1 passthrough decode is implied.
> > + */
> > + if (ctx.count == 1) {
> > + cxld = devm_cxl_add_decoder(host, port, 1, 0, -2, 1, 0,
>
> -2? I'm guessing that has some special meaning and perhaps warrants
> a nice define to let us know what that is.
Um, yes, I think I went back and forth on whether this should be a
zero-length range starting at zero or a zero-length range starting at
UINT64_MAX, and I ended up botching it with a range of 0 to UINT64_MAX
- 1. I'll fix this up to provide a "passthrough" version of
devm_cxl_add_decoder() for the cases like this where a port does not
adjust the decode from the parent port down to the next level.
>
> Given this interface is a bit long perhaps even wroth a comment here on
> why the values are what they are?
The passthrough helper will address this.
