From aa9e0ac63308a825f6e001bd54b4caa2997bb99a Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Fri, 24 Sep 2021 20:07:07 +0800 Subject: [PATCH] nvmf/vfio-user: support delete/create SQ Previously vfio-user will hit an error when creating a deleted IO SQ. For lookup_io_q() function, actually we should not use the queue's address to check the queue is exist or not, as when there is a memory removal, we will also set the queue's address to NULL and reset it again, so here we use the queue state to indicate the queue is exist or not. Fix issue #2174. Change-Id: I56bdd63947c2b30a598c427e64ff2dafe1cc1d2e Signed-off-by: Changpeng Liu Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9605 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- lib/nvmf/vfio_user.c | 48 ++++++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/lib/nvmf/vfio_user.c b/lib/nvmf/vfio_user.c index 676f488e2..3193cdf8e 100644 --- a/lib/nvmf/vfio_user.c +++ b/lib/nvmf/vfio_user.c @@ -130,7 +130,7 @@ struct nvme_q { enum nvmf_vfio_user_qpair_state { VFIO_USER_QPAIR_UNINITIALIZED = 0, VFIO_USER_QPAIR_ACTIVE, - VFIO_USER_QPAIR_DELETED, + VFIO_USER_QPAIR_SQ_DELETED, VFIO_USER_QPAIR_INACTIVE, VFIO_USER_QPAIR_ERROR, }; @@ -915,15 +915,17 @@ lookup_io_q(struct nvmf_vfio_user_ctrlr *ctrlr, const uint16_t qid, const bool i } if (is_cq) { + /* CQ is always exist if the queue pair wasn't null */ q = &ctrlr->qp[qid]->cq; + return q; } else { + if (ctrlr->qp[qid]->state == VFIO_USER_QPAIR_SQ_DELETED || + ctrlr->qp[qid]->state == VFIO_USER_QPAIR_UNINITIALIZED) { + return NULL; + } q = &ctrlr->qp[qid]->sq; } - if (q->addr == NULL) { - return NULL; - } - return q; } @@ -1208,19 +1210,25 @@ handle_create_io_q(struct nvmf_vfio_user_ctrlr *ctrlr, if (is_cq) { *hdbl(ctrlr, io_q) = 0; } else { - /* - * Create our new I/O qpair. This asynchronously invokes, on a - * suitable poll group, the nvmf_vfio_user_poll_group_add() - * callback, which will call spdk_nvmf_request_exec_fabrics() - * with a generated fabrics connect command. This command is - * then eventually completed via handle_queue_connect_rsp(). - */ vu_qpair = ctrlr->qp[qid]; - vu_qpair->create_io_sq_cmd = *cmd; - spdk_nvmf_tgt_new_qpair(ctrlr->transport->transport.tgt, - &vu_qpair->qpair); *tdbl(ctrlr, io_q) = 0; - return 0; + vu_qpair->sq.head = 0; + + if (vu_qpair->state == VFIO_USER_QPAIR_SQ_DELETED) { + vu_qpair->state = VFIO_USER_QPAIR_ACTIVE; + } else { + /* + * Create our new I/O qpair. This asynchronously invokes, on a + * suitable poll group, the nvmf_vfio_user_poll_group_add() + * callback, which will call spdk_nvmf_request_exec_fabrics() + * with a generated fabrics connect command. This command is + * then eventually completed via handle_queue_connect_rsp(). + */ + vu_qpair->create_io_sq_cmd = *cmd; + spdk_nvmf_tgt_new_qpair(ctrlr->transport->transport.tgt, + &vu_qpair->qpair); + return 0; + } } out: @@ -1273,7 +1281,7 @@ handle_del_io_q(struct nvmf_vfio_user_ctrlr *ctrlr, vu_qpair = ctrlr->qp[cmd->cdw10_bits.delete_io_q.qid]; if (is_cq) { /* SQ must have been deleted first */ - if (vu_qpair->state != VFIO_USER_QPAIR_DELETED) { + if (vu_qpair->state != VFIO_USER_QPAIR_SQ_DELETED) { SPDK_ERRLOG("%s: the associated SQ must be deleted first\n", ctrlr_id(ctrlr)); sct = SPDK_NVME_SCT_COMMAND_SPECIFIC; sc = SPDK_NVME_SC_INVALID_QUEUE_DELETION; @@ -1290,7 +1298,7 @@ handle_del_io_q(struct nvmf_vfio_user_ctrlr *ctrlr, spdk_nvmf_qpair_disconnect(&vu_qpair->qpair, vfio_user_qpair_delete_cb, ctx); return 0; } else { - if (vu_qpair->state == VFIO_USER_QPAIR_DELETED) { + if (vu_qpair->state == VFIO_USER_QPAIR_SQ_DELETED) { SPDK_DEBUGLOG(nvmf_vfio, "%s: SQ%u is already deleted\n", ctrlr_id(ctrlr), cmd->cdw10_bits.delete_io_q.qid); sct = SPDK_NVME_SCT_COMMAND_SPECIFIC; @@ -1303,7 +1311,9 @@ handle_del_io_q(struct nvmf_vfio_user_ctrlr *ctrlr, * function to skip checking this SQ. The queue pair will be disconnected in Delete * IO CQ command. */ - vu_qpair->state = VFIO_USER_QPAIR_DELETED; + vu_qpair->state = VFIO_USER_QPAIR_SQ_DELETED; + vfu_unmap_sg(ctrlr->endpoint->vfu_ctx, vu_qpair->sq.sg, &vu_qpair->sq.iov, 1); + vu_qpair->sq.addr = NULL; } out: