Re: [PATCH 3/7] cxl/mbox: Add sanitation handling machinery

[Dave Jiang <dave.jiang@intel.com>]

On 4/28/23 9:46 AM, Davidlohr Bueso wrote:
> On Fri, 28 Apr 2023, Dave Jiang wrote:
> 
>>>  static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
>>>  {
>>>     struct cxl_dev_state *cxlds = id;
>>> +    u64 reg;
>>> +    u16 opcode;
>>>     /* spurious or raced with hw? */
>>>     if (!cxl_mbox_background_complete(cxlds)) {
>>> @@ -107,12 +109,47 @@ static irqreturn_t cxl_pci_mbox_irq(int irq, 
>>> void *id)
>>>         goto done;
>>>     }
>>> -    /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
>>> -    wake_up(&mbox_wait);
>>> +    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) {
>>> +        dev_dbg(cxlds->dev, "Sanitation operation ended\n");
>>
>> I might be missing something. Do we not want to stop waiting as well
>> if the sanitation operation has ended?
> 
> The thing here is that sanitize won't ever use the mbox_wait, which is
> what makes it special (asynchronous).
> 
> So while in theory patch 2 enables sanitize to be in the synchronous path,
> it can never occur because there is nothing there yet to trigger it (or
> anything else for that matter). And this patch ensures that the sanitize
> is isolated within __cxl_pci_mbox_send_cmd().

Gotcha. Thanks for the explanation.
> 
> Thanks,
> Davidlohr