[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v4 07/23] xen/arm: probe domU kernels and initrds
Hi Stefano,
On 05/10/2018 19:47, Stefano Stabellini wrote:
Find addresses, sizes on device tree from kernel_probe.
Find the cmdline from the bootcmdlines array.
Introduce a new boot_module_find_by_addr_and_kind function to match not
just on boot module kind, but also by address so that we can support
multiple domains.
Introduce a boot_cmdline_find_by_name function to find the right struct
cmdline based on the device tree node name of the "xen,domain"
compatible node.
Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
---
Changes in v3:
- retrieve cmdline from bootcmdlines array
Changes in v2:
- fix indentation
- unify kernel_probe with kernel_probe_domU
- find cmdline on device_tree from kernel_probe
---
xen/arch/arm/domain_build.c | 2 +-
xen/arch/arm/kernel.c | 52 +++++++++++++++++++++++++++++++++++++++------
xen/arch/arm/kernel.h | 2 +-
xen/arch/arm/setup.c | 29 +++++++++++++++++++++++++
xen/include/asm-arm/setup.h | 3 +++
5 files changed, 79 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 64f8d6b..f073e6d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2137,7 +2137,7 @@ int __init construct_dom0(struct domain *d)
kinfo.unassigned_mem = dom0_mem;
kinfo.d = d;
- rc = kernel_probe(&kinfo);
+ rc = kernel_probe(&kinfo, NULL);
if ( rc < 0 )
return rc;
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index da8410e..e5b8213 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -421,22 +421,60 @@ static int __init kernel_zimage32_probe(struct
kernel_info *info,
return 0;
}
-int __init kernel_probe(struct kernel_info *info)
+int __init kernel_probe(struct kernel_info *info, struct dt_device_node
*domain)
{
- struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
+ struct bootmodule *mod = NULL;
+ struct bootcmdline *cmd = NULL;
+ struct dt_device_node *node;
+ u64 kernel_addr, initrd_addr, size;
+ const char *name = NULL;
Please try to limit scope of variables. For instance, name is only used
int rc;
+ if ( domain == NULL )
A line explain this is for the hardware domain would be useful.
Maybe with an ASSERT(is_hardware_domain(d)) in the if.
+ {
+ mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
+
+ info->kernel_bootmodule = mod;
+ info->initrd_bootmodule = boot_module_find_by_kind(BOOTMOD_RAMDISK);
+ } else {
Coding style:
}
else
{
+ dt_for_each_child_node(domain, node)
+ {
+ if ( dt_device_is_compatible(node, "multiboot,kernel") )
+ {
+ u32 len;
+ const __be32 *val;
Newline.
+ val = dt_get_property(node, "reg", &len);
+ dt_get_range(&val, node, &kernel_addr, &size);
Why don't you use dt_device_get_address?
+ }
+ else if ( dt_device_is_compatible(node, "multiboot,ramdisk") )
+ {
+ u32 len;
+ const __be32 *val;
Newline.
+ val = dt_get_property(node, "reg", &len);
+ dt_get_range(&val, node, &initrd_addr, &size);
Ditto.
+ }
+ else
+ continue;
+ }
+ if ( kernel_addr )
0 is a valid physical address. Please use INVALID_PADDR when checking
whether the user specified an address.
But, why don't you assign info->kernel_bootmodule when you found the
node associated to it?
+ info->kernel_bootmodule = mod = boot_module_find_by_addr_and_kind(
+ BOOTMOD_KERNEL, kernel_addr);
This code is not easy to read because of the indentation and the double
assignment. As you change half the user of mod below, then you can drop
it completely.
For the indentation, you could use a temporary variable. This would also
help to avoid using info-> everywhere below.
+ if ( initrd_addr )
+ info->initrd_bootmodule = boot_module_find_by_addr_and_kind(
+ BOOTMOD_RAMDISK, initrd_addr);
+ name = dt_node_name(domain);
+ cmd = boot_cmdline_find_by_name(name);
+ if ( cmd )
+ info->cmdline = &cmd->cmdline[0];
If you set command line for DomU here, then please also set the command
line for the hwdom above. So the function has the same behavior accross
all the domain.
+ }
if ( !mod || !mod->size )
{
printk(XENLOG_ERR "Missing kernel boot module?\n");
return -ENOENT;
}
- info->kernel_bootmodule = mod;
-
- printk("Loading kernel from boot module @ %"PRIpaddr"\n", mod->start);
-
- info->initrd_bootmodule = boot_module_find_by_kind(BOOTMOD_RAMDISK);
+ printk("Loading DomU kernel from boot module @ %"PRIpaddr"\n",
This message is wrong for the hardware domain. The best solution would
be to print dom%u.
+ info->kernel_bootmodule->start);
if ( info->initrd_bootmodule )
printk("Loading ramdisk from boot module @ %"PRIpaddr"\n",
info->initrd_bootmodule->start);
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 39b7828..4a65289 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -55,7 +55,7 @@ struct kernel_info {
* ->type
* ->load hook, and sets loader specific variables ->zimage
*/
-int kernel_probe(struct kernel_info *info);
+int kernel_probe(struct kernel_info *info, struct dt_device_node *domain);
/*
* Loads the kernel into guest RAM.
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index dbab232..d6d1546 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -263,6 +263,35 @@ struct bootcmdline * __init
boot_cmdline_find_by_kind(bootmodule_kind kind)
return NULL;
}
+struct bootcmdline * __init boot_cmdline_find_by_name(const char *name)
+{
+ struct bootcmdlines *mods = &bootinfo.cmdlines;
+ struct bootcmdline *mod;
+ int i;
Unsigned + newline.
+ for (i = 0 ; i < mods->nr_mods ; i++ )
+ {
+ mod = &mods->cmdline[i];
+ if ( strcmp(mod->dt_name, name) == 0 )
+ return mod;
+ }
+ return NULL;
+}
+
+struct bootmodule * __init boot_module_find_by_addr_and_kind(bootmodule_kind
kind,
+ paddr_t start)
+{
+ struct bootmodules *mods = &bootinfo.modules;
+ struct bootmodule *mod; > + int i;
Same here.
+ for (i = 0 ; i < mods->nr_mods ; i++ )
+ {
+ mod = &mods->module[i];
+ if ( mod->kind == kind && mod->start == start )
+ return mod;
+ }
+ return NULL;
+}
+
const char * __init boot_module_kind_as_string(bootmodule_kind kind)
{
switch ( kind )
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 711b4a2..177e8db 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -95,7 +95,10 @@ const char __init *boot_fdt_cmdline(const void *fdt);
struct bootmodule *add_boot_module(bootmodule_kind kind,
paddr_t start, paddr_t size, bool domU);
struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
+struct bootmodule * __init boot_module_find_by_addr_and_kind(bootmodule_kind
kind,
+ paddr_t start);
struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind);
+struct bootcmdline * __init boot_cmdline_find_by_name(const char *name);
const char * __init boot_module_kind_as_string(bootmodule_kind kind);
#endif
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
