Hi Gary,
On 11/17/2016 05:16 AM, Gary Bisson wrote:
> Hi Eric, All,
>
> On Wed, Nov 16, 2016 at 05:13:41PM -0700, Eric Nelson wrote:
>> These values can be used to sign a U-Boot image for use when
>> loading an image through the Serial Download Protocol (SDP).
>>
>> Note that the address of 0x910000 is usable with the stock
>> configuration of imx_usb_loader on i.MX6 and i.MX7 SOCs:
>>
>> https://github.com/boundarydevices/imx_usb_loader/blob/master/mx6_usb_work.conf#L3
>>
>> Refer to the section on imx_usb_loader in this post for more
>> details:
>>
>> https://boundarydevices.com/high-assurance-boot-hab-dummies/
>>
>> Signed-off-by: Eric Nelson <e...@nelint.com>
>
> Thanks, indeed such patch would ease the life of anybody that needs to
> deal with HAB when creating the CSF files.
>
>> ---
>> tools/imximage.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/imximage.c b/tools/imximage.c
>> index c9e42ec..2cd8d88 100644
>> --- a/tools/imximage.c
>> +++ b/tools/imximage.c
>> @@ -281,7 +281,6 @@ static void set_dcd_rst_v2(struct imx_header *imxhdr,
>> uint32_t dcd_len,
>> d = (struct dcd_v2_cmd *)(((char *)d) + len);
>>
>> len = (char *)d - (char *)&dcd_v2->header;
>> -
>
> Is this part of the patch intended?
>
Nope.
>> dcd_v2->header.tag = DCD_HEADER_TAG;
>> dcd_v2->header.length = cpu_to_be16(len);
>> dcd_v2->header.version = DCD_VERSION;
>> @@ -501,10 +500,19 @@ static void print_hdr_v2(struct imx_header *imx_hdr)
>> printf("Entry Point: %08x\n", (uint32_t)fhdr_v2->entry);
>> if (fhdr_v2->csf && (imximage_ivt_offset != UNDEFINED) &&
>> (imximage_csf_size != UNDEFINED)) {
>> + uint16_t dcdlen;
>> + int offs;
>> +
>> + dcdlen = hdr_v2->data.dcd_table.header.length;
>> + offs = (char *)&hdr_v2->data.dcd_table
>> + - (char *)hdr_v2;
>> +
>> printf("HAB Blocks: %08x %08x %08x\n",
>> (uint32_t)fhdr_v2->self, 0,
>
> This isn't part of the patch, but why is self cast into a uint32_t
> although it's already a uint32_t?
>
Unrelated, but good catch!
>> hdr_v2->boot_data.size - imximage_ivt_offset -
>> imximage_csf_size);
>> + printf("DCD Blocks: 00910000 %08x %08x\n",
>> + offs, be16_to_cpu(dcdlen));
>> }
>
> Not sure if "DCD Blocks" is the best naming, cause it really just
> applies to SDP protocol.
>
> This got me thinking and I think the printf above should also show the
> Blocks for encryption which is also missing right now.
>
> What about something like the snippet below?
>
> if (fhdr_v2->csf && (imximage_ivt_offset != UNDEFINED) &&
> (imximage_csf_size != UNDEFINED)) {
> uint16_t dcdlen;
> uint32_t dcdoff;
> uint32_t entryoff;
>
> dcdlen = hdr_v2->data.dcd_table.header.length;
> dcdoff = (char *)&hdr_v2->data.dcd_table
> - (char *)hdr_v2;
> entryoff = fhdr_v2->entry - fhdr_v2->self;
>
> printf("[HAB][Signature]\n");
> printf("Blocks: %08x %08x %08x\n",
> (uint32_t)fhdr_v2->self, 0,
I think somebody just pointed out that self is already a uint32_t,
so why the cast?
> hdr_v2->boot_data.size - imximage_ivt_offset -
> imximage_csf_size);
> printf("[HAB][Encryption]\n");
> printf("Blocks: %08x %08x %08x\n",
> fhdr_v2->self, 0, dcdoff + be16_to_cpu(dcdlen));
> printf("Blocks: %08x %08x %08x\n",
> fhdr_v2->entry, entryoff,
> hdr_v2->boot_data.size - imximage_ivt_offset -
> imximage_csf_size - entryoff);
> printf("[HAB][SDP]\n");
> printf("Blocks: 00910000 %08x %08x\n",
> dcdoff, be16_to_cpu(dcdlen));
> }
I like the more specific tags, but wonder if some minor edits wouldn't
make this more useful.
In particular, adding 0x before %08x would allow easier cut and paste
into signing scripts in a manual process.
I looked briefly at the output of 'mkimage -l' and found that it doesn't
reach this code block because imximage_ivt/csf_size variables are
set during the parsing of a .cfg file (they're UNDEFINED with
'mkimage -l').
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
