* [PATCH v6 1/8] efi_loader: add the number of image entries in efi_capsule_update_info
2023-05-19 10:32 [PATCH v6 0/8] FMP versioning support Masahisa Kojima
@ 2023-05-19 10:32 ` Masahisa Kojima
2023-05-22 7:34 ` Ilias Apalodimas
2023-05-22 20:42 ` Ilias Apalodimas
2023-05-19 10:32 ` [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable Masahisa Kojima
` (8 subsequent siblings)
9 siblings, 2 replies; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-19 10:32 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Takahiro Akashi, Masahisa Kojima, Philipp Tomsich, Kever Yang,
Ying-Chun Liu (PaulLiu), Tuomas Tynkkynen, Heiko Thiery,
Frieder Schrempf, Michael Walle, Mario Six, Jassi Brar,
Patrick Delaunay, Patrice Chotard, Michal Simek, Sughosh Ganu,
Etienne Carriere, moderated list:STM32MP1 BOARD
The number of image array entries global variable is required
to support EFI capsule update. This information is exposed as a
num_image_type_guids variable, but this information
should be included in the efi_capsule_update_info structure.
This commit adds the num_images member in the
efi_capsule_update_info structure. All board files supporting
EFI capsule update are updated.
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Newly created in v6
arch/arm/mach-rockchip/board.c | 4 ++--
board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 2 +-
board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 2 +-
board/emulation/qemu-arm/qemu-arm.c | 2 +-
board/kontron/pitx_imx8m/pitx_imx8m.c | 2 +-
board/kontron/sl-mx8mm/sl-mx8mm.c | 2 +-
board/kontron/sl28/sl28.c | 2 +-
board/rockchip/evb_rk3399/evb-rk3399.c | 2 +-
board/sandbox/sandbox.c | 2 +-
board/socionext/developerbox/developerbox.c | 2 +-
board/st/stm32mp1/stm32mp1.c | 2 +-
board/xilinx/common/board.c | 2 +-
include/efi_loader.h | 3 ++-
lib/efi_loader/efi_firmware.c | 6 +++---
lib/fwu_updates/fwu.c | 2 +-
15 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index f1f70c81d0..8daa74b3eb 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -41,7 +41,7 @@ static bool updatable_image(struct disk_partition *info)
uuid_str_to_bin(info->type_guid, image_type_guid.b,
UUID_STR_FORMAT_GUID);
- for (i = 0; i < num_image_type_guids; i++) {
+ for (i = 0; i < update_info.num_images; i++) {
if (!guidcmp(&fw_images[i].image_type_id, &image_type_guid)) {
ret = true;
break;
@@ -59,7 +59,7 @@ static void set_image_index(struct disk_partition *info, int index)
uuid_str_to_bin(info->type_guid, image_type_guid.b,
UUID_STR_FORMAT_GUID);
- for (i = 0; i < num_image_type_guids; i++) {
+ for (i = 0; i < update_info.num_images; i++) {
if (!guidcmp(&fw_images[i].image_type_id, &image_type_guid)) {
fw_images[i].image_index = index;
break;
diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
index 466174679e..b79a2380aa 100644
--- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
+++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
@@ -54,10 +54,10 @@ struct efi_fw_image fw_images[] = {
struct efi_capsule_update_info update_info = {
.dfu_string = "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1",
+ .num_images = ARRAY_SIZE(fw_images),
.images = fw_images,
};
-u8 num_image_type_guids = ARRAY_SIZE(fw_images);
#endif /* EFI_HAVE_CAPSULE_SUPPORT */
diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
index b373e45df9..af070ec315 100644
--- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
+++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
@@ -50,10 +50,10 @@ struct efi_fw_image fw_images[] = {
struct efi_capsule_update_info update_info = {
.dfu_string = "mmc 2=flash-bin raw 0x42 0x1D00 mmcpart 1",
+ .num_images = ARRAY_SIZE(fw_images),
.images = fw_images,
};
-u8 num_image_type_guids = ARRAY_SIZE(fw_images);
#endif /* EFI_HAVE_CAPSULE_SUPPORT */
int board_phys_sdram_size(phys_size_t *size)
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
index 34ed3e8ae6..dfea0d92a3 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -47,10 +47,10 @@ struct efi_fw_image fw_images[] = {
};
struct efi_capsule_update_info update_info = {
+ .num_images = ARRAY_SIZE(fw_images)
.images = fw_images,
};
-u8 num_image_type_guids = ARRAY_SIZE(fw_images);
#endif /* EFI_HAVE_CAPSULE_SUPPORT */
static struct mm_region qemu_arm64_mem_map[] = {
diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c b/board/kontron/pitx_imx8m/pitx_imx8m.c
index fcda86bc1b..4548e7c1df 100644
--- a/board/kontron/pitx_imx8m/pitx_imx8m.c
+++ b/board/kontron/pitx_imx8m/pitx_imx8m.c
@@ -43,10 +43,10 @@ struct efi_fw_image fw_images[] = {
struct efi_capsule_update_info update_info = {
.dfu_string = "mmc 0=flash-bin raw 0x42 0x1000 mmcpart 1",
+ .num_images = ARRAY_SIZE(fw_images),
.images = fw_images,
};
-u8 num_image_type_guids = ARRAY_SIZE(fw_images);
#endif /* EFI_HAVE_CAPSULE_SUPPORT */
int board_early_init_f(void)
diff --git a/board/kontron/sl-mx8mm/sl-mx8mm.c b/board/kontron/sl-mx8mm/sl-mx8mm.c
index 250195694b..ddb509eb66 100644
--- a/board/kontron/sl-mx8mm/sl-mx8mm.c
+++ b/board/kontron/sl-mx8mm/sl-mx8mm.c
@@ -29,10 +29,10 @@ struct efi_fw_image fw_images[] = {
struct efi_capsule_update_info update_info = {
.dfu_string = "sf 0:0=flash-bin raw 0x400 0x1f0000",
+ .num_images = ARRAY_SIZE(fw_images),
.images = fw_images,
};
-u8 num_image_type_guids = ARRAY_SIZE(fw_images);
#endif /* EFI_HAVE_CAPSULE_SUPPORT */
int board_phys_sdram_size(phys_size_t *size)
diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c
index 89948e087f..4ab221c12b 100644
--- a/board/kontron/sl28/sl28.c
+++ b/board/kontron/sl28/sl28.c
@@ -40,10 +40,10 @@ struct efi_fw_image fw_images[] = {
struct efi_capsule_update_info update_info = {
.dfu_string = "sf 0:0=u-boot-bin raw 0x210000 0x1d0000;"
"u-boot-env raw 0x3e0000 0x20000",
+ .num_images = ARRAY_SIZE(fw_images),
.images = fw_images,
};
-u8 num_image_type_guids = ARRAY_SIZE(fw_images);
#endif /* EFI_HAVE_CAPSULE_SUPPORT */
int board_early_init_f(void)
diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c
index c99ffdd75e..3c773d0930 100644
--- a/board/rockchip/evb_rk3399/evb-rk3399.c
+++ b/board/rockchip/evb_rk3399/evb-rk3399.c
@@ -18,10 +18,10 @@
static struct efi_fw_image fw_images[ROCKPI4_UPDATABLE_IMAGES] = {0};
struct efi_capsule_update_info update_info = {
+ .num_images = ROCKPI4_UPDATABLE_IMAGES,
.images = fw_images,
};
-u8 num_image_type_guids = ROCKPI4_UPDATABLE_IMAGES;
#endif
#ifndef CONFIG_SPL_BUILD
diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index 2e44bdf0df..c7b6cb78ff 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -67,10 +67,10 @@ struct efi_fw_image fw_images[] = {
struct efi_capsule_update_info update_info = {
.dfu_string = "sf 0:0=u-boot-bin raw 0x100000 0x50000;"
"u-boot-env raw 0x150000 0x200000",
+ .num_images = ARRAY_SIZE(fw_images),
.images = fw_images,
};
-u8 num_image_type_guids = ARRAY_SIZE(fw_images);
#endif /* EFI_HAVE_CAPSULE_SUPPORT */
#if !CONFIG_IS_ENABLED(OF_PLATDATA)
diff --git a/board/socionext/developerbox/developerbox.c b/board/socionext/developerbox/developerbox.c
index 16e14d4f7f..d92e1d0962 100644
--- a/board/socionext/developerbox/developerbox.c
+++ b/board/socionext/developerbox/developerbox.c
@@ -41,10 +41,10 @@ struct efi_capsule_update_info update_info = {
.dfu_string = "mtd nor1=u-boot.bin raw 200000 100000;"
"fip.bin raw 180000 78000;"
"optee.bin raw 500000 100000",
+ .num_images = ARRAY_SIZE(fw_images),
.images = fw_images,
};
-u8 num_image_type_guids = ARRAY_SIZE(fw_images);
#endif /* EFI_HAVE_CAPSULE_SUPPORT */
static struct mm_region sc2a11_mem_map[] = {
diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 1a1b1844c8..5b28ccd32e 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -92,10 +92,10 @@
struct efi_fw_image fw_images[1];
struct efi_capsule_update_info update_info = {
+ .num_images = ARRAY_SIZE(fw_images),
.images = fw_images,
};
-u8 num_image_type_guids = ARRAY_SIZE(fw_images);
#endif /* EFI_HAVE_CAPSULE_SUPPORT */
int board_early_init_f(void)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index d071ebfb9c..0328d68e75 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -52,10 +52,10 @@ struct efi_fw_image fw_images[] = {
};
struct efi_capsule_update_info update_info = {
+ .num_images = ARRAY_SIZE(fw_images),
.images = fw_images,
};
-u8 num_image_type_guids = ARRAY_SIZE(fw_images);
#endif /* EFI_HAVE_CAPSULE_SUPPORT */
#define EEPROM_HEADER_MAGIC 0xdaaddeed
diff --git a/include/efi_loader.h b/include/efi_loader.h
index b395eef9e7..941d63467c 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -1078,15 +1078,16 @@ struct efi_fw_image {
* platforms which enable capsule updates
*
* @dfu_string: String used to populate dfu_alt_info
+ * @num_images: The number of images array entries
* @images: Pointer to an array of updatable images
*/
struct efi_capsule_update_info {
const char *dfu_string;
+ int num_images;
struct efi_fw_image *images;
};
extern struct efi_capsule_update_info update_info;
-extern u8 num_image_type_guids;
/**
* Install the ESRT system table.
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 93e2b01c07..cc650e1443 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -131,7 +131,7 @@ static efi_status_t efi_fill_image_desc_array(
struct efi_fw_image *fw_array;
int i;
- total_size = sizeof(*image_info) * num_image_type_guids;
+ total_size = sizeof(*image_info) * update_info.num_images;
if (*image_info_size < total_size) {
*image_info_size = total_size;
@@ -141,13 +141,13 @@ static efi_status_t efi_fill_image_desc_array(
*image_info_size = total_size;
fw_array = update_info.images;
- *descriptor_count = num_image_type_guids;
+ *descriptor_count = update_info.num_images;
*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
*descriptor_size = sizeof(*image_info);
*package_version = 0xffffffff; /* not supported */
*package_version_name = NULL; /* not supported */
- for (i = 0; i < num_image_type_guids; i++) {
+ for (i = 0; i < update_info.num_images; i++) {
image_info[i].image_index = fw_array[i].image_index;
image_info[i].image_type_id = fw_array[i].image_type_id;
image_info[i].image_id = fw_array[i].image_index;
diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index 5313d07302..3b1785e7b1 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -151,7 +151,7 @@ static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id)
index = *image_index;
image = update_info.images;
- for (i = 0; i < num_image_type_guids; i++) {
+ for (i = 0; i < update_info.num_images; i++) {
if (index == image[i].image_index) {
guidcpy(image_type_id, &image[i].image_type_id);
return 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 1/8] efi_loader: add the number of image entries in efi_capsule_update_info
2023-05-19 10:32 ` [PATCH v6 1/8] efi_loader: add the number of image entries in efi_capsule_update_info Masahisa Kojima
@ 2023-05-22 7:34 ` Ilias Apalodimas
2023-05-22 20:42 ` Ilias Apalodimas
1 sibling, 0 replies; 27+ messages in thread
From: Ilias Apalodimas @ 2023-05-22 7:34 UTC (permalink / raw)
To: Masahisa Kojima
Cc: u-boot, Heinrich Schuchardt, Simon Glass, Takahiro Akashi,
Philipp Tomsich, Kever Yang, Ying-Chun Liu (PaulLiu),
Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
Mario Six, Jassi Brar, Patrick Delaunay, Patrice Chotard,
Michal Simek, Sughosh Ganu, Etienne Carriere,
moderated list:STM32MP1 BOARD
On Fri, May 19, 2023 at 07:32:07PM +0900, Masahisa Kojima wrote:
> The number of image array entries global variable is required
> to support EFI capsule update. This information is exposed as a
> num_image_type_guids variable, but this information
> should be included in the efi_capsule_update_info structure.
>
> This commit adds the num_images member in the
> efi_capsule_update_info structure. All board files supporting
> EFI capsule update are updated.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Newly created in v6
>
> arch/arm/mach-rockchip/board.c | 4 ++--
> board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 2 +-
> board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 2 +-
> board/emulation/qemu-arm/qemu-arm.c | 2 +-
> board/kontron/pitx_imx8m/pitx_imx8m.c | 2 +-
> board/kontron/sl-mx8mm/sl-mx8mm.c | 2 +-
> board/kontron/sl28/sl28.c | 2 +-
> board/rockchip/evb_rk3399/evb-rk3399.c | 2 +-
> board/sandbox/sandbox.c | 2 +-
> board/socionext/developerbox/developerbox.c | 2 +-
> board/st/stm32mp1/stm32mp1.c | 2 +-
> board/xilinx/common/board.c | 2 +-
> include/efi_loader.h | 3 ++-
> lib/efi_loader/efi_firmware.c | 6 +++---
> lib/fwu_updates/fwu.c | 2 +-
> 15 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
> index f1f70c81d0..8daa74b3eb 100644
> --- a/arch/arm/mach-rockchip/board.c
> +++ b/arch/arm/mach-rockchip/board.c
> @@ -41,7 +41,7 @@ static bool updatable_image(struct disk_partition *info)
> uuid_str_to_bin(info->type_guid, image_type_guid.b,
> UUID_STR_FORMAT_GUID);
>
> - for (i = 0; i < num_image_type_guids; i++) {
> + for (i = 0; i < update_info.num_images; i++) {
> if (!guidcmp(&fw_images[i].image_type_id, &image_type_guid)) {
> ret = true;
> break;
> @@ -59,7 +59,7 @@ static void set_image_index(struct disk_partition *info, int index)
> uuid_str_to_bin(info->type_guid, image_type_guid.b,
> UUID_STR_FORMAT_GUID);
>
> - for (i = 0; i < num_image_type_guids; i++) {
> + for (i = 0; i < update_info.num_images; i++) {
> if (!guidcmp(&fw_images[i].image_type_id, &image_type_guid)) {
> fw_images[i].image_index = index;
> break;
> diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> index 466174679e..b79a2380aa 100644
> --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> @@ -54,10 +54,10 @@ struct efi_fw_image fw_images[] = {
>
> struct efi_capsule_update_info update_info = {
> .dfu_string = "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
>
> diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> index b373e45df9..af070ec315 100644
> --- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> +++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> @@ -50,10 +50,10 @@ struct efi_fw_image fw_images[] = {
>
> struct efi_capsule_update_info update_info = {
> .dfu_string = "mmc 2=flash-bin raw 0x42 0x1D00 mmcpart 1",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> int board_phys_sdram_size(phys_size_t *size)
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index 34ed3e8ae6..dfea0d92a3 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -47,10 +47,10 @@ struct efi_fw_image fw_images[] = {
> };
>
> struct efi_capsule_update_info update_info = {
> + .num_images = ARRAY_SIZE(fw_images)
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> static struct mm_region qemu_arm64_mem_map[] = {
> diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c b/board/kontron/pitx_imx8m/pitx_imx8m.c
> index fcda86bc1b..4548e7c1df 100644
> --- a/board/kontron/pitx_imx8m/pitx_imx8m.c
> +++ b/board/kontron/pitx_imx8m/pitx_imx8m.c
> @@ -43,10 +43,10 @@ struct efi_fw_image fw_images[] = {
>
> struct efi_capsule_update_info update_info = {
> .dfu_string = "mmc 0=flash-bin raw 0x42 0x1000 mmcpart 1",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> int board_early_init_f(void)
> diff --git a/board/kontron/sl-mx8mm/sl-mx8mm.c b/board/kontron/sl-mx8mm/sl-mx8mm.c
> index 250195694b..ddb509eb66 100644
> --- a/board/kontron/sl-mx8mm/sl-mx8mm.c
> +++ b/board/kontron/sl-mx8mm/sl-mx8mm.c
> @@ -29,10 +29,10 @@ struct efi_fw_image fw_images[] = {
>
> struct efi_capsule_update_info update_info = {
> .dfu_string = "sf 0:0=flash-bin raw 0x400 0x1f0000",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> int board_phys_sdram_size(phys_size_t *size)
> diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c
> index 89948e087f..4ab221c12b 100644
> --- a/board/kontron/sl28/sl28.c
> +++ b/board/kontron/sl28/sl28.c
> @@ -40,10 +40,10 @@ struct efi_fw_image fw_images[] = {
> struct efi_capsule_update_info update_info = {
> .dfu_string = "sf 0:0=u-boot-bin raw 0x210000 0x1d0000;"
> "u-boot-env raw 0x3e0000 0x20000",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> int board_early_init_f(void)
> diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c
> index c99ffdd75e..3c773d0930 100644
> --- a/board/rockchip/evb_rk3399/evb-rk3399.c
> +++ b/board/rockchip/evb_rk3399/evb-rk3399.c
> @@ -18,10 +18,10 @@
> static struct efi_fw_image fw_images[ROCKPI4_UPDATABLE_IMAGES] = {0};
>
> struct efi_capsule_update_info update_info = {
> + .num_images = ROCKPI4_UPDATABLE_IMAGES,
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ROCKPI4_UPDATABLE_IMAGES;
> #endif
>
> #ifndef CONFIG_SPL_BUILD
> diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
> index 2e44bdf0df..c7b6cb78ff 100644
> --- a/board/sandbox/sandbox.c
> +++ b/board/sandbox/sandbox.c
> @@ -67,10 +67,10 @@ struct efi_fw_image fw_images[] = {
> struct efi_capsule_update_info update_info = {
> .dfu_string = "sf 0:0=u-boot-bin raw 0x100000 0x50000;"
> "u-boot-env raw 0x150000 0x200000",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> #if !CONFIG_IS_ENABLED(OF_PLATDATA)
> diff --git a/board/socionext/developerbox/developerbox.c b/board/socionext/developerbox/developerbox.c
> index 16e14d4f7f..d92e1d0962 100644
> --- a/board/socionext/developerbox/developerbox.c
> +++ b/board/socionext/developerbox/developerbox.c
> @@ -41,10 +41,10 @@ struct efi_capsule_update_info update_info = {
> .dfu_string = "mtd nor1=u-boot.bin raw 200000 100000;"
> "fip.bin raw 180000 78000;"
> "optee.bin raw 500000 100000",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> static struct mm_region sc2a11_mem_map[] = {
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 1a1b1844c8..5b28ccd32e 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -92,10 +92,10 @@
> struct efi_fw_image fw_images[1];
>
> struct efi_capsule_update_info update_info = {
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> int board_early_init_f(void)
> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
> index d071ebfb9c..0328d68e75 100644
> --- a/board/xilinx/common/board.c
> +++ b/board/xilinx/common/board.c
> @@ -52,10 +52,10 @@ struct efi_fw_image fw_images[] = {
> };
>
> struct efi_capsule_update_info update_info = {
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> #define EEPROM_HEADER_MAGIC 0xdaaddeed
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index b395eef9e7..941d63467c 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -1078,15 +1078,16 @@ struct efi_fw_image {
> * platforms which enable capsule updates
> *
> * @dfu_string: String used to populate dfu_alt_info
> + * @num_images: The number of images array entries
> * @images: Pointer to an array of updatable images
> */
> struct efi_capsule_update_info {
> const char *dfu_string;
> + int num_images;
> struct efi_fw_image *images;
> };
>
> extern struct efi_capsule_update_info update_info;
> -extern u8 num_image_type_guids;
>
> /**
> * Install the ESRT system table.
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 93e2b01c07..cc650e1443 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -131,7 +131,7 @@ static efi_status_t efi_fill_image_desc_array(
> struct efi_fw_image *fw_array;
> int i;
>
> - total_size = sizeof(*image_info) * num_image_type_guids;
> + total_size = sizeof(*image_info) * update_info.num_images;
>
> if (*image_info_size < total_size) {
> *image_info_size = total_size;
> @@ -141,13 +141,13 @@ static efi_status_t efi_fill_image_desc_array(
> *image_info_size = total_size;
>
> fw_array = update_info.images;
> - *descriptor_count = num_image_type_guids;
> + *descriptor_count = update_info.num_images;
> *descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> *descriptor_size = sizeof(*image_info);
> *package_version = 0xffffffff; /* not supported */
> *package_version_name = NULL; /* not supported */
>
> - for (i = 0; i < num_image_type_guids; i++) {
> + for (i = 0; i < update_info.num_images; i++) {
> image_info[i].image_index = fw_array[i].image_index;
> image_info[i].image_type_id = fw_array[i].image_type_id;
> image_info[i].image_id = fw_array[i].image_index;
> diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> index 5313d07302..3b1785e7b1 100644
> --- a/lib/fwu_updates/fwu.c
> +++ b/lib/fwu_updates/fwu.c
> @@ -151,7 +151,7 @@ static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id)
>
> index = *image_index;
> image = update_info.images;
> - for (i = 0; i < num_image_type_guids; i++) {
> + for (i = 0; i < update_info.num_images; i++) {
> if (index == image[i].image_index) {
> guidcpy(image_type_id, &image[i].image_type_id);
> return 0;
> --
> 2.17.1
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6 1/8] efi_loader: add the number of image entries in efi_capsule_update_info
2023-05-19 10:32 ` [PATCH v6 1/8] efi_loader: add the number of image entries in efi_capsule_update_info Masahisa Kojima
2023-05-22 7:34 ` Ilias Apalodimas
@ 2023-05-22 20:42 ` Ilias Apalodimas
1 sibling, 0 replies; 27+ messages in thread
From: Ilias Apalodimas @ 2023-05-22 20:42 UTC (permalink / raw)
To: Masahisa Kojima
Cc: u-boot, Heinrich Schuchardt, Simon Glass, Takahiro Akashi,
Philipp Tomsich, Kever Yang, Ying-Chun Liu (PaulLiu),
Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
Mario Six, Jassi Brar, Patrick Delaunay, Patrice Chotard,
Michal Simek, Sughosh Ganu, Etienne Carriere,
moderated list:STM32MP1 BOARD
On Fri, May 19, 2023 at 07:32:07PM +0900, Masahisa Kojima wrote:
> The number of image array entries global variable is required
> to support EFI capsule update. This information is exposed as a
> num_image_type_guids variable, but this information
> should be included in the efi_capsule_update_info structure.
>
> This commit adds the num_images member in the
> efi_capsule_update_info structure. All board files supporting
> EFI capsule update are updated.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Newly created in v6
>
> arch/arm/mach-rockchip/board.c | 4 ++--
> board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 2 +-
> board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 2 +-
> board/emulation/qemu-arm/qemu-arm.c | 2 +-
> board/kontron/pitx_imx8m/pitx_imx8m.c | 2 +-
> board/kontron/sl-mx8mm/sl-mx8mm.c | 2 +-
> board/kontron/sl28/sl28.c | 2 +-
> board/rockchip/evb_rk3399/evb-rk3399.c | 2 +-
> board/sandbox/sandbox.c | 2 +-
> board/socionext/developerbox/developerbox.c | 2 +-
> board/st/stm32mp1/stm32mp1.c | 2 +-
> board/xilinx/common/board.c | 2 +-
> include/efi_loader.h | 3 ++-
> lib/efi_loader/efi_firmware.c | 6 +++---
> lib/fwu_updates/fwu.c | 2 +-
> 15 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
> index f1f70c81d0..8daa74b3eb 100644
> --- a/arch/arm/mach-rockchip/board.c
> +++ b/arch/arm/mach-rockchip/board.c
> @@ -41,7 +41,7 @@ static bool updatable_image(struct disk_partition *info)
> uuid_str_to_bin(info->type_guid, image_type_guid.b,
> UUID_STR_FORMAT_GUID);
>
> - for (i = 0; i < num_image_type_guids; i++) {
> + for (i = 0; i < update_info.num_images; i++) {
> if (!guidcmp(&fw_images[i].image_type_id, &image_type_guid)) {
> ret = true;
> break;
> @@ -59,7 +59,7 @@ static void set_image_index(struct disk_partition *info, int index)
> uuid_str_to_bin(info->type_guid, image_type_guid.b,
> UUID_STR_FORMAT_GUID);
>
> - for (i = 0; i < num_image_type_guids; i++) {
> + for (i = 0; i < update_info.num_images; i++) {
> if (!guidcmp(&fw_images[i].image_type_id, &image_type_guid)) {
> fw_images[i].image_index = index;
> break;
> diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> index 466174679e..b79a2380aa 100644
> --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> @@ -54,10 +54,10 @@ struct efi_fw_image fw_images[] = {
>
> struct efi_capsule_update_info update_info = {
> .dfu_string = "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
>
> diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> index b373e45df9..af070ec315 100644
> --- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> +++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> @@ -50,10 +50,10 @@ struct efi_fw_image fw_images[] = {
>
> struct efi_capsule_update_info update_info = {
> .dfu_string = "mmc 2=flash-bin raw 0x42 0x1D00 mmcpart 1",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> int board_phys_sdram_size(phys_size_t *size)
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index 34ed3e8ae6..dfea0d92a3 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -47,10 +47,10 @@ struct efi_fw_image fw_images[] = {
> };
>
> struct efi_capsule_update_info update_info = {
> + .num_images = ARRAY_SIZE(fw_images)
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> static struct mm_region qemu_arm64_mem_map[] = {
> diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c b/board/kontron/pitx_imx8m/pitx_imx8m.c
> index fcda86bc1b..4548e7c1df 100644
> --- a/board/kontron/pitx_imx8m/pitx_imx8m.c
> +++ b/board/kontron/pitx_imx8m/pitx_imx8m.c
> @@ -43,10 +43,10 @@ struct efi_fw_image fw_images[] = {
>
> struct efi_capsule_update_info update_info = {
> .dfu_string = "mmc 0=flash-bin raw 0x42 0x1000 mmcpart 1",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> int board_early_init_f(void)
> diff --git a/board/kontron/sl-mx8mm/sl-mx8mm.c b/board/kontron/sl-mx8mm/sl-mx8mm.c
> index 250195694b..ddb509eb66 100644
> --- a/board/kontron/sl-mx8mm/sl-mx8mm.c
> +++ b/board/kontron/sl-mx8mm/sl-mx8mm.c
> @@ -29,10 +29,10 @@ struct efi_fw_image fw_images[] = {
>
> struct efi_capsule_update_info update_info = {
> .dfu_string = "sf 0:0=flash-bin raw 0x400 0x1f0000",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> int board_phys_sdram_size(phys_size_t *size)
> diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c
> index 89948e087f..4ab221c12b 100644
> --- a/board/kontron/sl28/sl28.c
> +++ b/board/kontron/sl28/sl28.c
> @@ -40,10 +40,10 @@ struct efi_fw_image fw_images[] = {
> struct efi_capsule_update_info update_info = {
> .dfu_string = "sf 0:0=u-boot-bin raw 0x210000 0x1d0000;"
> "u-boot-env raw 0x3e0000 0x20000",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> int board_early_init_f(void)
> diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c
> index c99ffdd75e..3c773d0930 100644
> --- a/board/rockchip/evb_rk3399/evb-rk3399.c
> +++ b/board/rockchip/evb_rk3399/evb-rk3399.c
> @@ -18,10 +18,10 @@
> static struct efi_fw_image fw_images[ROCKPI4_UPDATABLE_IMAGES] = {0};
>
> struct efi_capsule_update_info update_info = {
> + .num_images = ROCKPI4_UPDATABLE_IMAGES,
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ROCKPI4_UPDATABLE_IMAGES;
> #endif
>
> #ifndef CONFIG_SPL_BUILD
> diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
> index 2e44bdf0df..c7b6cb78ff 100644
> --- a/board/sandbox/sandbox.c
> +++ b/board/sandbox/sandbox.c
> @@ -67,10 +67,10 @@ struct efi_fw_image fw_images[] = {
> struct efi_capsule_update_info update_info = {
> .dfu_string = "sf 0:0=u-boot-bin raw 0x100000 0x50000;"
> "u-boot-env raw 0x150000 0x200000",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> #if !CONFIG_IS_ENABLED(OF_PLATDATA)
> diff --git a/board/socionext/developerbox/developerbox.c b/board/socionext/developerbox/developerbox.c
> index 16e14d4f7f..d92e1d0962 100644
> --- a/board/socionext/developerbox/developerbox.c
> +++ b/board/socionext/developerbox/developerbox.c
> @@ -41,10 +41,10 @@ struct efi_capsule_update_info update_info = {
> .dfu_string = "mtd nor1=u-boot.bin raw 200000 100000;"
> "fip.bin raw 180000 78000;"
> "optee.bin raw 500000 100000",
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> static struct mm_region sc2a11_mem_map[] = {
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 1a1b1844c8..5b28ccd32e 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -92,10 +92,10 @@
> struct efi_fw_image fw_images[1];
>
> struct efi_capsule_update_info update_info = {
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> int board_early_init_f(void)
> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
> index d071ebfb9c..0328d68e75 100644
> --- a/board/xilinx/common/board.c
> +++ b/board/xilinx/common/board.c
> @@ -52,10 +52,10 @@ struct efi_fw_image fw_images[] = {
> };
>
> struct efi_capsule_update_info update_info = {
> + .num_images = ARRAY_SIZE(fw_images),
> .images = fw_images,
> };
>
> -u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> #endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> #define EEPROM_HEADER_MAGIC 0xdaaddeed
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index b395eef9e7..941d63467c 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -1078,15 +1078,16 @@ struct efi_fw_image {
> * platforms which enable capsule updates
> *
> * @dfu_string: String used to populate dfu_alt_info
> + * @num_images: The number of images array entries
> * @images: Pointer to an array of updatable images
> */
> struct efi_capsule_update_info {
> const char *dfu_string;
> + int num_images;
> struct efi_fw_image *images;
> };
>
> extern struct efi_capsule_update_info update_info;
> -extern u8 num_image_type_guids;
>
> /**
> * Install the ESRT system table.
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 93e2b01c07..cc650e1443 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -131,7 +131,7 @@ static efi_status_t efi_fill_image_desc_array(
> struct efi_fw_image *fw_array;
> int i;
>
> - total_size = sizeof(*image_info) * num_image_type_guids;
> + total_size = sizeof(*image_info) * update_info.num_images;
>
> if (*image_info_size < total_size) {
> *image_info_size = total_size;
> @@ -141,13 +141,13 @@ static efi_status_t efi_fill_image_desc_array(
> *image_info_size = total_size;
>
> fw_array = update_info.images;
> - *descriptor_count = num_image_type_guids;
> + *descriptor_count = update_info.num_images;
> *descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> *descriptor_size = sizeof(*image_info);
> *package_version = 0xffffffff; /* not supported */
> *package_version_name = NULL; /* not supported */
>
> - for (i = 0; i < num_image_type_guids; i++) {
> + for (i = 0; i < update_info.num_images; i++) {
> image_info[i].image_index = fw_array[i].image_index;
> image_info[i].image_type_id = fw_array[i].image_type_id;
> image_info[i].image_id = fw_array[i].image_index;
> diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> index 5313d07302..3b1785e7b1 100644
> --- a/lib/fwu_updates/fwu.c
> +++ b/lib/fwu_updates/fwu.c
> @@ -151,7 +151,7 @@ static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id)
>
> index = *image_index;
> image = update_info.images;
> - for (i = 0; i < num_image_type_guids; i++) {
> + for (i = 0; i < update_info.num_images; i++) {
> if (index == image[i].image_index) {
> guidcpy(image_type_id, &image[i].image_type_id);
> return 0;
> --
> 2.17.1
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable
2023-05-19 10:32 [PATCH v6 0/8] FMP versioning support Masahisa Kojima
2023-05-19 10:32 ` [PATCH v6 1/8] efi_loader: add the number of image entries in efi_capsule_update_info Masahisa Kojima
@ 2023-05-19 10:32 ` Masahisa Kojima
2023-05-22 21:24 ` Ilias Apalodimas
2023-05-28 8:39 ` Heinrich Schuchardt
2023-05-19 10:32 ` [PATCH v6 3/8] efi_loader: versioning support in GetImageInfo Masahisa Kojima
` (7 subsequent siblings)
9 siblings, 2 replies; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-19 10:32 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Takahiro Akashi, Masahisa Kojima
Firmware version management is not implemented in the current
FMP protocol.
EDK II reference implementation capsule generation script inserts
the FMP Payload Header right before the payload, FMP Payload Header
contains the firmware version and lowest supported version.
This commit utilizes the FMP Payload Header, reads the header and
stores the firmware version into "FmpStateXXXX" EFI non-volatile variable.
XXXX indicates the image index, since FMP protocol handles multiple
image indexes.
Note that lowest supported version included in the FMP Payload Header
is not used. If the platform uses file-based EFI variable storage,
it can be tampered. The file-based EFI variable storage is not the
right place to store the lowest supported version for anti-rollback
protection.
This change is compatible with the existing FMP implementation.
This change does not mandate the FMP Payload Header.
If no FMP Payload Header is found in the capsule file, fw_version,
lowest supported version, last attempt version and last attempt
status is 0 and this is the same behavior as existing FMP
implementation.
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Changed in v6:
- only store the fw_version in the FmpState EFI variable
Changes in v4:
- move lines that are the same in both branches out of the if statement
- s/EDK2/EDK II/
- create print result function
- set last_attempt_version when capsule authentication failed
- use log_err() instead of printf()
Changes in v3:
- exclude CONFIG_FWU_MULTI_BANK_UPDATE case
- set image_type_id as a vendor field of FmpStateXXXX variable
- set READ_ONLY flag for FmpStateXXXX variable
- add error code for FIT image case
Changes in v2:
- modify indent
lib/efi_loader/efi_firmware.c | 161 ++++++++++++++++++++++++++++++----
1 file changed, 146 insertions(+), 15 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index cc650e1443..fc085e3c08 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -10,6 +10,7 @@
#include <charset.h>
#include <dfu.h>
#include <efi_loader.h>
+#include <efi_variable.h>
#include <fwu.h>
#include <image.h>
#include <signatures.h>
@@ -36,11 +37,52 @@ struct fmp_payload_header {
u32 lowest_supported_version;
};
+/**
+ * struct fmp_state - fmp firmware update state
+ *
+ * This structure describes the state of the firmware update
+ * through FMP protocol.
+ *
+ * @fw_version: Firmware versions used
+ * @lowest_supported_version: Lowest supported version
+ * @last_attempt_version: Last attempt version
+ * @last_attempt_status: Last attempt status
+ */
+struct fmp_state {
+ u32 fw_version;
+ u32 lowest_supported_version; /* not used */
+ u32 last_attempt_version; /* not used */
+ u32 last_attempt_status; /* not used */
+};
+
__weak void set_dfu_alt_info(char *interface, char *devstr)
{
env_set("dfu_alt_info", update_info.dfu_string);
}
+/**
+ * efi_firmware_get_image_type_id - get image_type_id
+ * @image_index: image index
+ *
+ * Return the image_type_id identified by the image index.
+ *
+ * Return: pointer to the image_type_id, NULL if image_index is invalid
+ */
+static
+efi_guid_t *efi_firmware_get_image_type_id(u8 image_index)
+{
+ int i;
+ struct efi_fw_image *fw_array;
+
+ fw_array = update_info.images;
+ for (i = 0; i < update_info.num_images; i++) {
+ if (fw_array[i].image_index == image_index)
+ return &fw_array[i].image_type_id;
+ }
+
+ return NULL;
+}
+
/* Place holder; not supported */
static
efi_status_t EFIAPI efi_firmware_get_image_unsupported(
@@ -194,8 +236,6 @@ efi_status_t efi_firmware_capsule_authenticate(const void **p_image,
{
const void *image = *p_image;
efi_uintn_t image_size = *p_image_size;
- u32 fmp_hdr_signature;
- struct fmp_payload_header *header;
void *capsule_payload;
efi_status_t status;
efi_uintn_t capsule_payload_size;
@@ -222,24 +262,107 @@ efi_status_t efi_firmware_capsule_authenticate(const void **p_image,
debug("Updating capsule without authenticating.\n");
}
- fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
- header = (void *)image;
+ *p_image = image;
+ *p_image_size = image_size;
+ return EFI_SUCCESS;
+}
+
+/**
+ * efi_firmware_set_fmp_state_var - set FmpStateXXXX variable
+ * @state: Pointer to fmp state
+ * @image_index: image index
+ *
+ * Update the FmpStateXXXX variable with the firmware update state.
+ *
+ * Return: status code
+ */
+static
+efi_status_t efi_firmware_set_fmp_state_var(struct fmp_state *state, u8 image_index)
+{
+ u16 varname[13]; /* u"FmpStateXXXX" */
+ efi_status_t ret;
+ efi_guid_t *image_type_id;
+ struct fmp_state var_state = { 0 };
+
+ image_type_id = efi_firmware_get_image_type_id(image_index);
+ if (!image_type_id)
+ return EFI_INVALID_PARAMETER;
+
+ efi_create_indexed_name(varname, sizeof(varname), "FmpState",
+ image_index);
+
+ /*
+ * Only the fw_version is set here.
+ * lowest_supported_version in FmpState variable is ignored since
+ * it can be tampered if the file based EFI variable storage is used.
+ */
+ var_state.fw_version = state->fw_version;
+
+ ret = efi_set_variable_int(varname, image_type_id,
+ EFI_VARIABLE_READ_ONLY |
+ EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS,
+ sizeof(var_state), &var_state, false);
+
+ return ret;
+}
+
+/**
+ * efi_firmware_get_fw_version - get fw_version from FMP payload header
+ * @p_image: Pointer to new image
+ * @p_image_size: Pointer to size of new image
+ * @state Pointer to fmp state
+ *
+ * Parse the FMP payload header and fill the fmp_state structure.
+ * If no FMP payload header is found, fmp_state structure is not updated.
+ *
+ */
+static void efi_firmware_get_fw_version(const void **p_image,
+ efi_uintn_t *p_image_size,
+ struct fmp_state *state)
+{
+ const void *image = *p_image;
+ efi_uintn_t image_size = *p_image_size;
+ const struct fmp_payload_header *header;
+ u32 fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
+
+ header = image;
+ if (header->signature == fmp_hdr_signature) {
+ /* FMP header is inserted above the capsule payload */
+ state->fw_version = header->fw_version;
- if (!memcmp(&header->signature, &fmp_hdr_signature,
- sizeof(fmp_hdr_signature))) {
- /*
- * When building the capsule with the scripts in
- * edk2, a FMP header is inserted above the capsule
- * payload. Compensate for this header to get the
- * actual payload that is to be updated.
- */
image += header->header_size;
image_size -= header->header_size;
}
*p_image = image;
*p_image_size = image_size;
- return EFI_SUCCESS;
+}
+
+/**
+ * efi_firmware_verify_image - verify image
+ * @p_image: Pointer to new image
+ * @p_image_size: Pointer to size of new image
+ * @image_index Image index
+ * @state Pointer to fmp state
+ *
+ * Verify the capsule file
+ *
+ * Return: status code
+ */
+static
+efi_status_t efi_firmware_verify_image(const void **p_image,
+ efi_uintn_t *p_image_size,
+ u8 image_index,
+ struct fmp_state *state)
+{
+ efi_status_t ret;
+
+ ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
+ efi_firmware_get_fw_version(p_image, p_image_size, state);
+
+ return ret;
}
/**
@@ -331,6 +454,7 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
u16 **abort_reason)
{
efi_status_t status;
+ struct fmp_state state = { 0 };
EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
image_size, vendor_code, progress, abort_reason);
@@ -338,13 +462,16 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
if (!image || image_index != 1)
return EFI_EXIT(EFI_INVALID_PARAMETER);
- status = efi_firmware_capsule_authenticate(&image, &image_size);
+ status = efi_firmware_verify_image(&image, &image_size, image_index,
+ &state);
if (status != EFI_SUCCESS)
return EFI_EXIT(status);
if (fit_update(image))
return EFI_EXIT(EFI_DEVICE_ERROR);
+ efi_firmware_set_fmp_state_var(&state, image_index);
+
return EFI_EXIT(EFI_SUCCESS);
}
@@ -392,6 +519,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
{
int ret;
efi_status_t status;
+ struct fmp_state state = { 0 };
EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
image_size, vendor_code, progress, abort_reason);
@@ -399,7 +527,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
if (!image)
return EFI_EXIT(EFI_INVALID_PARAMETER);
- status = efi_firmware_capsule_authenticate(&image, &image_size);
+ status = efi_firmware_verify_image(&image, &image_size, image_index,
+ &state);
if (status != EFI_SUCCESS)
return EFI_EXIT(status);
@@ -419,6 +548,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
NULL, NULL))
return EFI_EXIT(EFI_DEVICE_ERROR);
+ efi_firmware_set_fmp_state_var(&state, image_index);
+
return EFI_EXIT(EFI_SUCCESS);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable
2023-05-19 10:32 ` [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable Masahisa Kojima
@ 2023-05-22 21:24 ` Ilias Apalodimas
2023-05-23 1:55 ` Masahisa Kojima
2023-05-28 8:39 ` Heinrich Schuchardt
1 sibling, 1 reply; 27+ messages in thread
From: Ilias Apalodimas @ 2023-05-22 21:24 UTC (permalink / raw)
To: Masahisa Kojima; +Cc: u-boot, Heinrich Schuchardt, Simon Glass, Takahiro Akashi
Hi Kojima-san,
On Fri, May 19, 2023 at 07:32:08PM +0900, Masahisa Kojima wrote:
> Firmware version management is not implemented in the current
> FMP protocol.
> EDK II reference implementation capsule generation script inserts
> the FMP Payload Header right before the payload, FMP Payload Header
> contains the firmware version and lowest supported version.
>
> This commit utilizes the FMP Payload Header, reads the header and
> stores the firmware version into "FmpStateXXXX" EFI non-volatile variable.
> XXXX indicates the image index, since FMP protocol handles multiple
> image indexes.
> Note that lowest supported version included in the FMP Payload Header
> is not used. If the platform uses file-based EFI variable storage,
> it can be tampered. The file-based EFI variable storage is not the
> right place to store the lowest supported version for anti-rollback
> protection.
>
> This change is compatible with the existing FMP implementation.
> This change does not mandate the FMP Payload Header.
> If no FMP Payload Header is found in the capsule file, fw_version,
> lowest supported version, last attempt version and last attempt
> status is 0 and this is the same behavior as existing FMP
> implementation.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Changed in v6:
> - only store the fw_version in the FmpState EFI variable
>
> Changes in v4:
> - move lines that are the same in both branches out of the if statement
> - s/EDK2/EDK II/
> - create print result function
> - set last_attempt_version when capsule authentication failed
> - use log_err() instead of printf()
>
> Changes in v3:
> - exclude CONFIG_FWU_MULTI_BANK_UPDATE case
> - set image_type_id as a vendor field of FmpStateXXXX variable
> - set READ_ONLY flag for FmpStateXXXX variable
> - add error code for FIT image case
>
> Changes in v2:
> - modify indent
>
[...]
> +/**
> + * efi_firmware_get_fw_version - get fw_version from FMP payload header
> + * @p_image: Pointer to new image
> + * @p_image_size: Pointer to size of new image
> + * @state Pointer to fmp state
> + *
> + * Parse the FMP payload header and fill the fmp_state structure.
> + * If no FMP payload header is found, fmp_state structure is not updated.
> + *
> + */
> +static void efi_firmware_get_fw_version(const void **p_image,
> + efi_uintn_t *p_image_size,
> + struct fmp_state *state)
> +{
> + const void *image = *p_image;
> + efi_uintn_t image_size = *p_image_size;
> + const struct fmp_payload_header *header;
> + u32 fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
> +
> + header = image;
> + if (header->signature == fmp_hdr_signature) {
> + /* FMP header is inserted above the capsule payload */
> + state->fw_version = header->fw_version;
>
> - if (!memcmp(&header->signature, &fmp_hdr_signature,
> - sizeof(fmp_hdr_signature))) {
> - /*
> - * When building the capsule with the scripts in
> - * edk2, a FMP header is inserted above the capsule
> - * payload. Compensate for this header to get the
> - * actual payload that is to be updated.
> - */
> image += header->header_size;
> image_size -= header->header_size;
> }
>
> *p_image = image;
> *p_image_size = image_size;
Can we get rid of the extra image/image_size here and move this inside the
if()?
> - return EFI_SUCCESS;
> +}
> +
> +/**
> + * efi_firmware_verify_image - verify image
The name is a bit generic here, we need something which describes what
happens better. The verification already happens in
efi_firmware_capsule_authenticate().
Maybe efi_prepare_capsule() or something like that ?
> + * @p_image: Pointer to new image
> + * @p_image_size: Pointer to size of new image
> + * @image_index Image index
> + * @state Pointer to fmp state
> + *
> + * Verify the capsule file
> + *
> + * Return: status code
> + */
> +static
> +efi_status_t efi_firmware_verify_image(const void **p_image,
> + efi_uintn_t *p_image_size,
> + u8 image_index,
> + struct fmp_state *state)
> +{
> + efi_status_t ret;
> +
> + ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
> + efi_firmware_get_fw_version(p_image, p_image_size, state);
> +
> + return ret;
> }
>
> /**
> @@ -331,6 +454,7 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
> u16 **abort_reason)
> {
> efi_status_t status;
> + struct fmp_state state = { 0 };
>
> EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
> image_size, vendor_code, progress, abort_reason);
> @@ -338,13 +462,16 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
> if (!image || image_index != 1)
> return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> - status = efi_firmware_capsule_authenticate(&image, &image_size);
> + status = efi_firmware_verify_image(&image, &image_size, image_index,
> + &state);
> if (status != EFI_SUCCESS)
> return EFI_EXIT(status);
>
> if (fit_update(image))
> return EFI_EXIT(EFI_DEVICE_ERROR);
>
> + efi_firmware_set_fmp_state_var(&state, image_index);
> +
> return EFI_EXIT(EFI_SUCCESS);
> }
>
> @@ -392,6 +519,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> {
> int ret;
> efi_status_t status;
> + struct fmp_state state = { 0 };
>
> EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
> image_size, vendor_code, progress, abort_reason);
> @@ -399,7 +527,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> if (!image)
> return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> - status = efi_firmware_capsule_authenticate(&image, &image_size);
> + status = efi_firmware_verify_image(&image, &image_size, image_index,
> + &state);
> if (status != EFI_SUCCESS)
> return EFI_EXIT(status);
>
> @@ -419,6 +548,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> NULL, NULL))
> return EFI_EXIT(EFI_DEVICE_ERROR);
>
> + efi_firmware_set_fmp_state_var(&state, image_index);
> +
> return EFI_EXIT(EFI_SUCCESS);
> }
>
> --
> 2.17.1
>
Thanks
/Ilias
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable
2023-05-22 21:24 ` Ilias Apalodimas
@ 2023-05-23 1:55 ` Masahisa Kojima
0 siblings, 0 replies; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-23 1:55 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: u-boot, Heinrich Schuchardt, Simon Glass, Takahiro Akashi
On Tue, 23 May 2023 at 06:24, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Kojima-san,
>
> On Fri, May 19, 2023 at 07:32:08PM +0900, Masahisa Kojima wrote:
> > Firmware version management is not implemented in the current
> > FMP protocol.
> > EDK II reference implementation capsule generation script inserts
> > the FMP Payload Header right before the payload, FMP Payload Header
> > contains the firmware version and lowest supported version.
> >
> > This commit utilizes the FMP Payload Header, reads the header and
> > stores the firmware version into "FmpStateXXXX" EFI non-volatile variable.
> > XXXX indicates the image index, since FMP protocol handles multiple
> > image indexes.
> > Note that lowest supported version included in the FMP Payload Header
> > is not used. If the platform uses file-based EFI variable storage,
> > it can be tampered. The file-based EFI variable storage is not the
> > right place to store the lowest supported version for anti-rollback
> > protection.
> >
> > This change is compatible with the existing FMP implementation.
> > This change does not mandate the FMP Payload Header.
> > If no FMP Payload Header is found in the capsule file, fw_version,
> > lowest supported version, last attempt version and last attempt
> > status is 0 and this is the same behavior as existing FMP
> > implementation.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> > Changed in v6:
> > - only store the fw_version in the FmpState EFI variable
> >
> > Changes in v4:
> > - move lines that are the same in both branches out of the if statement
> > - s/EDK2/EDK II/
> > - create print result function
> > - set last_attempt_version when capsule authentication failed
> > - use log_err() instead of printf()
> >
> > Changes in v3:
> > - exclude CONFIG_FWU_MULTI_BANK_UPDATE case
> > - set image_type_id as a vendor field of FmpStateXXXX variable
> > - set READ_ONLY flag for FmpStateXXXX variable
> > - add error code for FIT image case
> >
> > Changes in v2:
> > - modify indent
> >
>
> [...]
>
> > +/**
> > + * efi_firmware_get_fw_version - get fw_version from FMP payload header
> > + * @p_image: Pointer to new image
> > + * @p_image_size: Pointer to size of new image
> > + * @state Pointer to fmp state
> > + *
> > + * Parse the FMP payload header and fill the fmp_state structure.
> > + * If no FMP payload header is found, fmp_state structure is not updated.
> > + *
> > + */
> > +static void efi_firmware_get_fw_version(const void **p_image,
> > + efi_uintn_t *p_image_size,
> > + struct fmp_state *state)
> > +{
> > + const void *image = *p_image;
> > + efi_uintn_t image_size = *p_image_size;
> > + const struct fmp_payload_header *header;
> > + u32 fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
> > +
> > + header = image;
> > + if (header->signature == fmp_hdr_signature) {
> > + /* FMP header is inserted above the capsule payload */
> > + state->fw_version = header->fw_version;
> >
> > - if (!memcmp(&header->signature, &fmp_hdr_signature,
> > - sizeof(fmp_hdr_signature))) {
> > - /*
> > - * When building the capsule with the scripts in
> > - * edk2, a FMP header is inserted above the capsule
> > - * payload. Compensate for this header to get the
> > - * actual payload that is to be updated.
> > - */
> > image += header->header_size;
> > image_size -= header->header_size;
> > }
>
> >
> > *p_image = image;
> > *p_image_size = image_size;
>
> Can we get rid of the extra image/image_size here and move this inside the
> if()?
Yes, thanks.
>
> > - return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + * efi_firmware_verify_image - verify image
>
> The name is a bit generic here, we need something which describes what
> happens better. The verification already happens in
> efi_firmware_capsule_authenticate().
In the "[PATCH v6 5/8] efi_loader: check lowest supported version" in
this series,
checking lowest_supported_version is added in efi_firmware_verify_image().
So, can we keep this function name?
Regards,
Masahisa Kojima
> Maybe efi_prepare_capsule() or something like that ?
>
> > + * @p_image: Pointer to new image
> > + * @p_image_size: Pointer to size of new image
> > + * @image_index Image index
> > + * @state Pointer to fmp state
> > + *
> > + * Verify the capsule file
> > + *
> > + * Return: status code
> > + */
> > +static
> > +efi_status_t efi_firmware_verify_image(const void **p_image,
> > + efi_uintn_t *p_image_size,
> > + u8 image_index,
> > + struct fmp_state *state)
> > +{
> > + efi_status_t ret;
> > +
> > + ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
> > + efi_firmware_get_fw_version(p_image, p_image_size, state);
> > +
> > + return ret;
> > }
> >
> > /**
> > @@ -331,6 +454,7 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
> > u16 **abort_reason)
> > {
> > efi_status_t status;
> > + struct fmp_state state = { 0 };
> >
> > EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
> > image_size, vendor_code, progress, abort_reason);
> > @@ -338,13 +462,16 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
> > if (!image || image_index != 1)
> > return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > - status = efi_firmware_capsule_authenticate(&image, &image_size);
> > + status = efi_firmware_verify_image(&image, &image_size, image_index,
> > + &state);
> > if (status != EFI_SUCCESS)
> > return EFI_EXIT(status);
> >
> > if (fit_update(image))
> > return EFI_EXIT(EFI_DEVICE_ERROR);
> >
> > + efi_firmware_set_fmp_state_var(&state, image_index);
> > +
> > return EFI_EXIT(EFI_SUCCESS);
> > }
> >
> > @@ -392,6 +519,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> > {
> > int ret;
> > efi_status_t status;
> > + struct fmp_state state = { 0 };
> >
> > EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
> > image_size, vendor_code, progress, abort_reason);
> > @@ -399,7 +527,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> > if (!image)
> > return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > - status = efi_firmware_capsule_authenticate(&image, &image_size);
> > + status = efi_firmware_verify_image(&image, &image_size, image_index,
> > + &state);
> > if (status != EFI_SUCCESS)
> > return EFI_EXIT(status);
> >
> > @@ -419,6 +548,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> > NULL, NULL))
> > return EFI_EXIT(EFI_DEVICE_ERROR);
> >
> > + efi_firmware_set_fmp_state_var(&state, image_index);
> > +
> > return EFI_EXIT(EFI_SUCCESS);
> > }
> >
> > --
> > 2.17.1
> >
>
> Thanks
> /Ilias
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable
2023-05-19 10:32 ` [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable Masahisa Kojima
2023-05-22 21:24 ` Ilias Apalodimas
@ 2023-05-28 8:39 ` Heinrich Schuchardt
2023-05-30 0:31 ` Masahisa Kojima
1 sibling, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2023-05-28 8:39 UTC (permalink / raw)
To: Masahisa Kojima; +Cc: Ilias Apalodimas, Simon Glass, Takahiro Akashi, u-boot
On 5/19/23 12:32, Masahisa Kojima wrote:
> Firmware version management is not implemented in the current
> FMP protocol.
> EDK II reference implementation capsule generation script inserts
> the FMP Payload Header right before the payload, FMP Payload Header
> contains the firmware version and lowest supported version.
>
> This commit utilizes the FMP Payload Header, reads the header and
> stores the firmware version into "FmpStateXXXX" EFI non-volatile variable.
> XXXX indicates the image index, since FMP protocol handles multiple
> image indexes.
> Note that lowest supported version included in the FMP Payload Header
> is not used. If the platform uses file-based EFI variable storage,
> it can be tampered. The file-based EFI variable storage is not the
> right place to store the lowest supported version for anti-rollback
> protection.
>
> This change is compatible with the existing FMP implementation.
> This change does not mandate the FMP Payload Header.
> If no FMP Payload Header is found in the capsule file, fw_version,
> lowest supported version, last attempt version and last attempt
> status is 0 and this is the same behavior as existing FMP
> implementation.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Changed in v6:
> - only store the fw_version in the FmpState EFI variable
>
> Changes in v4:
> - move lines that are the same in both branches out of the if statement
> - s/EDK2/EDK II/
> - create print result function
> - set last_attempt_version when capsule authentication failed
> - use log_err() instead of printf()
>
> Changes in v3:
> - exclude CONFIG_FWU_MULTI_BANK_UPDATE case
> - set image_type_id as a vendor field of FmpStateXXXX variable
> - set READ_ONLY flag for FmpStateXXXX variable
> - add error code for FIT image case
>
> Changes in v2:
> - modify indent
>
> lib/efi_loader/efi_firmware.c | 161 ++++++++++++++++++++++++++++++----
> 1 file changed, 146 insertions(+), 15 deletions(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index cc650e1443..fc085e3c08 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -10,6 +10,7 @@
> #include <charset.h>
> #include <dfu.h>
> #include <efi_loader.h>
> +#include <efi_variable.h>
> #include <fwu.h>
> #include <image.h>
> #include <signatures.h>
> @@ -36,11 +37,52 @@ struct fmp_payload_header {
> u32 lowest_supported_version;
> };
>
> +/**
> + * struct fmp_state - fmp firmware update state
> + *
> + * This structure describes the state of the firmware update
> + * through FMP protocol.
> + *
> + * @fw_version: Firmware versions used
> + * @lowest_supported_version: Lowest supported version
> + * @last_attempt_version: Last attempt version
> + * @last_attempt_status: Last attempt status
> + */
> +struct fmp_state {
> + u32 fw_version;
> + u32 lowest_supported_version; /* not used */
> + u32 last_attempt_version; /* not used */
> + u32 last_attempt_status; /* not used */
> +};
> +
> __weak void set_dfu_alt_info(char *interface, char *devstr)
> {
> env_set("dfu_alt_info", update_info.dfu_string);
> }
>
> +/**
> + * efi_firmware_get_image_type_id - get image_type_id
> + * @image_index: image index
> + *
> + * Return the image_type_id identified by the image index.
> + *
> + * Return: pointer to the image_type_id, NULL if image_index is invalid
> + */
> +static
> +efi_guid_t *efi_firmware_get_image_type_id(u8 image_index)
> +{
> + int i;
> + struct efi_fw_image *fw_array;
> +
> + fw_array = update_info.images;
> + for (i = 0; i < update_info.num_images; i++) {
> + if (fw_array[i].image_index == image_index)
> + return &fw_array[i].image_type_id;
> + }
> +
> + return NULL;
> +}
> +
> /* Place holder; not supported */
> static
> efi_status_t EFIAPI efi_firmware_get_image_unsupported(
> @@ -194,8 +236,6 @@ efi_status_t efi_firmware_capsule_authenticate(const void **p_image,
> {
> const void *image = *p_image;
> efi_uintn_t image_size = *p_image_size;
> - u32 fmp_hdr_signature;
> - struct fmp_payload_header *header;
> void *capsule_payload;
> efi_status_t status;
> efi_uintn_t capsule_payload_size;
> @@ -222,24 +262,107 @@ efi_status_t efi_firmware_capsule_authenticate(const void **p_image,
> debug("Updating capsule without authenticating.\n");
> }
>
> - fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
> - header = (void *)image;
> + *p_image = image;
> + *p_image_size = image_size;
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + * efi_firmware_set_fmp_state_var - set FmpStateXXXX variable
> + * @state: Pointer to fmp state
> + * @image_index: image index
> + *
> + * Update the FmpStateXXXX variable with the firmware update state.
> + *
> + * Return: status code
> + */
> +static
> +efi_status_t efi_firmware_set_fmp_state_var(struct fmp_state *state, u8 image_index)
> +{
> + u16 varname[13]; /* u"FmpStateXXXX" */
> + efi_status_t ret;
> + efi_guid_t *image_type_id;
> + struct fmp_state var_state = { 0 };
> +
> + image_type_id = efi_firmware_get_image_type_id(image_index);
> + if (!image_type_id)
> + return EFI_INVALID_PARAMETER;
> +
> + efi_create_indexed_name(varname, sizeof(varname), "FmpState",
> + image_index);
> +
> + /*
> + * Only the fw_version is set here.
> + * lowest_supported_version in FmpState variable is ignored since
> + * it can be tampered if the file based EFI variable storage is used.
> + */
> + var_state.fw_version = state->fw_version;
> +
> + ret = efi_set_variable_int(varname, image_type_id,
> + EFI_VARIABLE_READ_ONLY |
> + EFI_VARIABLE_NON_VOLATILE |
> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS,
> + sizeof(var_state), &var_state, false);
> +
> + return ret;
> +}
> +
> +/**
> + * efi_firmware_get_fw_version - get fw_version from FMP payload header
> + * @p_image: Pointer to new image
> + * @p_image_size: Pointer to size of new image
> + * @state Pointer to fmp state
Due to the missing colon this leads to a build failure for 'make htmldocs':
./lib/efi_loader/efi_firmware.c:401: warning: Function parameter or
member 'state' not described in 'efi_firmware_get_fw_version'
> + *
> + * Parse the FMP payload header and fill the fmp_state structure.
> + * If no FMP payload header is found, fmp_state structure is not updated.
> + *
> + */
> +static void efi_firmware_get_fw_version(const void **p_image,
> + efi_uintn_t *p_image_size,
> + struct fmp_state *state)
> +{
> + const void *image = *p_image;
> + efi_uintn_t image_size = *p_image_size;
> + const struct fmp_payload_header *header;
> + u32 fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
> +
> + header = image;
> + if (header->signature == fmp_hdr_signature) {
> + /* FMP header is inserted above the capsule payload */
> + state->fw_version = header->fw_version;
>
> - if (!memcmp(&header->signature, &fmp_hdr_signature,
> - sizeof(fmp_hdr_signature))) {
> - /*
> - * When building the capsule with the scripts in
> - * edk2, a FMP header is inserted above the capsule
> - * payload. Compensate for this header to get the
> - * actual payload that is to be updated.
> - */
> image += header->header_size;
> image_size -= header->header_size;
> }
>
> *p_image = image;
> *p_image_size = image_size;
> - return EFI_SUCCESS;
> +}
> +
> +/**
> + * efi_firmware_verify_image - verify image
> + * @p_image: Pointer to new image
> + * @p_image_size: Pointer to size of new image
> + * @image_index Image index
> + * @state Pointer to fmp state
Due to the missing colons this leads to a build failure for 'make htmldocs':
./lib/efi_loader/efi_firmware.c:437: warning: Function parameter or
member 'image_index' not described in 'efi_firmware_verify_image'
./lib/efi_loader/efi_firmware.c:437: warning: Function parameter or
member 'state' not described in 'efi_firmware_verify_image'
Best regards
Heinrich
> + *
> + * Verify the capsule file
> + *
> + * Return: status code
> + */
> +static
> +efi_status_t efi_firmware_verify_image(const void **p_image,
> + efi_uintn_t *p_image_size,
> + u8 image_index,
> + struct fmp_state *state)
> +{
> + efi_status_t ret;
> +
> + ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
> + efi_firmware_get_fw_version(p_image, p_image_size, state);
> +
> + return ret;
> }
>
> /**
> @@ -331,6 +454,7 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
> u16 **abort_reason)
> {
> efi_status_t status;
> + struct fmp_state state = { 0 };
>
> EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
> image_size, vendor_code, progress, abort_reason);
> @@ -338,13 +462,16 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
> if (!image || image_index != 1)
> return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> - status = efi_firmware_capsule_authenticate(&image, &image_size);
> + status = efi_firmware_verify_image(&image, &image_size, image_index,
> + &state);
> if (status != EFI_SUCCESS)
> return EFI_EXIT(status);
>
> if (fit_update(image))
> return EFI_EXIT(EFI_DEVICE_ERROR);
>
> + efi_firmware_set_fmp_state_var(&state, image_index);
> +
> return EFI_EXIT(EFI_SUCCESS);
> }
>
> @@ -392,6 +519,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> {
> int ret;
> efi_status_t status;
> + struct fmp_state state = { 0 };
>
> EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
> image_size, vendor_code, progress, abort_reason);
> @@ -399,7 +527,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> if (!image)
> return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> - status = efi_firmware_capsule_authenticate(&image, &image_size);
> + status = efi_firmware_verify_image(&image, &image_size, image_index,
> + &state);
> if (status != EFI_SUCCESS)
> return EFI_EXIT(status);
>
> @@ -419,6 +548,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> NULL, NULL))
> return EFI_EXIT(EFI_DEVICE_ERROR);
>
> + efi_firmware_set_fmp_state_var(&state, image_index);
> +
> return EFI_EXIT(EFI_SUCCESS);
> }
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable
2023-05-28 8:39 ` Heinrich Schuchardt
@ 2023-05-30 0:31 ` Masahisa Kojima
0 siblings, 0 replies; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-30 0:31 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Ilias Apalodimas, Simon Glass, Takahiro Akashi, u-boot
Hi Heinrich,
On Sun, 28 May 2023 at 17:39, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 5/19/23 12:32, Masahisa Kojima wrote:
> > Firmware version management is not implemented in the current
> > FMP protocol.
> > EDK II reference implementation capsule generation script inserts
> > the FMP Payload Header right before the payload, FMP Payload Header
> > contains the firmware version and lowest supported version.
> >
> > This commit utilizes the FMP Payload Header, reads the header and
> > stores the firmware version into "FmpStateXXXX" EFI non-volatile variable.
> > XXXX indicates the image index, since FMP protocol handles multiple
> > image indexes.
> > Note that lowest supported version included in the FMP Payload Header
> > is not used. If the platform uses file-based EFI variable storage,
> > it can be tampered. The file-based EFI variable storage is not the
> > right place to store the lowest supported version for anti-rollback
> > protection.
> >
> > This change is compatible with the existing FMP implementation.
> > This change does not mandate the FMP Payload Header.
> > If no FMP Payload Header is found in the capsule file, fw_version,
> > lowest supported version, last attempt version and last attempt
> > status is 0 and this is the same behavior as existing FMP
> > implementation.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> > Changed in v6:
> > - only store the fw_version in the FmpState EFI variable
> >
> > Changes in v4:
> > - move lines that are the same in both branches out of the if statement
> > - s/EDK2/EDK II/
> > - create print result function
> > - set last_attempt_version when capsule authentication failed
> > - use log_err() instead of printf()
> >
> > Changes in v3:
> > - exclude CONFIG_FWU_MULTI_BANK_UPDATE case
> > - set image_type_id as a vendor field of FmpStateXXXX variable
> > - set READ_ONLY flag for FmpStateXXXX variable
> > - add error code for FIT image case
> >
> > Changes in v2:
> > - modify indent
> >
> > lib/efi_loader/efi_firmware.c | 161 ++++++++++++++++++++++++++++++----
> > 1 file changed, 146 insertions(+), 15 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> > index cc650e1443..fc085e3c08 100644
> > --- a/lib/efi_loader/efi_firmware.c
> > +++ b/lib/efi_loader/efi_firmware.c
> > @@ -10,6 +10,7 @@
> > #include <charset.h>
> > #include <dfu.h>
> > #include <efi_loader.h>
> > +#include <efi_variable.h>
> > #include <fwu.h>
> > #include <image.h>
> > #include <signatures.h>
> > @@ -36,11 +37,52 @@ struct fmp_payload_header {
> > u32 lowest_supported_version;
> > };
> >
> > +/**
> > + * struct fmp_state - fmp firmware update state
> > + *
> > + * This structure describes the state of the firmware update
> > + * through FMP protocol.
> > + *
> > + * @fw_version: Firmware versions used
> > + * @lowest_supported_version: Lowest supported version
> > + * @last_attempt_version: Last attempt version
> > + * @last_attempt_status: Last attempt status
> > + */
> > +struct fmp_state {
> > + u32 fw_version;
> > + u32 lowest_supported_version; /* not used */
> > + u32 last_attempt_version; /* not used */
> > + u32 last_attempt_status; /* not used */
> > +};
> > +
> > __weak void set_dfu_alt_info(char *interface, char *devstr)
> > {
> > env_set("dfu_alt_info", update_info.dfu_string);
> > }
> >
> > +/**
> > + * efi_firmware_get_image_type_id - get image_type_id
> > + * @image_index: image index
> > + *
> > + * Return the image_type_id identified by the image index.
> > + *
> > + * Return: pointer to the image_type_id, NULL if image_index is invalid
> > + */
> > +static
> > +efi_guid_t *efi_firmware_get_image_type_id(u8 image_index)
> > +{
> > + int i;
> > + struct efi_fw_image *fw_array;
> > +
> > + fw_array = update_info.images;
> > + for (i = 0; i < update_info.num_images; i++) {
> > + if (fw_array[i].image_index == image_index)
> > + return &fw_array[i].image_type_id;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > /* Place holder; not supported */
> > static
> > efi_status_t EFIAPI efi_firmware_get_image_unsupported(
> > @@ -194,8 +236,6 @@ efi_status_t efi_firmware_capsule_authenticate(const void **p_image,
> > {
> > const void *image = *p_image;
> > efi_uintn_t image_size = *p_image_size;
> > - u32 fmp_hdr_signature;
> > - struct fmp_payload_header *header;
> > void *capsule_payload;
> > efi_status_t status;
> > efi_uintn_t capsule_payload_size;
> > @@ -222,24 +262,107 @@ efi_status_t efi_firmware_capsule_authenticate(const void **p_image,
> > debug("Updating capsule without authenticating.\n");
> > }
> >
> > - fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
> > - header = (void *)image;
> > + *p_image = image;
> > + *p_image_size = image_size;
> > + return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + * efi_firmware_set_fmp_state_var - set FmpStateXXXX variable
> > + * @state: Pointer to fmp state
> > + * @image_index: image index
> > + *
> > + * Update the FmpStateXXXX variable with the firmware update state.
> > + *
> > + * Return: status code
> > + */
> > +static
> > +efi_status_t efi_firmware_set_fmp_state_var(struct fmp_state *state, u8 image_index)
> > +{
> > + u16 varname[13]; /* u"FmpStateXXXX" */
> > + efi_status_t ret;
> > + efi_guid_t *image_type_id;
> > + struct fmp_state var_state = { 0 };
> > +
> > + image_type_id = efi_firmware_get_image_type_id(image_index);
> > + if (!image_type_id)
> > + return EFI_INVALID_PARAMETER;
> > +
> > + efi_create_indexed_name(varname, sizeof(varname), "FmpState",
> > + image_index);
> > +
> > + /*
> > + * Only the fw_version is set here.
> > + * lowest_supported_version in FmpState variable is ignored since
> > + * it can be tampered if the file based EFI variable storage is used.
> > + */
> > + var_state.fw_version = state->fw_version;
> > +
> > + ret = efi_set_variable_int(varname, image_type_id,
> > + EFI_VARIABLE_READ_ONLY |
> > + EFI_VARIABLE_NON_VOLATILE |
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS,
> > + sizeof(var_state), &var_state, false);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * efi_firmware_get_fw_version - get fw_version from FMP payload header
> > + * @p_image: Pointer to new image
> > + * @p_image_size: Pointer to size of new image
> > + * @state Pointer to fmp state
>
> Due to the missing colon this leads to a build failure for 'make htmldocs':
>
> ./lib/efi_loader/efi_firmware.c:401: warning: Function parameter or
> member 'state' not described in 'efi_firmware_get_fw_version'
>
>
> > + *
> > + * Parse the FMP payload header and fill the fmp_state structure.
> > + * If no FMP payload header is found, fmp_state structure is not updated.
> > + *
> > + */
> > +static void efi_firmware_get_fw_version(const void **p_image,
> > + efi_uintn_t *p_image_size,
> > + struct fmp_state *state)
> > +{
> > + const void *image = *p_image;
> > + efi_uintn_t image_size = *p_image_size;
> > + const struct fmp_payload_header *header;
> > + u32 fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
> > +
> > + header = image;
> > + if (header->signature == fmp_hdr_signature) {
> > + /* FMP header is inserted above the capsule payload */
> > + state->fw_version = header->fw_version;
> >
> > - if (!memcmp(&header->signature, &fmp_hdr_signature,
> > - sizeof(fmp_hdr_signature))) {
> > - /*
> > - * When building the capsule with the scripts in
> > - * edk2, a FMP header is inserted above the capsule
> > - * payload. Compensate for this header to get the
> > - * actual payload that is to be updated.
> > - */
> > image += header->header_size;
> > image_size -= header->header_size;
> > }
> >
> > *p_image = image;
> > *p_image_size = image_size;
> > - return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + * efi_firmware_verify_image - verify image
> > + * @p_image: Pointer to new image
> > + * @p_image_size: Pointer to size of new image
> > + * @image_index Image index
> > + * @state Pointer to fmp state
>
> Due to the missing colons this leads to a build failure for 'make htmldocs':
>
> ./lib/efi_loader/efi_firmware.c:437: warning: Function parameter or
> member 'image_index' not described in 'efi_firmware_verify_image'
> ./lib/efi_loader/efi_firmware.c:437: warning: Function parameter or
> member 'state' not described in 'efi_firmware_verify_image'
Sorry, I will fix it in the next version.
Thanks,
Masahisa Kojima
>
> Best regards
>
> Heinrich
>
> > + *
> > + * Verify the capsule file
> > + *
> > + * Return: status code
> > + */
> > +static
> > +efi_status_t efi_firmware_verify_image(const void **p_image,
> > + efi_uintn_t *p_image_size,
> > + u8 image_index,
> > + struct fmp_state *state)
> > +{
> > + efi_status_t ret;
> > +
> > + ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
> > + efi_firmware_get_fw_version(p_image, p_image_size, state);
> > +
> > + return ret;
> > }
> >
> > /**
> > @@ -331,6 +454,7 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
> > u16 **abort_reason)
> > {
> > efi_status_t status;
> > + struct fmp_state state = { 0 };
> >
> > EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
> > image_size, vendor_code, progress, abort_reason);
> > @@ -338,13 +462,16 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
> > if (!image || image_index != 1)
> > return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > - status = efi_firmware_capsule_authenticate(&image, &image_size);
> > + status = efi_firmware_verify_image(&image, &image_size, image_index,
> > + &state);
> > if (status != EFI_SUCCESS)
> > return EFI_EXIT(status);
> >
> > if (fit_update(image))
> > return EFI_EXIT(EFI_DEVICE_ERROR);
> >
> > + efi_firmware_set_fmp_state_var(&state, image_index);
> > +
> > return EFI_EXIT(EFI_SUCCESS);
> > }
> >
> > @@ -392,6 +519,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> > {
> > int ret;
> > efi_status_t status;
> > + struct fmp_state state = { 0 };
> >
> > EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
> > image_size, vendor_code, progress, abort_reason);
> > @@ -399,7 +527,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> > if (!image)
> > return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > - status = efi_firmware_capsule_authenticate(&image, &image_size);
> > + status = efi_firmware_verify_image(&image, &image_size, image_index,
> > + &state);
> > if (status != EFI_SUCCESS)
> > return EFI_EXIT(status);
> >
> > @@ -419,6 +548,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> > NULL, NULL))
> > return EFI_EXIT(EFI_DEVICE_ERROR);
> >
> > + efi_firmware_set_fmp_state_var(&state, image_index);
> > +
> > return EFI_EXIT(EFI_SUCCESS);
> > }
> >
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 3/8] efi_loader: versioning support in GetImageInfo
2023-05-19 10:32 [PATCH v6 0/8] FMP versioning support Masahisa Kojima
2023-05-19 10:32 ` [PATCH v6 1/8] efi_loader: add the number of image entries in efi_capsule_update_info Masahisa Kojima
2023-05-19 10:32 ` [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable Masahisa Kojima
@ 2023-05-19 10:32 ` Masahisa Kojima
2023-05-22 21:29 ` Ilias Apalodimas
2023-05-19 10:32 ` [PATCH v6 4/8] efi_loader: get lowest supported version from device tree Masahisa Kojima
` (6 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-19 10:32 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Takahiro Akashi, Masahisa Kojima
Current FMP->GetImageInfo() always return 0 for the firmware
version, user can not identify which firmware version is currently
running through the EFI interface.
This commit reads the "FmpStateXXXX" EFI variable, then fills the
firmware version in FMP->GetImageInfo().
Now FMP->GetImageInfo() and ESRT have the meaningful version number.
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Changes in v6:
- create function to fill the version information
lib/efi_loader/efi_firmware.c | 41 ++++++++++++++++++++++++++++++-----
1 file changed, 35 insertions(+), 6 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index fc085e3c08..64ceefa212 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -144,6 +144,39 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
return EFI_EXIT(EFI_UNSUPPORTED);
}
+/**
+ * efi_firmware_fill_version_info - fill the version information
+ * @image_info: Image information
+ * @fw_array: Pointer to size of new image
+ *
+ * Fill the version information into image_info strucrure.
+ *
+ */
+static
+void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_info,
+ struct efi_fw_image *fw_array)
+{
+ u16 varname[13]; /* u"FmpStateXXXX" */
+ efi_status_t ret;
+ efi_uintn_t size;
+ struct fmp_state var_state = { 0 };
+
+ efi_create_indexed_name(varname, sizeof(varname), "FmpState",
+ fw_array->image_index);
+ size = sizeof(var_state);
+ ret = efi_get_variable_int(varname, &fw_array->image_type_id,
+ NULL, &size, &var_state, NULL);
+ if (ret == EFI_SUCCESS)
+ image_info->version = var_state.fw_version;
+ else
+ image_info->version = 0;
+
+ image_info->version_name = NULL; /* not supported */
+ image_info->lowest_supported_image_version = 0;
+ image_info->last_attempt_version = 0;
+ image_info->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
+}
+
/**
* efi_fill_image_desc_array - populate image descriptor array
* @image_info_size: Size of @image_info
@@ -193,11 +226,10 @@ static efi_status_t efi_fill_image_desc_array(
image_info[i].image_index = fw_array[i].image_index;
image_info[i].image_type_id = fw_array[i].image_type_id;
image_info[i].image_id = fw_array[i].image_index;
-
image_info[i].image_id_name = fw_array[i].fw_name;
- image_info[i].version = 0; /* not supported */
- image_info[i].version_name = NULL; /* not supported */
+ efi_firmware_fill_version_info(&image_info[i], &fw_array[i]);
+
image_info[i].size = 0;
image_info[i].attributes_supported =
IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
@@ -210,9 +242,6 @@ static efi_status_t efi_fill_image_desc_array(
image_info[0].attributes_setting |=
IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
- 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;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 3/8] efi_loader: versioning support in GetImageInfo
2023-05-19 10:32 ` [PATCH v6 3/8] efi_loader: versioning support in GetImageInfo Masahisa Kojima
@ 2023-05-22 21:29 ` Ilias Apalodimas
0 siblings, 0 replies; 27+ messages in thread
From: Ilias Apalodimas @ 2023-05-22 21:29 UTC (permalink / raw)
To: Masahisa Kojima; +Cc: u-boot, Heinrich Schuchardt, Simon Glass, Takahiro Akashi
On Fri, May 19, 2023 at 07:32:09PM +0900, Masahisa Kojima wrote:
> Current FMP->GetImageInfo() always return 0 for the firmware
> version, user can not identify which firmware version is currently
> running through the EFI interface.
>
> This commit reads the "FmpStateXXXX" EFI variable, then fills the
> firmware version in FMP->GetImageInfo().
>
> Now FMP->GetImageInfo() and ESRT have the meaningful version number.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Changes in v6:
> - create function to fill the version information
>
> lib/efi_loader/efi_firmware.c | 41 ++++++++++++++++++++++++++++++-----
> 1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index fc085e3c08..64ceefa212 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -144,6 +144,39 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
> return EFI_EXIT(EFI_UNSUPPORTED);
> }
>
> +/**
> + * efi_firmware_fill_version_info - fill the version information
> + * @image_info: Image information
> + * @fw_array: Pointer to size of new image
> + *
> + * Fill the version information into image_info strucrure.
> + *
> + */
> +static
> +void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_info,
> + struct efi_fw_image *fw_array)
> +{
> + u16 varname[13]; /* u"FmpStateXXXX" */
> + efi_status_t ret;
> + efi_uintn_t size;
> + struct fmp_state var_state = { 0 };
> +
> + efi_create_indexed_name(varname, sizeof(varname), "FmpState",
> + fw_array->image_index);
> + size = sizeof(var_state);
> + ret = efi_get_variable_int(varname, &fw_array->image_type_id,
> + NULL, &size, &var_state, NULL);
> + if (ret == EFI_SUCCESS)
> + image_info->version = var_state.fw_version;
> + else
> + image_info->version = 0;
> +
> + image_info->version_name = NULL; /* not supported */
> + image_info->lowest_supported_image_version = 0;
> + image_info->last_attempt_version = 0;
> + image_info->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
> +}
> +
> /**
> * efi_fill_image_desc_array - populate image descriptor array
> * @image_info_size: Size of @image_info
> @@ -193,11 +226,10 @@ static efi_status_t efi_fill_image_desc_array(
> image_info[i].image_index = fw_array[i].image_index;
> image_info[i].image_type_id = fw_array[i].image_type_id;
> image_info[i].image_id = fw_array[i].image_index;
> -
> image_info[i].image_id_name = fw_array[i].fw_name;
>
> - image_info[i].version = 0; /* not supported */
> - image_info[i].version_name = NULL; /* not supported */
> + efi_firmware_fill_version_info(&image_info[i], &fw_array[i]);
> +
> image_info[i].size = 0;
> image_info[i].attributes_supported =
> IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
> @@ -210,9 +242,6 @@ static efi_status_t efi_fill_image_desc_array(
> image_info[0].attributes_setting |=
> IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
>
> - 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;
> }
> --
> 2.17.1
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 4/8] efi_loader: get lowest supported version from device tree
2023-05-19 10:32 [PATCH v6 0/8] FMP versioning support Masahisa Kojima
` (2 preceding siblings ...)
2023-05-19 10:32 ` [PATCH v6 3/8] efi_loader: versioning support in GetImageInfo Masahisa Kojima
@ 2023-05-19 10:32 ` Masahisa Kojima
2023-05-22 21:33 ` Ilias Apalodimas
2023-05-19 10:32 ` [PATCH v6 5/8] efi_loader: check lowest supported version Masahisa Kojima
` (5 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-19 10:32 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Takahiro Akashi, Masahisa Kojima
This commit gets the lowest supported version from device tree,
then fills the lowest supported version in FMP->GetImageInfo().
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Changed in v6:
- fw_version is removed from device tree
.../firmware/firmware-version.txt | 22 ++++++++
lib/efi_loader/efi_firmware.c | 50 ++++++++++++++++++-
2 files changed, 71 insertions(+), 1 deletion(-)
create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt b/doc/device-tree-bindings/firmware/firmware-version.txt
new file mode 100644
index 0000000000..ee90ce3117
--- /dev/null
+++ b/doc/device-tree-bindings/firmware/firmware-version.txt
@@ -0,0 +1,22 @@
+firmware-version bindings
+-------------------------------
+
+Required properties:
+- image-type-id : guid for image blob type
+- image-index : image index
+- lowest-supported-version : lowest supported version
+
+Example:
+
+ firmware-version {
+ image1 {
+ image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
+ image-index = <1>;
+ lowest-supported-version = <3>;
+ };
+ image2 {
+ image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0";
+ image-index = <2>;
+ lowest-supported-version = <7>;
+ };
+ };
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 64ceefa212..00cf9a088a 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -144,6 +144,51 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
return EFI_EXIT(EFI_UNSUPPORTED);
}
+/**
+ * efi_firmware_get_lsv_from_dtb - get lowest supported version from dtb
+ * @image_index: Image index
+ * @image_type_id: Image type id
+ * @lsv: Pointer to store the lowest supported version
+ *
+ * Read the firmware version information from dtb.
+ */
+static void efi_firmware_get_lsv_from_dtb(u8 image_index,
+ efi_guid_t *image_type_id, u32 *lsv)
+{
+ const void *fdt = gd->fdt_blob;
+ const fdt32_t *val;
+ const char *guid_str;
+ int len, offset, index;
+ int parent;
+
+ *lsv = 0;
+
+ parent = fdt_subnode_offset(fdt, 0, "firmware-version");
+ if (parent < 0)
+ return;
+
+ fdt_for_each_subnode(offset, fdt, parent) {
+ efi_guid_t guid;
+
+ guid_str = fdt_getprop(fdt, offset, "image-type-id", &len);
+ if (!guid_str)
+ continue;
+ uuid_str_to_bin(guid_str, guid.b, UUID_STR_FORMAT_GUID);
+
+ val = fdt_getprop(fdt, offset, "image-index", &len);
+ if (!val)
+ continue;
+ index = fdt32_to_cpu(*val);
+
+ if (!guidcmp(&guid, image_type_id) && index == image_index) {
+ val = fdt_getprop(fdt, offset,
+ "lowest-supported-version", &len);
+ if (val)
+ *lsv = fdt32_to_cpu(*val);
+ }
+ }
+}
+
/**
* efi_firmware_fill_version_info - fill the version information
* @image_info: Image information
@@ -171,8 +216,11 @@ void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
else
image_info->version = 0;
+ efi_firmware_get_lsv_from_dtb(fw_array->image_index,
+ &fw_array->image_type_id,
+ &image_info->lowest_supported_image_version);
+
image_info->version_name = NULL; /* not supported */
- image_info->lowest_supported_image_version = 0;
image_info->last_attempt_version = 0;
image_info->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 4/8] efi_loader: get lowest supported version from device tree
2023-05-19 10:32 ` [PATCH v6 4/8] efi_loader: get lowest supported version from device tree Masahisa Kojima
@ 2023-05-22 21:33 ` Ilias Apalodimas
0 siblings, 0 replies; 27+ messages in thread
From: Ilias Apalodimas @ 2023-05-22 21:33 UTC (permalink / raw)
To: Masahisa Kojima; +Cc: u-boot, Heinrich Schuchardt, Simon Glass, Takahiro Akashi
On Fri, May 19, 2023 at 07:32:10PM +0900, Masahisa Kojima wrote:
> This commit gets the lowest supported version from device tree,
> then fills the lowest supported version in FMP->GetImageInfo().
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Changed in v6:
> - fw_version is removed from device tree
>
> .../firmware/firmware-version.txt | 22 ++++++++
> lib/efi_loader/efi_firmware.c | 50 ++++++++++++++++++-
> 2 files changed, 71 insertions(+), 1 deletion(-)
> create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
>
> diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt b/doc/device-tree-bindings/firmware/firmware-version.txt
> new file mode 100644
> index 0000000000..ee90ce3117
> --- /dev/null
> +++ b/doc/device-tree-bindings/firmware/firmware-version.txt
> @@ -0,0 +1,22 @@
> +firmware-version bindings
> +-------------------------------
> +
> +Required properties:
> +- image-type-id : guid for image blob type
> +- image-index : image index
> +- lowest-supported-version : lowest supported version
> +
> +Example:
> +
> + firmware-version {
> + image1 {
> + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> + image-index = <1>;
> + lowest-supported-version = <3>;
> + };
> + image2 {
> + image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0";
> + image-index = <2>;
> + lowest-supported-version = <7>;
> + };
> + };
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 64ceefa212..00cf9a088a 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -144,6 +144,51 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
> return EFI_EXIT(EFI_UNSUPPORTED);
> }
>
> +/**
> + * efi_firmware_get_lsv_from_dtb - get lowest supported version from dtb
> + * @image_index: Image index
> + * @image_type_id: Image type id
> + * @lsv: Pointer to store the lowest supported version
> + *
> + * Read the firmware version information from dtb.
> + */
> +static void efi_firmware_get_lsv_from_dtb(u8 image_index,
> + efi_guid_t *image_type_id, u32 *lsv)
> +{
> + const void *fdt = gd->fdt_blob;
> + const fdt32_t *val;
> + const char *guid_str;
> + int len, offset, index;
> + int parent;
> +
> + *lsv = 0;
> +
> + parent = fdt_subnode_offset(fdt, 0, "firmware-version");
> + if (parent < 0)
> + return;
> +
> + fdt_for_each_subnode(offset, fdt, parent) {
> + efi_guid_t guid;
> +
> + guid_str = fdt_getprop(fdt, offset, "image-type-id", &len);
> + if (!guid_str)
> + continue;
> + uuid_str_to_bin(guid_str, guid.b, UUID_STR_FORMAT_GUID);
> +
> + val = fdt_getprop(fdt, offset, "image-index", &len);
> + if (!val)
> + continue;
> + index = fdt32_to_cpu(*val);
> +
> + if (!guidcmp(&guid, image_type_id) && index == image_index) {
> + val = fdt_getprop(fdt, offset,
> + "lowest-supported-version", &len);
> + if (val)
> + *lsv = fdt32_to_cpu(*val);
> + }
> + }
> +}
> +
> /**
> * efi_firmware_fill_version_info - fill the version information
> * @image_info: Image information
> @@ -171,8 +216,11 @@ void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
> else
> image_info->version = 0;
>
> + efi_firmware_get_lsv_from_dtb(fw_array->image_index,
> + &fw_array->image_type_id,
> + &image_info->lowest_supported_image_version);
> +
> image_info->version_name = NULL; /* not supported */
> - image_info->lowest_supported_image_version = 0;
> image_info->last_attempt_version = 0;
> image_info->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
> }
> --
> 2.17.1
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 5/8] efi_loader: check lowest supported version
2023-05-19 10:32 [PATCH v6 0/8] FMP versioning support Masahisa Kojima
` (3 preceding siblings ...)
2023-05-19 10:32 ` [PATCH v6 4/8] efi_loader: get lowest supported version from device tree Masahisa Kojima
@ 2023-05-19 10:32 ` Masahisa Kojima
2023-05-22 21:36 ` Ilias Apalodimas
2023-05-19 10:32 ` [PATCH v6 6/8] mkeficapsule: add FMP Payload Header Masahisa Kojima
` (4 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-19 10:32 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Takahiro Akashi, Masahisa Kojima
The FMP Payload Header which EDK II capsule generation scripts
insert has a firmware version.
This commit reads the lowest supported version stored in the
device tree, then check if the firmware version in FMP payload header
of the ongoing capsule is equal or greater than the
lowest supported version. If the firmware version is lower than
lowest supported version, capsule update will not be performed.
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Changes in v6:
- get aligned to the latest implementation
Changes in v5:
- newly implement the device tree based versioning
Changes in v4:
- use log_err() instead of printf()
Changes in v2:
- add error message when the firmware version is lower than
lowest supported version
lib/efi_loader/efi_firmware.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 00cf9a088a..7cd0016765 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -424,7 +424,8 @@ static void efi_firmware_get_fw_version(const void **p_image,
* @image_index Image index
* @state Pointer to fmp state
*
- * Verify the capsule file
+ * Verify the capsule authentication and check if the fw_version
+ * is equal or greater than the lowest supported version.
*
* Return: status code
*/
@@ -434,11 +435,27 @@ efi_status_t efi_firmware_verify_image(const void **p_image,
u8 image_index,
struct fmp_state *state)
{
+ u32 lsv;
efi_status_t ret;
+ efi_guid_t *image_type_id;
ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
efi_firmware_get_fw_version(p_image, p_image_size, state);
+ /* check lowest_supported_version if capsule authentication passes */
+ if (ret == EFI_SUCCESS) {
+ image_type_id = efi_firmware_get_image_type_id(image_index);
+ if (!image_type_id)
+ return EFI_INVALID_PARAMETER;
+
+ efi_firmware_get_lsv_from_dtb(image_index, image_type_id, &lsv);
+ if (state->fw_version < lsv) {
+ log_err("Firmware version %u too low. Expecting >= %u. Aborting update\n",
+ state->fw_version, lsv);
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 5/8] efi_loader: check lowest supported version
2023-05-19 10:32 ` [PATCH v6 5/8] efi_loader: check lowest supported version Masahisa Kojima
@ 2023-05-22 21:36 ` Ilias Apalodimas
2023-05-23 1:56 ` Masahisa Kojima
0 siblings, 1 reply; 27+ messages in thread
From: Ilias Apalodimas @ 2023-05-22 21:36 UTC (permalink / raw)
To: Masahisa Kojima; +Cc: u-boot, Heinrich Schuchardt, Simon Glass, Takahiro Akashi
On Fri, May 19, 2023 at 07:32:11PM +0900, Masahisa Kojima wrote:
> The FMP Payload Header which EDK II capsule generation scripts
> insert has a firmware version.
> This commit reads the lowest supported version stored in the
> device tree, then check if the firmware version in FMP payload header
> of the ongoing capsule is equal or greater than the
> lowest supported version. If the firmware version is lower than
> lowest supported version, capsule update will not be performed.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Changes in v6:
> - get aligned to the latest implementation
>
> Changes in v5:
> - newly implement the device tree based versioning
>
> Changes in v4:
> - use log_err() instead of printf()
>
> Changes in v2:
> - add error message when the firmware version is lower than
> lowest supported version
>
> lib/efi_loader/efi_firmware.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 00cf9a088a..7cd0016765 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -424,7 +424,8 @@ static void efi_firmware_get_fw_version(const void **p_image,
> * @image_index Image index
> * @state Pointer to fmp state
> *
> - * Verify the capsule file
> + * Verify the capsule authentication and check if the fw_version
> + * is equal or greater than the lowest supported version.
> *
> * Return: status code
> */
> @@ -434,11 +435,27 @@ efi_status_t efi_firmware_verify_image(const void **p_image,
> u8 image_index,
> struct fmp_state *state)
> {
> + u32 lsv;
> efi_status_t ret;
> + efi_guid_t *image_type_id;
>
> ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
> efi_firmware_get_fw_version(p_image, p_image_size, state);
>
> + /* check lowest_supported_version if capsule authentication passes */
> + if (ret == EFI_SUCCESS) {
What's the point of this here? Can;'t we move this check right after
efi_firmware_capsule_authenticate() and return a security violation if that
failed?
> + image_type_id = efi_firmware_get_image_type_id(image_index);
> + if (!image_type_id)
> + return EFI_INVALID_PARAMETER;
> +
> + efi_firmware_get_lsv_from_dtb(image_index, image_type_id, &lsv);
> + if (state->fw_version < lsv) {
> + log_err("Firmware version %u too low. Expecting >= %u. Aborting update\n",
> + state->fw_version, lsv);
> + return EFI_INVALID_PARAMETER;
> + }
> + }
> +
> return ret;
> }
>
> --
> 2.17.1
>
Thanks
/Ilias
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6 5/8] efi_loader: check lowest supported version
2023-05-22 21:36 ` Ilias Apalodimas
@ 2023-05-23 1:56 ` Masahisa Kojima
0 siblings, 0 replies; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-23 1:56 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: u-boot, Heinrich Schuchardt, Simon Glass, Takahiro Akashi
On Tue, 23 May 2023 at 06:36, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Fri, May 19, 2023 at 07:32:11PM +0900, Masahisa Kojima wrote:
> > The FMP Payload Header which EDK II capsule generation scripts
> > insert has a firmware version.
> > This commit reads the lowest supported version stored in the
> > device tree, then check if the firmware version in FMP payload header
> > of the ongoing capsule is equal or greater than the
> > lowest supported version. If the firmware version is lower than
> > lowest supported version, capsule update will not be performed.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> > Changes in v6:
> > - get aligned to the latest implementation
> >
> > Changes in v5:
> > - newly implement the device tree based versioning
> >
> > Changes in v4:
> > - use log_err() instead of printf()
> >
> > Changes in v2:
> > - add error message when the firmware version is lower than
> > lowest supported version
> >
> > lib/efi_loader/efi_firmware.c | 19 ++++++++++++++++++-
> > 1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> > index 00cf9a088a..7cd0016765 100644
> > --- a/lib/efi_loader/efi_firmware.c
> > +++ b/lib/efi_loader/efi_firmware.c
> > @@ -424,7 +424,8 @@ static void efi_firmware_get_fw_version(const void **p_image,
> > * @image_index Image index
> > * @state Pointer to fmp state
> > *
> > - * Verify the capsule file
> > + * Verify the capsule authentication and check if the fw_version
> > + * is equal or greater than the lowest supported version.
> > *
> > * Return: status code
> > */
> > @@ -434,11 +435,27 @@ efi_status_t efi_firmware_verify_image(const void **p_image,
> > u8 image_index,
> > struct fmp_state *state)
> > {
> > + u32 lsv;
> > efi_status_t ret;
> > + efi_guid_t *image_type_id;
> >
> > ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
> > efi_firmware_get_fw_version(p_image, p_image_size, state);
> >
> > + /* check lowest_supported_version if capsule authentication passes */
> > + if (ret == EFI_SUCCESS) {
>
> What's the point of this here? Can;'t we move this check right after
> efi_firmware_capsule_authenticate() and return a security violation if that
> failed?
Yes, I agree.
Thanks,
Masahisa Kojima
>
> > + image_type_id = efi_firmware_get_image_type_id(image_index);
> > + if (!image_type_id)
> > + return EFI_INVALID_PARAMETER;
> > +
> > + efi_firmware_get_lsv_from_dtb(image_index, image_type_id, &lsv);
> > + if (state->fw_version < lsv) {
> > + log_err("Firmware version %u too low. Expecting >= %u. Aborting update\n",
> > + state->fw_version, lsv);
> > + return EFI_INVALID_PARAMETER;
> > + }
> > + }
> > +
> > return ret;
> > }
> >
> > --
> > 2.17.1
> >
>
> Thanks
> /Ilias
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 6/8] mkeficapsule: add FMP Payload Header
2023-05-19 10:32 [PATCH v6 0/8] FMP versioning support Masahisa Kojima
` (4 preceding siblings ...)
2023-05-19 10:32 ` [PATCH v6 5/8] efi_loader: check lowest supported version Masahisa Kojima
@ 2023-05-19 10:32 ` Masahisa Kojima
2023-05-22 21:29 ` Ilias Apalodimas
2023-05-19 10:32 ` [PATCH v6 7/8] doc: uefi: add firmware versioning documentation Masahisa Kojima
` (3 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-19 10:32 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Takahiro Akashi, Masahisa Kojima, Sughosh Ganu, Etienne Carriere
Current mkeficapsule tool does not provide firmware
version management. EDK II reference implementation inserts
the FMP Payload Header right before the payload.
It coutains the fw_version and lowest supported version.
This commit adds a new parameters required to generate
the FMP Payload Header for mkeficapsule tool.
'-v' indicates the firmware version.
When mkeficapsule tool is invoked without '-v' option,
FMP Payload Header is not inserted, the behavior is same as
current implementation.
The lowest supported version included in the FMP Payload Header
is not used, the value stored in the device tree is used instead.
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
No update since v5
Changes in v5:
- remove --lsv since we use the lowest_supported_version in the dtb
Changes in v3:
- remove '-f' option
- move some definitions into tools/eficapsule.h
- add dependency check of fw_version and lowest_supported_version
- remove unexpected modification of existing fprintf() call
- add documentation
Newly created in v2
doc/mkeficapsule.1 | 10 ++++++++++
tools/eficapsule.h | 30 ++++++++++++++++++++++++++++++
tools/mkeficapsule.c | 37 +++++++++++++++++++++++++++++++++----
3 files changed, 73 insertions(+), 4 deletions(-)
diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
index 1ca245a10f..c4c2057d5c 100644
--- a/doc/mkeficapsule.1
+++ b/doc/mkeficapsule.1
@@ -61,6 +61,16 @@ Specify an image index
.BI "-I\fR,\fB --instance " instance
Specify a hardware instance
+.PP
+FMP Payload Header is inserted right before the payload if
+.BR --fw-version
+is specified
+
+
+.TP
+.BI "-v\fR,\fB --fw-version " firmware-version
+Specify a firmware version, 0 if omitted
+
.PP
For generation of firmware accept empty capsule
.BR --guid
diff --git a/tools/eficapsule.h b/tools/eficapsule.h
index 072a4b5598..753fb73313 100644
--- a/tools/eficapsule.h
+++ b/tools/eficapsule.h
@@ -113,4 +113,34 @@ struct efi_firmware_image_authentication {
struct win_certificate_uefi_guid auth_info;
} __packed;
+/* fmp payload header */
+#define SIGNATURE_16(A, B) ((A) | ((B) << 8))
+#define SIGNATURE_32(A, B, C, D) \
+ (SIGNATURE_16(A, B) | (SIGNATURE_16(C, D) << 16))
+
+#define FMP_PAYLOAD_HDR_SIGNATURE SIGNATURE_32('M', 'S', 'S', '1')
+
+/**
+ * struct fmp_payload_header - EDK2 header for the FMP payload
+ *
+ * This structure describes the header which is preprended to the
+ * FMP payload by the edk2 capsule generation scripts.
+ *
+ * @signature: Header signature used to identify the header
+ * @header_size: Size of the structure
+ * @fw_version: Firmware versions used
+ * @lowest_supported_version: Lowest supported version (not used)
+ */
+struct fmp_payload_header {
+ uint32_t signature;
+ uint32_t header_size;
+ uint32_t fw_version;
+ uint32_t lowest_supported_version;
+};
+
+struct fmp_payload_header_params {
+ bool have_header;
+ uint32_t fw_version;
+};
+
#endif /* _EFI_CAPSULE_H */
diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index b71537beee..52be1f122e 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -41,6 +41,7 @@ static struct option options[] = {
{"guid", required_argument, NULL, 'g'},
{"index", required_argument, NULL, 'i'},
{"instance", required_argument, NULL, 'I'},
+ {"fw-version", required_argument, NULL, 'v'},
{"private-key", required_argument, NULL, 'p'},
{"certificate", required_argument, NULL, 'c'},
{"monotonic-count", required_argument, NULL, 'm'},
@@ -60,6 +61,7 @@ static void print_usage(void)
"\t-g, --guid <guid string> guid for image blob type\n"
"\t-i, --index <index> update image index\n"
"\t-I, --instance <instance> update hardware instance\n"
+ "\t-v, --fw-version <version> firmware version\n"
"\t-p, --private-key <privkey file> private key file\n"
"\t-c, --certificate <cert file> signer's certificate file\n"
"\t-m, --monotonic-count <count> monotonic count\n"
@@ -402,6 +404,7 @@ static void free_sig_data(struct auth_context *ctx)
*/
static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
unsigned long index, unsigned long instance,
+ struct fmp_payload_header_params *fmp_ph_params,
uint64_t mcount, char *privkey_file, char *cert_file,
uint16_t oemflags)
{
@@ -410,10 +413,11 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
struct efi_firmware_management_capsule_image_header image;
struct auth_context auth_context;
FILE *f;
- uint8_t *data;
+ uint8_t *data, *new_data, *buf;
off_t bin_size;
uint64_t offset;
int ret;
+ struct fmp_payload_header payload_header;
#ifdef DEBUG
fprintf(stderr, "For output: %s\n", path);
@@ -423,6 +427,7 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
auth_context.sig_size = 0;
f = NULL;
data = NULL;
+ new_data = NULL;
ret = -1;
/*
@@ -431,12 +436,30 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
if (read_bin_file(bin, &data, &bin_size))
goto err;
+ buf = data;
+
+ /* insert fmp payload header right before the payload */
+ if (fmp_ph_params->have_header) {
+ new_data = malloc(bin_size + sizeof(payload_header));
+ if (!new_data)
+ goto err;
+
+ payload_header.signature = FMP_PAYLOAD_HDR_SIGNATURE;
+ payload_header.header_size = sizeof(payload_header);
+ payload_header.fw_version = fmp_ph_params->fw_version;
+ payload_header.lowest_supported_version = 0; /* not used */
+ memcpy(new_data, &payload_header, sizeof(payload_header));
+ memcpy(new_data + sizeof(payload_header), data, bin_size);
+ buf = new_data;
+ bin_size += sizeof(payload_header);
+ }
+
/* first, calculate signature to determine its size */
if (privkey_file && cert_file) {
auth_context.key_file = privkey_file;
auth_context.cert_file = cert_file;
auth_context.auth.monotonic_count = mcount;
- auth_context.image_data = data;
+ auth_context.image_data = buf;
auth_context.image_size = bin_size;
if (create_auth_data(&auth_context)) {
@@ -536,7 +559,7 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
/*
* firmware binary
*/
- if (write_capsule_file(f, data, bin_size, "Firmware binary"))
+ if (write_capsule_file(f, buf, bin_size, "Firmware binary"))
goto err;
ret = 0;
@@ -545,6 +568,7 @@ err:
fclose(f);
free_sig_data(&auth_context);
free(data);
+ free(new_data);
return ret;
}
@@ -644,6 +668,7 @@ int main(int argc, char **argv)
unsigned long oemflags;
char *privkey_file, *cert_file;
int c, idx;
+ struct fmp_payload_header_params fmp_ph_params = { 0 };
guid = NULL;
index = 0;
@@ -679,6 +704,10 @@ int main(int argc, char **argv)
case 'I':
instance = strtoul(optarg, NULL, 0);
break;
+ case 'v':
+ fmp_ph_params.fw_version = strtoul(optarg, NULL, 0);
+ fmp_ph_params.have_header = true;
+ break;
case 'p':
if (privkey_file) {
fprintf(stderr,
@@ -751,7 +780,7 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
} else if (create_fwbin(argv[argc - 1], argv[argc - 2], guid,
- index, instance, mcount, privkey_file,
+ index, instance, &fmp_ph_params, mcount, privkey_file,
cert_file, (uint16_t)oemflags) < 0) {
fprintf(stderr, "Creating firmware capsule failed\n");
exit(EXIT_FAILURE);
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 6/8] mkeficapsule: add FMP Payload Header
2023-05-19 10:32 ` [PATCH v6 6/8] mkeficapsule: add FMP Payload Header Masahisa Kojima
@ 2023-05-22 21:29 ` Ilias Apalodimas
0 siblings, 0 replies; 27+ messages in thread
From: Ilias Apalodimas @ 2023-05-22 21:29 UTC (permalink / raw)
To: Masahisa Kojima
Cc: u-boot, Heinrich Schuchardt, Simon Glass, Takahiro Akashi,
Sughosh Ganu, Etienne Carriere
On Fri, May 19, 2023 at 07:32:12PM +0900, Masahisa Kojima wrote:
> Current mkeficapsule tool does not provide firmware
> version management. EDK II reference implementation inserts
> the FMP Payload Header right before the payload.
> It coutains the fw_version and lowest supported version.
>
> This commit adds a new parameters required to generate
> the FMP Payload Header for mkeficapsule tool.
> '-v' indicates the firmware version.
>
> When mkeficapsule tool is invoked without '-v' option,
> FMP Payload Header is not inserted, the behavior is same as
> current implementation.
>
> The lowest supported version included in the FMP Payload Header
> is not used, the value stored in the device tree is used instead.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> No update since v5
>
> Changes in v5:
> - remove --lsv since we use the lowest_supported_version in the dtb
>
> Changes in v3:
> - remove '-f' option
> - move some definitions into tools/eficapsule.h
> - add dependency check of fw_version and lowest_supported_version
> - remove unexpected modification of existing fprintf() call
> - add documentation
>
> Newly created in v2
>
> doc/mkeficapsule.1 | 10 ++++++++++
> tools/eficapsule.h | 30 ++++++++++++++++++++++++++++++
> tools/mkeficapsule.c | 37 +++++++++++++++++++++++++++++++++----
> 3 files changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
> index 1ca245a10f..c4c2057d5c 100644
> --- a/doc/mkeficapsule.1
> +++ b/doc/mkeficapsule.1
> @@ -61,6 +61,16 @@ Specify an image index
> .BI "-I\fR,\fB --instance " instance
> Specify a hardware instance
>
> +.PP
> +FMP Payload Header is inserted right before the payload if
> +.BR --fw-version
> +is specified
> +
> +
> +.TP
> +.BI "-v\fR,\fB --fw-version " firmware-version
> +Specify a firmware version, 0 if omitted
> +
> .PP
> For generation of firmware accept empty capsule
> .BR --guid
> diff --git a/tools/eficapsule.h b/tools/eficapsule.h
> index 072a4b5598..753fb73313 100644
> --- a/tools/eficapsule.h
> +++ b/tools/eficapsule.h
> @@ -113,4 +113,34 @@ struct efi_firmware_image_authentication {
> struct win_certificate_uefi_guid auth_info;
> } __packed;
>
> +/* fmp payload header */
> +#define SIGNATURE_16(A, B) ((A) | ((B) << 8))
> +#define SIGNATURE_32(A, B, C, D) \
> + (SIGNATURE_16(A, B) | (SIGNATURE_16(C, D) << 16))
> +
> +#define FMP_PAYLOAD_HDR_SIGNATURE SIGNATURE_32('M', 'S', 'S', '1')
> +
> +/**
> + * struct fmp_payload_header - EDK2 header for the FMP payload
> + *
> + * This structure describes the header which is preprended to the
> + * FMP payload by the edk2 capsule generation scripts.
> + *
> + * @signature: Header signature used to identify the header
> + * @header_size: Size of the structure
> + * @fw_version: Firmware versions used
> + * @lowest_supported_version: Lowest supported version (not used)
> + */
> +struct fmp_payload_header {
> + uint32_t signature;
> + uint32_t header_size;
> + uint32_t fw_version;
> + uint32_t lowest_supported_version;
> +};
> +
> +struct fmp_payload_header_params {
> + bool have_header;
> + uint32_t fw_version;
> +};
> +
> #endif /* _EFI_CAPSULE_H */
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index b71537beee..52be1f122e 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -41,6 +41,7 @@ static struct option options[] = {
> {"guid", required_argument, NULL, 'g'},
> {"index", required_argument, NULL, 'i'},
> {"instance", required_argument, NULL, 'I'},
> + {"fw-version", required_argument, NULL, 'v'},
> {"private-key", required_argument, NULL, 'p'},
> {"certificate", required_argument, NULL, 'c'},
> {"monotonic-count", required_argument, NULL, 'm'},
> @@ -60,6 +61,7 @@ static void print_usage(void)
> "\t-g, --guid <guid string> guid for image blob type\n"
> "\t-i, --index <index> update image index\n"
> "\t-I, --instance <instance> update hardware instance\n"
> + "\t-v, --fw-version <version> firmware version\n"
> "\t-p, --private-key <privkey file> private key file\n"
> "\t-c, --certificate <cert file> signer's certificate file\n"
> "\t-m, --monotonic-count <count> monotonic count\n"
> @@ -402,6 +404,7 @@ static void free_sig_data(struct auth_context *ctx)
> */
> static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> unsigned long index, unsigned long instance,
> + struct fmp_payload_header_params *fmp_ph_params,
> uint64_t mcount, char *privkey_file, char *cert_file,
> uint16_t oemflags)
> {
> @@ -410,10 +413,11 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> struct efi_firmware_management_capsule_image_header image;
> struct auth_context auth_context;
> FILE *f;
> - uint8_t *data;
> + uint8_t *data, *new_data, *buf;
> off_t bin_size;
> uint64_t offset;
> int ret;
> + struct fmp_payload_header payload_header;
>
> #ifdef DEBUG
> fprintf(stderr, "For output: %s\n", path);
> @@ -423,6 +427,7 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> auth_context.sig_size = 0;
> f = NULL;
> data = NULL;
> + new_data = NULL;
> ret = -1;
>
> /*
> @@ -431,12 +436,30 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> if (read_bin_file(bin, &data, &bin_size))
> goto err;
>
> + buf = data;
> +
> + /* insert fmp payload header right before the payload */
> + if (fmp_ph_params->have_header) {
> + new_data = malloc(bin_size + sizeof(payload_header));
> + if (!new_data)
> + goto err;
> +
> + payload_header.signature = FMP_PAYLOAD_HDR_SIGNATURE;
> + payload_header.header_size = sizeof(payload_header);
> + payload_header.fw_version = fmp_ph_params->fw_version;
> + payload_header.lowest_supported_version = 0; /* not used */
> + memcpy(new_data, &payload_header, sizeof(payload_header));
> + memcpy(new_data + sizeof(payload_header), data, bin_size);
> + buf = new_data;
> + bin_size += sizeof(payload_header);
> + }
> +
> /* first, calculate signature to determine its size */
> if (privkey_file && cert_file) {
> auth_context.key_file = privkey_file;
> auth_context.cert_file = cert_file;
> auth_context.auth.monotonic_count = mcount;
> - auth_context.image_data = data;
> + auth_context.image_data = buf;
> auth_context.image_size = bin_size;
>
> if (create_auth_data(&auth_context)) {
> @@ -536,7 +559,7 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> /*
> * firmware binary
> */
> - if (write_capsule_file(f, data, bin_size, "Firmware binary"))
> + if (write_capsule_file(f, buf, bin_size, "Firmware binary"))
> goto err;
>
> ret = 0;
> @@ -545,6 +568,7 @@ err:
> fclose(f);
> free_sig_data(&auth_context);
> free(data);
> + free(new_data);
>
> return ret;
> }
> @@ -644,6 +668,7 @@ int main(int argc, char **argv)
> unsigned long oemflags;
> char *privkey_file, *cert_file;
> int c, idx;
> + struct fmp_payload_header_params fmp_ph_params = { 0 };
>
> guid = NULL;
> index = 0;
> @@ -679,6 +704,10 @@ int main(int argc, char **argv)
> case 'I':
> instance = strtoul(optarg, NULL, 0);
> break;
> + case 'v':
> + fmp_ph_params.fw_version = strtoul(optarg, NULL, 0);
> + fmp_ph_params.have_header = true;
> + break;
> case 'p':
> if (privkey_file) {
> fprintf(stderr,
> @@ -751,7 +780,7 @@ int main(int argc, char **argv)
> exit(EXIT_FAILURE);
> }
> } else if (create_fwbin(argv[argc - 1], argv[argc - 2], guid,
> - index, instance, mcount, privkey_file,
> + index, instance, &fmp_ph_params, mcount, privkey_file,
> cert_file, (uint16_t)oemflags) < 0) {
> fprintf(stderr, "Creating firmware capsule failed\n");
> exit(EXIT_FAILURE);
> --
> 2.17.1
>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 7/8] doc: uefi: add firmware versioning documentation
2023-05-19 10:32 [PATCH v6 0/8] FMP versioning support Masahisa Kojima
` (5 preceding siblings ...)
2023-05-19 10:32 ` [PATCH v6 6/8] mkeficapsule: add FMP Payload Header Masahisa Kojima
@ 2023-05-19 10:32 ` Masahisa Kojima
2023-05-22 0:35 ` Takahiro Akashi
2023-05-19 10:32 ` [PATCH v6 8/8] doc: uefi: add anti-rollback documentation Masahisa Kojima
` (2 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-19 10:32 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Takahiro Akashi, Masahisa Kojima
This commit describes the procedure to add the firmware version
into the capsule file.
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Newly created in v6
doc/develop/uefi/uefi.rst | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index ffe25ca231..efab0fc7b1 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -510,6 +510,35 @@ where signature.dts looks like::
};
};
+Enabling Firmware Versioning
+****************************
+
+The UEFI specification does not define the firmware versioning mechanism.
+EDK II reference implementation inserts the FMP Payload Header right before
+the payload. It coutains the fw_version and lowest supported version,
+EDK II reference implementation uses these information to implement the
+firmware versioning and anti-rollback protection, the firmware version and
+lowest supported version is stored into EFI non-volatile variable.
+
+In U-Boot, the firmware versioning is implemented utilizing
+the FMP Payload Header same as EDK II reference implementation,
+reads the FMP Payload Header and stores the firmware version into
+"FmpStateXXXX" EFI non-volatile variable. XXXX indicates the image index,
+since FMP protocol handles multiple image indexes.
+
+
+1. Run the following command to add firmware version into the capsule file
+
+.. code-block:: console
+
+ $ mkeficapsule --monotonic-count 1 \
+ --private-key CRT.key \
+ --certificate CRT.crt \
+ --index 1 --instance 0 \
+ --fw-version 5 \
+ [--fit | --raw | --guid <guid-string] \
+ <image_blob> <capsule_file_name>
+
Executing the boot manager
~~~~~~~~~~~~~~~~~~~~~~~~~~
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 7/8] doc: uefi: add firmware versioning documentation
2023-05-19 10:32 ` [PATCH v6 7/8] doc: uefi: add firmware versioning documentation Masahisa Kojima
@ 2023-05-22 0:35 ` Takahiro Akashi
2023-05-22 4:25 ` Masahisa Kojima
0 siblings, 1 reply; 27+ messages in thread
From: Takahiro Akashi @ 2023-05-22 0:35 UTC (permalink / raw)
To: Masahisa Kojima
Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass
On Fri, May 19, 2023 at 07:32:13PM +0900, Masahisa Kojima wrote:
> This commit describes the procedure to add the firmware version
> into the capsule file.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Newly created in v6
>
> doc/develop/uefi/uefi.rst | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index ffe25ca231..efab0fc7b1 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -510,6 +510,35 @@ where signature.dts looks like::
> };
> };
>
> +Enabling Firmware Versioning
> +****************************
> +
> +The UEFI specification does not define the firmware versioning mechanism.
> +EDK II reference implementation inserts the FMP Payload Header right before
> +the payload. It coutains the fw_version and lowest supported version,
> +EDK II reference implementation uses these information to implement the
> +firmware versioning and anti-rollback protection, the firmware version and
> +lowest supported version is stored into EFI non-volatile variable.
> +
> +In U-Boot, the firmware versioning is implemented utilizing
> +the FMP Payload Header same as EDK II reference implementation,
> +reads the FMP Payload Header and stores the firmware version into
> +"FmpStateXXXX" EFI non-volatile variable. XXXX indicates the image index,
> +since FMP protocol handles multiple image indexes.
I suggested that you should use "FmState" with the firmware's own guid
as a vendor guid of the variable.
In theory, UEFI may have different FMP drivers, then "index id" may
have the same value for different firmwares.
> +
> +1. Run the following command to add firmware version into the capsule file
Anyhow, you'd better clearly mention that an user needs to specify
"--fw-version" option and what happens (or not happen) if the option
is not there.
I think all the text here can be simply merged in "Creating a capsule file".
-Takahiro Akashi
> +.. code-block:: console
> +
> + $ mkeficapsule --monotonic-count 1 \
> + --private-key CRT.key \
> + --certificate CRT.crt \
> + --index 1 --instance 0 \
> + --fw-version 5 \
> + [--fit | --raw | --guid <guid-string] \
> + <image_blob> <capsule_file_name>
> +
> Executing the boot manager
> ~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v6 7/8] doc: uefi: add firmware versioning documentation
2023-05-22 0:35 ` Takahiro Akashi
@ 2023-05-22 4:25 ` Masahisa Kojima
0 siblings, 0 replies; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-22 4:25 UTC (permalink / raw)
To: Takahiro Akashi, Masahisa Kojima, u-boot, Heinrich Schuchardt,
Ilias Apalodimas, Simon Glass
Hi Akashi-san,
On Mon, 22 May 2023 at 09:35, Takahiro Akashi
<takahiro.akashi@linaro.org> wrote:
>
> On Fri, May 19, 2023 at 07:32:13PM +0900, Masahisa Kojima wrote:
> > This commit describes the procedure to add the firmware version
> > into the capsule file.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> > Newly created in v6
> >
> > doc/develop/uefi/uefi.rst | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> > index ffe25ca231..efab0fc7b1 100644
> > --- a/doc/develop/uefi/uefi.rst
> > +++ b/doc/develop/uefi/uefi.rst
> > @@ -510,6 +510,35 @@ where signature.dts looks like::
> > };
> > };
> >
> > +Enabling Firmware Versioning
> > +****************************
> > +
> > +The UEFI specification does not define the firmware versioning mechanism.
> > +EDK II reference implementation inserts the FMP Payload Header right before
> > +the payload. It coutains the fw_version and lowest supported version,
> > +EDK II reference implementation uses these information to implement the
> > +firmware versioning and anti-rollback protection, the firmware version and
> > +lowest supported version is stored into EFI non-volatile variable.
> > +
> > +In U-Boot, the firmware versioning is implemented utilizing
> > +the FMP Payload Header same as EDK II reference implementation,
> > +reads the FMP Payload Header and stores the firmware version into
> > +"FmpStateXXXX" EFI non-volatile variable. XXXX indicates the image index,
> > +since FMP protocol handles multiple image indexes.
>
> I suggested that you should use "FmState" with the firmware's own guid
> as a vendor guid of the variable.
Yes, this series uses firmware"s image_type_id as a vendor
guid of the "FmpStateXXXX" variable.
> In theory, UEFI may have different FMP drivers, then "index id" may
> have the same value for different firmwares.
>
> > +
> > +1. Run the following command to add firmware version into the capsule file
>
> Anyhow, you'd better clearly mention that an user needs to specify
> "--fw-version" option and what happens (or not happen) if the option
> is not there.
> I think all the text here can be simply merged in "Creating a capsule file".
OK, I will update.
Thanks,
Masahisa Kojima
>
> -Takahiro Akashi
>
>
> > +.. code-block:: console
> > +
> > + $ mkeficapsule --monotonic-count 1 \
> > + --private-key CRT.key \
> > + --certificate CRT.crt \
> > + --index 1 --instance 0 \
> > + --fw-version 5 \
> > + [--fit | --raw | --guid <guid-string] \
> > + <image_blob> <capsule_file_name>
> > +
> > Executing the boot manager
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 8/8] doc: uefi: add anti-rollback documentation
2023-05-19 10:32 [PATCH v6 0/8] FMP versioning support Masahisa Kojima
` (6 preceding siblings ...)
2023-05-19 10:32 ` [PATCH v6 7/8] doc: uefi: add firmware versioning documentation Masahisa Kojima
@ 2023-05-19 10:32 ` Masahisa Kojima
2023-05-22 0:27 ` Takahiro Akashi
2023-05-22 4:32 ` [PATCH v6 0/8] FMP versioning support Masahisa Kojima
2023-05-28 8:54 ` Heinrich Schuchardt
9 siblings, 1 reply; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-19 10:32 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Takahiro Akashi, Masahisa Kojima
This commit describe the procedure to configure lowest supported
version in the device tree for anti-rollback protection.
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Newly created in v6
doc/develop/uefi/uefi.rst | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index efab0fc7b1..f1f13bb993 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -539,6 +539,38 @@ since FMP protocol handles multiple image indexes.
[--fit | --raw | --guid <guid-string] \
<image_blob> <capsule_file_name>
+Anti-rollback Protection
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+The anti-rollback protection is implemented differently from firmware versioning.
+U-Boot implements the file-based EFI variable storage, it can be tampered
+and not the right place to store the lowest supported version.
+U-Boot uses device tree to store the lowest supported version, it is secured
+as long as dtb is authenticated together with U-Boot image by the authenticated
+capsule update, and the former stage boot loader verifies the image containing the dtb
+when the system boots.
+
+1. Insert the lowest supported version into a device tree
+
+.. code-block:: console
+
+ $ dtc -@ -I dts -O dtb -o version.dtbo version.dts
+ $ fdtoverlay -i orig.dtb -o new.dtb -v version.dtbo
+
+where version.dts looks like::
+
+ /dts-v1/;
+ /plugin/;
+ &{/} {
+ firmware-version {
+ image1 {
+ image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
+ image-index = <1>;
+ lowest-supported-version = <3>;
+ };
+ };
+ };
+
Executing the boot manager
~~~~~~~~~~~~~~~~~~~~~~~~~~
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 8/8] doc: uefi: add anti-rollback documentation
2023-05-19 10:32 ` [PATCH v6 8/8] doc: uefi: add anti-rollback documentation Masahisa Kojima
@ 2023-05-22 0:27 ` Takahiro Akashi
2023-05-22 4:30 ` Masahisa Kojima
0 siblings, 1 reply; 27+ messages in thread
From: Takahiro Akashi @ 2023-05-22 0:27 UTC (permalink / raw)
To: Masahisa Kojima
Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass
Hi Kojima-san,
On Fri, May 19, 2023 at 07:32:14PM +0900, Masahisa Kojima wrote:
> This commit describe the procedure to configure lowest supported
> version in the device tree for anti-rollback protection.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Newly created in v6
>
> doc/develop/uefi/uefi.rst | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index efab0fc7b1..f1f13bb993 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -539,6 +539,38 @@ since FMP protocol handles multiple image indexes.
> [--fit | --raw | --guid <guid-string] \
> <image_blob> <capsule_file_name>
>
> +Anti-rollback Protection
> +~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The anti-rollback protection is implemented differently from firmware versioning.
> +U-Boot implements the file-based EFI variable storage, it can be tampered
> +and not the right place to store the lowest supported version.
> +U-Boot uses device tree to store the lowest supported version, it is secured
> +as long as dtb is authenticated together with U-Boot image by the authenticated
> +capsule update, and the former stage boot loader verifies the image containing the dtb
> +when the system boots.
This is details of implementation.
You should rather mention the usage, i.e. how "anti-rollback" can be managed
and achieved using firmware-version and lowest-supported-version and that users
should always update their device tree to enforce the protection.
(If the lowest-supported-version is kept the same even after the firmware update,
anti-rollback won't work.)
-Takahiro Akashi
> +1. Insert the lowest supported version into a device tree
> +
> +.. code-block:: console
> +
> + $ dtc -@ -I dts -O dtb -o version.dtbo version.dts
> + $ fdtoverlay -i orig.dtb -o new.dtb -v version.dtbo
> +
> +where version.dts looks like::
> +
> + /dts-v1/;
> + /plugin/;
> + &{/} {
> + firmware-version {
> + image1 {
> + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> + image-index = <1>;
> + lowest-supported-version = <3>;
> + };
> + };
> + };
> +
> Executing the boot manager
> ~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6 8/8] doc: uefi: add anti-rollback documentation
2023-05-22 0:27 ` Takahiro Akashi
@ 2023-05-22 4:30 ` Masahisa Kojima
0 siblings, 0 replies; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-22 4:30 UTC (permalink / raw)
To: Takahiro Akashi, Masahisa Kojima, u-boot, Heinrich Schuchardt,
Ilias Apalodimas, Simon Glass
On Mon, 22 May 2023 at 09:27, Takahiro Akashi
<takahiro.akashi@linaro.org> wrote:
>
> Hi Kojima-san,
>
> On Fri, May 19, 2023 at 07:32:14PM +0900, Masahisa Kojima wrote:
> > This commit describe the procedure to configure lowest supported
> > version in the device tree for anti-rollback protection.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> > Newly created in v6
> >
> > doc/develop/uefi/uefi.rst | 32 ++++++++++++++++++++++++++++++++
> > 1 file changed, 32 insertions(+)
> >
> > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> > index efab0fc7b1..f1f13bb993 100644
> > --- a/doc/develop/uefi/uefi.rst
> > +++ b/doc/develop/uefi/uefi.rst
> > @@ -539,6 +539,38 @@ since FMP protocol handles multiple image indexes.
> > [--fit | --raw | --guid <guid-string] \
> > <image_blob> <capsule_file_name>
> >
> > +Anti-rollback Protection
> > +~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +The anti-rollback protection is implemented differently from firmware versioning.
> > +U-Boot implements the file-based EFI variable storage, it can be tampered
> > +and not the right place to store the lowest supported version.
> > +U-Boot uses device tree to store the lowest supported version, it is secured
> > +as long as dtb is authenticated together with U-Boot image by the authenticated
> > +capsule update, and the former stage boot loader verifies the image containing the dtb
> > +when the system boots.
>
> This is details of implementation.
> You should rather mention the usage, i.e. how "anti-rollback" can be managed
> and achieved using firmware-version and lowest-supported-version and that users
> should always update their device tree to enforce the protection.
OK, I will clearly describe how the lowest-supported-version interacts
with fw_version.
Thanks,
Masahisa Kojima
> (If the lowest-supported-version is kept the same even after the firmware update,
> anti-rollback won't work.)
>
> -Takahiro Akashi
>
> > +1. Insert the lowest supported version into a device tree
> > +
> > +.. code-block:: console
> > +
> > + $ dtc -@ -I dts -O dtb -o version.dtbo version.dts
> > + $ fdtoverlay -i orig.dtb -o new.dtb -v version.dtbo
> > +
> > +where version.dts looks like::
> > +
> > + /dts-v1/;
> > + /plugin/;
> > + &{/} {
> > + firmware-version {
> > + image1 {
> > + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> > + image-index = <1>;
> > + lowest-supported-version = <3>;
> > + };
> > + };
> > + };
> > +
> > Executing the boot manager
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v6 0/8] FMP versioning support
2023-05-19 10:32 [PATCH v6 0/8] FMP versioning support Masahisa Kojima
` (7 preceding siblings ...)
2023-05-19 10:32 ` [PATCH v6 8/8] doc: uefi: add anti-rollback documentation Masahisa Kojima
@ 2023-05-22 4:32 ` Masahisa Kojima
2023-05-28 8:54 ` Heinrich Schuchardt
9 siblings, 0 replies; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-22 4:32 UTC (permalink / raw)
To: u-boot; +Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Takahiro Akashi
On Fri, 19 May 2023 at 19:32, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>
> Firmware version management is not implemented in the current
> FMP implementation. This series aims to add the versioning support
> in FMP.
>
> Currently, there is no way to know the current running firmware
> version through the EFI interface. FMP->GetImageInfo() returns
> always 0 for the version number. So a user can not know that
> expected firmware is running after the capsule update.
>
> EDK II reference implementation utilizes the FMP Payload Header
> inserted right before the capsule payload.
> U-Boot also follows the EDK II implementation.
> With this series applied, version number can be specified
> in the capsule file generation with mkeficapsule tool, then
> user can know the running firmware version through
> FMP->GetImageInfo() and ESRT.
>
> There is a design consideration for lowest supported version.
> If the backing storage is a file we can't trust
> any of that information since anyone can tamper with the file,
> although the variables are defined as RO.
> With that, we store the lowest supported version in the device tree.
> We can trust the information from dtb as long as the former
> stage boot loader verifies the image containing the dtb.
> The firmware version can not be stored in device tree because
> not all the capsule files do not have a device tree.
>
> Note that this series does not mandate the FMP Payload Header,
> compatible with boards that are already using the existing
> U-Boot FMP implementation.
> If no FMP Payload Header is found in the capsule file, fw_version,
> lowest supported version, last attempt version and last attempt
> status is set to 0 and this is the same behavior as existing FMP
> implementation.
>
> Major Changes in v6:
> - change the location of fw_version and lowest supported version
> - fw_version is stored in FMP Payload Header in the capsule file
> - lowest_supported_version is stored in the device tree
I forgot to notice that this series does not include python tests.
After we agree on the design, python tests will follow.
- fw_version is in FMP Payload Header of the capsule file
- lowest-supported-version is in the dtb
Thanks,
Masahisa Kojima
>
> Major Changes in v5:
> - major design changes, versioning is implemented with
> device tree instead of EFI variable
>
> Major Changes in v4:
> - add python-based test
>
> Major Changes in v3:
> - exclude CONFIG_FWU_MULT
>
> Masahisa Kojima (8):
> efi_loader: add the number of image entries in efi_capsule_update_info
> efi_loader: store firmware version into FmpState variable
> efi_loader: versioning support in GetImageInfo
> efi_loader: get lowest supported version from device tree
> efi_loader: check lowest supported version
> mkeficapsule: add FMP Payload Header
> doc: uefi: add firmware versioning documentation
> doc: uefi: add anti-rollback documentation
>
> arch/arm/mach-rockchip/board.c | 4 +-
> .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 2 +-
> .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 2 +-
> board/emulation/qemu-arm/qemu-arm.c | 2 +-
> board/kontron/pitx_imx8m/pitx_imx8m.c | 2 +-
> board/kontron/sl-mx8mm/sl-mx8mm.c | 2 +-
> board/kontron/sl28/sl28.c | 2 +-
> board/rockchip/evb_rk3399/evb-rk3399.c | 2 +-
> board/sandbox/sandbox.c | 2 +-
> board/socionext/developerbox/developerbox.c | 2 +-
> board/st/stm32mp1/stm32mp1.c | 2 +-
> board/xilinx/common/board.c | 2 +-
> doc/develop/uefi/uefi.rst | 61 ++++
> .../firmware/firmware-version.txt | 22 ++
> doc/mkeficapsule.1 | 10 +
> include/efi_loader.h | 3 +-
> lib/efi_loader/efi_firmware.c | 273 ++++++++++++++++--
> lib/fwu_updates/fwu.c | 2 +-
> tools/eficapsule.h | 30 ++
> tools/mkeficapsule.c | 37 ++-
> 20 files changed, 421 insertions(+), 43 deletions(-)
> create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6 0/8] FMP versioning support
2023-05-19 10:32 [PATCH v6 0/8] FMP versioning support Masahisa Kojima
` (8 preceding siblings ...)
2023-05-22 4:32 ` [PATCH v6 0/8] FMP versioning support Masahisa Kojima
@ 2023-05-28 8:54 ` Heinrich Schuchardt
2023-05-30 0:32 ` Masahisa Kojima
9 siblings, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2023-05-28 8:54 UTC (permalink / raw)
To: Masahisa Kojima; +Cc: Ilias Apalodimas, Simon Glass, Takahiro Akashi, u-boot
On 5/19/23 12:32, Masahisa Kojima wrote:
> Firmware version management is not implemented in the current
> FMP implementation. This series aims to add the versioning support
> in FMP.
>
> Currently, there is no way to know the current running firmware
> version through the EFI interface. FMP->GetImageInfo() returns
> always 0 for the version number. So a user can not know that
> expected firmware is running after the capsule update.
>
> EDK II reference implementation utilizes the FMP Payload Header
> inserted right before the capsule payload.
> U-Boot also follows the EDK II implementation.
> With this series applied, version number can be specified
> in the capsule file generation with mkeficapsule tool, then
> user can know the running firmware version through
> FMP->GetImageInfo() and ESRT.
>
> There is a design consideration for lowest supported version.
> If the backing storage is a file we can't trust
> any of that information since anyone can tamper with the file,
> although the variables are defined as RO.
> With that, we store the lowest supported version in the device tree.
> We can trust the information from dtb as long as the former
> stage boot loader verifies the image containing the dtb.
> The firmware version can not be stored in device tree because
> not all the capsule files do not have a device tree.
>
> Note that this series does not mandate the FMP Payload Header,
> compatible with boards that are already using the existing
> U-Boot FMP implementation.
> If no FMP Payload Header is found in the capsule file, fw_version,
> lowest supported version, last attempt version and last attempt
> status is set to 0 and this is the same behavior as existing FMP
> implementation.
>
> Major Changes in v6:
> - change the location of fw_version and lowest supported version
> - fw_version is stored in FMP Payload Header in the capsule file
> - lowest_supported_version is stored in the device tree
>
> Major Changes in v5:
> - major design changes, versioning is implemented with
> device tree instead of EFI variable
>
> Major Changes in v4:
> - add python-based test
>
> Major Changes in v3:
> - exclude CONFIG_FWU_MULT
>
> Masahisa Kojima (8):
> efi_loader: add the number of image entries in efi_capsule_update_info
> efi_loader: store firmware version into FmpState variable
> efi_loader: versioning support in GetImageInfo
> efi_loader: get lowest supported version from device tree
> efi_loader: check lowest supported version
> mkeficapsule: add FMP Payload Header
> doc: uefi: add firmware versioning documentation
> doc: uefi: add anti-rollback documentation
>
> arch/arm/mach-rockchip/board.c | 4 +-
> .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 2 +-
> .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 2 +-
> board/emulation/qemu-arm/qemu-arm.c | 2 +-
> board/kontron/pitx_imx8m/pitx_imx8m.c | 2 +-
> board/kontron/sl-mx8mm/sl-mx8mm.c | 2 +-
> board/kontron/sl28/sl28.c | 2 +-
> board/rockchip/evb_rk3399/evb-rk3399.c | 2 +-
> board/sandbox/sandbox.c | 2 +-
> board/socionext/developerbox/developerbox.c | 2 +-
> board/st/stm32mp1/stm32mp1.c | 2 +-
> board/xilinx/common/board.c | 2 +-
> doc/develop/uefi/uefi.rst | 61 ++++
> .../firmware/firmware-version.txt | 22 ++
> doc/mkeficapsule.1 | 10 +
> include/efi_loader.h | 3 +-
> lib/efi_loader/efi_firmware.c | 273 ++++++++++++++++--
> lib/fwu_updates/fwu.c | 2 +-
> tools/eficapsule.h | 30 ++
> tools/mkeficapsule.c | 37 ++-
> 20 files changed, 421 insertions(+), 43 deletions(-)
> create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
>
Why are there no changes in directory test/ ?
Could you add a test for the rollback protection?
Best regards
Heinrich
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6 0/8] FMP versioning support
2023-05-28 8:54 ` Heinrich Schuchardt
@ 2023-05-30 0:32 ` Masahisa Kojima
0 siblings, 0 replies; 27+ messages in thread
From: Masahisa Kojima @ 2023-05-30 0:32 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Ilias Apalodimas, Simon Glass, Takahiro Akashi, u-boot
On Sun, 28 May 2023 at 17:54, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 5/19/23 12:32, Masahisa Kojima wrote:
> > Firmware version management is not implemented in the current
> > FMP implementation. This series aims to add the versioning support
> > in FMP.
> >
> > Currently, there is no way to know the current running firmware
> > version through the EFI interface. FMP->GetImageInfo() returns
> > always 0 for the version number. So a user can not know that
> > expected firmware is running after the capsule update.
> >
> > EDK II reference implementation utilizes the FMP Payload Header
> > inserted right before the capsule payload.
> > U-Boot also follows the EDK II implementation.
> > With this series applied, version number can be specified
> > in the capsule file generation with mkeficapsule tool, then
> > user can know the running firmware version through
> > FMP->GetImageInfo() and ESRT.
> >
> > There is a design consideration for lowest supported version.
> > If the backing storage is a file we can't trust
> > any of that information since anyone can tamper with the file,
> > although the variables are defined as RO.
> > With that, we store the lowest supported version in the device tree.
> > We can trust the information from dtb as long as the former
> > stage boot loader verifies the image containing the dtb.
> > The firmware version can not be stored in device tree because
> > not all the capsule files do not have a device tree.
> >
> > Note that this series does not mandate the FMP Payload Header,
> > compatible with boards that are already using the existing
> > U-Boot FMP implementation.
> > If no FMP Payload Header is found in the capsule file, fw_version,
> > lowest supported version, last attempt version and last attempt
> > status is set to 0 and this is the same behavior as existing FMP
> > implementation.
> >
> > Major Changes in v6:
> > - change the location of fw_version and lowest supported version
> > - fw_version is stored in FMP Payload Header in the capsule file
> > - lowest_supported_version is stored in the device tree
> >
> > Major Changes in v5:
> > - major design changes, versioning is implemented with
> > device tree instead of EFI variable
> >
> > Major Changes in v4:
> > - add python-based test
> >
> > Major Changes in v3:
> > - exclude CONFIG_FWU_MULT
> >
> > Masahisa Kojima (8):
> > efi_loader: add the number of image entries in efi_capsule_update_info
> > efi_loader: store firmware version into FmpState variable
> > efi_loader: versioning support in GetImageInfo
> > efi_loader: get lowest supported version from device tree
> > efi_loader: check lowest supported version
> > mkeficapsule: add FMP Payload Header
> > doc: uefi: add firmware versioning documentation
> > doc: uefi: add anti-rollback documentation
> >
> > arch/arm/mach-rockchip/board.c | 4 +-
> > .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 2 +-
> > .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 2 +-
> > board/emulation/qemu-arm/qemu-arm.c | 2 +-
> > board/kontron/pitx_imx8m/pitx_imx8m.c | 2 +-
> > board/kontron/sl-mx8mm/sl-mx8mm.c | 2 +-
> > board/kontron/sl28/sl28.c | 2 +-
> > board/rockchip/evb_rk3399/evb-rk3399.c | 2 +-
> > board/sandbox/sandbox.c | 2 +-
> > board/socionext/developerbox/developerbox.c | 2 +-
> > board/st/stm32mp1/stm32mp1.c | 2 +-
> > board/xilinx/common/board.c | 2 +-
> > doc/develop/uefi/uefi.rst | 61 ++++
> > .../firmware/firmware-version.txt | 22 ++
> > doc/mkeficapsule.1 | 10 +
> > include/efi_loader.h | 3 +-
> > lib/efi_loader/efi_firmware.c | 273 ++++++++++++++++--
> > lib/fwu_updates/fwu.c | 2 +-
> > tools/eficapsule.h | 30 ++
> > tools/mkeficapsule.c | 37 ++-
> > 20 files changed, 421 insertions(+), 43 deletions(-)
> > create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
> >
>
> Why are there no changes in directory test/ ?
> Could you add a test for the rollback protection?
Yes, I will include the test in the next version.
Thanks,
Masahisa Kojima
>
> Best regards
>
> Heinrich
^ permalink raw reply [flat|nested] 27+ messages in thread