| Message ID | 20200617025515.23585-12-takahiro.akashi@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | efi_loader: add capsule update support | expand |
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 common FIT image firmware in a capsule > file and apply the data using dfu backend storage drivers via > update_fit() interface. > > So "dfu_alt_info" variable should be properly set to specify a device > and location to be updated. Please read README.dfu. > > Fit image is a common file format for firmware update on U-Boot, and > this protocol works neatly just as a wrapper for one. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > --- > include/efi_api.h | 4 + > include/efi_loader.h | 2 + > lib/efi_loader/Kconfig | 11 ++ > lib/efi_loader/Makefile | 1 + > lib/efi_loader/efi_capsule.c | 12 +- > lib/efi_loader/efi_firmware.c | 253 ++++++++++++++++++++++++++++++++++ > 6 files changed, 282 insertions(+), 1 deletion(-) > create mode 100644 lib/efi_loader/efi_firmware.c > Like we had discussed this offline, I don't think that efi_firmware.c name accurately represents that the file is implementing the firmware management protocol. This can be changed to efi_fmp.c, or if you don't prefer acronyms, efi_firmware_mgmt_protocol.c -sughosh
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 common FIT image firmware in a capsule > file and apply the data using dfu backend storage drivers via > update_fit() interface. > > So "dfu_alt_info" variable should be properly set to specify a device > and location to be updated. Please read README.dfu. > > Fit image is a common file format for firmware update on U-Boot, and > this protocol works neatly just as a wrapper for one. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > --- > include/efi_api.h | 4 + > include/efi_loader.h | 2 + > lib/efi_loader/Kconfig | 11 ++ > lib/efi_loader/Makefile | 1 + > lib/efi_loader/efi_capsule.c | 12 +- > lib/efi_loader/efi_firmware.c | 253 ++++++++++++++++++++++++++++++++++ > 6 files changed, 282 insertions(+), 1 deletion(-) > create mode 100644 lib/efi_loader/efi_firmware.c > > diff --git a/include/efi_api.h b/include/efi_api.h > index b062720e8220..c3fc4edbedc4 100644 > --- a/include/efi_api.h > +++ b/include/efi_api.h > @@ -1843,6 +1843,10 @@ struct efi_signature_list { > EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \ > 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7) > > +#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \ > + EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \ > + 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47) > + > #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 bc58c7e3c1d7..5f574533e732 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -787,6 +787,8 @@ bool efi_secure_boot_enabled(void); > 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; > + > /* Capsule update */ > efi_status_t EFIAPI efi_update_capsule( > struct efi_capsule_header **capsule_header_array, > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index e1413c35e33c..305751f4b15c 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -86,6 +86,17 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT > Select this option if you want to enable capsule-based > firmware update using Firmware Management Protocol. > > +config EFI_CAPSULE_FIRMWARE_FIT > + bool "FMP driver for FIT image" > + depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT > + depends on FIT > + select UPDATE_FIT > + select DFU > + default n > + help > + Select this option if you want to enable firmware management > protocol > + driver for FIT 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 67bc5c9c0907..ea1fbc4a9deb 100644 > --- a/lib/efi_loader/Makefile > +++ b/lib/efi_loader/Makefile > @@ -24,6 +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 > Why can we not build this file based on CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT. That way, we do not have to declare the CONFIG_EFI_CAPSULE_FIRMWARE symbol on which this file is being built in a subsequent patch. -sughosh
On Sun, Jun 21, 2020 at 12:09:23AM +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 common FIT image firmware in a capsule > > file and apply the data using dfu backend storage drivers via > > update_fit() interface. > > > > So "dfu_alt_info" variable should be properly set to specify a device > > and location to be updated. Please read README.dfu. > > > > Fit image is a common file format for firmware update on U-Boot, and > > this protocol works neatly just as a wrapper for one. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > > --- > > include/efi_api.h | 4 + > > include/efi_loader.h | 2 + > > lib/efi_loader/Kconfig | 11 ++ > > lib/efi_loader/Makefile | 1 + > > lib/efi_loader/efi_capsule.c | 12 +- > > lib/efi_loader/efi_firmware.c | 253 ++++++++++++++++++++++++++++++++++ > > 6 files changed, 282 insertions(+), 1 deletion(-) > > create mode 100644 lib/efi_loader/efi_firmware.c > > > > Like we had discussed this offline, I don't think that efi_firmware.c name > accurately represents that the file is implementing the firmware management > protocol. This can be changed to efi_fmp.c, or if you don't prefer > acronyms, efi_firmware_mgmt_protocol.c I think that efi_firmware is a good prefix without any confusion given that there is no other features related to "firmware". -Takahiro Akashi > > -sughosh
On Sun, Jun 21, 2020 at 12:19:17AM +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 common FIT image firmware in a capsule > > file and apply the data using dfu backend storage drivers via > > update_fit() interface. > > > > So "dfu_alt_info" variable should be properly set to specify a device > > and location to be updated. Please read README.dfu. > > > > Fit image is a common file format for firmware update on U-Boot, and > > this protocol works neatly just as a wrapper for one. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > > --- > > include/efi_api.h | 4 + > > include/efi_loader.h | 2 + > > lib/efi_loader/Kconfig | 11 ++ > > lib/efi_loader/Makefile | 1 + > > lib/efi_loader/efi_capsule.c | 12 +- > > lib/efi_loader/efi_firmware.c | 253 ++++++++++++++++++++++++++++++++++ > > 6 files changed, 282 insertions(+), 1 deletion(-) > > create mode 100644 lib/efi_loader/efi_firmware.c > > > > diff --git a/include/efi_api.h b/include/efi_api.h > > index b062720e8220..c3fc4edbedc4 100644 > > --- a/include/efi_api.h > > +++ b/include/efi_api.h > > @@ -1843,6 +1843,10 @@ struct efi_signature_list { > > EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \ > > 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7) > > > > +#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \ > > + EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \ > > + 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47) > > + > > #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 bc58c7e3c1d7..5f574533e732 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -787,6 +787,8 @@ bool efi_secure_boot_enabled(void); > > 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; > > + > > /* Capsule update */ > > efi_status_t EFIAPI efi_update_capsule( > > struct efi_capsule_header **capsule_header_array, > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index e1413c35e33c..305751f4b15c 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -86,6 +86,17 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT > > Select this option if you want to enable capsule-based > > firmware update using Firmware Management Protocol. > > > > +config EFI_CAPSULE_FIRMWARE_FIT > > + bool "FMP driver for FIT image" > > + depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT > > + depends on FIT > > + select UPDATE_FIT > > + select DFU > > + default n > > + help > > + Select this option if you want to enable firmware management > > protocol > > + driver for FIT 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 67bc5c9c0907..ea1fbc4a9deb 100644 > > --- a/lib/efi_loader/Makefile > > +++ b/lib/efi_loader/Makefile > > @@ -24,6 +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 > > > > Why can we not build this file based on > CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT. That way, we do not have to declare > the CONFIG_EFI_CAPSULE_FIRMWARE symbol on which this file is being built in > a subsequent patch. That is because, in some cases, efi_firmware.c won't be compiled in as a specific platform may want to have its own protocol. -Takahiro Akashi > -sughosh
On Mon, 22 Jun 2020 at 06:39, AKASHI Takahiro <takahiro.akashi at linaro.org> wrote: > On Sun, Jun 21, 2020 at 12:19:17AM +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 common FIT image firmware in a capsule > > > file and apply the data using dfu backend storage drivers via > > > update_fit() interface. > > > > > > So "dfu_alt_info" variable should be properly set to specify a device > > > and location to be updated. Please read README.dfu. > > > > > > Fit image is a common file format for firmware update on U-Boot, and > > > this protocol works neatly just as a wrapper for one. > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > > > --- > > > include/efi_api.h | 4 + > > > include/efi_loader.h | 2 + > > > lib/efi_loader/Kconfig | 11 ++ > > > lib/efi_loader/Makefile | 1 + > > > lib/efi_loader/efi_capsule.c | 12 +- > > > lib/efi_loader/efi_firmware.c | 253 ++++++++++++++++++++++++++++++++++ > > > 6 files changed, 282 insertions(+), 1 deletion(-) > > > create mode 100644 lib/efi_loader/efi_firmware.c > > > > > > diff --git a/include/efi_api.h b/include/efi_api.h > > > index b062720e8220..c3fc4edbedc4 100644 > > > --- a/include/efi_api.h > > > +++ b/include/efi_api.h > > > @@ -1843,6 +1843,10 @@ struct efi_signature_list { > > > EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \ > > > 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7) > > > > > > +#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \ > > > + EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \ > > > + 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47) > > > + > > > #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 bc58c7e3c1d7..5f574533e732 100644 > > > --- a/include/efi_loader.h > > > +++ b/include/efi_loader.h > > > @@ -787,6 +787,8 @@ bool efi_secure_boot_enabled(void); > > > 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; > > > + > > > /* Capsule update */ > > > efi_status_t EFIAPI efi_update_capsule( > > > struct efi_capsule_header **capsule_header_array, > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > index e1413c35e33c..305751f4b15c 100644 > > > --- a/lib/efi_loader/Kconfig > > > +++ b/lib/efi_loader/Kconfig > > > @@ -86,6 +86,17 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT > > > Select this option if you want to enable capsule-based > > > firmware update using Firmware Management Protocol. > > > > > > +config EFI_CAPSULE_FIRMWARE_FIT > > > + bool "FMP driver for FIT image" > > > + depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT > > > + depends on FIT > > > + select UPDATE_FIT > > > + select DFU > > > + default n > > > + help > > > + Select this option if you want to enable firmware management > > > protocol > > > + driver for FIT 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 67bc5c9c0907..ea1fbc4a9deb 100644 > > > --- a/lib/efi_loader/Makefile > > > +++ b/lib/efi_loader/Makefile > > > @@ -24,6 +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 > > > > > > > Why can we not build this file based on > > CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT. That way, we do not have to > declare > > the CONFIG_EFI_CAPSULE_FIRMWARE symbol on which this file is being built > in > > a subsequent patch. > > That is because, in some cases, efi_firmware.c won't be compiled in > as a specific platform may want to have its own protocol. > I think the whole point of the review comments from Heinrich to my fmp implementation for qemu arm64 platform was to have a common fmp implementation that could be used on all platforms and architectures. Do you see a case for introducing a platform specific implementation now that we have support for a generic raw binary image. Any platform specific fixup if required, can be handled by adding pre-processing/post-processing functions. -sughosh
On Mon, Jun 22, 2020 at 01:28:07PM +0530, Sughosh Ganu wrote: > On Mon, 22 Jun 2020 at 06:39, AKASHI Takahiro <takahiro.akashi at linaro.org> > wrote: > > > On Sun, Jun 21, 2020 at 12:19:17AM +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 common FIT image firmware in a capsule > > > > file and apply the data using dfu backend storage drivers via > > > > update_fit() interface. > > > > > > > > So "dfu_alt_info" variable should be properly set to specify a device > > > > and location to be updated. Please read README.dfu. > > > > > > > > Fit image is a common file format for firmware update on U-Boot, and > > > > this protocol works neatly just as a wrapper for one. > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > > > > --- > > > > include/efi_api.h | 4 + > > > > include/efi_loader.h | 2 + > > > > lib/efi_loader/Kconfig | 11 ++ > > > > lib/efi_loader/Makefile | 1 + > > > > lib/efi_loader/efi_capsule.c | 12 +- > > > > lib/efi_loader/efi_firmware.c | 253 ++++++++++++++++++++++++++++++++++ > > > > 6 files changed, 282 insertions(+), 1 deletion(-) > > > > create mode 100644 lib/efi_loader/efi_firmware.c > > > > > > > > diff --git a/include/efi_api.h b/include/efi_api.h > > > > index b062720e8220..c3fc4edbedc4 100644 > > > > --- a/include/efi_api.h > > > > +++ b/include/efi_api.h > > > > @@ -1843,6 +1843,10 @@ struct efi_signature_list { > > > > EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \ > > > > 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7) > > > > > > > > +#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \ > > > > + EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \ > > > > + 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47) > > > > + > > > > #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 bc58c7e3c1d7..5f574533e732 100644 > > > > --- a/include/efi_loader.h > > > > +++ b/include/efi_loader.h > > > > @@ -787,6 +787,8 @@ bool efi_secure_boot_enabled(void); > > > > 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; > > > > + > > > > /* Capsule update */ > > > > efi_status_t EFIAPI efi_update_capsule( > > > > struct efi_capsule_header **capsule_header_array, > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > > index e1413c35e33c..305751f4b15c 100644 > > > > --- a/lib/efi_loader/Kconfig > > > > +++ b/lib/efi_loader/Kconfig > > > > @@ -86,6 +86,17 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT > > > > Select this option if you want to enable capsule-based > > > > firmware update using Firmware Management Protocol. > > > > > > > > +config EFI_CAPSULE_FIRMWARE_FIT > > > > + bool "FMP driver for FIT image" > > > > + depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT > > > > + depends on FIT > > > > + select UPDATE_FIT > > > > + select DFU > > > > + default n > > > > + help > > > > + Select this option if you want to enable firmware management > > > > protocol > > > > + driver for FIT 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 67bc5c9c0907..ea1fbc4a9deb 100644 > > > > --- a/lib/efi_loader/Makefile > > > > +++ b/lib/efi_loader/Makefile > > > > @@ -24,6 +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 > > > > > > > > > > Why can we not build this file based on > > > CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT. That way, we do not have to > > declare > > > the CONFIG_EFI_CAPSULE_FIRMWARE symbol on which this file is being built > > in > > > a subsequent patch. > > > > That is because, in some cases, efi_firmware.c won't be compiled in > > as a specific platform may want to have its own protocol. > > > > I think the whole point of the review comments from Heinrich to my fmp > implementation for qemu arm64 platform was to have a common fmp > implementation that could be used on all platforms and architectures. ? I believe that my patch meets Heinrich's requirement. > Do > you see a case for introducing a platform specific implementation now that > we have support for a generic raw binary image. Any platform specific fixup > if required, can be handled by adding pre-processing/post-processing > functions. I don't think we should exclude any possibility of adding platform-specific protocol as long as it doesn't break UEFI specification. Heinrich sometimes makes similar comments on different matters. -Takahiro Akashi > -sughosh
On Mon, 22 Jun 2020 at 13:37, AKASHI Takahiro <takahiro.akashi at linaro.org> wrote: > On Mon, Jun 22, 2020 at 01:28:07PM +0530, Sughosh Ganu wrote: > > On Mon, 22 Jun 2020 at 06:39, AKASHI Takahiro < > takahiro.akashi at linaro.org> > > wrote: > > > > > On Sun, Jun 21, 2020 at 12:19:17AM +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 common FIT image firmware in a > capsule > > > > > file and apply the data using dfu backend storage drivers via > > > > > update_fit() interface. > > > > > > > > > > So "dfu_alt_info" variable should be properly set to specify a > device > > > > > and location to be updated. Please read README.dfu. > > > > > > > > > > Fit image is a common file format for firmware update on U-Boot, > and > > > > > this protocol works neatly just as a wrapper for one. > > > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > > > > > --- > > > > > include/efi_api.h | 4 + > > > > > include/efi_loader.h | 2 + > > > > > lib/efi_loader/Kconfig | 11 ++ > > > > > lib/efi_loader/Makefile | 1 + > > > > > lib/efi_loader/efi_capsule.c | 12 +- > > > > > lib/efi_loader/efi_firmware.c | 253 > ++++++++++++++++++++++++++++++++++ > > > > > 6 files changed, 282 insertions(+), 1 deletion(-) > > > > > create mode 100644 lib/efi_loader/efi_firmware.c > > > > > > > > > > diff --git a/include/efi_api.h b/include/efi_api.h > > > > > index b062720e8220..c3fc4edbedc4 100644 > > > > > --- a/include/efi_api.h > > > > > +++ b/include/efi_api.h > > > > > @@ -1843,6 +1843,10 @@ struct efi_signature_list { > > > > > EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \ > > > > > 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7) > > > > > > > > > > +#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \ > > > > > + EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \ > > > > > + 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47) > > > > > + > > > > > #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 bc58c7e3c1d7..5f574533e732 100644 > > > > > --- a/include/efi_loader.h > > > > > +++ b/include/efi_loader.h > > > > > @@ -787,6 +787,8 @@ bool efi_secure_boot_enabled(void); > > > > > 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; > > > > > + > > > > > /* Capsule update */ > > > > > efi_status_t EFIAPI efi_update_capsule( > > > > > struct efi_capsule_header **capsule_header_array, > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > > > index e1413c35e33c..305751f4b15c 100644 > > > > > --- a/lib/efi_loader/Kconfig > > > > > +++ b/lib/efi_loader/Kconfig > > > > > @@ -86,6 +86,17 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT > > > > > Select this option if you want to enable capsule-based > > > > > firmware update using Firmware Management Protocol. > > > > > > > > > > +config EFI_CAPSULE_FIRMWARE_FIT > > > > > + bool "FMP driver for FIT image" > > > > > + depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT > > > > > + depends on FIT > > > > > + select UPDATE_FIT > > > > > + select DFU > > > > > + default n > > > > > + help > > > > > + Select this option if you want to enable firmware > management > > > > > protocol > > > > > + driver for FIT 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 67bc5c9c0907..ea1fbc4a9deb 100644 > > > > > --- a/lib/efi_loader/Makefile > > > > > +++ b/lib/efi_loader/Makefile > > > > > @@ -24,6 +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 > > > > > > > > > > > > > Why can we not build this file based on > > > > CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT. That way, we do not have to > > > declare > > > > the CONFIG_EFI_CAPSULE_FIRMWARE symbol on which this file is being > built > > > in > > > > a subsequent patch. > > > > > > That is because, in some cases, efi_firmware.c won't be compiled in > > > as a specific platform may want to have its own protocol. > > > > > > > I think the whole point of the review comments from Heinrich to my fmp > > implementation for qemu arm64 platform was to have a common fmp > > implementation that could be used on all platforms and architectures. > > ? I believe that my patch meets Heinrich's requirement. > Yes, and that was my point. About the need for having a platform specific fmp implementation after having your patch. > > > Do > > you see a case for introducing a platform specific implementation now > that > > we have support for a generic raw binary image. Any platform specific > fixup > > if required, can be handled by adding pre-processing/post-processing > > functions. > > I don't think we should exclude any possibility of adding > platform-specific protocol as long as it doesn't break > UEFI specification. Heinrich sometimes makes similar comments > on different matters. Ok. My comments were based on what Heinrich had mentioned as part of reviewing my fmp patches. In any case, i think there is no harm in building this with the CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT symbol. If any platform does add a fmp implementation, that should be a different, platform specific symbol. -sughosh
diff --git a/include/efi_api.h b/include/efi_api.h index b062720e8220..c3fc4edbedc4 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -1843,6 +1843,10 @@ struct efi_signature_list { EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \ 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7) +#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \ + EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \ + 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47) + #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 bc58c7e3c1d7..5f574533e732 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -787,6 +787,8 @@ bool efi_secure_boot_enabled(void); 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; + /* Capsule update */ efi_status_t EFIAPI efi_update_capsule( struct efi_capsule_header **capsule_header_array, diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index e1413c35e33c..305751f4b15c 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -86,6 +86,17 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT Select this option if you want to enable capsule-based firmware update using Firmware Management Protocol. +config EFI_CAPSULE_FIRMWARE_FIT + bool "FMP driver for FIT image" + depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT + depends on FIT + select UPDATE_FIT + select DFU + default n + help + Select this option if you want to enable firmware management protocol + driver for FIT 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 67bc5c9c0907..ea1fbc4a9deb 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -24,6 +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-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 22d15bc4d8cd..c2cd1d23f9ce 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -801,7 +801,17 @@ static void efi_capsule_scan_done(void) */ efi_status_t __weak arch_efi_load_capsule_drivers(void) { - return EFI_SUCCESS; + __maybe_unused efi_handle_t handle; + efi_status_t ret = EFI_SUCCESS; + + if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_FIT)) { + handle = NULL; + ret = EFI_CALL(efi_install_multiple_protocol_interfaces( + &handle, &efi_guid_firmware_management_protocol, + &efi_fmp_fit, NULL)); + } + + return ret; } /** diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c new file mode 100644 index 000000000000..28ce5647a2cc --- /dev/null +++ b/lib/efi_loader/efi_firmware.c @@ -0,0 +1,253 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * EFI Firmware management protocol + * + * Copyright (c) 2020 Linaro Limited + * Author: AKASHI Takahiro + */ + +#include <common.h> +#include <charset.h> +#include <dfu.h> +#include <efi_loader.h> +#include <image.h> +#include <linux/list.h> + +/* + * This FIRMWARE_MANAGEMENT_PROTOCOL driver provides a firmware update + * method with existing FIT image format, and handles + * - multiple regions of firmware via DFU + * but doesn't support + * - versioning of firmware image + * - package information + */ +const efi_guid_t efi_firmware_image_type_uboot_fit = + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; + +/** + * efi_firmware_fit_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_fit_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) +{ + struct dfu_entity *dfu; + size_t names_len, total_size; + int dfu_num, i; + u16 *name, *next; + efi_status_t ret = EFI_SUCCESS; + + 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_fit; + 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 */ +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 + * @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 + * have FIT image format commonly used in U-Boot. + * @vendor_code, @progress and @abort_reason are not supported. + * + * Return: status code + */ +static +efi_status_t EFIAPI efi_firmware_fit_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_ENTRY("%p %d %p %ld %p %p %p\n", this, image_index, image, + image_size, vendor_code, progress, abort_reason); + + if (!image || image_index != 1) + return EFI_EXIT(EFI_INVALID_PARAMETER); + + if (fit_update(image)) + return EFI_EXIT(EFI_DEVICE_ERROR); + + 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); + + 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); +} + +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, +};
In this commit, a very simple firmware management protocol driver is implemented. It will take a common FIT image firmware in a capsule file and apply the data using dfu backend storage drivers via update_fit() interface. So "dfu_alt_info" variable should be properly set to specify a device and location to be updated. Please read README.dfu. Fit image is a common file format for firmware update on U-Boot, and this protocol works neatly just as a wrapper for one. Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> --- include/efi_api.h | 4 + include/efi_loader.h | 2 + lib/efi_loader/Kconfig | 11 ++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_capsule.c | 12 +- lib/efi_loader/efi_firmware.c | 253 ++++++++++++++++++++++++++++++++++ 6 files changed, 282 insertions(+), 1 deletion(-) create mode 100644 lib/efi_loader/efi_firmware.c