* [PATCHv4 1/5] fwu: gpt: use cached meta-data partition numbers
2023-02-05 3:00 [PATCHv4 0/5] FWU: Handle meta-data in common code jassisinghbrar
@ 2023-02-05 3:01 ` jassisinghbrar
2023-02-05 3:01 ` [PATCHv4 2/5] fwu: move meta-data management in core jassisinghbrar
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: jassisinghbrar @ 2023-02-05 3:01 UTC (permalink / raw)
To: u-boot
Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
xypron.glpk, patrick.delaunay, patrice.chotard, Jassi Brar
From: Jassi Brar <jaswinder.singh@linaro.org>
Use cached values and avoid parsing and scanning through partitions
everytime for meta-data partitions because they can't change after bootup.
Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
drivers/fwu-mdata/gpt_blk.c | 43 +++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c
index d35ce49c5c..28f5d23e1e 100644
--- a/drivers/fwu-mdata/gpt_blk.c
+++ b/drivers/fwu-mdata/gpt_blk.c
@@ -24,8 +24,9 @@ enum {
MDATA_WRITE,
};
-static int gpt_get_mdata_partitions(struct blk_desc *desc,
- uint mdata_parts[2])
+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;
u32 nparts;
@@ -33,18 +34,19 @@ static int gpt_get_mdata_partitions(struct blk_desc *desc,
struct disk_partition info;
const efi_guid_t fwu_mdata_guid = FWU_MDATA_GUID;
+ /* if primary and secondary partitions already found */
+ if (g_mdata_part[0] && g_mdata_part[1])
+ return 0;
+
nparts = 0;
- for (i = 1; i < MAX_SEARCH_PARTITIONS; i++) {
+ for (i = 1; i < MAX_SEARCH_PARTITIONS && nparts < 2; i++) {
if (part_get_info(desc, i, &info))
continue;
uuid_str_to_bin(info.type_guid, part_type_guid.b,
UUID_STR_FORMAT_GUID);
- if (!guidcmp(&fwu_mdata_guid, &part_type_guid)) {
- if (nparts < 2)
- mdata_parts[nparts] = i;
- ++nparts;
- }
+ if (!guidcmp(&fwu_mdata_guid, &part_type_guid))
+ g_mdata_part[nparts++] = i;
}
if (nparts != 2) {
@@ -127,26 +129,25 @@ static int fwu_gpt_update_mdata(struct udevice *dev, struct fwu_mdata *mdata)
{
int ret;
struct blk_desc *desc;
- uint mdata_parts[2];
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, mdata_parts);
+ 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, mdata_parts[0]);
+ 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, mdata_parts[1]);
+ 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;
@@ -158,16 +159,14 @@ static int fwu_gpt_update_mdata(struct udevice *dev, struct fwu_mdata *mdata)
static int gpt_get_mdata(struct blk_desc *desc, struct fwu_mdata *mdata)
{
int ret;
- uint mdata_parts[2];
-
- ret = gpt_get_mdata_partitions(desc, mdata_parts);
+ 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, mdata_parts[0]);
+ 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;
@@ -182,7 +181,7 @@ static int gpt_get_mdata(struct blk_desc *desc, struct fwu_mdata *mdata)
* Try to read the replica.
*/
memset(mdata, '\0', sizeof(struct fwu_mdata));
- ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, mdata_parts[1]);
+ 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;
@@ -206,9 +205,15 @@ static int fwu_gpt_get_mdata(struct udevice *dev, struct fwu_mdata *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 gpt_get_mdata_partitions(dev_get_uclass_plat(priv->blk_dev),
- mdata_parts);
+ return err;
}
static int fwu_gpt_read_mdata_partition(struct udevice *dev,
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCHv4 2/5] fwu: move meta-data management in core
2023-02-05 3:00 [PATCHv4 0/5] FWU: Handle meta-data in common code jassisinghbrar
2023-02-05 3:01 ` [PATCHv4 1/5] fwu: gpt: use cached meta-data partition numbers jassisinghbrar
@ 2023-02-05 3:01 ` jassisinghbrar
2023-02-23 8:35 ` Ilias Apalodimas
2023-02-27 16:30 ` Etienne Carriere
2023-02-05 3:01 ` [PATCHv4 3/5] fwu: gpt: implement read_mdata and write_mdata callbacks jassisinghbrar
` (4 subsequent siblings)
6 siblings, 2 replies; 15+ messages in thread
From: jassisinghbrar @ 2023-02-05 3:01 UTC (permalink / raw)
To: u-boot
Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
xypron.glpk, patrick.delaunay, patrice.chotard, Jassi Brar
From: Jassi Brar <jaswinder.singh@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@linaro.org>
---
drivers/fwu-mdata/fwu-mdata-uclass.c | 34 +++++++
include/fwu.h | 41 ++++++++
lib/fwu_updates/fwu.c | 135 ++++++++++++++++++++++++++-
3 files changed, 206 insertions(+), 4 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
+ */
+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
+ * @primary: If primary or secondary copy of meta-data is to be read
+ *
+ * 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.
+ *
+ * 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..56299f1b2f 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,133 @@ 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 = 0;
+
+ /*
+ * 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);
+ 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 pri_ok, sec_ok;
+ struct fwu_mdata s, *p_mdata, *s_mdata;
+
+ p_mdata = &g_mdata;
+ s_mdata = &s;
+
+ /* if mdata already read and ready */
+ err = mdata_crc_check(p_mdata);
+ if (!err)
+ goto ret_mdata;
+ /* else read, verify and, if needed, fix mdata */
+
+ pri_ok = false;
+ err = fwu_read_mdata(g_dev, p_mdata, true);
+ if (!err) {
+ err = mdata_crc_check(p_mdata);
+ if (!err)
+ pri_ok = true;
+ else
+ log_debug("primary mdata: crc32 failed\n");
+ }
+
+ sec_ok = false;
+ err = fwu_read_mdata(g_dev, s_mdata, false);
+ if (!err) {
+ err = mdata_crc_check(s_mdata);
+ if (!err)
+ sec_ok = true;
+ else
+ log_debug("secondary mdata: crc32 failed\n");
+ }
+
+ if (pri_ok && sec_ok) {
+ /*
+ * Before returning, check that both the
+ * FWU metadata copies are the same.
+ */
+ err = memcmp(p_mdata, s_mdata, 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");
+ sec_ok = false;
+ }
+
+ if (!pri_ok) {
+ memcpy(p_mdata, s_mdata, sizeof(struct fwu_mdata));
+ err = fwu_sync_mdata(p_mdata, PRIMARY_PART);
+ if (err) {
+ log_debug("mdata : primary write failed\n");
+ return err;
+ }
+ }
+
+ if (!sec_ok) {
+ memcpy(s_mdata, p_mdata, sizeof(struct fwu_mdata));
+ err = fwu_sync_mdata(s_mdata, SECONDARY_PART);
+ if (err) {
+ log_debug("mdata : secondary write failed\n");
+ return err;
+ }
+ }
+
+ret_mdata:
+ if (!err && mdata)
+ memcpy(mdata, p_mdata, sizeof(struct fwu_mdata));
+
+ return err;
+}
+
/**
* fwu_verify_mdata() - Verify the FWU metadata
* @mdata: FWU metadata structure
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCHv4 2/5] fwu: move meta-data management in core
2023-02-05 3:01 ` [PATCHv4 2/5] fwu: move meta-data management in core jassisinghbrar
@ 2023-02-23 8:35 ` Ilias Apalodimas
2023-02-28 1:52 ` Jassi Brar
2023-02-27 16:30 ` Etienne Carriere
1 sibling, 1 reply; 15+ messages in thread
From: Ilias Apalodimas @ 2023-02-23 8:35 UTC (permalink / raw)
To: jassisinghbrar
Cc: u-boot, etienne.carriere, trini, sjg, sughosh.ganu, xypron.glpk,
patrick.delaunay, patrice.chotard, Jassi Brar
Hi Jassi,
Apologies for the delay
On Sat, Feb 04, 2023 at 09:01:46PM -0600, jassisinghbrar@gmail.com wrote:
> From: Jassi Brar <jaswinder.singh@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@linaro.org>
> ---
> drivers/fwu-mdata/fwu-mdata-uclass.c | 34 +++++++
> include/fwu.h | 41 ++++++++
> lib/fwu_updates/fwu.c | 135 ++++++++++++++++++++++++++-
> 3 files changed, 206 insertions(+), 4 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
> + */
> +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
> + * @primary: If primary or secondary copy of meta-data is to be read
> + *
> + * 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.
> + *
> + * 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..56299f1b2f 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,133 @@ 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 = 0;
> +
> + /*
> + * 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);
> + 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 pri_ok, sec_ok;
> + struct fwu_mdata s, *p_mdata, *s_mdata;
> +
> + p_mdata = &g_mdata;
> + s_mdata = &s;
> +
> + /* if mdata already read and ready */
> + err = mdata_crc_check(p_mdata);
> + if (!err)
> + goto ret_mdata;
> + /* else read, verify and, if needed, fix mdata */
> +
> + pri_ok = false;
> + err = fwu_read_mdata(g_dev, p_mdata, true);
> + if (!err) {
> + err = mdata_crc_check(p_mdata);
> + if (!err)
> + pri_ok = true;
> + else
> + log_debug("primary mdata: crc32 failed\n");
> + }
> +
> + sec_ok = false;
> + err = fwu_read_mdata(g_dev, s_mdata, false);
> + if (!err) {
> + err = mdata_crc_check(s_mdata);
> + if (!err)
> + sec_ok = true;
> + else
> + log_debug("secondary mdata: crc32 failed\n");
> + }
Isn't it better to define pri_ok, sec_ok and their equivalent mdata as
arrays ? IOW something along the lines of
bool parts_ok[2] = { false };
struct fwu_mdata parts_mdata[2];
parts_mdata[0] = &g_mdata;
parts_mdata[1] = .....
for (i = 0; i < 2; i++) {
err = fwu_read_mdata(g_dev, parts_mdata[i], !(i % 2) ? true : false);
if (!err)
err = mdata_crc_check(parts_mdata[i]);
etc....
}
> +
> + if (pri_ok && sec_ok) {
And then also adjust this part?
> + /*
> + * Before returning, check that both the
> + * FWU metadata copies are the same.
> + */
> + err = memcmp(p_mdata, s_mdata, 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");
> + sec_ok = false;
> + }
> +
> + if (!pri_ok) {
> + memcpy(p_mdata, s_mdata, sizeof(struct fwu_mdata));
> + err = fwu_sync_mdata(p_mdata, PRIMARY_PART);
> + if (err) {
> + log_debug("mdata : primary write failed\n");
> + return err;
> + }
> + }
> +
> + if (!sec_ok) {
> + memcpy(s_mdata, p_mdata, sizeof(struct fwu_mdata));
> + err = fwu_sync_mdata(s_mdata, SECONDARY_PART);
> + if (err) {
> + log_debug("mdata : secondary write failed\n");
> + return err;
> + }
> + }
And this could also be folded into a for loop
> +
> +ret_mdata:
> + if (!err && mdata)
> + memcpy(mdata, p_mdata, sizeof(struct fwu_mdata));
> +
> + return err;
> +}
> +
> /**
> * fwu_verify_mdata() - Verify the FWU metadata
> * @mdata: FWU metadata structure
> --
> 2.34.1
>
Thanks
/Ilias
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCHv4 2/5] fwu: move meta-data management in core
2023-02-23 8:35 ` Ilias Apalodimas
@ 2023-02-28 1:52 ` Jassi Brar
0 siblings, 0 replies; 15+ messages in thread
From: Jassi Brar @ 2023-02-28 1:52 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: u-boot, etienne.carriere, trini, sjg, sughosh.ganu, xypron.glpk,
patrick.delaunay, patrice.chotard, Jassi Brar
Hi Ilias,
On Thu, Feb 23, 2023 at 2:36 AM Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
> > +int fwu_get_verified_mdata(struct fwu_mdata *mdata)
> > +{
> > + int err;
> > + bool pri_ok, sec_ok;
> > + struct fwu_mdata s, *p_mdata, *s_mdata;
> > +
> > + p_mdata = &g_mdata;
> > + s_mdata = &s;
> > +
> > + /* if mdata already read and ready */
> > + err = mdata_crc_check(p_mdata);
> > + if (!err)
> > + goto ret_mdata;
> > + /* else read, verify and, if needed, fix mdata */
> > +
> > + pri_ok = false;
> > + err = fwu_read_mdata(g_dev, p_mdata, true);
> > + if (!err) {
> > + err = mdata_crc_check(p_mdata);
> > + if (!err)
> > + pri_ok = true;
> > + else
> > + log_debug("primary mdata: crc32 failed\n");
> > + }
> > +
> > + sec_ok = false;
> > + err = fwu_read_mdata(g_dev, s_mdata, false);
> > + if (!err) {
> > + err = mdata_crc_check(s_mdata);
> > + if (!err)
> > + sec_ok = true;
> > + else
> > + log_debug("secondary mdata: crc32 failed\n");
> > + }
>
> Isn't it better to define pri_ok, sec_ok and their equivalent mdata as
> arrays ? IOW something along the lines of
>
> bool parts_ok[2] = { false };
> struct fwu_mdata parts_mdata[2];
>
> parts_mdata[0] = &g_mdata;
> parts_mdata[1] = .....
> for (i = 0; i < 2; i++) {
> err = fwu_read_mdata(g_dev, parts_mdata[i], !(i % 2) ? true : false);
> if (!err)
> err = mdata_crc_check(parts_mdata[i]);
> etc....
> }
>
> > +
> > + if (pri_ok && sec_ok) {
>
> And then also adjust this part?
>
> > + /*
> > + * Before returning, check that both the
> > + * FWU metadata copies are the same.
> > + */
> > + err = memcmp(p_mdata, s_mdata, 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");
> > + sec_ok = false;
> > + }
> > +
> > + if (!pri_ok) {
> > + memcpy(p_mdata, s_mdata, sizeof(struct fwu_mdata));
> > + err = fwu_sync_mdata(p_mdata, PRIMARY_PART);
> > + if (err) {
> > + log_debug("mdata : primary write failed\n");
> > + return err;
> > + }
> > + }
> > +
> > + if (!sec_ok) {
> > + memcpy(s_mdata, p_mdata, sizeof(struct fwu_mdata));
> > + err = fwu_sync_mdata(s_mdata, SECONDARY_PART);
> > + if (err) {
> > + log_debug("mdata : secondary write failed\n");
> > + return err;
> > + }
> > + }
>
> And this could also be folded into a for loop
>
I have done these modifications and submitted v5.
Thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv4 2/5] fwu: move meta-data management in core
2023-02-05 3:01 ` [PATCHv4 2/5] fwu: move meta-data management in core jassisinghbrar
2023-02-23 8:35 ` Ilias Apalodimas
@ 2023-02-27 16:30 ` Etienne Carriere
2023-02-27 16:46 ` Jassi Brar
1 sibling, 1 reply; 15+ messages in thread
From: Etienne Carriere @ 2023-02-27 16:30 UTC (permalink / raw)
To: jassisinghbrar
Cc: u-boot, ilias.apalodimas, trini, sjg, sughosh.ganu, xypron.glpk,
patrick.delaunay, patrice.chotard, Jassi Brar
Hello Jassi,
On Sun, 5 Feb 2023 at 04:01, <jassisinghbrar@gmail.com> wrote:
>
> From: Jassi Brar <jaswinder.singh@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@linaro.org>
> ---
> drivers/fwu-mdata/fwu-mdata-uclass.c | 34 +++++++
> include/fwu.h | 41 ++++++++
> lib/fwu_updates/fwu.c | 135 ++++++++++++++++++++++++++-
> 3 files changed, 206 insertions(+), 4 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
> + */
> +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
> + * @primary: If primary or secondary copy of meta-data is to be read
> + *
> + * 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.
> + *
> + * 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..56299f1b2f 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,133 @@ 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 = 0;
> +
> + /*
> + * 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);
There is an issue here as arg @part can have 3 values: PRIMARY_PART,
SECONDARY_PART or BOTH_PARTS.
The implementation here does not consider the later case BOTH_PATHS.
I think something like the below code snippet should do the work:
switch (part) {
case PRIMARY_PART:
case SECONDARY_PART:
err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART);
break;
default: /* assuming BOTH_PARTS, or maybe we need sanitization of
invalid part values? */
err = fwu_write_mdata(g_dev, mdata, true);
if (!err)
err = fwu_write_mdata(g_dev, mdata, false);
break;
}
and adaptation for the error message below.
Or maybe fwu_write_mdata() should not have its 3rd argument and
backend handler should directly handle mdata replication update (if
needed) on any mdata update requests.
Br,
etienne
> + 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 pri_ok, sec_ok;
> + struct fwu_mdata s, *p_mdata, *s_mdata;
> +
> + p_mdata = &g_mdata;
> + s_mdata = &s;
> +
> + /* if mdata already read and ready */
> + err = mdata_crc_check(p_mdata);
> + if (!err)
> + goto ret_mdata;
> + /* else read, verify and, if needed, fix mdata */
> +
> + pri_ok = false;
> + err = fwu_read_mdata(g_dev, p_mdata, true);
> + if (!err) {
> + err = mdata_crc_check(p_mdata);
> + if (!err)
> + pri_ok = true;
> + else
> + log_debug("primary mdata: crc32 failed\n");
> + }
> +
> + sec_ok = false;
> + err = fwu_read_mdata(g_dev, s_mdata, false);
> + if (!err) {
> + err = mdata_crc_check(s_mdata);
> + if (!err)
> + sec_ok = true;
> + else
> + log_debug("secondary mdata: crc32 failed\n");
> + }
> +
> + if (pri_ok && sec_ok) {
> + /*
> + * Before returning, check that both the
> + * FWU metadata copies are the same.
> + */
> + err = memcmp(p_mdata, s_mdata, 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");
> + sec_ok = false;
> + }
> +
> + if (!pri_ok) {
> + memcpy(p_mdata, s_mdata, sizeof(struct fwu_mdata));
> + err = fwu_sync_mdata(p_mdata, PRIMARY_PART);
> + if (err) {
> + log_debug("mdata : primary write failed\n");
> + return err;
> + }
> + }
> +
> + if (!sec_ok) {
> + memcpy(s_mdata, p_mdata, sizeof(struct fwu_mdata));
> + err = fwu_sync_mdata(s_mdata, SECONDARY_PART);
> + if (err) {
> + log_debug("mdata : secondary write failed\n");
> + return err;
> + }
> + }
> +
> +ret_mdata:
> + if (!err && mdata)
> + memcpy(mdata, p_mdata, sizeof(struct fwu_mdata));
> +
> + return err;
> +}
> +
> /**
> * fwu_verify_mdata() - Verify the FWU metadata
> * @mdata: FWU metadata structure
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCHv4 2/5] fwu: move meta-data management in core
2023-02-27 16:30 ` Etienne Carriere
@ 2023-02-27 16:46 ` Jassi Brar
2023-02-27 23:15 ` Etienne Carriere
0 siblings, 1 reply; 15+ messages in thread
From: Jassi Brar @ 2023-02-27 16:46 UTC (permalink / raw)
To: Etienne Carriere
Cc: u-boot, ilias.apalodimas, trini, sjg, sughosh.ganu, xypron.glpk,
patrick.delaunay, patrice.chotard, Jassi Brar
On Mon, Feb 27, 2023 at 10:30 AM Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> Hello Jassi,
>
> On Sun, 5 Feb 2023 at 04:01, <jassisinghbrar@gmail.com> wrote:
> >
> > From: Jassi Brar <jaswinder.singh@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@linaro.org>
> > ---
> > drivers/fwu-mdata/fwu-mdata-uclass.c | 34 +++++++
> > include/fwu.h | 41 ++++++++
> > lib/fwu_updates/fwu.c | 135 ++++++++++++++++++++++++++-
> > 3 files changed, 206 insertions(+), 4 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
> > + */
> > +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
> > + * @primary: If primary or secondary copy of meta-data is to be read
> > + *
> > + * 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.
> > + *
> > + * 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..56299f1b2f 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,133 @@ 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 = 0;
> > +
> > + /*
> > + * 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);
>
> There is an issue here as arg @part can have 3 values: PRIMARY_PART,
> SECONDARY_PART or BOTH_PARTS.
> The implementation here does not consider the later case BOTH_PATHS.
> I think something like the below code snippet should do the work:
>
> switch (part) {
> case PRIMARY_PART:
> case SECONDARY_PART:
> err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART);
> break;
> default: /* assuming BOTH_PARTS, or maybe we need sanitization of
> invalid part values? */
> err = fwu_write_mdata(g_dev, mdata, true);
> if (!err)
> err = fwu_write_mdata(g_dev, mdata, false);
> break;
> }
> and adaptation for the error message below.
>
> Or maybe fwu_write_mdata() should not have its 3rd argument and
> backend handler should directly handle mdata replication update (if
> needed) on any mdata update requests.
>
Good catch. Thanks.
Even simpler fix is ...
static int fwu_sync_mdata(struct fwu_mdata *mdata, int part)
{
if (part == BOTH_PARTS) {
fwu_sync_mdata(mdata, PRIMARY_PART);
part = SECONDARY_PART;
}
.....
}
Thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCHv4 2/5] fwu: move meta-data management in core
2023-02-27 16:46 ` Jassi Brar
@ 2023-02-27 23:15 ` Etienne Carriere
0 siblings, 0 replies; 15+ messages in thread
From: Etienne Carriere @ 2023-02-27 23:15 UTC (permalink / raw)
To: Jassi Brar
Cc: u-boot, ilias.apalodimas, trini, sjg, sughosh.ganu, xypron.glpk,
patrick.delaunay, patrice.chotard, Jassi Brar
- err = fwu_write_mdata(g_dev, mdata, part & PRIMARY_PART ? true : false);
On Mon, 27 Feb 2023 at 17:46, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> On Mon, Feb 27, 2023 at 10:30 AM Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
> >
> > Hello Jassi,
> >
> > On Sun, 5 Feb 2023 at 04:01, <jassisinghbrar@gmail.com> wrote:
> > >
> > > From: Jassi Brar <jaswinder.singh@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@linaro.org>
> > > ---
> > > drivers/fwu-mdata/fwu-mdata-uclass.c | 34 +++++++
> > > include/fwu.h | 41 ++++++++
> > > lib/fwu_updates/fwu.c | 135 ++++++++++++++++++++++++++-
> > > 3 files changed, 206 insertions(+), 4 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
> > > + */
> > > +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
> > > + * @primary: If primary or secondary copy of meta-data is to be read
> > > + *
> > > + * 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.
> > > + *
> > > + * 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..56299f1b2f 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,133 @@ 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 = 0;
> > > +
> > > + /*
> > > + * 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);
> >
> > There is an issue here as arg @part can have 3 values: PRIMARY_PART,
> > SECONDARY_PART or BOTH_PARTS.
> > The implementation here does not consider the later case BOTH_PATHS.
> > I think something like the below code snippet should do the work:
> >
> > switch (part) {
> > case PRIMARY_PART:
> > case SECONDARY_PART:
> > err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART);
> > break;
> > default: /* assuming BOTH_PARTS, or maybe we need sanitization of
> > invalid part values? */
> > err = fwu_write_mdata(g_dev, mdata, true);
> > if (!err)
> > err = fwu_write_mdata(g_dev, mdata, false);
> > break;
> > }
> > and adaptation for the error message below.
> >
> > Or maybe fwu_write_mdata() should not have its 3rd argument and
> > backend handler should directly handle mdata replication update (if
> > needed) on any mdata update requests.
> >
> Good catch. Thanks.
>
> Even simpler fix is ...
>
> static int fwu_sync_mdata(struct fwu_mdata *mdata, int part)
> {
> if (part == BOTH_PARTS) {
> fwu_sync_mdata(mdata, PRIMARY_PART);
> part = SECONDARY_PART;
> }
> .....
> }
>
> Thanks.
True :) , (with error code management)
even if i think it makes the code less straightforward to read out.
maybe also, for simplicity,
- err = fwu_write_mdata(g_dev, mdata, part & PRIMARY_PART ? true : false);
+ err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART);
br,
etienne
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv4 3/5] fwu: gpt: implement read_mdata and write_mdata callbacks
2023-02-05 3:00 [PATCHv4 0/5] FWU: Handle meta-data in common code jassisinghbrar
2023-02-05 3:01 ` [PATCHv4 1/5] fwu: gpt: use cached meta-data partition numbers jassisinghbrar
2023-02-05 3:01 ` [PATCHv4 2/5] fwu: move meta-data management in core jassisinghbrar
@ 2023-02-05 3:01 ` jassisinghbrar
2023-02-05 3:02 ` [PATCHv4 4/5] fwu: meta-data: switch to management by common code jassisinghbrar
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: jassisinghbrar @ 2023-02-05 3:01 UTC (permalink / raw)
To: u-boot
Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
xypron.glpk, patrick.delaunay, patrice.chotard, Jassi Brar
From: Jassi Brar <jaswinder.singh@linaro.org>
Moving towards using common code for meta-data management,
implement the read/write mdata hooks.
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
drivers/fwu-mdata/gpt_blk.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c
index 28f5d23e1e..bdaa10cd1d 100644
--- a/drivers/fwu-mdata/gpt_blk.c
+++ b/drivers/fwu-mdata/gpt_blk.c
@@ -272,7 +272,43 @@ static int fwu_mdata_gpt_blk_probe(struct udevice *dev)
return 0;
}
+static int fwu_gpt_read_mdata(struct udevice *dev, struct fwu_mdata *mdata,
+ bool primary)
+{
+ struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
+ struct blk_desc *desc = dev_get_uclass_plat(priv->blk_dev);
+ int ret;
+
+ ret = gpt_get_mdata_partitions(desc);
+ if (ret < 0) {
+ log_debug("Error getting the FWU metadata partitions\n");
+ return -ENOENT;
+ }
+
+ return gpt_read_write_mdata(desc, mdata, MDATA_READ,
+ primary ? g_mdata_part[0] : g_mdata_part[1]);
+}
+
+static int fwu_gpt_write_mdata(struct udevice *dev, struct fwu_mdata *mdata,
+ bool primary)
+{
+ struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
+ struct blk_desc *desc = dev_get_uclass_plat(priv->blk_dev);
+ int ret;
+
+ ret = gpt_get_mdata_partitions(desc);
+ if (ret < 0) {
+ log_debug("Error getting the FWU metadata partitions\n");
+ return -ENOENT;
+ }
+
+ return gpt_read_write_mdata(desc, mdata, MDATA_WRITE,
+ primary ? g_mdata_part[0] : g_mdata_part[1]);
+}
+
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,
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCHv4 4/5] fwu: meta-data: switch to management by common code
2023-02-05 3:00 [PATCHv4 0/5] FWU: Handle meta-data in common code jassisinghbrar
` (2 preceding siblings ...)
2023-02-05 3:01 ` [PATCHv4 3/5] fwu: gpt: implement read_mdata and write_mdata callbacks jassisinghbrar
@ 2023-02-05 3:02 ` jassisinghbrar
2023-02-21 18:23 ` Tom Rini
2023-02-23 8:37 ` Ilias Apalodimas
2023-02-05 3:02 ` [PATCHv4 5/5] fwu: rename fwu_get_verified_mdata to fwu_get_mdata jassisinghbrar
` (2 subsequent siblings)
6 siblings, 2 replies; 15+ messages in thread
From: jassisinghbrar @ 2023-02-05 3:02 UTC (permalink / raw)
To: u-boot
Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
xypron.glpk, patrick.delaunay, patrice.chotard, Jassi Brar
From: Jassi Brar <jaswinder.singh@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.singh@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@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 bdaa10cd1d..b3c6953a6e 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 1a700c9e6a..23bd97fe86 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 56299f1b2f..9c0a8e2bb1 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;
@@ -288,136 +268,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
@@ -430,19 +280,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;
@@ -463,30 +308,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;
@@ -516,15 +356,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");
@@ -545,11 +380,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);
@@ -584,26 +419,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;
@@ -630,16 +460,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];
@@ -648,7 +473,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;
}
}
@@ -783,8 +608,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)) {
@@ -792,9 +615,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
@@ -830,11 +661,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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCHv4 4/5] fwu: meta-data: switch to management by common code
2023-02-05 3:02 ` [PATCHv4 4/5] fwu: meta-data: switch to management by common code jassisinghbrar
@ 2023-02-21 18:23 ` Tom Rini
2023-02-23 8:37 ` Ilias Apalodimas
1 sibling, 0 replies; 15+ messages in thread
From: Tom Rini @ 2023-02-21 18:23 UTC (permalink / raw)
To: jassisinghbrar
Cc: u-boot, ilias.apalodimas, etienne.carriere, sjg, sughosh.ganu,
xypron.glpk, patrick.delaunay, patrice.chotard, Jassi Brar
[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]
On Sat, Feb 04, 2023 at 09:02:02PM -0600, jassisinghbrar@gmail.com wrote:
> From: Jassi Brar <jaswinder.singh@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.singh@linaro.org>
> Reviewed-by: Etienne Carriere <etienne.carriere@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(-)
This forgot to update test/dm/fwu_mdata.c and so CI fails:
https://source.denx.de/u-boot/u-boot/-/jobs/580820#L347
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv4 4/5] fwu: meta-data: switch to management by common code
2023-02-05 3:02 ` [PATCHv4 4/5] fwu: meta-data: switch to management by common code jassisinghbrar
2023-02-21 18:23 ` Tom Rini
@ 2023-02-23 8:37 ` Ilias Apalodimas
1 sibling, 0 replies; 15+ messages in thread
From: Ilias Apalodimas @ 2023-02-23 8:37 UTC (permalink / raw)
To: jassisinghbrar
Cc: u-boot, etienne.carriere, trini, sjg, sughosh.ganu, xypron.glpk,
patrick.delaunay, patrice.chotard, Jassi Brar
Hi Jassi,
This looks good to me, but Sughosh will test in on the ST board.
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
On Sat, Feb 04, 2023 at 09:02:02PM -0600, jassisinghbrar@gmail.com wrote:
> From: Jassi Brar <jaswinder.singh@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.singh@linaro.org>
> Reviewed-by: Etienne Carriere <etienne.carriere@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 bdaa10cd1d..b3c6953a6e 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 1a700c9e6a..23bd97fe86 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 56299f1b2f..9c0a8e2bb1 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;
> @@ -288,136 +268,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
> @@ -430,19 +280,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;
> @@ -463,30 +308,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;
> @@ -516,15 +356,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");
> @@ -545,11 +380,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);
> @@ -584,26 +419,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;
> @@ -630,16 +460,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];
> @@ -648,7 +473,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;
> }
> }
> @@ -783,8 +608,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)) {
> @@ -792,9 +615,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
> @@ -830,11 +661,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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv4 5/5] fwu: rename fwu_get_verified_mdata to fwu_get_mdata
2023-02-05 3:00 [PATCHv4 0/5] FWU: Handle meta-data in common code jassisinghbrar
` (3 preceding siblings ...)
2023-02-05 3:02 ` [PATCHv4 4/5] fwu: meta-data: switch to management by common code jassisinghbrar
@ 2023-02-05 3:02 ` jassisinghbrar
2023-02-06 22:00 ` [PATCHv4 0/5] FWU: Handle meta-data in common code Simon Glass
2023-02-21 20:08 ` Tom Rini
6 siblings, 0 replies; 15+ messages in thread
From: jassisinghbrar @ 2023-02-05 3:02 UTC (permalink / raw)
To: u-boot
Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
xypron.glpk, patrick.delaunay, patrice.chotard, Jassi Brar
From: Jassi Brar <jaswinder.singh@linaro.org>
fwu_get_mdata() sounds more appropriate than fwu_get_verified_mdata()
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
cmd/fwu_mdata.c | 2 +-
include/fwu.h | 4 ++--
lib/fwu_updates/fwu.c | 6 +++---
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c
index 9b70340368..5ecda455df 100644
--- a/cmd/fwu_mdata.c
+++ b/cmd/fwu_mdata.c
@@ -46,7 +46,7 @@ int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag,
int ret = CMD_RET_SUCCESS, res;
struct fwu_mdata mdata;
- res = fwu_get_verified_mdata(&mdata);
+ res = fwu_get_mdata(&mdata);
if (res < 0) {
log_err("Unable to get valid FWU metadata\n");
ret = CMD_RET_FAILURE;
diff --git a/include/fwu.h b/include/fwu.h
index 23bd97fe86..ea25aca2cd 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -80,7 +80,7 @@ int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary);
int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary);
/**
- * fwu_get_verified_mdata() - Read, verify and return the FWU metadata
+ * fwu_get_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,
@@ -88,7 +88,7 @@ int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary);
*
* Return: 0 if OK, -ve on error
*/
-int fwu_get_verified_mdata(struct fwu_mdata *mdata);
+int fwu_get_mdata(struct fwu_mdata *mdata);
/**
* fwu_get_active_index() - Get active_index from the FWU metadata
diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index 9c0a8e2bb1..cf7e0ad078 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -182,7 +182,7 @@ static inline int mdata_crc_check(struct fwu_mdata *mdata)
}
/**
- * fwu_get_verified_mdata() - Read, verify and return the FWU metadata
+ * fwu_get_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,
@@ -190,7 +190,7 @@ static inline int mdata_crc_check(struct fwu_mdata *mdata)
*
* Return: 0 if OK, -ve on error
*/
-int fwu_get_verified_mdata(struct fwu_mdata *mdata)
+int fwu_get_mdata(struct fwu_mdata *mdata)
{
int err;
bool pri_ok, sec_ok;
@@ -621,7 +621,7 @@ static int fwu_boottime_checks(void *ctx, struct event *event)
return ret;
}
- ret = fwu_get_verified_mdata(NULL);
+ ret = fwu_get_mdata(NULL);
if (ret) {
log_debug("Unable to read meta-data\n");
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCHv4 0/5] FWU: Handle meta-data in common code
2023-02-05 3:00 [PATCHv4 0/5] FWU: Handle meta-data in common code jassisinghbrar
` (4 preceding siblings ...)
2023-02-05 3:02 ` [PATCHv4 5/5] fwu: rename fwu_get_verified_mdata to fwu_get_mdata jassisinghbrar
@ 2023-02-06 22:00 ` Simon Glass
2023-02-21 20:08 ` Tom Rini
6 siblings, 0 replies; 15+ messages in thread
From: Simon Glass @ 2023-02-06 22:00 UTC (permalink / raw)
To: jassisinghbrar
Cc: u-boot, ilias.apalodimas, etienne.carriere, trini, sughosh.ganu,
xypron.glpk, patrick.delaunay, patrice.chotard, Jassi Brar
Hi,
On Sat, 4 Feb 2023 at 20:01, <jassisinghbrar@gmail.com> wrote:
>
> From: Jassi Brar <jaswinder.singh@linaro.org>
>
> The patchset reduces ~400 lines of code, while keeping the functionality same and making
> meta-data operations much faster (by using cached structures).
>
> Issue:
> meta-data copies (primary and secondary) are being handled by the backend/storage layer
> instead of the common core in fwu.c (as also noted by Ilias) that is, gpt_blk.c manages
> meta-data and similarly raw_mtd.c will have to do the same when it arrives. The code
> could by make smaller, cleaner and optimised.
>
> Basic idea:
> Introduce .read_mdata() and .write_mdata() in fwu_mdata_ops that simply read/write
> meta-data copy. The core code takes care of integrity and redundancy of the meta-data,
> as a result we can get rid of every other callback .get_mdata() .update_mdata()
> .get_mdata_part_num() .read_mdata_partition() .write_mdata_partition() and the
> corresponding wrapper functions thereby making the code 100s of LOC smaller.
>
> Get rid of fwu_check_mdata_validity() and fwu_mdata_check() which expected underlying
> layer to manage and verify mdata copies.
> Implement fwu_get_verified_mdata(struct fwu_mdata *mdata) public function that reads,
> verifies and, if needed, fixes the meta-data copies.
>
> Verified copy of meta-data is now cached as 'g_mdata' in fwu.c, which avoids multiple
> low-level expensive read and parse calls.
> gpt meta-data partition numbers are now cached in gpt_blk.c, so that we don't have to do expensive part_get_info() and uid ops.
>
> Changes since v3:
> * Fix error log wording
> * call fwu_write_mdata() with part & PRIMARY_PART ? true: false
>
> Changes since v2:
> * Drop whitespace changes
> * Fix missing mdata copy before return
>
> Changes since v1:
> * Fix typos and misc cosmetic changes
> * Catch error returns
>
>
> Jassi Brar (5):
> fwu: gpt: use cached meta-data partition numbers
> fwu: move meta-data management in core
> fwu: gpt: implement read_mdata and write_mdata callbacks
> fwu: meta-data: switch to management by common code
> fwu: rename fwu_get_verified_mdata to fwu_get_mdata
>
> cmd/fwu_mdata.c | 17 +-
> drivers/fwu-mdata/fwu-mdata-uclass.c | 151 +-------------
> drivers/fwu-mdata/gpt_blk.c | 175 +++++-----------
> include/fwu.h | 198 ++----------------
> lib/fwu_updates/fwu.c | 294 +++++++++++----------------
> 5 files changed, 207 insertions(+), 628 deletions(-)
>
> --
> 2.34.1
>
Nice to see this. It would be great to expand the tests in
test/dm/fwu_mdata.c at some point.
Regards,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCHv4 0/5] FWU: Handle meta-data in common code
2023-02-05 3:00 [PATCHv4 0/5] FWU: Handle meta-data in common code jassisinghbrar
` (5 preceding siblings ...)
2023-02-06 22:00 ` [PATCHv4 0/5] FWU: Handle meta-data in common code Simon Glass
@ 2023-02-21 20:08 ` Tom Rini
6 siblings, 0 replies; 15+ messages in thread
From: Tom Rini @ 2023-02-21 20:08 UTC (permalink / raw)
To: jassisinghbrar
Cc: u-boot, ilias.apalodimas, etienne.carriere, sjg, sughosh.ganu,
xypron.glpk, patrick.delaunay, patrice.chotard, Jassi Brar
[-- Attachment #1: Type: text/plain, Size: 2737 bytes --]
On Sat, Feb 04, 2023 at 09:00:56PM -0600, jassisinghbrar@gmail.com wrote:
> From: Jassi Brar <jaswinder.singh@linaro.org>
>
> The patchset reduces ~400 lines of code, while keeping the functionality same and making
> meta-data operations much faster (by using cached structures).
>
> Issue:
> meta-data copies (primary and secondary) are being handled by the backend/storage layer
> instead of the common core in fwu.c (as also noted by Ilias) that is, gpt_blk.c manages
> meta-data and similarly raw_mtd.c will have to do the same when it arrives. The code
> could by make smaller, cleaner and optimised.
>
> Basic idea:
> Introduce .read_mdata() and .write_mdata() in fwu_mdata_ops that simply read/write
> meta-data copy. The core code takes care of integrity and redundancy of the meta-data,
> as a result we can get rid of every other callback .get_mdata() .update_mdata()
> .get_mdata_part_num() .read_mdata_partition() .write_mdata_partition() and the
> corresponding wrapper functions thereby making the code 100s of LOC smaller.
>
> Get rid of fwu_check_mdata_validity() and fwu_mdata_check() which expected underlying
> layer to manage and verify mdata copies.
> Implement fwu_get_verified_mdata(struct fwu_mdata *mdata) public function that reads,
> verifies and, if needed, fixes the meta-data copies.
>
> Verified copy of meta-data is now cached as 'g_mdata' in fwu.c, which avoids multiple
> low-level expensive read and parse calls.
> gpt meta-data partition numbers are now cached in gpt_blk.c, so that we don't have to do expensive part_get_info() and uid ops.
>
> Changes since v3:
> * Fix error log wording
> * call fwu_write_mdata() with part & PRIMARY_PART ? true: false
>
> Changes since v2:
> * Drop whitespace changes
> * Fix missing mdata copy before return
>
> Changes since v1:
> * Fix typos and misc cosmetic changes
> * Catch error returns
>
>
> Jassi Brar (5):
> fwu: gpt: use cached meta-data partition numbers
> fwu: move meta-data management in core
> fwu: gpt: implement read_mdata and write_mdata callbacks
> fwu: meta-data: switch to management by common code
> fwu: rename fwu_get_verified_mdata to fwu_get_mdata
>
> cmd/fwu_mdata.c | 17 +-
> drivers/fwu-mdata/fwu-mdata-uclass.c | 151 +-------------
> drivers/fwu-mdata/gpt_blk.c | 175 +++++-----------
> include/fwu.h | 198 ++----------------
> lib/fwu_updates/fwu.c | 294 +++++++++++----------------
> 5 files changed, 207 insertions(+), 628 deletions(-)
FWIW, aside from the failure I reported, everything else seems fine.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread