* [PATCH v2] ARM/multiboot: use more flexible node naming
@ 2013-09-11 14:06 Andre Przywara
2013-09-17 14:53 ` Ian Campbell
0 siblings, 1 reply; 7+ messages in thread
From: Andre Przywara @ 2013-09-11 14:06 UTC (permalink / raw)
To: julien.grall, Ian.Campbell; +Cc: stefano.stabellini, xen-devel
For the current "multiboot" on ARM support we look for a compatible
string of "xen,multiboot-module" in the device tree, and then
use "xen,linux-zimage" and "xen,linux-initrd" to differentiate
between the two supported module types.
To meet the more generic multiboot proposal in the device tree [1],
allow Xen to be more flexible in the compatible naming and also use
the new generic base name "boot,module".
The mapping to either Dom0 kernel or RAM disk works by providing a
more specific name ("xen,dom0-kernel" and "xen,ramdisk", preferably).
For compatibility reasons the older names above are still recognized.
Changes from v1:
* whitespace / coding style fixes (sorry for that mess!)
* removed module enumeration by using module@address
(this violates the EPAPR device tree spec).
* added __initconst to names array
[1] http://lists.xen.org/archives/html/xen-devel/2013-09/msg00083.html
Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
xen/common/device_tree.c | 44 +++++++++++++++++++++++++++++++++++++-------
1 file changed, 37 insertions(+), 7 deletions(-)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index eed77ce..3ae593f 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -433,22 +433,50 @@ static void __init process_cpu_node(const void *fdt, int node,
cpumask_set_cpu(start, &cpu_possible_map);
}
+static const char * const __initconst kernel_module_names[] = {
+ "xen,linux-zimage",
+ "xen,dom0-kernel",
+ "boot,kernel",
+ NULL
+};
+
+static const char * const __initconst initrd_module_names[] = {
+ "xen,linux-initrd",
+ "xen,ramdisk",
+ "boot,ramdisk",
+ NULL
+};
+
static void __init process_multiboot_node(const void *fdt, int node,
const char *name,
u32 address_cells, u32 size_cells)
{
const struct fdt_property *prop;
const u32 *cell;
- int nr;
+ int nr = -1;
struct dt_mb_module *mod;
int len;
+ const char* const * name_list;
- if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 )
- nr = MOD_KERNEL;
- else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
- nr = MOD_INITRD;
- else
- early_panic("%s not a known xen multiboot type\n", name);
+ for ( name_list = kernel_module_names; *name_list != NULL; name_list++ )
+ if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 )
+ {
+ nr = MOD_KERNEL;
+ break;
+ }
+
+ for ( name_list = initrd_module_names; *name_list != NULL; name_list++ )
+ if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 )
+ {
+ nr = MOD_INITRD;
+ break;
+ }
+
+ if ( nr == -1 )
+ {
+ early_printk("warning: %s not a known module type, ignoring\n", name);
+ return;
+ }
mod = &early_info.modules.module[nr];
@@ -486,6 +514,8 @@ static int __init early_scan_node(const void *fdt,
process_cpu_node(fdt, node, name, address_cells, size_cells);
else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) )
process_multiboot_node(fdt, node, name, address_cells, size_cells);
+ else if ( device_tree_node_compatible(fdt, node, "boot,module" ) )
+ process_multiboot_node(fdt, node, name, address_cells, size_cells);
return 0;
}
--
1.7.12.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM/multiboot: use more flexible node naming
2013-09-11 14:06 [PATCH v2] ARM/multiboot: use more flexible node naming Andre Przywara
@ 2013-09-17 14:53 ` Ian Campbell
2013-09-17 15:06 ` Julien Grall
2013-09-17 21:12 ` Andre Przywara
0 siblings, 2 replies; 7+ messages in thread
From: Ian Campbell @ 2013-09-17 14:53 UTC (permalink / raw)
To: Andre Przywara; +Cc: julien.grall, stefano.stabellini, xen-devel
On Wed, 2013-09-11 at 16:06 +0200, Andre Przywara wrote:
> For the current "multiboot" on ARM support we look for a compatible
> string of "xen,multiboot-module" in the device tree, and then
> use "xen,linux-zimage" and "xen,linux-initrd" to differentiate
> between the two supported module types.
> To meet the more generic multiboot proposal in the device tree [1],
> allow Xen to be more flexible in the compatible naming and also use
> the new generic base name "boot,module".
> The mapping to either Dom0 kernel or RAM disk works by providing a
> more specific name ("xen,dom0-kernel" and "xen,ramdisk", preferably).
> For compatibility reasons the older names above are still recognized.
>
> Changes from v1:
> * whitespace / coding style fixes (sorry for that mess!)
> * removed module enumeration by using module@address
> (this violates the EPAPR device tree spec).
> * added __initconst to names array
>
> [1] http://lists.xen.org/archives/html/xen-devel/2013-09/msg00083.html
>
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
> xen/common/device_tree.c | 44 +++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index eed77ce..3ae593f 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -433,22 +433,50 @@ static void __init process_cpu_node(const void *fdt, int node,
> cpumask_set_cpu(start, &cpu_possible_map);
> }
>
> +static const char * const __initconst kernel_module_names[] = {
> + "xen,linux-zimage",
> + "xen,dom0-kernel",
> + "boot,kernel",
I'm wondering about this..
The current "xen,linux-zimage" node does more than simply identifying
the location in memory of the kernel, it actually tells us that the boot
protocol used by that kernel is the Linux zImage protocol.
For "xen,dom0-kernel" and "boot,kernel" we don't get that -- so how do
we know how the kernel in question wants to be called?
I guess there is a magic number in the kernel itself, so maybe this is
OK? Any futyre kernel (I'm thinking *BSD here...) would either need to
be identifiable by magic number or use only a specific compatible value
which indicates this...
OK, I think I've convinced myself, I'll leave the commentary above in
case you can spot a flaw in my reasoning.
> + NULL
> +};
> +
> +static const char * const __initconst initrd_module_names[] = {
> + "xen,linux-initrd",
> + "xen,ramdisk",
> + "boot,ramdisk",
> + NULL
> +};
> +
> static void __init process_multiboot_node(const void *fdt, int node,
> const char *name,
> u32 address_cells, u32 size_cells)
> {
> const struct fdt_property *prop;
> const u32 *cell;
> - int nr;
> + int nr = -1;
> struct dt_mb_module *mod;
> int len;
> + const char* const * name_list;
>
> - if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 )
> - nr = MOD_KERNEL;
> - else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
> - nr = MOD_INITRD;
> - else
> - early_panic("%s not a known xen multiboot type\n", name);
> + for ( name_list = kernel_module_names; *name_list != NULL; name_list++ )
> + if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 )
I've just pushed Julien's big DTB series which adds a helper for
matching against a list of compatible nodes.
> + {
> + nr = MOD_KERNEL;
> + break;
> + }
> +
> + for ( name_list = initrd_module_names; *name_list != NULL; name_list++ )
> + if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 )
> + {
> + nr = MOD_INITRD;
> + break;
> + }
> +
> + if ( nr == -1 )
> + {
> + early_printk("warning: %s not a known module type, ignoring\n", name);
> + return;
> + }
>
> mod = &early_info.modules.module[nr];
>
> @@ -486,6 +514,8 @@ static int __init early_scan_node(const void *fdt,
> process_cpu_node(fdt, node, name, address_cells, size_cells);
> else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) )
Should we mark this as deprecated in the docs?
Speaking of which, this patch doesn't seem to ouch the docs tree ;-)
I know the doc should live somewhere else, but lets try and keep it up
to date until we move it...
> process_multiboot_node(fdt, node, name, address_cells, size_cells);
> + else if ( device_tree_node_compatible(fdt, node, "boot,module" ) )
> + process_multiboot_node(fdt, node, name, address_cells, size_cells);
>
> return 0;
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM/multiboot: use more flexible node naming
2013-09-17 14:53 ` Ian Campbell
@ 2013-09-17 15:06 ` Julien Grall
2013-09-17 15:09 ` Ian Campbell
2013-09-17 21:12 ` Andre Przywara
1 sibling, 1 reply; 7+ messages in thread
From: Julien Grall @ 2013-09-17 15:06 UTC (permalink / raw)
To: Ian Campbell; +Cc: stefano.stabellini, Andre Przywara, xen-devel
On 09/17/2013 03:53 PM, Ian Campbell wrote:
> On Wed, 2013-09-11 at 16:06 +0200, Andre Przywara wrote:
>> For the current "multiboot" on ARM support we look for a compatible
>> string of "xen,multiboot-module" in the device tree, and then
>> use "xen,linux-zimage" and "xen,linux-initrd" to differentiate
>> between the two supported module types.
>> To meet the more generic multiboot proposal in the device tree [1],
>> allow Xen to be more flexible in the compatible naming and also use
>> the new generic base name "boot,module".
>> The mapping to either Dom0 kernel or RAM disk works by providing a
>> more specific name ("xen,dom0-kernel" and "xen,ramdisk", preferably).
>> For compatibility reasons the older names above are still recognized.
>>
>> Changes from v1:
>> * whitespace / coding style fixes (sorry for that mess!)
>> * removed module enumeration by using module@address
>> (this violates the EPAPR device tree spec).
>> * added __initconst to names array
>>
>> [1] http://lists.xen.org/archives/html/xen-devel/2013-09/msg00083.html
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>> xen/common/device_tree.c | 44 +++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 37 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>> index eed77ce..3ae593f 100644
>> --- a/xen/common/device_tree.c
>> +++ b/xen/common/device_tree.c
>> @@ -433,22 +433,50 @@ static void __init process_cpu_node(const void *fdt, int node,
>> cpumask_set_cpu(start, &cpu_possible_map);
>> }
>>
>> +static const char * const __initconst kernel_module_names[] = {
>> + "xen,linux-zimage",
>> + "xen,dom0-kernel",
>> + "boot,kernel",
>
> I'm wondering about this..
>
> The current "xen,linux-zimage" node does more than simply identifying
> the location in memory of the kernel, it actually tells us that the boot
> protocol used by that kernel is the Linux zImage protocol.
>
> For "xen,dom0-kernel" and "boot,kernel" we don't get that -- so how do
> we know how the kernel in question wants to be called?
>
> I guess there is a magic number in the kernel itself, so maybe this is
> OK? Any futyre kernel (I'm thinking *BSD here...) would either need to
> be identifiable by magic number or use only a specific compatible value
> which indicates this...
>
> OK, I think I've convinced myself, I'll leave the commentary above in
> case you can spot a flaw in my reasoning.
>
>> + NULL
>> +};
>> +
>> +static const char * const __initconst initrd_module_names[] = {
>> + "xen,linux-initrd",
>> + "xen,ramdisk",
>> + "boot,ramdisk",
>> + NULL
>> +};
>> +
>> static void __init process_multiboot_node(const void *fdt, int node,
>> const char *name,
>> u32 address_cells, u32 size_cells)
>> {
>> const struct fdt_property *prop;
>> const u32 *cell;
>> - int nr;
>> + int nr = -1;
>> struct dt_mb_module *mod;
>> int len;
>> + const char* const * name_list;
>>
>> - if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 )
>> - nr = MOD_KERNEL;
>> - else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
>> - nr = MOD_INITRD;
>> - else
>> - early_panic("%s not a known xen multiboot type\n", name);
>> + for ( name_list = kernel_module_names; *name_list != NULL; name_list++ )
>> + if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 )
>
> I've just pushed Julien's big DTB series which adds a helper for
> matching against a list of compatible nodes.
The function only works with the device tree structure not the FDT.
>
>> + {
>> + nr = MOD_KERNEL;
>> + break;
>> + }
>> +
>> + for ( name_list = initrd_module_names; *name_list != NULL; name_list++ )
>> + if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 )
>> + {
>> + nr = MOD_INITRD;
>> + break;
>> + }
>> +
>> + if ( nr == -1 )
>> + {
>> + early_printk("warning: %s not a known module type, ignoring\n", name);
>> + return;
>> + }
>>
>> mod = &early_info.modules.module[nr];
>>
>> @@ -486,6 +514,8 @@ static int __init early_scan_node(const void *fdt,
>> process_cpu_node(fdt, node, name, address_cells, size_cells);
>> else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) )
>
> Should we mark this as deprecated in the docs?
>
> Speaking of which, this patch doesn't seem to ouch the docs tree ;-)
>
> I know the doc should live somewhere else, but lets try and keep it up
> to date until we move it...
>
>> process_multiboot_node(fdt, node, name, address_cells, size_cells);
>> + else if ( device_tree_node_compatible(fdt, node, "boot,module" ) )
>> + process_multiboot_node(fdt, node, name, address_cells, size_cells);
>>
>> return 0;
>> }
>
>
--
Julien Grall
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM/multiboot: use more flexible node naming
2013-09-17 15:06 ` Julien Grall
@ 2013-09-17 15:09 ` Ian Campbell
0 siblings, 0 replies; 7+ messages in thread
From: Ian Campbell @ 2013-09-17 15:09 UTC (permalink / raw)
To: Julien Grall; +Cc: stefano.stabellini, Andre Przywara, xen-devel
On Tue, 2013-09-17 at 16:06 +0100, Julien Grall wrote:
> > I've just pushed Julien's big DTB series which adds a helper for
> > matching against a list of compatible nodes.
>
> The function only works with the device tree structure not the FDT.
Oh right, I forgot this was early parsing..
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM/multiboot: use more flexible node naming
2013-09-17 14:53 ` Ian Campbell
2013-09-17 15:06 ` Julien Grall
@ 2013-09-17 21:12 ` Andre Przywara
2013-09-17 22:12 ` Ian Campbell
1 sibling, 1 reply; 7+ messages in thread
From: Andre Przywara @ 2013-09-17 21:12 UTC (permalink / raw)
To: Ian Campbell; +Cc: julien.grall, stefano.stabellini, xen-devel
On 09/17/2013 04:53 PM, Ian Campbell wrote:
> On Wed, 2013-09-11 at 16:06 +0200, Andre Przywara wrote:
>> For the current "multiboot" on ARM support we look for a compatible
>> string of "xen,multiboot-module" in the device tree, and then
>> use "xen,linux-zimage" and "xen,linux-initrd" to differentiate
>> between the two supported module types.
>> To meet the more generic multiboot proposal in the device tree [1],
>> allow Xen to be more flexible in the compatible naming and also use
>> the new generic base name "boot,module".
>> The mapping to either Dom0 kernel or RAM disk works by providing a
>> more specific name ("xen,dom0-kernel" and "xen,ramdisk", preferably).
>> For compatibility reasons the older names above are still recognized.
>>
>> Changes from v1:
>> * whitespace / coding style fixes (sorry for that mess!)
>> * removed module enumeration by using module@address
>> (this violates the EPAPR device tree spec).
>> * added __initconst to names array
>>
>> [1] http://lists.xen.org/archives/html/xen-devel/2013-09/msg00083.html
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>> xen/common/device_tree.c | 44 +++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 37 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>> index eed77ce..3ae593f 100644
>> --- a/xen/common/device_tree.c
>> +++ b/xen/common/device_tree.c
>> @@ -433,22 +433,50 @@ static void __init process_cpu_node(const void *fdt, int node,
>> cpumask_set_cpu(start, &cpu_possible_map);
>> }
>>
>> +static const char * const __initconst kernel_module_names[] = {
>> + "xen,linux-zimage",
>> + "xen,dom0-kernel",
>> + "boot,kernel",
>
> I'm wondering about this..
>
> The current "xen,linux-zimage" node does more than simply identifying
> the location in memory of the kernel, it actually tells us that the boot
> protocol used by that kernel is the Linux zImage protocol.
>
> For "xen,dom0-kernel" and "boot,kernel" we don't get that -- so how do
> we know how the kernel in question wants to be called?
Good point. Yes, we could fix this by looking for magics (ARM zImage has
one at 0x24, along with start and end address).
Hopefully BSD has something similar or we have to use it the lack of a
magic to detect those kernels.
How do we solve this on x86? Is that because the Dom0 kernel is PV there
that we know how to boot this? After all Xen just sees a multiboot
module, which is actually nothing more like a binary blob in that protocol.
Actually I modeled this along the x86 scheme: Multiboot provides no
further information about the image other than an order (first is
kernel, second is initrd). So as this order thing does not work in DT, I
wanted to use compatible strings to at least determine the type of binary.
> I guess there is a magic number in the kernel itself, so maybe this is
> OK? Any futyre kernel (I'm thinking *BSD here...) would either need to
> be identifiable by magic number or use only a specific compatible value
> which indicates this...
Right, my idea was just to do that: use specific compatible values
whenever possible or hope for support in Xen to detect the right way of
booting. So the early mapping code could just live with boot,dom0-kernel
or even boot,module without knowing anything about a specific boot
protocol at all. Later on when we actually launch the kernel, we use
more specific values to pick the right protocol.
Kind of moot discussion probably until we actually have a "second"
kernel for real.
> OK, I think I've convinced myself, I'll leave the commentary above in
> case you can spot a flaw in my reasoning.
>
>> + NULL
>> +};
>> +
>> +static const char * const __initconst initrd_module_names[] = {
>> + "xen,linux-initrd",
>> + "xen,ramdisk",
>> + "boot,ramdisk",
>> + NULL
>> +};
>> +
>> static void __init process_multiboot_node(const void *fdt, int node,
>> const char *name,
>> u32 address_cells, u32 size_cells)
>> {
>> const struct fdt_property *prop;
>> const u32 *cell;
>> - int nr;
>> + int nr = -1;
>> struct dt_mb_module *mod;
>> int len;
>> + const char* const * name_list;
>>
>> - if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 )
>> - nr = MOD_KERNEL;
>> - else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
>> - nr = MOD_INITRD;
>> - else
>> - early_panic("%s not a known xen multiboot type\n", name);
>> + for ( name_list = kernel_module_names; *name_list != NULL; name_list++ )
>> + if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 )
>
> I've just pushed Julien's big DTB series which adds a helper for
> matching against a list of compatible nodes.
>
>> + {
>> + nr = MOD_KERNEL;
>> + break;
>> + }
>> +
>> + for ( name_list = initrd_module_names; *name_list != NULL; name_list++ )
>> + if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 )
>> + {
>> + nr = MOD_INITRD;
>> + break;
>> + }
>> +
>> + if ( nr == -1 )
>> + {
>> + early_printk("warning: %s not a known module type, ignoring\n", name);
>> + return;
>> + }
>>
>> mod = &early_info.modules.module[nr];
>>
>> @@ -486,6 +514,8 @@ static int __init early_scan_node(const void *fdt,
>> process_cpu_node(fdt, node, name, address_cells, size_cells);
>> else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) )
>
> Should we mark this as deprecated in the docs?
>
> Speaking of which, this patch doesn't seem to ouch the docs tree ;-)
Actually I thought people would slap me earlier for that ;-)
I wanted to wait until we settle with this general device tree booting
proposal thing before writing the docs. But on the other hand this is
just an excuse, so I will respin the patch...
Regards,
Andre.
>
> I know the doc should live somewhere else, but lets try and keep it up
> to date until we move it...
>
>> process_multiboot_node(fdt, node, name, address_cells, size_cells);
>> + else if ( device_tree_node_compatible(fdt, node, "boot,module" ) )
>> + process_multiboot_node(fdt, node, name, address_cells, size_cells);
>>
>> return 0;
>> }
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM/multiboot: use more flexible node naming
2013-09-17 21:12 ` Andre Przywara
@ 2013-09-17 22:12 ` Ian Campbell
2013-09-18 9:36 ` Egger, Christoph
0 siblings, 1 reply; 7+ messages in thread
From: Ian Campbell @ 2013-09-17 22:12 UTC (permalink / raw)
To: Andre Przywara; +Cc: julien.grall, stefano.stabellini, xen-devel
On Tue, 2013-09-17 at 23:12 +0200, Andre Przywara wrote:
> On 09/17/2013 04:53 PM, Ian Campbell wrote:
> > On Wed, 2013-09-11 at 16:06 +0200, Andre Przywara wrote:
> >> For the current "multiboot" on ARM support we look for a compatible
> >> string of "xen,multiboot-module" in the device tree, and then
> >> use "xen,linux-zimage" and "xen,linux-initrd" to differentiate
> >> between the two supported module types.
> >> To meet the more generic multiboot proposal in the device tree [1],
> >> allow Xen to be more flexible in the compatible naming and also use
> >> the new generic base name "boot,module".
> >> The mapping to either Dom0 kernel or RAM disk works by providing a
> >> more specific name ("xen,dom0-kernel" and "xen,ramdisk", preferably).
> >> For compatibility reasons the older names above are still recognized.
> >>
> >> Changes from v1:
> >> * whitespace / coding style fixes (sorry for that mess!)
> >> * removed module enumeration by using module@address
> >> (this violates the EPAPR device tree spec).
> >> * added __initconst to names array
> >>
> >> [1] http://lists.xen.org/archives/html/xen-devel/2013-09/msg00083.html
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> >> ---
> >> xen/common/device_tree.c | 44 +++++++++++++++++++++++++++++++++++++-------
> >> 1 file changed, 37 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> >> index eed77ce..3ae593f 100644
> >> --- a/xen/common/device_tree.c
> >> +++ b/xen/common/device_tree.c
> >> @@ -433,22 +433,50 @@ static void __init process_cpu_node(const void *fdt, int node,
> >> cpumask_set_cpu(start, &cpu_possible_map);
> >> }
> >>
> >> +static const char * const __initconst kernel_module_names[] = {
> >> + "xen,linux-zimage",
> >> + "xen,dom0-kernel",
> >> + "boot,kernel",
> >
> > I'm wondering about this..
> >
> > The current "xen,linux-zimage" node does more than simply identifying
> > the location in memory of the kernel, it actually tells us that the boot
> > protocol used by that kernel is the Linux zImage protocol.
> >
> > For "xen,dom0-kernel" and "boot,kernel" we don't get that -- so how do
> > we know how the kernel in question wants to be called?
>
> Good point. Yes, we could fix this by looking for magics (ARM zImage has
> one at 0x24, along with start and end address).
Yes, we already do that thankfully.
> Hopefully BSD has something similar or we have to use it the lack of a
> magic to detect those kernels.
I'm not sure, they may just be normal ELF kernels?
> How do we solve this on x86? Is that because the Dom0 kernel is PV there
> that we know how to boot this?
Essentially yes. dom0 and domU kernels are an ELF file with a set of Xen
specific notes which tell us things like the virtual base address etc so
we can build the initial PTs etc
The x86 bzImage is actually just a wrapper around that (compressed) ELF
file, so we have some code which knows about that and can extract the
ELF.
The x86 kernels havea separate Xen entry point which we enter in 32-bit
mode so there's no need for real mode etc.
> After all Xen just sees a multiboot
> module, which is actually nothing more like a binary blob in that protocol.
> Actually I modeled this along the x86 scheme: Multiboot provides no
> further information about the image other than an order (first is
> kernel, second is initrd). So as this order thing does not work in DT, I
> wanted to use compatible strings to at least determine the type of binary.
Makes sense.
> > I guess there is a magic number in the kernel itself, so maybe this is
> > OK? Any futyre kernel (I'm thinking *BSD here...) would either need to
> > be identifiable by magic number or use only a specific compatible value
> > which indicates this...
>
> Right, my idea was just to do that: use specific compatible values
> whenever possible or hope for support in Xen to detect the right way of
> booting. So the early mapping code could just live with boot,dom0-kernel
> or even boot,module without knowing anything about a specific boot
> protocol at all. Later on when we actually launch the kernel, we use
> more specific values to pick the right protocol.
Right, that makes sense.
> Kind of moot discussion probably until we actually have a "second"
> kernel for real.
True. I think I've convinced myself we aren't painting ourselves into a
corner...
> > Should we mark this as deprecated in the docs?
> >
> > Speaking of which, this patch doesn't seem to ouch the docs tree ;-)
>
> Actually I thought people would slap me earlier for that ;-)
I must have been napping ;-)
> I wanted to wait until we settle with this general device tree booting
> proposal thing before writing the docs. But on the other hand this is
> just an excuse, so I will respin the patch...
Thanks!
Ian.
>
> Regards,
> Andre.
>
> >
> > I know the doc should live somewhere else, but lets try and keep it up
> > to date until we move it...
> >
> >> process_multiboot_node(fdt, node, name, address_cells, size_cells);
> >> + else if ( device_tree_node_compatible(fdt, node, "boot,module" ) )
> >> + process_multiboot_node(fdt, node, name, address_cells, size_cells);
> >>
> >> return 0;
> >> }
> >
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM/multiboot: use more flexible node naming
2013-09-17 22:12 ` Ian Campbell
@ 2013-09-18 9:36 ` Egger, Christoph
0 siblings, 0 replies; 7+ messages in thread
From: Egger, Christoph @ 2013-09-18 9:36 UTC (permalink / raw)
To: Ian Campbell; +Cc: julien.grall, xen-devel, Andre Przywara, stefano.stabellini
On 18.09.13 00:12, Ian Campbell wrote:
> On Tue, 2013-09-17 at 23:12 +0200, Andre Przywara wrote:
>> On 09/17/2013 04:53 PM, Ian Campbell wrote:
>>> On Wed, 2013-09-11 at 16:06 +0200, Andre Przywara wrote:
>>>> For the current "multiboot" on ARM support we look for a compatible
>>>> string of "xen,multiboot-module" in the device tree, and then
>>>> use "xen,linux-zimage" and "xen,linux-initrd" to differentiate
>>>> between the two supported module types.
>>>> To meet the more generic multiboot proposal in the device tree [1],
>>>> allow Xen to be more flexible in the compatible naming and also use
>>>> the new generic base name "boot,module".
>>>> The mapping to either Dom0 kernel or RAM disk works by providing a
>>>> more specific name ("xen,dom0-kernel" and "xen,ramdisk", preferably).
>>>> For compatibility reasons the older names above are still recognized.
>>>>
>>>> Changes from v1:
>>>> * whitespace / coding style fixes (sorry for that mess!)
>>>> * removed module enumeration by using module@address
>>>> (this violates the EPAPR device tree spec).
>>>> * added __initconst to names array
>>>>
>>>> [1] http://lists.xen.org/archives/html/xen-devel/2013-09/msg00083.html
>>>>
>>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>>> ---
>>>> xen/common/device_tree.c | 44 +++++++++++++++++++++++++++++++++++++-------
>>>> 1 file changed, 37 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>>>> index eed77ce..3ae593f 100644
>>>> --- a/xen/common/device_tree.c
>>>> +++ b/xen/common/device_tree.c
>>>> @@ -433,22 +433,50 @@ static void __init process_cpu_node(const void *fdt, int node,
>>>> cpumask_set_cpu(start, &cpu_possible_map);
>>>> }
>>>>
>>>> +static const char * const __initconst kernel_module_names[] = {
>>>> + "xen,linux-zimage",
>>>> + "xen,dom0-kernel",
>>>> + "boot,kernel",
>>>
>>> I'm wondering about this..
>>>
>>> The current "xen,linux-zimage" node does more than simply identifying
>>> the location in memory of the kernel, it actually tells us that the boot
>>> protocol used by that kernel is the Linux zImage protocol.
>>>
>>> For "xen,dom0-kernel" and "boot,kernel" we don't get that -- so how do
>>> we know how the kernel in question wants to be called?
>>
>> Good point. Yes, we could fix this by looking for magics (ARM zImage has
>> one at 0x24, along with start and end address).
>
> Yes, we already do that thankfully.
>
>> Hopefully BSD has something similar or we have to use it the lack of a
>> magic to detect those kernels.
>
> I'm not sure, they may just be normal ELF kernels?
Right. BSD has nothing like zImage or bzImage. The kernels are normal
ELF binaries. You can use gdb on them, for example which you cannot do
with zImage or bzImage.
Christoph
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-09-18 9:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-11 14:06 [PATCH v2] ARM/multiboot: use more flexible node naming Andre Przywara
2013-09-17 14:53 ` Ian Campbell
2013-09-17 15:06 ` Julien Grall
2013-09-17 15:09 ` Ian Campbell
2013-09-17 21:12 ` Andre Przywara
2013-09-17 22:12 ` Ian Campbell
2013-09-18 9:36 ` Egger, Christoph
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).