* [PATCH v3 0/4] cxl: Handle background commands
@ 2023-05-23 17:09 Davidlohr Bueso
2023-05-23 17:09 ` [PATCH 1/4] rcuwait: Support timeouts Davidlohr Bueso
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Davidlohr Bueso @ 2023-05-23 17:09 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
Jonathan.Cameron, fan.ni, a.manzanares, dave, linux-cxl,
linux-kernel
Hi,
This is the latest iteration of the synchronous background command handling
work. Currently there are no users, but it is expected that firmware update
and scan media be the first two to use it.
Patches 1-3 are requirements for the actual patch 4.
Changes from v1/v2 (https://lore.kernel.org/linux-cxl/20230502171841.21317-1-dave@stgolabs.net):
o Picked up tags for patches 1 and 2.
o Added more context to the changelog in the rcuwait patch 1. (Dan)
o Added new patch 3 for common request irq functionality. (Dan)
o Misc cleanups in patch 4. (Jonathan)
o Don't clobber the ctrl settings. (Dan)
o Removed bogus warning in irq handler. (Jonathan, Dan)
o Make sleep uninterruptible and clarify in the changelog that
timeouts are in fact unexpected.
Applies against 'fixes' from cxl.git.
Thanks!
Davidlohr Bueso (4):
rcuwait: Support timeouts
cxl/pci: Allocate irq vectors earlier during probe
cxl/pci: Introduce cxl_request_irq()
cxl/mbox: Add background cmd handling machinery
drivers/cxl/core/mbox.c | 3 +-
drivers/cxl/cxl.h | 8 +++
drivers/cxl/cxlmem.h | 7 +++
drivers/cxl/pci.c | 136 ++++++++++++++++++++++++++++++++++------
include/linux/rcuwait.h | 23 ++++++-
5 files changed, 153 insertions(+), 24 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/4] rcuwait: Support timeouts
2023-05-23 17:09 [PATCH v3 0/4] cxl: Handle background commands Davidlohr Bueso
@ 2023-05-23 17:09 ` Davidlohr Bueso
2023-05-23 17:09 ` [PATCH 2/4] cxl/pci: Allocate irq vectors earlier during probe Davidlohr Bueso
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Davidlohr Bueso @ 2023-05-23 17:09 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
Jonathan.Cameron, fan.ni, a.manzanares, dave, linux-cxl,
linux-kernel, Peter Zijlstra
The rcuwait utility provides an efficient and safe single
wait/wake mechanism. It is used in situations where queued
wait is the wrong semantics, and often too bulky. For example,
cases where the wait is already done under a lock.
In the past, rcuwait has been extended to support beyond only
uninterruptible sleep, and similarly, there are users that can
benefit for the addition of timeouts.
As such, tntroduce rcuwait_wait_event_timeout(), with semantics
equivalent to calls for queued wait counterparts.
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
include/linux/rcuwait.h | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/include/linux/rcuwait.h b/include/linux/rcuwait.h
index 8052d34da782..27343424225c 100644
--- a/include/linux/rcuwait.h
+++ b/include/linux/rcuwait.h
@@ -49,9 +49,9 @@ static inline void prepare_to_rcuwait(struct rcuwait *w)
extern void finish_rcuwait(struct rcuwait *w);
-#define rcuwait_wait_event(w, condition, state) \
+#define ___rcuwait_wait_event(w, condition, state, ret, cmd) \
({ \
- int __ret = 0; \
+ long __ret = ret; \
prepare_to_rcuwait(w); \
for (;;) { \
/* \
@@ -67,10 +67,27 @@ extern void finish_rcuwait(struct rcuwait *w);
break; \
} \
\
- schedule(); \
+ cmd; \
} \
finish_rcuwait(w); \
__ret; \
})
+#define rcuwait_wait_event(w, condition, state) \
+ ___rcuwait_wait_event(w, condition, state, 0, schedule())
+
+#define __rcuwait_wait_event_timeout(w, condition, state, timeout) \
+ ___rcuwait_wait_event(w, ___wait_cond_timeout(condition), \
+ state, timeout, \
+ __ret = schedule_timeout(__ret))
+
+#define rcuwait_wait_event_timeout(w, condition, state, timeout) \
+({ \
+ long __ret = timeout; \
+ if (!___wait_cond_timeout(condition)) \
+ __ret = __rcuwait_wait_event_timeout(w, condition, \
+ state, timeout); \
+ __ret; \
+})
+
#endif /* _LINUX_RCUWAIT_H_ */
--
2.40.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/4] cxl/pci: Allocate irq vectors earlier during probe
2023-05-23 17:09 [PATCH v3 0/4] cxl: Handle background commands Davidlohr Bueso
2023-05-23 17:09 ` [PATCH 1/4] rcuwait: Support timeouts Davidlohr Bueso
@ 2023-05-23 17:09 ` Davidlohr Bueso
2023-05-23 17:09 ` [PATCH 3/4] cxl/pci: Introduce cxl_request_irq() Davidlohr Bueso
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Davidlohr Bueso @ 2023-05-23 17:09 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
Jonathan.Cameron, fan.ni, a.manzanares, dave, linux-cxl,
linux-kernel
Move the cxl_alloc_irq_vectors() call further up in the probing
in order to allow for mailbox interrupt usage. No change in
semantics.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
drivers/cxl/pci.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 0872f2233ed0..f2039fe0805d 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -714,6 +714,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
else
dev_warn(&pdev->dev, "Media not active (%d)\n", rc);
+ rc = cxl_alloc_irq_vectors(pdev);
+ if (rc)
+ return rc;
+
rc = cxl_pci_setup_mailbox(cxlds);
if (rc)
return rc;
@@ -738,10 +742,6 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (rc)
return rc;
- rc = cxl_alloc_irq_vectors(pdev);
- if (rc)
- return rc;
-
cxlmd = devm_cxl_add_memdev(cxlds);
if (IS_ERR(cxlmd))
return PTR_ERR(cxlmd);
--
2.40.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] cxl/pci: Introduce cxl_request_irq()
2023-05-23 17:09 [PATCH v3 0/4] cxl: Handle background commands Davidlohr Bueso
2023-05-23 17:09 ` [PATCH 1/4] rcuwait: Support timeouts Davidlohr Bueso
2023-05-23 17:09 ` [PATCH 2/4] cxl/pci: Allocate irq vectors earlier during probe Davidlohr Bueso
@ 2023-05-23 17:09 ` Davidlohr Bueso
2023-05-23 18:43 ` Dan Williams
2023-05-23 17:09 ` [PATCH 4/4] cxl/mbox: Add background cmd handling machinery Davidlohr Bueso
2023-05-23 21:03 ` [PATCH v3 0/4] cxl: Handle background commands Dan Williams
4 siblings, 1 reply; 9+ messages in thread
From: Davidlohr Bueso @ 2023-05-23 17:09 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
Jonathan.Cameron, fan.ni, a.manzanares, dave, linux-cxl,
linux-kernel
Factor out common functionality/semantics for cxl shared interrupts
into a new helper on top of devm_request_irq().
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
drivers/cxl/pci.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index f2039fe0805d..18b8f3ce680c 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -84,6 +84,27 @@ static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
status & CXLMDEV_DEV_FATAL ? " fatal" : "", \
status & CXLMDEV_FW_HALT ? " firmware-halt" : "")
+struct cxl_dev_id {
+ struct cxl_dev_state *cxlds;
+};
+
+static int cxl_request_irq(struct cxl_dev_state *cxlds, int irq,
+ irq_handler_t handler, irq_handler_t thread_fn)
+{
+ struct device *dev = cxlds->dev;
+ struct cxl_dev_id *dev_id;
+
+ /* dev_id must be globally unique and must contain the cxlds */
+ dev_id = devm_kzalloc(dev, sizeof(*dev_id), GFP_KERNEL);
+ if (!dev_id)
+ return -ENOMEM;
+ dev_id->cxlds = cxlds;
+
+ return devm_request_threaded_irq(dev, irq, handler, thread_fn,
+ IRQF_SHARED | IRQF_ONESHOT,
+ NULL, dev_id);
+}
+
/**
* __cxl_pci_mbox_send_cmd() - Execute a mailbox command
* @cxlds: The device state to communicate with.
@@ -469,10 +490,6 @@ static int cxl_alloc_irq_vectors(struct pci_dev *pdev)
return 0;
}
-struct cxl_dev_id {
- struct cxl_dev_state *cxlds;
-};
-
static irqreturn_t cxl_event_thread(int irq, void *id)
{
struct cxl_dev_id *dev_id = id;
@@ -498,28 +515,18 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
{
- struct device *dev = cxlds->dev;
- struct pci_dev *pdev = to_pci_dev(dev);
- struct cxl_dev_id *dev_id;
+ struct pci_dev *pdev = to_pci_dev(cxlds->dev);
int irq;
if (FIELD_GET(CXLDEV_EVENT_INT_MODE_MASK, setting) != CXL_INT_MSI_MSIX)
return -ENXIO;
- /* dev_id must be globally unique and must contain the cxlds */
- dev_id = devm_kzalloc(dev, sizeof(*dev_id), GFP_KERNEL);
- if (!dev_id)
- return -ENOMEM;
- dev_id->cxlds = cxlds;
-
irq = pci_irq_vector(pdev,
FIELD_GET(CXLDEV_EVENT_INT_MSGNUM_MASK, setting));
if (irq < 0)
return irq;
- return devm_request_threaded_irq(dev, irq, NULL, cxl_event_thread,
- IRQF_SHARED | IRQF_ONESHOT, NULL,
- dev_id);
+ return cxl_request_irq(cxlds, irq, NULL, cxl_event_thread);
}
static int cxl_event_get_int_policy(struct cxl_dev_state *cxlds,
--
2.40.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] cxl/mbox: Add background cmd handling machinery
2023-05-23 17:09 [PATCH v3 0/4] cxl: Handle background commands Davidlohr Bueso
` (2 preceding siblings ...)
2023-05-23 17:09 ` [PATCH 3/4] cxl/pci: Introduce cxl_request_irq() Davidlohr Bueso
@ 2023-05-23 17:09 ` Davidlohr Bueso
2023-05-23 21:03 ` [PATCH v3 0/4] cxl: Handle background commands Dan Williams
4 siblings, 0 replies; 9+ messages in thread
From: Davidlohr Bueso @ 2023-05-23 17:09 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
Jonathan.Cameron, fan.ni, a.manzanares, dave, linux-cxl,
linux-kernel
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. Any out of sync
(timeout) between the driver and hardware is just disregarded as
an invalid state until the next successful submission. Such timeouts
are considered a rare occurrence, either a real device problem or a
driver issue that needs to reduce the size of the background operation
to fit the timeout.
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 | 8 ++++
drivers/cxl/cxlmem.h | 7 ++++
drivers/cxl/pci.c | 89 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 106 insertions(+), 1 deletion(-)
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 2c8dc7e2b84d..5993261e3e08 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 f93a28538962..ec69bda93aee 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -176,14 +176,22 @@ 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_BG_CMD_IRQ BIT(6)
+#define CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK GENMASK(10, 7)
#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_BG_CMD_COMMAND_VENDOR_MASK GENMASK_ULL(63, 48)
#define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20
/*
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index a2845a7a69d8..1d8e81c87c6a 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_ms: (input) Time between mailbox background command polling
+ * interval 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_ms;
u16 return_code;
};
@@ -331,6 +337,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 18b8f3ce680c..a78e40e6d0e0 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -105,6 +105,26 @@ static int cxl_request_irq(struct cxl_dev_state *cxlds, int irq,
NULL, dev_id);
}
+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_id *dev_id = id;
+ struct cxl_dev_state *cxlds = dev_id->cxlds;
+
+ /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
+ if (cxl_mbox_background_complete(cxlds))
+ rcuwait_wake_up(&cxlds->mbox_wait);
+
+ return IRQ_HANDLED;
+}
+
/**
* __cxl_pci_mbox_send_cmd() - Execute a mailbox command
* @cxlds: The device state to communicate with.
@@ -198,6 +218,50 @@ 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) {
+ u64 bg_status_reg;
+ int i, timeout = mbox_cmd->poll_interval_ms;
+
+ dev_dbg(dev, "Mailbox background operation (0x%04x) started\n",
+ mbox_cmd->opcode);
+
+ for (i = 0; i < mbox_cmd->poll_count; i++) {
+ if (rcuwait_wait_event_timeout(&cxlds->mbox_wait,
+ cxl_mbox_background_complete(cxlds),
+ TASK_UNINTERRUPTIBLE,
+ msecs_to_jiffies(timeout)) > 0)
+ break;
+ }
+
+ if (!cxl_mbox_background_complete(cxlds)) {
+ dev_err(dev, "timeout waiting for background (%d ms)\n",
+ timeout * mbox_cmd->poll_count);
+ 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));
@@ -292,6 +356,31 @@ 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) {
+ u32 ctrl;
+ 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 (cxl_request_irq(cxlds, irq, cxl_pci_mbox_irq, NULL))
+ goto mbox_poll;
+
+ /* enable background command mbox irq support */
+ ctrl = readl(cxlds->regs.mbox + CXLDEV_MBOX_CTRL_OFFSET);
+ ctrl |= CXLDEV_MBOX_CTRL_BG_CMD_IRQ;
+ writel(ctrl, cxlds->regs.mbox + CXLDEV_MBOX_CTRL_OFFSET);
+
+ return 0;
+ }
+
+mbox_poll:
+ dev_dbg(cxlds->dev, "Mailbox interrupts are unsupported");
return 0;
}
--
2.40.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 3/4] cxl/pci: Introduce cxl_request_irq()
2023-05-23 17:09 ` [PATCH 3/4] cxl/pci: Introduce cxl_request_irq() Davidlohr Bueso
@ 2023-05-23 18:43 ` Dan Williams
0 siblings, 0 replies; 9+ messages in thread
From: Dan Williams @ 2023-05-23 18:43 UTC (permalink / raw)
To: Davidlohr Bueso, dan.j.williams
Cc: dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
Jonathan.Cameron, fan.ni, a.manzanares, dave, linux-cxl,
linux-kernel
Davidlohr Bueso wrote:
> Factor out common functionality/semantics for cxl shared interrupts
> into a new helper on top of devm_request_irq().
>
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
> drivers/cxl/pci.c | 39 +++++++++++++++++++++++----------------
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index f2039fe0805d..18b8f3ce680c 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -84,6 +84,27 @@ static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
> status & CXLMDEV_DEV_FATAL ? " fatal" : "", \
> status & CXLMDEV_FW_HALT ? " firmware-halt" : "")
>
> +struct cxl_dev_id {
> + struct cxl_dev_state *cxlds;
> +};
> +
> +static int cxl_request_irq(struct cxl_dev_state *cxlds, int irq,
> + irq_handler_t handler, irq_handler_t thread_fn)
> +{
> + struct device *dev = cxlds->dev;
> + struct cxl_dev_id *dev_id;
> +
> + /* dev_id must be globally unique and must contain the cxlds */
> + dev_id = devm_kzalloc(dev, sizeof(*dev_id), GFP_KERNEL);
> + if (!dev_id)
> + return -ENOMEM;
> + dev_id->cxlds = cxlds;
> +
> + return devm_request_threaded_irq(dev, irq, handler, thread_fn,
> + IRQF_SHARED | IRQF_ONESHOT,
> + NULL, dev_id);
I was going to say drop the IRQF_ONESHOT in the case where @thread_fn is
NULL, but I could not convince myself that was safe. I assume that
pci_request_irq() gets away with that because most drivers are not
mixing combinations of @handler and @thread_fn being set to NULL.
So, with that, this looks good to me.
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v3 0/4] cxl: Handle background commands
2023-05-23 17:09 [PATCH v3 0/4] cxl: Handle background commands Davidlohr Bueso
` (3 preceding siblings ...)
2023-05-23 17:09 ` [PATCH 4/4] cxl/mbox: Add background cmd handling machinery Davidlohr Bueso
@ 2023-05-23 21:03 ` Dan Williams
2023-05-23 21:59 ` Davidlohr Bueso
4 siblings, 1 reply; 9+ messages in thread
From: Dan Williams @ 2023-05-23 21:03 UTC (permalink / raw)
To: Davidlohr Bueso, dan.j.williams
Cc: dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
Jonathan.Cameron, fan.ni, a.manzanares, dave, linux-cxl,
linux-kernel
Davidlohr Bueso wrote:
> Hi,
>
> This is the latest iteration of the synchronous background command handling
> work. Currently there are no users, but it is expected that firmware update
> and scan media be the first two to use it.
>
> Patches 1-3 are requirements for the actual patch 4.
>
> Changes from v1/v2 (https://lore.kernel.org/linux-cxl/20230502171841.21317-1-dave@stgolabs.net):
> o Picked up tags for patches 1 and 2.
> o Added more context to the changelog in the rcuwait patch 1. (Dan)
> o Added new patch 3 for common request irq functionality. (Dan)
> o Misc cleanups in patch 4. (Jonathan)
> o Don't clobber the ctrl settings. (Dan)
> o Removed bogus warning in irq handler. (Jonathan, Dan)
> o Make sleep uninterruptible and clarify in the changelog that
> timeouts are in fact unexpected.
>
> Applies against 'fixes' from cxl.git.
Like Linus I want to see the conflicts. Whenever possible base on latest
Linus tag like v6.4-rc3. In this case I will add this to a
for-6.5/cxl-background topic branch based on 'fixes' for now, but once
'fixes' hits v6.4-rc4 I will move the topic baseline just to have it not
be based on a random point in the history.
This also encourages me to be more proactive about getting fixes
upstream so that the next Linus tag can be used for development.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/4] cxl: Handle background commands
2023-05-23 21:03 ` [PATCH v3 0/4] cxl: Handle background commands Dan Williams
@ 2023-05-23 21:59 ` Davidlohr Bueso
2023-05-24 4:19 ` Dan Williams
0 siblings, 1 reply; 9+ messages in thread
From: Davidlohr Bueso @ 2023-05-23 21:59 UTC (permalink / raw)
To: Dan Williams
Cc: dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
Jonathan.Cameron, fan.ni, a.manzanares, linux-cxl, linux-kernel
On Tue, 23 May 2023, Dan Williams wrote:
>Like Linus I want to see the conflicts. Whenever possible base on latest
>Linus tag like v6.4-rc3.
So I'm clear, do you want me to resend this series based on -rc3?
Frankly I'm always confused as to what is the correct/preferred branch to
do development on.
>In this case I will add this to a
>for-6.5/cxl-background topic branch based on 'fixes' for now, but once
>'fixes' hits v6.4-rc4 I will move the topic baseline just to have it not
>be based on a random point in the history.
Ok, thanks for picking this up.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/4] cxl: Handle background commands
2023-05-23 21:59 ` Davidlohr Bueso
@ 2023-05-24 4:19 ` Dan Williams
0 siblings, 0 replies; 9+ messages in thread
From: Dan Williams @ 2023-05-24 4:19 UTC (permalink / raw)
To: Davidlohr Bueso, Dan Williams
Cc: dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
Jonathan.Cameron, fan.ni, a.manzanares, linux-cxl, linux-kernel
Davidlohr Bueso wrote:
> On Tue, 23 May 2023, Dan Williams wrote:
>
> >Like Linus I want to see the conflicts. Whenever possible base on latest
> >Linus tag like v6.4-rc3.
>
> So I'm clear, do you want me to resend this series based on -rc3?
No, not this time but for next time my preference is "when in doubt, pick
a mainline tag".
> Frankly I'm always confused as to what is the correct/preferred branch to
> do development on.
My plan is to do better about publishing topic branches for
work-in-progress items like this. So now we have for-6.5/cxl-background
for this common baseline for sanitization and firmware update. At least
one of those will need to be queued before this topic moves forward
since the new infrastructure needs a user.
The problem with picking 'next' or to a lesser extent 'fixes' as a
development baseline is that it ties unrelated topics together. If
something happens and we decided a topic neeeds to drop out of 'next' or
be rebased, that's much easier to do if random topics have not grown
implicit silent depenendencies over time.
Like in this case these patches do not apply to v6.4-rc3 because they
collide with the movement of cxl_await_media_ready() to cxl_pci.
However, nothing about that media-ready fix is needed for background
command support so the patches can just depend on a mainline tag.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-05-24 4:19 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-23 17:09 [PATCH v3 0/4] cxl: Handle background commands Davidlohr Bueso
2023-05-23 17:09 ` [PATCH 1/4] rcuwait: Support timeouts Davidlohr Bueso
2023-05-23 17:09 ` [PATCH 2/4] cxl/pci: Allocate irq vectors earlier during probe Davidlohr Bueso
2023-05-23 17:09 ` [PATCH 3/4] cxl/pci: Introduce cxl_request_irq() Davidlohr Bueso
2023-05-23 18:43 ` Dan Williams
2023-05-23 17:09 ` [PATCH 4/4] cxl/mbox: Add background cmd handling machinery Davidlohr Bueso
2023-05-23 21:03 ` [PATCH v3 0/4] cxl: Handle background commands Dan Williams
2023-05-23 21:59 ` Davidlohr Bueso
2023-05-24 4:19 ` Dan Williams
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®