cxl/pci: Cleanup 'sanitize' to always poll

In preparation for fixing the init/teardown of the 'sanitize' workqueue
and sysfs notification mechanism, arrange for cxl_mbox_sanitize_work()
to be the single location where the sysfs attribute is notified. With
that change there is no distinction between polled mode and interrupt
mode. All the interrupt does is accelerate the polling interval.

The change to check for "mds->security.sanitize_node" under the lock is
there to ensure that the interrupt, the work routine and the
setup/teardown code can all have a consistent view of the registered
notifier and the workqueue state. I.e. the expectation is that the
interrupt is live past the point that the sanitize sysfs attribute is
published, and it may race teardown, so it must be consulted under a
lock. Given that new locking requirement, cxl_pci_mbox_irq() is moved
from hard to thread irq context.

Lastly, some opportunistic replacements of
"queue_delayed_work(system_wq, ...)", which is just open coded
schedule_delayed_work(), are included.

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 14b547c07f547602fd243a4ca1bd0097c751948e..2a7a07f6d16526655c4b7bac3da7def29f57dcd4 100644 (file)
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -561,8 +561,7 @@ static void cxl_memdev_security_shutdown(struct device *dev)
        struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
        struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
 
-       if (mds->security.poll)
-               cancel_delayed_work_sync(&mds->security.poll_dwork);
+       cancel_delayed_work_sync(&mds->security.poll_dwork);
 }
 
 static void cxl_memdev_shutdown(struct device *dev)

diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 706f8a6d1ef43c78dfdebd62cd5b55347186fdbb..55f00ad17a772a51a22279bbab65658ab12e2b55 100644 (file)
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -360,7 +360,6 @@ struct cxl_fw_state {
  *
  * @state: state of last security operation
  * @enabled_cmds: All security commands enabled in the CEL
- * @poll: polling for sanitization is enabled, device has no mbox irq support
  * @poll_tmo_secs: polling timeout
  * @poll_dwork: polling work item
  * @sanitize_node: sanitation sysfs file to notify
@@ -368,7 +367,6 @@ struct cxl_fw_state {
 struct cxl_security_state {
        unsigned long state;
        DECLARE_BITMAP(enabled_cmds, CXL_SEC_ENABLED_MAX);
-       bool poll;
        int poll_tmo_secs;
        struct delayed_work poll_dwork;
        struct kernfs_node *sanitize_node;

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index aa1b3dd9e64c4f4f3be9891fb1f9ba04246715ac..49d9b2ef5c5c0b24718ff4daaabfd44270a8bf29 100644 (file)
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -128,10 +128,10 @@ static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
        reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
        opcode = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg);
        if (opcode == CXL_MBOX_OP_SANITIZE) {
+               mutex_lock(&mds->mbox_mutex);
                if (mds->security.sanitize_node)
-                       sysfs_notify_dirent(mds->security.sanitize_node);
-
-               dev_dbg(cxlds->dev, "Sanitization operation ended\n");
+                       mod_delayed_work(system_wq, &mds->security.poll_dwork, 0);
+               mutex_unlock(&mds->mbox_mutex);
        } else {
                /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
                rcuwait_wake_up(&mds->mbox_wait);
@@ -160,8 +160,7 @@ static void cxl_mbox_sanitize_work(struct work_struct *work)
                int timeout = mds->security.poll_tmo_secs + 10;
 
                mds->security.poll_tmo_secs = min(15 * 60, timeout);
-               queue_delayed_work(system_wq, &mds->security.poll_dwork,
-                                  timeout * HZ);
+               schedule_delayed_work(&mds->security.poll_dwork, timeout * HZ);
        }
        mutex_unlock(&mds->mbox_mutex);
 }
@@ -293,15 +292,11 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_memdev_state *mds,
                 * and allow userspace to poll(2) for completion.
                 */
                if (mbox_cmd->opcode == CXL_MBOX_OP_SANITIZE) {
-                       if (mds->security.poll) {
-                               /* give first timeout a second */
-                               timeout = 1;
-                               mds->security.poll_tmo_secs = timeout;
-                               queue_delayed_work(system_wq,
-                                                  &mds->security.poll_dwork,
-                                                  timeout * HZ);
-                       }
-
+                       /* give first timeout a second */
+                       timeout = 1;
+                       mds->security.poll_tmo_secs = timeout;
+                       schedule_delayed_work(&mds->security.poll_dwork,
+                                             timeout * HZ);
                        dev_dbg(dev, "Sanitization operation started\n");
                        goto success;
                }
@@ -384,7 +379,9 @@ static int cxl_pci_setup_mailbox(struct cxl_memdev_state *mds)
        const int cap = readl(cxlds->regs.mbox + CXLDEV_MBOX_CAPS_OFFSET);
        struct device *dev = cxlds->dev;
        unsigned long timeout;
+       int irq, msgnum;
        u64 md_status;
+       u32 ctrl;
 
        timeout = jiffies + mbox_ready_timeout * HZ;
        do {
@@ -432,33 +429,26 @@ static int cxl_pci_setup_mailbox(struct cxl_memdev_state *mds)
        dev_dbg(dev, "Mailbox payload sized %zu", mds->payload_size);
 
        rcuwait_init(&mds->mbox_wait);
+       INIT_DELAYED_WORK(&mds->security.poll_dwork, cxl_mbox_sanitize_work);
 
-       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;
+       /* background command interrupts are optional */
+       if (!(cap & CXLDEV_MBOX_CAP_BG_CMD_IRQ))
+               return 0;
 
-               /* 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);
+       msgnum = FIELD_GET(CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK, cap);
+       irq = pci_irq_vector(to_pci_dev(cxlds->dev), msgnum);
+       if (irq < 0)
+               return 0;
 
+       if (cxl_request_irq(cxlds, irq, NULL, cxl_pci_mbox_irq))
                return 0;
-       }
 
-mbox_poll:
-       mds->security.poll = true;
-       INIT_DELAYED_WORK(&mds->security.poll_dwork, cxl_mbox_sanitize_work);
+       dev_dbg(cxlds->dev, "Mailbox interrupts enabled\n");
+       /* 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);
 
-       dev_dbg(cxlds->dev, "Mailbox interrupts are unsupported");
        return 0;
 }