Re: [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe

[Hikaru Nishida]

ping...

2017-12-22 16:31 GMT+09:00 Hikaru Nishida <address@hidden>:
> ping
> http://patchwork.ozlabs.org/patch/849786/
>
> 2017-12-18 14:00 GMT+09:00 Hikaru Nishida <address@hidden>:
>> Pin-based interrupt of NVMe controller did not work properly
>> because using an obsolated function pci_irq_pulse().
>> To fix this, change to use pci_irq_assert() / pci_irq_deassert()
>> instead of pci_irq_pulse().
>>
>> Signed-off-by: Hikaru Nishida <address@hidden>
>> ---
>>  hw/block/nvme.c | 39 ++++++++++++++++++++++++++++++++++-----
>>  hw/block/nvme.h |  1 +
>>  2 files changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>> index 441e21e..2d164fc 100644
>> --- a/hw/block/nvme.c
>> +++ b/hw/block/nvme.c
>> @@ -82,13 +82,40 @@ static uint8_t nvme_sq_empty(NvmeSQueue *sq)
>>      return sq->head == sq->tail;
>>  }
>>
>> -static void nvme_isr_notify(NvmeCtrl *n, NvmeCQueue *cq)
>> +static void nvme_irq_check(NvmeCtrl *n)
>> +{
>> +    if (msix_enabled(&(n->parent_obj))) {
>> +        return;
>> +    }
>> +    if (~n->bar.intms & n->irq_status) {
>> +        pci_irq_assert(&n->parent_obj);
>> +    } else {
>> +        pci_irq_deassert(&n->parent_obj);
>> +    }
>> +}
>> +
>> +static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
>>  {
>>      if (cq->irq_enabled) {
>>          if (msix_enabled(&(n->parent_obj))) {
>>              msix_notify(&(n->parent_obj), cq->vector);
>>          } else {
>> -            pci_irq_pulse(&n->parent_obj);
>> +            assert(cq->cqid < 64);
>> +            n->irq_status |= 1 << cq->cqid;
>> +            nvme_irq_check(n);
>> +        }
>> +    }
>> +}
>> +
>> +static void nvme_irq_deassert(NvmeCtrl *n, NvmeCQueue *cq)
>> +{
>> +    if (cq->irq_enabled) {
>> +        if (msix_enabled(&(n->parent_obj))) {
>> +            return;
>> +        } else {
>> +            assert(cq->cqid < 64);
>> +            n->irq_status &= ~(1 << cq->cqid);
>> +            nvme_irq_check(n);
>>          }
>>      }
>>  }
>> @@ -220,7 +247,7 @@ static void nvme_post_cqes(void *opaque)
>>              sizeof(req->cqe));
>>          QTAILQ_INSERT_TAIL(&sq->req_list, req, entry);
>>      }
>> -    nvme_isr_notify(n, cq);
>> +    nvme_irq_assert(n, cq);
>>  }
>>
>>  static void nvme_enqueue_req_completion(NvmeCQueue *cq, NvmeRequest *req)
>> @@ -753,10 +780,12 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, 
>> uint64_t data,
>>      case 0xc:
>>          n->bar.intms |= data & 0xffffffff;
>>          n->bar.intmc = n->bar.intms;
>> +        nvme_irq_check(n);
>>          break;
>>      case 0x10:
>>          n->bar.intms &= ~(data & 0xffffffff);
>>          n->bar.intmc = n->bar.intms;
>> +        nvme_irq_check(n);
>>          break;
>>      case 0x14:
>>          /* Windows first sends data, then sends enable bit */
>> @@ -851,8 +880,8 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, 
>> int val)
>>              timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 
>> 500);
>>          }
>>
>> -        if (cq->tail != cq->head) {
>> -            nvme_isr_notify(n, cq);
>> +        if (cq->tail == cq->head) {
>> +            nvme_irq_deassert(n, cq);
>>          }
>>      } else {
>>          uint16_t new_tail = val & 0xffff;
>> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
>> index 6aab338..7b62dad 100644
>> --- a/hw/block/nvme.h
>> +++ b/hw/block/nvme.h
>> @@ -775,6 +775,7 @@ typedef struct NvmeCtrl {
>>      uint32_t    cmbsz;
>>      uint32_t    cmbloc;
>>      uint8_t     *cmbuf;
>> +    uint64_t    irq_status;
>>
>>      char            *serial;
>>      NvmeNamespace   *namespaces;
>> --
>> 2.7.4
>>