nvme: return virtual address in SGL callback

Instead of the next_sge callback returning the physical address
directly, make it return the virtual address and convert to physical
address inside the NVMe library.

This is necessary for NVMe over Fabrics host support, since the RDMA
userspace API requires virtual addresses rather than physical addresses.
It is also more consistent with the normal non-SGL NVMe functions that
already take virtual addresses.

Change-Id: I79a7af64ead987535f6bf3057b2b22aef3171c5b
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>

CHANGELOG.md

@@ -16,6 +16,10 @@ Dataset Management command's parameters.  Existing callers can be updated to use
 `spdk_nvme_ns_cmd_dataset_management()` with `SPDK_NVME_DSM_ATTR_DEALLOCATE` as the
 `type` parameter.
 
+The NVMe library SGL callback prototype has been changed to return virtual addresses
+rather than physical addresses.  Callers of `spdk_nvme_ns_cmd_readv()` and
+`spdk_nvme_ns_cmd_writev()` must update their `next_sge_fn` callbacks to match.
+
 Libpciaccess has been removed as a dependency and DPDK PCI enumeration is
 used instead. Prior to DPDK 16.07 enumeration by class code was not supported,
 so for earlier DPDK versions only Intel SSDs will be discovered. Starting with

include/spdk/nvme.h

@@ -667,10 +667,10 @@ typedef void (*spdk_nvme_req_reset_sgl_cb)(void *cb_arg, uint32_t offset);
  * entry for the next time the callback is invoked.
  *
  * The cb_arg parameter is the value passed to readv/writev.
- * The address parameter contains the physical address of this segment.
+ * The address parameter contains the virtual address of this segment.
  * The length parameter contains the length of this physical segment.
  */
-typedef int (*spdk_nvme_req_next_sge_cb)(void *cb_arg, uint64_t *address, uint32_t *length);
+typedef int (*spdk_nvme_req_next_sge_cb)(void *cb_arg, void **address, uint32_t *length);
 
 /**
  * \brief Submits a write I/O to the specified NVMe namespace.

lib/bdev/nvme/blockdev_nvme.c

@@ -640,7 +640,7 @@ queued_reset_sgl(void *ref, uint32_t sgl_offset)
 }
 
 static int
-queued_next_sge(void *ref, uint64_t *address, uint32_t *length)
+queued_next_sge(void *ref, void **address, uint32_t *length)
 {
 	struct nvme_blockio *bio = ref;
 	struct iovec *iov;
@@ -650,7 +650,7 @@ queued_next_sge(void *ref, uint64_t *address, uint32_t *length)
 	iov = &bio->iovs[bio->iovpos];
 	bio->iovpos++;
 
-	*address = spdk_vtophys(iov->iov_base);
+	*address = iov->iov_base;
 	*length = iov->iov_len;
 
 	if (bio->iov_offset) {

lib/nvme/nvme_pcie.c

@@ -1308,6 +1308,7 @@ nvme_pcie_qpair_build_hw_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_
 				     struct nvme_tracker *tr)
 {
 	int rc;
+	void *virt_addr;
 	uint64_t phys_addr;
 	uint32_t remaining_transfer_len, length;
 	struct spdk_nvme_sgl_descriptor *sgl;
@@ -1334,12 +1335,18 @@ nvme_pcie_qpair_build_hw_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_
 			return -1;
 		}
 
-		rc = req->payload.u.sgl.next_sge_fn(req->payload.u.sgl.cb_arg, &phys_addr, &length);
+		rc = req->payload.u.sgl.next_sge_fn(req->payload.u.sgl.cb_arg, &virt_addr, &length);
 		if (rc) {
 			nvme_pcie_fail_request_bad_vtophys(qpair, tr);
 			return -1;
 		}
 
+		phys_addr = spdk_vtophys(virt_addr);
+		if (phys_addr == SPDK_VTOPHYS_ERROR) {
+			nvme_pcie_fail_request_bad_vtophys(qpair, tr);
+			return -1;
+		}
+
 		length = nvme_min(remaining_transfer_len, length);
 		remaining_transfer_len -= length;
 
@@ -1380,6 +1387,7 @@ nvme_pcie_qpair_build_prps_sgl_request(struct spdk_nvme_qpair *qpair, struct nvm
 				       struct nvme_tracker *tr)
 {
 	int rc;
+	void *virt_addr;
 	uint64_t phys_addr;
 	uint32_t data_transferred, remaining_transfer_len, length;
 	uint32_t nseg, cur_nseg, total_nseg, last_nseg, modulo, unaligned;
@@ -1399,12 +1407,18 @@ nvme_pcie_qpair_build_prps_sgl_request(struct spdk_nvme_qpair *qpair, struct nvm
 
 	while (remaining_transfer_len > 0) {
 		assert(req->payload.u.sgl.next_sge_fn != NULL);
-		rc = req->payload.u.sgl.next_sge_fn(req->payload.u.sgl.cb_arg, &phys_addr, &length);
+		rc = req->payload.u.sgl.next_sge_fn(req->payload.u.sgl.cb_arg, &virt_addr, &length);
 		if (rc) {
 			nvme_pcie_fail_request_bad_vtophys(qpair, tr);
 			return -1;
 		}
 
+		phys_addr = spdk_vtophys(virt_addr);
+		if (phys_addr == SPDK_VTOPHYS_ERROR) {
+			nvme_pcie_fail_request_bad_vtophys(qpair, tr);
+			return -1;
+		}
+
 		/* Confirm that this sge is prp compatible. */
 		if (phys_addr & 0x3 ||
 		    (length < remaining_transfer_len && ((phys_addr + length) & (PAGE_SIZE - 1)))) {

test/lib/nvme/e2edp/nvme_dp.c

@@ -153,13 +153,13 @@ static void nvme_req_reset_sgl(void *cb_arg, uint32_t sgl_offset)
 	return;
 }
 
-static int nvme_req_next_sge(void *cb_arg, uint64_t *address, uint32_t *length)
+static int nvme_req_next_sge(void *cb_arg, void **address, uint32_t *length)
 {
 	struct io_request *req = (struct io_request *)cb_arg;
 	void *payload;
 
 	payload = req->contig + req->sgl_offset;
-	*address = spdk_vtophys(payload);
+	*address = payload;
 
 	*length = req->buf_size - req->sgl_offset;
 

test/lib/nvme/sgl/nvme_sgl.c

@@ -66,7 +66,6 @@ static int io_complete_flag = 0;
 
 struct sgl_element {
 	void *base;
-	uint64_t phys_addr;
 	size_t offset;
 	size_t len;
 };
@@ -97,25 +96,25 @@ static void nvme_request_reset_sgl(void *cb_arg, uint32_t sgl_offset)
 	return;
 }
 
-static int nvme_request_next_sge(void *cb_arg, uint64_t *address, uint32_t *length)
+static int nvme_request_next_sge(void *cb_arg, void **address, uint32_t *length)
 {
 	struct io_request *req = (struct io_request *)cb_arg;
 	struct sgl_element *iov;
 
 	if (req->current_iov_index >= req->nseg) {
 		*length = 0;
-		*address = 0;
+		*address = NULL;
 		return 0;
 	}
 
 	iov = &req->iovs[req->current_iov_index];
 
 	if (req->current_iov_bytes_left) {
-		*address = iov->phys_addr + iov->len - req->current_iov_bytes_left;
+		*address = iov->base + iov->offset + iov->len - req->current_iov_bytes_left;
 		*length = req->current_iov_bytes_left;
 		req->current_iov_bytes_left = 0;
 	} else {
-		*address = iov->phys_addr;
+		*address = iov->base + iov->offset;
 		*length = iov->len;
 	}
 
@@ -288,7 +287,6 @@ writev_readv_tests(struct dev *dev, nvme_build_io_req_fn_t build_io_fn, const ch
 	for (i = 0; i < req->nseg; i++) {
 		struct sgl_element *sge = &req->iovs[i];
 
-		sge->phys_addr = spdk_vtophys(sge->base) + sge->offset;
 		len += sge->len;
 	}
 

test/lib/nvme/unit/nvme_ns_cmd_c/nvme_ns_cmd_ut.c

@@ -51,7 +51,7 @@ static void nvme_request_reset_sgl(void *cb_arg, uint32_t sgl_offset)
 {
 }
 
-static int nvme_request_next_sge(void *cb_arg, uint64_t *address, uint32_t *length)
+static int nvme_request_next_sge(void *cb_arg, void **address, uint32_t *length)
 {
 	return 0;
 }

test/lib/nvme/unit/nvme_qpair_c/nvme_qpair_ut.c

@@ -88,24 +88,24 @@ static void nvme_request_reset_sgl(void *cb_arg, uint32_t sgl_offset)
 	return;
 }
 
-static int nvme_request_next_sge(void *cb_arg, uint64_t *address, uint32_t *length)
+static int nvme_request_next_sge(void *cb_arg, void **address, uint32_t *length)
 {
 	struct io_request *req = (struct io_request *)cb_arg;
 
 	if (req->address_offset == 0) {
 		if (req->invalid_addr) {
-			*address = 7;
+			*address = (void *)7;
 		} else {
-			*address = 4096 * req->address_offset;
+			*address = (void *)(4096 * req->address_offset);
 		}
 	} else if (req->address_offset == 1) {
 		if (req->invalid_second_addr) {
-			*address = 7;
+			*address = (void *)7;
 		} else {
-			*address = 4096 * req->address_offset;
+			*address = (void *)(4096 * req->address_offset);
 		}
 	} else {
-		*address = 4096 * req->address_offset;
+		*address = (void *)(4096 * req->address_offset);
 	}
 
 	req->address_offset += 1;