[PATCH v5 3/6] fwu: move meta-data management in core
Etienne Carriere
etienne.carriere at linaro.org
Wed Mar 1 12:16:20 CET 2023
Hello Jassi,
On Tue, 28 Feb 2023 at 01:52, <jassisinghbrar at gmail.com> wrote:
>
> From: Jassi Brar <jaswinder.singh at linaro.org>
>
> Instead of each i/f having to implement their own meta-data verification
> and storage, move the logic in common code. This simplifies the i/f code
> much simpler and compact.
>
> Signed-off-by: Jassi Brar <jaswinder.singh at linaro.org>
> ---
> drivers/fwu-mdata/fwu-mdata-uclass.c | 34 +++++++
> include/fwu.h | 41 +++++++++
> lib/fwu_updates/fwu.c | 130 +++++++++++++++++++++++++--
> 3 files changed, 200 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c b/drivers/fwu-mdata/fwu-mdata-uclass.c
> index b477e9603f..e03773c584 100644
> --- a/drivers/fwu-mdata/fwu-mdata-uclass.c
> +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c
> @@ -16,6 +16,40 @@
> #include <linux/types.h>
> #include <u-boot/crc.h>
>
> +/**
> + * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata()
> + *
> + * Return: 0 if OK, -ve on error
> + */
Inline description only in header file, or duplicated in source and
header files?
> +int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary)
> +{
> + const struct fwu_mdata_ops *ops = device_get_ops(dev);
> +
> + if (!ops->read_mdata) {
> + log_debug("read_mdata() method not defined\n");
> + return -ENOSYS;
> + }
> +
> + return ops->read_mdata(dev, mdata, primary);
> +}
> +
> +/**
> + * fwu_write_mdata() - Wrapper around fwu_mdata_ops.write_mdata()
> + *
> + * Return: 0 if OK, -ve on error
> + */
> +int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary)
> +{
> + const struct fwu_mdata_ops *ops = device_get_ops(dev);
> +
> + if (!ops->write_mdata) {
> + log_debug("write_mdata() method not defined\n");
> + return -ENOSYS;
> + }
> +
> + return ops->write_mdata(dev, mdata, primary);
> +}
> +
> /**
> * fwu_get_mdata_part_num() - Get the FWU metadata partition numbers
> * @dev: FWU metadata device
> diff --git a/include/fwu.h b/include/fwu.h
> index 0919ced812..1a700c9e6a 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -24,6 +24,26 @@ struct fwu_mdata_gpt_blk_priv {
> * @update_mdata() - Update the FWU metadata copy
> */
> struct fwu_mdata_ops {
> + /**
> + * read_mdata() - Populate the asked FWU metadata copy
> + * @dev: FWU metadata device
> + * @mdata: Copy of the FWU metadata
@mdata: Output FWU mdata read
> + * @primary: If primary or secondary copy of meta-data is to be read
s/meta-data/FWU metadata/
Ditto in .write_mdata description
> + *
> + * Return: 0 if OK, -ve on error
> + */
> + int (*read_mdata)(struct udevice *dev, struct fwu_mdata *mdata, bool primary);
> +
> + /**
> + * write_mdata() - Write the given FWU metadata copy
> + * @dev: FWU metadata device
> + * @mdata: Copy of the FWU metadata
> + * @primary: If primary or secondary copy of meta-data is to be written
> + *
> + * 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
> @@ -126,6 +146,27 @@ struct fwu_mdata_ops {
> EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
> 0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
>
> +/**
> + * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata()
> + */
> +int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary);
> +
> +/**
> + * fwu_write_mdata() - Wrapper around fwu_mdata_ops.write_mdata()
> + */
> +int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary);
> +
> +/**
> + * fwu_get_verified_mdata() - Read, verify and return the FWU metadata
> + *
> + * 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.
@mdata: Output FWU metadata read or NULL
> + *
> + * Return: 0 if OK, -ve on error
> +*/
> +int fwu_get_verified_mdata(struct fwu_mdata *mdata);
> +
> /**
> * fwu_check_mdata_validity() - Check for validity of the FWU metadata copies
> *
> diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> index 5313d07302..7d195964a2 100644
> --- a/lib/fwu_updates/fwu.c
> +++ b/lib/fwu_updates/fwu.c
> @@ -15,13 +15,13 @@
> #include <linux/errno.h>
> #include <linux/types.h>
>
> +#include <u-boot/crc.h>
> +
> +static struct fwu_mdata g_mdata; /* = {0} makes uninit crc32 always invalid */
> +static struct udevice *g_dev;
> static u8 in_trial;
> static u8 boottime_check;
>
> -#include <linux/errno.h>
> -#include <linux/types.h>
> -#include <u-boot/crc.h>
> -
> enum {
> IMAGE_ACCEPT_SET = 1,
> IMAGE_ACCEPT_CLEAR,
> @@ -161,6 +161,126 @@ static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id)
> return -ENOENT;
> }
>
> +/**
> + * fwu_sync_mdata() - Update given meta-data partition(s) with the copy provided
> + * @mdata: FWU metadata structure
> + * @part: Bitmask of FWU metadata partitions to be written to
> + *
> + * Return: 0 if OK, -ve on error
> + */
> +static int fwu_sync_mdata(struct fwu_mdata *mdata, int part)
> +{
> + void *buf = &mdata->version;
> + int err;
> +
> + if (part == BOTH_PARTS) {
> + err = fwu_sync_mdata(mdata, SECONDARY_PART);
> + if (err)
> + return err;
> + part = PRIMARY_PART;
> + }
> +
> + /*
> + * Calculate the crc32 for the updated FWU metadata
> + * and put the updated value in the FWU metadata crc32
> + * field
> + */
> + mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32));
> +
> + err = fwu_write_mdata(g_dev, mdata, part & PRIMARY_PART ? true : false);
Since this expects part is either PRIMARY_PART or SECONDARY_PART, prefer:
err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART);
And ditto below:
part == PRIMARY_PART ? "primary": "secondary");
> + if (err) {
> + log_err("Unable to write %s mdata\n",
> + part & PRIMARY_PART ? "primary": "secondary");
> + return err;
> + }
> +
> + /* update the cached copy of meta-data */
> + memcpy(&g_mdata, mdata, sizeof(struct fwu_mdata));
> +
> + return 0;
> +}
> +
> +static inline int mdata_crc_check(struct fwu_mdata *mdata)
> +{
> + void *buf = &mdata->version;
> + u32 calc_crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32));
> +
> + return calc_crc32 == mdata->crc32 ? 0 : -EINVAL;
> +}
> +
> +/**
> + * fwu_get_verified_mdata() - Read, verify and return the FWU metadata
> + *
> + * 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_get_verified_mdata(struct fwu_mdata *mdata)
> +{
> + int err;
> + bool parts_ok[2] = { false };
> + struct fwu_mdata s, *parts_mdata[2];
> +
> + parts_mdata[0] = &g_mdata;
> + parts_mdata[1] = &s;
> +
> + /* if mdata already read and ready */
> + err = mdata_crc_check(parts_mdata[0]);
> + if (!err)
> + goto ret_mdata;
> + /* else read, verify and, if needed, fix mdata */
> +
> + for (int i = 0; i < 2; i++) {
Define "int i;" at function block entry?
> + parts_ok[i] = false;
> + err = fwu_read_mdata(g_dev, parts_mdata[i], !i);
> + if (!err) {
> + err = mdata_crc_check(parts_mdata[i]);
> + if (!err)
> + parts_ok[i] = true;
> + else
> + log_debug("mdata : %s crc32 failed\n", i ? "secondary": "primary");
> + }
> + }
> +
> + if (parts_ok[0] && parts_ok[1]) {
> + /*
> + * Before returning, check that both the
> + * FWU metadata copies are the same.
> + */
> + err = memcmp(parts_mdata[0], parts_mdata[1], sizeof(struct fwu_mdata));
> + if (!err)
> + goto ret_mdata;
> +
> + /*
> + * If not, populate the secondary partition from the
> + * primary partition copy.
> + */
> + log_info("Both FWU metadata copies are valid but do not match.");
> + log_info(" Restoring the secondary partition from the primary\n");
> + parts_ok[1] = false;
> + }
> +
> + for (int i = 0; i < 2; i++) {
> + if (parts_ok[i])
> + continue;
> +
> + memcpy(parts_mdata[i], parts_mdata[1-i], sizeof(struct fwu_mdata));
> + err = fwu_sync_mdata(parts_mdata[i], i ? SECONDARY_PART : PRIMARY_PART);
> + if (err) {
> + log_debug("mdata : %s write failed\n", i ? "secondary": "primary");
> + return err;
> + }
> + }
> +
> +ret_mdata:
> + if (!err && mdata)
> + memcpy(mdata, parts_mdata[0], sizeof(struct fwu_mdata));
> +
> + return err;
> +}
> +
> /**
> * fwu_verify_mdata() - Verify the FWU metadata
> * @mdata: FWU metadata structure
> @@ -436,7 +556,7 @@ int fwu_get_image_index(u8 *image_index)
> }
> }
>
> - log_debug("Partition with the image type %pUs not found\n",
> + log_err("Partition with the image type %pUs not found\n",
> &image_type_id);
>
> out:
> --
> 2.34.1
>
More information about the U-Boot
mailing list