Re: [PATCH 1/2] bootcount: add bootcount flash driver
Am 22.04.20 um 12:46 schrieb Arnaud Ferraris:
> In order to save the bootcounter on raw flash device, this commit
> introduces a new bootcount driver, enabled using the
> CONFIG_BOOTCOUNT_FLASH option.
>
> The bootcounter is stored at address CONFIG_SYS_BOOTCOUNT_FLASH_ADDR
> (absolute address, can also be computed from
> CONFIG_SYS_BOOTCOUNT_FLASH_OFFSET), and it uses a data structure
> providing several useful enhancements:
> - a `flags` field, used to check whether the bootcounter should be
>written to flash (similar to the `upgrade_available` environment
>variable)
> - a `crc` field to ensure integrity of the structure, which will be
>used later on when adding redundancy support
> - a `data` field, which can be used to store and pass user data
>between u-boot and the OS (e.g boot partition selection)
>
> Signed-off-by: Arnaud Ferraris
> ---
>
> drivers/bootcount/Kconfig | 14 ++
> drivers/bootcount/Makefile | 1 +
> drivers/bootcount/bootcount_flash.c | 78 +
> include/bootcount.h | 12 +
> scripts/config_whitelist.txt| 2 +
> 5 files changed, 107 insertions(+)
> create mode 100644 drivers/bootcount/bootcount_flash.c
>
> diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig
> index 0e506c9ea2..94de3f5ef8 100644
> --- a/drivers/bootcount/Kconfig
> +++ b/drivers/bootcount/Kconfig
> @@ -66,6 +66,20 @@ config BOOTCOUNT_I2C
> CONFIG_SYS_BOOTCOUNT_ADDR = i2c addr which is used for
> the bootcounter.
>
> +config BOOTCOUNT_FLASH
> + bool "Boot counter on flash device"
> + help
> + Store the bootcounter on raw flash. The bootcounter will be stored
> + as a structure using one flash sector, and will only be written when
> + a specific flag is set in the structure. Additionnally, this driver
> + relies on one of the following to be defined:
> +
> + CONFIG_SYS_BOOTCOUNT_FLASH_ADDR = address used for storing the
> + bootcounter on flash.
> + or
> + CONFIG_SYS_BOOTCOUNT_FLASH_OFFSET = bootcounter offset on flash
> + (relative to SYS_FLASH_BASE).
> +
> config BOOTCOUNT_AT91
> bool "Boot counter for Atmel AT91SAM9XE"
> depends on AT91SAM9XE
> diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile
> index 73ccfb5a08..bddcd136f3 100644
> --- a/drivers/bootcount/Makefile
> +++ b/drivers/bootcount/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_BOOTCOUNT_RAM) += bootcount_ram.o
> obj-$(CONFIG_BOOTCOUNT_ENV) += bootcount_env.o
> obj-$(CONFIG_BOOTCOUNT_I2C) += bootcount_i2c.o
> obj-$(CONFIG_BOOTCOUNT_EXT) += bootcount_ext.o
> +obj-$(CONFIG_BOOTCOUNT_FLASH)+= bootcount_flash.o
>
> obj-$(CONFIG_DM_BOOTCOUNT) += bootcount-uclass.o
> obj-$(CONFIG_DM_BOOTCOUNT_RTC) += rtc.o
> diff --git a/drivers/bootcount/bootcount_flash.c
> b/drivers/bootcount/bootcount_flash.c
> new file mode 100644
> index 00..1222bb4ae0
> --- /dev/null
> +++ b/drivers/bootcount/bootcount_flash.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020 Collabora Ltd. All rights reserved.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#ifndef CONFIG_SYS_BOOTCOUNT_FLASH_ADDR
> +# ifdef CONFIG_SYS_BOOTCOUNT_FLASH_OFFSET
> +#define CONFIG_SYS_BOOTCOUNT_FLASH_ADDR \
> + (CONFIG_SYS_FLASH_BASE + CONFIG_SYS_BOOTCOUNT_FLASH_OFFSET)
> +# else
> +#error "Either CONFIG_SYS_BOOTCOUNT_FLASH_ADDR or " \
> +"CONFIG_SYS_BOOTCOUNT_FLASH_OFFSET should be defined!"
> +# endif
> +#endif
> +
> +#define BOOTCOUNT_CRC_SIZE (sizeof(*bootcount) - sizeof(bootcount->crc))
> +
> +static struct bootcount *bootcount = (void *)CONFIG_SYS_BOOTCOUNT_ADDR;
> +static ulong bc_flash = CONFIG_SYS_BOOTCOUNT_FLASH_ADDR;
> +static ulong bc_flash_end =
> + CONFIG_SYS_BOOTCOUNT_FLASH_ADDR + CONFIG_SYS_FLASH_SECT_SIZE - 1;
> +
> +static void bootcount_write(void)
> +{
> + if (flash_sect_protect(0, bc_flash, bc_flash_end))
> + return;
> +
> + puts("Erasing Flash...\n");
> + if (flash_sect_erase(bc_flash, bc_flash_end))
> + return;
> +
> + puts("Writing bootcount to Flash...\n");
> + if (flash_write((char *)bootcount, bc_flash, sizeof(*bootcount)))
> + return;
> +
> + flash_sect_protect(1, bc_flash, bc_flash_end);
Why don't you build this on top of the MTD layer to make it compatible
with all flash media types? Nowadays no one uses parallel NOR flash
devices anymore ;)
Also why don't you continously write new entries until the sector is
full? Erasing the sector on every boot kills the flash sector early.
> +}
> +
> +static void bootcount_init(void)
> +{
> + memset(bootcount, 0, sizeof(*bootcount));
> + bootcount->magic = CONFIG_SYS_BOOTCOUNT_MAGIC;
> + bootcount->crc =
[PATCH 1/2] bootcount: add bootcount flash driver
In order to save the bootcounter on raw flash device, this commit
introduces a new bootcount driver, enabled using the
CONFIG_BOOTCOUNT_FLASH option.
The bootcounter is stored at address CONFIG_SYS_BOOTCOUNT_FLASH_ADDR
(absolute address, can also be computed from
CONFIG_SYS_BOOTCOUNT_FLASH_OFFSET), and it uses a data structure
providing several useful enhancements:
- a `flags` field, used to check whether the bootcounter should be
written to flash (similar to the `upgrade_available` environment
variable)
- a `crc` field to ensure integrity of the structure, which will be
used later on when adding redundancy support
- a `data` field, which can be used to store and pass user data
between u-boot and the OS (e.g boot partition selection)
Signed-off-by: Arnaud Ferraris
---
drivers/bootcount/Kconfig | 14 ++
drivers/bootcount/Makefile | 1 +
drivers/bootcount/bootcount_flash.c | 78 +
include/bootcount.h | 12 +
scripts/config_whitelist.txt| 2 +
5 files changed, 107 insertions(+)
create mode 100644 drivers/bootcount/bootcount_flash.c
diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig
index 0e506c9ea2..94de3f5ef8 100644
--- a/drivers/bootcount/Kconfig
+++ b/drivers/bootcount/Kconfig
@@ -66,6 +66,20 @@ config BOOTCOUNT_I2C
CONFIG_SYS_BOOTCOUNT_ADDR = i2c addr which is used for
the bootcounter.
+config BOOTCOUNT_FLASH
+ bool "Boot counter on flash device"
+ help
+ Store the bootcounter on raw flash. The bootcounter will be stored
+ as a structure using one flash sector, and will only be written when
+ a specific flag is set in the structure. Additionnally, this driver
+ relies on one of the following to be defined:
+
+ CONFIG_SYS_BOOTCOUNT_FLASH_ADDR = address used for storing the
+ bootcounter on flash.
+ or
+ CONFIG_SYS_BOOTCOUNT_FLASH_OFFSET = bootcounter offset on flash
+ (relative to SYS_FLASH_BASE).
+
config BOOTCOUNT_AT91
bool "Boot counter for Atmel AT91SAM9XE"
depends on AT91SAM9XE
diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile
index 73ccfb5a08..bddcd136f3 100644
--- a/drivers/bootcount/Makefile
+++ b/drivers/bootcount/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_BOOTCOUNT_RAM) += bootcount_ram.o
obj-$(CONFIG_BOOTCOUNT_ENV)+= bootcount_env.o
obj-$(CONFIG_BOOTCOUNT_I2C)+= bootcount_i2c.o
obj-$(CONFIG_BOOTCOUNT_EXT)+= bootcount_ext.o
+obj-$(CONFIG_BOOTCOUNT_FLASH) += bootcount_flash.o
obj-$(CONFIG_DM_BOOTCOUNT) += bootcount-uclass.o
obj-$(CONFIG_DM_BOOTCOUNT_RTC) += rtc.o
diff --git a/drivers/bootcount/bootcount_flash.c
b/drivers/bootcount/bootcount_flash.c
new file mode 100644
index 00..1222bb4ae0
--- /dev/null
+++ b/drivers/bootcount/bootcount_flash.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Collabora Ltd. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#ifndef CONFIG_SYS_BOOTCOUNT_FLASH_ADDR
+# ifdef CONFIG_SYS_BOOTCOUNT_FLASH_OFFSET
+#define CONFIG_SYS_BOOTCOUNT_FLASH_ADDR\
+ (CONFIG_SYS_FLASH_BASE + CONFIG_SYS_BOOTCOUNT_FLASH_OFFSET)
+# else
+#error "Either CONFIG_SYS_BOOTCOUNT_FLASH_ADDR or " \
+ "CONFIG_SYS_BOOTCOUNT_FLASH_OFFSET should be defined!"
+# endif
+#endif
+
+#define BOOTCOUNT_CRC_SIZE (sizeof(*bootcount) - sizeof(bootcount->crc))
+
+static struct bootcount *bootcount = (void *)CONFIG_SYS_BOOTCOUNT_ADDR;
+static ulong bc_flash = CONFIG_SYS_BOOTCOUNT_FLASH_ADDR;
+static ulong bc_flash_end =
+ CONFIG_SYS_BOOTCOUNT_FLASH_ADDR + CONFIG_SYS_FLASH_SECT_SIZE - 1;
+
+static void bootcount_write(void)
+{
+ if (flash_sect_protect(0, bc_flash, bc_flash_end))
+ return;
+
+ puts("Erasing Flash...\n");
+ if (flash_sect_erase(bc_flash, bc_flash_end))
+ return;
+
+ puts("Writing bootcount to Flash...\n");
+ if (flash_write((char *)bootcount, bc_flash, sizeof(*bootcount)))
+ return;
+
+ flash_sect_protect(1, bc_flash, bc_flash_end);
+}
+
+static void bootcount_init(void)
+{
+ memset(bootcount, 0, sizeof(*bootcount));
+ bootcount->magic = CONFIG_SYS_BOOTCOUNT_MAGIC;
+ bootcount->crc = crc32(0, (uchar *)bootcount, BOOTCOUNT_CRC_SIZE);
+ bootcount_write();
+}
+
+void bootcount_store(ulong a)
+{
+ bootcount->count = a;
+ bootcount->crc = crc32(0, (uchar *)bootcount, BOOTCOUNT_CRC_SIZE);
+ if (bootcount->flags & BOOTCOUNT_FLAGS_UPDATE)
+ bootcount_write();
+}
+
+ulong bootcount_load(void)
+{
+ static int initialized;
+ u32 crc;
+
+ if (!initialized) {
+ memcpy(bootcount, (void *)bc_flash, sizeof(*bootcount));
+
