[PATCH-for-5.2] block/nvme: Fix nvme_submit_command() on big-endian host

[PATCH-for-5.2] block/nvme: Fix nvme_submit_command() on big-endian host

[Philippe Mathieu-Daudé]

The Completion Queue Command Identifier is a 16-bit value,
so nvme_submit_command() is unlikely to work on big-endian
hosts, as the relevant bits are truncated.

The "Completion Queue Entry: DW 2" describes it as:

  This identifier is assigned by host software when
  the command is submitted to the Submission

As the is just an opaque cookie, it is pointless to byte-swap it.

Fixes: bdd6a90a9e5 ("block: Add VFIO based NVMe driver")
Reported-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Based-on: <20201027135547.374946-1-philmd@redhat.com>
---
 block/nvme.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index ff645eefe6a..d9b2245db40 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -343,7 +343,7 @@ static inline int nvme_translate_error(const NvmeCqe *c)
         trace_nvme_error(le32_to_cpu(c->result),
                          le16_to_cpu(c->sq_head),
                          le16_to_cpu(c->sq_id),
-                         le16_to_cpu(c->cid),
+                         c->cid,
                          le16_to_cpu(status));
     }
     switch (status) {
@@ -400,7 +400,7 @@ static bool nvme_process_completion(NVMeQueuePair *q)
         if (!q->cq.head) {
             q->cq_phase = !q->cq_phase;
         }
-        cid = le16_to_cpu(c->cid);
+        cid = c->cid;
         if (cid == 0 || cid > NVME_QUEUE_SIZE) {
             warn_report("NVMe: Unexpected CID in completion queue: %"PRIu32", "
                         "queue size: %u", cid, NVME_QUEUE_SIZE);
@@ -468,7 +468,7 @@ static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,
     assert(!req->cb);
     req->cb = cb;
     req->opaque = opaque;
-    cmd->cid = cpu_to_le32(req->cid);
+    cmd->cid = req->cid;
 
     trace_nvme_submit_command(q->s, q->index, req->cid);
     nvme_trace_command(cmd);
-- 
2.26.2

Re: [PATCH-for-5.2] block/nvme: Fix nvme_submit_command() on big-endian host

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

On Tue, Oct 27, 2020 at 05:04:07PM +0100, Philippe Mathieu-Daudé wrote:
> The Completion Queue Command Identifier is a 16-bit value,
> so nvme_submit_command() is unlikely to work on big-endian
> hosts, as the relevant bits are truncated.
> 
> The "Completion Queue Entry: DW 2" describes it as:
> 
>   This identifier is assigned by host software when
>   the command is submitted to the Submission
> 
> As the is just an opaque cookie, it is pointless to byte-swap it.

The code does not make it clear that the missing byteswap is
intentional. Please either fix the byteswap (32 -> 16) or add code
comments explaining why the byteswap is not necessary.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]