disk_updater: Upgrade firmware immediately if possible

When upgrading with action 3 (update firmware without reboot),
we need first to upload the firmware with action 0 (upload without update).
Use one unused slot to do the operation.
If slot N is used, use slot N+1 or the first writable slot if we are
exceeding the number of slots.

BUG=chromium:669596
TEST=Unit test + tested on Eve. Build 2 firmware packages, and ping-pong
between firmware.

Change-Id: I6382d1a248beec6a330ba89cca94b1de00e30923
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/636702
Reviewed-by: Mike Frysinger <vapier@chromium.org>
(cherry picked from commit 976d03d733bfc7c557c5a6a1382532cd87c6a472)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1136990
Reviewed-by: Wei-Han Chen <stimim@chromium.org>
diff --git a/disk_updater/scripts/chromeos-disk-firmware-update.sh b/disk_updater/scripts/chromeos-disk-firmware-update.sh
index c251166..042d0ee 100755
--- a/disk_updater/scripts/chromeos-disk-firmware-update.sh
+++ b/disk_updater/scripts/chromeos-disk-firmware-update.sh
@@ -14,13 +14,13 @@
 # Temperaray directory to put device information
 DEFINE_string 'tmp_dir' '' "Use existing temporary directory."
 DEFINE_string 'fw_package_dir' '' "Location of the firmware package."
-DEFINE_string 'hdparm' '/sbin/hdparm' "hdparm binary to use."
+DEFINE_string 'hdparm' 'hdparm' "hdparm binary to use."
 DEFINE_string 'hdparm_kingston' '/opt/google/disk/bin/hdparm_kingston' \
               "hdparm for kingston recovery."
-DEFINE_string 'smartctl' '/usr/sbin/smartctl' "smartctl binary to use."
-DEFINE_string 'pwr_suspend' '/usr/bin/powerd_dbus_suspend' "To power cycle SSD"
-DEFINE_string 'mmc' '/usr/bin/mmc' "mmc binary to use."
-DEFINE_string 'nvme' '/usr/sbin/nvme' "nvme binary to use."
+DEFINE_string 'smartctl' 'smartctl' "smartctl binary to use."
+DEFINE_string 'pwr_suspend' 'powerd_dbus_suspend' "To power cycle SSD"
+DEFINE_string 'mmc' 'mmc' "mmc binary to use."
+DEFINE_string 'nvme' 'nvme' "nvme binary to use."
 DEFINE_string 'status' '' "Status file to write to."
 DEFINE_boolean 'test' ${FLAGS_FALSE} "For unit testing."
 
@@ -30,6 +30,7 @@
 #   disk_fw_file
 #   disk_exp_fw_rev
 #   disk_fw_opt
+#   nvme_out : A file where the output of "nvme id-ctrl" is stored.
 
 log_msg() {
   logger -t "chromeos-disk-firmware-update[${PPID}]" "$@"
@@ -45,6 +46,13 @@
 FLAGS "$@" || exit 1
 eval set -- "${FLAGS_ARGV}"
 
+# program_installed - check if the specified program is installed.
+program_installed() {
+  if ! command -v "$1" > /dev/null; then
+    log_msg "$1 is not installed"
+    return 1
+  fi
+}
 # disk_fw_select - Select the proper disk firmware to use.
 #
 # This code reuse old installer disk firmware upgrade code.
@@ -120,8 +128,7 @@
   local device="$1"
 
   # Test if we have the tool needed for an upgrade.
-  if [ ! -x "${FLAGS_hdparm}" ]; then
-    log_msg "hdparm is not installed"
+  if ! program_installed "${FLAGS_hdparm}"; then
     return 1
   fi
 
@@ -164,8 +171,7 @@
   disk_fw_rev=$(cat "/sys/block/$1/device/fwrev")
 
   # Test if we have the tool needed for an upgrade.
-  if [ ! -x "${FLAGS_mmc}" ]; then
-    log_msg "mmc is not installed"
+  if ! program_installed "${FLAGS_mmc}"; then
     return 1
   fi
 
@@ -228,11 +234,10 @@
 disk_nvme_info() {
   local device="$1"
   local rc=0
-  local nvme_out="${FLAGS_tmp_dir}/${device}"
+  nvme_out="${FLAGS_tmp_dir}/${device}"
 
   # Test if we have the tool needed for an upgrade.
-  if [ ! -x "${FLAGS_nvme}" ]; then
-    log_msg "nvme is not installed"
+  if ! program_installed "${FLAGS_mmc}"; then
     return 1
   fi
 
@@ -479,6 +484,62 @@
   echo 1 > "/sys/block/${device}n1/device/device/reset"
 }
 
+# disk_nvme_current_slot - Retrieve which slot the current firmware comes from.
+#
+# Information is located in the first byte of Log 3
+# (Firmware Slot Information Log).
+#
+# Return the slot number, assume nvme device is up.
+disk_nvme_current_slot() {
+  local device="$1"
+
+  local byte0="$("${FLAGS_nvme}" get-log "/dev/${device}" \
+      --raw-binary --log-id 3 --log-len 512 | \
+      od -An -t u1 -N 1)"
+  echo $((byte0 & 0x3))
+}
+
+# disk_nvme_get_frmw - get FRMW from identity data.
+#
+# Use already collected id-ctrl data, and extrace the Firmware Updates field.
+# The fields have the following format:
+#
+#  7    5  4 3   1 0
+#  +------+-+-----+-+
+#  |      | |     | |
+#  +------+-+-----+-+
+#          \   \   \
+#           \   \   --- slot 1 is read only, can not be use for update.
+#            \   ------ number of slot available (between 1 and 7)
+#             --------- action 3 (upgrade without reset available)
+disk_nvme_get_frmw() {
+  local device="$1"
+  grep "frmw" "${nvme_out}" | cut -d ':' -f 2
+}
+
+disk_nvme_get_min_writable_slot() {
+  local frmw="$1"
+  # Slot 1 can be read only, check frmw bit 0.
+  echo $(((frmw & 0x1) + 1))
+}
+
+disk_nvme_get_max_writable_slot() {
+  local frmw="$1"
+  # Bit 1 - 3 of frmw contains the number of slots.
+  echo $(((frmw & 0xe) >> 1))
+}
+
+# disk_nvme_action_supported - Return supported action
+#
+# Return the commit action the device is supporting:
+# - 3 : the device can upgrade without reset,
+# - 2 : the device needs reset to upgrade (mandatory).
+disk_nvme_action_supported() {
+  local frmw="$1"
+  # Bit 4 of frmw indicates if upgrade without reset is supported.
+  echo $((((frmw & 0x10) >> 4) + 2))
+}
+
 # disk_nvme_upgrade - Upgrade the firmware on the NVME storage
 #
 # Update the firmware on the disk.
@@ -486,7 +547,10 @@
 # inputs:
 #     device            -- the device name [nvme0,...]
 #     fw_file           -- the firmware image
-#     fw_options        -- the options from the rule file. (unused)
+#     fw_options        -- the options from the rule file.
+#
+# By default the NVMe device can be upgraded without reset (commit action 3),
+# see NVMe 1.3 Figure 76. If reset is needed, we use commit action 2.
 #
 # returns non 0 on error
 #
@@ -494,20 +558,49 @@
   local device="$1"
   local fw_file="$2"
   local fw_options="$3"
+  local frmw="$(disk_nvme_get_frmw "${device}")"
+  local action="$(disk_nvme_action_supported "${frmw}")"
+  local curr_slot="$(disk_nvme_current_slot "${device}")"
+  local min_slot="$(disk_nvme_get_min_writable_slot "${frmw}")"
+  local max_slot="$(disk_nvme_get_max_writable_slot "${frmw}")"
+  local new_slot rc
+
+  if [ ${curr_slot} -eq ${max_slot} ]; then
+    new_slot=${min_slot}
+  else
+    new_slot=$((curr_slot + 1))
+  fi
+  if echo "${fw_options}" | grep -q "separate_slot"; then
+    if [ ${new_slot} -eq ${curr_slot} ]; then
+      log_msg "Unable to find proper slot: current ${curr_slot}, " \
+              "min: ${min_slot}, max: ${max_slot}"
+      return 1
+    fi
+  fi
 
   "${FLAGS_nvme}" fw-download "/dev/${device}" --fw="${fw_file}"
-  if [ $? -ne 0 ]; then
-    log_msg "Unable to download ${fw_file} to  ${device}"
-    return $?
+  rc=$?
+  if [ ${rc} -ne 0 ]; then
+    log_msg "Unable to download ${fw_file} to ${device}"
+    return ${rc}
   fi
 
-  "${FLAGS_nvme}" fw-activate "/dev/${device}" --slot=1 --action=1
-  if [ $? -ne 0 ]; then
-    log_msg "Unable to activate ${fw_file} to  ${device}"
-    return $?
+  # Use action 0 to download image into slot.
+  "${FLAGS_nvme}" fw-activate "/dev/${device}" --slot="${new_slot}" --action=0
+  rc=$?
+  if [ ${rc} -ne 0 ]; then
+     log_msg "Unable to load ${fw_file} to ${device}"
+     return ${rc}
   fi
-
-  disk_nmve_reset "${device}"
+  "${FLAGS_nvme}" fw-activate "/dev/${device}" --slot="${new_slot}" \
+    --action="${action}"
+  rc=$?
+  if [ ${rc} -eq 11 -a ${action} -eq 2 ]; then
+    disk_nmve_reset "${device}"
+  elif [ ${rc} -ne 0 ]; then
+    log_msg "Unable to activate ${fw_file} to ${device}"
+    return ${rc}
+  fi
 }
 
 # disk_upgrade - Upgrade the firmware on the disk
diff --git a/disk_updater/tests/INTEL_SSDPEKKW256G7-PSF101C.nvme b/disk_updater/tests/INTEL_SSDPEKKW256G7-PSF101C.nvme
new file mode 100644
index 0000000..d10e251
--- /dev/null
+++ b/disk_updater/tests/INTEL_SSDPEKKW256G7-PSF101C.nvme
@@ -0,0 +1,55 @@
+NVME Identify Controller:
+vid     : 0x8086
+ssvid   : 0x8086
+sn      : BTPY634205V1256D    
+mn      : INTEL SSDPEKKW256G7                     
+fr      :  PSF100C
+rab     : 6
+ieee    : 5cd2e4
+cmic    : 0
+mdts    : 5
+cntlid  : 1
+ver     : 10200
+rtd3r   : 249f0
+rtd3e   : 13880
+oaes    : 0
+oacs    : 0x6
+acl     : 4
+aerl    : 7
+frmw    : 0x2
+lpa     : 0x3
+elpe    : 63
+npss    : 4
+avscc   : 0
+apsta   : 0x1
+wctemp  : 343
+cctemp  : 353
+mtfa    : 20
+hmpre   : 0
+hmmin   : 0
+tnvmcap : 0
+unvmcap : 0
+rpmbs   : 0
+sqes    : 0x66
+cqes    : 0x44
+nn      : 1
+oncs    : 0x1e
+fuses   : 0
+fna     : 0x4
+vwc     : 0x1
+awun    : 0
+awupf   : 0
+nvscc   : 0
+acwu    : 0
+sgls    : 0
+subnqn  : 
+ps    0 : mp:9.00W operational enlat:5 exlat:5 rrt:0 rrl:0
+          rwt:0 rwl:0 idle_power:- active_power:-
+ps    1 : mp:4.60W operational enlat:30 exlat:30 rrt:1 rrl:1
+          rwt:1 rwl:1 idle_power:- active_power:-
+ps    2 : mp:3.80W operational enlat:30 exlat:30 rrt:2 rrl:2
+          rwt:2 rwl:2 idle_power:- active_power:-
+ps    3 : mp:0.0700W non-operational enlat:10000 exlat:300 rrt:3 rrl:3
+          rwt:3 rwl:3 idle_power:- active_power:-
+ps    4 : mp:0.0050W non-operational enlat:2000 exlat:10000 rrt:4 rrl:4
+          rwt:4 rwl:4 idle_power:- active_power:-
diff --git a/disk_updater/tests/SAMSUNG_KUS040205M-DXC81G1E.nvme b/disk_updater/tests/SAMSUNG_KUS040205M-DXC81G1E.nvme
new file mode 100644
index 0000000..4f3efdf
--- /dev/null
+++ b/disk_updater/tests/SAMSUNG_KUS040205M-DXC81G1E.nvme
@@ -0,0 +1,55 @@
+NVME Identify Controller:
+vid     : 0x144d
+ssvid   : 0x144d
+sn      : S3VBNY0J708187      
+mn      : SAMSUNG KUS040205M-B001                 
+fr      : DXC81G1E
+rab     : 2
+ieee    : 002538
+cmic    : 0
+mdts    : 7
+cntlid  : 3
+ver     : 10200
+rtd3r   : 186a0
+rtd3e   : 989680
+oaes    : 0
+oacs    : 0x6
+acl     : 7
+aerl    : 3
+frmw    : 0x16
+lpa     : 0x3
+elpe    : 63
+npss    : 4
+avscc   : 0x1
+apsta   : 0x1
+wctemp  : 364
+cctemp  : 366
+mtfa    : 0
+hmpre   : 0
+hmmin   : 0
+tnvmcap : 512110190592
+unvmcap : 0
+rpmbs   : 0
+sqes    : 0x66
+cqes    : 0x44
+nn      : 1
+oncs    : 0x1f
+fuses   : 0
+fna     : 0
+vwc     : 0x1
+awun    : 255
+awupf   : 0
+nvscc   : 1
+acwu    : 0
+sgls    : 0
+subnqn  : 
+ps    0 : mp:3.00W operational enlat:0 exlat:0 rrt:0 rrl:0
+          rwt:0 rwl:0 idle_power:- active_power:-
+ps    1 : mp:2.40W operational enlat:5 exlat:5 rrt:1 rrl:1
+          rwt:1 rwl:1 idle_power:- active_power:-
+ps    2 : mp:1.90W operational enlat:10 exlat:10 rrt:2 rrl:2
+          rwt:2 rwl:2 idle_power:- active_power:-
+ps    3 : mp:0.0600W non-operational enlat:300 exlat:800 rrt:3 rrl:3
+          rwt:3 rwl:3 idle_power:- active_power:-
+ps    4 : mp:0.0050W non-operational enlat:1800 exlat:3700 rrt:4 rrl:4
+          rwt:4 rwl:4 idle_power:- active_power:-
diff --git a/disk_updater/tests/SAMSUNG_KUS040205M-DXC81G1T.nvme b/disk_updater/tests/SAMSUNG_KUS040205M-DXC81G1T.nvme
new file mode 100644
index 0000000..f87bc47
--- /dev/null
+++ b/disk_updater/tests/SAMSUNG_KUS040205M-DXC81G1T.nvme
@@ -0,0 +1,55 @@
+NVME Identify Controller:
+vid     : 0x144d
+ssvid   : 0x144d
+sn      : S3VBNY0J708187      
+mn      : SAMSUNG KUS040205M-B001                 
+fr      : DXC81G1T
+rab     : 2
+ieee    : 002538
+cmic    : 0
+mdts    : 7
+cntlid  : 3
+ver     : 10200
+rtd3r   : 186a0
+rtd3e   : 989680
+oaes    : 0
+oacs    : 0x6
+acl     : 7
+aerl    : 3
+frmw    : 0x16
+lpa     : 0x3
+elpe    : 63
+npss    : 4
+avscc   : 0x1
+apsta   : 0x1
+wctemp  : 364
+cctemp  : 366
+mtfa    : 0
+hmpre   : 0
+hmmin   : 0
+tnvmcap : 512110190592
+unvmcap : 0
+rpmbs   : 0
+sqes    : 0x66
+cqes    : 0x44
+nn      : 1
+oncs    : 0x1f
+fuses   : 0
+fna     : 0
+vwc     : 0x1
+awun    : 255
+awupf   : 0
+nvscc   : 1
+acwu    : 0
+sgls    : 0
+subnqn  : 
+ps    0 : mp:3.00W operational enlat:0 exlat:0 rrt:0 rrl:0
+          rwt:0 rwl:0 idle_power:- active_power:-
+ps    1 : mp:2.40W operational enlat:5 exlat:5 rrt:1 rrl:1
+          rwt:1 rwl:1 idle_power:- active_power:-
+ps    2 : mp:1.90W operational enlat:10 exlat:10 rrt:2 rrl:2
+          rwt:2 rwl:2 idle_power:- active_power:-
+ps    3 : mp:0.0600W non-operational enlat:300 exlat:800 rrt:3 rrl:3
+          rwt:3 rwl:3 idle_power:- active_power:-
+ps    4 : mp:0.0050W non-operational enlat:1800 exlat:3700 rrt:4 rrl:4
+          rwt:4 rwl:4 idle_power:- active_power:-
diff --git a/disk_updater/tests/chromeos-disk-firmware-nvme-test.sh b/disk_updater/tests/chromeos-disk-firmware-nvme-test.sh
index 298019d..dcc1319 100755
--- a/disk_updater/tests/chromeos-disk-firmware-nvme-test.sh
+++ b/disk_updater/tests/chromeos-disk-firmware-nvme-test.sh
@@ -80,9 +80,14 @@
   echo "nvme0"
 }
 
+disk_nvme_current_slot() {
+  echo "    1"
+}
+
+# Upgrade with reset.
 prepare_test
 nvme_id_files=(
-  'INTEL_SSDPEKKW256G7-PSF100C'
+  'INTEL_SSDPEKKW256G7-PSF101C'
   ''
   ''
   'INTEL_SSDPEKKW256G7-PSF109C'
@@ -115,12 +120,38 @@
 
 prepare_test
 nvme_id_files=(
-  'INTEL_SSDPEKKW256G7-PSF100C'
+  'INTEL_SSDPEKKW256G7-PSF101C'
 )
 nvme_id_rc=(0)
 run_test
 check_test 3 nvme_upgrade_failed 1 $?
 echo NVME PASS 3
 
+# Upgrade without reset.
+prepare_test
+nvme_id_files=(
+  'INTEL_SSDPEKKW256G7-PSF100C'
+  ''
+  ''
+  'INTEL_SSDPEKKW256G7-PSF109C'
+  'INTEL_SSDPEKKW256G7-PSF109C'
+)
+nvme_id_rc=(0 10 10 0 0)
+
+run_test
+check_test 4 nvme_upgraded 0 $?
+echo NVME PASS 4
+
+# Upgrade Samsung device
+prepare_test
+nvme_id_files=(
+  'SAMSUNG_KUS040205M-DXC81G1E'
+  'SAMSUNG_KUS040205M-DXC81G1T'
+  'SAMSUNG_KUS040205M-DXC81G1T'
+)
+nvme_id_rc=(0 0 0)
+run_test
+check_test 5 nvme_upgraded 0 $?
+echo NVME PASS 5
 
 rm -rf "${DISK_TEMP}"
diff --git a/disk_updater/tests/nvme_upgrade_failed_3 b/disk_updater/tests/nvme_upgrade_failed_3
index 6890f86..776333d 100644
--- a/disk_updater/tests/nvme_upgrade_failed_3
+++ b/disk_updater/tests/nvme_upgrade_failed_3
@@ -1,3 +1,4 @@
 nvme fw-download /dev/nvme0 --fw=tests/test_nvme_dir/fw.bin
-nvme fw-activate /dev/nvme0 --slot=1 --action=1
+nvme fw-activate /dev/nvme0 --slot=1 --action=0
+nvme fw-activate /dev/nvme0 --slot=1 --action=2
 Unable to upgrade nvme0 from PSF100C to PSF109C
diff --git a/disk_updater/tests/nvme_upgraded_1 b/disk_updater/tests/nvme_upgraded_1
index 3eaac57..16aeb3d 100644
--- a/disk_updater/tests/nvme_upgraded_1
+++ b/disk_updater/tests/nvme_upgraded_1
@@ -1,4 +1,5 @@
 nvme fw-download /dev/nvme0 --fw=tests/test_nvme_dir/fw.bin
-nvme fw-activate /dev/nvme0 --slot=1 --action=1
+nvme fw-activate /dev/nvme0 --slot=1 --action=0
+nvme fw-activate /dev/nvme0 --slot=1 --action=2
 mock reset for nvme0
 Upgraded nvme0:INTEL_SSDPEKKW256G7 from PSF100C to PSF109C
diff --git a/disk_updater/tests/nvme_upgraded_4 b/disk_updater/tests/nvme_upgraded_4
new file mode 100644
index 0000000..d337e2f
--- /dev/null
+++ b/disk_updater/tests/nvme_upgraded_4
@@ -0,0 +1,4 @@
+nvme fw-download /dev/nvme0 --fw=tests/test_nvme_dir/fw.bin
+nvme fw-activate /dev/nvme0 --slot=1 --action=0
+nvme fw-activate /dev/nvme0 --slot=1 --action=3
+Upgraded nvme0:INTEL_SSDPEKKW256G7 from PSF100C to PSF109C
diff --git a/disk_updater/tests/nvme_upgraded_5 b/disk_updater/tests/nvme_upgraded_5
new file mode 100644
index 0000000..ef837a9
--- /dev/null
+++ b/disk_updater/tests/nvme_upgraded_5
@@ -0,0 +1,4 @@
+nvme fw-download /dev/nvme0 --fw=tests/test_nvme_dir/fw.bin
+nvme fw-activate /dev/nvme0 --slot=2 --action=0
+nvme fw-activate /dev/nvme0 --slot=2 --action=3
+Upgraded nvme0:SAMSUNG_KUS040205M_B001 from DXC81G1E to DXC81G1T
diff --git a/disk_updater/tests/test_nvme_dir/rules b/disk_updater/tests/test_nvme_dir/rules
index 56a6daa..eedc74b 100644
--- a/disk_updater/tests/test_nvme_dir/rules
+++ b/disk_updater/tests/test_nvme_dir/rules
@@ -1 +1,4 @@
+# "reset_needed" for testing only, this SSD does not need it.
 INTEL_SSDPEKKW256G7 PSF100C PSF109C - fw.bin
+INTEL_SSDPEKKW256G7 PSF101C PSF109C - fw.bin
+SAMSUNG_KUS040205M_B001 DXC81G1E DXC81G1T pm971a fw.bin
diff --git a/disk_updater/tests/unit_test.sh b/disk_updater/tests/unit_test.sh
index 4192e68..6010ff8 100755
--- a/disk_updater/tests/unit_test.sh
+++ b/disk_updater/tests/unit_test.sh
@@ -4,4 +4,9 @@
 # found in the LICENSE file.
 #
 # Command for unit test.
-echo "$(basename "$0")" "$@" | sed "s#/tmp/test_fw\..\{6\}#temp_dir#g"
+echo "$(basename "$0")" "$@" | sed -E "s#/tmp/test_fw\..{6}#temp_dir#g"
+if echo "$@" | grep -qe "--action=2"; then
+  exit 11
+else
+  exit 0
+fi