Commit 27c42e31 authored by Darek Stojaczyk's avatar Darek Stojaczyk Committed by Changpeng Liu
Browse files

nvme: don't rely on phys_addr retrieved from spdk_malloc() 



The phys_addr param in spdk_*malloc() is about to be
deprecated, so use a separate spdk_vtophys() call to
retrieve physical addresses.

This patch also adds error checks against SPDK_VTOPHYS_ERROR.
The error handling paths are already there to account for
spdk_*malloc() failures themselves, so reuse them in case
of vtophys failures.

Change-Id: I377636e66b8c570d013c1bb2021f04bce4e6c0ce
Signed-off-by: default avatarDarek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/416998


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
parent 8165bf71

lib/nvme/nvme.c

+1 −2
Original line number Diff line number Diff line
@@ -202,10 +202,9 @@ nvme_allocate_request_user_copy(struct spdk_nvme_qpair *qpair, 
{

	struct nvme_request *req;

	void *dma_buffer = NULL;

	uint64_t phys_addr;



	if (buffer && payload_size) {

		dma_buffer = spdk_zmalloc(payload_size, 4096, &phys_addr,

		dma_buffer = spdk_zmalloc(payload_size, 4096, NULL,

					  SPDK_ENV_SOCKET_ID_ANY, SPDK_MALLOC_DMA);

		if (!dma_buffer) {

			return NULL;

lib/nvme/nvme_ctrlr.c

+22 −9
Original line number Diff line number Diff line
@@ -394,12 +394,11 @@ nvme_ctrlr_construct_intel_support_log_page_list(struct spdk_nvme_ctrlr *ctrlr, 
static int nvme_ctrlr_set_intel_support_log_pages(struct spdk_nvme_ctrlr *ctrlr)

{

	int rc = 0;

	uint64_t phys_addr = 0;

	struct nvme_completion_poll_status	status;

	struct spdk_nvme_intel_log_page_directory *log_page_directory;



	log_page_directory = spdk_zmalloc(sizeof(struct spdk_nvme_intel_log_page_directory),

					  64, &phys_addr, SPDK_ENV_SOCKET_ID_ANY, SPDK_MALLOC_DMA);

					  64, NULL, SPDK_ENV_SOCKET_ID_ANY, SPDK_MALLOC_DMA);

	if (log_page_directory == NULL) {

		SPDK_ERRLOG("could not allocate log_page_directory\n");

		return -ENXIO;
@@ -754,7 +753,7 @@ static int 
nvme_ctrlr_set_doorbell_buffer_config(struct spdk_nvme_ctrlr *ctrlr)

{

	int rc = 0;

	uint64_t prp1, prp2;

	uint64_t prp1, prp2, len;



	if (!ctrlr->cdata.oacs.doorbell_buffer_config) {

		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT,
@@ -770,18 +769,32 @@ nvme_ctrlr_set_doorbell_buffer_config(struct spdk_nvme_ctrlr *ctrlr) 


	/* only 1 page size for doorbell buffer */

	ctrlr->shadow_doorbell = spdk_dma_zmalloc(ctrlr->page_size, ctrlr->page_size,

				 &prp1);

				 NULL);

	if (ctrlr->shadow_doorbell == NULL) {

		rc = -ENOMEM;

		goto error;

	}



	ctrlr->eventidx = spdk_dma_zmalloc(ctrlr->page_size, ctrlr->page_size, &prp2);

	len = ctrlr->page_size;

	prp1 = spdk_vtophys(ctrlr->shadow_doorbell, &len);

	if (prp1 == SPDK_VTOPHYS_ERROR || len != ctrlr->page_size) {

		rc = -EFAULT;

		goto error;

	}



	ctrlr->eventidx = spdk_dma_zmalloc(ctrlr->page_size, ctrlr->page_size, NULL);

	if (ctrlr->eventidx == NULL) {

		rc = -ENOMEM;

		goto error;

	}



	len = ctrlr->page_size;

	prp2 = spdk_vtophys(ctrlr->eventidx, &len);

	if (prp2 == SPDK_VTOPHYS_ERROR || len != ctrlr->page_size) {

		rc = -EFAULT;

		goto error;

	}



	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_WAIT_FOR_DB_BUF_CFG,

			     ctrlr->opts.admin_timeout_ms);
@@ -1457,7 +1470,6 @@ nvme_ctrlr_construct_namespaces(struct spdk_nvme_ctrlr *ctrlr) 
{

	int rc = 0;

	uint32_t nn = ctrlr->cdata.nn;

	uint64_t phys_addr = 0;



	/* ctrlr->num_ns may be 0 (startup) or a different number of namespaces (reset),

	 * so check if we need to reallocate.
@@ -1470,15 +1482,16 @@ nvme_ctrlr_construct_namespaces(struct spdk_nvme_ctrlr *ctrlr) 
			return 0;

		}



		ctrlr->ns = spdk_zmalloc(nn * sizeof(struct spdk_nvme_ns), 64,

					 &phys_addr, SPDK_ENV_SOCKET_ID_ANY, SPDK_MALLOC_SHARE);

		ctrlr->ns = spdk_zmalloc(nn * sizeof(struct spdk_nvme_ns), 64, NULL,

					 SPDK_ENV_SOCKET_ID_ANY, SPDK_MALLOC_SHARE);

		if (ctrlr->ns == NULL) {

			rc = -ENOMEM;

			goto fail;

		}



		ctrlr->nsdata = spdk_zmalloc(nn * sizeof(struct spdk_nvme_ns_data), 64,

					     &phys_addr, SPDK_ENV_SOCKET_ID_ANY, SPDK_MALLOC_SHARE | SPDK_MALLOC_DMA);

					     NULL, SPDK_ENV_SOCKET_ID_ANY,

					     SPDK_MALLOC_SHARE | SPDK_MALLOC_DMA);

		if (ctrlr->nsdata == NULL) {

			rc = -ENOMEM;

			goto fail;

lib/nvme/nvme_pcie.c

+14 −2
Original line number Diff line number Diff line
@@ -1012,22 +1012,34 @@ nvme_pcie_qpair_construct(struct spdk_nvme_qpair *qpair) 
	 */

	if (pqpair->sq_in_cmb == false) {

		pqpair->cmd = spdk_zmalloc(pqpair->num_entries * sizeof(struct spdk_nvme_cmd),

					   page_align, &pqpair->cmd_bus_addr,

					   page_align, NULL,

					   SPDK_ENV_SOCKET_ID_ANY, flags);

		if (pqpair->cmd == NULL) {

			SPDK_ERRLOG("alloc qpair_cmd failed\n");

			return -ENOMEM;

		}



		pqpair->cmd_bus_addr = spdk_vtophys(pqpair->cmd, NULL);

		if (pqpair->cmd_bus_addr == SPDK_VTOPHYS_ERROR) {

			SPDK_ERRLOG("spdk_vtophys(pqpair->cmd) failed\n");

			return -EFAULT;

		}

	}



	pqpair->cpl = spdk_zmalloc(pqpair->num_entries * sizeof(struct spdk_nvme_cpl),

				   page_align, &pqpair->cpl_bus_addr,

				   page_align, NULL,

				   SPDK_ENV_SOCKET_ID_ANY, flags);

	if (pqpair->cpl == NULL) {

		SPDK_ERRLOG("alloc qpair_cpl failed\n");

		return -ENOMEM;

	}



	pqpair->cpl_bus_addr = spdk_vtophys(pqpair->cpl, NULL);

	if (pqpair->cpl_bus_addr == SPDK_VTOPHYS_ERROR) {

		SPDK_ERRLOG("spdk_vtophys(pqpair->cpl) failed\n");

		return -EFAULT;

	}



	doorbell_base = &pctrlr->regs->doorbell[0].sq_tdbl;

	pqpair->sq_tdbl = doorbell_base + (2 * qpair->id + 0) * pctrlr->doorbell_stride_u32;

	pqpair->cq_hdbl = doorbell_base + (2 * qpair->id + 1) * pctrlr->doorbell_stride_u32;

test/common/lib/test_env.c

+2 −0
Original line number Diff line number Diff line
@@ -153,6 +153,8 @@ spdk_dma_realloc(void *buf, size_t size, size_t align, uint64_t *phys_addr) 
void

spdk_free(void *buf)

{

	/* fix for false-positives in *certain* static analysis tools. */

	assert((uintptr_t)buf != UINTPTR_MAX);

	free(buf);

}