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>,
	Beata Michalska <beata.michalska@linaro.org>,
	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>
Subject: [PATCH v7 40/48] nvme: handle dma errors
Date: Wed, 15 Apr 2020 07:51:32 +0200	[thread overview]
Message-ID: <20200415055140.466900-41-its@irrelevant.dk> (raw)
In-Reply-To: <20200415055140.466900-1-its@irrelevant.dk>

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

Handling DMA errors gracefully is required for the device to pass the
block/011 test ("disable PCI device while doing I/O") in the blktests
suite.

With this patch the device passes the test by retrying "critical"
transfers (posting of completion entries and processing of submission
queue entries).

If DMA errors occur at any other point in the execution of the command
(say, while mapping the PRPs), the command is aborted with a Data
Transfer Error status code.

Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Acked-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 hw/block/nvme.c       | 45 ++++++++++++++++++++++++++++++++-----------
 hw/block/trace-events |  2 ++
 include/block/nvme.h  |  2 +-
 3 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index eb15a0bd3cf9..6dcd9c4b4cd0 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -74,14 +74,14 @@ static inline bool nvme_addr_is_cmb(NvmeCtrl *n, hwaddr addr)
     return addr >= low && addr < hi;
 }
 
-static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
+static int nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
 {
     if (n->bar.cmbsz && nvme_addr_is_cmb(n, addr)) {
         memcpy(buf, nvme_addr_to_cmb(n, addr), size);
-        return;
+        return 0;
     }
 
-    pci_dma_read(&n->parent_obj, addr, buf, size);
+    return pci_dma_read(&n->parent_obj, addr, buf, size);
 }
 
 static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid)
@@ -185,7 +185,7 @@ static uint16_t nvme_map_addr_cmb(NvmeCtrl *n, QEMUIOVector *iov, hwaddr addr,
     }
 
     if (!nvme_addr_is_cmb(n, addr) || !nvme_addr_is_cmb(n, addr + len - 1)) {
-        return NVME_DATA_TRAS_ERROR;
+        return NVME_DATA_TRANSFER_ERROR;
     }
 
     qemu_iovec_add(iov, nvme_addr_to_cmb(n, addr), len);
@@ -238,6 +238,7 @@ static uint16_t nvme_map_prp(NvmeCtrl *n, QEMUSGList *qsg, QEMUIOVector *iov,
     int num_prps = (len >> n->page_bits) + 1;
     uint16_t status;
     bool prp_list_in_cmb = false;
+    int ret;
 
     trace_nvme_dev_map_prp(nvme_cid(req), trans_len, len, prp1, prp2,
                            num_prps);
@@ -277,7 +278,12 @@ static uint16_t nvme_map_prp(NvmeCtrl *n, QEMUSGList *qsg, QEMUIOVector *iov,
 
             nents = (len + n->page_size - 1) >> n->page_bits;
             prp_trans = MIN(n->max_prp_ents, nents) * sizeof(uint64_t);
-            nvme_addr_read(n, prp2, (void *)prp_list, prp_trans);
+            ret = nvme_addr_read(n, prp2, (void *)prp_list, prp_trans);
+            if (ret) {
+                trace_nvme_dev_err_addr_read(prp2);
+                status = NVME_DATA_TRANSFER_ERROR;
+                goto unmap;
+            }
             while (len != 0) {
                 uint64_t prp_ent = le64_to_cpu(prp_list[i]);
 
@@ -296,8 +302,13 @@ static uint16_t nvme_map_prp(NvmeCtrl *n, QEMUSGList *qsg, QEMUIOVector *iov,
                     i = 0;
                     nents = (len + n->page_size - 1) >> n->page_bits;
                     prp_trans = MIN(n->max_prp_ents, nents) * sizeof(uint64_t);
-                    nvme_addr_read(n, prp_ent, (void *)prp_list,
-                        prp_trans);
+                    ret = nvme_addr_read(n, prp_ent, (void *)prp_list,
+                                         prp_trans);
+                    if (ret) {
+                        trace_nvme_dev_err_addr_read(prp_ent);
+                        status = NVME_DATA_TRANSFER_ERROR;
+                        goto unmap;
+                    }
                     prp_ent = le64_to_cpu(prp_list[i]);
                 }
 
@@ -502,6 +513,7 @@ static void nvme_post_cqes(void *opaque)
     NvmeCQueue *cq = opaque;
     NvmeCtrl *n = cq->ctrl;
     NvmeRequest *req, *next;
+    int ret;
 
     QTAILQ_FOREACH_SAFE(req, &cq->req_list, entry, next) {
         NvmeSQueue *sq;
@@ -511,15 +523,21 @@ static void nvme_post_cqes(void *opaque)
             break;
         }
 
-        QTAILQ_REMOVE(&cq->req_list, req, entry);
         sq = req->sq;
         req->cqe.status = cpu_to_le16((req->status << 1) | cq->phase);
         req->cqe.sq_id = cpu_to_le16(sq->sqid);
         req->cqe.sq_head = cpu_to_le16(sq->head);
         addr = cq->dma_addr + cq->tail * n->cqe_size;
+        ret = pci_dma_write(&n->parent_obj, addr, (void *)&req->cqe,
+                            sizeof(req->cqe));
+        if (ret) {
+            trace_nvme_dev_err_addr_write(addr);
+            timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
+                      500 * SCALE_MS);
+            break;
+        }
+        QTAILQ_REMOVE(&cq->req_list, req, entry);
         nvme_inc_cq_tail(cq);
-        pci_dma_write(&n->parent_obj, addr, (void *)&req->cqe,
-            sizeof(req->cqe));
         nvme_req_clear(req);
         QTAILQ_INSERT_TAIL(&sq->req_list, req, entry);
     }
@@ -1664,7 +1682,12 @@ static void nvme_process_sq(void *opaque)
 
     while (!(nvme_sq_empty(sq) || QTAILQ_EMPTY(&sq->req_list))) {
         addr = sq->dma_addr + sq->head * n->sqe_size;
-        nvme_addr_read(n, addr, (void *)&cmd, sizeof(cmd));
+        if (nvme_addr_read(n, addr, (void *)&cmd, sizeof(cmd))) {
+            trace_nvme_dev_err_addr_read(addr);
+            timer_mod(sq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
+                      500 * SCALE_MS);
+            break;
+        }
         nvme_inc_sq_head(sq);
 
         req = QTAILQ_FIRST(&sq->req_list);
diff --git a/hw/block/trace-events b/hw/block/trace-events
index 7c277a2999c0..75bde5e676a5 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -87,6 +87,8 @@ nvme_dev_mmio_doorbell_sq(uint16_t sqid, uint16_t new_tail) "cqid %"PRIu16" new_
 # nvme traces for error conditions
 nvme_dev_err_mdts(uint16_t cid, size_t mdts, size_t len) "cid %"PRIu16" mdts %"PRIu64" len %"PRIu64""
 nvme_dev_err_aio(uint16_t cid, void *aio, const char *blkname, uint64_t offset, const char *opc, void *req, uint16_t status) "cid %"PRIu16" aio %p blk \"%s\" offset %"PRIu64" opc \"%s\" req %p status 0x%"PRIx16""
+nvme_dev_err_addr_read(uint64_t addr) "addr 0x%"PRIx64""
+nvme_dev_err_addr_write(uint64_t addr) "addr 0x%"PRIx64""
 nvme_dev_err_invalid_dma(void) "PRP/SGL is too small for transfer size"
 nvme_dev_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null or not page aligned: 0x%"PRIx64""
 nvme_dev_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: 0x%"PRIx64""
diff --git a/include/block/nvme.h b/include/block/nvme.h
index c4c669e32fc4..03bee32c27c4 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -457,7 +457,7 @@ enum NvmeStatusCodes {
     NVME_INVALID_OPCODE         = 0x0001,
     NVME_INVALID_FIELD          = 0x0002,
     NVME_CID_CONFLICT           = 0x0003,
-    NVME_DATA_TRAS_ERROR        = 0x0004,
+    NVME_DATA_TRANSFER_ERROR    = 0x0004,
     NVME_POWER_LOSS_ABORT       = 0x0005,
     NVME_INTERNAL_DEV_ERROR     = 0x0006,
     NVME_CMD_ABORT_REQ          = 0x0007,
-- 
2.26.0



  parent reply	other threads:[~2020-04-15  6:11 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-15  5:50 [PATCH v7 00/48] nvme: support NVMe v1.3d, SGLs and multiple namespaces Klaus Jensen
2020-04-15  5:50 ` [PATCH v7 01/48] nvme: rename trace events to nvme_dev Klaus Jensen
2020-04-15  5:50 ` [PATCH v7 02/48] nvme: remove superfluous breaks Klaus Jensen
2020-04-15  6:57   ` Philippe Mathieu-Daudé
2020-04-15  5:50 ` [PATCH v7 03/48] nvme: move device parameters to separate struct Klaus Jensen
2020-04-15  6:58   ` Philippe Mathieu-Daudé
2020-04-15  5:50 ` [PATCH v7 04/48] nvme: bump spec data structures to v1.3 Klaus Jensen
2020-04-15  5:50 ` [PATCH v7 05/48] nvme: use constants in identify Klaus Jensen
2020-04-15  7:01   ` Philippe Mathieu-Daudé
2020-04-15  5:50 ` [PATCH v7 06/48] nvme: refactor nvme_addr_read Klaus Jensen
2020-04-15  7:03   ` Philippe Mathieu-Daudé
2020-04-15  7:46     ` Klaus Birkelund Jensen
2020-04-15  5:50 ` [PATCH v7 07/48] nvme: add support for the abort command Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 08/48] nvme: fix pci doorbell size calculation Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 09/48] nvme: add max_ioqpairs device parameter Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 10/48] nvme: remove redundant cmbloc/cmbsz members Klaus Jensen
2020-04-15  7:10   ` Philippe Mathieu-Daudé
2020-04-15  7:19     ` Klaus Birkelund Jensen
2020-04-15  7:48       ` Philippe Mathieu-Daudé
2020-04-15  5:51 ` [PATCH v7 11/48] nvme: refactor device realization Klaus Jensen
2020-04-15  7:14   ` Philippe Mathieu-Daudé
2020-04-15  7:25     ` Klaus Birkelund Jensen
2020-04-15  7:55       ` Philippe Mathieu-Daudé
2020-04-15  8:18         ` Klaus Birkelund Jensen
2020-04-15  5:51 ` [PATCH v7 12/48] nvme: add temperature threshold feature Klaus Jensen
2020-04-15  7:19   ` Philippe Mathieu-Daudé
2020-04-15  7:24     ` Klaus Birkelund Jensen
2020-04-15  7:28       ` Klaus Birkelund Jensen
2020-04-15  7:45         ` Philippe Mathieu-Daudé
2020-04-15  5:51 ` [PATCH v7 13/48] nvme: add support for the get log page command Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 14/48] nvme: add support for the asynchronous event request command Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 15/48] nvme: add missing mandatory features Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 16/48] nvme: additional tracing Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 17/48] nvme: make sure ncqr and nsqr is valid Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 18/48] nvme: add log specific field to trace events Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 19/48] nvme: support identify namespace descriptor list Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 20/48] nvme: enforce valid queue creation sequence Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 21/48] nvme: provide the mandatory subnqn field Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 22/48] nvme: bump supported version to v1.3 Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 23/48] nvme: memset preallocated requests structures Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 24/48] nvme: add mapping helpers Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 25/48] nvme: replace dma_acct with blk_acct equivalent Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 26/48] nvme: remove redundant has_sg member Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 27/48] nvme: refactor dma read/write Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 28/48] nvme: pass request along for tracing Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 29/48] nvme: add request mapping helper Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 30/48] nvme: verify validity of prp lists in the cmb Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 31/48] nvme: refactor request bounds checking Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 32/48] nvme: add check for mdts Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 33/48] nvme: be consistent about zeros vs zeroes Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 34/48] nvme: refactor NvmeRequest Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 35/48] nvme: remove NvmeCmd parameter Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 36/48] nvme: allow multiple aios per command Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 37/48] nvme: add nvme_check_rw helper Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 38/48] nvme: use preallocated qsg/iov in nvme_dma_prp Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 39/48] pci: pass along the return value of dma_memory_rw Klaus Jensen
2020-04-15  5:51 ` Klaus Jensen [this message]
2020-04-15  7:26   ` [PATCH v7 40/48] nvme: handle dma errors Philippe Mathieu-Daudé
2020-04-15  5:51 ` [PATCH v7 41/48] nvme: harden cmb access Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 42/48] nvme: add support for scatter gather lists Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 43/48] nvme: add support for sgl bit bucket descriptor Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 44/48] nvme: refactor identify active namespace id list Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 45/48] nvme: support multiple namespaces Klaus Jensen
2020-04-15  7:38   ` Philippe Mathieu-Daudé
2020-04-15  8:02     ` Klaus Birkelund Jensen
2020-04-15  5:51 ` [PATCH v7 46/48] pci: allocate pci id for nvme Klaus Jensen
2020-04-21  9:19   ` Gerd Hoffmann
2020-04-15  5:51 ` [PATCH v7 47/48] nvme: change controller pci id Klaus Jensen
2020-04-15  5:51 ` [PATCH v7 48/48] nvme: make lba data size configurable Klaus Jensen
2020-04-15  7:40   ` Philippe Mathieu-Daudé
2020-04-15  7:17 ` [PATCH v7 00/48] nvme: support NVMe v1.3d, SGLs and multiple namespaces no-reply
2020-04-15  8:02 ` no-reply

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=20200415055140.466900-41-its@irrelevant.dk \
    --to=its@irrelevant.dk \
    --cc=beata.michalska@linaro.org \
    --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=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.