[U-Boot] [PATCH v2 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode)
This patch series provides support for controlling bootcount limits in SPL. It also enables this feature on display5 board to present usage patterns.
This patch has been applied on top of u-boot/master: SHA1 (tag): v2018.03
Changes in v2: - New patch - None - None - 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 - Remove bootcount_init() from SPL specific board code - None
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: u-boot: Do not increment bootcount if already done in SPL 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 | 2 ++ common/spl/Kconfig | 9 +++++++++ common/spl/spl.c | 7 +++++++ configs/display5_defconfig | 4 ++++ drivers/Makefile | 1 + include/bootcount.h | 28 ++++++++++++++++++++++++++++ 7 files changed, 53 insertions(+), 1 deletion(-)
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.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: None
common/spl/Kconfig | 9 +++++++++ drivers/Makefile | 1 + 2 files changed, 10 insertions(+)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 9609fceea5..af6573af8f 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -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)) diff --git a/drivers/Makefile b/drivers/Makefile index 2673428cb6..a194039e91 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -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/
Lukasz Majewski lukma@denx.de hat am 14. März 2018 um 18:24 geschrieben:
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.
Signed-off-by: Lukasz Majewski lukma@denx.de
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - New patch
include/bootcount.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/bootcount.h b/include/bootcount.h index 06fb4d3578..e3b3f7028e 100644 --- a/include/bootcount.h +++ b/include/bootcount.h @@ -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__ */
On 14.03.2018 18:24, Lukasz Majewski wrote:
Signed-off-by: Lukasz Majewski lukma@denx.de
It is usually a requirement, to add a comment to the commit test. Even with such simple changes. So please add at least one line here.
Other thank this:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Those two functions can be used to provide easy bootcount management.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - None
include/bootcount.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/include/bootcount.h b/include/bootcount.h index e3b3f7028e..0ec9af6e2b 100644 --- a/include/bootcount.h +++ b/include/bootcount.h @@ -40,4 +40,29 @@ static inline u32 raw_bootcount_load(volatile u32 *addr) return in_be32(addr); } #endif + +#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT +static inline bool 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.\n", bootlimit); + return true; + } + + return false; +} + +static inline void bootcount_inc(void) +{ + unsigned long bootcount = bootcount_load(); + + bootcount_store(++bootcount); +} +#else +static inline bool bootcount_error(void) { return false; } +static inline void bootcount_inc(void) {} +#endif /* CONFIG_SPL_BOOTCOUNT_LIMIT */ #endif /* _BOOTCOUNT_H__ */
On 14.03.2018 18:24, Lukasz Majewski wrote:
Those two functions can be used to provide easy bootcount management.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
None
include/bootcount.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/include/bootcount.h b/include/bootcount.h index e3b3f7028e..0ec9af6e2b 100644 --- a/include/bootcount.h +++ b/include/bootcount.h @@ -40,4 +40,29 @@ static inline u32 raw_bootcount_load(volatile u32 *addr) return in_be32(addr); } #endif
+#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT +static inline bool 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.\n", bootlimit);return true;- }
- return false;
+}
+static inline void bootcount_inc(void) +{
- unsigned long bootcount = bootcount_load();
- bootcount_store(++bootcount);
+}
Why not:
static inline void bootcount_inc(void) { bootcount_store(bootcount_load() + 1); }
instead ?
+#else +static inline bool bootcount_error(void) { return false; } +static inline void bootcount_inc(void) {} +#endif /* CONFIG_SPL_BOOTCOUNT_LIMIT */ #endif /* _BOOTCOUNT_H__ */
And can't you use these helper functions in proper U-Boot as well (common/autoboot.c). Looking at this old code there, there is also room for improvement - perhaps as a new patch in this series. ;)
Thanks, Stefan
Hi Stefan,
On 14.03.2018 18:24, Lukasz Majewski wrote:
Those two functions can be used to provide easy bootcount management.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
None
include/bootcount.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/include/bootcount.h b/include/bootcount.h index e3b3f7028e..0ec9af6e2b 100644 --- a/include/bootcount.h +++ b/include/bootcount.h @@ -40,4 +40,29 @@ static inline u32 raw_bootcount_load(volatile u32 *addr) return in_be32(addr); } #endif
+#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT +static inline bool 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.\n",bootlimit);
return true;- }
- return false;
+}
+static inline void bootcount_inc(void) +{
- unsigned long bootcount = bootcount_load();
- bootcount_store(++bootcount);
+}
Why not:
static inline void bootcount_inc(void) { bootcount_store(bootcount_load() + 1); }
instead ?
+1
+#else +static inline bool bootcount_error(void) { return false; } +static inline void bootcount_inc(void) {} +#endif /* CONFIG_SPL_BOOTCOUNT_LIMIT */ #endif /* _BOOTCOUNT_H__ */
And can't you use these helper functions in proper U-Boot as well (common/autoboot.c). Looking at this old code there, there is also room for improvement
- perhaps as a new patch in this series. ;)
It would be best to have it on top of this series - as other patches depends on it....
But, OK I will check if this can be done.
Thanks for hint.
Thanks, Stefan
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@denx.de
If the CONFIG_SPL_BOOTCOUNT_LIMIT is defined, the bootcount variable is already incremented after each boot attempt.
For that reason we shall not increment it again in u-boot.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - None
common/autoboot.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/common/autoboot.c b/common/autoboot.c index 2eef7a04cc..87fca2ea92 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -298,7 +298,9 @@ const char *bootdelay_process(void)
#ifdef CONFIG_BOOTCOUNT_LIMIT bootcount = bootcount_load(); +#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT bootcount++; +#endif bootcount_store(bootcount); env_set_ulong("bootcount", bootcount); bootlimit = env_get_ulong("bootlimit", 10, 0);
On 14.03.2018 18:24, Lukasz Majewski wrote:
If the CONFIG_SPL_BOOTCOUNT_LIMIT is defined, the bootcount variable is already incremented after each boot attempt.
For that reason we shall not increment it again in u-boot.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
None
common/autoboot.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/common/autoboot.c b/common/autoboot.c index 2eef7a04cc..87fca2ea92 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -298,7 +298,9 @@ const char *bootdelay_process(void)
#ifdef CONFIG_BOOTCOUNT_LIMIT bootcount = bootcount_load(); +#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT bootcount++; +#endif
As mentioned in my other comment, please use the helper functions here as well. Perhaps you can move this #ifdef into the helper function, making this part here a bit clearer.
Thanks, Stefan
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).
Signed-off-by: Lukasz Majewski lukma@denx.de
---
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 | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/common/spl/spl.c b/common/spl/spl.c index b1ce56d0d0..01e7989869 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -20,6 +20,9 @@ #include <dm/root.h> #include <linux/compiler.h> #include <fdt_support.h> +#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT +#include <bootcount.h> +#endif
DECLARE_GLOBAL_DATA_PTR;
@@ -411,6 +414,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) spl_board_init(); #endif
+#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT + bootcount_inc(); +#endif + memset(&spl_image, '\0', sizeof(spl_image)); #ifdef CONFIG_SYS_SPL_ARGS_ADDR spl_image.arg = (void *)CONFIG_SYS_SPL_ARGS_ADDR;
Lukasz Majewski lukma@denx.de hat am 14. März 2018 um 18:24 geschrieben:
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).
Signed-off-by: Lukasz Majewski lukma@denx.de
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 | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/common/spl/spl.c b/common/spl/spl.c index b1ce56d0d0..01e7989869 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -20,6 +20,9 @@ #include <dm/root.h> #include <linux/compiler.h> #include <fdt_support.h> +#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT +#include <bootcount.h> +#endif
Is this #ifdef necessary? If not, please remove it.
DECLARE_GLOBAL_DATA_PTR;
@@ -411,6 +414,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) spl_board_init(); #endif
+#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT
- bootcount_inc();
+#endif
I think you can remove this #ifdef as well here. You have the #ifdef in the header already.
Thanks, Stefan
Hi Stefan,
Lukasz Majewski lukma@denx.de hat am 14. März 2018 um 18:24 geschrieben:
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).
Signed-off-by: Lukasz Majewski lukma@denx.de
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 | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/common/spl/spl.c b/common/spl/spl.c index b1ce56d0d0..01e7989869 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -20,6 +20,9 @@ #include <dm/root.h> #include <linux/compiler.h> #include <fdt_support.h> +#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT +#include <bootcount.h> +#endif
Is this #ifdef necessary? If not, please remove it.
The problem is that after removing those #ifdefs I do have SPL build breaks on .... x86 arch.
It seems like some kind of non-trival dependency introduced in generic bootcount.h u-boot file.
I even thought about introducing new file - like bootcount_spl.h
DECLARE_GLOBAL_DATA_PTR;
@@ -411,6 +414,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) spl_board_init(); #endif
+#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT
- bootcount_inc();
+#endif
I think you can remove this #ifdef as well here. You have the #ifdef in the header already.
Yes, it can be removed.
Thanks, Stefan
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@denx.de
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.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - Remove bootcount_init() from SPL specific board code
board/liebherr/display5/spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/liebherr/display5/spl.c b/board/liebherr/display5/spl.c index 437963e225..7712e5bc3f 100644 --- a/board/liebherr/display5/spl.c +++ b/board/liebherr/display5/spl.c @@ -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(); }
On 14.03.2018 18:24, Lukasz Majewski wrote:
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.
Signed-off-by: Lukasz Majewski lukma@denx.de
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
The boot count is enabled in both SPL and proper u-boot.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - None
configs/display5_defconfig | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/configs/display5_defconfig b/configs/display5_defconfig index 4d67700f4d..893794804c 100644 --- a/configs/display5_defconfig +++ b/configs/display5_defconfig @@ -16,6 +16,7 @@ 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=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 @@ -51,6 +52,9 @@ CONFIG_EFI_PARTITION=y # CONFIG_SPL_EFI_PARTITION is not set CONFIG_OF_CONTROL=y CONFIG_ENV_IS_IN_SPI_FLASH=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
On 14.03.2018 18:24, Lukasz Majewski wrote:
The boot count is enabled in both SPL and proper u-boot.
Signed-off-by: Lukasz Majewski lukma@denx.de
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
participants (3)
-
Lukasz Majewski -
Stefan Roese -
Stefan Roese