Hi Sugosh

On 11/25/21 8:01 AM, Sughosh Ganu wrote:

Add function to get the gpt header and partition entries filled. These would be used subsequently for multi bank firmware update support on devices where the images reside on GPT partitions.

Signed-off-by: Sughosh Ganu mailto:sughosh.ganu@linaro.org sughosh.ganu@linaro.org --- disk/part_efi.c | 10 ++++++++++ include/part.h | 14 ++++++++++++++ 2 files changed, 24 insertions(+)

diff --git a/disk/part_efi.c b/disk/part_efi.c index 0ca7effc32..792b9218a9 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -216,6 +216,16 @@ int get_disk_guid(struct blk_desc * dev_desc, char *guid) return 0; }

+int get_gpt_hdr_parts(struct blk_desc *desc, gpt_header *gpt_head, + gpt_entry **gpt_pte) +{ + /* This function validates and fills in the GPT header and PTE's */ + if (find_valid_gpt(desc, gpt_head, gpt_pte) != 1) + return -EINVAL; + + return 0; +} + void part_print_efi(struct blk_desc *dev_desc) { ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1, dev_desc->blksz); diff --git a/include/part.h b/include/part.h index b66b07a1f0..8e86485b97 100644 --- a/include/part.h +++ b/include/part.h @@ -345,6 +345,20 @@ struct part_driver {

#if CONFIG_IS_ENABLED(EFI_PARTITION) /* disk/part_efi.c */ + +/** + * get_gpt_hdr_parts() - Get information on the GPT Header and + * Partition Table Entries + * + * @param desc - block device descriptor + * @param gpt_h - pointer to GPT header representation

gpt_h need to be allocated / DMA alligned by caller

+ * @param gpt_e - pointer to GPT partition table entries

gpt_e is allocated in this fucntion need to freed by caller

+ * + * @return - zero on success, otherwise error + */ +int get_gpt_hdr_parts(struct blk_desc *desc, gpt_header *gpt_head, + gpt_entry **gpt_pte); + /** * write_gpt_table() - Write the GUID Partition Table to disk *

no need to export the low level function "find_valid_gpt"

the same services are already provided by part_get_info() ?

ST Restricted

2 information are used in the serie

=> gpt_head->num_partition_entries => &gpt_pte[i].partition_type_guid

type_guid is available in disk_partition, and num_partition_entries is not needed (replaced by MAX_SEARCH_PARTITIONS)

something like:

struct disk_partition info;

for (p = 1; p < MAX_SEARCH_PARTITIONS; p++) { if (part_get_info(desc, p, &info)) continue; => info.type_guid }

Regards

Patrick

On Wed, Dec 08, 2021 at 01:10:51PM +0530, Sughosh Ganu wrote:

hi Patrick,

On Tue, 7 Dec 2021 at 21:05, Patrick DELAUNAY patrick.delaunay@foss.st.com wrote:

...

Hi Sugosh

On 11/25/21 8:01 AM, Sughosh Ganu wrote:

Add function to get the gpt header and partition entries filled. These

would be used subsequently for multi bank firmware update support on

devices where the images reside on GPT partitions.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org sughosh.ganu@linaro.org

---

disk/part_efi.c | 10 ++++++++++

include/part.h | 14 ++++++++++++++

2 files changed, 24 insertions(+)

diff --git a/disk/part_efi.c b/disk/part_efi.c

index 0ca7effc32..792b9218a9 100644

--- a/disk/part_efi.c

+++ b/disk/part_efi.c

@@ -216,6 +216,16 @@ int get_disk_guid(struct blk_desc * dev_desc, char *guid)

return 0;

}

+int get_gpt_hdr_parts(struct blk_desc *desc, gpt_header *gpt_head,

•          gpt_entry **gpt_pte)
  
  

+{

• /* This function validates and fills in the GPT header and PTE's */

• if (find_valid_gpt(desc, gpt_head, gpt_pte) != 1)

•    return -EINVAL;
  
  

• return 0;

+}

void part_print_efi(struct blk_desc *dev_desc)

{

ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1, dev_desc->blksz);

diff --git a/include/part.h b/include/part.h

index b66b07a1f0..8e86485b97 100644

--- a/include/part.h

+++ b/include/part.h

@@ -345,6 +345,20 @@ struct part_driver {

#if CONFIG_IS_ENABLED(EFI_PARTITION)

/* disk/part_efi.c */

+/**

• • get_gpt_hdr_parts() - Get information on the GPT Header and

• •                   Partition Table Entries
    
    

• • @param desc - block device descriptor
• • @param gpt_h - pointer to GPT header representation

gpt_h need to be allocated / DMA alligned by caller

Will add this comment to the description.

...

• • @param gpt_e - pointer to GPT partition table entries

gpt_e is allocated in this fucntion need to freed by caller

Will add this comment to the description.

...

• • @return - zero on success, otherwise error

• */

+int get_gpt_hdr_parts(struct blk_desc *desc, gpt_header *gpt_head,

•          gpt_entry **gpt_pte);
  
  

/**

• write_gpt_table() - Write the GUID Partition Table to disk

no need to export the low level function "find_valid_gpt"

find_valid_gpt is not being exported. It is defined statically in part_efi.c. Do i misunderstand your comment?

...

the same services are already provided by part_get_info() ?

ST Restricted

2 information are used in the serie

=> gpt_head->num_partition_entries

=> &gpt_pte[i].partition_type_guid

type_guid is available in disk_partition,

and num_partition_entries is not needed (replaced by MAX_SEARCH_PARTITIONS)

something like:

    struct disk_partition info;



    for (p = 1; p < MAX_SEARCH_PARTITIONS; p++) {

           if (part_get_info(desc, p, &info))

                   continue;

           => info.type_guid

    }

I had actually explored using part_get_info. But the type_guid field is actually a char string. While I need the guid value to compare it using guidcmp. Which is the reason I introduced get_gpt_hdr_parts.

Think of using uuid_str_to_bin() if this is the only reason.

-Takahiro Akashi

-sughosh

...

Regards

Patrick

hi Etienne,

On Wed, 8 Dec 2021 at 18:44, Etienne Carriere etienne.carriere@linaro.org wrote:

On Thu, 25 Nov 2021 at 08:03, Sughosh Ganu sughosh.ganu@linaro.org wrote:

...

With the FWU multi bank update feature enabled, the dfu alt no that is used to identify the partition to be updated is derived at runtime and should match the partition number on the storage media. Achieve this by moving the ram partitions to the end of the dfu_alt_info variable.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

board/st/common/stm32mp_dfu.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/board/st/common/stm32mp_dfu.c

b/board/st/common/stm32mp_dfu.c

...

index 00d1fb8f59..394a2704ee 100644 --- a/board/st/common/stm32mp_dfu.c +++ b/board/st/common/stm32mp_dfu.c @@ -102,6 +102,7 @@ static void board_get_alt_info_mtd(struct mtd_info

*mtd, char *buf)

...

void set_dfu_alt_info(char *interface, char *devstr) {

•   int len;
    struct udevice *dev;
    struct mtd_info *mtd;
  
  

@@ -112,9 +113,6 @@ void set_dfu_alt_info(char *interface, char *devstr)

    memset(buf, 0, sizeof(buf));

•   snprintf(buf, DFU_ALT_BUF_LEN,
  

•            "ram 0=%s", CONFIG_DFU_ALT_RAM0);
  

•   if (CONFIG_IS_ENABLED(MMC)) {
            if (!uclass_get_device(UCLASS_MMC, 0, &dev))
                    board_get_alt_info_mmc(dev, buf);
  

@@ -151,6 +149,13 @@ void set_dfu_alt_info(char *interface, char *devstr) strncat(buf, "&virt 1=PMIC", DFU_ALT_BUF_LEN); }

•   len = strlen(buf);
  

•   if (buf[0] != '\0')
  

•           len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, "&");
  

•   snprintf(buf + len, DFU_ALT_BUF_LEN,
  

•            "ram 0=%s", CONFIG_DFU_ALT_RAM0);
  

s/DFU_ALT_BUF_LEN/DFU_ALT_BUF_LEN - len/

Should check snprintf() return value.

This patch would not be needed, if what Patrick suggested[1] works.

-sughosh

[1] - https://lists.denx.de/pipermail/u-boot/2021-December/469256.html

...

•   env_set("dfu_alt_info", buf);
    puts("DFU alt info setting: done\n");
  

}

2.17.1

Hi Sughosh,

On Thu, 25 Nov 2021 at 08:03, Sughosh Ganu sughosh.ganu@linaro.org wrote:

In the FWU Multi Bank Update feature, the information about the updatable images is stored as part of the metadata, which is stored on a dedicated partition. Add the metadata structure, and functions to access the metadata. These are generic API's, and implementations can be added based on parameters like how the metadata partition is accessed and what type of storage device houses the metadata.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

include/fwu_metadata.h | 125 +++++++++++++++ lib/fwu_updates/fwu_metadata.c | 275 +++++++++++++++++++++++++++++++++ 2 files changed, 400 insertions(+) create mode 100644 include/fwu_metadata.h create mode 100644 lib/fwu_updates/fwu_metadata.c

diff --git a/include/fwu_metadata.h b/include/fwu_metadata.h new file mode 100644 index 0000000000..e692ef7506 --- /dev/null +++ b/include/fwu_metadata.h @@ -0,0 +1,125 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Copyright (c) 2021, Linaro Limited
• */

+#if !defined _FWU_METADATA_H_

nitpicking: prefer #ifndef ?

+#define _FWU_METADATA_H_

+#include <blk.h> +#include <efi.h> +#include <uuid.h>

+#include <linux/types.h>

+/**

• • struct fwu_image_bank_info - firmware image information
• • @image_uuid: Guid value of the image in this bank
• • @accepted: Acceptance status of the image
• • @reserved: Reserved
• • The structure contains image specific fields which are
• • used to identify the image and to specify the image's
• • acceptance status
• */

+struct fwu_image_bank_info {

•   efi_guid_t  image_uuid;
  

•   u32 accepted;
  

•   u32 reserved;
  

+};

+/**

• • struct fwu_image_entry - information for a particular type of image
• • @image_type_uuid: Guid value for identifying the image type
• • @location_uuid: Guid of the storage volume where the image is located
• • @img_bank_info: Array containing properties of images
• • This structure contains information on various types of updatable
• • firmware images. Each image type then contains an array of image
• • information per bank.
• */

+struct fwu_image_entry {

•   efi_guid_t image_type_uuid;
  

•   efi_guid_t location_uuid;
  

•   struct fwu_image_bank_info img_bank_info[CONFIG_FWU_NUM_BANKS];
  

+};

+/**

• • struct fwu_metadata - Metadata structure for multi-bank updates
• • @crc32: crc32 value for the metadata
• • @version: Metadata version
• • @active_index: Index of the bank currently used for booting images
• • @previous_active_inde: Index of the bank used before the current bank

• •                    being used for booting
    

• • @img_entry: Array of information on various firmware images that can

• •         be updated
    

• • This structure is used to store all the needed information for performing
• • multi bank updates on the platform. This contains info on the bank being
• • used to boot along with the information needed for identification of
• • individual images
• */

+struct fwu_metadata {

•   u32 crc32;
  

•   u32 version;
  

•   u32 active_index;
  

•   u32 previous_active_index;
  

•   struct fwu_image_entry img_entry[CONFIG_FWU_NUM_IMAGES_PER_BANK];
  

+};

+/**

• • @get_active_index: get the current active_index value
• • @update_active_index: update the active_index value
• • @fill_partition_guid_array: fill the array with guid values of the

• •                         partitions found on the storage media
    

• • @get_image_alt_num: get the alt number to be used for the image
• • @metadata_check: check the validity of the metadata partitions
• • @revert_boot_index: set the active_index to previous_active_index
• • @set_accept_image: set the accepted bit for the image
• • @clear_accept_image: clear the accepted bit for the image
• • @get_metadata() - Get a metadata copy
• */

+struct fwu_metadata_ops {

•   int (*get_active_index)(u32 *active_idx);
  

•   int (*update_active_index)(u32 active_idx);
  

•   int (*fill_partition_guid_array)(efi_guid_t **part_guid_arr,
  

•                                    u32 *nparts);
  

•   int (*get_image_alt_num)(efi_guid_t image_type_id, u32 update_bank,
  

•                            int *alt_num);
  

•   int (*metadata_check)(void);
  

•   int (*revert_boot_index)(u32 *active_idx);
  

revert_active_index seems a better name (imho)

Is active_idx really needed as an output argument? One requiring this value could fetch it calling get_active_index ops afterward.

•   int (*set_accept_image)(efi_guid_t *img_type_id);
  

•   int (*clear_accept_image)(efi_guid_t *img_type_id, u32 bank);
  

•   int (*get_metadata)(struct fwu_metadata **metadata);
  

+};

+#define FWU_METADATA_GUID \

•   EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
  

•            0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
  

+#define FWU_METADATA_VERSION 0x1

+extern struct fwu_metadata_ops fwu_gpt_blk_ops;

+struct fwu_metadata_ops *get_plat_fwu_metadata_ops(void); +int fwu_get_active_index(u32 *active_idx); +int fwu_update_active_index(u32 active_idx); +int fwu_fill_partition_guid_array(efi_guid_t **part_guid_arr, u32 *nparts); +int fwu_get_image_alt_num(efi_guid_t image_type_id, u32 update_bank,

•                     int *alt_num);
  

+int fwu_metadata_check(void); +int fwu_revert_boot_index(u32 *active_idx); +int fwu_accept_image(efi_guid_t *img_type_id); +int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank); +int fwu_get_metadata(struct fwu_metadata **metadata);

+#endif /* _FWU_METADATA_H_ */ diff --git a/lib/fwu_updates/fwu_metadata.c b/lib/fwu_updates/fwu_metadata.c new file mode 100644 index 0000000000..ebc3eaa04a --- /dev/null +++ b/lib/fwu_updates/fwu_metadata.c @@ -0,0 +1,275 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (c) 2021, Linaro Limited
• */

+#include <fwu_metadata.h>

+#include <linux/errno.h> +#include <linux/types.h>

+static inline struct fwu_metadata_ops *get_fwu_metadata_ops(void)

static inline in a C-source file?

+{

•   return get_plat_fwu_metadata_ops();
  

Can't we use fwu_gpt_blk_ops reference straight in the below functions?

Regards, Etienne

+}

+/**

• • fwu_get_active_index() - Get active_index from the metadata
• • @active_idx: active_index value to be read
• • Read the active_index field from the metadata and place it in
• • the variable pointed to be the function argument.
• • Return: 0 if OK, -ve on error
• */

+int fwu_get_active_index(u32 *active_idx) +{

•   struct fwu_metadata_ops *ops;
  

•   ops = get_fwu_metadata_ops();
  

•   if (!ops) {
  

•           log_err("Unable to get fwu ops\n");
  

•           return -EPROTONOSUPPORT;
  

•   }
  

•   if (!ops->get_active_index) {
  

•           log_err("get_active_index() method not defined for the platform\n");
  

•           return -ENOSYS;
  

•   }
  

•   return ops->get_active_index(active_idx);
  

+}

+/**

• • fwu_update_active_index() - Update active_index from the metadata
• • @active_idx: active_index value to be updated
• • Update the active_index field in the metadata
• • Return: 0 if OK, -ve on error
• */

+int fwu_update_active_index(u32 active_idx) +{

•   struct fwu_metadata_ops *ops;
  

•   ops = get_fwu_metadata_ops();
  

•   if (!ops) {
  

•           log_err("Unable to get fwu ops\n");
  

•           return -EPROTONOSUPPORT;
  

•   }
  

•   if (!ops->update_active_index) {
  

•           log_err("update_active_index() method not defined for the platform\n");
  

•           return -ENOSYS;
  

•   }
  

•   return ops->update_active_index(active_idx);
  

+}

+/**

• • fwu_fill_partition_guid_array() - Fill the part_guid_arr array with the guid's of

• •                               the partitions
    

• • @part_guid_arr: array of partition guid's
• • @nparts: Number of gpt partitions on the device
• • Get the information on the partition guid's, filling the array with the guid
• • values and also the number of partitions.
• • Return: 0 if OK, -ve on error
• */

+int fwu_fill_partition_guid_array(efi_guid_t **part_guid_arr, u32 *nparts) +{

•   struct fwu_metadata_ops *ops;
  

•   ops = get_fwu_metadata_ops();
  

•   if (!ops) {
  

•           log_err("Unable to get fwu ops\n");
  

•           return -EPROTONOSUPPORT;
  

•   }
  

•   if (!ops->fill_partition_guid_array) {
  

•           log_err("fill_partition_guid_array() method not defined for the platform\n");
  

•           return -ENOSYS;
  

•   }
  

•   return ops->fill_partition_guid_array(part_guid_arr, nparts);
  

+}

+/**

• • fwu_get_image_alt_num() - Get the dfu alt number to be used for capsule update
• • @image_type_id: image guid as passed in the capsule
• • @update_bank: Bank to which the update is to be made
• • @alt_num: The alt_num for the image
• • Based on the guid value passed in the capsule, along with the bank to which the
• • image needs to be updated, get the dfu alt number which will be used for the
• • capsule update
• • Return: 0 if OK, -ve on error
• */

+int fwu_get_image_alt_num(efi_guid_t image_type_id, u32 update_bank,

•                     int *alt_num)
  

+{

•   struct fwu_metadata_ops *ops;
  

•   ops = get_fwu_metadata_ops();
  

•   if (!ops) {
  

•           log_err("Unable to get fwu ops\n");
  

•           return -EPROTONOSUPPORT;
  

•   }
  

•   if (!ops->get_image_alt_num) {
  

•           log_err("get_image_alt_num() method not defined for the platform\n");
  

•           return -ENOSYS;
  

•   }
  

•   return ops->get_image_alt_num(image_type_id, update_bank, alt_num);
  

+}

+/**

• • fwu_metadata_check() - Check if the metadata is valid
• • Validate both copies of metadata. If one of the copies
• • has gone bad, restore it from the other bad copy.
• • Return: 0 if OK, -ve on error
• */

+int fwu_metadata_check(void) +{

•   struct fwu_metadata_ops *ops;
  

•   ops = get_fwu_metadata_ops();
  

•   if (!ops) {
  

•           log_err("Unable to get fwu ops\n");
  

•           return -EPROTONOSUPPORT;
  

•   }
  

•   if (!ops->metadata_check) {
  

•           log_err("metadata_check() method not defined for the platform\n");
  

•           return -ENOSYS;
  

•   }
  

•   return ops->metadata_check();
  

+}

+/**

• • fwu_revert_boot_index() - Revert the active index in the metadata
• • @active_idx: Value of the updated active_index
• • Revert the active_index value in the metadata, by swapping the values
• • of active_index and previous_active_index in both copies of the
• • metadata.
• • Return: 0 if OK, -ve on error
• */

+int fwu_revert_boot_index(u32 *active_idx) +{

•   struct fwu_metadata_ops *ops;
  

•   ops = get_fwu_metadata_ops();
  

•   if (!ops) {
  

•           log_err("Unable to get fwu ops\n");
  

•           return -EPROTONOSUPPORT;
  

•   }
  

•   if (!ops->revert_boot_index) {
  

•           log_err("revert_boot_index() method not defined for the platform\n");
  

•           return -ENOSYS;
  

•   }
  

•   return ops->revert_boot_index(active_idx);
  

+}

+/**

• • fwu_accept_image() - Set the Acceptance bit for the image
• • @img_type_id: Guid of the image type for which the accepted bit is to be

• •           cleared
    

• • Set the accepted bit for the image specified by the img_guid parameter. This
• • indicates acceptance of image for subsequent boots by some governing component
• • like OS(or firmware).
• • Return: 0 if OK, -ve on error
• */

+int fwu_accept_image(efi_guid_t *img_type_id) +{

•   struct fwu_metadata_ops *ops;
  

•   ops = get_fwu_metadata_ops();
  

•   if (!ops) {
  

•           log_err("Unable to get fwu ops\n");
  

•           return -EPROTONOSUPPORT;
  

•   }
  

•   if (!ops->set_accept_image) {
  

•           log_err("set_accept_image() method not defined for the platform\n");
  

•           return -ENOSYS;
  

•   }
  

•   return ops->set_accept_image(img_type_id);
  

+}

+/**

• • fwu_clear_accept_image() - Clear the Acceptance bit for the image
• • @img_type_id: Guid of the image type for which the accepted bit is to be

• •           cleared
    

• • Clear the accepted bit for the image type specified by the img_type_id parameter.
• • This function is called after the image has been updated. The accepted bit is
• • cleared to be set subsequently after passing the image acceptance criteria, by
• • either the OS(or firmware)
• • Return: 0 if OK, -ve on error
• */

+int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank) +{

•   struct fwu_metadata_ops *ops;
  

•   ops = get_fwu_metadata_ops();
  

•   if (!ops) {
  

•           log_err("Unable to get fwu ops\n");
  

•           return -EPROTONOSUPPORT;
  

•   }
  

•   if (!ops->clear_accept_image) {
  

•           log_err("clear_accept_image() method not defined for the platform\n");
  

•           return -ENOSYS;
  

•   }
  

•   return ops->clear_accept_image(img_type_id, bank);
  

+}

+/**

• • fwu_get_metadata() - Get a metadata copy
• • @metadata: Copy of the metadata
• • Get a valid copy of the metadata.
• • Return: 0 if OK, -ve on error
• */

+int fwu_get_metadata(struct fwu_metadata **metadata) +{

•   struct fwu_metadata_ops *ops;
  

•   ops = get_fwu_metadata_ops();
  

•   if (!ops) {
  

•           log_err("Unable to get fwu ops\n");
  

•           return -EPROTONOSUPPORT;
  

•   }
  

•   if (!ops->get_metadata) {
  

•           log_err("get_metadata() method not defined for the platform\n");
  

•           return -ENOSYS;
  

•   }
  

•   return ops->get_metadata(metadata);
  

+}

2.17.1

hi Patrick,

On Tue, 7 Dec 2021 at 19:54, Patrick DELAUNAY patrick.delaunay@foss.st.com wrote:

Hi Sughsoh,

On 11/25/21 8:01 AM, Sughosh Ganu wrote:

...

In the FWU Multi Bank Update feature, the information about the updatable images is stored as part of the metadata, on a separate partition. Add functions for reading from and writing to the metadata when the updatable images and the metadata are stored on a block device which is formated with GPT based partition scheme.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

lib/fwu_updates/fwu_metadata_gpt_blk.c | 716 +++++++++++++++++++++++++ 1 file changed, 716 insertions(+) create mode 100644 lib/fwu_updates/fwu_metadata_gpt_blk.c

diff --git a/lib/fwu_updates/fwu_metadata_gpt_blk.c

b/lib/fwu_updates/fwu_metadata_gpt_blk.c

...

new file mode 100644 index 0000000000..98cc53f706 --- /dev/null +++ b/lib/fwu_updates/fwu_metadata_gpt_blk.c @@ -0,0 +1,716 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (c) 2021, Linaro Limited
• */

+#include <blk.h> +#include <efi_loader.h> +#include <fwu_metadata.h> +#include <malloc.h> +#include <memalign.h> +#include <part.h> +#include <part_efi.h>

+#include <linux/errno.h> +#include <linux/types.h> +#include <u-boot/crc.h>

+#define PRIMARY_VALID 0x1 +#define SECONDARY_VALID 0x2

+#define MDATA_READ (u8)0x1 +#define MDATA_WRITE (u8)0x2

+#define IMAGE_ACCEPT_SET (u8)0x1 +#define IMAGE_ACCEPT_CLEAR (u8)0x2

+static int gpt_verify_metadata(struct fwu_metadata *metadata, bool

pri_part)

...

+{

• u32 calc_crc32;
  

• void *buf;
  

• buf = &metadata->version;
  

• calc_crc32 = crc32(0, buf, sizeof(*metadata) - sizeof(u32));
  

• if (calc_crc32 != metadata->crc32) {
  

•         log_err("crc32 check failed for %s metadata partition\n",
  

•                 pri_part ? "primary" : "secondary");
  

•         return -1;
  

• }
  

• return 0;
  

+}

+static int gpt_get_metadata_partitions(struct blk_desc *desc,

•                                u32 *primary_mpart,
  

•                                u32 *secondary_mpart)
  

+{

• int i, ret;
  

• u32 nparts, mparts;
  

• gpt_entry *gpt_pte = NULL;
  

• const efi_guid_t fwu_metadata_guid = FWU_METADATA_GUID;
  

• ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1,
  

•                              desc->blksz);
  

• ret = get_gpt_hdr_parts(desc, gpt_head, &gpt_pte);
  

• if (ret < 0) {
  

•         log_err("Error getting GPT header and partitions\n");
  

•         ret = -EIO;
  

•         goto out;
  

• }
  

• nparts = gpt_head->num_partition_entries;
  

• for (i = 0, mparts = 0; i < nparts; i++) {
  

•         if (!guidcmp(&fwu_metadata_guid,
  

•                      &gpt_pte[i].partition_type_guid)) {
  

•                 ++mparts;
  

•                 if (!*primary_mpart)
  

•                         *primary_mpart = i + 1;
  

•                 else
  

•                         *secondary_mpart = i + 1;
  

•         }
  

• }
  

• if (mparts != 2) {
  

•         log_err("Expect two copies of the metadata instead of
  

%d\n",

...

•                 mparts);
  

•         ret = -EINVAL;
  

• } else {
  

•         ret = 0;
  

• }
  

+out:

• free(gpt_pte);
  

• return ret;
  

+}

+static int gpt_get_metadata_disk_part(struct blk_desc *desc,

•                               struct disk_partition *info,
  

•                               u32 part_num)
  

+{

• int ret;
  

• char *metadata_guid_str = "8a7a84a0-8387-40f6-ab41-a8b9a5a60d23";
  

• ret = part_get_info(desc, part_num, info);
  

• if (ret < 0) {
  

•         log_err("Unable to get the partition info for the metadata
  

part %d",

...

•                 part_num);
  

•         return -1;
  

• }
  

• /* Check that it is indeed the metadata partition */
  

• if (!strncmp(info->type_guid, metadata_guid_str, UUID_STR_LEN)) {
  

•         /* Found the metadata partition */
  

•         return 0;
  

• }
  

• return -1;
  

+}

+static int gpt_read_write_metadata(struct blk_desc *desc,

•                            struct fwu_metadata *metadata,
  

•                            u8 access, u32 part_num)
  

+{

• int ret;
  

• u32 len, blk_start, blkcnt;
  

• struct disk_partition info;
  

• ALLOC_CACHE_ALIGN_BUFFER_PAD(struct fwu_metadata, mdata, 1,
  

desc->blksz);

...

• ret = gpt_get_metadata_disk_part(desc, &info, part_num);
  

• if (ret < 0) {
  

•         printf("Unable to get the metadata partition\n");
  

•         return -ENODEV;
  

• }
  

• len = sizeof(*metadata);
  

• blkcnt = BLOCK_CNT(len, desc);
  

• if (blkcnt > info.size) {
  

•         log_err("Block count exceeds metadata partition size\n");
  

•         return -ERANGE;
  

• }
  

• blk_start = info.start;
  

• if (access == MDATA_READ) {
  

•         if (blk_dread(desc, blk_start, blkcnt, mdata) != blkcnt) {
  

•                 log_err("Error reading metadata from the
  

device\n");

...

•                 return -EIO;
  

•         }
  

•         memcpy(metadata, mdata, sizeof(struct fwu_metadata));
  

• } else {
  

•         if (blk_dwrite(desc, blk_start, blkcnt, metadata) !=
  

blkcnt) {

...

•                 log_err("Error writing metadata to the device\n");
  

•                 return -EIO;
  

•         }
  

• }
  

• return 0;
  

+}

+static int gpt_read_metadata(struct blk_desc *desc,

•                      struct fwu_metadata *metadata, u32 part_num)
  

+{

• return gpt_read_write_metadata(desc, metadata, MDATA_READ,
  

part_num);

...

+}

+static int gpt_write_metadata_partition(struct blk_desc *desc,

•                                 struct fwu_metadata *metadata,
  

•                                 u32 part_num)
  

+{

• return gpt_read_write_metadata(desc, metadata, MDATA_WRITE,
  

part_num);

...

+}

+static int gpt_update_metadata(struct fwu_metadata *metadata) +{

• int ret;
  

• struct blk_desc *desc;
  

• u32 primary_mpart, secondary_mpart;
  

• ret = fwu_plat_get_blk_desc(&desc);
  

• if (ret < 0) {
  

•         log_err("Block device not found\n");
  

•         return -ENODEV;
  

• }
  

• primary_mpart = secondary_mpart = 0;
  

• ret = gpt_get_metadata_partitions(desc, &primary_mpart,
  

•                                   &secondary_mpart);
  

• if (ret < 0) {
  

•         log_err("Error getting the metadata partitions\n");
  

•         return -ENODEV;
  

• }
  

• /* First write the primary partition*/
  

• ret = gpt_write_metadata_partition(desc, metadata, primary_mpart);
  

• if (ret < 0) {
  

•         log_err("Updating primary metadata partition failed\n");
  

•         return ret;
  

• }
  

• /* And now the replica */
  

• ret = gpt_write_metadata_partition(desc, metadata,
  

secondary_mpart);

...

• if (ret < 0) {
  

•         log_err("Updating secondary metadata partition failed\n");
  

•         return ret;
  

• }
  

• return 0;
  

+}

+static int gpt_get_valid_metadata(struct fwu_metadata **metadata) +{

• int ret;
  

• struct blk_desc *desc;
  

• u32 primary_mpart, secondary_mpart;
  

• ret = fwu_plat_get_blk_desc(&desc);
  

• if (ret < 0) {
  

•         log_err("Block device not found\n");
  

•         return -ENODEV;
  

• }
  

• primary_mpart = secondary_mpart = 0;
  

• ret = gpt_get_metadata_partitions(desc, &primary_mpart,
  

•                                   &secondary_mpart);
  

• if (ret < 0) {
  

•         log_err("Error getting the metadata partitions\n");
  

•         return -ENODEV;
  

• }
  

• *metadata = malloc(sizeof(struct fwu_metadata));
  

• if (!*metadata) {
  

•         log_err("Unable to allocate memory for reading
  

metadata\n");

...

•         return -ENOMEM;
  

• }
  

• ret = gpt_read_metadata(desc, *metadata, primary_mpart);
  

• if (ret < 0) {
  

•         log_err("Failed to read the metadata from the device\n");
  

•         return -EIO;
  

• }
  

• ret = gpt_verify_metadata(*metadata, 1);
  

• if (!ret)
  

•         return 0;
  

• /*
  

•  * Verification of the primary metadata copy failed.
  

•  * Try to read the replica.
  

•  */
  

• memset(*metadata, 0, sizeof(struct fwu_metadata));
  

• ret = gpt_read_metadata(desc, *metadata, secondary_mpart);
  

• if (ret < 0) {
  

•         log_err("Failed to read the metadata from the device\n");
  

•         return -EIO;
  

• }
  

• ret = gpt_verify_metadata(*metadata, 0);
  

• if (!ret)
  

•         return 0;
  

• /* Both the metadata copies are corrupted. */
  

• return -1;
  

+}

+static int gpt_check_metadata_validity(void) +{

• int ret;
  

• struct blk_desc *desc;
  

• struct fwu_metadata *pri_metadata;
  

• struct fwu_metadata *secondary_metadata;
  

• u32 primary_mpart, secondary_mpart;
  

• u32 valid_partitions;
  

• ret = fwu_plat_get_blk_desc(&desc);
  

• if (ret < 0) {
  

•         log_err("Block device not found\n");
  

•         return -ENODEV;
  

• }
  

• /*
  

•  * Two metadata partitions are expected.
  

•  * If we don't have two, user needs to create
  

•  * them first
  

•  */
  

• primary_mpart = secondary_mpart = 0;
  

• valid_partitions = 0;
  

• ret = gpt_get_metadata_partitions(desc, &primary_mpart,
  

•                                   &secondary_mpart);
  

• if (ret < 0) {
  

•         log_err("Error getting the metadata partitions\n");
  

•         return -ENODEV;
  

• }
  

• pri_metadata = malloc(sizeof(*pri_metadata));
  

• if (!pri_metadata) {
  

•         log_err("Unable to allocate memory for reading
  

metadata\n");

...

•         return -ENOMEM;
  

• }
  

• secondary_metadata = malloc(sizeof(*secondary_metadata));
  

• if (!secondary_metadata) {
  

•         log_err("Unable to allocate memory for reading
  

metadata\n");

...

•         return -ENOMEM;
  

• }
  

• ret = gpt_read_metadata(desc, pri_metadata, primary_mpart);
  

• if (ret < 0) {
  

•         log_err("Failed to read the metadata from the device\n");
  

•         ret = -EIO;
  

•         goto out;
  

• }
  

• ret = gpt_verify_metadata(pri_metadata, 1);
  

• if (!ret)
  

•         valid_partitions |= PRIMARY_VALID;
  

• /* Now check the secondary partition */
  

• ret = gpt_read_metadata(desc, secondary_metadata, secondary_mpart);
  

• if (ret < 0) {
  

•         log_err("Failed to read the metadata from the device\n");
  

•         ret = -EIO;
  

•         goto out;
  

• }
  

• ret = gpt_verify_metadata(secondary_metadata, 0);
  

• if (!ret)
  

•         valid_partitions |= SECONDARY_VALID;
  

• if (valid_partitions == (PRIMARY_VALID | SECONDARY_VALID)) {
  

•         ret = -1;
  

•         /*
  

•          * Before returning, check that both the
  

•          * metadata copies are the same. If not,
  

•          * the metadata copies need to be
  

•          * re-populated.
  

•          */
  

•         if (!memcmp(pri_metadata, secondary_metadata,
  

•                     sizeof(*pri_metadata)))
  

•                 ret = 0;
  

•         goto out;
  

• } else if (valid_partitions == SECONDARY_VALID) {
  

•         ret = gpt_write_metadata_partition(desc,
  

secondary_metadata,

...

•                                            primary_mpart);
  

•         if (ret < 0) {
  

•                 log_err("Restoring primary metadata partition
  

failed\n");

...

•                 goto out;
  

•         }
  

• } else if (valid_partitions == PRIMARY_VALID) {
  

•         ret = gpt_write_metadata_partition(desc, pri_metadata,
  

•                                            secondary_mpart);
  

•         if (ret < 0) {
  

•                 log_err("Restoring secondary metadata partition
  

failed\n");

...

•                 goto out;
  

•         }
  

• } else {
  

•         ret = -1;
  

• }
  

+out:

• free(pri_metadata);
  

• return ret;
  

+}

+static int gpt_fill_partition_guid_array(struct blk_desc *desc,

•                                  efi_guid_t **part_guid_arr,
  

•                                  u32 *nparts)
  

+{

• int ret, i;
  

• u32 parts;
  

• gpt_entry *gpt_pte = NULL;
  

• const efi_guid_t null_guid = NULL_GUID;
  

• ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1,
  

•                              desc->blksz);
  

• ret = get_gpt_hdr_parts(desc, gpt_head, &gpt_pte);
  

• if (ret < 0) {
  

•         log_err("Error getting GPT header and partitions\n");
  

•         ret = -EIO;
  

•         goto out;
  

• }
  

• *nparts = gpt_head->num_partition_entries;
  

• /*
  

•  * There could be a scenario where the number of partition entries
  

•  * configured in the GPT header is the default value of 128. Find
  

•  * the actual number of populated partitioned entries
  

•  */
  

• for (i = 0, parts = 0; i < *nparts; i++) {
  

•         if (!guidcmp(&gpt_pte[i].partition_type_guid, &null_guid))
  

•                 continue;
  

•         ++parts;
  

• }
  

• *nparts = parts;
  

• *part_guid_arr = malloc(sizeof(efi_guid_t) * *nparts);
  

• if (!part_guid_arr) {
  

•         log_err("Unable to allocate memory for guid array\n");
  

•         ret = -ENOMEM;
  

•         goto out;
  

• }
  

• for (i = 0; i < *nparts; i++) {
  

•         guidcpy((*part_guid_arr + i),
  

•                 &gpt_pte[i].partition_type_guid);
  

• }
  

+out:

• free(gpt_pte);
  

• return ret;
  

+}

+int fwu_gpt_get_active_index(u32 *active_idx) +{

• int ret;
  

• struct fwu_metadata *metadata;
  

• ret = gpt_get_valid_metadata(&metadata);
  

• if (ret < 0) {
  

•         log_err("Unable to get valid metadata\n");
  

•         goto out;
  

• }
  

• /*
  

•  * Found the metadata partition, now read the active_index
  

•  * value
  

•  */
  

• *active_idx = metadata->active_index;
  

• if (*active_idx > CONFIG_FWU_NUM_BANKS - 1) {
  

•         printf("Active index value read is incorrect\n");
  

•         ret = -EINVAL;
  

•         goto out;
  

• }
  

+out:

• free(metadata);
  

• return ret;
  

+}

+static int gpt_get_image_alt_num(struct blk_desc *desc,

•                          efi_guid_t image_type_id,
  

•                          u32 update_bank, int *alt_no)
  

+{

• int ret, i;
  

• u32 nparts;
  

• gpt_entry *gpt_pte = NULL;
  

• struct fwu_metadata *metadata;
  

• struct fwu_image_entry *img_entry;
  

• struct fwu_image_bank_info *img_bank_info;
  

• efi_guid_t unique_part_guid;
  

• efi_guid_t image_guid = NULL_GUID;
  

• ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1,
  

•                              desc->blksz);
  

• ret = gpt_get_valid_metadata(&metadata);
  

• if (ret < 0) {
  

•         log_err("Unable to read valid metadata\n");
  

•         goto out;
  

• }
  

• /*
  

•  * The metadata has been read. Now get the image_uuid for the
  

•  * image with the update_bank.
  

•  */
  

• for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
  

•         if (!guidcmp(&image_type_id,
  

•                      &metadata->img_entry[i].image_type_uuid)) {
  

•                 img_entry = &metadata->img_entry[i];
  

•                 img_bank_info =
  

&img_entry->img_bank_info[update_bank];

...

•                 guidcpy(&image_guid, &img_bank_info->image_uuid);
  

•         }
  

• }
  

• /*
  

•  * Now read the GPT Partition Table Entries to find a matching
  

•  * partition with UniquePartitionGuid value. We need to iterate
  

•  * through all the GPT partitions since they might be in any
  

•  * order
  

•  */
  

• ret = get_gpt_hdr_parts(desc, gpt_head, &gpt_pte);
  

• if (ret < 0) {
  

•         log_err("Error getting GPT header and partitions\n");
  

•         ret = -EIO;
  

•         goto out;
  

• }
  

• nparts = gpt_head->num_partition_entries;
  

• for (i = 0; i < nparts; i++) {
  

•         unique_part_guid = gpt_pte[i].unique_partition_guid;
  

•         if (!guidcmp(&unique_part_guid, &image_guid)) {
  

•                 /* Found the partition */
  

•                 *alt_no = i + 1;
  

here we found the partition = <i>

but I am not sure of the hard-coded mapping here <alt> = <part_id> + 1

at least I think it should be configured for each board

for example with a weak function ?

*alt_no = board_get_dfu_alt_fwu(i);

with:

/* default => dfu alternate use the same order than parttion */ weak int board_get_dfu_alt_fwu(int part) { return part + 1; )

But I think we can make something more dynamic here by parsing DFU information to found the alternate associated to the requested partition

by example by using dfu_get_entity() => search on the "mmc" dev / part

int mmc_get_dfu_alt_fwu(int dev, int part) {

for (i = 0; i < nb_alt; i++) { dfu = dfu_get_entity(i); /* only SDCard / eMMC suport with GPT partitioning */ if (dfu->dev_type != DFU_DEV_MMC) continue; if (dfu->layout = DFU_RAW_ADDR && dfu->data.mmc.dev == dev && dfu->data.mmc.part == part) return }

this request can support boards with several MMC instances as the partitions are not the necessary the first one

for example trying to update partition in mmc 1 when partition on mmc 0 are the first one in dfu alternate.

Thanks for this suggestion. I will try it out on my side and incorporate it in my V2. This should also get rid of patch 2, Move the ram partitions to the end of the dfu_alt_info variable.

-sughosh

...

•                 break;
  

•         }
  

• }
  

• if (i == nparts) {
  

•         log_err("Partition with the image guid not found\n");
  

•         ret = -EINVAL;
  

• }
  

+out:

• free(metadata);
  

• free(gpt_pte);
  

• return ret;
  

+}

+int fwu_gpt_update_active_index(u32 active_idx) +{

• int ret;
  

• void *buf;
  

• u32 cur_active_index;
  

• struct fwu_metadata *metadata;
  

• if (active_idx > CONFIG_FWU_NUM_BANKS) {
  

•         printf("Active index value to be updated is incorrect\n");
  

•         return -1;
  

• }
  

• ret = gpt_get_valid_metadata(&metadata);
  

• if (ret < 0) {
  

•         log_err("Unable to get valid metadata\n");
  

•         goto out;
  

• }
  

• /*
  

•  * Update the active index and previous_active_index fields
  

•  * in the metadata
  

•  */
  

• cur_active_index = metadata->active_index;
  

• metadata->active_index = active_idx;
  

• metadata->previous_active_index = cur_active_index;
  

• /*
  

•  * Calculate the crc32 for the updated metadata
  

•  * and put the updated value in the metadata crc32
  

•  * field
  

•  */
  

• buf = &metadata->version;
  

• metadata->crc32 = crc32(0, buf, sizeof(*metadata) - sizeof(u32));
  

• /*
  

•  * Now write this updated metadata to both the
  

•  * metadata partitions
  

•  */
  

• ret = gpt_update_metadata(metadata);
  

• if (ret < 0) {
  

•         log_err("Failed to update metadata partitions\n");
  

•         ret = -EIO;
  

• }
  

+out:

• free(metadata);
  

• return ret;
  

+}

+int fwu_gpt_fill_partition_guid_array(efi_guid_t **part_guid_arr, u32

*nparts)

...

+{

• int ret;
  

• struct blk_desc *desc;
  

• ret = fwu_plat_get_blk_desc(&desc);
  

• if (ret < 0) {
  

•         log_err("Block device not found\n");
  

•         return -ENODEV;
  

• }
  

• return gpt_fill_partition_guid_array(desc, part_guid_arr, nparts);
  

+}

+int fwu_gpt_get_image_alt_num(efi_guid_t image_type_id, u32 update_bank,

•                       int *alt_no)
  

+{

• int ret;
  

• struct blk_desc *desc;
  

• ret = fwu_plat_get_blk_desc(&desc);
  

• if (ret < 0) {
  

•         log_err("Block device not found\n");
  

•         return -ENODEV;
  

• }
  

• return gpt_get_image_alt_num(desc, image_type_id, update_bank,
  

alt_no);

...

+}

+int fwu_gpt_metadata_check(void) +{

• /*
  

•  * Check if both the copies of the metadata are valid.
  

•  * If one has gone bad, restore it from the other good
  

•  * copy.
  

•  */
  

• return gpt_check_metadata_validity();
  

+}

+int fwu_gpt_get_metadata(struct fwu_metadata **metadata) +{

• return gpt_get_valid_metadata(metadata);
  

+}

+int fwu_gpt_revert_boot_index(u32 *active_idx) +{

• int ret;
  

• void *buf;
  

• u32 cur_active_index;
  

• struct fwu_metadata *metadata;
  

• ret = gpt_get_valid_metadata(&metadata);
  

• if (ret < 0) {
  

•         log_err("Unable to get valid metadata\n");
  

•         goto out;
  

• }
  

• /*
  

•  * Swap the active index and previous_active_index fields
  

•  * in the metadata
  

•  */
  

• cur_active_index = metadata->active_index;
  

• metadata->active_index = metadata->previous_active_index;
  

• metadata->previous_active_index = cur_active_index;
  

• *active_idx = metadata->active_index;
  

• /*
  

•  * Calculate the crc32 for the updated metadata
  

•  * and put the updated value in the metadata crc32
  

•  * field
  

•  */
  

• buf = &metadata->version;
  

• metadata->crc32 = crc32(0, buf, sizeof(*metadata) - sizeof(u32));
  

• /*
  

•  * Now write this updated metadata to both the
  

•  * metadata partitions
  

•  */
  

• ret = gpt_update_metadata(metadata);
  

• if (ret < 0) {
  

•         log_err("Failed to update metadata partitions\n");
  

•         ret = -EIO;
  

• }
  

+out:

• free(metadata);
  

• return ret;
  

+}

+static int fwu_gpt_set_clear_image_accept(efi_guid_t *img_type_id,

•                                   u32 bank, u8 action)
  

+{

• void *buf;
  

• int ret, i;
  

• u32 nimages;
  

• struct fwu_metadata *metadata;
  

• struct fwu_image_entry *img_entry;
  

• struct fwu_image_bank_info *img_bank_info;
  

• ret = gpt_get_valid_metadata(&metadata);
  

• if (ret < 0) {
  

•         log_err("Unable to get valid metadata\n");
  

•         goto out;
  

• }
  

• if (action == IMAGE_ACCEPT_SET)
  

•         bank = metadata->active_index;
  

• nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
  

• img_entry = &metadata->img_entry[0];
  

• for (i = 0; i < nimages; i++) {
  

•         if (!guidcmp(&img_entry[i].image_type_uuid, img_type_id)) {
  

•                 img_bank_info = &img_entry[i].img_bank_info[bank];
  

•                 if (action == IMAGE_ACCEPT_SET)
  

•                         img_bank_info->accepted |=
  

FWU_IMAGE_ACCEPTED;

...

•                 else
  

•                         img_bank_info->accepted = 0;
  

•                 buf = &metadata->version;
  

•                 metadata->crc32 = crc32(0, buf, sizeof(*metadata) -
  

•                                         sizeof(u32));
  

•                 ret = gpt_update_metadata(metadata);
  

•                 goto out;
  

•         }
  

• }
  

• /* Image not found */
  

• ret = -EINVAL;
  

+out:

• free(metadata);
  

• return ret;
  

+}

+int fwu_gpt_accept_image(efi_guid_t *img_type_id) +{

• return fwu_gpt_set_clear_image_accept(img_type_id, 0,
  

•                                       IMAGE_ACCEPT_SET);
  

+}

+int fwu_gpt_clear_accept_image(efi_guid_t *img_type_id, u32 bank) +{

• return fwu_gpt_set_clear_image_accept(img_type_id, bank,
  

•                                       IMAGE_ACCEPT_CLEAR);
  

+}

+struct fwu_metadata_ops fwu_gpt_blk_ops = {

• .get_active_index = fwu_gpt_get_active_index,
  

• .update_active_index = fwu_gpt_update_active_index,
  

• .fill_partition_guid_array = fwu_gpt_fill_partition_guid_array,
  

• .get_image_alt_num = fwu_gpt_get_image_alt_num,
  

• .metadata_check = fwu_gpt_metadata_check,
  

• .revert_boot_index = fwu_gpt_revert_boot_index,
  

• .set_accept_image = fwu_gpt_accept_image,
  

• .clear_accept_image = fwu_gpt_clear_accept_image,
  

• .get_metadata = fwu_gpt_get_metadata,
  

+};

Regards

Patrick

On Thu, 9 Dec 2021 at 15:06, Jason Liu jason.hui.liu@nxp.com wrote:

...

-----Original Message----- From: Sughosh Ganu sughosh.ganu@linaro.org Sent: 2021年11月25日 15:02 To: u-boot@lists.denx.de Cc: Patrick Delaunay patrick.delaunay@foss.st.com; Patrice Chotard patrice.chotard@foss.st.com; Heinrich Schuchardt xypron.glpk@gmx.de; Alexander Graf agraf@csgraf.de; Simon Glass sjg@chromium.org; Bin Meng bmeng.cn@gmail.com; Peng Fan peng.fan@nxp.com; AKASHI Takahiro takahiro.akashi@linaro.org; Ilias Apalodimas ilias.apalodimas@linaro.org; Jose Marinho jose.marinho@arm.com;

Grant

...

Likely grant.likely@arm.com; Jason Liu jason.hui.liu@nxp.com;

Sughosh

...

Ganu sughosh.ganu@linaro.org Subject: [RFC PATCH 04/10] FWU: Add metadata access functions for GPT partitioned block devices

In the FWU Multi Bank Update feature, the information about the updatable images is stored as part of the metadata, on a separate partition. Add functions for reading from and writing to the metadata when the updatable images and the metadata are stored on a block device which is formated

with

...

GPT based partition scheme.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

lib/fwu_updates/fwu_metadata_gpt_blk.c | 716 +++++++++++++++++++++++++ 1 file changed, 716 insertions(+) create mode 100644 lib/fwu_updates/fwu_metadata_gpt_blk.c

diff --git a/lib/fwu_updates/fwu_metadata_gpt_blk.c b/lib/fwu_updates/fwu_metadata_gpt_blk.c new file mode 100644 index 0000000000..98cc53f706 --- /dev/null +++ b/lib/fwu_updates/fwu_metadata_gpt_blk.c @@ -0,0 +1,716 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (c) 2021, Linaro Limited
• */

+#include <blk.h> +#include <efi_loader.h> +#include <fwu_metadata.h> +#include <malloc.h> +#include <memalign.h> +#include <part.h> +#include <part_efi.h>

+#include <linux/errno.h> +#include <linux/types.h> +#include <u-boot/crc.h>

+#define PRIMARY_VALID 0x1 +#define SECONDARY_VALID 0x2

+#define MDATA_READ (u8)0x1 +#define MDATA_WRITE (u8)0x2

+#define IMAGE_ACCEPT_SET (u8)0x1 +#define IMAGE_ACCEPT_CLEAR (u8)0x2

Better to define as the followings:

#define MDATA_READ ((u8)0x1) #define MDATA_WRITE ((u8)0x2)

#define IMAGE_ACCEPT_SET ((u8)0x1) #define IMAGE_ACCEPT_CLEAR ((u8)0x2)

Ilias had suggested using the BIT macros for this. I have made that change. Hope that is fine.

...

+static int gpt_verify_metadata(struct fwu_metadata *metadata, bool +pri_part) {

• u32 calc_crc32;
  

• void *buf;
  

• buf = &metadata->version;
  

• calc_crc32 = crc32(0, buf, sizeof(*metadata) - sizeof(u32));
  

• if (calc_crc32 != metadata->crc32) {
  

•         log_err("crc32 check failed for %s metadata partition\n",
  

•                 pri_part ? "primary" : "secondary");
  

•         return -1;
  

• }
  

• return 0;
  

+}

+static int gpt_get_metadata_partitions(struct blk_desc *desc,

•                                u32 *primary_mpart,
  

•                                u32 *secondary_mpart)
  

+{

• int i, ret;
  

• u32 nparts, mparts;
  

• gpt_entry *gpt_pte = NULL;
  

• const efi_guid_t fwu_metadata_guid = FWU_METADATA_GUID;
  

• ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1,
  

•                              desc->blksz);
  

• ret = get_gpt_hdr_parts(desc, gpt_head, &gpt_pte);
  

• if (ret < 0) {
  

•         log_err("Error getting GPT header and partitions\n");
  

•         ret = -EIO;
  

•         goto out;
  

• }
  

• nparts = gpt_head->num_partition_entries;
  

• for (i = 0, mparts = 0; i < nparts; i++) {
  

•         if (!guidcmp(&fwu_metadata_guid,
  

•                      &gpt_pte[i].partition_type_guid)) {
  

•                 ++mparts;
  

•                 if (!*primary_mpart)
  

•                         *primary_mpart = i + 1;
  

•                 else
  

•                         *secondary_mpart = i + 1;
  

•         }
  

• }
  

• if (mparts != 2) {
  

•         log_err("Expect two copies of the metadata instead of
  

%d\n",

...

•                 mparts);
  

•         ret = -EINVAL;
  

• } else {
  

•         ret = 0;
  

• }
  

+out:

• free(gpt_pte);
  

• return ret;
  

+}

+static int gpt_get_metadata_disk_part(struct blk_desc *desc,

•                               struct disk_partition *info,
  

•                               u32 part_num)
  

+{

• int ret;
  

• char *metadata_guid_str = "8a7a84a0-8387-40f6-ab41-a8b9a5a60d23";
  

Is this hard-code guid_string intentioned?

Yes, this is mentioned in the FWU spec[1], Table 3, pg 14.

-sughosh

[1] - https://developer.arm.com/documentation/den0118/a

...

• ret = part_get_info(desc, part_num, info);
  

Jason Liu

Hi Sanghosh

On 11/25/21 8:01 AM, Sughosh Ganu wrote:

Add helper functions needed for accessing the metadata which contains information on the updatable images. These functions have been added for the STM32MP157C-DK2 board which has the updatable images on the uSD card, formatted as GPT partitions.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

board/st/stm32mp1/stm32mp1.c | 63 ++++++++++++++++++++++++++++++++++++ include/fwu_metadata.h | 3 ++ 2 files changed, 66 insertions(+)

diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c index 84592677e4..a6884d2772 100644 --- a/board/st/stm32mp1/stm32mp1.c +++ b/board/st/stm32mp1/stm32mp1.c @@ -7,6 +7,7 @@

#include <common.h> #include <adc.h> +#include <blk.h> #include <bootm.h> #include <clk.h> #include <config.h> @@ -14,6 +15,7 @@ #include <env.h> #include <env_internal.h> #include <fdt_support.h> +#include <fwu_metadata.h> #include <g_dnl.h> #include <generic-phy.h> #include <hang.h> @@ -23,6 +25,7 @@ #include <log.h> #include <malloc.h> #include <misc.h> +#include <mmc.h> #include <mtd_node.h> #include <net.h> #include <netdev.h> @@ -938,3 +941,63 @@ static void board_copro_image_process(ulong fw_image, size_t fw_size) }

U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_COPRO, board_copro_image_process);

+#ifdef CONFIG_FWU_MULTI_BANK_UPDATE

+int fwu_plat_get_update_index(u32 *update_idx) +{

• int ret;
• u32 active_idx;
• ret = fwu_get_active_index(&active_idx);
• if (ret < 0)

• return -1;
  

• *update_idx = active_idx ^= 0x1;
• return ret;

+}

+int fwu_plat_get_blk_desc(struct blk_desc **desc) +{

• int ret;
• struct mmc *mmc;
• struct udevice *dev;
• /*

• * Initial support is being added for the DK2
  

• * platform
  

• */
  

• if (CONFIG_IS_ENABLED(TARGET_ST_STM32MP15x) &&

•    (of_machine_is_compatible("st,stm32mp157c-dk2"))) {
  

• ret = uclass_get_device(UCLASS_MMC, 0, &dev);
  

• if (ret)
  

• 	return -1;
  

• mmc = mmc_get_mmc_dev(dev);
  

• if (!mmc)
  

• 	return -1;
  

• if (mmc_init(mmc))
  

• 	return -1;
  

• *desc = mmc_get_blk_desc(mmc);
  

• if (!*desc)
  

• 	return -1;
  

• }
• return 0;

+}

+struct fwu_metadata_ops *get_plat_fwu_metadata_ops(void) +{

• if (CONFIG_IS_ENABLED(TARGET_ST_STM32MP15x) &&

•    (of_machine_is_compatible("st,stm32mp157c-dk2"))) {
  

• return &fwu_gpt_blk_ops;
  

• }
• return NULL;

+}

+#endif /* CONFIG_FWU_MULTI_BANK_UPDATE */ diff --git a/include/fwu_metadata.h b/include/fwu_metadata.h index e692ef7506..6a5e814ab6 100644 --- a/include/fwu_metadata.h +++ b/include/fwu_metadata.h @@ -122,4 +122,7 @@ int fwu_accept_image(efi_guid_t *img_type_id); int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank); int fwu_get_metadata(struct fwu_metadata **metadata);

+int fwu_plat_get_update_index(u32 *update_idx); +int fwu_plat_get_blk_desc(struct blk_desc **desc);

• #endif /* _FWU_METADATA_H_ */

One general question:

How we can handle the EV1 board with 2 MMC devices => eMMC (mmc1) / SDCard (mmc0)

and how to managed HW partition of eMMC boot1 / boot2

=> for eMMC boot, the TF-A BL2 is located in these eMMC hw partition.

in the serie, I see only support for partition > 0 => partition in GPT but not the update of eMMC boot partition.

Regards

Patrick

hi Patrick,

On Tue, 7 Dec 2021 at 19:57, Patrick DELAUNAY patrick.delaunay@foss.st.com wrote:

Hi Sughosh,

On 11/25/21 8:01 AM, Sughosh Ganu wrote:

...

The FWU Multi Bank Update feature allows the platform to boot the firmware images from one of the partitions(banks). The first stage bootloader(fsbl) passes the value of the boot index, i.e. the bank from which the firmware images were booted from to U-Boot. On the STM32MP157C-DK2 board, this value is passed through one of the SoC's backup register. Add a function to read the boot index value from the backup register.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

arch/arm/mach-stm32mp/include/mach/stm32.h | 1 + board/st/stm32mp1/stm32mp1.c | 7 +++++++ include/fwu_metadata.h | 1 + 3 files changed, 9 insertions(+)

diff --git a/arch/arm/mach-stm32mp/include/mach/stm32.h

b/arch/arm/mach-stm32mp/include/mach/stm32.h

...

index c11a9903f2..21ed9f12e4 100644 --- a/arch/arm/mach-stm32mp/include/mach/stm32.h +++ b/arch/arm/mach-stm32mp/include/mach/stm32.h @@ -97,6 +97,7 @@ enum boot_device { #define TAMP_BACKUP_REGISTER(x) (STM32_TAMP_BASE + 0x100 +

4 * x)

...

#define TAMP_BACKUP_MAGIC_NUMBER TAMP_BACKUP_REGISTER(4) #define TAMP_BACKUP_BRANCH_ADDRESS TAMP_BACKUP_REGISTER(5) +#define TAMP_FWU_BOOT_IDX TAMP_BACKUP_REGISTER(10)

The used TAMP register need to be aligned with TF-A BL2, offset = 10 to be confirmed

Yes, the same register is being used in tf-a BL2 to pass the boot index. The patch for this support is being reviewed in tf-a[1].

-sughosh

[1] - https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/12577

...

#define TAMP_COPRO_RSC_TBL_ADDRESS TAMP_BACKUP_REGISTER(17) #define TAMP_COPRO_STATE TAMP_BACKUP_REGISTER(18) #define TAMP_BOOT_CONTEXT TAMP_BACKUP_REGISTER(20) diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c index a6884d2772..32bba71289 100644 --- a/board/st/stm32mp1/stm32mp1.c +++ b/board/st/stm32mp1/stm32mp1.c @@ -990,6 +990,13 @@ int fwu_plat_get_blk_desc(struct blk_desc **desc) return 0; }

+void fwu_plat_get_bootidx(void *boot_idx) +{

• u32 *bootidx = boot_idx;
  

• *bootidx = readl(TAMP_FWU_BOOT_IDX);
  

+}

• struct fwu_metadata_ops *get_plat_fwu_metadata_ops(void) { if (CONFIG_IS_ENABLED(TARGET_ST_STM32MP15x) &&

diff --git a/include/fwu_metadata.h b/include/fwu_metadata.h index 6a5e814ab6..44f06f4c6a 100644 --- a/include/fwu_metadata.h +++ b/include/fwu_metadata.h @@ -124,5 +124,6 @@ int fwu_get_metadata(struct fwu_metadata **metadata);

int fwu_plat_get_update_index(u32 *update_idx); int fwu_plat_get_blk_desc(struct blk_desc **desc); +void fwu_plat_get_bootidx(void *boot_idx);

#endif /* _FWU_METADATA_H_ */

Regards

Patrick