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'? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot