On 04/23/2015 01:06 PM, Simon Glass wrote:
> 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?
In each of the existing boot_get_<thing> functions, I saw that U-boot stores
various addresses in the images parameter: bootm_headers_t *images. I am making
an assumption that these addresses are used later for any possible "additional
behaviors." That could very well be a misunderstanding, but I thought those
addresses are used by u-boot later in the boot process.
>
>>
>> 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
