The firmware version sysfs entry needs to be updated after a successfully
firmware activation.
nvme-cli stopped issuing an Identify Controller command to list the
current firmware information and relies on sysfs showing the current
firmware version.
Signed-off-by: Daniel Wagner <[email protected]>
---
drivers/nvme/host/core.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 62612f87aafa..bb15d878e8a2 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4079,6 +4079,20 @@ static void nvme_get_fw_slot_info(struct nvme_ctrl *ctrl)
kfree(log);
}
+static void nvme_update_firmware_rev(struct nvme_ctrl *ctrl)
+{
+ struct nvme_id_ctrl *id;
+ int ret;
+
+ ret = nvme_identify_ctrl(ctrl, &id);
+ if (ret) {
+ dev_warn(ctrl->device, "Identify Controller failed (%d)\n", ret);
+ return;
+ }
+ memcpy(ctrl->subsys->firmware_rev, id->fr,
+ sizeof(ctrl->subsys->firmware_rev));
+}
+
static void nvme_fw_act_work(struct work_struct *work)
{
struct nvme_ctrl *ctrl = container_of(work,
@@ -4109,6 +4123,7 @@ static void nvme_fw_act_work(struct work_struct *work)
nvme_unquiesce_io_queues(ctrl);
/* read FW slot information to clear the AER */
nvme_get_fw_slot_info(ctrl);
+ nvme_update_firmware_rev(ctrl);
queue_work(nvme_wq, &ctrl->async_event_work);
}
--
2.42.0
On Fri, Oct 13, 2023 at 08:26:23AM +0200, Daniel Wagner wrote:
> The firmware version sysfs entry needs to be updated after a successfully
> firmware activation.
>
> nvme-cli stopped issuing an Identify Controller command to list the
> current firmware information and relies on sysfs showing the current
> firmware version.
>
> Signed-off-by: Daniel Wagner <[email protected]>
> ---
> drivers/nvme/host/core.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 62612f87aafa..bb15d878e8a2 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -4079,6 +4079,20 @@ static void nvme_get_fw_slot_info(struct nvme_ctrl *ctrl)
> kfree(log);
> }
>
> +static void nvme_update_firmware_rev(struct nvme_ctrl *ctrl)
> +{
> + struct nvme_id_ctrl *id;
> + int ret;
> +
> + ret = nvme_identify_ctrl(ctrl, &id);
Hello Daniel,
I understand that nvme_fw_act_work() is called when
receiving a NVME_AER_NOTICE_FW_ACT_STARTING AEN.
This AEN is received when a Firmware Activate command was
issued with value 0x3 "The image specified by the Firmware Slot
field is requested to be activated immediately without reset."
However, when upgrading firmware (even without a reset),
can't more things other than FW version change?
Sure, I understand that there is no need to do a reset
(as in the cases there the firmware deems that a reset is
needed to do the FW activation without reset, the Firmware Activate
command with value 0x3 returns a failure), but it just seems like
more fields than FW version needs to be re-read.
Theoretically, can't a FW upgrade go from NVMe 1.4 to NVMe 2.0,
and that firmware can return success for the Firmware Activate
command with value 0x3?
Kind regards,
Niklas
On Fri, Oct 13, 2023 at 08:26:23AM +0200, Daniel Wagner wrote:
> +static void nvme_update_firmware_rev(struct nvme_ctrl *ctrl)
> +{
> + struct nvme_id_ctrl *id;
> + int ret;
> +
> + ret = nvme_identify_ctrl(ctrl, &id);
> + if (ret) {
> + dev_warn(ctrl->device, "Identify Controller failed (%d)\n", ret);
> + return;
> + }
> + memcpy(ctrl->subsys->firmware_rev, id->fr,
> + sizeof(ctrl->subsys->firmware_rev));
> +}
> +
> static void nvme_fw_act_work(struct work_struct *work)
> {
> struct nvme_ctrl *ctrl = container_of(work,
> @@ -4109,6 +4123,7 @@ static void nvme_fw_act_work(struct work_struct *work)
> nvme_unquiesce_io_queues(ctrl);
> /* read FW slot information to clear the AER */
> nvme_get_fw_slot_info(ctrl);
> + nvme_update_firmware_rev(ctrl);
The "fw_slot_info()" call also gets the firmware version, so no need to
do it a different way. Just add the memcpy to that function instead of
introducing a new one.
On Fri, Oct 13, 2023 at 01:02:29PM +0000, Niklas Cassel wrote:
>
> However, when upgrading firmware (even without a reset),
> can't more things other than FW version change?
New firmware can change all sorts of things, but I think firmware_rev is
the only exported sysfs attribute we expect to change. Other attributes,
like model, serial and unique id's, should be stable across these types
of firmware upgrades. I don't think "activation without reset" can be
used for firmware upgrades that change fundamental ways the device
operates.