[PATCH] bootcount_ext: Add flag to enable/disable bootcount

Simon Glass sjg at chromium.org
Thu Mar 12 04:22:19 CET 2020


Hi Frédéric,

On Wed, 11 Mar 2020 at 05:00, Frédéric Danis <frederic.danis at collabora.com>
wrote:
>
> After a successful upgrade, multiple problem during boot sequence may
> trigger the altbootcmd process.
> This patch adds a 3rd byte to the bootcount file to enable/disable the
> bootcount check.
> When reading old bootcount file, 2 bytes long, it will consider that
> bootcount is enabled, and update the file accordingly.
>

Is this format documented somewhere? Should it perhaps have a version
number so you can copy with future changes?

> The bootcount file is only saved when `upgrade_available` is true, this
> allows to save writes to the filesystem.
>
> Signed-off-by: Frédéric Danis <frederic.danis at collabora.com>
> ---
>  drivers/bootcount/bootcount_ext.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/bootcount/bootcount_ext.c
b/drivers/bootcount/bootcount_ext.c
> index 075e590896..740bce0296 100644
> --- a/drivers/bootcount/bootcount_ext.c
> +++ b/drivers/bootcount/bootcount_ext.c
> @@ -9,6 +9,8 @@
>
>  #define BC_MAGIC       0xbc
>
> +static u8 upgrade_available = 1;
> +
>  void bootcount_store(ulong a)
>  {
>         u8 *buf;
> @@ -21,13 +23,18 @@ void bootcount_store(ulong a)
>                 return;
>         }
>
> -       buf = map_sysmem(CONFIG_SYS_BOOTCOUNT_ADDR, 2);
> +       /* Only update bootcount during upgrade process */
> +       if (!upgrade_available)
> +               return;
> +
> +       buf = map_sysmem(CONFIG_SYS_BOOTCOUNT_ADDR, 3);
>         buf[0] = BC_MAGIC;
>         buf[1] = (a & 0xff);
> +       buf[2] = upgrade_available;
>         unmap_sysmem(buf);
>
>         ret = fs_write(CONFIG_SYS_BOOTCOUNT_EXT_NAME,
> -                      CONFIG_SYS_BOOTCOUNT_ADDR, 0, 2, &len);
> +                      CONFIG_SYS_BOOTCOUNT_ADDR, 0, 3, &len);

Separate from this patch...to me it seems ugly to put this info in CONFIG.
Could it not be structured as a driver, with device tree used to hold the
information?

See for example fs_loader.txt

>         if (ret != 0)
>                 puts("Error storing bootcount\n");
>  }
> @@ -45,15 +52,20 @@ ulong bootcount_load(void)
>         }
>
>         ret = fs_read(CONFIG_SYS_BOOTCOUNT_EXT_NAME,
CONFIG_SYS_BOOTCOUNT_ADDR,
> -                     0, 2, &len_read);
> -       if (ret != 0 || len_read != 2) {
> +                     0, 3, &len_read);
> +       if (ret != 0 || len_read < 2 || len_read > 3) {
>                 puts("Error loading bootcount\n");
>                 return 0;
>         }
>
>         buf = map_sysmem(CONFIG_SYS_BOOTCOUNT_ADDR, 2);
> -       if (buf[0] == BC_MAGIC)
> -               ret = buf[1];
> +       /* Use the 3rd byte to enable/disable bootcount */
> +       if (buf[0] == BC_MAGIC) {
> +               if (len_read == 3)
> +                       upgrade_available = buf[2];
> +               if (upgrade_available)
> +                       ret = buf[1];
> +       }
>
>         unmap_sysmem(buf);
>
> --
> 2.18.0
>

Regards,
Simon


More information about the U-Boot mailing list