Hi Jassi,
On Mon, Jan 02, 2023 at 12:26:40PM -0600, Jassi Brar wrote:
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 | 142 ++++++++++++++++++++++++++- 3 files changed, 213 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_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));
- if (part & PRIMARY_PART)
err = fwu_write_mdata(g_dev, mdata, true);- if (err) {
log_err("Unable to write primary mdata\n");return err;- }
- if (part & SECONDARY_PART)
err = fwu_write_mdata(g_dev, mdata, false);- if (err) {
log_err("Unable to write secondary mdata\n");return err;- }
Can we write this err = fwu_write_mdata(g_dev, mdata, part & PRIMARY_PART ? true: false); if (err) log_err("Unable to write %s partition\n", part & PRIMARY_PART ? "primary": "secondary" ); ....
- /* 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;
Why are we defining it like this? Readability to have pointers for primary and secondary metadata?
- /* if mdata already read and ready */
- err = mdata_crc_check(p_mdata);
- if (!err)
goto ret_mdata;
Shouldn't we check the secondary metadata ? At least that's what the old fwu_check_mdata_validity() was doing.
- /* 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;elselog_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;elselog_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)goto ret_mdata;
The error print here is a bit misleading. It's a failed write, not a crc32 mismatch
- }
- if (!sec_ok) {
memcpy(s_mdata, p_mdata, sizeof(struct fwu_mdata));err = fwu_sync_mdata(s_mdata, SECONDARY_PART);if (err)goto ret_mdata;- }
+ret_mdata:
- if (err)
log_debug("mdata : crc32 failed\n");- else if (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
Regards /Ilias