linux-embedded.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] bootup: Add built-in kernel command line for x86
@ 2008-08-06 21:31 Tim Bird
  2008-08-06 22:04 ` Robert Schwebel
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Tim Bird @ 2008-08-06 21:31 UTC (permalink / raw)
  To: linux kernel, linux-embedded, Matt Mackall, H. Peter Anvin,
	Thomas Gleixner <>

Add support for a built-in command line for x86 architectures.
The Kconfig help gives the major rationale for this addition.

Note that this option is available for many other arches, and
its use is widespread in embedded projects.

This patch addresses concerns that were raised with an
earlier version, regarding precedence between the built-in
command line and the command line provided by the boot
loader.

See the thread at http://lkml.org/lkml/2006/6/11/115
for details.

The default behavior is to append the boot loader string
to this one.  However, there is a mechanism (leading '!')
to force the built-in string to override the boot loader
string.

This implementation covers some important cases mentioned
in the previous thread, namely:
 * boot loaders that can't pass args to the kernel
 * boot loaders that pass incorrect args to the kernel
 * automated testing of kernel command line options,
 without having to address lots of different bootloaders

Signed-off-by: Tim Bird <tim.bird@am.sony.com>
---

Note: If you're copied on this, it's because you seemed
interested in this the last time it was submitted.

This particular implementation adds a space to the
front of the command line, in the default case where
the built-in string is empty.  I don't think this is a
problem, but comments are welcome.  It would be trivial
to remove the extra space, and require people who set
the string to know what they are doing, and add their
own space at the end of the string in the .config.


 arch/x86/Kconfig        |   24 ++++++++++++++++++++++++
 arch/x86/kernel/setup.c |   11 +++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3d0f2b6..63c181e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1393,6 +1393,30 @@ config COMPAT_VDSO

 	  If unsure, say Y.

+config CMDLINE
+	string "Initial kernel command string"
+	default ""
+	help
+	  On some systems (e.g. embedded ones), there is no way for the
+	  boot loader to pass arguments to the kernel.  On some systems,
+	  the arguments passed by the boot loader are incorrect.  For these
+	  platforms, you can supply command-line options at build
+	  time by entering them here. These will be compiled into the kernel
+	  image and used at boot time.
+
+	  If the boot loader provides a command line at boot time, it is
+	  appended to this string.  To have the kernel ignore the boot loader
+	  command line, and use ONLY the string specified here, use an
+	  exclamation point as the first character of the string.
+	  Example: "!root=/dev/hda1 ro"
+
+         In most cases, the command line (whether built-in or provided
+	  by the boot loader) should specify the device for the root
+	  file system.
+
+	  Systems with fully functional boot loaders (i.e. non-embedded)
+	  should leave this string blank.
+
 endmenu

 config ARCH_ENABLE_MEMORY_HOTPLUG
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 2d88858..298bcee 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -223,6 +223,7 @@ unsigned long saved_video_mode;
 #define RAMDISK_LOAD_FLAG		0x4000

 static char __initdata command_line[COMMAND_LINE_SIZE];
+static char __initdata builtin_cmdline[COMMAND_LINE_SIZE] = CONFIG_CMDLINE;

 #if defined(CONFIG_EDD) || defined(CONFIG_EDD_MODULE)
 struct edd edd;
@@ -665,6 +666,16 @@ void __init setup_arch(char **cmdline_p)
 	bss_resource.start = virt_to_phys(&__bss_start);
 	bss_resource.end = virt_to_phys(&__bss_stop)-1;

+	/* append boot loader cmdline to builtin, unless builtin overrides it */
+	if (builtin_cmdline[0] != '!') {
+		strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
+		strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
+		strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+	} else {
+		strlcpy(boot_command_line, &builtin_cmdline[1],
+			COMMAND_LINE_SIZE);
+	}
+
 	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
 	*cmdline_p = command_line;

-- 
1.5.6


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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 21:31 [PATCH] bootup: Add built-in kernel command line for x86 Tim Bird
@ 2008-08-06 22:04 ` Robert Schwebel
  2008-08-06 22:22   ` Tim Bird
  2008-08-06 22:14 ` Matt Mackall
  2008-08-11 19:10 ` Ingo Molnar
  2 siblings, 1 reply; 17+ messages in thread
From: Robert Schwebel @ 2008-08-06 22:04 UTC (permalink / raw)
  To: Tim Bird
  Cc: linux kernel, linux-embedded, Matt Mackall, H. Peter Anvin,
	Thomas Gleixner

On Wed, Aug 06, 2008 at 02:31:48PM -0700, Tim Bird wrote:
> Add support for a built-in command line for x86 architectures.
> The Kconfig help gives the major rationale for this addition.

If this feature is desired on all architectures, shouldn't it go out of
arch/?

rsc
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 21:31 [PATCH] bootup: Add built-in kernel command line for x86 Tim Bird
  2008-08-06 22:04 ` Robert Schwebel
@ 2008-08-06 22:14 ` Matt Mackall
  2008-08-06 22:31   ` Tim Bird
  2008-08-11 19:10 ` Ingo Molnar
  2 siblings, 1 reply; 17+ messages in thread
From: Matt Mackall @ 2008-08-06 22:14 UTC (permalink / raw)
  To: Tim Bird; +Cc: linux kernel, linux-embedded, H. Peter Anvin, Thomas Gleixner


On Wed, 2008-08-06 at 14:31 -0700, Tim Bird wrote:
> The default behavior is to append the boot loader string
> to this one.  However, there is a mechanism (leading '!')
> to force the built-in string to override the boot loader
> string.

Nice solution.

Where is this relative to early boot option checking?

-- 
Mathematics is the supreme nostalgia of our time.

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 22:04 ` Robert Schwebel
@ 2008-08-06 22:22   ` Tim Bird
  2008-08-06 22:48     ` H. Peter Anvin
  0 siblings, 1 reply; 17+ messages in thread
From: Tim Bird @ 2008-08-06 22:22 UTC (permalink / raw)
  To: Robert Schwebel
  Cc: linux kernel, linux-embedded, Matt Mackall, H. Peter Anvin,
	Thomas Gleixner

Robert Schwebel wrote:
> On Wed, Aug 06, 2008 at 02:31:48PM -0700, Tim Bird wrote:
>> Add support for a built-in command line for x86 architectures.
>> The Kconfig help gives the major rationale for this addition.
> 
> If this feature is desired on all architectures, shouldn't it go out of
> arch/?

Different arches handle their command lines differently, but
with some elbow grease (and some buy-in from the different
arch maintainers), we could try unifying the approach here.

One difficulty is that the other arches' command lines
are not currently "broken", so there's no real incentive
to change them.

The only thing novel thing I'm adding here is the addition of
the leading '!' to allow for an override.  This is needed
in some x86 cases I'm familiar with, but I've haven't seen
any cases where it would be useful for other arches.
(not to say they don't exist - I just haven't seen them.)
 -- Tim

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 22:14 ` Matt Mackall
@ 2008-08-06 22:31   ` Tim Bird
  2008-08-06 22:48     ` Matt Mackall
  0 siblings, 1 reply; 17+ messages in thread
From: Tim Bird @ 2008-08-06 22:31 UTC (permalink / raw)
  To: Matt Mackall
  Cc: linux kernel, linux-embedded, H. Peter Anvin, Thomas Gleixner

Matt Mackall wrote:
> On Wed, 2008-08-06 at 14:31 -0700, Tim Bird wrote:
>> The default behavior is to append the boot loader string
>> to this one.  However, there is a mechanism (leading '!')
>> to force the built-in string to override the boot loader
>> string.
> 
> Nice solution.
> 
> Where is this relative to early boot option checking?

parse_early_param() is right AFTER this in the x86 setup_arch()
function (in arch/x86/kernel/setup.c).  All the other command-line
handling I could find is after this in init/main.c:start_kernel().

There is some stuff earlier in the setup_arch() routine about
boot_params, but that looks like it's related to the old(?)
binary data points you can jam into the kernel image.  (That is,
it doesn't look like it's related to the command line handling).
 -- Tim

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 22:22   ` Tim Bird
@ 2008-08-06 22:48     ` H. Peter Anvin
  2008-08-06 22:52       ` Matt Mackall
  2008-08-06 23:00       ` Tim Bird
  0 siblings, 2 replies; 17+ messages in thread
From: H. Peter Anvin @ 2008-08-06 22:48 UTC (permalink / raw)
  To: Tim Bird
  Cc: Robert Schwebel, linux kernel, linux-embedded, Matt Mackall,
	Thomas Gleixner

Tim Bird wrote:
> 
> One difficulty is that the other arches' command lines
> are not currently "broken", so there's no real incentive
> to change them.
> 
> The only thing novel thing I'm adding here is the addition of
> the leading '!' to allow for an override.  This is needed
> in some x86 cases I'm familiar with, but I've haven't seen
> any cases where it would be useful for other arches.
> (not to say they don't exist - I just haven't seen them.)
> 

Note that it could just as easily be done with a CONFIG_CMDLINE_OVERRIDE 
option, since the initial reason for a magic character was to be able to 
provide both prefix and suffix splicing.

CONFIG_CMDLINE_OVERRIDE is probably more palatable to other architectures.

	-hpa

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 22:31   ` Tim Bird
@ 2008-08-06 22:48     ` Matt Mackall
  0 siblings, 0 replies; 17+ messages in thread
From: Matt Mackall @ 2008-08-06 22:48 UTC (permalink / raw)
  To: Tim Bird; +Cc: linux kernel, linux-embedded, H. Peter Anvin, Thomas Gleixner


On Wed, 2008-08-06 at 15:31 -0700, Tim Bird wrote:
> Matt Mackall wrote:
> > On Wed, 2008-08-06 at 14:31 -0700, Tim Bird wrote:
> >> The default behavior is to append the boot loader string
> >> to this one.  However, there is a mechanism (leading '!')
> >> to force the built-in string to override the boot loader
> >> string.
> > 
> > Nice solution.
> > 
> > Where is this relative to early boot option checking?
> 
> parse_early_param() is right AFTER this in the x86 setup_arch()
> function (in arch/x86/kernel/setup.c).  All the other command-line
> handling I could find is after this in init/main.c:start_kernel().

Ok, there are a couple weird outliers outside of that still, but that
should make most people satisfied.

-- 
Mathematics is the supreme nostalgia of our time.

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 22:48     ` H. Peter Anvin
@ 2008-08-06 22:52       ` Matt Mackall
  2008-08-06 23:02         ` H. Peter Anvin
  2008-08-06 23:00       ` Tim Bird
  1 sibling, 1 reply; 17+ messages in thread
From: Matt Mackall @ 2008-08-06 22:52 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Tim Bird, Robert Schwebel, linux kernel, linux-embedded,
	Thomas Gleixner


On Wed, 2008-08-06 at 15:48 -0700, H. Peter Anvin wrote:
> Tim Bird wrote:
> > 
> > One difficulty is that the other arches' command lines
> > are not currently "broken", so there's no real incentive
> > to change them.
> > 
> > The only thing novel thing I'm adding here is the addition of
> > the leading '!' to allow for an override.  This is needed
> > in some x86 cases I'm familiar with, but I've haven't seen
> > any cases where it would be useful for other arches.
> > (not to say they don't exist - I just haven't seen them.)
> > 
> 
> Note that it could just as easily be done with a CONFIG_CMDLINE_OVERRIDE 
> option, since the initial reason for a magic character was to be able to 
> provide both prefix and suffix splicing.

You're right, I had forgotten about the suffix splicing and my brain is
a bit foggy on what motivated it.

> CONFIG_CMDLINE_OVERRIDE is probably more palatable to other architectures.

Yes, though I doubt we're in danger of introducing any real backwards
compatibility issues with the magic '!' at the beginning.

-- 
Mathematics is the supreme nostalgia of our time.

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 22:48     ` H. Peter Anvin
  2008-08-06 22:52       ` Matt Mackall
@ 2008-08-06 23:00       ` Tim Bird
  2008-08-06 23:03         ` H. Peter Anvin
  1 sibling, 1 reply; 17+ messages in thread
From: Tim Bird @ 2008-08-06 23:00 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Robert Schwebel, linux kernel, linux-embedded, Matt Mackall,
	Thomas Gleixner

H. Peter Anvin wrote:
> Tim Bird wrote:
>> The only thing novel thing I'm adding here is the addition of
>> the leading '!' to allow for an override.  This is needed
>> in some x86 cases I'm familiar with, but I've haven't seen
>> any cases where it would be useful for other arches.
>> (not to say they don't exist - I just haven't seen them.)
>>
> 
> Note that it could just as easily be done with a CONFIG_CMDLINE_OVERRIDE
> option, since the initial reason for a magic character was to be able to
> provide both prefix and suffix splicing.
Agreed.

> 
> CONFIG_CMDLINE_OVERRIDE is probably more palatable to other architectures.

I'd be OK implementing it with an option, rather than a magic char.
I was trying to avoid adding too many options, since many kernel
developers prefer fewer options where possible.  But the magic
char makes the code less straightforward.

If we ever move towards supporting both prefix and suffice splicing (or
even complicated in-the-middle splicing), then the magic char is
easier to develop into that.  But so far, I can only come up with
reasonable cases for append and override, and I don't want to add
superfluous handling for non-existent use cases.
 -- Tim

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 22:52       ` Matt Mackall
@ 2008-08-06 23:02         ` H. Peter Anvin
  0 siblings, 0 replies; 17+ messages in thread
From: H. Peter Anvin @ 2008-08-06 23:02 UTC (permalink / raw)
  To: Matt Mackall
  Cc: Tim Bird, Robert Schwebel, linux kernel, linux-embedded,
	Thomas Gleixner

Matt Mackall wrote:
> On Wed, 2008-08-06 at 15:48 -0700, H. Peter Anvin wrote:
>> Tim Bird wrote:
>>> One difficulty is that the other arches' command lines
>>> are not currently "broken", so there's no real incentive
>>> to change them.
>>>
>>> The only thing novel thing I'm adding here is the addition of
>>> the leading '!' to allow for an override.  This is needed
>>> in some x86 cases I'm familiar with, but I've haven't seen
>>> any cases where it would be useful for other arches.
>>> (not to say they don't exist - I just haven't seen them.)
>>>
>> Note that it could just as easily be done with a CONFIG_CMDLINE_OVERRIDE 
>> option, since the initial reason for a magic character was to be able to 
>> provide both prefix and suffix splicing.
> 
> You're right, I had forgotten about the suffix splicing and my brain is
> a bit foggy on what motivated it.
> 

Well, prefix = bootloader overrides; suffix = builtin overrides.

>> CONFIG_CMDLINE_OVERRIDE is probably more palatable to other architectures.
> 
> Yes, though I doubt we're in danger of introducing any real backwards
> compatibility issues with the magic '!' at the beginning.

Well, it does if we want to make this a generic feature, which I believe 
we should.

	-hpa

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 23:00       ` Tim Bird
@ 2008-08-06 23:03         ` H. Peter Anvin
  0 siblings, 0 replies; 17+ messages in thread
From: H. Peter Anvin @ 2008-08-06 23:03 UTC (permalink / raw)
  To: Tim Bird
  Cc: Robert Schwebel, linux kernel, linux-embedded, Matt Mackall,
	Thomas Gleixner

Tim Bird wrote:
> 
>> CONFIG_CMDLINE_OVERRIDE is probably more palatable to other architectures.
> 
> I'd be OK implementing it with an option, rather than a magic char.
> I was trying to avoid adding too many options, since many kernel
> developers prefer fewer options where possible.  But the magic
> char makes the code less straightforward.
> 
> If we ever move towards supporting both prefix and suffice splicing (or
> even complicated in-the-middle splicing), then the magic char is
> easier to develop into that.  But so far, I can only come up with
> reasonable cases for append and override, and I don't want to add
> superfluous handling for non-existent use cases.

I think I agree, and the override option would make it easier to make 
generic.

	-hpa

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-06 21:31 [PATCH] bootup: Add built-in kernel command line for x86 Tim Bird
  2008-08-06 22:04 ` Robert Schwebel
  2008-08-06 22:14 ` Matt Mackall
@ 2008-08-11 19:10 ` Ingo Molnar
  2008-08-11 19:23   ` H. Peter Anvin
  2 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2008-08-11 19:10 UTC (permalink / raw)
  To: Tim Bird
  Cc: linux kernel, linux-embedded, Matt Mackall, H. Peter Anvin,
	Thomas Gleixner


* Tim Bird <tim.bird@am.sony.com> wrote:

> Add support for a built-in command line for x86 architectures. The 
> Kconfig help gives the major rationale for this addition.

i have actually used a local hack quite similar to this to inject boot 
options into bzImages via randconfig - so i would find this feature 
rather useful.

a small observation:

> +	/* append boot loader cmdline to builtin, unless builtin overrides it */
> +	if (builtin_cmdline[0] != '!') {
> +		strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
> +		strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
> +		strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
> +	} else {
> +		strlcpy(boot_command_line, &builtin_cmdline[1],
> +			COMMAND_LINE_SIZE);
> +	}

the default branch changes existing command lines slightly: it appends a 
space to them. This could break scripts that rely on the precise 
contents of /proc/cmdline output. (i have some - they are arguably 
dodgy)

Best would be to make it really apparent in the code that nothing 
changes if this config option is not set. Preferably there should be no 
extra code at all in that case.

	Ingo

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-11 19:10 ` Ingo Molnar
@ 2008-08-11 19:23   ` H. Peter Anvin
  2008-08-11 22:40     ` Tim Bird
  2008-08-12 19:52     ` [PATCH] bootup: Add built-in kernel command line for x86 (v2) Tim Bird
  0 siblings, 2 replies; 17+ messages in thread
From: H. Peter Anvin @ 2008-08-11 19:23 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Tim Bird, linux kernel, linux-embedded, Matt Mackall,
	Thomas Gleixner

Ingo Molnar wrote:
> * Tim Bird <tim.bird@am.sony.com> wrote:
> 
>> Add support for a built-in command line for x86 architectures. The 
>> Kconfig help gives the major rationale for this addition.
> 
> i have actually used a local hack quite similar to this to inject boot 
> options into bzImages via randconfig - so i would find this feature 
> rather useful.
> 
> a small observation:
> 
>> +	/* append boot loader cmdline to builtin, unless builtin overrides it */
>> +	if (builtin_cmdline[0] != '!') {
>> +		strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
>> +		strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
>> +		strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
>> +	} else {
>> +		strlcpy(boot_command_line, &builtin_cmdline[1],
>> +			COMMAND_LINE_SIZE);
>> +	}
> 
> the default branch changes existing command lines slightly: it appends a 
> space to them. This could break scripts that rely on the precise 
> contents of /proc/cmdline output. (i have some - they are arguably 
> dodgy)
> 
> Best would be to make it really apparent in the code that nothing 
> changes if this config option is not set. Preferably there should be no 
> extra code at all in that case.
> 

I would like to see this:

#ifdef CONFIG_BUILTIN_CMDLINE
# ifdef CONFIG_BUILTIN_CMDLINE_OVERRIDE
	strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
# else
	if (boot_command_line) {
		strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
		strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
	}
# endif
#endif

	-hpa

		

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86
  2008-08-11 19:23   ` H. Peter Anvin
@ 2008-08-11 22:40     ` Tim Bird
  2008-08-12 19:52     ` [PATCH] bootup: Add built-in kernel command line for x86 (v2) Tim Bird
  1 sibling, 0 replies; 17+ messages in thread
From: Tim Bird @ 2008-08-11 22:40 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Ingo Molnar, linux kernel, linux-embedded, Matt Mackall,
	Thomas Gleixner

H. Peter Anvin wrote:
> Ingo Molnar wrote:
>> * Tim Bird <tim.bird@am.sony.com> wrote:
>>
>>> Add support for a built-in command line for x86 architectures. The
>>> Kconfig help gives the major rationale for this addition.
>>
>> i have actually used a local hack quite similar to this to inject boot
>> options into bzImages via randconfig - so i would find this feature
>> rather useful.
>>
>> a small observation:
>>
>>> +    /* append boot loader cmdline to builtin, unless builtin
>>> overrides it */
>>> +    if (builtin_cmdline[0] != '!') {
>>> +        strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
>>> +        strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
>>> +        strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
>>> +    } else {
>>> +        strlcpy(boot_command_line, &builtin_cmdline[1],
>>> +            COMMAND_LINE_SIZE);
>>> +    }
>>
>> the default branch changes existing command lines slightly: it appends
>> a space to them. This could break scripts that rely on the precise
>> contents of /proc/cmdline output. (i have some - they are arguably dodgy)

Yeah, I wasn't too comfortable with that.

>> Best would be to make it really apparent in the code that nothing
>> changes if this config option is not set. Preferably there should be
>> no extra code at all in that case.

Agreed.

> 
> I would like to see this:
> 
> #ifdef CONFIG_BUILTIN_CMDLINE
> # ifdef CONFIG_BUILTIN_CMDLINE_OVERRIDE
>     strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
> # else
>     if (boot_command_line) {
>         strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
>         strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
>     }
> # endif
> #endif

You missed copying builtin_cmdline back to boot_command_line, but
in general this looks OK to me.  If nobody objects to the ifdef
multiplicity, I'll work up a version tomorrow for review.  (Sorry,
I'm a bit swamped today.)
 -- Tim

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86 (v2)
  2008-08-11 19:23   ` H. Peter Anvin
  2008-08-11 22:40     ` Tim Bird
@ 2008-08-12 19:52     ` Tim Bird
  2008-08-15 14:03       ` Ingo Molnar
  2008-08-15 14:12       ` Ingo Molnar
  1 sibling, 2 replies; 17+ messages in thread
From: Tim Bird @ 2008-08-12 19:52 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Ingo Molnar, linux kernel, linux-embedded, Matt Mackall,
	Thomas Gleixner

Allow x86 to support a built-in kernel command line.  The built-in
command line can override the one provided by the boot loader, for
those cases where the boot loader is broken or it is difficult
to change the command line in the the boot loader.

Signed-off-by: Tim Bird <tim.bird@am.sony.com>
---
 arch/x86/Kconfig        |   45 +++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/setup.c |   16 ++++++++++++++++
 2 files changed, 61 insertions(+), 0 deletions(-)

H. Peter Anvin wrote:
> Ingo Molnar wrote:
>> Best would be to make it really apparent in the code that nothing
>> changes if this config option is not set. Preferably there should be
>> no extra code at all in that case.
>>
>
> I would like to see this:
[...Nested ifdefs...]

OK. This version changes absolutely nothing if CONFIG_CMDLINE_BOOL is not
set (the default).  Also, no space is appended even when CONFIG_CMDLINE_BOOL
is set, but the builtin string is empty.  This is less sloppy all the way
around, IMHO.

Note that I use the same option names as on other arches for
this feature.

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3d0f2b6..f7bbbd7 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1393,6 +1393,51 @@ config COMPAT_VDSO

 	  If unsure, say Y.

+config CMDLINE_BOOL
+	bool "Built-in kernel command line"
+	default n
+	help
+	  Allow for specifying boot arguments to the kernel at
+	  build time.  On some systems (e.g. embedded ones), it is
+	  necessary or convenient to provide some or all of the
+	  kernel boot arguments with the kernel itself (that is,
+	  to not rely on the boot loader to provide them.)
+
+	  To compile command line arguments into the kernel,
+         set this option to 'Y', then fill in the
+	  the boot arguments in CONFIG_CMDLINE.
+
+	  Systems with fully functional boot loaders (i.e. non-embedded)
+	  should leave this option set to 'N'.
+
+config CMDLINE
+	string "Built-in kernel command string"
+	depends on CMDLINE_BOOL
+	default ""
+	help
+	  Enter arguments here that should be compiled into the kernel
+	  image and used at boot time.  If the boot loader provides a
+	  command line at boot time, it is appended to this string to
+	  form the full kernel command line, when the system boots.
+
+	  However, you can use the CONFIG_CMDLINE_OVERRIDE option to
+	  change this behavior.
+
+         In most cases, the command line (whether built-in or provided
+	  by the boot loader) should specify the device for the root
+	  file system.
+
+config CMDLINE_OVERRIDE
+	bool "Built-in command line overrides boot loader arguments"
+	default n
+	depends on CMDLINE_BOOL
+	help
+	  Set this option to 'Y' to have the kernel ignore the boot loader
+	  command line, and use ONLY the built-in command line.
+
+	  This is used to work around broken boot loaders.  This should
+	  be set to 'N' under normal conditions.
+
 endmenu

 config ARCH_ENABLE_MEMORY_HOTPLUG
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 2d88858..492610c 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -223,6 +223,9 @@ unsigned long saved_video_mode;
 #define RAMDISK_LOAD_FLAG		0x4000

 static char __initdata command_line[COMMAND_LINE_SIZE];
+#ifdef CONFIG_CMDLINE_BOOL
+static char __initdata builtin_cmdline[COMMAND_LINE_SIZE] = CONFIG_CMDLINE;
+#endif

 #if defined(CONFIG_EDD) || defined(CONFIG_EDD_MODULE)
 struct edd edd;
@@ -665,6 +668,19 @@ void __init setup_arch(char **cmdline_p)
 	bss_resource.start = virt_to_phys(&__bss_start);
 	bss_resource.end = virt_to_phys(&__bss_stop)-1;

+#ifdef CONFIG_CMDLINE_BOOL
+#ifdef CONFIG_CMDLINE_OVERRIDE
+	strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+#else
+	if (builtin_cmdline[0]) {
+		/* append boot loader cmdline to builtin */
+		strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
+		strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
+		strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+	}
+#endif
+#endif
+
 	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
 	*cmdline_p = command_line;

-- 
1.5.6

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86 (v2)
  2008-08-12 19:52     ` [PATCH] bootup: Add built-in kernel command line for x86 (v2) Tim Bird
@ 2008-08-15 14:03       ` Ingo Molnar
  2008-08-15 14:12       ` Ingo Molnar
  1 sibling, 0 replies; 17+ messages in thread
From: Ingo Molnar @ 2008-08-15 14:03 UTC (permalink / raw)
  To: Tim Bird
  Cc: H. Peter Anvin, linux kernel, linux-embedded, Matt Mackall,
	Thomas Gleixner


* Tim Bird <tim.bird@am.sony.com> wrote:

> Allow x86 to support a built-in kernel command line.  The built-in 
> command line can override the one provided by the boot loader, for 
> those cases where the boot loader is broken or it is difficult to 
> change the command line in the the boot loader.
> 
> Signed-off-by: Tim Bird <tim.bird@am.sony.com>

applied to tip/x86/commandline for v2.6.28 merging - thanks Tim!

	Ingo

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

* Re: [PATCH] bootup: Add built-in kernel command line for x86 (v2)
  2008-08-12 19:52     ` [PATCH] bootup: Add built-in kernel command line for x86 (v2) Tim Bird
  2008-08-15 14:03       ` Ingo Molnar
@ 2008-08-15 14:12       ` Ingo Molnar
  1 sibling, 0 replies; 17+ messages in thread
From: Ingo Molnar @ 2008-08-15 14:12 UTC (permalink / raw)
  To: Tim Bird
  Cc: H. Peter Anvin, linux kernel, linux-embedded, Matt Mackall,
	Thomas Gleixner


* Tim Bird <tim.bird@am.sony.com> wrote:

> +	  To compile command line arguments into the kernel,
> +         set this option to 'Y', then fill in the

FYI, this tab-to-space whitespace error caused this build failure in 
-tip testing:

arch/x86/Kconfig:1500: unknown option "set"
arch/x86/Kconfig:1501: unknown option "the"
arch/x86/Kconfig:1503: unknown option "Systems"
arch/x86/Kconfig:1504: unknown option "should"
arch/x86/Kconfig:1519: unknown option "In"
arch/x86/Kconfig:1520: unknown option "by"
arch/x86/Kconfig:1521: unknown option "file"

i fixed it up. This line was broken too:

> +         In most cases, the command line (whether built-in or provided

	Ingo

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

end of thread, other threads:[~2008-08-15 14:12 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-06 21:31 [PATCH] bootup: Add built-in kernel command line for x86 Tim Bird
2008-08-06 22:04 ` Robert Schwebel
2008-08-06 22:22   ` Tim Bird
2008-08-06 22:48     ` H. Peter Anvin
2008-08-06 22:52       ` Matt Mackall
2008-08-06 23:02         ` H. Peter Anvin
2008-08-06 23:00       ` Tim Bird
2008-08-06 23:03         ` H. Peter Anvin
2008-08-06 22:14 ` Matt Mackall
2008-08-06 22:31   ` Tim Bird
2008-08-06 22:48     ` Matt Mackall
2008-08-11 19:10 ` Ingo Molnar
2008-08-11 19:23   ` H. Peter Anvin
2008-08-11 22:40     ` Tim Bird
2008-08-12 19:52     ` [PATCH] bootup: Add built-in kernel command line for x86 (v2) Tim Bird
2008-08-15 14:03       ` Ingo Molnar
2008-08-15 14:12       ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).