Hi Karl,
On 23 April 2015 at 07:15, Karl Apsite <karl.aps...@dornerworks.com> wrote:
>
> On 04/22/2015 09:55 PM, Simon Glass wrote:
>> +Tom
>>
>> Hi Karl,
>>
>> On 22 April 2015 at 13:05, Karl Apsite <karl.aps...@dornerworks.com> wrote:
>>> Hi!
>>>
>>> I work at DornerWorks with the Xen Hypervisor. We work with a variety of
>>> embedded systems, and we wanted to facilitate Xen's boot procedure through
>>> U-boot's Flattened Image Tree (FIT) format. I've already prototyped some
>>> of the
>>> functionality we were hoping to see, so we thought it'd be prudent to begin
>>> a
>>> discussion with denx to get your opinion on the matter,
>>>
>>> First Objective: (Summary of what was prototyped)
>>> A Flattened Image Tree is capable of holding all of the necessary binaries
>>> already, so we only need to make a quick change to allow u-boot to load an
>>> extra
>>> binary (in this case, a linux kernel) so that Xen can boot and load the
>>> kernel
>>> when it's ready. I started by simply adding a line in the configuration of
>>> my
>>> tree-source (.its) to look like:
>>>
>>> config@1 {
>>> description = "Xen 4.6.0-unstable configuration";
>>> kernel = "xen_kernel@1";
>>> fdt = "fdt@1";
>>> gen_bin0 = "linux_kernel@1";
>>> };
>>>
>>> I investigated what effort would be needed to load the additional binary.
>>>
>>> Booting Xen is easy (only a kernel and fdt are required), but Xen will look
>>> at a
>>> hard-coded memory address to try a load a linux kernel. This has to be
>>> placed
>>> in memory by u-boot. The only major addition I needed, was to make u-boot
>>> care
>>> about a config option named "generic-binary" and load it, no questions
>>> asked. I
>>> chose the name "generic binary" as I simply needed u-boot to load a [thing]
>>> without any additional behavior. I'm using it to specifically load a linux
>>> kernel at a specific memory address in preparation for xen, but there could
>>> be
>>> potential future uses, hence the ambiguous name.
>>
>> I wonder whether you should add a new type for the target kernel?
>> General binary seems a bit vague. Just a thought.
>
> I do agree, I don't really like the term "generic binary" either.
>
> When preparing to boot Xen, u-boot needs to take a binary, and simply put it
> in
> place. Unlike the other images/objects (kernel, fdt, ramdisk, etc) u-boot's
> role is very simple in this regard: "Take these bits, and make sure they go
> over
> here."
>
> In this scenario, the action taken by u-boot should be agnostic to what the
> image actually is. U-boot should simply move a binary, without any additional
> behavior. This led me to choose a name just as generic.
What is this additional behaviour you are referring to?
>
> Maybe changing the name to "Loadable(s)" would sound a bit better, but going
> so
> far as to name it "kernel" could be misleading.
>
>>
>>>
>>> The hack is pretty simple, as most everything was in place to boot xen, and
>>> it
>>> took a little extra legwork to make u-boot care about my gen_bin. I added
>>> the
>>> necessary structure to load another object using ramdisk functions as
>>> examples.
>>> By loading a separate binary, Xen was able to boot, and successively boot
>>> Linux
>>> as expected. If you have any thoughts on this process overall, we'd like to
>>> take your concerns into consideration.
>>>
>>> For a little more fun, I extended out the configuration to be able to load N
>>> number of generics.
>
> This plan was part of the reason we were trying to keep the name more generic.
> Keeping it generic allows for people other than xen users to take advantage of
> the new feature in various ways.
>
>>>
>>> Affected Files:
>>> common/bootm.c
>>> common/image-fit.c
>>> common/image.c
>>> doc/uImage.FIT/source_file_format.txt
>>> include/configs/xilinx_zynqmp.h
>>> include/image.h
>>>
>>> Second Objective:
>>> While I was there, I noticed that u-boot's binary loading logic isn't as
>>> modular
>>> as I first expected. Most objects eventually boil down to a
>>> boot_get_<thing>().
>>> Example: boot_get_ramdisk() or boot_get_kernel(). These functions are all
>>> very
>>> similar as they handle the various u-boot image types and load their
>>> specific
>>> binary. The functions appear to have grown similar in structure and
>>> operation
>>> overtime. I don't think it is too much to reduce the duplicated structure
>>> of
>>> the functions into a common boot_get_object() function, while replacing
>>> some of
>>> the extra function wrappings.
>>>
>>> Some quick diagrams to explain what I mean
>>> Initial: http://i.imgur.com/H44dXmN.png (29KB)
>>> Refactor: http://i.imgur.com/apEpyWz.png (24KB)
>>
>> SGTM.
>>
>>>
>>> The refactor will likely effect the following files, several of which
>>> contain
>>> trivial name changes, or small changes in a variable type.
>>> arch/arm/lib/bootm.c
>>> arch/avr32/lib/bootm.c
>>> arch/microblaze/lib/bootm.c
>>> arch/mips/lib/bootm.c
>>> arch/nds32/lib/bootm.c
>>> arch/nios2/lib/bootm.c
>>> arch/openrisc/lib/bootm.c
>>> arch/powerpc/lib/bootm.c
>>> arch/sh/lib/bootm.c
>>> arch/sparc/lib/bootm.c
>>> arch/x86/lib/bootm.c
>>> common/bootm.c
>>> common/bootm_os.c
>>> common/cmd_bootm.c
>>> common/image-fdt.c
>>> common/image.c
>>> include/bootm.h
>>> include/image.h
>>>
>>> Summary
>>> 1.) RFC: What is the opinion on implementing a FIT configuration option to
>>> permit a FIT to boot Xen
>>
>> Seems OK to me.
>>
>>> 2.) RFC: What is the opinion on a potential refactor concerning the
>>> image-loading functions (boot_get_ramdisk, boot_get_kernel, etc) in order to
>>> unify them into one common boot_get_object() function.
>>
>> That would be good. I spent a bit of time refactoring the code to
>> reduce duplication - fit_image_load() was one of the results of that.
>> If you think the code is duplicated now you should have seen it before
>> :-)
>>
>> Probably 'boot_get_image()' is better than 'boot_get_object()' given
>> the current naming.
>>
>> Please do add tests for the new functionality - see test/image for
>> some existing tests. Python is preferred if the test is non-trivial.
>
> Absolutely, I'll be sure to include some tests in the patch(es).
Great. This area of U-Boot has had a lot of undocumented or untested
behaviour. It has got a bit better but your boot function sounds like
another step in the right direction.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
