[start_kernel] Suggest to move parse_args() before trap_init()

[start_kernel] Suggest to move parse_args() before trap_init()

[Zhu, Yi]

Hi,

I'm not sure it is _correct_ to move parse_args() before trap_init() in
start_kernel(). Is there any potencial dependencies? I did this on my P4 UP
box, it boots OK.

My issue is if the parse_args() runs after trap_init(), the kernel
parameter "lapic" and "nolapic" takes no effect. Because lapic_enable()
is called after init_apic_mappings().


--- init/main.c.orig    2004-03-01 16:54:23.000000000 +0800
+++ init/main.c 2004-03-01 16:54:45.000000000 +0800
@@ -416,11 +416,11 @@

        build_all_zonelists();
        page_alloc_init();
-       trap_init();
        printk("Kernel command line: %s\n", saved_command_line);
        parse_args("Booting kernel", command_line, __start___param,
                   __stop___param - __start___param,
                   &unknown_bootoption);
+       trap_init();
        sort_main_extable();
        rcu_init();
        init_IRQ();


Thanks,
-- 
-----------------------------------------------------------------
Opinions expressed are those of the author and do not represent
Intel Corp.

Zhu Yi (Chuyee)

GnuPG v1.0.6 (GNU/Linux)
http://cn.geocities.com/chewie_chuyee/gpg.txt or
$ gpg --keyserver wwwkeys.pgp.net --recv-keys 71C34820
1024D/71C34820 C939 2B0B FBCE 1D51 109A  55E5 8650 DB90 71C3 4820

Re: [start_kernel] Suggest to move parse_args() before trap_init()

[Andrew Morton]

"Zhu, Yi" <yi.zhu@intel.com> wrote:
>
> I'm not sure it is _correct_ to move parse_args() before trap_init() in
>  start_kernel(). Is there any potencial dependencies? I did this on my P4 UP
>  box, it boots OK.
> 
>  My issue is if the parse_args() runs after trap_init(), the kernel
>  parameter "lapic" and "nolapic" takes no effect. Because lapic_enable()
>  is called after init_apic_mappings().
> 
> 
>  --- init/main.c.orig    2004-03-01 16:54:23.000000000 +0800
>  +++ init/main.c 2004-03-01 16:54:45.000000000 +0800
>  @@ -416,11 +416,11 @@
> 
>          build_all_zonelists();
>          page_alloc_init();
>  -       trap_init();
>          printk("Kernel command line: %s\n", saved_command_line);
>          parse_args("Booting kernel", command_line, __start___param,
>                     __stop___param - __start___param,
>                     &unknown_bootoption);
>  +       trap_init();
>          sort_main_extable();
>          rcu_init();
>          init_IRQ();

I think the only problem with this is if we get a fault during
parse_args(), the kernel flies off into outer space.  So you lose some
debuggability when using an early console.

But 2.4 does trap_init() after parse_args() and nobody has complained, as
did 2.6 until recently.  So the change is probably OK.

Re: [start_kernel] Suggest to move parse_args() before trap_init()

[Andi Kleen]

Andrew Morton <akpm@osdl.org> writes:

> I think the only problem with this is if we get a fault during
> parse_args(), the kernel flies off into outer space.  So you lose some
> debuggability when using an early console.
> 
> But 2.4 does trap_init() after parse_args() and nobody has complained, as
> did 2.6 until recently.  So the change is probably OK.

The standard way to fix this is to add an explicit check for lapic
to the early argument parsing in setup.c (but keep the __setup so that
no unknown argument is reported).

I think that's better than moving it and getting possible bugs that cannot
even be catched with early printk.

Another way that i've considered on x86-64 for 2.7 at least is a special
__early_setup() for this

-Andi

Re: [start_kernel] Suggest to move parse_args() before trap_init()

[Dave Jones]

On Mon, Mar 01, 2004 at 09:46:38PM +0100, Andi Kleen wrote:

 > > I think the only problem with this is if we get a fault during
 > > parse_args(), the kernel flies off into outer space.  So you lose some
 > > debuggability when using an early console.
 > > 
 > > But 2.4 does trap_init() after parse_args() and nobody has complained, as
 > > did 2.6 until recently.  So the change is probably OK.
 > 
 > The standard way to fix this is to add an explicit check for lapic
 > to the early argument parsing in setup.c (but keep the __setup so that
 > no unknown argument is reported).

This just got me thinking of a sort-of related problem.
Some laptops hang when local apic is enabled, and we couldn't
blacklist them in 2.4 due to us not doing the dmi scan early enough.

Did that get fixed in 2.6 ?

		Dave

Re: [start_kernel] Suggest to move parse_args() before trap_init()

[Andi Kleen]

On Mon, 1 Mar 2004 20:54:26 +0000
Dave Jones <davej@redhat.com> wrote:


> Did that get fixed in 2.6 ?

It's called directly after paging_init now. That should be early enough.

-Andi

Re: [start_kernel] Suggest to move parse_args() before trap_init()

[Dave Hansen]

On Mon, 2004-03-01 at 12:46, Andi Kleen wrote:
> Andrew Morton <akpm@osdl.org> writes:
> 
> > I think the only problem with this is if we get a fault during
> > parse_args(), the kernel flies off into outer space.  So you lose some
> > debuggability when using an early console.
> > 
> > But 2.4 does trap_init() after parse_args() and nobody has complained, as
> > did 2.6 until recently.  So the change is probably OK.
>...
> Another way that i've considered on x86-64 for 2.7 at least is a special
> __early_setup() for this

It might be nice to have the same thing as the initcalls:
#define core_initcall(fn)               __define_initcall("1",fn)
...
#define late_initcall(fn)               __define_initcall("7",fn)

If we had something like this, we could also use it for stuff like
early_printk(), in addition to things like mem= in setup_arch().  You
can actually do serial early printk in about 3 lines of code if you can
parse arguments really, really early.  

-- dave

Re: [start_kernel] Suggest to move parse_args() before trap_init()

[Mikael Pettersson]

On Mon, 1 Mar 2004 20:54:26 +0000, Dave Jones wrote:
>On Mon, Mar 01, 2004 at 09:46:38PM +0100, Andi Kleen wrote:
>
> > > I think the only problem with this is if we get a fault during
> > > parse_args(), the kernel flies off into outer space.  So you lose some
> > > debuggability when using an early console.
> > > 
> > > But 2.4 does trap_init() after parse_args() and nobody has complained, as
> > > did 2.6 until recently.  So the change is probably OK.
> > 
> > The standard way to fix this is to add an explicit check for lapic
> > to the early argument parsing in setup.c (but keep the __setup so that
> > no unknown argument is reported).
>
>This just got me thinking of a sort-of related problem.
>Some laptops hang when local apic is enabled, and we couldn't
>blacklist them in 2.4 due to us not doing the dmi scan early enough.
>
>Did that get fixed in 2.6 ?

My patch for early DMI scan and local APIC blacklisting
was included in 2.5.6 and 2.4.19. This was done mainly
to handle all those broken Dell laptops.

/Mikael