[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <62d606b0f0a9c_97b64294d9@dwillia2-xfh.jf.intel.com.notmuch>
Date: Mon, 18 Jul 2022 18:19:45 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>,
"Bjorn Helgaas" <bhelgaas@...gle.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>
CC: Ira Weiny <ira.weiny@...el.com>, Lukas Wunner <lukas@...ner.de>,
"Alison Schofield" <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>,
Dave Jiang <dave.jiang@...el.com>,
Ben Widawsky <bwidawsk@...nel.org>,
<linux-kernel@...r.kernel.org>, <linux-cxl@...r.kernel.org>,
<linux-pci@...r.kernel.org>
Subject: RE: [PATCH V14 6/7] cxl/port: Read CDAT table
ira.weiny@ wrote:
> From: Ira Weiny <ira.weiny@...el.com>
>
> The OS will need CDAT data from CXL devices to properly set up
> interleave sets. Currently this is supported through a DOE mailbox
> which supports CDAT.
>
> Search the DOE mailboxes available, query CDAT data, and cache the data
> for later parsing.
>
> Provide a sysfs binary attribute to allow dumping of the CDAT.
>
> Binary dumping is modeled on /sys/firmware/ACPI/tables/
>
> The ability to dump this table will be very useful for emulation of real
> devices once they become available as QEMU CXL type 3 device emulation will
> be able to load this file in.
>
> This does not support table updates at runtime. It will always provide
> whatever was there when first cached. Handling of table updates can be
> implemented later.
>
> Finally create a complete list of CDAT defines within cdat.h for code
> wishing to decode the CDAT table.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> Co-developed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> Signed-off-by: Ira Weiny <ira.weiny@...el.com>
>
> ---
> Changes from V13:
> Dan:
> Add entry in Documentation/ABI/testing/sysfs-bus-cxl
> Remove table parsing defines.
> s/cdat_sup/cdat_available
> s/cdat_mb/cdat_doe/
> Don't check endpoint in find_cdat_doe()
> Create CDAT_DOE_TASK macro
>
> Changes from V12:
> Fix checking for task.rv for errors
> Ensure no over run of non-DW aligned buffer length's
>
> Changes from V11:
> Adjust for the use of DOE mailbox xarray
> Dan Williams:
> Remove unnecessary get/put device
> Use new BIN_ATTR_ADMIN_RO macro
> Flag that CDAT was supported
> If there is a read error then the CDAT sysfs
> will return a 0 length entry
>
> Changes from V10:
> Ben Widawsky
> Failure to find CDAT should be a debug message not error
> Remove reference to cdat_mb from the port object
> Dropped [PATCH V10 5/9] cxl/port: Find a DOE mailbox which supports
> CDAT
> Iterate the mailboxes for the CDAT one each time.
> Define CXL_DOE_TABLE_ACCESS_LAST_ENTRY and add comment about
> it's use.
>
> Changes from V9:
> Add debugging output
> Jonathan Cameron
> Move read_cdat to port probe by using dev_groups for the
> sysfs attributes. This avoids issues with using devm
> before the driver is loaded while making sure the CDAT
> binary is available.
>
> Changes from V8:
> Fix length print format
> Incorporate feedback from Jonathan
> Move all this to cxl_port which can help support switches when
> the time comes.
>
> Changes from V6:
> Fix issue with devm use
> Move cached cdat data to cxl_dev_state
> Use new pci_doe_submit_task()
> Ensure the aux driver is locked while processing tasks
> Rebased on cxl-pending
>
> Changes from V5:
> Add proper guards around cdat.h
> Split out finding the CDAT DOE mailbox
> Use cxl_cdat to group CDAT data together
> Adjust to use auxiliary_find_device() to find the DOE device
> which supplies the CDAT protocol.
> Rebased to latest
> Remove dev_dbg(length)
> Remove unneeded DOE Table access defines
> Move CXL_DOE_PROTOCOL_TABLE_ACCESS define into this patch where
> it is used
>
> Changes from V4:
> Split this into it's own patch
> Rearchitect this such that the memdev driver calls into the DOE
> driver via the cxl_mem state object. This allows CDAT data to
> come from any type of cxl_mem object not just PCI DOE.
> Rebase on new struct cxl_dev_state
> ---
> Documentation/ABI/testing/sysfs-bus-cxl | 10 ++
> drivers/cxl/cdat.h | 61 +++++++++
> drivers/cxl/core/pci.c | 169 ++++++++++++++++++++++++
> drivers/cxl/cxl.h | 5 +
> drivers/cxl/cxlpci.h | 1 +
> drivers/cxl/port.c | 54 ++++++++
> 6 files changed, 300 insertions(+)
> create mode 100644 drivers/cxl/cdat.h
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 1fd5984b6158..6fb6459466f8 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -164,3 +164,13 @@ Description:
> 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.
> +
> +What: /sys/bus/cxl/devices/endpointX/CDAT/cdat
> +Date: July, 2022
> +KernelVersion: v5.19
> +Contact: linux-cxl@...r.kernel.org
> +Description:
> + (RO) If this sysfs entry is not present no DOE mailbox was
> + found to support CDAT data. If it is present and the length of
> + the data is 0 reading the CDAT data failed. Otherwise the CDAT
> + data is reported.
> diff --git a/drivers/cxl/cdat.h b/drivers/cxl/cdat.h
> new file mode 100644
> index 000000000000..67010717ffca
> --- /dev/null
> +++ b/drivers/cxl/cdat.h
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __CXL_CDAT_H__
> +#define __CXL_CDAT_H__
> +
> +/*
> + * Coherent Device Attribute table (CDAT)
> + *
> + * Specification available from UEFI.org
> + *
> + * Whilst CDAT is defined as a single table, the access via DOE maiboxes is
> + * done one entry at a time, where the first entry is the header.
> + */
> +
> +#define CXL_DOE_TABLE_ACCESS_REQ_CODE 0x000000ff
> +#define CXL_DOE_TABLE_ACCESS_REQ_CODE_READ 0
> +#define CXL_DOE_TABLE_ACCESS_TABLE_TYPE 0x0000ff00
> +#define CXL_DOE_TABLE_ACCESS_TABLE_TYPE_CDATA 0
> +#define CXL_DOE_TABLE_ACCESS_ENTRY_HANDLE 0xffff0000
> +#define CXL_DOE_TABLE_ACCESS_LAST_ENTRY 0xffff
> +
> +/*
> + * CDAT entries are little endian and are read from PCI config space which
> + * is also little endian.
> + * As such, on a big endian system these will have been reversed.
> + * This prevents us from making easy use of packed structures.
> + * Style form pci_regs.h
> + */
I do not get this... the PCI ops on big endian machines are already
handling the fact that PCI config space is in le-order. So you should be
able to use data structure definitions directly just like any other PCI
config data payload.
I dropped this along with the other revisions I spotted for this patch.
> +
> +#define CDAT_HEADER_LENGTH_DW 4
> +#define CDAT_HEADER_LENGTH_BYTES (CDAT_HEADER_LENGTH_DW * sizeof(u32))
> +#define CDAT_HEADER_DW0_LENGTH 0xffffffff
> +#define CDAT_HEADER_DW1_REVISION 0x000000ff
> +#define CDAT_HEADER_DW1_CHECKSUM 0x0000ff00
> +/* CDAT_HEADER_DW2_RESERVED */
> +#define CDAT_HEADER_DW3_SEQUENCE 0xffffffff
> +
> +/* All structures have a common first DW */
> +#define CDAT_STRUCTURE_DW0_TYPE 0x000000ff
> +#define CDAT_STRUCTURE_DW0_TYPE_DSMAS 0
> +#define CDAT_STRUCTURE_DW0_TYPE_DSLBIS 1
> +#define CDAT_STRUCTURE_DW0_TYPE_DSMSCIS 2
> +#define CDAT_STRUCTURE_DW0_TYPE_DSIS 3
> +#define CDAT_STRUCTURE_DW0_TYPE_DSEMTS 4
> +#define CDAT_STRUCTURE_DW0_TYPE_SSLBIS 5
> +
> +#define CDAT_STRUCTURE_DW0_LENGTH 0xffff0000
Dropped these too since they are unused.
Powered by blists - more mailing lists