From 7e3a11f98b6c2ae5d50ac2f65be162737c91a1e0 Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Sat, 30 Sep 2017 00:37:21 -0400 Subject: [PATCH] nvme: add doorbell buffer config support NVMe specification 1.3 added a new Admin command: Doorbell buffer config, which is used to enhance the performance of host software running in Virtual Machine, and the Doorbell buffer config feature is only used for emulated NVMe controllers. There are two buffers: "shadow doorbell" and "eventidx", host software running in VM will update appropriate entry in the Shadow doorbell buffer instead of controller's doorbell registers. Change-Id: I639ddb5b9a0ca0305bf84035ca2a5e215be06b46 Signed-off-by: Changpeng Liu Reviewed-on: https://review.gerrithub.io/383042 Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp Reviewed-by: Jim Harris --- lib/nvme/nvme_ctrlr.c | 73 +++++++++++++++++++ lib/nvme/nvme_ctrlr_cmd.c | 26 +++++++ lib/nvme/nvme_internal.h | 8 ++ lib/nvme/nvme_pcie.c | 60 ++++++++++++++- .../lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c | 24 ++++++ test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c | 15 +++- 6 files changed, 203 insertions(+), 3 deletions(-) diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index c63268b1d..c8d31bbe3 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -629,6 +629,67 @@ nvme_ctrlr_set_state(struct spdk_nvme_ctrlr *ctrlr, enum nvme_ctrlr_state state, } } +static void +nvme_ctrlr_free_doorbell_buffer(struct spdk_nvme_ctrlr *ctrlr) +{ + if (ctrlr->shadow_doorbell) { + spdk_dma_free(ctrlr->shadow_doorbell); + ctrlr->shadow_doorbell = NULL; + } + + if (ctrlr->eventidx) { + spdk_dma_free(ctrlr->eventidx); + ctrlr->eventidx = NULL; + } +} + +static int +nvme_ctrlr_set_doorbell_buffer_config(struct spdk_nvme_ctrlr *ctrlr) +{ + int rc; + struct nvme_completion_poll_status status; + uint64_t prp1, prp2; + + if (ctrlr->trid.trtype != SPDK_NVME_TRANSPORT_PCIE) { + return 0; + } + + /* only 1 page size for doorbell buffer */ + ctrlr->shadow_doorbell = spdk_dma_zmalloc(ctrlr->page_size, ctrlr->page_size, + &prp1); + if (ctrlr->shadow_doorbell == NULL) { + return -1; + } + + ctrlr->eventidx = spdk_dma_zmalloc(ctrlr->page_size, ctrlr->page_size, &prp2); + if (ctrlr->eventidx == NULL) { + goto error; + } + + status.done = false; + rc = nvme_ctrlr_cmd_doorbell_buffer_config(ctrlr, prp1, prp2, + nvme_completion_poll_cb, &status); + if (rc != 0) { + goto error; + } + + while (status.done == false) { + spdk_nvme_qpair_process_completions(ctrlr->adminq, 0); + } + if (spdk_nvme_cpl_is_error(&status.cpl)) { + goto error; + } + + SPDK_INFOLOG(SPDK_TRACE_NVME, "NVMe controller: %s doorbell buffer config enabled\n", + ctrlr->trid.traddr); + + return 0; + +error: + nvme_ctrlr_free_doorbell_buffer(ctrlr); + return -1; +} + int spdk_nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr) { @@ -665,6 +726,9 @@ spdk_nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr) nvme_qpair_disable(qpair); } + /* Doorbell buffer config is invalid during reset */ + nvme_ctrlr_free_doorbell_buffer(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); @@ -1521,6 +1585,13 @@ nvme_ctrlr_start(struct spdk_nvme_ctrlr *ctrlr) ctrlr->max_sges = nvme_transport_ctrlr_get_max_sges(ctrlr); } + if (ctrlr->cdata.oacs.doorbell_buffer_config) { + if (nvme_ctrlr_set_doorbell_buffer_config(ctrlr)) { + SPDK_WARNLOG("Doorbell buffer config failed\n"); + } + } + + if (nvme_ctrlr_set_keep_alive_timeout(ctrlr) != 0) { SPDK_ERRLOG("Setting keep alive timeout failed\n"); return -1; @@ -1608,6 +1679,8 @@ nvme_ctrlr_destruct(struct spdk_nvme_ctrlr *ctrlr) spdk_nvme_ctrlr_free_io_qpair(qpair); } + nvme_ctrlr_free_doorbell_buffer(ctrlr); + nvme_ctrlr_shutdown(ctrlr); nvme_ctrlr_destruct_namespaces(ctrlr); diff --git a/lib/nvme/nvme_ctrlr_cmd.c b/lib/nvme/nvme_ctrlr_cmd.c index afa782376..db2d872cd 100644 --- a/lib/nvme/nvme_ctrlr_cmd.c +++ b/lib/nvme/nvme_ctrlr_cmd.c @@ -261,6 +261,32 @@ nvme_ctrlr_cmd_delete_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, spdk_nvme return rc; } +int +nvme_ctrlr_cmd_doorbell_buffer_config(struct spdk_nvme_ctrlr *ctrlr, uint64_t prp1, uint64_t prp2, + spdk_nvme_cmd_cb cb_fn, void *cb_arg) +{ + struct nvme_request *req; + struct spdk_nvme_cmd *cmd; + int rc; + + nvme_robust_mutex_lock(&ctrlr->ctrlr_lock); + req = nvme_allocate_request_null(ctrlr->adminq, cb_fn, cb_arg); + if (req == NULL) { + nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock); + return -ENOMEM; + } + + cmd = &req->cmd; + cmd->opc = SPDK_NVME_OPC_DOORBELL_BUFFER_CONFIG; + cmd->dptr.prp.prp1 = prp1; + cmd->dptr.prp.prp2 = prp2; + + rc = nvme_ctrlr_submit_admin_request(ctrlr, req); + + nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock); + return rc; +} + int nvme_ctrlr_cmd_format(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, struct spdk_nvme_format *format, spdk_nvme_cmd_cb cb_fn, void *cb_arg) diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 1ebfe1642..514866d0d 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -427,6 +427,11 @@ struct spdk_nvme_ctrlr { struct spdk_nvme_qpair *adminq; + /** shadow doorbell buffer */ + uint32_t *shadow_doorbell; + /** eventidx buffer */ + uint32_t *eventidx; + /** * Identify Controller data. */ @@ -540,6 +545,9 @@ int nvme_ctrlr_cmd_detach_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, struct spdk_nvme_ctrlr_list *payload, spdk_nvme_cmd_cb cb_fn, void *cb_arg); int nvme_ctrlr_cmd_create_ns(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ns_data *payload, spdk_nvme_cmd_cb cb_fn, void *cb_arg); +int nvme_ctrlr_cmd_doorbell_buffer_config(struct spdk_nvme_ctrlr *ctrlr, + uint64_t prp1, uint64_t prp2, + spdk_nvme_cmd_cb cb_fn, void *cb_arg); int nvme_ctrlr_cmd_delete_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, spdk_nvme_cmd_cb cb_fn, void *cb_arg); int nvme_ctrlr_cmd_format(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, diff --git a/lib/nvme/nvme_pcie.c b/lib/nvme/nvme_pcie.c index 78cefa00e..754edd1f3 100644 --- a/lib/nvme/nvme_pcie.c +++ b/lib/nvme/nvme_pcie.c @@ -132,6 +132,18 @@ struct nvme_pcie_qpair { /* Completion queue head doorbell */ volatile uint32_t *cq_hdbl; + /* Submission queue shadow tail doorbell */ + volatile uint32_t *sq_shadow_tdbl; + + /* Completion queue shadow head doorbell */ + volatile uint32_t *cq_shadow_hdbl; + + /* Submission queue event index */ + volatile uint32_t *sq_eventidx; + + /* Completion queue event index */ + volatile uint32_t *cq_eventidx; + /* Submission queue */ struct spdk_nvme_cmd *cmd; @@ -1020,6 +1032,33 @@ nvme_pcie_qpair_complete_pending_admin_request(struct spdk_nvme_qpair *qpair) } } +static inline int +nvme_pcie_qpair_need_event(uint16_t event_idx, uint16_t new_idx, uint16_t old) +{ + return (uint16_t)(new_idx - event_idx) <= (uint16_t)(new_idx - old); +} + +static bool +nvme_pcie_qpair_update_mmio_required(struct spdk_nvme_qpair *qpair, uint16_t value, + volatile uint32_t *shadow_db, + volatile uint32_t *eventidx) +{ + uint16_t old; + + if (!shadow_db) { + return true; + } + + old = *shadow_db; + *shadow_db = value; + + if (!nvme_pcie_qpair_need_event(*eventidx, value, old)) { + return false; + } + + return true; +} + static void nvme_pcie_qpair_submit_tracker(struct spdk_nvme_qpair *qpair, struct nvme_tracker *tr) { @@ -1044,7 +1083,12 @@ nvme_pcie_qpair_submit_tracker(struct spdk_nvme_qpair *qpair, struct nvme_tracke spdk_wmb(); g_thread_mmio_ctrlr = pctrlr; - spdk_mmio_write_4(pqpair->sq_tdbl, pqpair->sq_tail); + if (spdk_likely(nvme_pcie_qpair_update_mmio_required(qpair, + pqpair->sq_tail, + pqpair->sq_shadow_tdbl, + pqpair->sq_eventidx))) { + spdk_mmio_write_4(pqpair->sq_tdbl, pqpair->sq_tail); + } g_thread_mmio_ctrlr = NULL; } @@ -1363,6 +1407,8 @@ static int _nvme_pcie_ctrlr_create_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_qpair *qpair, uint16_t qid) { + struct nvme_pcie_ctrlr *pctrlr = nvme_pcie_ctrlr(ctrlr); + struct nvme_pcie_qpair *pqpair = nvme_pcie_qpair(qpair); struct nvme_completion_poll_status status; int rc; @@ -1403,6 +1449,12 @@ _nvme_pcie_ctrlr_create_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme return -1; } + if (ctrlr->shadow_doorbell) { + pqpair->sq_shadow_tdbl = ctrlr->shadow_doorbell + (2 * qpair->id + 0) * pctrlr->doorbell_stride_u32; + pqpair->cq_shadow_hdbl = ctrlr->shadow_doorbell + (2 * qpair->id + 1) * pctrlr->doorbell_stride_u32; + pqpair->sq_eventidx = ctrlr->eventidx + (2 * qpair->id + 0) * pctrlr->doorbell_stride_u32; + pqpair->cq_eventidx = ctrlr->eventidx + (2 * qpair->id + 1) * pctrlr->doorbell_stride_u32; + } nvme_pcie_qpair_reset(qpair); return 0; @@ -1934,7 +1986,11 @@ nvme_pcie_qpair_process_completions(struct spdk_nvme_qpair *qpair, uint32_t max_ if (num_completions > 0) { g_thread_mmio_ctrlr = pctrlr; - spdk_mmio_write_4(pqpair->cq_hdbl, pqpair->cq_head); + if (spdk_likely(nvme_pcie_qpair_update_mmio_required(qpair, pqpair->cq_head, + pqpair->cq_shadow_hdbl, + pqpair->cq_eventidx))) { + spdk_mmio_write_4(pqpair->cq_hdbl, pqpair->cq_head); + } g_thread_mmio_ctrlr = NULL; } diff --git a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c index b82368124..b8c05c055 100644 --- a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c +++ b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c @@ -1574,6 +1574,28 @@ test_spdk_nvme_ctrlr_update_firmware(void) CU_ASSERT(ret == 0); } +int +nvme_ctrlr_cmd_doorbell_buffer_config(struct spdk_nvme_ctrlr *ctrlr, uint64_t prp1, uint64_t prp2, + spdk_nvme_cmd_cb cb_fn, void *cb_arg) +{ + fake_cpl_success(cb_fn, cb_arg); + return 0; +} + +static void +test_spdk_nvme_ctrlr_doorbell_buffer_config(void) +{ + struct spdk_nvme_ctrlr ctrlr = {}; + int ret = -1; + + ctrlr.cdata.oacs.doorbell_buffer_config = 1; + ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_PCIE; + ctrlr.page_size = 0x1000; + ret = nvme_ctrlr_set_doorbell_buffer_config(&ctrlr); + CU_ASSERT(ret == 0); + nvme_ctrlr_free_doorbell_buffer(&ctrlr); +} + int main(int argc, char **argv) { CU_pSuite suite = NULL; @@ -1616,6 +1638,8 @@ int main(int argc, char **argv) test_nvme_ctrlr_construct_intel_support_log_page_list) == NULL || CU_add_test(suite, "test nvme ctrlr function nvme_ctrlr_set_supported_features", test_nvme_ctrlr_set_supported_features) == NULL + || CU_add_test(suite, "test nvme ctrlr function nvme_ctrlr_set_doorbell_buffer_config", + test_spdk_nvme_ctrlr_doorbell_buffer_config) == NULL #if 0 /* TODO: move to PCIe-specific unit test */ || CU_add_test(suite, "test nvme ctrlr function nvme_ctrlr_alloc_cmb", test_nvme_ctrlr_alloc_cmb) == NULL diff --git a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c index 1bb0cb084..b6876ce80 100644 --- a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c +++ b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c @@ -785,6 +785,18 @@ test_prp_list_append(void) (NVME_MAX_PRP_LIST_ENTRIES + 1) * 0x1000, 0x1000) == -EINVAL); } +static void test_shadow_doorbell_update(void) +{ + bool ret; + + /* nvme_pcie_qpair_need_event(uint16_t event_idx, uint16_t new_idx, uint16_t old) */ + ret = nvme_pcie_qpair_need_event(10, 15, 14); + CU_ASSERT(ret == false); + + ret = nvme_pcie_qpair_need_event(14, 15, 14); + CU_ASSERT(ret == true); +} + int main(int argc, char **argv) { CU_pSuite suite = NULL; @@ -801,7 +813,8 @@ int main(int argc, char **argv) } if (CU_add_test(suite, "prp_list_append", test_prp_list_append) == NULL - ) { + || CU_add_test(suite, "shadow_doorbell_update", + test_shadow_doorbell_update) == NULL) { CU_cleanup_registry(); return CU_get_error(); }