Merge struct nvme_prp_list into struct nvme_tracker.

This simplifies the driver significantly where it is constructing
commands to be submitted to hardware.  By reducing the number of
PRPs (NVMe parlance for SGE) from 128 to 32, it ensures we do not
allocate too much memory for more common smaller I/O sizes, while
still supporting up to 128KB I/O sizes.

This also paves the way for pre-allocation of nvme_tracker objects
for each queue which will simplify the I/O path even further.

Sponsored by:	Intel

7 files changed, 36 insertions, 98 deletions

diff --git a/sys/dev/nvme/nvme.c b/sys/dev/nvme/nvme.c
index a3f3bcd..e2621ed 100644
--- a/sys/dev/nvme/nvme.c
+++ b/sys/dev/nvme/nvme.c
@@ -211,7 +211,6 @@ nvme_payload_map(void *arg, bus_dma_segment_t *seg, int nseg, int error)
 {
 	struct nvme_tracker 	*tr;
 	struct nvme_qpair 	*qpair;
-	struct nvme_prp_list	*prp_list;
 	uint32_t		cur_nseg;
 
 	KASSERT(error == 0, ("nvme_payload_map error != 0\n"));
@@ -230,13 +229,10 @@ nvme_payload_map(void *arg, bus_dma_segment_t *seg, int nseg, int error)
 	if (nseg == 2) {
 		tr->cmd.prp2 = seg[1].ds_addr;
 	} else if (nseg > 2) {
-		KASSERT(tr->prp_list,
-		    ("prp_list needed but not attached to tracker\n"));
 		cur_nseg = 1;
-		prp_list = tr->prp_list;
-		tr->cmd.prp2 = (uint64_t)prp_list->bus_addr;
+		tr->cmd.prp2 = (uint64_t)tr->prp_bus_addr;
 		while (cur_nseg < nseg) {
-			prp_list->prp[cur_nseg-1] =
+			tr->prp[cur_nseg-1] =
 			    (uint64_t)seg[cur_nseg].ds_addr;
 			cur_nseg++;
 		}
@@ -251,8 +247,6 @@ nvme_allocate_tracker(struct nvme_controller *ctrlr, boolean_t is_admin,
 {
 	struct nvme_tracker 	*tr;
 	struct nvme_qpair	*qpair;
-	uint32_t 		modulo, offset, num_prps;
-	boolean_t		alloc_prp_list = FALSE;
 
 	if (is_admin) {
 		qpair = &ctrlr->adminq;
@@ -263,17 +257,7 @@ nvme_allocate_tracker(struct nvme_controller *ctrlr, boolean_t is_admin,
 			qpair = &ctrlr->ioq[0];
 	}
 
-	num_prps = payload_size / PAGE_SIZE;
-	modulo = payload_size % PAGE_SIZE;
-	offset = (uint32_t)((uintptr_t)payload % PAGE_SIZE);
-
-	if (modulo || offset)
-		num_prps += 1 + (modulo + offset - 1) / PAGE_SIZE;
-
-	if (num_prps > 2)
-		alloc_prp_list = TRUE;
-
-	tr = nvme_qpair_allocate_tracker(qpair, alloc_prp_list);
+	tr = nvme_qpair_allocate_tracker(qpair);
 
 	if (tr == NULL)
 		return (NULL);
diff --git a/sys/dev/nvme/nvme_ctrlr_cmd.c b/sys/dev/nvme/nvme_ctrlr_cmd.c
index c145975..04b33bf 100644
--- a/sys/dev/nvme/nvme_ctrlr_cmd.c
+++ b/sys/dev/nvme/nvme_ctrlr_cmd.c
@@ -49,7 +49,7 @@ nvme_ctrlr_cmd_identify_controller(struct nvme_controller *ctrlr, void *payload,
 	 */
 	cmd->cdw10 = 1;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    tr->payload_size, nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
@@ -74,7 +74,7 @@ nvme_ctrlr_cmd_identify_namespace(struct nvme_controller *ctrlr, uint16_t nsid,
 	 */
 	cmd->nsid = nsid;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    tr->payload_size, nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
@@ -189,8 +189,8 @@ nvme_ctrlr_cmd_set_feature(struct nvme_controller *ctrlr, uint8_t feature,
 	cmd->cdw11 = cdw11;
 
 	if (payload_size > 0) {
-		err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
-		    payload_size, nvme_payload_map, tr, 0);
+		err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map,
+		    payload, payload_size, nvme_payload_map, tr, 0);
 
 		KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
 	} else
@@ -215,8 +215,8 @@ nvme_ctrlr_cmd_get_feature(struct nvme_controller *ctrlr, uint8_t feature,
 	cmd->cdw11 = cdw11;
 
 	if (payload_size > 0) {
-		err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
-		    payload_size, nvme_payload_map, tr, 0);
+		err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map,
+		    payload, payload_size, nvme_payload_map, tr, 0);
 
 		KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
 	} else
@@ -305,7 +305,7 @@ nvme_ctrlr_cmd_get_health_information_page(struct nvme_controller *ctrlr,
 	cmd->cdw10 = ((sizeof(*payload)/sizeof(uint32_t)) - 1) << 16;
 	cmd->cdw10 |= NVME_LOG_HEALTH_INFORMATION;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    sizeof(*payload), nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
diff --git a/sys/dev/nvme/nvme_ns_cmd.c b/sys/dev/nvme/nvme_ns_cmd.c
index fe5cc28..cba9032 100644
--- a/sys/dev/nvme/nvme_ns_cmd.c
+++ b/sys/dev/nvme/nvme_ns_cmd.c
@@ -51,7 +51,7 @@ nvme_ns_cmd_read(struct nvme_namespace *ns, void *payload, uint64_t lba,
 	*(uint64_t *)&cmd->cdw10 = lba;
 	cmd->cdw12 = lba_count-1;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    tr->payload_size, nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
@@ -81,7 +81,7 @@ nvme_ns_cmd_write(struct nvme_namespace *ns, void *payload, uint64_t lba,
 	*(uint64_t *)&cmd->cdw10 = lba;
 	cmd->cdw12 = lba_count-1;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    tr->payload_size, nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
@@ -111,7 +111,7 @@ nvme_ns_cmd_deallocate(struct nvme_namespace *ns, void *payload,
 	cmd->cdw10 = num_ranges;
 	cmd->cdw11 = NVME_DSM_ATTR_DEALLOCATE;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    tr->payload_size, nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
diff --git a/sys/dev/nvme/nvme_private.h b/sys/dev/nvme/nvme_private.h
index df182f7..835b3cb 100644
--- a/sys/dev/nvme/nvme_private.h
+++ b/sys/dev/nvme/nvme_private.h
@@ -55,14 +55,14 @@ MALLOC_DECLARE(M_NVME);
 
 #define IDT_PCI_ID		0x80d0111d
 
-#define NVME_MAX_PRP_LIST_ENTRIES	(128)
+#define NVME_MAX_PRP_LIST_ENTRIES	(32)
 
 /*
  * For commands requiring more than 2 PRP entries, one PRP will be
  *  embedded in the command (prp1), and the rest of the PRP entries
  *  will be in a list pointed to by the command (prp2).  This means
- *  that real max number of PRP entries we support is 128+1, which
- *  results in a max xfer size of 128*PAGE_SIZE.
+ *  that real max number of PRP entries we support is 32+1, which
+ *  results in a max xfer size of 32*PAGE_SIZE.
  */
 #define NVME_MAX_XFER_SIZE	NVME_MAX_PRP_LIST_ENTRIES * PAGE_SIZE
 
@@ -92,25 +92,21 @@ MALLOC_DECLARE(M_NVME);
 #define CACHE_LINE_SIZE		(64)
 #endif
 
-struct nvme_prp_list {
-	uint64_t			prp[NVME_MAX_PRP_LIST_ENTRIES];
-	SLIST_ENTRY(nvme_prp_list)	slist;
-	bus_addr_t			bus_addr;
-	bus_dmamap_t			dma_map;
-};
-
 struct nvme_tracker {
 
 	SLIST_ENTRY(nvme_tracker)	slist;
 	struct nvme_qpair		*qpair;
 	struct nvme_command		cmd;
 	struct callout			timer;
-	bus_dmamap_t			dma_map;
+	bus_dmamap_t			payload_dma_map;
 	nvme_cb_fn_t			cb_fn;
 	void				*cb_arg;
 	uint32_t			payload_size;
-	struct nvme_prp_list		*prp_list;
 	uint16_t			cid;
+
+	uint64_t			prp[NVME_MAX_PRP_LIST_ENTRIES];
+	bus_addr_t			prp_bus_addr;
+	bus_dmamap_t			prp_dma_map;
 };
 
 struct nvme_qpair {
@@ -148,14 +144,11 @@ struct nvme_qpair {
 	uint64_t		cpl_bus_addr;
 
 	uint32_t		num_tr;
-	uint32_t		num_prp_list;
 
 	SLIST_HEAD(, nvme_tracker)	free_tr;
 
 	struct nvme_tracker	**act_tr;
 
-	SLIST_HEAD(, nvme_prp_list)	free_prp_list;
-
 	struct mtx		lock __aligned(CACHE_LINE_SIZE);
 
 } __aligned(CACHE_LINE_SIZE);
@@ -347,8 +340,7 @@ void	nvme_qpair_construct(struct nvme_qpair *qpair, uint32_t id,
 void	nvme_qpair_submit_cmd(struct nvme_qpair *qpair,
 			      struct nvme_tracker *tr);
 void	nvme_qpair_process_completions(struct nvme_qpair *qpair);
-struct nvme_tracker *	nvme_qpair_allocate_tracker(struct nvme_qpair *qpair,
-						    boolean_t alloc_prp_list);
+struct nvme_tracker *	nvme_qpair_allocate_tracker(struct nvme_qpair *qpair);
 
 void	nvme_admin_qpair_destroy(struct nvme_qpair *qpair);
 
diff --git a/sys/dev/nvme/nvme_qpair.c b/sys/dev/nvme/nvme_qpair.c
index efe5055..8d416e3 100644
--- a/sys/dev/nvme/nvme_qpair.c
+++ b/sys/dev/nvme/nvme_qpair.c
@@ -75,10 +75,9 @@ nvme_completion_check_retry(const struct nvme_completion *cpl)
 }
 
 struct nvme_tracker *
-nvme_qpair_allocate_tracker(struct nvme_qpair *qpair, boolean_t alloc_prp_list)
+nvme_qpair_allocate_tracker(struct nvme_qpair *qpair)
 {
 	struct nvme_tracker	*tr;
-	struct nvme_prp_list	*prp_list;
 
 	mtx_lock(&qpair->lock);
 
@@ -102,33 +101,17 @@ nvme_qpair_allocate_tracker(struct nvme_qpair *qpair, boolean_t alloc_prp_list)
 			return (NULL);
 		}
 
-		bus_dmamap_create(qpair->dma_tag, 0, &tr->dma_map);
+		bus_dmamap_create(qpair->dma_tag, 0, &tr->payload_dma_map);
+		bus_dmamap_create(qpair->dma_tag, 0, &tr->prp_dma_map);
+
+		bus_dmamap_load(qpair->dma_tag, tr->prp_dma_map, tr->prp,
+		    sizeof(tr->prp), nvme_single_map, &tr->prp_bus_addr, 0);
+
 		callout_init_mtx(&tr->timer, &qpair->lock, 0);
 		tr->cid = qpair->num_tr++;
 	} else
 		SLIST_REMOVE_HEAD(&qpair->free_tr, slist);
 
-	if (alloc_prp_list) {
-		prp_list = SLIST_FIRST(&qpair->free_prp_list);
-
-		if (prp_list == NULL) {
-			prp_list = malloc(sizeof(struct nvme_prp_list),
-			    M_NVME, M_ZERO | M_NOWAIT);
-
-			bus_dmamap_create(qpair->dma_tag, 0, &prp_list->dma_map);
-
-			bus_dmamap_load(qpair->dma_tag, prp_list->dma_map,
-			    prp_list->prp, sizeof(struct nvme_prp_list),
-			    nvme_single_map, &prp_list->bus_addr, 0);
-
-			qpair->num_prp_list++;
-		} else {
-			SLIST_REMOVE_HEAD(&qpair->free_prp_list, slist);
-		}
-
-		tr->prp_list = prp_list;
-	}
-
 	return (tr);
 }
 
@@ -176,14 +159,9 @@ nvme_qpair_process_completions(struct nvme_qpair *qpair)
 			/* nvme_qpair_submit_cmd() will release the lock. */
 			nvme_qpair_submit_cmd(qpair, tr);
 		else {
-			if (tr->prp_list) {
-				SLIST_INSERT_HEAD(&qpair->free_prp_list,
-				    tr->prp_list, slist);
-				tr->prp_list = NULL;
-			}
-
 			if (tr->payload_size > 0)
-				bus_dmamap_unload(qpair->dma_tag, tr->dma_map);
+				bus_dmamap_unload(qpair->dma_tag,
+				    tr->payload_dma_map);
 
 			SLIST_INSERT_HEAD(&qpair->free_tr, tr, slist);
 
@@ -256,7 +234,6 @@ nvme_qpair_construct(struct nvme_qpair *qpair, uint32_t id,
 	qpair->num_cmds = 0;
 	qpair->num_intr_handler_calls = 0;
 	qpair->num_tr = 0;
-	qpair->num_prp_list = 0;
 	qpair->sq_head = qpair->sq_tail = qpair->cq_head = 0;
 
 	/* TODO: error checking on contigmalloc, bus_dmamap_load calls */
@@ -281,7 +258,6 @@ nvme_qpair_construct(struct nvme_qpair *qpair, uint32_t id,
 	qpair->cq_hdbl_off = nvme_mmio_offsetof(doorbell[id].cq_hdbl);
 
 	SLIST_INIT(&qpair->free_tr);
-	SLIST_INIT(&qpair->free_prp_list);
 
 	qpair->act_tr = malloc(sizeof(struct nvme_tracker *) * qpair->num_entries,
 	    M_NVME, M_ZERO | M_NOWAIT);
@@ -291,7 +267,6 @@ static void
 nvme_qpair_destroy(struct nvme_qpair *qpair)
 {
 	struct nvme_tracker *tr;
-	struct nvme_prp_list *prp_list;
 
 	if (qpair->tag)
 		bus_teardown_intr(qpair->ctrlr->dev, qpair->res, qpair->tag);
@@ -309,16 +284,10 @@ nvme_qpair_destroy(struct nvme_qpair *qpair)
 	while (!SLIST_EMPTY(&qpair->free_tr)) {
 		tr = SLIST_FIRST(&qpair->free_tr);
 		SLIST_REMOVE_HEAD(&qpair->free_tr, slist);
-		bus_dmamap_destroy(qpair->dma_tag, tr->dma_map);
+		bus_dmamap_destroy(qpair->dma_tag, tr->payload_dma_map);
+		bus_dmamap_destroy(qpair->dma_tag, tr->prp_dma_map);
 		free(tr, M_NVME);
 	}
-
-	while (!SLIST_EMPTY(&qpair->free_prp_list)) {
-		prp_list = SLIST_FIRST(&qpair->free_prp_list);
-		SLIST_REMOVE_HEAD(&qpair->free_prp_list, slist);
-		bus_dmamap_destroy(qpair->dma_tag, prp_list->dma_map);
-		free(prp_list, M_NVME);
-	}
 }
 
 void
diff --git a/sys/dev/nvme/nvme_sysctl.c b/sys/dev/nvme/nvme_sysctl.c
index ecf8b6e..9cfdcdd 100644
--- a/sys/dev/nvme/nvme_sysctl.c
+++ b/sys/dev/nvme/nvme_sysctl.c
@@ -195,9 +195,6 @@ nvme_sysctl_initialize_queue(struct nvme_qpair *qpair,
 	SYSCTL_ADD_UINT(ctrlr_ctx, que_list, OID_AUTO, "num_tr",
 	    CTLFLAG_RD, &qpair->num_tr, 0,
 	    "Number of trackers allocated");
-	SYSCTL_ADD_UINT(ctrlr_ctx, que_list, OID_AUTO, "num_prp_list",
-	    CTLFLAG_RD, &qpair->num_prp_list, 0,
-	    "Number of PRP lists allocated");
 	SYSCTL_ADD_UINT(ctrlr_ctx, que_list, OID_AUTO, "sq_head",
 	    CTLFLAG_RD, &qpair->sq_head, 0,
 	    "Current head of submission queue (as observed by driver)");
diff --git a/sys/dev/nvme/nvme_uio.c b/sys/dev/nvme/nvme_uio.c
index 354fac9..5492ba8 100644
--- a/sys/dev/nvme/nvme_uio.c
+++ b/sys/dev/nvme/nvme_uio.c
@@ -58,11 +58,7 @@ nvme_allocate_tracker_uio(struct nvme_controller *ctrlr, struct uio *uio)
 	else
 		qpair = &ctrlr->ioq[0];
 
-	/*
-	 * For uio, always allocate a PRP list, rather than walking
-	 *  the iovecs.
-	 */
-	tr = nvme_qpair_allocate_tracker(qpair, TRUE /* alloc_prp_list */);
+	tr = nvme_qpair_allocate_tracker(qpair);
 
 	if (tr == NULL)
 		return (NULL);
@@ -109,7 +105,7 @@ nvme_read_uio(struct nvme_namespace *ns, struct uio *uio)
 
 	cmd->cdw12 = (iosize / nvme_ns_get_sector_size(ns))-1;
 
-	err = bus_dmamap_load_uio(tr->qpair->dma_tag, tr->dma_map, uio,
+	err = bus_dmamap_load_uio(tr->qpair->dma_tag, tr->payload_dma_map, uio,
 	    nvme_payload_map_uio, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load_uio returned non-zero!\n"));
@@ -143,7 +139,7 @@ nvme_write_uio(struct nvme_namespace *ns, struct uio *uio)
 
 	cmd->cdw12 = (iosize / nvme_ns_get_sector_size(ns))-1;
 
-	err = bus_dmamap_load_uio(tr->qpair->dma_tag, tr->dma_map, uio,
+	err = bus_dmamap_load_uio(tr->qpair->dma_tag, tr->payload_dma_map, uio,
 	    nvme_payload_map_uio, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load_uio returned non-zero!\n"));