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

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

[Karl Apsite]

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.

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)

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

[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@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

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

[Karl Apsite]

On 04/22/2015 09:55 PM, Simon Glass wrote:
> +Tom
> 
> Hi Karl,
> 
> On 22 April 2015 at 13:05, Karl Apsite <Karl.Apsite@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.

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.

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

> 
> Regards,
> Simon
> 

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

[Simon Glass]

Hi Karl,

On 23 April 2015 at 07:15, Karl Apsite <Karl.Apsite@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.Apsite@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.
>
> 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] [RFC] Booting Xen from a FIT - Additional discussion about a refactor

[Karl Apsite]

On 04/23/2015 01:06 PM, Simon Glass wrote:
> Hi Karl,
> 
> On 23 April 2015 at 07:15, Karl Apsite <Karl.Apsite@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.Apsite@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.
>>
>> 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] [RFC] Booting Xen from a FIT - Additional discussion about a refactor

[Simon Glass]

Hi Karl,

On 23 April 2015 at 15:39, Karl Apsite <Karl.Apsite@dornerworks.com> wrote:
>
>
> On 04/23/2015 01:06 PM, Simon Glass wrote:
>> Hi Karl,
>>
>> On 23 April 2015 at 07:15, Karl Apsite <Karl.Apsite@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.Apsite@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.
>>>
>>> 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.

There is a load and an exec address. The first tells U-Boot where to
copy it (likely you will want this) and exec tells it where to start
execution (which is only used for some image types).

So I would be quite comfortable with you just adding a new image for
your special type of kernel.

[snip]

Regards,
Simon

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

[Karl Apsite]

Hi Simon,
I Added the email listed in the test script to this conversation

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

I took a look at the current set of tests, and I'm not sure what's required to
run them.  Is there an undocumented runner in the Makefile?  I can run the
specified make target to prepare for the tests it sounds like:
`make O=sandbox sandbox_config`

However trying to run the script just generated errors about missing images.

Do you know what's needed to run the script?

Log:
$ test/image/test-imagetools.sh

Building multi-file image...
# sandbox/tools/mkimage -A x86 -O linux -T multi -n "v1.0-test" -d
sandbox/boot/vmlinuz:sandbox/boot/initrd.img:sandbox/boot/System.map linux.img
test/image/test-imagetools.sh: line 73: sandbox/tools/mkimage: No such file or
directory
done.

Extracting multi-file image contents...
# sandbox/tools/dumpimage -T multi -i linux.img -p 0 vmlinuz
test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
directory
# sandbox/tools/dumpimage -T multi -i linux.img -p 1 initrd.img
test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
directory
# sandbox/tools/dumpimage -T multi -i linux.img -p 2 System.map
test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
directory
# sandbox/tools/dumpimage -T multi -i linux.img -p 2 System.map -o test_output
test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
directory
done.
diff: vmlinuz: No such file or directory
Failed.

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

[Simon Glass]

Hi Karl,

On 1 May 2015 at 10:21, Karl Apsite <Karl.Apsite@dornerworks.com> wrote:
> Hi Simon,
> I Added the email listed in the test script to this conversation
>
>>>>
>>>> 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.
>
> I took a look at the current set of tests, and I'm not sure what's required to
> run them.  Is there an undocumented runner in the Makefile?  I can run the
> specified make target to prepare for the tests it sounds like:
> `make O=sandbox sandbox_config`
>
> However trying to run the script just generated errors about missing images.
>
> Do you know what's needed to run the script?

The instructions are at the top of the script:

>
> Log:
> $ test/image/test-imagetools.sh
>
> Building multi-file image...
> # sandbox/tools/mkimage -A x86 -O linux -T multi -n "v1.0-test" -d
> sandbox/boot/vmlinuz:sandbox/boot/initrd.img:sandbox/boot/System.map linux.img
> test/image/test-imagetools.sh: line 73: sandbox/tools/mkimage: No such file or
> directory
> done.
>
> Extracting multi-file image contents...
> # sandbox/tools/dumpimage -T multi -i linux.img -p 0 vmlinuz
> test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
> directory
> # sandbox/tools/dumpimage -T multi -i linux.img -p 1 initrd.img
> test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
> directory
> # sandbox/tools/dumpimage -T multi -i linux.img -p 2 System.map
> test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
> directory
> # sandbox/tools/dumpimage -T multi -i linux.img -p 2 System.map -o test_output
> test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
> directory
> done.
> diff: vmlinuz: No such file or directory
> Failed.

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

[Simon Glass]

Hi Karl,

On 1 May 2015 at 10:21, Karl Apsite <Karl.Apsite@dornerworks.com> wrote:
> Hi Simon,
> I Added the email listed in the test script to this conversation
>
>>>>
>>>> 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.
>
> I took a look at the current set of tests, and I'm not sure what's required to
> run them.  Is there an undocumented runner in the Makefile?  I can run the
> specified make target to prepare for the tests it sounds like:
> `make O=sandbox sandbox_config`
>
> However trying to run the script just generated errors about missing images.
>
> Do you know what's needed to run the script?

The instructions are at the top of the script:

#
# To run this:
#
# make O=sandbox sandbox_config
# make O=sandbox
# ./test/image/test-fit.py -u sandbox/u-boot

>
> Log:
> $ test/image/test-imagetools.sh
>
> Building multi-file image...
> # sandbox/tools/mkimage -A x86 -O linux -T multi -n "v1.0-test" -d
> sandbox/boot/vmlinuz:sandbox/boot/initrd.img:sandbox/boot/System.map linux.img
> test/image/test-imagetools.sh: line 73: sandbox/tools/mkimage: No such file or
> directory
> done.
>
> Extracting multi-file image contents...
> # sandbox/tools/dumpimage -T multi -i linux.img -p 0 vmlinuz
> test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
> directory
> # sandbox/tools/dumpimage -T multi -i linux.img -p 1 initrd.img
> test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
> directory
> # sandbox/tools/dumpimage -T multi -i linux.img -p 2 System.map
> test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
> directory
> # sandbox/tools/dumpimage -T multi -i linux.img -p 2 System.map -o test_output
> test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
> directory
> done.
> diff: vmlinuz: No such file or directory
> Failed.

Regards,
Simon