[RFC PATCH] cmd: fwu: Dump custom fields from mdata structure

[Michal Simek]

On 3/21/25 08:40, Sughosh Ganu wrote:
> On Wed, 5 Jun 2024 at 20:25, Michal Simek <michal.simek at 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.simek at 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