On 3/21/25 08:40, Sughosh Ganu wrote:
On Wed, 5 Jun 2024 at 20:25, Michal Simek <michal.si...@amd.com> wrote:

The commit cb9ae40a16f0 ("tools: mkfwumdata: add logic to append vendor
data to the FWU metadata") added support for adding vendor data to mdata
structure but it is not visible anywhere that's why extend fwu command to
dump it.

Signed-off-by: Michal Simek <michal.si...@amd.com>
---

Looks good. Just a couple of nits.


I am using this for some time to check various configurations that's why it
can be useful for others too.
Sughosh: Maybe there is better way how to dump it.
---
  cmd/fwu_mdata.c | 25 +++++++++++++++++++++++++
  1 file changed, 25 insertions(+)

diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c
index 9c048d69a131..ff6435505167 100644
--- a/cmd/fwu_mdata.c
+++ b/cmd/fwu_mdata.c
@@ -7,6 +7,7 @@
  #include <dm.h>
  #include <fwu.h>
  #include <fwu_mdata.h>
+#include <hexdump.h>
  #include <log.h>
  #include <stdio.h>
  #include <stdlib.h>
@@ -45,6 +46,30 @@ static void print_mdata(struct fwu_data *data)
                                img_info->accepted == 0x1 ? "yes" : "no");
                 }
         }
+
+       if (data->version == 2) {
+               struct fwu_mdata *mdata = data->fwu_mdata;
+               struct fwu_fw_store_desc *desc;
+               void *end;
+               u32 diff;
+
+               /*
+                * fwu_mdata defines only header that's why taking it as array
+                * which exactly point to image description location
+                */
+               desc = (struct fwu_fw_store_desc *)&mdata[1];
+
+               /* Number of entries is taken from for loop - variable i */
+               end = &desc->img_entry[i];
+               debug("mdata %p, desc %p, end %p\n", mdata, desc, end);
+
+               diff = data->metadata_size - ((void *)end - (void *)mdata);
+               if (diff) {
+                       printf("Custom fields covered by CRC 0x%x\n", diff);

It would be better to mention that the value being printed is the
length of the vendor data. Or not print the value.


printf("Custom fields covered by CRC len: 0x%x\n", diff);


+                       print_hex_dump_bytes("CUSTOM ", DUMP_PREFIX_OFFSET,
+                                            end, diff);
+               }

Would be better to do a 'select HEXDUMP if FWU_MDATA_V2' under the
CMD_FWU_METADATA config.

I think select is too hard here because hexdump is implementing empty function if HEXDUMP is not enabled.

imply HEXDUMP if FWU_MDATA_V2

Thanks,
Michal

Reply via email to