[U-Boot] [PATCH] tools: imximage: display DCD block offset, length

[U-Boot] [PATCH] tools: imximage: display DCD block offset, length

[Eric Nelson]

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 <eric@nelint.com>
---
 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;
-
 		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,
 			       hdr_v2->boot_data.size - imximage_ivt_offset -
 			       imximage_csf_size);
+			printf("DCD Blocks:   00910000 %08x %08x\n",
+			       offs, be16_to_cpu(dcdlen));
 		}
 	} else {
 		imx_header_v2_t *next_hdr_v2;
-- 
2.7.4

[U-Boot] [PATCH] tools: imximage: display DCD block offset, length

[Gary Bisson]

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 <eric@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?

>  		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?

>  			       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,
			       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));
		}

Regards,
Gary

[U-Boot] [PATCH] tools: imximage: display DCD block offset, length

[Eric Nelson]

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 <eric@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] [PATCH] tools: imximage: display DCD block offset, length

[Stefano Babic]

Hi Eric, Gary,

On 17/11/2016 15:23, Eric Nelson wrote:
> 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 <eric@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.

Right.

> 
> In particular, adding 0x before %08x would allow easier cut and paste
> into signing scripts in a manual process.

Most important is to get values. How easy is then to copy&paste, well,
it is not so important.

I have applied the patch to u-boot-imx - thanks !

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================