All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8] xen: Allow a default compiled-in command line using Kconfig
@ 2017-03-21 11:18 Zhongze Liu
  2017-03-21 14:27 ` Jan Beulich
  0 siblings, 1 reply; 5+ messages in thread
From: Zhongze Liu @ 2017-03-21 11:18 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Zhongze Liu, George Dunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Jan Beulich

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/browse/XEN-41)

If CMDLINE is set, it will be parsed prior to the bootloader command line.
This order of parsing implies that if any non-cumulative options are set in
both CMDLINE and the bootloader command line, only the ones in the latter will
take effect. Furthermore, if CMDLINE_OVERRIDE is set to y, the whole
bootloader command line will be ignored, which will be useful to work around
broken bootloaders. A wrapper to the original common/kernel.c:cmdline_parse()
was introduced to complete this task.

Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changed since v7:
  * Dropped the redundant paragraphs
---
 xen/common/Kconfig  | 22 ++++++++++++++++++++++
 xen/common/kernel.c | 30 ++++++++++++++++++++++++------
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index f2ecbc43d6..e1c90b739a 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -237,4 +237,26 @@ config FAST_SYMBOL_LOOKUP
 	  The only user of this is Live patching.
 
 	  If unsure, say Y.
+
+config CMDLINE
+	string "Built-in hypervisor command string"
+	default ""
+	depends on EXPERT = "y"
+	---help---
+	  Enter arguments here that should be compiled into the hypervisor
+	  image and used at boot time. When the system boots, this string
+	  will be parsed prior to the bootloader command line. So if a
+	  non-cumulative option is set both in this string and in the
+	  bootloader command line, only the latter one will take effect.
+
+config CMDLINE_OVERRIDE
+	bool "Built-in command line overrides bootloader arguments"
+	default n
+	depends on CMDLINE != ""
+	---help---
+	  Set this option to 'Y' to have the hypervisor ignore the bootloader
+	  command line, and use ONLY the built-in command line.
+
+	  This is used to work around broken bootloaders. This should
+	  be set to 'N' under normal conditions.
 endmenu
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 4b87c60845..64920e8304 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -23,6 +23,7 @@
 enum system_state system_state = SYS_STATE_early_boot;
 
 xen_commandline_t saved_cmdline;
+static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
 static void __init assign_integer_param(
     const struct kernel_param *param, uint64_t val)
@@ -46,18 +47,13 @@ static void __init assign_integer_param(
     }
 }
 
-void __init cmdline_parse(const char *cmdline)
+static void __init _cmdline_parse(const char *cmdline)
 {
     char opt[100], *optval, *optkey, *q;
     const char *p = cmdline;
     const struct kernel_param *param;
     int bool_assert;
 
-    if ( cmdline == NULL )
-        return;
-
-    safe_strcpy(saved_cmdline, cmdline);
-
     for ( ; ; )
     {
         /* Skip whitespace. */
@@ -147,6 +143,28 @@ void __init cmdline_parse(const char *cmdline)
     }
 }
 
+/**
+ *    cmdline_parse -- parses the xen command line.
+ * If CONFIG_CMDLINE is set, it would be parsed prior to @cmdline.
+ * But if CONFIG_CMDLINE_OVERRIDE is set to y, @cmdline will be ignored.
+ */
+void __init cmdline_parse(const char *cmdline)
+{
+    if ( opt_builtin_cmdline[0] )
+    {
+        printk("Built-in command line: %s\n", opt_builtin_cmdline);
+        _cmdline_parse(opt_builtin_cmdline);
+    }
+
+#ifndef CONFIG_CMDLINE_OVERRIDE
+    if ( cmdline == NULL )
+        return;
+
+    safe_strcpy(saved_cmdline, cmdline);
+    _cmdline_parse(cmdline);
+#endif
+}
+
 int __init parse_bool(const char *s)
 {
     if ( !strcmp("no", s) ||
-- 
2.12.0


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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v8] xen: Allow a default compiled-in command line using Kconfig
  2017-03-21 11:18 [PATCH v8] xen: Allow a default compiled-in command line using Kconfig Zhongze Liu
@ 2017-03-21 14:27 ` Jan Beulich
  2017-03-21 14:56   ` Zhongze Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2017-03-21 14:27 UTC (permalink / raw)
  To: Zhongze Liu
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, xen-devel

>>> On 21.03.17 at 12:18, <blackskygg@gmail.com> wrote:
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -237,4 +237,26 @@ config FAST_SYMBOL_LOOKUP
>  	  The only user of this is Live patching.
>  
>  	  If unsure, say Y.
> +
> +config CMDLINE
> +	string "Built-in hypervisor command string"
> +	default ""
> +	depends on EXPERT = "y"

Luckily I've noticed the non-EXPERT build failure in my pre-push
tests: During review of patch 3 I had asked for converting the
"depends on" into a conditional prompt, and I don't see why
this wasn't done. Did you ever do a non-EXPERT build with your
patch?

Jan


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8] xen: Allow a default compiled-in command line using Kconfig
  2017-03-21 14:27 ` Jan Beulich
@ 2017-03-21 14:56   ` Zhongze Liu
  2017-03-21 15:22     ` Jan Beulich
  0 siblings, 1 reply; 5+ messages in thread
From: Zhongze Liu @ 2017-03-21 14:56 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, xen-devel

2017-03-21 22:27 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>> On 21.03.17 at 12:18, <blackskygg@gmail.com> wrote:
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -237,4 +237,26 @@ config FAST_SYMBOL_LOOKUP
>>         The only user of this is Live patching.
>>
>>         If unsure, say Y.
>> +
>> +config CMDLINE
>> +     string "Built-in hypervisor command string"
>> +     default ""
>> +     depends on EXPERT = "y"
>
> Luckily I've noticed the non-EXPERT build failure in my pre-push
> tests: During review of patch 3 I had asked for converting the
> "depends on" into a conditional prompt, and I don't see why
> this wasn't done. Did you ever do a non-EXPERT build with your
> patch?
>
Yes, I've built and tested my patch with and without XEN_CONFIG_EXPERT=y.
Could you please show me the error message?

BTW, my environment:
uname -a:
    Linux sky-arch 4.10.2-1-ARCH #1 SMP PREEMPT Mon Mar 13 17:13:41
CET 2017 x86_64 GNU/Linux

gcc -v
    Using built-in specs.
    COLLECT_GCC=gcc
    COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.1/lto-wrapper
    Target: x86_64-pc-linux-gnu
    Configured with: /build/gcc/src/gcc/configure --prefix=/usr
--libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man
--infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/
--enable-
languages=c,c++,ada,fortran,go,lto,objc,obj-c++ --enable-shared
--enable-threads=posix --enable-libmpx --with-system-zlib --with-isl
--enable-__cxa_atexit --disable-libunwind-exceptions
--enable-clocale=gnu --disable-libstdcxx-pch --disable-libssp
--enable-gnu-unique-object --enable-linker-build-id --enable-lto
--enable-plugin --enable-install-libiberty
--with-linker-hash-style=gnu --enable-gnu-indirect-function
--disable-multilib --disable-werror --enable-checking=release
Thread model: posix
    gcc version 6.3.1 20170306 (GCC)

I used x86_64-w64-mingw32 binutils to build the EFI stub.
and the configuration:
   ./configure \
    PYTHON=/usr/bin/python2 \
    --prefix=/usr \
    --sbindir=/usr/bin \
    --with-sysconfig-leaf-dir=conf.d \
    --with-initddir=/etc/init.d \
    --enable-systemd \
    --disable-docs \
    --disable-stubdom \
    --with-extra-qemuu-configure-args="--disable-bluez --disable-gtk
--enable-spice --enable-usb-redir";

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8] xen: Allow a default compiled-in command line using Kconfig
  2017-03-21 14:56   ` Zhongze Liu
@ 2017-03-21 15:22     ` Jan Beulich
  2017-03-21 16:34       ` Zhongze Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2017-03-21 15:22 UTC (permalink / raw)
  To: Zhongze Liu
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, xen-devel

>>> On 21.03.17 at 15:56, <blackskygg@gmail.com> wrote:
> 2017-03-21 22:27 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>>> On 21.03.17 at 12:18, <blackskygg@gmail.com> wrote:
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -237,4 +237,26 @@ config FAST_SYMBOL_LOOKUP
>>>         The only user of this is Live patching.
>>>
>>>         If unsure, say Y.
>>> +
>>> +config CMDLINE
>>> +     string "Built-in hypervisor command string"
>>> +     default ""
>>> +     depends on EXPERT = "y"
>>
>> Luckily I've noticed the non-EXPERT build failure in my pre-push
>> tests: During review of patch 3 I had asked for converting the
>> "depends on" into a conditional prompt, and I don't see why
>> this wasn't done. Did you ever do a non-EXPERT build with your
>> patch?
>>
> Yes, I've built and tested my patch with and without XEN_CONFIG_EXPERT=y.
> Could you please show me the error message?

The error I've got was that CONFIG_CMDLINE was undefined in
xen/common/kernel.c, which isn't really surprising with the option
having had a "depends on", but the use being unconditional.

Jan


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8] xen: Allow a default compiled-in command line using Kconfig
  2017-03-21 15:22     ` Jan Beulich
@ 2017-03-21 16:34       ` Zhongze Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Zhongze Liu @ 2017-03-21 16:34 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, xen-devel

2017-03-21 23:22 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>> On 21.03.17 at 15:56, <blackskygg@gmail.com> wrote:
>> 2017-03-21 22:27 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>>>> On 21.03.17 at 12:18, <blackskygg@gmail.com> wrote:
>>>> --- a/xen/common/Kconfig
>>>> +++ b/xen/common/Kconfig
>>>> @@ -237,4 +237,26 @@ config FAST_SYMBOL_LOOKUP
>>>>         The only user of this is Live patching.
>>>>
>>>>         If unsure, say Y.
>>>> +
>>>> +config CMDLINE
>>>> +     string "Built-in hypervisor command string"
>>>> +     default ""
>>>> +     depends on EXPERT = "y"
>>>
>>> Luckily I've noticed the non-EXPERT build failure in my pre-push
>>> tests: During review of patch 3 I had asked for converting the
>>> "depends on" into a conditional prompt, and I don't see why
>>> this wasn't done. Did you ever do a non-EXPERT build with your
>>> patch?
>>>
>> Yes, I've built and tested my patch with and without XEN_CONFIG_EXPERT=y.
>> Could you please show me the error message?
>
> The error I've got was that CONFIG_CMDLINE was undefined in
> xen/common/kernel.c, which isn't really surprising with the option
> having had a "depends on", but the use being unconditional.
>
After doing a full clean and making it again. I got the same error message.
This seems to be a huge mistake. I've now changed the "depends on" into a
conditional prompt, and have done both EXPERT and non-EXPERT build
again (with a full clean every time before building). So this time it shouldn't
report this error again. But anyway, please do test it out again.

Cheers.

Zhongze Liu.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-03-21 16:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-21 11:18 [PATCH v8] xen: Allow a default compiled-in command line using Kconfig Zhongze Liu
2017-03-21 14:27 ` Jan Beulich
2017-03-21 14:56   ` Zhongze Liu
2017-03-21 15:22     ` Jan Beulich
2017-03-21 16:34       ` Zhongze Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.