| Message ID | 20240830214730.1621-4-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Boot modules for Hyperlaunch | expand |
On 30/08/2024 10:46 pm, Daniel P. Smith wrote: > Transition Xen's command line to being held in struct boot_info. > > No functional change intended. > > Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index 432b7d1701e4..a945fa10555f 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -1049,11 +1058,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > > multiboot_to_bootinfo(mbi); > > - /* Parse the command-line options. */ > - if ( mbi->flags & MBI_CMDLINE ) > - cmdline = cmdline_cook(__va(mbi->cmdline), boot_info->boot_loader_name); > - > - if ( (kextra = strstr(cmdline, " -- ")) != NULL ) > + if ( (kextra = strstr(boot_info->cmdline, " -- ")) != NULL ) > { > /* > * Options after ' -- ' separator belong to dom0. > @@ -1064,7 +1069,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > kextra += 3; > while ( kextra[1] == ' ' ) kextra++; > } > - cmdline_parse(cmdline); > + cmdline_parse(boot_info->cmdline); It would be nice to get this kextra handling out of __start_xen(), but I'm not entirely sure how. It shouldn't live in multiboot_fill_boot_info() if that's going to be split for pvh, yet it really ought to live with the other editing of bi->cmdline. Something that is very subtle is that the *kextra = '\0' between these two hunks ends up truncating bi->cmdline. Perhaps best to leave it alone until inspiration strikes. ~Andrew
On 9/3/24 19:04, Andrew Cooper wrote: > On 30/08/2024 10:46 pm, Daniel P. Smith wrote: >> Transition Xen's command line to being held in struct boot_info. >> >> No functional change intended. >> >> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Thank you. >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >> index 432b7d1701e4..a945fa10555f 100644 >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -1049,11 +1058,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) >> >> multiboot_to_bootinfo(mbi); >> >> - /* Parse the command-line options. */ >> - if ( mbi->flags & MBI_CMDLINE ) >> - cmdline = cmdline_cook(__va(mbi->cmdline), boot_info->boot_loader_name); >> - >> - if ( (kextra = strstr(cmdline, " -- ")) != NULL ) >> + if ( (kextra = strstr(boot_info->cmdline, " -- ")) != NULL ) >> { >> /* >> * Options after ' -- ' separator belong to dom0. >> @@ -1064,7 +1069,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) >> kextra += 3; >> while ( kextra[1] == ' ' ) kextra++; >> } >> - cmdline_parse(cmdline); >> + cmdline_parse(boot_info->cmdline); > > It would be nice to get this kextra handling out of __start_xen(), but > I'm not entirely sure how. > > It shouldn't live in multiboot_fill_boot_info() if that's going to be > split for pvh, yet it really ought to live with the other editing of > bi->cmdline. > > Something that is very subtle is that the *kextra = '\0' between these > two hunks ends up truncating bi->cmdline. > > Perhaps best to leave it alone until inspiration strikes. One thought is to move all the logic into a function and make it the responsibility of fill function(s) to call it, since each entry point has its own unique way for the xen command line to be passed in. v/r, dps
diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h index e69feb1bb8be..d2ca077d2356 100644 --- a/xen/arch/x86/include/asm/bootinfo.h +++ b/xen/arch/x86/include/asm/bootinfo.h @@ -12,6 +12,7 @@ struct boot_info { unsigned int nr_mods; const char *boot_loader_name; + const char *cmdline; }; #endif diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 432b7d1701e4..a945fa10555f 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -276,6 +276,8 @@ static int __init cf_check parse_acpi_param(const char *s) } custom_param("acpi", parse_acpi_param); +static const char *cmdline_cook(const char *p, const char *loader_name); + static const module_t *__initdata initial_images; static struct boot_info __initdata *boot_info; @@ -288,6 +290,13 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi) info.boot_loader_name = (mbi->flags & MBI_LOADERNAME) ? __va(mbi->boot_loader_name) : "unknown"; + /* Parse the command-line options. */ + if ( mbi->flags & MBI_CMDLINE ) + info.cmdline = cmdline_cook(__va(mbi->cmdline), + info.boot_loader_name); + else + info.cmdline = ""; + boot_info = &info; } @@ -996,7 +1005,7 @@ static struct domain *__init create_dom0(const module_t *image, void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) { - const char *memmap_type = NULL, *cmdline = ""; + const char *memmap_type = NULL; char *kextra; void *bsp_stack; struct cpu_info *info = get_cpu_info(), *bsp_info; @@ -1049,11 +1058,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) multiboot_to_bootinfo(mbi); - /* Parse the command-line options. */ - if ( mbi->flags & MBI_CMDLINE ) - cmdline = cmdline_cook(__va(mbi->cmdline), boot_info->boot_loader_name); - - if ( (kextra = strstr(cmdline, " -- ")) != NULL ) + if ( (kextra = strstr(boot_info->cmdline, " -- ")) != NULL ) { /* * Options after ' -- ' separator belong to dom0. @@ -1064,7 +1069,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) kextra += 3; while ( kextra[1] == ' ' ) kextra++; } - cmdline_parse(cmdline); + cmdline_parse(boot_info->cmdline); /* Must be after command line argument parsing and before * allocing any xenheap structures wanted in lower memory. */ @@ -1094,7 +1099,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) printk("Bootloader: %s\n", boot_info->boot_loader_name); - printk("Command line: %s\n", cmdline); + printk("Command line: %s\n", boot_info->cmdline); printk("Xen image load base address: %#lx\n", xen_phys_start); if ( hypervisor_name )
Transition Xen's command line to being held in struct boot_info. No functional change intended. Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> --- xen/arch/x86/include/asm/bootinfo.h | 1 + xen/arch/x86/setup.c | 21 +++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-)