| Message ID | 20221002235132.344240-1-jassisinghbrar@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | FWU: Add support for mtd backed feature on DeveloperBox | expand |
Hi Jassi, Thanks for the patches and apologies for the late reply On Sun, Oct 02, 2022 at 06:51:32PM -0500, jassisinghbrar@gmail.com wrote: > From: Sughosh Ganu <sughosh.ganu@linaro.org> > > In the FWU Multi Bank Update feature, the information about the > updatable images is stored as part of the metadata, on a separate > region. Add a driver for reading from and writing to the metadata > when the updatable images and the metadata are stored on a raw > MTD region. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org> > --- > drivers/fwu-mdata/Kconfig | 17 +- > drivers/fwu-mdata/Makefile | 1 + [...] > + > +/* Internal helper structure to move data around */ > +struct fwu_mdata_mtd_priv { > + struct mtd_info *mtd; > + u32 pri_offset; > + u32 sec_offset; > +}; > + > +enum fwu_mtd_op { > + FWU_MTD_READ, > + FWU_MTD_WRITE, > +}; > + > +#define FWU_MDATA_PRIMARY true > +#define FWU_MDATA_SECONDARY false > + > +static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size) > +{ > + return !do_div(size, mtd->erasesize); > +} > + Can we please add some sphinx style documentation overall ? > +static int mtd_io_data(struct mtd_info *mtd, u32 offs, u32 size, void *data, > + enum fwu_mtd_op op) > +{ > + struct mtd_oob_ops io_op ={}; > + u64 lock_offs, lock_len; > + size_t len; > + void *buf; > + int ret; > + > + if (!mtd_is_aligned_with_block_size(mtd, offs)) { > + log_err("Offset unaligned with a block (0x%x)\n", mtd->erasesize); > + return -EINVAL; > + } > + > + lock_offs = offs; > + /* This will expand erase size to align with the block size */ > + lock_len = round_up(size, mtd->erasesize); > + > + ret = mtd_unlock(mtd, lock_offs, lock_len); > + if (ret && ret != -EOPNOTSUPP) > + return ret; > + > + if (op == FWU_MTD_WRITE) { > + struct erase_info erase_op = {}; > + > + erase_op.mtd = mtd; > + erase_op.addr = lock_offs; > + erase_op.len = lock_len; > + erase_op.scrub = 0; > + > + ret = mtd_erase(mtd, &erase_op); > + if (ret) > + goto lock; > + } > + > + /* Also, expand the write size to align with the write size */ > + len = round_up(size, mtd->writesize); > + > + buf = memalign(ARCH_DMA_MINALIGN, len); > + if (!buf) { > + ret = -ENOMEM; > + goto lock; > + } > + memset(buf, 0xff, len); > + > + io_op.mode = MTD_OPS_AUTO_OOB; > + io_op.len = len; > + io_op.ooblen = 0; > + io_op.datbuf = buf; > + io_op.oobbuf = NULL; > + > + if (op == FWU_MTD_WRITE) { > + memcpy(buf, data, size); > + ret = mtd_write_oob(mtd, offs, &io_op); > + } else { > + ret = mtd_read_oob(mtd, offs, &io_op); > + if (!ret) > + memcpy(data, buf, size); > + } > + free(buf); > + > +lock: > + mtd_lock(mtd, lock_offs, lock_len); > + > + return ret; > +} > + > +static int fwu_mtd_load_mdata(struct mtd_info *mtd, struct fwu_mdata *mdata, > + u32 offs, bool primary) > +{ > + size_t size = sizeof(struct fwu_mdata); > + int ret; > + > + ret = mtd_io_data(mtd, offs, size, mdata, FWU_MTD_READ); > + if (ret >= 0) > + ret = fwu_verify_mdata(mdata, primary); > + > + return ret; > +} > + > +static int fwu_mtd_save_primary_mdata(struct fwu_mdata_mtd_priv *mtd_priv, > + struct fwu_mdata *mdata) > +{ > + return mtd_io_data(mtd_priv->mtd, mtd_priv->pri_offset, > + sizeof(struct fwu_mdata), mdata, FWU_MTD_WRITE); > +} > + > +static int fwu_mtd_save_secondary_mdata(struct fwu_mdata_mtd_priv *mtd_priv, > + struct fwu_mdata *mdata) > +{ > + return mtd_io_data(mtd_priv->mtd, mtd_priv->sec_offset, > + sizeof(struct fwu_mdata), mdata, FWU_MTD_WRITE); > +} > + > +static int fwu_mtd_get_valid_mdata(struct fwu_mdata_mtd_priv *mtd_priv, > + struct fwu_mdata *mdata) > +{ > + int ret; > + > + ret = fwu_mtd_load_mdata(mtd_priv->mtd, mdata, > + mtd_priv->pri_offset, FWU_MDATA_PRIMARY); > + if (!ret) > + return 0; > + > + log_debug("Failed to load primary mdata. Trying secondary...\n"); > + > + ret = fwu_mtd_load_mdata(mtd_priv->mtd, mdata, > + mtd_priv->sec_offset, FWU_MDATA_SECONDARY); > + if (ret) { > + log_debug("Failed to load secondary mdata.\n"); > + return -1; > + } > + > + return 0; > +} > + > +static int fwu_mtd_update_mdata(struct udevice *dev, struct fwu_mdata *mdata) > +{ > + struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev); > + int ret; > + > + /* First write the primary mdata */ > + ret = fwu_mtd_save_primary_mdata(mtd_priv, mdata); > + if (ret < 0) { > + log_debug("Failed to update the primary mdata.\n"); > + return ret; > + } > + > + /* And now the replica */ > + ret = fwu_mtd_save_secondary_mdata(mtd_priv, mdata); > + if (ret < 0) { > + log_debug("Failed to update the secondary mdata.\n"); > + return ret; > + } Looking at Sughosh patches as well, we can probably abstract this even more with either ptrs or just including the device type in the function arguments, since all update mdata functions are calling xxx_save_primary_mdata() -> xxx_ave_secondary_mdata(). But I am fine as-is for now. We can clean that up once the patches get in > + > + return 0; > +} > + > +static int fwu_mtd_check_mdata(struct udevice *dev) > +{ > + struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev); > + struct fwu_mdata primary, secondary; > + bool pri = false, sec = false; > + int ret; > + > + ret = fwu_mtd_load_mdata(mtd_priv->mtd, &primary, > + mtd_priv->pri_offset, FWU_MDATA_PRIMARY); > + if (ret < 0) > + log_debug("Failed to read the primary mdata: %d\n", ret); > + else > + pri = true; > + > + ret = fwu_mtd_load_mdata(mtd_priv->mtd, &secondary, > + mtd_priv->sec_offset, FWU_MDATA_SECONDARY); > + if (ret < 0) > + log_debug("Failed to read the secondary mdata: %d\n", ret); > + else > + sec = true; > + > + if (pri && sec) { > + if (memcmp(&primary, &secondary, sizeof(struct fwu_mdata))) { > + log_debug("The primary and the secondary mdata are different\n"); > + ret = -1; > + } > + } else if (pri) { > + ret = fwu_mtd_save_secondary_mdata(mtd_priv, &primary); > + if (ret < 0) > + log_debug("Restoring secondary mdata partition failed\n"); > + } else if (sec) { > + ret = fwu_mtd_save_primary_mdata(mtd_priv, &secondary); > + if (ret < 0) > + log_debug("Restoring primary mdata partition failed\n"); > + } Same on this one. The requirements here are - Read our metadata - Compare the 2 partitions The only thing that's 'hardware' specific here is reading the metadata. We should at least unify the comparing part and restoration in case of failures, no ? > + > + return ret; > +} > + > +static int fwu_mtd_get_mdata(struct udevice *dev, struct fwu_mdata *mdata) > +{ > + struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev); > + > + return fwu_mtd_get_valid_mdata(mtd_priv, mdata); > +} > + > +/** > + * fwu_mdata_mtd_of_to_plat() - Translate from DT to fwu mdata device > + */ Thanks /Ilias
On Fri, Oct 14, 2022 at 2:07 AM Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > On Sun, Oct 02, 2022 at 06:51:32PM -0500, jassisinghbrar@gmail.com wrote: > > + > > +static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size) > > +{ > > + return !do_div(size, mtd->erasesize); > > +} > > + > > Can we please add some sphinx style documentation overall ? > I can but I thought that is for public api and not static helper functions? > > + > > +static int fwu_mtd_check_mdata(struct udevice *dev) > > +{ > > + struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev); > > + struct fwu_mdata primary, secondary; > > + bool pri = false, sec = false; > > + int ret; > > + > > + ret = fwu_mtd_load_mdata(mtd_priv->mtd, &primary, > > + mtd_priv->pri_offset, FWU_MDATA_PRIMARY); > > + if (ret < 0) > > + log_debug("Failed to read the primary mdata: %d\n", ret); > > + else > > + pri = true; > > + > > + ret = fwu_mtd_load_mdata(mtd_priv->mtd, &secondary, > > + mtd_priv->sec_offset, FWU_MDATA_SECONDARY); > > + if (ret < 0) > > + log_debug("Failed to read the secondary mdata: %d\n", ret); > > + else > > + sec = true; > > + > > + if (pri && sec) { > > + if (memcmp(&primary, &secondary, sizeof(struct fwu_mdata))) { > > + log_debug("The primary and the secondary mdata are different\n"); > > + ret = -1; > > + } > > + } else if (pri) { > > + ret = fwu_mtd_save_secondary_mdata(mtd_priv, &primary); > > + if (ret < 0) > > + log_debug("Restoring secondary mdata partition failed\n"); > > + } else if (sec) { > > + ret = fwu_mtd_save_primary_mdata(mtd_priv, &secondary); > > + if (ret < 0) > > + log_debug("Restoring primary mdata partition failed\n"); > > + } > > Same on this one. The requirements here are > - Read our metadata > - Compare the 2 partitions > > The only thing that's 'hardware' specific here is reading the metadata. We > should at least unify the comparing part and restoration in case of > failures, no ? > Yes. Since redundant copy of meta-data is a requirement of the spec, we should maintain that in the fwu core code. Maybe something like adding 'bool primary' argument to get_mdata() and update_mdata() thanks
diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig index 36c4479a59..841c6f5b2d 100644 --- a/drivers/fwu-mdata/Kconfig +++ b/drivers/fwu-mdata/Kconfig @@ -6,11 +6,26 @@ config FWU_MDATA FWU Metadata partitions reside on the same storage device which contains the other FWU updatable firmware images. +choice + prompt "Storage Layout Scheme" + depends on FWU_MDATA + default FWU_MDATA_GPT_BLK + config FWU_MDATA_GPT_BLK - bool "FWU Metadata access for GPT partitioned Block devices" + bool "GPT Partitioned Block devices" select PARTITION_TYPE_GUID select PARTITION_UUIDS depends on FWU_MDATA && BLK && EFI_PARTITION help Enable support for accessing FWU Metadata on GPT partitioned block devices. + +config FWU_MDATA_MTD + bool "Raw MTD devices" + depends on MTD + help + Enable support for accessing FWU Metadata on non-partitioned + (or non-GPT partitioned, e.g. partition nodes in devicetree) + MTD devices. + +endchoice diff --git a/drivers/fwu-mdata/Makefile b/drivers/fwu-mdata/Makefile index 3fee64c10c..06c49747ba 100644 --- a/drivers/fwu-mdata/Makefile +++ b/drivers/fwu-mdata/Makefile @@ -6,3 +6,4 @@ obj-$(CONFIG_FWU_MDATA) += fwu-mdata-uclass.o obj-$(CONFIG_FWU_MDATA_GPT_BLK) += gpt_blk.o +obj-$(CONFIG_FWU_MDATA_MTD) += raw_mtd.o diff --git a/drivers/fwu-mdata/raw_mtd.c b/drivers/fwu-mdata/raw_mtd.c new file mode 100644 index 0000000000..ff2064b442 --- /dev/null +++ b/drivers/fwu-mdata/raw_mtd.c @@ -0,0 +1,305 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2022, Linaro Limited + */ + +#define LOG_CATEGORY UCLASS_FWU_MDATA + +#include <efi_loader.h> +#include <fwu.h> +#include <fwu_mdata.h> +#include <memalign.h> +#include <spi.h> +#include <spi_flash.h> +#include <flash.h> + +#include <linux/errno.h> +#include <linux/types.h> +#include <u-boot/crc.h> + +/* Internal helper structure to move data around */ +struct fwu_mdata_mtd_priv { + struct mtd_info *mtd; + u32 pri_offset; + u32 sec_offset; +}; + +enum fwu_mtd_op { + FWU_MTD_READ, + FWU_MTD_WRITE, +}; + +#define FWU_MDATA_PRIMARY true +#define FWU_MDATA_SECONDARY false + +static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size) +{ + return !do_div(size, mtd->erasesize); +} + +static int mtd_io_data(struct mtd_info *mtd, u32 offs, u32 size, void *data, + enum fwu_mtd_op op) +{ + struct mtd_oob_ops io_op ={}; + u64 lock_offs, lock_len; + size_t len; + void *buf; + int ret; + + if (!mtd_is_aligned_with_block_size(mtd, offs)) { + log_err("Offset unaligned with a block (0x%x)\n", mtd->erasesize); + return -EINVAL; + } + + lock_offs = offs; + /* This will expand erase size to align with the block size */ + lock_len = round_up(size, mtd->erasesize); + + ret = mtd_unlock(mtd, lock_offs, lock_len); + if (ret && ret != -EOPNOTSUPP) + return ret; + + if (op == FWU_MTD_WRITE) { + struct erase_info erase_op = {}; + + erase_op.mtd = mtd; + erase_op.addr = lock_offs; + erase_op.len = lock_len; + erase_op.scrub = 0; + + ret = mtd_erase(mtd, &erase_op); + if (ret) + goto lock; + } + + /* Also, expand the write size to align with the write size */ + len = round_up(size, mtd->writesize); + + buf = memalign(ARCH_DMA_MINALIGN, len); + if (!buf) { + ret = -ENOMEM; + goto lock; + } + memset(buf, 0xff, len); + + io_op.mode = MTD_OPS_AUTO_OOB; + io_op.len = len; + io_op.ooblen = 0; + io_op.datbuf = buf; + io_op.oobbuf = NULL; + + if (op == FWU_MTD_WRITE) { + memcpy(buf, data, size); + ret = mtd_write_oob(mtd, offs, &io_op); + } else { + ret = mtd_read_oob(mtd, offs, &io_op); + if (!ret) + memcpy(data, buf, size); + } + free(buf); + +lock: + mtd_lock(mtd, lock_offs, lock_len); + + return ret; +} + +static int fwu_mtd_load_mdata(struct mtd_info *mtd, struct fwu_mdata *mdata, + u32 offs, bool primary) +{ + size_t size = sizeof(struct fwu_mdata); + int ret; + + ret = mtd_io_data(mtd, offs, size, mdata, FWU_MTD_READ); + if (ret >= 0) + ret = fwu_verify_mdata(mdata, primary); + + return ret; +} + +static int fwu_mtd_save_primary_mdata(struct fwu_mdata_mtd_priv *mtd_priv, + struct fwu_mdata *mdata) +{ + return mtd_io_data(mtd_priv->mtd, mtd_priv->pri_offset, + sizeof(struct fwu_mdata), mdata, FWU_MTD_WRITE); +} + +static int fwu_mtd_save_secondary_mdata(struct fwu_mdata_mtd_priv *mtd_priv, + struct fwu_mdata *mdata) +{ + return mtd_io_data(mtd_priv->mtd, mtd_priv->sec_offset, + sizeof(struct fwu_mdata), mdata, FWU_MTD_WRITE); +} + +static int fwu_mtd_get_valid_mdata(struct fwu_mdata_mtd_priv *mtd_priv, + struct fwu_mdata *mdata) +{ + int ret; + + ret = fwu_mtd_load_mdata(mtd_priv->mtd, mdata, + mtd_priv->pri_offset, FWU_MDATA_PRIMARY); + if (!ret) + return 0; + + log_debug("Failed to load primary mdata. Trying secondary...\n"); + + ret = fwu_mtd_load_mdata(mtd_priv->mtd, mdata, + mtd_priv->sec_offset, FWU_MDATA_SECONDARY); + if (ret) { + log_debug("Failed to load secondary mdata.\n"); + return -1; + } + + return 0; +} + +static int fwu_mtd_update_mdata(struct udevice *dev, struct fwu_mdata *mdata) +{ + struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev); + int ret; + + /* First write the primary mdata */ + ret = fwu_mtd_save_primary_mdata(mtd_priv, mdata); + if (ret < 0) { + log_debug("Failed to update the primary mdata.\n"); + return ret; + } + + /* And now the replica */ + ret = fwu_mtd_save_secondary_mdata(mtd_priv, mdata); + if (ret < 0) { + log_debug("Failed to update the secondary mdata.\n"); + return ret; + } + + return 0; +} + +static int fwu_mtd_check_mdata(struct udevice *dev) +{ + struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev); + struct fwu_mdata primary, secondary; + bool pri = false, sec = false; + int ret; + + ret = fwu_mtd_load_mdata(mtd_priv->mtd, &primary, + mtd_priv->pri_offset, FWU_MDATA_PRIMARY); + if (ret < 0) + log_debug("Failed to read the primary mdata: %d\n", ret); + else + pri = true; + + ret = fwu_mtd_load_mdata(mtd_priv->mtd, &secondary, + mtd_priv->sec_offset, FWU_MDATA_SECONDARY); + if (ret < 0) + log_debug("Failed to read the secondary mdata: %d\n", ret); + else + sec = true; + + if (pri && sec) { + if (memcmp(&primary, &secondary, sizeof(struct fwu_mdata))) { + log_debug("The primary and the secondary mdata are different\n"); + ret = -1; + } + } else if (pri) { + ret = fwu_mtd_save_secondary_mdata(mtd_priv, &primary); + if (ret < 0) + log_debug("Restoring secondary mdata partition failed\n"); + } else if (sec) { + ret = fwu_mtd_save_primary_mdata(mtd_priv, &secondary); + if (ret < 0) + log_debug("Restoring primary mdata partition failed\n"); + } + + return ret; +} + +static int fwu_mtd_get_mdata(struct udevice *dev, struct fwu_mdata *mdata) +{ + struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev); + + return fwu_mtd_get_valid_mdata(mtd_priv, mdata); +} + +/** + * fwu_mdata_mtd_of_to_plat() - Translate from DT to fwu mdata device + */ +static int fwu_mdata_mtd_of_to_plat(struct udevice *dev) +{ + struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev); + const fdt32_t *phandle_p = NULL; + struct udevice *mtd_dev; + struct mtd_info *mtd; + int ret, size; + u32 phandle; + + /* Find the FWU mdata storage device */ + phandle_p = ofnode_get_property(dev_ofnode(dev), + "fwu-mdata-store", &size); + if (!phandle_p) { + log_err("FWU meta data store not defined in device-tree\n"); + return -ENOENT; + } + + phandle = fdt32_to_cpu(*phandle_p); + + ret = device_get_global_by_ofnode(ofnode_get_by_phandle(phandle), + &mtd_dev); + if (ret) { + log_err("FWU: failed to get mtd device\n"); + return ret; + } + + mtd_probe_devices(); + + mtd_for_each_device(mtd) { + if (mtd->dev == mtd_dev) { + mtd_priv->mtd = mtd; + log_debug("Found the FWU mdata mtd device %s\n", mtd->name); + break; + } + } + if (!mtd_priv->mtd) { + log_err("Failed to find mtd device by fwu-mdata-store\n"); + return -ENOENT; + } + + /* Get the offset of primary and seconday mdata */ + ret = ofnode_read_u32_index(dev_ofnode(dev), "mdata-offsets", 0, + &mtd_priv->pri_offset); + if (ret) + return ret; + ret = ofnode_read_u32_index(dev_ofnode(dev), "mdata-offsets", 1, + &mtd_priv->sec_offset); + if (ret) + return ret; + + return 0; +} + +static int fwu_mdata_mtd_probe(struct udevice *dev) +{ + /* Ensure the metadata can be read. */ + return fwu_mtd_check_mdata(dev); +} + +static struct fwu_mdata_ops fwu_mtd_ops = { + .check_mdata = fwu_mtd_check_mdata, + .get_mdata = fwu_mtd_get_mdata, + .update_mdata = fwu_mtd_update_mdata, +}; + +static const struct udevice_id fwu_mdata_ids[] = { + { .compatible = "u-boot,fwu-mdata-mtd" }, + { } +}; + +U_BOOT_DRIVER(fwu_mdata_mtd) = { + .name = "fwu-mdata-mtd", + .id = UCLASS_FWU_MDATA, + .of_match = fwu_mdata_ids, + .ops = &fwu_mtd_ops, + .probe = fwu_mdata_mtd_probe, + .of_to_plat = fwu_mdata_mtd_of_to_plat, + .priv_auto = sizeof(struct fwu_mdata_mtd_priv), +};