From d6f6ffd2741b9ebc12bbeb56dcbd14c6408e31b6 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Thu, 27 May 2021 00:04:58 +0000 Subject: [PATCH] nvme: add NVME_CTRLR_STATE_CONNECT_ADMINQ Connect the adminq as part of controller initialization instead of controller construction. We never actually 'connected' the adminq for PCIe or vfio-user transports, since its a nop. But their connect_qpair transport ops function is also a nop for the adminq, so it's fine to generically connect the adminq across all transports. Note that we cannot read registers (cc or csts) during controller initialization now until after the adminq has been connected since reading fabrics registers depends on a connected adminq. This gets special cased for now, but eventually reading cc and csts will need to be part of the state machine itself to make it asynchronous. Signed-off-by: Jim Harris Change-Id: Ia5566d7c549d78d24b94ea253df51e697da6237f Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8079 Tested-by: SPDK CI Jenkins Community-CI: Mellanox Build Bot Reviewed-by: Ziye Yang Reviewed-by: Aleksey Marchuk Reviewed-by: Changpeng Liu --- lib/nvme/nvme_ctrlr.c | 25 +++++++++++++++---------- lib/nvme/nvme_internal.h | 11 ++++++++--- lib/nvme/nvme_rdma.c | 6 ------ lib/nvme/nvme_tcp.c | 7 ------- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 8dd2b83efc..96f64798a8 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -1135,6 +1135,8 @@ nvme_ctrlr_state_string(enum nvme_ctrlr_state state) switch (state) { case NVME_CTRLR_STATE_INIT_DELAY: return "delay init"; + case NVME_CTRLR_STATE_CONNECT_ADMINQ: + return "connect adminq"; case NVME_CTRLR_STATE_READ_VS: return "read vs"; case NVME_CTRLR_STATE_READ_CAP: @@ -1409,11 +1411,6 @@ spdk_nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr) ctrlr->adminq->transport_failure_reason = SPDK_NVME_QPAIR_FAILURE_LOCAL; nvme_transport_ctrlr_disconnect_qpair(ctrlr, ctrlr->adminq); - rc = nvme_transport_ctrlr_connect_qpair(ctrlr, ctrlr->adminq); - if (rc != 0) { - NVME_CTRLR_ERRLOG(ctrlr, "Controller reinitialization failed.\n"); - goto out; - } /* Doorbell buffer config is invalid during reset */ nvme_ctrlr_free_doorbell_buffer(ctrlr); @@ -1426,7 +1423,6 @@ spdk_nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr) /* Set the state back to INIT to cause a full hardware reset. */ nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_INIT, NVME_TIMEOUT_INFINITE); - nvme_qpair_set_state(ctrlr->adminq, NVME_QPAIR_ENABLED); while (ctrlr->state != NVME_CTRLR_STATE_READY) { if (nvme_ctrlr_process_init(ctrlr) != 0) { NVME_CTRLR_ERRLOG(ctrlr, "controller reinitialization failed\n"); @@ -1456,7 +1452,6 @@ spdk_nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr) } } -out: if (rc) { nvme_ctrlr_fail(ctrlr, false); } @@ -3094,8 +3089,8 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr) } ctrlr->sleep_timeout_tsc = 0; - if (nvme_ctrlr_get_cc(ctrlr, &cc) || - nvme_ctrlr_get_csts(ctrlr, &csts)) { + if (ctrlr->state > NVME_CTRLR_STATE_CONNECT_ADMINQ && + (nvme_ctrlr_get_cc(ctrlr, &cc) || nvme_ctrlr_get_csts(ctrlr, &csts))) { if (!ctrlr->is_failed && ctrlr->state_timeout_tsc != NVME_TIMEOUT_INFINITE) { /* While a device is resetting, it may be unable to service MMIO reads * temporarily. Allow for this case. @@ -3129,7 +3124,17 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr) } break; - case NVME_CTRLR_STATE_READ_VS: /* synonymous with NVME_CTRLR_STATE_INIT */ + case NVME_CTRLR_STATE_CONNECT_ADMINQ: /* synonymous with NVME_CTRLR_STATE_INIT */ + rc = nvme_transport_ctrlr_connect_qpair(ctrlr, ctrlr->adminq); + if (rc == 0) { + nvme_qpair_set_state(ctrlr->adminq, NVME_QPAIR_ENABLED); + nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_READ_VS, NVME_TIMEOUT_INFINITE); + } else { + nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_ERROR, NVME_TIMEOUT_INFINITE); + } + break; + + case NVME_CTRLR_STATE_READ_VS: nvme_ctrlr_get_vs(ctrlr, &ctrlr->vs); nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_READ_CAP, NVME_TIMEOUT_INFINITE); break; diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 61ff3201eb..98607a06a7 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -518,14 +518,19 @@ enum nvme_ctrlr_state { NVME_CTRLR_STATE_INIT_DELAY, /** - * Read Version (VS) register. + * Connect the admin queue. */ - NVME_CTRLR_STATE_READ_VS, + NVME_CTRLR_STATE_CONNECT_ADMINQ, /** * Controller has not started initialized yet. */ - NVME_CTRLR_STATE_INIT = NVME_CTRLR_STATE_READ_VS, + NVME_CTRLR_STATE_INIT = NVME_CTRLR_STATE_CONNECT_ADMINQ, + + /** + * Read Version (VS) register. + */ + NVME_CTRLR_STATE_READ_VS, /** * Read Capabilities (CAP) register. diff --git a/lib/nvme/nvme_rdma.c b/lib/nvme/nvme_rdma.c index e52992205b..fe2a8d004a 100644 --- a/lib/nvme/nvme_rdma.c +++ b/lib/nvme/nvme_rdma.c @@ -1825,12 +1825,6 @@ static struct spdk_nvme_ctrlr *nvme_rdma_ctrlr_construct(const struct spdk_nvme_ goto destruct_ctrlr; } - rc = nvme_transport_ctrlr_connect_qpair(&rctrlr->ctrlr, rctrlr->ctrlr.adminq); - if (rc < 0) { - SPDK_ERRLOG("failed to connect admin qpair\n"); - goto destruct_ctrlr; - } - if (nvme_ctrlr_add_process(&rctrlr->ctrlr, 0) != 0) { SPDK_ERRLOG("nvme_ctrlr_add_process() failed\n"); goto destruct_ctrlr; diff --git a/lib/nvme/nvme_tcp.c b/lib/nvme/nvme_tcp.c index 21e6dba648..dacbd5d108 100644 --- a/lib/nvme/nvme_tcp.c +++ b/lib/nvme/nvme_tcp.c @@ -1959,13 +1959,6 @@ static struct spdk_nvme_ctrlr *nvme_tcp_ctrlr_construct(const struct spdk_nvme_t return NULL; } - rc = nvme_transport_ctrlr_connect_qpair(&tctrlr->ctrlr, tctrlr->ctrlr.adminq); - if (rc < 0) { - SPDK_ERRLOG("failed to connect admin qpair\n"); - nvme_tcp_ctrlr_destruct(&tctrlr->ctrlr); - return NULL; - } - if (nvme_ctrlr_add_process(&tctrlr->ctrlr, 0) != 0) { SPDK_ERRLOG("nvme_ctrlr_add_process() failed\n"); nvme_ctrlr_destruct(&tctrlr->ctrlr);