[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] ARM/multiboot: use more flexible node naming



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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.