Re: [PATCH v7 09/13] FWU: Add support for the FWU Multi Bank Update feature

Hi Sughosh,

[...]

#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */

+static bool fwu_empty_capsule(struct efi_capsule_header *capsule) +{

•   return !guidcmp(&capsule->capsule_guid,
  

•                   &fwu_guid_os_request_fw_revert) ||
  

•           !guidcmp(&capsule->capsule_guid,
  

•                    &fwu_guid_os_request_fw_accept);
  

+}

+static efi_status_t fwu_empty_capsule_process(

•   struct efi_capsule_header *capsule)
  

+{

•   int status;
  

•   u32 active_idx;
  

•   efi_status_t ret;
  

•   efi_guid_t *image_guid;
  

•   if (!guidcmp(&capsule->capsule_guid,
  

•                &fwu_guid_os_request_fw_revert)) {
  

•           /*
  

•            * One of the previously updated image has
  

•            * failed the OS acceptance test. OS has
  

•            * requested to revert back to the earlier
  

•            * boot index
  

•            */
  

•           status = fwu_revert_boot_index();
  

•           if (status < 0) {
  

•                   log_err("Failed to revert the FWU boot index\n");
  

•                   if (status == -ENODEV ||
  

•                       status == -ERANGE ||
  

•                       status == -EIO)
  

•                           ret = EFI_DEVICE_ERROR;
  

•                   else if (status == -EINVAL)
  

•                           ret = EFI_INVALID_PARAMETER;
  

•                   else
  

•                           ret = EFI_OUT_OF_RESOURCES;
  

In all the case you carry those if statements, define a function like

static efi_status_t fwu_to_efi_error (int err) { switch(err) { case -ENODEV: case -ERANGE: case -EIO: return EFI_DEVICE_ERROR; } ..... } and use it in the error handling below. That should make the weird looking error handling go away and adding more cases in the future a lot easier.

•           } else {
  

•                   ret = EFI_SUCCESS;
  

•                   log_err("Reverted the FWU active_index. Recommend rebooting the system\n");
  

Does it really need log_err?

[...]

Regards /Ilias