From 14b866e6d650645881cac041db64f67158ced24e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 13:22:03 +0100
Subject: tools: kwbimage: Fix generating, verifying and extracting SDIO
 kwbimage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Despite the official specification, Marvell BootROM does not interpret
srcaddr from SDIO image as offset in number of sectors (like for SATA
image), but as offset in bytes (like for all other images except SATA).

To generate SDIO kwbimage compatible with Marvell BootROM, it is needed to
have srcaddr in bytes. This change fixes SDIO images for Armada 38x SoCs.

Fixes: 501a54a29cc2 ("tools: kwbimage: Fix generation of SATA, SDIO and PCIe images")
Fixes: 5c61710c9880 ("tools: kwbimage: Properly set srcaddr in kwbimage v0")
Fixes: e0c243c398a7 ("tools: kwbimage: Validate data checksum of v1 images")
Fixes: aa6943ca3122 ("kwbimage: Add support for extracting images via dumpimage tool")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 6abb9f2d5c0..09d52d47652 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -1021,15 +1021,6 @@ static void *image_create_v0(size_t *imagesz, struct image_tool_params *params,
 	if (main_hdr->blockid == IBR_HDR_SATA_ID)
 		main_hdr->srcaddr = cpu_to_le32(headersz / 512 + 1);
 
-	/*
-	 * For SDIO srcaddr is specified in number of sectors starting from
-	 * sector 0. The main header is stored at sector number 0.
-	 * This expects sector size to be 512 bytes.
-	 * Header size is already aligned.
-	 */
-	if (main_hdr->blockid == IBR_HDR_SDIO_ID)
-		main_hdr->srcaddr = cpu_to_le32(headersz / 512);
-
 	/* For PCIe srcaddr is not used and must be set to 0xFFFFFFFF. */
 	if (main_hdr->blockid == IBR_HDR_PEX_ID)
 		main_hdr->srcaddr = cpu_to_le32(0xFFFFFFFF);
@@ -1478,15 +1469,6 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
 	if (main_hdr->blockid == IBR_HDR_SATA_ID)
 		main_hdr->srcaddr = cpu_to_le32(headersz / 512 + 1);
 
-	/*
-	 * For SDIO srcaddr is specified in number of sectors starting from
-	 * sector 0. The main header is stored at sector number 0.
-	 * This expects sector size to be 512 bytes.
-	 * Header size is already aligned.
-	 */
-	if (main_hdr->blockid == IBR_HDR_SDIO_ID)
-		main_hdr->srcaddr = cpu_to_le32(headersz / 512);
-
 	/* For PCIe srcaddr is not used and must be set to 0xFFFFFFFF. */
 	if (main_hdr->blockid == IBR_HDR_PEX_ID)
 		main_hdr->srcaddr = cpu_to_le32(0xFFFFFFFF);
@@ -2039,14 +2021,6 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size,
 		offset *= 512;
 	}
 
-	/*
-	 * For SDIO srcaddr is specified in number of sectors.
-	 * This expects that sector size is 512 bytes and recalculates
-	 * data offset to bytes.
-	 */
-	if (blockid == IBR_HDR_SDIO_ID)
-		offset *= 512;
-
 	/*
 	 * For PCIe srcaddr is always set to 0xFFFFFFFF.
 	 * This expects that data starts after all headers.
@@ -2408,9 +2382,6 @@ static int kwbimage_extract_subimage(void *ptr, struct image_tool_params *params
 			offset *= 512;
 		}
 
-		if (mhdr->blockid == IBR_HDR_SDIO_ID)
-			offset *= 512;
-
 		if (mhdr->blockid == IBR_HDR_PEX_ID && offset == 0xFFFFFFFF)
 			offset = header_size;
 
-- 
cgit v1.2.3


From 8562a1c6a4572550f752d4deca95d9efdd9b5265 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 13:20:20 +0100
Subject: tools: kwboot: Fix parsing SDIO kwbimage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Despite the official specification, Marvell BootROM does not interpret
srcaddr from SDIO image as offset in number of sectors (like for SATA
image), but as offset in bytes (like for all other images except SATA).

To parse SDIO kwbimage in the same way as Marvell BootROM, it is needed to
interpret srcaddr in bytes. This change fixes loading of SDIO images via
kwboot over UART.

Fixes: 792e42355083 ("tools: kwboot: Patch source address in image header")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 4 ----
 1 file changed, 4 deletions(-)

(limited to 'tools')

diff --git a/tools/kwboot.c b/tools/kwboot.c
index da4fe32da22..188f944263f 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1894,10 +1894,6 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
 		hdr->srcaddr = cpu_to_le32((srcaddr - 1) * 512);
 		break;
 
-	case IBR_HDR_SDIO_ID:
-		hdr->srcaddr = cpu_to_le32(srcaddr * 512);
-		break;
-
 	case IBR_HDR_PEX_ID:
 		if (srcaddr == 0xFFFFFFFF)
 			hdr->srcaddr = cpu_to_le32(hdrsz);
-- 
cgit v1.2.3


From 954c94aaccf825d0142b3a36ff65f46721f4c733 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 21 Jan 2023 13:34:55 +0100
Subject: tools: kwbimage: Fix generating, verifying and extracting SATA
 kwbimage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Despite the official specification, Marvell BootROM does not interpret
srcaddr from SATA image as number of sectors the beginning of the hard
drive, but as number of sectors relative to the main header.

The main header is stored at absolute sector number 1. So do not add or
subtract it when calculating with relative offsets to the main header.

Fixes: 501a54a29cc2 ("tools: kwbimage: Fix generation of SATA, SDIO and PCIe images")
Fixes: 5c61710c9880 ("tools: kwbimage: Properly set srcaddr in kwbimage v0")
Fixes: e0c243c398a7 ("tools: kwbimage: Validate data checksum of v1 images")
Fixes: aa6943ca3122 ("kwbimage: Add support for extracting images via dumpimage tool")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 09d52d47652..67b45503e46 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -1013,13 +1013,12 @@ static void *image_create_v0(size_t *imagesz, struct image_tool_params *params,
 					     sizeof(struct main_hdr_v0));
 
 	/*
-	 * For SATA srcaddr is specified in number of sectors starting from
-	 * sector 0. The main header is stored at sector number 1.
+	 * For SATA srcaddr is specified in number of sectors.
 	 * This expects the sector size to be 512 bytes.
 	 * Header size is already aligned.
 	 */
 	if (main_hdr->blockid == IBR_HDR_SATA_ID)
-		main_hdr->srcaddr = cpu_to_le32(headersz / 512 + 1);
+		main_hdr->srcaddr = cpu_to_le32(headersz / 512);
 
 	/* For PCIe srcaddr is not used and must be set to 0xFFFFFFFF. */
 	if (main_hdr->blockid == IBR_HDR_PEX_ID)
@@ -1461,13 +1460,12 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
 		main_hdr->flags = e->debug ? 0x1 : 0;
 
 	/*
-	 * For SATA srcaddr is specified in number of sectors starting from
-	 * sector 0. The main header is stored at sector number 1.
+	 * For SATA srcaddr is specified in number of sectors.
 	 * This expects the sector size to be 512 bytes.
 	 * Header size is already aligned.
 	 */
 	if (main_hdr->blockid == IBR_HDR_SATA_ID)
-		main_hdr->srcaddr = cpu_to_le32(headersz / 512 + 1);
+		main_hdr->srcaddr = cpu_to_le32(headersz / 512);
 
 	/* For PCIe srcaddr is not used and must be set to 0xFFFFFFFF. */
 	if (main_hdr->blockid == IBR_HDR_PEX_ID)
@@ -2010,16 +2008,10 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size,
 
 	/*
 	 * For SATA srcaddr is specified in number of sectors.
-	 * The main header is must be stored at sector number 1.
-	 * This expects that sector size is 512 bytes and recalculates
-	 * data offset to bytes relative to the main header.
+	 * This expects that sector size is 512 bytes.
 	 */
-	if (blockid == IBR_HDR_SATA_ID) {
-		if (offset < 1)
-			return -FDT_ERR_BADSTRUCTURE;
-		offset -= 1;
+	if (blockid == IBR_HDR_SATA_ID)
 		offset *= 512;
-	}
 
 	/*
 	 * For PCIe srcaddr is always set to 0xFFFFFFFF.
@@ -2377,10 +2369,8 @@ static int kwbimage_extract_subimage(void *ptr, struct image_tool_params *params
 		/* Extract data image when -p is not specified or when '-p 0' is specified */
 		offset = le32_to_cpu(mhdr->srcaddr);
 
-		if (mhdr->blockid == IBR_HDR_SATA_ID) {
-			offset -= 1;
+		if (mhdr->blockid == IBR_HDR_SATA_ID)
 			offset *= 512;
-		}
 
 		if (mhdr->blockid == IBR_HDR_PEX_ID && offset == 0xFFFFFFFF)
 			offset = header_size;
-- 
cgit v1.2.3


From e1c4ed57d5190e3064ae10ae3a87cdc75d2786fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 21 Jan 2023 13:45:36 +0100
Subject: tools: kwboot: Fix parsing SATA kwbimage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Despite the official specification, Marvell BootROM does not interpret
srcaddr from SATA image as number of sectors the beginning of the hard
drive, but as number of sectors relative to the main header.

To parse SATA kwbimage in the same way as Marvell BootROM, it is needed to
interpret srcaddr as relative offset to the main header. This change fixes
loading of SATA images via kwboot over UART.

Fixes: 792e42355083 ("tools: kwboot: Patch source address in image header")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

(limited to 'tools')

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 188f944263f..bf410520de6 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1888,10 +1888,7 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
 
 	switch (hdr->blockid) {
 	case IBR_HDR_SATA_ID:
-		if (srcaddr < 1)
-			goto err;
-
-		hdr->srcaddr = cpu_to_le32((srcaddr - 1) * 512);
+		hdr->srcaddr = cpu_to_le32(srcaddr * 512);
 		break;
 
 	case IBR_HDR_PEX_ID:
-- 
cgit v1.2.3


From 29b92bb790a87b35b361321a84c0e48b808a2556 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 13:34:24 +0100
Subject: tools: kwboot: Add more documentation references
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add reference to Avanta Boot Flow documentation, BobCat2, AlleyCat3 and
PONCat3 BootROM Firmware documentation and links to public Marvell tools:
hdrparser.c and doimage.c

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

(limited to 'tools')

diff --git a/tools/kwboot.c b/tools/kwboot.c
index bf410520de6..aae7393aeef 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -15,6 +15,12 @@
  *   Processor, and High-Definition Video Decoder: Functional Specifications"
  *   August 3, 2011. Chapter 5 "BootROM Firmware"
  *   https://web.archive.org/web/20120130172443/https://www.marvell.com/application-processors/armada-500/assets/Armada-510-Functional-Spec.pdf
+ * - "88F6665, 88F6660, 88F6658, 88F6655, 88F6655F, 88F6650, 88F6650F, 88F6610,
+ *   and 88F6610F Avanta LP Family Integrated Single/Dual CPU Ecosystem for
+ *   Gateway (GW), Home Gateway Unit (HGU), and Single Family Unit (SFU)
+ *   Functional Specifications" Doc. No. MV-S108952-00, Rev. A. November 7, 2013.
+ *   Chapter 7 "Boot Flow"
+ *   CONFIDENTIAL, no public documentation available
  * - "88F6710, 88F6707, and 88F6W11: ARMADA(R) 370 SoC: Functional Specifications"
  *   May 26, 2014. Chapter 6 "BootROM Firmware".
  *   https://web.archive.org/web/20140617183701/https://www.marvell.com/embedded-processors/armada-300/assets/ARMADA370-FunctionalSpec-datasheet.pdf
@@ -22,6 +28,15 @@
  *   Multi-Core ARMv7 Based SoC Processors: Functional Specifications"
  *   May 29, 2014. Chapter 6 "BootROM Firmware".
  *   https://web.archive.org/web/20180829171131/https://www.marvell.com/embedded-processors/armada-xp/assets/ARMADA-XP-Functional-SpecDatasheet.pdf
+ * - "BobCat2 Control and Management Subsystem Functional Specifications"
+ *   Doc. No. MV-S109400-00, Rev. A. December 4, 2014.
+ *   Chapter 1.6 BootROM Firmware
+ *   CONFIDENTIAL, no public documentation available
+ * - "AlleyCat3 and PONCat3 Highly Integrated 1/10 Gigabit Ethernet Switch
+ *   Control and Management Subsystem: Functional Specifications"
+ *   Doc. No. MV-S109693-00, Rev. A. May 20, 2014.
+ *   Chapter 1.6 BootROM Firmware
+ *   CONFIDENTIAL, no public documentation available
  * - "ARMADA(R) 375 Value-Performance Dual Core CPU System on Chip: Functional
  *   Specifications" Doc. No. MV-S109377-00, Rev. A. September 18, 2013.
  *   Chapter 7 "Boot Sequence"
@@ -35,6 +50,10 @@
  *   System on Chip Functional Specifications" Doc. No. MV-S109896-00, Rev. B.
  *   December 22, 2015. Chapter 7 "Boot Flow"
  *   CONFIDENTIAL, no public documentation available
+ * - "Marvell boot image parser", Marvell U-Boot 2013.01, version 18.06. September 17, 2015.
+ *   https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/blob/u-boot-2013.01-armada-18.06/tools/marvell/doimage_mv/hdrparser.c
+ * - "Marvell doimage Tool", Marvell U-Boot 2013.01, version 18.06. August 30, 2015.
+ *   https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/blob/u-boot-2013.01-armada-18.06/tools/marvell/doimage_mv/doimage.c
  */
 
 #include "kwbimage.h"
-- 
cgit v1.2.3


From fa03279e198d220d05898e2d35a139fd599b4acf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 21 Jan 2023 19:57:28 +0100
Subject: tools: kwboot: Add image type documentation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add information of all available image types and where they should be
stored. Storage location offsets where documented from the disassembly of
the A385 BootROM image dump.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

(limited to 'tools')

diff --git a/tools/kwboot.c b/tools/kwboot.c
index aae7393aeef..7a7dd5bf3d7 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -54,6 +54,68 @@
  *   https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/blob/u-boot-2013.01-armada-18.06/tools/marvell/doimage_mv/hdrparser.c
  * - "Marvell doimage Tool", Marvell U-Boot 2013.01, version 18.06. August 30, 2015.
  *   https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/blob/u-boot-2013.01-armada-18.06/tools/marvell/doimage_mv/doimage.c
+ *
+ * Storage location / offset of different image types:
+ * - IBR_HDR_SPI_ID (0x5A):
+ *   SPI image can be stored at any 2 MB aligned offset in the first 16 MB of
+ *   SPI-NOR or parallel-NOR. Despite the type name it really can be stored on
+ *   parallel-NOR and cannot be stored on other SPI devices, like SPI-NAND.
+ *   So it should have been named NOR image, not SPI image. This image type
+ *   supports XIP - Execute In Place directly from NOR memory.
+ *
+ * - IBR_HDR_NAND_ID (0x8B):
+ *   NAND image can be stored either at any 2 MB aligned offset in the first
+ *   16 MB of SPI-NAND or at any blocksize aligned offset in the first 64 MB
+ *   of parallel-NAND.
+ *
+ * - IBR_HDR_PEX_ID (0x9C):
+ *   PEX image is used for booting from PCI Express device. Source address
+ *   stored in image is ignored by BootROM. It is not the BootROM who parses
+ *   or loads data part of the PEX image. BootROM just configures SoC to the
+ *   PCIe endpoint mode and let the PCIe device on the other end of the PCIe
+ *   link (which must be in Root Complex mode) to load kwbimage into SoC's
+ *   memory and tell BootROM physical address.
+ *
+ * - IBR_HDR_UART_ID (0x69):
+ *   UART image can be transfered via xmodem protocol over first UART.
+ *
+ * - IBR_HDR_I2C_ID (0x4D):
+ *   It is unknown for what kind of storage is used this image. It is not
+ *   specified in any document from References section.
+ *
+ * - IBR_HDR_SATA_ID (0x78):
+ *   SATA image can be stored at sector 1 (after the MBR table), sector 34
+ *   (after the GPT table) or at any next sector which is aligned to 2 MB and
+ *   is in the first 16 MB of SATA disk. Note that source address in SATA image
+ *   is stored in sector unit and not in bytes like for any other images.
+ *   Unfortunately sector size is disk specific, in most cases it is 512 bytes
+ *   but there are also Native 4K SATA disks which have 4096 bytes long sectors.
+ *
+ * - IBR_HDR_SDIO_ID (0xAE):
+ *   SDIO image can be stored on different medias:
+ *   - SD(SC) card
+ *   - SDHC/SDXC card
+ *   - eMMC HW boot partition
+ *   - eMMC user data partition / MMC card
+ *   It cannot be stored on SDIO card despite the image name.
+ *
+ *   For SD(SC)/SDHC/SDXC cards, image can be stored at the same locations as
+ *   the SATA image (sector 1, sector 34 or any 2 MB aligned sector) but within
+ *   the first 64 MB. SDHC and SDXC cards have fixed 512 bytes long sector size.
+ *   Old SD(SC) cards unfortunately can have also different sector sizes, mostly
+ *   1024 bytes long sector sizes and also can be changed at runtime.
+ *
+ *   For MMC-compatible devices, image can be stored at offset 0 or at offset
+ *   2 MB. If MMC device supports HW boot partitions then image must be stored
+ *   on the HW partition as is configured in the EXT_CSC register (it can be
+ *   either boot or user data).
+ *
+ *   Note that source address for SDIO image is stored in byte unit, like for
+ *   any other images (except SATA). Marvell Functional Specifications for
+ *   A38x and A39x SoCs say that source address is in sector units, but this
+ *   is purely incorrect information. A385 BootROM really expects source address
+ *   for SDIO images in bytes and also Marvell tools generate SDIO image with
+ *   source address in byte units.
  */
 
 #include "kwbimage.h"
-- 
cgit v1.2.3


From 7665ed2fa04e0726e142e13bcd77b738e912357f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 13:38:27 +0100
Subject: tools: kwboot: Fix parsing UART image without data checksum
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The 32-bit data checksum in UART image is not checked by the BootROM and
also Marvell tools do not generate it.

So if data checksum stored in UART image does not match calculated checksum
from the image then treat those checksum bytes as part of the executable
image code (and not as the checksum) and for compatibility with the rest of
the code manually insert data checksum into the in-memory image after the
executable code, without overwriting it.

This should allow to boot UART images generated by Marvell tools.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

(limited to 'tools')

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 7a7dd5bf3d7..da840864b56 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1990,8 +1990,18 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
 	    *size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize))
 		goto err;
 
-	if (kwboot_img_csum32(img) != *kwboot_img_csum32_ptr(img))
-		goto err;
+	/*
+	 * The 32-bit data checksum is optional for UART image. If it is not
+	 * present (checksum detected as invalid) then grow data part of the
+	 * image for the checksum, so it can be inserted there.
+	 */
+	if (kwboot_img_csum32(img) != *kwboot_img_csum32_ptr(img)) {
+		if (hdr->blockid != IBR_HDR_UART_ID) {
+			fprintf(stderr, "Image has invalid data checksum\n");
+			goto err;
+		}
+		kwboot_img_grow_data_right(img, size, sizeof(uint32_t));
+	}
 
 	is_secure = kwboot_img_is_secure(img);
 
@@ -2256,6 +2266,7 @@ main(int argc, char **argv)
 				 KWBOOT_XM_BLKSZ +
 				 sizeof(kwboot_baud_code) +
 				 sizeof(kwboot_baud_code_data_jump) +
+				 sizeof(uint32_t) +
 				 KWBOOT_XM_BLKSZ;
 
 	if (imgpath) {
-- 
cgit v1.2.3


From 53ee6ec82744666719f2c9954a013c4397b77be9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 13:42:07 +0100
Subject: tools: kwboot: Validate optional kwbimage v1 headers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before starting parsing of kwbimage, first validate that all optional v1
headers and correct. This prevents kwboot crashes on invalid input.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 8 ++++++++
 1 file changed, 8 insertions(+)

(limited to 'tools')

diff --git a/tools/kwboot.c b/tools/kwboot.c
index da840864b56..c8c7a8d2465 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1939,6 +1939,7 @@ static int
 kwboot_img_patch(void *img, size_t *size, int baudrate)
 {
 	struct main_hdr_v1 *hdr;
+	struct opt_hdr_v1 *ohdr;
 	uint32_t srcaddr;
 	uint8_t csum;
 	size_t hdrsz;
@@ -1990,6 +1991,13 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
 	    *size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize))
 		goto err;
 
+	for_each_opt_hdr_v1 (ohdr, hdr) {
+		if (!opt_hdr_v1_valid_size(ohdr, (const uint8_t *)hdr + hdrsz)) {
+			fprintf(stderr, "Invalid optional image header\n");
+			goto err;
+		}
+	}
+
 	/*
 	 * The 32-bit data checksum is optional for UART image. If it is not
 	 * present (checksum detected as invalid) then grow data part of the
-- 
cgit v1.2.3


From a190667b111bd2731a8cef173c0e84e14fb14218 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 13:46:14 +0100
Subject: tools: kwboot: Add check that kwbimage contains DDR init code
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Some NOR images may be execute-in-place and do not contain DDR init code in
its kwbimage header. Such images cannot be booted over UART as BootROM
loads them to RAM. Add check that kwbimage contains DDR init code in its
header (either as binary code header or as the simple register-value set).

In some cases it is possible to load very small image into L2SRAM and when
DDR init code is not required. So check for L2SRAM load address and skip
DDR init code check in this case.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

(limited to 'tools')

diff --git a/tools/kwboot.c b/tools/kwboot.c
index c8c7a8d2465..f624edc7798 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1780,6 +1780,47 @@ kwboot_img_is_secure(void *img)
 	return 0;
 }
 
+static int
+kwboot_img_has_ddr_init(void *img)
+{
+	const struct register_set_hdr_v1 *rhdr;
+	const struct main_hdr_v0 *hdr0;
+	struct opt_hdr_v1 *ohdr;
+	u32 ohdrsz;
+	int last;
+
+	/*
+	 * kwbimage v0 image headers contain DDR init code either in
+	 * extension header or in binary code header.
+	 */
+	if (kwbimage_version(img) == 0) {
+		hdr0 = img;
+		return hdr0->ext || hdr0->bin;
+	}
+
+	/*
+	 * kwbimage v1 image headers contain DDR init code either in binary
+	 * code header or in a register set list header with SDRAM_SETUP.
+	 */
+	for_each_opt_hdr_v1 (ohdr, img) {
+		if (ohdr->headertype == OPT_HDR_V1_BINARY_TYPE)
+			return 1;
+		if (ohdr->headertype == OPT_HDR_V1_REGISTER_TYPE) {
+			rhdr = (const struct register_set_hdr_v1 *)ohdr;
+			ohdrsz = opt_hdr_v1_size(ohdr);
+			if (ohdrsz >= sizeof(*ohdr) + sizeof(rhdr->data[0].last_entry)) {
+				ohdrsz -= sizeof(*ohdr) + sizeof(rhdr->data[0].last_entry);
+				last = ohdrsz / sizeof(rhdr->data[0].entry);
+				if (rhdr->data[last].last_entry.delay ==
+				    REGISTER_SET_HDR_OPT_DELAY_SDRAM_SETUP)
+					return 1;
+			}
+		}
+	}
+
+	return 0;
+}
+
 static void *
 kwboot_img_grow_data_right(void *img, size_t *size, size_t grow)
 {
@@ -2011,6 +2052,13 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
 		kwboot_img_grow_data_right(img, size, sizeof(uint32_t));
 	}
 
+	if (!kwboot_img_has_ddr_init(img) &&
+	    (le32_to_cpu(hdr->destaddr) < 0x40000000 ||
+	     le32_to_cpu(hdr->destaddr) + le32_to_cpu(hdr->blocksize) > 0x40034000)) {
+		fprintf(stderr, "Image does not contain DDR init code needed for UART booting\n");
+		goto err;
+	}
+
 	is_secure = kwboot_img_is_secure(img);
 
 	if (hdr->blockid != IBR_HDR_UART_ID) {
-- 
cgit v1.2.3


From 7bfc15efa78483ccdf6254154b8145c4d3e49454 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 21 Jan 2023 12:59:20 +0100
Subject: tools: kwboot: Fix patching of SPI/NOR XIP images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Marvell BootROM interprets execaddr of SPI/NOR XIP images as relative byte
offset from the from the beginning of the flash device. So if data image
offset and execute offset are not same then it is needed to adjust them
also in DDR RAM.

Fixes: f2c644e0b8bc ("tools: kwboot: Patch destination address to DDR area for SPI image")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'tools')

diff --git a/tools/kwboot.c b/tools/kwboot.c
index f624edc7798..cb31d5b858c 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -2022,8 +2022,8 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
 	case IBR_HDR_SPI_ID:
 		if (hdr->destaddr == cpu_to_le32(0xFFFFFFFF)) {
 			kwboot_printv("Patching destination and execution addresses from SPI/NOR XIP area to DDR area 0x00800000\n");
-			hdr->destaddr = cpu_to_le32(0x00800000);
-			hdr->execaddr = cpu_to_le32(0x00800000);
+			hdr->destaddr = cpu_to_le32(0x00800000 + le32_to_cpu(hdr->srcaddr));
+			hdr->execaddr = cpu_to_le32(0x00800000 + le32_to_cpu(hdr->execaddr));
 		}
 		break;
 	}
-- 
cgit v1.2.3


From 5b039dced38162f21fa078e65b0f5fc733439fac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Tue, 10 Jan 2023 22:33:56 +0100
Subject: tools: kwboot: Show image type and error parsing reasons
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Show image type and version during parsing of kwbimage.
And show reasons in error messages when parsing failed.
This can help to debug issues with invalid images.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

(limited to 'tools')

diff --git a/tools/kwboot.c b/tools/kwboot.c
index cb31d5b858c..7c666486f31 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1976,6 +1976,21 @@ _inject_baudrate_change_code(void *img, size_t *size, int for_data,
 	}
 }
 
+static const char *
+kwboot_img_type(uint8_t blockid)
+{
+	switch (blockid) {
+	case IBR_HDR_I2C_ID: return "I2C";
+	case IBR_HDR_SPI_ID: return "SPI";
+	case IBR_HDR_NAND_ID: return "NAND";
+	case IBR_HDR_SATA_ID: return "SATA";
+	case IBR_HDR_PEX_ID: return "PEX";
+	case IBR_HDR_UART_ID: return "UART";
+	case IBR_HDR_SDIO_ID: return "SDIO";
+	default: return "unknown";
+	}
+}
+
 static int
 kwboot_img_patch(void *img, size_t *size, int baudrate)
 {
@@ -1989,8 +2004,10 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
 
 	hdr = img;
 
-	if (*size < sizeof(struct main_hdr_v1))
+	if (*size < sizeof(struct main_hdr_v1)) {
+		fprintf(stderr, "Invalid image header size\n");
 		goto err;
+	}
 
 	image_ver = kwbimage_version(img);
 	if (image_ver != 0 && image_ver != 1) {
@@ -2000,12 +2017,18 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
 
 	hdrsz = kwbheader_size(hdr);
 
-	if (*size < hdrsz)
+	if (*size < hdrsz) {
+		fprintf(stderr, "Invalid image header size\n");
 		goto err;
+	}
+
+	kwboot_printv("Detected kwbimage v%d with %s boot signature\n", image_ver, kwboot_img_type(hdr->blockid));
 
 	csum = kwboot_hdr_csum8(hdr) - hdr->checksum;
-	if (csum != hdr->checksum)
+	if (csum != hdr->checksum) {
+		fprintf(stderr, "Image has invalid header checksum stored in image header\n");
 		goto err;
+	}
 
 	srcaddr = le32_to_cpu(hdr->srcaddr);
 
@@ -2028,9 +2051,15 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
 		break;
 	}
 
-	if (hdrsz > le32_to_cpu(hdr->srcaddr) ||
-	    *size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize))
+	if (hdrsz > le32_to_cpu(hdr->srcaddr)) {
+		fprintf(stderr, "Image has invalid data offset stored in image header\n");
+		goto err;
+	}
+
+	if (*size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize)) {
+		fprintf(stderr, "Image has invalid data size stored in image header\n");
 		goto err;
+	}
 
 	for_each_opt_hdr_v1 (ohdr, hdr) {
 		if (!opt_hdr_v1_valid_size(ohdr, (const uint8_t *)hdr + hdrsz)) {
-- 
cgit v1.2.3


From 908801dcfe8a1e45b5e6e37c4b7e43f79dfc36ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 13:53:48 +0100
Subject: tools: kwbimage: Fix dumping register set / DATA commands
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Upper-bound for iterating for-loop over register set entries is incorrect.
Fix it byt calculating correct number of entries.

And fix also dumping the last entry DATA_DELAY, which is the last and not
first (zero).

Fixes: 1a8e6b63e24f ("tools: kwbimage: Dump kwbimage config file on '-p -1' option")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 67b45503e46..1719b0415da 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -2148,6 +2148,7 @@ static int kwbimage_generate_config(void *ptr, struct image_tool_params *params)
 	struct ext_hdr_v0 *ehdr0;
 	struct bin_hdr_v0 *bhdr0;
 	struct opt_hdr_v1 *ohdr;
+	int regset_count;
 	int params_count;
 	unsigned offset;
 	int is_v0_ext;
@@ -2232,18 +2233,20 @@ static int kwbimage_generate_config(void *ptr, struct image_tool_params *params)
 			cur_idx++;
 		} else if (ohdr->headertype == OPT_HDR_V1_REGISTER_TYPE) {
 			regset_hdr = (struct register_set_hdr_v1 *)ohdr;
-			for (i = 0;
-			     i < opt_hdr_v1_size(ohdr) - sizeof(struct opt_hdr_v1) -
-				 sizeof(regset_hdr->data[0].last_entry);
-			     i++)
+			if (opt_hdr_v1_size(ohdr) > sizeof(*ohdr))
+				regset_count = (opt_hdr_v1_size(ohdr) - sizeof(*ohdr)) /
+					       sizeof(regset_hdr->data[0].entry);
+			else
+				regset_count = 0;
+			for (i = 0; i < regset_count; i++)
 				fprintf(f, "DATA 0x%08x 0x%08x\n",
 					le32_to_cpu(regset_hdr->data[i].entry.address),
 					le32_to_cpu(regset_hdr->data[i].entry.value));
-			if (opt_hdr_v1_size(ohdr) - sizeof(struct opt_hdr_v1) >=
-			    sizeof(regset_hdr->data[0].last_entry)) {
-				if (regset_hdr->data[0].last_entry.delay)
+			if (regset_count > 0) {
+				if (regset_hdr->data[regset_count-1].last_entry.delay !=
+				    REGISTER_SET_HDR_OPT_DELAY_SDRAM_SETUP)
 					fprintf(f, "DATA_DELAY %u\n",
-						(unsigned)regset_hdr->data[0].last_entry.delay);
+						(unsigned)regset_hdr->data[regset_count-1].last_entry.delay);
 				else
 					fprintf(f, "DATA_DELAY SDRAM_SETUP\n");
 			}
-- 
cgit v1.2.3


From aab9b063b5f6cea143d77dc1d27c004c1f89ab41 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 14 Jan 2023 14:31:00 +0100
Subject: tools: kwbimage: Fix endianity when dumping NAND_PAGE_SIZE
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes: 1a8e6b63e24f ("tools: kwbimage: Dump kwbimage config file on '-p -1' option")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 1719b0415da..a6f6f1578c7 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -2182,7 +2182,7 @@ static int kwbimage_generate_config(void *ptr, struct image_tool_params *params)
 		fprintf(f, "NAND_ECC_MODE %s\n", image_nand_ecc_mode_name(mhdr0->nandeccmode));
 
 	if (mhdr->blockid == IBR_HDR_NAND_ID)
-		fprintf(f, "NAND_PAGE_SIZE 0x%x\n", (unsigned)mhdr->nandpagesize);
+		fprintf(f, "NAND_PAGE_SIZE 0x%x\n", (unsigned)le16_to_cpu(mhdr->nandpagesize));
 
 	if (version != 0 && mhdr->blockid == IBR_HDR_NAND_ID)
 		fprintf(f, "NAND_BLKSZ 0x%x\n", (unsigned)mhdr->nandblocksize);
-- 
cgit v1.2.3


From e060779e59e79f8b1ff85ae03502e4a19c414608 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 14 Jan 2023 14:46:09 +0100
Subject: tools: kwbimage: Fix dumping NAND_BADBLK_LOCATION
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Value 0x0 for NAND_BADBLK_LOCATION/nandbadblklocation means that BBI is on
the first or second page and value 0x1 means that BBI is on the last page.
This indicates also NAND Flash Technology, value 0x0 is SLC NAND and value
0x1 is MLC NAND.

Therefore we need to dump NAND_BADBLK_LOCATION also when it is zero.

Note that in v0 images, nandbadblklocation field overlaps with ddrinitdelay
field in one union. ddrinitdelay is used in Kirkwood and nandbadblklocation
is used in Dove. For Dove images is_v0_ext should be set, so use it to
distinguish if nandbadblklocation is available or not. In v1 images there
is always nandbadblklocation field.

Fixes: 1a8e6b63e24f ("tools: kwbimage: Dump kwbimage config file on '-p -1' option")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index a6f6f1578c7..4e9ba5ddfae 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -2187,7 +2187,7 @@ static int kwbimage_generate_config(void *ptr, struct image_tool_params *params)
 	if (version != 0 && mhdr->blockid == IBR_HDR_NAND_ID)
 		fprintf(f, "NAND_BLKSZ 0x%x\n", (unsigned)mhdr->nandblocksize);
 
-	if (mhdr->blockid == IBR_HDR_NAND_ID && (mhdr->nandbadblklocation != 0 || is_v0_ext))
+	if (mhdr->blockid == IBR_HDR_NAND_ID && (version != 0 || is_v0_ext))
 		fprintf(f, "NAND_BADBLK_LOCATION 0x%x\n", (unsigned)mhdr->nandbadblklocation);
 
 	if (version == 0 && mhdr->blockid == IBR_HDR_SATA_ID)
-- 
cgit v1.2.3


From 226abde8677537de55905d88973eef1a71b4d3e0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 14 Jan 2023 13:42:14 +0100
Subject: tools: kwbimage: Fix dumping NAND_BLKSZ
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

kwbimage nandblocksize field is in 64 kB unit, but NAND_BLKSZ command
expects it in bytes. So do required unit conversion.

Also zero value in nandblocksize field has special meaning. When this field
is set to zero, the default block size is used. This default size is
defined by the NAND flash page size (16 KB for a 512B page or small page
NAND and 64 KB for a large page NAND flash).

Fixes: 1a8e6b63e24f ("tools: kwbimage: Dump kwbimage config file on '-p -1' option")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 4e9ba5ddfae..b6deb978f61 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -2184,8 +2184,14 @@ static int kwbimage_generate_config(void *ptr, struct image_tool_params *params)
 	if (mhdr->blockid == IBR_HDR_NAND_ID)
 		fprintf(f, "NAND_PAGE_SIZE 0x%x\n", (unsigned)le16_to_cpu(mhdr->nandpagesize));
 
-	if (version != 0 && mhdr->blockid == IBR_HDR_NAND_ID)
-		fprintf(f, "NAND_BLKSZ 0x%x\n", (unsigned)mhdr->nandblocksize);
+	if (version != 0 && mhdr->blockid == IBR_HDR_NAND_ID) {
+		if (mhdr->nandblocksize != 0) /* block size explicitly set in 64 kB unit */
+			fprintf(f, "NAND_BLKSZ 0x%x\n", (unsigned)mhdr->nandblocksize * 64*1024);
+		else if (le16_to_cpu(mhdr->nandpagesize) > 512)
+			fprintf(f, "NAND_BLKSZ 0x10000\n"); /* large page NAND flash = 64 kB block size */
+		else
+			fprintf(f, "NAND_BLKSZ 0x4000\n"); /* small page NAND flash = 16 kB block size */
+	}
 
 	if (mhdr->blockid == IBR_HDR_NAND_ID && (version != 0 || is_v0_ext))
 		fprintf(f, "NAND_BADBLK_LOCATION 0x%x\n", (unsigned)mhdr->nandbadblklocation);
-- 
cgit v1.2.3


From ee3da92d85aea4dad6d6d7c82b23407b85547325 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Mon, 9 Jan 2023 01:35:13 +0100
Subject: tools: kwbimage: Fix generating of kwbimage v0 header checksum
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Checksum for v0 image must be generated after filling all fields in the
main header. Otherwise it would be invalid.

Exactly same problem for v1 images was already fixed in the past in commit
9203c73895ab ("tools: kwbimage: Fix checksum calculation for v1 images").

Fixes: 5c61710c9880 ("tools: kwbimage: Properly set srcaddr in kwbimage v0")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index b6deb978f61..1128c934dda 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -1009,8 +1009,6 @@ static void *image_create_v0(size_t *imagesz, struct image_tool_params *params,
 	e = image_find_option(IMAGE_CFG_NAND_BADBLK_LOCATION);
 	if (e)
 		main_hdr->nandbadblklocation = e->nandbadblklocation;
-	main_hdr->checksum = image_checksum8(image,
-					     sizeof(struct main_hdr_v0));
 
 	/*
 	 * For SATA srcaddr is specified in number of sectors.
@@ -1049,6 +1047,9 @@ static void *image_create_v0(size_t *imagesz, struct image_tool_params *params,
 						    sizeof(struct ext_hdr_v0));
 	}
 
+	main_hdr->checksum = image_checksum8(image,
+					     sizeof(struct main_hdr_v0));
+
 	*imagesz = headersz;
 	return image;
 }
-- 
cgit v1.2.3


From 9f39f1992607a95ec9b72c02b34e2f8c15a02bef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 13:56:42 +0100
Subject: tools: kwbimage: Fix endianity when printing kwbimage header
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

All fields in kwbimage header are in little endian format.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 1128c934dda..97be3bed79c 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -1928,9 +1928,9 @@ static void kwbimage_print_header(const void *ptr)
 	}
 
 	printf("Data Size:    ");
-	genimg_print_size(mhdr->blocksize - sizeof(uint32_t));
-	printf("Load Address: %08x\n", mhdr->destaddr);
-	printf("Entry Point:  %08x\n", mhdr->execaddr);
+	genimg_print_size(le32_to_cpu(mhdr->blocksize) - sizeof(uint32_t));
+	printf("Load Address: %08x\n", le32_to_cpu(mhdr->destaddr));
+	printf("Entry Point:  %08x\n", le32_to_cpu(mhdr->execaddr));
 }
 
 static int kwbimage_check_image_types(uint8_t type)
-- 
cgit v1.2.3


From 0201244c3c8b6bdaa1e3ff47ab51ba3626d9c060 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 21 Jan 2023 13:00:21 +0100
Subject: tools: kwbimage: Reject mkimage -F option
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

mkimage -F option (re-sign existing FIT image) signaled by fflag is not
supported by kwbimage. So mark its usage as invalid parameter.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 97be3bed79c..0c3b40d075e 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -2440,7 +2440,7 @@ static int kwbimage_check_params(struct image_tool_params *params)
 	}
 
 	return (params->dflag && (params->fflag || params->lflag)) ||
-		(params->fflag && (params->dflag || params->lflag)) ||
+		(params->fflag) ||
 		(params->lflag && (params->dflag || params->fflag)) ||
 		(params->xflag);
 }
-- 
cgit v1.2.3


From 0a3a392c7122fccc1977d026cea9e48652b75688 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 16:22:34 +0100
Subject: tools: kwbimage: Add support for dumping NAND_BLKSZ for v0 images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In Dove functional specification, which use kwbimage v0, is also defined
nand block size field. So dump NAND_BLKSZ also for v0 images.

In Kirkwood functional specification, which also use kwbimage v0, this
field is not defined. So when it is zero and Kirkwood is detected, do not
dump it.

Fixes: f76ae2571fe0 ("tools: kwbimage: Add support for dumping extended and binary v0 headers")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 0c3b40d075e..eb99ac944d2 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -2185,7 +2185,7 @@ static int kwbimage_generate_config(void *ptr, struct image_tool_params *params)
 	if (mhdr->blockid == IBR_HDR_NAND_ID)
 		fprintf(f, "NAND_PAGE_SIZE 0x%x\n", (unsigned)le16_to_cpu(mhdr->nandpagesize));
 
-	if (version != 0 && mhdr->blockid == IBR_HDR_NAND_ID) {
+	if (mhdr->blockid == IBR_HDR_NAND_ID && (version != 0 || is_v0_ext || mhdr->nandblocksize != 0)) {
 		if (mhdr->nandblocksize != 0) /* block size explicitly set in 64 kB unit */
 			fprintf(f, "NAND_BLKSZ 0x%x\n", (unsigned)mhdr->nandblocksize * 64*1024);
 		else if (le16_to_cpu(mhdr->nandpagesize) > 512)
-- 
cgit v1.2.3


From 63cf0d726725464ed552f5835fe96401c21c3964 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 23:27:11 +0100
Subject: tools: kwbimage: Print binary image offset as size
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Use for it pretty print function: genimg_print_size(). This makes it more
human readable, like other offset and sizes printed by this tool.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index eb99ac944d2..a5de9855aa5 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -1914,9 +1914,9 @@ static void kwbimage_print_header(const void *ptr)
 			printf("BIN Img Size: ");
 			genimg_print_size(opt_hdr_v1_size(ohdr) - 12 -
 					  4 * ohdr->data[0]);
-			printf("BIN Img Offs: %08x\n",
-				(unsigned)((uint8_t *)ohdr - (uint8_t *)mhdr) +
-				8 + 4 * ohdr->data[0]);
+			printf("BIN Img Offs: ");
+			genimg_print_size(((uint8_t *)ohdr - (uint8_t *)mhdr) +
+					  8 + 4 * ohdr->data[0]);
 		}
 	}
 
-- 
cgit v1.2.3


From 443894a8215102873b9b653503dc9af79b50247e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 13:58:26 +0100
Subject: tools: kwbimage: Print image data offset when printing kwbimage
 header
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

For all images except SATA is data offset in bytes. For SATA it is in LBA
format (number of sectors). This is how Marvell BootROM interprets it.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 6 ++++++
 1 file changed, 6 insertions(+)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index a5de9855aa5..5f62ed159c4 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -1929,6 +1929,12 @@ static void kwbimage_print_header(const void *ptr)
 
 	printf("Data Size:    ");
 	genimg_print_size(le32_to_cpu(mhdr->blocksize) - sizeof(uint32_t));
+	printf("Data Offset:  ");
+	if (mhdr->blockid == IBR_HDR_SATA_ID)
+		printf("%u Sector%s (LBA)\n", le32_to_cpu(mhdr->srcaddr),
+		       le32_to_cpu(mhdr->srcaddr) != 1 ? "s" : "");
+	else
+		genimg_print_size(le32_to_cpu(mhdr->srcaddr));
 	printf("Load Address: %08x\n", le32_to_cpu(mhdr->destaddr));
 	printf("Entry Point:  %08x\n", le32_to_cpu(mhdr->execaddr));
 }
-- 
cgit v1.2.3


From dd13ac5495792325793ffcc381187afa3ba89f01 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 29 Jan 2023 13:08:10 +0100
Subject: tools: kwbimage: Simplify add_secure_header_v1()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

To make add_secure_header_v1() function more readable, call it directly
with arguments: header pointer with header size and data image pointer with
data image size. No functional change.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 5f62ed159c4..857af6a438a 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -1322,16 +1322,14 @@ static int kwb_sign_csk_with_kak(struct image_tool_params *params,
 	return 0;
 }
 
-static int add_secure_header_v1(struct image_tool_params *params, uint8_t *ptr,
-				int payloadsz, size_t headersz, uint8_t *image,
+static int add_secure_header_v1(struct image_tool_params *params, uint8_t *image_ptr,
+				size_t image_size, uint8_t *header_ptr, size_t headersz,
 				struct secure_hdr_v1 *secure_hdr)
 {
 	struct image_cfg_element *e_jtagdelay;
 	struct image_cfg_element *e_boxid;
 	struct image_cfg_element *e_flashid;
 	RSA *csk = NULL;
-	unsigned char *image_ptr;
-	size_t image_size;
 	struct sig_v1 tmp_sig;
 	bool specialized_img = image_get_spezialized_img();
 
@@ -1357,14 +1355,11 @@ static int add_secure_header_v1(struct image_tool_params *params, uint8_t *ptr,
 	if (kwb_sign_csk_with_kak(params, secure_hdr, csk))
 		return 1;
 
-	image_ptr = ptr + headersz;
-	image_size = payloadsz - headersz;
-
 	if (kwb_sign_and_verify(csk, image_ptr, image_size,
 				&secure_hdr->imgsig, "image") < 0)
 		return 1;
 
-	if (kwb_sign_and_verify(csk, image, headersz, &tmp_sig, "header") < 0)
+	if (kwb_sign_and_verify(csk, header_ptr, headersz, &tmp_sig, "header") < 0)
 		return 1;
 
 	secure_hdr->hdrsig = tmp_sig;
@@ -1533,8 +1528,8 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
 					      &datai, delay);
 	}
 
-	if (secure_hdr && add_secure_header_v1(params, ptr, payloadsz + headersz,
-					       headersz, image, secure_hdr))
+	if (secure_hdr && add_secure_header_v1(params, ptr + headersz, payloadsz,
+					       image, headersz, secure_hdr))
 		return NULL;
 
 	*imagesz = headersz;
-- 
cgit v1.2.3


From 39c78724f4e79227f0c4a13bd95ca44474204a07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 29 Jan 2023 13:17:21 +0100
Subject: tools: kwbimage: Rename imagesz to dataoff
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Variable imagesz in functions image_create_v0(), image_create_v1() and
kwbimage_set_header() stores offset to data from the beginning of the main
header. So it is not image size.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 857af6a438a..b32f845b7e2 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -962,7 +962,7 @@ static size_t image_headersz_v0(int *hasext)
 	return image_headersz_align(headersz, image_get_bootfrom());
 }
 
-static void *image_create_v0(size_t *imagesz, struct image_tool_params *params,
+static void *image_create_v0(size_t *dataoff, struct image_tool_params *params,
 			     int payloadsz)
 {
 	struct image_cfg_element *e;
@@ -1050,7 +1050,7 @@ static void *image_create_v0(size_t *imagesz, struct image_tool_params *params,
 	main_hdr->checksum = image_checksum8(image,
 					     sizeof(struct main_hdr_v0));
 
-	*imagesz = headersz;
+	*dataoff = headersz;
 	return image;
 }
 
@@ -1385,7 +1385,7 @@ static void finish_register_set_header_v1(uint8_t **cur, uint8_t **next_ext,
 	*datai = 0;
 }
 
-static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
+static void *image_create_v1(size_t *dataoff, struct image_tool_params *params,
 			     uint8_t *ptr, int payloadsz)
 {
 	struct image_cfg_element *e;
@@ -1532,7 +1532,7 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
 					       image, headersz, secure_hdr))
 		return NULL;
 
-	*imagesz = headersz;
+	*dataoff = headersz;
 
 	/* Fill the real header size without padding into the main header */
 	headersz = sizeof(*main_hdr);
@@ -1811,7 +1811,7 @@ static void kwbimage_set_header(void *ptr, struct stat *sbuf, int ifd,
 	FILE *fcfg;
 	void *image = NULL;
 	int version;
-	size_t headersz = 0;
+	size_t dataoff = 0;
 	size_t datasz;
 	uint32_t checksum;
 	struct stat s;
@@ -1862,11 +1862,11 @@ static void kwbimage_set_header(void *ptr, struct stat *sbuf, int ifd,
 		 */
 	case -1:
 	case 0:
-		image = image_create_v0(&headersz, params, datasz + 4);
+		image = image_create_v0(&dataoff, params, datasz + 4);
 		break;
 
 	case 1:
-		image = image_create_v1(&headersz, params, ptr, datasz + 4);
+		image = image_create_v1(&dataoff, params, ptr, datasz + 4);
 		break;
 
 	default:
@@ -1884,12 +1884,12 @@ static void kwbimage_set_header(void *ptr, struct stat *sbuf, int ifd,
 	free(image_cfg);
 
 	/* Build and add image data checksum */
-	checksum = cpu_to_le32(image_checksum32((uint8_t *)ptr + headersz,
+	checksum = cpu_to_le32(image_checksum32((uint8_t *)ptr + dataoff,
 						datasz));
-	memcpy((uint8_t *)ptr + headersz + datasz, &checksum, sizeof(uint32_t));
+	memcpy((uint8_t *)ptr + dataoff + datasz, &checksum, sizeof(uint32_t));
 
 	/* Finally copy the header into the image area */
-	memcpy(ptr, image, headersz);
+	memcpy(ptr, image, dataoff);
 
 	free(image);
 }
-- 
cgit v1.2.3


From bf78a57e9a84ef4c882acd8c8710d364ed90730e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 29 Jan 2023 14:33:36 +0100
Subject: tools: kwbimage: Fix generating secure boot data image signature
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Secure boot data image signature is calculated from the data image without
trailing 4-bit checksum. Commit 37cb9c15d70d ("tools: kwbimage: Simplify
aligning and calculating checksum") unintentionally broke this calculation
when it increased payloadsz variable by 4 bytes which was propagated also
into the add_secure_header_v1() function. Fix this issue by decreasing size
of buffer by 4 bytes from which is calculated secure boot data image
signature.

Fixes: 37cb9c15d70d ("tools: kwbimage: Simplify aligning and calculating checksum")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index b32f845b7e2..a8a59c154b9 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -1355,7 +1355,7 @@ static int add_secure_header_v1(struct image_tool_params *params, uint8_t *image
 	if (kwb_sign_csk_with_kak(params, secure_hdr, csk))
 		return 1;
 
-	if (kwb_sign_and_verify(csk, image_ptr, image_size,
+	if (kwb_sign_and_verify(csk, image_ptr, image_size - 4,
 				&secure_hdr->imgsig, "image") < 0)
 		return 1;
 
-- 
cgit v1.2.3


From 9b4531f685fafeb2bb0139e323f635d3cda150f7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 29 Jan 2023 15:00:45 +0100
Subject: tools: kwbimage: Fix invalid secure boot header signature
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Secure boot header signature is calculated from the image header with
zeroed header checksum. Calculation is done in add_secure_header_v1()
function. So after calling this function no header member except
main_hdr->checksum can be modified. Commit 2b0980c24027 ("tools: kwbimage:
Fill the real header size into the main header") broke this requirement as
final header size started to be filled into main_hdr->headersz_* members
after the add_secure_header_v1() call.

Fix this issue by following steps:
- Split header size and image data offset into two variables (headersz and
  *dataoff).
- Change image_headersz_v0() and add_binary_header_v1() functions to return
  real (unaligned) header size instead of image data offset.
- On every place use correct variable (headersz or *dataoff)

After these steps variable headersz is correctly filled into the
main_hdr->headersz_* members and so overwriting them in the end of the
image_create_v1() function is not needed anymore. Remove those overwriting
which effectively reverts changes in problematic commit without affecting
value in main_hdr->headersz_* members and makes secure boot header
signature valid again.

Fixes: 2b0980c24027 ("tools: kwbimage: Fill the real header size into the main header")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 41 ++++++++++++++---------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index a8a59c154b9..da539541742 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -959,7 +959,7 @@ static size_t image_headersz_v0(int *hasext)
 			*hasext = 1;
 	}
 
-	return image_headersz_align(headersz, image_get_bootfrom());
+	return headersz;
 }
 
 static void *image_create_v0(size_t *dataoff, struct image_tool_params *params,
@@ -972,10 +972,11 @@ static void *image_create_v0(size_t *dataoff, struct image_tool_params *params,
 	int has_ext = 0;
 
 	/*
-	 * Calculate the size of the header and the size of the
+	 * Calculate the size of the header and the offset of the
 	 * payload
 	 */
 	headersz = image_headersz_v0(&has_ext);
+	*dataoff = image_headersz_align(headersz, image_get_bootfrom());
 
 	image = malloc(headersz);
 	if (!image) {
@@ -990,7 +991,7 @@ static void *image_create_v0(size_t *dataoff, struct image_tool_params *params,
 	/* Fill in the main header */
 	main_hdr->blocksize =
 		cpu_to_le32(payloadsz);
-	main_hdr->srcaddr   = cpu_to_le32(headersz);
+	main_hdr->srcaddr   = cpu_to_le32(*dataoff);
 	main_hdr->ext       = has_ext;
 	main_hdr->version   = 0;
 	main_hdr->destaddr  = cpu_to_le32(params->addr);
@@ -1013,10 +1014,9 @@ static void *image_create_v0(size_t *dataoff, struct image_tool_params *params,
 	/*
 	 * For SATA srcaddr is specified in number of sectors.
 	 * This expects the sector size to be 512 bytes.
-	 * Header size is already aligned.
 	 */
 	if (main_hdr->blockid == IBR_HDR_SATA_ID)
-		main_hdr->srcaddr = cpu_to_le32(headersz / 512);
+		main_hdr->srcaddr = cpu_to_le32(le32_to_cpu(main_hdr->srcaddr) / 512);
 
 	/* For PCIe srcaddr is not used and must be set to 0xFFFFFFFF. */
 	if (main_hdr->blockid == IBR_HDR_PEX_ID)
@@ -1050,7 +1050,6 @@ static void *image_create_v0(size_t *dataoff, struct image_tool_params *params,
 	main_hdr->checksum = image_checksum8(image,
 					     sizeof(struct main_hdr_v0));
 
-	*dataoff = headersz;
 	return image;
 }
 
@@ -1064,10 +1063,6 @@ static size_t image_headersz_v1(int *hasext)
 	int cfgi;
 	int ret;
 
-	/*
-	 * Calculate the size of the header and the size of the
-	 * payload
-	 */
 	headersz = sizeof(struct main_hdr_v1);
 
 	if (image_get_csk_index() >= 0) {
@@ -1163,7 +1158,7 @@ static size_t image_headersz_v1(int *hasext)
 	if (count > 0)
 		headersz += sizeof(struct register_set_hdr_v1) + 8 * count + 4;
 
-	return image_headersz_align(headersz, image_get_bootfrom());
+	return headersz;
 }
 
 static int add_binary_header_v1(uint8_t **cur, uint8_t **next_ext,
@@ -1390,7 +1385,6 @@ static void *image_create_v1(size_t *dataoff, struct image_tool_params *params,
 {
 	struct image_cfg_element *e;
 	struct main_hdr_v1 *main_hdr;
-	struct opt_hdr_v1 *ohdr;
 	struct register_set_hdr_v1 *register_set_hdr;
 	struct secure_hdr_v1 *secure_hdr = NULL;
 	size_t headersz;
@@ -1401,12 +1395,13 @@ static void *image_create_v1(size_t *dataoff, struct image_tool_params *params,
 	uint8_t delay;
 
 	/*
-	 * Calculate the size of the header and the size of the
+	 * Calculate the size of the header and the offset of the
 	 * payload
 	 */
 	headersz = image_headersz_v1(&hasext);
 	if (headersz == 0)
 		return NULL;
+	*dataoff = image_headersz_align(headersz, image_get_bootfrom());
 
 	image = malloc(headersz);
 	if (!image) {
@@ -1428,7 +1423,7 @@ static void *image_create_v1(size_t *dataoff, struct image_tool_params *params,
 	main_hdr->headersz_msb = (headersz & 0xFFFF0000) >> 16;
 	main_hdr->destaddr     = cpu_to_le32(params->addr);
 	main_hdr->execaddr     = cpu_to_le32(params->ep);
-	main_hdr->srcaddr      = cpu_to_le32(headersz);
+	main_hdr->srcaddr      = cpu_to_le32(*dataoff);
 	main_hdr->ext          = hasext;
 	main_hdr->version      = 1;
 	main_hdr->blockid      = image_get_bootfrom();
@@ -1458,10 +1453,9 @@ static void *image_create_v1(size_t *dataoff, struct image_tool_params *params,
 	/*
 	 * For SATA srcaddr is specified in number of sectors.
 	 * This expects the sector size to be 512 bytes.
-	 * Header size is already aligned.
 	 */
 	if (main_hdr->blockid == IBR_HDR_SATA_ID)
-		main_hdr->srcaddr = cpu_to_le32(headersz / 512);
+		main_hdr->srcaddr = cpu_to_le32(le32_to_cpu(main_hdr->srcaddr) / 512);
 
 	/* For PCIe srcaddr is not used and must be set to 0xFFFFFFFF. */
 	if (main_hdr->blockid == IBR_HDR_PEX_ID)
@@ -1528,19 +1522,10 @@ static void *image_create_v1(size_t *dataoff, struct image_tool_params *params,
 					      &datai, delay);
 	}
 
-	if (secure_hdr && add_secure_header_v1(params, ptr + headersz, payloadsz,
+	if (secure_hdr && add_secure_header_v1(params, ptr + *dataoff, payloadsz,
 					       image, headersz, secure_hdr))
 		return NULL;
 
-	*dataoff = headersz;
-
-	/* Fill the real header size without padding into the main header */
-	headersz = sizeof(*main_hdr);
-	for_each_opt_hdr_v1 (ohdr, main_hdr)
-		headersz += opt_hdr_v1_size(ohdr);
-	main_hdr->headersz_lsb = cpu_to_le16(headersz & 0xFFFF);
-	main_hdr->headersz_msb = (headersz & 0xFFFF0000) >> 16;
-
 	/* Calculate and set the header checksum */
 	main_hdr->checksum = image_checksum8(main_hdr, headersz);
 
@@ -1889,7 +1874,7 @@ static void kwbimage_set_header(void *ptr, struct stat *sbuf, int ifd,
 	memcpy((uint8_t *)ptr + dataoff + datasz, &checksum, sizeof(uint32_t));
 
 	/* Finally copy the header into the image area */
-	memcpy(ptr, image, dataoff);
+	memcpy(ptr, image, kwbheader_size(image));
 
 	free(image);
 }
@@ -2109,6 +2094,8 @@ static int kwbimage_generate(struct image_tool_params *params,
 		exit(EXIT_FAILURE);
 	}
 
+	alloc_len = image_headersz_align(alloc_len, image_get_bootfrom());
+
 	free(image_cfg);
 
 	hdr = malloc(alloc_len);
-- 
cgit v1.2.3


From 27670acaac82f370b635f1af103594a335322bcf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 21 Jan 2023 20:05:43 +0100
Subject: tools: mkimage: Do not fill legacy_img_hdr for non-legacy XIP images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Skip filling legacy_img_hdr structure for XIP images which do not use
legacy_img_hdr structure header. Adding unwanted header to other image
formats, like kwbimage cause generation of broken image.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/mkimage.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'tools')

diff --git a/tools/mkimage.c b/tools/mkimage.c
index af7b0e09b3b..0b342159e5f 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -860,7 +860,9 @@ copy_file (int ifd, const char *datafile, int pad)
 		exit (EXIT_FAILURE);
 	}
 
-	if (params.xflag) {
+	if (params.xflag &&
+	    (((params.type > IH_TYPE_INVALID) && (params.type < IH_TYPE_FLATDT)) ||
+	     (params.type == IH_TYPE_KERNEL_NOLOAD) || (params.type == IH_TYPE_FIRMWARE_IVT))) {
 		unsigned char *p = NULL;
 		/*
 		 * XIP: do not append the struct legacy_img_hdr at the
-- 
cgit v1.2.3


From cccc5b4f3d06dd2b021eaf690f8f828c3d4c9af5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Wed, 18 Jan 2023 21:42:40 +0100
Subject: tools: kwbimage: Add support for XIP SPI/NOR images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Marvell BootROM can execute SPI images directly from NOR (either SPI/serial
or parallel) without copying them to DDR RAM. This is know at XIP - execute
in place. To achieve that, destination address in kwbimage must be set to
0xFFFFFFFF and execute address to the offset in bytes from the beginning of
NOR memory.

Kirkwood and Dove which use kwbimage v0 format and have SPI address space
mapped to physical memory at 0xE8000000-0xEFFFFFFF by BootROM.

Armada SoCs use kwbimage v1 format and have SPI address space mapped to
physical memory at 0xD4000000-0xD7FFFFFF and Device bus address space (used
for parallel NOR) at 0xD8000000-0xDFFFFFFF.

Add support for generating XIP kwbimages by mkimage -x flag and mark xflag
as valid option in kwbimage.c.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 89 insertions(+), 7 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index da539541742..7ebb625d03b 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -927,6 +927,71 @@ done:
 	return ret;
 }
 
+static int image_fill_xip_header(void *image, struct image_tool_params *params)
+{
+	struct main_hdr_v1 *main_hdr = image; /* kwbimage v0 and v1 have same XIP members */
+	int version = kwbimage_version(image);
+	uint32_t srcaddr = le32_to_cpu(main_hdr->srcaddr);
+	uint32_t startaddr = 0;
+
+	if (main_hdr->blockid != IBR_HDR_SPI_ID) {
+		fprintf(stderr, "XIP is supported only for SPI images\n");
+		return 0;
+	}
+
+	if (version == 0 &&
+		   params->addr >= 0xE8000000 && params->addr < 0xEFFFFFFF &&
+		   params->ep >= 0xE8000000 && params->ep < 0xEFFFFFFF) {
+		/* Load and Execute address is in SPI address space (kwbimage v0) */
+		startaddr = 0xE8000000;
+	} else if (version != 0 &&
+		   params->addr >= 0xD4000000 && params->addr < 0xD7FFFFFF &&
+		   params->ep >= 0xD4000000 && params->ep < 0xD7FFFFFF) {
+		/* Load and Execute address is in SPI address space (kwbimage v1) */
+		startaddr = 0xD4000000;
+	} else if (version != 0 &&
+		   params->addr >= 0xD8000000 && params->addr < 0xDFFFFFFF &&
+		   params->ep >= 0xD8000000 && params->ep < 0xDFFFFFFF) {
+		/* Load and Execute address is in Device bus space (kwbimage v1) */
+		startaddr = 0xD8000000;
+	} else if (params->addr != 0x0) {
+		/* Load address is non-zero */
+		if (version == 0)
+			fprintf(stderr, "XIP Load Address or XIP Entry Point is not in SPI address space\n");
+		else
+			fprintf(stderr, "XIP Load Address or XIP Entry Point is not in SPI nor in Device bus address space\n");
+		return 0;
+	}
+
+	/*
+	 * For XIP destaddr must be set to 0xFFFFFFFF and
+	 * execaddr relative to the start of XIP memory address space.
+	 */
+	main_hdr->destaddr = cpu_to_le32(0xFFFFFFFF);
+
+	if (startaddr == 0) {
+		/*
+		 * mkimage's --load-address 0x0 means that binary is Position
+		 * Independent and in this case mkimage's --entry-point address
+		 * is relative offset from beginning of the data part of image.
+		 */
+		main_hdr->execaddr = cpu_to_le32(srcaddr + params->ep);
+	} else {
+		/* The lowest possible load address is after the header at srcaddr. */
+		if (params->addr - startaddr < srcaddr) {
+			fprintf(stderr,
+				"Invalid XIP Load Address 0x%08x.\n"
+				"The lowest address for this configuration is 0x%08x.\n",
+				params->addr, (unsigned)(startaddr + srcaddr));
+			return 0;
+		}
+		main_hdr->srcaddr = cpu_to_le32(params->addr - startaddr);
+		main_hdr->execaddr = cpu_to_le32(params->ep - startaddr);
+	}
+
+	return 1;
+}
+
 static size_t image_headersz_align(size_t headersz, uint8_t blockid)
 {
 	/*
@@ -1022,6 +1087,14 @@ static void *image_create_v0(size_t *dataoff, struct image_tool_params *params,
 	if (main_hdr->blockid == IBR_HDR_PEX_ID)
 		main_hdr->srcaddr = cpu_to_le32(0xFFFFFFFF);
 
+	if (params->xflag) {
+		if (!image_fill_xip_header(main_hdr, params)) {
+			free(image);
+			return NULL;
+		}
+		*dataoff = le32_to_cpu(main_hdr->srcaddr);
+	}
+
 	/* Generate the ext header */
 	if (has_ext) {
 		struct ext_hdr_v0 *ext_hdr;
@@ -1461,6 +1534,14 @@ static void *image_create_v1(size_t *dataoff, struct image_tool_params *params,
 	if (main_hdr->blockid == IBR_HDR_PEX_ID)
 		main_hdr->srcaddr = cpu_to_le32(0xFFFFFFFF);
 
+	if (params->xflag) {
+		if (!image_fill_xip_header(main_hdr, params)) {
+			free(image);
+			return NULL;
+		}
+		*dataoff = le32_to_cpu(main_hdr->srcaddr);
+	}
+
 	if (image_get_csk_index() >= 0) {
 		/*
 		 * only reserve the space here; we fill the header later since
@@ -1915,8 +1996,13 @@ static void kwbimage_print_header(const void *ptr)
 		       le32_to_cpu(mhdr->srcaddr) != 1 ? "s" : "");
 	else
 		genimg_print_size(le32_to_cpu(mhdr->srcaddr));
-	printf("Load Address: %08x\n", le32_to_cpu(mhdr->destaddr));
-	printf("Entry Point:  %08x\n", le32_to_cpu(mhdr->execaddr));
+	if (mhdr->blockid == IBR_HDR_SPI_ID && le32_to_cpu(mhdr->destaddr) == 0xFFFFFFFF) {
+		printf("Load Address: XIP\n");
+		printf("Execute Offs: %08x\n", le32_to_cpu(mhdr->execaddr));
+	} else {
+		printf("Load Address: %08x\n", le32_to_cpu(mhdr->destaddr));
+		printf("Entry Point:  %08x\n", le32_to_cpu(mhdr->execaddr));
+	}
 }
 
 static int kwbimage_check_image_types(uint8_t type)
@@ -2414,9 +2500,6 @@ static int kwbimage_extract_subimage(void *ptr, struct image_tool_params *params
 	return imagetool_save_subimage(params->outfile, image, size);
 }
 
-/*
- * Report Error if xflag is set in addition to default
- */
 static int kwbimage_check_params(struct image_tool_params *params)
 {
 	if (!params->lflag && !params->iflag && !params->pflag &&
@@ -2429,8 +2512,7 @@ static int kwbimage_check_params(struct image_tool_params *params)
 
 	return (params->dflag && (params->fflag || params->lflag)) ||
 		(params->fflag) ||
-		(params->lflag && (params->dflag || params->fflag)) ||
-		(params->xflag);
+		(params->lflag && (params->dflag || params->fflag));
 }
 
 /*
-- 
cgit v1.2.3


From 2f6855a6aa8d55d8341f454f87cabc784a890193 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sun, 8 Jan 2023 23:28:39 +0100
Subject: tools: mkimage: Print human readable error when -d is not specified
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When asking mkimage to create a new image file and option -d is not
specified then mkimage show human unfriendly error message:

  mkimage: Can't open (null): Bad address

Without debugger it is hard to debug what is the issue. Function open() is
being called with file name set to NULL. So add a check for this and if it
happens then show human readable message that option -d was not specified.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/mkimage.c | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'tools')

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 0b342159e5f..5e0bb91cea0 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -600,6 +600,11 @@ int main(int argc, char **argv)
 	}
 
 	if ((params.type != IH_TYPE_MULTI) && (params.type != IH_TYPE_SCRIPT)) {
+		if (!params.datafile) {
+			fprintf(stderr, "%s: Option -d with image data file was not specified\n",
+				params.cmdname);
+			exit(EXIT_FAILURE);
+		}
 		dfd = open(params.datafile, O_RDONLY | O_BINARY);
 		if (dfd < 0) {
 			fprintf(stderr, "%s: Can't open %s: %s\n",
-- 
cgit v1.2.3


From b07965b8a9a9887a37f41254d6155f8fc38ad006 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 21 Jan 2023 20:09:26 +0100
Subject: tools: mkimage: Do not try to open datafile when it is skipped
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When mkimage was instructed to skip datafile via option -s then do not try
to validate or open datafile as it does not have to exist or to be
specified via -d option.

This change allows to use -s option for skipping datafile when -d option
for datafile was not specified.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/mkimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'tools')

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 5e0bb91cea0..a92d9d5ca57 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -599,7 +599,7 @@ int main(int argc, char **argv)
 		exit (retval);
 	}
 
-	if ((params.type != IH_TYPE_MULTI) && (params.type != IH_TYPE_SCRIPT)) {
+	if (!params.skipcpy && params.type != IH_TYPE_MULTI && params.type != IH_TYPE_SCRIPT) {
 		if (!params.datafile) {
 			fprintf(stderr, "%s: Option -d with image data file was not specified\n",
 				params.cmdname);
-- 
cgit v1.2.3


From 3a521f08677914821479647008d488bb4f15b17a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org>
Date: Sat, 21 Jan 2023 20:11:28 +0100
Subject: tools: kwbimage: Add support for creating an image with no data
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This change add support for mkimage's -s option to kwbimage format. It will
create an kwbimage with empty data part of image (data part would contain
only required 32-bit checksum). mkimage's -s option is indicated by skipcpy
flag and it is basically in conflict with mkimage's -d (datafile) option.

"Empty" kwbimage with no data can still contain headers. For example it can
contain binary executable header which is copied by BootROM into L2SRAM.
This is useful for example for small images which can do not require DDR
RAM and can be run in L2SRAM (which do not require any initialization).

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

(limited to 'tools')

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 7ebb625d03b..309657a5637 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -1887,7 +1887,9 @@ static void kwbimage_set_header(void *ptr, struct stat *sbuf, int ifd,
 	 * Do not use sbuf->st_size as it contains size with padding.
 	 * We need original image data size, so stat original file.
 	 */
-	if (stat(params->datafile, &s)) {
+	if (params->skipcpy) {
+		s.st_size = 0;
+	} else if (stat(params->datafile, &s)) {
 		fprintf(stderr, "Could not stat data file %s: %s\n",
 			params->datafile, strerror(errno));
 		exit(EXIT_FAILURE);
@@ -2106,6 +2108,8 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size,
 	return 0;
 }
 
+static int kwbimage_align_size(int bootfrom, int alloc_len, struct stat s);
+
 static int kwbimage_generate(struct image_tool_params *params,
 			     struct image_type_params *tparams)
 {
@@ -2124,7 +2128,9 @@ static int kwbimage_generate(struct image_tool_params *params,
 		exit(EXIT_FAILURE);
 	}
 
-	if (stat(params->datafile, &s)) {
+	if (params->skipcpy) {
+		s.st_size = 0;
+	} else if (stat(params->datafile, &s)) {
 		fprintf(stderr, "Could not stat data file %s: %s\n",
 			params->datafile, strerror(errno));
 		exit(EXIT_FAILURE);
@@ -2195,6 +2201,22 @@ static int kwbimage_generate(struct image_tool_params *params,
 	tparams->header_size = alloc_len;
 	tparams->hdr = hdr;
 
+	/*
+	 * This function should return aligned size of the datafile.
+	 * When skipcpy is set (datafile is skipped) then return value of this
+	 * function is ignored, so we have to put required kwbimage aligning
+	 * into the preallocated header size.
+	 */
+	if (params->skipcpy) {
+		tparams->header_size += kwbimage_align_size(bootfrom, alloc_len, s);
+		return 0;
+	} else {
+		return kwbimage_align_size(bootfrom, alloc_len, s);
+	}
+}
+
+static int kwbimage_align_size(int bootfrom, int alloc_len, struct stat s)
+{
 	/*
 	 * The resulting image needs to be 4-byte aligned. At least
 	 * the Marvell hdrparser tool complains if its unaligned.
@@ -2510,7 +2532,7 @@ static int kwbimage_check_params(struct image_tool_params *params)
 		return 1;
 	}
 
-	return (params->dflag && (params->fflag || params->lflag)) ||
+	return (params->dflag && (params->fflag || params->lflag || params->skipcpy)) ||
 		(params->fflag) ||
 		(params->lflag && (params->dflag || params->fflag));
 }
-- 
cgit v1.2.3