| Message ID | 20200317165909.12009-1-frederic.danis@collabora.com |
|---|---|
| State | Accepted |
| Commit | df928f8549ab4470b45bdf3ca2a1b45c9ffd4317 |
| Headers | show |
| Series | [v2] bootcount_ext: Add flag to enable/disable bootcount | expand |
Hi Fr?d?ric, On Tue, 17 Mar 2020 at 10:59, 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 version and an upgrade_available entries to the > bootcount file to enable/disable the bootcount check. > When failing to read the bootcount file it will consider that bootcount is > enabled, acting as previously, and update the file accordingly. > > 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> Is there a change log? I see that this is v2. Regards, Simon
Hi Simon, Sorry I missed to add the change log. Since v1: - Add doc/README.bootcount - Add version number in bootcount_ext file and change Magic byte Regards, Fred On 18/03/2020 03:17, Simon Glass wrote: > Hi Fr?d?ric, > > On Tue, 17 Mar 2020 at 10:59, 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 version and an upgrade_available entries to the >> bootcount file to enable/disable the bootcount check. >> When failing to read the bootcount file it will consider that bootcount is >> enabled, acting as previously, and update the file accordingly. >> >> 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> > Is there a change log? I see that this is v2. > > Regards, > Simon
On Wed, 18 Mar 2020 at 02:17, Fr?d?ric Danis <frederic.danis at collabora.com> wrote: > > Hi Simon, > > Sorry I missed to add the change log. > > Since v1: > - Add doc/README.bootcount > - Add version number in bootcount_ext file and change Magic byte Reviewed-by: Simon Glass <sjg at chromium.org> > > Regards, > Fred > > On 18/03/2020 03:17, Simon Glass wrote: > > Hi Fr?d?ric, > > > > On Tue, 17 Mar 2020 at 10:59, 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 version and an upgrade_available entries to the > >> bootcount file to enable/disable the bootcount check. > >> When failing to read the bootcount file it will consider that bootcount is > >> enabled, acting as previously, and update the file accordingly. > >> > >> 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> > > Is there a change log? I see that this is v2. > > > > Regards, > > Simon >
Hi Simon, On 19/03/2020 17:18, Simon Glass wrote: > On Wed, 18 Mar 2020 at 02:17, Fr?d?ric Danis > <frederic.danis at collabora.com> wrote: >> Hi Simon, >> >> Sorry I missed to add the change log. >> >> Since v1: >> - Add doc/README.bootcount >> - Add version number in bootcount_ext file and change Magic byte > Reviewed-by: Simon Glass <sjg at chromium.org> Thanks for the review. What should be the next steps to get this commit merged? Should I need to send a new version with your Reviewed-By, or should it need to be reviewed by someone else? Best regards, Fred
Hi Fred, On Mon, 23 Mar 2020 at 10:20, Fr?d?ric Danis <frederic.danis at collabora.com> wrote: > > Hi Simon, > > On 19/03/2020 17:18, Simon Glass wrote: > > On Wed, 18 Mar 2020 at 02:17, Fr?d?ric Danis > > <frederic.danis at collabora.com> wrote: > >> Hi Simon, > >> > >> Sorry I missed to add the change log. > >> > >> Since v1: > >> - Add doc/README.bootcount > >> - Add version number in bootcount_ext file and change Magic byte > > Reviewed-by: Simon Glass <sjg at chromium.org> > > Thanks for the review. > What should be the next steps to get this commit merged? > Should I need to send a new version with your Reviewed-By, or should it > need to be reviewed by someone else? I'm not sure if anyone else will review it. Tom may pull it into his next branch, or otherwise it will be merged when the merge window opens. Regards, Simon
diff --git a/doc/README.bootcount b/doc/README.bootcount new file mode 100644 index 0000000000..b1c22905c6 --- /dev/null +++ b/doc/README.bootcount @@ -0,0 +1,51 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Boot Count Limit +================ + +This allows to detect multiple failed attempts to boot Linux. + +After a power-on reset, "bootcount" variable will be initialized with 1, and +each reboot will increment the value by 1. + +If, after a reboot, the new value of "bootcount" exceeds the value of +"bootlimit", then instead of the standard boot action (executing the contents of +"bootcmd") an alternate boot action will be performed, and the contents of +"altbootcmd" will be executed. + +If the variable "bootlimit" is not defined in the environment, the Boot Count +Limit feature is disabled. If it is enabled, but "altbootcmd" is not defined, +then U-Boot will drop into interactive mode and remain there. + +It is the responsibility of some application code (typically a Linux +application) to reset the variable "bootcount", thus allowing for more boot +cycles. + +BOOTCOUNT_EXT +------------- + +This adds support for maintaining boot count in a file on an EXT filesystem. +The file to use is define by: + +SYS_BOOTCOUNT_EXT_INTERFACE +SYS_BOOTCOUNT_EXT_DEVPART +SYS_BOOTCOUNT_EXT_NAME + +The format of the file is: + +==== ================= +type entry +==== ================= +u8 magic +u8 version +u8 bootcount +u8 upgrade_available +==== ================= + +To prevent unattended usage of "altbootcmd" the "upgrade_available" variable is +used. +If "upgrade_available" is 0, "bootcount" is not saved, if "upgrade_available" is +1 "bootcount" is save. +So the Userspace Application must set the "upgrade_available" and "bootcount" +variables to 0, if a boot was successfully. +This also prevents writes on all reboots. diff --git a/drivers/bootcount/bootcount_ext.c b/drivers/bootcount/bootcount_ext.c index 075e590896..9639e638e9 100644 --- a/drivers/bootcount/bootcount_ext.c +++ b/drivers/bootcount/bootcount_ext.c @@ -7,11 +7,21 @@ #include <fs.h> #include <mapmem.h> -#define BC_MAGIC 0xbc +#define BC_MAGIC 0xbd +#define BC_VERSION 1 + +typedef struct { + u8 magic; + u8 version; + u8 bootcount; + u8 upgrade_available; +} bootcount_ext_t; + +static u8 upgrade_available = 1; void bootcount_store(ulong a) { - u8 *buf; + bootcount_ext_t *buf; loff_t len; int ret; @@ -21,20 +31,27 @@ void bootcount_store(ulong a) return; } - buf = map_sysmem(CONFIG_SYS_BOOTCOUNT_ADDR, 2); - buf[0] = BC_MAGIC; - buf[1] = (a & 0xff); + /* Only update bootcount during upgrade process */ + if (!upgrade_available) + return; + + buf = map_sysmem(CONFIG_SYS_BOOTCOUNT_ADDR, sizeof(bootcount_ext_t)); + buf->magic = BC_MAGIC; + buf->version = BC_VERSION; + buf->bootcount = (a & 0xff); + buf->upgrade_available = 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, sizeof(bootcount_ext_t), + &len); if (ret != 0) puts("Error storing bootcount\n"); } ulong bootcount_load(void) { - u8 *buf; + bootcount_ext_t *buf; loff_t len_read; int ret; @@ -45,15 +62,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, sizeof(bootcount_ext_t), &len_read); + if (ret != 0 || len_read != sizeof(bootcount_ext_t)) { puts("Error loading bootcount\n"); return 0; } - buf = map_sysmem(CONFIG_SYS_BOOTCOUNT_ADDR, 2); - if (buf[0] == BC_MAGIC) - ret = buf[1]; + buf = map_sysmem(CONFIG_SYS_BOOTCOUNT_ADDR, sizeof(bootcount_ext_t)); + if (buf->magic == BC_MAGIC && buf->version == BC_VERSION) { + upgrade_available = buf->upgrade_available; + if (upgrade_available) + ret = buf->bootcount; + } else { + puts("Incorrect bootcount file\n"); + } unmap_sysmem(buf);
After a successful upgrade, multiple problem during boot sequence may trigger the altbootcmd process. This patch adds a version and an upgrade_available entries to the bootcount file to enable/disable the bootcount check. When failing to read the bootcount file it will consider that bootcount is enabled, acting as previously, and update the file accordingly. 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> --- doc/README.bootcount | 51 +++++++++++++++++++++++++++++++ drivers/bootcount/bootcount_ext.c | 46 ++++++++++++++++++++-------- 2 files changed, 85 insertions(+), 12 deletions(-) create mode 100644 doc/README.bootcount