On Mon, Mar 06, 2023 at 05:18:41PM -0600, jassisinghb...@gmail.com wrote:
> From: Jassi Brar <jaswinder.si...@linaro.org>
>
> The common code can now read, verify and fix meta-data copies
> while exposing one consistent structure to users.
>  Only the .read_mdata() and .write_mdata() callbacks of fwu_mdata_ops
> are needed. Get rid of .get_mdata() .update_mdata() .get_mdata_part_num()
> .read_mdata_partition() and .write_mdata_partition() and also the
> corresponding wrapper functions.
>
> Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> Reviewed-by: Etienne Carriere <etienne.carri...@linaro.org>
> ---
>  cmd/fwu_mdata.c                      |  17 +-
>  drivers/fwu-mdata/fwu-mdata-uclass.c | 165 -------------------
>  drivers/fwu-mdata/gpt_blk.c          | 124 +-------------
>  include/fwu.h                        | 199 -----------------------
>  lib/fwu_updates/fwu.c                | 235 ++++-----------------------
>  5 files changed, 38 insertions(+), 702 deletions(-)
>
> diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c
> index f04af27de6..9b70340368 100644
> --- a/cmd/fwu_mdata.c
> +++ b/cmd/fwu_mdata.c
> @@ -43,23 +43,10 @@ static void print_mdata(struct fwu_mdata *mdata)
>  int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag,
>                    int argc, char * const argv[])
>  {
> -     struct udevice *dev;
>       int ret = CMD_RET_SUCCESS, res;
> -     struct fwu_mdata mdata = { 0 };
> +     struct fwu_mdata mdata;
>
> -     if (uclass_get_device(UCLASS_FWU_MDATA, 0, &dev) || !dev) {
> -             log_err("Unable to get FWU metadata device\n");
> -             return CMD_RET_FAILURE;
> -     }
> -
> -     res = fwu_check_mdata_validity();
> -     if (res < 0) {
> -             log_err("FWU Metadata check failed\n");
> -             ret = CMD_RET_FAILURE;
> -             goto out;
> -     }
> -
> -     res = fwu_get_mdata(dev, &mdata);
> +     res = fwu_get_verified_mdata(&mdata);
>       if (res < 0) {
>               log_err("Unable to get valid FWU metadata\n");
>               ret = CMD_RET_FAILURE;
> diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c 
> b/drivers/fwu-mdata/fwu-mdata-uclass.c
> index e03773c584..0a8edaaa41 100644
> --- a/drivers/fwu-mdata/fwu-mdata-uclass.c
> +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c
> @@ -14,7 +14,6 @@
>
>  #include <linux/errno.h>
>  #include <linux/types.h>
> -#include <u-boot/crc.h>
>
>  /**
>   * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata()
> @@ -50,170 +49,6 @@ int fwu_write_mdata(struct udevice *dev, struct fwu_mdata 
> *mdata, bool primary)
>       return ops->write_mdata(dev, mdata, primary);
>  }
>
> -/**
> - * fwu_get_mdata_part_num() - Get the FWU metadata partition numbers
> - * @dev: FWU metadata device
> - * @mdata_parts: array for storing the metadata partition numbers
> - *
> - * Get the partition numbers on the storage device on which the
> - * FWU metadata is stored. Two partition numbers will be returned.
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_get_mdata_part_num(struct udevice *dev, uint *mdata_parts)
> -{
> -     const struct fwu_mdata_ops *ops = device_get_ops(dev);
> -
> -     if (!ops->get_mdata_part_num) {
> -             log_debug("get_mdata_part_num() method not defined\n");
> -             return -ENOSYS;
> -     }
> -
> -     return ops->get_mdata_part_num(dev, mdata_parts);
> -}
> -
> -/**
> - * fwu_read_mdata_partition() - Read the FWU metadata from a partition
> - * @dev: FWU metadata device
> - * @mdata: Copy of the FWU metadata
> - * @part_num: Partition number from which FWU metadata is to be read
> - *
> - * Read the FWU metadata from the specified partition number
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_read_mdata_partition(struct udevice *dev, struct fwu_mdata *mdata,
> -                          uint part_num)
> -{
> -     const struct fwu_mdata_ops *ops = device_get_ops(dev);
> -
> -     if (!ops->read_mdata_partition) {
> -             log_debug("read_mdata_partition() method not defined\n");
> -             return -ENOSYS;
> -     }
> -
> -     return ops->read_mdata_partition(dev, mdata, part_num);
> -}
> -
> -/**
> - * fwu_write_mdata_partition() - Write the FWU metadata to a partition
> - * @dev: FWU metadata device
> - * @mdata: Copy of the FWU metadata
> - * @part_num: Partition number to which FWU metadata is to be written
> - *
> - * Write the FWU metadata to the specified partition number
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_write_mdata_partition(struct udevice *dev, struct fwu_mdata *mdata,
> -                           uint part_num)
> -{
> -     const struct fwu_mdata_ops *ops = device_get_ops(dev);
> -
> -     if (!ops->write_mdata_partition) {
> -             log_debug("write_mdata_partition() method not defined\n");
> -             return -ENOSYS;
> -     }
> -
> -     return ops->write_mdata_partition(dev, mdata, part_num);
> -}
> -
> -/**
> - * fwu_mdata_check() - Check if the FWU metadata is valid
> - * @dev: FWU metadata device
> - *
> - * Validate both copies of the FWU metadata. If one of the copies
> - * has gone bad, restore it from the other copy.
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_mdata_check(struct udevice *dev)
> -{
> -     const struct fwu_mdata_ops *ops = device_get_ops(dev);
> -
> -     if (!ops->check_mdata) {
> -             log_debug("check_mdata() method not defined\n");
> -             return -ENOSYS;
> -     }
> -
> -     return ops->check_mdata(dev);
> -}
> -
> -/**
> - * fwu_get_mdata() - Get a FWU metadata copy
> - * @dev: FWU metadata device
> - * @mdata: Copy of the FWU metadata
> - *
> - * Get a valid copy of the FWU metadata.
> - *
> - * Note: This function is to be called first when modifying any fields
> - * in the metadata. The sequence of calls to modify any field in the
> - * metadata would  be 1) fwu_get_mdata 2) Modify metadata, followed by
> - * 3) fwu_update_mdata
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_get_mdata(struct udevice *dev, struct fwu_mdata *mdata)
> -{
> -     const struct fwu_mdata_ops *ops = device_get_ops(dev);
> -
> -     if (!ops->get_mdata) {
> -             log_debug("get_mdata() method not defined\n");
> -             return -ENOSYS;
> -     }
> -
> -     return ops->get_mdata(dev, mdata);
> -}
> -
> -/**
> - * fwu_update_mdata() - Update the FWU metadata
> - * @dev: FWU metadata device
> - * @mdata: Copy of the FWU metadata
> - *
> - * Update the FWU metadata structure by writing to the
> - * FWU metadata partitions.
> - *
> - * Note: This function is not to be called directly to update the
> - * metadata fields. The sequence of function calls should be
> - * 1) fwu_get_mdata() 2) Modify the medata fields 3) fwu_update_mdata()
> - *
> - * The sequence of updating the partitions should be, update the
> - * primary metadata partition (first partition encountered), followed
> - * by updating the secondary partition. With this update sequence, in
> - * the rare scenario that the two metadata partitions are valid but do
> - * not match, maybe due to power outage at the time of updating the
> - * metadata copies, the secondary partition can be updated from the
> - * primary.
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_update_mdata(struct udevice *dev, struct fwu_mdata *mdata)
> -{
> -     void *buf;
> -     const struct fwu_mdata_ops *ops = device_get_ops(dev);
> -
> -     if (!ops->update_mdata) {
> -             log_debug("get_mdata() method not defined\n");
> -             return -ENOSYS;
> -     }
> -
> -     /*
> -      * Calculate the crc32 for the updated FWU metadata
> -      * and put the updated value in the FWU metadata crc32
> -      * field
> -      */
> -     buf = &mdata->version;
> -     mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32));
> -
> -     return ops->update_mdata(dev, mdata);
> -}
> -
>  UCLASS_DRIVER(fwu_mdata) = {
>       .id             = UCLASS_FWU_MDATA,
>       .name           = "fwu-mdata",
> diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c
> index be1a6b03a1..c8a046c105 100644
> --- a/drivers/fwu-mdata/gpt_blk.c
> +++ b/drivers/fwu-mdata/gpt_blk.c
> @@ -28,7 +28,7 @@ static uint g_mdata_part[2]; /* = {0, 0} to check against 
> uninit parts */
>
>  static int gpt_get_mdata_partitions(struct blk_desc *desc)
>  {
> -     int i, ret;
> +     int i;
>       u32 nparts;
>       efi_guid_t part_type_guid;
>       struct disk_partition info;
> @@ -52,12 +52,12 @@ static int gpt_get_mdata_partitions(struct blk_desc *desc)
>       if (nparts != 2) {
>               log_debug("Expect two copies of the FWU metadata instead of 
> %d\n",
>                         nparts);
> -             ret = -EINVAL;
> -     } else {
> -             ret = 0;
> +             g_mdata_part[0] = 0;
> +             g_mdata_part[1] = 0;
> +             return -EINVAL;
>       }
>
> -     return ret;
> +     return 0;
>  }
>
>  static int gpt_get_mdata_disk_part(struct blk_desc *desc,
> @@ -125,115 +125,6 @@ static int gpt_read_write_mdata(struct blk_desc *desc,
>       return 0;
>  }
>
> -static int fwu_gpt_update_mdata(struct udevice *dev, struct fwu_mdata *mdata)
> -{
> -     int ret;
> -     struct blk_desc *desc;
> -     struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
> -
> -     desc = dev_get_uclass_plat(priv->blk_dev);
> -
> -     ret = gpt_get_mdata_partitions(desc);
> -     if (ret < 0) {
> -             log_debug("Error getting the FWU metadata partitions\n");
> -             return -ENOENT;
> -     }
> -
> -     /* First write the primary partition */
> -     ret = gpt_read_write_mdata(desc, mdata, MDATA_WRITE, g_mdata_part[0]);
> -     if (ret < 0) {
> -             log_debug("Updating primary FWU metadata partition failed\n");
> -             return ret;
> -     }
> -
> -     /* And now the replica */
> -     ret = gpt_read_write_mdata(desc, mdata, MDATA_WRITE, g_mdata_part[1]);
> -     if (ret < 0) {
> -             log_debug("Updating secondary FWU metadata partition failed\n");
> -             return ret;
> -     }
> -
> -     return 0;
> -}
> -
> -static int gpt_get_mdata(struct blk_desc *desc, struct fwu_mdata *mdata)
> -{
> -     int ret;
> -
> -     ret = gpt_get_mdata_partitions(desc);
> -     if (ret < 0) {
> -             log_debug("Error getting the FWU metadata partitions\n");
> -             return -ENOENT;
> -     }
> -
> -     ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, g_mdata_part[0]);
> -     if (ret < 0) {
> -             log_debug("Failed to read the FWU metadata from the device\n");
> -             return -EIO;
> -     }
> -
> -     ret = fwu_verify_mdata(mdata, 1);
> -     if (!ret)
> -             return 0;
> -
> -     /*
> -      * Verification of the primary FWU metadata copy failed.
> -      * Try to read the replica.
> -      */
> -     memset(mdata, '\0', sizeof(struct fwu_mdata));
> -     ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, g_mdata_part[1]);
> -     if (ret < 0) {
> -             log_debug("Failed to read the FWU metadata from the device\n");
> -             return -EIO;
> -     }
> -
> -     ret = fwu_verify_mdata(mdata, 0);
> -     if (!ret)
> -             return 0;
> -
> -     /* Both the FWU metadata copies are corrupted. */
> -     return -EIO;
> -}
> -
> -static int fwu_gpt_get_mdata(struct udevice *dev, struct fwu_mdata *mdata)
> -{
> -     struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
> -
> -     return gpt_get_mdata(dev_get_uclass_plat(priv->blk_dev), mdata);
> -}
> -
> -static int fwu_gpt_get_mdata_partitions(struct udevice *dev, uint 
> *mdata_parts)
> -{
> -     struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
> -     int err;
> -
> -     err = gpt_get_mdata_partitions(dev_get_uclass_plat(priv->blk_dev));
> -     if (!err) {
> -             mdata_parts[0] = g_mdata_part[0];
> -             mdata_parts[1] = g_mdata_part[1];
> -     }
> -
> -     return err;
> -}
> -
> -static int fwu_gpt_read_mdata_partition(struct udevice *dev,
> -                                     struct fwu_mdata *mdata, uint part_num)
> -{
> -     struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
> -
> -     return gpt_read_write_mdata(dev_get_uclass_plat(priv->blk_dev),
> -                                 mdata, MDATA_READ, part_num);
> -}
> -
> -static int fwu_gpt_write_mdata_partition(struct udevice *dev,
> -                                     struct fwu_mdata *mdata, uint part_num)
> -{
> -     struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
> -
> -     return gpt_read_write_mdata(dev_get_uclass_plat(priv->blk_dev),
> -                                 mdata, MDATA_WRITE, part_num);
> -}
> -
>  static int fwu_get_mdata_device(struct udevice *dev, struct udevice 
> **mdata_dev)
>  {
>       u32 phandle;
> @@ -309,11 +200,6 @@ static int fwu_gpt_write_mdata(struct udevice *dev, 
> struct fwu_mdata *mdata,
>  static const struct fwu_mdata_ops fwu_gpt_blk_ops = {
>       .read_mdata = fwu_gpt_read_mdata,
>       .write_mdata = fwu_gpt_write_mdata,
> -     .get_mdata = fwu_gpt_get_mdata,
> -     .update_mdata = fwu_gpt_update_mdata,
> -     .get_mdata_part_num = fwu_gpt_get_mdata_partitions,
> -     .read_mdata_partition = fwu_gpt_read_mdata_partition,
> -     .write_mdata_partition = fwu_gpt_write_mdata_partition,
>  };
>
>  static const struct udevice_id fwu_mdata_ids[] = {
> diff --git a/include/fwu.h b/include/fwu.h
> index 13f8fdeb28..314aadea59 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -18,11 +18,6 @@ struct fwu_mdata_gpt_blk_priv {
>       struct udevice *blk_dev;
>  };
>
> -/**
> - * @mdata_check: check the validity of the FWU metadata partitions
> - * @get_mdata() - Get a FWU metadata copy
> - * @update_mdata() - Update the FWU metadata copy
> - */
>  struct fwu_mdata_ops {
>       /**
>        * read_mdata() - Populate the asked FWU metadata copy
> @@ -43,78 +38,6 @@ struct fwu_mdata_ops {
>        * Return: 0 if OK, -ve on error
>        */
>       int (*write_mdata)(struct udevice *dev, struct fwu_mdata *mdata, bool 
> primary);
> -
> -     /**
> -      * check_mdata() - Check if the FWU metadata is valid
> -      * @dev:        FWU device
> -      *
> -      * Validate both copies of the FWU metadata. If one of the copies
> -      * has gone bad, restore it from the other copy.
> -      *
> -      * Return: 0 if OK, -ve on error
> -      */
> -     int (*check_mdata)(struct udevice *dev);
> -
> -     /**
> -      * get_mdata() - Get a FWU metadata copy
> -      * @dev:        FWU device
> -      * @mdata:      Pointer to FWU metadata
> -      *
> -      * Get a valid copy of the FWU metadata.
> -      *
> -      * Return: 0 if OK, -ve on error
> -      */
> -     int (*get_mdata)(struct udevice *dev, struct fwu_mdata *mdata);
> -
> -     /**
> -      * update_mdata() - Update the FWU metadata
> -      * @dev:        FWU device
> -      * @mdata:      Copy of the FWU metadata
> -      *
> -      * Update the FWU metadata structure by writing to the
> -      * FWU metadata partitions.
> -      *
> -      * Return: 0 if OK, -ve on error
> -      */
> -     int (*update_mdata)(struct udevice *dev, struct fwu_mdata *mdata);
> -
> -     /**
> -      * get_mdata_part_num() - Get the FWU metadata partition numbers
> -      * @dev:                FWU metadata device
> -      * @mdata_parts:         array for storing the metadata partition 
> numbers
> -      *
> -      * Get the partition numbers on the storage device on which the
> -      * FWU metadata is stored. Two partition numbers will be returned.
> -      *
> -      * Return: 0 if OK, -ve on error
> -      */
> -     int (*get_mdata_part_num)(struct udevice *dev, uint *mdata_parts);
> -
> -     /**
> -      * read_mdata_partition() - Read the FWU metadata from a partition
> -      * @dev:        FWU metadata device
> -      * @mdata:      Copy of the FWU metadata
> -      * @part_num:   Partition number from which FWU metadata is to be read
> -      *
> -      * Read the FWU metadata from the specified partition number
> -      *
> -      * Return: 0 if OK, -ve on error
> -      */
> -     int (*read_mdata_partition)(struct udevice *dev,
> -                                 struct fwu_mdata *mdata, uint part_num);
> -
> -     /**
> -      * write_mdata_partition() - Write the FWU metadata to a partition
> -      * @dev:        FWU metadata device
> -      * @mdata:      Copy of the FWU metadata
> -      * @part_num:   Partition number to which FWU metadata is to be written
> -      *
> -      * Write the FWU metadata to the specified partition number
> -      *
> -      * Return: 0 if OK, -ve on error
> -      */
> -     int (*write_mdata_partition)(struct udevice *dev,
> -                                  struct fwu_mdata *mdata, uint part_num);
>  };
>
>  #define FWU_MDATA_VERSION    0x1
> @@ -167,102 +90,6 @@ int fwu_write_mdata(struct udevice *dev, struct 
> fwu_mdata *mdata, bool primary);
>  */
>  int fwu_get_verified_mdata(struct fwu_mdata *mdata);
>
> -/**
> - * fwu_check_mdata_validity() - Check for validity of the FWU metadata copies
> - *
> - * Read both the metadata copies from the storage media, verify their
> - * checksum, and ascertain that both copies match. If one of the copies
> - * has gone bad, restore it from the good copy.
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_check_mdata_validity(void);
> -
> -/**
> - * fwu_get_mdata_part_num() - Get the FWU metadata partition numbers
> - * @dev: FWU metadata device
> - * @mdata_parts: array for storing the metadata partition numbers
> - *
> - * Get the partition numbers on the storage device on which the
> - * FWU metadata is stored. Two partition numbers will be returned
> - * through the array.
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_get_mdata_part_num(struct udevice *dev, uint *mdata_parts);
> -
> -/**
> - * fwu_read_mdata_partition() - Read the FWU metadata from a partition
> - * @dev: FWU metadata device
> - * @mdata: Copy of the FWU metadata
> - * @part_num: Partition number from which FWU metadata is to be read
> - *
> - * Read the FWU metadata from the specified partition number
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_read_mdata_partition(struct udevice *dev, struct fwu_mdata *mdata,
> -                          uint part_num);
> -
> -/**
> - * fwu_write_mdata_partition() - Write the FWU metadata to a partition
> - * @dev: FWU metadata device
> - * @mdata: Copy of the FWU metadata
> - * @part_num: Partition number to which FWU metadata is to be written
> - *
> - * Write the FWU metadata to the specified partition number
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_write_mdata_partition(struct udevice *dev, struct fwu_mdata *mdata,
> -                           uint part_num);
> -
> -/**
> - * fwu_get_mdata() - Get a FWU metadata copy
> - * @dev: FWU metadata device
> - * @mdata: Copy of the FWU metadata
> - *
> - * Get a valid copy of the FWU metadata.
> - *
> - * Note: This function is to be called first when modifying any fields
> - * in the metadata. The sequence of calls to modify any field in the
> - * metadata would  be 1) fwu_get_mdata 2) Modify metadata, followed by
> - * 3) fwu_update_mdata
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_get_mdata(struct udevice *dev, struct fwu_mdata *mdata);
> -
> -/**
> - * fwu_update_mdata() - Update the FWU metadata
> - * @dev: FWU metadata device
> - * @mdata: Copy of the FWU metadata
> - *
> - * Update the FWU metadata structure by writing to the
> - * FWU metadata partitions.
> - *
> - * Note: This function is not to be called directly to update the
> - * metadata fields. The sequence of function calls should be
> - * 1) fwu_get_mdata() 2) Modify the medata fields 3) fwu_update_mdata()
> - *
> - * The sequence of updating the partitions should be, update the
> - * primary metadata partition (first partition encountered), followed
> - * by updating the secondary partition. With this update sequence, in
> - * the rare scenario that the two metadata partitions are valid but do
> - * not match, maybe due to power outage at the time of updating the
> - * metadata copies, the secondary partition can be updated from the
> - * primary.
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_update_mdata(struct udevice *dev, struct fwu_mdata *mdata);
> -
>  /**
>   * fwu_get_active_index() - Get active_index from the FWU metadata
>   * @active_idxp: active_index value to be read
> @@ -303,18 +130,6 @@ int fwu_set_active_index(uint active_idx);
>   */
>  int fwu_get_image_index(u8 *image_index);
>
> -/**
> - * fwu_mdata_check() - Check if the FWU metadata is valid
> - * @dev: FWU metadata device
> - *
> - * Validate both copies of the FWU metadata. If one of the copies
> - * has gone bad, restore it from the other copy.
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_mdata_check(struct udevice *dev);
> -
>  /**
>   * fwu_revert_boot_index() - Revert the active index in the FWU metadata
>   *
> @@ -327,20 +142,6 @@ int fwu_mdata_check(struct udevice *dev);
>   */
>  int fwu_revert_boot_index(void);
>
> -/**
> - * fwu_verify_mdata() - Verify the FWU metadata
> - * @mdata: FWU metadata structure
> - * @pri_part: FWU metadata partition is primary or secondary
> - *
> - * Verify the FWU metadata by computing the CRC32 for the metadata
> - * structure and comparing it against the CRC32 value stored as part
> - * of the structure.
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_verify_mdata(struct fwu_mdata *mdata, bool pri_part);
> -
>  /**
>   * 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
> diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> index 8f1c05ad1c..49c9316c70 100644
> --- a/lib/fwu_updates/fwu.c
> +++ b/lib/fwu_updates/fwu.c
> @@ -33,26 +33,6 @@ enum {
>       BOTH_PARTS,
>  };
>
> -static int fwu_get_dev_mdata(struct udevice **dev, struct fwu_mdata *mdata)
> -{
> -     int ret;
> -
> -     ret = uclass_first_device_err(UCLASS_FWU_MDATA, dev);
> -     if (ret) {
> -             log_debug("Cannot find fwu device\n");
> -             return ret;
> -     }
> -
> -     if (!mdata)
> -             return 0;
> -
> -     ret = fwu_get_mdata(*dev, mdata);
> -     if (ret < 0)
> -             log_debug("Unable to get valid FWU metadata\n");
> -
> -     return ret;
> -}
> -
>  static int trial_counter_update(u16 *trial_state_ctr)
>  {
>       bool delete;
> @@ -282,136 +262,6 @@ ret_mdata:
>       return err;
>  }
>
> -/**
> - * fwu_verify_mdata() - Verify the FWU metadata
> - * @mdata: FWU metadata structure
> - * @pri_part: FWU metadata partition is primary or secondary
> - *
> - * Verify the FWU metadata by computing the CRC32 for the metadata
> - * structure and comparing it against the CRC32 value stored as part
> - * of the structure.
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_verify_mdata(struct fwu_mdata *mdata, bool pri_part)
> -{
> -     u32 calc_crc32;
> -     void *buf;
> -
> -     buf = &mdata->version;
> -     calc_crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32));
> -
> -     if (calc_crc32 != mdata->crc32) {
> -             log_debug("crc32 check failed for %s FWU metadata partition\n",
> -                       pri_part ? "primary" : "secondary");
> -             return -EINVAL;
> -     }
> -
> -     return 0;
> -}
> -
> -/**
> - * fwu_check_mdata_validity() - Check for validity of the FWU metadata copies
> - *
> - * Read both the metadata copies from the storage media, verify their 
> checksum,
> - * and ascertain that both copies match. If one of the copies has gone bad,
> - * restore it from the good copy.
> - *
> - * Return: 0 if OK, -ve on error
> - *
> - */
> -int fwu_check_mdata_validity(void)
> -{
> -     int ret;
> -     struct udevice *dev;
> -     struct fwu_mdata pri_mdata;
> -     struct fwu_mdata secondary_mdata;
> -     uint mdata_parts[2];
> -     uint valid_partitions, invalid_partitions;
> -
> -     ret = fwu_get_dev_mdata(&dev, NULL);
> -     if (ret)
> -             return ret;
> -
> -     /*
> -      * Check if the platform has defined its own
> -      * function to check the metadata partitions'
> -      * validity. If so, that takes precedence.
> -      */
> -     ret = fwu_mdata_check(dev);
> -     if (!ret || ret != -ENOSYS)
> -             return ret;
> -
> -     /*
> -      * Two FWU metadata partitions are expected.
> -      * If we don't have two, user needs to create
> -      * them first
> -      */
> -     valid_partitions = 0;
> -     ret = fwu_get_mdata_part_num(dev, mdata_parts);
> -     if (ret < 0) {
> -             log_debug("Error getting the FWU metadata partitions\n");
> -             return -ENOENT;
> -     }
> -
> -     ret = fwu_read_mdata_partition(dev, &pri_mdata, mdata_parts[0]);
> -     if (!ret) {
> -             ret = fwu_verify_mdata(&pri_mdata, 1);
> -             if (!ret)
> -                     valid_partitions |= PRIMARY_PART;
> -     }
> -
> -     ret = fwu_read_mdata_partition(dev, &secondary_mdata, mdata_parts[1]);
> -     if (!ret) {
> -             ret = fwu_verify_mdata(&secondary_mdata, 0);
> -             if (!ret)
> -                     valid_partitions |= SECONDARY_PART;
> -     }
> -
> -     if (valid_partitions == (PRIMARY_PART | SECONDARY_PART)) {
> -             /*
> -              * Before returning, check that both the
> -              * FWU metadata copies are the same. If not,
> -              * populate the secondary partition from the
> -              * primary partition copy.
> -              */
> -             if (!memcmp(&pri_mdata, &secondary_mdata,
> -                         sizeof(struct fwu_mdata))) {
> -                     ret = 0;
> -             } else {
> -                     log_info("Both FWU metadata copies are valid but do not 
> match.");
> -                     log_info(" Restoring the secondary partition from the 
> primary\n");
> -                     ret = fwu_write_mdata_partition(dev, &pri_mdata,
> -                                                     mdata_parts[1]);
> -                     if (ret)
> -                             log_debug("Restoring secondary FWU metadata 
> partition failed\n");
> -             }
> -             goto out;
> -     }
> -
> -     if (!(valid_partitions & BOTH_PARTS)) {
> -             log_info("Both FWU metadata partitions invalid\n");
> -             ret = -EBADMSG;
> -             goto out;
> -     }
> -
> -     invalid_partitions = valid_partitions ^ BOTH_PARTS;
> -     ret = fwu_write_mdata_partition(dev,
> -                                     (invalid_partitions == PRIMARY_PART) ?
> -                                     &secondary_mdata : &pri_mdata,
> -                                     (invalid_partitions == PRIMARY_PART) ?
> -                                     mdata_parts[0] : mdata_parts[1]);
> -
> -     if (ret)
> -             log_debug("Restoring %s FWU metadata partition failed\n",
> -                       (invalid_partitions == PRIMARY_PART) ?
> -                       "primary" : "secondary");
> -
> -out:
> -     return ret;
> -}
> -
>  /**
>   * fwu_get_active_index() - Get active_index from the FWU metadata
>   * @active_idx: active_index value to be read
> @@ -424,19 +274,14 @@ out:
>   */
>  int fwu_get_active_index(uint *active_idx)
>  {
> -     int ret;
> -     struct udevice *dev;
> -     struct fwu_mdata mdata = { 0 };
> -
> -     ret = fwu_get_dev_mdata(&dev, &mdata);
> -     if (ret)
> -             return ret;
> +     int ret = 0;
> +     struct fwu_mdata *mdata = &g_mdata;
>
>       /*
>        * Found the FWU metadata partition, now read the active_index
>        * value
>        */
> -     *active_idx = mdata.active_index;
> +     *active_idx = mdata->active_index;
>       if (*active_idx >= CONFIG_FWU_NUM_BANKS) {
>               log_debug("Active index value read is incorrect\n");
>               ret = -EINVAL;
> @@ -457,30 +302,25 @@ int fwu_get_active_index(uint *active_idx)
>  int fwu_set_active_index(uint active_idx)
>  {
>       int ret;
> -     struct udevice *dev;
> -     struct fwu_mdata mdata = { 0 };
> +     struct fwu_mdata *mdata = &g_mdata;
>
>       if (active_idx >= CONFIG_FWU_NUM_BANKS) {
>               log_debug("Invalid active index value\n");
>               return -EINVAL;
>       }
>
> -     ret = fwu_get_dev_mdata(&dev, &mdata);
> -     if (ret)
> -             return ret;
> -
>       /*
>        * Update the active index and previous_active_index fields
>        * in the FWU metadata
>        */
> -     mdata.previous_active_index = mdata.active_index;
> -     mdata.active_index = active_idx;
> +     mdata->previous_active_index = mdata->active_index;
> +     mdata->active_index = active_idx;
>
>       /*
>        * Now write this updated FWU metadata to both the
>        * FWU metadata partitions
>        */
> -     ret = fwu_update_mdata(dev, &mdata);
> +     ret = fwu_sync_mdata(mdata, BOTH_PARTS);
>       if (ret) {
>               log_debug("Failed to update FWU metadata partitions\n");
>               ret = -EIO;
> @@ -510,15 +350,10 @@ int fwu_get_image_index(u8 *image_index)
>       u8 alt_num;
>       uint update_bank;
>       efi_guid_t *image_guid, image_type_id;
> -     struct udevice *dev;
> -     struct fwu_mdata mdata = { 0 };
> +     struct fwu_mdata *mdata = &g_mdata;
>       struct fwu_image_entry *img_entry;
>       struct fwu_image_bank_info *img_bank_info;
>
> -     ret = fwu_get_dev_mdata(&dev, &mdata);
> -     if (ret)
> -             return ret;
> -
>       ret = fwu_plat_get_update_index(&update_bank);
>       if (ret) {
>               log_debug("Failed to get the FWU update bank\n");
> @@ -539,11 +374,11 @@ int fwu_get_image_index(u8 *image_index)
>        */
>       for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
>               if (!guidcmp(&image_type_id,
> -                          &mdata.img_entry[i].image_type_uuid)) {
> -                     img_entry = &mdata.img_entry[i];
> +                          &mdata->img_entry[i].image_type_uuid)) {
> +                     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(dev, image_guid, &alt_num);
> +                     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);
> @@ -578,26 +413,21 @@ int fwu_revert_boot_index(void)
>  {
>       int ret;
>       u32 cur_active_index;
> -     struct udevice *dev;
> -     struct fwu_mdata mdata = { 0 };
> -
> -     ret = fwu_get_dev_mdata(&dev, &mdata);
> -     if (ret)
> -             return ret;
> +     struct fwu_mdata *mdata = &g_mdata;
>
>       /*
>        * Swap the active index and previous_active_index fields
>        * in the FWU metadata
>        */
> -     cur_active_index = mdata.active_index;
> -     mdata.active_index = mdata.previous_active_index;
> -     mdata.previous_active_index = cur_active_index;
> +     cur_active_index = mdata->active_index;
> +     mdata->active_index = mdata->previous_active_index;
> +     mdata->previous_active_index = cur_active_index;
>
>       /*
>        * Now write this updated FWU metadata to both the
>        * FWU metadata partitions
>        */
> -     ret = fwu_update_mdata(dev, &mdata);
> +     ret = fwu_sync_mdata(mdata, BOTH_PARTS);
>       if (ret) {
>               log_debug("Failed to update FWU metadata partitions\n");
>               ret = -EIO;
> @@ -624,16 +454,11 @@ int fwu_revert_boot_index(void)
>  static int fwu_clrset_image_accept(efi_guid_t *img_type_id, u32 bank, u8 
> action)
>  {
>       int ret, i;
> -     struct udevice *dev;
> -     struct fwu_mdata mdata = { 0 };
> +     struct fwu_mdata *mdata = &g_mdata;
>       struct fwu_image_entry *img_entry;
>       struct fwu_image_bank_info *img_bank_info;
>
> -     ret = fwu_get_dev_mdata(&dev, &mdata);
> -     if (ret)
> -             return ret;
> -
> -     img_entry = &mdata.img_entry[0];
> +     img_entry = &mdata->img_entry[0];
>       for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
>               if (!guidcmp(&img_entry[i].image_type_uuid, img_type_id)) {
>                       img_bank_info = &img_entry[i].img_bank_info[bank];
> @@ -642,7 +467,7 @@ static int fwu_clrset_image_accept(efi_guid_t 
> *img_type_id, u32 bank, u8 action)
>                       else
>                               img_bank_info->accepted = 0;
>
> -                     ret = fwu_update_mdata(dev, &mdata);
> +                     ret = fwu_sync_mdata(mdata, BOTH_PARTS);
>                       goto out;
>               }
>       }
> @@ -777,8 +602,6 @@ static int fwu_boottime_checks(void *ctx, struct event 
> *event)
>  {
>       int ret;
>       u32 boot_idx, active_idx;
> -     struct udevice *dev;
> -     struct fwu_mdata mdata = { 0 };
>
>       /* Don't have boot time checks on sandbox */
>       if (IS_ENABLED(CONFIG_SANDBOX)) {
> @@ -786,9 +609,17 @@ static int fwu_boottime_checks(void *ctx, struct event 
> *event)
>               return 0;
>       }
>
> -     ret = fwu_check_mdata_validity();
> -     if (ret)
> -             return 0;
> +     ret = uclass_first_device_err(UCLASS_FWU_MDATA, &g_dev);
> +     if (ret) {
> +             log_debug("Cannot find fwu device\n");
> +             return ret;
> +     }
> +
> +     ret = fwu_get_verified_mdata(NULL);
> +     if (ret) {
> +             log_debug("Unable to read meta-data\n");
> +             return ret;
> +     }
>
>       /*
>        * Get the Boot Index, i.e. the bank from
> @@ -824,11 +655,7 @@ static int fwu_boottime_checks(void *ctx, struct event 
> *event)
>       if (efi_init_obj_list() != EFI_SUCCESS)
>               return 0;
>
> -     ret = fwu_get_dev_mdata(&dev, &mdata);
> -     if (ret)
> -             return ret;
> -
> -     in_trial = in_trial_state(&mdata);
> +     in_trial = in_trial_state(&g_mdata);
>       if (!in_trial || (ret = fwu_trial_count_update()) > 0)
>               ret = trial_counter_update(NULL);
>
> --
> 2.34.1
>
Etienne, Sughosh, this looks correct, but can someone
verify it doesn't break the ST board?

Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>

Reply via email to