| Message ID | 20201027160407.418423-1-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCH-for-5.2] block/nvme: Fix nvme_submit_command() on big-endian host | expand |
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.
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);
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(-)