All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] spl: mmc: Factor out eMMC boot partition selection code
@ 2021-06-30 23:08 ` Marek Vasut
  2021-06-30 23:08   ` [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log Marek Vasut
  2021-06-30 23:36   ` [PATCH 1/2] spl: mmc: Factor out eMMC boot partition selection code Jaehoon Chung
  0 siblings, 2 replies; 7+ messages in thread
From: Marek Vasut @ 2021-06-30 23:08 UTC (permalink / raw)
  To: u-boot
  Cc: Marek Vasut, Faiz Abbas, Harald Seiler, Lokesh Vutla,
	Simon Glass, Fabio Estevam, Peng Fan, Stefano Babic, Ye Li

Factor out eMMC boot partition selection code into
default_spl_mmc_emmc_boot_partition() function and implement
weak spl_mmc_emmc_boot_partition(), so that architecture or
board code can override the eMMC boot partition selection.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Faiz Abbas <faiz_abbas@ti.com>
Cc: Harald Seiler <hws@denx.de>
Cc: Lokesh Vutla <lokeshvutla@ti.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Ye Li <ye.li@nxp.com>
---
 common/spl/spl_mmc.c | 37 ++++++++++++++++++++++++-------------
 include/spl.h        | 25 +++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index add2785b4e3..2377d0937d1 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -324,6 +324,29 @@ unsigned long __weak spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
 	return raw_sect;
 }
 
+int default_spl_mmc_emmc_boot_partition(struct mmc *mmc)
+{
+	int part;
+#ifdef CONFIG_SYS_MMCSD_RAW_MODE_EMMC_BOOT_PARTITION
+	part = CONFIG_SYS_MMCSD_RAW_MODE_EMMC_BOOT_PARTITION;
+#else
+	/*
+	 * We need to check what the partition is configured to.
+	 * 1 and 2 match up to boot0 / boot1 and 7 is user data
+	 * which is the first physical partition (0).
+	 */
+	part = (mmc->part_config >> 3) & PART_ACCESS_MASK;
+	if (part == 7)
+		part = 0;
+#endif
+	return part;
+}
+
+int __weak spl_mmc_emmc_boot_partition(struct mmc *mmc)
+{
+	return default_spl_mmc_emmc_boot_partition(mmc);
+}
+
 int spl_mmc_load(struct spl_image_info *spl_image,
 		 struct spl_boot_device *bootdev,
 		 const char *filename,
@@ -355,19 +378,7 @@ int spl_mmc_load(struct spl_image_info *spl_image,
 	err = -EINVAL;
 	switch (boot_mode) {
 	case MMCSD_MODE_EMMCBOOT:
-#ifdef CONFIG_SYS_MMCSD_RAW_MODE_EMMC_BOOT_PARTITION
-		part = CONFIG_SYS_MMCSD_RAW_MODE_EMMC_BOOT_PARTITION;
-#else
-		/*
-		 * We need to check what the partition is configured to.
-		 * 1 and 2 match up to boot0 / boot1 and 7 is user data
-		 * which is the first physical partition (0).
-		 */
-		part = (mmc->part_config >> 3) & PART_ACCESS_MASK;
-
-		if (part == 7)
-			part = 0;
-#endif
+		part = spl_mmc_emmc_boot_partition(mmc);
 
 		if (CONFIG_IS_ENABLED(MMC_TINY))
 			err = mmc_switch_part(mmc, part);
diff --git a/include/spl.h b/include/spl.h
index cee9a42ddb5..b6698a686a7 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -357,6 +357,31 @@ u32 spl_mmc_boot_mode(const u32 boot_device);
  * If not overridden, it is weakly defined in common/spl/spl_mmc.c.
  */
 int spl_mmc_boot_partition(const u32 boot_device);
+
+struct mmc;
+/**
+ * default_spl_mmc_emmc_boot_partition() - eMMC boot partition to load U-Boot from.
+ * @boot_device:	ID of the device which the MMC driver wants to load
+ *			U-Boot from.
+ *
+ * This function should return the eMMC boot partition number which
+ * the SPL should load U-Boot from (on the given boot_device).
+ */
+int default_spl_mmc_emmc_boot_partition(struct mmc *mmc);
+
+/**
+ * spl_mmc_emmc_boot_partition() - eMMC boot partition to load U-Boot from.
+ * @boot_device:	ID of the device which the MMC driver wants to load
+ *			U-Boot from.
+ *
+ * This function should return the eMMC boot partition number which
+ * the SPL should load U-Boot from (on the given boot_device).
+ *
+ * If not overridden, it is weakly defined in common/spl/spl_mmc.c
+ * and calls default_spl_mmc_emmc_boot_partition();
+ */
+int spl_mmc_emmc_boot_partition(struct mmc *mmc);
+
 void spl_set_bd(void);
 
 /**
-- 
2.30.2


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

* [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log
  2021-06-30 23:08 ` [PATCH 1/2] spl: mmc: Factor out eMMC boot partition selection code Marek Vasut
@ 2021-06-30 23:08   ` Marek Vasut
  2021-07-01 10:07     ` Teresa Remmet
  2021-07-01 11:22     ` Harald Seiler
  2021-06-30 23:36   ` [PATCH 1/2] spl: mmc: Factor out eMMC boot partition selection code Jaehoon Chung
  1 sibling, 2 replies; 7+ messages in thread
From: Marek Vasut @ 2021-06-30 23:08 UTC (permalink / raw)
  To: u-boot
  Cc: Marek Vasut, Faiz Abbas, Harald Seiler, Lokesh Vutla,
	Simon Glass, Fabio Estevam, Peng Fan, Stefano Babic, Ye Li

In case the iMX8M boot from eMMC boot partition and the primary image
is corrupted, the BootROM is capable of starting a secondary image in
the other eMMC boot partition as a fallback.

However, the BootROM leaves the eMMC BOOT_PARTITION_ENABLE setting as
it was, i.e. pointing to the boot partition containing the corrupted
image, and the BootROM does not provide any indication that this sort
of fallback occured.

According to AN12853 i.MX ROMs Log Events, Rev. 0, May 2020, it is
possible to determine whether fallback event occurred by parsing the
ROM event log. In case ROM event ID 0x51 is present, fallback event
did occur.

This patch implements ROM event log parsing and search for event ID
0x51 for all iMX8M SoCs, and based on that corrects the eMMC boot
partition selection. This way, the SPL loads the remaining boot
components from the same eMMC boot partition from which it was
started, even in case of the fallback.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Faiz Abbas <faiz_abbas@ti.com>
Cc: Harald Seiler <hws@denx.de>
Cc: Lokesh Vutla <lokeshvutla@ti.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Ye Li <ye.li@nxp.com>
---
 arch/arm/mach-imx/imx8m/soc.c | 61 +++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 0c44022a6dc..92a71b6ba29 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -571,6 +571,67 @@ enum boot_device get_boot_device(void)
 }
 #endif
 
+#if defined(CONFIG_IMX8M)
+#include <spl.h>
+int spl_mmc_emmc_boot_partition(struct mmc *mmc)
+{
+	u32 *rom_log_addr = (u32 *)0x9e0;
+	u32 *rom_log;
+	u8 event_id;
+	int i, part;
+
+	part = default_spl_mmc_emmc_boot_partition(mmc);
+
+	/* If the ROM event log pointer is not valid. */
+	if (*rom_log_addr < 0x900000 || *rom_log_addr >= 0xb00000 ||
+	    *rom_log_addr & 0x3)
+		return part;
+
+	/* Parse the ROM event ID version 2 log */
+	rom_log = (u32 *)(uintptr_t)(*rom_log_addr);
+	for (i = 0; i < 128; i++) {
+		event_id = rom_log[i] >> 24;
+		switch (event_id) {
+		case 0x00: /* End of list */
+			break;
+		/* Log entries with 1 parameter, skip 1 */
+		case 0x80: /* Start to perform the device initialization */
+		case 0x81: /* The boot device initialization completes */
+		case 0x8f: /* The boot device initialization fails */
+		case 0x90: /* Start to read data from boot device */
+		case 0x91: /* Reading data from boot device completes */
+		case 0x9f: /* Reading data from boot device fails */
+			i += 1;
+			continue;
+		/* Log entries with 2 parameters, skip 2 */
+		case 0xa0: /* Image authentication result */
+		case 0xc0: /* Jump to the boot image soon */
+			i += 2;
+			continue;
+		/* Boot from the secondary boot image */
+		case 0x51:
+			/*
+			 * Swap the eMMC boot partitions in case there was a
+			 * fallback event (i.e. primary image was corrupted
+			 * and that corruption was recognized by the BootROM),
+			 * so the SPL loads the rest of the U-Boot from the
+			 * correct eMMC boot partition, since the BootROM
+			 * leaves the boot partition set to the corrupted one.
+			 */
+			if (part == 1)
+				part = 2;
+			else if (part == 2)
+				part = 1;
+			continue;
+		default:
+			continue;
+		}
+	}
+
+	return part;
+}
+#endif
+
 bool is_usb_boot(void)
 {
 	return get_boot_device() == USB_BOOT;
-- 
2.30.2


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

* Re: [PATCH 1/2] spl: mmc: Factor out eMMC boot partition selection code
  2021-06-30 23:08 ` [PATCH 1/2] spl: mmc: Factor out eMMC boot partition selection code Marek Vasut
  2021-06-30 23:08   ` [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log Marek Vasut
@ 2021-06-30 23:36   ` Jaehoon Chung
  1 sibling, 0 replies; 7+ messages in thread
From: Jaehoon Chung @ 2021-06-30 23:36 UTC (permalink / raw)
  To: Marek Vasut, u-boot
  Cc: Faiz Abbas, Harald Seiler, Lokesh Vutla, Simon Glass,
	Fabio Estevam, Peng Fan, Stefano Babic, Ye Li

Hi Marek,

On 7/1/21 8:08 AM, Marek Vasut wrote:
> Factor out eMMC boot partition selection code into
> default_spl_mmc_emmc_boot_partition() function and implement
> weak spl_mmc_emmc_boot_partition(), so that architecture or
> board code can override the eMMC boot partition selection.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Faiz Abbas <faiz_abbas@ti.com>
> Cc: Harald Seiler <hws@denx.de>
> Cc: Lokesh Vutla <lokeshvutla@ti.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Ye Li <ye.li@nxp.com>
> ---
>  common/spl/spl_mmc.c | 37 ++++++++++++++++++++++++-------------
>  include/spl.h        | 25 +++++++++++++++++++++++++
>  2 files changed, 49 insertions(+), 13 deletions(-)
> 
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index add2785b4e3..2377d0937d1 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -324,6 +324,29 @@ unsigned long __weak spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
>  	return raw_sect;
>  }
>  
> +int default_spl_mmc_emmc_boot_partition(struct mmc *mmc)
> +{
> +	int part;
> +#ifdef CONFIG_SYS_MMCSD_RAW_MODE_EMMC_BOOT_PARTITION
> +	part = CONFIG_SYS_MMCSD_RAW_MODE_EMMC_BOOT_PARTITION;
> +#else
> +	/*
> +	 * We need to check what the partition is configured to.
> +	 * 1 and 2 match up to boot0 / boot1 and 7 is user data
> +	 * which is the first physical partition (0).
> +	 */
> +	part = (mmc->part_config >> 3) & PART_ACCESS_MASK;
> +	if (part == 7)
> +		part = 0;
> +#endif
> +	return part;
> +}
> +
> +int __weak spl_mmc_emmc_boot_partition(struct mmc *mmc)
> +{
> +	return default_spl_mmc_emmc_boot_partition(mmc);
> +}
> +
>  int spl_mmc_load(struct spl_image_info *spl_image,
>  		 struct spl_boot_device *bootdev,
>  		 const char *filename,
> @@ -355,19 +378,7 @@ int spl_mmc_load(struct spl_image_info *spl_image,
>  	err = -EINVAL;
>  	switch (boot_mode) {
>  	case MMCSD_MODE_EMMCBOOT:
> -#ifdef CONFIG_SYS_MMCSD_RAW_MODE_EMMC_BOOT_PARTITION
> -		part = CONFIG_SYS_MMCSD_RAW_MODE_EMMC_BOOT_PARTITION;
> -#else
> -		/*
> -		 * We need to check what the partition is configured to.
> -		 * 1 and 2 match up to boot0 / boot1 and 7 is user data
> -		 * which is the first physical partition (0).
> -		 */
> -		part = (mmc->part_config >> 3) & PART_ACCESS_MASK;
> -
> -		if (part == 7)
> -			part = 0;
> -#endif
> +		part = spl_mmc_emmc_boot_partition(mmc);
>  
>  		if (CONFIG_IS_ENABLED(MMC_TINY))
>  			err = mmc_switch_part(mmc, part);
> diff --git a/include/spl.h b/include/spl.h
> index cee9a42ddb5..b6698a686a7 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -357,6 +357,31 @@ u32 spl_mmc_boot_mode(const u32 boot_device);
>   * If not overridden, it is weakly defined in common/spl/spl_mmc.c.
>   */
>  int spl_mmc_boot_partition(const u32 boot_device);
> +
> +struct mmc;
> +/**
> + * default_spl_mmc_emmc_boot_partition() - eMMC boot partition to load U-Boot from.
> + * @boot_device:	ID of the device which the MMC driver wants to load
> + *			U-Boot from.

Is "@boot_device" right? Argument is mmc.

> + *
> + * This function should return the eMMC boot partition number which
> + * the SPL should load U-Boot from (on the given boot_device).
> + */
> +int default_spl_mmc_emmc_boot_partition(struct mmc *mmc);
> +
> +/**
> + * spl_mmc_emmc_boot_partition() - eMMC boot partition to load U-Boot from.
> + * @boot_device:	ID of the device which the MMC driver wants to load
> + *			U-Boot from.

Ditto.

Best Regards,
Jaehoon Chung

> + *
> + * This function should return the eMMC boot partition number which
> + * the SPL should load U-Boot from (on the given boot_device).
> + *
> + * If not overridden, it is weakly defined in common/spl/spl_mmc.c
> + * and calls default_spl_mmc_emmc_boot_partition();
> + */
> +int spl_mmc_emmc_boot_partition(struct mmc *mmc);
> +
>  void spl_set_bd(void);
>  
>  /**
> 


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

* Re: [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log
  2021-06-30 23:08   ` [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log Marek Vasut
@ 2021-07-01 10:07     ` Teresa Remmet
  2021-07-03  3:05       ` Marek Vasut
  2021-07-01 11:22     ` Harald Seiler
  1 sibling, 1 reply; 7+ messages in thread
From: Teresa Remmet @ 2021-07-01 10:07 UTC (permalink / raw)
  To: marex, u-boot
  Cc: ye.li, faiz_abbas, festevam, peng.fan, sbabic, lokeshvutla, hws, sjg

Hello Marek,

Am Donnerstag, den 01.07.2021, 01:08 +0200 schrieb Marek Vasut:
> In case the iMX8M boot from eMMC boot partition and the primary image
> is corrupted, the BootROM is capable of starting a secondary image in
> the other eMMC boot partition as a fallback.

I would like to ask a more general question. As I could not find
information about that. What are the criteria for the BootROM to
consider a image as corrupted? 
I remember on other platforms with NAND where it was a erased page or
too many bitflips. What is it here with eMMC?

Thanks,
Teresa

> 
> However, the BootROM leaves the eMMC BOOT_PARTITION_ENABLE setting as
> it was, i.e. pointing to the boot partition containing the corrupted
> image, and the BootROM does not provide any indication that this sort
> of fallback occured.
> 
> According to AN12853 i.MX ROMs Log Events, Rev. 0, May 2020, it is
> possible to determine whether fallback event occurred by parsing the
> ROM event log. In case ROM event ID 0x51 is present, fallback event
> did occur.
> 
> This patch implements ROM event log parsing and search for event ID
> 0x51 for all iMX8M SoCs, and based on that corrects the eMMC boot
> partition selection. This way, the SPL loads the remaining boot
> components from the same eMMC boot partition from which it was
> started, even in case of the fallback.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Faiz Abbas <faiz_abbas@ti.com>
> Cc: Harald Seiler <hws@denx.de>
> Cc: Lokesh Vutla <lokeshvutla@ti.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Ye Li <ye.li@nxp.com>
> ---
>  arch/arm/mach-imx/imx8m/soc.c | 61
> +++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-
> imx/imx8m/soc.c
> index 0c44022a6dc..92a71b6ba29 100644
> --- a/arch/arm/mach-imx/imx8m/soc.c
> +++ b/arch/arm/mach-imx/imx8m/soc.c
> @@ -571,6 +571,67 @@ enum boot_device get_boot_device(void)
>  }
>  #endif
>  
> +#if defined(CONFIG_IMX8M)
> +#include <spl.h>
> +int spl_mmc_emmc_boot_partition(struct mmc *mmc)
> +{
> +	u32 *rom_log_addr = (u32 *)0x9e0;
> +	u32 *rom_log;
> +	u8 event_id;
> +	int i, part;
> +
> +	part = default_spl_mmc_emmc_boot_partition(mmc);
> +
> +	/* If the ROM event log pointer is not valid. */
> +	if (*rom_log_addr < 0x900000 || *rom_log_addr >= 0xb00000 ||
> +	    *rom_log_addr & 0x3)
> +		return part;
> +
> +	/* Parse the ROM event ID version 2 log */
> +	rom_log = (u32 *)(uintptr_t)(*rom_log_addr);
> +	for (i = 0; i < 128; i++) {
> +		event_id = rom_log[i] >> 24;
> +		switch (event_id) {
> +		case 0x00: /* End of list */
> +			break;
> +		/* Log entries with 1 parameter, skip 1 */
> +		case 0x80: /* Start to perform the device
> initialization */
> +		case 0x81: /* The boot device initialization completes
> */
> +		case 0x8f: /* The boot device initialization fails */
> +		case 0x90: /* Start to read data from boot device */
> +		case 0x91: /* Reading data from boot device completes
> */
> +		case 0x9f: /* Reading data from boot device fails */
> +			i += 1;
> +			continue;
> +		/* Log entries with 2 parameters, skip 2 */
> +		case 0xa0: /* Image authentication result */
> +		case 0xc0: /* Jump to the boot image soon */
> +			i += 2;
> +			continue;
> +		/* Boot from the secondary boot image */
> +		case 0x51:
> +			/*
> +			 * Swap the eMMC boot partitions in case there
> was a
> +			 * fallback event (i.e. primary image was
> corrupted
> +			 * and that corruption was recognized by the
> BootROM),
> +			 * so the SPL loads the rest of the U-Boot from
> the
> +			 * correct eMMC boot partition, since the
> BootROM
> +			 * leaves the boot partition set to the
> corrupted one.
> +			 */
> +			if (part == 1)
> +				part = 2;
> +			else if (part == 2)
> +				part = 1;
> +			continue;
> +		default:
> +			continue;
> +		}
> +	}
> +
> +	return part;
> +}
> +#endif
> +
>  bool is_usb_boot(void)
>  {
>  	return get_boot_device() == USB_BOOT;

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

* Re: [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log
  2021-06-30 23:08   ` [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log Marek Vasut
  2021-07-01 10:07     ` Teresa Remmet
@ 2021-07-01 11:22     ` Harald Seiler
  2021-07-03  2:55       ` Marek Vasut
  1 sibling, 1 reply; 7+ messages in thread
From: Harald Seiler @ 2021-07-01 11:22 UTC (permalink / raw)
  To: Marek Vasut, u-boot
  Cc: Faiz Abbas, Lokesh Vutla, Simon Glass, Fabio Estevam, Peng Fan,
	Stefano Babic, Ye Li

Hi,

On Thu, 2021-07-01 at 01:08 +0200, Marek Vasut wrote:
> In case the iMX8M boot from eMMC boot partition and the primary image
> is corrupted, the BootROM is capable of starting a secondary image in
> the other eMMC boot partition as a fallback.
> 
> However, the BootROM leaves the eMMC BOOT_PARTITION_ENABLE setting as
> it was, i.e. pointing to the boot partition containing the corrupted
> image, and the BootROM does not provide any indication that this sort
> of fallback occured.
> 
> According to AN12853 i.MX ROMs Log Events, Rev. 0, May 2020, it is
> possible to determine whether fallback event occurred by parsing the
> ROM event log. In case ROM event ID 0x51 is present, fallback event
> did occur.
> 
> This patch implements ROM event log parsing and search for event ID
> 0x51 for all iMX8M SoCs, and based on that corrects the eMMC boot
> partition selection. This way, the SPL loads the remaining boot
> components from the same eMMC boot partition from which it was
> started, even in case of the fallback.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Faiz Abbas <faiz_abbas@ti.com>
> Cc: Harald Seiler <hws@denx.de>
> Cc: Lokesh Vutla <lokeshvutla@ti.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Ye Li <ye.li@nxp.com>
> ---
>  arch/arm/mach-imx/imx8m/soc.c | 61 +++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> index 0c44022a6dc..92a71b6ba29 100644
> --- a/arch/arm/mach-imx/imx8m/soc.c
> +++ b/arch/arm/mach-imx/imx8m/soc.c
> @@ -571,6 +571,67 @@ enum boot_device get_boot_device(void)
>  }
>  #endif
>  
> 
> +#if defined(CONFIG_IMX8M)
> +#include <spl.h>
> +int spl_mmc_emmc_boot_partition(struct mmc *mmc)
> +{
> +	u32 *rom_log_addr = (u32 *)0x9e0;
> +	u32 *rom_log;
> +	u8 event_id;
> +	int i, part;
> +
> +	part = default_spl_mmc_emmc_boot_partition(mmc);
> +
> +	/* If the ROM event log pointer is not valid. */
> +	if (*rom_log_addr < 0x900000 || *rom_log_addr >= 0xb00000 ||
> +	    *rom_log_addr & 0x3)
> +		return part;
> +
> +	/* Parse the ROM event ID version 2 log */
> +	rom_log = (u32 *)(uintptr_t)(*rom_log_addr);
> +	for (i = 0; i < 128; i++) {
> +		event_id = rom_log[i] >> 24;
> +		switch (event_id) {
> +		case 0x00: /* End of list */
> +			break;

I assume your intention here is to break from the for loop?  This `break`
will only exit the switch statement, so the loop will continue running on
the data following the "End of list".  Or is this behavior intentional?
In that case I'd find the use of `continue` in the other branches a bit
odd, as `continue` and `break` do the same thing in this situation.

-- 
Harald

> +		/* Log entries with 1 parameter, skip 1 */
> +		case 0x80: /* Start to perform the device initialization */
> +		case 0x81: /* The boot device initialization completes */
> +		case 0x8f: /* The boot device initialization fails */
> +		case 0x90: /* Start to read data from boot device */
> +		case 0x91: /* Reading data from boot device completes */
> +		case 0x9f: /* Reading data from boot device fails */
> +			i += 1;
> +			continue;
> +		/* Log entries with 2 parameters, skip 2 */
> +		case 0xa0: /* Image authentication result */
> +		case 0xc0: /* Jump to the boot image soon */
> +			i += 2;
> +			continue;
> +		/* Boot from the secondary boot image */
> +		case 0x51:
> +			/*
> +			 * Swap the eMMC boot partitions in case there was a
> +			 * fallback event (i.e. primary image was corrupted
> +			 * and that corruption was recognized by the BootROM),
> +			 * so the SPL loads the rest of the U-Boot from the
> +			 * correct eMMC boot partition, since the BootROM
> +			 * leaves the boot partition set to the corrupted one.
> +			 */
> +			if (part == 1)
> +				part = 2;
> +			else if (part == 2)
> +				part = 1;
> +			continue;
> +		default:
> +			continue;
> +		}
> +	}
> +
> +	return part;
> +}
> +#endif
> +
>  bool is_usb_boot(void)
>  {
>  	return get_boot_device() == USB_BOOT;


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

* Re: [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log
  2021-07-01 11:22     ` Harald Seiler
@ 2021-07-03  2:55       ` Marek Vasut
  0 siblings, 0 replies; 7+ messages in thread
From: Marek Vasut @ 2021-07-03  2:55 UTC (permalink / raw)
  To: Harald Seiler, u-boot
  Cc: Faiz Abbas, Lokesh Vutla, Simon Glass, Fabio Estevam, Peng Fan,
	Stefano Babic, Ye Li

On 7/1/21 1:22 PM, Harald Seiler wrote:

[...]

>> +	/* Parse the ROM event ID version 2 log */
>> +	rom_log = (u32 *)(uintptr_t)(*rom_log_addr);
>> +	for (i = 0; i < 128; i++) {
>> +		event_id = rom_log[i] >> 24;
>> +		switch (event_id) {
>> +		case 0x00: /* End of list */
>> +			break;
> 
> I assume your intention here is to break from the for loop?  This `break`
> will only exit the switch statement, so the loop will continue running on
> the data following the "End of list".  Or is this behavior intentional?
> In that case I'd find the use of `continue` in the other branches a bit
> odd, as `continue` and `break` do the same thing in this situation.

Nope, that should indeed be 'return part' here.

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

* Re: [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log
  2021-07-01 10:07     ` Teresa Remmet
@ 2021-07-03  3:05       ` Marek Vasut
  0 siblings, 0 replies; 7+ messages in thread
From: Marek Vasut @ 2021-07-03  3:05 UTC (permalink / raw)
  To: Teresa Remmet, u-boot
  Cc: ye.li, faiz_abbas, festevam, peng.fan, sbabic, lokeshvutla, hws, sjg

On 7/1/21 12:07 PM, Teresa Remmet wrote:

Hi

[...]

> Am Donnerstag, den 01.07.2021, 01:08 +0200 schrieb Marek Vasut:
>> In case the iMX8M boot from eMMC boot partition and the primary image
>> is corrupted, the BootROM is capable of starting a secondary image in
>> the other eMMC boot partition as a fallback.
> 
> I would like to ask a more general question. As I could not find
> information about that. What are the criteria for the BootROM to
> consider a image as corrupted?
> I remember on other platforms with NAND where it was a erased page or
> too many bitflips. What is it here with eMMC?

I didn't find much information on the unsigned images, so there I 
suspect it is just that parsing the IVT header fails, the DCD tag is 
missing, something along those lines. There is no checksum, so if there 
is a bitflip in the image itself, I can imagine it would go undetected. 
At least in the SPL part of flash.bin, the rest is fitImage and that has 
checksum.

For signed images, if there is a bitflip, the signature would no longer 
be valid, so this could be used to detect image corruption.

Maybe Peng (on CC) can clarify this better ?

[...]

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

end of thread, other threads:[~2021-07-03  3:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210630230824epcas1p40231e1adbda82e8ed745349f83e07a5f@epcas1p4.samsung.com>
2021-06-30 23:08 ` [PATCH 1/2] spl: mmc: Factor out eMMC boot partition selection code Marek Vasut
2021-06-30 23:08   ` [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log Marek Vasut
2021-07-01 10:07     ` Teresa Remmet
2021-07-03  3:05       ` Marek Vasut
2021-07-01 11:22     ` Harald Seiler
2021-07-03  2:55       ` Marek Vasut
2021-06-30 23:36   ` [PATCH 1/2] spl: mmc: Factor out eMMC boot partition selection code Jaehoon Chung

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