From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gary Bisson <gary.bisson@boundarydevices.com>
Date: Thu, 17 Nov 2016 13:16:45 +0100
Subject: [U-Boot] [PATCH] tools: imximage: display DCD block offset,
	length
In-Reply-To: <1479341621-30021-1-git-send-email-eric@nelint.com>
References: <1479341621-30021-1-git-send-email-eric@nelint.com>
Message-ID: <20161117121645.nrxecqtxznuacwg6@t450s.lan>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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