From 8065ab2c2790c453c15ac526dac7d4a418515063 Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Thu, 19 Mar 2020 01:04:05 -0400 Subject: [PATCH] nvme/pcie: pass dword aligned requirement based on controller flag We only set the flag to false when the controller reports SGL supported and can use byte contiguous buffer. Also check the data block's alignment for hardware SGL. Change-Id: Id936c49823963000d0543fc95fbb6edba3118feb Signed-off-by: Changpeng Liu Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1352 Tested-by: SPDK CI Jenkins Reviewed-by: Aleksey Marchuk Reviewed-by: Ben Walker Reviewed-by: Shuhei Matsumoto --- lib/nvme/nvme_pcie.c | 42 ++++++++++++++----- test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c | 6 +-- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/lib/nvme/nvme_pcie.c b/lib/nvme/nvme_pcie.c index 46c4bc314..d5c476031 100644 --- a/lib/nvme/nvme_pcie.c +++ b/lib/nvme/nvme_pcie.c @@ -1837,7 +1837,7 @@ nvme_pcie_prp_list_append(struct nvme_tracker *tr, uint32_t *prp_index, void *vi static int nvme_pcie_qpair_build_request_invalid(struct spdk_nvme_qpair *qpair, - struct nvme_request *req, struct nvme_tracker *tr) + struct nvme_request *req, struct nvme_tracker *tr, bool dword_aligned) { assert(0); nvme_pcie_fail_request_bad_vtophys(qpair, tr); @@ -1849,7 +1849,7 @@ nvme_pcie_qpair_build_request_invalid(struct spdk_nvme_qpair *qpair, */ static int nvme_pcie_qpair_build_contig_request(struct spdk_nvme_qpair *qpair, struct nvme_request *req, - struct nvme_tracker *tr) + struct nvme_tracker *tr, bool dword_aligned) { uint32_t prp_index = 0; int rc; @@ -1871,7 +1871,7 @@ nvme_pcie_qpair_build_contig_request(struct spdk_nvme_qpair *qpair, struct nvme_ */ static int nvme_pcie_qpair_build_contig_hw_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_request *req, - struct nvme_tracker *tr) + struct nvme_tracker *tr, bool dword_aligned) { void *virt_addr; uint64_t phys_addr, mapping_length; @@ -1896,6 +1896,12 @@ nvme_pcie_qpair_build_contig_hw_sgl_request(struct spdk_nvme_qpair *qpair, struc return -EFAULT; } + if (dword_aligned && ((uintptr_t)virt_addr & 3)) { + SPDK_ERRLOG("virt_addr %p not dword aligned\n", virt_addr); + nvme_pcie_fail_request_bad_vtophys(qpair, tr); + return -EFAULT; + } + phys_addr = spdk_vtophys(virt_addr, &mapping_length); if (phys_addr == SPDK_VTOPHYS_ERROR) { nvme_pcie_fail_request_bad_vtophys(qpair, tr); @@ -1943,7 +1949,7 @@ nvme_pcie_qpair_build_contig_hw_sgl_request(struct spdk_nvme_qpair *qpair, struc */ static int nvme_pcie_qpair_build_hw_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_request *req, - struct nvme_tracker *tr) + struct nvme_tracker *tr, bool dword_aligned) { int rc; void *virt_addr; @@ -1979,14 +1985,18 @@ nvme_pcie_qpair_build_hw_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_ remaining_transfer_len -= remaining_user_sge_len; while (remaining_user_sge_len > 0) { if (nseg >= NVME_MAX_SGL_DESCRIPTORS) { - nvme_pcie_fail_request_bad_vtophys(qpair, tr); - return -EFAULT; + SPDK_ERRLOG("Too many SGL entries\n"); + goto exit; + } + + if (dword_aligned && ((uintptr_t)virt_addr & 3)) { + SPDK_ERRLOG("virt_addr %p not dword aligned\n", virt_addr); + goto exit; } phys_addr = spdk_vtophys(virt_addr, NULL); if (phys_addr == SPDK_VTOPHYS_ERROR) { - nvme_pcie_fail_request_bad_vtophys(qpair, tr); - return -EFAULT; + goto exit; } length = spdk_min(remaining_user_sge_len, VALUE_2MB - _2MB_OFFSET(virt_addr)); @@ -2030,6 +2040,10 @@ nvme_pcie_qpair_build_hw_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_ } return 0; + +exit: + nvme_pcie_fail_request_bad_vtophys(qpair, tr); + return -EFAULT; } /** @@ -2037,7 +2051,7 @@ nvme_pcie_qpair_build_hw_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_ */ static int nvme_pcie_qpair_build_prps_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_request *req, - struct nvme_tracker *tr) + struct nvme_tracker *tr, bool dword_aligned) { int rc; void *virt_addr; @@ -2084,7 +2098,8 @@ nvme_pcie_qpair_build_prps_sgl_request(struct spdk_nvme_qpair *qpair, struct nvm return 0; } -typedef int(*build_req_fn)(struct spdk_nvme_qpair *, struct nvme_request *, struct nvme_tracker *); +typedef int(*build_req_fn)(struct spdk_nvme_qpair *, struct nvme_request *, struct nvme_tracker *, + bool); static build_req_fn const g_nvme_pcie_build_req_table[][2] = { [NVME_PAYLOAD_TYPE_INVALID] = { @@ -2111,6 +2126,7 @@ nvme_pcie_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_reques struct nvme_pcie_qpair *pqpair = nvme_pcie_qpair(qpair); enum nvme_payload_type payload_type; bool sgl_supported; + bool dword_aligned = true; if (spdk_unlikely(nvme_qpair_is_admin_queue(qpair))) { nvme_robust_mutex_lock(&ctrlr->ctrlr_lock); @@ -2151,7 +2167,11 @@ nvme_pcie_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_reques */ sgl_supported = (ctrlr->flags & SPDK_NVME_CTRLR_SGL_SUPPORTED) != 0 && !nvme_qpair_is_admin_queue(qpair); - rc = g_nvme_pcie_build_req_table[payload_type][sgl_supported](qpair, req, tr); + + if (sgl_supported && !(ctrlr->flags & SPDK_NVME_CTRLR_SGL_REQUIRES_DWORD_ALIGNMENT)) { + dword_aligned = false; + } + rc = g_nvme_pcie_build_req_table[payload_type][sgl_supported](qpair, req, tr, dword_aligned); } if (rc < 0) { diff --git a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c index 3532fa9ec..478b19ffa 100644 --- a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c +++ b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c @@ -418,7 +418,7 @@ test_build_contig_hw_sgl_request(void) g_vtophys_size = 100; MOCK_SET(spdk_vtophys, 0xDEADBEEF); - rc = nvme_pcie_qpair_build_contig_hw_sgl_request(&qpair, &req, &tr); + rc = nvme_pcie_qpair_build_contig_hw_sgl_request(&qpair, &req, &tr, 0); CU_ASSERT(rc == 0); CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.type == SPDK_NVME_SGL_TYPE_DATA_BLOCK); CU_ASSERT(req.cmd.dptr.sgl1.address == 0xDEADBEEF); @@ -437,7 +437,7 @@ test_build_contig_hw_sgl_request(void) g_vtophys_size = 1000; MOCK_SET(spdk_vtophys, 0xDEADBEEF); - rc = nvme_pcie_qpair_build_contig_hw_sgl_request(&qpair, &req, &tr); + rc = nvme_pcie_qpair_build_contig_hw_sgl_request(&qpair, &req, &tr, 0); CU_ASSERT(rc == 0); CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.type == SPDK_NVME_SGL_TYPE_DATA_BLOCK); CU_ASSERT(req.cmd.dptr.sgl1.address == 0xDEADBEEF); @@ -456,7 +456,7 @@ test_build_contig_hw_sgl_request(void) tr.prp_sgl_bus_addr = 0xFF0FF; MOCK_SET(spdk_vtophys, 0xDEADBEEF); - rc = nvme_pcie_qpair_build_contig_hw_sgl_request(&qpair, &req, &tr); + rc = nvme_pcie_qpair_build_contig_hw_sgl_request(&qpair, &req, &tr, 0); CU_ASSERT(rc == 0); CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.type == SPDK_NVME_SGL_TYPE_LAST_SEGMENT); CU_ASSERT(req.cmd.dptr.sgl1.address == tr.prp_sgl_bus_addr);