[U-Boot] [RFC] Booting Xen from a FIT - Additional discussion about a refactor

[Simon Glass]

+Tom

Hi Karl,

On 22 April 2015 at 13:05, Karl Apsite <Karl.Apsite at 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 at 1 {
>     description = "Xen 4.6.0-unstable configuration";
>     kernel = "xen_kernel at 1";
>     fdt = "fdt at 1";
>     gen_bin0 = "linux_kernel at 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.

>
> 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.
>
> 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.

Regards,
Simon