On Mon, 24 Aug 2009, Ingo Molnar wrote:
> * Yinghai Lu <[email protected]> wrote:
> > Hugh Dickins wrote:
> > > I find the "mem=" boot parameter disabled in today's linux-next:
> > > reverting the tip commit below fixes that.
> > >
> > > Hugh
> > >
> > > From: Xiao Guangrong <[email protected]>
> > > Date: Thu, 20 Aug 2009 12:23:11 +0000 (+0800)
> > > Subject: x86: Fix system crash when loading with "reservetop" parameter
> > > X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fmingo%2Flinux-2.6-x86.git;a=commitdiff_plain;h=8126dec32738421afa362114337331337b4be17f
> > >
> > > x86: Fix system crash when loading with "reservetop" parameter
> > >
> > > The system will die if the kernel is booted with "reservetop"
> > > parameter, in present code, parse "reservetop" parameter after
> > > early_ioremap_init(), and some function still use
> > > early_ioremap() after it.
> > >
> > > The problem is, "reservetop" parameter can modify
> > > 'FIXADDR_TOP', then the virtual address got by early_ioremap()
> > > is base on old 'FIXADDR_TOP', but the page mapping is base on
> > > new 'FIXADDR_TOP', it will occur page fault, and the IDT is not
> > > prepare yet, so, the system is dead.
> > >
> > > So, put parse_early_param() in the front of
> > > early_ioremap_init() in this patch.
> > >
> > > Signed-off-by: Xiao Guangrong <[email protected]>
> > > Cc: [email protected]
> > > Cc: Andrew Morton <[email protected]>
> > > LKML-Reference: <[email protected]>
> > > Signed-off-by: Ingo Molnar <[email protected]>
> > > ---
> > >
> > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > > index 63f32d2..02643cc 100644
> > > --- a/arch/x86/kernel/setup.c
> > > +++ b/arch/x86/kernel/setup.c
> > > @@ -711,6 +711,11 @@ void __init setup_arch(char **cmdline_p)
> > > printk(KERN_INFO "Command line: %s\n", boot_command_line);
> > > #endif
> > >
> > > + strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
> > > + *cmdline_p = command_line;
> > > +
> > > + parse_early_param();
> > > +
> > > /* VMI may relocate the fixmap; do this before touching ioremap area */
> > > vmi_init();
> > >
> > > @@ -793,11 +798,6 @@ void __init setup_arch(char **cmdline_p)
> > > #endif
> > > #endif
> > >
> > > - strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
> > > - *cmdline_p = command_line;
> > > -
> > > - parse_early_param();
> > > -
> > > #ifdef CONFIG_X86_64
> > > check_efer();
> > > #endif
> >
> > yes, that patch will break other built-in command too.
> >
> > need drop that patch.
>
> done. Was nervous about the patch already:
Somehow it seems to have undropped itself, so mem= stopped working
again in recent mmotm/linux-next; and it looks like today the
undropped patch has made its way to Linus - I've not built current
git to check, but I think you'll find mem= is now broken there.
Hugh
>
> http://lkml.org/lkml/2009/8/21/127
>
> > also the problem was caused by vmi patch, and that commit should
> > be reverted.
> >
> > commit ae8d04e2ecbb233926860e9ce145eac19c7835dc
> > Author: Zachary Amsden <[email protected]>
> > Date: Sat Dec 13 12:36:58 2008 -0800
> >
> > x86 Fix VMI crash on boot in 2.6.28-rc8
> >
> > VMI initialiation can relocate the fixmap, causing early_ioremap to
> > malfunction if it is initialized before the relocation. To fix this,
> > VMI activation is split into two phases; the detection, which must
> > happen before setting up ioremap, and the activation, which must happen
> > after parsing early boot parameters.
> >
> > This fixes a crash on boot when VMI is enabled under VMware.
> >
> > Signed-off-by: Zachary Amsden <[email protected]>
> > Signed-off-by: Linus Torvalds <[email protected]>
> >
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index 9d5674f..bdec76e 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -794,6 +794,9 @@ void __init setup_arch(char **cmdline_p)
> > printk(KERN_INFO "Command line: %s\n", boot_command_line);
> > #endif
> >
> > + /* VMI may relocate the fixmap; do this before touching ioremap area */
> > + vmi_init();
> > +
> > early_cpu_init();
> > early_ioremap_init();
> >
> > @@ -880,13 +883,8 @@ void __init setup_arch(char **cmdline_p)
> > check_efer();
> > #endif
> >
> > -#if defined(CONFIG_VMI) && defined(CONFIG_X86_32)
> > - /*
> > - * Must be before kernel pagetables are setup
> > - * or fixmap area is touched.
> > - */
> > - vmi_init();
> > -#endif
> > + /* Must be before kernel pagetables are setup */
> > + vmi_activate();
> >
> > /* after early param, so could get panic from serial */
> > reserve_early_setup_data();
> >
> >
> > and according to
> > http://lkml.org/lkml/2008/12/10/388
> > http://lkml.org/lkml/2008/12/10/456
> >
> > Zachary should split reserve_top_address() to two functions...
> > before sending that patch to Linus
>
> mind looking at this yourself if interested? Zachary has not been
> active for quite some time.
>
> Ingo
* Hugh Dickins <[email protected]> wrote:
> On Mon, 24 Aug 2009, Ingo Molnar wrote:
> > * Yinghai Lu <[email protected]> wrote:
> > > Hugh Dickins wrote:
> > > > I find the "mem=" boot parameter disabled in today's linux-next:
> > > > reverting the tip commit below fixes that.
> > > >
> > > > Hugh
> > > >
> > > > From: Xiao Guangrong <[email protected]>
> > > > Date: Thu, 20 Aug 2009 12:23:11 +0000 (+0800)
> > > > Subject: x86: Fix system crash when loading with "reservetop" parameter
> > > > X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fmingo%2Flinux-2.6-x86.git;a=commitdiff_plain;h=8126dec32738421afa362114337331337b4be17f
> > > >
> > > > x86: Fix system crash when loading with "reservetop" parameter
> > > >
> > > > The system will die if the kernel is booted with "reservetop"
> > > > parameter, in present code, parse "reservetop" parameter after
> > > > early_ioremap_init(), and some function still use
> > > > early_ioremap() after it.
> > > >
> > > > The problem is, "reservetop" parameter can modify
> > > > 'FIXADDR_TOP', then the virtual address got by early_ioremap()
> > > > is base on old 'FIXADDR_TOP', but the page mapping is base on
> > > > new 'FIXADDR_TOP', it will occur page fault, and the IDT is not
> > > > prepare yet, so, the system is dead.
> > > >
> > > > So, put parse_early_param() in the front of
> > > > early_ioremap_init() in this patch.
> > > >
> > > > Signed-off-by: Xiao Guangrong <[email protected]>
> > > > Cc: [email protected]
> > > > Cc: Andrew Morton <[email protected]>
> > > > LKML-Reference: <[email protected]>
> > > > Signed-off-by: Ingo Molnar <[email protected]>
> > > > ---
> > > >
> > > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > > > index 63f32d2..02643cc 100644
> > > > --- a/arch/x86/kernel/setup.c
> > > > +++ b/arch/x86/kernel/setup.c
> > > > @@ -711,6 +711,11 @@ void __init setup_arch(char **cmdline_p)
> > > > printk(KERN_INFO "Command line: %s\n", boot_command_line);
> > > > #endif
> > > >
> > > > + strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
> > > > + *cmdline_p = command_line;
> > > > +
> > > > + parse_early_param();
> > > > +
> > > > /* VMI may relocate the fixmap; do this before touching ioremap area */
> > > > vmi_init();
> > > >
> > > > @@ -793,11 +798,6 @@ void __init setup_arch(char **cmdline_p)
> > > > #endif
> > > > #endif
> > > >
> > > > - strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
> > > > - *cmdline_p = command_line;
> > > > -
> > > > - parse_early_param();
> > > > -
> > > > #ifdef CONFIG_X86_64
> > > > check_efer();
> > > > #endif
> > >
> > > yes, that patch will break other built-in command too.
> > >
> > > need drop that patch.
> >
> > done. Was nervous about the patch already:
>
> Somehow it seems to have undropped itself, so mem= stopped working
> again in recent mmotm/linux-next; and it looks like today the
> undropped patch has made its way to Linus - I've not built current git
> to check, but I think you'll find mem= is now broken there.
I thoroughly zapped it. Do you know about any commit ID where it snuck
in?
Ingo
Ingo Molnar wrote:
> I thoroughly zapped it. Do you know about any commit ID where it snuck
> in?
after close looking, it will break
1. some cpu feature in early stage too, like cpu_has_x2apic
2. will break built-in-command line
3. will break other memmap= and mem=
4. early_dbgp and early_console that will use early_ioremap to access mmio (?)
YH
commit 8126dec32738421afa362114337331337b4be17f
Author: Xiao Guangrong <[email protected]>
Date: Thu Aug 20 20:23:11 2009 +0800
x86: Fix system crash when loading with "reservetop" parameter
The system will die if the kernel is booted with "reservetop"
parameter, in present code, parse "reservetop" parameter after
early_ioremap_init(), and some function still use
early_ioremap() after it.
The problem is, "reservetop" parameter can modify
'FIXADDR_TOP', then the virtual address got by early_ioremap()
is base on old 'FIXADDR_TOP', but the page mapping is base on
new 'FIXADDR_TOP', it will occur page fault, and the IDT is not
prepare yet, so, the system is dead.
So, put parse_early_param() in the front of
early_ioremap_init() in this patch.
Signed-off-by: Xiao Guangrong <[email protected]>
Cc: [email protected]
Cc: Andrew Morton <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 63f32d2..02643cc 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -711,6 +711,11 @@ void __init setup_arch(char **cmdline_p)
printk(KERN_INFO "Command line: %s\n", boot_command_line);
#endif
+ strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
+ *cmdline_p = command_line;
+
+ parse_early_param();
+
/* VMI may relocate the fixmap; do this before touching ioremap area */
vmi_init();
@@ -793,11 +798,6 @@ void __init setup_arch(char **cmdline_p)
#endif
#endif
- strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
- *cmdline_p = command_line;
-
- parse_early_param();
-
#ifdef CONFIG_X86_64
check_efer();
#endif
Yinghai Lu wrote:
> Ingo Molnar wrote:
>> I thoroughly zapped it. Do you know about any commit ID where it snuck
>> in?
>
> after close looking, it will break
> 1. some cpu feature in early stage too, like cpu_has_x2apic
> 2. will break built-in-command line
> 3. will break other memmap= and mem=
> 4. early_dbgp and early_console that will use early_ioremap to access mmio (?)
>
> YH
>
> commit 8126dec32738421afa362114337331337b4be17f
> Author: Xiao Guangrong <[email protected]>
> Date: Thu Aug 20 20:23:11 2009 +0800
>
> x86: Fix system crash when loading with "reservetop" parameter
>
> The system will die if the kernel is booted with "reservetop"
> parameter, in present code, parse "reservetop" parameter after
> early_ioremap_init(), and some function still use
> early_ioremap() after it.
>
> The problem is, "reservetop" parameter can modify
> 'FIXADDR_TOP', then the virtual address got by early_ioremap()
> is base on old 'FIXADDR_TOP', but the page mapping is base on
> new 'FIXADDR_TOP', it will occur page fault, and the IDT is not
> prepare yet, so, the system is dead.
>
> So, put parse_early_param() in the front of
> early_ioremap_init() in this patch.
>
> Signed-off-by: Xiao Guangrong <[email protected]>
> Cc: [email protected]
> Cc: Andrew Morton <[email protected]>
> LKML-Reference: <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
>
after revet it, it seem mem=32g etc still broken, maybe some other commit about x86 platform support from Thomas broke it.
YH
---
arch/x86/kernel/setup.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
Index: linux-2.6/arch/x86/kernel/setup.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/setup.c
+++ linux-2.6/arch/x86/kernel/setup.c
@@ -697,21 +697,6 @@ void __init setup_arch(char **cmdline_p)
printk(KERN_INFO "Command line: %s\n", boot_command_line);
#endif
- strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
- *cmdline_p = command_line;
-
-#ifdef CONFIG_X86_64
- /*
- * Must call this twice: Once just to detect whether hardware doesn't
- * support NX (so that the early EHCI debug console setup can safely
- * call set_fixmap(), and then again after parsing early parameters to
- * honor the respective command line option.
- */
- check_efer();
-#endif
-
- parse_early_param();
-
/* VMI may relocate the fixmap; do this before touching ioremap area */
vmi_init();
@@ -794,6 +779,21 @@ void __init setup_arch(char **cmdline_p)
#endif
#endif
+ strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
+ *cmdline_p = command_line;
+
+#ifdef CONFIG_X86_64
+ /*
+ * Must call this twice: Once just to detect whether hardware doesn't
+ * support NX (so that the early EHCI debug console setup can safely
+ * call set_fixmap(), and then again after parsing early parameters to
+ * honor the respective command line option.
+ */
+ check_efer();
+#endif
+
+ parse_early_param();
+
#ifdef CONFIG_X86_64
check_efer();
#endif
* Yinghai Lu <[email protected]> wrote:
> Yinghai Lu wrote:
> > Ingo Molnar wrote:
> >> I thoroughly zapped it. Do you know about any commit ID where it snuck
> >> in?
> >
> > after close looking, it will break
> > 1. some cpu feature in early stage too, like cpu_has_x2apic
> > 2. will break built-in-command line
> > 3. will break other memmap= and mem=
> > 4. early_dbgp and early_console that will use early_ioremap to access mmio (?)
thanks Yinghai - i have queued up your revert. Thanks Hugh for noticing
it!
> after revet it, it seem mem=32g etc still broken, maybe some other
> commit about x86 platform support from Thomas broke it.
Thomas?
Ingo
Commit-ID: eda6da9286ad5b35b1eb70f6368958a8ee41a9dd
Gitweb: http://git.kernel.org/tip/eda6da9286ad5b35b1eb70f6368958a8ee41a9dd
Author: Yinghai Lu <[email protected]>
AuthorDate: Sat, 19 Sep 2009 11:07:57 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 19 Sep 2009 20:31:33 +0200
Revert 'x86: Fix system crash when loading with "reservetop" parameter'
After close looking, commit 8126dec3 will break:
1. some cpu feature in early stage too, like cpu_has_x2apic
2. will break built-in-command line
3. will break other memmap= and mem=
4. early_dbgp and early_console that will use early_ioremap to access mmio (?)
So revert it.
Reported-by: Hugh Dickins <[email protected]>,
Cc: Linus Torvalds <[email protected]>,
Cc: Andrew Morton <[email protected]>,
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/setup.c | 30 +++++++++++++++---------------
1 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 19f15c4..f5baa2a 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -712,21 +712,6 @@ void __init setup_arch(char **cmdline_p)
printk(KERN_INFO "Command line: %s\n", boot_command_line);
#endif
- strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
- *cmdline_p = command_line;
-
-#ifdef CONFIG_X86_64
- /*
- * Must call this twice: Once just to detect whether hardware doesn't
- * support NX (so that the early EHCI debug console setup can safely
- * call set_fixmap(), and then again after parsing early parameters to
- * honor the respective command line option.
- */
- check_efer();
-#endif
-
- parse_early_param();
-
/* VMI may relocate the fixmap; do this before touching ioremap area */
vmi_init();
@@ -809,6 +794,21 @@ void __init setup_arch(char **cmdline_p)
#endif
#endif
+ strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
+ *cmdline_p = command_line;
+
+#ifdef CONFIG_X86_64
+ /*
+ * Must call this twice: Once just to detect whether hardware doesn't
+ * support NX (so that the early EHCI debug console setup can safely
+ * call set_fixmap(), and then again after parsing early parameters to
+ * honor the respective command line option.
+ */
+ check_efer();
+#endif
+
+ parse_early_param();
+
#ifdef CONFIG_X86_64
check_efer();
#endif