Re: [PATCH v5 25/29] image: Remove #ifdefs from select_ramdisk()

Re: [PATCH v5 25/29] image: Remove #ifdefs from select_ramdisk()

[Art Nikpal]

hi Neil

>   Revert "image: Remove #ifdefs from select_ramdisk()"
>   This reverts commit f33a2c1bd0fb371511a485cac1f182ba50db51be.
>
>  This causes a crash on some platforms as seen here:
>   https://lore.kernel.org/r/f153017b-c41a-0d32-67b9-f288e695f900@baylibre.com/

Maybe the next patch was the solution for your problem ?

https://patchwork.ozlabs.org/project/uboot/patch/20211125030859.2896352-1-art@khadas.com/

Re: [PATCH v5 25/29] image: Remove #ifdefs from select_ramdisk()

[Neil Armstrong]

Hi,

On 09/10/2021 03:41, Tom Rini wrote:
> On Sat, Sep 25, 2021 at 07:43:38PM -0600, Simon Glass wrote:
> 
>> Use boolean variables to deal with the strange #ifdef logic of this
>> function, so we can remove the #ifdefs.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
> 
> Applied to u-boot/master, thanks!
> 

This commit causes an abort on Libretech-CC when trying to load an initrd (using libretech-cc_defconfig & gcc-arm-9.2-2019.12):

U-Boot 2021.10-00076-gf33a2c1bd0 (Dec 17 2021 - 18:21:08 +0100) libretech-cc

Model: Libre Computer AML-S905X-CC
SoC:   Amlogic Meson GXL (S905X) Revision 21:d (84:2)
DRAM:  2 GiB
MMC:   mmc@72000: 0, mmc@74000: 1
Loading Environment from nowhere... OK
In:    serial
Out:   serial
Err:   serial
[BL31]: tee size: 0
[BL31]: tee size: 0
Net:   eth0: ethernet@c9410000
Hit any key to stop autoboot:  0
(ROM USB boot)
## Executing script at 08000000

Moving Image from 0x8080000 to 0x8200000, end=a6c0000
"Synchronous Abort" handler, esr 0x96000210
elr: 000000000101bd44 lr : 000000000101bf3c (reloc)
elr: 000000007df70d44 lr : 000000007df70f3c
x0 : 0000000000000000 x1 : 000000007dfcd6fc
x2 : 0000000000000003 x3 : 0000000000000000
x4 : 000000007bf74b20 x5 : 0000000000000030
x6 : 0000000000000000 x7 : 0000000000000002
x8 : 000000007bf441c0 x9 : 0000000000000000
x10: 000000007fffffff x11: 0000000000000000
x12: 000000000a6c0000 x13: 000000000a6bffff
x14: 000000007bf49210 x15: 0000000000000020
x16: 000000007df853bc x17: 0000000000000000
x18: 000000007bf52dd0 x19: 000000007dff4e98
x20: 000000007dff4f88 x21: 000000007dff4f90
x22: 0000000000000016 x23: 000000007bf74b20
x24: 0000000000000000 x25: 000000007bf74b48
x26: 0000000000000000 x27: 0000000000000000
x28: 000000007bf74b70 x29: 000000007bf481f0

Code: a9017fff 97fffff5 a8c27bfd d65f03c0 (b9400002)
Resetting CPU ...

Abort is probably in genimg_get_format()

Reverting on master fixes the issue.

Neil

Re: [PATCH v5 25/29] image: Remove #ifdefs from select_ramdisk()

[Tom Rini]

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

On Sat, Sep 25, 2021 at 07:43:38PM -0600, Simon Glass wrote:

> Use boolean variables to deal with the strange #ifdef logic of this
> function, so we can remove the #ifdefs.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

[PATCH v5 25/29] image: Remove #ifdefs from select_ramdisk()

[Simon Glass]

Use boolean variables to deal with the strange #ifdef logic of this
function, so we can remove the #ifdefs.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Rebase to next

 common/image-board.c | 139 ++++++++++++++++++++++---------------------
 1 file changed, 72 insertions(+), 67 deletions(-)

diff --git a/common/image-board.c b/common/image-board.c
index fecc4bf6365..742ca66298b 100644
--- a/common/image-board.c
+++ b/common/image-board.c
@@ -26,7 +26,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
 /**
  * image_get_ramdisk - get and verify ramdisk image
  * @rd_addr: ramdisk image start address
@@ -85,7 +84,6 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, u8 arch,
 
 	return rd_hdr;
 }
-#endif
 
 /*****************************************************************************/
 /* Shared dual-format routines */
@@ -326,16 +324,18 @@ int genimg_has_config(bootm_headers_t *images)
 static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 			  ulong *rd_datap, ulong *rd_lenp)
 {
-	ulong rd_addr;
+	ulong rd_addr = 0;
 	char *buf;
+	const char *fit_uname_config = images->fit_uname_cfg;
+	const char *fit_uname_ramdisk = NULL;
+	bool processed;
+	int rd_noffset;
 
-#if CONFIG_IS_ENABLED(FIT)
-		const char *fit_uname_config = images->fit_uname_cfg;
-		const char *fit_uname_ramdisk = NULL;
-		int rd_noffset;
+	if (select) {
+		ulong default_addr;
+		bool done = true;
 
-		if (select) {
-			ulong default_addr;
+		if (CONFIG_IS_ENABLED(FIT)) {
 			/*
 			 * If the init ramdisk comes from the FIT image and
 			 * the FIT image address is omitted in the command
@@ -347,8 +347,8 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 			else
 				default_addr = image_load_addr;
 
-			if (fit_parse_conf(select, default_addr,
-					   &rd_addr, &fit_uname_config)) {
+			if (fit_parse_conf(select, default_addr, &rd_addr,
+					   &fit_uname_config)) {
 				debug("*  ramdisk: config '%s' from image at 0x%08lx\n",
 				      fit_uname_config, rd_addr);
 			} else if (fit_parse_subimage(select, default_addr,
@@ -356,60 +356,58 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 						      &fit_uname_ramdisk)) {
 				debug("*  ramdisk: subimage '%s' from image at 0x%08lx\n",
 				      fit_uname_ramdisk, rd_addr);
-			} else
-#endif
-			{
-				rd_addr = hextoul(select, NULL);
-				debug("*  ramdisk: cmdline image address = 0x%08lx\n",
-				      rd_addr);
+			} else {
+				done = false;
 			}
-#if CONFIG_IS_ENABLED(FIT)
-		} else {
-			/* use FIT configuration provided in first bootm
-			 * command argument. If the property is not defined,
-			 * quit silently (with -ENOPKG)
-			 */
-			rd_addr = map_to_sysmem(images->fit_hdr_os);
-			rd_noffset = fit_get_node_from_config(images,
-							      FIT_RAMDISK_PROP,
-							      rd_addr);
-			if (rd_noffset == -ENOENT)
-				return -ENOPKG;
-			else if (rd_noffset < 0)
-				return rd_noffset;
 		}
-#endif
-
-		/*
-		 * Check if there is an initrd image at the
-		 * address provided in the second bootm argument
-		 * check image type, for FIT images get FIT node.
+		if (!done) {
+			rd_addr = hextoul(select, NULL);
+			debug("*  ramdisk: cmdline image address = 0x%08lx\n",
+			      rd_addr);
+		}
+	} else if (CONFIG_IS_ENABLED(FIT)) {
+		/* use FIT configuration provided in first bootm
+		 * command argument. If the property is not defined,
+		 * quit silently (with -ENOPKG	)
 		 */
-		buf = map_sysmem(rd_addr, 0);
-		switch (genimg_get_format(buf)) {
-#if CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)
-		case IMAGE_FORMAT_LEGACY: {
+		rd_addr = map_to_sysmem(images->fit_hdr_os);
+		rd_noffset = fit_get_node_from_config(images, FIT_RAMDISK_PROP,
+						      rd_addr);
+		if (rd_noffset == -ENOENT)
+			return -ENOPKG;
+		else if (rd_noffset < 0)
+			return rd_noffset;
+	}
+
+	/*
+	 * Check if there is an initrd image at the
+	 * address provided in the second bootm argument
+	 * check image type, for FIT images get FIT node.
+	 */
+	buf = map_sysmem(rd_addr, 0);
+	processed = false;
+	switch (genimg_get_format(buf)) {
+	case IMAGE_FORMAT_LEGACY:
+		if (CONFIG_IS_ENABLED(LEGACY_IMAGE_FORMAT)) {
 			const image_header_t *rd_hdr;
 
 			printf("## Loading init Ramdisk from Legacy Image at %08lx ...\n",
 			       rd_addr);
 
 			bootstage_mark(BOOTSTAGE_ID_CHECK_RAMDISK);
-			rd_hdr = image_get_ramdisk(rd_addr, arch,
-						   images->verify);
-
+			rd_hdr = image_get_ramdisk(rd_addr, arch, images->verify);
 			if (!rd_hdr)
 				return -ENOENT;
 
 			*rd_datap = image_get_data(rd_hdr);
 			*rd_lenp = image_get_data_size(rd_hdr);
-			break;
+			processed = true;
 		}
-#endif
-#if CONFIG_IS_ENABLED(FIT)
-		case IMAGE_FORMAT_FIT:
-			rd_noffset = fit_image_load(images,
-						    rd_addr, &fit_uname_ramdisk,
+		break;
+	case IMAGE_FORMAT_FIT:
+		if (CONFIG_IS_ENABLED(FIT)) {
+			rd_noffset = fit_image_load(images, rd_addr,
+						    &fit_uname_ramdisk,
 						    &fit_uname_config, arch,
 						    IH_TYPE_RAMDISK,
 						    BOOTSTAGE_ID_FIT_RD_START,
@@ -421,29 +419,36 @@ static int select_ramdisk(bootm_headers_t *images, const char *select, u8 arch,
 			images->fit_hdr_rd = map_sysmem(rd_addr, 0);
 			images->fit_uname_rd = fit_uname_ramdisk;
 			images->fit_noffset_rd = rd_noffset;
-			break;
-#endif
-#ifdef CONFIG_ANDROID_BOOT_IMAGE
-		case IMAGE_FORMAT_ANDROID:
+			processed = true;
+		}
+		break;
+	case IMAGE_FORMAT_ANDROID:
+		if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
 			android_image_get_ramdisk((void *)images->os.start,
 						  rd_datap, rd_lenp);
-			break;
-#endif
-		default:
-			if (IS_ENABLED(CONFIG_SUPPORT_RAW_INITRD)) {
-				char *end = NULL;
-
-				if (select)
-					end = strchr(select, ':');
-				if (end) {
-					*rd_lenp = hextoul(++end, NULL);
-					*rd_datap = rd_addr;
-					break;
-				}
+			processed = true;
+		}
+		break;
+	}
+
+	if (!processed) {
+		if (IS_ENABLED(CONFIG_SUPPORT_RAW_INITRD)) {
+			char *end = NULL;
+
+			if (select)
+				end = strchr(select, ':');
+			if (end) {
+				*rd_lenp = hextoul(++end, NULL);
+				*rd_datap = rd_addr;
+				processed = true;
 			}
+		}
+
+		if (!processed) {
 			puts("Wrong Ramdisk Image Format\n");
 			return -EINVAL;
 		}
+	}
 
 	return 0;
 }
-- 
2.33.0.685.g46640cef36-goog