Re: [RFC 5/8] param: arch_get_boot_command_line()

From: Rusty Russell
Date: Tue Dec 02 2008 - 19:42:36 EST

On Wednesday 03 December 2008 04:14:57 Russell King wrote:
> On Tue, Dec 02, 2008 at 12:43:37PM +1030, Rusty Russell wrote:
> > A couple of #if 0 around code I don't think can happen (even in the
> > orignal place I moved it from?)
>
> Looking at just those...
>
> > @@ -697,32 +693,48 @@ void __init setup_arch(char **cmdline_p)
> > */
> > if (tags->hdr.tag != ATAG_CORE)
> > convert_to_tag_list(tags);
> > +#if 0
> > if (tags->hdr.tag != ATAG_CORE)
> > tags = (struct tag *)&init_tags;
> > +#endif
>
> This prevents 'init_tags' from ever being used, which wil happen if
> convert_to_tag_list() doesn't find a param_struct to convert.

Ah, I missed this test in convert_to_tag_list:

if (params->u1.s.page_size != PAGE_SIZE) {
printk(KERN_WARNING "Warning: bad configuration page, "
"trying to continue\n");
return;
}

Here's the new arm version, followed by an __early_param conversion.
Compile-tested only.

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -115,7 +115,6 @@ static struct meminfo meminfo __initdata
static struct meminfo meminfo __initdata = { 0, };
static const char *cpu_name;
static const char *machine_name;
-static char __initdata command_line[COMMAND_LINE_SIZE];

static char default_command_line[COMMAND_LINE_SIZE] __initdata = CONFIG_CMDLINE;
static union { char c[4]; unsigned long l; } endian_test __initdata = { { 'l', '?', '?', 'b'
} };
@@ -414,10 +413,12 @@ __early_param("mem=", early_mem);

/*
* Initial parsing of the command line.
+ * FIXME: Use generic core_param. This actually removes args from the
+ * cmdline as seen in /proc!
*/
-static void __init parse_cmdline(char **cmdline_p, char *from)
+static void __init parse_cmdline(char *from)
{
- char c = ' ', *to = command_line;
+ char c = ' ', *to = boot_command_line;
int len = 0;

for (;;) {
@@ -429,7 +430,7 @@ static void __init parse_cmdline(char **
int arglen = strlen(p->arg);

if (memcmp(from, p->arg, arglen) == 0) {
- if (to != command_line)
+ if (to != boot_command_line)
to -= 1;
from += arglen;
p->fn(&from);
@@ -448,7 +449,6 @@ static void __init parse_cmdline(char **
*to++ = c;
}
*to = '\0';
- *cmdline_p = command_line;
}

static void __init
@@ -673,7 +673,8 @@ static int __init customize_machine(void
}
arch_initcall(customize_machine);

-void __init setup_arch(char **cmdline_p)
+/* We not only get the command line here, we parse the tags as well. */
+void __init arch_get_boot_command_line(void)
{
struct tag *tags = (struct tag *)&init_tags;
struct machine_desc *mdesc;
@@ -681,10 +682,6 @@ void __init setup_arch(char **cmdline_p)

setup_processor();
mdesc = setup_machine(machine_arch_type);
- machine_name = mdesc->name;
-
- if (mdesc->soft_reboot)
- reboot_setup("s");

if (__atags_pointer)
tags = phys_to_virt(__atags_pointer);
@@ -703,21 +700,29 @@ void __init setup_arch(char **cmdline_p)
if (mdesc->fixup)
mdesc->fixup(mdesc, tags, &from, &meminfo);

- if (tags->hdr.tag == ATAG_CORE) {
- if (meminfo.nr_banks != 0)
- squash_mem_tags(tags);
- save_atags(tags);
- parse_tags(tags);
- }
+ if (meminfo.nr_banks != 0)
+ squash_mem_tags(tags);
+ save_atags(tags);
+ parse_tags(tags);
+
+ /* This copies into boot_command_line */
+ parse_cmdline(from);
+}
+
+void __init setup_arch(void)
+{
+ struct machine_desc *mdesc = setup_machine(machine_arch_type);
+
+ machine_name = mdesc->name;
+
+ if (mdesc->soft_reboot)
+ reboot_setup("s");

init_mm.start_code = (unsigned long) &_text;
init_mm.end_code = (unsigned long) &_etext;
init_mm.end_data = (unsigned long) &_edata;
init_mm.brk = (unsigned long) &_end;

- memcpy(boot_command_line, from, COMMAND_LINE_SIZE);
- boot_command_line[COMMAND_LINE_SIZE-1] = '\0';
- parse_cmdline(cmdline_p, from);
paging_init(&meminfo, mdesc);
request_standard_resources(&meminfo, mdesc);

===
arm: use generic early_param

early_param() was inspired by arm's __early_param: now it's called earlier
in generic code, it can be used by arm as well.

This also allows us to get rid of default_command_line.

A possible cleanup would be to have fortunet_fixup do the strlcpy into
boot_command_line itself, and remove the &cmdline arg to ->fixup.

Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>

diff --git a/arch/arm/include/asm/setup.h b/arch/arm/include/asm/setup.h
--- a/arch/arm/include/asm/setup.h
+++ b/arch/arm/include/asm/setup.h
@@ -220,18 +220,6 @@ struct meminfo {
#define bank_phys_end(bank) ((bank)->start + (bank)->size)
#define bank_phys_size(bank) (bank)->size

-/*
- * Early command line parameters.
- */
-struct early_params {
- const char *arg;
- void (*fn)(char **p);
-};
-
-#define __early_param(name,fn) \
-static struct early_params __early_##fn __used \
-__attribute__((__section__(".early_param.init"))) = { name, fn }
-
#endif /* __KERNEL__ */

#endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -116,7 +116,6 @@ static const char *cpu_name;
static const char *cpu_name;
static const char *machine_name;

-static char default_command_line[COMMAND_LINE_SIZE] __initdata = CONFIG_CMDLINE;
static union { char c[4]; unsigned long l; } endian_test __initdata = { { 'l', '?', '?', 'b'
} };
#define ENDIANNESS ((char)endian_test.l)

@@ -387,10 +386,13 @@ static void __init arm_add_memory(unsign
* Pick out the memory size. We look for mem=size@start,
* where start and size are "size[KkMm]"
*/
-static void __init early_mem(char **p)
+static int __init early_mem(char *p)
{
static int usermem __initdata = 0;
unsigned long size, start;
+
+ if (!p)
+ return -EINVAL;

/*
* If the user specifies memory size, we
@@ -403,53 +405,14 @@ static void __init early_mem(char **p)
}

start = PHYS_OFFSET;
- size = memparse(*p, p);
- if (**p == '@')
- start = memparse(*p + 1, p);
+ size = memparse(p, &p);
+ if (*p == '@')
+ start = memparse(p + 1, NULL);

arm_add_memory(start, size);
+ return 0;
}
-__early_param("mem=", early_mem);
-
-/*
- * Initial parsing of the command line.
- * FIXME: Use generic core_param. This actually removes args from the
- * cmdline as seen in /proc!
- */
-static void __init parse_cmdline(char *from)
-{
- char c = ' ', *to = boot_command_line;
- int len = 0;
-
- for (;;) {
- if (c == ' ') {
- extern struct early_params __early_begin, __early_end;
- struct early_params *p;
-
- for (p = &__early_begin; p < &__early_end; p++) {
- int arglen = strlen(p->arg);
-
- if (memcmp(from, p->arg, arglen) == 0) {
- if (to != boot_command_line)
- to -= 1;
- from += arglen;
- p->fn(&from);
-
- while (*from != ' ' && *from != '\0')
- from++;
- break;
- }
- }
- }
- c = *from++;
- if (!c)
- break;
- if (COMMAND_LINE_SIZE <= ++len)
- break;
- *to++ = c;
- }
- *to = '\0';
-}
+early_param("mem", early_mem);

static void __init
setup_ramdisk(int doload, int prompt, int image_start, unsigned int rd_sz)
@@ -607,7 +570,7 @@ __tagtable(ATAG_REVISION, parse_tag_revi

static int __init parse_tag_cmdline(const struct tag *tag)
{
- strlcpy(default_command_line, tag->u.cmdline.cmdline, COMMAND_LINE_SIZE);
+ strlcpy(boot_command_line, tag->u.cmdline.cmdline, COMMAND_LINE_SIZE);
return 0;
}

@@ -678,7 +641,8 @@ void __init arch_get_boot_command_line(v
{
struct tag *tags = (struct tag *)&init_tags;
struct machine_desc *mdesc;
- char *from = default_command_line;
+
+ strcpy(boot_command_line, CONFIG_CMDLINE);

setup_processor();
mdesc = setup_machine(machine_arch_type);
@@ -697,16 +661,17 @@ void __init arch_get_boot_command_line(v
if (tags->hdr.tag != ATAG_CORE)
tags = (struct tag *)&init_tags;

- if (mdesc->fixup)
+ if (mdesc->fixup) {
+ char *from = boot_command_line;
mdesc->fixup(mdesc, tags, &from, &meminfo);
+ if (from != boot_command_line)
+ strlcpy(boot_command_line, from, COMMAND_LINE_SIZE);
+ }

if (meminfo.nr_banks != 0)
squash_mem_tags(tags);
save_atags(tags);
parse_tags(tags);
-
- /* This copies into boot_command_line */
- parse_cmdline(from);
}

void __init setup_arch(void)
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -45,9 +45,6 @@ SECTIONS
__setup_start = .;
*(.init.setup)
__setup_end = .;
- __early_begin = .;
- *(.early_param.init)
- __early_end = .;
__initcall_start = .;
INITCALLS
__initcall_end = .;
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -29,19 +29,24 @@ static unsigned long phys_initrd_start _
static unsigned long phys_initrd_start __initdata = 0;
static unsigned long phys_initrd_size __initdata = 0;

-static void __init early_initrd(char **p)
+static int __init early_initrd(char *p)
{
unsigned long start, size;

- start = memparse(*p, p);
- if (**p == ',') {
- size = memparse((*p) + 1, p);
+ if (!p)
+ return -EINVAL;

- phys_initrd_start = start;
- phys_initrd_size = size;
- }
+ start = memparse(p, &p);
+ if (*p != ',')
+ return -EINVAL;
+
+ size = memparse(p + 1, &p);
+
+ phys_initrd_start = start;
+ phys_initrd_size = size;
+ return 0;
}
-__early_param("initrd=", early_initrd);
+early_param("initrd", early_initrd);

static int __init parse_tag_initrd(const struct tag *tag)
{
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -96,18 +96,21 @@ static struct cachepolicy cache_policies
* writebuffer to be turned off. (Note: the write
* buffer should not be on and the cache off).
*/
-static void __init early_cachepolicy(char **p)
+static int __init early_cachepolicy(char *p)
{
int i;
+
+ if (!p)
+ return -EINVAL;

for (i = 0; i < ARRAY_SIZE(cache_policies); i++) {
int len = strlen(cache_policies[i].policy);

- if (memcmp(*p, cache_policies[i].policy, len) == 0) {
+ if (memcmp(p, cache_policies[i].policy, len) == 0) {
cachepolicy = i;
cr_alignment &= ~cache_policies[i].cr_mask;
cr_no_alignment &= ~cache_policies[i].cr_mask;
- *p += len;
+ p += len;
break;
}
}
@@ -119,36 +122,42 @@ static void __init early_cachepolicy(cha
}
flush_cache_all();
set_cr(cr_alignment);
+ return 0;
}
-__early_param("cachepolicy=", early_cachepolicy);
+early_param("cachepolicy", early_cachepolicy);

-static void __init early_nocache(char **__unused)
+static int __init early_nocache(char *p)
{
- char *p = "buffered";
+ p = "buffered";
printk(KERN_WARNING "nocache is deprecated; use cachepolicy=%s\n", p);
- early_cachepolicy(&p);
+ return early_cachepolicy(p);
}
-__early_param("nocache", early_nocache);
+early_param("nocache", early_nocache);

-static void __init early_nowrite(char **__unused)
+static int __init early_nowrite(char *p)
{
- char *p = "uncached";
+ p = "uncached";
printk(KERN_WARNING "nowb is deprecated; use cachepolicy=%s\n", p);
- early_cachepolicy(&p);
+ return early_cachepolicy(p);
}
-__early_param("nowb", early_nowrite);
+early_param("nowb", early_nowrite);

-static void __init early_ecc(char **p)
+static int __init early_ecc(char *p)
{
- if (memcmp(*p, "on", 2) == 0) {
+ if (!p)
+ return -EINVAL;
+
+ if (memcmp(p, "on", 2) == 0) {
ecc_mask = PMD_PROTECTION;
- *p += 2;
- } else if (memcmp(*p, "off", 3) == 0) {
+ p += 2;
+ } else if (memcmp(p, "off", 3) == 0) {
ecc_mask = 0;
- *p += 3;
- }
+ p += 3;
+ } else
+ return -EINVAL;
+ return 0;
}
-__early_param("ecc=", early_ecc);
+early_param("ecc", early_ecc);

static int __init noalign_setup(char *__unused)
{
@@ -636,9 +645,12 @@ static unsigned long __initdata vmalloc_
* bytes. This can be used to increase (or decrease) the vmalloc
* area - the default is 128m.
*/
-static void __init early_vmalloc(char **arg)
+static void __init early_vmalloc(char *arg)
{
- vmalloc_reserve = memparse(*arg, arg);
+ if (!arg)
+ return -EINVAL;
+
+ vmalloc_reserve = memparse(arg, &arg);

if (vmalloc_reserve < SZ_16M) {
vmalloc_reserve = SZ_16M;
@@ -646,8 +658,9 @@ static void __init early_vmalloc(char **
"vmalloc area too small, limiting to %luMB\n",
vmalloc_reserve >> 20);
}
+ return 0;
}
-__early_param("vmalloc=", early_vmalloc);
+early_param("vmalloc", early_vmalloc);

#define VMALLOC_MIN (void *)(VMALLOC_END - vmalloc_reserve)





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/