- 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