* [PATCH] ARM/multiboot: use more flexible node naming
@ 2013-09-05 13:43 Andre Przywara
2013-09-05 13:54 ` Egger, Christoph
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Andre Przywara @ 2013-09-05 13:43 UTC (permalink / raw)
To: Ian.Campbell, julien.grall, xen-devel; +Cc: stefano.stabellini
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 either by
providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
or by using the enumeration order of the device tree nodes
(module@0 = kernel, module@1 = initrd). This allows bootloaders
without any specific Xen knowledge to boot Xen anyways.
[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 | 57 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 50 insertions(+), 7 deletions(-)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index ec0d5e2..e10c035 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -439,22 +439,63 @@ static void __init process_cpu_node(const void *fdt, int node,
cpumask_set_cpu(start, &cpu_possible_map);
}
+static const char * const kernel_module_names[] = {
+ "xen,linux-zimage",
+ "xen,dom0-kernel",
+ "boot,kernel",
+ NULL
+};
+
+static const char * const 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) {
+ char *s;
+
+ if ( fdt_node_check_compatible(fdt, node, "boot,module") != 0 ) {
+ early_panic("%s not a known xen multiboot type\n", name);
+ return;
+ }
+ s = strchr(name, '@');
+ if (s == NULL)
+ nr = early_info.modules.nr_mods + 1;
+ else {
+ nr = simple_strtoll(s + 1, NULL, 10) + 1;
+ }
+ }
+
+ if (nr >= NR_MODULES) {
+ early_panic("only supporting %d multiboot modules\n",
+ NR_MODULES - MOD_DISCARD_FIRST);
+ return;
+ }
mod = &early_info.modules.module[nr];
@@ -492,6 +533,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] 10+ messages in thread
* Re: [PATCH] ARM/multiboot: use more flexible node naming
2013-09-05 13:43 [PATCH] ARM/multiboot: use more flexible node naming Andre Przywara
@ 2013-09-05 13:54 ` Egger, Christoph
2013-09-05 14:03 ` Ian Campbell
2013-09-06 11:11 ` Julien Grall
2013-09-09 12:00 ` Ian Campbell
2 siblings, 1 reply; 10+ messages in thread
From: Egger, Christoph @ 2013-09-05 13:54 UTC (permalink / raw)
To: Andre Przywara; +Cc: julien.grall, Ian.Campbell, stefano.stabellini, xen-devel
Is there a chance to get NetBSD Dom0 [1] to boot to justify that your
approach is *really* generic.
xen,linux-zimage and xen,linux-initrd do not sound generic
but xen,dom0-kernel and xen,ramdisk do sound generic at least.
Christoph
[1] NetBSD Dom0 hasn't been ported to ARM yet.
On 05.09.13 15:43, 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 either by
> providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
> or by using the enumeration order of the device tree nodes
> (module@0 = kernel, module@1 = initrd). This allows bootloaders
> without any specific Xen knowledge to boot Xen anyways.
>
> [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 | 57 ++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 50 insertions(+), 7 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index ec0d5e2..e10c035 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -439,22 +439,63 @@ static void __init process_cpu_node(const void *fdt, int node,
> cpumask_set_cpu(start, &cpu_possible_map);
> }
>
> +static const char * const kernel_module_names[] = {
> + "xen,linux-zimage",
> + "xen,dom0-kernel",
> + "boot,kernel",
> + NULL
> +};
> +
> +static const char * const 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) {
> + char *s;
> +
> + if ( fdt_node_check_compatible(fdt, node, "boot,module") != 0 ) {
> + early_panic("%s not a known xen multiboot type\n", name);
> + return;
> + }
> + s = strchr(name, '@');
> + if (s == NULL)
> + nr = early_info.modules.nr_mods + 1;
> + else {
> + nr = simple_strtoll(s + 1, NULL, 10) + 1;
> + }
> + }
> +
> + if (nr >= NR_MODULES) {
> + early_panic("only supporting %d multiboot modules\n",
> + NR_MODULES - MOD_DISCARD_FIRST);
> + return;
> + }
>
> mod = &early_info.modules.module[nr];
>
> @@ -492,6 +533,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;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ARM/multiboot: use more flexible node naming
2013-09-05 13:54 ` Egger, Christoph
@ 2013-09-05 14:03 ` Ian Campbell
2013-09-05 14:22 ` Egger, Christoph
0 siblings, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2013-09-05 14:03 UTC (permalink / raw)
To: Egger, Christoph
Cc: stefano.stabellini, julien.grall, Andre Przywara, xen-devel
On Thu, 2013-09-05 at 15:54 +0200, Egger, Christoph wrote:
> Is there a chance to get NetBSD Dom0 [1] to boot to justify that your
> approach is *really* generic.
>
> xen,linux-zimage and xen,linux-initrd do not sound generic
> but xen,dom0-kernel and xen,ramdisk do sound generic at least.
Does NetBSD follow the Linux zImage boot protocol
(Documentation/arm/Booting.txt in a Linux source tree)?
If not then you need to define (or link to) the NetBSD boot protocol and
we probably will eventually need xen,netbsd-fooimg and suitable code in
Xen to handle that format.
> [1] NetBSD Dom0 hasn't been ported to ARM yet.
So surely you have answered your own question then, unless you are
asking Andre to port NetBSD to ARM? I don't think that is a reasonable
prerequisite for this patch.
>
>
> On 05.09.13 15:43, 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 either by
> > providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
> > or by using the enumeration order of the device tree nodes
> > (module@0 = kernel, module@1 = initrd). This allows bootloaders
> > without any specific Xen knowledge to boot Xen anyways.
> >
> > [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 | 57 ++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 50 insertions(+), 7 deletions(-)
> >
> > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> > index ec0d5e2..e10c035 100644
> > --- a/xen/common/device_tree.c
> > +++ b/xen/common/device_tree.c
> > @@ -439,22 +439,63 @@ static void __init process_cpu_node(const void *fdt, int node,
> > cpumask_set_cpu(start, &cpu_possible_map);
> > }
> >
> > +static const char * const kernel_module_names[] = {
> > + "xen,linux-zimage",
> > + "xen,dom0-kernel",
> > + "boot,kernel",
> > + NULL
> > +};
> > +
> > +static const char * const 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) {
> > + char *s;
> > +
> > + if ( fdt_node_check_compatible(fdt, node, "boot,module") != 0 ) {
> > + early_panic("%s not a known xen multiboot type\n", name);
> > + return;
> > + }
> > + s = strchr(name, '@');
> > + if (s == NULL)
> > + nr = early_info.modules.nr_mods + 1;
> > + else {
> > + nr = simple_strtoll(s + 1, NULL, 10) + 1;
> > + }
> > + }
> > +
> > + if (nr >= NR_MODULES) {
> > + early_panic("only supporting %d multiboot modules\n",
> > + NR_MODULES - MOD_DISCARD_FIRST);
> > + return;
> > + }
> >
> > mod = &early_info.modules.module[nr];
> >
> > @@ -492,6 +533,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;
> > }
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ARM/multiboot: use more flexible node naming
2013-09-05 14:03 ` Ian Campbell
@ 2013-09-05 14:22 ` Egger, Christoph
2013-09-05 14:59 ` Ian Campbell
0 siblings, 1 reply; 10+ messages in thread
From: Egger, Christoph @ 2013-09-05 14:22 UTC (permalink / raw)
To: Ian Campbell; +Cc: stefano.stabellini, julien.grall, Andre Przywara, xen-devel
On 05.09.13 16:03, Ian Campbell wrote:
> On Thu, 2013-09-05 at 15:54 +0200, Egger, Christoph wrote:
>> Is there a chance to get NetBSD Dom0 [1] to boot to justify that your
>> approach is *really* generic.
>>
>> xen,linux-zimage and xen,linux-initrd do not sound generic
>> but xen,dom0-kernel and xen,ramdisk do sound generic at least.
>
> Does NetBSD follow the Linux zImage boot protocol
> (Documentation/arm/Booting.txt in a Linux source tree)?
>
> If not then you need to define (or link to) the NetBSD boot protocol and
> we probably will eventually need xen,netbsd-fooimg and suitable code in
> Xen to handle that format.
>
>> [1] NetBSD Dom0 hasn't been ported to ARM yet.
>
> So surely you have answered your own question then, unless you are
> asking Andre to port NetBSD to ARM? I don't think that is a reasonable
> prerequisite for this patch.
No, it is not a prerequisite. My question is related to the design
if it is "I allow Linux only to boot" or "I allow any Dom0 to boot".
Christoph
>
>>
>>
>> On 05.09.13 15:43, 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 either by
>>> providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
>>> or by using the enumeration order of the device tree nodes
>>> (module@0 = kernel, module@1 = initrd). This allows bootloaders
>>> without any specific Xen knowledge to boot Xen anyways.
>>>
>>> [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 | 57 ++++++++++++++++++++++++++++++++++++++++++------
>>> 1 file changed, 50 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>>> index ec0d5e2..e10c035 100644
>>> --- a/xen/common/device_tree.c
>>> +++ b/xen/common/device_tree.c
>>> @@ -439,22 +439,63 @@ static void __init process_cpu_node(const void *fdt, int node,
>>> cpumask_set_cpu(start, &cpu_possible_map);
>>> }
>>>
>>> +static const char * const kernel_module_names[] = {
>>> + "xen,linux-zimage",
>>> + "xen,dom0-kernel",
>>> + "boot,kernel",
>>> + NULL
>>> +};
>>> +
>>> +static const char * const 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) {
>>> + char *s;
>>> +
>>> + if ( fdt_node_check_compatible(fdt, node, "boot,module") != 0 ) {
>>> + early_panic("%s not a known xen multiboot type\n", name);
>>> + return;
>>> + }
>>> + s = strchr(name, '@');
>>> + if (s == NULL)
>>> + nr = early_info.modules.nr_mods + 1;
>>> + else {
>>> + nr = simple_strtoll(s + 1, NULL, 10) + 1;
>>> + }
>>> + }
>>> +
>>> + if (nr >= NR_MODULES) {
>>> + early_panic("only supporting %d multiboot modules\n",
>>> + NR_MODULES - MOD_DISCARD_FIRST);
>>> + return;
>>> + }
>>>
>>> mod = &early_info.modules.module[nr];
>>>
>>> @@ -492,6 +533,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;
>>> }
>>>
>>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ARM/multiboot: use more flexible node naming
2013-09-05 14:22 ` Egger, Christoph
@ 2013-09-05 14:59 ` Ian Campbell
2013-09-06 8:44 ` Andre Przywara
0 siblings, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2013-09-05 14:59 UTC (permalink / raw)
To: Egger, Christoph
Cc: stefano.stabellini, julien.grall, Andre Przywara, xen-devel
On Thu, 2013-09-05 at 16:22 +0200, Egger, Christoph wrote:
> On 05.09.13 16:03, Ian Campbell wrote:
> > On Thu, 2013-09-05 at 15:54 +0200, Egger, Christoph wrote:
> >> Is there a chance to get NetBSD Dom0 [1] to boot to justify that your
> >> approach is *really* generic.
> >>
> >> xen,linux-zimage and xen,linux-initrd do not sound generic
> >> but xen,dom0-kernel and xen,ramdisk do sound generic at least.
> >
> > Does NetBSD follow the Linux zImage boot protocol
> > (Documentation/arm/Booting.txt in a Linux source tree)?
> >
> > If not then you need to define (or link to) the NetBSD boot protocol and
> > we probably will eventually need xen,netbsd-fooimg and suitable code in
> > Xen to handle that format.
> >
> >> [1] NetBSD Dom0 hasn't been ported to ARM yet.
> >
> > So surely you have answered your own question then, unless you are
> > asking Andre to port NetBSD to ARM? I don't think that is a reasonable
> > prerequisite for this patch.
>
> No, it is not a prerequisite. My question is related to the design
> if it is "I allow Linux only to boot" or "I allow any Dom0 to boot".
The design allows for any dom0, since it includes a compatibility string
which corresponds to the boot protocol to use. The implementation
currently only covers Linux though.
> Christoph
>
> >
> >>
> >>
> >> On 05.09.13 15:43, 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 either by
> >>> providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
> >>> or by using the enumeration order of the device tree nodes
> >>> (module@0 = kernel, module@1 = initrd). This allows bootloaders
> >>> without any specific Xen knowledge to boot Xen anyways.
> >>>
> >>> [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 | 57 ++++++++++++++++++++++++++++++++++++++++++------
> >>> 1 file changed, 50 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> >>> index ec0d5e2..e10c035 100644
> >>> --- a/xen/common/device_tree.c
> >>> +++ b/xen/common/device_tree.c
> >>> @@ -439,22 +439,63 @@ static void __init process_cpu_node(const void *fdt, int node,
> >>> cpumask_set_cpu(start, &cpu_possible_map);
> >>> }
> >>>
> >>> +static const char * const kernel_module_names[] = {
> >>> + "xen,linux-zimage",
> >>> + "xen,dom0-kernel",
> >>> + "boot,kernel",
> >>> + NULL
> >>> +};
> >>> +
> >>> +static const char * const 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) {
> >>> + char *s;
> >>> +
> >>> + if ( fdt_node_check_compatible(fdt, node, "boot,module") != 0 ) {
> >>> + early_panic("%s not a known xen multiboot type\n", name);
> >>> + return;
> >>> + }
> >>> + s = strchr(name, '@');
> >>> + if (s == NULL)
> >>> + nr = early_info.modules.nr_mods + 1;
> >>> + else {
> >>> + nr = simple_strtoll(s + 1, NULL, 10) + 1;
> >>> + }
> >>> + }
> >>> +
> >>> + if (nr >= NR_MODULES) {
> >>> + early_panic("only supporting %d multiboot modules\n",
> >>> + NR_MODULES - MOD_DISCARD_FIRST);
> >>> + return;
> >>> + }
> >>>
> >>> mod = &early_info.modules.module[nr];
> >>>
> >>> @@ -492,6 +533,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;
> >>> }
> >>>
> >>
> >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ARM/multiboot: use more flexible node naming
2013-09-05 14:59 ` Ian Campbell
@ 2013-09-06 8:44 ` Andre Przywara
0 siblings, 0 replies; 10+ messages in thread
From: Andre Przywara @ 2013-09-06 8:44 UTC (permalink / raw)
To: Ian Campbell
Cc: stefano.stabellini, Egger, Christoph, julien.grall, xen-devel
On 09/05/2013 04:59 PM, Ian Campbell wrote:
> On Thu, 2013-09-05 at 16:22 +0200, Egger, Christoph wrote:
>> On 05.09.13 16:03, Ian Campbell wrote:
>>> On Thu, 2013-09-05 at 15:54 +0200, Egger, Christoph wrote:
>>>> Is there a chance to get NetBSD Dom0 [1] to boot to justify that your
>>>> approach is *really* generic.
>>>>
>>>> xen,linux-zimage and xen,linux-initrd do not sound generic
>>>> but xen,dom0-kernel and xen,ramdisk do sound generic at least.
>>>
>>> Does NetBSD follow the Linux zImage boot protocol
>>> (Documentation/arm/Booting.txt in a Linux source tree)?
>>>
>>> If not then you need to define (or link to) the NetBSD boot protocol and
>>> we probably will eventually need xen,netbsd-fooimg and suitable code in
>>> Xen to handle that format.
>>>
>>>> [1] NetBSD Dom0 hasn't been ported to ARM yet.
>>>
>>> So surely you have answered your own question then, unless you are
>>> asking Andre to port NetBSD to ARM? I don't think that is a reasonable
>>> prerequisite for this patch.
>>
>> No, it is not a prerequisite. My question is related to the design
>> if it is "I allow Linux only to boot" or "I allow any Dom0 to boot".
>
> The design allows for any dom0, since it includes a compatibility string
> which corresponds to the boot protocol to use. The implementation
> currently only covers Linux though.
I think the whole discussion is somewhat moot here, as NetBSD does not
seem to support device trees. I see FreeBSD has support for it, but with
some quick googling I cannot find anything in NetBSD.
Regards,
Andre.
>>>>
>>>> On 05.09.13 15:43, 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 either by
>>>>> providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
>>>>> or by using the enumeration order of the device tree nodes
>>>>> (module@0 = kernel, module@1 = initrd). This allows bootloaders
>>>>> without any specific Xen knowledge to boot Xen anyways.
>>>>>
>>>>> [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 | 57 ++++++++++++++++++++++++++++++++++++++++++------
>>>>> 1 file changed, 50 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>>>>> index ec0d5e2..e10c035 100644
>>>>> --- a/xen/common/device_tree.c
>>>>> +++ b/xen/common/device_tree.c
>>>>> @@ -439,22 +439,63 @@ static void __init process_cpu_node(const void *fdt, int node,
>>>>> cpumask_set_cpu(start, &cpu_possible_map);
>>>>> }
>>>>>
>>>>> +static const char * const kernel_module_names[] = {
>>>>> + "xen,linux-zimage",
>>>>> + "xen,dom0-kernel",
>>>>> + "boot,kernel",
>>>>> + NULL
>>>>> +};
>>>>> +
>>>>> +static const char * const 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) {
>>>>> + char *s;
>>>>> +
>>>>> + if ( fdt_node_check_compatible(fdt, node, "boot,module") != 0 ) {
>>>>> + early_panic("%s not a known xen multiboot type\n", name);
>>>>> + return;
>>>>> + }
>>>>> + s = strchr(name, '@');
>>>>> + if (s == NULL)
>>>>> + nr = early_info.modules.nr_mods + 1;
>>>>> + else {
>>>>> + nr = simple_strtoll(s + 1, NULL, 10) + 1;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + if (nr >= NR_MODULES) {
>>>>> + early_panic("only supporting %d multiboot modules\n",
>>>>> + NR_MODULES - MOD_DISCARD_FIRST);
>>>>> + return;
>>>>> + }
>>>>>
>>>>> mod = &early_info.modules.module[nr];
>>>>>
>>>>> @@ -492,6 +533,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;
>>>>> }
>>>>>
>>>>
>>>
>>>
>>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ARM/multiboot: use more flexible node naming
2013-09-05 13:43 [PATCH] ARM/multiboot: use more flexible node naming Andre Przywara
2013-09-05 13:54 ` Egger, Christoph
@ 2013-09-06 11:11 ` Julien Grall
2013-09-09 13:52 ` Andre Przywara
2013-09-09 12:00 ` Ian Campbell
2 siblings, 1 reply; 10+ messages in thread
From: Julien Grall @ 2013-09-06 11:11 UTC (permalink / raw)
To: Andre Przywara; +Cc: stefano.stabellini, Ian.Campbell, xen-devel
Hi,
I have also modified this function with my patch series "Allow Xen to
boot with a raw Device Tree". Can you rebase this patch on top of this
serie?
Other comments below.
On 09/05/2013 02:43 PM, 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 either by
> providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
> or by using the enumeration order of the device tree nodes
> (module@0 = kernel, module@1 = initrd). This allows bootloaders
> without any specific Xen knowledge to boot Xen anyways.
The ePAR (section 2.2.1) requires the unit-address (name@unit-address)
to match the first address of the "reg" property. I think, with this
solution we doesn't comply to the specification.
If the bootloader is able to add "boot,kernel" and "boot,initramfs" to
the compatible property, we should have a generic solution for multiboot.
> [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 | 57 ++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 50 insertions(+), 7 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index ec0d5e2..e10c035 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -439,22 +439,63 @@ static void __init process_cpu_node(const void *fdt, int node,
> cpumask_set_cpu(start, &cpu_possible_map);
> }
>
> +static const char * const kernel_module_names[] = {
These variable are only used durint the initialization, you should add
__initconst.
> + "xen,linux-zimage",
> + "xen,dom0-kernel",
> + "boot,kernel",
> + NULL
Xen uses spaces instead of tabulations.
> +};
> +
> +static const char * const initrd_module_names[] = {
__initconst
> + "xen,linux-initrd",
> + "xen,ramdisk",
> + "boot,ramdisk",
> + NULL
tabulations
> +};
> +
> 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++)
the coding style for "for" is for ( ... )
> + if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 ) {
newline before {
> + nr = MOD_KERNEL;
> + break;
> + }
> +
> + for (name_list = initrd_module_names; *name_list != NULL; name_list++)
for ( ... )
> + if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 ) {
newline before {
> + nr = MOD_INITRD;
> + break;
> + }
> +
> + if (nr == -1) {
newline before {
> + char *s;
there is a tabulation here.
> +
> + if ( fdt_node_check_compatible(fdt, node, "boot,module") != 0 ) {
newline before {
> + early_panic("%s not a known xen multiboot type\n", name);
early_panic is a bit tough, can we simply ignore the node?
> + return;
> + }
> + s = strchr(name, '@');
> + if (s == NULL)
if ( ... )
> + nr = early_info.modules.nr_mods + 1;
> + else {
newline before {
> + nr = simple_strtoll(s + 1, NULL, 10) + 1;
> + }
> + }
> +
> + if (nr >= NR_MODULES) {
if ( ... )
You should also check nr < 0. Strtoll could return a negative value.
> + early_panic("only supporting %d multiboot modules\n",
> + NR_MODULES - MOD_DISCARD_FIRST);
indentation here.
> + return;
> + }
>
> mod = &early_info.modules.module[nr];
>
> @@ -492,6 +533,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);
Do we consider the multiboot node can live outside /chosen?
>
> return 0;
> }
>
--
Julien Grall
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ARM/multiboot: use more flexible node naming
2013-09-05 13:43 [PATCH] ARM/multiboot: use more flexible node naming Andre Przywara
2013-09-05 13:54 ` Egger, Christoph
2013-09-06 11:11 ` Julien Grall
@ 2013-09-09 12:00 ` Ian Campbell
2013-09-09 14:01 ` Andre Przywara
2 siblings, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2013-09-09 12:00 UTC (permalink / raw)
To: Andre Przywara; +Cc: julien.grall, stefano.stabellini, xen-devel
On Thu, 2013-09-05 at 15:43 +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 either by
> providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
> or by using the enumeration order of the device tree nodes
> (module@0 = kernel, module@1 = initrd). This allows bootloaders
> without any specific Xen knowledge to boot Xen anyways.
>
> [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 | 57 ++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 50 insertions(+), 7 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index ec0d5e2..e10c035 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -439,22 +439,63 @@ static void __init process_cpu_node(const void *fdt, int node,
> cpumask_set_cpu(start, &cpu_possible_map);
> }
>
> +static const char * const kernel_module_names[] = {
> + "xen,linux-zimage",
> + "xen,dom0-kernel",
> + "boot,kernel",
> + NULL
> +};
> +
> +static const char * const 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 ) {
I think Julien's big dtb series adds a helper which would make this
simpler?
That could be a later fix up, are there any other ordering constraints
with that series?
> + nr = MOD_INITRD;
> + break;
> + }
> +
> + if (nr == -1) {
> + char *s;
> +
> + if ( fdt_node_check_compatible(fdt, node, "boot,module") != 0 ) {
> + early_panic("%s not a known xen multiboot type\n", name);
> + return;
> + }
> + s = strchr(name, '@');
> + if (s == NULL)
> + nr = early_info.modules.nr_mods + 1;
> + else {
> + nr = simple_strtoll(s + 1, NULL, 10) + 1;
> + }
> + }
> +
> + if (nr >= NR_MODULES) {
> + early_panic("only supporting %d multiboot modules\n",
> + NR_MODULES - MOD_DISCARD_FIRST);
> + return;
> + }
>
> mod = &early_info.modules.module[nr];
>
> @@ -492,6 +533,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;
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ARM/multiboot: use more flexible node naming
2013-09-06 11:11 ` Julien Grall
@ 2013-09-09 13:52 ` Andre Przywara
0 siblings, 0 replies; 10+ messages in thread
From: Andre Przywara @ 2013-09-09 13:52 UTC (permalink / raw)
To: Julien Grall; +Cc: stefano.stabellini, Ian.Campbell, xen-devel
On 09/06/2013 01:11 PM, Julien Grall wrote:
> Hi,
>
> I have also modified this function with my patch series "Allow Xen to
> boot with a raw Device Tree". Can you rebase this patch on top of this
> serie?
>
> Other comments below.
>
> On 09/05/2013 02:43 PM, 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 either by
>> providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
>> or by using the enumeration order of the device tree nodes
>> (module@0 = kernel, module@1 = initrd). This allows bootloaders
>> without any specific Xen knowledge to boot Xen anyways.
>
> The ePAR (section 2.2.1) requires the unit-address (name@unit-address)
> to match the first address of the "reg" property. I think, with this
> solution we doesn't comply to the specification.
Yes, you are right. As it seems to be of no use to use the start address
to do the enumeration (in my testing I load the initrd at a lower
address for instance), I will drop this part of the patch and rely on
proper, though generic naming of the nodes instead.
>
> If the bootloader is able to add "boot,kernel" and "boot,initramfs" to
> the compatible property, we should have a generic solution for multiboot.
>
>> [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 | 57
>> ++++++++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 50 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>> index ec0d5e2..e10c035 100644
>> --- a/xen/common/device_tree.c
>> +++ b/xen/common/device_tree.c
>> @@ -439,22 +439,63 @@ static void __init process_cpu_node(const void
>> *fdt, int node,
>> cpumask_set_cpu(start, &cpu_possible_map);
>> }
>>
>> +static const char * const kernel_module_names[] = {
> These variable are only used durint the initialization, you should add
> __initconst.
>
As already discussed on IRC, it seems like __initconst is not yet
upstream and __initdata doesn't work yet, either.
So I will keep this part as it is for now with the option to fix this later.
>> + "xen,linux-zimage",
>> + "xen,dom0-kernel",
>> + "boot,kernel",
>> + NULL
>
> Xen uses spaces instead of tabulations.
Sorry for all these whitespace issues. Seems like the switch between
u-boot and Xen didn't switch the whole context ;-)
....
>
>> + early_panic("%s not a known xen multiboot type\n", name);
>
> early_panic is a bit tough, can we simply ignore the node?
In fact this was copied from the place before. Not sure if the situation
has now changed with the wider, not necessarily Xen-centric scope of the
modules.
But in fact we could print a warning about this. If there is no kernel
module specified, we will panic later anyways.
....
>> @@ -492,6 +533,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);
>
> Do we consider the multiboot node can live outside /chosen?
Did we care for the old node type? I just copied what was used before.
Maybe for bootloaders it is technically easier to put it under the root
node, so as long as it doesn't hurt us, I'd like to keep it this way.
Will send out a reworked version ASAP.
Thanks for the review,
Andre.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ARM/multiboot: use more flexible node naming
2013-09-09 12:00 ` Ian Campbell
@ 2013-09-09 14:01 ` Andre Przywara
0 siblings, 0 replies; 10+ messages in thread
From: Andre Przywara @ 2013-09-09 14:01 UTC (permalink / raw)
To: Ian Campbell; +Cc: julien.grall, stefano.stabellini, xen-devel
On 09/09/2013 02:00 PM, Ian Campbell wrote:
> On Thu, 2013-09-05 at 15:43 +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 either by
>> providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
>> or by using the enumeration order of the device tree nodes
>> (module@0 = kernel, module@1 = initrd). This allows bootloaders
>> without any specific Xen knowledge to boot Xen anyways.
>>
>> [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 | 57 ++++++++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 50 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>> index ec0d5e2..e10c035 100644
>> --- a/xen/common/device_tree.c
>> +++ b/xen/common/device_tree.c
>> @@ -439,22 +439,63 @@ static void __init process_cpu_node(const void *fdt, int node,
>> cpumask_set_cpu(start, &cpu_possible_map);
>> }
>>
>> +static const char * const kernel_module_names[] = {
>> + "xen,linux-zimage",
>> + "xen,dom0-kernel",
>> + "boot,kernel",
>> + NULL
>> +};
>> +
>> +static const char * const 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 ) {
>
> I think Julien's big dtb series adds a helper which would make this
> simpler?
>
> That could be a later fix up, are there any other ordering constraints
> with that series?
Not that I am aware of. I think I will send out my new version so far
for discussion and review and will happily rebase it later on if needed.
Thanks,
Andre.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-09-09 14:01 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-05 13:43 [PATCH] ARM/multiboot: use more flexible node naming Andre Przywara
2013-09-05 13:54 ` Egger, Christoph
2013-09-05 14:03 ` Ian Campbell
2013-09-05 14:22 ` Egger, Christoph
2013-09-05 14:59 ` Ian Campbell
2013-09-06 8:44 ` Andre Przywara
2013-09-06 11:11 ` Julien Grall
2013-09-09 13:52 ` Andre Przywara
2013-09-09 12:00 ` Ian Campbell
2013-09-09 14:01 ` Andre Przywara
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).