Re: [PATCH] x86 built-in command line

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 12, 2006 at 10:12:47AM -0700, H. Peter Anvin wrote:
> Followup to:  <[email protected]>
> By author:    Matt Mackall <[email protected]>
> In newsgroup: linux.dev.kernel
> >
> > This patch allows building in a kernel command line on x86 as is
> > possible on several other arches.
> > 
> > Index: linux/arch/i386/kernel/setup.c
> > ===================================================================
> > --- linux.orig/arch/i386/kernel/setup.c	2006-05-26 16:18:13.000000000 -0500
> > +++ linux/arch/i386/kernel/setup.c	2006-06-11 16:23:51.000000000 -0500
> > @@ -713,6 +713,10 @@ static void __init parse_cmdline_early (
> >  	int len = 0;
> >  	int userdef = 0;
> >  
> > +#ifdef CONFIG_CMDLINE_BOOL
> > +	strlcpy(saved_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > +#endif
> > +
> >  	/* Save unparsed command line copy for /proc/cmdline */
> >  	saved_command_line[COMMAND_LINE_SIZE-1] = '\0';
> >  
> 
> NAK.
> 
> a. Please make the patch available for x86-64 as well as x86.  The two
> are coupled enough that they need to agree.

I don't think that's a show-stopper. I'll provide one after we decide
how this should work.
 
> b. This patch will override a user-provided command line if one
> exists.  This is the wrong behaviour; instead, the builtin command
> line should only apply if no user-specified command line is present.

There are four possible behaviors:

a) internal supercedes external (what's in the patch)
b) external supercedes internal (what you proposed)
c) external is appended to internal (what Tim proposed)
d) internal is appended to external (not very interesting)

And there are some possible uses:

1) bootloader doesn't support command line
2) bootloader has hardcoded or otherwise difficult-to-change command line
3) automated testing for tftp boot, etc.
4) bypassing boot protocol command line length (not yet supported)
5) setting defaults like ACPI off, etc.

(a) works for 1, 2, 3, and 4.
(b) works for 1, 3, and 4, provided you clear the command line in your
boot loader
(c) works for 1, 3, and 4, provided you're not trying to override
earlier arguments

(5) generally is unworkable because our parser doesn't generally do the
right thing for options like "acpi=on acpi=off" (though it might work
in the specific case of acpi, haven't checked). If, for instance, you
try to override console, you'll get two consoles.

So I think (a) is the way to go. If there's a use case for (b) that's
important, it's not jumping out at me. Finally, at least the arch I
inspected when preparing this patch had gone with (a) too.

(As an example of (2), the last x86 bootloader I dealt with was written
in Forth, and getting a fresh build of it meant getting some cycles
from the last two Forth hackers on the planet.)

-- 
Mathematics is the supreme nostalgia of our time.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux