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

["Li, Ming" <ming4.li@intel.com>]

On 5/3/2023 10:57 PM, Davidlohr Bueso wrote:

>  /**
>   * __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) {
> +        long ret;
> +        u64 bg_status_reg;
> +        int i, 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,
> +                    cxl_mbox_background_complete(cxlds),
> +                    TASK_INTERRUPTIBLE,
> +                    msecs_to_jiffies(timeout));
> +            if (ret > 0)
> +                break;
> +            if (ret < 0) /* interrupted by a signal */
> +                return ret;

What do you think if adding a dev_dbg() here for outputting current percentage complete for debugging? is it helpful?

Thanks
Ming


> +        }
> +
> +        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