| Message ID | 20251228151824.25667-5-dario.binacchi@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
ne 28. 12. 2025 v 16:19 odesílatel Dario Binacchi <dario.binacchi@amarulasolutions.com> napsal: > > Allow reading FWU metadata from userspace. > > According to [1], after updating the software on bank B, setting > active_index to point to it (i. e. 1) and marking it as a Valid bank, it > must be possible to read this information from userspace on the next > boot to verify whether the boot chain of bank B succeeded. > > The metadata must then be updated again, marking the bank as INVALID if > the active_index points to bank A (i. e. 0) or as VALID if the boot was > successful. > > To allow reading the active_index and bank state, this new tool has been > added. > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > --- > > tools/Kconfig | 7 +++ > tools/Makefile | 4 ++ > tools/dumpfwumdata.c | 128 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 139 insertions(+) > create mode 100644 tools/dumpfwumdata.c > > diff --git a/tools/Kconfig b/tools/Kconfig > index 652b0f225579..9de7eed1bbbb 100644 > --- a/tools/Kconfig > +++ b/tools/Kconfig > @@ -194,6 +194,13 @@ config LUT_SEQUENCE > help > Look Up Table Sequence > > +config TOOLS_DUMPFWUMDATA > + bool "Build dumpfwumdata command" > + default y if FWU_MULTI_BANK_UPDATE > + help > + This command allows users to read and display the raw FWU > + metadata. The exact access method depends on the platform. > + > config TOOLS_MKFWUMDATA > bool "Build mkfwumdata command" > default y if FWU_MULTI_BANK_UPDATE > diff --git a/tools/Makefile b/tools/Makefile > index ae6a30526466..bc105a086927 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -271,6 +271,10 @@ mkeficapsule-objs := generated/lib/uuid.o \ > mkeficapsule.o > hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule > > +dumpfwumdata-objs := dumpfwumdata.o generated/lib/crc32.o > +HOSTLDLIBS_dumpfwumdata += -luuid > +hostprogs-$(CONFIG_TOOLS_DUMPFWUMDATA) += dumpfwumdata you need to update this based on changes commit bd3f9ee679b4d1456d0d3c261ab76788950e6096 Author: Sughosh Ganu <sughosh.ganu@linaro.org> AuthorDate: Tue Dec 16 11:16:24 2025 +0200 Commit: Tom Rini <trini@konsulko.com> CommitDate: Fri Jan 2 10:28:14 2026 -0600 kbuild: Bump the build system to 6.1 add missing always. > + > mkfwumdata-objs := mkfwumdata.o generated/lib/crc32.o > HOSTLDLIBS_mkfwumdata += -luuid > hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata > diff --git a/tools/dumpfwumdata.c b/tools/dumpfwumdata.c > new file mode 100644 > index 000000000000..4965484df1be > --- /dev/null > +++ b/tools/dumpfwumdata.c > @@ -0,0 +1,128 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2025, Amarula Solutions > + */ > + > +#include <errno.h> > +#include <getopt.h> > +#include <limits.h> > +#include <stdio.h> > +#include <stdint.h> > +#include <stdlib.h> > +#include <string.h> > +#include <unistd.h> > +#include <generated/autoconf.h> > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <u-boot/crc.h> > +#include <uuid/uuid.h> > + > +typedef uint8_t u8; > +typedef int16_t s16; > +typedef uint16_t u16; > +typedef uint32_t u32; > +typedef uint64_t u64; > + > +#undef CONFIG_FWU_NUM_BANKS > +#undef CONFIG_FWU_NUM_IMAGES_PER_BANK > + > +/* This will dynamically allocate the fwu_mdata */ > +#define CONFIG_FWU_NUM_BANKS 0 > +#define CONFIG_FWU_NUM_IMAGES_PER_BANK 0 > + > +#include <fwu_mdata.h> > + > +static const char *opts_short = "h"; > + > +static struct option options[] = { > + {"help", no_argument, NULL, 'h'}, > + {NULL, 0, NULL, 0}, > +}; > + > +static void print_usage(void) > +{ > + fprintf(stderr, "Usage: dumpfwumdata [options] <fwumdata file>\n"); > + fprintf(stderr, "Options:\n" > + "\t-h, --help print a help message\n" > + ); > +} > + > +static void print_mdata(struct fwu_mdata *data) > +{ > + int i; > + > + fprintf(stdout, "FWU Metadata\n"); > + fprintf(stdout, "crc32: %#x\n", data->crc32); > + fprintf(stdout, "version: %#x\n", data->version); > + fprintf(stdout, "active_index: %#x\n", data->active_index); > + fprintf(stdout, "previous_active_index: %#x\n", > + data->previous_active_index); > + > + if (data->version == 2) { > + uint8_t state; > + char cstate; > + for (i = 0; i < 4; i++) { > + state = data->bank_state[i]; > + if (state == FWU_BANK_ACCEPTED) > + cstate = 'A'; > + else if (state == FWU_BANK_VALID) > + cstate = 'V'; > + else if (state == FWU_BANK_INVALID) > + cstate = 'I'; > + else > + cstate = '?'; Isn't it better to print accepted, valid, invalid instead? I had to look at code to understand what that print value means. > + > + printf("bank_state[%d]: %c\n", i, cstate); > + } > + } It is a good start but I think more information should be printed. Number images, custom fields, etc. > +} > + > +static int fwu_read_mdata(struct fwu_mdata *mdata, const char *mdata_file) > +{ > + FILE *mfile = NULL; > + size_t ret, size = sizeof(struct fwu_mdata); > + > + mfile = fopen(mdata_file, "r"); > + if (!mfile) { > + fprintf(stderr, "Error: Failed to open %s\n", > + mdata_file); > + return -1; > + } > + > + ret = fread(mdata, 1, size, mfile); > + fclose(mfile); > + if (ret != size) { > + fprintf(stderr, "Error: Failed to read from %s\n", > + mdata_file); > + return -1; > + } > + > + return 0; > +} > + > +int main(int argc, char *argv[]) > +{ > + int c, ret; > + struct fwu_mdata mdata; > + > + if (argc < 3) { > + print_usage(); > + return -EINVAL; > + } above you have only help and here you are asking for 3 args which seems to me wrong. > + > + do { > + c = getopt_long(argc, argv, opts_short, options, NULL); > + switch (c) { > + case 'h': > + print_usage(); > + return 0; > + } > + } while (c != -1); > + > + ret = fwu_read_mdata(&mdata, argv[argc - 1]); > + if (ret) > + return ret; > + > + print_mdata(&mdata); > + return 0; > +} > -- > 2.43.0 > M
Hello Michal, On Mon, Jan 5, 2026 at 4:40 PM Michal Simek <monstr@monstr.eu> wrote: > > ne 28. 12. 2025 v 16:19 odesílatel Dario Binacchi > <dario.binacchi@amarulasolutions.com> napsal: > > > > Allow reading FWU metadata from userspace. > > > > According to [1], after updating the software on bank B, setting > > active_index to point to it (i. e. 1) and marking it as a Valid bank, it > > must be possible to read this information from userspace on the next > > boot to verify whether the boot chain of bank B succeeded. > > > > The metadata must then be updated again, marking the bank as INVALID if > > the active_index points to bank A (i. e. 0) or as VALID if the boot was > > successful. > > > > To allow reading the active_index and bank state, this new tool has been > > added. > > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > > --- > > > > tools/Kconfig | 7 +++ > > tools/Makefile | 4 ++ > > tools/dumpfwumdata.c | 128 +++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 139 insertions(+) > > create mode 100644 tools/dumpfwumdata.c > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > index 652b0f225579..9de7eed1bbbb 100644 > > --- a/tools/Kconfig > > +++ b/tools/Kconfig > > @@ -194,6 +194,13 @@ config LUT_SEQUENCE > > help > > Look Up Table Sequence > > > > +config TOOLS_DUMPFWUMDATA > > + bool "Build dumpfwumdata command" > > + default y if FWU_MULTI_BANK_UPDATE > > + help > > + This command allows users to read and display the raw FWU > > + metadata. The exact access method depends on the platform. > > + > > config TOOLS_MKFWUMDATA > > bool "Build mkfwumdata command" > > default y if FWU_MULTI_BANK_UPDATE > > diff --git a/tools/Makefile b/tools/Makefile > > index ae6a30526466..bc105a086927 100644 > > --- a/tools/Makefile > > +++ b/tools/Makefile > > @@ -271,6 +271,10 @@ mkeficapsule-objs := generated/lib/uuid.o \ > > mkeficapsule.o > > hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule > > > > +dumpfwumdata-objs := dumpfwumdata.o generated/lib/crc32.o > > +HOSTLDLIBS_dumpfwumdata += -luuid > > +hostprogs-$(CONFIG_TOOLS_DUMPFWUMDATA) += dumpfwumdata > > you need to update this based on changes > > commit bd3f9ee679b4d1456d0d3c261ab76788950e6096 > Author: Sughosh Ganu <sughosh.ganu@linaro.org> > AuthorDate: Tue Dec 16 11:16:24 2025 +0200 > Commit: Tom Rini <trini@konsulko.com> > CommitDate: Fri Jan 2 10:28:14 2026 -0600 > > kbuild: Bump the build system to 6.1 > > > add missing always. > > > + > > mkfwumdata-objs := mkfwumdata.o generated/lib/crc32.o > > HOSTLDLIBS_mkfwumdata += -luuid > > hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata > > diff --git a/tools/dumpfwumdata.c b/tools/dumpfwumdata.c > > new file mode 100644 > > index 000000000000..4965484df1be > > --- /dev/null > > +++ b/tools/dumpfwumdata.c > > @@ -0,0 +1,128 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (c) 2025, Amarula Solutions > > + */ > > + > > +#include <errno.h> > > +#include <getopt.h> > > +#include <limits.h> > > +#include <stdio.h> > > +#include <stdint.h> > > +#include <stdlib.h> > > +#include <string.h> > > +#include <unistd.h> > > +#include <generated/autoconf.h> > > +#include <sys/types.h> > > +#include <sys/stat.h> > > +#include <u-boot/crc.h> > > +#include <uuid/uuid.h> > > + > > +typedef uint8_t u8; > > +typedef int16_t s16; > > +typedef uint16_t u16; > > +typedef uint32_t u32; > > +typedef uint64_t u64; > > + > > +#undef CONFIG_FWU_NUM_BANKS > > +#undef CONFIG_FWU_NUM_IMAGES_PER_BANK > > + > > +/* This will dynamically allocate the fwu_mdata */ > > +#define CONFIG_FWU_NUM_BANKS 0 > > +#define CONFIG_FWU_NUM_IMAGES_PER_BANK 0 > > + > > +#include <fwu_mdata.h> > > + > > +static const char *opts_short = "h"; > > + > > +static struct option options[] = { > > + {"help", no_argument, NULL, 'h'}, > > + {NULL, 0, NULL, 0}, > > +}; > > + > > +static void print_usage(void) > > +{ > > + fprintf(stderr, "Usage: dumpfwumdata [options] <fwumdata file>\n"); > > + fprintf(stderr, "Options:\n" > > + "\t-h, --help print a help message\n" > > + ); > > +} > > + > > +static void print_mdata(struct fwu_mdata *data) > > +{ > > + int i; > > + > > + fprintf(stdout, "FWU Metadata\n"); > > + fprintf(stdout, "crc32: %#x\n", data->crc32); > > + fprintf(stdout, "version: %#x\n", data->version); > > + fprintf(stdout, "active_index: %#x\n", data->active_index); > > + fprintf(stdout, "previous_active_index: %#x\n", > > + data->previous_active_index); > > + > > + if (data->version == 2) { > > + uint8_t state; > > + char cstate; > > + for (i = 0; i < 4; i++) { > > + state = data->bank_state[i]; > > + if (state == FWU_BANK_ACCEPTED) > > + cstate = 'A'; > > + else if (state == FWU_BANK_VALID) > > + cstate = 'V'; > > + else if (state == FWU_BANK_INVALID) > > + cstate = 'I'; > > + else > > + cstate = '?'; > > Isn't it better to print accepted, valid, invalid instead? > > I had to look at code to understand what that print value means. > > > + > > + printf("bank_state[%d]: %c\n", i, cstate); > > + } > > + } > > It is a good start but I think more information should be printed. > Number images, custom fields, etc. > > > +} > > + > > +static int fwu_read_mdata(struct fwu_mdata *mdata, const char *mdata_file) > > +{ > > + FILE *mfile = NULL; > > + size_t ret, size = sizeof(struct fwu_mdata); > > + > > + mfile = fopen(mdata_file, "r"); > > + if (!mfile) { > > + fprintf(stderr, "Error: Failed to open %s\n", > > + mdata_file); > > + return -1; > > + } > > + > > + ret = fread(mdata, 1, size, mfile); > > + fclose(mfile); > > + if (ret != size) { > > + fprintf(stderr, "Error: Failed to read from %s\n", > > + mdata_file); > > + return -1; > > + } > > + > > + return 0; > > +} > > + > > +int main(int argc, char *argv[]) > > +{ > > + int c, ret; > > + struct fwu_mdata mdata; > > + > > + if (argc < 3) { > > + print_usage(); > > + return -EINVAL; > > + } > > above you have only help and here you are asking for 3 args which > seems to me wrong. Thank you for your review and the feedback on my patch series. After I sent the series, I realized that this series had previously been posted: https://lore.kernel.org/all/20251212-feature_fwumdata-v2-0-ad51572fbe79@bootlin.com/. From what I understand, it starts from the same need as mine, namely the implementation of a tool that allows reading FWU metadata from Linux. My version is definitely more minimal compared to what Kory has done. Therefore, before sending a v2, I think it would be useful to understand which of the two makes more sense to move forward with. Could you help me with that? I have also CCed Kory to keep the discussion shared. Thanks and regards, Dario > > > + > > + do { > > + c = getopt_long(argc, argv, opts_short, options, NULL); > > + switch (c) { > > + case 'h': > > + print_usage(); > > + return 0; > > + } > > + } while (c != -1); > > + > > + ret = fwu_read_mdata(&mdata, argv[argc - 1]); > > + if (ret) > > + return ret; > > + > > + print_mdata(&mdata); > > + return 0; > > +} > > -- > > 2.43.0 > > > > M > > > -- > Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 > w: www.monstr.eu p: +42-0-721842854 > Maintainer of Linux kernel - Xilinx Microblaze > Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs > U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
On 1/11/26 16:17, Dario Binacchi wrote: > Hello Michal, > > On Mon, Jan 5, 2026 at 4:40 PM Michal Simek <monstr@monstr.eu> wrote: >> >> ne 28. 12. 2025 v 16:19 odesílatel Dario Binacchi >> <dario.binacchi@amarulasolutions.com> napsal: >>> >>> Allow reading FWU metadata from userspace. >>> >>> According to [1], after updating the software on bank B, setting >>> active_index to point to it (i. e. 1) and marking it as a Valid bank, it >>> must be possible to read this information from userspace on the next >>> boot to verify whether the boot chain of bank B succeeded. >>> >>> The metadata must then be updated again, marking the bank as INVALID if >>> the active_index points to bank A (i. e. 0) or as VALID if the boot was >>> successful. >>> >>> To allow reading the active_index and bank state, this new tool has been >>> added. >>> >>> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> >>> --- >>> >>> tools/Kconfig | 7 +++ >>> tools/Makefile | 4 ++ >>> tools/dumpfwumdata.c | 128 +++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 139 insertions(+) >>> create mode 100644 tools/dumpfwumdata.c >>> >>> diff --git a/tools/Kconfig b/tools/Kconfig >>> index 652b0f225579..9de7eed1bbbb 100644 >>> --- a/tools/Kconfig >>> +++ b/tools/Kconfig >>> @@ -194,6 +194,13 @@ config LUT_SEQUENCE >>> help >>> Look Up Table Sequence >>> >>> +config TOOLS_DUMPFWUMDATA >>> + bool "Build dumpfwumdata command" >>> + default y if FWU_MULTI_BANK_UPDATE >>> + help >>> + This command allows users to read and display the raw FWU >>> + metadata. The exact access method depends on the platform. >>> + >>> config TOOLS_MKFWUMDATA >>> bool "Build mkfwumdata command" >>> default y if FWU_MULTI_BANK_UPDATE >>> diff --git a/tools/Makefile b/tools/Makefile >>> index ae6a30526466..bc105a086927 100644 >>> --- a/tools/Makefile >>> +++ b/tools/Makefile >>> @@ -271,6 +271,10 @@ mkeficapsule-objs := generated/lib/uuid.o \ >>> mkeficapsule.o >>> hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule >>> >>> +dumpfwumdata-objs := dumpfwumdata.o generated/lib/crc32.o >>> +HOSTLDLIBS_dumpfwumdata += -luuid >>> +hostprogs-$(CONFIG_TOOLS_DUMPFWUMDATA) += dumpfwumdata >> >> you need to update this based on changes >> >> commit bd3f9ee679b4d1456d0d3c261ab76788950e6096 >> Author: Sughosh Ganu <sughosh.ganu@linaro.org> >> AuthorDate: Tue Dec 16 11:16:24 2025 +0200 >> Commit: Tom Rini <trini@konsulko.com> >> CommitDate: Fri Jan 2 10:28:14 2026 -0600 >> >> kbuild: Bump the build system to 6.1 >> >> >> add missing always. >> >>> + >>> mkfwumdata-objs := mkfwumdata.o generated/lib/crc32.o >>> HOSTLDLIBS_mkfwumdata += -luuid >>> hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata >>> diff --git a/tools/dumpfwumdata.c b/tools/dumpfwumdata.c >>> new file mode 100644 >>> index 000000000000..4965484df1be >>> --- /dev/null >>> +++ b/tools/dumpfwumdata.c >>> @@ -0,0 +1,128 @@ >>> +// SPDX-License-Identifier: GPL-2.0+ >>> +/* >>> + * Copyright (c) 2025, Amarula Solutions >>> + */ >>> + >>> +#include <errno.h> >>> +#include <getopt.h> >>> +#include <limits.h> >>> +#include <stdio.h> >>> +#include <stdint.h> >>> +#include <stdlib.h> >>> +#include <string.h> >>> +#include <unistd.h> >>> +#include <generated/autoconf.h> >>> +#include <sys/types.h> >>> +#include <sys/stat.h> >>> +#include <u-boot/crc.h> >>> +#include <uuid/uuid.h> >>> + >>> +typedef uint8_t u8; >>> +typedef int16_t s16; >>> +typedef uint16_t u16; >>> +typedef uint32_t u32; >>> +typedef uint64_t u64; >>> + >>> +#undef CONFIG_FWU_NUM_BANKS >>> +#undef CONFIG_FWU_NUM_IMAGES_PER_BANK >>> + >>> +/* This will dynamically allocate the fwu_mdata */ >>> +#define CONFIG_FWU_NUM_BANKS 0 >>> +#define CONFIG_FWU_NUM_IMAGES_PER_BANK 0 >>> + >>> +#include <fwu_mdata.h> >>> + >>> +static const char *opts_short = "h"; >>> + >>> +static struct option options[] = { >>> + {"help", no_argument, NULL, 'h'}, >>> + {NULL, 0, NULL, 0}, >>> +}; >>> + >>> +static void print_usage(void) >>> +{ >>> + fprintf(stderr, "Usage: dumpfwumdata [options] <fwumdata file>\n"); >>> + fprintf(stderr, "Options:\n" >>> + "\t-h, --help print a help message\n" >>> + ); >>> +} >>> + >>> +static void print_mdata(struct fwu_mdata *data) >>> +{ >>> + int i; >>> + >>> + fprintf(stdout, "FWU Metadata\n"); >>> + fprintf(stdout, "crc32: %#x\n", data->crc32); >>> + fprintf(stdout, "version: %#x\n", data->version); >>> + fprintf(stdout, "active_index: %#x\n", data->active_index); >>> + fprintf(stdout, "previous_active_index: %#x\n", >>> + data->previous_active_index); >>> + >>> + if (data->version == 2) { >>> + uint8_t state; >>> + char cstate; >>> + for (i = 0; i < 4; i++) { >>> + state = data->bank_state[i]; >>> + if (state == FWU_BANK_ACCEPTED) >>> + cstate = 'A'; >>> + else if (state == FWU_BANK_VALID) >>> + cstate = 'V'; >>> + else if (state == FWU_BANK_INVALID) >>> + cstate = 'I'; >>> + else >>> + cstate = '?'; >> >> Isn't it better to print accepted, valid, invalid instead? >> >> I had to look at code to understand what that print value means. >> >>> + >>> + printf("bank_state[%d]: %c\n", i, cstate); >>> + } >>> + } >> >> It is a good start but I think more information should be printed. >> Number images, custom fields, etc. >> >>> +} >>> + >>> +static int fwu_read_mdata(struct fwu_mdata *mdata, const char *mdata_file) >>> +{ >>> + FILE *mfile = NULL; >>> + size_t ret, size = sizeof(struct fwu_mdata); >>> + >>> + mfile = fopen(mdata_file, "r"); >>> + if (!mfile) { >>> + fprintf(stderr, "Error: Failed to open %s\n", >>> + mdata_file); >>> + return -1; >>> + } >>> + >>> + ret = fread(mdata, 1, size, mfile); >>> + fclose(mfile); >>> + if (ret != size) { >>> + fprintf(stderr, "Error: Failed to read from %s\n", >>> + mdata_file); >>> + return -1; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +int main(int argc, char *argv[]) >>> +{ >>> + int c, ret; >>> + struct fwu_mdata mdata; >>> + >>> + if (argc < 3) { >>> + print_usage(); >>> + return -EINVAL; >>> + } >> >> above you have only help and here you are asking for 3 args which >> seems to me wrong. > > Thank you for your review and the feedback on my patch series. > After I sent the series, I realized that this series had previously > been posted: > https://lore.kernel.org/all/20251212-feature_fwumdata-v2-0-ad51572fbe79@bootlin.com/. > From what I understand, it starts from the same need as mine, namely > the implementation of a tool that > allows reading FWU metadata from Linux. My version is definitely more > minimal compared to what Kory > has done. > Therefore, before sending a v2, I think it would be useful to > understand which of the two makes more sense > to move forward with. Could you help me with that? I have also CCed > Kory to keep the discussion shared. Kory's version looks more complete. If Kory is happy to send another version I think you should test it, review it. If he is not going to send v3 I think you should merge your series with his and send another version. Thanks, Michal To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
On Mon, 12 Jan 2026 10:05:50 +0100 Michal Simek <monstr@monstr.eu> wrote: > On 1/11/26 16:17, Dario Binacchi wrote: > > Hello Michal, > > > > On Mon, Jan 5, 2026 at 4:40 PM Michal Simek <monstr@monstr.eu> wrote: > >> > >> ne 28. 12. 2025 v 16:19 odesílatel Dario Binacchi > >> <dario.binacchi@amarulasolutions.com> napsal: > >>> > >>> Allow reading FWU metadata from userspace. > >>> > >>> According to [1], after updating the software on bank B, setting > >>> active_index to point to it (i. e. 1) and marking it as a Valid bank, it > >>> must be possible to read this information from userspace on the next > >>> boot to verify whether the boot chain of bank B succeeded. > >>> > >>> The metadata must then be updated again, marking the bank as INVALID if > >>> the active_index points to bank A (i. e. 0) or as VALID if the boot was > >>> successful. > >>> > >>> To allow reading the active_index and bank state, this new tool has been > >>> added. > >>> > >>> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> ... > >>> +} > >>> + > >>> +static int fwu_read_mdata(struct fwu_mdata *mdata, const char > >>> *mdata_file) +{ > >>> + FILE *mfile = NULL; > >>> + size_t ret, size = sizeof(struct fwu_mdata); > >>> + > >>> + mfile = fopen(mdata_file, "r"); > >>> + if (!mfile) { > >>> + fprintf(stderr, "Error: Failed to open %s\n", > >>> + mdata_file); > >>> + return -1; > >>> + } > >>> + > >>> + ret = fread(mdata, 1, size, mfile); > >>> + fclose(mfile); > >>> + if (ret != size) { > >>> + fprintf(stderr, "Error: Failed to read from %s\n", > >>> + mdata_file); > >>> + return -1; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +int main(int argc, char *argv[]) > >>> +{ > >>> + int c, ret; > >>> + struct fwu_mdata mdata; > >>> + > >>> + if (argc < 3) { > >>> + print_usage(); > >>> + return -EINVAL; > >>> + } > >> > >> above you have only help and here you are asking for 3 args which > >> seems to me wrong. > > > > Thank you for your review and the feedback on my patch series. > > After I sent the series, I realized that this series had previously > > been posted: > > https://lore.kernel.org/all/20251212-feature_fwumdata-v2-0-ad51572fbe79@bootlin.com/. > > From what I understand, it starts from the same need as mine, namely > > the implementation of a tool that > > allows reading FWU metadata from Linux. My version is definitely more > > minimal compared to what Kory > > has done. > > Therefore, before sending a v2, I think it would be useful to > > understand which of the two makes more sense > > to move forward with. Could you help me with that? I have also CCed > > Kory to keep the discussion shared. > > Kory's version looks more complete. If Kory is happy to send another version > I think you should test it, review it. If he is not going to send v3 I think > you should merge your series with his and send another version. Hello, I was going to ping reviews on my series. I will be glad to send a 3rd version but there will be no change from v2 as nobody reviewed it for now, but if needed I can resend it. It would be nice if you review and test it on you side to know if it match your requirements. Regards,
Hello Kory, On Mon, Jan 12, 2026 at 10:24 AM Kory Maincent <kory.maincent@bootlin.com> wrote: > > On Mon, 12 Jan 2026 10:05:50 +0100 > Michal Simek <monstr@monstr.eu> wrote: > > > On 1/11/26 16:17, Dario Binacchi wrote: > > > Hello Michal, > > > > > > On Mon, Jan 5, 2026 at 4:40 PM Michal Simek <monstr@monstr.eu> wrote: > > >> > > >> ne 28. 12. 2025 v 16:19 odesílatel Dario Binacchi > > >> <dario.binacchi@amarulasolutions.com> napsal: > > >>> > > >>> Allow reading FWU metadata from userspace. > > >>> > > >>> According to [1], after updating the software on bank B, setting > > >>> active_index to point to it (i. e. 1) and marking it as a Valid bank, it > > >>> must be possible to read this information from userspace on the next > > >>> boot to verify whether the boot chain of bank B succeeded. > > >>> > > >>> The metadata must then be updated again, marking the bank as INVALID if > > >>> the active_index points to bank A (i. e. 0) or as VALID if the boot was > > >>> successful. > > >>> > > >>> To allow reading the active_index and bank state, this new tool has been > > >>> added. > > >>> > > >>> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > > ... > > > >>> +} > > >>> + > > >>> +static int fwu_read_mdata(struct fwu_mdata *mdata, const char > > >>> *mdata_file) +{ > > >>> + FILE *mfile = NULL; > > >>> + size_t ret, size = sizeof(struct fwu_mdata); > > >>> + > > >>> + mfile = fopen(mdata_file, "r"); > > >>> + if (!mfile) { > > >>> + fprintf(stderr, "Error: Failed to open %s\n", > > >>> + mdata_file); > > >>> + return -1; > > >>> + } > > >>> + > > >>> + ret = fread(mdata, 1, size, mfile); > > >>> + fclose(mfile); > > >>> + if (ret != size) { > > >>> + fprintf(stderr, "Error: Failed to read from %s\n", > > >>> + mdata_file); > > >>> + return -1; > > >>> + } > > >>> + > > >>> + return 0; > > >>> +} > > >>> + > > >>> +int main(int argc, char *argv[]) > > >>> +{ > > >>> + int c, ret; > > >>> + struct fwu_mdata mdata; > > >>> + > > >>> + if (argc < 3) { > > >>> + print_usage(); > > >>> + return -EINVAL; > > >>> + } > > >> > > >> above you have only help and here you are asking for 3 args which > > >> seems to me wrong. > > > > > > Thank you for your review and the feedback on my patch series. > > > After I sent the series, I realized that this series had previously > > > been posted: > > > https://lore.kernel.org/all/20251212-feature_fwumdata-v2-0-ad51572fbe79@bootlin.com/. > > > From what I understand, it starts from the same need as mine, namely > > > the implementation of a tool that > > > allows reading FWU metadata from Linux. My version is definitely more > > > minimal compared to what Kory > > > has done. > > > Therefore, before sending a v2, I think it would be useful to > > > understand which of the two makes more sense > > > to move forward with. Could you help me with that? I have also CCed > > > Kory to keep the discussion shared. > > > > Kory's version looks more complete. If Kory is happy to send another version > > I think you should test it, review it. If he is not going to send v3 I think > > you should merge your series with his and send another version. > > Hello, > > I was going to ping reviews on my series. > I will be glad to send a 3rd version but there will be no change from v2 as > nobody reviewed it for now, but if needed I can resend it. > It would be nice if you review and test it on you side to know if it match your > requirements. I'll be resuming work on swupdate for our STM32MP-based custom board shortly and will test your series as soon as possible. Just one question: I was expecting the introduction of a tool to read the data contained in fwu-metadata. Why does your new tool also add write functionality, considering that mkfwumdata already exists? Thanks and regards, Dario > > Regards, > -- > Köry Maincent, Bootlin > Embedded Linux and kernel engineering > https://bootlin.com
On Sat, 17 Jan 2026 12:11:18 +0100 Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote: > Hello Kory, > > On Mon, Jan 12, 2026 at 10:24 AM Kory Maincent > <kory.maincent@bootlin.com> wrote: > > > > On Mon, 12 Jan 2026 10:05:50 +0100 > > Michal Simek <monstr@monstr.eu> wrote: > > > > > On 1/11/26 16:17, Dario Binacchi wrote: > > > > Hello Michal, > > > > > > > > On Mon, Jan 5, 2026 at 4:40 PM Michal Simek <monstr@monstr.eu> wrote: > > > >> > > > >> ne 28. 12. 2025 v 16:19 odesílatel Dario Binacchi > > > >> <dario.binacchi@amarulasolutions.com> napsal: > > > >>> > > > >>> Allow reading FWU metadata from userspace. > > > >>> > > > >>> According to [1], after updating the software on bank B, setting > > > >>> active_index to point to it (i. e. 1) and marking it as a Valid bank, > > > >>> it must be possible to read this information from userspace on the > > > >>> next boot to verify whether the boot chain of bank B succeeded. > > > >>> > > > >>> The metadata must then be updated again, marking the bank as INVALID > > > >>> if the active_index points to bank A (i. e. 0) or as VALID if the > > > >>> boot was successful. > > > >>> > > > >>> To allow reading the active_index and bank state, this new tool has > > > >>> been added. > > > >>> > > > >>> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > > > > ... > > > > > >>> +} > > > >>> + > > > >>> +static int fwu_read_mdata(struct fwu_mdata *mdata, const char > > > >>> *mdata_file) +{ > > > >>> + FILE *mfile = NULL; > > > >>> + size_t ret, size = sizeof(struct fwu_mdata); > > > >>> + > > > >>> + mfile = fopen(mdata_file, "r"); > > > >>> + if (!mfile) { > > > >>> + fprintf(stderr, "Error: Failed to open %s\n", > > > >>> + mdata_file); > > > >>> + return -1; > > > >>> + } > > > >>> + > > > >>> + ret = fread(mdata, 1, size, mfile); > > > >>> + fclose(mfile); > > > >>> + if (ret != size) { > > > >>> + fprintf(stderr, "Error: Failed to read from %s\n", > > > >>> + mdata_file); > > > >>> + return -1; > > > >>> + } > > > >>> + > > > >>> + return 0; > > > >>> +} > > > >>> + > > > >>> +int main(int argc, char *argv[]) > > > >>> +{ > > > >>> + int c, ret; > > > >>> + struct fwu_mdata mdata; > > > >>> + > > > >>> + if (argc < 3) { > > > >>> + print_usage(); > > > >>> + return -EINVAL; > > > >>> + } > > > >> > > > >> above you have only help and here you are asking for 3 args which > > > >> seems to me wrong. > > > > > > > > Thank you for your review and the feedback on my patch series. > > > > After I sent the series, I realized that this series had previously > > > > been posted: > > > > https://lore.kernel.org/all/20251212-feature_fwumdata-v2-0-ad51572fbe79@bootlin.com/. > > > > From what I understand, it starts from the same need as mine, namely > > > > the implementation of a tool that > > > > allows reading FWU metadata from Linux. My version is definitely more > > > > minimal compared to what Kory > > > > has done. > > > > Therefore, before sending a v2, I think it would be useful to > > > > understand which of the two makes more sense > > > > to move forward with. Could you help me with that? I have also CCed > > > > Kory to keep the discussion shared. > > > > > > Kory's version looks more complete. If Kory is happy to send another > > > version I think you should test it, review it. If he is not going to send > > > v3 I think you should merge your series with his and send another > > > version. > > > > Hello, > > > > I was going to ping reviews on my series. > > I will be glad to send a 3rd version but there will be no change from v2 as > > nobody reviewed it for now, but if needed I can resend it. > > It would be nice if you review and test it on you side to know if it match > > your requirements. > > I'll be resuming work on swupdate for our STM32MP-based custom board shortly > and will test your series as soon as possible. > > Just one question: I was expecting the introduction of a tool to read the > data contained in fwu-metadata. Why does your new tool also add write > functionality, considering that mkfwumdata already exists? Hello, Because we don't want to generate a new FWU metadata every time and then write it on the FWU metadata partition. It is better to read and edit part of it directly with the same tool. The way I saw it was that mkfwumdata should only to be used on the host side to generate the first FWU metadata. Regards,
diff --git a/tools/Kconfig b/tools/Kconfig index 652b0f225579..9de7eed1bbbb 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -194,6 +194,13 @@ config LUT_SEQUENCE help Look Up Table Sequence +config TOOLS_DUMPFWUMDATA + bool "Build dumpfwumdata command" + default y if FWU_MULTI_BANK_UPDATE + help + This command allows users to read and display the raw FWU + metadata. The exact access method depends on the platform. + config TOOLS_MKFWUMDATA bool "Build mkfwumdata command" default y if FWU_MULTI_BANK_UPDATE diff --git a/tools/Makefile b/tools/Makefile index ae6a30526466..bc105a086927 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -271,6 +271,10 @@ mkeficapsule-objs := generated/lib/uuid.o \ mkeficapsule.o hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule +dumpfwumdata-objs := dumpfwumdata.o generated/lib/crc32.o +HOSTLDLIBS_dumpfwumdata += -luuid +hostprogs-$(CONFIG_TOOLS_DUMPFWUMDATA) += dumpfwumdata + mkfwumdata-objs := mkfwumdata.o generated/lib/crc32.o HOSTLDLIBS_mkfwumdata += -luuid hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata diff --git a/tools/dumpfwumdata.c b/tools/dumpfwumdata.c new file mode 100644 index 000000000000..4965484df1be --- /dev/null +++ b/tools/dumpfwumdata.c @@ -0,0 +1,128 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2025, Amarula Solutions + */ + +#include <errno.h> +#include <getopt.h> +#include <limits.h> +#include <stdio.h> +#include <stdint.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <generated/autoconf.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <u-boot/crc.h> +#include <uuid/uuid.h> + +typedef uint8_t u8; +typedef int16_t s16; +typedef uint16_t u16; +typedef uint32_t u32; +typedef uint64_t u64; + +#undef CONFIG_FWU_NUM_BANKS +#undef CONFIG_FWU_NUM_IMAGES_PER_BANK + +/* This will dynamically allocate the fwu_mdata */ +#define CONFIG_FWU_NUM_BANKS 0 +#define CONFIG_FWU_NUM_IMAGES_PER_BANK 0 + +#include <fwu_mdata.h> + +static const char *opts_short = "h"; + +static struct option options[] = { + {"help", no_argument, NULL, 'h'}, + {NULL, 0, NULL, 0}, +}; + +static void print_usage(void) +{ + fprintf(stderr, "Usage: dumpfwumdata [options] <fwumdata file>\n"); + fprintf(stderr, "Options:\n" + "\t-h, --help print a help message\n" + ); +} + +static void print_mdata(struct fwu_mdata *data) +{ + int i; + + fprintf(stdout, "FWU Metadata\n"); + fprintf(stdout, "crc32: %#x\n", data->crc32); + fprintf(stdout, "version: %#x\n", data->version); + fprintf(stdout, "active_index: %#x\n", data->active_index); + fprintf(stdout, "previous_active_index: %#x\n", + data->previous_active_index); + + if (data->version == 2) { + uint8_t state; + char cstate; + for (i = 0; i < 4; i++) { + state = data->bank_state[i]; + if (state == FWU_BANK_ACCEPTED) + cstate = 'A'; + else if (state == FWU_BANK_VALID) + cstate = 'V'; + else if (state == FWU_BANK_INVALID) + cstate = 'I'; + else + cstate = '?'; + + printf("bank_state[%d]: %c\n", i, cstate); + } + } +} + +static int fwu_read_mdata(struct fwu_mdata *mdata, const char *mdata_file) +{ + FILE *mfile = NULL; + size_t ret, size = sizeof(struct fwu_mdata); + + mfile = fopen(mdata_file, "r"); + if (!mfile) { + fprintf(stderr, "Error: Failed to open %s\n", + mdata_file); + return -1; + } + + ret = fread(mdata, 1, size, mfile); + fclose(mfile); + if (ret != size) { + fprintf(stderr, "Error: Failed to read from %s\n", + mdata_file); + return -1; + } + + return 0; +} + +int main(int argc, char *argv[]) +{ + int c, ret; + struct fwu_mdata mdata; + + if (argc < 3) { + print_usage(); + return -EINVAL; + } + + do { + c = getopt_long(argc, argv, opts_short, options, NULL); + switch (c) { + case 'h': + print_usage(); + return 0; + } + } while (c != -1); + + ret = fwu_read_mdata(&mdata, argv[argc - 1]); + if (ret) + return ret; + + print_mdata(&mdata); + return 0; +}
Allow reading FWU metadata from userspace. According to [1], after updating the software on bank B, setting active_index to point to it (i. e. 1) and marking it as a Valid bank, it must be possible to read this information from userspace on the next boot to verify whether the boot chain of bank B succeeded. The metadata must then be updated again, marking the bank as INVALID if the active_index points to bank A (i. e. 0) or as VALID if the boot was successful. To allow reading the active_index and bank state, this new tool has been added. Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> --- tools/Kconfig | 7 +++ tools/Makefile | 4 ++ tools/dumpfwumdata.c | 128 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+) create mode 100644 tools/dumpfwumdata.c