Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 29 Jun 2020 00:31:38 +0000 (UTC)
From:      Chuck Tuffli <chuck@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r362752 - head/usr.sbin/bhyve
Message-ID:  <202006290031.05T0VcBV047243@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: chuck
Date: Mon Jun 29 00:31:37 2020
New Revision: 362752
URL: https://svnweb.freebsd.org/changeset/base/362752

Log:
  bhyve: fix NVMe queue creation and deletion
  
  Add checks for various types of invalid I/O Queue Create and Delete
  command parameters, including:
   - QID=0
   - QID>MAX
   - QID already in use
   - Delete an Active CQ
   - Invalid QSIZE
   - Invalid CQID (SQ creation)
   - Invalid interrupt vector (CQ creation)
  
  Fixes UNH Tests 1.4.2-5,7-8
  
  Tested by:	Jason Tubnor
  MFC after:	2 weeks
  Differential Revision: https://reviews.freebsd.org/D24886

Modified:
  head/usr.sbin/bhyve/pci_nvme.c

Modified: head/usr.sbin/bhyve/pci_nvme.c
==============================================================================
--- head/usr.sbin/bhyve/pci_nvme.c	Mon Jun 29 00:31:34 2020	(r362751)
+++ head/usr.sbin/bhyve/pci_nvme.c	Mon Jun 29 00:31:37 2020	(r362752)
@@ -705,7 +705,8 @@ nvme_opc_delete_io_sq(struct pci_nvme_softc* sc, struc
 	uint16_t qid = command->cdw10 & 0xffff;
 
 	DPRINTF("%s DELETE_IO_SQ %u", __func__, qid);
-	if (qid == 0 || qid > sc->num_squeues) {
+	if (qid == 0 || qid > sc->num_squeues ||
+	    (sc->submit_queues[qid].qbase == NULL)) {
 		WPRINTF("%s NOT PERMITTED queue id %u / num_squeues %u",
 		        __func__, qid, sc->num_squeues);
 		pci_nvme_status_tc(&compl->status, NVME_SCT_COMMAND_SPECIFIC,
@@ -714,6 +715,7 @@ nvme_opc_delete_io_sq(struct pci_nvme_softc* sc, struc
 	}
 
 	sc->submit_queues[qid].qbase = NULL;
+	sc->submit_queues[qid].cqid = 0;
 	pci_nvme_status_genc(&compl->status, NVME_SC_SUCCESS);
 	return (1);
 }
@@ -726,7 +728,8 @@ nvme_opc_create_io_sq(struct pci_nvme_softc* sc, struc
 		uint16_t qid = command->cdw10 & 0xffff;
 		struct nvme_submission_queue *nsq;
 
-		if ((qid == 0) || (qid > sc->num_squeues)) {
+		if ((qid == 0) || (qid > sc->num_squeues) ||
+		    (sc->submit_queues[qid].qbase != NULL)) {
 			WPRINTF("%s queue index %u > num_squeues %u",
 			        __func__, qid, sc->num_squeues);
 			pci_nvme_status_tc(&compl->status,
@@ -737,12 +740,39 @@ nvme_opc_create_io_sq(struct pci_nvme_softc* sc, struc
 
 		nsq = &sc->submit_queues[qid];
 		nsq->size = ONE_BASED((command->cdw10 >> 16) & 0xffff);
+		DPRINTF("%s size=%u (max=%u)", __func__, nsq->size, sc->max_qentries);
+		if ((nsq->size < 2) || (nsq->size > sc->max_qentries)) {
+			/*
+			 * Queues must specify at least two entries
+			 * NOTE: "MAXIMUM QUEUE SIZE EXCEEDED" was renamed to
+			 * "INVALID QUEUE SIZE" in the NVM Express 1.3 Spec
+			 */
+			pci_nvme_status_tc(&compl->status,
+			    NVME_SCT_COMMAND_SPECIFIC,
+			    NVME_SC_MAXIMUM_QUEUE_SIZE_EXCEEDED);
+			return (1);
+		}
 
-		nsq->qbase = vm_map_gpa(sc->nsc_pi->pi_vmctx, command->prp1,
-		              sizeof(struct nvme_command) * (size_t)nsq->size);
 		nsq->cqid = (command->cdw11 >> 16) & 0xffff;
+		if ((nsq->cqid == 0) || (nsq->cqid > sc->num_cqueues)) {
+			pci_nvme_status_tc(&compl->status,
+			    NVME_SCT_COMMAND_SPECIFIC,
+			    NVME_SC_INVALID_QUEUE_IDENTIFIER);
+			return (1);
+		}
+
+		if (sc->compl_queues[nsq->cqid].qbase == NULL) {
+			pci_nvme_status_tc(&compl->status,
+			    NVME_SCT_COMMAND_SPECIFIC,
+			    NVME_SC_COMPLETION_QUEUE_INVALID);
+			return (1);
+		}
+
 		nsq->qpriority = (command->cdw11 >> 1) & 0x03;
 
+		nsq->qbase = vm_map_gpa(sc->nsc_pi->pi_vmctx, command->prp1,
+		              sizeof(struct nvme_command) * (size_t)nsq->size);
+
 		DPRINTF("%s sq %u size %u gaddr %p cqid %u", __func__,
 		        qid, nsq->size, nsq->qbase, nsq->cqid);
 
@@ -768,9 +798,11 @@ nvme_opc_delete_io_cq(struct pci_nvme_softc* sc, struc
 	struct nvme_completion* compl)
 {
 	uint16_t qid = command->cdw10 & 0xffff;
+	uint16_t sqid;
 
 	DPRINTF("%s DELETE_IO_CQ %u", __func__, qid);
-	if (qid == 0 || qid > sc->num_cqueues) {
+	if (qid == 0 || qid > sc->num_cqueues ||
+	    (sc->compl_queues[qid].qbase == NULL)) {
 		WPRINTF("%s queue index %u / num_cqueues %u",
 		        __func__, qid, sc->num_cqueues);
 		pci_nvme_status_tc(&compl->status, NVME_SCT_COMMAND_SPECIFIC,
@@ -778,6 +810,15 @@ nvme_opc_delete_io_cq(struct pci_nvme_softc* sc, struc
 		return (1);
 	}
 
+	/* Deleting an Active CQ is an error */
+	for (sqid = 1; sqid < sc->num_squeues + 1; sqid++)
+		if (sc->submit_queues[sqid].cqid == qid) {
+			pci_nvme_status_tc(&compl->status,
+			    NVME_SCT_COMMAND_SPECIFIC,
+			    NVME_SC_INVALID_QUEUE_DELETION);
+			return (1);
+		}
+
 	sc->compl_queues[qid].qbase = NULL;
 	pci_nvme_status_genc(&compl->status, NVME_SC_SUCCESS);
 	return (1);
@@ -787,41 +828,57 @@ static int
 nvme_opc_create_io_cq(struct pci_nvme_softc* sc, struct nvme_command* command,
 	struct nvme_completion* compl)
 {
+	struct nvme_completion_queue *ncq;
+	uint16_t qid = command->cdw10 & 0xffff;
 
-	if (command->cdw11 & NVME_CMD_CDW11_PC) {
-		uint16_t qid = command->cdw10 & 0xffff;
-		struct nvme_completion_queue *ncq;
+	/* Only support Physically Contiguous queues */
+	if ((command->cdw11 & NVME_CMD_CDW11_PC) == 0) {
+		WPRINTF("%s unsupported non-contig (list-based) "
+		         "create i/o completion queue",
+		         __func__);
 
-		if ((qid == 0) || (qid > sc->num_cqueues)) {
-			WPRINTF("%s queue index %u > num_cqueues %u",
-			        __func__, qid, sc->num_cqueues);
-			pci_nvme_status_tc(&compl->status,
-			    NVME_SCT_COMMAND_SPECIFIC,
-			    NVME_SC_INVALID_QUEUE_IDENTIFIER);
-			return (1);
-		}
+		pci_nvme_status_genc(&compl->status, NVME_SC_INVALID_FIELD);
+		return (1);
+	}
 
-		ncq = &sc->compl_queues[qid];
-		ncq->intr_en = (command->cdw11 & NVME_CMD_CDW11_IEN) >> 1;
-		ncq->intr_vec = (command->cdw11 >> 16) & 0xffff;
-		ncq->size = ONE_BASED((command->cdw10 >> 16) & 0xffff);
+	if ((qid == 0) || (qid > sc->num_cqueues) ||
+	    (sc->compl_queues[qid].qbase != NULL)) {
+		WPRINTF("%s queue index %u > num_cqueues %u",
+			__func__, qid, sc->num_cqueues);
+		pci_nvme_status_tc(&compl->status,
+		    NVME_SCT_COMMAND_SPECIFIC,
+		    NVME_SC_INVALID_QUEUE_IDENTIFIER);
+		return (1);
+ 	}
 
-		ncq->qbase = vm_map_gpa(sc->nsc_pi->pi_vmctx,
-		             command->prp1,
-		             sizeof(struct nvme_command) * (size_t)ncq->size);
+	ncq = &sc->compl_queues[qid];
+	ncq->intr_en = (command->cdw11 & NVME_CMD_CDW11_IEN) >> 1;
+	ncq->intr_vec = (command->cdw11 >> 16) & 0xffff;
+	if (ncq->intr_vec > (sc->max_queues + 1)) {
+		pci_nvme_status_tc(&compl->status,
+		    NVME_SCT_COMMAND_SPECIFIC,
+		    NVME_SC_INVALID_INTERRUPT_VECTOR);
+		return (1);
+	}
 
-		pci_nvme_status_genc(&compl->status, NVME_SC_SUCCESS);
-	} else {
-		/* 
-		 * Non-contig completion queue unsupported.
+	ncq->size = ONE_BASED((command->cdw10 >> 16) & 0xffff);
+	if ((ncq->size < 2) || (ncq->size > sc->max_qentries))  {
+		/*
+		 * Queues must specify at least two entries
+		 * NOTE: "MAXIMUM QUEUE SIZE EXCEEDED" was renamed to
+		 * "INVALID QUEUE SIZE" in the NVM Express 1.3 Spec
 		 */
-		WPRINTF("%s unsupported non-contig (list-based) "
-		         "create i/o completion queue",
-		         __func__);
-
-		/* 0x12 = Invalid Use of Controller Memory Buffer */
-		pci_nvme_status_genc(&compl->status, 0x12);
+		pci_nvme_status_tc(&compl->status,
+		    NVME_SCT_COMMAND_SPECIFIC,
+		    NVME_SC_MAXIMUM_QUEUE_SIZE_EXCEEDED);
+		return (1);
 	}
+	ncq->qbase = vm_map_gpa(sc->nsc_pi->pi_vmctx,
+		     command->prp1,
+		     sizeof(struct nvme_command) * (size_t)ncq->size);
+
+	pci_nvme_status_genc(&compl->status, NVME_SC_SUCCESS);
+
 
 	return (1);
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202006290031.05T0VcBV047243>