Hello Sughosh and all,
On Mon, 26 Sept 2022 at 10:46, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Ilias,
On Thu, 22 Sept 2022 at 14:16, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Sughosh
On Thu, Sep 15, 2022 at 01:44:39PM +0530, Sughosh Ganu wrote:
In the FWU Multi Bank Update feature, the information about the updatable images is stored as part of the metadata, on a separate partition. Add a driver for reading from and writing to the metadata when the updatable images and the metadata are stored on a block device which is formatted with GPT based partition scheme.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com Reviewed-by: Etienne Carriere etienne.carriere@linaro.org
Changes since V9:
- s/formated/formatted in the commit message
- Add a '\n' character in the log message
drivers/fwu-mdata/Kconfig | 16 ++ drivers/fwu-mdata/Makefile | 8 + drivers/fwu-mdata/gpt_blk.c | 378 ++++++++++++++++++++++++++++++++++++ include/fwu.h | 4 + 4 files changed, 406 insertions(+) create mode 100644 drivers/fwu-mdata/Kconfig create mode 100644 drivers/fwu-mdata/Makefile create mode 100644 drivers/fwu-mdata/gpt_blk.c
diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig new file mode 100644 index 0000000000..7322da48b1 --- /dev/null +++ b/drivers/fwu-mdata/Kconfig @@ -0,0 +1,16 @@ +config FWU_MDATA
bool "Driver support for accessing FWU Metadata"depends on DMhelpEnable support for accessing FWU Metadata partitions. TheFWU Metadata partitions reside on the same storage devicewhich contains the other FWU updatable firmware images.+config FWU_MDATA_GPT_BLK
bool "FWU Metadata access for GPT partitioned Block devices"select PARTITION_TYPE_GUIDselect PARTITION_UUIDSdepends on DM && HAVE_BLOCK_DEVICE && EFI_PARTITIONhelpEnable support for accessing FWU Metadata on GPT partitionedblock devices.
By the way, drop HAVE_BLOCK_DEVICE that is no more needed here. See https://source.denx.de/u-boot/u-boot/-/commit/7f8967c2b82f9917987b69fbf43f8f...
diff --git a/drivers/fwu-mdata/Makefile b/drivers/fwu-mdata/Makefile new file mode 100644 index 0000000000..3fee64c10c --- /dev/null +++ b/drivers/fwu-mdata/Makefile @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# +# Copyright (c) 2022, Linaro Limited +#
+obj-$(CONFIG_FWU_MDATA) += fwu-mdata-uclass.o +obj-$(CONFIG_FWU_MDATA_GPT_BLK) += gpt_blk.o diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c new file mode 100644 index 0000000000..df115575e0 --- /dev/null +++ b/drivers/fwu-mdata/gpt_blk.c @@ -0,0 +1,378 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) 2022, Linaro Limited
- */
+#define LOG_CATEGORY UCLASS_FWU_MDATA
+#include <blk.h> +#include <dm.h> +#include <efi_loader.h> +#include <fwu.h> +#include <fwu_mdata.h> +#include <log.h> +#include <memalign.h> +#include <part.h> +#include <part_efi.h>
+#include <dm/device-internal.h> +#include <linux/errno.h> +#include <linux/types.h>
+#define PRIMARY_PART BIT(0) +#define SECONDARY_PART BIT(1) +#define BOTH_PARTS (PRIMARY_PART | SECONDARY_PART)
+#define MDATA_READ BIT(0) +#define MDATA_WRITE BIT(1)
+static int gpt_get_mdata_partitions(struct blk_desc *desc,
uint *primary_mpart,uint *secondary_mpart)+{
int i, ret;u32 mdata_parts;efi_guid_t part_type_guid;struct disk_partition info;const efi_guid_t fwu_mdata_guid = FWU_MDATA_GUID;mdata_parts = 0;for (i = 1; i < MAX_SEARCH_PARTITIONS; 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)) {++mdata_parts;if (!*primary_mpart)*primary_mpart = i;else*secondary_mpart = i;}}if (mdata_parts != 2) {log_debug("Expect two copies of the FWU metadata instead of %d\n",mdata_parts);ret = -EINVAL;} else {ret = 0;}Can we change that a bit? There are are some assumptions in this code, e.g the user must pass the values zero'ed out. Can we instead get an array of
I don't agree. If the function returns an error code, caller shall not use the output content. That is what callers of gpt_get_mdata_partitions() do in this patch, so returning -EINVAL if fine. (note that I don't mind this gets an array ref rather than 2 pointers).
best regards, etienne
2 instead? In that case you don't care if any of the values are zero and you can just fill in the array. Something like:
Okay, will make use of the array as you suggest.
if (!guidcmp(&fwu_mdata_guid, &part_type_guid)) { array[mdata_parts] = i; mdata_parts++; You can also move the check in that if and make the func a lot easier to read.
Sorry, I do not get this part.
return ret;+}
+static int gpt_get_mdata_disk_part(struct blk_desc *desc,
struct disk_partition *info,u32 part_num)+{
int ret;char *mdata_guid_str = "8a7a84a0-8387-40f6-ab41-a8b9a5a60d23";ret = part_get_info(desc, part_num, info);if (ret < 0) {log_debug("Unable to get the partition info for the FWU metadata part %d\n",part_num);return -ENOENT;}/* Check that it is indeed the FWU metadata partition */if (!strncmp(info->type_guid, mdata_guid_str, UUID_STR_LEN)) {/* Found the FWU metadata partition */return 0;}return -ENOENT;+}
+static int gpt_read_write_mdata(struct blk_desc *desc,
struct fwu_mdata *mdata,u8 access, u32 part_num)+{
int ret;u32 len, blk_start, blkcnt;struct disk_partition info;ALLOC_CACHE_ALIGN_BUFFER_PAD(struct fwu_mdata, mdata_aligned, 1,desc->blksz);ret = gpt_get_mdata_disk_part(desc, &info, part_num);if (ret < 0) {printf("Unable to get the FWU metadata partition\n");return -ENOENT;}len = sizeof(*mdata);blkcnt = BLOCK_CNT(len, desc);if (blkcnt > info.size) {log_debug("Block count exceeds FWU metadata partition size\n");return -ERANGE;}blk_start = info.start;if (access == MDATA_READ) {if (blk_dread(desc, blk_start, blkcnt, mdata_aligned) != blkcnt) {log_debug("Error reading FWU metadata from the device\n");return -EIO;}memcpy(mdata, mdata_aligned, sizeof(struct fwu_mdata));} else {if (blk_dwrite(desc, blk_start, blkcnt, mdata) != blkcnt) {log_debug("Error writing FWU metadata to the device\n");return -EIO;}}return 0;+}
+static int gpt_read_mdata(struct blk_desc *desc,
struct fwu_mdata *mdata, u32 part_num)+{
return gpt_read_write_mdata(desc, mdata, MDATA_READ, part_num);+}
+static int gpt_write_mdata_partition(struct blk_desc *desc,
struct fwu_mdata *mdata,u32 part_num)+{
return gpt_read_write_mdata(desc, mdata, MDATA_WRITE, part_num);+}
+static int fwu_gpt_update_mdata(struct udevice *dev, struct fwu_mdata *mdata) +{
int ret;struct blk_desc *desc;uint primary_mpart = 0, secondary_mpart = 0;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, &primary_mpart,&secondary_mpart);if (ret < 0) {log_debug("Error getting the FWU metadata partitions\n");return -ENOENT;}/* First write the primary partition */ret = gpt_write_mdata_partition(desc, mdata, primary_mpart);if (ret < 0) {log_debug("Updating primary FWU metadata partition failed\n");return ret;}/* And now the replica */ret = gpt_write_mdata_partition(desc, mdata, secondary_mpart);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;uint primary_mpart = 0, secondary_mpart = 0;ret = gpt_get_mdata_partitions(desc, &primary_mpart,&secondary_mpart);if (ret < 0) {log_debug("Error getting the FWU metadata partitions\n");return -ENOENT;}ret = gpt_read_mdata(desc, mdata, primary_mpart);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_mdata(desc, mdata, secondary_mpart);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 gpt_check_mdata_validity(struct udevice *dev) +{
int ret;struct blk_desc *desc;struct fwu_mdata pri_mdata;struct fwu_mdata secondary_mdata;uint primary_mpart = 0, secondary_mpart = 0;uint valid_partitions, invalid_partitions;struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);desc = dev_get_uclass_plat(priv->blk_dev);/** Two FWU metadata partitions are expected.* If we don't have two, user needs to create* them first*/valid_partitions = 0;ret = gpt_get_mdata_partitions(desc, &primary_mpart,&secondary_mpart);if (ret < 0) {log_debug("Error getting the FWU metadata partitions\n");return -ENOENT;}ret = gpt_read_mdata(desc, &pri_mdata, primary_mpart);if (ret < 0) {log_debug("Failed to read the FWU metadata from the device\n");goto secondary_read;Can't we skip the goto here and just make the if above (if ret == 0)?
Will change
}ret = fwu_verify_mdata(&pri_mdata, 1);if (!ret)valid_partitions |= PRIMARY_PART;+secondary_read:
/* Now check the secondary partition */ret = gpt_read_mdata(desc, &secondary_mdata, secondary_mpart);if (ret < 0) {log_debug("Failed to read the FWU metadata from the device\n");Same here
Will change
-sughosh
goto mdata_restore;}ret = fwu_verify_mdata(&secondary_mdata, 0);if (!ret)valid_partitions |= SECONDARY_PART;+mdata_restore:
if (valid_partitions == (PRIMARY_PART | SECONDARY_PART)) {ret = -1;/** Before returning, check that both the* FWU metadata copies are the same. If not,* the FWU metadata copies need to be* re-populated.*/if (!memcmp(&pri_mdata, &secondary_mdata,sizeof(struct fwu_mdata))) {ret = 0;} else {log_debug("Both FWU metadata copies are valid but do not match. Please check!\n");}goto out;}ret = -1;if (!(valid_partitions & BOTH_PARTS))goto out;invalid_partitions = valid_partitions ^ BOTH_PARTS;ret = gpt_write_mdata_partition(desc,(invalid_partitions == PRIMARY_PART) ?&secondary_mdata : &pri_mdata,(invalid_partitions == PRIMARY_PART) ?primary_mpart : secondary_mpart);if (ret < 0)log_debug("Restoring %s FWU metadata partition failed\n",(invalid_partitions == PRIMARY_PART) ?"primary" : "secondary");+out:
return ret;+}
+static int fwu_gpt_mdata_check(struct udevice *dev) +{
/** Check if both the copies of the FWU* metadata are valid. If one has gone* bad, restore it from the other good* copy.*/return gpt_check_mdata_validity(dev);+}
+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_get_mdata_device(struct udevice *dev, struct udevice **mdata_dev) +{
u32 phandle;int ret, size;struct udevice *parent;const fdt32_t *phandle_p = NULL;phandle_p = dev_read_prop(dev, "fwu-mdata-store", &size);if (!phandle_p) {log_debug("fwu-mdata-store property not found\n");return -ENOENT;}phandle = fdt32_to_cpu(*phandle_p);ret = device_get_global_by_ofnode(ofnode_get_by_phandle(phandle),&parent);if (ret)return ret;return blk_get_from_parent(parent, mdata_dev);+}
+static int fwu_mdata_gpt_blk_probe(struct udevice *dev) +{
int ret;struct udevice *mdata_dev = NULL;struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);ret = fwu_get_mdata_device(dev, &mdata_dev);if (ret)return ret;priv->blk_dev = mdata_dev;return 0;+}
+static const struct fwu_mdata_ops fwu_gpt_blk_ops = {
.mdata_check = fwu_gpt_mdata_check,.get_mdata = fwu_gpt_get_mdata,.update_mdata = fwu_gpt_update_mdata,+};
+static const struct udevice_id fwu_mdata_ids[] = {
{ .compatible = "u-boot,fwu-mdata-gpt" },{ }+};
+U_BOOT_DRIVER(fwu_mdata_gpt_blk) = {
.name = "fwu-mdata-gpt-blk",.id = UCLASS_FWU_MDATA,.of_match = fwu_mdata_ids,.ops = &fwu_gpt_blk_ops,.probe = fwu_mdata_gpt_blk_probe,.priv_auto = sizeof(struct fwu_mdata_gpt_blk_priv),+}; diff --git a/include/fwu.h b/include/fwu.h index 745f6225d0..1e16253f69 100644 --- a/include/fwu.h +++ b/include/fwu.h @@ -14,6 +14,10 @@ struct fwu_mdata; struct udevice;
+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
-- 2.34.1
Cheers /Ilias