2008-12-23 03:05:32

by Hugh Dickins

[permalink] [raw]
Subject: linux-next: parsing mem=700M broken

Hi Rusty,

I find that bootparam "mem=700M" isn't working in linux-next or mmotm,
and have bisected it down to your patch below; but now I'm off to bed
without working out just what goes wrong (I'll bet it's the "=").

Hugh

commit 5c886584a758edba7e25ad9df974cf15a4a1f59d
Author: Rusty Russell <[email protected]>
Date: Wed Dec 3 13:34:34 2008 +1030

Call early_param earlier.

We delete all the arch calls: we call it from start_kernel earlier
now. We also no longer take a temporary copy, but parse in place.

Note: IA64 needs to parse "machvec=" before other commandline options
but machvec_init_from_cmdline() needs efi_init() and io_port_init(),
so they are all moved into arch_get_boot_command_line().

Signed-off-by: Rusty Russell <[email protected]>
Acked-by: Tony Luck <[email protected]>

diff --git a/arch/avr32/kernel/setup.c b/arch/avr32/kernel/setup.c
index fd27063..7b4c0eb 100644
--- a/arch/avr32/kernel/setup.c
+++ b/arch/avr32/kernel/setup.c
@@ -580,8 +580,6 @@ void __init setup_arch(void)
((cpu_hz + 500) / 1000) % 1000);
}

- parse_early_param();
-
setup_bootmem();

#ifdef CONFIG_VT
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 74868fa..0951971 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -533,15 +533,6 @@ void __init arch_get_boot_command_line(void)
{
strlcpy(boot_command_line, __va(ia64_boot_param->command_line),
COMMAND_LINE_SIZE);
-}
-
-void __init setup_arch(void)
-{
- unw_init();
-
- paravirt_arch_setup_early();
-
- ia64_patch_vtop((u64) __start___vtop_patchlist, (u64) __end___vtop_patchlist);

efi_init();
io_port_init();
@@ -554,8 +545,15 @@ void __init setup_arch(void)
*/
machvec_init_from_cmdline(boot_command_line);
#endif
+}
+
+void __init setup_arch(void)
+{
+ unw_init();
+
+ paravirt_arch_setup_early();

- parse_early_param();
+ ia64_patch_vtop((u64) __start___vtop_patchlist, (u64) __end___vtop_patchlist);

if (early_console_setup(boot_command_line) == 0)
mark_bsp_online();
diff --git a/arch/m68k/kernel/setup.c b/arch/m68k/kernel/setup.c
index ebb870b..d78e3c9 100644
--- a/arch/m68k/kernel/setup.c
+++ b/arch/m68k/kernel/setup.c
@@ -268,8 +268,6 @@ void __init setup_arch(void)
init_mm.end_data = (unsigned long) &_edata;
init_mm.brk = (unsigned long) &_end;

- parse_early_param();
-
#ifdef CONFIG_DUMMY_CONSOLE
conswitchp = &dummy_con;
#endif
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 2d9d479..7c02f39 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -482,8 +482,6 @@ static void __init arch_mem_init(void)
pr_info("Determined physical RAM map:\n");
print_memory_map();

- parse_early_param();
-
if (usermem) {
pr_info("User-defined physical RAM map:\n");
print_memory_map();
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 3a2dc7e..d36cbd0 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -1188,7 +1188,6 @@ void __init early_init_devtree(void *params)

/* Save command line for /proc/cmdline and then parse parameters */
strlcpy(boot_command_line, cmd_line, COMMAND_LINE_SIZE);
- parse_early_param();

/* Reserve LMB regions used by kernel, initrd, dt, etc... */
lmb_reserve(PHYSICAL_START, __pa(klimit) - PHYSICAL_START);
diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
index f736c68..7cb2868 100644
--- a/arch/s390/kernel/setup.c
+++ b/arch/s390/kernel/setup.c
@@ -785,8 +785,6 @@ setup_arch(void)
else
memcpy(&uaccess, &uaccess_std, sizeof(uaccess));

- parse_early_param();
-
setup_ipl();
setup_memory_end();
setup_addressing_mode();
diff --git a/arch/sh/kernel/setup.c b/arch/sh/kernel/setup.c
index 58089b3..2fe5992 100644
--- a/arch/sh/kernel/setup.c
+++ b/arch/sh/kernel/setup.c
@@ -382,8 +382,6 @@ void __init setup_arch(void)
if (!memory_end)
memory_end = memory_start + __MEMORY_SIZE;

- parse_early_param();
-
sh_mv_setup();

/*
diff --git a/arch/sparc/kernel/setup.c b/arch/sparc/kernel/setup.c
index 883bade..dac4a9e 100644
--- a/arch/sparc/kernel/setup.c
+++ b/arch/sparc/kernel/setup.c
@@ -215,9 +215,6 @@ void __init setup_arch(void)

sparc_ttable = (struct tt_entry *) &start;

- /* Initialize PROM console. */
- parse_early_param();
-
/* Set sparc_cpu_model */
sparc_cpu_model = sun_unknown;
if (!strcmp(&cputypval,"sun4 "))
diff --git a/arch/sparc64/kernel/setup.c b/arch/sparc64/kernel/setup.c
index 8827c2a..431cb97 100644
--- a/arch/sparc64/kernel/setup.c
+++ b/arch/sparc64/kernel/setup.c
@@ -283,9 +283,6 @@ void __init arch_get_boot_command_line(void)

void __init setup_arch(void)
{
- /* Initialize PROM console. */
- parse_early_param();
-
boot_flags_init(boot_command_line);
register_console(&prom_early_console);

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 1669ce2..13fec0e 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -876,8 +876,6 @@ void __init setup_arch(void)
bss_resource.start = virt_to_phys(&__bss_start);
bss_resource.end = virt_to_phys(&__bss_stop)-1;

- parse_early_param();
-
#ifdef CONFIG_X86_64
check_efer();
#endif
@@ -915,7 +913,6 @@ void __init setup_arch(void)
probe_roms();
#endif

- /* after parse_early_param, so could debug it */
insert_resource(&iomem_resource, &code_resource);
insert_resource(&iomem_resource, &data_resource);
insert_resource(&iomem_resource, &bss_resource);
diff --git a/include/linux/init.h b/include/linux/init.h
index 9b57a61..8cd1a93 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -246,8 +246,6 @@ struct obs_kernel_param {
#define early_param(str, fn) \
__setup_param(str, fn, fn, 1)

-/* Relies on boot_command_line being set */
-void __init parse_early_param(void);
#endif /* __ASSEMBLY__ */

/**
diff --git a/init/main.c b/init/main.c
index 718027b..145579d 100644
--- a/init/main.c
+++ b/init/main.c
@@ -206,7 +206,7 @@ static int __init obsolete_checksetup(char *line)
int n = strlen(p->str);
if (!strncmp(line, p->str, n)) {
if (p->early) {
- /* Already done in parse_early_param?
+ /* Already done in do_early_param?
* (Needs exact match on param part).
* Keep iterating, as we can have early
* params and __setups of same names 8( */
@@ -523,21 +523,6 @@ static int __init do_early_param(char *param, char *val)
return 0;
}

-/* Arch code calls this early on, or if not, just before other parsing. */
-void __init parse_early_param(void)
-{
- static __initdata int done = 0;
- static __initdata char tmp_cmdline[COMMAND_LINE_SIZE];
-
- if (done)
- return;
-
- /* All fall through to do_early_param. */
- strlcpy(tmp_cmdline, boot_command_line, COMMAND_LINE_SIZE);
- parse_args("early options", tmp_cmdline, NULL, 0, do_early_param, true);
- done = 1;
-}
-
/*
* Activate the first processor.
*/
@@ -566,6 +551,9 @@ asmlinkage void __init start_kernel(void)
parse_args("Core params", boot_command_line, __start___core_param,
__stop___core_param - __start___core_param,
unknown_core_ok, true);
+ /* All fall through to do_early_param. */
+ parse_args("early options", boot_command_line, NULL, 0, do_early_param,
+ true);

smp_setup_processor_id();

@@ -615,7 +603,6 @@ asmlinkage void __init start_kernel(void)
build_all_zonelists();
page_alloc_init();
printk(KERN_NOTICE "Kernel command line: %s\n", boot_command_line);
- parse_early_param();
parse_args("Booting kernel", static_command_line, __start___param,
__stop___param - __start___param,
&unknown_bootoption, false);


2008-12-23 05:52:29

by Yinghai Lu

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Mon, Dec 22, 2008 at 7:06 PM, Hugh Dickins <[email protected]> wrote:
> Hi Rusty,
>
> I find that bootparam "mem=700M" isn't working in linux-next or mmotm,
> and have bisected it down to your patch below; but now I'm off to bed
> without working out just what goes wrong (I'll bet it's the "=").
>
> Hugh
>
> commit 5c886584a758edba7e25ad9df974cf15a4a1f59d
> Author: Rusty Russell <[email protected]>
> Date: Wed Dec 3 13:34:34 2008 +1030
>
> Call early_param earlier.
>
> We delete all the arch calls: we call it from start_kernel earlier
> now. We also no longer take a temporary copy, but parse in place.
>
> Note: IA64 needs to parse "machvec=" before other commandline options
> but machvec_init_from_cmdline() needs efi_init() and io_port_init(),
> so they are all moved into arch_get_boot_command_line().
>

yeah, you are right.

static int __init parse_memopt(char *p)
{
u64 mem_size;

if (!p)
return -EINVAL;

#ifdef CONFIG_X86_32
if (!strcmp(p, "nopentium")) {
setup_clear_cpu_cap(X86_FEATURE_PSE);
return 0;
}
#endif

userdef = 1;
mem_size = memparse(p, &p);
e820_remove_range(mem_size, ULLONG_MAX - mem_size, E820_RAM, 1);

return 0;
}
early_param("mem", parse_memopt);


in x86 we use that param analyser much later ( after we check the e820 table..)

other cpu flag will be overwrite too..

anyway, this patch is not go through x86 tip..., some strange!

YH

2008-12-23 14:34:17

by Ingo Molnar

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken


* Yinghai Lu <[email protected]> wrote:

> On Mon, Dec 22, 2008 at 7:06 PM, Hugh Dickins <[email protected]> wrote:
> > Hi Rusty,
> >
> > I find that bootparam "mem=700M" isn't working in linux-next or mmotm,
> > and have bisected it down to your patch below; but now I'm off to bed
> > without working out just what goes wrong (I'll bet it's the "=").
> >
> > Hugh
> >
> > commit 5c886584a758edba7e25ad9df974cf15a4a1f59d
> > Author: Rusty Russell <[email protected]>
> > Date: Wed Dec 3 13:34:34 2008 +1030
> >
> > Call early_param earlier.
> >
> > We delete all the arch calls: we call it from start_kernel earlier
> > now. We also no longer take a temporary copy, but parse in place.
> >
> > Note: IA64 needs to parse "machvec=" before other commandline options
> > but machvec_init_from_cmdline() needs efi_init() and io_port_init(),
> > so they are all moved into arch_get_boot_command_line().
> >
>
> yeah, you are right.
>
> static int __init parse_memopt(char *p)
> {
> u64 mem_size;
>
> if (!p)
> return -EINVAL;
>
> #ifdef CONFIG_X86_32
> if (!strcmp(p, "nopentium")) {
> setup_clear_cpu_cap(X86_FEATURE_PSE);
> return 0;
> }
> #endif
>
> userdef = 1;
> mem_size = memparse(p, &p);
> e820_remove_range(mem_size, ULLONG_MAX - mem_size, E820_RAM, 1);
>
> return 0;
> }
> early_param("mem", parse_memopt);
>
>
> in x86 we use that param analyser much later ( after we check the e820
> table..)
>
> other cpu flag will be overwrite too..
>
> anyway, this patch is not go through x86 tip..., some strange!

the x86 impact is incidental:

arch/avr32/kernel/setup.c | 2 --
arch/ia64/kernel/setup.c | 18 ++++++++----------
arch/m68k/kernel/setup.c | 2 --
arch/mips/kernel/setup.c | 2 --
arch/powerpc/kernel/prom.c | 1 -
arch/s390/kernel/setup.c | 2 --
arch/sh/kernel/setup.c | 2 --
arch/sparc/kernel/setup.c | 3 ---
arch/sparc64/kernel/setup.c | 3 ---
arch/x86/kernel/setup.c | 3 ---
include/linux/init.h | 2 --
init/main.c | 21 ++++-----------------
12 files changed, 12 insertions(+), 49 deletions(-)

we generally do not require patches to go via the x86 tree that just
happen to touch it with the intention of not breaking anything.

Rusty, will you fix or revert it?

Ingo

2008-12-24 07:38:39

by Rusty Russell

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Wednesday 24 December 2008 01:03:27 Ingo Molnar wrote:
>
> * Yinghai Lu <[email protected]> wrote:
>
> > On Mon, Dec 22, 2008 at 7:06 PM, Hugh Dickins <[email protected]> wrote:
> > > Hi Rusty,
> > >
> > > I find that bootparam "mem=700M" isn't working in linux-next or mmotm,
> > > and have bisected it down to your patch below; but now I'm off to bed
> > > without working out just what goes wrong (I'll bet it's the "=").

Thanks for the report and analysis.

> Rusty, will you fix or revert it?

mem= is actually easy, memmap= is harder. This is tested and pushed.

This commit is actually orthogonal to the other changes, but makes most
sense I think in the bootparam tree.

commit b69cab1078c69a568afdd2eb8ee6f8559b769e2c
Author: Rusty Russell <[email protected]>
Date: Wed Dec 24 18:04:19 2008 +1030

Fix mem= and memmap= parsing: now it happens *before* e820 setup.

Signed-off-by: Rusty Russell <[email protected]>

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 7aafeb5..cab24a6 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -37,6 +37,8 @@
* copied. It doesn't get modified afterwards. It's registered for the
* /sys/firmware/memmap interface.
*
+ * user_e820, exactmap and memlimit are set on cmdline by mem= and memmap=.
+ *
* That memory map is not modified and is used as base for kexec. The kexec'd
* kernel should get the same memory map as the firmware provides. Then the
* user can e.g. boot the original kernel with mem=1G while still booting the
@@ -44,6 +46,9 @@
*/
struct e820map e820;
struct e820map e820_saved;
+static struct e820map user_e820 __initdata;
+static bool exactmap __initdata;
+static u64 memlimit __initdata;

/* For PCI or other memory-mapped resources */
unsigned long pci_mem_start = 0xaeedbabe;
@@ -107,22 +112,28 @@ int __init e820_all_mapped(u64 start, u64 end, unsigned type)
return 0;
}

-/*
- * Add a memory region to the kernel e820 map.
- */
-void __init e820_add_region(u64 start, u64 size, int type)
+static void __init __e820_add_region(struct e820map *e820,
+ u64 start, u64 size, int type)
{
- int x = e820.nr_map;
+ int x = e820->nr_map;

- if (x == ARRAY_SIZE(e820.map)) {
+ if (x == ARRAY_SIZE(e820->map)) {
printk(KERN_ERR "Ooops! Too many entries in the memory map!\n");
return;
}

- e820.map[x].addr = start;
- e820.map[x].size = size;
- e820.map[x].type = type;
- e820.nr_map++;
+ e820->map[x].addr = start;
+ e820->map[x].size = size;
+ e820->map[x].type = type;
+ e820->nr_map++;
+}
+
+/*
+ * Add a memory region to the kernel e820 map.
+ */
+void __init e820_add_region(u64 start, u64 size, int type)
+{
+ __e820_add_region(&e820, start, size, type);
}

void __init e820_print_map(char *who)
@@ -1173,13 +1184,9 @@ static void early_panic(char *msg)
panic(msg);
}

-static int userdef __initdata;
-
/* "mem=nopentium" disables the 4MB page tables. */
static int __init parse_memopt(char *p)
{
- u64 mem_size;
-
if (!p)
return -EINVAL;

@@ -1190,10 +1197,7 @@ static int __init parse_memopt(char *p)
}
#endif

- userdef = 1;
- mem_size = memparse(p, &p);
- e820_remove_range(mem_size, ULLONG_MAX - mem_size, E820_RAM, 1);
-
+ memlimit = memparse(p, &p);
return 0;
}
early_param("mem", parse_memopt);
@@ -1207,16 +1211,7 @@ static int __init parse_memmap_opt(char *p)
return -EINVAL;

if (!strncmp(p, "exactmap", 8)) {
-#ifdef CONFIG_CRASH_DUMP
- /*
- * If we are doing a crash dump, we still need to know
- * the real mem size before original memory map is
- * reset.
- */
- saved_max_pfn = e820_end_of_ram_pfn();
-#endif
- e820.nr_map = 0;
- userdef = 1;
+ exactmap = true;
return 0;
}

@@ -1225,18 +1220,18 @@ static int __init parse_memmap_opt(char *p)
if (p == oldp)
return -EINVAL;

- userdef = 1;
if (*p == '@') {
start_at = memparse(p+1, &p);
- e820_add_region(start_at, mem_size, E820_RAM);
+ __e820_add_region(&user_e820, start_at, mem_size, E820_RAM);
} else if (*p == '#') {
start_at = memparse(p+1, &p);
- e820_add_region(start_at, mem_size, E820_ACPI);
+ __e820_add_region(&user_e820, start_at, mem_size, E820_ACPI);
} else if (*p == '$') {
start_at = memparse(p+1, &p);
- e820_add_region(start_at, mem_size, E820_RESERVED);
+ __e820_add_region(&user_e820, start_at, mem_size,
+ E820_RESERVED);
} else
- e820_remove_range(mem_size, ULLONG_MAX - mem_size, E820_RAM, 1);
+ memlimit = mem_size;

return *p == '\0' ? 0 : -EINVAL;
}
@@ -1244,6 +1239,33 @@ early_param("memmap", parse_memmap_opt);

void __init finish_e820_parsing(void)
{
+ bool userdef;
+ int i;
+
+ if (memlimit) {
+ e820_remove_range(memlimit, ULLONG_MAX - memlimit, E820_RAM, 1);
+ userdef = true;
+ }
+
+ if (exactmap) {
+#ifdef CONFIG_CRASH_DUMP
+ /*
+ * If we are doing a crash dump, we still need to know
+ * the real mem size before original memory map is
+ * reset.
+ */
+ saved_max_pfn = e820_end_of_ram_pfn();
+#endif
+ e820.nr_map = 0;
+ userdef = true;
+ }
+
+ for (i = 0; i < user_e820.nr_map; i++) {
+ e820_add_region(user_e820.map[i].addr, user_e820.map[i].size,
+ user_e820.map[i].type);
+ userdef = true;
+ }
+
if (userdef) {
int nr = e820.nr_map;

2008-12-24 08:10:08

by Yinghai Lu

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

Rusty Russell wrote:
> On Wednesday 24 December 2008 01:03:27 Ingo Molnar wrote:
>> * Yinghai Lu <[email protected]> wrote:
>>
>>> On Mon, Dec 22, 2008 at 7:06 PM, Hugh Dickins <[email protected]> wrote:
>>>> Hi Rusty,
>>>>
>>>> I find that bootparam "mem=700M" isn't working in linux-next or mmotm,
>>>> and have bisected it down to your patch below; but now I'm off to bed
>>>> without working out just what goes wrong (I'll bet it's the "=").
>
> Thanks for the report and analysis.
>
>> Rusty, will you fix or revert it?
>
> mem= is actually easy, memmap= is harder. This is tested and pushed.
>
> This commit is actually orthogonal to the other changes, but makes most
> sense I think in the bootparam tree.
>
> commit b69cab1078c69a568afdd2eb8ee6f8559b769e2c
> Author: Rusty Russell <[email protected]>
> Date: Wed Dec 24 18:04:19 2008 +1030
>
> Fix mem= and memmap= parsing: now it happens *before* e820 setup.
>
> Signed-off-by: Rusty Russell <[email protected]>
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 7aafeb5..cab24a6 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -37,6 +37,8 @@
> * copied. It doesn't get modified afterwards. It's registered for the
> * /sys/firmware/memmap interface.
> *
> + * user_e820, exactmap and memlimit are set on cmdline by mem= and memmap=.
> + *
> * That memory map is not modified and is used as base for kexec. The kexec'd
> * kernel should get the same memory map as the firmware provides. Then the
> * user can e.g. boot the original kernel with mem=1G while still booting the
> @@ -44,6 +46,9 @@
> */
> struct e820map e820;
> struct e820map e820_saved;
> +static struct e820map user_e820 __initdata;
> +static bool exactmap __initdata;
> +static u64 memlimit __initdata;
>
> /* For PCI or other memory-mapped resources */
> unsigned long pci_mem_start = 0xaeedbabe;
> @@ -107,22 +112,28 @@ int __init e820_all_mapped(u64 start, u64 end, unsigned type)
> return 0;
> }
>
> -/*
> - * Add a memory region to the kernel e820 map.
> - */
> -void __init e820_add_region(u64 start, u64 size, int type)
> +static void __init __e820_add_region(struct e820map *e820,
> + u64 start, u64 size, int type)
> {
> - int x = e820.nr_map;
> + int x = e820->nr_map;
>
> - if (x == ARRAY_SIZE(e820.map)) {
> + if (x == ARRAY_SIZE(e820->map)) {
> printk(KERN_ERR "Ooops! Too many entries in the memory map!\n");
> return;
> }
>
> - e820.map[x].addr = start;
> - e820.map[x].size = size;
> - e820.map[x].type = type;
> - e820.nr_map++;
> + e820->map[x].addr = start;
> + e820->map[x].size = size;
> + e820->map[x].type = type;
> + e820->nr_map++;
> +}
> +
> +/*
> + * Add a memory region to the kernel e820 map.
> + */
> +void __init e820_add_region(u64 start, u64 size, int type)
> +{
> + __e820_add_region(&e820, start, size, type);
> }
>
> void __init e820_print_map(char *who)
> @@ -1173,13 +1184,9 @@ static void early_panic(char *msg)
> panic(msg);
> }
>
> -static int userdef __initdata;
> -
> /* "mem=nopentium" disables the 4MB page tables. */
> static int __init parse_memopt(char *p)
> {
> - u64 mem_size;
> -
> if (!p)
> return -EINVAL;
>
> @@ -1190,10 +1197,7 @@ static int __init parse_memopt(char *p)
> }
> #endif
>
> - userdef = 1;
> - mem_size = memparse(p, &p);
> - e820_remove_range(mem_size, ULLONG_MAX - mem_size, E820_RAM, 1);
> -
> + memlimit = memparse(p, &p);
> return 0;
> }
> early_param("mem", parse_memopt);
> @@ -1207,16 +1211,7 @@ static int __init parse_memmap_opt(char *p)
> return -EINVAL;
>
> if (!strncmp(p, "exactmap", 8)) {
> -#ifdef CONFIG_CRASH_DUMP
> - /*
> - * If we are doing a crash dump, we still need to know
> - * the real mem size before original memory map is
> - * reset.
> - */
> - saved_max_pfn = e820_end_of_ram_pfn();
> -#endif
> - e820.nr_map = 0;
> - userdef = 1;
> + exactmap = true;
> return 0;
> }
>
> @@ -1225,18 +1220,18 @@ static int __init parse_memmap_opt(char *p)
> if (p == oldp)
> return -EINVAL;
>
> - userdef = 1;
> if (*p == '@') {
> start_at = memparse(p+1, &p);
> - e820_add_region(start_at, mem_size, E820_RAM);
> + __e820_add_region(&user_e820, start_at, mem_size, E820_RAM);
> } else if (*p == '#') {
> start_at = memparse(p+1, &p);
> - e820_add_region(start_at, mem_size, E820_ACPI);
> + __e820_add_region(&user_e820, start_at, mem_size, E820_ACPI);
> } else if (*p == '$') {
> start_at = memparse(p+1, &p);
> - e820_add_region(start_at, mem_size, E820_RESERVED);
> + __e820_add_region(&user_e820, start_at, mem_size,
> + E820_RESERVED);
> } else
> - e820_remove_range(mem_size, ULLONG_MAX - mem_size, E820_RAM, 1);
> + memlimit = mem_size;
>
> return *p == '\0' ? 0 : -EINVAL;
> }
> @@ -1244,6 +1239,33 @@ early_param("memmap", parse_memmap_opt);
>
> void __init finish_e820_parsing(void)
> {
> + bool userdef;
> + int i;
> +
> + if (memlimit) {
> + e820_remove_range(memlimit, ULLONG_MAX - memlimit, E820_RAM, 1);
> + userdef = true;
> + }
> +
> + if (exactmap) {
> +#ifdef CONFIG_CRASH_DUMP
> + /*
> + * If we are doing a crash dump, we still need to know
> + * the real mem size before original memory map is
> + * reset.
> + */
> + saved_max_pfn = e820_end_of_ram_pfn();
> +#endif
> + e820.nr_map = 0;
> + userdef = true;
> + }
> +
> + for (i = 0; i < user_e820.nr_map; i++) {
> + e820_add_region(user_e820.map[i].addr, user_e820.map[i].size,
> + user_e820.map[i].type);
> + userdef = true;
> + }
> +
> if (userdef) {
> int nr = e820.nr_map;
>

looks good.

for cpu caps on boot cpu setup still need

could be in tip your tree.

---

[PATCH] x86: clean up setup_clear/force_cpu_cap handling

Impact: fix and cleanup

setup_force_cpu_cap() only have one user xen
but it should not reuse cleared_cpu_cpus. it will have problem
for smp.

need to have cpu_cpus_set array too.
also need to setup handling before all cpus cap AND

Signed-off-by: Yinghai Lu <[email protected]>

---
arch/x86/include/asm/cpufeature.h | 4 ++--
arch/x86/include/asm/processor.h | 3 ++-
arch/x86/kernel/cpu/common.c | 17 ++++++++++++-----
3 files changed, 16 insertions(+), 8 deletions(-)

Index: linux-2.6/arch/x86/include/asm/cpufeature.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/cpufeature.h
+++ linux-2.6/arch/x86/include/asm/cpufeature.h
@@ -190,11 +190,11 @@ extern const char * const x86_power_flag
#define clear_cpu_cap(c, bit) clear_bit(bit, (unsigned long *)((c)->x86_capability))
#define setup_clear_cpu_cap(bit) do { \
clear_cpu_cap(&boot_cpu_data, bit); \
- set_bit(bit, (unsigned long *)cleared_cpu_caps); \
+ set_bit(bit, (unsigned long *)cpu_caps_cleared); \
} while (0)
#define setup_force_cpu_cap(bit) do { \
set_cpu_cap(&boot_cpu_data, bit); \
- clear_bit(bit, (unsigned long *)cleared_cpu_caps); \
+ set_bit(bit, (unsigned long *)cpu_caps_set); \
} while (0)

#define cpu_has_fpu boot_cpu_has(X86_FEATURE_FPU)
Index: linux-2.6/arch/x86/include/asm/processor.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/processor.h
+++ linux-2.6/arch/x86/include/asm/processor.h
@@ -134,7 +134,8 @@ extern struct cpuinfo_x86 boot_cpu_data;
extern struct cpuinfo_x86 new_cpu_data;

extern struct tss_struct doublefault_tss;
-extern __u32 cleared_cpu_caps[NCAPINTS];
+extern __u32 cpu_caps_cleared[NCAPINTS];
+extern __u32 cpu_caps_set[NCAPINTS];

#ifdef CONFIG_SMP
DECLARE_PER_CPU(struct cpuinfo_x86, cpu_info);
Index: linux-2.6/arch/x86/kernel/cpu/common.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/common.c
+++ linux-2.6/arch/x86/kernel/cpu/common.c
@@ -221,7 +221,8 @@ static char __cpuinit *table_lookup_mode
return NULL; /* Not found */
}

-__u32 cleared_cpu_caps[NCAPINTS] __cpuinitdata;
+__u32 cpu_caps_cleared[NCAPINTS] __cpuinitdata;
+__u32 cpu_caps_set[NCAPINTS] __cpuinitdata;

/* Current gdt points %fs at the "master" per-cpu area: after this,
* it's on the real one. */
@@ -706,6 +707,16 @@ static void __cpuinit identify_cpu(struc
#endif

init_hypervisor(c);
+
+ /*
+ * Clear/Set all flags overriden by options, need do it
+ * before following smp all cpus cap AND.
+ */
+ for (i = 0; i < NCAPINTS; i++) {
+ c->x86_capability[i] &= ~cpu_caps_cleared[i];
+ c->x86_capability[i] |= cpu_caps_set[i];
+ }
+
/*
* On SMP, boot_cpu_data holds the common feature set between
* all CPUs; so make sure that we indicate which features are
@@ -718,10 +729,6 @@ static void __cpuinit identify_cpu(struc
boot_cpu_data.x86_capability[i] &= c->x86_capability[i];
}

- /* Clear all flags overriden by options */
- for (i = 0; i < NCAPINTS; i++)
- c->x86_capability[i] &= ~cleared_cpu_caps[i];
-
#ifdef CONFIG_X86_MCE
/* Init Machine Check Exception if available. */
mcheck_init(c);


---
could be put in next:

[PATCH] x86: update boot_cpu x86 cap according in early_identify_cpu

Impact: fix

early_param is moved more earlier. So need to update boot_cpu_data x86 cap
in early_identify_cpu() to make sure that is right.

Signed-off-by: Yinghai Lu <[email protected]>

---
arch/x86/kernel/cpu/common.c | 10 ++++++++++
1 file changed, 10 insertions(+)

Index: linux-2.6/arch/x86/kernel/cpu/common.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/common.c
+++ linux-2.6/arch/x86/kernel/cpu/common.c
@@ -552,6 +552,16 @@ static void __init early_identify_cpu(st
if (this_cpu->c_early_init)
this_cpu->c_early_init(c);

+ /*
+ * Clear/Set all flags overriden by options, need do it
+ * because early_param is done before early_cpu_init now.
+ * We can get boot_cpu_data.x86_capability right.
+ */
+ for (i = 0; i < NCAPINTS; i++) {
+ c->x86_capability[i] &= ~cpu_caps_cleared[i];
+ c->x86_capability[i] |= cpu_caps_set[i];
+ }
+
validate_pat_support(c);

#ifdef CONFIG_SMP

2008-12-24 12:30:29

by Ingo Oeser

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

Hello Rusty,

On Wednesday 24 December 2008, Rusty Russell wrote:
> void __init finish_e820_parsing(void)
> {
> + bool userdef;

maybe init with userdef = false;

> + int i;
> +
> + if (memlimit) {
> + e820_remove_range(memlimit, ULLONG_MAX - memlimit, E820_RAM, 1);
> + userdef = true;
> + }
> +
> + if (exactmap) {
> +#ifdef CONFIG_CRASH_DUMP
> + /*
> + * If we are doing a crash dump, we still need to know
> + * the real mem size before original memory map is
> + * reset.
> + */
> + saved_max_pfn = e820_end_of_ram_pfn();
> +#endif
> + e820.nr_map = 0;
> + userdef = true;
> + }
> +
> + for (i = 0; i < user_e820.nr_map; i++) {
> + e820_add_region(user_e820.map[i].addr, user_e820.map[i].size,
> + user_e820.map[i].type);
> + userdef = true;
> + }
> +
> if (userdef) {
> int nr = e820.nr_map;

Because here it might be used uninitialized.

Best Regards

Ingo Oeser

2008-12-24 14:44:17

by Hugh Dickins

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Wed, 24 Dec 2008, Rusty Russell wrote:
> On Wednesday 24 December 2008 01:03:27 Ingo Molnar wrote:
> > * Yinghai Lu <[email protected]> wrote:
> > > On Mon, Dec 22, 2008 at 7:06 PM, Hugh Dickins <[email protected]> wrote:
> > > >
> > > > I find that bootparam "mem=700M" isn't working in linux-next or mmotm,
> > > > and have bisected it down to your patch below; but now I'm off to bed
> > > > without working out just what goes wrong (I'll bet it's the "=").
(I lost that bet)
>
> Thanks for the report and analysis.
>
> > Rusty, will you fix or revert it?
>
> mem= is actually easy, memmap= is harder. This is tested and pushed.

Thanks, that gets my "mem=700M" respected on x86_32 and x86_64.
But (of course: it's a patch to arch/x86) doesn't help at all on
ppc64; and I presume other architectures also remain broken...

Hugh

2008-12-24 23:09:24

by Yinghai Lu

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Wed, Dec 24, 2008 at 12:09 AM, Yinghai Lu <[email protected]> wrote:
> Rusty Russell wrote:
>> On Wednesday 24 December 2008 01:03:27 Ingo Molnar wrote:
>>> * Yinghai Lu <[email protected]> wrote:
>>>
>>>> On Mon, Dec 22, 2008 at 7:06 PM, Hugh Dickins <[email protected]> wrote:
>>>>> Hi Rusty,
>>>>>
>>>>> I find that bootparam "mem=700M" isn't working in linux-next or mmotm,
>>>>> and have bisected it down to your patch below; but now I'm off to bed
>>>>> without working out just what goes wrong (I'll bet it's the "=").
>>
>> Thanks for the report and analysis.
>>
>>> Rusty, will you fix or revert it?
>>
>> mem= is actually easy, memmap= is harder. This is tested and pushed.
>>
>> This commit is actually orthogonal to the other changes, but makes most
>> sense I think in the bootparam tree.
>>
>> commit b69cab1078c69a568afdd2eb8ee6f8559b769e2c
>> Author: Rusty Russell <[email protected]>
>> Date: Wed Dec 24 18:04:19 2008 +1030
>>
>> Fix mem= and memmap= parsing: now it happens *before* e820 setup.
>>
>> Signed-off-by: Rusty Russell <[email protected]>
>>
>
> for cpu caps on boot cpu setup still need
>
> could be in tip your tree.
>
> ---
>
> [PATCH] x86: clean up setup_clear/force_cpu_cap handling
>
> Impact: fix and cleanup
>
> setup_force_cpu_cap() only have one user xen
> but it should not reuse cleared_cpu_cpus. it will have problem
> for smp.
>
> need to have cpu_cpus_set array too.
> also need to setup handling before all cpus cap AND
>
> Signed-off-by: Yinghai Lu <[email protected]>
>
> ---
> ---
> could be put in next:
>
> [PATCH] x86: update boot_cpu x86 cap according in early_identify_cpu
>
> Impact: fix
>
> early_param is moved more earlier. So need to update boot_cpu_data x86 cap
> in early_identify_cpu() to make sure that is right.
>
> Signed-off-by: Yinghai Lu <[email protected]>
>
> ---
> arch/x86/kernel/cpu/common.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> Index: linux-2.6/arch/x86/kernel/cpu/common.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/cpu/common.c
> +++ linux-2.6/arch/x86/kernel/cpu/common.c
> @@ -552,6 +552,16 @@ static void __init early_identify_cpu(st
> if (this_cpu->c_early_init)
> this_cpu->c_early_init(c);
>
> + /*
> + * Clear/Set all flags overriden by options, need do it
> + * because early_param is done before early_cpu_init now.
> + * We can get boot_cpu_data.x86_capability right.
> + */
> + for (i = 0; i < NCAPINTS; i++) {
> + c->x86_capability[i] &= ~cpu_caps_cleared[i];
> + c->x86_capability[i] |= cpu_caps_set[i];
> + }
> +
> validate_pat_support(c);
>
> #ifdef CONFIG_SMP
> --

...


also

/*
* noexec = on|off
*
* Control non executable mappings.
*
* on Enable
* off Disable
*/
static int __init noexec_setup(char *str)
{
if (!str || !strcmp(str, "on")) {
if (cpu_has_nx) {
__supported_pte_mask |= _PAGE_NX;
disable_nx = 0;
}
} else {
if (!strcmp(str, "off")) {
disable_nx = 1;
__supported_pte_mask &= ~_PAGE_NX;
} else {
return -EINVAL;
}
}

return 0;
}
early_param("noexec", noexec_setup);


they are using cpu_has_nx already.

wonder if just move early_cpu_init() and before in setup_arch() to
x86_start_kernel/x86_64_start_kernel
directly.

YH

2008-12-25 03:28:28

by Rusty Russell

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Thursday 25 December 2008 01:14:35 Hugh Dickins wrote:
> On Wed, 24 Dec 2008, Rusty Russell wrote:
> > mem= is actually easy, memmap= is harder. This is tested and pushed.
>
> Thanks, that gets my "mem=700M" respected on x86_32 and x86_64.
> But (of course: it's a patch to arch/x86) doesn't help at all on
> ppc64; and I presume other architectures also remain broken...

No, the problem is that early_param() get parsed *earlier* with these
changes. x86 relied on the e820 map already being set up. Most archs
(like powerpc) just set a "memory_limit" and handle it later; this
is in fact how the vast majority of early_param() work.

This cleanup is getting hairier, however we should see it as a chance
to clean some of this code too I guess...

Thanks,
Rusty.

2008-12-25 05:50:38

by Rusty Russell

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Wednesday 24 December 2008 23:00:27 Ingo Oeser wrote:
> Hello Rusty,
>
> On Wednesday 24 December 2008, Rusty Russell wrote:
> > void __init finish_e820_parsing(void)
> > {
> > + bool userdef;
>
> maybe init with userdef = false;

Thanks! Gcc 3.4 warns about that, gcc 4.1, 4.2 and 4.3 don't.

Strange.
Rusty.

2008-12-25 11:40:52

by Rusty Russell

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Wednesday 24 December 2008 18:39:04 Yinghai Lu wrote:
> for cpu caps on boot cpu setup still need

OK, how about these? I cleaned up description a little, and put
it in own function.

Subject: [PATCH] x86: clean up setup_clear/force_cpu_cap handling
From: Yinghai Lu <[email protected]>

Impact: fix and cleanup

setup_force_cpu_cap() only has one user (xen) but it should not reuse
cleared_cpu_cpus. It will have problems for smp.

Need to have cpu_cpus_set array too, and need to combine it before
we AND into boot_cpu_data.x86_capability.

Signed-off-by: Yinghai Lu <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
---
arch/x86/include/asm/cpufeature.h | 4 ++--
arch/x86/include/asm/processor.h | 3 ++-
arch/x86/kernel/cpu/common.c | 17 ++++++++++++-----
3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -188,11 +188,11 @@ extern const char * const x86_power_flag
#define clear_cpu_cap(c, bit) clear_bit(bit, (unsigned long *)((c)->x86_capability))
#define setup_clear_cpu_cap(bit) do { \
clear_cpu_cap(&boot_cpu_data, bit); \
- set_bit(bit, (unsigned long *)cleared_cpu_caps); \
+ set_bit(bit, (unsigned long *)cpu_caps_cleared); \
} while (0)
#define setup_force_cpu_cap(bit) do { \
set_cpu_cap(&boot_cpu_data, bit); \
- clear_bit(bit, (unsigned long *)cleared_cpu_caps); \
+ set_bit(bit, (unsigned long *)cpu_caps_set); \
} while (0)

#define cpu_has_fpu boot_cpu_has(X86_FEATURE_FPU)
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -130,7 +130,8 @@ extern struct cpuinfo_x86 new_cpu_data;
extern struct cpuinfo_x86 new_cpu_data;

extern struct tss_struct doublefault_tss;
-extern __u32 cleared_cpu_caps[NCAPINTS];
+extern __u32 cpu_caps_cleared[NCAPINTS];
+extern __u32 cpu_caps_set[NCAPINTS];

#ifdef CONFIG_SMP
DECLARE_PER_CPU(struct cpuinfo_x86, cpu_info);
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -219,7 +219,8 @@ static char __cpuinit *table_lookup_mode
return NULL; /* Not found */
}

-__u32 cleared_cpu_caps[NCAPINTS] __cpuinitdata;
+__u32 cpu_caps_cleared[NCAPINTS] __cpuinitdata;
+__u32 cpu_caps_set[NCAPINTS] __cpuinitdata;

/* Current gdt points %fs at the "master" per-cpu area: after this,
* it's on the real one. */
@@ -512,6 +513,16 @@ static void __cpuinit identify_cpu_witho
#endif
}

+static void __cpuinit override_capabilities(u32 x86_capability[])
+{
+ unsigned int i;
+
+ for (i = 0; i < NCAPINTS; i++) {
+ x86_capability[i] &= ~cpu_caps_cleared[i];
+ x86_capability[i] |= cpu_caps_set[i];
+ }
+}
+
/*
* Do minimum CPU detection early.
* Fields really needed: vendor, cpuid_level, family, model, mask,
@@ -703,6 +714,9 @@ static void __cpuinit identify_cpu(struc
detect_ht(c);
#endif

+ /* Command-line override before we AND into smp all cpus cap. */
+ override_capabilities(c->x86_capability);
+
/*
* On SMP, boot_cpu_data holds the common feature set between
* all CPUs; so make sure that we indicate which features are
@@ -714,10 +728,6 @@ static void __cpuinit identify_cpu(struc
for (i = 0; i < NCAPINTS; i++)
boot_cpu_data.x86_capability[i] &= c->x86_capability[i];
}
-
- /* Clear all flags overriden by options */
- for (i = 0; i < NCAPINTS; i++)
- c->x86_capability[i] &= ~cleared_cpu_caps[i];

#ifdef CONFIG_X86_MCE
/* Init Machine Check Exception if available. */


Subject: x86: override_capabilities in early_identify_cpu
From: Rusty Russell <[email protected]>

Impact: future-proof

This does nothing at the moment, but once early_param() are parsed before
setup_arch, it will ensure we respect commandline overrides on the boot
cpu's capabilities.

Signed-off-by: Rusty Russell <[email protected]>
Cc: Yinghai Lu <[email protected]>

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -559,6 +559,8 @@ static void __init early_identify_cpu(st

if (this_cpu->c_early_init)
this_cpu->c_early_init(c);
+
+ override_capabilities(c->x86_capability);

validate_pat_support(c);

2008-12-25 12:17:29

by Rusty Russell

[permalink] [raw]
Subject: [RFC] boot parameter handling cleanup II

(Not much change since last time, just consolidated fixes aired here, esp.
x86 early_param fixes thanks to Hugh and Yinghai.)

The following changes since commit 4a6908a3a050aacc9c3a2f36b276b46c0629ad91:
Linus Torvalds (1):
Linux 2.6.28

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux-2.6-boot-params.git master

David Howells (2):
param: Adapt MN10300 to the new parameter handling regime
param: Adapt FRV to the new parameter handling regime

Geert Uytterhoeven (1):
m68k: Use asm/sections.h in kernel/setup.c

Rusty Russell (13):
USB: Don't use __module_param_call; use core_param.
x86: act on mem= and memmap= later in boot
x86: prepare noexec for being called earlier
x86: override_capabilities in early_identify_cpu
param: allow parse_args to work non-destructively.
param: put core_param in separate section
param: make sure charp isn't used as a core_param
param: arch_get_boot_command_line()
param: move core_param() parsing to before setup_arch.
param: Call early_param earlier.
param: combine core_param and early_param parsing.
param: don't use alloc_bootmem for saved_command_line
param: move banner printing to top of start_kernel.

Yinghai Lu (1):
x86: clean up setup_clear/force_cpu_cap handling

arch/alpha/kernel/setup.c | 36 +++++++-----
arch/arm/kernel/setup.c | 43 ++++++++------
arch/avr32/kernel/setup.c | 15 ++---
arch/blackfin/kernel/setup.c | 22 ++++----
arch/cris/kernel/setup.c | 29 +++++-----
arch/frv/kernel/setup.c | 50 ++++++----------
arch/h8300/kernel/setup.c | 27 +++++----
arch/ia64/dig/setup.c | 2 +-
arch/ia64/hp/sim/hpsim_setup.c | 2 +-
arch/ia64/include/asm/machvec.h | 4 +-
arch/ia64/include/asm/paravirt.h | 8 +-
arch/ia64/kernel/machvec.c | 8 ++-
arch/ia64/kernel/setup.c | 32 +++++-----
arch/ia64/sn/kernel/setup.c | 13 ++--
arch/ia64/uv/kernel/setup.c | 2 +-
arch/ia64/xen/xen_pv_ops.c | 2 +-
arch/m32r/kernel/setup.c | 16 +++---
arch/m68k/kernel/setup.c | 37 +++++++-----
arch/m68knommu/include/asm/machdep.h | 2 +-
arch/m68knommu/kernel/setup.c | 33 +++++-----
arch/m68knommu/platform/5206/config.c | 2 +-
arch/m68knommu/platform/5206e/config.c | 14 +++--
arch/m68knommu/platform/520x/config.c | 2 +-
arch/m68knommu/platform/523x/config.c | 2 +-
arch/m68knommu/platform/5249/config.c | 2 +-
arch/m68knommu/platform/5272/config.c | 23 ++++----
arch/m68knommu/platform/527x/config.c | 2 +-
arch/m68knommu/platform/528x/config.c | 2 +-
arch/m68knommu/platform/5307/config.c | 13 +++-
arch/m68knommu/platform/532x/config.c | 22 +++----
arch/m68knommu/platform/5407/config.c | 2 +-
arch/m68knommu/platform/68328/config.c | 2 +-
arch/m68knommu/platform/68328/head-pilot.S | 2 +-
arch/m68knommu/platform/68360/config.c | 6 +-
arch/m68knommu/platform/68EZ328/config.c | 4 +-
arch/m68knommu/platform/68VZ328/config.c | 12 ++--
arch/mips/kernel/setup.c | 20 +++---
arch/mn10300/kernel/setup.c | 61 ++++++++------------
arch/parisc/kernel/setup.c | 32 +++++------
arch/powerpc/kernel/prom.c | 1 -
arch/powerpc/kernel/setup_32.c | 10 ++-
arch/powerpc/kernel/setup_64.c | 10 ++-
arch/s390/kernel/setup.c | 8 +--
arch/sh/boards/board-magicpanelr2.c | 2 +-
arch/sh/boards/board-sh7785lcr.c | 2 +-
arch/sh/boards/mach-dreamcast/setup.c | 2 +-
arch/sh/boards/mach-highlander/setup.c | 2 +-
arch/sh/boards/mach-hp6xx/setup.c | 2 +-
arch/sh/boards/mach-landisk/setup.c | 2 +-
arch/sh/boards/mach-microdev/setup.c | 2 +-
arch/sh/boards/mach-migor/setup.c | 2 +-
arch/sh/boards/mach-r2d/setup.c | 2 +-
arch/sh/boards/mach-sdk7780/setup.c | 2 +-
arch/sh/boards/mach-se/7343/setup.c | 2 +-
arch/sh/boards/mach-se/770x/setup.c | 2 +-
arch/sh/boards/mach-se/7721/setup.c | 2 +-
arch/sh/boards/mach-se/7722/setup.c | 2 +-
arch/sh/boards/mach-se/7780/setup.c | 2 +-
arch/sh/boards/mach-sh03/setup.c | 2 +-
arch/sh/boards/mach-sh7763rdp/setup.c | 2 +-
arch/sh/include/asm/machvec.h | 2 +-
arch/sh/kernel/setup.c | 28 +++++-----
arch/sparc/kernel/setup.c | 14 ++--
arch/sparc64/kernel/setup.c | 12 ++--
arch/um/kernel/um_arch.c | 10 ++-
arch/x86/include/asm/cpufeature.h | 4 +-
arch/x86/include/asm/processor.h | 3 +-
arch/x86/kernel/cpu/common.c | 22 +++++--
arch/x86/kernel/e820.c | 88 +++++++++++++++++----------
arch/x86/kernel/setup.c | 41 ++++++-------
arch/x86/mm/init_32.c | 11 ++-
arch/xtensa/kernel/setup.c | 21 ++++---
arch/xtensa/platforms/iss/setup.c | 2 +-
drivers/usb/core/usb.c | 8 ++-
include/asm-generic/vmlinux.lds.h | 3 +
include/asm-xtensa/platform.h | 5 +-
include/linux/init.h | 5 +-
include/linux/moduleparam.h | 43 ++++++++++----
init/main.c | 73 ++++++++++-------------
kernel/kexec.c | 1 +
kernel/module.c | 2 +-
kernel/params.c | 74 ++++++++++++++++++-----
82 files changed, 625 insertions(+), 518 deletions(-)

2008-12-25 12:20:59

by Rusty Russell

[permalink] [raw]
Subject: [RFC PATCH] param: start_kernel_with_args()

This is a cute corollary of the cleanup patches. IA64 could particularly
benefit I think.

Impact: cleanup, new API

This gives a nicer entry point for archs to start_kernel; they don't
have to use boot_command_line (though they can if they want).

A nice side effect is that archs which use this entry point don't have
a COMMAND_LINE_SIZE limit any more (and if you don't define it,
boot_command_line does not exist).

Signed-off-by: Rusty Russell <[email protected]>

diff --git a/include/linux/start_kernel.h b/include/linux/start_kernel.h
--- a/include/linux/start_kernel.h
+++ b/include/linux/start_kernel.h
@@ -7,6 +7,13 @@
/* Define the prototype for start_kernel here, rather than cluttering
up something else. */

+/*
+ * Command-line pointer must be writable, must be NUL-terminated, but
+ * has no length limit and may be __initdata.
+ */
+extern asmlinkage void __init start_kernel_with_args(char *cmdline);
+
+/* Old entry point: calls arch_get_boot_command_line(). */
extern asmlinkage void __init start_kernel(void);

#endif /* _LINUX_START_KERNEL_H */
diff --git a/init/main.c b/init/main.c
--- a/init/main.c
+++ b/init/main.c
@@ -119,8 +119,11 @@ void (*late_time_init)(void);
void (*late_time_init)(void);
extern void softirq_init(void);

+#ifdef COMMAND_LINE_SIZE
/* Untouched command line saved by arch-specific code. */
char __initdata boot_command_line[COMMAND_LINE_SIZE];
+#endif
+
/* Untouched saved command line (eg. for /proc) */
char *saved_command_line;

@@ -522,15 +525,22 @@ void __init __weak thread_info_cache_ini
{
}

+#ifdef COMMAND_LINE_SIZE
asmlinkage void __init start_kernel(void)
+{
+ arch_get_boot_command_line();
+ start_kernel_with_args(boot_command_line);
+}
+#endif
+
+asmlinkage void __init start_kernel_with_args(char *cmdline)
{
char *static_command_line;

printk(KERN_NOTICE);
printk(linux_banner);

- arch_get_boot_command_line();
- parse_args("Core and early params", boot_command_line,
+ parse_args("Core and early params", cmdline,
__start___core_param,
__stop___core_param - __start___core_param,
do_early_param, true);
@@ -579,10 +589,10 @@ asmlinkage void __init start_kernel(void
preempt_disable();
build_all_zonelists();
page_alloc_init();
- printk(KERN_NOTICE "Kernel command line: %s\n", boot_command_line);
+ printk(KERN_NOTICE "Kernel command line: %s\n", cmdline);
/* param parsing can keep pointers to the commandline. */
- static_command_line = alloc_bootmem(strlen(boot_command_line)+1);
- strcpy(static_command_line, boot_command_line);
+ static_command_line = alloc_bootmem(strlen(cmdline)+1);
+ strcpy(static_command_line, cmdline);
parse_args("Booting kernel", static_command_line, __start___param,
__stop___param - __start___param,
&unknown_bootoption, false);
@@ -684,7 +694,7 @@ asmlinkage void __init start_kernel(void

ftrace_init();

- saved_command_line = kstrdup(boot_command_line, GFP_KERNEL);
+ saved_command_line = kstrdup(cmdline, GFP_KERNEL);

/* Do the rest non-__init'ed, we're now alive */
rest_init();

2008-12-25 22:46:23

by Yinghai Lu

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Thu, Dec 25, 2008 at 3:40 AM, Rusty Russell <[email protected]> wrote:
> On Wednesday 24 December 2008 18:39:04 Yinghai Lu wrote:
>> for cpu caps on boot cpu setup still need
>
> OK, how about these? I cleaned up description a little, and put
> it in own function.

looks good.

YH

>
> Subject: [PATCH] x86: clean up setup_clear/force_cpu_cap handling
> From: Yinghai Lu <[email protected]>
>
> Impact: fix and cleanup
>
> setup_force_cpu_cap() only has one user (xen) but it should not reuse
> cleared_cpu_cpus. It will have problems for smp.
>
> Need to have cpu_cpus_set array too, and need to combine it before
> we AND into boot_cpu_data.x86_capability.
>
> Signed-off-by: Yinghai Lu <[email protected]>
> Signed-off-by: Rusty Russell <[email protected]>
> ---
> arch/x86/include/asm/cpufeature.h | 4 ++--
> arch/x86/include/asm/processor.h | 3 ++-
> arch/x86/kernel/cpu/common.c | 17 ++++++++++++-----
> 3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -188,11 +188,11 @@ extern const char * const x86_power_flag
> #define clear_cpu_cap(c, bit) clear_bit(bit, (unsigned long *)((c)->x86_capability))
> #define setup_clear_cpu_cap(bit) do { \
> clear_cpu_cap(&boot_cpu_data, bit); \
> - set_bit(bit, (unsigned long *)cleared_cpu_caps); \
> + set_bit(bit, (unsigned long *)cpu_caps_cleared); \
> } while (0)
> #define setup_force_cpu_cap(bit) do { \
> set_cpu_cap(&boot_cpu_data, bit); \
> - clear_bit(bit, (unsigned long *)cleared_cpu_caps); \
> + set_bit(bit, (unsigned long *)cpu_caps_set); \
> } while (0)
>
> #define cpu_has_fpu boot_cpu_has(X86_FEATURE_FPU)
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -130,7 +130,8 @@ extern struct cpuinfo_x86 new_cpu_data;
> extern struct cpuinfo_x86 new_cpu_data;
>
> extern struct tss_struct doublefault_tss;
> -extern __u32 cleared_cpu_caps[NCAPINTS];
> +extern __u32 cpu_caps_cleared[NCAPINTS];
> +extern __u32 cpu_caps_set[NCAPINTS];
>
> #ifdef CONFIG_SMP
> DECLARE_PER_CPU(struct cpuinfo_x86, cpu_info);
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -219,7 +219,8 @@ static char __cpuinit *table_lookup_mode
> return NULL; /* Not found */
> }
>
> -__u32 cleared_cpu_caps[NCAPINTS] __cpuinitdata;
> +__u32 cpu_caps_cleared[NCAPINTS] __cpuinitdata;
> +__u32 cpu_caps_set[NCAPINTS] __cpuinitdata;
>
> /* Current gdt points %fs at the "master" per-cpu area: after this,
> * it's on the real one. */
> @@ -512,6 +513,16 @@ static void __cpuinit identify_cpu_witho
> #endif
> }
>
> +static void __cpuinit override_capabilities(u32 x86_capability[])
> +{
> + unsigned int i;
> +
> + for (i = 0; i < NCAPINTS; i++) {
> + x86_capability[i] &= ~cpu_caps_cleared[i];
> + x86_capability[i] |= cpu_caps_set[i];
> + }
> +}
> +
> /*
> * Do minimum CPU detection early.
> * Fields really needed: vendor, cpuid_level, family, model, mask,
> @@ -703,6 +714,9 @@ static void __cpuinit identify_cpu(struc
> detect_ht(c);
> #endif
>
> + /* Command-line override before we AND into smp all cpus cap. */
> + override_capabilities(c->x86_capability);
> +
> /*
> * On SMP, boot_cpu_data holds the common feature set between
> * all CPUs; so make sure that we indicate which features are
> @@ -714,10 +728,6 @@ static void __cpuinit identify_cpu(struc
> for (i = 0; i < NCAPINTS; i++)
> boot_cpu_data.x86_capability[i] &= c->x86_capability[i];
> }
> -
> - /* Clear all flags overriden by options */
> - for (i = 0; i < NCAPINTS; i++)
> - c->x86_capability[i] &= ~cleared_cpu_caps[i];
>
> #ifdef CONFIG_X86_MCE
> /* Init Machine Check Exception if available. */
>
>
> Subject: x86: override_capabilities in early_identify_cpu
> From: Rusty Russell <[email protected]>
>
> Impact: future-proof
>
> This does nothing at the moment, but once early_param() are parsed before
> setup_arch, it will ensure we respect commandline overrides on the boot
> cpu's capabilities.
>
> Signed-off-by: Rusty Russell <[email protected]>
> Cc: Yinghai Lu <[email protected]>
>
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -559,6 +559,8 @@ static void __init early_identify_cpu(st
>
> if (this_cpu->c_early_init)
> this_cpu->c_early_init(c);
> +
> + override_capabilities(c->x86_capability);
>
> validate_pat_support(c);
>
> --
> 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/
>

2008-12-27 10:15:15

by Ingo Molnar

[permalink] [raw]
Subject: Re: [RFC] boot parameter handling cleanup II


* Rusty Russell <[email protected]> wrote:

> (Not much change since last time, just consolidated fixes aired here, esp.
> x86 early_param fixes thanks to Hugh and Yinghai.)
>
> The following changes since commit 4a6908a3a050aacc9c3a2f36b276b46c0629ad91:
> Linus Torvalds (1):
> Linux 2.6.28
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux-2.6-boot-params.git master

(it would be useful to embedd the full patch in such cases in the email -
makes for a quicker review)

> arch/x86/include/asm/cpufeature.h | 4 +-
> arch/x86/include/asm/processor.h | 3 +-
> arch/x86/kernel/cpu/common.c | 22 +++++--
> arch/x86/kernel/e820.c | 88 +++++++++++++++++----------
> arch/x86/kernel/setup.c | 41 ++++++-------
> arch/x86/mm/init_32.c | 11 ++-

no objections from the x86 perspective, as long as it does not break
stuff. If it breaks stuff it's all your fault ;-) I checked these commits:

218a372: x86: override_capabilities in early_identify_cpu
5144b68: x86: prepare noexec for being called earlier
5689919: x86: clean up setup_clear/force_cpu_cap handling
b41144e: x86: act on mem= and memmap= later in boot

and they definitely look like a step forward.

One detail: please send a pull request to Linus after the pending x86 tree
went upstream, so that there are no conflicts with existing x86 items.
There will be a (trivial) context-overlap conflict in
arch/x86/kernel/cpu/common.c.

Ingo

2008-12-27 13:13:17

by Rusty Russell

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Thursday 25 December 2008 01:14:35 Hugh Dickins wrote:> But (of course: it's a patch to arch/x86) doesn't help at all on> ppc64; and I presume other architectures also remain broken...
Confirmed ppc64. But it's broken in a *different* way.
Whereas most archs called parse_early_param from setup_arch, powerpcdoes it before start_kernel. So instead of moving parsing earlier,my patch actually moved it later for powerpc.
Too late, for mem=.
Rather than try to untangle the powerpc boot process (most archs woulddo most of this in setup_arch), I think I have to expose the parseragain so they can call it:
commit 25bf48b74b9fb23b347d00656b604f9e55c72183Author: Rusty Russell <[email protected]>Date: Sat Dec 27 23:40:37 2008 +1030
Fix powerpc (tested on ppc64) command line handling. Powerpc used to call parse_early_param() really early; the change made it too late. Put it back.
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.cindex d36cbd0..e5f2387 100644--- a/arch/powerpc/kernel/prom.c+++ b/arch/powerpc/kernel/prom.c@@ -32,6 +32,7 @@ #include <linux/debugfs.h> #include <linux/irq.h> #include <linux/lmb.h>+#include <linux/start_kernel.h> #include <asm/prom.h> #include <asm/rtas.h>@@ -1188,6 +1189,7 @@ void __init early_init_devtree(void *params) /* Save command line for /proc/cmdline and then parse parameters */ strlcpy(boot_command_line, cmd_line, COMMAND_LINE_SIZE);+ parse_early_and_core_params(boot_command_line); /* Reserve LMB regions used by kernel, initrd, dt, etc... */ lmb_reserve(PHYSICAL_START, __pa(klimit) - PHYSICAL_START);diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.cindex a237371..52ca63a 100644--- a/arch/powerpc/kernel/setup_32.c+++ b/arch/powerpc/kernel/setup_32.c@@ -264,12 +264,6 @@ static void __init exc_lvl_early_init(void) #define exc_lvl_early_init() #endif -void arch_get_boot_command_line(void)-{- /* FIXME: Get rid of cmd_line in favor of boot_command_line? */- strlcpy(boot_command_line, cmd_line, COMMAND_LINE_SIZE);-}- /* Warning, IO base is not yet inited */ void __init setup_arch(void) {diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.cindex 7d25b42..348bc8a 100644--- a/arch/powerpc/kernel/setup_64.c+++ b/arch/powerpc/kernel/setup_64.c@@ -502,12 +502,6 @@ static void __init emergency_stack_init(void) } } -void arch_get_boot_command_line(void)-{- /* FIXME: Get rid of cmd_line in favor of boot_command_line? */- strlcpy(boot_command_line, cmd_line, COMMAND_LINE_SIZE);-}- /* * Called into from start_kernel, after lock_kernel has been called. * Initializes bootmem, which is unsed to manage page allocation untildiff --git a/include/linux/start_kernel.h b/include/linux/start_kernel.hindex d3e5f27..347cc95 100644--- a/include/linux/start_kernel.h+++ b/include/linux/start_kernel.h@@ -9,4 +9,7 @@ extern asmlinkage void __init start_kernel(void); +/* Usually called by start_kernel, but some nasty archs need it earlier. */+void __init parse_early_and_core_params(char *cmdline);+ #endif /* _LINUX_START_KERNEL_H */diff --git a/init/main.c b/init/main.cindex fb00387..ad933ad 100644--- a/init/main.c+++ b/init/main.c@@ -522,6 +522,21 @@ void __init __weak thread_info_cache_init(void) { } +/* Non-destructive early parse of commandline. PowerPC calls this early. */+void __init parse_early_and_core_params(char *cmdline)+{+ static bool __initdata done = false;++ if (done)+ return;++ parse_args("Core and early params", cmdline,+ __start___core_param,+ __stop___core_param - __start___core_param,+ do_early_param, true);+ done = true;+}+ asmlinkage void __init start_kernel(void) { char *static_command_line;@@ -530,10 +545,7 @@ asmlinkage void __init start_kernel(void) printk(linux_banner); arch_get_boot_command_line();- parse_args("Core and early params", boot_command_line,- __start___core_param,- __stop___core_param - __start___core_param,- do_early_param, true);+ parse_early_and_core_params(boot_command_line); smp_setup_processor_id(); ????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2008-12-28 00:43:12

by Hugh Dickins

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Sat, 27 Dec 2008, Rusty Russell wrote:
> > On Thursday 25 December 2008 01:14:35 Hugh Dickins wrote:
> > But (of course: it's a patch to arch/x86) doesn't help at all on
> > ppc64; and I presume other architectures also remain broken...
>
> Confirmed ppc64. But it's broken in a *different* way.
>
> Whereas most archs called parse_early_param from setup_arch, powerpc
> does it before start_kernel. So instead of moving parsing earlier,
> my patch actually moved it later for powerpc.
>
> Too late, for mem=.
>
> Rather than try to untangle the powerpc boot process (most archs would
> do most of this in setup_arch), I think I have to expose the parser
> again so they can call it:
>
> commit 25bf48b74b9fb23b347d00656b604f9e55c72183
> Author: Rusty Russell <[email protected]>
> Date: Sat Dec 27 23:40:37 2008 +1030
>
> Fix powerpc (tested on ppc64) command line handling.
> Powerpc used to call parse_early_param() really early; the change made
> it too late. Put it back.

I'll make no pretence of reviewing any of this, but this has
indeed got mem= back working for me on a 2.6.28-rc9-mm1 - thanks!

I have to wonder if all this comes too late in the cycle for 2.6.29:
changing the early param handling of all the arches is difficult,
and apparently hasn't been much tested in the short time that it's
been out there in linux-next. But of course, not for me to decide.

Hugh

2008-12-28 02:08:18

by Yinghai Lu

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Sat, Dec 27, 2008 at 4:43 PM, Hugh Dickins <[email protected]> wrote:
> On Sat, 27 Dec 2008, Rusty Russell wrote:
>> > On Thursday 25 December 2008 01:14:35 Hugh Dickins wrote:
>> > But (of course: it's a patch to arch/x86) doesn't help at all on
>> > ppc64; and I presume other architectures also remain broken...
>>
>> Confirmed ppc64. But it's broken in a *different* way.
>>
>> Whereas most archs called parse_early_param from setup_arch, powerpc
>> does it before start_kernel. So instead of moving parsing earlier,
>> my patch actually moved it later for powerpc.
>>
>> Too late, for mem=.
>>
>> Rather than try to untangle the powerpc boot process (most archs would
>> do most of this in setup_arch), I think I have to expose the parser
>> again so they can call it:
>>
>> commit 25bf48b74b9fb23b347d00656b604f9e55c72183
>> Author: Rusty Russell <[email protected]>
>> Date: Sat Dec 27 23:40:37 2008 +1030
>>
>> Fix powerpc (tested on ppc64) command line handling.
>> Powerpc used to call parse_early_param() really early; the change made
>> it too late. Put it back.
>
> I'll make no pretence of reviewing any of this, but this has
> indeed got mem= back working for me on a 2.6.28-rc9-mm1 - thanks!
>
> I have to wonder if all this comes too late in the cycle for 2.6.29:
> changing the early param handling of all the arches is difficult,
> and apparently hasn't been much tested in the short time that it's
> been out there in linux-next. But of course, not for me to decide.
>

should be ok, just need to go over all early_param()

YH

2009-01-01 03:43:18

by Rusty Russell

[permalink] [raw]
Subject: Re: linux-next: parsing mem=700M broken

On Sunday 28 December 2008 12:37:59 Yinghai Lu wrote:
> On Sat, Dec 27, 2008 at 4:43 PM, Hugh Dickins <[email protected]> wrote:
> > I have to wonder if all this comes too late in the cycle for 2.6.29:
> > changing the early param handling of all the arches is difficult,
> > and apparently hasn't been much tested in the short time that it's
> > been out there in linux-next. But of course, not for me to decide.
>
> should be ok, just need to go over all early_param()

Yes, and understanding the intricacies of all 20+ archs boot code.
For example, I've just noticed that MIPS sets up the command line
in prom_init, called from setup_arch, so I've broken that entirely.

git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux-2.6-boot-params.git

This is not going to make this merge window: I need to audit each arch
more deeply, then get help testing.

Thanks,
Rusty.