From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sughosh Ganu <sughosh.ganu@linaro.org>
Date: Sun, 21 Jun 2020 00:27:07 +0530
Subject: [PATCH v2 13/17] efi_loader: add firmware management protocol for
 raw image
In-Reply-To: <20200617025515.23585-14-takahiro.akashi@linaro.org>
References: <20200617025515.23585-1-takahiro.akashi@linaro.org>
 <20200617025515.23585-14-takahiro.akashi@linaro.org>
Message-ID: <CADg8p95-px+1gT2rX-PvxjUhg+F-ew2ZX_tKwCUykG5HPyWkMA@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Wed, 17 Jun 2020 at 08:26, AKASHI Takahiro <takahiro.akashi@linaro.org>
wrote:

> In this commit, a very simple firmware management protocol driver
> is implemented. It will take a binary image in a capsule file and
> apply the data using dfu backend storage drivers via dfu_write_by_alt()
> interface.
>
> So "dfu_alt_info" variable should be properly set to specify a device
> and location to be updated. Please read README.dfu.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  include/efi_api.h             |   4 +
>  include/efi_loader.h          |   1 +
>  lib/efi_loader/Kconfig        |  16 ++
>  lib/efi_loader/Makefile       |   2 +-
>  lib/efi_loader/efi_capsule.c  |   8 +
>  lib/efi_loader/efi_firmware.c | 273 ++++++++++++++++++++++++++++------
>  6 files changed, 256 insertions(+), 48 deletions(-)
>

<snip>


> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 28ce5647a2cc..db8fdf30ace0 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -13,6 +13,68 @@
>  #include <image.h>
>  #include <linux/list.h>
>
>
<snip>


> +#ifdef CONFIG_EFI_CAPSULE_FIRMWARE_RAW
> +/*
> + * This FIRMWARE_MANAGEMENT_PROTOCOL driver provides a firmware update
> + * method with raw data.
> + */
> +const efi_guid_t efi_firmware_image_type_uboot_raw =
> +       EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
>
> -/* Place holder; not supported */
> +/**
> + * efi_firmware_raw_get_image_info - return information about the current
> +                                    firmware image
> + * @this:                      Protocol instance
> + * @image_info_size:           Size of @image_info
> + * @image_info:                        Image information
> + * @descriptor_version:                Pointer to version number
> + * @descriptor_count:          Pointer to number of descriptors
> + * @descriptor_size:           Pointer to descriptor size
> + * package_version:            Package version
> + * package_version_name:       Package version's name
> + *
> + * Return information bout the current firmware image in @image_info.
> + * @image_info will consist of a number of descriptors.
> + * Each descriptor will be created based on "dfu_alt_info" variable.
> + *
> + * Return              status code
> + */
>  static
> -efi_status_t EFIAPI efi_firmware_get_package_info_unsupported(
> +efi_status_t EFIAPI efi_firmware_raw_get_image_info(
>         struct efi_firmware_management_protocol *this,
> +       efi_uintn_t *image_info_size,
> +       struct efi_firmware_image_descriptor *image_info,
> +       u32 *descriptor_version,
> +       u8 *descriptor_count,
> +       efi_uintn_t *descriptor_size,
>         u32 *package_version,
> -       u16 **package_version_name,
> -       u32 *package_version_name_maxlen,
> -       u64 *attributes_supported,
> -       u64 *attributes_setting)
> +       u16 **package_version_name)
>  {
> -       EFI_ENTRY("%p %p %p %p %p %p\n", this, package_version,
> -                 package_version_name, package_version_name_maxlen,
> -                 attributes_supported, attributes_setting);
> +       struct dfu_entity *dfu;
> +       size_t names_len, total_size;
> +       int dfu_num, i;
> +       u16 *name, *next;
> +       efi_status_t ret = EFI_SUCCESS;
>
> -       return EFI_EXIT(EFI_UNSUPPORTED);
> +       EFI_ENTRY("%p %p %p %p %p %p %p %p\n", this,
> +                 image_info_size, image_info,
> +                 descriptor_version, descriptor_count, descriptor_size,
> +                 package_version, package_version_name);
> +
> +       if (!image_info_size || (*image_info_size && !image_info))
> +               return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> +       dfu_init_env_entities(NULL, NULL);
> +
> +       names_len = 0;
> +       dfu_num = 0;
> +       list_for_each_entry(dfu, &dfu_list, list) {
> +               names_len += (utf8_utf16_strlen(dfu->name) + 1) * 2;
> +               dfu_num++;
> +       }
> +       if (!dfu_num) {
> +               EFI_PRINT("Probably dfu_alt_info not defined\n");
> +               *image_info_size = 0;
> +               dfu_free_entities();
> +
> +               return EFI_EXIT(EFI_SUCCESS);
> +       }
> +
> +       total_size = sizeof(*image_info) * dfu_num + names_len;
> +       /*
> +        * we will assume that sizeof(*image_info) * dfu_name
> +        * is, at least, a multiple of 2. So the start address for
> +        * image_id_name would be aligned with 2 bytes.
> +        */
> +       if (*image_info_size < total_size) {
> +               *image_info_size = total_size;
> +               dfu_free_entities();
> +
> +               return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
> +       }
> +       *image_info_size = total_size;
> +
> +       if (descriptor_version)
> +               *descriptor_version =
> EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> +       if (descriptor_count)
> +               *descriptor_count = dfu_num;
> +       if (descriptor_size)
> +               *descriptor_size = sizeof(*image_info);
> +       if (package_version)
> +               *package_version = 0xffffffff; /* not supported */
> +       if (package_version_name)
> +               *package_version_name = NULL; /* not supported */
> +
> +       /* DFU alt number should correspond to image_index */
> +       i = 0;
> +       /* Name area starts just after descriptors */
> +       name = (u16 *)((u8 *)image_info + sizeof(*image_info) * dfu_num);
> +       next = name;
> +       list_for_each_entry(dfu, &dfu_list, list) {
> +               image_info[i].image_index = dfu->alt + 1;
> +               image_info[i].image_type_id =
> efi_firmware_image_type_uboot_raw;
> +               image_info[i].image_id = dfu->alt;
> +
> +               /* copy the DFU entity name */
> +               utf8_utf16_strcpy(&next, dfu->name);
> +               image_info[i].image_id_name = name;
> +               name = ++next;
> +
> +               image_info[i].version = 0; /* not supported */
> +               image_info[i].version_name = NULL; /* not supported */
> +               image_info[i].size = 0;
> +               image_info[i].attributes_supported =
> +                               EFI_IMAGE_ATTRIBUTE_IMAGE_UPDATABLE;
> +               image_info[i].attributes_setting =
> +                               EFI_IMAGE_ATTRIBUTE_IMAGE_UPDATABLE;
> +               image_info[i].lowest_supported_image_version = 0;
> +               image_info[i].last_attempt_version = 0;
> +               image_info[i].last_attempt_status =
> LAST_ATTEMPT_STATUS_SUCCESS;
> +               image_info[i].hardware_instance = 1;
> +               image_info[i].dependencies = NULL;
> +
> +               i++;
> +       }
> +
> +       dfu_free_entities();
> +
> +       return EFI_EXIT(ret);
>  }
>

I think this function is the same as that used for the FIT image, except
for the image_type_id. I guess the common part can be refactored into a
common function.


>
> -/* Place holder; not supported */
> +/**
> + * efi_firmware_raw_set_image - update the firmware image
> + * @this:              Protocol instance
> + * @image_index:       Image index number
> + * @image:             New image
> + * @image_size:                Size of new image
> + * @vendor_code:       Vendor-specific update policy
> + * @progress:          Function to report the progress of update
> + * @abort_reason:      Pointer to string of abort reason
> + *
> + * Update the firmware to new image, using dfu. The new image should
> + * be a single raw image.
> + * @vendor_code, @progress and @abort_reason are not supported.
> + *
> + * Return:             status code
> + */
>  static
> -efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
> +efi_status_t EFIAPI efi_firmware_raw_set_image(
>         struct efi_firmware_management_protocol *this,
> +       u8 image_index,
>         const void *image,
> -       efi_uintn_t *image_size,
> +       efi_uintn_t image_size,
>         const void *vendor_code,
> -       u32 package_version,
> -       const u16 *package_version_name)
> +       efi_status_t (*progress)(efi_uintn_t completion),
> +       u16 **abort_reason)
>  {
> -       EFI_ENTRY("%p %p %p %p %x %p\n", this, image, image_size,
> vendor_code,
> -                 package_version, package_version_name);
> +       EFI_ENTRY("%p %d %p %ld %p %p %p\n", this, image_index, image,
> +                 image_size, vendor_code, progress, abort_reason);
>
> -       return EFI_EXIT(EFI_UNSUPPORTED);
> +       if (!image)
> +               return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> +       if (dfu_write_by_alt(image_index - 1, (uintptr_t)image, image_size,
> +                            NULL, NULL))
>

As stated in an earlier patch, we need to pass the image parameter as a
void pointer.

+               return EFI_EXIT(EFI_DEVICE_ERROR);
> +
> +       return EFI_EXIT(EFI_SUCCESS);
>  }
>

A print, or at least a debug message stating the status of the capsule
update would be helpful. Same applies for the set_image implementation for
the FIT images as well.

-sughosh