All of lore.kernel.org
 help / color / mirror / Atom feed
From: Klaus Jensen <its@irrelevant.dk>
To: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Cc: "Fam Zheng" <fam@euphon.net>, "Kevin Wolf" <kwolf@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	qemu-block@nongnu.org, "Klaus Jensen" <k.jensen@samsung.com>,
	qemu-stable@nongnu.org, "Max Reitz" <mreitz@redhat.com>,
	"Jakub Jermář" <jakub.jermar@kernkonzept.com>,
	"Keith Busch" <kbusch@kernel.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Klaus Jensen" <its@irrelevant.dk>
Subject: [PULL 22/23] hw/nvme: fix pin-based interrupt behavior (again)
Date: Tue, 29 Jun 2021 20:47:42 +0200	[thread overview]
Message-ID: <20210629184743.230173-23-its@irrelevant.dk> (raw)
In-Reply-To: <20210629184743.230173-1-its@irrelevant.dk>

From: Klaus Jensen <k.jensen@samsung.com>

Jakub noticed[1] that, when using pin-based interrupts, the device will
unconditionally deasssert when any CQEs are acknowledged. However, the
pin should not be deasserted if other completion queues still holds
unacknowledged CQEs.

The bug is an artifact of commit ca247d35098d ("hw/block/nvme: fix
pin-based interrupt behavior") which fixed one bug but introduced
another. This is the third time someone tries to fix pin-based
interrupts (see commit 5e9aa92eb1a5 ("hw/block: Fix pin-based interrupt
behaviour of NVMe"))...

Third time's the charm, so fix it, again, by keeping track of how many
CQs have unacknowledged CQEs and only deassert when all are cleared.

  [1]: <20210610114624.304681-1-jakub.jermar@kernkonzept.com>

Cc: qemu-stable@nongnu.org
Fixes: ca247d35098d ("hw/block/nvme: fix pin-based interrupt behavior")
Reported-by: Jakub Jermář <jakub.jermar@kernkonzept.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Keith Busch <kbusch@kernel.org>
---
 hw/nvme/nvme.h |  1 +
 hw/nvme/ctrl.c | 18 +++++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 2509b8b039ef..56f8eceed2ad 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -410,6 +410,7 @@ typedef struct NvmeCtrl {
     uint32_t    max_q_ents;
     uint8_t     outstanding_aers;
     uint32_t    irq_status;
+    int         cq_pending;
     uint64_t    host_timestamp;                 /* Timestamp sent by the host */
     uint64_t    timestamp_set_qemu_clock_ms;    /* QEMU clock time */
     uint64_t    starttime_ms;
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 5c6c7d3455c3..629b0d38c2a2 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -473,7 +473,9 @@ static void nvme_irq_deassert(NvmeCtrl *n, NvmeCQueue *cq)
             return;
         } else {
             assert(cq->vector < 32);
-            n->irq_status &= ~(1 << cq->vector);
+            if (!n->cq_pending) {
+                n->irq_status &= ~(1 << cq->vector);
+            }
             nvme_irq_check(n);
         }
     }
@@ -1253,6 +1255,7 @@ static void nvme_post_cqes(void *opaque)
     NvmeCQueue *cq = opaque;
     NvmeCtrl *n = cq->ctrl;
     NvmeRequest *req, *next;
+    bool pending = cq->head != cq->tail;
     int ret;
 
     QTAILQ_FOREACH_SAFE(req, &cq->req_list, entry, next) {
@@ -1282,6 +1285,10 @@ static void nvme_post_cqes(void *opaque)
         QTAILQ_INSERT_TAIL(&sq->req_list, req, entry);
     }
     if (cq->tail != cq->head) {
+        if (cq->irq_enabled && !pending) {
+            n->cq_pending++;
+        }
+
         nvme_irq_assert(n, cq);
     }
 }
@@ -4297,6 +4304,11 @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeRequest *req)
         trace_pci_nvme_err_invalid_del_cq_notempty(qid);
         return NVME_INVALID_QUEUE_DEL;
     }
+
+    if (cq->irq_enabled && cq->tail != cq->head) {
+        n->cq_pending--;
+    }
+
     nvme_irq_deassert(n, cq);
     trace_pci_nvme_del_cq(qid);
     nvme_free_cq(cq, n);
@@ -6039,6 +6051,10 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
         }
 
         if (cq->tail == cq->head) {
+            if (cq->irq_enabled) {
+                n->cq_pending--;
+            }
+
             nvme_irq_deassert(n, cq);
         }
     } else {
-- 
2.32.0



  parent reply	other threads:[~2021-06-29 19:21 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-29 18:47 [PULL 00/23] hw/nvme patches Klaus Jensen
2021-06-29 18:47 ` [PULL 01/23] hw/nvme: fix style Klaus Jensen
2021-06-29 18:47 ` [PULL 02/23] hw/nvme: add identify namespace flbas/mc enums Klaus Jensen
2021-06-29 18:47 ` [PULL 03/23] hw/nvme: fix lbaf formats initialization Klaus Jensen
2021-06-29 18:47 ` [PULL 04/23] hw/nvme: add param to control auto zone transitioning to zone state closed Klaus Jensen
2021-06-29 18:47 ` [PULL 05/23] hw/nvme: fix csi field for cns 0x00 and 0x11 Klaus Jensen
2021-06-29 18:47 ` [PULL 06/23] hw/nvme: namespace parameter for EUI-64 Klaus Jensen
2021-08-09 10:18   ` Peter Maydell
2021-08-09 10:44     ` Klaus Jensen
2021-06-29 18:47 ` [PULL 07/23] hw/nvme: default for namespace EUI-64 Klaus Jensen
2021-06-29 18:47 ` [PULL 08/23] hw/nvme: reimplement flush to allow cancellation Klaus Jensen
2021-06-29 18:47 ` [PULL 09/23] hw/nvme: add nvme_block_status_all helper Klaus Jensen
2021-06-29 18:47 ` [PULL 10/23] hw/nvme: reimplement dsm to allow cancellation Klaus Jensen
2021-06-29 18:47 ` [PULL 11/23] hw/nvme: save reftag when generating pi Klaus Jensen
2021-06-29 18:47 ` [PULL 12/23] hw/nvme: remove assert from nvme_get_zone_by_slba Klaus Jensen
2021-06-29 18:47 ` [PULL 13/23] hw/nvme: use prinfo directly in nvme_check_prinfo and nvme_dif_check Klaus Jensen
2021-06-29 18:47 ` [PULL 14/23] hw/nvme: add dw0/1 to the req completion trace event Klaus Jensen
2021-06-29 18:47 ` [PULL 15/23] hw/nvme: reimplement the copy command to allow aio cancellation Klaus Jensen
2021-06-29 18:47 ` [PULL 16/23] hw/nvme: reimplement zone reset to allow cancellation Klaus Jensen
2021-06-29 18:47 ` [PULL 17/23] hw/nvme: reimplement format nvm " Klaus Jensen
2021-06-29 18:47 ` [PULL 18/23] Partially revert "hw/block/nvme: drain namespaces on sq deletion" Klaus Jensen
2021-06-29 18:47 ` [PULL 19/23] hw/nvme: fix endianess conversion and add controller list Klaus Jensen
2021-06-29 18:47 ` [PULL 20/23] hw/nvme: documentation fix Klaus Jensen
2021-06-29 18:47 ` [PULL 21/23] hw/nvme: fix missing check for PMR capability Klaus Jensen
2021-06-29 18:47 ` Klaus Jensen [this message]
2021-06-29 18:47 ` [PULL 23/23] hw/nvme: add 'zoned.zasl' to documentation Klaus Jensen
2021-07-01  9:07 ` [PULL 00/23] hw/nvme patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210629184743.230173-23-its@irrelevant.dk \
    --to=its@irrelevant.dk \
    --cc=ehabkost@redhat.com \
    --cc=fam@euphon.net \
    --cc=jakub.jermar@kernkonzept.com \
    --cc=k.jensen@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.