From b769dcd4fd2b34e92d7af304fc3c218d52566e4c Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Tue, 9 Nov 2021 21:12:38 +0800 Subject: [PATCH] nvme/compliance: add a case to test CREATE IO SQ with out of range CQID Change-Id: Ie1c80d33d7fcc704321948a4b1f713f6256dc6e5 Signed-off-by: Changpeng Liu Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10151 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris Reviewed-by: Konrad Sztyber Reviewed-by: Aleksey Marchuk --- lib/nvmf/vfio_user.c | 16 +++++----- test/nvme/compliance/nvme_compliance.c | 41 ++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/lib/nvmf/vfio_user.c b/lib/nvmf/vfio_user.c index bd16f1355..43262d94d 100644 --- a/lib/nvmf/vfio_user.c +++ b/lib/nvmf/vfio_user.c @@ -1105,7 +1105,7 @@ static int handle_create_io_q(struct nvmf_vfio_user_ctrlr *ctrlr, struct spdk_nvme_cmd *cmd, const bool is_cq) { - uint16_t qid; + uint16_t qid, cqid; uint32_t qsize; uint16_t sc = SPDK_NVME_SC_SUCCESS; uint16_t sct = SPDK_NVME_SCT_GENERIC; @@ -1170,17 +1170,17 @@ handle_create_io_q(struct nvmf_vfio_user_ctrlr *ctrlr, io_q->iv = cmd->cdw11_bits.create_io_cq.iv; io_q->phase = true; } else { - if (cmd->cdw11_bits.create_io_sq.cqid == 0) { - SPDK_ERRLOG("%s: invalid CQID 0\n", ctrlr_id(ctrlr)); + cqid = cmd->cdw11_bits.create_io_sq.cqid; + if (cqid == 0 || cqid >= vu_transport->transport.opts.max_qpairs_per_ctrlr) { + SPDK_ERRLOG("%s: invalid CQID %u\n", ctrlr_id(ctrlr), cqid); sct = SPDK_NVME_SCT_COMMAND_SPECIFIC; sc = SPDK_NVME_SC_INVALID_QUEUE_IDENTIFIER; goto out; } /* CQ must be created before SQ */ - if (!io_q_exists(ctrlr, cmd->cdw11_bits.create_io_sq.cqid, true)) { - SPDK_ERRLOG("%s: CQ%d does not exist\n", ctrlr_id(ctrlr), - cmd->cdw11_bits.create_io_sq.cqid); + if (!io_q_exists(ctrlr, cqid, true)) { + SPDK_ERRLOG("%s: CQ%u does not exist\n", ctrlr_id(ctrlr), cqid); sct = SPDK_NVME_SCT_COMMAND_SPECIFIC; sc = SPDK_NVME_SC_COMPLETION_QUEUE_INVALID; goto out; @@ -1192,14 +1192,14 @@ handle_create_io_q(struct nvmf_vfio_user_ctrlr *ctrlr, goto out; } /* TODO: support shared IO CQ */ - if (qid != cmd->cdw11_bits.create_io_sq.cqid) { + if (qid != cqid) { SPDK_ERRLOG("%s: doesn't support shared CQ now\n", ctrlr_id(ctrlr)); sct = SPDK_NVME_SCT_COMMAND_SPECIFIC; sc = SPDK_NVME_SC_INVALID_QUEUE_IDENTIFIER; } io_q = &ctrlr->qp[qid]->sq; - io_q->cqid = cmd->cdw11_bits.create_io_sq.cqid; + io_q->cqid = cqid; SPDK_DEBUGLOG(nvmf_vfio, "%s: SQ%d CQID=%d\n", ctrlr_id(ctrlr), qid, io_q->cqid); } diff --git a/test/nvme/compliance/nvme_compliance.c b/test/nvme/compliance/nvme_compliance.c index 1f59e3999..7c4c28c3b 100644 --- a/test/nvme/compliance/nvme_compliance.c +++ b/test/nvme/compliance/nvme_compliance.c @@ -327,6 +327,7 @@ delete_create_io_sq(void) void *buf; uint32_t nlbas; uint64_t dma_addr; + uint32_t ncqr; int rc; SPDK_CU_ASSERT_FATAL(spdk_nvme_transport_id_parse(&g_trid, g_trid_str) == 0); @@ -355,6 +356,19 @@ delete_create_io_sq(void) CU_ASSERT(s.cpl.status.sc == SPDK_NVME_SC_SUCCESS); spdk_dma_free(buf); + memset(&cmd, 0, sizeof(cmd)); + cmd.opc = SPDK_NVME_OPC_GET_FEATURES; + /* Get Maximum Number of CQs */ + cmd.cdw10_bits.get_features.fid = SPDK_NVME_FEAT_NUMBER_OF_QUEUES; + + s.done = false; + rc = spdk_nvme_ctrlr_cmd_admin_raw(ctrlr, &cmd, NULL, 0, test_cb, &s); + CU_ASSERT(rc == 0); + wait_for_admin_completion(&s, ctrlr); + CU_ASSERT(!spdk_nvme_cpl_is_error(&s.cpl)); + + ncqr = ((s.cpl.cdw0 & 0xffff0000u) >> 16) + 1; + /* Delete SQ 1, this is valid. */ memset(&cmd, 0, sizeof(cmd)); cmd.opc = SPDK_NVME_OPC_DELETE_IO_SQ; @@ -405,14 +419,37 @@ delete_create_io_sq(void) CU_ASSERT(s.cpl.status.sct == SPDK_NVME_SCT_COMMAND_SPECIFIC); CU_ASSERT(s.cpl.status.sc == SPDK_NVME_SC_INVALID_QUEUE_SIZE); - /* Create SQ 1 again, qsize is MQES, this is valid. */ - cmd.cdw10_bits.create_io_q.qsize = cap.bits.mqes; /* 0 based value */ + /* Create SQ 1 again, CQID is 0, this is invalid. */ + cmd.cdw10_bits.create_io_q.qsize = cap.bits.mqes; /* 0 based value, valid */ + cmd.cdw11_bits.create_io_sq.cqid = 0; s.done = false; rc = spdk_nvme_ctrlr_cmd_admin_raw(ctrlr, &cmd, NULL, 0, test_cb, &s); CU_ASSERT(rc == 0); wait_for_admin_completion(&s, ctrlr); + CU_ASSERT(s.cpl.status.sct == SPDK_NVME_SCT_COMMAND_SPECIFIC); + CU_ASSERT(s.cpl.status.sc == SPDK_NVME_SC_INVALID_QUEUE_IDENTIFIER); + + /* Create SQ 1 again, CQID is NCQR + 1, this is invalid. */ + cmd.cdw11_bits.create_io_sq.cqid = ncqr + 1; + s.done = false; + rc = spdk_nvme_ctrlr_cmd_admin_raw(ctrlr, &cmd, NULL, 0, test_cb, &s); + CU_ASSERT(rc == 0); + + wait_for_admin_completion(&s, ctrlr); + + CU_ASSERT(s.cpl.status.sct == SPDK_NVME_SCT_COMMAND_SPECIFIC); + CU_ASSERT(s.cpl.status.sc == SPDK_NVME_SC_INVALID_QUEUE_IDENTIFIER); + + /* Create SQ 1 again, CQID is 1, this is valid. */ + s.done = false; + cmd.cdw11_bits.create_io_sq.cqid = 1; + rc = spdk_nvme_ctrlr_cmd_admin_raw(ctrlr, &cmd, NULL, 0, test_cb, &s); + CU_ASSERT(rc == 0); + + wait_for_admin_completion(&s, ctrlr); + CU_ASSERT(s.cpl.status.sct == SPDK_NVME_SCT_GENERIC); CU_ASSERT(s.cpl.status.sc == SPDK_NVME_SC_SUCCESS);