[RFC PATCH 00/14] FWU: Add FWU Multi Bank Update for DeveloerBox

[RFC PATCH 00/14] FWU: Add FWU Multi Bank Update for DeveloerBox

[Masami Hiramatsu]

Hi,

Here is an RFC series of patches for the FWU Multi Bank Update
support for the DeveloperBox platform. This series depends on
Sughosh's Multi Bank Update v3 [1]. Thus if that is updated,
this must be rebased and updated too.

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=281875

This series includes my previous DFU updates[2] as [01/14]-[05/14],
which is still under review in the U-Boot ML. For "nicely" indent
the dfu_alt_info lines, I need that series. :-)

[2] https://patchwork.ozlabs.org/project/uboot/list/?series=275293

I also added some patches which updates Sughosh's series as [06/14]
- [08/14]. Thus the [09/14] - [14/14] are the changes for DeveloperBox.

Unlike the STM32MP board, DeveloperBox (SynQuacer) loads the firmware
from SPI NOR flash. Thus it doesn't use GPT partitions to store the
firmware banks and the FWU metadata. Instead, it stores those data
at fixed address areas on SPI NOR flash. I carefully chose the areas
which doesn't overlap the previous firmware and EDK2.

I introduced fwu_metadata_sf.c driver for this FWU multi bank support
on SPI flash, which does not use GPT too.

Since there is no GPT, the location GUID for images and the image
GUID for banks are Null GUID. Anyway, for managing firmware image
banks, we only need the ImageType GUID.

And the SynQuacer also does not have any non-volatile register.
Thus this allocates the platform defined boot index on the SPI flash
too.

So, in summary, on the DeveloperBox, this FWU Multi Bank update is
only for avoidance of unexpected bricking by firmware update. If there
are multiple banks, when the user (or firmware developer) update it
with a wrong image, they can revert the wrong one. Even if it does
not boot, SCP firmware can detect it and roll back to the previous
bank. But please note that this is not "security" enhancement at all
at least on the DeveloperBox because all images are still on normal
SPI NOR flash. This is rather like a "safety" feature like a fool proof.

NOTE: To use this series, you also need to update SCP firmware[3] and
TF-A[4] on the DeveloperBox. Those are under cleaning up.

[3] https://git.linaro.org/people/masami.hiramatsu/SCP-firmware.git/
[4] https://git.linaro.org/people/masami.hiramatsu/arm-trusted-firmware.git/

Thank you,

---

Masami Hiramatsu (14):
      DFU: Do not copy the entity name over the buffer size
      DFU: Accept redundant spaces and tabs in dfu_alt_info
      DFU: Check the number of arguments and argument string strictly
      doc: usage: DFU: Fix dfu_alt_info document
      cmd/dfu: Enable 'dfu list' command without DFU_OVER_USB
      FWU: Calculate CRC32 in gpt_update_mdata()
      FWU: Free metadata copy if gpt_get_mdata() failed
      FWU: Move FWU metadata operation code in fwu_mdata.c
      synquacer: Update for TBBR based new FIP layout
      FWU: Reboot soon after successfully install the new firmware
      FWU: Add FWU Multi Bank Update on SPI Flash
      FWU: synquacer: Add FWU Multi bank update support for DeveloperBox
      FWU: synquacer: Initialize broken metadata
      configs: synquacer: Add FWU support for DeveloperBox


 .../dts/synquacer-sc2a11-developerbox-u-boot.dtsi  |   26 +-
 board/socionext/developerbox/Kconfig               |   31 +++
 board/socionext/developerbox/Makefile              |    1 
 board/socionext/developerbox/fwu_plat.c            |  217 ++++++++++++++++++
 cmd/dfu.c                                          |    6 
 configs/synquacer_developerbox_defconfig           |   12 +
 doc/usage/dfu.rst                                  |   45 +++-
 drivers/dfu/dfu.c                                  |   37 ++-
 drivers/dfu/dfu_mmc.c                              |   55 +++--
 drivers/dfu/dfu_mtd.c                              |   34 ++-
 drivers/dfu/dfu_nand.c                             |   34 ++-
 drivers/dfu/dfu_ram.c                              |   24 +-
 drivers/dfu/dfu_sf.c                               |   34 ++-
 drivers/dfu/dfu_virt.c                             |    5 
 include/configs/synquacer.h                        |   14 +
 include/dfu.h                                      |   33 ++-
 include/efi_loader.h                               |    3 
 include/fwu.h                                      |   22 +-
 lib/efi_loader/efi_capsule.c                       |   10 +
 lib/efi_loader/efi_firmware.c                      |   14 +
 lib/fwu_updates/Kconfig                            |   43 ++++
 lib/fwu_updates/Makefile                           |    5 
 lib/fwu_updates/fwu_mdata.c                        |  123 ++++++----
 lib/fwu_updates/fwu_mdata_gpt_blk.c                |  109 +--------
 lib/fwu_updates/fwu_mdata_sf.c                     |  241 ++++++++++++++++++++
 25 files changed, 888 insertions(+), 290 deletions(-)
 create mode 100644 board/socionext/developerbox/fwu_plat.c
 create mode 100644 lib/fwu_updates/fwu_mdata_sf.c

--
Masami Hiramatsu <masami.hiramatsu@linaro.org>

[RFC PATCH 01/14] DFU: Do not copy the entity name over the buffer size

[Masami Hiramatsu]

Use strlcpy() instead of strcpy() to prevent copying the
entity name over the name buffer size.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/dfu/dfu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index af3975925a..66c41b5e76 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -503,7 +503,7 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt,
 
 	debug("%s: %s interface: %s dev: %s\n", __func__, s, interface, devstr);
 	st = strsep(&s, " ");
-	strcpy(dfu->name, st);
+	strlcpy(dfu->name, st, DFU_NAME_SIZE);
 
 	dfu->alt = alt;
 	dfu->max_buf_size = 0;

[RFC PATCH 02/14] DFU: Accept redundant spaces and tabs in dfu_alt_info

[Masami Hiramatsu]

If dfu_alt_info has repeated spaces or tab (for indentation or
readability), the dfu fails to parse it. For example, if
dfu_alt_info="mtd nor1=image raw  100000 200000" (double spaces
after "raw"), the image entity start address is '0' and the size
'0x100000'. This is because the repeated space is not skipped.

Use space and tab as a separater and apply skip_spaces() to
skip redundant spaces and tabs.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/dfu/dfu.c      |    6 ++++--
 drivers/dfu/dfu_mmc.c  |   11 ++++++++---
 drivers/dfu/dfu_mtd.c  |    8 ++++++--
 drivers/dfu/dfu_nand.c |   11 ++++++++---
 drivers/dfu/dfu_ram.c  |    3 ++-
 drivers/dfu/dfu_sf.c   |   12 +++++++++---
 6 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 66c41b5e76..18154774f9 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -123,9 +123,10 @@ int dfu_config_interfaces(char *env)
 	s = env;
 	while (s) {
 		ret = -EINVAL;
-		i = strsep(&s, " ");
+		i = strsep(&s, " \t");
 		if (!i)
 			break;
+		s = skip_spaces(s);
 		d = strsep(&s, "=");
 		if (!d)
 			break;
@@ -502,8 +503,9 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt,
 	char *st;
 
 	debug("%s: %s interface: %s dev: %s\n", __func__, s, interface, devstr);
-	st = strsep(&s, " ");
+	st = strsep(&s, " \t");
 	strlcpy(dfu->name, st, DFU_NAME_SIZE);
+	s = skip_spaces(s);
 
 	dfu->alt = alt;
 	dfu->max_buf_size = 0;
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index 3dab5a5f63..d747ede66c 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -351,11 +351,12 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s)
 	dfu->data.mmc.dev_num = dectoul(devstr, NULL);
 
 	for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
-		*parg = strsep(&s, " ");
+		*parg = strsep(&s, " \t");
 		if (*parg == NULL) {
 			pr_err("Invalid number of arguments.\n");
 			return -ENODEV;
 		}
+		s = skip_spaces(s);
 	}
 
 	entity_type = argv[0];
@@ -390,9 +391,11 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s)
 		 * specifying the mmc HW defined partition number
 		 */
 		if (s)
-			if (!strcmp(strsep(&s, " "), "mmcpart"))
+			if (!strcmp(strsep(&s, " \t"), "mmcpart")) {
+				s = skip_spaces(s);
 				dfu->data.mmc.hw_partition =
 					simple_strtoul(s, NULL, 0);
+			}
 
 	} else if (!strcmp(entity_type, "part")) {
 		struct disk_partition partinfo;
@@ -412,8 +415,10 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s)
 		 * specifying the mmc HW defined partition number
 		 */
 		if (s)
-			if (!strcmp(strsep(&s, " "), "offset"))
+			if (!strcmp(strsep(&s, " \t"), "offset")) {
+				s = skip_spaces(s);
 				offset = simple_strtoul(s, NULL, 0);
+			}
 
 		dfu->layout			= DFU_RAW_ADDR;
 		dfu->data.mmc.lba_start		= partinfo.start + offset;
diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c
index 0b7f17761f..ee534a9d10 100644
--- a/drivers/dfu/dfu_mtd.c
+++ b/drivers/dfu/dfu_mtd.c
@@ -12,6 +12,7 @@
 #include <mtd.h>
 #include <jffs2/load_kernel.h>
 #include <linux/err.h>
+#include <linux/ctype.h>
 
 static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size)
 {
@@ -265,11 +266,14 @@ int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s)
 	dfu->data.mtd.info = mtd;
 	dfu->max_buf_size = mtd->erasesize;
 
-	st = strsep(&s, " ");
+	st = strsep(&s, " \t");
+	s = skip_spaces(s);
 	if (!strcmp(st, "raw")) {
 		dfu->layout = DFU_RAW_ADDR;
 		dfu->data.mtd.start = hextoul(s, &s);
-		s++;
+		if (!isspace(*s))
+			return -1;
+		s = skip_spaces(s);
 		dfu->data.mtd.size = hextoul(s, &s);
 	} else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) {
 		char mtd_id[32];
diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index e53b35e42b..76761939ab 100644
--- a/drivers/dfu/dfu_nand.c
+++ b/drivers/dfu/dfu_nand.c
@@ -201,11 +201,14 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s)
 
 	dfu->data.nand.ubi = 0;
 	dfu->dev_type = DFU_DEV_NAND;
-	st = strsep(&s, " ");
+	st = strsep(&s, " \t");
+	s = skip_spaces(s);
 	if (!strcmp(st, "raw")) {
 		dfu->layout = DFU_RAW_ADDR;
 		dfu->data.nand.start = hextoul(s, &s);
-		s++;
+		if (!isspace(*s))
+			return -1;
+		s = skip_spaces(s);
 		dfu->data.nand.size = hextoul(s, &s);
 	} else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) {
 		char mtd_id[32];
@@ -216,7 +219,9 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s)
 		dfu->layout = DFU_RAW_ADDR;
 
 		dev = dectoul(s, &s);
-		s++;
+		if (!isspace(*s))
+			return -1;
+		s = skip_spaces(s);
 		part = dectoul(s, &s);
 
 		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
diff --git a/drivers/dfu/dfu_ram.c b/drivers/dfu/dfu_ram.c
index cc7e45ba33..361a3ff8af 100644
--- a/drivers/dfu/dfu_ram.c
+++ b/drivers/dfu/dfu_ram.c
@@ -60,11 +60,12 @@ int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s)
 	const char **parg = argv;
 
 	for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
-		*parg = strsep(&s, " ");
+		*parg = strsep(&s, " \t");
 		if (*parg == NULL) {
 			pr_err("Invalid number of arguments.\n");
 			return -ENODEV;
 		}
+		s = skip_spaces(s);
 	}
 
 	dfu->dev_type = DFU_DEV_RAM;
diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c
index b72493ced8..993e951bc3 100644
--- a/drivers/dfu/dfu_sf.c
+++ b/drivers/dfu/dfu_sf.c
@@ -13,6 +13,7 @@
 #include <spi_flash.h>
 #include <jffs2/load_kernel.h>
 #include <linux/mtd/mtd.h>
+#include <linux/ctype.h>
 
 static int dfu_get_medium_size_sf(struct dfu_entity *dfu, u64 *size)
 {
@@ -178,11 +179,14 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s)
 	dfu->dev_type = DFU_DEV_SF;
 	dfu->max_buf_size = dfu->data.sf.dev->sector_size;
 
-	st = strsep(&s, " ");
+	st = strsep(&s, " \t");
+	s = skip_spaces(s);
 	if (!strcmp(st, "raw")) {
 		dfu->layout = DFU_RAW_ADDR;
 		dfu->data.sf.start = hextoul(s, &s);
-		s++;
+		if (!isspace(*s))
+			return -1;
+		s = skip_spaces(s);
 		dfu->data.sf.size = hextoul(s, &s);
 	} else if (CONFIG_IS_ENABLED(DFU_SF_PART) &&
 		   (!strcmp(st, "part") || !strcmp(st, "partubi"))) {
@@ -195,7 +199,9 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s)
 		dfu->layout = DFU_RAW_ADDR;
 
 		dev = dectoul(s, &s);
-		s++;
+		if (!isspace(*s))
+			return -1;
+		s = skip_spaces(s);
 		part = dectoul(s, &s);
 
 		sprintf(mtd_id, "%s%d,%d", "nor", dev, part - 1);

[RFC PATCH 03/14] DFU: Check the number of arguments and argument string strictly

[Masami Hiramatsu]

When parsing the dfu_alt_info, check the number of arguments
and argument string strictly. If there is any garbage data
(which is not able to be parsed correctly) in dfu_alt_info,
that means something wrong and user may make a typo or mis-
understanding about the syntax. Since the dfu_alt_info is
used for updating the firmware, this mistake may lead to
brick the hardware.
Thus it should be checked strictly for making sure there
is no mistake.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/dfu/dfu.c      |   31 +++++++++++++++++++++------
 drivers/dfu/dfu_mmc.c  |   56 ++++++++++++++++++++++++++----------------------
 drivers/dfu/dfu_mtd.c  |   36 +++++++++++++++++++------------
 drivers/dfu/dfu_nand.c |   39 ++++++++++++++++++---------------
 drivers/dfu/dfu_ram.c  |   25 ++++++++++-----------
 drivers/dfu/dfu_sf.c   |   38 +++++++++++++++++----------------
 drivers/dfu/dfu_virt.c |    5 +++-
 include/dfu.h          |   33 ++++++++++++++++++----------
 8 files changed, 154 insertions(+), 109 deletions(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 18154774f9..516dda6179 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -500,12 +500,29 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt,
 			   char *interface, char *devstr)
 {
+	char *argv[DFU_MAX_ENTITY_ARGS];
+	int argc;
 	char *st;
 
 	debug("%s: %s interface: %s dev: %s\n", __func__, s, interface, devstr);
 	st = strsep(&s, " \t");
 	strlcpy(dfu->name, st, DFU_NAME_SIZE);
-	s = skip_spaces(s);
+
+	/* Parse arguments */
+	for (argc = 0; s && argc < DFU_MAX_ENTITY_ARGS; argc++) {
+		s = skip_spaces(s);
+		if (!*s)
+			break;
+		argv[argc] = strsep(&s, " \t");
+	}
+
+	if (argc == DFU_MAX_ENTITY_ARGS && s) {
+		s = skip_spaces(s);
+		if (*s) {
+			log_err("Too many arguments for %s\n", dfu->name);
+			return -EINVAL;
+		}
+	}
 
 	dfu->alt = alt;
 	dfu->max_buf_size = 0;
@@ -513,22 +530,22 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt,
 
 	/* Specific for mmc device */
 	if (strcmp(interface, "mmc") == 0) {
-		if (dfu_fill_entity_mmc(dfu, devstr, s))
+		if (dfu_fill_entity_mmc(dfu, devstr, argv, argc))
 			return -1;
 	} else if (strcmp(interface, "mtd") == 0) {
-		if (dfu_fill_entity_mtd(dfu, devstr, s))
+		if (dfu_fill_entity_mtd(dfu, devstr, argv, argc))
 			return -1;
 	} else if (strcmp(interface, "nand") == 0) {
-		if (dfu_fill_entity_nand(dfu, devstr, s))
+		if (dfu_fill_entity_nand(dfu, devstr, argv, argc))
 			return -1;
 	} else if (strcmp(interface, "ram") == 0) {
-		if (dfu_fill_entity_ram(dfu, devstr, s))
+		if (dfu_fill_entity_ram(dfu, devstr, argv, argc))
 			return -1;
 	} else if (strcmp(interface, "sf") == 0) {
-		if (dfu_fill_entity_sf(dfu, devstr, s))
+		if (dfu_fill_entity_sf(dfu, devstr, argv, argc))
 			return -1;
 	} else if (strcmp(interface, "virt") == 0) {
-		if (dfu_fill_entity_virt(dfu, devstr, s))
+		if (dfu_fill_entity_virt(dfu, devstr, argv, argc))
 			return -1;
 	} else {
 		printf("%s: Device %s not (yet) supported!\n",
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index d747ede66c..a91da972d4 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -337,35 +337,34 @@ void dfu_free_entity_mmc(struct dfu_entity *dfu)
  *	4th (optional):
  *		mmcpart <num> (access to HW eMMC partitions)
  */
-int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
 	const char *entity_type;
 	ssize_t second_arg;
 	size_t third_arg;
-
 	struct mmc *mmc;
+	char *s;
 
-	const char *argv[3];
-	const char **parg = argv;
-
-	dfu->data.mmc.dev_num = dectoul(devstr, NULL);
-
-	for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
-		*parg = strsep(&s, " \t");
-		if (*parg == NULL) {
-			pr_err("Invalid number of arguments.\n");
-			return -ENODEV;
-		}
-		s = skip_spaces(s);
+	if (argc < 3) {
+		pr_err("The number of parameters are not enough.\n");
+		return -EINVAL;
 	}
 
+	dfu->data.mmc.dev_num = dectoul(devstr, &s);
+	if (*s)
+		return -EINVAL;
+
 	entity_type = argv[0];
 	/*
 	 * Base 0 means we'll accept (prefixed with 0x or 0) base 16, 8,
 	 * with default 10.
 	 */
-	second_arg = simple_strtol(argv[1], NULL, 0);
-	third_arg = simple_strtoul(argv[2], NULL, 0);
+	second_arg = simple_strtol(argv[1], &s, 0);
+	if (*s)
+		return -EINVAL;
+	third_arg = simple_strtoul(argv[2], &s, 0);
+	if (*s)
+		return -EINVAL;
 
 	mmc = find_mmc_device(dfu->data.mmc.dev_num);
 	if (mmc == NULL) {
@@ -390,12 +389,14 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s)
 		 * Check for an extra entry at dfu_alt_info env variable
 		 * specifying the mmc HW defined partition number
 		 */
-		if (s)
-			if (!strcmp(strsep(&s, " \t"), "mmcpart")) {
-				s = skip_spaces(s);
-				dfu->data.mmc.hw_partition =
-					simple_strtoul(s, NULL, 0);
+		if (argc > 3) {
+			if (argc != 5 || strcmp(argv[3], "mmcpart")) {
+				pr_err("DFU mmc raw accept 'mmcpart <partnum>' option.\n");
+				return -EINVAL;
 			}
+			dfu->data.mmc.hw_partition =
+				simple_strtoul(argv[4], NULL, 0);
+		}
 
 	} else if (!strcmp(entity_type, "part")) {
 		struct disk_partition partinfo;
@@ -414,15 +415,18 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s)
 		 * Check for an extra entry at dfu_alt_info env variable
 		 * specifying the mmc HW defined partition number
 		 */
-		if (s)
-			if (!strcmp(strsep(&s, " \t"), "offset")) {
-				s = skip_spaces(s);
-				offset = simple_strtoul(s, NULL, 0);
+		if (argc > 3) {
+			if (argc != 5 || strcmp(argv[3], "offset")) {
+				pr_err("DFU mmc raw accept 'mmcpart <partnum>' option.\n");
+				return -EINVAL;
 			}
+			dfu->data.mmc.hw_partition =
+				simple_strtoul(argv[4], NULL, 0);
+		}
 
 		dfu->layout			= DFU_RAW_ADDR;
 		dfu->data.mmc.lba_start		= partinfo.start + offset;
-		dfu->data.mmc.lba_size		= partinfo.size-offset;
+		dfu->data.mmc.lba_size		= partinfo.size - offset;
 		dfu->data.mmc.lba_blk_size	= partinfo.blksz;
 	} else if (!strcmp(entity_type, "fat")) {
 		dfu->layout = DFU_FS_FAT;
diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c
index ee534a9d10..81357769ff 100644
--- a/drivers/dfu/dfu_mtd.c
+++ b/drivers/dfu/dfu_mtd.c
@@ -251,9 +251,9 @@ static unsigned int dfu_polltimeout_mtd(struct dfu_entity *dfu)
 	return DFU_DEFAULT_POLL_TIMEOUT;
 }
 
-int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
-	char *st;
+	char *s;
 	struct mtd_info *mtd;
 	int ret, part;
 
@@ -265,25 +265,33 @@ int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s)
 	dfu->dev_type = DFU_DEV_MTD;
 	dfu->data.mtd.info = mtd;
 	dfu->max_buf_size = mtd->erasesize;
+	if (argc < 1)
+		return -EINVAL;
 
-	st = strsep(&s, " \t");
-	s = skip_spaces(s);
-	if (!strcmp(st, "raw")) {
+	if (!strcmp(argv[0], "raw")) {
+		if (argc != 3)
+			return -EINVAL;
 		dfu->layout = DFU_RAW_ADDR;
-		dfu->data.mtd.start = hextoul(s, &s);
-		if (!isspace(*s))
-			return -1;
-		s = skip_spaces(s);
-		dfu->data.mtd.size = hextoul(s, &s);
-	} else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) {
+		dfu->data.mtd.start = hextoul(argv[1], &s);
+		if (*s)
+			return -EINVAL;
+		dfu->data.mtd.size = hextoul(argv[2], &s);
+		if (*s)
+			return -EINVAL;
+	} else if ((!strcmp(argv[0], "part")) || (!strcmp(argv[0], "partubi"))) {
 		char mtd_id[32];
 		struct mtd_device *mtd_dev;
 		u8 part_num;
 		struct part_info *pi;
 
+		if (argc != 2)
+			return -EINVAL;
+
 		dfu->layout = DFU_RAW_ADDR;
 
-		part = dectoul(s, &s);
+		part = dectoul(argv[1], &s);
+		if (*s)
+			return -EINVAL;
 
 		sprintf(mtd_id, "%s,%d", devstr, part - 1);
 		printf("using id '%s'\n", mtd_id);
@@ -298,10 +306,10 @@ int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s)
 
 		dfu->data.mtd.start = pi->offset;
 		dfu->data.mtd.size = pi->size;
-		if (!strcmp(st, "partubi"))
+		if (!strcmp(argv[0], "partubi"))
 			dfu->data.mtd.ubi = 1;
 	} else {
-		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+		printf("%s: Memory layout (%s) not supported!\n", __func__, argv[0]);
 		return -1;
 	}
 
diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index 76761939ab..08e8cf5cdb 100644
--- a/drivers/dfu/dfu_nand.c
+++ b/drivers/dfu/dfu_nand.c
@@ -194,23 +194,25 @@ unsigned int dfu_polltimeout_nand(struct dfu_entity *dfu)
 	return DFU_DEFAULT_POLL_TIMEOUT;
 }
 
-int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
-	char *st;
+	char *s;
 	int ret, dev, part;
 
 	dfu->data.nand.ubi = 0;
 	dfu->dev_type = DFU_DEV_NAND;
-	st = strsep(&s, " \t");
-	s = skip_spaces(s);
-	if (!strcmp(st, "raw")) {
+	if (argc != 3)
+		return -EINVAL;
+
+	if (!strcmp(argv[0], "raw")) {
 		dfu->layout = DFU_RAW_ADDR;
-		dfu->data.nand.start = hextoul(s, &s);
-		if (!isspace(*s))
-			return -1;
-		s = skip_spaces(s);
-		dfu->data.nand.size = hextoul(s, &s);
-	} else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) {
+		dfu->data.nand.start = hextoul(argv[1], &s);
+		if (*s)
+			return -EINVAL;
+		dfu->data.nand.size = hextoul(argv[2], &s);
+		if (*s)
+			return -EINVAL;
+	} else if ((!strcmp(argv[0], "part")) || (!strcmp(argv[0], "partubi"))) {
 		char mtd_id[32];
 		struct mtd_device *mtd_dev;
 		u8 part_num;
@@ -218,11 +220,12 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s)
 
 		dfu->layout = DFU_RAW_ADDR;
 
-		dev = dectoul(s, &s);
-		if (!isspace(*s))
-			return -1;
-		s = skip_spaces(s);
-		part = dectoul(s, &s);
+		dev = dectoul(argv[1], &s);
+		if (*s)
+			return -EINVAL;
+		part = dectoul(argv[2], &s);
+		if (*s)
+			return -EINVAL;
 
 		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
 		debug("using id '%s'\n", mtd_id);
@@ -237,10 +240,10 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s)
 
 		dfu->data.nand.start = pi->offset;
 		dfu->data.nand.size = pi->size;
-		if (!strcmp(st, "partubi"))
+		if (!strcmp(argv[0], "partubi"))
 			dfu->data.nand.ubi = 1;
 	} else {
-		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+		printf("%s: Memory layout (%s) not supported!\n", __func__, argv[0]);
 		return -1;
 	}
 
diff --git a/drivers/dfu/dfu_ram.c b/drivers/dfu/dfu_ram.c
index 361a3ff8af..9d10303164 100644
--- a/drivers/dfu/dfu_ram.c
+++ b/drivers/dfu/dfu_ram.c
@@ -54,18 +54,13 @@ static int dfu_read_medium_ram(struct dfu_entity *dfu, u64 offset,
 	return dfu_transfer_medium_ram(DFU_OP_READ, dfu, offset, buf, len);
 }
 
-int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
-	const char *argv[3];
-	const char **parg = argv;
-
-	for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
-		*parg = strsep(&s, " \t");
-		if (*parg == NULL) {
-			pr_err("Invalid number of arguments.\n");
-			return -ENODEV;
-		}
-		s = skip_spaces(s);
+	char *s;
+
+	if (argc != 3) {
+		pr_err("Invalid number of arguments.\n");
+		return -EINVAL;
 	}
 
 	dfu->dev_type = DFU_DEV_RAM;
@@ -75,8 +70,12 @@ int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s)
 	}
 
 	dfu->layout = DFU_RAM_ADDR;
-	dfu->data.ram.start = hextoul(argv[1], NULL);
-	dfu->data.ram.size = hextoul(argv[2], NULL);
+	dfu->data.ram.start = hextoul(argv[1], &s);
+	if (*s)
+		return -EINVAL;
+	dfu->data.ram.size = hextoul(argv[2], &s);
+	if (*s)
+		return -EINVAL;
 
 	dfu->write_medium = dfu_write_medium_ram;
 	dfu->get_medium_size = dfu_get_medium_size_ram;
diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c
index 993e951bc3..25a9c81850 100644
--- a/drivers/dfu/dfu_sf.c
+++ b/drivers/dfu/dfu_sf.c
@@ -166,9 +166,9 @@ static struct spi_flash *parse_dev(char *devstr)
 	return dev;
 }
 
-int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
-	char *st;
+	char *s;
 	char *devstr_bkup = strdup(devstr);
 
 	dfu->data.sf.dev = parse_dev(devstr_bkup);
@@ -179,17 +179,18 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s)
 	dfu->dev_type = DFU_DEV_SF;
 	dfu->max_buf_size = dfu->data.sf.dev->sector_size;
 
-	st = strsep(&s, " \t");
-	s = skip_spaces(s);
-	if (!strcmp(st, "raw")) {
+	if (argc != 3)
+		return -EINVAL;
+	if (!strcmp(argv[0], "raw")) {
 		dfu->layout = DFU_RAW_ADDR;
-		dfu->data.sf.start = hextoul(s, &s);
-		if (!isspace(*s))
-			return -1;
-		s = skip_spaces(s);
-		dfu->data.sf.size = hextoul(s, &s);
+		dfu->data.sf.start = hextoul(argv[1], &s);
+		if (*s)
+			return -EINVAL;
+		dfu->data.sf.size = hextoul(argv[2], &s);
+		if (*s)
+			return -EINVAL;
 	} else if (CONFIG_IS_ENABLED(DFU_SF_PART) &&
-		   (!strcmp(st, "part") || !strcmp(st, "partubi"))) {
+		   (!strcmp(argv[0], "part") || !strcmp(argv[0], "partubi"))) {
 		char mtd_id[32];
 		struct mtd_device *mtd_dev;
 		u8 part_num;
@@ -198,11 +199,12 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s)
 
 		dfu->layout = DFU_RAW_ADDR;
 
-		dev = dectoul(s, &s);
-		if (!isspace(*s))
-			return -1;
-		s = skip_spaces(s);
-		part = dectoul(s, &s);
+		dev = dectoul(argv[1], &s);
+		if (*s)
+			return -EINVAL;
+		part = dectoul(argv[2], &s);
+		if (*s)
+			return -EINVAL;
 
 		sprintf(mtd_id, "%s%d,%d", "nor", dev, part - 1);
 		printf("using id '%s'\n", mtd_id);
@@ -216,10 +218,10 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s)
 		}
 		dfu->data.sf.start = pi->offset;
 		dfu->data.sf.size = pi->size;
-		if (!strcmp(st, "partubi"))
+		if (!strcmp(argv[0], "partubi"))
 			dfu->data.sf.ubi = 1;
 	} else {
-		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+		printf("%s: Memory layout (%s) not supported!\n", __func__, argv[0]);
 		spi_flash_free(dfu->data.sf.dev);
 		return -1;
 	}
diff --git a/drivers/dfu/dfu_virt.c b/drivers/dfu/dfu_virt.c
index 80c99cb06e..29f7a08f67 100644
--- a/drivers/dfu/dfu_virt.c
+++ b/drivers/dfu/dfu_virt.c
@@ -32,10 +32,13 @@ int __weak dfu_read_medium_virt(struct dfu_entity *dfu, u64 offset,
 	return 0;
 }
 
-int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
 	debug("%s: devstr = %s\n", __func__, devstr);
 
+	if (argc != 0)
+		return -EINVAL;
+
 	dfu->dev_type = DFU_DEV_VIRT;
 	dfu->layout = DFU_RAW_ADDR;
 	dfu->data.virt.dev_num = dectoul(devstr, NULL);
diff --git a/include/dfu.h b/include/dfu.h
index f6868982df..dcb9cd9d79 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -432,11 +432,15 @@ static inline void dfu_set_defer_flush(struct dfu_entity *dfu)
 int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size);
 
 /* Device specific */
+/* Each entity has 5 arguments in maximum. */
+#define DFU_MAX_ENTITY_ARGS	5
+
 #if CONFIG_IS_ENABLED(DFU_MMC)
-extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s);
+extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr,
+			       char **argv, int argc);
 #else
 static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr,
-				      char *s)
+				      char **argv, int argc)
 {
 	puts("MMC support not available!\n");
 	return -1;
@@ -444,10 +448,11 @@ static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr,
 #endif
 
 #if CONFIG_IS_ENABLED(DFU_NAND)
-extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s);
+extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr,
+				char **argv, int argc);
 #else
 static inline int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr,
-				       char *s)
+				       char **argv, int argc)
 {
 	puts("NAND support not available!\n");
 	return -1;
@@ -455,10 +460,11 @@ static inline int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr,
 #endif
 
 #if CONFIG_IS_ENABLED(DFU_RAM)
-extern int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s);
+extern int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr,
+			       char **argv, int argc);
 #else
 static inline int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr,
-				      char *s)
+				      char **argv, int argc)
 {
 	puts("RAM support not available!\n");
 	return -1;
@@ -466,10 +472,11 @@ static inline int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr,
 #endif
 
 #if CONFIG_IS_ENABLED(DFU_SF)
-extern int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s);
+extern int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr,
+			      char **argv, int argc);
 #else
 static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr,
-				     char *s)
+				     char **argv, int argc)
 {
 	puts("SF support not available!\n");
 	return -1;
@@ -477,10 +484,11 @@ static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr,
 #endif
 
 #if CONFIG_IS_ENABLED(DFU_MTD)
-int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s);
+extern int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr,
+			       char **argv, int argc);
 #else
 static inline int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr,
-				      char *s)
+				      char **argv, int argc)
 {
 	puts("MTD support not available!\n");
 	return -1;
@@ -488,7 +496,8 @@ static inline int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr,
 #endif
 
 #ifdef CONFIG_DFU_VIRT
-int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, char *s);
+int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr,
+			 char **argv, int argc);
 int dfu_write_medium_virt(struct dfu_entity *dfu, u64 offset,
 			  void *buf, long *len);
 int dfu_get_medium_size_virt(struct dfu_entity *dfu, u64 *size);
@@ -496,7 +505,7 @@ int dfu_read_medium_virt(struct dfu_entity *dfu, u64 offset,
 			 void *buf, long *len);
 #else
 static inline int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr,
-				       char *s)
+				       char **argv, int argc)
 {
 	puts("VIRT support not available!\n");
 	return -1;

[RFC PATCH 04/14] doc: usage: DFU: Fix dfu_alt_info document

[Masami Hiramatsu]

Fix some typo and wrong information about dfu_alt_info.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 doc/usage/dfu.rst |   45 +++++++++++++++++++++++++++++++++------------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/doc/usage/dfu.rst b/doc/usage/dfu.rst
index 11c88072b8..25517637ae 100644
--- a/doc/usage/dfu.rst
+++ b/doc/usage/dfu.rst
@@ -113,9 +113,9 @@ mmc
     each element in *dfu_alt_info* being
 
     * <name> raw <offset> <size> [mmcpart <num>]   raw access to mmc device
-    * <name> part <dev> <part_id> [mmcpart <num>]  raw access to partition
-    * <name> fat <dev> <part_id> [mmcpart <num>]   file in FAT partition
-    * <name> ext4 <dev> <part_id> [mmcpart <num>]  file in EXT4 partition
+    * <name> part <dev> <part_id> [offset <byte>]  raw access to partition
+    * <name> fat <dev> <part_id>                   file in FAT partition
+    * <name> ext4 <dev> <part_id>                  file in EXT4 partition
     * <name> skip 0 0                              ignore flashed data
     * <name> script 0 0                            execute commands in shell
 
@@ -169,14 +169,20 @@ nand
 
     each element in *dfu_alt_info* being either of
 
-    * <name> raw <offset> <size>   raw access to mmc device
-    * <name> part <dev> <part_id>  raw acces to partition
-    * <name> partubi <dev> <part_id>  raw acces to ubi partition
+    * <name> raw <offset> <size>        raw access to nand device
+    * <name> part <devid> <part_id>     raw access to partition
+    * <name> partubi <devid> <part_id>  raw access to ubi partition
 
     with
 
+    offset
+        is the offset in the nand device (hexadecimal without "0x")
+    size
+        is the size of the access area (hexadecimal without "0x")
+    devid
+        is the NAND device index (decimal only)
     partid
-        is the MTD partition index
+        is the NAND partition index (decimal only)
 
 ram
     raw access to ram::
@@ -190,6 +196,13 @@ ram
 
       <name> ram <offset> <size>  raw access to ram
 
+    with
+
+    offset
+        is the offset in the ram device (hexadecimal without "0x")
+    size
+        is the size of the access area (hexadecimal without "0x")
+
 sf
     serial flash : NOR::
 
@@ -203,8 +216,12 @@ sf
 
     with
 
+    offset
+        is the offset in the mtd device (hexadecimal without "0x")
+    size
+        is the size of the access area (hexadecimal without "0x")
     partid
-        is the MTD partition index
+        is the MTD partition index (decimal)
 
 mtd
     all MTD device: NAND, SPI-NOR, SPI-NAND,...::
@@ -219,14 +236,18 @@ mtd
 
     each element in *dfu_alt_info* being either of:
 
-    * <name> raw <offset> <size> forraw access to mtd device
-    * <name> part <dev> <part_id> for raw acces to partition
-    * <name> partubi <dev> <part_id> for raw acces to ubi partition
+    * <name> raw <offset> <size>  for raw access to mtd device
+    * <name> part <part_id>       for raw access to partition
+    * <name> partubi <part_id>    for raw access to ubi partition
 
     with
 
+    offset
+        is the offset in the mtd device (hexadecimal without "0x")
+    size
+        is the size of the access area (hexadecimal without "0x")
     partid
-        is the MTD partition index
+        is the MTD partition index (decimal only)
 
 virt
     virtual flash back end for DFU

[RFC PATCH 05/14] cmd/dfu: Enable 'dfu list' command without DFU_OVER_USB

[Masami Hiramatsu]

Since dfu is not only used for USB, and some platform only
supports DFU_OVER_TFTP or EFI capsule update, dfu_alt_info
is defined on such platforms too.

For such platform, 'dfu list' command is useful to check
how the current dfu_alt_info setting is parsed.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 cmd/dfu.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmd/dfu.c b/cmd/dfu.c
index 4a288f74c2..208555c888 100644
--- a/cmd/dfu.c
+++ b/cmd/dfu.c
@@ -50,7 +50,6 @@ static int do_dfu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	if (!strcmp(argv[1], "tftp"))
 		return update_tftp(value, interface, devstring);
 #endif
-#ifdef CONFIG_DFU_OVER_USB
 	ret = dfu_init_env_entities(interface, devstring);
 	if (ret)
 		goto done;
@@ -65,6 +64,7 @@ static int do_dfu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		goto done;
 	}
 
+#ifdef CONFIG_DFU_OVER_USB
 	int controller_index = simple_strtoul(usb_controller, NULL, 0);
 	bool retry = false;
 	do {
@@ -79,9 +79,9 @@ static int do_dfu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		}
 	} while (retry);
 
+#endif
 done:
 	dfu_free_entities();
-#endif
 	return ret;
 }
 
@@ -100,8 +100,8 @@ U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu,
 #ifdef CONFIG_DFU_TIMEOUT
 	"    [<timeout>] - specify inactivity timeout in seconds\n"
 #endif
-	"    [list] - list available alt settings\n"
 #endif
+	"    [list] - list available alt settings\n"
 #ifdef CONFIG_DFU_OVER_TFTP
 #ifdef CONFIG_DFU_OVER_USB
 	"dfu "

[RFC PATCH 06/14] FWU: Calculate CRC32 in gpt_update_mdata()

[Masami Hiramatsu]

To avoid calculating crc32 in several places, do it in
gpt_update_mdata(). This also ensures the mdata crc32
is always sane.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 lib/fwu_updates/fwu_mdata.c         |   18 ------------------
 lib/fwu_updates/fwu_mdata_gpt_blk.c |   13 ++++++++-----
 2 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/lib/fwu_updates/fwu_mdata.c b/lib/fwu_updates/fwu_mdata.c
index 252fcf50f6..1324771a71 100644
--- a/lib/fwu_updates/fwu_mdata.c
+++ b/lib/fwu_updates/fwu_mdata.c
@@ -92,7 +92,6 @@ int fwu_get_active_index(u32 *active_idx)
 int fwu_update_active_index(u32 active_idx)
 {
 	int ret;
-	void *buf;
 	struct fwu_mdata *mdata = NULL;
 
 	if (active_idx > CONFIG_FWU_NUM_BANKS) {
@@ -113,14 +112,6 @@ int fwu_update_active_index(u32 active_idx)
 	mdata->previous_active_index = mdata->active_index;
 	mdata->active_index = active_idx;
 
-	/*
-	 * Calculate the crc32 for the updated FWU metadata
-	 * and put the updated value in the FWU metadata crc32
-	 * field
-	 */
-	buf = &mdata->version;
-	mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32));
-
 	/*
 	 * Now write this updated FWU metadata to both the
 	 * FWU metadata partitions
@@ -205,7 +196,6 @@ int fwu_mdata_check(void)
 int fwu_revert_boot_index(void)
 {
 	int ret;
-	void *buf;
 	u32 cur_active_index;
 	struct fwu_mdata *mdata = NULL;
 
@@ -223,14 +213,6 @@ int fwu_revert_boot_index(void)
 	mdata->active_index = mdata->previous_active_index;
 	mdata->previous_active_index = cur_active_index;
 
-	/*
-	 * Calculate the crc32 for the updated FWU metadata
-	 * and put the updated value in the FWU metadata crc32
-	 * field
-	 */
-	buf = &mdata->version;
-	mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32));
-
 	/*
 	 * Now write this updated FWU metadata to both the
 	 * FWU metadata partitions
diff --git a/lib/fwu_updates/fwu_mdata_gpt_blk.c b/lib/fwu_updates/fwu_mdata_gpt_blk.c
index 796b08e76f..0259da37c3 100644
--- a/lib/fwu_updates/fwu_mdata_gpt_blk.c
+++ b/lib/fwu_updates/fwu_mdata_gpt_blk.c
@@ -161,6 +161,14 @@ static int fwu_gpt_update_mdata(struct fwu_mdata *mdata)
 		return -ENODEV;
 	}
 
+	/*
+	 * Calculate the crc32 for the updated FWU metadata
+	 * and put the updated value in the FWU metadata crc32
+	 * field
+	 */
+	mdata->crc32 = crc32(0, (void *)&mdata->version,
+			     sizeof(*mdata) - sizeof(u32));
+
 	/* First write the primary partition*/
 	ret = gpt_write_mdata_partition(desc, mdata, primary_mpart);
 	if (ret < 0) {
@@ -457,7 +465,6 @@ int fwu_gpt_get_mdata(struct fwu_mdata **mdata)
 static int fwu_gpt_set_clear_image_accept(efi_guid_t *img_type_id,
 					  u32 bank, u8 action)
 {
-	void *buf;
 	int ret, i;
 	u32 nimages;
 	struct fwu_mdata *mdata = NULL;
@@ -480,10 +487,6 @@ static int fwu_gpt_set_clear_image_accept(efi_guid_t *img_type_id,
 			else
 				img_bank_info->accepted = 0;
 
-			buf = &mdata->version;
-			mdata->crc32 = crc32(0, buf, sizeof(*mdata) -
-					     sizeof(u32));
-
 			ret = fwu_gpt_update_mdata(mdata);
 			goto out;
 		}

[RFC PATCH 07/14] FWU: Free metadata copy if gpt_get_mdata() failed

[Masami Hiramatsu]

It is better if a function which returns an error then release
all allocated memory resources. This simplifies the mind model
and less chance to forgot to free memory and double free.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 lib/fwu_updates/fwu_mdata_gpt_blk.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/fwu_updates/fwu_mdata_gpt_blk.c b/lib/fwu_updates/fwu_mdata_gpt_blk.c
index 0259da37c3..7f8b8b68fc 100644
--- a/lib/fwu_updates/fwu_mdata_gpt_blk.c
+++ b/lib/fwu_updates/fwu_mdata_gpt_blk.c
@@ -215,7 +215,8 @@ static int gpt_get_mdata(struct fwu_mdata **mdata)
 	ret = gpt_read_mdata(desc, *mdata, primary_mpart);
 	if (ret < 0) {
 		log_err("Failed to read the FWU metadata from the device\n");
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
 
 	ret = fwu_verify_mdata(*mdata, 1);
@@ -230,7 +231,8 @@ static int gpt_get_mdata(struct fwu_mdata **mdata)
 	ret = gpt_read_mdata(desc, *mdata, secondary_mpart);
 	if (ret < 0) {
 		log_err("Failed to read the FWU metadata from the device\n");
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
 
 	ret = fwu_verify_mdata(*mdata, 0);
@@ -238,7 +240,10 @@ static int gpt_get_mdata(struct fwu_mdata **mdata)
 		return 0;
 
 	/* Both the FWU metadata copies are corrupted. */
-	return -1;
+	ret = -1;
+out:
+	free(*mdata);
+	return ret;
 }
 
 static int gpt_check_mdata_validity(void)

[RFC PATCH 08/14] FWU: Move FWU metadata operation code in fwu_mdata.c

[Masami Hiramatsu]

Remove some FWU metadata operations which only access or
modify the metadata itself from fwu_mdata_ops and move
it in fwu_mdata.c.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 include/fwu.h                       |    9 ---
 lib/fwu_updates/fwu_mdata.c         |  105 ++++++++++++++++++++++++-----------
 lib/fwu_updates/fwu_mdata_gpt_blk.c |   85 ----------------------------
 3 files changed, 71 insertions(+), 128 deletions(-)

diff --git a/include/fwu.h b/include/fwu.h
index 6c9b64a9f1..7af94988b7 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -14,26 +14,17 @@
 struct fwu_mdata;
 
 /**
- * @get_active_index: get the current active_index value
  * @get_image_alt_num: get the alt number to be used for the image
  * @mdata_check: check the validity of the FWU metadata partitions
- * @set_accept_image: set the accepted bit for the image
- * @clear_accept_image: clear the accepted bit for the image
  * @get_mdata() - Get a FWU metadata copy
  * @update_mdata() - Update the FWU metadata copy
  */
 struct fwu_mdata_ops {
-	int (*get_active_index)(u32 *active_idx);
-
 	int (*get_image_alt_num)(efi_guid_t image_type_id, u32 update_bank,
 				 int *alt_num);
 
 	int (*mdata_check)(void);
 
-	int (*set_accept_image)(efi_guid_t *img_type_id, u32 bank);
-
-	int (*clear_accept_image)(efi_guid_t *img_type_id, u32 bank);
-
 	int (*get_mdata)(struct fwu_mdata **mdata);
 
 	int (*update_mdata)(struct fwu_mdata *mdata);
diff --git a/lib/fwu_updates/fwu_mdata.c b/lib/fwu_updates/fwu_mdata.c
index 1324771a71..505f4f3630 100644
--- a/lib/fwu_updates/fwu_mdata.c
+++ b/lib/fwu_updates/fwu_mdata.c
@@ -3,6 +3,7 @@
  * Copyright (c) 2021, Linaro Limited
  */
 
+#include <efi_loader.h>
 #include <fwu.h>
 #include <fwu_mdata.h>
 #include <log.h>
@@ -12,6 +13,9 @@
 #include <linux/types.h>
 #include <u-boot/crc.h>
 
+#define IMAGE_ACCEPT_SET	BIT(0)
+#define IMAGE_ACCEPT_CLEAR	BIT(1)
+
 static struct fwu_mdata_ops *get_fwu_mdata_ops(void)
 {
 	struct fwu_mdata_ops *ops;
@@ -66,18 +70,28 @@ int fwu_verify_mdata(struct fwu_mdata *mdata, bool pri_part)
  */
 int fwu_get_active_index(u32 *active_idx)
 {
-	struct fwu_mdata_ops *ops;
+	int ret;
+	struct fwu_mdata *mdata = NULL;
 
-	ops = get_fwu_mdata_ops();
-	if (!ops)
-		return -EPROTONOSUPPORT;
+	ret = fwu_get_mdata(&mdata);
+	if (ret < 0) {
+		log_err("Unable to get valid FWU metadata\n");
+		return ret;
+	}
 
-	if (!ops->get_active_index) {
-		log_err("get_active_index() method not defined for the platform\n");
-		return -ENOSYS;
+	/*
+	 * Found the FWU metadata partition, now read the active_index
+	 * value
+	 */
+	*active_idx = mdata->active_index;
+	if (*active_idx > CONFIG_FWU_NUM_BANKS - 1) {
+		printf("Active index value read is incorrect\n");
+		ret = -EINVAL;
 	}
 
-	return ops->get_active_index(active_idx);
+	free(mdata);
+
+	return ret;
 }
 
 /**
@@ -197,12 +211,12 @@ int fwu_revert_boot_index(void)
 {
 	int ret;
 	u32 cur_active_index;
-	struct fwu_mdata *mdata = NULL;
+	struct fwu_mdata *mdata;
 
 	ret = fwu_get_mdata(&mdata);
 	if (ret < 0) {
 		log_err("Unable to get valid FWU metadata\n");
-		goto out;
+		return ret;
 	}
 
 	/*
@@ -223,6 +237,49 @@ int fwu_revert_boot_index(void)
 		ret = -EIO;
 	}
 
+	free(mdata);
+
+	return ret;
+}
+
+static int fwu_set_clear_image_accept(efi_guid_t *img_type_id,
+				      u32 bank, u8 action)
+{
+	void *buf;
+	int ret, i;
+	u32 nimages;
+	struct fwu_mdata *mdata = NULL;
+	struct fwu_image_entry *img_entry;
+	struct fwu_image_bank_info *img_bank_info;
+
+	ret = fwu_get_mdata(&mdata);
+	if (ret < 0) {
+		log_err("Unable to get valid FWU metadata\n");
+		return ret;
+	}
+
+	nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
+	img_entry = &mdata->img_entry[0];
+	for (i = 0; i < nimages; i++) {
+		if (!guidcmp(&img_entry[i].image_type_uuid, img_type_id)) {
+			img_bank_info = &img_entry[i].img_bank_info[bank];
+			if (action == IMAGE_ACCEPT_SET)
+				img_bank_info->accepted |= FWU_IMAGE_ACCEPTED;
+			else
+				img_bank_info->accepted = 0;
+
+			buf = &mdata->version;
+			mdata->crc32 = crc32(0, buf, sizeof(*mdata) -
+					     sizeof(u32));
+
+			ret = fwu_update_mdata(mdata);
+			goto out;
+		}
+	}
+
+	/* Image not found */
+	ret = -EINVAL;
+
 out:
 	free(mdata);
 
@@ -244,18 +301,8 @@ out:
  */
 int fwu_accept_image(efi_guid_t *img_type_id, u32 bank)
 {
-	struct fwu_mdata_ops *ops;
-
-	ops = get_fwu_mdata_ops();
-	if (!ops)
-		return -EPROTONOSUPPORT;
-
-	if (!ops->set_accept_image) {
-		log_err("set_accept_image() method not defined for the platform\n");
-		return -ENOSYS;
-	}
-
-	return ops->set_accept_image(img_type_id, bank);
+	return fwu_set_clear_image_accept(img_type_id, bank,
+					  IMAGE_ACCEPT_SET);
 }
 
 /**
@@ -274,18 +321,8 @@ int fwu_accept_image(efi_guid_t *img_type_id, u32 bank)
  */
 int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank)
 {
-	struct fwu_mdata_ops *ops;
-
-	ops = get_fwu_mdata_ops();
-	if (!ops)
-		return -EPROTONOSUPPORT;
-
-	if (!ops->clear_accept_image) {
-		log_err("clear_accept_image() method not defined for the platform\n");
-		return -ENOSYS;
-	}
-
-	return ops->clear_accept_image(img_type_id, bank);
+	return fwu_set_clear_image_accept(img_type_id, bank,
+					  IMAGE_ACCEPT_CLEAR);
 }
 
 /**
diff --git a/lib/fwu_updates/fwu_mdata_gpt_blk.c b/lib/fwu_updates/fwu_mdata_gpt_blk.c
index 7f8b8b68fc..af9b1adf94 100644
--- a/lib/fwu_updates/fwu_mdata_gpt_blk.c
+++ b/lib/fwu_updates/fwu_mdata_gpt_blk.c
@@ -24,9 +24,6 @@
 #define MDATA_READ		BIT(0)
 #define MDATA_WRITE		BIT(1)
 
-#define IMAGE_ACCEPT_SET	BIT(0)
-#define IMAGE_ACCEPT_CLEAR	BIT(1)
-
 static int gpt_get_mdata_partitions(struct blk_desc *desc,
 				    u16 *primary_mpart,
 				    u16 *secondary_mpart)
@@ -335,34 +332,6 @@ out:
 	return ret;
 }
 
-int fwu_gpt_get_active_index(u32 *active_idx)
-{
-	int ret;
-	struct fwu_mdata *mdata = NULL;
-
-	ret = gpt_get_mdata(&mdata);
-	if (ret < 0) {
-		log_err("Unable to get valid FWU metadata\n");
-		goto out;
-	}
-
-	/*
-	 * Found the FWU metadata partition, now read the active_index
-	 * value
-	 */
-	*active_idx = mdata->active_index;
-	if (*active_idx > CONFIG_FWU_NUM_BANKS - 1) {
-		printf("Active index value read is incorrect\n");
-		ret = -EINVAL;
-		goto out;
-	}
-
-out:
-	free(mdata);
-
-	return ret;
-}
-
 static int gpt_get_image_alt_num(struct blk_desc *desc,
 				 efi_guid_t image_type_id,
 				 u32 update_bank, int *alt_no)
@@ -467,63 +436,9 @@ int fwu_gpt_get_mdata(struct fwu_mdata **mdata)
 	return gpt_get_mdata(mdata);
 }
 
-static int fwu_gpt_set_clear_image_accept(efi_guid_t *img_type_id,
-					  u32 bank, u8 action)
-{
-	int ret, i;
-	u32 nimages;
-	struct fwu_mdata *mdata = NULL;
-	struct fwu_image_entry *img_entry;
-	struct fwu_image_bank_info *img_bank_info;
-
-	ret = gpt_get_mdata(&mdata);
-	if (ret < 0) {
-		log_err("Unable to get valid FWU metadata\n");
-		goto out;
-	}
-
-	nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
-	img_entry = &mdata->img_entry[0];
-	for (i = 0; i < nimages; i++) {
-		if (!guidcmp(&img_entry[i].image_type_uuid, img_type_id)) {
-			img_bank_info = &img_entry[i].img_bank_info[bank];
-			if (action == IMAGE_ACCEPT_SET)
-				img_bank_info->accepted |= FWU_IMAGE_ACCEPTED;
-			else
-				img_bank_info->accepted = 0;
-
-			ret = fwu_gpt_update_mdata(mdata);
-			goto out;
-		}
-	}
-
-	/* Image not found */
-	ret = -EINVAL;
-
-out:
-	free(mdata);
-
-	return ret;
-}
-
-static int fwu_gpt_accept_image(efi_guid_t *img_type_id, u32 bank)
-{
-	return fwu_gpt_set_clear_image_accept(img_type_id, bank,
-					      IMAGE_ACCEPT_SET);
-}
-
-static int fwu_gpt_clear_accept_image(efi_guid_t *img_type_id, u32 bank)
-{
-	return fwu_gpt_set_clear_image_accept(img_type_id, bank,
-					      IMAGE_ACCEPT_CLEAR);
-}
-
 struct fwu_mdata_ops fwu_gpt_blk_ops = {
-	.get_active_index = fwu_gpt_get_active_index,
 	.get_image_alt_num = fwu_gpt_get_image_alt_num,
 	.mdata_check = fwu_gpt_mdata_check,
-	.set_accept_image = fwu_gpt_accept_image,
-	.clear_accept_image = fwu_gpt_clear_accept_image,
 	.get_mdata = fwu_gpt_get_mdata,
 	.update_mdata = fwu_gpt_update_mdata,
 };

[RFC PATCH 09/14] synquacer: Update for TBBR based new FIP layout

[Masami Hiramatsu]

This changes SPI NOR flash partition layout for TBBR and
also make the U-Boot as position independent executable
again because BL33 is loaded on the memory.

With enabling TBBR, TF-A BL2 loads all BL3x images from
FIP image, and the U-Boot image is added to the FIP image
as BL33, and loaded to memory when boot instead of XIP
on SPI NOR flash. To avoid mixing up with the legacy images,
this new FIP image is stored on unused area (0x600000-) and
the U-Boot env vars are also stored at 0x580000 so that
it will not break existing EDK2 area.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 .../dts/synquacer-sc2a11-developerbox-u-boot.dtsi  |   26 ++++++++++++++------
 configs/synquacer_developerbox_defconfig           |    5 ++--
 include/configs/synquacer.h                        |    4 +--
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi b/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
index 7a56116d6f..095727e03c 100644
--- a/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
@@ -56,7 +56,7 @@
 				};
 
 				partition@180000 {
-					label = "FIP-TFA";
+					label = "LegacyFIP";
 					reg = <0x180000 0x78000>;
 				};
 
@@ -66,18 +66,28 @@
 				};
 
 				partition@200000 {
-					label = "U-Boot";
-					reg = <0x200000 0x100000>;
+					label = "EDK2";
+					reg = <0x200000 0x200000>;
 				};
 
-				partition@300000 {
-					label = "UBoot-Env";
-					reg = <0x300000 0x100000>;
+				partition@400000 {
+					label = "EDK2-Env";
+					reg = <0x400000 0x100000>;
 				};
 
 				partition@500000 {
-					label = "Ex-OPTEE";
-					reg = <0x500000 0x200000>;
+					label = "Metadata";
+					reg = <0x500000 0x80000>;
+				};
+
+				partition@580000 {
+					label = "UBoot-Env";
+					reg = <0x580000 0x80000>;
+				};
+
+				partition@600000 {
+					label = "FIP";
+					reg = <0x600000 0x400000>;
 				};
 			};
 		};
diff --git a/configs/synquacer_developerbox_defconfig b/configs/synquacer_developerbox_defconfig
index da57dc288f..484f6b7336 100644
--- a/configs/synquacer_developerbox_defconfig
+++ b/configs/synquacer_developerbox_defconfig
@@ -1,9 +1,10 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SYNQUACER=y
-CONFIG_SYS_TEXT_BASE=0x08200000
+CONFIG_POSITION_INDEPENDENT=y
+CONFIG_SYS_TEXT_BASE=0
 CONFIG_SYS_MALLOC_LEN=0x1000000
 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"
diff --git a/include/configs/synquacer.h b/include/configs/synquacer.h
index 0409589152..6d67bd2af5 100644
--- a/include/configs/synquacer.h
+++ b/include/configs/synquacer.h
@@ -48,9 +48,7 @@
 /* Since U-Boot 64bit PCIe support is limited, disable 64bit MMIO support */
 
 #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"
+			"mtd nor1=fip.bin raw 600000 400000\0"
 
 /* Distro boot settings */
 #ifndef CONFIG_SPL_BUILD

[RFC PATCH 10/14] FWU: Reboot soon after successfully install the new firmware

[Masami Hiramatsu]

Reboot to the trial state soon after successfully installing
the new firmware to the next bank and updating the active_index.
This is enabled by CONFIG_FWU_REBOOT_AFTER_UPDATE and is a
recommended option.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 lib/efi_loader/efi_capsule.c |   10 ++++++++--
 lib/fwu_updates/Kconfig      |    9 +++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 83c89a0cbb..0928425b5f 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -1355,10 +1355,16 @@ efi_status_t efi_launch_capsules(void)
 			} else {
 				log_debug("Successfully updated the active_index\n");
 				status = fwu_trial_state_ctr_start();
-				if (status < 0)
+				if (status < 0) {
 					ret = EFI_DEVICE_ERROR;
-				else
+				} else {
 					ret = EFI_SUCCESS;
+					if (IS_ENABLED(CONFIG_FWU_REBOOT_AFTER_UPDATE)) {
+						log_info("New firmware is installed in bank#%d. Reboot from that bank.\n",
+							 update_index);
+						do_reset(NULL, 0, 0, NULL);
+					}
+				}
 			}
 		} else if (capsule_update == true && update_status == false) {
 			log_err("All capsules were not updated. Not updating FWU metadata\n");
diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig
index 6de28e0c9c..0940a90747 100644
--- a/lib/fwu_updates/Kconfig
+++ b/lib/fwu_updates/Kconfig
@@ -29,3 +29,12 @@ config FWU_TRIAL_STATE_CNT
 	  With FWU Multi Bank Update feature enabled, number of times
 	  the platform is allowed to boot in Trial State after an
 	  update.
+
+config FWU_REBOOT_AFTER_UPDATE
+	bool "Reboot soon after installing new firmware"
+	depends on FWU_MULTI_BANK_UPDATE
+	default y
+	help
+	  Reboot the machine soon after installing a new firmware
+	  and start trial boot. You can disable this option for
+	  debugging or FWU development, but recommended to enable it.

[RFC PATCH 11/14] FWU: Add FWU Multi Bank Update on SPI Flash

[Masami Hiramatsu]

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 include/fwu.h                  |   13 ++
 lib/fwu_updates/Kconfig        |   34 ++++++
 lib/fwu_updates/Makefile       |    5 -
 lib/fwu_updates/fwu_mdata_sf.c |  241 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 288 insertions(+), 5 deletions(-)
 create mode 100644 lib/fwu_updates/fwu_mdata_sf.c

diff --git a/include/fwu.h b/include/fwu.h
index 7af94988b7..a013170321 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -64,9 +64,18 @@ int fwu_accept_image(efi_guid_t *img_type_id, u32 bank);
 int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank);
 
 int fwu_plat_get_update_index(u32 *update_idx);
-int fwu_plat_get_blk_desc(struct blk_desc **desc);
-int fwu_plat_get_alt_num(void *identifier);
 int fwu_plat_fill_partition_guids(efi_guid_t **part_guid_arr);
 void fwu_plat_get_bootidx(void *boot_idx);
 
+#ifdef CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK
+int fwu_plat_get_blk_desc(struct blk_desc **desc);
+int fwu_plat_get_alt_num(void *identifier);
+#endif
+
+#ifdef CONFIG_FWU_MULTI_BANK_UPDATE_SF
+struct fwu_mdata_ops *fwu_sf_get_fwu_mdata_ops(void);
+int fwu_plat_get_image_alt_num(efi_guid_t image_type_id, u32 update_bank,
+			       int *alt_no);
+#endif
+
 #endif /* _FWU_H_ */
diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig
index 0940a90747..8a369b9872 100644
--- a/lib/fwu_updates/Kconfig
+++ b/lib/fwu_updates/Kconfig
@@ -38,3 +38,37 @@ config FWU_REBOOT_AFTER_UPDATE
 	  Reboot the machine soon after installing a new firmware
 	  and start trial boot. You can disable this option for
 	  debugging or FWU development, but recommended to enable it.
+
+config FWU_MULTI_BANK_UPDATE_GPT_BLK
+	bool "Enable FWU Multi Bank Update for GPT on blockdev"
+	depends on FWU_MULTI_BANK_UPDATE
+	select EFI_PARTITION
+	help
+	  Enable FWU Multi Bank Update for GPT on a block device. This
+	  driver depends on GPT and the block device. FWU meatadata and
+	  firmware will be stored on a block device with GPT.
+
+config FWU_MULTI_BANK_UPDATE_SF
+	bool "Enable FWU Multi Bank Update for SPI Flash"
+	depends on FWU_MULTI_BANK_UPDATE
+	help
+	  Enable FWU Multi Bank Update for SPI flash driver. This
+	  driver does not depend on GPT. Instead, the platform must
+	  provide some APIs and define the offset of the primary and
+	  the secondary metadata.
+
+config FWU_SF_PRIMARY_MDATA_OFFSET
+	hex "The offset of the primary metadata"
+	depends on FWU_MULTI_BANK_UPDATE_SF
+	help
+	  The offset of the primary metadata on SPI Flash in
+	  hexadecimal.
+
+config FWU_SF_SECONDARY_MDATA_OFFSET
+	hex "The offset of the secondary metadata"
+	depends on FWU_MULTI_BANK_UPDATE_SF
+	help
+	  The offset of the secondary metadata on SPI Flash in
+	  hexadecimal.
+	  This must NOT be the same offset of the primary
+	  metadata.
diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
index 73099a30cb..80669344f2 100644
--- a/lib/fwu_updates/Makefile
+++ b/lib/fwu_updates/Makefile
@@ -6,6 +6,5 @@
 obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
 obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
 
-ifdef CONFIG_EFI_PARTITION
-obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
-endif
+obj-$(CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK) += fwu_mdata_gpt_blk.o
+obj-$(CONFIG_FWU_MULTI_BANK_UPDATE_SF) += fwu_mdata_sf.o
diff --git a/lib/fwu_updates/fwu_mdata_sf.c b/lib/fwu_updates/fwu_mdata_sf.c
new file mode 100644
index 0000000000..9e3cae7b68
--- /dev/null
+++ b/lib/fwu_updates/fwu_mdata_sf.c
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021, Linaro Limited
+ */
+
+#include <efi_loader.h>
+#include <fwu.h>
+#include <fwu_mdata.h>
+#include <malloc.h>
+#include <memalign.h>
+#include <spi.h>
+#include <spi_flash.h>
+#include <flash.h>
+
+#include <linux/errno.h>
+#include <linux/types.h>
+#include <u-boot/crc.h>
+
+/*
+ * For the FWU SPI flash driver, the platform must define below functions
+ * according to its dfu_alt_info.
+ */
+extern int fwu_plat_get_image_alt_num(efi_guid_t image_type_id,
+				      u32 update_bank, int *alt_no);
+
+static struct spi_flash *fwu_spi_flash;
+
+static int __fwu_sf_get_flash(void)
+{
+	if (IS_ENABLED(CONFIG_DM_SPI_FLASH)) {
+		struct udevice *new;
+		int	ret;
+
+		ret = spi_flash_probe_bus_cs(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
+					     CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE,
+					     &new);
+		if (ret)
+			return ret;
+
+		fwu_spi_flash = dev_get_uclass_priv(new);
+	} else {
+		fwu_spi_flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
+						CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE);
+		if (!fwu_spi_flash)
+			return -EIO;
+	}
+	return 0;
+}
+
+static int fwu_sf_get_flash(struct spi_flash **flash)
+{
+	int ret = 0;
+
+	if (!fwu_spi_flash)
+		ret = __fwu_sf_get_flash();
+
+	*flash = fwu_spi_flash;
+
+	return ret;
+}
+
+static int sf_load_data(u32 offs, u32 size, void **data)
+{
+	struct spi_flash *flash;
+	int ret;
+
+	ret = fwu_sf_get_flash(&flash);
+	if (ret < 0)
+		return ret;
+
+	*data = memalign(ARCH_DMA_MINALIGN, size);
+	if (!*data)
+		return -ENOMEM;
+
+	ret = spi_flash_read(flash, offs, size, *data);
+	if (ret < 0) {
+		free(*data);
+		*data = NULL;
+	}
+
+	return ret;
+}
+
+static int sf_save_data(u32 offs, u32 size, void *data)
+{
+	struct spi_flash *flash;
+	u32 sect_size, nsect;
+	void *buf;
+	int ret;
+
+	ret = fwu_sf_get_flash(&flash);
+	if (ret < 0)
+		return ret;
+
+	sect_size = flash->mtd.erasesize;
+	nsect = DIV_ROUND_UP(size, sect_size);
+	ret = spi_flash_erase(flash, offs, nsect * sect_size);
+	if (ret < 0)
+		return ret;
+
+	buf = memalign(ARCH_DMA_MINALIGN, size);
+	if (!buf)
+		return -ENOMEM;
+	memcpy(buf, data, size);
+
+	ret = spi_flash_write(flash, offs, size, buf);
+
+	free(buf);
+
+	return ret;
+}
+
+static int fwu_sf_load_mdata(struct fwu_mdata **mdata, u32 offs)
+{
+	int ret;
+
+	ret = sf_load_data(offs, sizeof(struct fwu_mdata), (void **)mdata);
+
+	if (ret >= 0) {
+		ret = fwu_verify_mdata(*mdata,
+				offs == CONFIG_FWU_SF_PRIMARY_MDATA_OFFSET);
+		if (ret < 0) {
+			free(*mdata);
+			*mdata = NULL;
+		}
+	}
+
+	return ret;
+}
+
+static int fwu_sf_load_primary_mdata(struct fwu_mdata **mdata)
+{
+	return fwu_sf_load_mdata(mdata, CONFIG_FWU_SF_PRIMARY_MDATA_OFFSET);
+}
+
+static int fwu_sf_load_secondary_mdata(struct fwu_mdata **mdata)
+{
+	return fwu_sf_load_mdata(mdata, CONFIG_FWU_SF_SECONDARY_MDATA_OFFSET);
+}
+
+static int fwu_sf_save_primary_mdata(struct fwu_mdata *mdata)
+{
+	return sf_save_data(CONFIG_FWU_SF_PRIMARY_MDATA_OFFSET,
+			    sizeof(struct fwu_mdata), mdata);
+}
+
+static int fwu_sf_save_secondary_mdata(struct fwu_mdata *mdata)
+{
+	return sf_save_data(CONFIG_FWU_SF_SECONDARY_MDATA_OFFSET,
+			    sizeof(struct fwu_mdata), mdata);
+}
+
+static int fwu_sf_get_valid_mdata(struct fwu_mdata **mdata)
+{
+	if (fwu_sf_load_primary_mdata(mdata) == 0)
+		return 0;
+
+	log_err("Failed to load/verify primary mdata. Try secondary.\n");
+
+	if (fwu_sf_load_secondary_mdata(mdata) == 0)
+		return 0;
+
+	log_err("Failed to load/verify secondary mdata.\n");
+
+	return -1;
+}
+
+static int fwu_sf_update_mdata(struct fwu_mdata *mdata)
+{
+	int ret;
+
+	/* Update mdata crc32 field */
+	mdata->crc32 = crc32(0, (void *)&mdata->version,
+			     sizeof(*mdata) - sizeof(u32));
+
+	/* First write the primary mdata */
+	ret = fwu_sf_save_primary_mdata(mdata);
+	if (ret < 0) {
+		log_err("Failed to update the primary mdata.\n");
+		return ret;
+	}
+
+	/* And now the replica */
+	ret = fwu_sf_save_secondary_mdata(mdata);
+	if (ret < 0) {
+		log_err("Failed to update the secondary mdata.\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int fwu_sf_mdata_check(void)
+{
+	struct fwu_mdata *primary = NULL, *secondary = NULL;
+	int ret;
+
+	ret = fwu_sf_load_primary_mdata(&primary);
+	if (ret < 0)
+		log_err("Failed to read the primary mdata: %d\n", ret);
+
+	ret = fwu_sf_load_secondary_mdata(&secondary);
+	if (ret < 0)
+		log_err("Failed to read the secondary mdata: %d\n", ret);
+
+	if (primary && secondary) {
+		if (memcmp(primary, secondary, sizeof(struct fwu_mdata))) {
+			log_err("The primary and the secondary mdata are different\n");
+			ret = -1;
+		}
+	} else if (primary) {
+		ret = fwu_sf_save_secondary_mdata(primary);
+		if (ret < 0)
+			log_err("Restoring secondary mdata partition failed\n");
+	} else if (secondary) {
+		ret = fwu_sf_save_primary_mdata(secondary);
+		if (ret < 0)
+			log_err("Restoring primary mdata partition failed\n");
+	}
+
+	free(primary);
+	free(secondary);
+	return ret;
+}
+
+static int fwu_sf_get_mdata(struct fwu_mdata **mdata)
+{
+	return fwu_sf_get_valid_mdata(mdata);
+}
+
+static struct fwu_mdata_ops fwu_sf_mdata_ops = {
+	.get_image_alt_num = fwu_plat_get_image_alt_num,
+	.mdata_check = fwu_sf_mdata_check,
+	.get_mdata = fwu_sf_get_mdata,
+	.update_mdata = fwu_sf_update_mdata,
+};
+
+struct fwu_mdata_ops *fwu_sf_get_fwu_mdata_ops(void)
+{
+	return &fwu_sf_mdata_ops;
+}

[RFC PATCH 12/14] FWU: synquacer: Add FWU Multi bank update support for DeveloperBox

[Masami Hiramatsu]

The DeveloperBox platform can support the FWU Multi bank
update. SCP firmware will switch the boot mode by DSW3-4
and load the Multi bank update supported TF-A BL2 from
0x600000 offset on the SPI flash. Thus it can co-exist
with the legacy boot mode (legacy U-Boot or EDK2).

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 board/socionext/developerbox/Kconfig    |   19 ++++
 board/socionext/developerbox/Makefile   |    1 
 board/socionext/developerbox/fwu_plat.c |  158 +++++++++++++++++++++++++++++++
 include/configs/synquacer.h             |   10 ++
 include/efi_loader.h                    |    3 +
 lib/efi_loader/efi_firmware.c           |   14 +--
 6 files changed, 198 insertions(+), 7 deletions(-)
 create mode 100644 board/socionext/developerbox/fwu_plat.c

diff --git a/board/socionext/developerbox/Kconfig b/board/socionext/developerbox/Kconfig
index c181d26a44..4e2c341aad 100644
--- a/board/socionext/developerbox/Kconfig
+++ b/board/socionext/developerbox/Kconfig
@@ -32,4 +32,23 @@ config SYS_CONFIG_NAME
 	default "synquacer"
 
 endif
+
+config FWU_MULTI_BANK_UPDATE
+	select FWU_MULTI_BANK_UPDATE_SF
+
+config FWU_MULTI_BANK_UPDATE_SF
+	select DM_SPI_FLASH
+
+config FWU_NUM_BANKS
+	default 6
+
+config FWU_NUM_IMAGES_PER_BANK
+	default 1
+
+config FWU_SF_PRIMARY_MDATA_OFFSET
+	default 0x500000
+
+config FWU_SF_SECONDARY_MDATA_OFFSET
+	default 0x520000
+
 endif
diff --git a/board/socionext/developerbox/Makefile b/board/socionext/developerbox/Makefile
index 4a46de995a..15cce9c57e 100644
--- a/board/socionext/developerbox/Makefile
+++ b/board/socionext/developerbox/Makefile
@@ -7,3 +7,4 @@
 #
 
 obj-y	:= developerbox.o
+obj-$(CONFIG_FWU_MULTI_BANK_UPDATE_SF) += fwu_plat.o
diff --git a/board/socionext/developerbox/fwu_plat.c b/board/socionext/developerbox/fwu_plat.c
new file mode 100644
index 0000000000..dbb814f1fd
--- /dev/null
+++ b/board/socionext/developerbox/fwu_plat.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021, Linaro Limited
+ */
+
+#include <efi_loader.h>
+#include <fwu.h>
+#include <fwu_mdata.h>
+#include <malloc.h>
+#include <memalign.h>
+#include <spi.h>
+#include <spi_flash.h>
+#include <flash.h>
+
+#include <linux/errno.h>
+#include <linux/types.h>
+#include <u-boot/crc.h>
+
+/* SPI Flash accessors */
+static struct spi_flash *plat_spi_flash;
+
+static int __plat_sf_get_flash(void)
+{
+	struct udevice *new;
+	int	ret;
+
+	//TODO: use CONFIG_FWU_SPI_*
+	ret = spi_flash_probe_bus_cs(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
+				     CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE,
+				     &new);
+	if (ret)
+		return ret;
+
+	plat_spi_flash = dev_get_uclass_priv(new);
+	return 0;
+}
+
+static int plat_sf_get_flash(struct spi_flash **flash)
+{
+	int ret = 0;
+
+	if (!plat_spi_flash)
+		ret = __plat_sf_get_flash();
+
+	*flash = plat_spi_flash;
+
+	return ret;
+}
+
+static int sf_load_data(u32 offs, u32 size, void **data)
+{
+	struct spi_flash *flash;
+	int ret;
+
+	ret = plat_sf_get_flash(&flash);
+	if (ret < 0)
+		return ret;
+
+	*data = memalign(ARCH_DMA_MINALIGN, size);
+	if (!*data)
+		return -ENOMEM;
+
+	ret = spi_flash_read(flash, offs, size, *data);
+	if (ret < 0) {
+		free(*data);
+		*data = NULL;
+	}
+
+	return ret;
+}
+
+/* Platform dependent GUIDs */
+
+/* The GUID for the SNI FIP image type GUID */
+#define FWU_IMAGE_TYPE_DEVBOX_FIP_GUID \
+	EFI_GUID(0x7d6dc310, 0x52ca, 0x43b8, 0xb7, 0xb9, \
+		 0xf9, 0xd6, 0xc5, 0x01, 0xd1, 0x08)
+
+#define PLAT_METADATA_OFFSET	0x510000
+#define PLAT_METADATA_SIZE	(sizeof(struct devbox_metadata))
+
+struct __packed devbox_metadata {
+	u32 boot_index;
+	u32 boot_count;
+	u32 invert_dual;
+} *devbox_plat_metadata;
+
+static const efi_guid_t devbox_fip_image_type_guid = FWU_IMAGE_TYPE_DEVBOX_FIP_GUID;
+
+int fwu_plat_get_image_alt_num(efi_guid_t image_type_id, u32 update_bank,
+			       int *alt_no)
+{
+	/* DeveloperBox FWU Multi bank only supports FIP image. */
+	if (guidcmp(&image_type_id, &devbox_fip_image_type_guid))
+		return -EOPNOTSUPP;
+
+	/*
+	 * DeveloperBox FWU expects Bank:Image = 1:1, and the dfu_alt_info
+	 * only has the entries for banks. Thus the alt_no should be equal
+	 * to the update_bank.
+	 */
+	update_bank %= CONFIG_FWU_NUM_BANKS;
+	*alt_no = update_bank;
+
+	return 0;
+}
+
+/* SPI flash doesn't have GPT, and it's not blk device */
+int fwu_plat_fill_partition_guids(efi_guid_t **part_guid_arr)
+{
+	efi_status_t ret;
+
+	free(*part_guid_arr);
+
+	ret = efi_fill_part_guid_array(&devbox_fip_image_type_guid, part_guid_arr);
+	return (ret != EFI_SUCCESS) ? -ENOMEM : 0;
+}
+
+/* TBD: add a usage counter for wear leveling */
+int fwu_plat_get_update_index(u32 *update_idx)
+{
+	int ret;
+	u32 active_idx;
+
+	ret = fwu_get_active_index(&active_idx);
+
+	if (ret < 0)
+		return -1;
+
+	*update_idx = (active_idx + 1) % CONFIG_FWU_NUM_BANKS;
+
+	return ret;
+}
+
+static int devbox_load_plat_metadata(void)
+{
+	if (devbox_plat_metadata)
+		return 0;
+
+	return sf_load_data(PLAT_METADATA_OFFSET, PLAT_METADATA_SIZE,
+			 (void **)&devbox_plat_metadata);
+}
+
+void fwu_plat_get_bootidx(void *boot_idx)
+{
+	u32 *bootidx = boot_idx;
+
+	if (devbox_load_plat_metadata() < 0)
+		*bootidx = 0;
+	else
+		*bootidx = devbox_plat_metadata->boot_index;
+}
+
+struct fwu_mdata_ops *get_plat_fwu_mdata_ops(void)
+{
+	return fwu_sf_get_fwu_mdata_ops();
+}
+
diff --git a/include/configs/synquacer.h b/include/configs/synquacer.h
index 6d67bd2af5..f859237550 100644
--- a/include/configs/synquacer.h
+++ b/include/configs/synquacer.h
@@ -47,8 +47,18 @@
 
 /* Since U-Boot 64bit PCIe support is limited, disable 64bit MMIO support */
 
+#ifdef CONFIG_FWU_MULTI_BANK_UPDATE
+#define DEFAULT_DFU_ALT_INFO "dfu_alt_info="				\
+				"mtd nor1=bank0 raw  600000 400000;"	\
+					 "bank1 raw  a00000 400000;"	\
+					 "bank2 raw  e00000 400000;"	\
+					 "bank3 raw 1200000 400000;"	\
+					 "bank4 raw 1600000 400000;"	\
+					 "bank5 raw 1a00000 400000\0"
+#else
 #define DEFAULT_DFU_ALT_INFO "dfu_alt_info="				\
 			"mtd nor1=fip.bin raw 600000 400000\0"
+#endif
 
 /* Distro boot settings */
 #ifndef CONFIG_SPL_BUILD
diff --git a/include/efi_loader.h b/include/efi_loader.h
index f20d361876..d6dc173feb 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -953,6 +953,9 @@ void efi_memcpy_runtime(void *dest, const void *src, size_t n);
 u16 *efi_create_indexed_name(u16 *buffer, size_t buffer_size, const char *name,
 			     unsigned int index);
 
+efi_status_t efi_fill_part_guid_array(const efi_guid_t *guid,
+				      efi_guid_t **part_guid_arr);
+
 extern const struct efi_firmware_management_protocol efi_fmp_fit;
 extern const struct efi_firmware_management_protocol efi_fmp_raw;
 
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 107f0cb074..c100be35d3 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -97,8 +97,8 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
 	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
-static efi_status_t fill_part_guid_array(const efi_guid_t *guid,
-					 efi_guid_t **part_guid_arr)
+efi_status_t efi_fill_part_guid_array(const efi_guid_t *guid,
+				      efi_guid_t **part_guid_arr)
 {
 	int i;
 	int dfu_num = 0;
@@ -309,8 +309,8 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info(
 	     !descriptor_size || !package_version || !package_version_name))
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
-	ret = fill_part_guid_array(&efi_firmware_image_type_uboot_fit,
-				   &part_guid_arr);
+	ret = efi_fill_part_guid_array(&efi_firmware_image_type_uboot_fit,
+				       &part_guid_arr);
 	if (ret != EFI_SUCCESS)
 		goto out;
 
@@ -429,7 +429,7 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info(
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
 	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
-		ret = fill_part_guid_array(&null_guid, &part_guid_arr);
+		ret = efi_fill_part_guid_array(&null_guid, &part_guid_arr);
 		if (ret != EFI_SUCCESS)
 			goto out;
 
@@ -444,8 +444,8 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info(
 			goto out;
 		}
 	} else {
-		ret = fill_part_guid_array(&efi_firmware_image_type_uboot_raw,
-					   &part_guid_arr);
+		ret = efi_fill_part_guid_array(&efi_firmware_image_type_uboot_raw,
+					       &part_guid_arr);
 		if (ret != EFI_SUCCESS)
 			goto out;
 	}

[RFC PATCH 13/14] FWU: synquacer: Initialize broken metadata

[Masami Hiramatsu]

Since the FWU metadata is not initialized at the installation,
if it is broken, it should be initialized. Usually, the FWU
metadata is not covered by capsule update, so it is safe to
initialize the metadata portion if it seems broken.

But for the production device, usually firmware will be installed
with initialized metadata, and the broken metadata means the
device can be compromized. In that case, build U-Boot without
this option.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 board/socionext/developerbox/Kconfig    |   12 ++++++
 board/socionext/developerbox/fwu_plat.c |   59 +++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/board/socionext/developerbox/Kconfig b/board/socionext/developerbox/Kconfig
index 4e2c341aad..1b36e10f3b 100644
--- a/board/socionext/developerbox/Kconfig
+++ b/board/socionext/developerbox/Kconfig
@@ -51,4 +51,16 @@ config FWU_SF_PRIMARY_MDATA_OFFSET
 config FWU_SF_SECONDARY_MDATA_OFFSET
 	default 0x520000
 
+config FWU_INIT_BROKEN_METADATA
+	bool "Initialize FWU metadata if broken"
+	select BOARD_LATE_INIT
+	default n
+	help
+	  Initialize FWU metadata if the metadata is broken.
+	  This option is only for the development environment, since if the
+	  metadata is broken, it means someone may compromize it. In that case
+	  the production device must be bricked.
+	  But for the development environment, or initial installation of the
+	  FWU multi-bank update firmware, this will be useful.
+
 endif
diff --git a/board/socionext/developerbox/fwu_plat.c b/board/socionext/developerbox/fwu_plat.c
index dbb814f1fd..2982e47a16 100644
--- a/board/socionext/developerbox/fwu_plat.c
+++ b/board/socionext/developerbox/fwu_plat.c
@@ -156,3 +156,62 @@ struct fwu_mdata_ops *get_plat_fwu_mdata_ops(void)
 	return fwu_sf_get_fwu_mdata_ops();
 }
 
+#ifdef CONFIG_FWU_INIT_BROKEN_METADATA
+
+static void devbox_init_fwu_mdata(void)
+{
+	const efi_guid_t null_guid = NULL_GUID;
+	struct fwu_image_bank_info *bank;
+	struct fwu_mdata *metadata;
+	int i, j, ret;
+
+	metadata = memalign(ARCH_DMA_MINALIGN, sizeof(*metadata));
+	if (!metadata) {
+		log_err("Failed to allocate initial metadata.\n");
+		return;
+	}
+
+	metadata->version = 1;
+	metadata->active_index = 0;
+	metadata->previous_active_index = 0;
+
+	/*
+	 * Since the DeveloperBox doesn't use GPT, both of
+	 * fwu_image_entry::location_uuid and
+	 * fwu_img_bank_info::image_uuid are null GUID.
+	 */
+	for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
+		guidcpy(&metadata->img_entry[i].image_type_uuid,
+			&devbox_fip_image_type_guid);
+		guidcpy(&metadata->img_entry[i].location_uuid,
+			&null_guid);
+		bank = metadata->img_entry[i].img_bank_info;
+
+		for (j = 0; j < CONFIG_FWU_NUM_BANKS; j++) {
+			guidcpy(&bank[j].image_uuid, &null_guid);
+			bank[j].accepted = (j == 0) ? 1 : 0;
+			bank[j].reserved = 0;
+		}
+	}
+
+	ret = fwu_update_mdata(metadata);
+	if (ret < 0)
+		log_err("Failed to initialize FWU metadata\n");
+	else
+		log_err("Initialized FWU metadata\n");
+	free(metadata);
+}
+
+int board_late_init(void)
+{
+	struct fwu_mdata *metadata;
+
+	if (fwu_get_mdata(&metadata) < 0) {
+		// Initialize FWU metadata if broken
+		log_err("Unable to get a valid metadata. Initialize it.\n");
+		devbox_init_fwu_mdata();
+	}
+	return 0;
+}
+
+#endif

[RFC PATCH 14/14] configs: synquacer: Add FWU support for DeveloperBox

[Masami Hiramatsu]

Enable FWU Multi-Bank support for DeveloperBox SynQuacer platform.
This also enables fwu_metadata_read command and "reboot soon after
update" option.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 configs/synquacer_developerbox_defconfig |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/configs/synquacer_developerbox_defconfig b/configs/synquacer_developerbox_defconfig
index 484f6b7336..02d87bc403 100644
--- a/configs/synquacer_developerbox_defconfig
+++ b/configs/synquacer_developerbox_defconfig
@@ -91,3 +91,10 @@ 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_MULTI_BANK_UPDATE_SF=y
+CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK=n
+CONFIG_CMD_FWU_METADATA=y
+CONFIG_FWU_REBOOT_AFTER_UPDATE=y
+CONFIG_FWU_INIT_BROKEN_METADATA=y

Re: [RFC PATCH 10/14] FWU: Reboot soon after successfully install the new firmware

[AKASHI Takahiro]

On Fri, Jan 21, 2022 at 12:31:00AM +0900, Masami Hiramatsu wrote:
> Reboot to the trial state soon after successfully installing
> the new firmware to the next bank and updating the active_index.
> This is enabled by CONFIG_FWU_REBOOT_AFTER_UPDATE and is a
> recommended option.

EFI_CAPSULE_HEADER.Flags may have a flag, CAPSULE_FLAGS_INITIATE_RESET.
See Section "8.5.3 Update Capsule" in the UEFI specification.

I think that we'd better implement the feature rather than adding
CONFIG_FWU_REBOOT_AFTER_UPDATE.

-Takahiro Akashi

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  lib/efi_loader/efi_capsule.c |   10 ++++++++--
>  lib/fwu_updates/Kconfig      |    9 +++++++++
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 83c89a0cbb..0928425b5f 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -1355,10 +1355,16 @@ efi_status_t efi_launch_capsules(void)
>  			} else {
>  				log_debug("Successfully updated the active_index\n");
>  				status = fwu_trial_state_ctr_start();
> -				if (status < 0)
> +				if (status < 0) {
>  					ret = EFI_DEVICE_ERROR;
> -				else
> +				} else {
>  					ret = EFI_SUCCESS;
> +					if (IS_ENABLED(CONFIG_FWU_REBOOT_AFTER_UPDATE)) {
> +						log_info("New firmware is installed in bank#%d. Reboot from that bank.\n",
> +							 update_index);
> +						do_reset(NULL, 0, 0, NULL);
> +					}
> +				}
>  			}
>  		} else if (capsule_update == true && update_status == false) {
>  			log_err("All capsules were not updated. Not updating FWU metadata\n");
> diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig
> index 6de28e0c9c..0940a90747 100644
> --- a/lib/fwu_updates/Kconfig
> +++ b/lib/fwu_updates/Kconfig
> @@ -29,3 +29,12 @@ config FWU_TRIAL_STATE_CNT
>  	  With FWU Multi Bank Update feature enabled, number of times
>  	  the platform is allowed to boot in Trial State after an
>  	  update.
> +
> +config FWU_REBOOT_AFTER_UPDATE
> +	bool "Reboot soon after installing new firmware"
> +	depends on FWU_MULTI_BANK_UPDATE
> +	default y
> +	help
> +	  Reboot the machine soon after installing a new firmware
> +	  and start trial boot. You can disable this option for
> +	  debugging or FWU development, but recommended to enable it.
> 

Re: [RFC PATCH 11/14] FWU: Add FWU Multi Bank Update on SPI Flash

[AKASHI Takahiro]

On Fri, Jan 21, 2022 at 12:31:10AM +0900, Masami Hiramatsu wrote:
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  include/fwu.h                  |   13 ++
>  lib/fwu_updates/Kconfig        |   34 ++++++
>  lib/fwu_updates/Makefile       |    5 -
>  lib/fwu_updates/fwu_mdata_sf.c |  241 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 288 insertions(+), 5 deletions(-)
>  create mode 100644 lib/fwu_updates/fwu_mdata_sf.c
> 
> diff --git a/include/fwu.h b/include/fwu.h
> index 7af94988b7..a013170321 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -64,9 +64,18 @@ int fwu_accept_image(efi_guid_t *img_type_id, u32 bank);
>  int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank);
>  
>  int fwu_plat_get_update_index(u32 *update_idx);
> -int fwu_plat_get_blk_desc(struct blk_desc **desc);
> -int fwu_plat_get_alt_num(void *identifier);
>  int fwu_plat_fill_partition_guids(efi_guid_t **part_guid_arr);
>  void fwu_plat_get_bootidx(void *boot_idx);
>  
> +#ifdef CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK
> +int fwu_plat_get_blk_desc(struct blk_desc **desc);
> +int fwu_plat_get_alt_num(void *identifier);
> +#endif

We can (and should) remove "#ifdef ..." from headers.

-Takahiro Akashi

> +#ifdef CONFIG_FWU_MULTI_BANK_UPDATE_SF
> +struct fwu_mdata_ops *fwu_sf_get_fwu_mdata_ops(void);
> +int fwu_plat_get_image_alt_num(efi_guid_t image_type_id, u32 update_bank,
> +			       int *alt_no);
> +#endif
> +
>  #endif /* _FWU_H_ */
> diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig
> index 0940a90747..8a369b9872 100644
> --- a/lib/fwu_updates/Kconfig
> +++ b/lib/fwu_updates/Kconfig
> @@ -38,3 +38,37 @@ config FWU_REBOOT_AFTER_UPDATE
>  	  Reboot the machine soon after installing a new firmware
>  	  and start trial boot. You can disable this option for
>  	  debugging or FWU development, but recommended to enable it.
> +
> +config FWU_MULTI_BANK_UPDATE_GPT_BLK
> +	bool "Enable FWU Multi Bank Update for GPT on blockdev"
> +	depends on FWU_MULTI_BANK_UPDATE
> +	select EFI_PARTITION
> +	help
> +	  Enable FWU Multi Bank Update for GPT on a block device. This
> +	  driver depends on GPT and the block device. FWU meatadata and
> +	  firmware will be stored on a block device with GPT.
> +
> +config FWU_MULTI_BANK_UPDATE_SF
> +	bool "Enable FWU Multi Bank Update for SPI Flash"
> +	depends on FWU_MULTI_BANK_UPDATE
> +	help
> +	  Enable FWU Multi Bank Update for SPI flash driver. This
> +	  driver does not depend on GPT. Instead, the platform must
> +	  provide some APIs and define the offset of the primary and
> +	  the secondary metadata.
> +
> +config FWU_SF_PRIMARY_MDATA_OFFSET
> +	hex "The offset of the primary metadata"
> +	depends on FWU_MULTI_BANK_UPDATE_SF
> +	help
> +	  The offset of the primary metadata on SPI Flash in
> +	  hexadecimal.
> +
> +config FWU_SF_SECONDARY_MDATA_OFFSET
> +	hex "The offset of the secondary metadata"
> +	depends on FWU_MULTI_BANK_UPDATE_SF
> +	help
> +	  The offset of the secondary metadata on SPI Flash in
> +	  hexadecimal.
> +	  This must NOT be the same offset of the primary
> +	  metadata.
> diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> index 73099a30cb..80669344f2 100644
> --- a/lib/fwu_updates/Makefile
> +++ b/lib/fwu_updates/Makefile
> @@ -6,6 +6,5 @@
>  obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
>  obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
>  
> -ifdef CONFIG_EFI_PARTITION
> -obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
> -endif
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK) += fwu_mdata_gpt_blk.o
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE_SF) += fwu_mdata_sf.o
> diff --git a/lib/fwu_updates/fwu_mdata_sf.c b/lib/fwu_updates/fwu_mdata_sf.c
> new file mode 100644
> index 0000000000..9e3cae7b68
> --- /dev/null
> +++ b/lib/fwu_updates/fwu_mdata_sf.c
> @@ -0,0 +1,241 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2021, Linaro Limited
> + */
> +
> +#include <efi_loader.h>
> +#include <fwu.h>
> +#include <fwu_mdata.h>
> +#include <malloc.h>
> +#include <memalign.h>
> +#include <spi.h>
> +#include <spi_flash.h>
> +#include <flash.h>
> +
> +#include <linux/errno.h>
> +#include <linux/types.h>
> +#include <u-boot/crc.h>
> +
> +/*
> + * For the FWU SPI flash driver, the platform must define below functions
> + * according to its dfu_alt_info.
> + */
> +extern int fwu_plat_get_image_alt_num(efi_guid_t image_type_id,
> +				      u32 update_bank, int *alt_no);
> +
> +static struct spi_flash *fwu_spi_flash;
> +
> +static int __fwu_sf_get_flash(void)
> +{
> +	if (IS_ENABLED(CONFIG_DM_SPI_FLASH)) {
> +		struct udevice *new;
> +		int	ret;
> +
> +		ret = spi_flash_probe_bus_cs(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
> +					     CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE,
> +					     &new);
> +		if (ret)
> +			return ret;
> +
> +		fwu_spi_flash = dev_get_uclass_priv(new);
> +	} else {
> +		fwu_spi_flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
> +						CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE);
> +		if (!fwu_spi_flash)
> +			return -EIO;
> +	}
> +	return 0;
> +}
> +
> +static int fwu_sf_get_flash(struct spi_flash **flash)
> +{
> +	int ret = 0;
> +
> +	if (!fwu_spi_flash)
> +		ret = __fwu_sf_get_flash();
> +
> +	*flash = fwu_spi_flash;
> +
> +	return ret;
> +}
> +
> +static int sf_load_data(u32 offs, u32 size, void **data)
> +{
> +	struct spi_flash *flash;
> +	int ret;
> +
> +	ret = fwu_sf_get_flash(&flash);
> +	if (ret < 0)
> +		return ret;
> +
> +	*data = memalign(ARCH_DMA_MINALIGN, size);
> +	if (!*data)
> +		return -ENOMEM;
> +
> +	ret = spi_flash_read(flash, offs, size, *data);
> +	if (ret < 0) {
> +		free(*data);
> +		*data = NULL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int sf_save_data(u32 offs, u32 size, void *data)
> +{
> +	struct spi_flash *flash;
> +	u32 sect_size, nsect;
> +	void *buf;
> +	int ret;
> +
> +	ret = fwu_sf_get_flash(&flash);
> +	if (ret < 0)
> +		return ret;
> +
> +	sect_size = flash->mtd.erasesize;
> +	nsect = DIV_ROUND_UP(size, sect_size);
> +	ret = spi_flash_erase(flash, offs, nsect * sect_size);
> +	if (ret < 0)
> +		return ret;
> +
> +	buf = memalign(ARCH_DMA_MINALIGN, size);
> +	if (!buf)
> +		return -ENOMEM;
> +	memcpy(buf, data, size);
> +
> +	ret = spi_flash_write(flash, offs, size, buf);
> +
> +	free(buf);
> +
> +	return ret;
> +}
> +
> +static int fwu_sf_load_mdata(struct fwu_mdata **mdata, u32 offs)
> +{
> +	int ret;
> +
> +	ret = sf_load_data(offs, sizeof(struct fwu_mdata), (void **)mdata);
> +
> +	if (ret >= 0) {
> +		ret = fwu_verify_mdata(*mdata,
> +				offs == CONFIG_FWU_SF_PRIMARY_MDATA_OFFSET);
> +		if (ret < 0) {
> +			free(*mdata);
> +			*mdata = NULL;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int fwu_sf_load_primary_mdata(struct fwu_mdata **mdata)
> +{
> +	return fwu_sf_load_mdata(mdata, CONFIG_FWU_SF_PRIMARY_MDATA_OFFSET);
> +}
> +
> +static int fwu_sf_load_secondary_mdata(struct fwu_mdata **mdata)
> +{
> +	return fwu_sf_load_mdata(mdata, CONFIG_FWU_SF_SECONDARY_MDATA_OFFSET);
> +}
> +
> +static int fwu_sf_save_primary_mdata(struct fwu_mdata *mdata)
> +{
> +	return sf_save_data(CONFIG_FWU_SF_PRIMARY_MDATA_OFFSET,
> +			    sizeof(struct fwu_mdata), mdata);
> +}
> +
> +static int fwu_sf_save_secondary_mdata(struct fwu_mdata *mdata)
> +{
> +	return sf_save_data(CONFIG_FWU_SF_SECONDARY_MDATA_OFFSET,
> +			    sizeof(struct fwu_mdata), mdata);
> +}
> +
> +static int fwu_sf_get_valid_mdata(struct fwu_mdata **mdata)
> +{
> +	if (fwu_sf_load_primary_mdata(mdata) == 0)
> +		return 0;
> +
> +	log_err("Failed to load/verify primary mdata. Try secondary.\n");
> +
> +	if (fwu_sf_load_secondary_mdata(mdata) == 0)
> +		return 0;
> +
> +	log_err("Failed to load/verify secondary mdata.\n");
> +
> +	return -1;
> +}
> +
> +static int fwu_sf_update_mdata(struct fwu_mdata *mdata)
> +{
> +	int ret;
> +
> +	/* Update mdata crc32 field */
> +	mdata->crc32 = crc32(0, (void *)&mdata->version,
> +			     sizeof(*mdata) - sizeof(u32));
> +
> +	/* First write the primary mdata */
> +	ret = fwu_sf_save_primary_mdata(mdata);
> +	if (ret < 0) {
> +		log_err("Failed to update the primary mdata.\n");
> +		return ret;
> +	}
> +
> +	/* And now the replica */
> +	ret = fwu_sf_save_secondary_mdata(mdata);
> +	if (ret < 0) {
> +		log_err("Failed to update the secondary mdata.\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fwu_sf_mdata_check(void)
> +{
> +	struct fwu_mdata *primary = NULL, *secondary = NULL;
> +	int ret;
> +
> +	ret = fwu_sf_load_primary_mdata(&primary);
> +	if (ret < 0)
> +		log_err("Failed to read the primary mdata: %d\n", ret);
> +
> +	ret = fwu_sf_load_secondary_mdata(&secondary);
> +	if (ret < 0)
> +		log_err("Failed to read the secondary mdata: %d\n", ret);
> +
> +	if (primary && secondary) {
> +		if (memcmp(primary, secondary, sizeof(struct fwu_mdata))) {
> +			log_err("The primary and the secondary mdata are different\n");
> +			ret = -1;
> +		}
> +	} else if (primary) {
> +		ret = fwu_sf_save_secondary_mdata(primary);
> +		if (ret < 0)
> +			log_err("Restoring secondary mdata partition failed\n");
> +	} else if (secondary) {
> +		ret = fwu_sf_save_primary_mdata(secondary);
> +		if (ret < 0)
> +			log_err("Restoring primary mdata partition failed\n");
> +	}
> +
> +	free(primary);
> +	free(secondary);
> +	return ret;
> +}
> +
> +static int fwu_sf_get_mdata(struct fwu_mdata **mdata)
> +{
> +	return fwu_sf_get_valid_mdata(mdata);
> +}
> +
> +static struct fwu_mdata_ops fwu_sf_mdata_ops = {
> +	.get_image_alt_num = fwu_plat_get_image_alt_num,
> +	.mdata_check = fwu_sf_mdata_check,
> +	.get_mdata = fwu_sf_get_mdata,
> +	.update_mdata = fwu_sf_update_mdata,
> +};
> +
> +struct fwu_mdata_ops *fwu_sf_get_fwu_mdata_ops(void)
> +{
> +	return &fwu_sf_mdata_ops;
> +}
> 

Re: [RFC PATCH 12/14] FWU: synquacer: Add FWU Multi bank update support for DeveloperBox

[AKASHI Takahiro]

On Fri, Jan 21, 2022 at 12:31:20AM +0900, Masami Hiramatsu wrote:
> The DeveloperBox platform can support the FWU Multi bank
> update. SCP firmware will switch the boot mode by DSW3-4
> and load the Multi bank update supported TF-A BL2 from
> 0x600000 offset on the SPI flash. Thus it can co-exist
> with the legacy boot mode (legacy U-Boot or EDK2).
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  board/socionext/developerbox/Kconfig    |   19 ++++
>  board/socionext/developerbox/Makefile   |    1 
>  board/socionext/developerbox/fwu_plat.c |  158 +++++++++++++++++++++++++++++++
>  include/configs/synquacer.h             |   10 ++
>  include/efi_loader.h                    |    3 +
>  lib/efi_loader/efi_firmware.c           |   14 +--
>  6 files changed, 198 insertions(+), 7 deletions(-)
>  create mode 100644 board/socionext/developerbox/fwu_plat.c
> 
> diff --git a/board/socionext/developerbox/Kconfig b/board/socionext/developerbox/Kconfig
> index c181d26a44..4e2c341aad 100644
> --- a/board/socionext/developerbox/Kconfig
> +++ b/board/socionext/developerbox/Kconfig
> @@ -32,4 +32,23 @@ config SYS_CONFIG_NAME
>  	default "synquacer"
>  
>  endif
> +
> +config FWU_MULTI_BANK_UPDATE
> +	select FWU_MULTI_BANK_UPDATE_SF
> +
> +config FWU_MULTI_BANK_UPDATE_SF
> +	select DM_SPI_FLASH
> +
> +config FWU_NUM_BANKS
> +	default 6
> +
> +config FWU_NUM_IMAGES_PER_BANK
> +	default 1
> +
> +config FWU_SF_PRIMARY_MDATA_OFFSET
> +	default 0x500000
> +
> +config FWU_SF_SECONDARY_MDATA_OFFSET
> +	default 0x520000

Are those configs DeveloperBox-specific?

-Takahiro Akashi

> +
>  endif
> diff --git a/board/socionext/developerbox/Makefile b/board/socionext/developerbox/Makefile
> index 4a46de995a..15cce9c57e 100644
> --- a/board/socionext/developerbox/Makefile
> +++ b/board/socionext/developerbox/Makefile
> @@ -7,3 +7,4 @@
>  #
>  
>  obj-y	:= developerbox.o
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE_SF) += fwu_plat.o
> diff --git a/board/socionext/developerbox/fwu_plat.c b/board/socionext/developerbox/fwu_plat.c
> new file mode 100644
> index 0000000000..dbb814f1fd
> --- /dev/null
> +++ b/board/socionext/developerbox/fwu_plat.c
> @@ -0,0 +1,158 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2021, Linaro Limited
> + */
> +
> +#include <efi_loader.h>
> +#include <fwu.h>
> +#include <fwu_mdata.h>
> +#include <malloc.h>
> +#include <memalign.h>
> +#include <spi.h>
> +#include <spi_flash.h>
> +#include <flash.h>
> +
> +#include <linux/errno.h>
> +#include <linux/types.h>
> +#include <u-boot/crc.h>
> +
> +/* SPI Flash accessors */
> +static struct spi_flash *plat_spi_flash;
> +
> +static int __plat_sf_get_flash(void)
> +{
> +	struct udevice *new;
> +	int	ret;
> +
> +	//TODO: use CONFIG_FWU_SPI_*
> +	ret = spi_flash_probe_bus_cs(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
> +				     CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE,
> +				     &new);
> +	if (ret)
> +		return ret;
> +
> +	plat_spi_flash = dev_get_uclass_priv(new);
> +	return 0;
> +}
> +
> +static int plat_sf_get_flash(struct spi_flash **flash)
> +{
> +	int ret = 0;
> +
> +	if (!plat_spi_flash)
> +		ret = __plat_sf_get_flash();
> +
> +	*flash = plat_spi_flash;
> +
> +	return ret;
> +}
> +
> +static int sf_load_data(u32 offs, u32 size, void **data)
> +{
> +	struct spi_flash *flash;
> +	int ret;
> +
> +	ret = plat_sf_get_flash(&flash);
> +	if (ret < 0)
> +		return ret;
> +
> +	*data = memalign(ARCH_DMA_MINALIGN, size);
> +	if (!*data)
> +		return -ENOMEM;
> +
> +	ret = spi_flash_read(flash, offs, size, *data);
> +	if (ret < 0) {
> +		free(*data);
> +		*data = NULL;
> +	}
> +
> +	return ret;
> +}
> +
> +/* Platform dependent GUIDs */
> +
> +/* The GUID for the SNI FIP image type GUID */
> +#define FWU_IMAGE_TYPE_DEVBOX_FIP_GUID \
> +	EFI_GUID(0x7d6dc310, 0x52ca, 0x43b8, 0xb7, 0xb9, \
> +		 0xf9, 0xd6, 0xc5, 0x01, 0xd1, 0x08)
> +
> +#define PLAT_METADATA_OFFSET	0x510000
> +#define PLAT_METADATA_SIZE	(sizeof(struct devbox_metadata))
> +
> +struct __packed devbox_metadata {
> +	u32 boot_index;
> +	u32 boot_count;
> +	u32 invert_dual;
> +} *devbox_plat_metadata;
> +
> +static const efi_guid_t devbox_fip_image_type_guid = FWU_IMAGE_TYPE_DEVBOX_FIP_GUID;
> +
> +int fwu_plat_get_image_alt_num(efi_guid_t image_type_id, u32 update_bank,
> +			       int *alt_no)
> +{
> +	/* DeveloperBox FWU Multi bank only supports FIP image. */
> +	if (guidcmp(&image_type_id, &devbox_fip_image_type_guid))
> +		return -EOPNOTSUPP;
> +
> +	/*
> +	 * DeveloperBox FWU expects Bank:Image = 1:1, and the dfu_alt_info
> +	 * only has the entries for banks. Thus the alt_no should be equal
> +	 * to the update_bank.
> +	 */
> +	update_bank %= CONFIG_FWU_NUM_BANKS;
> +	*alt_no = update_bank;
> +
> +	return 0;
> +}
> +
> +/* SPI flash doesn't have GPT, and it's not blk device */
> +int fwu_plat_fill_partition_guids(efi_guid_t **part_guid_arr)
> +{
> +	efi_status_t ret;
> +
> +	free(*part_guid_arr);
> +
> +	ret = efi_fill_part_guid_array(&devbox_fip_image_type_guid, part_guid_arr);
> +	return (ret != EFI_SUCCESS) ? -ENOMEM : 0;
> +}
> +
> +/* TBD: add a usage counter for wear leveling */
> +int fwu_plat_get_update_index(u32 *update_idx)
> +{
> +	int ret;
> +	u32 active_idx;
> +
> +	ret = fwu_get_active_index(&active_idx);
> +
> +	if (ret < 0)
> +		return -1;
> +
> +	*update_idx = (active_idx + 1) % CONFIG_FWU_NUM_BANKS;
> +
> +	return ret;
> +}
> +
> +static int devbox_load_plat_metadata(void)
> +{
> +	if (devbox_plat_metadata)
> +		return 0;
> +
> +	return sf_load_data(PLAT_METADATA_OFFSET, PLAT_METADATA_SIZE,
> +			 (void **)&devbox_plat_metadata);
> +}
> +
> +void fwu_plat_get_bootidx(void *boot_idx)
> +{
> +	u32 *bootidx = boot_idx;
> +
> +	if (devbox_load_plat_metadata() < 0)
> +		*bootidx = 0;
> +	else
> +		*bootidx = devbox_plat_metadata->boot_index;
> +}
> +
> +struct fwu_mdata_ops *get_plat_fwu_mdata_ops(void)
> +{
> +	return fwu_sf_get_fwu_mdata_ops();
> +}
> +
> diff --git a/include/configs/synquacer.h b/include/configs/synquacer.h
> index 6d67bd2af5..f859237550 100644
> --- a/include/configs/synquacer.h
> +++ b/include/configs/synquacer.h
> @@ -47,8 +47,18 @@
>  
>  /* Since U-Boot 64bit PCIe support is limited, disable 64bit MMIO support */
>  
> +#ifdef CONFIG_FWU_MULTI_BANK_UPDATE
> +#define DEFAULT_DFU_ALT_INFO "dfu_alt_info="				\
> +				"mtd nor1=bank0 raw  600000 400000;"	\
> +					 "bank1 raw  a00000 400000;"	\
> +					 "bank2 raw  e00000 400000;"	\
> +					 "bank3 raw 1200000 400000;"	\
> +					 "bank4 raw 1600000 400000;"	\
> +					 "bank5 raw 1a00000 400000\0"
> +#else
>  #define DEFAULT_DFU_ALT_INFO "dfu_alt_info="				\
>  			"mtd nor1=fip.bin raw 600000 400000\0"
> +#endif
>  
>  /* Distro boot settings */
>  #ifndef CONFIG_SPL_BUILD
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index f20d361876..d6dc173feb 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -953,6 +953,9 @@ void efi_memcpy_runtime(void *dest, const void *src, size_t n);
>  u16 *efi_create_indexed_name(u16 *buffer, size_t buffer_size, const char *name,
>  			     unsigned int index);
>  
> +efi_status_t efi_fill_part_guid_array(const efi_guid_t *guid,
> +				      efi_guid_t **part_guid_arr);
> +
>  extern const struct efi_firmware_management_protocol efi_fmp_fit;
>  extern const struct efi_firmware_management_protocol efi_fmp_raw;
>  
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 107f0cb074..c100be35d3 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -97,8 +97,8 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
>  	return EFI_EXIT(EFI_UNSUPPORTED);
>  }
>  
> -static efi_status_t fill_part_guid_array(const efi_guid_t *guid,
> -					 efi_guid_t **part_guid_arr)
> +efi_status_t efi_fill_part_guid_array(const efi_guid_t *guid,
> +				      efi_guid_t **part_guid_arr)
>  {
>  	int i;
>  	int dfu_num = 0;
> @@ -309,8 +309,8 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info(
>  	     !descriptor_size || !package_version || !package_version_name))
>  		return EFI_EXIT(EFI_INVALID_PARAMETER);
>  
> -	ret = fill_part_guid_array(&efi_firmware_image_type_uboot_fit,
> -				   &part_guid_arr);
> +	ret = efi_fill_part_guid_array(&efi_firmware_image_type_uboot_fit,
> +				       &part_guid_arr);
>  	if (ret != EFI_SUCCESS)
>  		goto out;
>  
> @@ -429,7 +429,7 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info(
>  		return EFI_EXIT(EFI_INVALID_PARAMETER);
>  
>  	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> -		ret = fill_part_guid_array(&null_guid, &part_guid_arr);
> +		ret = efi_fill_part_guid_array(&null_guid, &part_guid_arr);
>  		if (ret != EFI_SUCCESS)
>  			goto out;
>  
> @@ -444,8 +444,8 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info(
>  			goto out;
>  		}
>  	} else {
> -		ret = fill_part_guid_array(&efi_firmware_image_type_uboot_raw,
> -					   &part_guid_arr);
> +		ret = efi_fill_part_guid_array(&efi_firmware_image_type_uboot_raw,
> +					       &part_guid_arr);
>  		if (ret != EFI_SUCCESS)
>  			goto out;
>  	}
> 

Re: [RFC PATCH 10/14] FWU: Reboot soon after successfully install the new firmware

[Masami Hiramatsu]

Hi Takahiro,

2022年1月21日(金) 10:46 AKASHI Takahiro <takahiro.akashi@linaro.org>:
>
> On Fri, Jan 21, 2022 at 12:31:00AM +0900, Masami Hiramatsu wrote:
> > Reboot to the trial state soon after successfully installing
> > the new firmware to the next bank and updating the active_index.
> > This is enabled by CONFIG_FWU_REBOOT_AFTER_UPDATE and is a
> > recommended option.
>
> EFI_CAPSULE_HEADER.Flags may have a flag, CAPSULE_FLAGS_INITIATE_RESET.
> See Section "8.5.3 Update Capsule" in the UEFI specification.
>
> I think that we'd better implement the feature rather than adding
> CONFIG_FWU_REBOOT_AFTER_UPDATE.

Thanks for pointing it! I agree with you, the flag is more useful.

Regards,

>
> -Takahiro Akashi
>
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > ---
> >  lib/efi_loader/efi_capsule.c |   10 ++++++++--
> >  lib/fwu_updates/Kconfig      |    9 +++++++++
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index 83c89a0cbb..0928425b5f 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -1355,10 +1355,16 @@ efi_status_t efi_launch_capsules(void)
> >                       } else {
> >                               log_debug("Successfully updated the active_index\n");
> >                               status = fwu_trial_state_ctr_start();
> > -                             if (status < 0)
> > +                             if (status < 0) {
> >                                       ret = EFI_DEVICE_ERROR;
> > -                             else
> > +                             } else {
> >                                       ret = EFI_SUCCESS;
> > +                                     if (IS_ENABLED(CONFIG_FWU_REBOOT_AFTER_UPDATE)) {
> > +                                             log_info("New firmware is installed in bank#%d. Reboot from that bank.\n",
> > +                                                      update_index);
> > +                                             do_reset(NULL, 0, 0, NULL);
> > +                                     }
> > +                             }
> >                       }
> >               } else if (capsule_update == true && update_status == false) {
> >                       log_err("All capsules were not updated. Not updating FWU metadata\n");
> > diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig
> > index 6de28e0c9c..0940a90747 100644
> > --- a/lib/fwu_updates/Kconfig
> > +++ b/lib/fwu_updates/Kconfig
> > @@ -29,3 +29,12 @@ config FWU_TRIAL_STATE_CNT
> >         With FWU Multi Bank Update feature enabled, number of times
> >         the platform is allowed to boot in Trial State after an
> >         update.
> > +
> > +config FWU_REBOOT_AFTER_UPDATE
> > +     bool "Reboot soon after installing new firmware"
> > +     depends on FWU_MULTI_BANK_UPDATE
> > +     default y
> > +     help
> > +       Reboot the machine soon after installing a new firmware
> > +       and start trial boot. You can disable this option for
> > +       debugging or FWU development, but recommended to enable it.
> >



-- 
Masami Hiramatsu

Re: [RFC PATCH 12/14] FWU: synquacer: Add FWU Multi bank update support for DeveloperBox

[Masami Hiramatsu]

2022年1月21日(金) 11:22 AKASHI Takahiro <takahiro.akashi@linaro.org>:
>
> On Fri, Jan 21, 2022 at 12:31:20AM +0900, Masami Hiramatsu wrote:
> > The DeveloperBox platform can support the FWU Multi bank
> > update. SCP firmware will switch the boot mode by DSW3-4
> > and load the Multi bank update supported TF-A BL2 from
> > 0x600000 offset on the SPI flash. Thus it can co-exist
> > with the legacy boot mode (legacy U-Boot or EDK2).
> >
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > ---
> >  board/socionext/developerbox/Kconfig    |   19 ++++
> >  board/socionext/developerbox/Makefile   |    1
> >  board/socionext/developerbox/fwu_plat.c |  158 +++++++++++++++++++++++++++++++
> >  include/configs/synquacer.h             |   10 ++
> >  include/efi_loader.h                    |    3 +
> >  lib/efi_loader/efi_firmware.c           |   14 +--
> >  6 files changed, 198 insertions(+), 7 deletions(-)
> >  create mode 100644 board/socionext/developerbox/fwu_plat.c
> >
> > diff --git a/board/socionext/developerbox/Kconfig b/board/socionext/developerbox/Kconfig
> > index c181d26a44..4e2c341aad 100644
> > --- a/board/socionext/developerbox/Kconfig
> > +++ b/board/socionext/developerbox/Kconfig
> > @@ -32,4 +32,23 @@ config SYS_CONFIG_NAME
> >       default "synquacer"
> >
> >  endif
> > +
> > +config FWU_MULTI_BANK_UPDATE
> > +     select FWU_MULTI_BANK_UPDATE_SF
> > +
> > +config FWU_MULTI_BANK_UPDATE_SF
> > +     select DM_SPI_FLASH
> > +
> > +config FWU_NUM_BANKS
> > +     default 6
> > +
> > +config FWU_NUM_IMAGES_PER_BANK
> > +     default 1
> > +
> > +config FWU_SF_PRIMARY_MDATA_OFFSET
> > +     default 0x500000
> > +
> > +config FWU_SF_SECONDARY_MDATA_OFFSET
> > +     default 0x520000
>
> Are those configs DeveloperBox-specific?

Yes, these default values are DeveloperBox specific, since it depends
on the firmware/metadata layout on the SPI flash.
Each platform has to define appropriate values.

Thank you,

>
> -Takahiro Akashi
>
> > +
> >  endif
> > diff --git a/board/socionext/developerbox/Makefile b/board/socionext/developerbox/Makefile
> > index 4a46de995a..15cce9c57e 100644
> > --- a/board/socionext/developerbox/Makefile
> > +++ b/board/socionext/developerbox/Makefile
> > @@ -7,3 +7,4 @@
> >  #
> >
> >  obj-y        := developerbox.o
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE_SF) += fwu_plat.o
> > diff --git a/board/socionext/developerbox/fwu_plat.c b/board/socionext/developerbox/fwu_plat.c
> > new file mode 100644
> > index 0000000000..dbb814f1fd
> > --- /dev/null
> > +++ b/board/socionext/developerbox/fwu_plat.c
> > @@ -0,0 +1,158 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2021, Linaro Limited
> > + */
> > +
> > +#include <efi_loader.h>
> > +#include <fwu.h>
> > +#include <fwu_mdata.h>
> > +#include <malloc.h>
> > +#include <memalign.h>
> > +#include <spi.h>
> > +#include <spi_flash.h>
> > +#include <flash.h>
> > +
> > +#include <linux/errno.h>
> > +#include <linux/types.h>
> > +#include <u-boot/crc.h>
> > +
> > +/* SPI Flash accessors */
> > +static struct spi_flash *plat_spi_flash;
> > +
> > +static int __plat_sf_get_flash(void)
> > +{
> > +     struct udevice *new;
> > +     int     ret;
> > +
> > +     //TODO: use CONFIG_FWU_SPI_*
> > +     ret = spi_flash_probe_bus_cs(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
> > +                                  CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE,
> > +                                  &new);
> > +     if (ret)
> > +             return ret;
> > +
> > +     plat_spi_flash = dev_get_uclass_priv(new);
> > +     return 0;
> > +}
> > +
> > +static int plat_sf_get_flash(struct spi_flash **flash)
> > +{
> > +     int ret = 0;
> > +
> > +     if (!plat_spi_flash)
> > +             ret = __plat_sf_get_flash();
> > +
> > +     *flash = plat_spi_flash;
> > +
> > +     return ret;
> > +}
> > +
> > +static int sf_load_data(u32 offs, u32 size, void **data)
> > +{
> > +     struct spi_flash *flash;
> > +     int ret;
> > +
> > +     ret = plat_sf_get_flash(&flash);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     *data = memalign(ARCH_DMA_MINALIGN, size);
> > +     if (!*data)
> > +             return -ENOMEM;
> > +
> > +     ret = spi_flash_read(flash, offs, size, *data);
> > +     if (ret < 0) {
> > +             free(*data);
> > +             *data = NULL;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +/* Platform dependent GUIDs */
> > +
> > +/* The GUID for the SNI FIP image type GUID */
> > +#define FWU_IMAGE_TYPE_DEVBOX_FIP_GUID \
> > +     EFI_GUID(0x7d6dc310, 0x52ca, 0x43b8, 0xb7, 0xb9, \
> > +              0xf9, 0xd6, 0xc5, 0x01, 0xd1, 0x08)
> > +
> > +#define PLAT_METADATA_OFFSET 0x510000
> > +#define PLAT_METADATA_SIZE   (sizeof(struct devbox_metadata))
> > +
> > +struct __packed devbox_metadata {
> > +     u32 boot_index;
> > +     u32 boot_count;
> > +     u32 invert_dual;
> > +} *devbox_plat_metadata;
> > +
> > +static const efi_guid_t devbox_fip_image_type_guid = FWU_IMAGE_TYPE_DEVBOX_FIP_GUID;
> > +
> > +int fwu_plat_get_image_alt_num(efi_guid_t image_type_id, u32 update_bank,
> > +                            int *alt_no)
> > +{
> > +     /* DeveloperBox FWU Multi bank only supports FIP image. */
> > +     if (guidcmp(&image_type_id, &devbox_fip_image_type_guid))
> > +             return -EOPNOTSUPP;
> > +
> > +     /*
> > +      * DeveloperBox FWU expects Bank:Image = 1:1, and the dfu_alt_info
> > +      * only has the entries for banks. Thus the alt_no should be equal
> > +      * to the update_bank.
> > +      */
> > +     update_bank %= CONFIG_FWU_NUM_BANKS;
> > +     *alt_no = update_bank;
> > +
> > +     return 0;
> > +}
> > +
> > +/* SPI flash doesn't have GPT, and it's not blk device */
> > +int fwu_plat_fill_partition_guids(efi_guid_t **part_guid_arr)
> > +{
> > +     efi_status_t ret;
> > +
> > +     free(*part_guid_arr);
> > +
> > +     ret = efi_fill_part_guid_array(&devbox_fip_image_type_guid, part_guid_arr);
> > +     return (ret != EFI_SUCCESS) ? -ENOMEM : 0;
> > +}
> > +
> > +/* TBD: add a usage counter for wear leveling */
> > +int fwu_plat_get_update_index(u32 *update_idx)
> > +{
> > +     int ret;
> > +     u32 active_idx;
> > +
> > +     ret = fwu_get_active_index(&active_idx);
> > +
> > +     if (ret < 0)
> > +             return -1;
> > +
> > +     *update_idx = (active_idx + 1) % CONFIG_FWU_NUM_BANKS;
> > +
> > +     return ret;
> > +}
> > +
> > +static int devbox_load_plat_metadata(void)
> > +{
> > +     if (devbox_plat_metadata)
> > +             return 0;
> > +
> > +     return sf_load_data(PLAT_METADATA_OFFSET, PLAT_METADATA_SIZE,
> > +                      (void **)&devbox_plat_metadata);
> > +}
> > +
> > +void fwu_plat_get_bootidx(void *boot_idx)
> > +{
> > +     u32 *bootidx = boot_idx;
> > +
> > +     if (devbox_load_plat_metadata() < 0)
> > +             *bootidx = 0;
> > +     else
> > +             *bootidx = devbox_plat_metadata->boot_index;
> > +}
> > +
> > +struct fwu_mdata_ops *get_plat_fwu_mdata_ops(void)
> > +{
> > +     return fwu_sf_get_fwu_mdata_ops();
> > +}
> > +
> > diff --git a/include/configs/synquacer.h b/include/configs/synquacer.h
> > index 6d67bd2af5..f859237550 100644
> > --- a/include/configs/synquacer.h
> > +++ b/include/configs/synquacer.h
> > @@ -47,8 +47,18 @@
> >
> >  /* Since U-Boot 64bit PCIe support is limited, disable 64bit MMIO support */
> >
> > +#ifdef CONFIG_FWU_MULTI_BANK_UPDATE
> > +#define DEFAULT_DFU_ALT_INFO "dfu_alt_info="                         \
> > +                             "mtd nor1=bank0 raw  600000 400000;"    \
> > +                                      "bank1 raw  a00000 400000;"    \
> > +                                      "bank2 raw  e00000 400000;"    \
> > +                                      "bank3 raw 1200000 400000;"    \
> > +                                      "bank4 raw 1600000 400000;"    \
> > +                                      "bank5 raw 1a00000 400000\0"
> > +#else
> >  #define DEFAULT_DFU_ALT_INFO "dfu_alt_info="                         \
> >                       "mtd nor1=fip.bin raw 600000 400000\0"
> > +#endif
> >
> >  /* Distro boot settings */
> >  #ifndef CONFIG_SPL_BUILD
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index f20d361876..d6dc173feb 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -953,6 +953,9 @@ void efi_memcpy_runtime(void *dest, const void *src, size_t n);
> >  u16 *efi_create_indexed_name(u16 *buffer, size_t buffer_size, const char *name,
> >                            unsigned int index);
> >
> > +efi_status_t efi_fill_part_guid_array(const efi_guid_t *guid,
> > +                                   efi_guid_t **part_guid_arr);
> > +
> >  extern const struct efi_firmware_management_protocol efi_fmp_fit;
> >  extern const struct efi_firmware_management_protocol efi_fmp_raw;
> >
> > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> > index 107f0cb074..c100be35d3 100644
> > --- a/lib/efi_loader/efi_firmware.c
> > +++ b/lib/efi_loader/efi_firmware.c
> > @@ -97,8 +97,8 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
> >       return EFI_EXIT(EFI_UNSUPPORTED);
> >  }
> >
> > -static efi_status_t fill_part_guid_array(const efi_guid_t *guid,
> > -                                      efi_guid_t **part_guid_arr)
> > +efi_status_t efi_fill_part_guid_array(const efi_guid_t *guid,
> > +                                   efi_guid_t **part_guid_arr)
> >  {
> >       int i;
> >       int dfu_num = 0;
> > @@ -309,8 +309,8 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info(
> >            !descriptor_size || !package_version || !package_version_name))
> >               return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > -     ret = fill_part_guid_array(&efi_firmware_image_type_uboot_fit,
> > -                                &part_guid_arr);
> > +     ret = efi_fill_part_guid_array(&efi_firmware_image_type_uboot_fit,
> > +                                    &part_guid_arr);
> >       if (ret != EFI_SUCCESS)
> >               goto out;
> >
> > @@ -429,7 +429,7 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info(
> >               return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> >       if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > -             ret = fill_part_guid_array(&null_guid, &part_guid_arr);
> > +             ret = efi_fill_part_guid_array(&null_guid, &part_guid_arr);
> >               if (ret != EFI_SUCCESS)
> >                       goto out;
> >
> > @@ -444,8 +444,8 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info(
> >                       goto out;
> >               }
> >       } else {
> > -             ret = fill_part_guid_array(&efi_firmware_image_type_uboot_raw,
> > -                                        &part_guid_arr);
> > +             ret = efi_fill_part_guid_array(&efi_firmware_image_type_uboot_raw,
> > +                                            &part_guid_arr);
> >               if (ret != EFI_SUCCESS)
> >                       goto out;
> >       }
> >



-- 
Masami Hiramatsu

Re: [RFC PATCH 11/14] FWU: Add FWU Multi Bank Update on SPI Flash

[Masami Hiramatsu]

2022年1月21日(金) 11:20 AKASHI Takahiro <takahiro.akashi@linaro.org>:

>
> On Fri, Jan 21, 2022 at 12:31:10AM +0900, Masami Hiramatsu wrote:
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > ---
> >  include/fwu.h                  |   13 ++
> >  lib/fwu_updates/Kconfig        |   34 ++++++
> >  lib/fwu_updates/Makefile       |    5 -
> >  lib/fwu_updates/fwu_mdata_sf.c |  241 ++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 288 insertions(+), 5 deletions(-)
> >  create mode 100644 lib/fwu_updates/fwu_mdata_sf.c
> >
> > diff --git a/include/fwu.h b/include/fwu.h
> > index 7af94988b7..a013170321 100644
> > --- a/include/fwu.h
> > +++ b/include/fwu.h
> > @@ -64,9 +64,18 @@ int fwu_accept_image(efi_guid_t *img_type_id, u32 bank);
> >  int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank);
> >
> >  int fwu_plat_get_update_index(u32 *update_idx);
> > -int fwu_plat_get_blk_desc(struct blk_desc **desc);
> > -int fwu_plat_get_alt_num(void *identifier);
> >  int fwu_plat_fill_partition_guids(efi_guid_t **part_guid_arr);
> >  void fwu_plat_get_bootidx(void *boot_idx);
> >
> > +#ifdef CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK
> > +int fwu_plat_get_blk_desc(struct blk_desc **desc);
> > +int fwu_plat_get_alt_num(void *identifier);
> > +#endif
>
> We can (and should) remove "#ifdef ..." from headers.

Oops, indeed. We just need some comments for those platform dependent functions.
Thank you!

>
> -Takahiro Akashi
>
> > +#ifdef CONFIG_FWU_MULTI_BANK_UPDATE_SF
> > +struct fwu_mdata_ops *fwu_sf_get_fwu_mdata_ops(void);
> > +int fwu_plat_get_image_alt_num(efi_guid_t image_type_id, u32 update_bank,
> > +                            int *alt_no);
> > +#endif
> > +
> >  #endif /* _FWU_H_ */
> > diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig
> > index 0940a90747..8a369b9872 100644
> > --- a/lib/fwu_updates/Kconfig
> > +++ b/lib/fwu_updates/Kconfig
> > @@ -38,3 +38,37 @@ config FWU_REBOOT_AFTER_UPDATE
> >         Reboot the machine soon after installing a new firmware
> >         and start trial boot. You can disable this option for
> >         debugging or FWU development, but recommended to enable it.
> > +
> > +config FWU_MULTI_BANK_UPDATE_GPT_BLK
> > +     bool "Enable FWU Multi Bank Update for GPT on blockdev"
> > +     depends on FWU_MULTI_BANK_UPDATE
> > +     select EFI_PARTITION
> > +     help
> > +       Enable FWU Multi Bank Update for GPT on a block device. This
> > +       driver depends on GPT and the block device. FWU meatadata and
> > +       firmware will be stored on a block device with GPT.
> > +
> > +config FWU_MULTI_BANK_UPDATE_SF
> > +     bool "Enable FWU Multi Bank Update for SPI Flash"
> > +     depends on FWU_MULTI_BANK_UPDATE
> > +     help
> > +       Enable FWU Multi Bank Update for SPI flash driver. This
> > +       driver does not depend on GPT. Instead, the platform must
> > +       provide some APIs and define the offset of the primary and
> > +       the secondary metadata.
> > +
> > +config FWU_SF_PRIMARY_MDATA_OFFSET
> > +     hex "The offset of the primary metadata"
> > +     depends on FWU_MULTI_BANK_UPDATE_SF
> > +     help
> > +       The offset of the primary metadata on SPI Flash in
> > +       hexadecimal.
> > +
> > +config FWU_SF_SECONDARY_MDATA_OFFSET
> > +     hex "The offset of the secondary metadata"
> > +     depends on FWU_MULTI_BANK_UPDATE_SF
> > +     help
> > +       The offset of the secondary metadata on SPI Flash in
> > +       hexadecimal.
> > +       This must NOT be the same offset of the primary
> > +       metadata.
> > diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> > index 73099a30cb..80669344f2 100644
> > --- a/lib/fwu_updates/Makefile
> > +++ b/lib/fwu_updates/Makefile
> > @@ -6,6 +6,5 @@
> >  obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
> >  obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
> >
> > -ifdef CONFIG_EFI_PARTITION
> > -obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
> > -endif
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK) += fwu_mdata_gpt_blk.o
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE_SF) += fwu_mdata_sf.o
> > diff --git a/lib/fwu_updates/fwu_mdata_sf.c b/lib/fwu_updates/fwu_mdata_sf.c
> > new file mode 100644
> > index 0000000000..9e3cae7b68
> > --- /dev/null
> > +++ b/lib/fwu_updates/fwu_mdata_sf.c
> > @@ -0,0 +1,241 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2021, Linaro Limited
> > + */
> > +
> > +#include <efi_loader.h>
> > +#include <fwu.h>
> > +#include <fwu_mdata.h>
> > +#include <malloc.h>
> > +#include <memalign.h>
> > +#include <spi.h>
> > +#include <spi_flash.h>
> > +#include <flash.h>
> > +
> > +#include <linux/errno.h>
> > +#include <linux/types.h>
> > +#include <u-boot/crc.h>
> > +
> > +/*
> > + * For the FWU SPI flash driver, the platform must define below functions
> > + * according to its dfu_alt_info.
> > + */
> > +extern int fwu_plat_get_image_alt_num(efi_guid_t image_type_id,
> > +                                   u32 update_bank, int *alt_no);
> > +
> > +static struct spi_flash *fwu_spi_flash;
> > +
> > +static int __fwu_sf_get_flash(void)
> > +{
> > +     if (IS_ENABLED(CONFIG_DM_SPI_FLASH)) {
> > +             struct udevice *new;
> > +             int     ret;
> > +
> > +             ret = spi_flash_probe_bus_cs(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
> > +                                          CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE,
> > +                                          &new);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             fwu_spi_flash = dev_get_uclass_priv(new);
> > +     } else {
> > +             fwu_spi_flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
> > +                                             CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE);
> > +             if (!fwu_spi_flash)
> > +                     return -EIO;
> > +     }
> > +     return 0;
> > +}
> > +
> > +static int fwu_sf_get_flash(struct spi_flash **flash)
> > +{
> > +     int ret = 0;
> > +
> > +     if (!fwu_spi_flash)
> > +             ret = __fwu_sf_get_flash();
> > +
> > +     *flash = fwu_spi_flash;
> > +
> > +     return ret;
> > +}
> > +
> > +static int sf_load_data(u32 offs, u32 size, void **data)
> > +{
> > +     struct spi_flash *flash;
> > +     int ret;
> > +
> > +     ret = fwu_sf_get_flash(&flash);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     *data = memalign(ARCH_DMA_MINALIGN, size);
> > +     if (!*data)
> > +             return -ENOMEM;
> > +
> > +     ret = spi_flash_read(flash, offs, size, *data);
> > +     if (ret < 0) {
> > +             free(*data);
> > +             *data = NULL;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static int sf_save_data(u32 offs, u32 size, void *data)
> > +{
> > +     struct spi_flash *flash;
> > +     u32 sect_size, nsect;
> > +     void *buf;
> > +     int ret;
> > +
> > +     ret = fwu_sf_get_flash(&flash);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     sect_size = flash->mtd.erasesize;
> > +     nsect = DIV_ROUND_UP(size, sect_size);
> > +     ret = spi_flash_erase(flash, offs, nsect * sect_size);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     buf = memalign(ARCH_DMA_MINALIGN, size);
> > +     if (!buf)
> > +             return -ENOMEM;
> > +     memcpy(buf, data, size);
> > +
> > +     ret = spi_flash_write(flash, offs, size, buf);
> > +
> > +     free(buf);
> > +
> > +     return ret;
> > +}
> > +
> > +static int fwu_sf_load_mdata(struct fwu_mdata **mdata, u32 offs)
> > +{
> > +     int ret;
> > +
> > +     ret = sf_load_data(offs, sizeof(struct fwu_mdata), (void **)mdata);
> > +
> > +     if (ret >= 0) {
> > +             ret = fwu_verify_mdata(*mdata,
> > +                             offs == CONFIG_FWU_SF_PRIMARY_MDATA_OFFSET);
> > +             if (ret < 0) {
> > +                     free(*mdata);
> > +                     *mdata = NULL;
> > +             }
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static int fwu_sf_load_primary_mdata(struct fwu_mdata **mdata)
> > +{
> > +     return fwu_sf_load_mdata(mdata, CONFIG_FWU_SF_PRIMARY_MDATA_OFFSET);
> > +}
> > +
> > +static int fwu_sf_load_secondary_mdata(struct fwu_mdata **mdata)
> > +{
> > +     return fwu_sf_load_mdata(mdata, CONFIG_FWU_SF_SECONDARY_MDATA_OFFSET);
> > +}
> > +
> > +static int fwu_sf_save_primary_mdata(struct fwu_mdata *mdata)
> > +{
> > +     return sf_save_data(CONFIG_FWU_SF_PRIMARY_MDATA_OFFSET,
> > +                         sizeof(struct fwu_mdata), mdata);
> > +}
> > +
> > +static int fwu_sf_save_secondary_mdata(struct fwu_mdata *mdata)
> > +{
> > +     return sf_save_data(CONFIG_FWU_SF_SECONDARY_MDATA_OFFSET,
> > +                         sizeof(struct fwu_mdata), mdata);
> > +}
> > +
> > +static int fwu_sf_get_valid_mdata(struct fwu_mdata **mdata)
> > +{
> > +     if (fwu_sf_load_primary_mdata(mdata) == 0)
> > +             return 0;
> > +
> > +     log_err("Failed to load/verify primary mdata. Try secondary.\n");
> > +
> > +     if (fwu_sf_load_secondary_mdata(mdata) == 0)
> > +             return 0;
> > +
> > +     log_err("Failed to load/verify secondary mdata.\n");
> > +
> > +     return -1;
> > +}
> > +
> > +static int fwu_sf_update_mdata(struct fwu_mdata *mdata)
> > +{
> > +     int ret;
> > +
> > +     /* Update mdata crc32 field */
> > +     mdata->crc32 = crc32(0, (void *)&mdata->version,
> > +                          sizeof(*mdata) - sizeof(u32));
> > +
> > +     /* First write the primary mdata */
> > +     ret = fwu_sf_save_primary_mdata(mdata);
> > +     if (ret < 0) {
> > +             log_err("Failed to update the primary mdata.\n");
> > +             return ret;
> > +     }
> > +
> > +     /* And now the replica */
> > +     ret = fwu_sf_save_secondary_mdata(mdata);
> > +     if (ret < 0) {
> > +             log_err("Failed to update the secondary mdata.\n");
> > +             return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int fwu_sf_mdata_check(void)
> > +{
> > +     struct fwu_mdata *primary = NULL, *secondary = NULL;
> > +     int ret;
> > +
> > +     ret = fwu_sf_load_primary_mdata(&primary);
> > +     if (ret < 0)
> > +             log_err("Failed to read the primary mdata: %d\n", ret);
> > +
> > +     ret = fwu_sf_load_secondary_mdata(&secondary);
> > +     if (ret < 0)
> > +             log_err("Failed to read the secondary mdata: %d\n", ret);
> > +
> > +     if (primary && secondary) {
> > +             if (memcmp(primary, secondary, sizeof(struct fwu_mdata))) {
> > +                     log_err("The primary and the secondary mdata are different\n");
> > +                     ret = -1;
> > +             }
> > +     } else if (primary) {
> > +             ret = fwu_sf_save_secondary_mdata(primary);
> > +             if (ret < 0)
> > +                     log_err("Restoring secondary mdata partition failed\n");
> > +     } else if (secondary) {
> > +             ret = fwu_sf_save_primary_mdata(secondary);
> > +             if (ret < 0)
> > +                     log_err("Restoring primary mdata partition failed\n");
> > +     }
> > +
> > +     free(primary);
> > +     free(secondary);
> > +     return ret;
> > +}
> > +
> > +static int fwu_sf_get_mdata(struct fwu_mdata **mdata)
> > +{
> > +     return fwu_sf_get_valid_mdata(mdata);
> > +}
> > +
> > +static struct fwu_mdata_ops fwu_sf_mdata_ops = {
> > +     .get_image_alt_num = fwu_plat_get_image_alt_num,
> > +     .mdata_check = fwu_sf_mdata_check,
> > +     .get_mdata = fwu_sf_get_mdata,
> > +     .update_mdata = fwu_sf_update_mdata,
> > +};
> > +
> > +struct fwu_mdata_ops *fwu_sf_get_fwu_mdata_ops(void)
> > +{
> > +     return &fwu_sf_mdata_ops;
> > +}
> >



--
Masami Hiramatsu

Re: [RFC PATCH 10/14] FWU: Reboot soon after successfully install the new firmware

[Masami Hiramatsu]

Hi,

2022年1月21日(金) 13:35 Masami Hiramatsu <masami.hiramatsu@linaro.org>:
>
> Hi Takahiro,
>
> 2022年1月21日(金) 10:46 AKASHI Takahiro <takahiro.akashi@linaro.org>:
> >
> > On Fri, Jan 21, 2022 at 12:31:00AM +0900, Masami Hiramatsu wrote:
> > > Reboot to the trial state soon after successfully installing
> > > the new firmware to the next bank and updating the active_index.
> > > This is enabled by CONFIG_FWU_REBOOT_AFTER_UPDATE and is a
> > > recommended option.
> >
> > EFI_CAPSULE_HEADER.Flags may have a flag, CAPSULE_FLAGS_INITIATE_RESET.
> > See Section "8.5.3 Update Capsule" in the UEFI specification.
> >
> > I think that we'd better implement the feature rather than adding
> > CONFIG_FWU_REBOOT_AFTER_UPDATE.
>
> Thanks for pointing it! I agree with you, the flag is more useful.

According to the UEFI spec 2.9, we need to consider implementing some
related things.

In 8.5.3 Update Capsule
 ----
A capsule which has the CAPSULE_FLAGS_INITIATE_RESET Flag must have
CAPSULE_FLAGS_PERSIST_ACROSS_RESET set in its header as well.
 [...]
If a capsule has the CAPSULE_FLAGS_PERSIST_ACROSS_RESET Flag set in its
header, the firmware will process the capsules after system reset. The
caller must
ensure to reset the system using the required reset value obtained from
QueryCapsuleCapabilities.
 ----
In Table 8-8 Flag Firmware Behavior
----
CAPSULE_FLAGS_PERSIST_ACROSS_RESET +
CAPSULE_FLAGS_INITIATE_RESET

Firmware will attempt to process or launch the capsule
across a reset. The firmware will initiate a reset which is
compatible with the passed-in capsule request and will
not return back to the caller. If the capsule is not
recognized, can expect an error. If the processing requires
a reset which is unsupported by the platform, expect an
error.
----

So, I have 2 questions;

1) Should we implement CAPSULE_FLAGS_PERSIST_ACROSS_RESET too?
 Since U-Boot only supports capsule update on disk, it seems the capsule already
 applied "across a reset". :-)
2) If there are multiple capsule files and only one file (e.g. aaaa.cap) has
  CAPSULE_FLAGS_INITIATE_RESET flag, should U-Boot resets after
   applying that capsule, or wait after all capsule files are applied?
   (current implementation is the latter one)

Thank you,

>
> Regards,
>
> >
> > -Takahiro Akashi
> >
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > ---
> > >  lib/efi_loader/efi_capsule.c |   10 ++++++++--
> > >  lib/fwu_updates/Kconfig      |    9 +++++++++
> > >  2 files changed, 17 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > index 83c89a0cbb..0928425b5f 100644
> > > --- a/lib/efi_loader/efi_capsule.c
> > > +++ b/lib/efi_loader/efi_capsule.c
> > > @@ -1355,10 +1355,16 @@ efi_status_t efi_launch_capsules(void)
> > >                       } else {
> > >                               log_debug("Successfully updated the active_index\n");
> > >                               status = fwu_trial_state_ctr_start();
> > > -                             if (status < 0)
> > > +                             if (status < 0) {
> > >                                       ret = EFI_DEVICE_ERROR;
> > > -                             else
> > > +                             } else {
> > >                                       ret = EFI_SUCCESS;
> > > +                                     if (IS_ENABLED(CONFIG_FWU_REBOOT_AFTER_UPDATE)) {
> > > +                                             log_info("New firmware is installed in bank#%d. Reboot from that bank.\n",
> > > +                                                      update_index);
> > > +                                             do_reset(NULL, 0, 0, NULL);
> > > +                                     }
> > > +                             }
> > >                       }
> > >               } else if (capsule_update == true && update_status == false) {
> > >                       log_err("All capsules were not updated. Not updating FWU metadata\n");
> > > diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig
> > > index 6de28e0c9c..0940a90747 100644
> > > --- a/lib/fwu_updates/Kconfig
> > > +++ b/lib/fwu_updates/Kconfig
> > > @@ -29,3 +29,12 @@ config FWU_TRIAL_STATE_CNT
> > >         With FWU Multi Bank Update feature enabled, number of times
> > >         the platform is allowed to boot in Trial State after an
> > >         update.
> > > +
> > > +config FWU_REBOOT_AFTER_UPDATE
> > > +     bool "Reboot soon after installing new firmware"
> > > +     depends on FWU_MULTI_BANK_UPDATE
> > > +     default y
> > > +     help
> > > +       Reboot the machine soon after installing a new firmware
> > > +       and start trial boot. You can disable this option for
> > > +       debugging or FWU development, but recommended to enable it.
> > >
>
>
>
> --
> Masami Hiramatsu



-- 
Masami Hiramatsu

Re: [RFC PATCH 10/14] FWU: Reboot soon after successfully install the new firmware

[AKASHI Takahiro]

On Fri, Jan 21, 2022 at 03:54:12PM +0900, Masami Hiramatsu wrote:
> Hi,
> 
> 2022年1月21日(金) 13:35 Masami Hiramatsu <masami.hiramatsu@linaro.org>:
> >
> > Hi Takahiro,
> >
> > 2022年1月21日(金) 10:46 AKASHI Takahiro <takahiro.akashi@linaro.org>:
> > >
> > > On Fri, Jan 21, 2022 at 12:31:00AM +0900, Masami Hiramatsu wrote:
> > > > Reboot to the trial state soon after successfully installing
> > > > the new firmware to the next bank and updating the active_index.
> > > > This is enabled by CONFIG_FWU_REBOOT_AFTER_UPDATE and is a
> > > > recommended option.
> > >
> > > EFI_CAPSULE_HEADER.Flags may have a flag, CAPSULE_FLAGS_INITIATE_RESET.
> > > See Section "8.5.3 Update Capsule" in the UEFI specification.
> > >
> > > I think that we'd better implement the feature rather than adding
> > > CONFIG_FWU_REBOOT_AFTER_UPDATE.
> >
> > Thanks for pointing it! I agree with you, the flag is more useful.
> 
> According to the UEFI spec 2.9, we need to consider implementing some
> related things.
> 
> In 8.5.3 Update Capsule
>  ----
> A capsule which has the CAPSULE_FLAGS_INITIATE_RESET Flag must have
> CAPSULE_FLAGS_PERSIST_ACROSS_RESET set in its header as well.
>  [...]
> If a capsule has the CAPSULE_FLAGS_PERSIST_ACROSS_RESET Flag set in its
> header, the firmware will process the capsules after system reset. The
> caller must
> ensure to reset the system using the required reset value obtained from
> QueryCapsuleCapabilities.
>  ----
> In Table 8-8 Flag Firmware Behavior
> ----
> CAPSULE_FLAGS_PERSIST_ACROSS_RESET +
> CAPSULE_FLAGS_INITIATE_RESET
> 
> Firmware will attempt to process or launch the capsule
> across a reset. The firmware will initiate a reset which is
> compatible with the passed-in capsule request and will
> not return back to the caller. If the capsule is not
> recognized, can expect an error. If the processing requires
> a reset which is unsupported by the platform, expect an
> error.
> ----
> 
> So, I have 2 questions;
> 
> 1) Should we implement CAPSULE_FLAGS_PERSIST_ACROSS_RESET too?
>  Since U-Boot only supports capsule update on disk, it seems the capsule already
>  applied "across a reset". :-)

Yeah, I suppose that PERSIST_ACROSS_RESET is most for capsules
via UpdateCapsule API.

> 2) If there are multiple capsule files and only one file (e.g. aaaa.cap) has
>   CAPSULE_FLAGS_INITIATE_RESET flag, should U-Boot resets after
>    applying that capsule, or wait after all capsule files are applied?
>    (current implementation is the latter one)

The order of capsules applied can be controlled by their file names.
So IIUC, a reset should be initiated immediately after processing
a capsule with INITIATE_RESET. The rest of capsules can be processed
even after the reboot.
I think that this behavior gives us more flexibility.

-Takahiro Akashi


> Thank you,
> 
> >
> > Regards,
> >
> > >
> > > -Takahiro Akashi
> > >
> > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > > ---
> > > >  lib/efi_loader/efi_capsule.c |   10 ++++++++--
> > > >  lib/fwu_updates/Kconfig      |    9 +++++++++
> > > >  2 files changed, 17 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > > index 83c89a0cbb..0928425b5f 100644
> > > > --- a/lib/efi_loader/efi_capsule.c
> > > > +++ b/lib/efi_loader/efi_capsule.c
> > > > @@ -1355,10 +1355,16 @@ efi_status_t efi_launch_capsules(void)
> > > >                       } else {
> > > >                               log_debug("Successfully updated the active_index\n");
> > > >                               status = fwu_trial_state_ctr_start();
> > > > -                             if (status < 0)
> > > > +                             if (status < 0) {
> > > >                                       ret = EFI_DEVICE_ERROR;
> > > > -                             else
> > > > +                             } else {
> > > >                                       ret = EFI_SUCCESS;
> > > > +                                     if (IS_ENABLED(CONFIG_FWU_REBOOT_AFTER_UPDATE)) {
> > > > +                                             log_info("New firmware is installed in bank#%d. Reboot from that bank.\n",
> > > > +                                                      update_index);
> > > > +                                             do_reset(NULL, 0, 0, NULL);
> > > > +                                     }
> > > > +                             }
> > > >                       }
> > > >               } else if (capsule_update == true && update_status == false) {
> > > >                       log_err("All capsules were not updated. Not updating FWU metadata\n");
> > > > diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig
> > > > index 6de28e0c9c..0940a90747 100644
> > > > --- a/lib/fwu_updates/Kconfig
> > > > +++ b/lib/fwu_updates/Kconfig
> > > > @@ -29,3 +29,12 @@ config FWU_TRIAL_STATE_CNT
> > > >         With FWU Multi Bank Update feature enabled, number of times
> > > >         the platform is allowed to boot in Trial State after an
> > > >         update.
> > > > +
> > > > +config FWU_REBOOT_AFTER_UPDATE
> > > > +     bool "Reboot soon after installing new firmware"
> > > > +     depends on FWU_MULTI_BANK_UPDATE
> > > > +     default y
> > > > +     help
> > > > +       Reboot the machine soon after installing a new firmware
> > > > +       and start trial boot. You can disable this option for
> > > > +       debugging or FWU development, but recommended to enable it.
> > > >
> >
> >
> >
> > --
> > Masami Hiramatsu
> 
> 
> 
> -- 
> Masami Hiramatsu