[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v2 09/21] xen/arm: move cmdline out of boot_modules
Hi,
On 07/07/18 00:12, Stefano Stabellini wrote:
Remove the cmdline field from struct boot_module, cmdline is stored
independently out of the boot_modules array as dom0_cmdline.
I am not entirely convince of this patch, this does not seem to go
towards a better code base because dom0_cmdline is only set if
"bootargs". This may raise some confusing to the developer.
I would still prefer to keep the command-line in the boot module
structure and find a way to associated the bootmodule with a node.
Add a pointer to struct kernel_info to point to the cmdline for a given
kernel.
boot_fdt_cmdline is only used to retrieve the Xen cmdline. Remove the
code to return the dom0 cmdline when the Xen cmdline is not available.
None of the code in boot_fdt_cmdline will return Dom0 cmdline. What the
code does is looking whether xen,dom0-bootargs or "bootargs" in the boot
kernel module has been defined. If not, it means "bootargs" of the
chosen node will be used for Xen.
Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
---
Changes in v2:
- new patch
---
xen/arch/arm/bootfdt.c | 25 +++++++------------------
xen/arch/arm/domain_build.c | 11 +++++------
xen/arch/arm/kernel.h | 1 +
xen/arch/arm/setup.c | 10 +++-------
xen/include/asm-arm/setup.h | 5 ++---
5 files changed, 18 insertions(+), 34 deletions(-)
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 8eba42c..b3e1e00 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -226,11 +226,10 @@ static void __init process_multiboot_node(const void
*fdt, int node,
if ( len > BOOTMOD_MAX_CMDLINE )
panic("module %s command line too long\n", name);
cmdline = prop->data;
+ safe_strcpy(dom0_cmdline, cmdline);
I don't think anything promise you that "bootargs" will actually be for
the kernel.
Looking at the documentation, the only wording is "Command line
associated with this module".
}
- else
- cmdline = NULL;
-
- add_boot_module(kind, start, size, cmdline);
+
+ add_boot_module(kind, start, size);
}
static void __init process_chosen_node(const void *fdt, int node,
@@ -276,7 +275,7 @@ static void __init process_chosen_node(const void *fdt, int
node,
printk("Initrd %"PRIpaddr"-%"PRIpaddr"\n", start, end);
- add_boot_module(BOOTMOD_RAMDISK, start, end-start, NULL);
+ add_boot_module(BOOTMOD_RAMDISK, start, end-start);
}
static int __init early_scan_node(const void *fdt,
@@ -307,12 +306,11 @@ static void __init early_print_info(void)
mi->bank[i].start + mi->bank[i].size - 1);
printk("\n");
for ( i = 0 ; i < mods->nr_mods; i++ )
- printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %-12s %s\n",
+ printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %-12s\n",
i,
mods->module[i].start,
mods->module[i].start + mods->module[i].size,
- boot_module_kind_as_string(mods->module[i].kind),
- mods->module[i].cmdline);
+ boot_module_kind_as_string(mods->module[i].kind));
nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
for ( i = 0; i < nr_rsvd; i++ )
{
@@ -341,7 +339,7 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
if ( ret < 0 )
panic("No valid device tree\n");
- add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), NULL);
+ add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt));
device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
early_print_info();
@@ -360,15 +358,6 @@ const char *boot_fdt_cmdline(const void *fdt)
prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL);
if ( prop == NULL )
- {
- struct bootmodule *dom0_mod =
- boot_module_find_by_kind(BOOTMOD_KERNEL);
-
- if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL) ||
- ( dom0_mod && dom0_mod->cmdline[0] ) )
- prop = fdt_get_property(fdt, node, "bootargs", NULL);
- }
See above, this code should not be dropped.
- if ( prop == NULL )
return NULL;
return prop->data;
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 4d06584..d7e642b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -402,10 +402,8 @@ static int write_properties(struct domain *d, struct
kernel_info *kinfo,
int res = 0;
int had_dom0_bootargs = 0;
- const struct bootmodule *kernel = kinfo->kernel_bootmodule;
-
- if ( kernel && kernel->cmdline[0] )
- bootargs = &kernel->cmdline[0];
+ if ( dom0_cmdline[0] )
+ bootargs = &dom0_cmdline[0];
dt_for_each_property_node (node, prop)
{
@@ -971,9 +969,9 @@ static int make_chosen_node(const struct kernel_info *kinfo)
if ( res )
return res;
- if ( mod && mod->cmdline[0] )
+ if ( kinfo->cmdline && kinfo->cmdline[0] )
{
- bootargs = &mod->cmdline[0];
+ bootargs = &kinfo->cmdline[0];
res = fdt_property(fdt, "bootargs", bootargs, strlen(bootargs) + 1);
if ( res )
return res;
@@ -2173,6 +2171,7 @@ int __init construct_dom0(struct domain *d)
#endif
+ kinfo.cmdline = &dom0_cmdline[0];
allocate_memory(d, &kinfo);
find_gnttab_region(d, &kinfo);
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 6d69509..a47aa4c 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -30,6 +30,7 @@ struct kernel_info {
/* boot blob load addresses */
const struct bootmodule *kernel_bootmodule, *initrd_bootmodule;
+ const char* cmdline;
paddr_t dtb_paddr;
paddr_t initrd_paddr;
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 1d6f6bf..188b2cb 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -51,6 +51,7 @@
#include <asm/acpi.h>
struct bootinfo __initdata bootinfo;
+char __initdata dom0_cmdline[BOOTMOD_MAX_CMDLINE];
struct cpuinfo_arm __read_mostly boot_cpu_data;
@@ -202,8 +203,7 @@ void dt_unreserved_regions(paddr_t s, paddr_t e,
}
struct bootmodule *add_boot_module(bootmodule_kind kind,
- paddr_t start, paddr_t size,
- const char *cmdline)
+ paddr_t start, paddr_t size)
{
struct bootmodules *mods = &bootinfo.modules;
struct bootmodule *mod;
@@ -219,10 +219,6 @@ struct bootmodule *add_boot_module(bootmodule_kind kind,
mod->kind = kind;
mod->start = start;
mod->size = size;
- if ( cmdline )
- safe_strcpy(mod->cmdline, cmdline);
- else
- mod->cmdline[0] = 0;
return mod;
}
@@ -725,7 +721,7 @@ void __init start_xen(unsigned long boot_phys_offset,
/* Register Xen's load address as a boot module. */
xen_bootmodule = add_boot_module(BOOTMOD_XEN,
(paddr_t)(uintptr_t)(_start + boot_phys_offset),
- (paddr_t)(uintptr_t)(_end - _start + 1), NULL);
+ (paddr_t)(uintptr_t)(_end - _start + 1));
BUG_ON(!xen_bootmodule);
xen_paddr = get_xen_paddr();
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index f1e4a3f..6d08eb4 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -35,13 +35,13 @@ struct bootmodule {
bootmodule_kind kind;
paddr_t start;
paddr_t size;
- char cmdline[BOOTMOD_MAX_CMDLINE];
};
struct bootmodules {
int nr_mods;
struct bootmodule module[MAX_MODULES];
};
+extern char dom0_cmdline[BOOTMOD_MAX_CMDLINE];
struct bootinfo {
struct meminfo mem;
@@ -78,8 +78,7 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr);
const char __init *boot_fdt_cmdline(const void *fdt);
struct bootmodule *add_boot_module(bootmodule_kind kind,
- paddr_t start, paddr_t size,
- const char *cmdline);
+ paddr_t start, paddr_t size);
struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
const char * __init boot_module_kind_as_string(bootmodule_kind kind);
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
