From f2168e1d736a9986cb82450d59389b5c0dc5df00 Mon Sep 17 00:00:00 2001 From: Cunyin Chang Date: Tue, 3 May 2016 13:18:39 +0800 Subject: [PATCH] nvme: Add firmware upgrade interface and unit test suite Change-Id: If66e5f97f6793df0388629fab7c3d0e9f9d5eb67 Signed-off-by: Cunyin Chang --- include/spdk/nvme.h | 15 ++++ include/spdk/nvme_spec.h | 47 +++++++++++- lib/nvme/nvme_ctrlr.c | 71 +++++++++++++++++++ lib/nvme/nvme_ctrlr_cmd.c | 55 ++++++++++++++ lib/nvme/nvme_internal.h | 6 ++ .../nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c | 15 ++++ .../unit/nvme_ctrlr_cmd_c/nvme_ctrlr_cmd_ut.c | 42 +++++++++++ 7 files changed, 250 insertions(+), 1 deletion(-) diff --git a/include/spdk/nvme.h b/include/spdk/nvme.h index 253dcfcf8..572fb96d1 100644 --- a/include/spdk/nvme.h +++ b/include/spdk/nvme.h @@ -478,6 +478,21 @@ int spdk_nvme_ctrlr_delete_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid); int spdk_nvme_ctrlr_format(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, struct spdk_nvme_format *format); +/** + * \brief Download a new firmware image. + * + * \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. + * + * \return 0 if successfully submitted, ENOMEM if resources could not be allocated for this request, + * -1 if the size is not multiple of 4. + * + * 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); + /** * \brief Get the identify namespace data as defined by the NVMe specification. * diff --git a/include/spdk/nvme_spec.h b/include/spdk/nvme_spec.h index 19d972b54..321074281 100644 --- a/include/spdk/nvme_spec.h +++ b/include/spdk/nvme_spec.h @@ -664,7 +664,10 @@ struct __attribute__((packed)) spdk_nvme_ctrlr_data { /* number of firmware slots */ uint8_t num_slots : 3; - uint8_t frmw_rsvd : 4; + /* support activation without reset */ + uint8_t activation_without_reset : 1; + + uint8_t frmw_rsvd : 3; } frmw; /** log page attributes */ @@ -1283,6 +1286,48 @@ struct spdk_nvme_protection_info { }; SPDK_STATIC_ASSERT(sizeof(struct spdk_nvme_protection_info) == 8, "Incorrect size"); +/** Parameters for SPDK_NVME_OPC_FIRMWARE_COMMIT cdw10: commit action */ +enum spdk_nvme_fw_commit_action { + /** + * Downloaded image replaces the image specified by + * the Firmware Slot field. This image is not activated. + */ + SPDK_NVME_FW_COMMIT_REPLACE_IMG = 0x0, + /** + * Downloaded image replaces the image specified by + * the Firmware Slot field. This image is activated at the next reset. + */ + SPDK_NVME_FW_COMMIT_REPLACE_AND_ENABLE_IMG = 0x1, + /** + * The image specified by the Firmware Slot field is + * activated at the next reset. + */ + SPDK_NVME_FW_COMMIT_ENABLE_IMG = 0x2, + /** + * The image specified by the Firmware Slot field is + * requested to be activated immediately without reset. + */ + SPDK_NVME_FW_COMMIT_RUN_IMG = 0x3, +}; + +/** Parameters for SPDK_NVME_OPC_FIRMWARE_COMMIT cdw10 */ +struct spdk_nvme_fw_commit { + /** + * Firmware Slot. Specifies the firmware slot that shall be used for the + * Commit Action. The controller shall choose the firmware slot (slot 1 - 7) + * to use for the operation if the value specified is 0h. + */ + uint32_t fs : 3; + /** + * Commit Action. Specifies the action that is taken on the image downloaded + * with the Firmware Image Download command or on a previously downloaded and + * placed image. + */ + uint32_t ca : 3; + uint32_t reserved : 26; +}; +SPDK_STATIC_ASSERT(sizeof(struct spdk_nvme_fw_commit) == 4, "Incorrect size"); + #define spdk_nvme_cpl_is_error(cpl) \ ((cpl)->status.sc != 0 || (cpl)->status.sct != 0) diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 8334dd084..80d936b7c 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -1205,3 +1205,74 @@ spdk_nvme_ctrlr_format(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, return spdk_nvme_ctrlr_reset(ctrlr); } + +int +spdk_nvme_ctrlr_update_firmware(struct spdk_nvme_ctrlr *ctrlr, void *payload, uint32_t size, + int slot) +{ + struct spdk_nvme_fw_commit fw_commit; + struct nvme_completion_poll_status status; + int res; + unsigned int size_remaining; + unsigned int offset; + unsigned int transfer; + void *p; + + if (size % 4) { + nvme_printf(ctrlr, "spdk_nvme_ctrlr_update_firmware invalid size!\n"); + return -1; + } + + /* Firmware download */ + size_remaining = size; + offset = 0; + p = payload; + + while (size_remaining > 0) { + transfer = nvme_min(size_remaining, ctrlr->min_page_size); + status.done = false; + + res = nvme_ctrlr_cmd_fw_image_download(ctrlr, transfer, offset, p, + nvme_completion_poll_cb, + &status); + if (res) + return res; + + while (status.done == false) { + nvme_mutex_lock(&ctrlr->ctrlr_lock); + spdk_nvme_qpair_process_completions(&ctrlr->adminq, 0); + nvme_mutex_unlock(&ctrlr->ctrlr_lock); + } + if (spdk_nvme_cpl_is_error(&status.cpl)) { + nvme_printf(ctrlr, "spdk_nvme_ctrlr_fw_image_download failed!\n"); + return ENXIO; + } + p += transfer; + offset += transfer; + size_remaining -= transfer; + } + + /* 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; + + status.done = false; + + res = nvme_ctrlr_cmd_fw_commit(ctrlr, &fw_commit, nvme_completion_poll_cb, + &status); + if (res) + return res; + + while (status.done == false) { + nvme_mutex_lock(&ctrlr->ctrlr_lock); + spdk_nvme_qpair_process_completions(&ctrlr->adminq, 0); + nvme_mutex_unlock(&ctrlr->ctrlr_lock); + } + if (spdk_nvme_cpl_is_error(&status.cpl)) { + nvme_printf(ctrlr, "nvme_ctrlr_cmd_fw_commit failed!\n"); + return ENXIO; + } + + return spdk_nvme_ctrlr_reset(ctrlr); +} diff --git a/lib/nvme/nvme_ctrlr_cmd.c b/lib/nvme/nvme_ctrlr_cmd.c index 7d0c9920c..d07410929 100644 --- a/lib/nvme/nvme_ctrlr_cmd.c +++ b/lib/nvme/nvme_ctrlr_cmd.c @@ -480,3 +480,58 @@ nvme_ctrlr_cmd_abort(struct spdk_nvme_ctrlr *ctrlr, uint16_t cid, return nvme_ctrlr_submit_admin_request(ctrlr, req); } + +int +nvme_ctrlr_cmd_fw_commit(struct spdk_nvme_ctrlr *ctrlr, + const struct spdk_nvme_fw_commit *fw_commit, + spdk_nvme_cmd_cb cb_fn, void *cb_arg) +{ + struct nvme_request *req; + struct spdk_nvme_cmd *cmd; + int rc; + + nvme_mutex_lock(&ctrlr->ctrlr_lock); + req = nvme_allocate_request_null(cb_fn, cb_arg); + if (req == NULL) { + nvme_mutex_unlock(&ctrlr->ctrlr_lock); + return ENOMEM; + } + + cmd = &req->cmd; + cmd->opc = SPDK_NVME_OPC_FIRMWARE_COMMIT; + memcpy(&cmd->cdw10, fw_commit, sizeof(uint32_t)); + + rc = nvme_ctrlr_submit_admin_request(ctrlr, req); + nvme_mutex_unlock(&ctrlr->ctrlr_lock); + + return rc; + +} + +int +nvme_ctrlr_cmd_fw_image_download(struct spdk_nvme_ctrlr *ctrlr, + uint32_t size, uint32_t offset, void *payload, + spdk_nvme_cmd_cb cb_fn, void *cb_arg) +{ + struct nvme_request *req; + struct spdk_nvme_cmd *cmd; + int rc; + + nvme_mutex_lock(&ctrlr->ctrlr_lock); + req = nvme_allocate_request_contig(payload, size, + cb_fn, cb_arg); + if (req == NULL) { + nvme_mutex_unlock(&ctrlr->ctrlr_lock); + return ENOMEM; + } + + cmd = &req->cmd; + cmd->opc = SPDK_NVME_OPC_FIRMWARE_IMAGE_DOWNLOAD; + cmd->cdw10 = (size >> 2) - 1; + cmd->cdw11 = offset >> 2; + + rc = nvme_ctrlr_submit_admin_request(ctrlr, req); + nvme_mutex_unlock(&ctrlr->ctrlr_lock); + + return rc; +} diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 88c137419..379af9ec3 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -523,6 +523,12 @@ int nvme_ctrlr_cmd_delete_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, spdk_ void *cb_arg); 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); +int nvme_ctrlr_cmd_fw_commit(struct spdk_nvme_ctrlr *ctrlr, + const struct spdk_nvme_fw_commit *fw_commit, + spdk_nvme_cmd_cb cb_fn, void *cb_arg); +int nvme_ctrlr_cmd_fw_image_download(struct spdk_nvme_ctrlr *ctrlr, + uint32_t size, uint32_t offset, void *payload, + spdk_nvme_cmd_cb cb_fn, void *cb_arg); void nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl); int nvme_ctrlr_construct(struct spdk_nvme_ctrlr *ctrlr, void *devhandle); diff --git a/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c b/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c index c459b76bf..55f08a085 100644 --- a/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c +++ b/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c @@ -253,6 +253,21 @@ nvme_ctrlr_cmd_format(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, struct spdk_ return 0; } +int +nvme_ctrlr_cmd_fw_commit(struct spdk_nvme_ctrlr *ctrlr, const struct spdk_nvme_fw_commit *fw_commit, + spdk_nvme_cmd_cb cb_fn, void *cb_arg) +{ + return 0; +} + +int +nvme_ctrlr_cmd_fw_image_download(struct spdk_nvme_ctrlr *ctrlr, + uint32_t size, uint32_t offset, void *payload, + spdk_nvme_cmd_cb cb_fn, void *cb_arg) +{ + return 0; +} + void nvme_ns_destruct(struct spdk_nvme_ns *ns) { diff --git a/test/lib/nvme/unit/nvme_ctrlr_cmd_c/nvme_ctrlr_cmd_ut.c b/test/lib/nvme/unit/nvme_ctrlr_cmd_c/nvme_ctrlr_cmd_ut.c index d8f76446f..0c53c1dbd 100644 --- a/test/lib/nvme/unit/nvme_ctrlr_cmd_c/nvme_ctrlr_cmd_ut.c +++ b/test/lib/nvme/unit/nvme_ctrlr_cmd_c/nvme_ctrlr_cmd_ut.c @@ -49,6 +49,8 @@ uint32_t feature_cdw11 = 1; uint32_t feature_cdw12 = 1; uint8_t get_feature = 1; uint32_t get_feature_cdw11 = 1; +uint32_t fw_img_size = 1024; +uint32_t fw_img_offset = 0; uint16_t abort_cid = 1; uint16_t abort_sqid = 1; uint32_t namespace_management_nsid = 1; @@ -225,6 +227,19 @@ static void verify_format_nvme(struct nvme_request *req) CU_ASSERT(req->cmd.nsid == format_nvme_nsid); } +static void verify_fw_commit(struct nvme_request *req) +{ + CU_ASSERT(req->cmd.opc == SPDK_NVME_OPC_FIRMWARE_COMMIT); + CU_ASSERT(req->cmd.cdw10 == 0x09); +} + +static void verify_fw_image_download(struct nvme_request *req) +{ + CU_ASSERT(req->cmd.opc == SPDK_NVME_OPC_FIRMWARE_IMAGE_DOWNLOAD); + CU_ASSERT(req->cmd.cdw10 == (fw_img_size >> 2) - 1); + CU_ASSERT(req->cmd.cdw11 == fw_img_offset >> 2); +} + struct nvme_request * nvme_allocate_request(const struct nvme_payload *payload, uint32_t payload_size, spdk_nvme_cmd_cb cb_fn, @@ -513,6 +528,31 @@ test_format_nvme(void) nvme_ctrlr_cmd_format(&ctrlr, format_nvme_nsid, &format, NULL, NULL); } +static void +test_fw_commit(void) +{ + struct spdk_nvme_ctrlr ctrlr = {}; + struct spdk_nvme_fw_commit fw_commit = {}; + + fw_commit.ca = SPDK_NVME_FW_COMMIT_REPLACE_AND_ENABLE_IMG; + fw_commit.fs = 1; + + verify_fn = verify_fw_commit; + + nvme_ctrlr_cmd_fw_commit(&ctrlr, &fw_commit, NULL, NULL); +} + +static void +test_fw_image_download(void) +{ + struct spdk_nvme_ctrlr ctrlr = {}; + + verify_fn = verify_fw_image_download; + + nvme_ctrlr_cmd_fw_image_download(&ctrlr, fw_img_size, fw_img_offset, NULL, + NULL, NULL); +} + int main(int argc, char **argv) { CU_pSuite suite = NULL; @@ -539,6 +579,8 @@ int main(int argc, char **argv) || CU_add_test(suite, "test ctrlr cmd namespace_create", test_namespace_create) == NULL || CU_add_test(suite, "test ctrlr cmd namespace_delete", test_namespace_delete) == NULL || CU_add_test(suite, "test ctrlr cmd format_nvme", test_format_nvme) == NULL + || CU_add_test(suite, "test ctrlr cmd fw_commit", test_fw_commit) == NULL + || CU_add_test(suite, "test ctrlr cmd fw_image_download", test_fw_image_download) == NULL ) { CU_cleanup_registry(); return CU_get_error();