Re: [RFC PATCH v2 2/4] PCI/doe: Initial support PCI Data Object Exchange

[Bjorn Helgaas <helgaas@kernel.org>]

To match subject line history:

  PCI/DOE: Add Data Object Exchange support

On Wed, Apr 14, 2021 at 12:01:57AM +0800, Jonathan Cameron wrote:
> Introduced in a PCI ECN [1], DOE provides a config space
> based mailbox with standard protocol discovery.  Each mailbox
> is accessed through a DOE PCIE Extended Capability.

s/PCIE//

I don't think DOE depends on anything specific to PCIe.

> A device may have 1 or more DOE mailboxes, each of which is allowed
> to support any number of protocols (some DOE protocols

s/DOE protocols/DOE protocol/

> specifications apply additional restrictions).  A given protocol
> may be supported on more than one DOE mailbox on a given function.
> 
> If a driver wishes to access any number of DOE instances / protocols
> it makes a single call to pcie_doe_register_all() which will find
> available DOEs, create the required infrastructure and cache the
> protocols they support.  pcie_doe_find() can then retrieve a
> pointer to an appropriate DOE instance.

Maybe mention the run-time interface (pcie_doe_sync()) here as well.
Can we make that name more descriptive?  It'd be nice to include a
verb.

I think we should globally s/pcie/pci/ in filenames, struct,
interfaces, config option, etc.

> Testing conducted against QEMU using:
> 
> https://lore.kernel.org/qemu-devel/1612900760-7361-1-git-send-email-cbrowy@avery-design.com/
> + fix for interrupt flag mentioned in that thread and a whole load
> of hacks to exercise error paths etc.
> 
> [1] https://members.pcisig.com/wg/PCI-SIG/document/14143

Can you please include the title and data of approval here?  The URL
is useless except to PCI-SIG members, and the URL is likely to become
stale eventually even to them.

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> 
> Thanks to Dan Williams for reviewing v1.
> Note code has changed a lot, so I may well have missed stuff in this
> log. Details on approaches to the DOE serialization discussed in
> the cover letter.
> 
> Changes since v1:
> Major stuff:
> * Rework the registration of DOE. DOE instances are added to a list in
>   the struct pci_dev. A single call to pcie_doe_register_all() finds
>   all DOEs present on a device and caches which protocols they support.
>   The lifetime of these can then all be managed alongside the
>   struct pci_dev avoiding need for reference counting etc whilst allowing
>   a driver not to care if different protocols are using the same DOE
>   instance, or different ones. The driver issues a call to
>   pcie_doe_find() to get a pointer to the first appropriate DOE.
>   Note, a side effect of minimizing impact of this on struct pci_dev
>   is that we can't build DOE support as a module.
> * Instead of serializing on a mutex, move to a more complex state machine
>   based scheme in which the DOE config space registers are only
>   accessed from a single delayed work item. The state machine uses
>   two states for normal operation, IDLE and WAITING. Two states also
>   exist for ABORT to indicate if it was called in response to error,
>   or at startup. Interrupts result in mod_delayed_work()
>   to force an immediate check on the status register.
>   Message queuing is implemented with a list of tasks and kicking
>   the state machine work item as needed.
> * Decide, inside the core DOE code, whether to poll based
>   on if the MSI/MSIX interrupts are enabled and the DOE
>   instance advertises an interrupt.
> * Define a struct pcie_doe_exchange to encapsulate header info
>   with payloads etc. Specific fields used for VID and protocol +
>   lengths so the DOE header can be constructed inside the DOE
>   functions rather than relying on callers to do that.
> * Don't hide away devm_ stuff in calls that don't make it obvious.
> 
> Minor stuff:
> * pcie_doe_init() and pcie_doe_register separation to cover the
>   bits that can fail in register() and the bits that can't in init()
> * Many typos
> * Rename cap_offset field to cap
> * Use new PCI_VENDOR_ID_PCI_SIG define to avoid open coding the
>   value
> * Add defines for timeouts etc.
> * Missing EXPORT_SYMBOL_GPL() added to allow DOE access from
>   modules.
> 
>  drivers/pci/pcie/Kconfig      |   8 +
>  drivers/pci/pcie/Makefile     |   1 +
>  drivers/pci/pcie/doe.c        | 590 ++++++++++++++++++++++++++++++++++

I'd move this up to drivers/pci/ since I don't think there's any
actual PCIe dependency.

>  include/linux/pci.h           |   3 +
>  include/linux/pcie-doe.h      |  85 +++++

>  include/uapi/linux/pci_regs.h |  29 +-
>  6 files changed, 715 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
> index 45a2ef702b45..1b566d08bfef 100644
> --- a/drivers/pci/pcie/Kconfig
> +++ b/drivers/pci/pcie/Kconfig
> @@ -142,3 +142,11 @@ config PCIE_EDR
>  	  the PCI Firmware Specification r3.2.  Enable this if you want to
>  	  support hybrid DPC model which uses both firmware and OS to
>  	  implement DPC.
> +
> +config PCIE_DOE
> +       bool
> +       help
> +         This enables library support PCI Data Object Exchange capability.
> +         DOE provides a simple mailbox in PCI express config space that is
> +         used by a number of different protocols.
> +         It is defined in the Data Object Exchange ECN to PCI 5.0.

s/support PCI/support for the PCI/
s/PCI express/PCI/

> diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
> index b2980db88cc0..801fdd5fbfc1 100644
> --- a/drivers/pci/pcie/Makefile
> +++ b/drivers/pci/pcie/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_PCIE_PME)		+= pme.o
>  obj-$(CONFIG_PCIE_DPC)		+= dpc.o
>  obj-$(CONFIG_PCIE_PTM)		+= ptm.o
>  obj-$(CONFIG_PCIE_EDR)		+= edr.o
> +obj-$(CONFIG_PCIE_DOE)		+= doe.o
> diff --git a/drivers/pci/pcie/doe.c b/drivers/pci/pcie/doe.c
> new file mode 100644
> index 000000000000..b8a27c744d09
> --- /dev/null
> +++ b/drivers/pci/pcie/doe.c
> @@ -0,0 +1,590 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Data Object Exchange ECN
> + * https://members.pcisig.com/wg/PCI-SIG/document/14143
> + *
> + * Copyright (C) 2021 Huawei
> + *     Jonathan Cameron <Jonathan.Cameron@huawei.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/jiffies.h>
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/pci.h>
> +#include <linux/pcie-doe.h>
> +#include <linux/workqueue.h>
> +
> +#define PCI_DOE_PROTOCOL_DISCOVERY 0
> +
> +#define PCI_DOE_BUSY_MAX_RETRIES 16
> +#define PCI_DOE_POLL_INTERVAL (HZ / 128)
> +
> +/* Timeout of 1 second from 6.xx.1 ECN - Data Object Exchange */

Unfortunate that the ECN has two 6.xx.1 sections and no 6.xx.2
section ;)  I think this is from the second 6.xx.1 section, i.e., the
one that *should* have been 6.xx.2.

> +#define PCI_DOE_TIMEOUT HZ
> +
> +static irqreturn_t pcie_doe_irq(int irq, void *data)
> +{
> +	struct pcie_doe *doe = data;
> +	struct pci_dev *pdev = doe->pdev;
> +	u32 val;
> +
> +	pci_read_config_dword(pdev, doe->cap + PCI_DOE_STATUS, &val);
> +	if (FIELD_GET(PCI_DOE_STATUS_INT_STATUS, val)) {
> +		pci_write_config_dword(pdev, doe->cap + PCI_DOE_STATUS, val);
> +		mod_delayed_work(system_wq, &doe->statemachine, 0);
> +		return IRQ_HANDLED;
> +	}
> +	/* Leave the error case to be handled outside irq */
> +	if (FIELD_GET(PCI_DOE_STATUS_ERROR, val)) {
> +		mod_delayed_work(system_wq, &doe->statemachine, 0);
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +/*
> + * Only call when safe to directly access the DOE, either because no tasks
> + * yet queued, or called from doe_statemachine_work() which has exclusive
> + * access to the DOE config space.
> + */
> +static void pcie_doe_abort_start(struct pcie_doe *doe)
> +{
> +	struct pci_dev *pdev = doe->pdev;
> +
> +	pci_write_config_dword(pdev, doe->cap + PCI_DOE_CTRL,
> +			       PCI_DOE_CTRL_ABORT);
> +
> +	doe->timeout_jiffies = jiffies + HZ;
> +	schedule_delayed_work(&doe->statemachine, HZ);
> +}
> +
> +static int pcie_doe_send_req(struct pcie_doe *doe, struct pcie_doe_exchange *ex)
> +{
> +	struct pci_dev *pdev = doe->pdev;
> +	u32 val;
> +	int i;
> +
> +	/*
> +	 * Check the DOE busy bit is not set.
> +	 * If it is set, this could indicate someone other than Linux is
> +	 * using the mailbox.

Rewrap to fill 80 columns (or add blank line if you want two
paragraphs).  What are the possibilities other than Linux?  Do you
envision user-space usage?  Is there a denial-of-service attack here
if that other user leaves the busy bit set?

> +	 */
> +	pci_read_config_dword(pdev, doe->cap + PCI_DOE_STATUS, &val);
> +	if (FIELD_GET(PCI_DOE_STATUS_BUSY, val))
> +		return -EBUSY;
> +
> +	if (FIELD_GET(PCI_DOE_STATUS_ERROR, val))
> +		return -EIO;
> +
> +	/* Write DOE Header */
> +	val = FIELD_PREP(PCI_DOE_DATA_OBJECT_HEADER_1_VID, ex->vid) |
> +		FIELD_PREP(PCI_DOE_DATA_OBJECT_HEADER_1_TYPE, ex->protocol);
> +	pci_write_config_dword(pdev, doe->cap + PCI_DOE_WRITE, val);
> +	/* Length is 2 DW of header + length of payload in DW */
> +	pci_write_config_dword(pdev, doe->cap + PCI_DOE_WRITE,
> +			       FIELD_PREP(PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH,
> +					  2 + ex->request_pl_sz / 4));
> +	for (i = 0; i < ex->request_pl_sz / 4; i++)
> +		pci_write_config_dword(pdev, doe->cap + PCI_DOE_WRITE,
> +				       ex->request_pl[i]);
> +
> +	pci_write_config_dword(pdev, doe->cap + PCI_DOE_CTRL, PCI_DOE_CTRL_GO);
> +	/* Request is sent - now wait for poll or irq */
> +	return 0;
> +}
> +
> +static int pcie_doe_recv_resp(struct pcie_doe *doe, struct pcie_doe_exchange *ex)
> +{
> +	struct pci_dev *pdev = doe->pdev;
> +	size_t length;
> +	u32 val;
> +	int i;
> +
> +	/* Read the first two dwords to get the length and protocol */
> +	pci_read_config_dword(pdev, doe->cap + PCI_DOE_READ, &val);
> +	if ((FIELD_GET(PCI_DOE_DATA_OBJECT_HEADER_1_VID, val) != ex->vid) ||
> +	    (FIELD_GET(PCI_DOE_DATA_OBJECT_HEADER_1_TYPE, val) != ex->protocol)) {
> +		dev_err(&pdev->dev, "Expected [VID, Protocol] = [%x, %x], got [%x, %x]\n",

pci_err() (one more below)

> +			ex->vid, ex->protocol,
> +			FIELD_GET(PCI_DOE_DATA_OBJECT_HEADER_1_VID, val),
> +			FIELD_GET(PCI_DOE_DATA_OBJECT_HEADER_1_TYPE, val));
> +		return -EIO;
> +	}
> +
> +	pci_write_config_dword(pdev, doe->cap + PCI_DOE_READ, 0);
> +	pci_read_config_dword(pdev, doe->cap + PCI_DOE_READ, &val);
> +	pci_write_config_dword(pdev, doe->cap + PCI_DOE_READ, 0);
> +
> +	length = FIELD_GET(PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH, val);
> +	if (length > SZ_1M)
> +		return -EIO;
> +
> +	/* Read the rest of the response payload */
> +	for (i = 0; i < min(length, ex->response_pl_sz / 4); i++) {
> +		pci_read_config_dword(pdev, doe->cap + PCI_DOE_READ,
> +				      &ex->response_pl[i]);
> +		pci_write_config_dword(pdev, doe->cap + PCI_DOE_READ, 0);
> +	}
> +
> +	/* Flush excess length */
> +	for (; i < length; i++) {
> +		pci_read_config_dword(pdev, doe->cap + PCI_DOE_READ, &val);
> +		pci_write_config_dword(pdev, doe->cap + PCI_DOE_READ, 0);
> +	}
> +	/* Final error check to pick up on any since Data Object Ready */
> +	pci_read_config_dword(pdev, doe->cap + PCI_DOE_STATUS, &val);
> +	if (FIELD_GET(PCI_DOE_STATUS_ERROR, val))
> +		return -EIO;
> +
> +	return min(length, ex->response_pl_sz / 4) * 4;
> +}
> +
> +static void pcie_doe_task_complete(void *private)
> +{
> +	complete(private);
> +}
> +
> +/**
> + * struct pcie_doe_task - description of a query / response task
> + * @h: Head to add it to the list of outstanding tasks.
> + * @ex: The info details of the task to be done.
> + * @rv: Return value.  Length of received response or error.
> + * @cb: Callback for completion of task.
> + * @private: Private data passed to callback on completion.

Some doc has terminating periods, some doesn't.

> + */
> +struct pcie_doe_task {
> +	struct list_head h;
> +	struct pcie_doe_exchange *ex;
> +	int rv;
> +	void (*cb)(void *private);
> +	void *private;
> +};
> +
> +/**
> + * pcie_doe_sync() - Send a request, then wait for and receive response.
> + * @doe: DOE mailbox state structure
> + * @ex: Description of the buffers and vid, type used in this
> + *      request/response pair

s/vid/Vendor ID/ (I think?)

> + * Excess data will be discarded.
> + *
> + * Return: payload in bytes on success, < 0 on error
> + */
> +int pcie_doe_sync(struct pcie_doe *doe, struct pcie_doe_exchange *ex)
> +{
> +	struct pcie_doe_task task;
> +	DECLARE_COMPLETION_ONSTACK(c);
> +	bool only_task;

list_empty() actually returns "int".

> +	/* DOE requests must be a whole number of DW */
> +	if (ex->request_pl_sz % sizeof(u32))
> +		return -EINVAL;
> +
> +	task.ex = ex;
> +	task.cb = pcie_doe_task_complete;
> +	task.private = &c;
> +
> +	mutex_lock(&doe->tasks_lock);
> +	if (doe->dead) {
> +		mutex_unlock(&doe->tasks_lock);
> +		return -EIO;
> +	}
> +	only_task = list_empty(&doe->tasks);
> +	list_add_tail(&task.h, &doe->tasks);
> +	if (only_task)
> +		schedule_delayed_work(&doe->statemachine, 0);
> +	mutex_unlock(&doe->tasks_lock);
> +	wait_for_completion(&c);
> +
> +	return task.rv;
> +}
> +
> +static void doe_statemachine_work(struct work_struct *work)
> +{
> +	struct delayed_work *w = to_delayed_work(work);
> +	struct pcie_doe *doe = container_of(w, struct pcie_doe, statemachine);
> +	struct pci_dev *pdev = doe->pdev;
> +	struct pcie_doe_task *task;
> +	bool abort;
> +	u32 val;
> +	int rc;
> +
> +	mutex_lock(&doe->tasks_lock);
> +	task = list_first_entry_or_null(&doe->tasks, struct pcie_doe_task, h);
> +	abort = doe->abort;
> +	doe->abort = false;
> +	mutex_unlock(&doe->tasks_lock);
> +
> +	if (abort) {
> +		/*
> +		 * Currently only used during init - care needed if we want to generally
> +		 * expose pcie_doe_abort() as it would impact queries in flight.

Wrap to fit in 80 columns (more below).

> +		 */
> +		WARN_ON(task);
> +		doe->state = DOE_WAIT_ABORT;
> +		pcie_doe_abort_start(doe);
> +		return;
> +	}
> +
> +	switch (doe->state) {
> +	case DOE_IDLE:
> +		if (task == NULL)
> +			return;
> +
> +		/* Nothing currently in flight so queue a task */
> +		rc = pcie_doe_send_req(doe, task->ex);
> +		/*
> +		 * The specification does not provide any guidance on how long some other
> +		 * entity could keep the DOE busy, so try for 1 second then fail.
> +		 * Busy handling is best effort only, because there is not way of avoiding
> +		 * racing against another user of the DOE.

s/not way/no way/

> +		 */
> +		if (rc == -EBUSY) {
> +			doe->busy_retries++;
> +			if (doe->busy_retries == PCI_DOE_BUSY_MAX_RETRIES) {
> +				/* Long enough, fail this request */
> +				doe->busy_retries = 0;
> +				goto busy;
> +			}
> +			schedule_delayed_work(w, HZ / PCI_DOE_BUSY_MAX_RETRIES);
> +			return;
> +		}
> +		if (rc)
> +			goto abort;
> +		doe->busy_retries = 0;
> +
> +		doe->state = DOE_WAIT_RESP;
> +		doe->timeout_jiffies = jiffies + HZ;
> +		/* Now poll or wait for IRQ with timeout */

Capitalize "IRQ" elsewhere to match this usage.

> +		if (doe->irq > 0)
> +			schedule_delayed_work(w, PCI_DOE_TIMEOUT);
> +		else
> +			schedule_delayed_work(w, PCI_DOE_POLL_INTERVAL);
> +		return;
> +
> +	case DOE_WAIT_RESP:
> +		/* Not possible to get here with NULL task */
> +		pci_read_config_dword(pdev, doe->cap + PCI_DOE_STATUS, &val);
> +		if (FIELD_GET(PCI_DOE_STATUS_ERROR, val)) {
> +			rc = -EIO;
> +			goto abort;
> +		}
> +
> +		if (!FIELD_GET(PCI_DOE_STATUS_DATA_OBJECT_READY, val)) {
> +			/* If not yet at timeout reschedule otherwise abort */
> +			if (time_after(jiffies, doe->timeout_jiffies)) {
> +				rc = -ETIMEDOUT;
> +				goto abort;
> +			}
> +			schedule_delayed_work(w, PCI_DOE_POLL_INTERVAL);
> +			return;
> +		}
> +
> +		rc  = pcie_doe_recv_resp(doe, task->ex);
> +		if (rc < 0)
> +			goto abort;
> +
> +		doe->state = DOE_IDLE;
> +
> +		mutex_lock(&doe->tasks_lock);
> +		list_del(&task->h);
> +		if (!list_empty(&doe->tasks))
> +			schedule_delayed_work(w, 0);
> +		mutex_unlock(&doe->tasks_lock);
> +
> +		/* Set the return value to the length of received payload */
> +		task->rv = rc;
> +		task->cb(task->private);
> +		return;
> +
> +	case DOE_WAIT_ABORT:
> +	case DOE_WAIT_ABORT_ON_ERR:
> +		pci_read_config_dword(pdev, doe->cap + PCI_DOE_STATUS, &val);
> +
> +		if (!FIELD_GET(PCI_DOE_STATUS_ERROR, val) &&
> +		    !FIELD_GET(PCI_DOE_STATUS_BUSY, val)) {
> +			/* Back to normal state - carry on */
> +			mutex_lock(&doe->tasks_lock);
> +			if (!list_empty(&doe->tasks))
> +				schedule_delayed_work(w, 0);
> +			mutex_unlock(&doe->tasks_lock);
> +
> +			/* For deliberately triggered abort, someone is waiting */
> +			if (doe->state == DOE_WAIT_ABORT)
> +				complete(&doe->abort_c);
> +			doe->state = DOE_IDLE;
> +
> +			return;
> +		}
> +		if (time_after(jiffies, doe->timeout_jiffies)) {
> +			struct pcie_doe_task *t, *n;
> +
> +			/* We are dead - abort all queued tasks */
> +			dev_err(&pdev->dev, "DOE ABORT timed out\n");
> +			mutex_lock(&doe->tasks_lock);
> +			doe->dead = true;
> +			list_for_each_entry_safe(t, n, &doe->tasks, h) {
> +				t->rv = -EIO;
> +				t->cb(t->private);
> +				list_del(&t->h);
> +			}
> +
> +			mutex_unlock(&doe->tasks_lock);
> +			if (doe->state == DOE_WAIT_ABORT)
> +				complete(&doe->abort_c);
> +		}
> +		return;
> +	}
> +
> +abort:
> +	pcie_doe_abort_start(doe);
> +	doe->state = DOE_WAIT_ABORT_ON_ERR;
> +busy:
> +	mutex_lock(&doe->tasks_lock);
> +	list_del(&task->h);
> +	mutex_unlock(&doe->tasks_lock);
> +
> +	task->rv = rc;
> +	task->cb(task->private);
> +	/* If we got here via busy, and the queue isn't empty then we need to go again */
> +	if (doe->state == DOE_IDLE) {
> +		mutex_lock(&doe->tasks_lock);
> +		if (!list_empty(&doe->tasks))
> +			schedule_delayed_work(w, 0);
> +		mutex_unlock(&doe->tasks_lock);
> +	}
> +}
> +
> +static int pcie_doe_discovery(struct pcie_doe *doe, u8 *index, u16 *vid,
> +			      u8 *protocol)
> +{
> +	u32 request_pl = FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX, *index);
> +	u32 response_pl;
> +	struct pcie_doe_exchange ex = {
> +		.vid = PCI_VENDOR_ID_PCI_SIG,
> +		.protocol = PCI_DOE_PROTOCOL_DISCOVERY,
> +		.request_pl = &request_pl,
> +		.request_pl_sz = sizeof(request_pl),
> +		.response_pl = &response_pl,
> +		.response_pl_sz = sizeof(response_pl),
> +	};
> +	int ret;
> +
> +	ret = pcie_doe_sync(doe, &ex);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret != sizeof(response_pl))
> +		return -EIO;
> +
> +	*vid = FIELD_GET(PCI_DOE_DATA_OBJECT_DISC_RSP_3_VID, response_pl);
> +	*protocol = FIELD_GET(PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL, response_pl);
> +	*index = FIELD_GET(PCI_DOE_DATA_OBJECT_DISC_RSP_3_NEXT_INDEX, response_pl);
> +
> +	return 0;
> +}
> +
> +static int pcie_doe_cache_protocols(struct pcie_doe *doe)
> +{
> +	u8 index = 0;
> +	int rc;
> +
> +	/* Discovery protocol must always be supported and must report itself */
> +	doe->num_prots = 1;
> +	doe->prots = kzalloc(sizeof(*doe->prots) * doe->num_prots, GFP_KERNEL);
> +	if (doe->prots == NULL)
> +		return -ENOMEM;
> +
> +	do {
> +		struct pcie_doe_prot *prot, *prot_new;
> +
> +		prot = &doe->prots[doe->num_prots - 1];
> +		rc = pcie_doe_discovery(doe, &index, &prot->vid, &prot->type);
> +		if (rc)
> +			goto free_prots;
> +
> +		if (index) {
> +			prot_new = krealloc(doe->prots,
> +					    sizeof(*doe->prots) * doe->num_prots,
> +					    GFP_KERNEL);
> +			if (prot_new == NULL) {
> +				rc = -ENOMEM;
> +				goto free_prots;
> +			}
> +			doe->prots = prot_new;
> +			doe->num_prots++;
> +		}
> +	} while (index);
> +
> +	return 0;
> +
> +free_prots:
> +	kfree(doe->prots);
> +	return rc;
> +}
> +
> +static void pcie_doe_init(struct pcie_doe *doe, struct pci_dev *pdev, int doe_offset)
> +{
> +	mutex_init(&doe->tasks_lock);
> +	init_completion(&doe->abort_c);
> +	doe->cap = doe_offset;
> +	doe->pdev = pdev;
> +	INIT_LIST_HEAD(&doe->tasks);
> +	INIT_DELAYED_WORK(&doe->statemachine, doe_statemachine_work);
> +}
> +
> +static int pcie_doe_abort(struct pcie_doe *doe)
> +{
> +	reinit_completion(&doe->abort_c);
> +	mutex_lock(&doe->tasks_lock);
> +	doe->abort = true;
> +	mutex_unlock(&doe->tasks_lock);
> +	schedule_delayed_work(&doe->statemachine, 0);
> +	wait_for_completion(&doe->abort_c);
> +
> +	if (doe->dead)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int pcie_doe_register(struct pcie_doe *doe)
> +{
> +	struct pci_dev *pdev = doe->pdev;
> +	bool poll = !pci_dev_msi_enabled(pdev);
> +	int rc, irq;
> +	u32 val;
> +
> +	pci_read_config_dword(pdev, doe->cap + PCI_DOE_CAP, &val);
> +
> +	if (!poll && FIELD_GET(PCI_DOE_CAP_INT, val)) {
> +		irq = pci_irq_vector(pdev, FIELD_GET(PCI_DOE_CAP_IRQ, val));
> +		if (irq < 0)
> +			return irq;
> +
> +		rc = request_irq(irq, pcie_doe_irq, 0, "DOE", doe);
> +		if (rc)
> +			return rc;
> +
> +		doe->irq = irq;
> +		pci_write_config_dword(pdev, doe->cap + PCI_DOE_CTRL,
> +				       FIELD_PREP(PCI_DOE_CTRL_INT_EN, 1));
> +	}
> +
> +	/* Reset the mailbox by issuing an abort */
> +	rc = pcie_doe_abort(doe);
> +	if (rc)
> +		goto err_free_irqs;
> +
> +	return 0;
> +
> +err_free_irqs:
> +	if (doe->irq > 0)
> +		free_irq(doe->irq, doe);
> +
> +	return rc;
> +}
> +
> +static void pcie_doe_unregister(struct pcie_doe *doe)
> +{
> +	if (doe->irq > 0)
> +		free_irq(doe->irq, doe);
> +}
> +
> +void pcie_doe_unregister_all(struct pci_dev *pdev)
> +{
> +	struct pcie_doe *doe, *next;
> +
> +	list_for_each_entry_safe(doe, next, &pdev->doe_list, h) {
> +		/* First halt the state machine */
> +		cancel_delayed_work_sync(&doe->statemachine);
> +		kfree(doe->prots);
> +		pcie_doe_unregister(doe);
> +		kfree(doe);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(pcie_doe_unregister_all);
> +
> +/**
> + * pcie_doe_register_all() - Find and register all DOE mailboxes
> + * @pdev: PCI device whose DOE mailboxes we are finding.
> + *
> + * Will locate any DOE mailboxes present on the device and cache the
> + * protocols so that pcie_doe_find() can be used to retrieve a suitable DOE
> + * instance.
> + * DOE mailboxes are available until pcie_doe_unregister_all()
> + * is called.
> + *
> + * RETURNS: 0 on success, < 0 on error

Different styles, pick one:

 * Return: payload in bytes on success, < 0 on error
 * RETURNS: 0 on success, < 0 on error
 * RETURNS: Pointer to DOE instance, or NULL if no suitable instance available.

> + */
> +int pcie_doe_register_all(struct pci_dev *pdev)
> +{
> +	struct pcie_doe *doe;
> +	int pos = 0;
> +	int rc;
> +
> +	INIT_LIST_HEAD(&pdev->doe_list);
> +
> +	/* Walk the DOE extended capabilities and add to per pci_dev list */
> +	while (true) {
> +
> +		pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DOE);
> +		if (!pos)
> +			return 0;
> +
> +		doe = kzalloc(sizeof(*doe), GFP_KERNEL);
> +		if (!doe) {
> +			rc = -ENOMEM;
> +			goto free_does;
> +		}
> +
> +		pcie_doe_init(doe, pdev, pos);
> +		rc = pcie_doe_register(doe);
> +		if (rc) {
> +			kfree(doe);
> +			goto free_does;
> +		}
> +
> +		rc = pcie_doe_cache_protocols(doe);
> +		if (rc) {
> +			pcie_doe_unregister(doe);
> +			kfree(doe);
> +			goto free_does;
> +		}
> +
> +		list_add(&doe->h, &pdev->doe_list);
> +	}
> +
> +free_does:
> +	pcie_doe_unregister_all(pdev);
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(pcie_doe_register_all);
> +
> +/**
> + * pcie_doe_find() - Find the first DOE instance that supports a given protocol
> + * @pdev: Device on which to find the DOE instance.
> + * @vid: Vendor ID.
> + * @type: Specific protocol for this vendor.
> + *
> + * RETURNS: Pointer to DOE instance, or NULL if no suitable instance available.
> + */
> +struct pcie_doe *pcie_doe_find(struct pci_dev *pdev, u16 vid, u8 type)
> +{
> +	struct pcie_doe *doe;
> +	int i;
> +
> +	list_for_each_entry(doe, &pdev->doe_list, h) {
> +		for (i = 0; i < doe->num_prots; i++)
> +			if ((doe->prots[i].vid == vid) &&
> +			    (doe->prots[i].type == type))
> +				return doe;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(pcie_doe_find);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 86c799c97b77..5d958a63e8c7 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -332,6 +332,9 @@ struct pci_dev {
>  #ifdef CONFIG_PCIEPORTBUS
>  	struct rcec_ea	*rcec_ea;	/* RCEC cached endpoint association */
>  	struct pci_dev  *rcec;          /* Associated RCEC device */
> +#endif
> +#ifdef CONFIG_PCIE_DOE
> +	struct list_head doe_list;	/* Data Object Exchange mailboxes */
>  #endif
>  	u8		pcie_cap;	/* PCIe capability offset */
>  	u8		msi_cap;	/* MSI capability offset */
> diff --git a/include/linux/pcie-doe.h b/include/linux/pcie-doe.h
> new file mode 100644
> index 000000000000..794d6f090779
> --- /dev/null
> +++ b/include/linux/pcie-doe.h
> @@ -0,0 +1,85 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Data Object Exchange was added to the PCI spec as an ECN to 5.0.
> + *
> + * Copyright (C) 2021 Huawei
> + *     Jonathan Cameron <Jonathan.Cameron@huawei.com>
> + */
> +
> +#include <linux/completion.h>
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +
> +#ifndef LINUX_PCIE_DOE_H
> +#define LINUX_PCIE_DOE_H
> +
> +struct pcie_doe_prot {
> +	u16 vid;
> +	u8 type;
> +};
> +
> +struct workqueue_struct;
> +
> +enum pcie_doe_state {
> +	DOE_IDLE,
> +	DOE_WAIT_RESP,
> +	DOE_WAIT_ABORT,
> +	DOE_WAIT_ABORT_ON_ERR,
> +};
> +
> +struct pcie_doe_exchange {
> +	u16 vid;
> +	u8 protocol;
> +	u32 *request_pl;
> +	size_t request_pl_sz;
> +	u32 *response_pl;
> +	size_t response_pl_sz;
> +};
> +
> +/**
> + * struct pcie_doe - State to support use of DOE mailbox
> + * @cap: Config space offset to base of DOE capability.
> + * @pdev: PCI device that hosts this DOE.
> + * @abort_c: Completion used for initial abort handling.
> + * @irq: Interrupt used for signaling DOE ready or abort.
> + * @prots: Cache of identifiers for protocols supported.
> + * @num_prots: Size of prots cache.
> + * @h: Used for DOE instance lifetime management.
> + * @wq: Workqueue used to handle state machine and polling / timeouts
> + * @tasks: List of task in flight + pending.
> + * @tasks_lock: Protect the tasks list.
> + * @statemachine: Work item for the DOE statemachine
> + * @state: Current state of this DOE.
> + * @timeout_jiffies: 1 second after GO set.
> + * @busy_retries: Count of retry attempts
> + * @abort: Request a manual abort (e.g. on init)
> + * @dead: Used to mark a DOE for which an ABORT has timed out.
> + *        Further messages will immediately be aborted with error.
> + */
> +struct pcie_doe {
> +	int cap;
> +	struct pci_dev *pdev;
> +	struct completion abort_c;
> +	int irq;
> +	struct pcie_doe_prot *prots;
> +	int num_prots;
> +	struct list_head h;
> +
> +	struct workqueue_struct *wq;
> +	struct list_head tasks;
> +	struct mutex tasks_lock;
> +	struct delayed_work statemachine;
> +	enum pcie_doe_state state;
> +	unsigned long timeout_jiffies;
> +	unsigned int busy_retries;
> +	bool abort;
> +	bool dead;

Slight preference for "unsigned int abort:1" in a struct:
https://lore.kernel.org/r/CA+55aFzKQ6Pj18TB8p4Yr0M4t+S+BsiHH=BJNmn=76-NcjTj-g@mail.gmail.com/

> +};
> +
> +int pcie_doe_register_all(struct pci_dev *pdev);
> +void pcie_doe_unregister_all(struct pci_dev *pdev);
> +struct pcie_doe *pcie_doe_find(struct pci_dev *pdev, u16 vid, u8 type);
> +
> +int pcie_doe_sync(struct pcie_doe *doe, struct pcie_doe_exchange *ex);
> +
> +#endif
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e709ae8235e7..4d8a5fee2cdf 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -730,7 +730,8 @@
>  #define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
>  #define PCI_EXT_CAP_ID_DLF	0x25	/* Data Link Feature */
>  #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
> -#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_PL_16GT
> +#define PCI_EXT_CAP_ID_DOE	0x2E	/* Data Object Exchange */
> +#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_DOE
>  
>  #define PCI_EXT_CAP_DSN_SIZEOF	12
>  #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
> @@ -1092,4 +1093,30 @@
>  #define  PCI_PL_16GT_LE_CTRL_USP_TX_PRESET_MASK		0x000000F0
>  #define  PCI_PL_16GT_LE_CTRL_USP_TX_PRESET_SHIFT	4
>  
> +/* Data Object Exchange */
> +#define PCI_DOE_CAP		0x04	/* DOE Capabilities Register */
> +#define  PCI_DOE_CAP_INT			0x00000001  /* Interrupt Support */
> +#define  PCI_DOE_CAP_IRQ			0x00000ffe  /* Interrupt Message Number */
> +#define PCI_DOE_CTRL		0x08	/* DOE Control Register */
> +#define  PCI_DOE_CTRL_ABORT			0x00000001  /* DOE Abort */
> +#define  PCI_DOE_CTRL_INT_EN			0x00000002  /* DOE Interrupt Enable */
> +#define  PCI_DOE_CTRL_GO			0x80000000  /* DOE Go */
> +#define PCI_DOE_STATUS		0x0C	/* DOE Status Register */
> +#define  PCI_DOE_STATUS_BUSY			0x00000001  /* DOE Busy */
> +#define  PCI_DOE_STATUS_INT_STATUS		0x00000002  /* DOE Interrupt Status */
> +#define  PCI_DOE_STATUS_ERROR			0x00000004  /* DOE Error */
> +#define  PCI_DOE_STATUS_DATA_OBJECT_READY	0x80000000  /* Data Object Ready */
> +#define PCI_DOE_WRITE		0x10	/* DOE Write Data Mailbox Register */
> +#define PCI_DOE_READ		0x14	/* DOE Read Data Mailbox Register */
> +
> +/* DOE Data Object - note not actually registers */
> +#define PCI_DOE_DATA_OBJECT_HEADER_1_VID	0x0000FFFF
> +#define PCI_DOE_DATA_OBJECT_HEADER_1_TYPE	0x00FF0000
> +#define PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH	0x0003FFFF
> +
> +#define PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX	0x000000FF
> +#define PCI_DOE_DATA_OBJECT_DISC_RSP_3_VID	0x0000FFFF
> +#define PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL	0x00FF0000
> +#define PCI_DOE_DATA_OBJECT_DISC_RSP_3_NEXT_INDEX 0xFF000000

This file is a mix, but the consensus is lower-case in hex.  I'd leave
PCI_EXT_CAP_ID_DOE as-is to match the local context and use
lower-case consistently in this hunk.

>  #endif /* LINUX_PCI_REGS_H */
> -- 
> 2.19.1
>