All of lore.kernel.org
 help / color / mirror / Atom feed
From: Klaus Jensen <its@irrelevant.dk>
To: qemu-block@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Klaus Jensen" <k.jensen@samsung.com>,
	qemu-devel@nongnu.org, "Max Reitz" <mreitz@redhat.com>,
	"Klaus Jensen" <its@irrelevant.dk>,
	"Keith Busch" <kbusch@kernel.org>,
	"Javier Gonzalez" <javier.gonz@samsung.com>,
	"Maxim Levitsky" <mlevitsk@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: [PATCH v7 07/22] hw/block/nvme: fix pin-based interrupt behavior
Date: Tue,  9 Jun 2020 21:03:18 +0200	[thread overview]
Message-ID: <20200609190333.59390-8-its@irrelevant.dk> (raw)
In-Reply-To: <20200609190333.59390-1-its@irrelevant.dk>

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

First, since the device only supports MSI-X or pin-based interrupt, if
MSI-X is not enabled, it should not accept interrupt vectors different
from 0 when creating completion queues.

Secondly, the irq_status NvmeCtrl member is meant to be compared to the
INTMS register, so it should only be 32 bits wide. And it is really only
useful when used with multi-message MSI.

Third, since we do not force a 1-to-1 correspondence between cqid and
interrupt vector, the irq_status register should not have bits set
according to cqid, but according to the associated interrupt vector.

Fix these issues, but keep irq_status available so we can easily support
multi-message MSI down the line.

Fixes: 5e9aa92eb1a5 ("hw/block: Fix pin-based interrupt behaviour of NVMe")
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Message-Id: <20200514044611.734782-8-its@irrelevant.dk>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/nvme.c | 12 ++++++++----
 hw/block/nvme.h |  2 +-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d6fcf078a4c9..ee514625ee85 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -137,8 +137,8 @@ static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
             msix_notify(&(n->parent_obj), cq->vector);
         } else {
             trace_pci_nvme_irq_pin();
-            assert(cq->cqid < 64);
-            n->irq_status |= 1 << cq->cqid;
+            assert(cq->vector < 32);
+            n->irq_status |= 1 << cq->vector;
             nvme_irq_check(n);
         }
     } else {
@@ -152,8 +152,8 @@ static void nvme_irq_deassert(NvmeCtrl *n, NvmeCQueue *cq)
         if (msix_enabled(&(n->parent_obj))) {
             return;
         } else {
-            assert(cq->cqid < 64);
-            n->irq_status &= ~(1 << cq->cqid);
+            assert(cq->vector < 32);
+            n->irq_status &= ~(1 << cq->vector);
             nvme_irq_check(n);
         }
     }
@@ -652,6 +652,10 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
         trace_pci_nvme_err_invalid_create_cq_addr(prp1);
         return NVME_INVALID_FIELD | NVME_DNR;
     }
+    if (unlikely(!msix_enabled(&n->parent_obj) && vector)) {
+        trace_pci_nvme_err_invalid_create_cq_vector(vector);
+        return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
+    }
     if (unlikely(vector > n->params.num_queues)) {
         trace_pci_nvme_err_invalid_create_cq_vector(vector);
         return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 9df244c93c02..91f16c812582 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -84,7 +84,7 @@ typedef struct NvmeCtrl {
     uint32_t    cmbsz;
     uint32_t    cmbloc;
     uint8_t     *cmbuf;
-    uint64_t    irq_status;
+    uint32_t    irq_status;
     uint64_t    host_timestamp;                 /* Timestamp sent by the host */
     uint64_t    timestamp_set_qemu_clock_ms;    /* QEMU clock time */
 
-- 
2.27.0



  parent reply	other threads:[~2020-06-09 19:10 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-09 19:03 [PATCH v7 00/22] nvme: small fixes, refactoring and cleanups Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 01/22] hw/block/nvme: fix pci doorbell size calculation Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 02/22] hw/block/nvme: rename trace events to pci_nvme Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 03/22] hw/block/nvme: remove superfluous breaks Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 04/22] hw/block/nvme: move device parameters to separate struct Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 05/22] hw/block/nvme: use constants in identify Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 06/22] hw/block/nvme: refactor nvme_addr_read Klaus Jensen
2020-06-09 19:03 ` Klaus Jensen [this message]
2020-06-09 19:03 ` [PATCH v7 08/22] hw/block/nvme: add max_ioqpairs device parameter Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 09/22] hw/block/nvme: remove redundant cmbloc/cmbsz members Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 10/22] hw/block/nvme: factor out property/constraint checks Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 11/22] hw/block/nvme: factor out device state setup Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 12/22] hw/block/nvme: factor out block backend setup Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 13/22] hw/block/nvme: add namespace helpers Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 14/22] hw/block/nvme: factor out namespace setup Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 15/22] hw/block/nvme: factor out pci setup Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 16/22] hw/block/nvme: factor out cmb setup Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 17/22] hw/block/nvme: factor out pmr setup Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 18/22] hw/block/nvme: do cmb/pmr init as part of pci init Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 19/22] hw/block/nvme: factor out controller identify setup Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 20/22] hw/block/nvme: Verify msix_vector_use() returned value Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 21/22] hw/block/nvme: add msix_qsize parameter Klaus Jensen
2020-06-09 19:03 ` [PATCH v7 22/22] hw/block/nvme: verify msix_init_exclusive_bar() return value Klaus Jensen
2020-06-15 16:21 ` [PATCH v7 00/22] nvme: small fixes, refactoring and cleanups Kevin Wolf

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=20200609190333.59390-8-its@irrelevant.dk \
    --to=its@irrelevant.dk \
    --cc=javier.gonz@samsung.com \
    --cc=k.jensen@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=kwolf@redhat.com \
    --cc=mlevitsk@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=mst@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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.