Hi Tom, Simon, On 01/07/2016 18:45, Tom Rini wrote: > On Fri, Jul 01, 2016 at 09:15:11AM -0700, Simon Glass wrote: >> Hi, >> >> On 1 July 2016 at 08:48, Tom Rini <tr...@konsulko.com> wrote: >>> On Fri, Jul 01, 2016 at 10:44:04AM +0200, Stefano Babic wrote: >>>> Hi Simon, >>>> >>>> On 30/06/2016 18:52, Simon Glass wrote: >>>>> There are a few problems when mkimage is provided with invalid arguments. >>>>> In one case it crashes. When an invalid image type it is provided it lists >>>>> the valid types, but this is not implemented for compression, architecture >>>>> or OS. >>>>> >>>> >>>> There is another issue related to mkimage. It looks like it is broken >>>> since a lot of time, but it appears it was not noted. >>>> >>>> mkimage -l is broken. Testing with i.MX images (imximage), it does not >>>> show the header, but it reports the output as it as a "gpimage". >>>> >>>> In fact: >>>> >>>> ./tools/mkimage -l test.imx >>>> GP Header: Size d1002040 LoadAddr 8017 >>>> >>>> It should be: >>>> >>>> ./tools/mkimage -l test.imx >>>> Image Type: Freescale IMX Boot Image >>>> Image Ver: 2 (i.MX53/6/7 compatible) >>>> Data Size: 331776 Bytes = 324.00 kB = 0.32 MB >>>> Load Address: 177ff420 >>>> Entry Point: 17800000 >>>> >>>> The reason is due to the format of the gpimage itself. It has no magic >>>> number, and checking for its correctness means in gph_verify_header() >>>> just check that size and address are not NULL. That let think that the >>>> image is a gpimage, it is not. gpimage simply uses the image and does >>>> not let to check for further image headers that are put int the (sorted) >>>> list with the U_BOOT_IMAGE_TYPE. Image types that are in the list >>>> *before* gpimage are correctly recognized and printed, the following (as >>>> imximage) not. >>>> >>>> A quick fix (maybe just for the release ?) should be to let gpimage (but >>>> I do not know if there is another image type that misbehaves as it does) >>>> as the last one in the list: if all detections fail, the last one >>>> without any possibility for detection (gpimage) runs. The following >>>> patch works for me: >>>> >>>> diff --git a/tools/Makefile b/tools/Makefile >>>> index 63355aa..f72294a 100644 >>>> --- a/tools/Makefile >>>> +++ b/tools/Makefile >>>> @@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \ >>>> lib/fdtdec.o \ >>>> fit_common.o \ >>>> fit_image.o \ >>>> - gpimage.o \ >>>> - gpimage-common.o \ >>>> common/image-fit.o \ >>>> image-host.o \ >>>> common/image.o \ >>>> @@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \ >>>> zynqimage.o \ >>>> zynqmpimage.o \ >>>> $(LIBFDT_OBJS) \ >>>> + gpimage.o \ >>>> + gpimage-common.o \ >>>> $(RSA_OBJS-y) >>>> >>>> dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o >>>> >>>> What do you think ? If this could be a solution for release, I send a >>>> formal patch. >>> >>> Ug, I think we need what you're saying at least for release. I'm >>> kicking off a big bisect now to see just when this last worked right. >> >> That patch seems reasonable to me for now. My series is intended for >> the next release. >> >> Perhaps verify_header() should return a value meaning 'possibly'? > > So, this was broken by: > commit 0ca6691c2ec20ff264d882854c339795579991f5 > Author: Guilherme Maciel Ferreira <guilherme.maciel.ferre...@gmail.com> > Date: Thu Jan 15 02:48:05 2015 -0200 > > imagetool: move common code to imagetool module >
Yes, I had bisected myself and I get this commit - but the patch is not the readon of the breakage. > And yes, perhaps some way to say to say "maybe" is enough. Right, but there is a side effect. Even if a "maybe" is returned (or let's say, a "I can't check"), the effect is that "GP Header: Size XXXX" is still printed, because for the gpimage this *could* be a correct image. > Or maybe we > just need to allow some to say that they can't verify? I think we're thinking > along the same lines. Regards, Stefano -- ===================================================================== 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: sba...@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
