diff mbox series

[v2,13/17] efi_loader: add firmware management protocol for raw image

Message ID 20200617025515.23585-14-takahiro.akashi@linaro.org
State New
Headers show
Series efi_loader: add capsule update support | expand

Commit Message

AKASHI Takahiro June 17, 2020, 2:55 a.m. UTC
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 at 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(-)

Comments

Sughosh Ganu June 20, 2020, 6:57 p.m. UTC | #1
On Wed, 17 Jun 2020 at 08:26, AKASHI Takahiro <takahiro.akashi at 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 at 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
AKASHI Takahiro June 22, 2020, 1:21 a.m. UTC | #2
On Sun, Jun 21, 2020 at 12:27:07AM +0530, Sughosh Ganu wrote:
> On Wed, 17 Jun 2020 at 08:26, AKASHI Takahiro <takahiro.akashi at 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 at 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.

If you don't need to add anything here for your patch,
I will extract a common function here.

> 
> >
> > -/* 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.

Well, we already have error messages in efi_capsule_update_firmware(),
while another message can be placed specifically in efi_fmp_find().

-Takahiro Akashi

> 
> -sughosh
Sughosh Ganu June 22, 2020, 7:53 a.m. UTC | #3
On Mon, 22 Jun 2020 at 06:51, AKASHI Takahiro <takahiro.akashi at linaro.org>
wrote:

> On Sun, Jun 21, 2020 at 12:27:07AM +0530, Sughosh Ganu wrote:
> > On Wed, 17 Jun 2020 at 08:26, AKASHI Takahiro <
> takahiro.akashi at 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 at 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.
>
> If you don't need to add anything here for your patch,
> I will extract a common function here.
>

Yes, please do.


>
> >
> > >
> > > -/* 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.
>
> Well, we already have error messages in efi_capsule_update_firmware(),
> while another message can be placed specifically in efi_fmp_find().
>

I think that it would be good to have a debug message about the final
status of the update. A debug message would not hurt.

-sughosh
diff mbox series

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index c3fc4edbedc4..afd7ff922ee4 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -1847,6 +1847,10 @@  struct efi_signature_list {
 	EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \
 		 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47)
 
+#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID \
+	EFI_GUID(0xe2bb9c06, 0x70e9, 0x4b14, 0x97, 0xa3, \
+		 0x5a, 0x79, 0x13, 0x17, 0x6e, 0x3f)
+
 #define EFI_IMAGE_ATTRIBUTE_IMAGE_UPDATABLE		0x1
 #define EFI_IMAGE_ATTRIBUTE_RESET_REQUIRED		0x2
 #define EFI_IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED	0x4
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 5f574533e732..5c5f0e165d11 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -788,6 +788,7 @@  bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
 		     WIN_CERTIFICATE **auth, size_t *auth_len);
 
 extern const struct efi_firmware_management_protocol efi_fmp_fit;
+extern const struct efi_firmware_management_protocol efi_fmp_raw;
 
 /* Capsule update */
 efi_status_t EFIAPI efi_update_capsule(
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 305751f4b15c..eed33744a019 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -78,6 +78,10 @@  config EFI_CAPSULE_ON_DISK_EARLY
 	  executed as part of U-Boot initialisation so that they will
 	  surely take place whatever is set to distro_bootcmd.
 
+config EFI_CAPSULE_FIRMWARE
+	bool
+	default n
+
 config EFI_CAPSULE_FIRMWARE_MANAGEMENT
 	bool "Capsule: Firmware Management Protocol"
 	depends on EFI_HAVE_CAPSULE_SUPPORT
@@ -92,11 +96,23 @@  config EFI_CAPSULE_FIRMWARE_FIT
 	depends on FIT
 	select UPDATE_FIT
 	select DFU
+	select EFI_CAPSULE_FIRMWARE
 	default n
 	help
 	  Select this option if you want to enable firmware management protocol
 	  driver for FIT image
 
+config EFI_CAPSULE_FIRMWARE_RAW
+	bool "FMP driver for raw image"
+	depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
+	select DFU
+	select DFU_ALT
+	select EFI_CAPSULE_FIRMWARE
+	default n
+	help
+	  Select this option if you want to enable firmware management protocol
+	  driver for raw image
+
 config EFI_DEVICE_PATH_TO_TEXT
 	bool "Device path to text protocol"
 	default y
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index ea1fbc4a9deb..08ce3bf56429 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -24,7 +24,7 @@  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
 obj-y += efi_bootmgr.o
 obj-y += efi_boottime.o
 obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
-obj-$(CONFIG_EFI_CAPSULE_FIRMWARE_FIT) += efi_firmware.o
+obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
 obj-y += efi_console.o
 obj-y += efi_device_path.o
 obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index c2cd1d23f9ce..360b40011417 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -811,6 +811,14 @@  efi_status_t __weak arch_efi_load_capsule_drivers(void)
 				&efi_fmp_fit, NULL));
 	}
 
+	if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)) {
+		handle = NULL;
+		ret = EFI_CALL(efi_install_multiple_protocol_interfaces(
+				&efi_root,
+				&efi_guid_firmware_management_protocol,
+				&efi_fmp_raw, NULL));
+	}
+
 	return ret;
 }
 
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>
 
+/* Place holder; not supported */
+static
+efi_status_t EFIAPI efi_firmware_get_image_unsupported(
+	struct efi_firmware_management_protocol *this,
+	u8 image_index,
+	void *image,
+	efi_uintn_t *image_size)
+{
+	EFI_ENTRY("%p %d %p %p\n", this, image_index, image, image_size);
+
+	return EFI_EXIT(EFI_UNSUPPORTED);
+}
+
+/* Place holder; not supported */
+static
+efi_status_t EFIAPI efi_firmware_check_image_unsupported(
+	struct efi_firmware_management_protocol *this,
+	u8 image_index,
+	const void *image,
+	efi_uintn_t *image_size,
+	u32 *image_updatable)
+{
+	EFI_ENTRY("%p %d %p %p %p\n", this, image_index, image, image_size,
+		  image_updatable);
+
+	return EFI_EXIT(EFI_UNSUPPORTED);
+}
+
+/* Place holder; not supported */
+static
+efi_status_t EFIAPI efi_firmware_get_package_info_unsupported(
+	struct efi_firmware_management_protocol *this,
+	u32 *package_version,
+	u16 **package_version_name,
+	u32 *package_version_name_maxlen,
+	u64 *attributes_supported,
+	u64 *attributes_setting)
+{
+	EFI_ENTRY("%p %p %p %p %p %p\n", this, package_version,
+		  package_version_name, package_version_name_maxlen,
+		  attributes_supported, attributes_setting);
+
+	return EFI_EXIT(EFI_UNSUPPORTED);
+}
+
+/* Place holder; not supported */
+static
+efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
+	struct efi_firmware_management_protocol *this,
+	const void *image,
+	efi_uintn_t *image_size,
+	const void *vendor_code,
+	u32 package_version,
+	const u16 *package_version_name)
+{
+	EFI_ENTRY("%p %p %p %p %x %p\n", this, image, image_size, vendor_code,
+		  package_version, package_version_name);
+
+	return EFI_EXIT(EFI_UNSUPPORTED);
+}
+
+#ifdef CONFIG_EFI_CAPSULE_FIRMWARE_FIT
 /*
  * This FIRMWARE_MANAGEMENT_PROTOCOL driver provides a firmware update
  * method with existing FIT image format, and handles
@@ -144,19 +206,6 @@  efi_status_t EFIAPI efi_firmware_fit_get_image_info(
 	return EFI_EXIT(ret);
 }
 
-/* Place holder; not supported */
-static
-efi_status_t EFIAPI efi_firmware_get_image_unsupported(
-	struct efi_firmware_management_protocol *this,
-	u8 image_index,
-	void *image,
-	efi_uintn_t *image_size)
-{
-	EFI_ENTRY("%p %d %p %p\n", this, image_index, image, image_size);
-
-	return EFI_EXIT(EFI_UNSUPPORTED);
-}
-
 /**
  * efi_firmware_fit_set_image - update the firmware image
  * @this:		Protocol instance
@@ -195,59 +244,189 @@  efi_status_t EFIAPI efi_firmware_fit_set_image(
 	return EFI_EXIT(EFI_SUCCESS);
 }
 
-/* Place holder; not supported */
-static
-efi_status_t EFIAPI efi_firmware_check_image_unsupported(
-	struct efi_firmware_management_protocol *this,
-	u8 image_index,
-	const void *image,
-	efi_uintn_t *image_size,
-	u32 *image_updatable)
-{
-	EFI_ENTRY("%p %d %p %p %p\n", this, image_index, image, image_size,
-		  image_updatable);
+const struct efi_firmware_management_protocol efi_fmp_fit = {
+	.get_image_info = efi_firmware_fit_get_image_info,
+	.get_image = efi_firmware_get_image_unsupported,
+	.set_image = efi_firmware_fit_set_image,
+	.check_image = efi_firmware_check_image_unsupported,
+	.get_package_info = efi_firmware_get_package_info_unsupported,
+	.set_package_info = efi_firmware_set_package_info_unsupported,
+};
+#endif /* CONFIG_EFI_CAPSULE_FIRMWARE_FIT */
 
-	return EFI_EXIT(EFI_UNSUPPORTED);
-}
+#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);
 }
 
-/* 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))
+		return EFI_EXIT(EFI_DEVICE_ERROR);
+
+	return EFI_EXIT(EFI_SUCCESS);
 }
 
-const struct efi_firmware_management_protocol efi_fmp_fit = {
-	.get_image_info = efi_firmware_fit_get_image_info,
+const struct efi_firmware_management_protocol efi_fmp_raw = {
+	.get_image_info = efi_firmware_raw_get_image_info,
 	.get_image = efi_firmware_get_image_unsupported,
-	.set_image = efi_firmware_fit_set_image,
+	.set_image = efi_firmware_raw_set_image,
 	.check_image = efi_firmware_check_image_unsupported,
 	.get_package_info = efi_firmware_get_package_info_unsupported,
 	.set_package_info = efi_firmware_set_package_info_unsupported,
 };
+#endif /* CONFIG_EFI_CAPSULE_FIRMWARE_RAW */