diff --git a/examples/nvme/nvme_manage/nvme_manage.c b/examples/nvme/nvme_manage/nvme_manage.c index 82541cc88..15d04a7de 100644 --- a/examples/nvme/nvme_manage/nvme_manage.c +++ b/examples/nvme/nvme_manage/nvme_manage.c @@ -771,6 +771,8 @@ update_firmware_image(void) void *fw_image; struct dev *ctrlr; const struct spdk_nvme_ctrlr_data *cdata; + enum spdk_nvme_fw_commit_action commit_action; + struct spdk_nvme_status status; ctrlr = get_controller(); if (ctrlr == NULL) { @@ -836,8 +838,12 @@ update_firmware_image(void) return; } - rc = spdk_nvme_ctrlr_update_firmware(ctrlr->ctrlr, fw_image, size, slot); - if (rc) { + commit_action = SPDK_NVME_FW_COMMIT_REPLACE_AND_ENABLE_IMG; + rc = spdk_nvme_ctrlr_update_firmware(ctrlr->ctrlr, fw_image, size, slot, commit_action, &status); + if (rc == -ENXIO && status.sct == SPDK_NVME_SCT_COMMAND_SPECIFIC && + status.sc == SPDK_NVME_SC_FIRMWARE_REQ_CONVENTIONAL_RESET) { + printf("conventional reset is needed to enable firmware !\n"); + } else if (rc) { printf("spdk_nvme_ctrlr_update_firmware failed\n"); } else { printf("spdk_nvme_ctrlr_update_firmware success\n"); diff --git a/include/spdk/nvme.h b/include/spdk/nvme.h index 49105fd5d..38b96a48c 100644 --- a/include/spdk/nvme.h +++ b/include/spdk/nvme.h @@ -771,6 +771,8 @@ int spdk_nvme_ctrlr_format(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, * \param payload The data buffer for the firmware image. * \param size The data size will be downloaded. * \param slot The slot that the firmware image will be committed to. + * \param commit_action The action to perform when firmware is committed. + * \param completion_status output parameter. Contains the completion status of the firmware commit operation. * * \return 0 if successfully submitted, ENOMEM if resources could not be allocated for this request, * -1 if the size is not multiple of 4. @@ -778,7 +780,8 @@ int spdk_nvme_ctrlr_format(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, * This function is thread safe and can be called at any point after spdk_nvme_attach(). */ int spdk_nvme_ctrlr_update_firmware(struct spdk_nvme_ctrlr *ctrlr, void *payload, uint32_t size, - int slot); + int slot, enum spdk_nvme_fw_commit_action commit_action, + struct spdk_nvme_status *completion_status); /** * \brief Get the identify namespace data as defined by the NVMe specification. diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 14faed869..ee62752f8 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -1683,7 +1683,7 @@ spdk_nvme_ctrlr_format(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, int spdk_nvme_ctrlr_update_firmware(struct spdk_nvme_ctrlr *ctrlr, void *payload, uint32_t size, - int slot) + int slot, enum spdk_nvme_fw_commit_action commit_action, struct spdk_nvme_status *completion_status) { struct spdk_nvme_fw_commit fw_commit; struct nvme_completion_poll_status status; @@ -1693,11 +1693,24 @@ spdk_nvme_ctrlr_update_firmware(struct spdk_nvme_ctrlr *ctrlr, void *payload, ui unsigned int transfer; void *p; + if (!completion_status) { + return -EINVAL; + } + memset(completion_status, 0, sizeof(struct spdk_nvme_status)); if (size % 4) { SPDK_ERRLOG("spdk_nvme_ctrlr_update_firmware invalid size!\n"); return -1; } + /* Current support only for SPDK_NVME_FW_COMMIT_REPLACE_IMG + * and SPDK_NVME_FW_COMMIT_REPLACE_AND_ENABLE_IMG + */ + if ((commit_action != SPDK_NVME_FW_COMMIT_REPLACE_IMG) && + (commit_action != SPDK_NVME_FW_COMMIT_REPLACE_AND_ENABLE_IMG)) { + SPDK_ERRLOG("spdk_nvme_ctrlr_update_firmware invalid command!\n"); + return -1; + } + /* Firmware download */ size_remaining = size; offset = 0; @@ -1730,7 +1743,7 @@ spdk_nvme_ctrlr_update_firmware(struct spdk_nvme_ctrlr *ctrlr, void *payload, ui /* Firmware commit */ memset(&fw_commit, 0, sizeof(struct spdk_nvme_fw_commit)); fw_commit.fs = slot; - fw_commit.ca = SPDK_NVME_FW_COMMIT_REPLACE_IMG; + fw_commit.ca = commit_action; status.done = false; @@ -1744,9 +1757,18 @@ spdk_nvme_ctrlr_update_firmware(struct spdk_nvme_ctrlr *ctrlr, void *payload, ui spdk_nvme_qpair_process_completions(ctrlr->adminq, 0); nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock); } + memcpy(completion_status, &status.cpl.status, sizeof(struct spdk_nvme_status)); if (spdk_nvme_cpl_is_error(&status.cpl)) { - SPDK_ERRLOG("nvme_ctrlr_cmd_fw_commit failed!\n"); - return -ENXIO; + if (status.cpl.status.sct != SPDK_NVME_SCT_COMMAND_SPECIFIC || + status.cpl.status.sc != SPDK_NVME_SC_FIRMWARE_REQ_NVM_RESET) { + if (status.cpl.status.sct == SPDK_NVME_SCT_COMMAND_SPECIFIC && + status.cpl.status.sc == SPDK_NVME_SC_FIRMWARE_REQ_CONVENTIONAL_RESET) { + SPDK_NOTICELOG("firmware activation requires conventional reset to be performed. !\n"); + } else { + SPDK_ERRLOG("nvme_ctrlr_cmd_fw_commit failed!\n"); + } + return -ENXIO; + } } return spdk_nvme_ctrlr_reset(ctrlr); 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 818cdd4ed..48ebbae91 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 @@ -1401,23 +1401,25 @@ test_spdk_nvme_ctrlr_update_firmware(void) int point_payload = 1; int slot = 0; int ret = 0; + struct spdk_nvme_status status; + enum spdk_nvme_fw_commit_action commit_action = SPDK_NVME_FW_COMMIT_REPLACE_IMG; /* Set invalid size check function return value */ set_size = 5; - ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot); + ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot, commit_action, &status); CU_ASSERT(ret == -1); /* When payload is NULL but set_size < min_page_size */ set_size = 4; ctrlr.min_page_size = 5; - ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot); + ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot, commit_action, &status); CU_ASSERT(ret == -1); /* When payload not NULL but min_page_size is 0 */ set_size = 4; ctrlr.min_page_size = 0; payload = &point_payload; - ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot); + ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot, commit_action, &status); CU_ASSERT(ret == -1); /* Check firmware image download when payload not NULL and min_page_size not 0 , status.cpl value is 1 */ @@ -1425,7 +1427,7 @@ test_spdk_nvme_ctrlr_update_firmware(void) set_size = 4; ctrlr.min_page_size = 5; payload = &point_payload; - ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot); + ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot, commit_action, &status); CU_ASSERT(ret == -ENXIO); /* Check firmware image download and set status.cpl value is 0 */ @@ -1433,7 +1435,7 @@ test_spdk_nvme_ctrlr_update_firmware(void) set_size = 4; ctrlr.min_page_size = 5; payload = &point_payload; - ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot); + ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot, commit_action, &status); CU_ASSERT(ret == -1); /* Check firmware commit */ @@ -1443,7 +1445,7 @@ test_spdk_nvme_ctrlr_update_firmware(void) set_size = 4; ctrlr.min_page_size = 5; payload = &point_payload; - ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot); + ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot, commit_action, &status); CU_ASSERT(ret == -ENXIO); /* Set size check firmware download and firmware commit */ @@ -1453,7 +1455,7 @@ test_spdk_nvme_ctrlr_update_firmware(void) set_size = 4; ctrlr.min_page_size = 5; payload = &point_payload; - ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot); + ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot, commit_action, &status); CU_ASSERT(ret == 0); }