From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Babic Date: Tue, 29 Nov 2016 17:24:12 +0100 Subject: [U-Boot] [PATCH] tools: imximage: display DCD block offset, length In-Reply-To: References: <1479341621-30021-1-git-send-email-eric@nelint.com> <20161117121645.nrxecqtxznuacwg6@t450s.lan> Message-ID: <02260c0b-49f5-8831-4cdf-63b84622fa59@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >> >> 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 =====================================================================