Jonathan Cameron wrote: > On Mon, 18 Jul 2022 18:55:01 -0700 > Dan Williams <dan.j.williams@xxxxxxxxx> wrote: > > > From: Ira Weiny <ira.weiny@xxxxxxxxx> > > > > The per-device CDAT data provides performance data that is relevant for > > mapping which CXL devices can participate in which CXL ranges by QTG > > (QoS Throttling Group) (per ECN: CXL 2.0 CEDT CFMWS & QTG_DSM) [1]. The > > QTG association specified in the ECN is advisory. Until the > > cxl_acpi driver grows support for invoking the QTG _DSM method the CDAT > > data is only of interest to userspace that may need it for debug > > purposes. > > > > Search the DOE mailboxes available, query CDAT data, cache the data and > > make it available via a sysfs binary attribute per endpoint at: > > > > /sys/bus/cxl/devices/endpointX/CDAT > > > > ...similar to other ACPI-structured table data in > > /sys/firmware/ACPI/tables. The CDAT is relative to 'struct cxl_port' > > objects since switches in addition to endpoints can host a CDAT > > instance. Switch CDAT support is not implemented. > > > > This does not support table updates at runtime. It will always provide > > whatever was there when first cached. It is also the case that table > > updates are not expected outside of explicit DPA address map affecting > > commands like Set Partition with the immediate flag set. Given that the > > driver does not support Set Partition with the immediate flag set there > > is no current need for update support. > > > > Link: https://www.computeexpresslink.org/spec-landing [1] > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > > Co-developed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > > Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx> > > [djbw: drop in-kernel parsing infra for now, and other minor fixups] > > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> > One comment inline that basically says the error message will do for now > if we race with an autonomous update of CDAT by the device. (see response > to earlier version on why that can happen). > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > > --- > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > > index 7672789c3225..9240df53ed87 100644 > > --- a/drivers/cxl/core/pci.c > > +++ b/drivers/cxl/core/pci.c > > > > +/** > > + * read_cdat_data - Read the CDAT data on this port > > + * @port: Port to read data from > > + * > > + * This call will sleep waiting for responses from the DOE mailbox. > > + */ > > +void read_cdat_data(struct cxl_port *port) > > +{ > > + struct pci_doe_mb *cdat_doe; > > + struct device *dev = &port->dev; > > + struct device *uport = port->uport; > > + size_t cdat_length; > > + int rc; > > + > > + cdat_doe = find_cdat_doe(uport); > > + if (!cdat_doe) { > > + dev_dbg(dev, "No CDAT mailbox\n"); > > + return; > > + } > > + > > + port->cdat_available = true; > > + > > + if (cxl_cdat_get_length(dev, cdat_doe, &cdat_length)) { > > + dev_dbg(dev, "No CDAT length\n"); > > + return; > > + } > > + > > + port->cdat.table = devm_kzalloc(dev, cdat_length, GFP_KERNEL); > > + if (!port->cdat.table) > > + return; > > + > > + port->cdat.length = cdat_length; > > + rc = cxl_cdat_read_table(dev, cdat_doe, &port->cdat); > > + if (rc) { > > + /* Don't leave table data allocated on error */ > > + devm_kfree(dev, port->cdat.table); > > + port->cdat.table = NULL; > > + port->cdat.length = 0; > > + dev_err(dev, "CDAT data read error\n"); > > This will be sufficient for now for the race against autonomous CDAT > update potential issue mentioned in earlier review. If it happens > and someone really cares they can unbind and rebind the device. Even it is too late to change the spec, which I do not think it is, it is not too late to make a public statement that Linux has no interest in supporting such shenanigans outside of a documented set of explicit commands that have a CDAT-update side effect.
