[U-Boot] [PATCH v5 3/7] bootcount: Add function wrappers to handle bootcount increment and error checking

[Alex Kiernan]

On Wed, May 2, 2018 at 3:11 PM Lukasz Majewski <lukma at denx.de> wrote:

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

> Signed-off-by: Lukasz Majewski <lukma at denx.de>

> Reviewed-by: Tom Rini <trini at konsulko.com>
> Reviewed-by: Stefan Roese <sr at denx.de>
> ---

> Changes in v5:
> - Provide parenthesis for #if defined(FOO) && ...

> 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(+)

> diff --git a/include/bootcount.h b/include/bootcount.h
> index e3b3f7028e..a886c98f44 100644
> --- a/include/bootcount.h
> +++ b/include/bootcount.h
> @@ -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 */
> +}
> +

I'm kinda confused by this code... isn't this equivalent.?

   static inline void bootcount_inc(void)
   {
          unsigned long bootcount = bootcount_load();

          bootcount_store(++bootcount);
   #ifndef CONFIG_SPL_BUILD
          env_set_ulong("bootcount", bootcount);
   #endif /* !CONFIG_SPL_BUILD */
   }

Also I suspect bootcount_store() will fail at link time on boards where the
bootcount is stored in ext4?

> +#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__ */
> --
> 2.11.0



-- 
Alex Kiernan