Re: [Xen-devel] [PATCH] xen: Allow a default compiled-in command line using Kconfig

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.03.17 at 11:50, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 06/03/17 02:38, Sky Liu wrote:
>> Added two new config entries in common/Kconfig: CMDLINE_BOOL and CMDLINE.
> 
> Hello.  Thankyou for looking into this task.
> 
> As a start, why do you introduce CMDLINE_BOOL?  you don't actually use it.
> 
>> These two entries enable an embedded command line
>> to be compiled in the hypervisor.
>> If CMDLINE_BOOL is set to Y, both arm and x86 startup routines
>> will call cmdline_parse() first on CMDLINE,
>> then on the line passed by the bootloader.
>> This allows downstreams to set their defaults
>> without modifying the source code all over the place.
>> Also probably useful for the embedded space.
>>
>> See Also: 
> https://xenproject.atlassian.net/projects/XEN/issues/XEN-41?filter=allopeniss 
> ues
> 
> If you are going to include this URL, please drop the ?filter=allopenissues.
> 
>>
>> Signed-off-by: Zhongze Liu <blackskygg@xxxxxxxxx>
>> ---
>>  xen/arch/arm/setup.c |  5 +++++
>>  xen/arch/x86/setup.c |  5 +++++
>>  xen/common/Kconfig   | 27 +++++++++++++++++++++++++++
>>  3 files changed, 37 insertions(+)
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 92a2de6b70..55860a1299 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -729,6 +729,11 @@ void __init start_xen(unsigned long boot_phys_offset,
>>          + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
>>      fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr);
>>
>> +#ifdef CONFIG_CMDLINE
>> +    printk("Compiled-in command line: %s\n", CONFIG_CMDLINE);
>> +    cmdline_parse(cmdline);
>> +#endif
> 
> You can be rather more cunning, and avoid the #ifdefary here.
> 
> Given something like this:
> 
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index 4b87c60..2da9cfe 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -24,6 +24,8 @@ enum system_state system_state = SYS_STATE_early_boot;
>  
>  xen_commandline_t saved_cmdline;
>  
> +const char opt_def_cmdline[] = CONFIG_CMDLINE;
> +
>  static void __init assign_integer_param(
>      const struct kernel_param *param, uint64_t val)
>  {
> diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
> index 548b64d..99a428d 100644
> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -100,5 +100,7 @@ extern enum system_state {
>  
>  bool_t is_active_kernel_text(unsigned long addr);
>  
> +extern const char opt_def_cmdline[];
> +
>  #endif /* _LINUX_KERNEL_H */
>  
> 
> Your code in the various setup.c's can be
> 
> if ( strlen(opt_def_cmdline) )
> {
>     printk("Compiled-in command line: %s\n", opt_def_cmdline);
>     cmdline_parse(opt_def_cmdline);
> }
> 
> which is rather neater and not liable to code-rot.

I would go even farther and have cmdline_parse() invoke itself with
the static string, instead of repeating the same code per arch. This
also avoids the need to make opt_def_cmdline[] non-static.

Jan


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