11203 Support for NVMe drive firmware updates

Review Request #2013 - Created June 21, 2019 and updated

Information
Paul Winder
illumos-gate
Reviewers
general
hans, rm

Adds three new sub-commands to nvmeadm:

  • nvmeadm load-firmware - loads firmware files into the drive's holding area.
  • nvmeadm commit-firmware - takes the firmware in the holding area and commits to a slot.
  • nvmeadm activate-firmware - activates a given slot. The firmware in the active slot is loaded by the drive when it is next reset, and becomes the running firmware.

https://www.illumos.org/issues/11203 has an example of how the commands are used.

This process is used by WDC to update NVMe firmware in their IntelliFlash products during manufacturing process.

Issues

  • 0
  • 27
  • 2
  • 29
Description From Last Updated
C Fraire
Hans Rosenfeld
Paul Winder
C Fraire
Robert Mustacchi
Hans Rosenfeld
Gergő Mihály Doma
Hans Rosenfeld
C Fraire
Gergő Mihály Doma
Gergő Mihály Doma
Robert Mustacchi
C Fraire
Paul Winder
Hans Rosenfeld
Paul Winder
Review request changed

Change Summary:

Changes are because of this email from Gergő Mihály Doma.
"I have some bad news.

First, get_slot_number() also affected by the negation in the input.
The second is that when I tested get_slot_number() I found a bug
in the strtoul() implementation.
(See here: Bug #11326 - libc: strtoul() forgets to set errno to EINVAL when no conversion could be performed)

Because of these reasons, I recommend the following replacements in get_slot_number():
"ulong_t slot;" with "long slot;" and
"strtoul" with "strtol".

Another thing what I noticed is in the error handling:
strtol(), strtoll(), etc. functions can set errno to 'ERANGE' which triggers the "numeric slot required"
error message (in get_slot_number()).
This can be confusing, because the entered value is really numeric, just way too large (or too low).
'get_slot_number()' accepts anything which starts as numeric, but 'get_fw_offsetb()' accepts arguments only
numeric, why are they so different?
* If get_slot_number() going to be more restrictive, then a more appropriate error message will be necessary.
For example, when somebody wants to "multi-slotting" (like "1, 2, 3" with double quotes) now accepted as
slot number 1. In a restrictive version with old error message the user will see this: "numeric slot required".
Maybe something more general error message would be better, just like in 'get_fw_offsetb()': "slot \"%s\" is invalid".

Gergő"

There was a question of "multi-slotting". It was never intended to do multi-slotting - only one slot can be specified.

I have changed some of the error messages:
root@omnios-athena:~# nvmeadm load-firmware nvme3 KNGND113.bin -234232
nvmeadm: Offset must be numeric and in the range of 0 to 17179869180
root@omnios-athena:~# nvmeadm load-firmware nvme3 KNGND113.bin 234213443444
nvmeadm: Offset must be numeric and in the range of 0 to 17179869180
root@omnios-athena:~# nvmeadm load-firmware nvme3 KNGND113.bin 2344x
nvmeadm: Offset must be numeric and in the range of 0 to 17179869180
root@omnios-athena:~# nvmeadm load-firmware nvme3 KNGND113.bin dsdsf
nvmeadm: Offset must be numeric and in the range of 0 to 17179869180
root@omnios-athena:~# nvmeadm load-firmware nvme3 KNGND113.bin 23443
nvmeadm: Offset must be multiple of 4

root@omnios-athena:~# nvmeadm commit-firmware nvme3 -2312
nvmeadm: Slot must be numeric and in the range of 1 to 7
root@omnios-athena:~# nvmeadm commit-firmware nvme3 xxx
nvmeadm: Slot must be numeric and in the range of 1 to 7
root@omnios-athena:~# nvmeadm commit-firmware nvme3 1x
nvmeadm: Slot must be numeric and in the range of 1 to 7
root@omnios-athena:~# nvmeadm commit-firmware nvme3 1345234513451354341
nvmeadm: Slot must be numeric and in the range of 1 to 7
root@omnios-athena:~# nvmeadm commit-firmware nvme3 "1,2"
nvmeadm: Slot must be numeric and in the range of 1 to 7
root@omnios-athena:~# nvmeadm commit-firmware nvme3 1,2
nvmeadm: Slot must be numeric and in the range of 1 to 7

Diff:

Revision 6 (+840 -95)

Show changes

Gergő Mihály Doma
Ship It!
C Fraire
Ship It!
Loading...