qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Klaus Jensen <its@irrelevant.dk>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	qemu-block@nongnu.org, Klaus Jensen <k.jensen@samsung.com>,
	Gollu Appalanaidu <anaidu.gollu@samsung.com>,
	Max Reitz <mreitz@redhat.com>, Klaus Jensen <its@irrelevant.dk>,
	Keith Busch <kbusch@kernel.org>
Subject: [PATCH] hw/block/nvme: fix prp mapping status codes
Date: Mon, 19 Oct 2020 13:30:39 +0200	[thread overview]
Message-ID: <20201019113039.76146-1-its@irrelevant.dk> (raw)

From: Gollu Appalanaidu <anaidu.gollu@samsung.com>

Differentiate between missing PRPs and misaligned PRPs, return the
relevant status code and streamline the trace event naming.

See NVMe Express v1.3d, Section 4.3 ("Physical Region Page Entry and
List").

Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 include/block/nvme.h  |  1 +
 hw/block/nvme.c       | 22 ++++++++++++++++------
 hw/block/trace-events |  5 +++--
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index 6de2d5aa75a9..8a46d9cf015f 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -655,6 +655,7 @@ enum NvmeStatusCodes {
     NVME_MD_SGL_LEN_INVALID     = 0x0010,
     NVME_SGL_DESCR_TYPE_INVALID = 0x0011,
     NVME_INVALID_USE_OF_CMB     = 0x0012,
+    NVME_INVALID_PRP_OFFSET     = 0x0013,
     NVME_LBA_RANGE              = 0x0080,
     NVME_CAP_EXCEEDED           = 0x0081,
     NVME_NS_NOT_READY           = 0x0082,
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 9d30ca69dcf1..785a87af0138 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -328,7 +328,7 @@ static uint16_t nvme_map_prp(NvmeCtrl *n, uint64_t prp1, uint64_t prp2,
     trace_pci_nvme_map_prp(trans_len, len, prp1, prp2, num_prps);
 
     if (unlikely(!prp1)) {
-        trace_pci_nvme_err_invalid_prp();
+        trace_pci_nvme_err_invalid_prp1_missing();
         return NVME_INVALID_FIELD | NVME_DNR;
     }
 
@@ -370,11 +370,16 @@ static uint16_t nvme_map_prp(NvmeCtrl *n, uint64_t prp1, uint64_t prp2,
                 uint64_t prp_ent = le64_to_cpu(prp_list[i]);
 
                 if (i == n->max_prp_ents - 1 && len > n->page_size) {
-                    if (unlikely(!prp_ent || prp_ent & (n->page_size - 1))) {
-                        trace_pci_nvme_err_invalid_prplist_ent(prp_ent);
+                    if (unlikely(!prp_ent)) {
+                        trace_pci_nvme_err_invalid_prplist_ent_missing();
                         return NVME_INVALID_FIELD | NVME_DNR;
                     }
 
+                    if (unlikely(prp_ent & (n->page_size - 1))) {
+                        trace_pci_nvme_err_invalid_prplist_ent(prp_ent);
+                        return NVME_INVALID_PRP_OFFSET | NVME_DNR;
+                    }
+
                     if (prp_list_in_cmb != nvme_addr_is_cmb(n, prp_ent)) {
                         return NVME_INVALID_USE_OF_CMB | NVME_DNR;
                     }
@@ -391,11 +396,16 @@ static uint16_t nvme_map_prp(NvmeCtrl *n, uint64_t prp1, uint64_t prp2,
                     prp_ent = le64_to_cpu(prp_list[i]);
                 }
 
-                if (unlikely(!prp_ent || prp_ent & (n->page_size - 1))) {
-                    trace_pci_nvme_err_invalid_prplist_ent(prp_ent);
+                if (unlikely(!prp_ent)) {
+                    trace_pci_nvme_err_invalid_prplist_ent_missing();
                     return NVME_INVALID_FIELD | NVME_DNR;
                 }
 
+                if (unlikely(prp_ent & (n->page_size - 1))) {
+                    trace_pci_nvme_err_invalid_prplist_ent(prp_ent);
+                    return NVME_INVALID_PRP_OFFSET | NVME_DNR;
+                }
+
                 trans_len = MIN(len, n->page_size);
                 status = nvme_map_addr(n, qsg, iov, prp_ent, trans_len);
                 if (status) {
@@ -408,7 +418,7 @@ static uint16_t nvme_map_prp(NvmeCtrl *n, uint64_t prp1, uint64_t prp2,
         } else {
             if (unlikely(prp2 & (n->page_size - 1))) {
                 trace_pci_nvme_err_invalid_prp2_align(prp2);
-                return NVME_INVALID_FIELD | NVME_DNR;
+                return NVME_INVALID_PRP_OFFSET | NVME_DNR;
             }
             status = nvme_map_addr(n, qsg, iov, prp2, len);
             if (status) {
diff --git a/hw/block/trace-events b/hw/block/trace-events
index cab9913b1f2d..2bafbed256e8 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -97,10 +97,11 @@ pci_nvme_err_invalid_sgld(uint16_t cid, uint8_t typ) "cid %"PRIu16" type 0x%"PRI
 pci_nvme_err_invalid_num_sgld(uint16_t cid, uint8_t typ) "cid %"PRIu16" type 0x%"PRIx8""
 pci_nvme_err_invalid_sgl_excess_length(uint16_t cid) "cid %"PRIu16""
 pci_nvme_err_invalid_dma(void) "PRP/SGL is too small for transfer size"
-pci_nvme_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null or not page aligned: 0x%"PRIx64""
+pci_nvme_err_invalid_prp1_missing(void) "PRP1 is null"
 pci_nvme_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: 0x%"PRIx64""
 pci_nvme_err_invalid_prp2_missing(void) "PRP2 is null and more data to be transferred"
-pci_nvme_err_invalid_prp(void) "invalid PRP"
+pci_nvme_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry not page aligned: 0x%"PRIx64""
+pci_nvme_err_invalid_prplist_ent_missing(void) "PRP list entry is null and more data to be transferred"
 pci_nvme_err_invalid_opc(uint8_t opc) "invalid opcode 0x%"PRIx8""
 pci_nvme_err_invalid_admin_opc(uint8_t opc) "invalid admin opcode 0x%"PRIx8""
 pci_nvme_err_invalid_lba_range(uint64_t start, uint64_t len, uint64_t limit) "Invalid LBA start=%"PRIu64" len=%"PRIu64" limit=%"PRIu64""
-- 
2.28.0



             reply	other threads:[~2020-10-19 11:35 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-19 11:30 Klaus Jensen [this message]
2020-10-19 16:34 ` [PATCH] hw/block/nvme: fix prp mapping status codes Keith Busch
2020-10-19 17:31   ` Klaus Jensen

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=20201019113039.76146-1-its@irrelevant.dk \
    --to=its@irrelevant.dk \
    --cc=anaidu.gollu@samsung.com \
    --cc=k.jensen@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=kwolf@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).