All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox
@ 2023-04-10 23:01 jaswinder.singh
  2023-04-10 23:02 ` [PATCH v5 1/6] FWU: Add FWU metadata access driver for MTD storage regions jaswinder.singh
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: jaswinder.singh @ 2023-04-10 23:01 UTC (permalink / raw)
  To: u-boot
  Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
	xypron.glpk, takahiro.akashi, michal.simek, Jassi Brar

From: Jassi Brar <jaswinder.singh@linaro.org>

Introduce support for mtd backed storage for FWU feature and enable it on
Synquacer platform based DeveloperBox.

This revision is rebased onto patchset that trims the FWU api
 https://lore.kernel.org/u-boot/20230306231747.1888513-1-jassisinghbrar@gmail.com/

Changes since v4:
	* Provide default/weak implementations of fwu_plat_get_alt_num and fwu_plat_get_bootidx
	* Provide man page for mkfwumdata
	* Misc typo fixes and cosmetic changes

Changes since v3:
	* Fix and Update documentation to also build optee for FWU FIP image.
	* Fixed checkpatch warnings
	* Made local functions static.
	* Split config changes to a separate patch
	* Fix authorship of three patches.

Jassi Brar (4):
  dt: fwu: developerbox: enable fwu banks and mdata regions
  configs: move to new flash layout and boot flow
  fwu: DeveloperBox: add support for FWU
  fwu: provide default fwu_plat_get_bootidx

Masami Hiramatsu (2):
  FWU: Add FWU metadata access driver for MTD storage regions
  tools: Add mkfwumdata tool for FWU metadata image

 .../synquacer-sc2a11-developerbox-u-boot.dtsi |  49 ++-
 board/socionext/developerbox/Makefile         |   1 +
 board/socionext/developerbox/developerbox.c   |   8 +
 board/socionext/developerbox/fwu_plat.c       |  37 ++
 configs/synquacer_developerbox_defconfig      |  12 +-
 doc/board/socionext/developerbox.rst          | 155 +++++++-
 doc/mkfwumdata.1                              |  89 +++++
 drivers/fwu-mdata/Kconfig                     |  15 +
 drivers/fwu-mdata/Makefile                    |   1 +
 drivers/fwu-mdata/raw_mtd.c                   | 272 ++++++++++++++
 include/configs/synquacer.h                   |  10 +
 include/fwu.h                                 |  32 ++
 lib/fwu_updates/Makefile                      |   1 +
 lib/fwu_updates/fwu.c                         |  18 +
 lib/fwu_updates/fwu_mtd.c                     | 185 ++++++++++
 tools/Kconfig                                 |   9 +
 tools/Makefile                                |   4 +
 tools/mkfwumdata.c                            | 334 ++++++++++++++++++
 18 files changed, 1221 insertions(+), 11 deletions(-)
 create mode 100644 board/socionext/developerbox/fwu_plat.c
 create mode 100644 doc/mkfwumdata.1
 create mode 100644 drivers/fwu-mdata/raw_mtd.c
 create mode 100644 lib/fwu_updates/fwu_mtd.c
 create mode 100644 tools/mkfwumdata.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 1/6] FWU: Add FWU metadata access driver for MTD storage regions
  2023-04-10 23:01 [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox jaswinder.singh
@ 2023-04-10 23:02 ` jaswinder.singh
  2023-04-11 17:34   ` Etienne Carriere
  2023-04-10 23:03 ` [PATCH v5 2/6] tools: Add mkfwumdata tool for FWU metadata image jaswinder.singh
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: jaswinder.singh @ 2023-04-10 23:02 UTC (permalink / raw)
  To: u-boot
  Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
	xypron.glpk, takahiro.akashi, michal.simek, Masami Hiramatsu,
	Jassi Brar

From: Masami Hiramatsu <masami.hiramatsu@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.
The code is divided into core under drivers/fwu-mdata/ and some helper
functions clubbed together under lib/fwu_updates/

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 drivers/fwu-mdata/Kconfig   |  15 ++
 drivers/fwu-mdata/Makefile  |   1 +
 drivers/fwu-mdata/raw_mtd.c | 272 ++++++++++++++++++++++++++++++++++++
 include/fwu.h               |  32 +++++
 lib/fwu_updates/Makefile    |   1 +
 lib/fwu_updates/fwu_mtd.c   | 185 ++++++++++++++++++++++++
 6 files changed, 506 insertions(+)
 create mode 100644 drivers/fwu-mdata/raw_mtd.c
 create mode 100644 lib/fwu_updates/fwu_mtd.c

diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig
index 36c4479a59..42736a5e43 100644
--- a/drivers/fwu-mdata/Kconfig
+++ b/drivers/fwu-mdata/Kconfig
@@ -6,6 +6,11 @@ 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"
 	select PARTITION_TYPE_GUID
@@ -14,3 +19,13 @@ config FWU_MDATA_GPT_BLK
 	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..25c1aa33ec
--- /dev/null
+++ b/drivers/fwu-mdata/raw_mtd.c
@@ -0,0 +1,272 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023, Linaro Limited
+ */
+
+#define LOG_CATEGORY UCLASS_FWU_MDATA
+
+#include <fwu.h>
+#include <fwu_mdata.h>
+#include <memalign.h>
+
+#include <linux/errno.h>
+#include <linux/types.h>
+
+/* Internal helper structure to move data around */
+struct fwu_mdata_mtd_priv {
+	struct mtd_info *mtd;
+	char pri_label[50];
+	char sec_label[50];
+	u32 pri_offset;
+	u32 sec_offset;
+};
+
+enum fwu_mtd_op {
+	FWU_MTD_READ,
+	FWU_MTD_WRITE,
+};
+
+extern struct fwu_mtd_image_info fwu_mtd_images[];
+
+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_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary)
+{
+	struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev);
+	struct mtd_info *mtd = mtd_priv->mtd;
+	u32 offs = primary ? mtd_priv->pri_offset : mtd_priv->sec_offset;
+
+	return mtd_io_data(mtd, offs, sizeof(struct fwu_mdata), mdata, FWU_MTD_READ);
+}
+
+static int fwu_mtd_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary)
+{
+	struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev);
+	struct mtd_info *mtd = mtd_priv->mtd;
+	u32 offs = primary ? mtd_priv->pri_offset : mtd_priv->sec_offset;
+
+	return mtd_io_data(mtd, offs, sizeof(struct fwu_mdata), mdata, FWU_MTD_WRITE);
+}
+
+static int flash_partition_offset(struct udevice *dev, const char *part_name, fdt_addr_t *offset)
+{
+	ofnode node, parts_node;
+	fdt_addr_t size = 0;
+
+	parts_node = ofnode_by_compatible(dev_ofnode(dev), "fixed-partitions");
+	node = ofnode_by_prop_value(parts_node, "label", part_name, strlen(part_name) + 1);
+	if (!ofnode_valid(node)) {
+		log_err("Warning: Failed to find partition by label <%s>\n", part_name);
+		return -ENOENT;
+	}
+
+	*offset = ofnode_get_addr_size_index_notrans(node, 0, &size);
+
+	return (int)size;
+}
+
+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;
+	const char *label;
+	fdt_addr_t offset;
+	int ret, size;
+	u32 phandle;
+	ofnode bank;
+	int off_img;
+
+	/* 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 -ENODEV;
+	}
+
+	/* Get the offset of primary and secondary mdata */
+	ret = ofnode_read_string_index(dev_ofnode(dev), "mdata-parts", 0, &label);
+	if (ret)
+		return ret;
+	strcpy(mtd_priv->pri_label, label);
+
+	ret = flash_partition_offset(mtd_dev, mtd_priv->pri_label, &offset);
+	if (ret <= 0)
+		return ret;
+	mtd_priv->pri_offset = offset;
+
+	ret = ofnode_read_string_index(dev_ofnode(dev), "mdata-parts", 1, &label);
+	if (ret)
+		return ret;
+	strcpy(mtd_priv->sec_label, label);
+
+	ret = flash_partition_offset(mtd_dev, mtd_priv->sec_label, &offset);
+	if (ret <= 0)
+		return ret;
+	mtd_priv->sec_offset = offset;
+
+	off_img = 0;
+
+	ofnode_for_each_subnode(bank, dev_ofnode(dev)) {
+		int bank_num, bank_offset, bank_size;
+		const char *bank_name;
+		ofnode image;
+
+		ofnode_read_u32(bank, "id", &bank_num);
+		bank_name = ofnode_read_string(bank, "label");
+		bank_size = flash_partition_offset(mtd_dev, bank_name, &offset);
+		if (bank_size <= 0)
+			return bank_size;
+		bank_offset = offset;
+		log_debug("Bank%d: %s [0x%x - 0x%x]\n",
+			  bank_num, bank_name, bank_offset, bank_offset + bank_size);
+
+		ofnode_for_each_subnode(image, bank) {
+			int image_num, image_offset, image_size;
+			const char *uuid;
+
+			if (off_img == CONFIG_FWU_NUM_BANKS *
+						CONFIG_FWU_NUM_IMAGES_PER_BANK) {
+				log_err("DT provides images more than configured!\n");
+				break;
+			}
+
+			uuid = ofnode_read_string(image, "uuid");
+			ofnode_read_u32(image, "id", &image_num);
+			ofnode_read_u32(image, "offset", &image_offset);
+			ofnode_read_u32(image, "size", &image_size);
+
+			fwu_mtd_images[off_img].start = bank_offset + image_offset;
+			fwu_mtd_images[off_img].size = image_size;
+			fwu_mtd_images[off_img].bank_num = bank_num;
+			fwu_mtd_images[off_img].image_num = image_num;
+			strcpy(fwu_mtd_images[off_img].uuidbuf, uuid);
+			log_debug("\tImage%d: %s @0x%x\n\n",
+				  image_num, uuid, bank_offset + image_offset);
+			off_img++;
+		}
+	}
+
+	return 0;
+}
+
+static int fwu_mdata_mtd_probe(struct udevice *dev)
+{
+	/* Ensure the metadata can be read. */
+	return fwu_get_mdata(NULL);
+}
+
+static struct fwu_mdata_ops fwu_mtd_ops = {
+	.read_mdata = fwu_mtd_read_mdata,
+	.write_mdata = fwu_mtd_write_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),
+};
diff --git a/include/fwu.h b/include/fwu.h
index 33b4d0b3db..edb7e9da90 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -8,6 +8,8 @@
 
 #include <blk.h>
 #include <efi.h>
+#include <mtd.h>
+#include <uuid.h>
 
 #include <linux/types.h>
 
@@ -18,6 +20,12 @@ struct fwu_mdata_gpt_blk_priv {
 	struct udevice *blk_dev;
 };
 
+struct fwu_mtd_image_info {
+	u32 start, size;
+	int bank_num, image_num;
+	char uuidbuf[UUID_STR_LEN + 1];
+};
+
 struct fwu_mdata_ops {
 	/**
 	 * read_mdata() - Populate the asked FWU metadata copy
@@ -251,4 +259,28 @@ u8 fwu_empty_capsule_checks_pass(void);
  */
 int fwu_trial_state_ctr_start(void);
 
+/**
+ * fwu_gen_alt_info_from_mtd() - Parse dfu_alt_info from metadata in mtd
+ * @buf: Buffer into which the dfu_alt_info is filled
+ * @len: Maximum characters that can be written in buf
+ * @mtd: Pointer to underlying MTD device
+ *
+ * Parse dfu_alt_info from metadata in mtd. Used for setting the env.
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int fwu_gen_alt_info_from_mtd(char *buf, size_t len, struct mtd_info *mtd);
+
+/**
+ * fwu_mtd_get_alt_num() - Mapping of fwu_plat_get_alt_num for MTD device
+ * @image_guid: Image GUID for which DFU alt number needs to be retrieved
+ * @alt_num: Pointer to the alt_num
+ * @mtd_dev: Name of mtd device instance
+ *
+ * To map fwu_plat_get_alt_num onto mtd based metadata implementation.
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int fwu_mtd_get_alt_num(efi_guid_t *image_guid, u8 *alt_num, const char *mtd_dev);
+
 #endif /* _FWU_H_ */
diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
index 1993088e5b..c9e3c06b48 100644
--- a/lib/fwu_updates/Makefile
+++ b/lib/fwu_updates/Makefile
@@ -5,3 +5,4 @@
 
 obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
 obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_gpt.o
+obj-$(CONFIG_FWU_MDATA_MTD) += fwu_mtd.o
diff --git a/lib/fwu_updates/fwu_mtd.c b/lib/fwu_updates/fwu_mtd.c
new file mode 100644
index 0000000000..f22c8b9443
--- /dev/null
+++ b/lib/fwu_updates/fwu_mtd.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023, Linaro Limited
+ */
+
+#include <dm.h>
+#include <dfu.h>
+#include <fwu.h>
+#include <fwu_mdata.h>
+#include <log.h>
+#include <malloc.h>
+#include <mtd.h>
+#include <uuid.h>
+#include <vsprintf.h>
+
+#include <dm/ofnode.h>
+
+struct fwu_mtd_image_info
+fwu_mtd_images[CONFIG_FWU_NUM_BANKS * CONFIG_FWU_NUM_IMAGES_PER_BANK];
+
+static struct fwu_mtd_image_info *mtd_img_by_uuid(const char *uuidbuf)
+{
+	int num_images = ARRAY_SIZE(fwu_mtd_images);
+
+	for (int i = 0; i < num_images; i++)
+		if (!strcmp(uuidbuf, fwu_mtd_images[i].uuidbuf))
+			return &fwu_mtd_images[i];
+
+	return NULL;
+}
+
+int fwu_mtd_get_alt_num(efi_guid_t *image_id, u8 *alt_num,
+			const char *mtd_dev)
+{
+	struct fwu_mtd_image_info *mtd_img_info;
+	char uuidbuf[UUID_STR_LEN + 1];
+	fdt_addr_t offset, size = 0;
+	struct dfu_entity *dfu;
+	int i, nalt, ret;
+
+	mtd_probe_devices();
+
+	uuid_bin_to_str(image_id->b, uuidbuf, UUID_STR_FORMAT_STD);
+
+	mtd_img_info = mtd_img_by_uuid(uuidbuf);
+	if (!mtd_img_info) {
+		log_err("%s: Not found partition for image %s\n", __func__, uuidbuf);
+		return -ENOENT;
+	}
+
+	offset = mtd_img_info->start;
+	size = mtd_img_info->size;
+
+	ret = dfu_init_env_entities(NULL, NULL);
+	if (ret)
+		return -ENOENT;
+
+	nalt = 0;
+	list_for_each_entry(dfu, &dfu_list, list)
+		nalt++;
+
+	if (!nalt) {
+		log_warning("No entities in dfu_alt_info\n");
+		dfu_free_entities();
+		return -ENOENT;
+	}
+
+	ret = -1;
+	for (i = 0; i < nalt; i++) {
+		dfu = dfu_get_entity(i);
+
+		/* Only MTD RAW access */
+		if (!dfu || dfu->dev_type != DFU_DEV_MTD ||
+		    dfu->layout != DFU_RAW_ADDR ||
+			dfu->data.mtd.start != offset ||
+			dfu->data.mtd.size != size)
+			continue;
+
+		*alt_num = dfu->alt;
+		ret = 0;
+		break;
+	}
+
+	dfu_free_entities();
+
+	log_debug("%s: %s -> %d\n", __func__, uuidbuf, *alt_num);
+	return ret;
+}
+
+/**
+ * fwu_plat_get_alt_num() - Get the DFU Alt Num for the image from the platform
+ * @dev: FWU device
+ * @image_id: Image GUID for which DFU alt number needs to be retrieved
+ * @alt_num: Pointer to the alt_num
+ *
+ * Get the DFU alt number from the platform for the image specified by the
+ * image GUID.
+ *
+ * Note: This is a weak function and platforms can override this with
+ * their own implementation for obtaining the alt number value.
+ *
+ * Return: 0 if OK, -ve on error
+ */
+__weak int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_id,
+				u8 *alt_num)
+{
+	return fwu_mtd_get_alt_num(image_id, alt_num, "nor1");
+}
+
+static int gen_image_alt_info(char *buf, size_t len, int sidx,
+			      struct fwu_image_entry *img, struct mtd_info *mtd)
+{
+	char *p = buf, *end = buf + len;
+	int i;
+
+	p += snprintf(p, end - p, "mtd %s", mtd->name);
+	if (end < p) {
+		log_err("%s:%d Run out of buffer\n", __func__, __LINE__);
+		return -E2BIG;
+	}
+
+	/*
+	 * List the image banks in the FWU mdata and search the corresponding
+	 * partition based on partition's uuid.
+	 */
+	for (i = 0; i < CONFIG_FWU_NUM_BANKS; i++) {
+		struct fwu_mtd_image_info *mtd_img_info;
+		struct fwu_image_bank_info *bank;
+		char uuidbuf[UUID_STR_LEN + 1];
+		u32 offset, size;
+
+		/* Query a partition by image UUID */
+		bank = &img->img_bank_info[i];
+		uuid_bin_to_str(bank->image_uuid.b, uuidbuf, UUID_STR_FORMAT_STD);
+
+		mtd_img_info = mtd_img_by_uuid(uuidbuf);
+		if (!mtd_img_info) {
+			log_err("%s: Not found partition for image %s\n", __func__, uuidbuf);
+			break;
+		}
+
+		offset = mtd_img_info->start;
+		size = mtd_img_info->size;
+
+		p += snprintf(p, end - p, "%sbank%d raw %x %x",
+			      i == 0 ? "=" : ";", i, offset, size);
+		if (end < p) {
+			log_err("%s:%d Run out of buffer\n", __func__, __LINE__);
+			return -E2BIG;
+		}
+	}
+
+	if (i == CONFIG_FWU_NUM_BANKS)
+		return 0;
+
+	return -ENOENT;
+}
+
+int fwu_gen_alt_info_from_mtd(char *buf, size_t len, struct mtd_info *mtd)
+{
+	struct fwu_mdata mdata;
+	int i, l, ret;
+
+	ret = fwu_get_mdata(&mdata);
+	if (ret < 0) {
+		log_err("Failed to get the FWU mdata.\n");
+		return ret;
+	}
+
+	for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
+		ret = gen_image_alt_info(buf, len, i * CONFIG_FWU_NUM_BANKS,
+					 &mdata.img_entry[i], mtd);
+		if (ret)
+			break;
+
+		l = strlen(buf);
+		/* Replace the last ';' with '&' if there is another image. */
+		if (i != CONFIG_FWU_NUM_IMAGES_PER_BANK - 1 && l)
+			buf[l - 1] = '&';
+		len -= l;
+		buf += l;
+	}
+
+	return ret;
+}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v5 2/6] tools: Add mkfwumdata tool for FWU metadata image
  2023-04-10 23:01 [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox jaswinder.singh
  2023-04-10 23:02 ` [PATCH v5 1/6] FWU: Add FWU metadata access driver for MTD storage regions jaswinder.singh
@ 2023-04-10 23:03 ` jaswinder.singh
  2023-04-11 17:28   ` Etienne Carriere
  2023-04-10 23:03 ` [PATCH v5 3/6] dt: fwu: developerbox: enable fwu banks and mdata regions jaswinder.singh
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: jaswinder.singh @ 2023-04-10 23:03 UTC (permalink / raw)
  To: u-boot
  Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
	xypron.glpk, takahiro.akashi, michal.simek, Masami Hiramatsu,
	Jassi Brar

From: Masami Hiramatsu <masami.hiramatsu@linaro.org>

Add 'mkfwumdata' tool to generate FWU metadata image for the meta-data
partition to be used in A/B Update imeplementation.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 doc/mkfwumdata.1   |  89 ++++++++++++
 tools/Kconfig      |   9 ++
 tools/Makefile     |   4 +
 tools/mkfwumdata.c | 334 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 436 insertions(+)
 create mode 100644 doc/mkfwumdata.1
 create mode 100644 tools/mkfwumdata.c

diff --git a/doc/mkfwumdata.1 b/doc/mkfwumdata.1
new file mode 100644
index 0000000000..7dd718b26e
--- /dev/null
+++ b/doc/mkfwumdata.1
@@ -0,0 +1,89 @@
+.\" SPDX-License-Identifier: GPL-2.0-or-later
+.\" Copyright (C) 2023 Jassi Brar <jaswinder.singh@linaro.org>
+.TH MKFWUMDATA 1 2023-04-10 U-Boot
+.SH NAME
+mkfwumdata \- create FWU metadata image
+.
+.SH SYNOPSIS
+.SY mkfwumdata
+.OP \-a activeidx
+.OP \-p previousidx
+.OP \-g
+.BI \-i\~ imagecount
+.BI \-b\~ bankcount
+.I UUIDs
+.I outputimage
+.YS
+.SY mkfwumdata
+.B \-h
+.YS
+.
+.SH DESCRIPTION
+.B mkfwumdata
+creates metadata info to be used with FWU.
+.
+.SH OPTIONS
+.TP
+.B \-h
+Print usage information and exit.
+.
+.TP
+.B \-a
+Set 
+.IR activeidx
+as the currently active Bank. Default is 0.
+.
+.TP
+.B \-p
+Set 
+.IR previousidx
+as the previous active Bank. Default is
+.IR activeidx "-1"
+or
+.IR bankcount "-1,"
+whichever is non-negative.
+.
+.TP
+.B \-g
+Convert the
+.IR UUIDs
+as GUIDs before use.
+.
+.TP
+.B \-i
+Specify there are
+.IR imagecount
+images in each bank.
+.
+.TP
+.B \-b
+Specify there are a total of
+.IR bankcount
+banks.
+.
+.TP
+.IR UUIDs
+Comma-separated list of UUIDs required to create the metadata :-
+location_uuid,image_type_uuid,<images per bank uuid list of all banks>
+.
+.TP
+.IR outputimage
+Specify the name of the metadata image file to be created.
+.
+.SH BUGS
+Please report bugs to the
+.UR https://\:source\:.denx\:.de/\:u-boot/\:u-boot/\:issues
+U-Boot bug tracker
+.UE .
+.SH EXAMPLES
+Create a metadata image with 2 banks and 1 image/bank, BankAct=0, BankPrev=1:
+.PP
+.EX
+.in +4
+$ \c
+.B mkfwumdata \-a 0 \-p 1 \-b 2 \-i 1 \\\\\&
+.in +6
+.B 17e86d77-41f9-4fd7-87ec-a55df9842de5,\\\\\&
+.B 10c36d7d-ca52-b843-b7b9-f9d6c501d108,\\\\\&
+.B 5a66a702-99fd-4fef-a392-c26e261a2828,a8f868a1-6e5c-4757-878d-ce63375ef2c0 \\\\\&
+.B fwu-mdata.img
diff --git a/tools/Kconfig b/tools/Kconfig
index 539708f277..6e23f44d55 100644
--- a/tools/Kconfig
+++ b/tools/Kconfig
@@ -157,4 +157,13 @@ config LUT_SEQUENCE
 	help
 	  Look Up Table Sequence
 
+config TOOLS_MKFWUMDATA
+	bool "Build mkfwumdata command"
+	default y if FWU_MULTI_BANK_UPDATE
+	help
+	  This command allows users to create a raw image of the FWU
+	  metadata for initial installation of the FWU multi bank
+	  update on the board. The installation method depends on
+	  the platform.
+
 endmenu
diff --git a/tools/Makefile b/tools/Makefile
index 38699b069d..1e3fce0b1c 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -250,6 +250,10 @@ HOSTLDLIBS_mkeficapsule += \
 	$(shell pkg-config --libs uuid 2> /dev/null || echo "-luuid")
 hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
 
+mkfwumdata-objs := mkfwumdata.o lib/crc32.o
+HOSTLDLIBS_mkfwumdata += -luuid
+hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata
+
 # We build some files with extra pedantic flags to try to minimize things
 # that won't build on some weird host compiler -- though there are lots of
 # exceptions for files that aren't complaint.
diff --git a/tools/mkfwumdata.c b/tools/mkfwumdata.c
new file mode 100644
index 0000000000..43dabf3b72
--- /dev/null
+++ b/tools/mkfwumdata.c
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2023, Linaro Limited
+ */
+
+#include <errno.h>
+#include <getopt.h>
+#include <limits.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#include <u-boot/crc.h>
+#include <unistd.h>
+#include <uuid/uuid.h>
+
+/* This will dynamically allocate the fwu_mdata */
+#define CONFIG_FWU_NUM_BANKS		0
+#define CONFIG_FWU_NUM_IMAGES_PER_BANK	0
+
+/* Since we can not include fwu.h, redefine version here. */
+#define FWU_MDATA_VERSION		1
+
+typedef uint8_t u8;
+typedef int16_t s16;
+typedef uint16_t u16;
+typedef uint32_t u32;
+typedef uint64_t u64;
+
+#include <fwu_mdata.h>
+
+/* TODO: Endianness conversion may be required for some arch. */
+
+static const char *opts_short = "b:i:a:p:gh";
+
+static struct option options[] = {
+	{"banks", required_argument, NULL, 'b'},
+	{"images", required_argument, NULL, 'i'},
+	{"guid", required_argument, NULL, 'g'},
+	{"active-bank", required_argument, NULL, 'a'},
+	{"previous-bank", required_argument, NULL, 'p'},
+	{"help", no_argument, NULL, 'h'},
+	{NULL, 0, NULL, 0},
+};
+
+static void print_usage(void)
+{
+	fprintf(stderr, "Usage: mkfwumdata [options] <UUIDs list> <output file>\n");
+	fprintf(stderr, "Options:\n"
+		"\t-i, --images <num>          Number of images\n"
+		"\t-b, --banks  <num>          Number of banks\n"
+		"\t-a, --active-bank  <num>    Active bank\n"
+		"\t-p, --previous-bank  <num>  Previous active bank\n"
+		"\t-g, --guid                  Use GUID instead of UUID\n"
+		"\t-h, --help                  print a help message\n"
+		);
+	fprintf(stderr, "  UUIDs list syntax:\n"
+		"\t  <location uuid>,<image type uuid>,<images uuid list>\n"
+		"\t     images uuid list syntax:\n"
+		"\t        img_uuid_00,img_uuid_01...img_uuid_0b,\n"
+		"\t        img_uuid_10,img_uuid_11...img_uuid_1b,\n"
+		"\t        ...,\n"
+		"\t        img_uuid_i0,img_uuid_i1...img_uuid_ib,\n"
+		"\t          where 'b' and 'i' are number of banks and number\n"
+		"\t          of images in a bank respectively.\n"
+	       );
+}
+
+struct fwu_mdata_object {
+	size_t images;
+	size_t banks;
+	size_t size;
+	struct fwu_mdata *mdata;
+};
+
+static int previous_bank, active_bank;
+static bool __use_guid;
+
+static struct fwu_mdata_object *fwu_alloc_mdata(size_t images, size_t banks)
+{
+	struct fwu_mdata_object *mobj;
+
+	mobj = calloc(1, sizeof(*mobj));
+	if (!mobj)
+		return NULL;
+
+	mobj->size = sizeof(struct fwu_mdata) +
+		(sizeof(struct fwu_image_entry) +
+		 sizeof(struct fwu_image_bank_info) * banks) * images;
+	mobj->images = images;
+	mobj->banks = banks;
+
+	mobj->mdata = calloc(1, mobj->size);
+	if (!mobj->mdata) {
+		free(mobj);
+		return NULL;
+	}
+
+	return mobj;
+}
+
+static struct fwu_image_entry *
+fwu_get_image(struct fwu_mdata_object *mobj, size_t idx)
+{
+	size_t offset;
+
+	offset = sizeof(struct fwu_mdata) +
+		(sizeof(struct fwu_image_entry) +
+		 sizeof(struct fwu_image_bank_info) * mobj->banks) * idx;
+
+	return (struct fwu_image_entry *)((char *)mobj->mdata + offset);
+}
+
+static struct fwu_image_bank_info *
+fwu_get_bank(struct fwu_mdata_object *mobj, size_t img_idx, size_t bnk_idx)
+{
+	size_t offset;
+
+	offset = sizeof(struct fwu_mdata) +
+		(sizeof(struct fwu_image_entry) +
+		 sizeof(struct fwu_image_bank_info) * mobj->banks) * img_idx +
+		sizeof(struct fwu_image_entry) +
+		sizeof(struct fwu_image_bank_info) * bnk_idx;
+
+	return (struct fwu_image_bank_info *)((char *)mobj->mdata + offset);
+}
+
+/**
+ * convert_uuid_to_guid() - convert UUID to GUID
+ * @buf:	UUID binary
+ *
+ * UUID and GUID have the same data structure, but their binary
+ * formats are different due to the endianness. See lib/uuid.c.
+ * Since uuid_parse() can handle only UUID, this function must
+ * be called to get correct data for GUID when parsing a string.
+ *
+ * The correct data will be returned in @buf.
+ */
+static void convert_uuid_to_guid(unsigned char *buf)
+{
+	unsigned char c;
+
+	c = buf[0];
+	buf[0] = buf[3];
+	buf[3] = c;
+	c = buf[1];
+	buf[1] = buf[2];
+	buf[2] = c;
+
+	c = buf[4];
+	buf[4] = buf[5];
+	buf[5] = c;
+
+	c = buf[6];
+	buf[6] = buf[7];
+	buf[7] = c;
+}
+
+static int uuid_guid_parse(char *uuidstr, unsigned char *uuid)
+{
+	int ret;
+
+	ret = uuid_parse(uuidstr, uuid);
+	if (ret < 0)
+		return ret;
+
+	if (__use_guid)
+		convert_uuid_to_guid(uuid);
+
+	return ret;
+}
+
+static int
+fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
+			  size_t idx, char *uuids)
+{
+	struct fwu_image_entry *image = fwu_get_image(mobj, idx);
+	struct fwu_image_bank_info *bank;
+	char *p = uuids, *uuid;
+	int i;
+
+	if (!image)
+		return -ENOENT;
+
+	/* Image location UUID */
+	uuid = strsep(&p, ",");
+	if (!uuid)
+		return -EINVAL;
+
+	if (strcmp(uuid, "0") &&
+	    uuid_guid_parse(uuid, (unsigned char *)&image->location_uuid) < 0)
+		return -EINVAL;
+
+	/* Image type UUID */
+	uuid = strsep(&p, ",");
+	if (!uuid)
+		return -EINVAL;
+
+	if (uuid_guid_parse(uuid, (unsigned char *)&image->image_type_uuid) < 0)
+		return -EINVAL;
+
+	/* Fill bank image-UUID */
+	for (i = 0; i < mobj->banks; i++) {
+		bank = fwu_get_bank(mobj, idx, i);
+		if (!bank)
+			return -ENOENT;
+		bank->accepted = 1;
+		uuid = strsep(&p, ",");
+		if (!uuid)
+			return -EINVAL;
+
+		if (strcmp(uuid, "0") &&
+		    uuid_guid_parse(uuid, (unsigned char *)&bank->image_uuid) < 0)
+			return -EINVAL;
+	}
+	return 0;
+}
+
+/* Caller must ensure that @uuids[] has @mobj->images entries. */
+static int fwu_parse_fill_uuids(struct fwu_mdata_object *mobj, char *uuids[])
+{
+	struct fwu_mdata *mdata = mobj->mdata;
+	int i, ret;
+
+	mdata->version = FWU_MDATA_VERSION;
+	mdata->active_index = active_bank;
+	mdata->previous_active_index = previous_bank;
+
+	for (i = 0; i < mobj->images; i++) {
+		ret = fwu_parse_fill_image_uuid(mobj, i, uuids[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	mdata->crc32 = crc32(0, (const unsigned char *)&mdata->version,
+			     mobj->size - sizeof(uint32_t));
+
+	return 0;
+}
+
+static int
+fwu_make_mdata(size_t images, size_t banks, char *uuids[], char *output)
+{
+	struct fwu_mdata_object *mobj;
+	FILE *file;
+	int ret;
+
+	mobj = fwu_alloc_mdata(images, banks);
+	if (!mobj)
+		return -ENOMEM;
+
+	ret = fwu_parse_fill_uuids(mobj, uuids);
+	if (ret < 0)
+		goto done_make;
+
+	file = fopen(output, "w");
+	if (!file) {
+		ret = -errno;
+		goto done_make;
+	}
+
+	ret = fwrite(mobj->mdata, mobj->size, 1, file);
+	if (ret != mobj->size)
+		ret = -errno;
+	else
+		ret = 0;
+
+	fclose(file);
+
+done_make:
+	free(mobj->mdata);
+	free(mobj);
+
+	return ret;
+}
+
+int main(int argc, char *argv[])
+{
+	unsigned long banks = 0, images = 0;
+	int c, ret;
+
+	/* Explicitly initialize defaults */
+	active_bank = 0;
+	__use_guid = false;
+	previous_bank = INT_MAX;
+
+	do {
+		c = getopt_long(argc, argv, opts_short, options, NULL);
+		switch (c) {
+		case 'h':
+			print_usage();
+			return 0;
+		case 'b':
+			banks = strtoul(optarg, NULL, 0);
+			break;
+		case 'i':
+			images = strtoul(optarg, NULL, 0);
+			break;
+		case 'g':
+			__use_guid = true;
+			break;
+		case 'p':
+			previous_bank = strtoul(optarg, NULL, 0);
+			break;
+		case 'a':
+			active_bank = strtoul(optarg, NULL, 0);
+			break;
+		}
+	} while (c != -1);
+
+	if (!banks || !images) {
+		fprintf(stderr, "Error: The number of banks and images must not be 0.\n");
+		return -EINVAL;
+	}
+
+	/* This command takes UUIDs * images and output file. */
+	if (optind + images + 1 != argc) {
+		fprintf(stderr, "Error: UUID list or output file is not specified or too much.\n");
+		print_usage();
+		return -ERANGE;
+	}
+
+	if (previous_bank == INT_MAX) {
+		/* set to the earlier bank in round-robin scheme */
+		previous_bank = active_bank > 0 ? active_bank - 1 : banks - 1;
+	}
+
+	ret = fwu_make_mdata(images, banks, argv + optind, argv[argc - 1]);
+	if (ret < 0)
+		fprintf(stderr, "Error: Failed to parse and write image: %s\n",
+			strerror(-ret));
+
+	return ret;
+}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v5 3/6] dt: fwu: developerbox: enable fwu banks and mdata regions
  2023-04-10 23:01 [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox jaswinder.singh
  2023-04-10 23:02 ` [PATCH v5 1/6] FWU: Add FWU metadata access driver for MTD storage regions jaswinder.singh
  2023-04-10 23:03 ` [PATCH v5 2/6] tools: Add mkfwumdata tool for FWU metadata image jaswinder.singh
@ 2023-04-10 23:03 ` jaswinder.singh
  2023-04-10 23:04 ` [PATCH v5 4/6] configs: move to new flash layout and boot flow jaswinder.singh
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: jaswinder.singh @ 2023-04-10 23:03 UTC (permalink / raw)
  To: u-boot
  Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
	xypron.glpk, takahiro.akashi, michal.simek, Jassi Brar

From: Jassi Brar <jaswinder.singh@linaro.org>

Specify Bank-0/1 and fwu metadata mtd regions.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 .../synquacer-sc2a11-developerbox-u-boot.dtsi | 49 +++++++++++++++++--
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi b/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
index 9f9837b33b..9957646a46 100644
--- a/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
@@ -21,7 +21,7 @@
 		#size-cells = <0>;
 		status = "okay";
 
-		flash@0 {
+		flash0: flash@0 {
 			#address-cells = <1>;
 			#size-cells = <1>;
 			compatible = "jedec,spi-nor";
@@ -74,8 +74,24 @@
 				};
 
 				partition@500000 {
-					label = "Ex-OPTEE";
-					reg = <0x500000 0x200000>;
+					label = "MDATA-Pri";
+					reg = <0x500000 0x1000>;
+				};
+
+				partition@530000 {
+					label = "MDATA-Sec";
+					reg = <0x530000 0x1000>;
+				};
+
+				/* FWU Multi bank update partitions */
+				partition@600000 {
+					label = "FIP-Bank0";
+					reg = <0x600000 0x400000>;
+				};
+
+				partition@a00000 {
+					label = "FIP-Bank1";
+					reg = <0xa00000 0x400000>;
 				};
 			};
 		};
@@ -102,6 +118,33 @@
 		optee {
 			status = "okay";
 		};
+
+		fwu-mdata {
+			compatible = "u-boot,fwu-mdata-mtd";
+			fwu-mdata-store = <&flash0>;
+			mdata-parts = "MDATA-Pri", "MDATA-Sec";
+
+			fwu-bank0 {
+				id = <0>;
+				label = "FIP-Bank0";
+				fwu-image0 {
+					id = <0>;
+					offset = <0x0>;
+					size = <0x400000>;
+					uuid = "5a66a702-99fd-4fef-a392-c26e261a2828";
+				};
+			};
+			fwu-bank1 {
+				id = <1>;
+				label = "FIP-Bank1";
+				fwu-image0 {
+					id = <0>;
+					offset = <0x0>;
+					size = <0x400000>;
+					uuid = "a8f868a1-6e5c-4757-878d-ce63375ef2c0";
+				};
+			};
+		};
 	};
 };
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v5 4/6] configs: move to new flash layout and boot flow
  2023-04-10 23:01 [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox jaswinder.singh
                   ` (2 preceding siblings ...)
  2023-04-10 23:03 ` [PATCH v5 3/6] dt: fwu: developerbox: enable fwu banks and mdata regions jaswinder.singh
@ 2023-04-10 23:04 ` jaswinder.singh
  2023-04-11 17:12   ` Etienne Carriere
  2023-04-10 23:04 ` [PATCH v5 5/6] fwu: DeveloperBox: add support for FWU jaswinder.singh
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: jaswinder.singh @ 2023-04-10 23:04 UTC (permalink / raw)
  To: u-boot
  Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
	xypron.glpk, takahiro.akashi, michal.simek, Jassi Brar

From: Jassi Brar <jaswinder.singh@linaro.org>

Towards enabling FWU and supporting new firmware layout in NOR flash,
make u-boot PIC and adjust uboot env offset in flash.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 configs/synquacer_developerbox_defconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configs/synquacer_developerbox_defconfig b/configs/synquacer_developerbox_defconfig
index 08f19a90cb..09e12b739b 100644
--- a/configs/synquacer_developerbox_defconfig
+++ b/configs/synquacer_developerbox_defconfig
@@ -1,12 +1,12 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SYNQUACER=y
-CONFIG_TEXT_BASE=0x08200000
+CONFIG_POSITION_INDEPENDENT=y
 CONFIG_SYS_MALLOC_LEN=0x1000000
 CONFIG_SYS_MALLOC_F_LEN=0x400
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xe0000000
 CONFIG_ENV_SIZE=0x30000
-CONFIG_ENV_OFFSET=0x300000
+CONFIG_ENV_OFFSET=0x580000
 CONFIG_ENV_SECT_SIZE=0x10000
 CONFIG_DM_GPIO=y
 CONFIG_DEFAULT_DEVICE_TREE="synquacer-sc2a11-developerbox"
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v5 5/6] fwu: DeveloperBox: add support for FWU
  2023-04-10 23:01 [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox jaswinder.singh
                   ` (3 preceding siblings ...)
  2023-04-10 23:04 ` [PATCH v5 4/6] configs: move to new flash layout and boot flow jaswinder.singh
@ 2023-04-10 23:04 ` jaswinder.singh
  2023-04-11 17:09   ` Etienne Carriere
  2023-04-10 23:04 ` [PATCH v5 6/6] fwu: provide default fwu_plat_get_bootidx jaswinder.singh
  2023-04-14 14:18 ` [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox Michal Simek
  6 siblings, 1 reply; 12+ messages in thread
From: jaswinder.singh @ 2023-04-10 23:04 UTC (permalink / raw)
  To: u-boot
  Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
	xypron.glpk, takahiro.akashi, michal.simek, Jassi Brar

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=yes, Size: 11880 bytes --]

From: Jassi Brar <jaswinder.singh@linaro.org>

Add code to support FWU_MULTI_BANK_UPDATE.
The platform does not have gpt-partition storage for
Banks and MetaData, rather it used SPI-NOR backed
mtd regions for the purpose.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 board/socionext/developerbox/Makefile       |   1 +
 board/socionext/developerbox/developerbox.c |   8 +
 board/socionext/developerbox/fwu_plat.c     |  37 +++++
 configs/synquacer_developerbox_defconfig    |   8 +
 doc/board/socionext/developerbox.rst        | 155 +++++++++++++++++++-
 include/configs/synquacer.h                 |  10 ++
 6 files changed, 213 insertions(+), 6 deletions(-)
 create mode 100644 board/socionext/developerbox/fwu_plat.c

diff --git a/board/socionext/developerbox/Makefile b/board/socionext/developerbox/Makefile
index 4a46de995a..1acd067a7e 100644
--- a/board/socionext/developerbox/Makefile
+++ b/board/socionext/developerbox/Makefile
@@ -7,3 +7,4 @@
 #
 
 obj-y	:= developerbox.o
+obj-$(CONFIG_FWU_MDATA_MTD) += fwu_plat.o
diff --git a/board/socionext/developerbox/developerbox.c b/board/socionext/developerbox/developerbox.c
index 16e14d4f7f..ce2cccf4f0 100644
--- a/board/socionext/developerbox/developerbox.c
+++ b/board/socionext/developerbox/developerbox.c
@@ -20,6 +20,13 @@
 
 #if IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT)
 struct efi_fw_image fw_images[] = {
+#if CONFIG_IS_ENABLED(FWU_MULTI_BANK_UPDATE)
+	{
+		.image_type_id = DEVELOPERBOX_FIP_IMAGE_GUID,
+		.fw_name = u"DEVELOPERBOX-FIP",
+		.image_index = 1,
+	},
+#else
 	{
 		.image_type_id = DEVELOPERBOX_UBOOT_IMAGE_GUID,
 		.fw_name = u"DEVELOPERBOX-UBOOT",
@@ -35,6 +42,7 @@ struct efi_fw_image fw_images[] = {
 		.fw_name = u"DEVELOPERBOX-OPTEE",
 		.image_index = 3,
 	},
+#endif
 };
 
 struct efi_capsule_update_info update_info = {
diff --git a/board/socionext/developerbox/fwu_plat.c b/board/socionext/developerbox/fwu_plat.c
new file mode 100644
index 0000000000..e5dae0ff11
--- /dev/null
+++ b/board/socionext/developerbox/fwu_plat.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023, Linaro Limited
+ */
+
+#include <efi_loader.h>
+#include <fwu.h>
+#include <fwu_mdata.h>
+#include <memalign.h>
+#include <mtd.h>
+
+#define DFU_ALT_BUF_LEN 256
+
+/* Generate dfu_alt_info from partitions */
+void set_dfu_alt_info(char *interface, char *devstr)
+{
+	int ret;
+	struct mtd_info *mtd;
+
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
+	memset(buf, 0, sizeof(buf));
+
+	mtd_probe_devices();
+
+	mtd = get_mtd_device_nm("nor1");
+	if (IS_ERR_OR_NULL(mtd))
+		return;
+
+	ret = fwu_gen_alt_info_from_mtd(buf, DFU_ALT_BUF_LEN, mtd);
+	if (ret < 0) {
+		log_err("Error: Failed to generate dfu_alt_info. (%d)\n", ret);
+		return;
+	}
+	log_debug("Make dfu_alt_info: '%s'\n", buf);
+
+	env_set("dfu_alt_info", buf);
+}
diff --git a/configs/synquacer_developerbox_defconfig b/configs/synquacer_developerbox_defconfig
index 09e12b739b..d09684153a 100644
--- a/configs/synquacer_developerbox_defconfig
+++ b/configs/synquacer_developerbox_defconfig
@@ -97,3 +97,11 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_IGNORE_OSINDICATIONS=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
+CONFIG_EFI_SECURE_BOOT=y
+CONFIG_FWU_MULTI_BANK_UPDATE=y
+CONFIG_FWU_MDATA=y
+CONFIG_FWU_MDATA_MTD=y
+CONFIG_FWU_NUM_BANKS=2
+CONFIG_FWU_NUM_IMAGES_PER_BANK=1
+CONFIG_CMD_FWU_METADATA=y
+CONFIG_TOOLS_MKFWUMDATA=y
diff --git a/doc/board/socionext/developerbox.rst b/doc/board/socionext/developerbox.rst
index 2d943c23be..908d3a7e6f 100644
--- a/doc/board/socionext/developerbox.rst
+++ b/doc/board/socionext/developerbox.rst
@@ -57,14 +57,20 @@ Installation
 
 You can install the SNI_NOR_UBOOT.fd via NOR flash writer.
 
-Flashing the U-Boot image on DeveloperBox requires a 96boards UART mezzanine or other mezzanine which can connect to LS-UART0 port.
-Connect USB cable from host to the LS-UART0 and set DSW2-7 to ON, and turn the board on again. The flash writer program will be started automatically; don’t forget to turn the DSW2-7 off again after flashing.
+Flashing the U-Boot image on DeveloperBox requires a 96boards UART mezzanine
+or other mezzanine which can connect to LS-UART0 port.
+Connect USB cable from host to the LS-UART0 and set DSW2-7 to ON, and turn the
+board on again. The flash writer program will be started automatically;
+don't forget to turn the DSW2-7 off again after flashing.
 
-*!!CAUTION!! If you failed to write the U-Boot image on wrong address, the board can be bricked. See below page if you need to recover the bricked board. See the following page for more detail*
+*!!CAUTION!! If you write the U-Boot image on wrong address, the board can
+be bricked. See below page if you need to recover the bricked board. See
+the following page for more detail*
 
 https://www.96boards.org/documentation/enterprise/developerbox/installation/board-recovery.md.html
 
-When the serial flasher is running correctly is will show the following boot messages shown via LS-UART0::
+When the serial flasher is running correctly is will show the following boot
+messages shown via LS-UART0::
 
 
   /*------------------------------------------*/
@@ -81,7 +87,144 @@ Once the flasher tool is running we are ready flash the UEFI image::
   flash rawwrite 200000 100000
   >> Send SPI_NOR_UBOOT.fd via XMODEM (Control-A S in minicom) <<
 
-*!!NOTE!! The flasher command parameter is different from the command for board recovery. U-Boot uses the offset 200000 (2-five-0, 2M in hex) and the size 100000 (1-five-0, 1M in hex).*
+*!!NOTE!! The flasher command parameter is different from the command for
+board recovery. U-Boot uses the offset 200000 (2-five-0, 2M in hex) and the
+size 100000 (1-five-0, 1M in hex).*
 
-After transferring the SPI_NOR_UBOOT.fd, turn off the DSW2-7 and reset the board.
+After transferring the SPI_NOR_UBOOT.fd, turn off the DSW2-7 and
+reset the board.
 
+
+Enable FWU Multi Bank Update
+============================
+
+DeveloperBox supports the FWU Multi Bank Update. You *MUST* update both
+*SCP firmware* and *TF-A* for this feature. This will change the layout and
+the boot process but you can switch back to the normal one by changing
+the DSW 1-4 off.
+
+Configure U-Boot
+----------------
+
+To enable the FWU Multi Bank Update on the DeveloperBox, you need to add
+following configurations to configs/synquacer_developerbox_defconfig ::
+
+ CONFIG_FWU_MULTI_BANK_UPDATE=y
+ CONFIG_FWU_MDATA=y
+ CONFIG_FWU_MDATA_MTD=y
+ CONFIG_FWU_NUM_BANKS=2
+ CONFIG_FWU_NUM_IMAGES_PER_BANK=1
+ CONFIG_CMD_FWU_METADATA=y
+
+And build it::
+
+  cd u-boot/
+  export ARCH=arm64
+  export CROSS_COMPILE=aarch64-linux-gnu-
+  make synquacer_developerbox_defconfig
+  make -j `noproc`
+  cd ../
+
+By default, the CONFIG_FWU_NUM_BANKS and CONFIG_FWU_NUM_IMAGES_PER_BANKS are
+set to 2 and 1 respectively. This uses FIP (Firmware Image Package) type image
+which contains TF-A, U-Boot and OP-TEE (the OP-TEE is optional.)
+You can use fiptool to compose the FIP image from those firmware images.
+
+Rebuild SCP firmware
+--------------------
+
+Rebuild SCP firmware which supports FWU Multi Bank Update as below::
+
+  cd SCP-firmware/
+  OUT=./build/product/synquacer
+  ROMFW_FILE=$OUT/scp_romfw/$SCP_BUILD_MODE/bin/scp_romfw.bin
+  RAMFW_FILE=$OUT/scp_ramfw/$SCP_BUILD_MODE/bin/scp_ramfw.bin
+  ROMRAMFW_FILE=scp_romramfw_release.bin
+
+  make CC=arm-none-eabi-gcc PRODUCT=synquacer MODE=release
+  tr "\000" "\377" < /dev/zero | dd of=${ROMRAMFW_FILE} bs=1 count=196608
+  dd if=${ROMFW_FILE} of=${ROMRAMFW_FILE} bs=1 conv=notrunc seek=0
+  dd if=${RAMFW_FILE} of=${ROMRAMFW_FILE} bs=1 seek=65536
+  cd ../
+
+And you can get the `scp_romramfw_release.bin` file
+
+Rebuild OPTEE firmware
+----------------------
+
+Rebuild OPTEE to use in new-layout fip as below::
+
+  cd optee_os/
+  make -j`nproc` PLATFORM=synquacer ARCH=arm \
+    CROSS_COMPILE64=aarch64-linux-gnu- CFG_ARM64_core=y \
+    CFG_CRYPTO_WITH_CE=y CFG_CORE_HEAP_SIZE=524288 CFG_CORE_DYN_SHM=y \
+    CFG_CORE_ARM64_PA_BITS=48 CFG_TEE_CORE_LOG_LEVEL=1 CFG_TEE_TA_LOG_LEVEL=1
+  cp out/arm-plat-synquacer/core/tee-pager_v2.bin ../arm-trusted-firmware/
+
+The produced `tee-pager_v2.bin` is to be used while building TF-A next.
+
+
+Rebuild TF-A and FIP
+--------------------
+
+Rebuild TF-A which supports FWU Multi Bank Update as below::
+
+  cd arm-trusted-firmware/
+  make CROSS_COMPILE=aarch64-linux-gnu- -j`nproc` PLAT=synquacer \
+     TRUSTED_BOARD_BOOT=1 SPD=opteed SQ_RESET_TO_BL2=1 GENERATE_COT=1 \
+     MBEDTLS_DIR=../mbedtls BL32=tee-pager_v2.bin \
+     BL33=../u-boot/u-boot.bin all fip fiptool
+
+And make a FIP image.::
+
+  cp build/synquacer/release/fip.bin SPI_NOR_NEWFIP.fd
+  tools/fiptool/fiptool update --tb-fw build/synquacer/release/bl2.bin SPI_NOR_NEWFIP.fd
+
+UUIDs for the FWU Multi Bank Update
+-----------------------------------
+
+FWU multi-bank update requires some UUIDs. The DeveloperBox platform uses
+following UUIDs.
+
+ - Location UUID for the FIP image: 17e86d77-41f9-4fd7-87ec-a55df9842de5
+ - Image type UUID for the FIP image: 10c36d7d-ca52-b843-b7b9-f9d6c501d108
+ - Image UUID for Bank0 : 5a66a702-99fd-4fef-a392-c26e261a2828
+ - Image UUID for Bank1 : a8f868a1-6e5c-4757-878d-ce63375ef2c0
+
+These UUIDs are used for making a FWU metadata image.
+
+u-boot$ ./tools/mkfwumdata -i 1 -b 2 \
+	17e86d77-41f9-4fd7-87ec-a55df9842de5,10c36d7d-ca52-b843-b7b9-f9d6c501d108,5a66a702-99fd-4fef-a392-c26e261a2828,a8f868a1-6e5c-4757-878d-ce63375ef2c0 \
+	../devbox-fwu-mdata.img
+
+Create Accept & Revert capsules
+
+u-boot$ ./tools/mkeficapsule -A -g 7d6dc310-52ca-43b8-b7b9-f9d6c501d108 NEWFIP_accept.Cap
+u-boot$ ./tools/mkeficapsule -R NEWFIP_revert.Cap
+
+Install via flash writer
+------------------------
+
+As explained in above section, the new FIP image and the FWU metadata image
+can be installed via NOR flash writer. Note that the installation offsets for
+the FWU multi bank update supported firmware.
+
+Once the flasher tool is running we are ready flash the images.::
+Write the FIP image to the Bank-0 & 1 at 6MB and 10MB offset.::
+
+  flash rawwrite 600000 180000
+  flash rawwrite a00000 180000
+  >> Send SPI_NOR_NEWFIP.fd via XMODEM (Control-A S in minicom) <<
+
+  flash rawwrite 500000 1000
+  flash rawwrite 530000 1000
+  >> Send devbox-fwu-mdata.img via XMODEM (Control-A S in minicom) <<
+
+And write the new SCP firmware.::
+
+  flash write cm3
+  >> Send scp_romramfw_release.bin via XMODEM (Control-A S in minicom) <<
+
+At last, turn on the DSW 3-4 on the board, and reboot.
+Note that if DSW 3-4 is turned off, the DeveloperBox will boot from
+the original EDK2 firmware (or non-FWU U-Boot if you already installed.)
diff --git a/include/configs/synquacer.h b/include/configs/synquacer.h
index 8f44c6f66a..cd7359c2f8 100644
--- a/include/configs/synquacer.h
+++ b/include/configs/synquacer.h
@@ -40,19 +40,29 @@
 
 /* Since U-Boot 64bit PCIe support is limited, disable 64bit MMIO support */
 
+#ifdef CONFIG_FWU_MULTI_BANK_UPDATE
+#define DEFAULT_DFU_ALT_INFO
+#else
 #define DEFAULT_DFU_ALT_INFO "dfu_alt_info="				\
 			"mtd nor1=u-boot.bin raw 200000 100000;"	\
 			"fip.bin raw 180000 78000;"			\
 			"optee.bin raw 500000 100000\0"
+#endif
 
 /* GUIDs for capsule updatable firmware images */
 #define DEVELOPERBOX_UBOOT_IMAGE_GUID \
 	EFI_GUID(0x53a92e83, 0x4ef4, 0x473a, 0x8b, 0x0d, \
 		 0xb5, 0xd8, 0xc7, 0xb2, 0xd6, 0x00)
 
+#ifdef CONFIG_FWU_MULTI_BANK_UPDATE
+#define DEVELOPERBOX_FIP_IMAGE_GUID \
+	EFI_GUID(0x7d6dc310, 0x52ca, 0x43b8, 0xb7, 0xb9, \
+		 0xf9, 0xd6, 0xc5, 0x01, 0xd1, 0x08)
+#else
 #define DEVELOPERBOX_FIP_IMAGE_GUID \
 	EFI_GUID(0x880866e9, 0x84ba, 0x4793, 0xa9, 0x08, \
 		 0x33, 0xe0, 0xb9, 0x16, 0xf3, 0x98)
+#endif
 
 #define DEVELOPERBOX_OPTEE_IMAGE_GUID \
 	EFI_GUID(0xc1b629f1, 0xce0e, 0x4894, 0x82, 0xbf, \
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v5 6/6] fwu: provide default fwu_plat_get_bootidx
  2023-04-10 23:01 [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox jaswinder.singh
                   ` (4 preceding siblings ...)
  2023-04-10 23:04 ` [PATCH v5 5/6] fwu: DeveloperBox: add support for FWU jaswinder.singh
@ 2023-04-10 23:04 ` jaswinder.singh
  2023-04-14 14:18 ` [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox Michal Simek
  6 siblings, 0 replies; 12+ messages in thread
From: jaswinder.singh @ 2023-04-10 23:04 UTC (permalink / raw)
  To: u-boot
  Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
	xypron.glpk, takahiro.akashi, michal.simek, Jassi Brar

From: Jassi Brar <jaswinder.singh@linaro.org>

Just like fwu_plat_get_update_index, provide a default/weak
implementation of fwu_plat_get_bootidx. So that most platforms
wouldn't have to re-implement the likely case.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 lib/fwu_updates/fwu.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index a24ccf567a..c9996f4d6d 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -545,6 +545,24 @@ __weak int fwu_plat_get_update_index(uint *update_idx)
 	return ret;
 }
 
+/**
+ * fwu_plat_get_bootidx() - Get the value of the boot index
+ * @boot_idx: Boot index value
+ *
+ * Get the value of the bank(partition) from which the platform
+ * has booted. This value is passed to U-Boot from the earlier
+ * stage bootloader which loads and boots all the relevant
+ * firmware images
+ */
+__weak void fwu_plat_get_bootidx(uint *boot_idx)
+{
+	int ret;
+
+	ret = fwu_get_active_index(boot_idx);
+	if (ret < 0)
+		*boot_idx = 0; /* Dummy value */
+}
+
 /**
  * fwu_update_checks_pass() - Check if FWU update can be done
  *
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 5/6] fwu: DeveloperBox: add support for FWU
  2023-04-10 23:04 ` [PATCH v5 5/6] fwu: DeveloperBox: add support for FWU jaswinder.singh
@ 2023-04-11 17:09   ` Etienne Carriere
  0 siblings, 0 replies; 12+ messages in thread
From: Etienne Carriere @ 2023-04-11 17:09 UTC (permalink / raw)
  To: jaswinder.singh
  Cc: u-boot, ilias.apalodimas, trini, sjg, sughosh.ganu, xypron.glpk,
	takahiro.akashi, michal.simek

Hello Jassi,

On Tue, 11 Apr 2023 at 01:04, <jaswinder.singh@linaro.org> wrote:
>
> From: Jassi Brar <jaswinder.singh@linaro.org>
>
> Add code to support FWU_MULTI_BANK_UPDATE.
> The platform does not have gpt-partition storage for
> Banks and MetaData, rather it used SPI-NOR backed
> mtd regions for the purpose.
>

I think you should mention this change enables EFI secure boot.

> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
>  board/socionext/developerbox/Makefile       |   1 +
>  board/socionext/developerbox/developerbox.c |   8 +
>  board/socionext/developerbox/fwu_plat.c     |  37 +++++
>  configs/synquacer_developerbox_defconfig    |   8 +
>  doc/board/socionext/developerbox.rst        | 155 +++++++++++++++++++-
>  include/configs/synquacer.h                 |  10 ++
>  6 files changed, 213 insertions(+), 6 deletions(-)
>  create mode 100644 board/socionext/developerbox/fwu_plat.c
>
> diff --git a/board/socionext/developerbox/Makefile b/board/socionext/developerbox/Makefile
> index 4a46de995a..1acd067a7e 100644
> --- a/board/socionext/developerbox/Makefile
> +++ b/board/socionext/developerbox/Makefile
> @@ -7,3 +7,4 @@
>  #
>
>  obj-y  := developerbox.o
> +obj-$(CONFIG_FWU_MDATA_MTD) += fwu_plat.o
> diff --git a/board/socionext/developerbox/developerbox.c b/board/socionext/developerbox/developerbox.c
> index 16e14d4f7f..ce2cccf4f0 100644
> --- a/board/socionext/developerbox/developerbox.c
> +++ b/board/socionext/developerbox/developerbox.c
> @@ -20,6 +20,13 @@
>
>  #if IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT)
>  struct efi_fw_image fw_images[] = {
> +#if CONFIG_IS_ENABLED(FWU_MULTI_BANK_UPDATE)
> +       {
> +               .image_type_id = DEVELOPERBOX_FIP_IMAGE_GUID,
> +               .fw_name = u"DEVELOPERBOX-FIP",
> +               .image_index = 1,
> +       },
> +#else
>         {
>                 .image_type_id = DEVELOPERBOX_UBOOT_IMAGE_GUID,
>                 .fw_name = u"DEVELOPERBOX-UBOOT",
> @@ -35,6 +42,7 @@ struct efi_fw_image fw_images[] = {
>                 .fw_name = u"DEVELOPERBOX-OPTEE",
>                 .image_index = 3,
>         },
> +#endif
>  };
>
>  struct efi_capsule_update_info update_info = {
> diff --git a/board/socionext/developerbox/fwu_plat.c b/board/socionext/developerbox/fwu_plat.c
> new file mode 100644
> index 0000000000..e5dae0ff11
> --- /dev/null
> +++ b/board/socionext/developerbox/fwu_plat.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include <efi_loader.h>
> +#include <fwu.h>
> +#include <fwu_mdata.h>
> +#include <memalign.h>
> +#include <mtd.h>
> +
> +#define DFU_ALT_BUF_LEN 256
> +
> +/* Generate dfu_alt_info from partitions */
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> +       int ret;
> +       struct mtd_info *mtd;
> +
> +       ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);

ALLOC_CACHE_ALIGN_BUFFER() should be used within local variable
definitions at block entry. Prefer:

+       ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
+       struct mtd_info *mtd;
+       int ret;
+


> +       memset(buf, 0, sizeof(buf));
> +
> +       mtd_probe_devices();
> +
> +       mtd = get_mtd_device_nm("nor1");
> +       if (IS_ERR_OR_NULL(mtd))
> +               return;
> +
> +       ret = fwu_gen_alt_info_from_mtd(buf, DFU_ALT_BUF_LEN, mtd);
> +       if (ret < 0) {
> +               log_err("Error: Failed to generate dfu_alt_info. (%d)\n", ret);
> +               return;
> +       }
> +       log_debug("Make dfu_alt_info: '%s'\n", buf);
> +
> +       env_set("dfu_alt_info", buf);
> +}
> diff --git a/configs/synquacer_developerbox_defconfig b/configs/synquacer_developerbox_defconfig
> index 09e12b739b..d09684153a 100644
> --- a/configs/synquacer_developerbox_defconfig
> +++ b/configs/synquacer_developerbox_defconfig
> @@ -97,3 +97,11 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
>  CONFIG_EFI_CAPSULE_ON_DISK=y
>  CONFIG_EFI_IGNORE_OSINDICATIONS=y
>  CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> +CONFIG_EFI_SECURE_BOOT=y
> +CONFIG_FWU_MULTI_BANK_UPDATE=y
> +CONFIG_FWU_MDATA=y
> +CONFIG_FWU_MDATA_MTD=y
> +CONFIG_FWU_NUM_BANKS=2
> +CONFIG_FWU_NUM_IMAGES_PER_BANK=1
> +CONFIG_CMD_FWU_METADATA=y
> +CONFIG_TOOLS_MKFWUMDATA=y
> diff --git a/doc/board/socionext/developerbox.rst b/doc/board/socionext/developerbox.rst
> index 2d943c23be..908d3a7e6f 100644
> --- a/doc/board/socionext/developerbox.rst
> +++ b/doc/board/socionext/developerbox.rst
> @@ -57,14 +57,20 @@ Installation
>
>  You can install the SNI_NOR_UBOOT.fd via NOR flash writer.
>
> -Flashing the U-Boot image on DeveloperBox requires a 96boards UART mezzanine or other mezzanine which can connect to LS-UART0 port.
> -Connect USB cable from host to the LS-UART0 and set DSW2-7 to ON, and turn the board on again. The flash writer program will be started automatically; don’t forget to turn the DSW2-7 off again after flashing.
> +Flashing the U-Boot image on DeveloperBox requires a 96boards UART mezzanine
> +or other mezzanine which can connect to LS-UART0 port.

nitpicking: ...to **the** LS-UART0..

> +Connect USB cable from host to the LS-UART0 and set DSW2-7 to ON, and turn the
> +board on again. The flash writer program will be started automatically;
> +don't forget to turn the DSW2-7 off again after flashing.
>
> -*!!CAUTION!! If you failed to write the U-Boot image on wrong address, the board can be bricked. See below page if you need to recover the bricked board. See the following page for more detail*
> +*!!CAUTION!! If you write the U-Boot image on wrong address, the board can
> +be bricked. See below page if you need to recover the bricked board. See
> +the following page for more detail*

s/detail/details/

>
>  https://www.96boards.org/documentation/enterprise/developerbox/installation/board-recovery.md.html
>
> -When the serial flasher is running correctly is will show the following boot messages shown via LS-UART0::
> +When the serial flasher is running correctly is will show the following boot

s/is/it/

> +messages shown via LS-UART0::

I would replace "shown via" with "printed to LS-UART0 console".

>
>
>    /*------------------------------------------*/
> @@ -81,7 +87,144 @@ Once the flasher tool is running we are ready flash the UEFI image::
>    flash rawwrite 200000 100000
>    >> Send SPI_NOR_UBOOT.fd via XMODEM (Control-A S in minicom) <<
>
> -*!!NOTE!! The flasher command parameter is different from the command for board recovery. U-Boot uses the offset 200000 (2-five-0, 2M in hex) and the size 100000 (1-five-0, 1M in hex).*
> +*!!NOTE!! The flasher command parameter is different from the command for
> +board recovery. U-Boot uses the offset 200000 (2-five-0, 2M in hex) and the
> +size 100000 (1-five-0, 1M in hex).*
>
> -After transferring the SPI_NOR_UBOOT.fd, turn off the DSW2-7 and reset the board.
> +After transferring the SPI_NOR_UBOOT.fd, turn off the DSW2-7 and
> +reset the board.
>
> +
> +Enable FWU Multi Bank Update
> +============================
> +
> +DeveloperBox supports the FWU Multi Bank Update. You *MUST* update both
> +*SCP firmware* and *TF-A* for this feature. This will change the layout and

Youmlean SCP-firmware and TF-A **and U-Boot**?

> +the boot process but you can switch back to the normal one by changing
> +the DSW 1-4 off.
> +
> +Configure U-Boot
> +----------------
> +
> +To enable the FWU Multi Bank Update on the DeveloperBox, you need to add
> +following configurations to configs/synquacer_developerbox_defconfig ::

Actually this change already updates defconfig file. Maybe:
- you need to add following configurations to
configs/synquacer_developerbox_defconfig
+ Board configs/synquacer_developerbox_defconfig enables default FWU
+ configuration:



> +
> + CONFIG_FWU_MULTI_BANK_UPDATE=y
> + CONFIG_FWU_MDATA=y
> + CONFIG_FWU_MDATA_MTD=y
> + CONFIG_FWU_NUM_BANKS=2
> + CONFIG_FWU_NUM_IMAGES_PER_BANK=1
> + CONFIG_CMD_FWU_METADATA=y
> +
> +And build it::
> +
> +  cd u-boot/
> +  export ARCH=arm64
> +  export CROSS_COMPILE=aarch64-linux-gnu-
> +  make synquacer_developerbox_defconfig
> +  make -j `noproc`
> +  cd ../
> +
> +By default, the CONFIG_FWU_NUM_BANKS and CONFIG_FWU_NUM_IMAGES_PER_BANKS are
> +set to 2 and 1 respectively. This uses FIP (Firmware Image Package) type image
> +which contains TF-A, U-Boot and OP-TEE (the OP-TEE is optional.)

s/.)/)./


> +You can use fiptool to compose the FIP image from those firmware images.
> +
> +Rebuild SCP firmware
> +--------------------
> +
> +Rebuild SCP firmware which supports FWU Multi Bank Update as below::
> +
> +  cd SCP-firmware/
> +  OUT=./build/product/synquacer
> +  ROMFW_FILE=$OUT/scp_romfw/$SCP_BUILD_MODE/bin/scp_romfw.bin
> +  RAMFW_FILE=$OUT/scp_ramfw/$SCP_BUILD_MODE/bin/scp_ramfw.bin
> +  ROMRAMFW_FILE=scp_romramfw_release.bin
> +
> +  make CC=arm-none-eabi-gcc PRODUCT=synquacer MODE=release
> +  tr "\000" "\377" < /dev/zero | dd of=${ROMRAMFW_FILE} bs=1 count=196608
> +  dd if=${ROMFW_FILE} of=${ROMRAMFW_FILE} bs=1 conv=notrunc seek=0
> +  dd if=${RAMFW_FILE} of=${ROMRAMFW_FILE} bs=1 seek=65536
> +  cd ../
> +
> +And you can get the `scp_romramfw_release.bin` file

Add a period '.'

> +
> +Rebuild OPTEE firmware
> +----------------------
> +
> +Rebuild OPTEE to use in new-layout fip as below::

s/fip/FIP/g

> +
> +  cd optee_os/
> +  make -j`nproc` PLATFORM=synquacer ARCH=arm \
> +    CROSS_COMPILE64=aarch64-linux-gnu- CFG_ARM64_core=y \
> +    CFG_CRYPTO_WITH_CE=y CFG_CORE_HEAP_SIZE=524288 CFG_CORE_DYN_SHM=y \
> +    CFG_CORE_ARM64_PA_BITS=48 CFG_TEE_CORE_LOG_LEVEL=1 CFG_TEE_TA_LOG_LEVEL=1
> +  cp out/arm-plat-synquacer/core/tee-pager_v2.bin ../arm-trusted-firmware/
> +
> +The produced `tee-pager_v2.bin` is to be used while building TF-A next.
> +
> +
> +Rebuild TF-A and FIP
> +--------------------
> +
> +Rebuild TF-A which supports FWU Multi Bank Update as below::
> +
> +  cd arm-trusted-firmware/
> +  make CROSS_COMPILE=aarch64-linux-gnu- -j`nproc` PLAT=synquacer \
> +     TRUSTED_BOARD_BOOT=1 SPD=opteed SQ_RESET_TO_BL2=1 GENERATE_COT=1 \
> +     MBEDTLS_DIR=../mbedtls BL32=tee-pager_v2.bin \
> +     BL33=../u-boot/u-boot.bin all fip fiptool
> +
> +And make a FIP image.::
> +
> +  cp build/synquacer/release/fip.bin SPI_NOR_NEWFIP.fd
> +  tools/fiptool/fiptool update --tb-fw build/synquacer/release/bl2.bin SPI_NOR_NEWFIP.fd
> +
> +UUIDs for the FWU Multi Bank Update
> +-----------------------------------
> +
> +FWU multi-bank update requires some UUIDs. The DeveloperBox platform uses
> +following UUIDs.
> +
> + - Location UUID for the FIP image: 17e86d77-41f9-4fd7-87ec-a55df9842de5
> + - Image type UUID for the FIP image: 10c36d7d-ca52-b843-b7b9-f9d6c501d108
> + - Image UUID for Bank0 : 5a66a702-99fd-4fef-a392-c26e261a2828
> + - Image UUID for Bank1 : a8f868a1-6e5c-4757-878d-ce63375ef2c0
> +
> +These UUIDs are used for making a FWU metadata image.
> +
> +u-boot$ ./tools/mkfwumdata -i 1 -b 2 \
> +       17e86d77-41f9-4fd7-87ec-a55df9842de5,10c36d7d-ca52-b843-b7b9-f9d6c501d108,5a66a702-99fd-4fef-a392-c26e261a2828,a8f868a1-6e5c-4757-878d-ce63375ef2c0 \
> +       ../devbox-fwu-mdata.img
> +
> +Create Accept & Revert capsules
> +
> +u-boot$ ./tools/mkeficapsule -A -g 7d6dc310-52ca-43b8-b7b9-f9d6c501d108 NEWFIP_accept.Cap
> +u-boot$ ./tools/mkeficapsule -R NEWFIP_revert.Cap
> +
> +Install via flash writer
> +------------------------
> +
> +As explained in above section, the new FIP image and the FWU metadata image
> +can be installed via NOR flash writer. Note that the installation offsets for
> +the FWU multi bank update supported firmware.

This last sentence sounds strange. Is something missing?

> +
> +Once the flasher tool is running we are ready flash the images.::
> +Write the FIP image to the Bank-0 & 1 at 6MB and 10MB offset.::
> +
> +  flash rawwrite 600000 180000
> +  flash rawwrite a00000 180000
> +  >> Send SPI_NOR_NEWFIP.fd via XMODEM (Control-A S in minicom) <<
> +
> +  flash rawwrite 500000 1000
> +  flash rawwrite 530000 1000
> +  >> Send devbox-fwu-mdata.img via XMODEM (Control-A S in minicom) <<
> +
> +And write the new SCP firmware.::
> +
> +  flash write cm3
> +  >> Send scp_romramfw_release.bin via XMODEM (Control-A S in minicom) <<
> +
> +At last, turn on the DSW 3-4 on the board, and reboot.
> +Note that if DSW 3-4 is turned off, the DeveloperBox will boot from
> +the original EDK2 firmware (or non-FWU U-Boot if you already installed.)

swap the 2 last characters: s/.)/)./

> diff --git a/include/configs/synquacer.h b/include/configs/synquacer.h
> index 8f44c6f66a..cd7359c2f8 100644
> --- a/include/configs/synquacer.h
> +++ b/include/configs/synquacer.h
> @@ -40,19 +40,29 @@
>
>  /* Since U-Boot 64bit PCIe support is limited, disable 64bit MMIO support */
>
> +#ifdef CONFIG_FWU_MULTI_BANK_UPDATE
> +#define DEFAULT_DFU_ALT_INFO
> +#else
>  #define DEFAULT_DFU_ALT_INFO "dfu_alt_info="                           \
>                         "mtd nor1=u-boot.bin raw 200000 100000;"        \
>                         "fip.bin raw 180000 78000;"                     \
>                         "optee.bin raw 500000 100000\0"
> +#endif
>
>  /* GUIDs for capsule updatable firmware images */
>  #define DEVELOPERBOX_UBOOT_IMAGE_GUID \
>         EFI_GUID(0x53a92e83, 0x4ef4, 0x473a, 0x8b, 0x0d, \
>                  0xb5, 0xd8, 0xc7, 0xb2, 0xd6, 0x00)
>
> +#ifdef CONFIG_FWU_MULTI_BANK_UPDATE
> +#define DEVELOPERBOX_FIP_IMAGE_GUID \
> +       EFI_GUID(0x7d6dc310, 0x52ca, 0x43b8, 0xb7, 0xb9, \
> +                0xf9, 0xd6, 0xc5, 0x01, 0xd1, 0x08)
> +#else
>  #define DEVELOPERBOX_FIP_IMAGE_GUID \
>         EFI_GUID(0x880866e9, 0x84ba, 0x4793, 0xa9, 0x08, \
>                  0x33, 0xe0, 0xb9, 0x16, 0xf3, 0x98)
> +#endif
>
>  #define DEVELOPERBOX_OPTEE_IMAGE_GUID \
>         EFI_GUID(0xc1b629f1, 0xce0e, 0x4894, 0x82, 0xbf, \
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 4/6] configs: move to new flash layout and boot flow
  2023-04-10 23:04 ` [PATCH v5 4/6] configs: move to new flash layout and boot flow jaswinder.singh
@ 2023-04-11 17:12   ` Etienne Carriere
  0 siblings, 0 replies; 12+ messages in thread
From: Etienne Carriere @ 2023-04-11 17:12 UTC (permalink / raw)
  To: jaswinder.singh
  Cc: u-boot, ilias.apalodimas, trini, sjg, sughosh.ganu, xypron.glpk,
	takahiro.akashi, michal.simek

Hello Jassi,

On Tue, 11 Apr 2023 at 01:04, <jaswinder.singh@linaro.org> wrote:
>
> From: Jassi Brar <jaswinder.singh@linaro.org>
>
> Towards enabling FWU and supporting new firmware layout in NOR flash,
> make u-boot PIC and adjust uboot env offset in flash.
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
>  configs/synquacer_developerbox_defconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I think the commit header line should mention developerbox defconfig.
Aside from that, Acked-by: Etienne Carriere <etienne.carriere@linaro.org> fwiw.

Regards,
Etienne


>
> diff --git a/configs/synquacer_developerbox_defconfig b/configs/synquacer_developerbox_defconfig
> index 08f19a90cb..09e12b739b 100644
> --- a/configs/synquacer_developerbox_defconfig
> +++ b/configs/synquacer_developerbox_defconfig
> @@ -1,12 +1,12 @@
>  CONFIG_ARM=y
>  CONFIG_ARCH_SYNQUACER=y
> -CONFIG_TEXT_BASE=0x08200000
> +CONFIG_POSITION_INDEPENDENT=y
>  CONFIG_SYS_MALLOC_LEN=0x1000000
>  CONFIG_SYS_MALLOC_F_LEN=0x400
>  CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xe0000000
>  CONFIG_ENV_SIZE=0x30000
> -CONFIG_ENV_OFFSET=0x300000
> +CONFIG_ENV_OFFSET=0x580000
>  CONFIG_ENV_SECT_SIZE=0x10000
>  CONFIG_DM_GPIO=y
>  CONFIG_DEFAULT_DEVICE_TREE="synquacer-sc2a11-developerbox"
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/6] tools: Add mkfwumdata tool for FWU metadata image
  2023-04-10 23:03 ` [PATCH v5 2/6] tools: Add mkfwumdata tool for FWU metadata image jaswinder.singh
@ 2023-04-11 17:28   ` Etienne Carriere
  0 siblings, 0 replies; 12+ messages in thread
From: Etienne Carriere @ 2023-04-11 17:28 UTC (permalink / raw)
  To: jaswinder.singh
  Cc: u-boot, ilias.apalodimas, trini, sjg, sughosh.ganu, xypron.glpk,
	takahiro.akashi, michal.simek, Masami Hiramatsu

On Tue, 11 Apr 2023 at 01:03, <jaswinder.singh@linaro.org> wrote:
>
> From: Masami Hiramatsu <masami.hiramatsu@linaro.org>
>
> Add 'mkfwumdata' tool to generate FWU metadata image for the meta-data
> partition to be used in A/B Update imeplementation.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  doc/mkfwumdata.1   |  89 ++++++++++++
>  tools/Kconfig      |   9 ++
>  tools/Makefile     |   4 +
>  tools/mkfwumdata.c | 334 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 436 insertions(+)
>  create mode 100644 doc/mkfwumdata.1
>  create mode 100644 tools/mkfwumdata.c
>
> diff --git a/doc/mkfwumdata.1 b/doc/mkfwumdata.1
> new file mode 100644
> index 0000000000..7dd718b26e
> --- /dev/null
> +++ b/doc/mkfwumdata.1
> @@ -0,0 +1,89 @@
> +.\" SPDX-License-Identifier: GPL-2.0-or-later
> +.\" Copyright (C) 2023 Jassi Brar <jaswinder.singh@linaro.org>
> +.TH MKFWUMDATA 1 2023-04-10 U-Boot
> +.SH NAME
> +mkfwumdata \- create FWU metadata image
> +.
> +.SH SYNOPSIS
> +.SY mkfwumdata
> +.OP \-a activeidx
> +.OP \-p previousidx
> +.OP \-g
> +.BI \-i\~ imagecount
> +.BI \-b\~ bankcount
> +.I UUIDs
> +.I outputimage
> +.YS
> +.SY mkfwumdata
> +.B \-h
> +.YS
> +.
> +.SH DESCRIPTION
> +.B mkfwumdata
> +creates metadata info to be used with FWU.
> +.
> +.SH OPTIONS
> +.TP
> +.B \-h
> +Print usage information and exit.
> +.
> +.TP
> +.B \-a
> +Set
> +.IR activeidx
> +as the currently active Bank. Default is 0.
> +.
> +.TP
> +.B \-p
> +Set
> +.IR previousidx
> +as the previous active Bank. Default is
> +.IR activeidx "-1"
> +or
> +.IR bankcount "-1,"
> +whichever is non-negative.
> +.
> +.TP
> +.B \-g
> +Convert the
> +.IR UUIDs
> +as GUIDs before use.
> +.
> +.TP
> +.B \-i
> +Specify there are
> +.IR imagecount
> +images in each bank.
> +.
> +.TP
> +.B \-b
> +Specify there are a total of
> +.IR bankcount
> +banks.
> +.
> +.TP
> +.IR UUIDs
> +Comma-separated list of UUIDs required to create the metadata :-
> +location_uuid,image_type_uuid,<images per bank uuid list of all banks>
> +.
> +.TP
> +.IR outputimage
> +Specify the name of the metadata image file to be created.
> +.
> +.SH BUGS
> +Please report bugs to the
> +.UR https://\:source\:.denx\:.de/\:u-boot/\:u-boot/\:issues
> +U-Boot bug tracker
> +.UE .
> +.SH EXAMPLES
> +Create a metadata image with 2 banks and 1 image/bank, BankAct=0, BankPrev=1:
> +.PP
> +.EX
> +.in +4
> +$ \c
> +.B mkfwumdata \-a 0 \-p 1 \-b 2 \-i 1 \\\\\&
> +.in +6
> +.B 17e86d77-41f9-4fd7-87ec-a55df9842de5,\\\\\&
> +.B 10c36d7d-ca52-b843-b7b9-f9d6c501d108,\\\\\&
> +.B 5a66a702-99fd-4fef-a392-c26e261a2828,a8f868a1-6e5c-4757-878d-ce63375ef2c0 \\\\\&
> +.B fwu-mdata.img
> diff --git a/tools/Kconfig b/tools/Kconfig
> index 539708f277..6e23f44d55 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
> @@ -157,4 +157,13 @@ config LUT_SEQUENCE
>         help
>           Look Up Table Sequence
>
> +config TOOLS_MKFWUMDATA
> +       bool "Build mkfwumdata command"
> +       default y if FWU_MULTI_BANK_UPDATE
> +       help
> +         This command allows users to create a raw image of the FWU
> +         metadata for initial installation of the FWU multi bank
> +         update on the board. The installation method depends on
> +         the platform.
> +
>  endmenu
> diff --git a/tools/Makefile b/tools/Makefile
> index 38699b069d..1e3fce0b1c 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -250,6 +250,10 @@ HOSTLDLIBS_mkeficapsule += \
>         $(shell pkg-config --libs uuid 2> /dev/null || echo "-luuid")
>  hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
>
> +mkfwumdata-objs := mkfwumdata.o lib/crc32.o
> +HOSTLDLIBS_mkfwumdata += -luuid
> +hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata
> +
>  # We build some files with extra pedantic flags to try to minimize things
>  # that won't build on some weird host compiler -- though there are lots of
>  # exceptions for files that aren't complaint.
> diff --git a/tools/mkfwumdata.c b/tools/mkfwumdata.c
> new file mode 100644
> index 0000000000..43dabf3b72
> --- /dev/null
> +++ b/tools/mkfwumdata.c
> @@ -0,0 +1,334 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include <errno.h>
> +#include <getopt.h>
> +#include <limits.h>
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <u-boot/crc.h>
> +#include <unistd.h>
> +#include <uuid/uuid.h>
> +
> +/* This will dynamically allocate the fwu_mdata */
> +#define CONFIG_FWU_NUM_BANKS           0
> +#define CONFIG_FWU_NUM_IMAGES_PER_BANK 0
> +
> +/* Since we can not include fwu.h, redefine version here. */
> +#define FWU_MDATA_VERSION              1
> +
> +typedef uint8_t u8;
> +typedef int16_t s16;
> +typedef uint16_t u16;
> +typedef uint32_t u32;
> +typedef uint64_t u64;
> +
> +#include <fwu_mdata.h>
> +
> +/* TODO: Endianness conversion may be required for some arch. */
> +
> +static const char *opts_short = "b:i:a:p:gh";
> +
> +static struct option options[] = {
> +       {"banks", required_argument, NULL, 'b'},
> +       {"images", required_argument, NULL, 'i'},
> +       {"guid", required_argument, NULL, 'g'},
> +       {"active-bank", required_argument, NULL, 'a'},
> +       {"previous-bank", required_argument, NULL, 'p'},
> +       {"help", no_argument, NULL, 'h'},
> +       {NULL, 0, NULL, 0},
> +};
> +
> +static void print_usage(void)
> +{
> +       fprintf(stderr, "Usage: mkfwumdata [options] <UUIDs list> <output file>\n");
> +       fprintf(stderr, "Options:\n"
> +               "\t-i, --images <num>          Number of images\n"
> +               "\t-b, --banks  <num>          Number of banks\n"

State that these arguments are mandatory.

> +               "\t-a, --active-bank  <num>    Active bank\n"
> +               "\t-p, --previous-bank  <num>  Previous active bank\n"

Could you state that active-bank default to 0
and previous-bank default to active bank minus 1 (or image number - 1)

> +               "\t-g, --guid                  Use GUID instead of UUID\n"
> +               "\t-h, --help                  print a help message\n"
> +               );
> +       fprintf(stderr, "  UUIDs list syntax:\n"
> +               "\t  <location uuid>,<image type uuid>,<images uuid list>\n"
> +               "\t     images uuid list syntax:\n"
> +               "\t        img_uuid_00,img_uuid_01...img_uuid_0b,\n"
> +               "\t        img_uuid_10,img_uuid_11...img_uuid_1b,\n"
> +               "\t        ...,\n"
> +               "\t        img_uuid_i0,img_uuid_i1...img_uuid_ib,\n"
> +               "\t          where 'b' and 'i' are number of banks and number\n"
> +               "\t          of images in a bank respectively.\n"
> +              );
> +}
> +
> +struct fwu_mdata_object {
> +       size_t images;
> +       size_t banks;
> +       size_t size;
> +       struct fwu_mdata *mdata;
> +};

I think an inline description for struct's is always welcome in U-Boot
source files.

> +
> +static int previous_bank, active_bank;
> +static bool __use_guid;
> +
> +static struct fwu_mdata_object *fwu_alloc_mdata(size_t images, size_t banks)
> +{
> +       struct fwu_mdata_object *mobj;
> +
> +       mobj = calloc(1, sizeof(*mobj));
> +       if (!mobj)
> +               return NULL;
> +
> +       mobj->size = sizeof(struct fwu_mdata) +
> +               (sizeof(struct fwu_image_entry) +
> +                sizeof(struct fwu_image_bank_info) * banks) * images;
> +       mobj->images = images;
> +       mobj->banks = banks;
> +
> +       mobj->mdata = calloc(1, mobj->size);
> +       if (!mobj->mdata) {
> +               free(mobj);
> +               return NULL;
> +       }
> +
> +       return mobj;
> +}
> +
> +static struct fwu_image_entry *
> +fwu_get_image(struct fwu_mdata_object *mobj, size_t idx)
> +{
> +       size_t offset;
> +
> +       offset = sizeof(struct fwu_mdata) +
> +               (sizeof(struct fwu_image_entry) +
> +                sizeof(struct fwu_image_bank_info) * mobj->banks) * idx;
> +
> +       return (struct fwu_image_entry *)((char *)mobj->mdata + offset);
> +}
> +
> +static struct fwu_image_bank_info *
> +fwu_get_bank(struct fwu_mdata_object *mobj, size_t img_idx, size_t bnk_idx)
> +{
> +       size_t offset;
> +
> +       offset = sizeof(struct fwu_mdata) +
> +               (sizeof(struct fwu_image_entry) +
> +                sizeof(struct fwu_image_bank_info) * mobj->banks) * img_idx +
> +               sizeof(struct fwu_image_entry) +
> +               sizeof(struct fwu_image_bank_info) * bnk_idx;
> +
> +       return (struct fwu_image_bank_info *)((char *)mobj->mdata + offset);
> +}
> +
> +/**
> + * convert_uuid_to_guid() - convert UUID to GUID
> + * @buf:       UUID binary
> + *
> + * UUID and GUID have the same data structure, but their binary
> + * formats are different due to the endianness. See lib/uuid.c.
> + * Since uuid_parse() can handle only UUID, this function must
> + * be called to get correct data for GUID when parsing a string.
> + *
> + * The correct data will be returned in @buf.
> + */
> +static void convert_uuid_to_guid(unsigned char *buf)
> +{
> +       unsigned char c;
> +
> +       c = buf[0];
> +       buf[0] = buf[3];
> +       buf[3] = c;
> +       c = buf[1];
> +       buf[1] = buf[2];
> +       buf[2] = c;
> +
> +       c = buf[4];
> +       buf[4] = buf[5];
> +       buf[5] = c;
> +
> +       c = buf[6];
> +       buf[6] = buf[7];
> +       buf[7] = c;
> +}
> +
> +static int uuid_guid_parse(char *uuidstr, unsigned char *uuid)
> +{
> +       int ret;
> +
> +       ret = uuid_parse(uuidstr, uuid);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (__use_guid)
> +               convert_uuid_to_guid(uuid);
> +
> +       return ret;

nitpicking: return 0

> +}
> +
> +static int
> +fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
> +                         size_t idx, char *uuids)
> +{
> +       struct fwu_image_entry *image = fwu_get_image(mobj, idx);
> +       struct fwu_image_bank_info *bank;
> +       char *p = uuids, *uuid;
> +       int i;
> +
> +       if (!image)
> +               return -ENOENT;
> +
> +       /* Image location UUID */
> +       uuid = strsep(&p, ",");
> +       if (!uuid)
> +               return -EINVAL;
> +
> +       if (strcmp(uuid, "0") &&
> +           uuid_guid_parse(uuid, (unsigned char *)&image->location_uuid) < 0)
> +               return -EINVAL;
> +
> +       /* Image type UUID */
> +       uuid = strsep(&p, ",");
> +       if (!uuid)
> +               return -EINVAL;
> +
> +       if (uuid_guid_parse(uuid, (unsigned char *)&image->image_type_uuid) < 0)
> +               return -EINVAL;
> +
> +       /* Fill bank image-UUID */
> +       for (i = 0; i < mobj->banks; i++) {
> +               bank = fwu_get_bank(mobj, idx, i);
> +               if (!bank)
> +                       return -ENOENT;
> +               bank->accepted = 1;
> +               uuid = strsep(&p, ",");
> +               if (!uuid)
> +                       return -EINVAL;
> +
> +               if (strcmp(uuid, "0") &&
> +                   uuid_guid_parse(uuid, (unsigned char *)&bank->image_uuid) < 0)
> +                       return -EINVAL;
> +       }
> +       return 0;
> +}
> +
> +/* Caller must ensure that @uuids[] has @mobj->images entries. */
> +static int fwu_parse_fill_uuids(struct fwu_mdata_object *mobj, char *uuids[])
> +{
> +       struct fwu_mdata *mdata = mobj->mdata;
> +       int i, ret;
> +
> +       mdata->version = FWU_MDATA_VERSION;
> +       mdata->active_index = active_bank;
> +       mdata->previous_active_index = previous_bank;
> +
> +       for (i = 0; i < mobj->images; i++) {
> +               ret = fwu_parse_fill_image_uuid(mobj, i, uuids[i]);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
> +       mdata->crc32 = crc32(0, (const unsigned char *)&mdata->version,
> +                            mobj->size - sizeof(uint32_t));
> +
> +       return 0;
> +}
> +
> +static int
> +fwu_make_mdata(size_t images, size_t banks, char *uuids[], char *output)
> +{
> +       struct fwu_mdata_object *mobj;
> +       FILE *file;
> +       int ret;
> +
> +       mobj = fwu_alloc_mdata(images, banks);
> +       if (!mobj)
> +               return -ENOMEM;
> +
> +       ret = fwu_parse_fill_uuids(mobj, uuids);
> +       if (ret < 0)
> +               goto done_make;
> +
> +       file = fopen(output, "w");
> +       if (!file) {
> +               ret = -errno;
> +               goto done_make;
> +       }
> +
> +       ret = fwrite(mobj->mdata, mobj->size, 1, file);
> +       if (ret != mobj->size)
> +               ret = -errno;
> +       else
> +               ret = 0;
> +
> +       fclose(file);
> +
> +done_make:
> +       free(mobj->mdata);
> +       free(mobj);
> +
> +       return ret;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +       unsigned long banks = 0, images = 0;
> +       int c, ret;
> +
> +       /* Explicitly initialize defaults */
> +       active_bank = 0;
> +       __use_guid = false;
> +       previous_bank = INT_MAX;

IMHO it would be better to init them where defined, at source file top

Regards,
Etienne

> +
> +       do {
> +               c = getopt_long(argc, argv, opts_short, options, NULL);
> +               switch (c) {
> +               case 'h':
> +                       print_usage();
> +                       return 0;
> +               case 'b':
> +                       banks = strtoul(optarg, NULL, 0);
> +                       break;
> +               case 'i':
> +                       images = strtoul(optarg, NULL, 0);
> +                       break;
> +               case 'g':
> +                       __use_guid = true;
> +                       break;
> +               case 'p':
> +                       previous_bank = strtoul(optarg, NULL, 0);
> +                       break;
> +               case 'a':
> +                       active_bank = strtoul(optarg, NULL, 0);
> +                       break;
> +               }
> +       } while (c != -1);
> +
> +       if (!banks || !images) {
> +               fprintf(stderr, "Error: The number of banks and images must not be 0.\n");
> +               return -EINVAL;
> +       }
> +
> +       /* This command takes UUIDs * images and output file. */
> +       if (optind + images + 1 != argc) {
> +               fprintf(stderr, "Error: UUID list or output file is not specified or too much.\n");
> +               print_usage();
> +               return -ERANGE;
> +       }
> +
> +       if (previous_bank == INT_MAX) {
> +               /* set to the earlier bank in round-robin scheme */
> +               previous_bank = active_bank > 0 ? active_bank - 1 : banks - 1;
> +       }
> +
> +       ret = fwu_make_mdata(images, banks, argv + optind, argv[argc - 1]);
> +       if (ret < 0)
> +               fprintf(stderr, "Error: Failed to parse and write image: %s\n",
> +                       strerror(-ret));
> +
> +       return ret;
> +}
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 1/6] FWU: Add FWU metadata access driver for MTD storage regions
  2023-04-10 23:02 ` [PATCH v5 1/6] FWU: Add FWU metadata access driver for MTD storage regions jaswinder.singh
@ 2023-04-11 17:34   ` Etienne Carriere
  0 siblings, 0 replies; 12+ messages in thread
From: Etienne Carriere @ 2023-04-11 17:34 UTC (permalink / raw)
  To: jaswinder.singh
  Cc: u-boot, ilias.apalodimas, trini, sjg, sughosh.ganu, xypron.glpk,
	takahiro.akashi, michal.simek, Masami Hiramatsu

Hello Jassi,

Few comments below.

On Tue, 11 Apr 2023 at 01:03, <jaswinder.singh@linaro.org> wrote:
>
> From: Masami Hiramatsu <masami.hiramatsu@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.
> The code is divided into core under drivers/fwu-mdata/ and some helper
> functions clubbed together under lib/fwu_updates/
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
>  drivers/fwu-mdata/Kconfig   |  15 ++
>  drivers/fwu-mdata/Makefile  |   1 +
>  drivers/fwu-mdata/raw_mtd.c | 272 ++++++++++++++++++++++++++++++++++++
>  include/fwu.h               |  32 +++++
>  lib/fwu_updates/Makefile    |   1 +
>  lib/fwu_updates/fwu_mtd.c   | 185 ++++++++++++++++++++++++
>  6 files changed, 506 insertions(+)
>  create mode 100644 drivers/fwu-mdata/raw_mtd.c
>  create mode 100644 lib/fwu_updates/fwu_mtd.c
>
> diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig
> index 36c4479a59..42736a5e43 100644
> --- a/drivers/fwu-mdata/Kconfig
> +++ b/drivers/fwu-mdata/Kconfig
> @@ -6,6 +6,11 @@ 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"
>         select PARTITION_TYPE_GUID
> @@ -14,3 +19,13 @@ config FWU_MDATA_GPT_BLK
>         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..25c1aa33ec
> --- /dev/null
> +++ b/drivers/fwu-mdata/raw_mtd.c
> @@ -0,0 +1,272 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#define LOG_CATEGORY UCLASS_FWU_MDATA
> +
> +#include <fwu.h>
> +#include <fwu_mdata.h>
> +#include <memalign.h>
> +

I think you can remove the empty line.

> +#include <linux/errno.h>
> +#include <linux/types.h>
> +
> +/* Internal helper structure to move data around */
> +struct fwu_mdata_mtd_priv {
> +       struct mtd_info *mtd;
> +       char pri_label[50];
> +       char sec_label[50];

50 chars is always enough? Maybe allocate buffer when size is known,
e.g. with strdup().

> +       u32 pri_offset;
> +       u32 sec_offset;
> +};
> +
> +enum fwu_mtd_op {
> +       FWU_MTD_READ,
> +       FWU_MTD_WRITE,
> +};
> +
> +extern struct fwu_mtd_image_info fwu_mtd_images[];
> +
> +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;

lock_offs and offs are the same. Could drop lock_offs and use only 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;

Could drop  io_op.ooblen = 0 and 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_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary)
> +{
> +       struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev);
> +       struct mtd_info *mtd = mtd_priv->mtd;
> +       u32 offs = primary ? mtd_priv->pri_offset : mtd_priv->sec_offset;
> +
> +       return mtd_io_data(mtd, offs, sizeof(struct fwu_mdata), mdata, FWU_MTD_READ);
> +}
> +
> +static int fwu_mtd_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary)
> +{
> +       struct fwu_mdata_mtd_priv *mtd_priv = dev_get_priv(dev);
> +       struct mtd_info *mtd = mtd_priv->mtd;
> +       u32 offs = primary ? mtd_priv->pri_offset : mtd_priv->sec_offset;
> +
> +       return mtd_io_data(mtd, offs, sizeof(struct fwu_mdata), mdata, FWU_MTD_WRITE);
> +}
> +
> +static int flash_partition_offset(struct udevice *dev, const char *part_name, fdt_addr_t *offset)
> +{
> +       ofnode node, parts_node;
> +       fdt_addr_t size = 0;
> +
> +       parts_node = ofnode_by_compatible(dev_ofnode(dev), "fixed-partitions");
> +       node = ofnode_by_prop_value(parts_node, "label", part_name, strlen(part_name) + 1);
> +       if (!ofnode_valid(node)) {
> +               log_err("Warning: Failed to find partition by label <%s>\n", part_name);
> +               return -ENOENT;
> +       }
> +
> +       *offset = ofnode_get_addr_size_index_notrans(node, 0, &size);
> +
> +       return (int)size;
> +}
> +
> +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;
> +       const char *label;
> +       fdt_addr_t offset;
> +       int ret, size;
> +       u32 phandle;
> +       ofnode bank;
> +       int off_img;
> +
> +       /* 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 -ENODEV;
> +       }
> +
> +       /* Get the offset of primary and secondary mdata */
> +       ret = ofnode_read_string_index(dev_ofnode(dev), "mdata-parts", 0, &label);
> +       if (ret)
> +               return ret;
> +       strcpy(mtd_priv->pri_label, label);

should use strncpy().

> +
> +       ret = flash_partition_offset(mtd_dev, mtd_priv->pri_label, &offset);
> +       if (ret <= 0)
> +               return ret;
> +       mtd_priv->pri_offset = offset;
> +
> +       ret = ofnode_read_string_index(dev_ofnode(dev), "mdata-parts", 1, &label);
> +       if (ret)
> +               return ret;
> +       strcpy(mtd_priv->sec_label, label);
> +
> +       ret = flash_partition_offset(mtd_dev, mtd_priv->sec_label, &offset);
> +       if (ret <= 0)
> +               return ret;
> +       mtd_priv->sec_offset = offset;
> +
> +       off_img = 0;
> +
> +       ofnode_for_each_subnode(bank, dev_ofnode(dev)) {
> +               int bank_num, bank_offset, bank_size;
> +               const char *bank_name;
> +               ofnode image;
> +
> +               ofnode_read_u32(bank, "id", &bank_num);
> +               bank_name = ofnode_read_string(bank, "label");
> +               bank_size = flash_partition_offset(mtd_dev, bank_name, &offset);
> +               if (bank_size <= 0)
> +                       return bank_size;
> +               bank_offset = offset;
> +               log_debug("Bank%d: %s [0x%x - 0x%x]\n",
> +                         bank_num, bank_name, bank_offset, bank_offset + bank_size);
> +
> +               ofnode_for_each_subnode(image, bank) {
> +                       int image_num, image_offset, image_size;
> +                       const char *uuid;
> +
> +                       if (off_img == CONFIG_FWU_NUM_BANKS *
> +                                               CONFIG_FWU_NUM_IMAGES_PER_BANK) {
> +                               log_err("DT provides images more than configured!\n");

nitpicking: s/images more/more images/

> +                               break;
> +                       }
> +
> +                       uuid = ofnode_read_string(image, "uuid");
> +                       ofnode_read_u32(image, "id", &image_num);
> +                       ofnode_read_u32(image, "offset", &image_offset);
> +                       ofnode_read_u32(image, "size", &image_size);
> +
> +                       fwu_mtd_images[off_img].start = bank_offset + image_offset;
> +                       fwu_mtd_images[off_img].size = image_size;
> +                       fwu_mtd_images[off_img].bank_num = bank_num;
> +                       fwu_mtd_images[off_img].image_num = image_num;
> +                       strcpy(fwu_mtd_images[off_img].uuidbuf, uuid);
> +                       log_debug("\tImage%d: %s @0x%x\n\n",
> +                                 image_num, uuid, bank_offset + image_offset);
> +                       off_img++;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static int fwu_mdata_mtd_probe(struct udevice *dev)
> +{
> +       /* Ensure the metadata can be read. */
> +       return fwu_get_mdata(NULL);
> +}
> +
> +static struct fwu_mdata_ops fwu_mtd_ops = {
> +       .read_mdata = fwu_mtd_read_mdata,
> +       .write_mdata = fwu_mtd_write_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),
> +};
> diff --git a/include/fwu.h b/include/fwu.h
> index 33b4d0b3db..edb7e9da90 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -8,6 +8,8 @@
>
>  #include <blk.h>
>  #include <efi.h>
> +#include <mtd.h>
> +#include <uuid.h>
>
>  #include <linux/types.h>
>
> @@ -18,6 +20,12 @@ struct fwu_mdata_gpt_blk_priv {
>         struct udevice *blk_dev;
>  };
>
> +struct fwu_mtd_image_info {
> +       u32 start, size;
> +       int bank_num, image_num;

1 field per line in structure definition?

> +       char uuidbuf[UUID_STR_LEN + 1];
> +};
> +
>  struct fwu_mdata_ops {
>         /**
>          * read_mdata() - Populate the asked FWU metadata copy
> @@ -251,4 +259,28 @@ u8 fwu_empty_capsule_checks_pass(void);
>   */
>  int fwu_trial_state_ctr_start(void);
>
> +/**
> + * fwu_gen_alt_info_from_mtd() - Parse dfu_alt_info from metadata in mtd
> + * @buf: Buffer into which the dfu_alt_info is filled
> + * @len: Maximum characters that can be written in buf
> + * @mtd: Pointer to underlying MTD device
> + *
> + * Parse dfu_alt_info from metadata in mtd. Used for setting the env.
> + *
> + * Return: 0 if OK, -ve on error
> + */
> +int fwu_gen_alt_info_from_mtd(char *buf, size_t len, struct mtd_info *mtd);
> +
> +/**
> + * fwu_mtd_get_alt_num() - Mapping of fwu_plat_get_alt_num for MTD device
> + * @image_guid: Image GUID for which DFU alt number needs to be retrieved
> + * @alt_num: Pointer to the alt_num
> + * @mtd_dev: Name of mtd device instance
> + *
> + * To map fwu_plat_get_alt_num onto mtd based metadata implementation.
> + *
> + * Return: 0 if OK, -ve on error
> + */
> +int fwu_mtd_get_alt_num(efi_guid_t *image_guid, u8 *alt_num, const char *mtd_dev);
> +
>  #endif /* _FWU_H_ */
> diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> index 1993088e5b..c9e3c06b48 100644
> --- a/lib/fwu_updates/Makefile
> +++ b/lib/fwu_updates/Makefile
> @@ -5,3 +5,4 @@
>
>  obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
>  obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_gpt.o
> +obj-$(CONFIG_FWU_MDATA_MTD) += fwu_mtd.o
> diff --git a/lib/fwu_updates/fwu_mtd.c b/lib/fwu_updates/fwu_mtd.c
> new file mode 100644
> index 0000000000..f22c8b9443
> --- /dev/null
> +++ b/lib/fwu_updates/fwu_mtd.c
> @@ -0,0 +1,185 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include <dm.h>
> +#include <dfu.h>
> +#include <fwu.h>
> +#include <fwu_mdata.h>
> +#include <log.h>
> +#include <malloc.h>
> +#include <mtd.h>
> +#include <uuid.h>
> +#include <vsprintf.h>
> +
> +#include <dm/ofnode.h>
> +
> +struct fwu_mtd_image_info
> +fwu_mtd_images[CONFIG_FWU_NUM_BANKS * CONFIG_FWU_NUM_IMAGES_PER_BANK];
> +
> +static struct fwu_mtd_image_info *mtd_img_by_uuid(const char *uuidbuf)
> +{
> +       int num_images = ARRAY_SIZE(fwu_mtd_images);
> +
> +       for (int i = 0; i < num_images; i++)
> +               if (!strcmp(uuidbuf, fwu_mtd_images[i].uuidbuf))
> +                       return &fwu_mtd_images[i];
> +
> +       return NULL;
> +}
> +
> +int fwu_mtd_get_alt_num(efi_guid_t *image_id, u8 *alt_num,
> +                       const char *mtd_dev)
> +{
> +       struct fwu_mtd_image_info *mtd_img_info;
> +       char uuidbuf[UUID_STR_LEN + 1];
> +       fdt_addr_t offset, size = 0;
> +       struct dfu_entity *dfu;
> +       int i, nalt, ret;
> +
> +       mtd_probe_devices();
> +
> +       uuid_bin_to_str(image_id->b, uuidbuf, UUID_STR_FORMAT_STD);
> +
> +       mtd_img_info = mtd_img_by_uuid(uuidbuf);
> +       if (!mtd_img_info) {
> +               log_err("%s: Not found partition for image %s\n", __func__, uuidbuf);
> +               return -ENOENT;
> +       }
> +
> +       offset = mtd_img_info->start;
> +       size = mtd_img_info->size;
> +
> +       ret = dfu_init_env_entities(NULL, NULL);
> +       if (ret)
> +               return -ENOENT;
> +
> +       nalt = 0;
> +       list_for_each_entry(dfu, &dfu_list, list)
> +               nalt++;
> +
> +       if (!nalt) {
> +               log_warning("No entities in dfu_alt_info\n");
> +               dfu_free_entities();
> +               return -ENOENT;
> +       }
> +
> +       ret = -1;

ret = -ENOENT  ?

> +       for (i = 0; i < nalt; i++) {
> +               dfu = dfu_get_entity(i);
> +
> +               /* Only MTD RAW access */
> +               if (!dfu || dfu->dev_type != DFU_DEV_MTD ||
> +                   dfu->layout != DFU_RAW_ADDR ||
> +                       dfu->data.mtd.start != offset ||
> +                       dfu->data.mtd.size != size)

Indentation

> +                       continue;
> +
> +               *alt_num = dfu->alt;
> +               ret = 0;
> +               break;
> +       }
> +
> +       dfu_free_entities();
> +
> +       log_debug("%s: %s -> %d\n", __func__, uuidbuf, *alt_num);
> +       return ret;
> +}
> +
> +/**
> + * fwu_plat_get_alt_num() - Get the DFU Alt Num for the image from the platform
> + * @dev: FWU device
> + * @image_id: Image GUID for which DFU alt number needs to be retrieved
> + * @alt_num: Pointer to the alt_num
> + *
> + * Get the DFU alt number from the platform for the image specified by the
> + * image GUID.
> + *
> + * Note: This is a weak function and platforms can override this with
> + * their own implementation for obtaining the alt number value.
> + *
> + * Return: 0 if OK, -ve on error
> + */
> +__weak int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_id,
> +                               u8 *alt_num)
> +{
> +       return fwu_mtd_get_alt_num(image_id, alt_num, "nor1");
> +}
> +
> +static int gen_image_alt_info(char *buf, size_t len, int sidx,
> +                             struct fwu_image_entry *img, struct mtd_info *mtd)
> +{
> +       char *p = buf, *end = buf + len;
> +       int i;
> +
> +       p += snprintf(p, end - p, "mtd %s", mtd->name);
> +       if (end < p) {
> +               log_err("%s:%d Run out of buffer\n", __func__, __LINE__);
> +               return -E2BIG;
> +       }
> +
> +       /*
> +        * List the image banks in the FWU mdata and search the corresponding
> +        * partition based on partition's uuid.
> +        */
> +       for (i = 0; i < CONFIG_FWU_NUM_BANKS; i++) {
> +               struct fwu_mtd_image_info *mtd_img_info;
> +               struct fwu_image_bank_info *bank;
> +               char uuidbuf[UUID_STR_LEN + 1];
> +               u32 offset, size;
> +
> +               /* Query a partition by image UUID */
> +               bank = &img->img_bank_info[i];
> +               uuid_bin_to_str(bank->image_uuid.b, uuidbuf, UUID_STR_FORMAT_STD);
> +
> +               mtd_img_info = mtd_img_by_uuid(uuidbuf);
> +               if (!mtd_img_info) {
> +                       log_err("%s: Not found partition for image %s\n", __func__, uuidbuf);
> +                       break;
> +               }
> +
> +               offset = mtd_img_info->start;
> +               size = mtd_img_info->size;
> +
> +               p += snprintf(p, end - p, "%sbank%d raw %x %x",
> +                             i == 0 ? "=" : ";", i, offset, size);
> +               if (end < p) {
> +                       log_err("%s:%d Run out of buffer\n", __func__, __LINE__);
> +                       return -E2BIG;
> +               }
> +       }
> +
> +       if (i == CONFIG_FWU_NUM_BANKS)
> +               return 0;
> +
> +       return -ENOENT;
> +}
> +
> +int fwu_gen_alt_info_from_mtd(char *buf, size_t len, struct mtd_info *mtd)
> +{
> +       struct fwu_mdata mdata;
> +       int i, l, ret;
> +
> +       ret = fwu_get_mdata(&mdata);
> +       if (ret < 0) {
> +               log_err("Failed to get the FWU mdata.\n");
> +               return ret;
> +       }
> +
> +       for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
> +               ret = gen_image_alt_info(buf, len, i * CONFIG_FWU_NUM_BANKS,
> +                                        &mdata.img_entry[i], mtd);
> +               if (ret)
> +                       break;
> +
> +               l = strlen(buf);
> +               /* Replace the last ';' with '&' if there is another image. */
> +               if (i != CONFIG_FWU_NUM_IMAGES_PER_BANK - 1 && l)
> +                       buf[l - 1] = '&';
> +               len -= l;
> +               buf += l;
> +       }
> +
> +       return ret;
> +}
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox
  2023-04-10 23:01 [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox jaswinder.singh
                   ` (5 preceding siblings ...)
  2023-04-10 23:04 ` [PATCH v5 6/6] fwu: provide default fwu_plat_get_bootidx jaswinder.singh
@ 2023-04-14 14:18 ` Michal Simek
  6 siblings, 0 replies; 12+ messages in thread
From: Michal Simek @ 2023-04-14 14:18 UTC (permalink / raw)
  To: jaswinder.singh, u-boot
  Cc: ilias.apalodimas, etienne.carriere, trini, sjg, sughosh.ganu,
	xypron.glpk, takahiro.akashi



On 4/11/23 01:01, jaswinder.singh@linaro.org wrote:
> From: Jassi Brar <jaswinder.singh@linaro.org>
> 
> Introduce support for mtd backed storage for FWU feature and enable it on
> Synquacer platform based DeveloperBox.
> 
> This revision is rebased onto patchset that trims the FWU api
>   https://lore.kernel.org/u-boot/20230306231747.1888513-1-jassisinghbrar@gmail.com/
> 
> Changes since v4:
> 	* Provide default/weak implementations of fwu_plat_get_alt_num and fwu_plat_get_bootidx
> 	* Provide man page for mkfwumdata
> 	* Misc typo fixes and cosmetic changes
> 
> Changes since v3:
> 	* Fix and Update documentation to also build optee for FWU FIP image.
> 	* Fixed checkpatch warnings
> 	* Made local functions static.
> 	* Split config changes to a separate patch
> 	* Fix authorship of three patches.
> 
> Jassi Brar (4):
>    dt: fwu: developerbox: enable fwu banks and mdata regions
>    configs: move to new flash layout and boot flow
>    fwu: DeveloperBox: add support for FWU
>    fwu: provide default fwu_plat_get_bootidx
> 
> Masami Hiramatsu (2):
>    FWU: Add FWU metadata access driver for MTD storage regions
>    tools: Add mkfwumdata tool for FWU metadata image
> 
>   .../synquacer-sc2a11-developerbox-u-boot.dtsi |  49 ++-
>   board/socionext/developerbox/Makefile         |   1 +
>   board/socionext/developerbox/developerbox.c   |   8 +
>   board/socionext/developerbox/fwu_plat.c       |  37 ++
>   configs/synquacer_developerbox_defconfig      |  12 +-
>   doc/board/socionext/developerbox.rst          | 155 +++++++-
>   doc/mkfwumdata.1                              |  89 +++++
>   drivers/fwu-mdata/Kconfig                     |  15 +
>   drivers/fwu-mdata/Makefile                    |   1 +
>   drivers/fwu-mdata/raw_mtd.c                   | 272 ++++++++++++++
>   include/configs/synquacer.h                   |  10 +
>   include/fwu.h                                 |  32 ++
>   lib/fwu_updates/Makefile                      |   1 +
>   lib/fwu_updates/fwu.c                         |  18 +
>   lib/fwu_updates/fwu_mtd.c                     | 185 ++++++++++
>   tools/Kconfig                                 |   9 +
>   tools/Makefile                                |   4 +
>   tools/mkfwumdata.c                            | 334 ++++++++++++++++++
>   18 files changed, 1221 insertions(+), 11 deletions(-)
>   create mode 100644 board/socionext/developerbox/fwu_plat.c
>   create mode 100644 doc/mkfwumdata.1
>   create mode 100644 drivers/fwu-mdata/raw_mtd.c
>   create mode 100644 lib/fwu_updates/fwu_mtd.c
>   create mode 100644 tools/mkfwumdata.c
> 

Jassi sent email to dt guys and I would wait for finding the right location for 
dt binding and get it review before this can be merged.

I have commented some stuff in v4 which I would like to see in v6 especially to 
explain that uuid which is not used anywhere.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-04-14 14:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-10 23:01 [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox jaswinder.singh
2023-04-10 23:02 ` [PATCH v5 1/6] FWU: Add FWU metadata access driver for MTD storage regions jaswinder.singh
2023-04-11 17:34   ` Etienne Carriere
2023-04-10 23:03 ` [PATCH v5 2/6] tools: Add mkfwumdata tool for FWU metadata image jaswinder.singh
2023-04-11 17:28   ` Etienne Carriere
2023-04-10 23:03 ` [PATCH v5 3/6] dt: fwu: developerbox: enable fwu banks and mdata regions jaswinder.singh
2023-04-10 23:04 ` [PATCH v5 4/6] configs: move to new flash layout and boot flow jaswinder.singh
2023-04-11 17:12   ` Etienne Carriere
2023-04-10 23:04 ` [PATCH v5 5/6] fwu: DeveloperBox: add support for FWU jaswinder.singh
2023-04-11 17:09   ` Etienne Carriere
2023-04-10 23:04 ` [PATCH v5 6/6] fwu: provide default fwu_plat_get_bootidx jaswinder.singh
2023-04-14 14:18 ` [PATCH v5 0/6] FWU: Add support for mtd backed feature on DeveloperBox Michal Simek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.