Skip to contentSkip to conversation

Provide SPL support for bootcount (in the case of using falcon boot mode)

PATCH Superseded Lukasz Majewski 3 participants 11 messages May 2, 2018

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v3 cover Provide SPL support for bootcount (in the case of using falcon boot mode)

This patch series provides support for controlling bootcount limits in SPL. Moreover, the common code has been identified and reused in the common/autoboot.c file.
It also enables this feature on display5 board to present usage patterns.

This patch has been applied on top of u-boot/master:
SHA1 : v2018.05-rc3

Test HW: Beagle Bone Black (am335x) , Display5 (imx6q)
Travis-Ci: https://travis-ci.org/lmajewski/u-boot-dfu/builds/373639971

Lukasz Majewski (7):
bootcount: spl: Enable bootcount support in SPL
bootcount: Add include guards into bootcount.h file
bootcount: Add function wrappers to handle bootcount increment and
error checking
bootcount: Rewrite autoboot to use wrapper functions from bootcount.h
bootcount: spl: Extend SPL to support bootcount incrementation
bootcount: display5: spl: Extend DISPLAY5 board SPL to support
bootcount checking
bootcount: display5: config: Enable boot count feature in the display5
board

 board/liebherr/display5/spl.c |  3 ++-
 common/autoboot.c             | 23 +++++---------------
 common/spl/Kconfig            |  9 ++++++++
 common/spl/spl.c              |  3 +++
 configs/display5_defconfig    |  4 ++++
 drivers/Makefile              |  1 +
 include/bootcount.h           | 50 +++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 74 insertions(+), 19 deletions(-)
Signature · 1 line
2.11.0
#1 lore raw

PATCH v3 1/7 bootcount: spl: Enable bootcount support in SPL

New, SPL related config option - CONFIG_SPL_BOOTCOUNT_LIMIT has been added to allow drivers/bootcount code re-usage in SPL.

This code is necessary to use and setup bootcount in SPL in the case of falcon boot mode.

---

Changes in v4:
- None

Changes in v3:
- None

Changes in v2:
- None

 common/spl/Kconfig | 9 +++++++++
 drivers/Makefile   | 1 +
 2 files changed, 10 insertions(+)
common/spl/Kconfig +9 -0
index 259f96607e..431710a93b 100644
@@ -54,6 +54,15 @@ config SPL_BOOTROM_SUPPORT
 	  BOOT_DEVICE_BOOTROM (or fall-through to the next boot device in the
 	  boot device list, if not implemented for a given board)
 
+config SPL_BOOTCOUNT_LIMIT
+	bool "Support bootcount in SPL"
+	depends on SPL_ENV_SUPPORT
+	help
+	  On some boards, which use 'falcon' mode, it is necessary to check
+	  and increment the number of boot attempts. Such boards do not
+	  use proper U-Boot for normal boot flow and hence needs those
+	  adjustments to be done in the SPL.
+
 config SPL_RAW_IMAGE_SUPPORT
 	bool "Support SPL loading and booting of RAW images"
 	default n if (ARCH_MX6 && (SPL_MMC_SUPPORT || SPL_SATA_SUPPORT))
drivers/Makefile +1 -0
index 6846d181aa..061331eadd 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_$(SPL_TPL_)TIMER) += timer/
 ifndef CONFIG_TPL_BUILD
 ifdef CONFIG_SPL_BUILD
 
+obj-$(CONFIG_SPL_BOOTCOUNT_LIMIT) += bootcount/
 obj-$(CONFIG_SPL_CPU_SUPPORT) += cpu/
 obj-$(CONFIG_SPL_CRYPTO_SUPPORT) += crypto/
 obj-$(CONFIG_SPL_GPIO_SUPPORT) += gpio/
Signature · 1 line
2.11.0
#2 lore raw

PATCH v3 2/7 bootcount: Add include guards into bootcount.h file

In reply to Lukasz Majewski

This patch adds missing include guards for bootcount.h file.

---

Changes in v4:
- None

Changes in v3:
- None

Changes in v2:
- New patch

 include/bootcount.h | 3 +++
 1 file changed, 3 insertions(+)
include/bootcount.h +3 -0
index 06fb4d3578..e3b3f7028e 100644
@@ -4,6 +4,8 @@
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
+#ifndef _BOOTCOUNT_H__
+#define _BOOTCOUNT_H__
 
 #include <common.h>
 #include <asm/io.h>
@@ -38,3 +40,4 @@ static inline u32 raw_bootcount_load(volatile u32 *addr)
 	return in_be32(addr);
 }
 #endif
+#endif /* _BOOTCOUNT_H__ */
Signature · 1 line
2.11.0
#3 lore raw

PATCH v3 3/7 bootcount: Add function wrappers to handle bootcount increment and error checking

In reply to Lukasz Majewski

Those two functions can be used to provide easy bootcount management.

---

Changes in v4:
- Remove enum bootcount_context and replace it with checking gd_t->flags (The GD_FLG_SPL_INIT is only set in SPL, it is cleared in u-boot proper, so can be used as indication if we are in u-boot or SPL).
- Do not call bootcount_store() twice when it is not needed.
- Call env_set_ulong("bootcount", bootcount); only in NON SPL context - Boards with TINY_PRINTF (in newest mainline) will build break as this function requires simple_itoa() from vsprintf.c (now not always build in SPL).

Changes in v3:
- Unify those functions to also work with common/autoboot.c code
- Add enum bootcount_context to distinguish between u-boot proper and SPL

Changes in v2:
- None

 include/bootcount.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)
include/bootcount.h +47 -0
index e3b3f7028e..919735c19a 100644
@@ -11,6 +11,8 @@
 #include <asm/io.h>
 #include <asm/byteorder.h>
 
+#if defined CONFIG_SPL_BOOTCOUNT_LIMIT || defined CONFIG_BOOTCOUNT_LIMIT
+
 #if !defined(CONFIG_SYS_BOOTCOUNT_LE) && !defined(CONFIG_SYS_BOOTCOUNT_BE)
 # if __BYTE_ORDER == __LITTLE_ENDIAN
 #  define CONFIG_SYS_BOOTCOUNT_LE
@@ -40,4 +42,49 @@ static inline u32 raw_bootcount_load(volatile u32 *addr)
 	return in_be32(addr);
 }
 #endif
+
+DECLARE_GLOBAL_DATA_PTR;
+static inline int bootcount_error(void)
+{
+	unsigned long bootcount = bootcount_load();
+	unsigned long bootlimit = env_get_ulong("bootlimit", 10, 0);
+
+	if (bootlimit && bootcount > bootlimit) {
+		printf("Warning: Bootlimit (%lu) exceeded.", bootlimit);
+		if (!(gd->flags & GD_FLG_SPL_INIT))
+			printf(" Using altbootcmd.");
+		printf("\n");
+
+		return 1;
+	}
+
+	return 0;
+}
+
+static inline void bootcount_inc(void)
+{
+	unsigned long bootcount = bootcount_load();
+
+	if (gd->flags & GD_FLG_SPL_INIT) {
+		bootcount_store(++bootcount);
+		return;
+	}
+
+#ifndef CONFIG_SPL_BUILD
+	/* Only increment bootcount when no bootcount support in SPL */
+#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT
+	bootcount_store(++bootcount);
+#endif
+	env_set_ulong("bootcount", bootcount);
+#endif /* !CONFIG_SPL_BUILD */
+}
+
+#if defined CONFIG_SPL_BUILD && !defined CONFIG_SPL_BOOTCOUNT_LIMIT
+void bootcount_store(ulong a) {};
+ulong bootcount_load(void) { return 0; }
+#endif /* CONFIG_SPL_BUILD && !CONFIG_SPL_BOOTCOUNT_LIMIT */
+#else
+static inline int bootcount_error(void) { return 0; }
+static inline void bootcount_inc(void) {}
+#endif /* CONFIG_SPL_BOOTCOUNT_LIMIT || CONFIG_BOOTCOUNT_LIMIT */
 #endif /* _BOOTCOUNT_H__ */
Signature · 1 line
2.11.0
#4 lore raw

PATCH v3 3/7 bootcount: Add function wrappers to handle bootcount increment and error checking

Quoted 79 lines · Lukasz Majewski

Those two functions can be used to provide easy bootcount management.

---

Changes in v4:
- Remove enum bootcount_context and replace it with checking gd_t->flags (The GD_FLG_SPL_INIT is only set in SPL, it is cleared in u-boot proper, so can be used as indication if we are in u-boot or SPL).
- Do not call bootcount_store() twice when it is not needed.
- Call env_set_ulong("bootcount", bootcount); only in NON SPL context - Boards with TINY_PRINTF (in newest mainline) will build break as this function requires simple_itoa() from vsprintf.c (now not always build in SPL).

Changes in v3:
- Unify those functions to also work with common/autoboot.c code
- Add enum bootcount_context to distinguish between u-boot proper and SPL

Changes in v2:
- None

  include/bootcount.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 47 insertions(+)
include/bootcount.h +38 -0
index e3b3f7028e..919735c19a 100644
@@ -11,6 +11,8 @@
  #include <asm/io.h>
  #include <asm/byteorder.h>
 
+#if defined CONFIG_SPL_BOOTCOUNT_LIMIT || defined CONFIG_BOOTCOUNT_LIMIT
+
  #if !defined(CONFIG_SYS_BOOTCOUNT_LE) && !defined(CONFIG_SYS_BOOTCOUNT_BE)
  # if __BYTE_ORDER == __LITTLE_ENDIAN
  #  define CONFIG_SYS_BOOTCOUNT_LE
@@ -40,4 +42,49 @@ static inline u32 raw_bootcount_load(volatile u32 *addr)
  	return in_be32(addr);
  }
  #endif
+
+DECLARE_GLOBAL_DATA_PTR;
+static inline int bootcount_error(void)
+{
+	unsigned long bootcount = bootcount_load();
+	unsigned long bootlimit = env_get_ulong("bootlimit", 10, 0);
+
+	if (bootlimit && bootcount > bootlimit) {
+		printf("Warning: Bootlimit (%lu) exceeded.", bootlimit);
+		if (!(gd->flags & GD_FLG_SPL_INIT))
+			printf(" Using altbootcmd.");
+		printf("\n");
+
+		return 1;
+	}
+
+	return 0;
+}
+
+static inline void bootcount_inc(void)
+{
+	unsigned long bootcount = bootcount_load();
+
+	if (gd->flags & GD_FLG_SPL_INIT) {
+		bootcount_store(++bootcount);
+		return;
+	}
+
+#ifndef CONFIG_SPL_BUILD
+	/* Only increment bootcount when no bootcount support in SPL */
+#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT
+	bootcount_store(++bootcount);
+#endif
+	env_set_ulong("bootcount", bootcount);
+#endif /* !CONFIG_SPL_BUILD */
+}

include/bootcount.h static inline u32 raw_bootcount_load(volatile u32 *addr) · line 90

Quoted 10 lines
include/bootcount.h +9 -0
+
+#if defined CONFIG_SPL_BUILD && !defined CONFIG_SPL_BOOTCOUNT_LIMIT
+void bootcount_store(ulong a) {};
+ulong bootcount_load(void) { return 0; }
+#endif /* CONFIG_SPL_BUILD && !CONFIG_SPL_BOOTCOUNT_LIMIT */
+#else
+static inline int bootcount_error(void) { return 0; }
+static inline void bootcount_inc(void) {}
+#endif /* CONFIG_SPL_BOOTCOUNT_LIMIT || CONFIG_BOOTCOUNT_LIMIT */
  #endif /* _BOOTCOUNT_H__ */

Looks better this way. Thanks for working on this:

Thanks,
Stefan

#5 lore raw

PATCH v3 3/7 bootcount: Add function wrappers to handle bootcount increment and error checking

In reply to Lukasz Majewski
Quoted 3 lines · Lukasz Majewski

Those two functions can be used to provide easy bootcount management.

[snip]

include/bootcount.h line 16

Quoted 3 lines
include/bootcount.h +2 -0
+#if defined CONFIG_SPL_BOOTCOUNT_LIMIT || defined CONFIG_BOOTCOUNT_LIMIT
+
 #if !defined(CONFIG_SYS_BOOTCOUNT_LE) && !defined(CONFIG_SYS_BOOTCOUNT_BE)

Style problem. We have a few places where we don't use parens in the code today, but this is the first time I've noticed and had to check quickly to see that. Please follow the normal convention of if defined(FOO) .... Otherwise:

And, erm, since you'll need to repost this one, please v5 the whole series since you accidentally did v3 again in the subject just to make it extra easy to see what version I should pull in later? Thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180502/cd219810/attachment.sig>
#6 lore raw

PATCH v3 3/7 bootcount: Add function wrappers to handle bootcount increment and error checking

Hi Tom,

Quoted 13 lines · Lukasz Majewski
Quoted 5 lines

Those two functions can be used to provide easy bootcount management.

[snip]

Quoted 4 lines

+#if defined CONFIG_SPL_BOOTCOUNT_LIMIT || defined CONFIG_BOOTCOUNT_LIMIT +
#if !defined(CONFIG_SYS_BOOTCOUNT_LE)
&& !defined(CONFIG_SYS_BOOTCOUNT_BE)

Quoted 4 lines

Style problem. We have a few places where we don't use parens in the code today, but this is the first time I've noticed and had to check quickly to see that. Please follow the normal convention of if defined(FOO) .... Otherwise:

I've just sent the corrected v5 to ML.

Quoted 7 lines

And, erm, since you'll need to repost this one, please v5 the whole series since you accidentally did v3 again in the subject just to make it extra easy to see what version I should pull in later? Thanks!

I suppose that this code shall be applied to -next anyway (as it is already too late in the current release cycle (rc3)).

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de -------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180502/dc8577bb/attachment.sig>

#7 lore raw

PATCH v3 4/7 bootcount: Rewrite autoboot to use wrapper functions from bootcount.h

In reply to Lukasz Majewski

The code has been refactored to use common wrappers from bootcount.h header.

---

Changes in v4:
- Use global data pointer (gd_t *) instead of bootcount specific enum

Changes in v3:
- New patch

Changes in v2:
- None

 common/autoboot.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)
common/autoboot.c +5 -18
index 2eef7a04cc..a0f7822c9e 100644
@@ -14,6 +14,7 @@
 #include <menu.h>
 #include <post.h>
 #include <u-boot/sha256.h>
+#include <bootcount.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -291,18 +292,8 @@ const char *bootdelay_process(void)
 {
 	char *s;
 	int bootdelay;
-#ifdef CONFIG_BOOTCOUNT_LIMIT
-	unsigned long bootcount = 0;
-	unsigned long bootlimit = 0;
-#endif /* CONFIG_BOOTCOUNT_LIMIT */
-
-#ifdef CONFIG_BOOTCOUNT_LIMIT
-	bootcount = bootcount_load();
-	bootcount++;
-	bootcount_store(bootcount);
-	env_set_ulong("bootcount", bootcount);
-	bootlimit = env_get_ulong("bootlimit", 10, 0);
-#endif /* CONFIG_BOOTCOUNT_LIMIT */
+
+	bootcount_inc();
 
 	s = env_get("bootdelay");
 	bootdelay = s ? (int)simple_strtol(s, NULL, 10) : CONFIG_BOOTDELAY;
@@ -324,13 +315,9 @@ const char *bootdelay_process(void)
 		s = env_get("failbootcmd");
 	} else
 #endif /* CONFIG_POST */
-#ifdef CONFIG_BOOTCOUNT_LIMIT
-	if (bootlimit && (bootcount > bootlimit)) {
-		printf("Warning: Bootlimit (%u) exceeded. Using altbootcmd.\n",
-		       (unsigned)bootlimit);
+	if (bootcount_error())
 		s = env_get("altbootcmd");
-	} else
-#endif /* CONFIG_BOOTCOUNT_LIMIT */
+	else
 		s = env_get("bootcmd");
 
 	process_fdt_options(gd->fdt_blob);
Signature · 1 line
2.11.0
#8 lore raw

PATCH v3 5/7 bootcount: spl: Extend SPL to support bootcount incrementation

In reply to Lukasz Majewski

This patch adds support for incrementation of the bootcount in SPL. Such feature is necessary when we do want to use this feature with 'falcon' boot mode (which loads OS directly in SPL).

---

Changes in v4:
- Use gd_t global data pointer instead of bootcount specific enum

Changes in v3:
- Remove not needed #ifdefs
- Add enum bootcount_context parameter to bootcount_inc() function

Changes in v2:
- New patch - as suggested by Stefan Roese - bootcount_inc() is called in common SPL code (./common/spl/spl.c), so other boards can also reuse it without modification

 common/spl/spl.c | 3 +++
 1 file changed, 3 insertions(+)
common/spl/spl.c +3 -0
index 794dbd0312..6eb50f3534 100644
@@ -20,6 +20,7 @@
 #include <dm/root.h>
 #include <linux/compiler.h>
 #include <fdt_support.h>
+#include <bootcount.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -417,6 +418,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	spl_board_init();
 #endif
 
+	bootcount_inc();
+
 	memset(&spl_image, '\0', sizeof(spl_image));
 #ifdef CONFIG_SYS_SPL_ARGS_ADDR
 	spl_image.arg = (void *)CONFIG_SYS_SPL_ARGS_ADDR;
Signature · 1 line
2.11.0
#9 lore raw

PATCH v3 6/7 bootcount: display5: spl: Extend DISPLAY5 board SPL to support bootcount checking

In reply to Lukasz Majewski

This patch is necessary for providing basic bootcount checking in the case of using "falcon" boot mode in that board.

It forces u-boot proper boot, when we exceed the number of errors.

---

Changes in v4:
- Use global data pointer (gd) instead of bootcount specific enum (SPL)

Changes in v3:
- The bootcount_error now accepts enum bootcount_error input parameter

Changes in v2:
- Remove bootcount_init() from SPL specific board code

 board/liebherr/display5/spl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
board/liebherr/display5/spl.c +2 -1
index 437963e225..7712e5bc3f 100644
@@ -20,6 +20,7 @@
 #include <environment.h>
 #include <fsl_esdhc.h>
 #include <netdev.h>
+#include <bootcount.h>
 #include "common.h"
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -214,7 +215,7 @@ void board_boot_order(u32 *spl_boot_list)
 	env_load();
 
 	s = env_get("BOOT_FROM");
-	if (s && strcmp(s, "ACTIVE") == 0) {
+	if (s && !bootcount_error() && strcmp(s, "ACTIVE") == 0) {
 		spl_boot_list[0] = BOOT_DEVICE_MMC1;
 		spl_boot_list[1] = spl_boot_device();
 	}
Signature · 1 line
2.11.0
#10 lore raw

PATCH v3 7/7 bootcount: display5: config: Enable boot count feature in the display5 board

In reply to Lukasz Majewski

The boot count is enabled in both SPL and proper u-boot.

---

Changes in v4:
- None

Changes in v3:
- None

Changes in v2:
- None

 configs/display5_defconfig | 4 ++++
 1 file changed, 4 insertions(+)
configs/display5_defconfig +4 -0
index e52f4e00af..db8212ca7c 100644
@@ -16,6 +16,7 @@ CONFIG_SPL_LOAD_FIT=y
 CONFIG_OF_BOARD_SETUP=y
 CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=arch/arm/mach-imx/spl_sd.cfg,MX6Q"
 CONFIG_SUPPORT_RAW_INITRD=y
+CONFIG_SPL_BOOTCOUNT_LIMIT=y
 # CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR is not set
 CONFIG_SPL_DMA_SUPPORT=y
 CONFIG_SPL_ENV_SUPPORT=y
@@ -53,6 +54,9 @@ CONFIG_EFI_PARTITION=y
 CONFIG_OF_CONTROL=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_FSL_ESDHC=y
+CONFIG_BOOTCOUNT_LIMIT=y
+CONFIG_SYS_BOOTCOUNT_SINGLEWORD=y
+CONFIG_SYS_BOOTCOUNT_ADDR=0x020CC068
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_BAR=y
 CONFIG_SPI_FLASH_SPANSION=y
Signature · 1 line
2.11.0