[v9,04/11] efi_loader: capsule: support firmware update

Message ID 20201117002805.13902-5-takahiro.akashi@linaro.org
State New
Headers show
Series
  • efi_loader: add capsule update support
Related show

Commit Message

AKASHI Takahiro Nov. 17, 2020, 12:27 a.m.
A capsule tagged with the guid, EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID,
is handled as a firmware update object.
What efi_update_capsule() basically does is to load any firmware management
protocol (or fmp) drivers contained in a capsule, find out an appropriate
fmp driver and then invoke its set_image() interface against each binary
in a capsule.
In this commit, however, loading drivers is not supported.

The result of applying a capsule is set to be stored in "CapsuleXXXX"
variable, but its implementation is deferred to a fmp driver.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

---
 include/efi_api.h            | 129 +++++++++++++++++++
 include/efi_loader.h         |   2 +
 lib/efi_loader/Kconfig       |   8 ++
 lib/efi_loader/efi_capsule.c | 238 ++++++++++++++++++++++++++++++++++-
 lib/efi_loader/efi_setup.c   |   4 +
 5 files changed, 380 insertions(+), 1 deletion(-)

-- 
2.28.0

Comments

Sughosh Ganu Nov. 21, 2020, 6:02 p.m. | #1
hi Takahiro,

On Tue, 17 Nov 2020 at 05:58, AKASHI Takahiro <takahiro.akashi@linaro.org>
wrote:

> A capsule tagged with the guid, EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID,

> is handled as a firmware update object.

> What efi_update_capsule() basically does is to load any firmware management

> protocol (or fmp) drivers contained in a capsule, find out an appropriate

> fmp driver and then invoke its set_image() interface against each binary

> in a capsule.

> In this commit, however, loading drivers is not supported.

>

> The result of applying a capsule is set to be stored in "CapsuleXXXX"

> variable, but its implementation is deferred to a fmp driver.

>

> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

> ---

>  include/efi_api.h            | 129 +++++++++++++++++++

>  include/efi_loader.h         |   2 +

>  lib/efi_loader/Kconfig       |   8 ++

>  lib/efi_loader/efi_capsule.c | 238 ++++++++++++++++++++++++++++++++++-

>  lib/efi_loader/efi_setup.c   |   4 +

>  5 files changed, 380 insertions(+), 1 deletion(-)

>

> diff --git a/include/efi_api.h b/include/efi_api.h

> index 7a2a087c60ed..966bc6e590bf 100644

> --- a/include/efi_api.h

> +++ b/include/efi_api.h

> @@ -217,6 +217,9 @@ enum efi_reset_type {

>  #define CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE    0x00020000

>  #define CAPSULE_FLAGS_INITIATE_RESET           0x00040000

>

> +#define CAPSULE_SUPPORT_AUTHENTICATION         0x0000000000000001

> +#define CAPSULE_SUPPORT_DEPENDENCY             0x0000000000000002

> +

>  #define EFI_CAPSULE_REPORT_GUID \

>         EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \

>                  0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)

> @@ -225,6 +228,10 @@ enum efi_reset_type {

>         EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \

>                  0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)

>

> +#define EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID \

> +       EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \

> +                0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)

> +

>  struct efi_capsule_header {

>         efi_guid_t capsule_guid;

>         u32 header_size;

> @@ -253,6 +260,33 @@ struct efi_memory_range_capsule {

>         struct efi_memory_range memory_ranges[];

>  } __packed;

>

> +struct efi_firmware_management_capsule_header {

> +       u32 version;

> +       u16 embedded_driver_count;

> +       u16 payload_item_count;

> +       u64 item_offset_list[];

> +} __packed;

> +

> +struct efi_firmware_management_capsule_image_header {

> +       u32 version;

> +       efi_guid_t update_image_type_id;

> +       u8 update_image_index;

> +       u8 reserved[3];

> +       u32 update_image_size;

> +       u32 update_vendor_code_size;

> +       u64 update_hardware_instance;

> +       u64 image_capsule_support;

> +} __packed;

> +

> +struct efi_capsule_result_variable_fmp {

> +       u16 version;

> +       u8 payload_index;

> +       u8 update_image_index;

> +       efi_guid_t update_image_type_id;

> +       // u16 capsule_file_name[];

> +       // u16 capsule_target[];

> +} __packed;

> +

>


<snip>


> +/**

> + * efi_capsule_update_firmware - update firmware from capsule

> + * @capsule_data:      Capsule

> + *

> + * Update firmware, using a capsule, @capsule_data. Loading any FMP

> + * drivers embedded in a capsule is not supported.

> + *

> + * Return:             status code

> + */

> +static efi_status_t efi_capsule_update_firmware(

> +               struct efi_capsule_header *capsule_data)

> +{

> +       struct efi_firmware_management_capsule_header *capsule;

> +       struct efi_firmware_management_capsule_image_header *image;

> +       size_t capsule_size;

> +       void *image_binary, *vendor_code;

> +       efi_handle_t *handles;

> +       efi_uintn_t no_handles;

> +       int item;

> +       struct efi_firmware_management_protocol *fmp;

> +       u16 *abort_reason;

> +       efi_status_t ret = EFI_SUCCESS;

> +

> +       /* sanity check */

> +       if (capsule_data->header_size < sizeof(*capsule) ||

> +           capsule_data->header_size >= capsule_data->capsule_image_size)

> +               return EFI_INVALID_PARAMETER;

> +

> +       capsule = (void *)capsule_data + capsule_data->header_size;

> +       capsule_size = capsule_data->capsule_image_size

> +                       - capsule_data->header_size;

> +

> +       if (capsule->version != 0x00000001)

> +               return EFI_INVALID_PARAMETER;

> +

> +       /* Drivers */

> +       /* TODO: support loading drivers */

> +

> +       handles = NULL;

> +       ret = EFI_CALL(efi_locate_handle_buffer(

> +                       BY_PROTOCOL,

> +                       &efi_guid_firmware_management_protocol,

> +                       NULL, &no_handles, (efi_handle_t **)&handles));

> +       if (ret != EFI_SUCCESS)

> +               return EFI_UNSUPPORTED;

> +

> +       /* Payload */

> +       for (item = capsule->embedded_driver_count;

> +            item < capsule->embedded_driver_count

> +                   + capsule->payload_item_count; item++) {

> +               /* sanity check */

> +               if ((capsule->item_offset_list[item] + sizeof(*image)

> +                                >= capsule_size)) {

> +                       printf("EFI: A capsule has not enough size of

> data\n");

> +                       ret = EFI_INVALID_PARAMETER;

> +                       goto out;

> +               }

> +

> +               image = (void *)capsule + capsule->item_offset_list[item];

> +

> +               if (image->version != 0x00000001 &&

> +                   image->version != 0x00000002 &&

> +                   image->version != 0x00000003) {

> +                       ret = EFI_INVALID_PARAMETER;

> +                       goto out;

> +               }

> +

> +               /* find a device for update firmware */

> +               /* TODO: should we pass index as well, or nothing but

> type? */

> +               fmp = efi_fmp_find(&image->update_image_type_id,

> +                                  image->version == 0x1 ? 0 :

> +                                       image->update_hardware_instance,

> +                                  handles, no_handles);

> +               if (!fmp) {

> +                       printf("EFI Capsule: driver not found for firmware

> type: %pUl, hardware instance: %lld\n",

> +                              &image->update_image_type_id,

> +                              image->version == 0x1 ? 0 :

> +                                       image->update_hardware_instance);

> +                       ret = EFI_UNSUPPORTED;

> +                       goto out;

> +               }

> +

> +               /* do it */

> +               image_binary = (void *)image + sizeof(*image);

>


Sorry for not having spotted this earlier. But since we are supporting the
first three versions of the  struct
efi_firmware_management_capsule_image_header, the above logic needs to be
changed to accomodate versions 1 and 2. So in case of a version 1
structure, the fields update_hardware_instance and image_capsule_support
need to be discarded. Similarly for a version 2 structure, the
image_capsule_support field needs to be discarded while calculating the
image_binary pointer.

-sughosh
AKASHI Takahiro Nov. 24, 2020, 5:51 a.m. | #2
Sughosh,

On Sat, Nov 21, 2020 at 11:32:43PM +0530, Sughosh Ganu wrote:
> hi Takahiro,

> 

> On Tue, 17 Nov 2020 at 05:58, AKASHI Takahiro <takahiro.akashi@linaro.org>

> wrote:

> 

> > A capsule tagged with the guid, EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID,

> > is handled as a firmware update object.

> > What efi_update_capsule() basically does is to load any firmware management

> > protocol (or fmp) drivers contained in a capsule, find out an appropriate

> > fmp driver and then invoke its set_image() interface against each binary

> > in a capsule.

> > In this commit, however, loading drivers is not supported.

> >

> > The result of applying a capsule is set to be stored in "CapsuleXXXX"

> > variable, but its implementation is deferred to a fmp driver.

> >

> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

> > ---

> >  include/efi_api.h            | 129 +++++++++++++++++++

> >  include/efi_loader.h         |   2 +

> >  lib/efi_loader/Kconfig       |   8 ++

> >  lib/efi_loader/efi_capsule.c | 238 ++++++++++++++++++++++++++++++++++-

> >  lib/efi_loader/efi_setup.c   |   4 +

> >  5 files changed, 380 insertions(+), 1 deletion(-)

> >

> > diff --git a/include/efi_api.h b/include/efi_api.h

> > index 7a2a087c60ed..966bc6e590bf 100644

> > --- a/include/efi_api.h

> > +++ b/include/efi_api.h

> > @@ -217,6 +217,9 @@ enum efi_reset_type {

> >  #define CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE    0x00020000

> >  #define CAPSULE_FLAGS_INITIATE_RESET           0x00040000

> >

> > +#define CAPSULE_SUPPORT_AUTHENTICATION         0x0000000000000001

> > +#define CAPSULE_SUPPORT_DEPENDENCY             0x0000000000000002

> > +

> >  #define EFI_CAPSULE_REPORT_GUID \

> >         EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \

> >                  0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)

> > @@ -225,6 +228,10 @@ enum efi_reset_type {

> >         EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \

> >                  0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)

> >

> > +#define EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID \

> > +       EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \

> > +                0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)

> > +

> >  struct efi_capsule_header {

> >         efi_guid_t capsule_guid;

> >         u32 header_size;

> > @@ -253,6 +260,33 @@ struct efi_memory_range_capsule {

> >         struct efi_memory_range memory_ranges[];

> >  } __packed;

> >

> > +struct efi_firmware_management_capsule_header {

> > +       u32 version;

> > +       u16 embedded_driver_count;

> > +       u16 payload_item_count;

> > +       u64 item_offset_list[];

> > +} __packed;

> > +

> > +struct efi_firmware_management_capsule_image_header {

> > +       u32 version;

> > +       efi_guid_t update_image_type_id;

> > +       u8 update_image_index;

> > +       u8 reserved[3];

> > +       u32 update_image_size;

> > +       u32 update_vendor_code_size;

> > +       u64 update_hardware_instance;

> > +       u64 image_capsule_support;

> > +} __packed;

> > +

> > +struct efi_capsule_result_variable_fmp {

> > +       u16 version;

> > +       u8 payload_index;

> > +       u8 update_image_index;

> > +       efi_guid_t update_image_type_id;

> > +       // u16 capsule_file_name[];

> > +       // u16 capsule_target[];

> > +} __packed;

> > +

> >

> 

> <snip>

> 

> 

> > +/**

> > + * efi_capsule_update_firmware - update firmware from capsule

> > + * @capsule_data:      Capsule

> > + *

> > + * Update firmware, using a capsule, @capsule_data. Loading any FMP

> > + * drivers embedded in a capsule is not supported.

> > + *

> > + * Return:             status code

> > + */

> > +static efi_status_t efi_capsule_update_firmware(

> > +               struct efi_capsule_header *capsule_data)

> > +{

> > +       struct efi_firmware_management_capsule_header *capsule;

> > +       struct efi_firmware_management_capsule_image_header *image;

> > +       size_t capsule_size;

> > +       void *image_binary, *vendor_code;

> > +       efi_handle_t *handles;

> > +       efi_uintn_t no_handles;

> > +       int item;

> > +       struct efi_firmware_management_protocol *fmp;

> > +       u16 *abort_reason;

> > +       efi_status_t ret = EFI_SUCCESS;

> > +

> > +       /* sanity check */

> > +       if (capsule_data->header_size < sizeof(*capsule) ||

> > +           capsule_data->header_size >= capsule_data->capsule_image_size)

> > +               return EFI_INVALID_PARAMETER;

> > +

> > +       capsule = (void *)capsule_data + capsule_data->header_size;

> > +       capsule_size = capsule_data->capsule_image_size

> > +                       - capsule_data->header_size;

> > +

> > +       if (capsule->version != 0x00000001)

> > +               return EFI_INVALID_PARAMETER;

> > +

> > +       /* Drivers */

> > +       /* TODO: support loading drivers */

> > +

> > +       handles = NULL;

> > +       ret = EFI_CALL(efi_locate_handle_buffer(

> > +                       BY_PROTOCOL,

> > +                       &efi_guid_firmware_management_protocol,

> > +                       NULL, &no_handles, (efi_handle_t **)&handles));

> > +       if (ret != EFI_SUCCESS)

> > +               return EFI_UNSUPPORTED;

> > +

> > +       /* Payload */

> > +       for (item = capsule->embedded_driver_count;

> > +            item < capsule->embedded_driver_count

> > +                   + capsule->payload_item_count; item++) {

> > +               /* sanity check */

> > +               if ((capsule->item_offset_list[item] + sizeof(*image)

> > +                                >= capsule_size)) {

> > +                       printf("EFI: A capsule has not enough size of

> > data\n");

> > +                       ret = EFI_INVALID_PARAMETER;

> > +                       goto out;

> > +               }

> > +

> > +               image = (void *)capsule + capsule->item_offset_list[item];

> > +

> > +               if (image->version != 0x00000001 &&

> > +                   image->version != 0x00000002 &&

> > +                   image->version != 0x00000003) {

> > +                       ret = EFI_INVALID_PARAMETER;

> > +                       goto out;

> > +               }

> > +

> > +               /* find a device for update firmware */

> > +               /* TODO: should we pass index as well, or nothing but

> > type? */

> > +               fmp = efi_fmp_find(&image->update_image_type_id,

> > +                                  image->version == 0x1 ? 0 :

> > +                                       image->update_hardware_instance,

> > +                                  handles, no_handles);

> > +               if (!fmp) {

> > +                       printf("EFI Capsule: driver not found for firmware

> > type: %pUl, hardware instance: %lld\n",

> > +                              &image->update_image_type_id,

> > +                              image->version == 0x1 ? 0 :

> > +                                       image->update_hardware_instance);

> > +                       ret = EFI_UNSUPPORTED;

> > +                       goto out;

> > +               }

> > +

> > +               /* do it */

> > +               image_binary = (void *)image + sizeof(*image);

> >

> 

> Sorry for not having spotted this earlier. But since we are supporting the

> first three versions of the  struct

> efi_firmware_management_capsule_image_header, the above logic needs to be

> changed to accomodate versions 1 and 2. So in case of a version 1

> structure, the fields update_hardware_instance and image_capsule_support

> need to be discarded. Similarly for a version 2 structure, the

> image_capsule_support field needs to be discarded while calculating the

> image_binary pointer.


Thank you for catching this issue, you're right.

Given, however, that the firmware update is quite firmware-specific
and that we don't have any "backward compatibility" requirement,
it would be better to simply drop the support for previous versions
and to support the current/latest one, in this case, 3.

We have a similar issue on 'struct efi_firmware_image_descriptor'.
I hope that imposing a version check/restriction won't bring any
inconvenience to anybody.

-Takahiro Akashi

> -sughosh
Sughosh Ganu Nov. 24, 2020, 7:37 a.m. | #3
Takahiro,

On Tue, 24 Nov 2020 at 11:21, AKASHI Takahiro <takahiro.akashi@linaro.org>
wrote:

> Sughosh,

>

> On Sat, Nov 21, 2020 at 11:32:43PM +0530, Sughosh Ganu wrote:

> > hi Takahiro,

> >

> > On Tue, 17 Nov 2020 at 05:58, AKASHI Takahiro <

> takahiro.akashi@linaro.org>

> > wrote:

> >

> > > A capsule tagged with the guid,

> EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID,

> > > is handled as a firmware update object.

> > > What efi_update_capsule() basically does is to load any firmware

> management

> > > protocol (or fmp) drivers contained in a capsule, find out an

> appropriate

> > > fmp driver and then invoke its set_image() interface against each

> binary

> > > in a capsule.

> > > In this commit, however, loading drivers is not supported.

> > >

> > > The result of applying a capsule is set to be stored in "CapsuleXXXX"

> > > variable, but its implementation is deferred to a fmp driver.

> > >

> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

> > > ---

> > >  include/efi_api.h            | 129 +++++++++++++++++++

> > >  include/efi_loader.h         |   2 +

> > >  lib/efi_loader/Kconfig       |   8 ++

> > >  lib/efi_loader/efi_capsule.c | 238 ++++++++++++++++++++++++++++++++++-

> > >  lib/efi_loader/efi_setup.c   |   4 +

> > >  5 files changed, 380 insertions(+), 1 deletion(-)

> > >

> > > diff --git a/include/efi_api.h b/include/efi_api.h

> > > index 7a2a087c60ed..966bc6e590bf 100644

> > > --- a/include/efi_api.h

> > > +++ b/include/efi_api.h

> > > @@ -217,6 +217,9 @@ enum efi_reset_type {

> > >  #define CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE    0x00020000

> > >  #define CAPSULE_FLAGS_INITIATE_RESET           0x00040000

> > >

> > > +#define CAPSULE_SUPPORT_AUTHENTICATION         0x0000000000000001

> > > +#define CAPSULE_SUPPORT_DEPENDENCY             0x0000000000000002

> > > +

> > >  #define EFI_CAPSULE_REPORT_GUID \

> > >         EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \

> > >                  0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)

> > > @@ -225,6 +228,10 @@ enum efi_reset_type {

> > >         EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \

> > >                  0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)

> > >

> > > +#define EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID \

> > > +       EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \

> > > +                0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)

> > > +

> > >  struct efi_capsule_header {

> > >         efi_guid_t capsule_guid;

> > >         u32 header_size;

> > > @@ -253,6 +260,33 @@ struct efi_memory_range_capsule {

> > >         struct efi_memory_range memory_ranges[];

> > >  } __packed;

> > >

> > > +struct efi_firmware_management_capsule_header {

> > > +       u32 version;

> > > +       u16 embedded_driver_count;

> > > +       u16 payload_item_count;

> > > +       u64 item_offset_list[];

> > > +} __packed;

> > > +

> > > +struct efi_firmware_management_capsule_image_header {

> > > +       u32 version;

> > > +       efi_guid_t update_image_type_id;

> > > +       u8 update_image_index;

> > > +       u8 reserved[3];

> > > +       u32 update_image_size;

> > > +       u32 update_vendor_code_size;

> > > +       u64 update_hardware_instance;

> > > +       u64 image_capsule_support;

> > > +} __packed;

> > > +

> > > +struct efi_capsule_result_variable_fmp {

> > > +       u16 version;

> > > +       u8 payload_index;

> > > +       u8 update_image_index;

> > > +       efi_guid_t update_image_type_id;

> > > +       // u16 capsule_file_name[];

> > > +       // u16 capsule_target[];

> > > +} __packed;

> > > +

> > >

> >

> > <snip>

> >

> >

> > > +/**

> > > + * efi_capsule_update_firmware - update firmware from capsule

> > > + * @capsule_data:      Capsule

> > > + *

> > > + * Update firmware, using a capsule, @capsule_data. Loading any FMP

> > > + * drivers embedded in a capsule is not supported.

> > > + *

> > > + * Return:             status code

> > > + */

> > > +static efi_status_t efi_capsule_update_firmware(

> > > +               struct efi_capsule_header *capsule_data)

> > > +{

> > > +       struct efi_firmware_management_capsule_header *capsule;

> > > +       struct efi_firmware_management_capsule_image_header *image;

> > > +       size_t capsule_size;

> > > +       void *image_binary, *vendor_code;

> > > +       efi_handle_t *handles;

> > > +       efi_uintn_t no_handles;

> > > +       int item;

> > > +       struct efi_firmware_management_protocol *fmp;

> > > +       u16 *abort_reason;

> > > +       efi_status_t ret = EFI_SUCCESS;

> > > +

> > > +       /* sanity check */

> > > +       if (capsule_data->header_size < sizeof(*capsule) ||

> > > +           capsule_data->header_size >=

> capsule_data->capsule_image_size)

> > > +               return EFI_INVALID_PARAMETER;

> > > +

> > > +       capsule = (void *)capsule_data + capsule_data->header_size;

> > > +       capsule_size = capsule_data->capsule_image_size

> > > +                       - capsule_data->header_size;

> > > +

> > > +       if (capsule->version != 0x00000001)

> > > +               return EFI_INVALID_PARAMETER;

> > > +

> > > +       /* Drivers */

> > > +       /* TODO: support loading drivers */

> > > +

> > > +       handles = NULL;

> > > +       ret = EFI_CALL(efi_locate_handle_buffer(

> > > +                       BY_PROTOCOL,

> > > +                       &efi_guid_firmware_management_protocol,

> > > +                       NULL, &no_handles, (efi_handle_t **)&handles));

> > > +       if (ret != EFI_SUCCESS)

> > > +               return EFI_UNSUPPORTED;

> > > +

> > > +       /* Payload */

> > > +       for (item = capsule->embedded_driver_count;

> > > +            item < capsule->embedded_driver_count

> > > +                   + capsule->payload_item_count; item++) {

> > > +               /* sanity check */

> > > +               if ((capsule->item_offset_list[item] + sizeof(*image)

> > > +                                >= capsule_size)) {

> > > +                       printf("EFI: A capsule has not enough size of

> > > data\n");

> > > +                       ret = EFI_INVALID_PARAMETER;

> > > +                       goto out;

> > > +               }

> > > +

> > > +               image = (void *)capsule +

> capsule->item_offset_list[item];

> > > +

> > > +               if (image->version != 0x00000001 &&

> > > +                   image->version != 0x00000002 &&

> > > +                   image->version != 0x00000003) {

> > > +                       ret = EFI_INVALID_PARAMETER;

> > > +                       goto out;

> > > +               }

> > > +

> > > +               /* find a device for update firmware */

> > > +               /* TODO: should we pass index as well, or nothing but

> > > type? */

> > > +               fmp = efi_fmp_find(&image->update_image_type_id,

> > > +                                  image->version == 0x1 ? 0 :

> > > +

>  image->update_hardware_instance,

> > > +                                  handles, no_handles);

> > > +               if (!fmp) {

> > > +                       printf("EFI Capsule: driver not found for

> firmware

> > > type: %pUl, hardware instance: %lld\n",

> > > +                              &image->update_image_type_id,

> > > +                              image->version == 0x1 ? 0 :

> > > +

>  image->update_hardware_instance);

> > > +                       ret = EFI_UNSUPPORTED;

> > > +                       goto out;

> > > +               }

> > > +

> > > +               /* do it */

> > > +               image_binary = (void *)image + sizeof(*image);

> > >

> >

> > Sorry for not having spotted this earlier. But since we are supporting

> the

> > first three versions of the  struct

> > efi_firmware_management_capsule_image_header, the above logic needs to be

> > changed to accomodate versions 1 and 2. So in case of a version 1

> > structure, the fields update_hardware_instance and image_capsule_support

> > need to be discarded. Similarly for a version 2 structure, the

> > image_capsule_support field needs to be discarded while calculating the

> > image_binary pointer.

>

> Thank you for catching this issue, you're right.

>

> Given, however, that the firmware update is quite firmware-specific

> and that we don't have any "backward compatibility" requirement,

> it would be better to simply drop the support for previous versions

> and to support the current/latest one, in this case, 3.

>


The edk2 scripts which i use for generating the capsules are currently
using version 2. So if it's not too much of a hassle, i will prefer having
support for version 2 at least. However, if you don't want to respin the
series again, I can take care of this in my series for adding support for
capsule update on the qemu arm platform.

-sughosh


> We have a similar issue on 'struct efi_firmware_image_descriptor'.

> I hope that imposing a version check/restriction won't bring any

> inconvenience to anybody.

>

> -Takahiro Akashi

>

> > -sughosh

>
AKASHI Takahiro Nov. 24, 2020, 8:31 a.m. | #4
Sughosh,

On Tue, Nov 24, 2020 at 01:07:53PM +0530, Sughosh Ganu wrote:
> Takahiro,

> 

> On Tue, 24 Nov 2020 at 11:21, AKASHI Takahiro <takahiro.akashi@linaro.org>

> wrote:

> 

> > Sughosh,

> >

> > On Sat, Nov 21, 2020 at 11:32:43PM +0530, Sughosh Ganu wrote:

> > > hi Takahiro,

> > >

> > > On Tue, 17 Nov 2020 at 05:58, AKASHI Takahiro <

> > takahiro.akashi@linaro.org>

> > > wrote:

> > >

> > > > A capsule tagged with the guid,

> > EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID,

> > > > is handled as a firmware update object.

> > > > What efi_update_capsule() basically does is to load any firmware

> > management

> > > > protocol (or fmp) drivers contained in a capsule, find out an

> > appropriate

> > > > fmp driver and then invoke its set_image() interface against each

> > binary

> > > > in a capsule.

> > > > In this commit, however, loading drivers is not supported.

> > > >

> > > > The result of applying a capsule is set to be stored in "CapsuleXXXX"

> > > > variable, but its implementation is deferred to a fmp driver.

> > > >

> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

> > > > ---

> > > >  include/efi_api.h            | 129 +++++++++++++++++++

> > > >  include/efi_loader.h         |   2 +

> > > >  lib/efi_loader/Kconfig       |   8 ++

> > > >  lib/efi_loader/efi_capsule.c | 238 ++++++++++++++++++++++++++++++++++-

> > > >  lib/efi_loader/efi_setup.c   |   4 +

> > > >  5 files changed, 380 insertions(+), 1 deletion(-)

> > > >

> > > > diff --git a/include/efi_api.h b/include/efi_api.h

> > > > index 7a2a087c60ed..966bc6e590bf 100644

> > > > --- a/include/efi_api.h

> > > > +++ b/include/efi_api.h

> > > > @@ -217,6 +217,9 @@ enum efi_reset_type {

> > > >  #define CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE    0x00020000

> > > >  #define CAPSULE_FLAGS_INITIATE_RESET           0x00040000

> > > >

> > > > +#define CAPSULE_SUPPORT_AUTHENTICATION         0x0000000000000001

> > > > +#define CAPSULE_SUPPORT_DEPENDENCY             0x0000000000000002

> > > > +

> > > >  #define EFI_CAPSULE_REPORT_GUID \

> > > >         EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \

> > > >                  0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)

> > > > @@ -225,6 +228,10 @@ enum efi_reset_type {

> > > >         EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \

> > > >                  0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)

> > > >

> > > > +#define EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID \

> > > > +       EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \

> > > > +                0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)

> > > > +

> > > >  struct efi_capsule_header {

> > > >         efi_guid_t capsule_guid;

> > > >         u32 header_size;

> > > > @@ -253,6 +260,33 @@ struct efi_memory_range_capsule {

> > > >         struct efi_memory_range memory_ranges[];

> > > >  } __packed;

> > > >

> > > > +struct efi_firmware_management_capsule_header {

> > > > +       u32 version;

> > > > +       u16 embedded_driver_count;

> > > > +       u16 payload_item_count;

> > > > +       u64 item_offset_list[];

> > > > +} __packed;

> > > > +

> > > > +struct efi_firmware_management_capsule_image_header {

> > > > +       u32 version;

> > > > +       efi_guid_t update_image_type_id;

> > > > +       u8 update_image_index;

> > > > +       u8 reserved[3];

> > > > +       u32 update_image_size;

> > > > +       u32 update_vendor_code_size;

> > > > +       u64 update_hardware_instance;

> > > > +       u64 image_capsule_support;

> > > > +} __packed;

> > > > +

> > > > +struct efi_capsule_result_variable_fmp {

> > > > +       u16 version;

> > > > +       u8 payload_index;

> > > > +       u8 update_image_index;

> > > > +       efi_guid_t update_image_type_id;

> > > > +       // u16 capsule_file_name[];

> > > > +       // u16 capsule_target[];

> > > > +} __packed;

> > > > +

> > > >

> > >

> > > <snip>

> > >

> > >

> > > > +/**

> > > > + * efi_capsule_update_firmware - update firmware from capsule

> > > > + * @capsule_data:      Capsule

> > > > + *

> > > > + * Update firmware, using a capsule, @capsule_data. Loading any FMP

> > > > + * drivers embedded in a capsule is not supported.

> > > > + *

> > > > + * Return:             status code

> > > > + */

> > > > +static efi_status_t efi_capsule_update_firmware(

> > > > +               struct efi_capsule_header *capsule_data)

> > > > +{

> > > > +       struct efi_firmware_management_capsule_header *capsule;

> > > > +       struct efi_firmware_management_capsule_image_header *image;

> > > > +       size_t capsule_size;

> > > > +       void *image_binary, *vendor_code;

> > > > +       efi_handle_t *handles;

> > > > +       efi_uintn_t no_handles;

> > > > +       int item;

> > > > +       struct efi_firmware_management_protocol *fmp;

> > > > +       u16 *abort_reason;

> > > > +       efi_status_t ret = EFI_SUCCESS;

> > > > +

> > > > +       /* sanity check */

> > > > +       if (capsule_data->header_size < sizeof(*capsule) ||

> > > > +           capsule_data->header_size >=

> > capsule_data->capsule_image_size)

> > > > +               return EFI_INVALID_PARAMETER;

> > > > +

> > > > +       capsule = (void *)capsule_data + capsule_data->header_size;

> > > > +       capsule_size = capsule_data->capsule_image_size

> > > > +                       - capsule_data->header_size;

> > > > +

> > > > +       if (capsule->version != 0x00000001)

> > > > +               return EFI_INVALID_PARAMETER;

> > > > +

> > > > +       /* Drivers */

> > > > +       /* TODO: support loading drivers */

> > > > +

> > > > +       handles = NULL;

> > > > +       ret = EFI_CALL(efi_locate_handle_buffer(

> > > > +                       BY_PROTOCOL,

> > > > +                       &efi_guid_firmware_management_protocol,

> > > > +                       NULL, &no_handles, (efi_handle_t **)&handles));

> > > > +       if (ret != EFI_SUCCESS)

> > > > +               return EFI_UNSUPPORTED;

> > > > +

> > > > +       /* Payload */

> > > > +       for (item = capsule->embedded_driver_count;

> > > > +            item < capsule->embedded_driver_count

> > > > +                   + capsule->payload_item_count; item++) {

> > > > +               /* sanity check */

> > > > +               if ((capsule->item_offset_list[item] + sizeof(*image)

> > > > +                                >= capsule_size)) {

> > > > +                       printf("EFI: A capsule has not enough size of

> > > > data\n");

> > > > +                       ret = EFI_INVALID_PARAMETER;

> > > > +                       goto out;

> > > > +               }

> > > > +

> > > > +               image = (void *)capsule +

> > capsule->item_offset_list[item];

> > > > +

> > > > +               if (image->version != 0x00000001 &&

> > > > +                   image->version != 0x00000002 &&

> > > > +                   image->version != 0x00000003) {

> > > > +                       ret = EFI_INVALID_PARAMETER;

> > > > +                       goto out;

> > > > +               }

> > > > +

> > > > +               /* find a device for update firmware */

> > > > +               /* TODO: should we pass index as well, or nothing but

> > > > type? */

> > > > +               fmp = efi_fmp_find(&image->update_image_type_id,

> > > > +                                  image->version == 0x1 ? 0 :

> > > > +

> >  image->update_hardware_instance,

> > > > +                                  handles, no_handles);

> > > > +               if (!fmp) {

> > > > +                       printf("EFI Capsule: driver not found for

> > firmware

> > > > type: %pUl, hardware instance: %lld\n",

> > > > +                              &image->update_image_type_id,

> > > > +                              image->version == 0x1 ? 0 :

> > > > +

> >  image->update_hardware_instance);

> > > > +                       ret = EFI_UNSUPPORTED;

> > > > +                       goto out;

> > > > +               }

> > > > +

> > > > +               /* do it */

> > > > +               image_binary = (void *)image + sizeof(*image);

> > > >

> > >

> > > Sorry for not having spotted this earlier. But since we are supporting

> > the

> > > first three versions of the  struct

> > > efi_firmware_management_capsule_image_header, the above logic needs to be

> > > changed to accomodate versions 1 and 2. So in case of a version 1

> > > structure, the fields update_hardware_instance and image_capsule_support

> > > need to be discarded. Similarly for a version 2 structure, the

> > > image_capsule_support field needs to be discarded while calculating the

> > > image_binary pointer.

> >

> > Thank you for catching this issue, you're right.

> >

> > Given, however, that the firmware update is quite firmware-specific

> > and that we don't have any "backward compatibility" requirement,

> > it would be better to simply drop the support for previous versions

> > and to support the current/latest one, in this case, 3.

> >

> 

> The edk2 scripts which i use for generating the capsules are currently

> using version 2.


Personally, I think edk2's script should be updated to catch up with
the latest specification.

> So if it's not too much of a hassle, i will prefer having

> support for version 2 at least. However, if you don't want to respin the

> series again, I can take care of this in my series for adding support for

> capsule update on the qemu arm platform.


I will have to respin the code to drop the current "half-baked"
support for previous versions if Heinrich anticipates it.

After that, it's up to you to implement additional support
as a follow-up patch.

-Takahiro Akashi


> -sughosh

> 

> 

> > We have a similar issue on 'struct efi_firmware_image_descriptor'.

> > I hope that imposing a version check/restriction won't bring any

> > inconvenience to anybody.

> >

> > -Takahiro Akashi

> >

> > > -sughosh

> >
Heinrich Schuchardt Nov. 25, 2020, 1 a.m. | #5
On 11/17/20 1:27 AM, AKASHI Takahiro wrote:
> A capsule tagged with the guid, EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID,

> is handled as a firmware update object.

> What efi_update_capsule() basically does is to load any firmware management

> protocol (or fmp) drivers contained in a capsule, find out an appropriate

> fmp driver and then invoke its set_image() interface against each binary

> in a capsule.

> In this commit, however, loading drivers is not supported.

>

> The result of applying a capsule is set to be stored in "CapsuleXXXX"

> variable, but its implementation is deferred to a fmp driver.

>

> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

> ---

>   include/efi_api.h            | 129 +++++++++++++++++++

>   include/efi_loader.h         |   2 +

>   lib/efi_loader/Kconfig       |   8 ++

>   lib/efi_loader/efi_capsule.c | 238 ++++++++++++++++++++++++++++++++++-

>   lib/efi_loader/efi_setup.c   |   4 +

>   5 files changed, 380 insertions(+), 1 deletion(-)

>

> diff --git a/include/efi_api.h b/include/efi_api.h

> index 7a2a087c60ed..966bc6e590bf 100644

> --- a/include/efi_api.h

> +++ b/include/efi_api.h

> @@ -217,6 +217,9 @@ enum efi_reset_type {

>   #define CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE	0x00020000

>   #define CAPSULE_FLAGS_INITIATE_RESET		0x00040000

>

> +#define CAPSULE_SUPPORT_AUTHENTICATION		0x0000000000000001

> +#define CAPSULE_SUPPORT_DEPENDENCY		0x0000000000000002

> +

>   #define EFI_CAPSULE_REPORT_GUID \

>   	EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \

>   		 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)

> @@ -225,6 +228,10 @@ enum efi_reset_type {

>   	EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \

>   		 0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)

>

> +#define EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID \

> +	EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \

> +		 0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)

> +

>   struct efi_capsule_header {

>   	efi_guid_t capsule_guid;

>   	u32 header_size;

> @@ -253,6 +260,33 @@ struct efi_memory_range_capsule {

>   	struct efi_memory_range memory_ranges[];

>   } __packed;

>

> +struct efi_firmware_management_capsule_header {

> +	u32 version;

> +	u16 embedded_driver_count;

> +	u16 payload_item_count;

> +	u64 item_offset_list[];

> +} __packed;

> +

> +struct efi_firmware_management_capsule_image_header {

> +	u32 version;

> +	efi_guid_t update_image_type_id;

> +	u8 update_image_index;

> +	u8 reserved[3];

> +	u32 update_image_size;

> +	u32 update_vendor_code_size;

> +	u64 update_hardware_instance;

> +	u64 image_capsule_support;

> +} __packed;

> +

> +struct efi_capsule_result_variable_fmp {

> +	u16 version;

> +	u8 payload_index;

> +	u8 update_image_index;

> +	efi_guid_t update_image_type_id;

> +	// u16 capsule_file_name[];

> +	// u16 capsule_target[];

> +} __packed;

> +

>   #define EFI_RT_SUPPORTED_GET_TIME			0x0001

>   #define EFI_RT_SUPPORTED_SET_TIME			0x0002

>   #define EFI_RT_SUPPORTED_GET_WAKEUP_TIME		0x0004

> @@ -1808,4 +1842,99 @@ struct efi_signature_list {

>   /*	struct efi_signature_data signatures[...][signature_size]; */

>   } __attribute__((__packed__));

>

> +/*

> + * Firmware management protocol

> + */

> +#define EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID \

> +	EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \

> +		 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7)

> +

> +#define IMAGE_ATTRIBUTE_IMAGE_UPDATABLE		0x0000000000000001

> +#define IMAGE_ATTRIBUTE_RESET_REQUIRED		0x0000000000000002

> +#define IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED	0x0000000000000004

> +#define IMAGE_ATTRIBUTE_IN_USE			0x0000000000000008

> +#define IMAGE_ATTRIBUTE_UEFI_IMAGE		0x0000000000000010

> +#define IMAGE_ATTRIBUTE_DEPENDENCY		0x0000000000000020

> +

> +#define IMAGE_COMPATIBILITY_CHECK_SUPPORTED	0x0000000000000001

> +

> +#define IMAGE_UPDATABLE_VALID			0x0000000000000001

> +#define IMAGE_UPDATABLE_INVALID			0x0000000000000002

> +#define IMAGE_UPDATABLE_INVALID_TYPE		0x0000000000000004

> +#define IMAGE_UPDATABLE_INVALID_OLLD		0x0000000000000008

> +#define IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE	0x0000000000000010

> +

> +#define PACKAGE_ATTRIBUTE_VERSION_UPDATABLE		0x0000000000000001

> +#define PACKAGE_ATTRIBUTE_RESET_REQUIRED		0x0000000000000002

> +#define PACKAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED	0x0000000000000004

> +

> +#define EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION	4

> +

> +typedef struct efi_firmware_image_dependencies {

> +	u8 dependencies[0];

> +} efi_firmware_image_dep_t;

> +

> +struct efi_firmware_image_descriptor {

> +	u8 image_index;

> +	efi_guid_t image_type_id;

> +	u64 image_id;

> +	u16 *image_id_name;

> +	u32 version;

> +	u16 *version_name;

> +	efi_uintn_t size;

> +	u64 attributes_supported;

> +	u64 attributes_setting;

> +	u64 compatibilities;

> +	u32 lowest_supported_image_version;

> +	u32 last_attempt_version;

> +	u32 last_attempt_status;

> +	u64 hardware_instance;

> +	efi_firmware_image_dep_t *dependencies;

> +};

> +

> +struct efi_firmware_management_protocol {

> +	efi_status_t (EFIAPI *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);

> +	efi_status_t (EFIAPI *get_image)(

> +			struct efi_firmware_management_protocol *this,

> +			u8 image_index,

> +			void *image,

> +			efi_uintn_t *image_size);

> +	efi_status_t (EFIAPI *set_image)(

> +			struct efi_firmware_management_protocol *this,

> +			u8 image_index,

> +			const void *image,

> +			efi_uintn_t image_size,

> +			const void *vendor_code,

> +			efi_status_t (*progress)(efi_uintn_t completion),

> +			u16 **abort_reason);

> +	efi_status_t (EFIAPI *check_image)(

> +			struct efi_firmware_management_protocol *this,

> +			u8 image_index,

> +			const void *image,

> +			efi_uintn_t *image_size,

> +			u32 *image_updatable);

> +	efi_status_t (EFIAPI *get_package_info)(

> +			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_status_t (EFIAPI *set_package_info)(

> +			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);

> +};

> +

>   #endif

> diff --git a/include/efi_loader.h b/include/efi_loader.h

> index eb57e7455eb1..1a728bf9702d 100644

> --- a/include/efi_loader.h

> +++ b/include/efi_loader.h

> @@ -209,6 +209,8 @@ extern const efi_guid_t efi_guid_cert_type_pkcs7;

>   extern const efi_guid_t efi_guid_rng_protocol;

>   /* GUID of capsule update result */

>   extern const efi_guid_t efi_guid_capsule_report;

> +/* GUID of firmware management protocol */

> +extern const efi_guid_t efi_guid_firmware_management_protocol;

>

>   extern unsigned int __efi_runtime_start, __efi_runtime_stop;

>   extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;

> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> index e1ac5ac055de..5cb34687c26a 100644

> --- a/lib/efi_loader/Kconfig

> +++ b/lib/efi_loader/Kconfig

> @@ -126,6 +126,14 @@ 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_MANAGEMENT

> +	bool "Capsule: Firmware Management Protocol"

> +	depends on EFI_HAVE_CAPSULE_SUPPORT

> +	default y

> +	help

> +	  Select this option if you want to enable capsule-based

> +	  firmware update using Firmware Management Protocol.

> +

>   config EFI_DEVICE_PATH_TO_TEXT

>   	bool "Device path to text protocol"

>   	default y

> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c

> index b3c7d1b735b6..2f41c17f5057 100644

> --- a/lib/efi_loader/efi_capsule.c

> +++ b/lib/efi_loader/efi_capsule.c

> @@ -15,6 +15,10 @@

>   #include <sort.h>

>

>   const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID;

> +static const efi_guid_t efi_guid_firmware_management_capsule_id =

> +		EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;

> +const efi_guid_t efi_guid_firmware_management_protocol =

> +		EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;

>

>   #ifdef CONFIG_EFI_CAPSULE_ON_DISK

>   /* for file system access */

> @@ -88,6 +92,218 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,

>   		printf("EFI: creating %ls failed\n", variable_name16);

>   }

>

> +#ifdef CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT

> +/**

> + * efi_fmp_find - search for Firmware Management Protocol drivers

> + * @image_type:		Image type guid

> + * @instance:		Instance number

> + * @handles:		Handles of FMP drivers

> + * @no_handles:		Number of handles

> + *

> + * Search for Firmware Management Protocol drivers, matching the image

> + * type, @image_type and the machine instance, @instance, from the list,

> + * @handles.

> + *

> + * Return:

> + * * Protocol instance	- on success

> + * * NULL		- on failure

> + */

> +static struct efi_firmware_management_protocol *

> +efi_fmp_find(efi_guid_t *image_type, u64 instance, efi_handle_t *handles,

> +	     efi_uintn_t no_handles)

> +{

> +	efi_handle_t *handle;

> +	struct efi_firmware_management_protocol *fmp;

> +	struct efi_firmware_image_descriptor *image_info, *desc;

> +	efi_uintn_t info_size, descriptor_size;

> +	u32 descriptor_version;

> +	u8 descriptor_count;

> +	u32 package_version;

> +	u16 *package_version_name;

> +	bool found = false;

> +	int i, j;

> +	efi_status_t ret;

> +

> +	for (i = 0, handle = handles; i < no_handles; i++, handle++) {

> +		ret = EFI_CALL(efi_handle_protocol(

> +				*handle,

> +				&efi_guid_firmware_management_protocol,

> +				(void **)&fmp));

> +		if (ret != EFI_SUCCESS)

> +			continue;

> +

> +		/* get device's image info */

> +		info_size = 0;

> +		image_info = NULL;

> +		descriptor_version = 0;

> +		descriptor_count = 0;

> +		descriptor_size = 0;

> +		package_version = 0;

> +		package_version_name = NULL;

> +		ret = EFI_CALL(fmp->get_image_info(fmp, &info_size,

> +						   image_info,

> +						   &descriptor_version,

> +						   &descriptor_count,

> +						   &descriptor_size,

> +						   &package_version,

> +						   &package_version_name));

> +		if (ret != EFI_BUFFER_TOO_SMALL)

> +			goto skip;

> +

> +		image_info = malloc(info_size);

> +		if (!image_info)

> +			goto skip;

> +

> +		ret = EFI_CALL(fmp->get_image_info(fmp, &info_size,

> +						   image_info,

> +						   &descriptor_version,

> +						   &descriptor_count,

> +						   &descriptor_size,

> +						   &package_version,

> +						   &package_version_name));

> +		if (ret != EFI_SUCCESS)

> +			goto skip;

> +

> +		/* matching */

> +		for (j = 0, desc = image_info; j < descriptor_count;

> +		     j++, desc = (void *)desc + descriptor_size) {

> +			EFI_PRINT("+++ desc[%d] index: %d, name: %ls\n",

> +				  j, desc->image_index, desc->image_id_name);

> +			if (!guidcmp(&desc->image_type_id, image_type) &&

> +			    (!instance ||

> +			     !desc->hardware_instance ||

> +			     (descriptor_version >= 3 &&

> +			      desc->hardware_instance == instance)))

> +				found = true;

> +		}

> +

> +skip:

> +		efi_free_pool(package_version_name);

> +		free(image_info);

> +		EFI_CALL(efi_close_protocol(

> +				(efi_handle_t)fmp,

> +				&efi_guid_firmware_management_protocol,

> +				NULL, NULL));

> +		if (found)

> +			return fmp;

> +	}

> +

> +	return NULL;

> +}

> +

> +/**

> + * efi_capsule_update_firmware - update firmware from capsule

> + * @capsule_data:	Capsule

> + *

> + * Update firmware, using a capsule, @capsule_data. Loading any FMP

> + * drivers embedded in a capsule is not supported.

> + *

> + * Return:		status code

> + */

> +static efi_status_t efi_capsule_update_firmware(

> +		struct efi_capsule_header *capsule_data)

> +{

> +	struct efi_firmware_management_capsule_header *capsule;

> +	struct efi_firmware_management_capsule_image_header *image;

> +	size_t capsule_size;

> +	void *image_binary, *vendor_code;

> +	efi_handle_t *handles;

> +	efi_uintn_t no_handles;

> +	int item;

> +	struct efi_firmware_management_protocol *fmp;

> +	u16 *abort_reason;

> +	efi_status_t ret = EFI_SUCCESS;

> +

> +	/* sanity check */

> +	if (capsule_data->header_size < sizeof(*capsule) ||

> +	    capsule_data->header_size >= capsule_data->capsule_image_size)

> +		return EFI_INVALID_PARAMETER;

> +

> +	capsule = (void *)capsule_data + capsule_data->header_size;

> +	capsule_size = capsule_data->capsule_image_size

> +			- capsule_data->header_size;

> +

> +	if (capsule->version != 0x00000001)

> +		return EFI_INVALID_PARAMETER;


For an unsupported capsule format we should return EFI_UNSUPPORTED
according to the spec.

> +

> +	/* Drivers */

> +	/* TODO: support loading drivers */


Please, describe the requirement in the spec that you do not yet fulfill
yet more clearly.

> +

> +	handles = NULL;

> +	ret = EFI_CALL(efi_locate_handle_buffer(

> +			BY_PROTOCOL,

> +			&efi_guid_firmware_management_protocol,

> +			NULL, &no_handles, (efi_handle_t **)&handles));

> +	if (ret != EFI_SUCCESS)

> +		return EFI_UNSUPPORTED;

> +

> +	/* Payload */

> +	for (item = capsule->embedded_driver_count;

> +	     item < capsule->embedded_driver_count

> +		    + capsule->payload_item_count; item++) {

> +		/* sanity check */

> +		if ((capsule->item_offset_list[item] + sizeof(*image)

> +				 >= capsule_size)) {

> +			printf("EFI: A capsule has not enough size of data\n");


%s/enough size of data/enough data/

> +			ret = EFI_INVALID_PARAMETER;

> +			goto out;

> +		}

> +

> +		image = (void *)capsule + capsule->item_offset_list[item];

> +

> +		if (image->version != 0x00000001 &&

> +		    image->version != 0x00000002 &&

> +		    image->version != 0x00000003) {

> +			ret = EFI_INVALID_PARAMETER;

> +			goto out;

> +		}


UEFI 2.8 has EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.Version = 3.
Why should we support outdated versions?

Do you really support all three versions? Are they all tested? Sughosh's
mail sounded like 'no'.

For unsupported capsule format the return value should be EFI_UNSUPPORTED.

> +

> +		/* find a device for update firmware */

> +		/* TODO: should we pass index as well, or nothing but type? */


Does this mean that the patch is not yet ready for merging?

> +		fmp = efi_fmp_find(&image->update_image_type_id,

> +				   image->version == 0x1 ? 0 :

> +					image->update_hardware_instance,

> +				   handles, no_handles);

> +		if (!fmp) {

> +			printf("EFI Capsule: driver not found for firmware type: %pUl, hardware instance: %lld\n",

> +			       &image->update_image_type_id,

> +			       image->version == 0x1 ? 0 :

> +					image->update_hardware_instance);

> +			ret = EFI_UNSUPPORTED;

> +			goto out;

> +		}

> +

> +		/* do it */


Do what?

> +		image_binary = (void *)image + sizeof(*image);

> +		vendor_code = image_binary + image->update_image_size;

> +

> +		abort_reason = NULL;

> +		ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,

> +					      image_binary,

> +					      image->update_image_size,

> +					      vendor_code, NULL,

> +					      &abort_reason));

> +		if (ret != EFI_SUCCESS) {

> +			printf("EFI Capsule: firmware update failed: %ls\n",

> +			       abort_reason);


Why don't you use log_err() or EFI_PRINT()?

> +			efi_free_pool(abort_reason);

> +			goto out;

> +		}

> +	}

> +

> +out:

> +	efi_free_pool(handles);

> +

> +	return ret;

> +}

> +#else

> +static efi_status_t efi_capsule_update_firmware(

> +		struct efi_capsule_header *capsule_data)

> +{

> +	return EFI_UNSUPPORTED;

> +}

> +#endif /* CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT */

> +

>   /**

>    * efi_update_capsule() - process information from operating system

>    * @capsule_header_array:	Array of virtual address pointers

> @@ -118,9 +334,29 @@ efi_status_t EFIAPI efi_update_capsule(

>   		goto out;

>   	}

>

> -	ret = EFI_UNSUPPORTED;

> +	ret = EFI_SUCCESS;

>   	for (i = 0, capsule = *capsule_header_array; i < capsule_count;

>   	     i++, capsule = *(++capsule_header_array)) {

> +		/* sanity check */

> +		if (capsule->header_size < sizeof(*capsule) ||

> +		    capsule->capsule_image_size < sizeof(*capsule)) {

> +			printf("EFI: A capsule has not enough size of data\n");


%s/enough size of data/enough data/

> +			continue;


You print an error message above. Why don't you exit with an error status?

Best regards

Heinrich

> +		}

> +

> +		EFI_PRINT("Capsule[%d] (guid:%pUl)\n",

> +			  i, &capsule->capsule_guid);

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

> +			     &efi_guid_firmware_management_capsule_id)) {

> +			ret  = efi_capsule_update_firmware(capsule);

> +		} else {

> +			printf("EFI: not support capsule type: %pUl\n",

> +			       &capsule->capsule_guid);

> +			ret = EFI_UNSUPPORTED;

> +		}

> +

> +		if (ret != EFI_SUCCESS)

> +			goto out;

>   	}

>   out:

>   	return EFI_EXIT(ret);

> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c

> index 0735e4755b60..feec9b53c8ed 100644

> --- a/lib/efi_loader/efi_setup.c

> +++ b/lib/efi_loader/efi_setup.c

> @@ -159,6 +159,10 @@ static efi_status_t efi_init_os_indications(void)

>   		os_indications_supported |=

>   			EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED;

>

> +	if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT))

> +		os_indications_supported |=

> +			EFI_OS_INDICATIONS_FMP_CAPSULE_SUPPORTED;

> +

>   	return efi_set_variable_int(L"OsIndicationsSupported",

>   				    &efi_global_variable_guid,

>   				    EFI_VARIABLE_BOOTSERVICE_ACCESS |

>
AKASHI Takahiro Nov. 25, 2020, 2:12 a.m. | #6
Heinrich,

On Wed, Nov 25, 2020 at 02:00:22AM +0100, Heinrich Schuchardt wrote:
> On 11/17/20 1:27 AM, AKASHI Takahiro wrote:

> > A capsule tagged with the guid, EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID,

> > is handled as a firmware update object.

> > What efi_update_capsule() basically does is to load any firmware management

> > protocol (or fmp) drivers contained in a capsule, find out an appropriate

> > fmp driver and then invoke its set_image() interface against each binary

> > in a capsule.

> > In this commit, however, loading drivers is not supported.

> > 

> > The result of applying a capsule is set to be stored in "CapsuleXXXX"

> > variable, but its implementation is deferred to a fmp driver.

> > 

> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

> > ---

> >   include/efi_api.h            | 129 +++++++++++++++++++

> >   include/efi_loader.h         |   2 +

> >   lib/efi_loader/Kconfig       |   8 ++

> >   lib/efi_loader/efi_capsule.c | 238 ++++++++++++++++++++++++++++++++++-

> >   lib/efi_loader/efi_setup.c   |   4 +

> >   5 files changed, 380 insertions(+), 1 deletion(-)

> > 

> > diff --git a/include/efi_api.h b/include/efi_api.h

> > index 7a2a087c60ed..966bc6e590bf 100644

> > --- a/include/efi_api.h

> > +++ b/include/efi_api.h

> > @@ -217,6 +217,9 @@ enum efi_reset_type {

> >   #define CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE	0x00020000

> >   #define CAPSULE_FLAGS_INITIATE_RESET		0x00040000

> > 

> > +#define CAPSULE_SUPPORT_AUTHENTICATION		0x0000000000000001

> > +#define CAPSULE_SUPPORT_DEPENDENCY		0x0000000000000002

> > +

> >   #define EFI_CAPSULE_REPORT_GUID \

> >   	EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \

> >   		 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)

> > @@ -225,6 +228,10 @@ enum efi_reset_type {

> >   	EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \

> >   		 0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)

> > 

> > +#define EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID \

> > +	EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \

> > +		 0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)

> > +

> >   struct efi_capsule_header {

> >   	efi_guid_t capsule_guid;

> >   	u32 header_size;

> > @@ -253,6 +260,33 @@ struct efi_memory_range_capsule {

> >   	struct efi_memory_range memory_ranges[];

> >   } __packed;

> > 

> > +struct efi_firmware_management_capsule_header {

> > +	u32 version;

> > +	u16 embedded_driver_count;

> > +	u16 payload_item_count;

> > +	u64 item_offset_list[];

> > +} __packed;

> > +

> > +struct efi_firmware_management_capsule_image_header {

> > +	u32 version;

> > +	efi_guid_t update_image_type_id;

> > +	u8 update_image_index;

> > +	u8 reserved[3];

> > +	u32 update_image_size;

> > +	u32 update_vendor_code_size;

> > +	u64 update_hardware_instance;

> > +	u64 image_capsule_support;

> > +} __packed;

> > +

> > +struct efi_capsule_result_variable_fmp {

> > +	u16 version;

> > +	u8 payload_index;

> > +	u8 update_image_index;

> > +	efi_guid_t update_image_type_id;

> > +	// u16 capsule_file_name[];

> > +	// u16 capsule_target[];

> > +} __packed;

> > +

> >   #define EFI_RT_SUPPORTED_GET_TIME			0x0001

> >   #define EFI_RT_SUPPORTED_SET_TIME			0x0002

> >   #define EFI_RT_SUPPORTED_GET_WAKEUP_TIME		0x0004

> > @@ -1808,4 +1842,99 @@ struct efi_signature_list {

> >   /*	struct efi_signature_data signatures[...][signature_size]; */

> >   } __attribute__((__packed__));

> > 

> > +/*

> > + * Firmware management protocol

> > + */

> > +#define EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID \

> > +	EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \

> > +		 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7)

> > +

> > +#define IMAGE_ATTRIBUTE_IMAGE_UPDATABLE		0x0000000000000001

> > +#define IMAGE_ATTRIBUTE_RESET_REQUIRED		0x0000000000000002

> > +#define IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED	0x0000000000000004

> > +#define IMAGE_ATTRIBUTE_IN_USE			0x0000000000000008

> > +#define IMAGE_ATTRIBUTE_UEFI_IMAGE		0x0000000000000010

> > +#define IMAGE_ATTRIBUTE_DEPENDENCY		0x0000000000000020

> > +

> > +#define IMAGE_COMPATIBILITY_CHECK_SUPPORTED	0x0000000000000001

> > +

> > +#define IMAGE_UPDATABLE_VALID			0x0000000000000001

> > +#define IMAGE_UPDATABLE_INVALID			0x0000000000000002

> > +#define IMAGE_UPDATABLE_INVALID_TYPE		0x0000000000000004

> > +#define IMAGE_UPDATABLE_INVALID_OLLD		0x0000000000000008

> > +#define IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE	0x0000000000000010

> > +

> > +#define PACKAGE_ATTRIBUTE_VERSION_UPDATABLE		0x0000000000000001

> > +#define PACKAGE_ATTRIBUTE_RESET_REQUIRED		0x0000000000000002

> > +#define PACKAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED	0x0000000000000004

> > +

> > +#define EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION	4

> > +

> > +typedef struct efi_firmware_image_dependencies {

> > +	u8 dependencies[0];

> > +} efi_firmware_image_dep_t;

> > +

> > +struct efi_firmware_image_descriptor {

> > +	u8 image_index;

> > +	efi_guid_t image_type_id;

> > +	u64 image_id;

> > +	u16 *image_id_name;

> > +	u32 version;

> > +	u16 *version_name;

> > +	efi_uintn_t size;

> > +	u64 attributes_supported;

> > +	u64 attributes_setting;

> > +	u64 compatibilities;

> > +	u32 lowest_supported_image_version;

> > +	u32 last_attempt_version;

> > +	u32 last_attempt_status;

> > +	u64 hardware_instance;

> > +	efi_firmware_image_dep_t *dependencies;

> > +};

> > +

> > +struct efi_firmware_management_protocol {

> > +	efi_status_t (EFIAPI *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);

> > +	efi_status_t (EFIAPI *get_image)(

> > +			struct efi_firmware_management_protocol *this,

> > +			u8 image_index,

> > +			void *image,

> > +			efi_uintn_t *image_size);

> > +	efi_status_t (EFIAPI *set_image)(

> > +			struct efi_firmware_management_protocol *this,

> > +			u8 image_index,

> > +			const void *image,

> > +			efi_uintn_t image_size,

> > +			const void *vendor_code,

> > +			efi_status_t (*progress)(efi_uintn_t completion),

> > +			u16 **abort_reason);

> > +	efi_status_t (EFIAPI *check_image)(

> > +			struct efi_firmware_management_protocol *this,

> > +			u8 image_index,

> > +			const void *image,

> > +			efi_uintn_t *image_size,

> > +			u32 *image_updatable);

> > +	efi_status_t (EFIAPI *get_package_info)(

> > +			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_status_t (EFIAPI *set_package_info)(

> > +			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);

> > +};

> > +

> >   #endif

> > diff --git a/include/efi_loader.h b/include/efi_loader.h

> > index eb57e7455eb1..1a728bf9702d 100644

> > --- a/include/efi_loader.h

> > +++ b/include/efi_loader.h

> > @@ -209,6 +209,8 @@ extern const efi_guid_t efi_guid_cert_type_pkcs7;

> >   extern const efi_guid_t efi_guid_rng_protocol;

> >   /* GUID of capsule update result */

> >   extern const efi_guid_t efi_guid_capsule_report;

> > +/* GUID of firmware management protocol */

> > +extern const efi_guid_t efi_guid_firmware_management_protocol;

> > 

> >   extern unsigned int __efi_runtime_start, __efi_runtime_stop;

> >   extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;

> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > index e1ac5ac055de..5cb34687c26a 100644

> > --- a/lib/efi_loader/Kconfig

> > +++ b/lib/efi_loader/Kconfig

> > @@ -126,6 +126,14 @@ 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_MANAGEMENT

> > +	bool "Capsule: Firmware Management Protocol"

> > +	depends on EFI_HAVE_CAPSULE_SUPPORT

> > +	default y

> > +	help

> > +	  Select this option if you want to enable capsule-based

> > +	  firmware update using Firmware Management Protocol.

> > +

> >   config EFI_DEVICE_PATH_TO_TEXT

> >   	bool "Device path to text protocol"

> >   	default y

> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c

> > index b3c7d1b735b6..2f41c17f5057 100644

> > --- a/lib/efi_loader/efi_capsule.c

> > +++ b/lib/efi_loader/efi_capsule.c

> > @@ -15,6 +15,10 @@

> >   #include <sort.h>

> > 

> >   const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID;

> > +static const efi_guid_t efi_guid_firmware_management_capsule_id =

> > +		EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;

> > +const efi_guid_t efi_guid_firmware_management_protocol =

> > +		EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;

> > 

> >   #ifdef CONFIG_EFI_CAPSULE_ON_DISK

> >   /* for file system access */

> > @@ -88,6 +92,218 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,

> >   		printf("EFI: creating %ls failed\n", variable_name16);

> >   }

> > 

> > +#ifdef CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT

> > +/**

> > + * efi_fmp_find - search for Firmware Management Protocol drivers

> > + * @image_type:		Image type guid

> > + * @instance:		Instance number

> > + * @handles:		Handles of FMP drivers

> > + * @no_handles:		Number of handles

> > + *

> > + * Search for Firmware Management Protocol drivers, matching the image

> > + * type, @image_type and the machine instance, @instance, from the list,

> > + * @handles.

> > + *

> > + * Return:

> > + * * Protocol instance	- on success

> > + * * NULL		- on failure

> > + */

> > +static struct efi_firmware_management_protocol *

> > +efi_fmp_find(efi_guid_t *image_type, u64 instance, efi_handle_t *handles,

> > +	     efi_uintn_t no_handles)

> > +{

> > +	efi_handle_t *handle;

> > +	struct efi_firmware_management_protocol *fmp;

> > +	struct efi_firmware_image_descriptor *image_info, *desc;

> > +	efi_uintn_t info_size, descriptor_size;

> > +	u32 descriptor_version;

> > +	u8 descriptor_count;

> > +	u32 package_version;

> > +	u16 *package_version_name;

> > +	bool found = false;

> > +	int i, j;

> > +	efi_status_t ret;

> > +

> > +	for (i = 0, handle = handles; i < no_handles; i++, handle++) {

> > +		ret = EFI_CALL(efi_handle_protocol(

> > +				*handle,

> > +				&efi_guid_firmware_management_protocol,

> > +				(void **)&fmp));

> > +		if (ret != EFI_SUCCESS)

> > +			continue;

> > +

> > +		/* get device's image info */

> > +		info_size = 0;

> > +		image_info = NULL;

> > +		descriptor_version = 0;

> > +		descriptor_count = 0;

> > +		descriptor_size = 0;

> > +		package_version = 0;

> > +		package_version_name = NULL;

> > +		ret = EFI_CALL(fmp->get_image_info(fmp, &info_size,

> > +						   image_info,

> > +						   &descriptor_version,

> > +						   &descriptor_count,

> > +						   &descriptor_size,

> > +						   &package_version,

> > +						   &package_version_name));

> > +		if (ret != EFI_BUFFER_TOO_SMALL)

> > +			goto skip;

> > +

> > +		image_info = malloc(info_size);

> > +		if (!image_info)

> > +			goto skip;

> > +

> > +		ret = EFI_CALL(fmp->get_image_info(fmp, &info_size,

> > +						   image_info,

> > +						   &descriptor_version,

> > +						   &descriptor_count,

> > +						   &descriptor_size,

> > +						   &package_version,

> > +						   &package_version_name));

> > +		if (ret != EFI_SUCCESS)

> > +			goto skip;

> > +

> > +		/* matching */

> > +		for (j = 0, desc = image_info; j < descriptor_count;

> > +		     j++, desc = (void *)desc + descriptor_size) {

> > +			EFI_PRINT("+++ desc[%d] index: %d, name: %ls\n",

> > +				  j, desc->image_index, desc->image_id_name);

> > +			if (!guidcmp(&desc->image_type_id, image_type) &&

> > +			    (!instance ||

> > +			     !desc->hardware_instance ||

> > +			     (descriptor_version >= 3 &&

> > +			      desc->hardware_instance == instance)))

> > +				found = true;

> > +		}

> > +

> > +skip:

> > +		efi_free_pool(package_version_name);

> > +		free(image_info);

> > +		EFI_CALL(efi_close_protocol(

> > +				(efi_handle_t)fmp,

> > +				&efi_guid_firmware_management_protocol,

> > +				NULL, NULL));

> > +		if (found)

> > +			return fmp;

> > +	}

> > +

> > +	return NULL;

> > +}

> > +

> > +/**

> > + * efi_capsule_update_firmware - update firmware from capsule

> > + * @capsule_data:	Capsule

> > + *

> > + * Update firmware, using a capsule, @capsule_data. Loading any FMP

> > + * drivers embedded in a capsule is not supported.

> > + *

> > + * Return:		status code

> > + */

> > +static efi_status_t efi_capsule_update_firmware(

> > +		struct efi_capsule_header *capsule_data)

> > +{

> > +	struct efi_firmware_management_capsule_header *capsule;

> > +	struct efi_firmware_management_capsule_image_header *image;

> > +	size_t capsule_size;

> > +	void *image_binary, *vendor_code;

> > +	efi_handle_t *handles;

> > +	efi_uintn_t no_handles;

> > +	int item;

> > +	struct efi_firmware_management_protocol *fmp;

> > +	u16 *abort_reason;

> > +	efi_status_t ret = EFI_SUCCESS;

> > +

> > +	/* sanity check */

> > +	if (capsule_data->header_size < sizeof(*capsule) ||

> > +	    capsule_data->header_size >= capsule_data->capsule_image_size)

> > +		return EFI_INVALID_PARAMETER;

> > +

> > +	capsule = (void *)capsule_data + capsule_data->header_size;

> > +	capsule_size = capsule_data->capsule_image_size

> > +			- capsule_data->header_size;

> > +

> > +	if (capsule->version != 0x00000001)

> > +		return EFI_INVALID_PARAMETER;

> 

> For an unsupported capsule format we should return EFI_UNSUPPORTED

> according to the spec.


I don't mind which error should be returned here, but I didn't
find any explicit description for the case. Did you?

> > +

> > +	/* Drivers */

> > +	/* TODO: support loading drivers */

> 

> Please, describe the requirement in the spec that you do not yet fulfill

> yet more clearly.


Driver is a driver, I think that the description is trivial.
Anyhow, this is not "TODO" as U-Boot has no ability to load/
support any dynamic modules.
So it would be better to drop the text itself.

> > +

> > +	handles = NULL;

> > +	ret = EFI_CALL(efi_locate_handle_buffer(

> > +			BY_PROTOCOL,

> > +			&efi_guid_firmware_management_protocol,

> > +			NULL, &no_handles, (efi_handle_t **)&handles));

> > +	if (ret != EFI_SUCCESS)

> > +		return EFI_UNSUPPORTED;

> > +

> > +	/* Payload */

> > +	for (item = capsule->embedded_driver_count;

> > +	     item < capsule->embedded_driver_count

> > +		    + capsule->payload_item_count; item++) {

> > +		/* sanity check */

> > +		if ((capsule->item_offset_list[item] + sizeof(*image)

> > +				 >= capsule_size)) {

> > +			printf("EFI: A capsule has not enough size of data\n");

> 

> %s/enough size of data/enough data/


I don't think that it is a wrong English, but I don't care either.


> > +			ret = EFI_INVALID_PARAMETER;

> > +			goto out;

> > +		}

> > +

> > +		image = (void *)capsule + capsule->item_offset_list[item];

> > +

> > +		if (image->version != 0x00000001 &&

> > +		    image->version != 0x00000002 &&

> > +		    image->version != 0x00000003) {

> > +			ret = EFI_INVALID_PARAMETER;

> > +			goto out;

> > +		}

> 

> UEFI 2.8 has EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.Version = 3.

> Why should we support outdated versions?

> 

> Do you really support all three versions? Are they all tested? Sughosh's

> mail sounded like 'no'.


That's why I said, in my reply to Sughosh, that I would drop the code
for *half-baked* support.

> For unsupported capsule format the return value should be EFI_UNSUPPORTED.


See above.

> > +

> > +		/* find a device for update firmware */

> > +		/* TODO: should we pass index as well, or nothing but type? */

> 

> Does this mean that the patch is not yet ready for merging?


I'm asking.
If you don't mind, I will drop the text here.

> > +		fmp = efi_fmp_find(&image->update_image_type_id,

> > +				   image->version == 0x1 ? 0 :

> > +					image->update_hardware_instance,

> > +				   handles, no_handles);

> > +		if (!fmp) {

> > +			printf("EFI Capsule: driver not found for firmware type: %pUl, hardware instance: %lld\n",

> > +			       &image->update_image_type_id,

> > +			       image->version == 0x1 ? 0 :

> > +					image->update_hardware_instance);

> > +			ret = EFI_UNSUPPORTED;

> > +			goto out;

> > +		}

> > +

> > +		/* do it */

> 

> Do what?


Do update (set_image).

> 

> > +		image_binary = (void *)image + sizeof(*image);

> > +		vendor_code = image_binary + image->update_image_size;

> > +

> > +		abort_reason = NULL;

> > +		ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,

> > +					      image_binary,

> > +					      image->update_image_size,

> > +					      vendor_code, NULL,

> > +					      &abort_reason));

> > +		if (ret != EFI_SUCCESS) {

> > +			printf("EFI Capsule: firmware update failed: %ls\n",

> > +			       abort_reason);

> 

> Why don't you use log_err() or EFI_PRINT()?


In the past, I used to use printf().
You recently changed your policy, preferring log_xxx().


> > +			efi_free_pool(abort_reason);

> > +			goto out;

> > +		}

> > +	}

> > +

> > +out:

> > +	efi_free_pool(handles);

> > +

> > +	return ret;

> > +}

> > +#else

> > +static efi_status_t efi_capsule_update_firmware(

> > +		struct efi_capsule_header *capsule_data)

> > +{

> > +	return EFI_UNSUPPORTED;

> > +}

> > +#endif /* CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT */

> > +

> >   /**

> >    * efi_update_capsule() - process information from operating system

> >    * @capsule_header_array:	Array of virtual address pointers

> > @@ -118,9 +334,29 @@ efi_status_t EFIAPI efi_update_capsule(

> >   		goto out;

> >   	}

> > 

> > -	ret = EFI_UNSUPPORTED;

> > +	ret = EFI_SUCCESS;

> >   	for (i = 0, capsule = *capsule_header_array; i < capsule_count;

> >   	     i++, capsule = *(++capsule_header_array)) {

> > +		/* sanity check */

> > +		if (capsule->header_size < sizeof(*capsule) ||

> > +		    capsule->capsule_image_size < sizeof(*capsule)) {

> > +			printf("EFI: A capsule has not enough size of data\n");

> 

> %s/enough size of data/enough data/

> 

> > +			continue;

> 

> You print an error message above. Why don't you exit with an error status?


Different capsules may modify different part of system images
(or different firmware).

So one failure may not always imply the immediate termination of
the whole process.

I think that, without any concrete "recovery" story, it would make
little sense to discuss this issue.

Sughosh will have a future plan of, what is said, A/B update.

-Takahiro Akashi

> Best regards

> 

> Heinrich

> 

> > +		}

> > +

> > +		EFI_PRINT("Capsule[%d] (guid:%pUl)\n",

> > +			  i, &capsule->capsule_guid);

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

> > +			     &efi_guid_firmware_management_capsule_id)) {

> > +			ret  = efi_capsule_update_firmware(capsule);

> > +		} else {

> > +			printf("EFI: not support capsule type: %pUl\n",

> > +			       &capsule->capsule_guid);

> > +			ret = EFI_UNSUPPORTED;

> > +		}

> > +

> > +		if (ret != EFI_SUCCESS)

> > +			goto out;

> >   	}

> >   out:

> >   	return EFI_EXIT(ret);

> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c

> > index 0735e4755b60..feec9b53c8ed 100644

> > --- a/lib/efi_loader/efi_setup.c

> > +++ b/lib/efi_loader/efi_setup.c

> > @@ -159,6 +159,10 @@ static efi_status_t efi_init_os_indications(void)

> >   		os_indications_supported |=

> >   			EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED;

> > 

> > +	if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT))

> > +		os_indications_supported |=

> > +			EFI_OS_INDICATIONS_FMP_CAPSULE_SUPPORTED;

> > +

> >   	return efi_set_variable_int(L"OsIndicationsSupported",

> >   				    &efi_global_variable_guid,

> >   				    EFI_VARIABLE_BOOTSERVICE_ACCESS |

> > 

>

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index 7a2a087c60ed..966bc6e590bf 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -217,6 +217,9 @@  enum efi_reset_type {
 #define CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE	0x00020000
 #define CAPSULE_FLAGS_INITIATE_RESET		0x00040000
 
+#define CAPSULE_SUPPORT_AUTHENTICATION		0x0000000000000001
+#define CAPSULE_SUPPORT_DEPENDENCY		0x0000000000000002
+
 #define EFI_CAPSULE_REPORT_GUID \
 	EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \
 		 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)
@@ -225,6 +228,10 @@  enum efi_reset_type {
 	EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \
 		 0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)
 
+#define EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID \
+	EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \
+		 0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)
+
 struct efi_capsule_header {
 	efi_guid_t capsule_guid;
 	u32 header_size;
@@ -253,6 +260,33 @@  struct efi_memory_range_capsule {
 	struct efi_memory_range memory_ranges[];
 } __packed;
 
+struct efi_firmware_management_capsule_header {
+	u32 version;
+	u16 embedded_driver_count;
+	u16 payload_item_count;
+	u64 item_offset_list[];
+} __packed;
+
+struct efi_firmware_management_capsule_image_header {
+	u32 version;
+	efi_guid_t update_image_type_id;
+	u8 update_image_index;
+	u8 reserved[3];
+	u32 update_image_size;
+	u32 update_vendor_code_size;
+	u64 update_hardware_instance;
+	u64 image_capsule_support;
+} __packed;
+
+struct efi_capsule_result_variable_fmp {
+	u16 version;
+	u8 payload_index;
+	u8 update_image_index;
+	efi_guid_t update_image_type_id;
+	// u16 capsule_file_name[];
+	// u16 capsule_target[];
+} __packed;
+
 #define EFI_RT_SUPPORTED_GET_TIME			0x0001
 #define EFI_RT_SUPPORTED_SET_TIME			0x0002
 #define EFI_RT_SUPPORTED_GET_WAKEUP_TIME		0x0004
@@ -1808,4 +1842,99 @@  struct efi_signature_list {
 /*	struct efi_signature_data signatures[...][signature_size]; */
 } __attribute__((__packed__));
 
+/*
+ * Firmware management protocol
+ */
+#define EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID \
+	EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \
+		 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7)
+
+#define IMAGE_ATTRIBUTE_IMAGE_UPDATABLE		0x0000000000000001
+#define IMAGE_ATTRIBUTE_RESET_REQUIRED		0x0000000000000002
+#define IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED	0x0000000000000004
+#define IMAGE_ATTRIBUTE_IN_USE			0x0000000000000008
+#define IMAGE_ATTRIBUTE_UEFI_IMAGE		0x0000000000000010
+#define IMAGE_ATTRIBUTE_DEPENDENCY		0x0000000000000020
+
+#define IMAGE_COMPATIBILITY_CHECK_SUPPORTED	0x0000000000000001
+
+#define IMAGE_UPDATABLE_VALID			0x0000000000000001
+#define IMAGE_UPDATABLE_INVALID			0x0000000000000002
+#define IMAGE_UPDATABLE_INVALID_TYPE		0x0000000000000004
+#define IMAGE_UPDATABLE_INVALID_OLLD		0x0000000000000008
+#define IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE	0x0000000000000010
+
+#define PACKAGE_ATTRIBUTE_VERSION_UPDATABLE		0x0000000000000001
+#define PACKAGE_ATTRIBUTE_RESET_REQUIRED		0x0000000000000002
+#define PACKAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED	0x0000000000000004
+
+#define EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION	4
+
+typedef struct efi_firmware_image_dependencies {
+	u8 dependencies[0];
+} efi_firmware_image_dep_t;
+
+struct efi_firmware_image_descriptor {
+	u8 image_index;
+	efi_guid_t image_type_id;
+	u64 image_id;
+	u16 *image_id_name;
+	u32 version;
+	u16 *version_name;
+	efi_uintn_t size;
+	u64 attributes_supported;
+	u64 attributes_setting;
+	u64 compatibilities;
+	u32 lowest_supported_image_version;
+	u32 last_attempt_version;
+	u32 last_attempt_status;
+	u64 hardware_instance;
+	efi_firmware_image_dep_t *dependencies;
+};
+
+struct efi_firmware_management_protocol {
+	efi_status_t (EFIAPI *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);
+	efi_status_t (EFIAPI *get_image)(
+			struct efi_firmware_management_protocol *this,
+			u8 image_index,
+			void *image,
+			efi_uintn_t *image_size);
+	efi_status_t (EFIAPI *set_image)(
+			struct efi_firmware_management_protocol *this,
+			u8 image_index,
+			const void *image,
+			efi_uintn_t image_size,
+			const void *vendor_code,
+			efi_status_t (*progress)(efi_uintn_t completion),
+			u16 **abort_reason);
+	efi_status_t (EFIAPI *check_image)(
+			struct efi_firmware_management_protocol *this,
+			u8 image_index,
+			const void *image,
+			efi_uintn_t *image_size,
+			u32 *image_updatable);
+	efi_status_t (EFIAPI *get_package_info)(
+			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_status_t (EFIAPI *set_package_info)(
+			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);
+};
+
 #endif
diff --git a/include/efi_loader.h b/include/efi_loader.h
index eb57e7455eb1..1a728bf9702d 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -209,6 +209,8 @@  extern const efi_guid_t efi_guid_cert_type_pkcs7;
 extern const efi_guid_t efi_guid_rng_protocol;
 /* GUID of capsule update result */
 extern const efi_guid_t efi_guid_capsule_report;
+/* GUID of firmware management protocol */
+extern const efi_guid_t efi_guid_firmware_management_protocol;
 
 extern unsigned int __efi_runtime_start, __efi_runtime_stop;
 extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e1ac5ac055de..5cb34687c26a 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -126,6 +126,14 @@  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_MANAGEMENT
+	bool "Capsule: Firmware Management Protocol"
+	depends on EFI_HAVE_CAPSULE_SUPPORT
+	default y
+	help
+	  Select this option if you want to enable capsule-based
+	  firmware update using Firmware Management Protocol.
+
 config EFI_DEVICE_PATH_TO_TEXT
 	bool "Device path to text protocol"
 	default y
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index b3c7d1b735b6..2f41c17f5057 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -15,6 +15,10 @@ 
 #include <sort.h>
 
 const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID;
+static const efi_guid_t efi_guid_firmware_management_capsule_id =
+		EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
+const efi_guid_t efi_guid_firmware_management_protocol =
+		EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
 
 #ifdef CONFIG_EFI_CAPSULE_ON_DISK
 /* for file system access */
@@ -88,6 +92,218 @@  void set_capsule_result(int index, struct efi_capsule_header *capsule,
 		printf("EFI: creating %ls failed\n", variable_name16);
 }
 
+#ifdef CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT
+/**
+ * efi_fmp_find - search for Firmware Management Protocol drivers
+ * @image_type:		Image type guid
+ * @instance:		Instance number
+ * @handles:		Handles of FMP drivers
+ * @no_handles:		Number of handles
+ *
+ * Search for Firmware Management Protocol drivers, matching the image
+ * type, @image_type and the machine instance, @instance, from the list,
+ * @handles.
+ *
+ * Return:
+ * * Protocol instance	- on success
+ * * NULL		- on failure
+ */
+static struct efi_firmware_management_protocol *
+efi_fmp_find(efi_guid_t *image_type, u64 instance, efi_handle_t *handles,
+	     efi_uintn_t no_handles)
+{
+	efi_handle_t *handle;
+	struct efi_firmware_management_protocol *fmp;
+	struct efi_firmware_image_descriptor *image_info, *desc;
+	efi_uintn_t info_size, descriptor_size;
+	u32 descriptor_version;
+	u8 descriptor_count;
+	u32 package_version;
+	u16 *package_version_name;
+	bool found = false;
+	int i, j;
+	efi_status_t ret;
+
+	for (i = 0, handle = handles; i < no_handles; i++, handle++) {
+		ret = EFI_CALL(efi_handle_protocol(
+				*handle,
+				&efi_guid_firmware_management_protocol,
+				(void **)&fmp));
+		if (ret != EFI_SUCCESS)
+			continue;
+
+		/* get device's image info */
+		info_size = 0;
+		image_info = NULL;
+		descriptor_version = 0;
+		descriptor_count = 0;
+		descriptor_size = 0;
+		package_version = 0;
+		package_version_name = NULL;
+		ret = EFI_CALL(fmp->get_image_info(fmp, &info_size,
+						   image_info,
+						   &descriptor_version,
+						   &descriptor_count,
+						   &descriptor_size,
+						   &package_version,
+						   &package_version_name));
+		if (ret != EFI_BUFFER_TOO_SMALL)
+			goto skip;
+
+		image_info = malloc(info_size);
+		if (!image_info)
+			goto skip;
+
+		ret = EFI_CALL(fmp->get_image_info(fmp, &info_size,
+						   image_info,
+						   &descriptor_version,
+						   &descriptor_count,
+						   &descriptor_size,
+						   &package_version,
+						   &package_version_name));
+		if (ret != EFI_SUCCESS)
+			goto skip;
+
+		/* matching */
+		for (j = 0, desc = image_info; j < descriptor_count;
+		     j++, desc = (void *)desc + descriptor_size) {
+			EFI_PRINT("+++ desc[%d] index: %d, name: %ls\n",
+				  j, desc->image_index, desc->image_id_name);
+			if (!guidcmp(&desc->image_type_id, image_type) &&
+			    (!instance ||
+			     !desc->hardware_instance ||
+			     (descriptor_version >= 3 &&
+			      desc->hardware_instance == instance)))
+				found = true;
+		}
+
+skip:
+		efi_free_pool(package_version_name);
+		free(image_info);
+		EFI_CALL(efi_close_protocol(
+				(efi_handle_t)fmp,
+				&efi_guid_firmware_management_protocol,
+				NULL, NULL));
+		if (found)
+			return fmp;
+	}
+
+	return NULL;
+}
+
+/**
+ * efi_capsule_update_firmware - update firmware from capsule
+ * @capsule_data:	Capsule
+ *
+ * Update firmware, using a capsule, @capsule_data. Loading any FMP
+ * drivers embedded in a capsule is not supported.
+ *
+ * Return:		status code
+ */
+static efi_status_t efi_capsule_update_firmware(
+		struct efi_capsule_header *capsule_data)
+{
+	struct efi_firmware_management_capsule_header *capsule;
+	struct efi_firmware_management_capsule_image_header *image;
+	size_t capsule_size;
+	void *image_binary, *vendor_code;
+	efi_handle_t *handles;
+	efi_uintn_t no_handles;
+	int item;
+	struct efi_firmware_management_protocol *fmp;
+	u16 *abort_reason;
+	efi_status_t ret = EFI_SUCCESS;
+
+	/* sanity check */
+	if (capsule_data->header_size < sizeof(*capsule) ||
+	    capsule_data->header_size >= capsule_data->capsule_image_size)
+		return EFI_INVALID_PARAMETER;
+
+	capsule = (void *)capsule_data + capsule_data->header_size;
+	capsule_size = capsule_data->capsule_image_size
+			- capsule_data->header_size;
+
+	if (capsule->version != 0x00000001)
+		return EFI_INVALID_PARAMETER;
+
+	/* Drivers */
+	/* TODO: support loading drivers */
+
+	handles = NULL;
+	ret = EFI_CALL(efi_locate_handle_buffer(
+			BY_PROTOCOL,
+			&efi_guid_firmware_management_protocol,
+			NULL, &no_handles, (efi_handle_t **)&handles));
+	if (ret != EFI_SUCCESS)
+		return EFI_UNSUPPORTED;
+
+	/* Payload */
+	for (item = capsule->embedded_driver_count;
+	     item < capsule->embedded_driver_count
+		    + capsule->payload_item_count; item++) {
+		/* sanity check */
+		if ((capsule->item_offset_list[item] + sizeof(*image)
+				 >= capsule_size)) {
+			printf("EFI: A capsule has not enough size of data\n");
+			ret = EFI_INVALID_PARAMETER;
+			goto out;
+		}
+
+		image = (void *)capsule + capsule->item_offset_list[item];
+
+		if (image->version != 0x00000001 &&
+		    image->version != 0x00000002 &&
+		    image->version != 0x00000003) {
+			ret = EFI_INVALID_PARAMETER;
+			goto out;
+		}
+
+		/* find a device for update firmware */
+		/* TODO: should we pass index as well, or nothing but type? */
+		fmp = efi_fmp_find(&image->update_image_type_id,
+				   image->version == 0x1 ? 0 :
+					image->update_hardware_instance,
+				   handles, no_handles);
+		if (!fmp) {
+			printf("EFI Capsule: driver not found for firmware type: %pUl, hardware instance: %lld\n",
+			       &image->update_image_type_id,
+			       image->version == 0x1 ? 0 :
+					image->update_hardware_instance);
+			ret = EFI_UNSUPPORTED;
+			goto out;
+		}
+
+		/* do it */
+		image_binary = (void *)image + sizeof(*image);
+		vendor_code = image_binary + image->update_image_size;
+
+		abort_reason = NULL;
+		ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,
+					      image_binary,
+					      image->update_image_size,
+					      vendor_code, NULL,
+					      &abort_reason));
+		if (ret != EFI_SUCCESS) {
+			printf("EFI Capsule: firmware update failed: %ls\n",
+			       abort_reason);
+			efi_free_pool(abort_reason);
+			goto out;
+		}
+	}
+
+out:
+	efi_free_pool(handles);
+
+	return ret;
+}
+#else
+static efi_status_t efi_capsule_update_firmware(
+		struct efi_capsule_header *capsule_data)
+{
+	return EFI_UNSUPPORTED;
+}
+#endif /* CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT */
+
 /**
  * efi_update_capsule() - process information from operating system
  * @capsule_header_array:	Array of virtual address pointers
@@ -118,9 +334,29 @@  efi_status_t EFIAPI efi_update_capsule(
 		goto out;
 	}
 
-	ret = EFI_UNSUPPORTED;
+	ret = EFI_SUCCESS;
 	for (i = 0, capsule = *capsule_header_array; i < capsule_count;
 	     i++, capsule = *(++capsule_header_array)) {
+		/* sanity check */
+		if (capsule->header_size < sizeof(*capsule) ||
+		    capsule->capsule_image_size < sizeof(*capsule)) {
+			printf("EFI: A capsule has not enough size of data\n");
+			continue;
+		}
+
+		EFI_PRINT("Capsule[%d] (guid:%pUl)\n",
+			  i, &capsule->capsule_guid);
+		if (!guidcmp(&capsule->capsule_guid,
+			     &efi_guid_firmware_management_capsule_id)) {
+			ret  = efi_capsule_update_firmware(capsule);
+		} else {
+			printf("EFI: not support capsule type: %pUl\n",
+			       &capsule->capsule_guid);
+			ret = EFI_UNSUPPORTED;
+		}
+
+		if (ret != EFI_SUCCESS)
+			goto out;
 	}
 out:
 	return EFI_EXIT(ret);
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 0735e4755b60..feec9b53c8ed 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -159,6 +159,10 @@  static efi_status_t efi_init_os_indications(void)
 		os_indications_supported |=
 			EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED;
 
+	if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT))
+		os_indications_supported |=
+			EFI_OS_INDICATIONS_FMP_CAPSULE_SUPPORTED;
+
 	return efi_set_variable_int(L"OsIndicationsSupported",
 				    &efi_global_variable_guid,
 				    EFI_VARIABLE_BOOTSERVICE_ACCESS |