[PATCH] hw/nvme: fix cq_pending underflow that wedges pin-based interrupts
Itai Handler <[email protected]>
| Newsgroups | org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
n->cq_pending counts the completion queues that hold CQEs the host has not acknowledged yet. With pin-based interrupts it is what keeps the line asserted while any queue still has unacknowledged CQEs: nvme_irq_deassert() clears the vector's bit in irq_status only when the count drops to zero. The accounting is not symmetric. nvme_post_cqes() increments the counter once per empty -> non-empty transition of a CQ, but the two CQ head doorbell paths, nvme_process_db() and nvme_cq_notifier(), decrement it on every update that leaves the queue empty, including an update to a queue that was already empty. nvme_del_cq() gets this right and checks cq->tail != cq->head before decrementing; the doorbell paths do not. A host that writes the CQ head doorbell without having consumed a CQE - a redundant write, e.g. from a completion path racing with another one that already drained the queue - thus decrements more often than the device incremented, and cq_pending goes negative. Once it is negative it never gets back to zero: nvme_irq_deassert() stops clearing irq_status, nvme_irq_check() keeps re-asserting the pin, and the guest takes an endless interrupt for an empty completion queue. Linux eventually gives up on the line irq 26: nobody cared (try booting with the "irqpoll" option) handlers: [<...>] nvme_irq Disabling IRQ #26 and all I/O to the device hangs. MSI-X guests are not affected, they never look at cq_pending. Only decrement when the queue actually had unacknowledged CQEs. That restores the pairing with the increment in nvme_post_cqes() and makes the counter unable to underflow. Cc: [email protected] Fixes: 83d7ed5c570d ("hw/nvme: fix pin-based interrupt behavior (again)") Signed-off-by: Itai Handler <[email protected]> --- hw/nvme/ctrl.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index bd6ad64..655af08 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -4761,15 +4761,18 @@ static void nvme_cq_notifier(EventNotifier *e) { NvmeCQueue *cq = container_of(e, NvmeCQueue, notifier); NvmeCtrl *n = cq->ctrl; + bool pending; if (!event_notifier_test_and_clear(e)) { return; } + pending = cq->tail != cq->head; + nvme_update_cq_head(cq); if (cq->tail == cq->head) { - if (cq->irq_enabled) { + if (cq->irq_enabled && pending) { n->cq_pending--; } @@ -8509,6 +8512,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val) uint16_t new_head = val & 0xffff; NvmeCQueue *cq; + bool pending; qid = (addr - (0x1000 + (1 << 2))) >> 3; if (unlikely(nvme_check_cqid(n, qid))) { @@ -8563,13 +8567,14 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val) qemu_bh_schedule(cq->bh); } + pending = cq->tail != cq->head; cq->head = new_head; if (!qid && n->dbbuf_enabled) { stl_le_pci_dma(pci, cq->db_addr, cq->head, MEMTXATTRS_UNSPECIFIED); } if (cq->tail == cq->head) { - if (cq->irq_enabled) { + if (cq->irq_enabled && pending) { n->cq_pending--; }