diff mbox series

[v2,1/2] fwu: fix fwu_get_image_index interface

Message ID 20231221095258.592275-2-masahisa.kojima@linaro.org
State Superseded
Headers show
Series fix FMP versioning for FWU multi bank update | expand

Commit Message

Masahisa Kojima Dec. 21, 2023, 9:52 a.m. UTC
The capsule update uses the DFU framework for updating
storage. fwu_get_image_index() currently returns the
image_index calculated by (dfu_alt_num + 1), but this is
different from the image_index in UEFI terminology.

Since capsule update implementation calls dfu_write_by_alt
function, it is better that FWU returns the dfu_alt_num.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
 include/fwu.h                 | 13 +++++--------
 lib/efi_loader/efi_firmware.c | 11 +++++++++--
 lib/fwu_updates/fwu.c         | 32 ++++++++++++--------------------
 3 files changed, 26 insertions(+), 30 deletions(-)

Comments

Ilias Apalodimas Dec. 27, 2023, 2:37 p.m. UTC | #1
On Thu, 21 Dec 2023 at 11:54, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>
> The capsule update uses the DFU framework for updating
> storage. fwu_get_image_index() currently returns the
> image_index calculated by (dfu_alt_num + 1), but this is
> different from the image_index in UEFI terminology.
>
> Since capsule update implementation calls dfu_write_by_alt
> function, it is better that FWU returns the dfu_alt_num.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>  include/fwu.h                 | 13 +++++--------
>  lib/efi_loader/efi_firmware.c | 11 +++++++++--
>  lib/fwu_updates/fwu.c         | 32 ++++++++++++--------------------
>  3 files changed, 26 insertions(+), 30 deletions(-)
>
> diff --git a/include/fwu.h b/include/fwu.h
> index ac5c5de870..eb5638f4f3 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -122,21 +122,18 @@ int fwu_get_active_index(uint *active_idxp);
>  int fwu_set_active_index(uint active_idx);
>
>  /**
> - * fwu_get_image_index() - Get the Image Index to be used for capsule update
> - * @image_index: The Image Index for the image
> - *
> - * The FWU multi bank update feature computes the value of image_index at
> - * runtime, based on the bank to which the image needs to be written to.
> - * Derive the image_index value for the image.
> + * fwu_get_dfu_alt_num() - Get the dfu_alt_num to be used for capsule update
> + * @image_index:       The Image Index for the image
> + * @alt_num:           pointer to store dfu_alt_num
>   *
>   * Currently, the capsule update driver uses the DFU framework for
>   * the updates. This function gets the DFU alt number which is to
> - * be used as the Image Index
> + * be used for capsule update.
>   *
>   * Return: 0 if OK, -ve on error
>   *
>   */
> -int fwu_get_image_index(u8 *image_index);
> +int fwu_get_dfu_alt_num(u8 image_index, u8 *alt_num);
>
>  /**
>   * fwu_revert_boot_index() - Revert the active index in the FWU metadata
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 9abb29f1df..9b8630625f 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -611,6 +611,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
>         u16 **abort_reason)
>  {
>         int ret;
> +       u8 dfu_alt_num;
>         efi_status_t status;
>         struct fmp_state state = { 0 };
>
> @@ -625,19 +626,25 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
>         if (status != EFI_SUCCESS)
>                 return EFI_EXIT(status);
>
> +       /*
> +        * dfu_alt_num is assigned from 0 while image_index starts from 1.
> +        * dfu_alt_num is calculated by (image_index - 1) when multi bank update
> +        * is not used.
> +        */
> +       dfu_alt_num = image_index - 1;
>         if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
>                 /*
>                  * Based on the value of update bank, derive the
>                  * image index value.
>                  */
> -               ret = fwu_get_image_index(&image_index);
> +               ret = fwu_get_dfu_alt_num(image_index, &dfu_alt_num);
>                 if (ret) {
>                         log_debug("Unable to get FWU image_index\n");
>                         return EFI_EXIT(EFI_DEVICE_ERROR);
>                 }
>         }
>
> -       if (dfu_write_by_alt(image_index - 1, (void *)image, image_size,
> +       if (dfu_write_by_alt(dfu_alt_num, (void *)image, image_size,
>                              NULL, NULL))
>                 return EFI_EXIT(EFI_DEVICE_ERROR);
>
> diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> index b580574015..86518108c2 100644
> --- a/lib/fwu_updates/fwu.c
> +++ b/lib/fwu_updates/fwu.c
> @@ -125,16 +125,14 @@ static int in_trial_state(struct fwu_mdata *mdata)
>         return 0;
>  }
>
> -static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id)
> +static int fwu_get_image_type_id(u8 image_index, efi_guid_t *image_type_id)
>  {
> -       u8 index;
>         int i;
>         struct efi_fw_image *image;
>
> -       index = *image_index;
>         image = update_info.images;
>         for (i = 0; i < update_info.num_images; i++) {
> -               if (index == image[i].image_index) {
> +               if (image_index == image[i].image_index) {
>                         guidcpy(image_type_id, &image[i].image_type_id);
>                         return 0;
>                 }
> @@ -332,24 +330,20 @@ int fwu_set_active_index(uint active_idx)
>  }
>
>  /**
> - * fwu_get_image_index() - Get the Image Index to be used for capsule update
> - * @image_index: The Image Index for the image
> - *
> - * The FWU multi bank update feature computes the value of image_index at
> - * runtime, based on the bank to which the image needs to be written to.
> - * Derive the image_index value for the image.
> + * fwu_get_dfu_alt_num() - Get the dfu_alt_num to be used for capsule update
> + * @image_index:       The Image Index for the image
> + * @alt_num:           pointer to store dfu_alt_num
>   *
>   * Currently, the capsule update driver uses the DFU framework for
>   * the updates. This function gets the DFU alt number which is to
> - * be used as the Image Index
> + * be used for capsule update.
>   *
>   * Return: 0 if OK, -ve on error
>   *
>   */
> -int fwu_get_image_index(u8 *image_index)
> +int fwu_get_dfu_alt_num(u8 image_index, u8 *alt_num)
>  {
>         int ret, i;
> -       u8 alt_num;
>         uint update_bank;
>         efi_guid_t *image_guid, image_type_id;
>         struct fwu_mdata *mdata = &g_mdata;
> @@ -365,7 +359,7 @@ int fwu_get_image_index(u8 *image_index)
>         ret = fwu_get_image_type_id(image_index, &image_type_id);
>         if (ret) {
>                 log_debug("Unable to get image_type_id for image_index %u\n",
> -                         *image_index);
> +                         image_index);
>                 goto out;
>         }
>
> @@ -380,15 +374,13 @@ int fwu_get_image_index(u8 *image_index)
>                         img_entry = &mdata->img_entry[i];
>                         img_bank_info = &img_entry->img_bank_info[update_bank];
>                         image_guid = &img_bank_info->image_uuid;
> -                       ret = fwu_plat_get_alt_num(g_dev, image_guid, &alt_num);
> -                       if (ret) {
> +                       ret = fwu_plat_get_alt_num(g_dev, image_guid, alt_num);
> +                       if (ret)
>                                 log_debug("alt_num not found for partition with GUID %pUs\n",
>                                           image_guid);
> -                       } else {
> +                       else
>                                 log_debug("alt_num %d for partition %pUs\n",
> -                                         alt_num, image_guid);
> -                               *image_index = alt_num + 1;
> -                       }
> +                                         *alt_num, image_guid);
>
>                         goto out;
>                 }
> --
> 2.34.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff mbox series

Patch

diff --git a/include/fwu.h b/include/fwu.h
index ac5c5de870..eb5638f4f3 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -122,21 +122,18 @@  int fwu_get_active_index(uint *active_idxp);
 int fwu_set_active_index(uint active_idx);
 
 /**
- * fwu_get_image_index() - Get the Image Index to be used for capsule update
- * @image_index: The Image Index for the image
- *
- * The FWU multi bank update feature computes the value of image_index at
- * runtime, based on the bank to which the image needs to be written to.
- * Derive the image_index value for the image.
+ * fwu_get_dfu_alt_num() - Get the dfu_alt_num to be used for capsule update
+ * @image_index:	The Image Index for the image
+ * @alt_num:		pointer to store dfu_alt_num
  *
  * Currently, the capsule update driver uses the DFU framework for
  * the updates. This function gets the DFU alt number which is to
- * be used as the Image Index
+ * be used for capsule update.
  *
  * Return: 0 if OK, -ve on error
  *
  */
-int fwu_get_image_index(u8 *image_index);
+int fwu_get_dfu_alt_num(u8 image_index, u8 *alt_num);
 
 /**
  * fwu_revert_boot_index() - Revert the active index in the FWU metadata
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 9abb29f1df..9b8630625f 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -611,6 +611,7 @@  efi_status_t EFIAPI efi_firmware_raw_set_image(
 	u16 **abort_reason)
 {
 	int ret;
+	u8 dfu_alt_num;
 	efi_status_t status;
 	struct fmp_state state = { 0 };
 
@@ -625,19 +626,25 @@  efi_status_t EFIAPI efi_firmware_raw_set_image(
 	if (status != EFI_SUCCESS)
 		return EFI_EXIT(status);
 
+	/*
+	 * dfu_alt_num is assigned from 0 while image_index starts from 1.
+	 * dfu_alt_num is calculated by (image_index - 1) when multi bank update
+	 * is not used.
+	 */
+	dfu_alt_num = image_index - 1;
 	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
 		/*
 		 * Based on the value of update bank, derive the
 		 * image index value.
 		 */
-		ret = fwu_get_image_index(&image_index);
+		ret = fwu_get_dfu_alt_num(image_index, &dfu_alt_num);
 		if (ret) {
 			log_debug("Unable to get FWU image_index\n");
 			return EFI_EXIT(EFI_DEVICE_ERROR);
 		}
 	}
 
-	if (dfu_write_by_alt(image_index - 1, (void *)image, image_size,
+	if (dfu_write_by_alt(dfu_alt_num, (void *)image, image_size,
 			     NULL, NULL))
 		return EFI_EXIT(EFI_DEVICE_ERROR);
 
diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index b580574015..86518108c2 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -125,16 +125,14 @@  static int in_trial_state(struct fwu_mdata *mdata)
 	return 0;
 }
 
-static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id)
+static int fwu_get_image_type_id(u8 image_index, efi_guid_t *image_type_id)
 {
-	u8 index;
 	int i;
 	struct efi_fw_image *image;
 
-	index = *image_index;
 	image = update_info.images;
 	for (i = 0; i < update_info.num_images; i++) {
-		if (index == image[i].image_index) {
+		if (image_index == image[i].image_index) {
 			guidcpy(image_type_id, &image[i].image_type_id);
 			return 0;
 		}
@@ -332,24 +330,20 @@  int fwu_set_active_index(uint active_idx)
 }
 
 /**
- * fwu_get_image_index() - Get the Image Index to be used for capsule update
- * @image_index: The Image Index for the image
- *
- * The FWU multi bank update feature computes the value of image_index at
- * runtime, based on the bank to which the image needs to be written to.
- * Derive the image_index value for the image.
+ * fwu_get_dfu_alt_num() - Get the dfu_alt_num to be used for capsule update
+ * @image_index:	The Image Index for the image
+ * @alt_num:		pointer to store dfu_alt_num
  *
  * Currently, the capsule update driver uses the DFU framework for
  * the updates. This function gets the DFU alt number which is to
- * be used as the Image Index
+ * be used for capsule update.
  *
  * Return: 0 if OK, -ve on error
  *
  */
-int fwu_get_image_index(u8 *image_index)
+int fwu_get_dfu_alt_num(u8 image_index, u8 *alt_num)
 {
 	int ret, i;
-	u8 alt_num;
 	uint update_bank;
 	efi_guid_t *image_guid, image_type_id;
 	struct fwu_mdata *mdata = &g_mdata;
@@ -365,7 +359,7 @@  int fwu_get_image_index(u8 *image_index)
 	ret = fwu_get_image_type_id(image_index, &image_type_id);
 	if (ret) {
 		log_debug("Unable to get image_type_id for image_index %u\n",
-			  *image_index);
+			  image_index);
 		goto out;
 	}
 
@@ -380,15 +374,13 @@  int fwu_get_image_index(u8 *image_index)
 			img_entry = &mdata->img_entry[i];
 			img_bank_info = &img_entry->img_bank_info[update_bank];
 			image_guid = &img_bank_info->image_uuid;
-			ret = fwu_plat_get_alt_num(g_dev, image_guid, &alt_num);
-			if (ret) {
+			ret = fwu_plat_get_alt_num(g_dev, image_guid, alt_num);
+			if (ret)
 				log_debug("alt_num not found for partition with GUID %pUs\n",
 					  image_guid);
-			} else {
+			else
 				log_debug("alt_num %d for partition %pUs\n",
-					  alt_num, image_guid);
-				*image_index = alt_num + 1;
-			}
+					  *alt_num, image_guid);
 
 			goto out;
 		}