[4/5] tools: Add dumpfwumdata tool for FWU metadata image

Message ID 20251228151824.25667-5-dario.binacchi@amarulasolutions.com
State New
Headers show
Series
  • Add dumpfwumdata tool for FWU metadata image
Related show

Commit Message

Dario Binacchi Dec. 28, 2025, 3:17 p.m. UTC
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

Comments

Michal Simek Jan. 5, 2026, 3:40 p.m. UTC | #1
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
Dario Binacchi Jan. 11, 2026, 3:17 p.m. UTC | #2
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
Michal Simek Jan. 12, 2026, 9:05 a.m. UTC | #3
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.
'Rob Herring (Arm)' via Amarula Linux Jan. 12, 2026, 9:24 a.m. UTC | #4
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,
Dario Binacchi Jan. 17, 2026, 11:11 a.m. UTC | #5
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

Patch

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;
+}