Re: [PATCH v4 03/44] x86/boot: move cmdline to boot info

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx>

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> 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