From e93449910a458c6473db1e3bafedef38a77f6523 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 1 Nov 2019 11:16:43 -0700 Subject: [PATCH] nvme: use -EFAULT for vtophys-related failures Currently we have a mix of -1 and -EINVAL which is confusing, especially since these types of failures also result in the caller's callback routine getting invoked. While here, document this new -EFAULT return code for all of the functions that could return it. Fixes issue #797. Signed-off-by: Jim Harris Change-Id: I8dfbba0ec0b83db0f2ec055b15830981af1965df Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473054 Community-CI: Broadcom SPDK FC-NVMe CI Tested-by: SPDK CI Jenkins Reviewed-by: yidong0635 Reviewed-by: Shuhei Matsumoto Reviewed-by: Tomasz Zawadzki --- include/spdk/nvme.h | 38 +++++++++++++++++++ lib/nvme/nvme_pcie.c | 16 ++++---- test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c | 10 ++--- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/include/spdk/nvme.h b/include/spdk/nvme.h index 269b250d53..99faf785b9 100644 --- a/include/spdk/nvme.h +++ b/include/spdk/nvme.h @@ -1179,6 +1179,8 @@ int spdk_nvme_ctrlr_io_cmd_raw_no_payload_build(struct spdk_nvme_ctrlr *ctrlr, * \return 0 if successfully submitted, negated errnos on the following error conditions: * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ctrlr_cmd_io_raw(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_qpair *qpair, @@ -1213,6 +1215,8 @@ int spdk_nvme_ctrlr_cmd_io_raw(struct spdk_nvme_ctrlr *ctrlr, * \return 0 if successfully submitted, negated errnos on the following error conditions: * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ctrlr_cmd_io_raw_with_md(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_qpair *qpair, @@ -1947,6 +1951,8 @@ typedef int (*spdk_nvme_req_next_sge_cb)(void *cb_arg, void **address, uint32_t * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_write(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, void *payload, uint64_t lba, uint32_t lba_count, spdk_nvme_cmd_cb cb_fn, @@ -1974,6 +1980,8 @@ int spdk_nvme_ns_cmd_write(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpai * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_writev(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, uint64_t lba, uint32_t lba_count, @@ -2007,6 +2015,8 @@ int spdk_nvme_ns_cmd_writev(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpa * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_writev_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, uint64_t lba, uint32_t lba_count, @@ -2040,6 +2050,8 @@ int spdk_nvme_ns_cmd_writev_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qp * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_write_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, void *payload, void *metadata, @@ -2067,6 +2079,8 @@ int spdk_nvme_ns_cmd_write_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpa * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_write_zeroes(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, uint64_t lba, uint32_t lba_count, @@ -2091,6 +2105,8 @@ int spdk_nvme_ns_cmd_write_zeroes(struct spdk_nvme_ns *ns, struct spdk_nvme_qpai * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_write_uncorrectable(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, uint64_t lba, uint32_t lba_count, @@ -2116,6 +2132,8 @@ int spdk_nvme_ns_cmd_write_uncorrectable(struct spdk_nvme_ns *ns, struct spdk_nv * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_read(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, void *payload, uint64_t lba, uint32_t lba_count, spdk_nvme_cmd_cb cb_fn, @@ -2143,6 +2161,8 @@ int spdk_nvme_ns_cmd_read(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_readv(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, uint64_t lba, uint32_t lba_count, @@ -2175,6 +2195,8 @@ int spdk_nvme_ns_cmd_readv(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpai * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_readv_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, uint64_t lba, uint32_t lba_count, @@ -2207,6 +2229,8 @@ int spdk_nvme_ns_cmd_readv_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpa * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_read_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, void *payload, void *metadata, @@ -2285,6 +2309,8 @@ int spdk_nvme_ns_cmd_flush(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpai * \return 0 if successfully submitted, negated errnos on the following error conditions: * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_reservation_register(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, @@ -2313,6 +2339,8 @@ int spdk_nvme_ns_cmd_reservation_register(struct spdk_nvme_ns *ns, * \return 0 if successfully submitted, negated errnos on the following error conditions: * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_reservation_release(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, @@ -2341,6 +2369,8 @@ int spdk_nvme_ns_cmd_reservation_release(struct spdk_nvme_ns *ns, * \return 0 if successfully submitted, negated errnos on the following error conditions: * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_reservation_acquire(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, @@ -2367,6 +2397,8 @@ int spdk_nvme_ns_cmd_reservation_acquire(struct spdk_nvme_ns *ns, * \return 0 if successfully submitted, negated errnos on the following error conditions: * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_reservation_report(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, @@ -2393,6 +2425,8 @@ int spdk_nvme_ns_cmd_reservation_report(struct spdk_nvme_ns *ns, * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_compare(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, void *payload, uint64_t lba, uint32_t lba_count, spdk_nvme_cmd_cb cb_fn, @@ -2420,6 +2454,8 @@ int spdk_nvme_ns_cmd_compare(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qp * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_comparev(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, uint64_t lba, uint32_t lba_count, @@ -2451,6 +2487,8 @@ int spdk_nvme_ns_cmd_comparev(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *q * -EINVAL: The request is malformed. * -ENOMEM: The request cannot be allocated. * -ENXIO: The qpair is failed at the transport level. + * -EFAULT: Invalid address was specified as part of payload. cb_fn is also called + * with error status including dnr=1 in this case. */ int spdk_nvme_ns_cmd_compare_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, void *payload, void *metadata, diff --git a/lib/nvme/nvme_pcie.c b/lib/nvme/nvme_pcie.c index da1f03faea..61aa5852dc 100644 --- a/lib/nvme/nvme_pcie.c +++ b/lib/nvme/nvme_pcie.c @@ -1734,7 +1734,7 @@ nvme_pcie_prp_list_append(struct nvme_tracker *tr, uint32_t *prp_index, void *vi if (spdk_unlikely(((uintptr_t)virt_addr & 3) != 0)) { SPDK_ERRLOG("virt_addr %p not dword aligned\n", virt_addr); - return -EINVAL; + return -EFAULT; } i = *prp_index; @@ -1747,13 +1747,13 @@ nvme_pcie_prp_list_append(struct nvme_tracker *tr, uint32_t *prp_index, void *vi */ if (spdk_unlikely(i > SPDK_COUNTOF(tr->u.prp))) { SPDK_ERRLOG("out of PRP entries\n"); - return -EINVAL; + return -EFAULT; } phys_addr = spdk_vtophys(virt_addr, NULL); if (spdk_unlikely(phys_addr == SPDK_VTOPHYS_ERROR)) { SPDK_ERRLOG("vtophys(%p) failed\n", virt_addr); - return -EINVAL; + return -EFAULT; } if (i == 0) { @@ -1763,7 +1763,7 @@ nvme_pcie_prp_list_append(struct nvme_tracker *tr, uint32_t *prp_index, void *vi } else { if ((phys_addr & page_mask) != 0) { SPDK_ERRLOG("PRP %u not page aligned (%p)\n", i, virt_addr); - return -EINVAL; + return -EFAULT; } SPDK_DEBUGLOG(SPDK_LOG_NVME, "prp[%u] = %p\n", i - 1, (void *)phys_addr); @@ -1845,7 +1845,7 @@ nvme_pcie_qpair_build_hw_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_ &virt_addr, &remaining_user_sge_len); if (rc) { nvme_pcie_fail_request_bad_vtophys(qpair, tr); - return -1; + return -EFAULT; } remaining_user_sge_len = spdk_min(remaining_user_sge_len, remaining_transfer_len); @@ -1853,13 +1853,13 @@ nvme_pcie_qpair_build_hw_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_ while (remaining_user_sge_len > 0) { if (nseg >= NVME_MAX_SGL_DESCRIPTORS) { nvme_pcie_fail_request_bad_vtophys(qpair, tr); - return -1; + return -EFAULT; } phys_addr = spdk_vtophys(virt_addr, NULL); if (phys_addr == SPDK_VTOPHYS_ERROR) { nvme_pcie_fail_request_bad_vtophys(qpair, tr); - return -1; + return -EFAULT; } length = spdk_min(remaining_user_sge_len, VALUE_2MB - _2MB_OFFSET(virt_addr)); @@ -1929,7 +1929,7 @@ nvme_pcie_qpair_build_prps_sgl_request(struct spdk_nvme_qpair *qpair, struct nvm rc = req->payload.next_sge_fn(req->payload.contig_or_cb_arg, &virt_addr, &length); if (rc) { nvme_pcie_fail_request_bad_vtophys(qpair, tr); - return -1; + return -EFAULT; } length = spdk_min(remaining_transfer_len, length); 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 655d35ebe8..64edca2c6b 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 @@ -681,7 +681,7 @@ test_prp_list_append(void) /* Non-DWORD-aligned buffer (invalid) */ prp_list_prep(&tr, &req, &prp_index); - CU_ASSERT(nvme_pcie_prp_list_append(&tr, &prp_index, (void *)0x100001, 0x1000, 0x1000) == -EINVAL); + CU_ASSERT(nvme_pcie_prp_list_append(&tr, &prp_index, (void *)0x100001, 0x1000, 0x1000) == -EFAULT); /* 512-byte buffer, 4K aligned */ prp_list_prep(&tr, &req, &prp_index); @@ -767,13 +767,13 @@ test_prp_list_append(void) prp_list_prep(&tr, &req, &prp_index); CU_ASSERT(nvme_pcie_prp_list_append(&tr, &prp_index, (void *)0x100800, 0x1000, 0x1000) == 0); CU_ASSERT(prp_index == 2); - CU_ASSERT(nvme_pcie_prp_list_append(&tr, &prp_index, (void *)0x900800, 0x1000, 0x1000) == -EINVAL); + CU_ASSERT(nvme_pcie_prp_list_append(&tr, &prp_index, (void *)0x900800, 0x1000, 0x1000) == -EFAULT); CU_ASSERT(prp_index == 2); /* 4K buffer, 4K aligned, but vtophys fails */ MOCK_SET(spdk_vtophys, SPDK_VTOPHYS_ERROR); prp_list_prep(&tr, &req, &prp_index); - CU_ASSERT(nvme_pcie_prp_list_append(&tr, &prp_index, (void *)0x100000, 0x1000, 0x1000) == -EINVAL); + CU_ASSERT(nvme_pcie_prp_list_append(&tr, &prp_index, (void *)0x100000, 0x1000, 0x1000) == -EFAULT); MOCK_CLEAR(spdk_vtophys); /* Largest aligned buffer that can be described in NVME_MAX_PRP_LIST_ENTRIES (plus PRP1) */ @@ -791,12 +791,12 @@ test_prp_list_append(void) /* Buffer too large to be described in NVME_MAX_PRP_LIST_ENTRIES */ prp_list_prep(&tr, &req, &prp_index); CU_ASSERT(nvme_pcie_prp_list_append(&tr, &prp_index, (void *)0x100000, - (NVME_MAX_PRP_LIST_ENTRIES + 2) * 0x1000, 0x1000) == -EINVAL); + (NVME_MAX_PRP_LIST_ENTRIES + 2) * 0x1000, 0x1000) == -EFAULT); /* Non-4K-aligned buffer too large to be described in NVME_MAX_PRP_LIST_ENTRIES */ prp_list_prep(&tr, &req, &prp_index); CU_ASSERT(nvme_pcie_prp_list_append(&tr, &prp_index, (void *)0x100800, - (NVME_MAX_PRP_LIST_ENTRIES + 1) * 0x1000, 0x1000) == -EINVAL); + (NVME_MAX_PRP_LIST_ENTRIES + 1) * 0x1000, 0x1000) == -EFAULT); } static void test_shadow_doorbell_update(void)