Commit e9344991 authored by Jim Harris's avatar Jim Harris
Browse files

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: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: I8dfbba0ec0b83db0f2ec055b15830981af1965df

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473054


Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avataryidong0635 <dongx.yi@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
parent 0f481877

include/spdk/nvme.h

+38 −0
Original line number Diff line number Diff line
@@ -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,

lib/nvme/nvme_pcie.c

+8 −8
Original line number Diff line number Diff line
@@ -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);

test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c

+5 −5
Original line number Diff line number Diff line
@@ -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)