[PATCH v2 0/6] corstone1000: fwu metadata and GPT

[PATCH v2 0/6] corstone1000: fwu metadata and GPT

[Rui Miguel Silva]

Now that the nvmxip block driver is merged we can add on top
of it the platform code to use GPT and FWU metadata in the
Corstone1000.

But first, push 2 fixes that are needed to make all this work:
 - move nvmxip header to include
 - setup fwu metadata structures as packed (we have a 32bit
   writer - Secure enclave Cortex-M0 and a 64bit reader host
   Cortex-A35)

Cheers,
     Rui

v1 [0]-> v2:
Ilias:
    - add Reviewed-by tag in patch 1/6

Heinrich:
    - fix test include nvmxip header after the move to include/
        in patch 2/6

[0]: https://lore.kernel.org/u-boot/20230502131200.2551513-1-rui.silva@linaro.org/

Rui Miguel Silva (6):
  fwu_metadata: make sure structures are packed
  nvmxip: move header to include
  corstone1000: add fwu-metadata store info
  corstone1000: add boot index
  corstone1000: set kernel_addr based on boot_idx
  corstone1000: add nvmxip, fwu-mdata and gpt options

 arch/arm/dts/corstone1000.dtsi             |  7 ++-
 board/armltd/corstone1000/corstone1000.c   | 73 +++++++++++++++++++++-
 board/armltd/corstone1000/corstone1000.env | 10 +--
 configs/corstone1000_defconfig             | 14 ++++-
 include/fwu_mdata.h                        |  6 +-
 {drivers/mtd/nvmxip => include}/nvmxip.h   |  0
 test/dm/nvmxip.c                           |  2 +-
 7 files changed, 96 insertions(+), 16 deletions(-)
 rename {drivers/mtd/nvmxip => include}/nvmxip.h (100%)

-- 
2.40.1

[PATCH v2 1/6] fwu_metadata: make sure structures are packed

[Rui Miguel Silva]

The fwu metadata in the metadata partitions
should/are packed to guarantee that the info is
correct in all platforms. Also the size of them
are used to calculate the crc32 and that is important
to get it right.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 include/fwu_mdata.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/fwu_mdata.h b/include/fwu_mdata.h
index 8fda4f4ac225..c61221a91735 100644
--- a/include/fwu_mdata.h
+++ b/include/fwu_mdata.h
@@ -22,7 +22,7 @@ struct fwu_image_bank_info {
 	efi_guid_t  image_uuid;
 	uint32_t accepted;
 	uint32_t reserved;
-};
+} __packed;
 
 /**
  * struct fwu_image_entry - information for a particular type of image
@@ -38,7 +38,7 @@ struct fwu_image_entry {
 	efi_guid_t image_type_uuid;
 	efi_guid_t location_uuid;
 	struct fwu_image_bank_info img_bank_info[CONFIG_FWU_NUM_BANKS];
-};
+} __packed;
 
 /**
  * struct fwu_mdata - FWU metadata structure for multi-bank updates
@@ -62,6 +62,6 @@ struct fwu_mdata {
 	uint32_t previous_active_index;
 
 	struct fwu_image_entry img_entry[CONFIG_FWU_NUM_IMAGES_PER_BANK];
-};
+} __packed;
 
 #endif /* _FWU_MDATA_H_ */
-- 
2.40.1

[PATCH v2 2/6] nvmxip: move header to include

[Rui Miguel Silva]

Move header to include to allow external code
to get the internal bdev structures to access
block device operations.

as at it, just add the UCLASS_NVMXIP string
so we get the correct output in partitions
listing.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 {drivers/mtd/nvmxip => include}/nvmxip.h | 0
 test/dm/nvmxip.c                         | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename {drivers/mtd/nvmxip => include}/nvmxip.h (100%)

diff --git a/drivers/mtd/nvmxip/nvmxip.h b/include/nvmxip.h
similarity index 100%
rename from drivers/mtd/nvmxip/nvmxip.h
rename to include/nvmxip.h
diff --git a/test/dm/nvmxip.c b/test/dm/nvmxip.c
index e934748eb5d2..89bf481f6161 100644
--- a/test/dm/nvmxip.c
+++ b/test/dm/nvmxip.c
@@ -17,7 +17,7 @@
 #include <linux/bitops.h>
 #include <test/test.h>
 #include <test/ut.h>
-#include "../../drivers/mtd/nvmxip/nvmxip.h"
+#include <nvmxip.h>
 
 /* NVMXIP devices described in the device tree */
 #define SANDBOX_NVMXIP_DEVICES 2
-- 
2.40.1

[PATCH v2 3/6] corstone1000: add fwu-metadata store info

[Rui Miguel Silva]

Add fwu-mdata node and handle for the reference
nvmxip-qspi.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 arch/arm/dts/corstone1000.dtsi | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/corstone1000.dtsi b/arch/arm/dts/corstone1000.dtsi
index 533dfdf8e1ca..1e0ec075e4cd 100644
--- a/arch/arm/dts/corstone1000.dtsi
+++ b/arch/arm/dts/corstone1000.dtsi
@@ -38,7 +38,7 @@
 		reg = <0x88200000 0x77e00000>;
 	};
 
-	nvmxip-qspi@08000000 {
+	nvmxip: nvmxip-qspi@08000000 {
 		compatible = "nvmxip,qspi";
 		reg = <0x08000000 0x2000000>;
 		lba_shift = <9>;
@@ -106,6 +106,11 @@
 		method = "smc";
 	};
 
+	fwu-mdata {
+		compatible = "u-boot,fwu-mdata-gpt";
+		fwu-mdata-store = <&nvmxip>;
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;
-- 
2.40.1

[PATCH v2 4/6] corstone1000: add boot index

[Rui Miguel Silva]

it is expected that the firmware that runs before
u-boot somehow provide the information of the bank
for now we will fetch the info from the metadata
since the Secure enclave is the one responsible for
this information.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 board/armltd/corstone1000/corstone1000.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/board/armltd/corstone1000/corstone1000.c b/board/armltd/corstone1000/corstone1000.c
index 6ec8e6144fb4..1bead7a0a8b4 100644
--- a/board/armltd/corstone1000/corstone1000.c
+++ b/board/armltd/corstone1000/corstone1000.c
@@ -8,6 +8,8 @@
 #include <common.h>
 #include <cpu_func.h>
 #include <dm.h>
+#include <env.h>
+#include <env.h>
 #include <netdev.h>
 #include <dm/platform_data/serial_pl01x.h>
 #include <asm/armv8/mmu.h>
@@ -87,6 +89,20 @@ int dram_init_banksize(void)
 	return 0;
 }
 
-void reset_cpu(void)
+void fwu_plat_get_bootidx(uint *boot_idx)
 {
+	int ret;
+
+	/*
+	 * in our platform, the Secure Enclave is the one who controls
+	 * all the boot tries and status, so, every time we get here
+	 * we know that the we are booting from the active index
+	 */
+	ret = fwu_get_active_index(boot_idx);
+	if (ret < 0) {
+		*boot_idx = CONFIG_FWU_NUM_BANKS;
+		log_err("corstone1000: failed to read active index\n");
+	}
+
+	return ret;
 }
-- 
2.40.1

[PATCH v2 5/6] corstone1000: set kernel_addr based on boot_idx

[Rui Miguel Silva]

We need to distinguish between boot banks and from which
partition to load the kernel+initramfs to memory.

For that, fetch the boot index, fetch the correspondent
partition, calculate the correct kernel address and
then set the env variable kernel_addr with that value.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 board/armltd/corstone1000/corstone1000.c | 56 +++++++++++++++++++++++-
 configs/corstone1000_defconfig           |  1 +
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/board/armltd/corstone1000/corstone1000.c b/board/armltd/corstone1000/corstone1000.c
index 1bead7a0a8b4..a4567449f1be 100644
--- a/board/armltd/corstone1000/corstone1000.c
+++ b/board/armltd/corstone1000/corstone1000.c
@@ -5,16 +5,24 @@
  * Rui Miguel Silva <rui.silva@linaro.org>
  */
 
+#include <blk.h>
 #include <common.h>
 #include <cpu_func.h>
 #include <dm.h>
 #include <env.h>
-#include <env.h>
 #include <netdev.h>
+#include <nvmxip.h>
+#include <part.h>
 #include <dm/platform_data/serial_pl01x.h>
 #include <asm/armv8/mmu.h>
 #include <asm/global_data.h>
 
+#define CORSTONE1000_KERNEL_PARTS 2
+#define CORSTONE1000_KERNEL_PRIMARY "kernel_primary"
+#define CORSTONE1000_KERNEL_SECONDARY "kernel_secondary"
+
+static int corstone1000_boot_idx;
+
 static struct mm_region corstone1000_mem_map[] = {
 	{
 		/* CVM */
@@ -103,6 +111,52 @@ void fwu_plat_get_bootidx(uint *boot_idx)
 		*boot_idx = CONFIG_FWU_NUM_BANKS;
 		log_err("corstone1000: failed to read active index\n");
 	}
+}
+
+int board_late_init(void)
+{
+	struct disk_partition part_info;
+	struct udevice *dev, *bdev;
+	struct nvmxip_plat *plat;
+	struct blk_desc *desc;
+	int ret;
+
+	ret = uclass_first_device_err(UCLASS_NVMXIP, &dev);
+	if (ret < 0) {
+		log_err("Cannot find kernel device\n");
+		return ret;
+	}
+
+	plat = dev_get_plat(dev);
+	device_find_first_child(dev, &bdev);
+	desc = dev_get_uclass_plat(bdev);
+	ret = fwu_get_active_index(&corstone1000_boot_idx);
+	if (ret < 0) {
+		log_err("corstone1000: failed to read boot index\n");
+		return ret;
+	}
+
+	if (!corstone1000_boot_idx)
+		ret = part_get_info_by_name(desc, CORSTONE1000_KERNEL_PRIMARY,
+					    &part_info);
+	else
+		ret = part_get_info_by_name(desc, CORSTONE1000_KERNEL_SECONDARY,
+					    &part_info);
+
+	if (ret < 0) {
+		log_err("failed to fetch kernel partition index: %d\n",
+			corstone1000_boot_idx);
+		return ret;
+	}
+
+	ret = 0;
+
+	ret |= env_set_hex("kernel_addr", plat->phys_base +
+			   (part_info.start * part_info.blksz));
+	ret |= env_set_hex("kernel_size", part_info.size * part_info.blksz);
+
+	if (ret < 0)
+		log_err("failed to setup kernel addr and size\n");
 
 	return ret;
 }
diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig
index 2d391048cd67..5be5335bdfc1 100644
--- a/configs/corstone1000_defconfig
+++ b/configs/corstone1000_defconfig
@@ -20,6 +20,7 @@ CONFIG_CONSOLE_RECORD=y
 CONFIG_LOGLEVEL=7
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_BOARD_LATE_INIT=y
 CONFIG_SYS_MAXARGS=64
 CONFIG_SYS_CBSIZE=512
 # CONFIG_CMD_CONSOLE is not set
-- 
2.40.1

[PATCH v2 6/6] corstone1000: add nvmxip, fwu-mdata and gpt options

[Rui Miguel Silva]

Enable the newest features: nvmxip, fwu-metadata and
gpt. Commands to print the partition info, gpt info
and fwu metadata will be available.

Adjust also env boot script the address of the
bootbank with the new gpt layout, and also remove
the not needed kernel address bank0 and bank1
and retrieve function that would test the bank flag
before and now we are getting the info from the fwu
metadata.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 board/armltd/corstone1000/corstone1000.c   |  1 +
 board/armltd/corstone1000/corstone1000.env | 10 +---------
 configs/corstone1000_defconfig             | 13 ++++++++++++-
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/board/armltd/corstone1000/corstone1000.c b/board/armltd/corstone1000/corstone1000.c
index a4567449f1be..01c80aaf9d77 100644
--- a/board/armltd/corstone1000/corstone1000.c
+++ b/board/armltd/corstone1000/corstone1000.c
@@ -10,6 +10,7 @@
 #include <cpu_func.h>
 #include <dm.h>
 #include <env.h>
+#include <fwu.h>
 #include <netdev.h>
 #include <nvmxip.h>
 #include <part.h>
diff --git a/board/armltd/corstone1000/corstone1000.env b/board/armltd/corstone1000/corstone1000.env
index b24ff07fc6bd..ee318b1b1c30 100644
--- a/board/armltd/corstone1000/corstone1000.env
+++ b/board/armltd/corstone1000/corstone1000.env
@@ -1,13 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 
 usb_pgood_delay=250
-boot_bank_flag=0x08002000
-kernel_addr_bank_0=0x083EE000
-kernel_addr_bank_1=0x0936E000
-retrieve_kernel_load_addr=
-	if itest.l *${boot_bank_flag} == 0; then
-		setenv kernel_addr $kernel_addr_bank_0;
-	else
-		setenv kernel_addr $kernel_addr_bank_1;
-	fi;
+boot_bank_flag=0x08005006
 kernel_addr_r=0x88200000
diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig
index 5be5335bdfc1..a8a79fd10568 100644
--- a/configs/corstone1000_defconfig
+++ b/configs/corstone1000_defconfig
@@ -15,7 +15,7 @@ CONFIG_DISTRO_DEFAULTS=y
 CONFIG_BOOTDELAY=3
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyAMA0 loglevel=9 ip=dhcp earlyprintk"
-CONFIG_BOOTCOMMAND="run retrieve_kernel_load_addr; echo Loading kernel from $kernel_addr to memory ... ; loadm $kernel_addr $kernel_addr_r 0xc00000; usb start; usb reset; run distro_bootcmd; bootefi $kernel_addr_r $fdtcontroladdr;"
+CONFIG_BOOTCOMMAND="echo Loading kernel from $kernel_addr to memory ... ; loadm $kernel_addr $kernel_addr_r 0xc00000; usb start; usb reset; run distro_bootcmd; bootefi $kernel_addr_r $fdtcontroladdr;"
 CONFIG_CONSOLE_RECORD=y
 CONFIG_LOGLEVEL=7
 # CONFIG_DISPLAY_CPUINFO is not set
@@ -24,11 +24,16 @@ CONFIG_BOARD_LATE_INIT=y
 CONFIG_SYS_MAXARGS=64
 CONFIG_SYS_CBSIZE=512
 # CONFIG_CMD_CONSOLE is not set
+CONFIG_CMD_FWU_METADATA=y
 CONFIG_CMD_BOOTZ=y
 CONFIG_SYS_BOOTM_LEN=0x800000
 # CONFIG_CMD_XIMG is not set
+CONFIG_CMD_NVMXIP=y
+CONFIG_CMD_GPT=y
+# CONFIG_RANDOM_UUID is not set
 CONFIG_CMD_LOADM=y
 # CONFIG_CMD_LOADS is not set
+CONFIG_CMD_MMC=y
 CONFIG_CMD_USB=y
 # CONFIG_CMD_SETEXPR is not set
 # CONFIG_CMD_NFS is not set
@@ -40,6 +45,8 @@ CONFIG_OF_CONTROL=y
 CONFIG_VERSION_VARIABLE=y
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_REGMAP=y
+CONFIG_FWU_MDATA=y
+CONFIG_FWU_MDATA_GPT_BLK=y
 CONFIG_MISC=y
 # CONFIG_MMC is not set
 CONFIG_NVMXIP_QSPI=y
@@ -51,6 +58,10 @@ CONFIG_RAM=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_EMULATION=y
 CONFIG_DM_SERIAL=y
+CONFIG_SYSRESET=y
 CONFIG_USB=y
 CONFIG_USB_ISP1760=y
+CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_IGNORE_OSINDICATIONS=y
+CONFIG_FWU_MULTI_BANK_UPDATE=y
 CONFIG_ERRNO_STR=y
-- 
2.40.1

Re: [PATCH v2 0/6] corstone1000: fwu metadata and GPT

[Rui Miguel Silva]

Hi all,
Rui Miguel Silva <rui.silva@linaro.org> writes:

> Now that the nvmxip block driver is merged we can add on top
> of it the platform code to use GPT and FWU metadata in the
> Corstone1000.
>
> But first, push 2 fixes that are needed to make all this work:
>  - move nvmxip header to include
>  - setup fwu metadata structures as packed (we have a 32bit
>    writer - Secure enclave Cortex-M0 and a 64bit reader host
>    Cortex-A35)
>
> Cheers,
>      Rui
>
> v1 [0]-> v2:
> Ilias:
>     - add Reviewed-by tag in patch 1/6
>
> Heinrich:
>     - fix test include nvmxip header after the move to include/
>         in patch 2/6

Any other feedback on this version?
Thanks in advance.

Cheers,
   Rui
   
>
> [0]: https://lore.kernel.org/u-boot/20230502131200.2551513-1-rui.silva@linaro.org/
>
> Rui Miguel Silva (6):
>   fwu_metadata: make sure structures are packed
>   nvmxip: move header to include
>   corstone1000: add fwu-metadata store info
>   corstone1000: add boot index
>   corstone1000: set kernel_addr based on boot_idx
>   corstone1000: add nvmxip, fwu-mdata and gpt options
>
>  arch/arm/dts/corstone1000.dtsi             |  7 ++-
>  board/armltd/corstone1000/corstone1000.c   | 73 +++++++++++++++++++++-
>  board/armltd/corstone1000/corstone1000.env | 10 +--
>  configs/corstone1000_defconfig             | 14 ++++-
>  include/fwu_mdata.h                        |  6 +-
>  {drivers/mtd/nvmxip => include}/nvmxip.h   |  0
>  test/dm/nvmxip.c                           |  2 +-
>  7 files changed, 96 insertions(+), 16 deletions(-)
>  rename {drivers/mtd/nvmxip => include}/nvmxip.h (100%)
>
> -- 
> 2.40.1

Re: [PATCH v2 0/6] corstone1000: fwu metadata and GPT

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

On Mon, May 29, 2023 at 03:02:23PM +0100, Rui Miguel Silva wrote:

> Now that the nvmxip block driver is merged we can add on top
> of it the platform code to use GPT and FWU metadata in the
> Corstone1000.
> 
> But first, push 2 fixes that are needed to make all this work:
>  - move nvmxip header to include
>  - setup fwu metadata structures as packed (we have a 32bit
>    writer - Secure enclave Cortex-M0 and a 64bit reader host
>    Cortex-A35)
> 
> Cheers,
>      Rui
> 
> v1 [0]-> v2:
> Ilias:
>     - add Reviewed-by tag in patch 1/6
> 
> Heinrich:
>     - fix test include nvmxip header after the move to include/
>         in patch 2/6
> 
> [0]: https://lore.kernel.org/u-boot/20230502131200.2551513-1-rui.silva@linaro.org/
> 
> Rui Miguel Silva (6):
>   fwu_metadata: make sure structures are packed

This patch leads to build failures now, please rebase on top of current
-next, thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]