diff mbox series

[v8,10/13] FWU: cmd: Add a command to read FWU metadata

Message ID 20220817124323.375968-11-sughosh.ganu@linaro.org
State Superseded
Headers show
Series FWU: Add FWU Multi Bank Update feature support | expand

Commit Message

Sughosh Ganu Aug. 17, 2022, 12:43 p.m. UTC
Add a command to read the metadata as specified in the FWU
specification and print the fields of the metadata.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Changes since V7: None

 cmd/Kconfig     |  7 +++++
 cmd/Makefile    |  1 +
 cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+)
 create mode 100644 cmd/fwu_mdata.c

Comments

Simon Glass Aug. 18, 2022, 3:21 a.m. UTC | #1
Hi Sugosh,

On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> Add a command to read the metadata as specified in the FWU
> specification and print the fields of the metadata.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> Changes since V7: None
>
>  cmd/Kconfig     |  7 +++++
>  cmd/Makefile    |  1 +
>  cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 88 insertions(+)
>  create mode 100644 cmd/fwu_mdata.c

This needs docs and a test.

BTW I forgot to mention that the uclass needs a simple test of some sort.

https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html

Regards,
SImon
Sughosh Ganu Aug. 18, 2022, 11:53 a.m. UTC | #2
hi Simon,

On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sugosh,
>
> On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > Add a command to read the metadata as specified in the FWU
> > specification and print the fields of the metadata.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> > Changes since V7: None
> >
> >  cmd/Kconfig     |  7 +++++
> >  cmd/Makefile    |  1 +
> >  cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 88 insertions(+)
> >  create mode 100644 cmd/fwu_mdata.c
>
> This needs docs and a test.
>
> BTW I forgot to mention that the uclass needs a simple test of some sort.
>
> https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html

Okay. I will check how this can be done. Btw, there are plans to add a
test for the feature once support for the feature has been added on
the Synquacer platform. That test will exercise the above command as
well as the driver code. Do we still need a standalone test for the
uclass?

-sughosh
Simon Glass Aug. 18, 2022, 5:49 p.m. UTC | #3
Hi Sughosh,

On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sugosh,
> >
> > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org>
wrote:
> > >
> > > Add a command to read the metadata as specified in the FWU
> > > specification and print the fields of the metadata.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > ---
> > > Changes since V7: None
> > >
> > >  cmd/Kconfig     |  7 +++++
> > >  cmd/Makefile    |  1 +
> > >  cmd/fwu_mdata.c | 80
+++++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 88 insertions(+)
> > >  create mode 100644 cmd/fwu_mdata.c
> >
> > This needs docs and a test.
> >
> > BTW I forgot to mention that the uclass needs a simple test of some
sort.
> >
> > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html
>
> Okay. I will check how this can be done. Btw, there are plans to add a
> test for the feature once support for the feature has been added on
> the Synquacer platform. That test will exercise the above command as
> well as the driver code. Do we still need a standalone test for the
> uclass?

Yes, testing on real hardware has nothing to do with the uclass test, which
runs on sandbox. It should be a small unit test like others in test/dm/...

Regards,
Simon
Sughosh Ganu Aug. 19, 2022, 7:41 a.m. UTC | #4
hi Simon,

On Thu, 18 Aug 2022 at 23:19, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sugosh,
> > >
> > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > >
> > > > Add a command to read the metadata as specified in the FWU
> > > > specification and print the fields of the metadata.
> > > >
> > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > ---
> > > > Changes since V7: None
> > > >
> > > >  cmd/Kconfig     |  7 +++++
> > > >  cmd/Makefile    |  1 +
> > > >  cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 88 insertions(+)
> > > >  create mode 100644 cmd/fwu_mdata.c
> > >
> > > This needs docs and a test.
> > >
> > > BTW I forgot to mention that the uclass needs a simple test of some sort.
> > >
> > > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html
> >
> > Okay. I will check how this can be done. Btw, there are plans to add a
> > test for the feature once support for the feature has been added on
> > the Synquacer platform. That test will exercise the above command as
> > well as the driver code. Do we still need a standalone test for the
> > uclass?
>
> Yes, testing on real hardware has nothing to do with the uclass test, which runs on sandbox. It should be a small unit test like others in test/dm/...

I am not talking about testing on real hardware, but a test to be run
on sandbox. I had posted the relevant patch [1] in an earlier version
of the patch series. But this test relies on support being added for
the feature on the Synquacer platform. Once those patches get in, I
will be adding the test for the feature as well. And this test
exercises both the fwu_mdata_read command as well as the driver code.
Which is why I was asking if it is necessary to add additional tests
for the command and dm code.

-sughosh

[1] - https://lists.denx.de/pipermail/u-boot/2022-June/485992.html

>
> Regards,
> Simon
Simon Glass Aug. 19, 2022, 3:24 p.m. UTC | #5
Hi Sughosh,

On Fri, 19 Aug 2022 at 01:41, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Thu, 18 Aug 2022 at 23:19, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > > hi Simon,
> > >
> > > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Sugosh,
> > > >
> > > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > >
> > > > > Add a command to read the metadata as specified in the FWU
> > > > > specification and print the fields of the metadata.
> > > > >
> > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > ---
> > > > > Changes since V7: None
> > > > >
> > > > >  cmd/Kconfig     |  7 +++++
> > > > >  cmd/Makefile    |  1 +
> > > > >  cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 88 insertions(+)
> > > > >  create mode 100644 cmd/fwu_mdata.c
> > > >
> > > > This needs docs and a test.
> > > >
> > > > BTW I forgot to mention that the uclass needs a simple test of some sort.
> > > >
> > > > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html
> > >
> > > Okay. I will check how this can be done. Btw, there are plans to add a
> > > test for the feature once support for the feature has been added on
> > > the Synquacer platform. That test will exercise the above command as
> > > well as the driver code. Do we still need a standalone test for the
> > > uclass?
> >
> > Yes, testing on real hardware has nothing to do with the uclass test, which runs on sandbox. It should be a small unit test like others in test/dm/...
>
> I am not talking about testing on real hardware, but a test to be run
> on sandbox. I had posted the relevant patch [1] in an earlier version
> of the patch series. But this test relies on support being added for
> the feature on the Synquacer platform. Once those patches get in, I
> will be adding the test for the feature as well. And this test
> exercises both the fwu_mdata_read command as well as the driver code.
> Which is why I was asking if it is necessary to add additional tests
> for the command and dm code.

It looks like that 'functional' test should be split into several unit
tests that check particular things. See how this works with bootstd,
in test/boot - the Python sets things up and the unit tests cover
particular areas. The test seems to rely on things happening at
reboot, so create a command to do those things, to provide for
testability.

So for example 'ut boot fwu_prepare_update' could set up the update
and 'ut boot fwu_apply_update' could apply it.

Regards,
Simon


>
> -sughosh
>
> [1] - https://lists.denx.de/pipermail/u-boot/2022-June/485992.html
>
> >
> > Regards,
> > Simon
Sughosh Ganu Aug. 22, 2022, 4:59 a.m. UTC | #6
hi Simon,

On Fri, 19 Aug 2022 at 20:55, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Fri, 19 Aug 2022 at 01:41, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Thu, 18 Aug 2022 at 23:19, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Sugosh,
> > > > >
> > > > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > > >
> > > > > > Add a command to read the metadata as specified in the FWU
> > > > > > specification and print the fields of the metadata.
> > > > > >
> > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > > ---
> > > > > > Changes since V7: None
> > > > > >
> > > > > >  cmd/Kconfig     |  7 +++++
> > > > > >  cmd/Makefile    |  1 +
> > > > > >  cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  3 files changed, 88 insertions(+)
> > > > > >  create mode 100644 cmd/fwu_mdata.c
> > > > >
> > > > > This needs docs and a test.
> > > > >
> > > > > BTW I forgot to mention that the uclass needs a simple test of some sort.
> > > > >
> > > > > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html
> > > >
> > > > Okay. I will check how this can be done. Btw, there are plans to add a
> > > > test for the feature once support for the feature has been added on
> > > > the Synquacer platform. That test will exercise the above command as
> > > > well as the driver code. Do we still need a standalone test for the
> > > > uclass?
> > >
> > > Yes, testing on real hardware has nothing to do with the uclass test, which runs on sandbox. It should be a small unit test like others in test/dm/...
> >
> > I am not talking about testing on real hardware, but a test to be run
> > on sandbox. I had posted the relevant patch [1] in an earlier version
> > of the patch series. But this test relies on support being added for
> > the feature on the Synquacer platform. Once those patches get in, I
> > will be adding the test for the feature as well. And this test
> > exercises both the fwu_mdata_read command as well as the driver code.
> > Which is why I was asking if it is necessary to add additional tests
> > for the command and dm code.
>
> It looks like that 'functional' test should be split into several unit
> tests that check particular things. See how this works with bootstd,
> in test/boot - the Python sets things up and the unit tests cover
> particular areas. The test seems to rely on things happening at
> reboot, so create a command to do those things, to provide for
> testability.

The testing of the feature is being done on similar lines to how the
capsule update feature is being tested -- in fact, the FWU feature
relies on the capsule update for the underlying update functionality.
I think it would be good to have a script to test the feature. If you
want, I can see how I can add a test for the command, although I think
it would be superfluous given that the command will be tested as part
of the feature testing.

-sughosh

>
> So for example 'ut boot fwu_prepare_update' could set up the update
> and 'ut boot fwu_apply_update' could apply it.
>
> Regards,
> Simon
>
>
> >
> > -sughosh
> >
> > [1] - https://lists.denx.de/pipermail/u-boot/2022-June/485992.html
> >
> > >
> > > Regards,
> > > Simon
Simon Glass Aug. 22, 2022, 4:39 p.m. UTC | #7
Hi Sughosh,

On Sun, 21 Aug 2022 at 22:59, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Fri, 19 Aug 2022 at 20:55, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Fri, 19 Aug 2022 at 01:41, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > > hi Simon,
> > >
> > > On Thu, 18 Aug 2022 at 23:19, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Sugosh,
> > > > > >
> > > > > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > > > >
> > > > > > > Add a command to read the metadata as specified in the FWU
> > > > > > > specification and print the fields of the metadata.
> > > > > > >
> > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > > > ---
> > > > > > > Changes since V7: None
> > > > > > >
> > > > > > >  cmd/Kconfig     |  7 +++++
> > > > > > >  cmd/Makefile    |  1 +
> > > > > > >  cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > >  3 files changed, 88 insertions(+)
> > > > > > >  create mode 100644 cmd/fwu_mdata.c
> > > > > >
> > > > > > This needs docs and a test.
> > > > > >
> > > > > > BTW I forgot to mention that the uclass needs a simple test of some sort.
> > > > > >
> > > > > > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html
> > > > >
> > > > > Okay. I will check how this can be done. Btw, there are plans to add a
> > > > > test for the feature once support for the feature has been added on
> > > > > the Synquacer platform. That test will exercise the above command as
> > > > > well as the driver code. Do we still need a standalone test for the
> > > > > uclass?
> > > >
> > > > Yes, testing on real hardware has nothing to do with the uclass test, which runs on sandbox. It should be a small unit test like others in test/dm/...
> > >
> > > I am not talking about testing on real hardware, but a test to be run
> > > on sandbox. I had posted the relevant patch [1] in an earlier version
> > > of the patch series. But this test relies on support being added for
> > > the feature on the Synquacer platform. Once those patches get in, I
> > > will be adding the test for the feature as well. And this test
> > > exercises both the fwu_mdata_read command as well as the driver code.
> > > Which is why I was asking if it is necessary to add additional tests
> > > for the command and dm code.
> >
> > It looks like that 'functional' test should be split into several unit
> > tests that check particular things. See how this works with bootstd,
> > in test/boot - the Python sets things up and the unit tests cover
> > particular areas. The test seems to rely on things happening at
> > reboot, so create a command to do those things, to provide for
> > testability.
>
> The testing of the feature is being done on similar lines to how the
> capsule update feature is being tested -- in fact, the FWU feature
> relies on the capsule update for the underlying update functionality.
> I think it would be good to have a script to test the feature. If you
> want, I can see how I can add a test for the command, although I think
> it would be superfluous given that the command will be tested as part
> of the feature testing.

The uclass needs a sandbox test. The capsule-update thing seems to
rely on restarting the executable which is not a good thing for
debugging a test in gdb, as you can imagine. Please read through the
testing docs if you haven't already. We need a simple test that checks
that the uclass and sandbox driver does what it should.

Regards,
Simon


>
> -sughosh
>
> >
> > So for example 'ut boot fwu_prepare_update' could set up the update
> > and 'ut boot fwu_apply_update' could apply it.
> >
> > Regards,
> > Simon
> >
> >
> > >
> > > -sughosh
> > >
> > > [1] - https://lists.denx.de/pipermail/u-boot/2022-June/485992.html
> > >
> > > >
> > > > Regards,
> > > > Simon
diff mbox series

Patch

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 211ebe9c87..95e762d21c 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -161,6 +161,13 @@  config CMD_CPU
 	  internal name) and clock frequency. Other information may be
 	  available depending on the CPU driver.
 
+config CMD_FWU_METADATA
+	bool "fwu metadata read"
+	depends on FWU_MULTI_BANK_UPDATE
+	default y
+	help
+	  Command to read the metadata and dump it's contents
+
 config CMD_LICENSE
 	bool "license"
 	select BUILD_BIN2C
diff --git a/cmd/Makefile b/cmd/Makefile
index 6e87522b62..ff6e160f4a 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -76,6 +76,7 @@  obj-$(CONFIG_CMD_FPGA) += fpga.o
 obj-$(CONFIG_CMD_FPGAD) += fpgad.o
 obj-$(CONFIG_CMD_FS_GENERIC) += fs.o
 obj-$(CONFIG_CMD_FUSE) += fuse.o
+obj-$(CONFIG_CMD_FWU_METADATA) += fwu_mdata.o
 obj-$(CONFIG_CMD_GETTIME) += gettime.o
 obj-$(CONFIG_CMD_GPIO) += gpio.o
 obj-$(CONFIG_CMD_HVC) += smccc.o
diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c
new file mode 100644
index 0000000000..ee9d035374
--- /dev/null
+++ b/cmd/fwu_mdata.c
@@ -0,0 +1,80 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#include <command.h>
+#include <dm.h>
+#include <fwu.h>
+#include <fwu_mdata.h>
+#include <log.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <linux/types.h>
+
+static void print_mdata(struct fwu_mdata *mdata)
+{
+	int i, j;
+	struct fwu_image_entry *img_entry;
+	struct fwu_image_bank_info *img_info;
+
+	printf("\tFWU Metadata\n");
+	printf("crc32: %#x\n", mdata->crc32);
+	printf("version: %#x\n", mdata->version);
+	printf("active_index: %#x\n", mdata->active_index);
+	printf("previous_active_index: %#x\n", mdata->previous_active_index);
+
+	printf("\tImage Info\n");
+	for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
+		img_entry = &mdata->img_entry[i];
+		printf("\nImage Type Guid: %pUL\n",
+		       &img_entry->image_type_uuid);
+		printf("Location Guid: %pUL\n", &img_entry->location_uuid);
+		for (j = 0; j < CONFIG_FWU_NUM_BANKS; j++) {
+			img_info = &img_entry->img_bank_info[j];
+			printf("Image Guid:  %pUL\n", &img_info->image_uuid);
+			printf("Image Acceptance: %s\n",
+			       img_info->accepted == 0x1 ? "yes" : "no");
+		}
+	}
+}
+
+int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag,
+		     int argc, char * const argv[])
+{
+	struct udevice *dev;
+	int ret = CMD_RET_SUCCESS, res;
+	struct fwu_mdata *mdata = NULL;
+
+	if (uclass_get_device(UCLASS_FWU_MDATA, 0, &dev) || !dev) {
+		log_err("Unable to get FWU metadata device\n");
+		return CMD_RET_FAILURE;
+	}
+
+	res = fwu_mdata_check();
+	if (res < 0) {
+		log_err("FWU Metadata check failed\n");
+		ret = CMD_RET_FAILURE;
+		goto out;
+	}
+
+	res = fwu_get_mdata(&mdata);
+	if (res < 0) {
+		log_err("Unable to get valid FWU metadata\n");
+		ret = CMD_RET_FAILURE;
+		goto out;
+	}
+
+	print_mdata(mdata);
+
+out:
+	free(mdata);
+	return ret;
+}
+
+U_BOOT_CMD(
+	fwu_mdata_read,	1,	1,	do_fwu_mdata_read,
+	"Read and print FWU metadata",
+	""
+);