All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Tikhov <d.tihov@yadro.com>
To: <qemu-devel@nongnu.org>
Cc: kbusch@kernel.org, its@irrelevant.dk, ddtikhov@gmail.com,
	qemu-block@nongnu.org, linux@yadro.com
Subject: [PATCH 2/2] hw/nvme: fix copy cmd for pi enabled namespaces
Date: Wed, 20 Apr 2022 12:03:36 +0300	[thread overview]
Message-ID: <20220420090336.10124-3-d.tihov@yadro.com> (raw)
In-Reply-To: <20220420090336.10124-1-d.tihov@yadro.com>

Current implementation have two problems:
First in the read part of copy command. Because there is no metadata
mangling before nvme_dif_check invocation, reftag error is thrown for
blocks of namespace that have not been previously written to.
Second in the write part. Reftag in the protection information section
of the source metadata should not be copied as is to the destination.
Source range start lba and destination range start lba could differ so
recalculation of reftag is always needed.

Signed-off-by: Dmitry Tikhov <d.tihov@yadro.com>
---
 hw/nvme/ctrl.c |  5 ++++
 hw/nvme/dif.c  | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/nvme/dif.h  |  2 ++
 3 files changed, 72 insertions(+)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 74540a03d5..cb493f4506 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -2787,6 +2787,10 @@ static void nvme_copy_in_completed_cb(void *opaque, int ret)
         size_t mlen = nvme_m2b(ns, nlb);
         uint8_t *mbounce = iocb->bounce + nvme_l2b(ns, nlb);
 
+        status = nvme_dif_mangle_mdata(ns, mbounce, mlen, reftag);
+        if (status) {
+            goto invalid;
+        }
         status = nvme_dif_check(ns, iocb->bounce, len, mbounce, mlen, prinfor,
                                 slba, apptag, appmask, &reftag);
         if (status) {
@@ -2805,6 +2809,7 @@ static void nvme_copy_in_completed_cb(void *opaque, int ret)
             nvme_dif_pract_generate_dif(ns, iocb->bounce, len, mbounce, mlen,
                                         apptag, &iocb->reftag);
         } else {
+            nvme_dif_restore_reftag(ns, mbounce, mlen, iocb->reftag);
             status = nvme_dif_check(ns, iocb->bounce, len, mbounce, mlen,
                                     prinfow, iocb->slba, apptag, appmask,
                                     &iocb->reftag);
diff --git a/hw/nvme/dif.c b/hw/nvme/dif.c
index 0f65687396..f29c5893e2 100644
--- a/hw/nvme/dif.c
+++ b/hw/nvme/dif.c
@@ -385,6 +385,71 @@ uint16_t nvme_dif_check(NvmeNamespace *ns, uint8_t *buf, size_t len,
     return NVME_SUCCESS;
 }
 
+static void nvme_dif_restore_reftag_crc16(NvmeNamespace *ns, uint8_t *mbuf,
+                                          size_t mlen, uint64_t reftag,
+                                          int16_t pil)
+{
+    uint8_t *mbufp, *end = mbuf + mlen;
+
+    for (mbufp = mbuf; mbufp < end; mbufp += ns->lbaf.ms) {
+        NvmeDifTuple *dif = (NvmeDifTuple *)(mbufp + pil);
+
+        if (!nvme_dif_is_disabled_crc16(ns, dif)) {
+            dif->g16.reftag = cpu_to_be32(reftag++);
+        }
+
+    }
+
+    return;
+}
+
+static void nvme_dif_restore_reftag_crc64(NvmeNamespace *ns, uint8_t *mbuf,
+                                          size_t mlen, uint64_t reftag,
+                                          int16_t pil)
+{
+    uint8_t *mbufp, *end = mbuf + mlen;
+
+    for (mbufp = mbuf; mbufp < end; mbufp += ns->lbaf.ms) {
+        NvmeDifTuple *dif = (NvmeDifTuple *)(mbufp + pil);
+
+        if (!nvme_dif_is_disabled_crc64(ns, dif)) {
+            dif->g64.sr[0] = reftag >> 40;
+            dif->g64.sr[1] = reftag >> 32;
+            dif->g64.sr[2] = reftag >> 24;
+            dif->g64.sr[3] = reftag >> 16;
+            dif->g64.sr[4] = reftag >> 8;
+            dif->g64.sr[5] = reftag;
+
+            reftag++;
+        }
+
+    }
+
+    return;
+}
+
+void nvme_dif_restore_reftag(NvmeNamespace *ns, uint8_t *mbuf,
+                             size_t mlen, uint64_t reftag)
+{
+    int16_t pil = 0;
+
+    if (!(ns->id_ns.dps & NVME_ID_NS_DPS_FIRST_EIGHT)) {
+        pil = ns->lbaf.ms - nvme_pi_tuple_size(ns);
+    }
+
+    switch (ns->pif) {
+    case NVME_PI_GUARD_16:
+        nvme_dif_restore_reftag_crc16(ns, mbuf, mlen, reftag, pil);
+        return;
+    case NVME_PI_GUARD_64:
+        nvme_dif_restore_reftag_crc64(ns, mbuf, mlen, reftag, pil);
+        return;
+    default:
+        abort();
+    }
+
+}
+
 uint16_t nvme_dif_mangle_mdata(NvmeNamespace *ns, uint8_t *mbuf, size_t mlen,
                                uint64_t slba)
 {
diff --git a/hw/nvme/dif.h b/hw/nvme/dif.h
index fe1e5828d7..3203837658 100644
--- a/hw/nvme/dif.h
+++ b/hw/nvme/dif.h
@@ -186,6 +186,8 @@ uint16_t nvme_dif_check(NvmeNamespace *ns, uint8_t *buf, size_t len,
                         uint8_t *mbuf, size_t mlen, uint8_t prinfo,
                         uint64_t slba, uint16_t apptag,
                         uint16_t appmask, uint64_t *reftag);
+void nvme_dif_restore_reftag(NvmeNamespace *ns, uint8_t *mbuf,
+                             size_t mlen, uint64_t reftag);
 bool nvme_dif_is_disabled(NvmeNamespace *ns, NvmeDifTuple *dif);
 uint16_t nvme_dif_rw(NvmeCtrl *n, NvmeRequest *req);
 
-- 
2.35.1



  parent reply	other threads:[~2022-04-20  9:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-20  9:03 [PATCH 0/2] Fix nvme copy command with pi metadata Dmitry Tikhov
2022-04-20  9:03 ` [PATCH 1/2] hw/nvme: refactor check of disabled dif Dmitry Tikhov
2022-04-20  9:03 ` Dmitry Tikhov [this message]
2022-04-20 10:04   ` [PATCH 2/2] hw/nvme: fix copy cmd for pi enabled namespaces Klaus Jensen
2022-04-20 19:16     ` Klaus Jensen
2022-04-21  7:41       ` Dmitry Tikhov
2022-04-21 10:13         ` 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=20220420090336.10124-3-d.tihov@yadro.com \
    --to=d.tihov@yadro.com \
    --cc=ddtikhov@gmail.com \
    --cc=its@irrelevant.dk \
    --cc=kbusch@kernel.org \
    --cc=linux@yadro.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.