[PATCH 3/3] cxl/mbox: Add background cmd handling machinery

[PATCH 3/3] cxl/mbox: Add background cmd handling machinery

[Davidlohr Bueso]

This adds support for handling background operations, as defined in
the CXL 3.0 spec. Commands that can take too long (over ~2 seconds)
can run in the background asynchronously (to the hardware).

The driver will deal with such commands synchronously, blocking all
other incoming commands for a specified period of time, allowing
time-slicing the command such that the caller can send incremental
requests to avoid monopolizing the driver/device. This approach
makes the code simpler, where any out of sync (timeout) between the
driver and hardware is just disregarded as an invalid state until
the next successful submission.

On devices where mbox interrupts are supported, this will still use
a poller that will wakeup in the specified wait intervals. The irq
handler will simply awake the blocked cmd, which is also safe vs a
task that is either waking (timing out) or already awoken. Similarly
any irq setup error during the probing falls back to polling, thus
avoids unnecessarily erroring out.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/cxl/core/mbox.c |   3 +-
 drivers/cxl/cxl.h       |   7 +++
 drivers/cxl/cxlmem.h    |   7 +++
 drivers/cxl/pci.c       | 102 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 23b9ff920d7e..7345ed4118b0 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -220,7 +220,8 @@ int cxl_internal_send_cmd(struct cxl_dev_state *cxlds,
 	if (rc)
 		return rc;
 
-	if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS)
+	if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS &&
+	    mbox_cmd->return_code != CXL_MBOX_CMD_RC_BACKGROUND)
 		return cxl_mbox_cmd_rc2errno(mbox_cmd);
 
 	if (!out_size)
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 044a92d9813e..72731a896f58 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -176,14 +176,21 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)
 /* CXL 2.0 8.2.8.4 Mailbox Registers */
 #define CXLDEV_MBOX_CAPS_OFFSET 0x00
 #define   CXLDEV_MBOX_CAP_PAYLOAD_SIZE_MASK GENMASK(4, 0)
+#define   CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK GENMASK(10, 7)
+#define   CXLDEV_MBOX_CAP_BG_CMD_IRQ BIT(6)
 #define CXLDEV_MBOX_CTRL_OFFSET 0x04
 #define   CXLDEV_MBOX_CTRL_DOORBELL BIT(0)
+#define   CXLDEV_MBOX_CTRL_BG_CMD_IRQ BIT(2)
 #define CXLDEV_MBOX_CMD_OFFSET 0x08
 #define   CXLDEV_MBOX_CMD_COMMAND_OPCODE_MASK GENMASK_ULL(15, 0)
 #define   CXLDEV_MBOX_CMD_PAYLOAD_LENGTH_MASK GENMASK_ULL(36, 16)
 #define CXLDEV_MBOX_STATUS_OFFSET 0x10
+#define   CXLDEV_MBOX_STATUS_BG_CMD BIT(0)
 #define   CXLDEV_MBOX_STATUS_RET_CODE_MASK GENMASK_ULL(47, 32)
 #define CXLDEV_MBOX_BG_CMD_STATUS_OFFSET 0x18
+#define   CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK GENMASK_ULL(15, 0)
+#define   CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK GENMASK_ULL(22, 16)
+#define   CXLDEV_MBOX_BG_CMD_COMMAND_RC_MASK GENMASK_ULL(47, 32)
 #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20
 
 /*
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index db12b6313afb..d2f751d6583c 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -5,6 +5,7 @@
 #include <uapi/linux/cxl_mem.h>
 #include <linux/cdev.h>
 #include <linux/uuid.h>
+#include <linux/rcuwait.h>
 #include "cxl.h"
 
 /* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */
@@ -108,6 +109,9 @@ static inline struct cxl_ep *cxl_ep_load(struct cxl_port *port,
  *            variable sized output commands, it tells the exact number of bytes
  *            written.
  * @min_out: (input) internal command output payload size validation
+ * @poll_count: (input)  Number of timeouts to attempt.
+ * @poll_interval: (input) Number of ms between mailbox background command
+ *                 polling intervals timeouts.
  * @return_code: (output) Error code returned from hardware.
  *
  * This is the primary mechanism used to send commands to the hardware.
@@ -123,6 +127,8 @@ struct cxl_mbox_cmd {
 	size_t size_in;
 	size_t size_out;
 	size_t min_out;
+	int poll_count;
+	int poll_interval;
 	u16 return_code;
 };
 
@@ -329,6 +335,7 @@ struct cxl_dev_state {
 	struct cxl_event_state event;
 	struct cxl_poison_state poison;
 
+	struct rcuwait mbox_wait;
 	int (*mbox_send)(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd);
 };
 
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 8bdf58c0c643..5ca1423a4d92 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -51,6 +51,7 @@ static unsigned short mbox_ready_timeout = 60;
 module_param(mbox_ready_timeout, ushort, 0644);
 MODULE_PARM_DESC(mbox_ready_timeout, "seconds to wait for mailbox ready");
 
+
 static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
 {
 	const unsigned long start = jiffies;
@@ -84,6 +85,33 @@ static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
 			    status & CXLMDEV_DEV_FATAL ? " fatal" : "",        \
 			    status & CXLMDEV_FW_HALT ? " firmware-halt" : "")
 
+static bool cxl_mbox_background_complete(struct cxl_dev_state *cxlds)
+{
+	u64 reg;
+
+	reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
+	return FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK, reg) == 100;
+}
+
+static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
+{
+	struct cxl_dev_state *cxlds = id;
+
+	/* spurious or raced with hw? */
+	if (unlikely(!cxl_mbox_background_complete(cxlds))) {
+		struct pci_dev *pdev = to_pci_dev(cxlds->dev);
+
+		dev_warn(&pdev->dev,
+			 "Mailbox background operation IRQ but incomplete\n");
+		goto done;
+	}
+
+	/* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
+	rcuwait_wake_up(&cxlds->mbox_wait);
+done:
+	return IRQ_HANDLED;
+}
+
 /**
  * __cxl_pci_mbox_send_cmd() - Execute a mailbox command
  * @cxlds: The device state to communicate with.
@@ -177,6 +205,57 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
 	mbox_cmd->return_code =
 		FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg);
 
+	/*
+	 * Handle the background command in a synchronous manner.
+	 *
+	 * All other mailbox commands will serialize/queue on the mbox_mutex,
+	 * which we currently hold. Furthermore this also guarantees that
+	 * cxl_mbox_background_complete() checks are safe amongst each other,
+	 * in that no new bg operation can occur in between.
+	 *
+	 * Background operations are timesliced in accordance with the nature
+	 * of the command. In the event of timeout, the mailbox state is
+	 * indeterminate until the next successful command submission and the
+	 * driver can get back in sync with the hardware state.
+	 */
+	if (mbox_cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {
+		int i, ret;
+		u64 bg_status_reg;
+		int timeout = mbox_cmd->poll_interval;
+
+		dev_dbg(dev, "Mailbox background operation (0x%04x) started\n",
+			mbox_cmd->opcode);
+
+		for (i = 0; i < mbox_cmd->poll_count; i++) {
+			ret = rcuwait_wait_event_timeout(&cxlds->mbox_wait,
+					TASK_INTERRUPTIBLE,
+					cxl_mbox_background_complete(cxlds),
+					msecs_to_jiffies(timeout));
+			if (ret > 0)
+				break;
+			if (ret < 0) /* interrupted by a signal */
+				return ret;
+		}
+
+		if (!cxl_mbox_background_complete(cxlds)) {
+			u64 md_status =
+				readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);
+
+			cxl_cmd_err(cxlds->dev, mbox_cmd, md_status,
+				    "background timeout");
+			return -ETIMEDOUT;
+		}
+
+		bg_status_reg = readq(cxlds->regs.mbox +
+				      CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
+		mbox_cmd->return_code =
+			FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_RC_MASK,
+				  bg_status_reg);
+		dev_dbg(dev,
+			"Mailbox background operation (0x%04x) completed\n",
+			mbox_cmd->opcode);
+	}
+
 	if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS) {
 		dev_dbg(dev, "Mailbox operation had an error: %s\n",
 			cxl_mbox_cmd_rc2str(mbox_cmd));
@@ -271,6 +350,29 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
 	dev_dbg(cxlds->dev, "Mailbox payload sized %zu",
 		cxlds->payload_size);
 
+	rcuwait_init(&cxlds->mbox_wait);
+	if (cap & CXLDEV_MBOX_CAP_BG_CMD_IRQ) {
+		int irq, msgnum;
+		struct pci_dev *pdev = to_pci_dev(cxlds->dev);
+
+		msgnum = FIELD_GET(CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK, cap);
+		irq = pci_irq_vector(pdev, msgnum);
+		if (irq < 0)
+			goto mbox_poll;
+
+		if (devm_request_irq(cxlds->dev, irq, cxl_pci_mbox_irq,
+				     IRQF_SHARED, NULL, cxlds))
+			goto mbox_poll;
+
+		/* only enable background cmd mbox irq support */
+		writel(CXLDEV_MBOX_CTRL_BG_CMD_IRQ,
+		       cxlds->regs.mbox + CXLDEV_MBOX_CTRL_OFFSET);
+
+		return 0;
+	}
+
+mbox_poll:
+	dev_dbg(cxlds->dev, "Mailbox interrupts are unsupported");
 	return 0;
 }
 
-- 
2.40.1

Re: [PATCH 3/3] cxl/mbox: Add background cmd handling machinery

[Davidlohr Bueso]

On Tue, 02 May 2023, Davidlohr Bueso wrote:

>+	/*
>+	 * Handle the background command in a synchronous manner.
>+	 *
>+	 * All other mailbox commands will serialize/queue on the mbox_mutex,
>+	 * which we currently hold. Furthermore this also guarantees that
>+	 * cxl_mbox_background_complete() checks are safe amongst each other,
>+	 * in that no new bg operation can occur in between.
>+	 *
>+	 * Background operations are timesliced in accordance with the nature
>+	 * of the command. In the event of timeout, the mailbox state is
>+	 * indeterminate until the next successful command submission and the
>+	 * driver can get back in sync with the hardware state.
>+	 */
>+	if (mbox_cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {
>+		int i, ret;

bleh this "ret" really wants to be a long.

Re: [PATCH 3/3] cxl/mbox: Add background cmd handling machinery

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230502171841.21317-4-dave@stgolabs.net>
References: <20230502171841.21317-4-dave@stgolabs.net>
TO: Davidlohr Bueso <dave@stgolabs.net>

Hi Davidlohr,

kernel test robot noticed the following build warnings:

[auto build test WARNING on cxl/next]
[also build test WARNING on cxl/pending linus/master v6.3 next-20230428]
[cannot apply to paulmck-rcu/dev]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Davidlohr-Bueso/rcuwait-Support-timeouts/20230503-015128
base:   https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git next
patch link:    https://lore.kernel.org/r/20230502171841.21317-4-dave%40stgolabs.net
patch subject: [PATCH 3/3] cxl/mbox: Add background cmd handling machinery
:::::: branch date: 19 hours ago
:::::: commit date: 19 hours ago
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20230503/202305032140.JWW3VebM-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202305032140.JWW3VebM-lkp@intel.com/

smatch warnings:
drivers/cxl/pci.c:230 __cxl_pci_mbox_send_cmd() warn: maybe use && instead of &

vim +230 drivers/cxl/pci.c

914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  114  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  115  /**
ed97afb53365cd drivers/cxl/pci.c Ben Widawsky    2021-09-13  116   * __cxl_pci_mbox_send_cmd() - Execute a mailbox command
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  117   * @cxlds: The device state to communicate with.
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  118   * @mbox_cmd: Command to send to the memory device.
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  119   *
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  120   * Context: Any context. Expects mbox_mutex to be held.
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  121   * Return: -ETIMEDOUT if timeout occurred waiting for completion. 0 on success.
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  122   *         Caller should check the return code in @mbox_cmd to make sure it
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  123   *         succeeded.
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  124   *
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  125   * This is a generic form of the CXL mailbox send command thus only using the
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  126   * registers defined by the mailbox capability ID - CXL 2.0 8.2.8.4. Memory
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  127   * devices, and perhaps other types of CXL devices may have further information
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  128   * available upon error conditions. Driver facilities wishing to send mailbox
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  129   * commands should use the wrapper command.
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  130   *
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  131   * The CXL spec allows for up to two mailboxes. The intention is for the primary
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  132   * mailbox to be OS controlled and the secondary mailbox to be used by system
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  133   * firmware. This allows the OS and firmware to communicate with the device and
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  134   * not need to coordinate with each other. The driver only uses the primary
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  135   * mailbox.
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  136   */
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  137  static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
b64955a9292934 drivers/cxl/pci.c Dan Williams    2021-09-08  138  				   struct cxl_mbox_cmd *mbox_cmd)
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  139  {
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  140  	void __iomem *payload = cxlds->regs.mbox + CXLDEV_MBOX_PAYLOAD_OFFSET;
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  141  	struct device *dev = cxlds->dev;
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  142  	u64 cmd_reg, status_reg;
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  143  	size_t out_len;
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  144  	int rc;
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  145  
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  146  	lockdep_assert_held(&cxlds->mbox_mutex);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  147  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  148  	/*
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  149  	 * Here are the steps from 8.2.8.4 of the CXL 2.0 spec.
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  150  	 *   1. Caller reads MB Control Register to verify doorbell is clear
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  151  	 *   2. Caller writes Command Register
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  152  	 *   3. Caller writes Command Payload Registers if input payload is non-empty
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  153  	 *   4. Caller writes MB Control Register to set doorbell
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  154  	 *   5. Caller either polls for doorbell to be clear or waits for interrupt if configured
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  155  	 *   6. Caller reads MB Status Register to fetch Return code
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  156  	 *   7. If command successful, Caller reads Command Register to get Payload Length
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  157  	 *   8. If output payload is non-empty, host reads Command Payload Registers
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  158  	 *
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  159  	 * Hardware is free to do whatever it wants before the doorbell is rung,
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  160  	 * and isn't allowed to change anything after it clears the doorbell. As
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  161  	 * such, steps 2 and 3 can happen in any order, and steps 6, 7, 8 can
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  162  	 * also happen in any order (though some orders might not make sense).
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  163  	 */
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  164  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  165  	/* #1 */
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  166  	if (cxl_doorbell_busy(cxlds)) {
4f195ee73ade1a drivers/cxl/pci.c Dan Williams    2022-01-23  167  		u64 md_status =
4f195ee73ade1a drivers/cxl/pci.c Dan Williams    2022-01-23  168  			readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);
4f195ee73ade1a drivers/cxl/pci.c Dan Williams    2022-01-23  169  
4f195ee73ade1a drivers/cxl/pci.c Dan Williams    2022-01-23  170  		cxl_cmd_err(cxlds->dev, mbox_cmd, md_status,
4f195ee73ade1a drivers/cxl/pci.c Dan Williams    2022-01-23  171  			    "mailbox queue busy");
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  172  		return -EBUSY;
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  173  	}
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  174  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  175  	cmd_reg = FIELD_PREP(CXLDEV_MBOX_CMD_COMMAND_OPCODE_MASK,
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  176  			     mbox_cmd->opcode);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  177  	if (mbox_cmd->size_in) {
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  178  		if (WARN_ON(!mbox_cmd->payload_in))
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  179  			return -EINVAL;
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  180  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  181  		cmd_reg |= FIELD_PREP(CXLDEV_MBOX_CMD_PAYLOAD_LENGTH_MASK,
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  182  				      mbox_cmd->size_in);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  183  		memcpy_toio(payload, mbox_cmd->payload_in, mbox_cmd->size_in);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  184  	}
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  185  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  186  	/* #2, #3 */
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  187  	writeq(cmd_reg, cxlds->regs.mbox + CXLDEV_MBOX_CMD_OFFSET);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  188  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  189  	/* #4 */
852db33c6c180a drivers/cxl/pci.c Robert Richter  2023-01-03  190  	dev_dbg(dev, "Sending command: 0x%04x\n", mbox_cmd->opcode);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  191  	writel(CXLDEV_MBOX_CTRL_DOORBELL,
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  192  	       cxlds->regs.mbox + CXLDEV_MBOX_CTRL_OFFSET);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  193  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  194  	/* #5 */
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  195  	rc = cxl_pci_mbox_wait_for_doorbell(cxlds);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  196  	if (rc == -ETIMEDOUT) {
4f195ee73ade1a drivers/cxl/pci.c Dan Williams    2022-01-23  197  		u64 md_status = readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);
4f195ee73ade1a drivers/cxl/pci.c Dan Williams    2022-01-23  198  
4f195ee73ade1a drivers/cxl/pci.c Dan Williams    2022-01-23  199  		cxl_cmd_err(cxlds->dev, mbox_cmd, md_status, "mailbox timeout");
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  200  		return rc;
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  201  	}
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  202  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  203  	/* #6 */
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  204  	status_reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_STATUS_OFFSET);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  205  	mbox_cmd->return_code =
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  206  		FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  207  
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  208  	/*
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  209  	 * Handle the background command in a synchronous manner.
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  210  	 *
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  211  	 * All other mailbox commands will serialize/queue on the mbox_mutex,
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  212  	 * which we currently hold. Furthermore this also guarantees that
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  213  	 * cxl_mbox_background_complete() checks are safe amongst each other,
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  214  	 * in that no new bg operation can occur in between.
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  215  	 *
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  216  	 * Background operations are timesliced in accordance with the nature
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  217  	 * of the command. In the event of timeout, the mailbox state is
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  218  	 * indeterminate until the next successful command submission and the
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  219  	 * driver can get back in sync with the hardware state.
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  220  	 */
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  221  	if (mbox_cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  222  		int i, ret;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  223  		u64 bg_status_reg;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  224  		int timeout = mbox_cmd->poll_interval;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  225  
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  226  		dev_dbg(dev, "Mailbox background operation (0x%04x) started\n",
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  227  			mbox_cmd->opcode);
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  228  
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  229  		for (i = 0; i < mbox_cmd->poll_count; i++) {
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02 @230  			ret = rcuwait_wait_event_timeout(&cxlds->mbox_wait,
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  231  					TASK_INTERRUPTIBLE,
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  232  					cxl_mbox_background_complete(cxlds),
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  233  					msecs_to_jiffies(timeout));
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  234  			if (ret > 0)
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  235  				break;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  236  			if (ret < 0) /* interrupted by a signal */
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  237  				return ret;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  238  		}
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  239  
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  240  		if (!cxl_mbox_background_complete(cxlds)) {
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  241  			u64 md_status =
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  242  				readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  243  
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  244  			cxl_cmd_err(cxlds->dev, mbox_cmd, md_status,
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  245  				    "background timeout");
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  246  			return -ETIMEDOUT;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  247  		}
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  248  
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  249  		bg_status_reg = readq(cxlds->regs.mbox +
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  250  				      CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  251  		mbox_cmd->return_code =
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  252  			FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_RC_MASK,
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  253  				  bg_status_reg);
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  254  		dev_dbg(dev,
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  255  			"Mailbox background operation (0x%04x) completed\n",
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  256  			mbox_cmd->opcode);
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  257  	}
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  258  
92fcc1abab095d drivers/cxl/pci.c Davidlohr Bueso 2022-04-03  259  	if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS) {
c43e036d6f861f drivers/cxl/pci.c Davidlohr Bueso 2022-04-03  260  		dev_dbg(dev, "Mailbox operation had an error: %s\n",
c43e036d6f861f drivers/cxl/pci.c Davidlohr Bueso 2022-04-03  261  			cxl_mbox_cmd_rc2str(mbox_cmd));
cbe83a2052682c drivers/cxl/pci.c Davidlohr Bueso 2022-04-03  262  		return 0; /* completed but caller must check return_code */
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  263  	}
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  264  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  265  	/* #7 */
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  266  	cmd_reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_CMD_OFFSET);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  267  	out_len = FIELD_GET(CXLDEV_MBOX_CMD_PAYLOAD_LENGTH_MASK, cmd_reg);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  268  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  269  	/* #8 */
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  270  	if (out_len && mbox_cmd->payload_out) {
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  271  		/*
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  272  		 * Sanitize the copy. If hardware misbehaves, out_len per the
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  273  		 * spec can actually be greater than the max allowed size (21
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  274  		 * bits available but spec defined 1M max). The caller also may
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  275  		 * have requested less data than the hardware supplied even
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  276  		 * within spec.
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  277  		 */
5e2411ae807161 drivers/cxl/pci.c Ira Weiny       2021-11-02  278  		size_t n = min3(mbox_cmd->size_out, cxlds->payload_size, out_len);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  279  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  280  		memcpy_fromio(mbox_cmd->payload_out, payload, n);
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  281  		mbox_cmd->size_out = n;
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  282  	} else {
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  283  		mbox_cmd->size_out = 0;
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  284  	}
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  285  
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  286  	return 0;
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  287  }
8adaf747c9f0b4 drivers/cxl/mem.c Ben Widawsky    2021-02-16  288  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 3/3] cxl/mbox: Add background cmd handling machinery

[Dan Carpenter]

Hi Davidlohr,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Davidlohr-Bueso/rcuwait-Support-timeouts/20230503-015128
base:   https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git next
patch link:    https://lore.kernel.org/r/20230502171841.21317-4-dave%40stgolabs.net
patch subject: [PATCH 3/3] cxl/mbox: Add background cmd handling machinery
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20230503/202305032140.JWW3VebM-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202305032140.JWW3VebM-lkp@intel.com/

smatch warnings:
drivers/cxl/pci.c:230 __cxl_pci_mbox_send_cmd() warn: maybe use && instead of &

vim +230 drivers/cxl/pci.c

914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  221  	if (mbox_cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  222  		int i, ret;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  223  		u64 bg_status_reg;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  224  		int timeout = mbox_cmd->poll_interval;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  225  
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  226  		dev_dbg(dev, "Mailbox background operation (0x%04x) started\n",
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  227  			mbox_cmd->opcode);
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  228  
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  229  		for (i = 0; i < mbox_cmd->poll_count; i++) {
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02 @230  			ret = rcuwait_wait_event_timeout(&cxlds->mbox_wait,
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  231  					TASK_INTERRUPTIBLE,
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  232  					cxl_mbox_background_complete(cxlds),
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  233  					msecs_to_jiffies(timeout));

These TASK_INTERRUPTIBLE and cxl_mbox_background_complete() arguments
are swapped.  The condition is supposed the second argument.

914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  234  			if (ret > 0)
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  235  				break;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  236  			if (ret < 0) /* interrupted by a signal */
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  237  				return ret;
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  238  		}
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  239  
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  240  		if (!cxl_mbox_background_complete(cxlds)) {
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  241  			u64 md_status =
914e64b06cee69 drivers/cxl/pci.c Davidlohr Bueso 2023-05-02  242  				readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests