Re: [U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address
Dear Stephen Warren,
In message 74CDBE0F657A3D45AFBB94109FB122FF173F9A5424@HQMAIL01.nvidia.com you wrote:
bootm is for uImage format. I see no sense in "extending" it.
bootm already supports two completely different formats; legacy uImage and FIT images. To me, it seems logical to simply add support for a third image format for the kernel at least. Do you completely disagree with this? Well, bootm would need to recognize raw (non-uImage-wrapped) initrd and FDT blobs too, since currently bootm expects everything to be uImage-wrapped.
Right, once you start this way you will quickly have a mess.
Yes, bootm supports both uImage and FIT format images, which are considered "U-Boot native" image formats. For other formats we use different commands - there is "bootelf" for ELF files, or there is bootvx() to boot VxWorks images.
Given the different set of requirements for zImage it makes more sense to me to provide a separate command for it. This will also allow for less #ifdef's for the case you do not want to enable "bootz" support in the configuration.
One potential advantage of extending bootz to recognize zImage directly would be the re-use of the overall bootm flow and arch functions such as arch/arm/lib/bootm.c:do_bootm_linux(). I /think/ that creating a new
I guess this is a typo above, and you mean "extending bootm" ? Well, imagine how many #ifdef's would be needed to make this "bootz" support configurable.
separate bootz command would require duplicating a lot of code and might make re-using do_bootm_linux() more complex, although again I'd need to look at the code in more detail to say for sure.
Eventually common parts may be factored out.
Are you willing to entertain extending bootm to recognize a third image format if this makes the patches less invasive, and/or leads to more maintainable code?
I have to admit that I don't like the idea, but I will not argue over hard facts. But please keep in mind that bootz support shall be a configuration option, that can be selected or omittet at build time. My feeling is that this would require quite a number of new #ifdef's if you try to add it into the existing code.
Best regards,
Wolfgang Denk
Dear Stephen,
In message 20111108194433.7C9A013BE0C5@gemini.denx.de I wrote:
Are you willing to entertain extending bootm to recognize a third image format if this makes the patches less invasive, and/or leads to more maintainable code?
I have to admit that I don't like the idea, but I will not argue over hard facts. But please keep in mind that bootz support shall be a configuration option, that can be selected or omittet at build time. My feeling is that this would require quite a number of new #ifdef's if you try to add it into the existing code.
Thinking about this again I wonder if this is really what is needed. Why do we need a third image format?
What would happen if we just create a new image type IH_TYPE_ZIMAGE?
Then you can use plain "bootm", and probably most of the existing code. A few tests may need to be extended by some "|| (type == IH_TYPE_ZIMAGE)", and in the end you can just skip the parts that deal with loading and starting.
For the sake of consistency, such images should be built with IH_COMP_NONE, then.
What do you think?
Best regards,
Wolfgang Denk
On 11/08/2011 02:17 PM, Wolfgang Denk wrote:
Dear Stephen,
In message 20111108194433.7C9A013BE0C5@gemini.denx.de I wrote:
Are you willing to entertain extending bootm to recognize a third image format if this makes the patches less invasive, and/or leads to more maintainable code?
I have to admit that I don't like the idea, but I will not argue over hard facts. But please keep in mind that bootz support shall be a configuration option, that can be selected or omittet at build time. My feeling is that this would require quite a number of new #ifdef's if you try to add it into the existing code.
Thinking about this again I wonder if this is really what is needed. Why do we need a third image format?
What would happen if we just create a new image type IH_TYPE_ZIMAGE?
That would cover the kernel uImage case. We'd also need a new image type for "use in place" FDTs, since that also gets relocated to the image load address before use. I think initrds don't for some reason.
The advantage of using a -1 load address was that it avoided a proliferation of new IH_TYPE_* values, and an associated requirement for everyone to update their mkimage tool.
Another advantage of directly recognizing non-uImage-wrapper zImage, initrd, and FDT (in either bootm or bootz), is that distros wouldn't have to run mkimage ever; they could just dump the kernel in e.g. /boot/zImage (plus say /boot/initrd.img and /boot/fdt.bin), and expect U-Boot to pick it up and use it without any further processing. This would isolate more of the distro kernel install scripts from U-Boot, and keep them generic.
...
For the sake of consistency, such images should be built with IH_COMP_NONE, then.
Certainly.
Dear Stephen Warren,
In message 4EB9ACDF.90702@nvidia.com you wrote:
What would happen if we just create a new image type IH_TYPE_ZIMAGE?
That would cover the kernel uImage case. We'd also need a new image type for "use in place" FDTs, since that also gets relocated to the image load address before use. I think initrds don't for some reason.
Did you check how PPC handles this? We usually load the DTB as a separate file (the plain DTB, not any image) and just pass it's address to the bootm command. U-Boot then knows how to handle the DTB on a specific board.
Why would an image type for DTBs be needed?
Another advantage of directly recognizing non-uImage-wrapper zImage, initrd, and FDT (in either bootm or bootz), is that distros wouldn't have to run mkimage ever; they could just dump the kernel in e.g. /boot/zImage (plus say /boot/initrd.img and /boot/fdt.bin), and expect U-Boot to pick it up and use it without any further processing. This would isolate more of the distro kernel install scripts from U-Boot, and keep them generic.
I have no real preference for one implementation versus the other. My gut feeling is that bootm + IH_TYPE_ZIMAGE is much more straightfor- ward, leaner and easier to implement. But it will add the 64 byte uImage header which might be a nuisance in some cases. On the other hand, bootz as to implement all the existing code again for handling ramdisk/initrd and DTB, basicly identically. And you miss features you might find useful in some cases - like that uImages have a name, a timestamp, or a checksum, so you can identify and verify what has been loaded.
I would go for IH_TYPE_ZIMAGE, but I'm not going to implement it.
Best regards,
Wolfgang Denk
participants (2)
-
Stephen Warren -
Wolfgang Denk