[PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt - Kernel

This is a discussion on [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt - Kernel ; Function machine_halt (resp. native_machine_halt) is empty for x86 architectures. When command 'halt -f' is invoked, the message "System halted." is displayed but this is not really true because all CPUs are still running. There are also similar inconsistencies for other ...

+ Reply to Thread
Results 1 to 11 of 11

Thread: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt

  1. [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt

    Function machine_halt (resp. native_machine_halt) is empty for x86
    architectures. When command 'halt -f' is invoked, the message
    "System halted." is displayed but this is not really true because
    all CPUs are still running.
    There are also similar inconsistencies for other arches (some uses
    power-off for halt or forever-loop with IRQs enabled/disabled).
    IMO there should be used the same approach for all architectures
    OR what does the message "System halted" really mean?

    Signed-off-by: Ivan Vecera
    ---
    arch/x86/kernel/reboot.c | 8 ++++++++
    1 files changed, 8 insertions(+), 0 deletions(-)

    diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
    index f4c93f1..15ad949 100644
    --- a/arch/x86/kernel/reboot.c
    +++ b/arch/x86/kernel/reboot.c
    @@ -465,6 +465,14 @@ static void native_machine_restart(char *__unused)

    static void native_machine_halt(void)
    {
    + /* stop other cpus and apics */
    + machine_shutdown();
    +
    + /* stop this cpu */
    + local_irq_disable();
    + if (hlt_works(smp_processor_id()))
    + for (; halt();
    + for (;;
    }

    static void native_machine_power_off(void)
    --
    1.5.6.3

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

  2. Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt

    On Mon, Oct 20, 2008 at 02:13:07PM +0200, Ivan Vecera wrote:
    > Function machine_halt (resp. native_machine_halt) is empty for x86
    > architectures. When command 'halt -f' is invoked, the message
    > "System halted." is displayed but this is not really true because
    > all CPUs are still running.
    > There are also similar inconsistencies for other arches (some uses
    > power-off for halt or forever-loop with IRQs enabled/disabled).
    > IMO there should be used the same approach for all architectures
    > OR what does the message "System halted" really mean?
    >
    > Signed-off-by: Ivan Vecera
    > ---
    > arch/x86/kernel/reboot.c | 8 ++++++++
    > 1 files changed, 8 insertions(+), 0 deletions(-)
    >
    > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
    > index f4c93f1..15ad949 100644
    > --- a/arch/x86/kernel/reboot.c
    > +++ b/arch/x86/kernel/reboot.c
    > @@ -465,6 +465,14 @@ static void native_machine_restart(char *__unused)
    >
    > static void native_machine_halt(void)
    > {
    > + /* stop other cpus and apics */
    > + machine_shutdown();
    > +
    > + /* stop this cpu */
    > + local_irq_disable();
    > + if (hlt_works(smp_processor_id()))
    > + for (; halt();
    > + for (;;
    > }
    >
    > static void native_machine_power_off(void)
    > --
    > 1.5.6.3
    >


    Acked-by: Neil Horman

    --
    /************************************************** *
    *Neil Horman
    *Senior Software Engineer
    *Red Hat, Inc.
    *nhorman@redhat.com
    *gpg keyid: 1024D / 0x92A74FA1
    *http://pgp.mit.edu
    ************************************************** */
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt


    * Ivan Vecera wrote:

    > Function machine_halt (resp. native_machine_halt) is empty for x86
    > architectures. When command 'halt -f' is invoked, the message "System
    > halted." is displayed but this is not really true because all CPUs are
    > still running. There are also similar inconsistencies for other arches
    > (some uses power-off for halt or forever-loop with IRQs
    > enabled/disabled). IMO there should be used the same approach for all
    > architectures OR what does the message "System halted" really mean?


    no fundamental objections, but could you please do it a bit cleaner:

    > static void native_machine_halt(void)
    > {
    > + /* stop other cpus and apics */
    > + machine_shutdown();
    > +
    > + /* stop this cpu */
    > + local_irq_disable();
    > + if (hlt_works(smp_processor_id()))
    > + for (; halt();
    > + for (;;
    > }


    the code in arch/x86/kernel/smp.c::stop_this_cpu() is very similar to
    this and could be shared. You could move the stop_this_cpu() function to
    arch/x86/kernel/process.c (out of smp.c), so that it can be used by
    reboot.c.

    furthermore, native_machine_power_off() should probably fall back to
    native_machine_halt() as well - should pm_power_off() be disabled (or if
    it fails to stop the machine).

    hm?

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

  4. Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt

    Ingo Molnar wrote:
    > the code in arch/x86/kernel/smp.c::stop_this_cpu() is very similar to
    > this and could be shared. You could move the stop_this_cpu() function to
    > arch/x86/kernel/process.c (out of smp.c), so that it can be used by
    > reboot.c.
    >

    Yes, this make sense. Here is the patch.

    Ivan

    ---
    arch/x86/kernel/process.c | 16 ++++++++++++++++
    arch/x86/kernel/reboot.c | 5 +++++
    arch/x86/kernel/smp.c | 13 -------------
    include/asm-x86/system.h | 2 ++
    4 files changed, 23 insertions(+), 13 deletions(-)

    diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
    index c622772..af6904e 100644
    --- a/arch/x86/kernel/process.c
    +++ b/arch/x86/kernel/process.c
    @@ -8,6 +8,7 @@
    #include
    #include
    #include
    +#include

    unsigned long idle_halt;
    EXPORT_SYMBOL(idle_halt);
    @@ -122,6 +123,21 @@ void default_idle(void)
    EXPORT_SYMBOL(default_idle);
    #endif

    +void stop_this_cpu(void *dummy)
    +{
    + local_irq_disable();
    + /*
    + * Remove this CPU:
    + */
    + cpu_clear(smp_processor_id(), cpu_online_map);
    +#ifdef CONFIG_X86_LOCAL_APIC
    + disable_local_APIC();
    +#endif
    + if (hlt_works(smp_processor_id()))
    + for (; halt();
    + for (;;
    +}
    +
    static void do_nothing(void *unused)
    {
    }
    diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
    index f4c93f1..3113d9e 100644
    --- a/arch/x86/kernel/reboot.c
    +++ b/arch/x86/kernel/reboot.c
    @@ -465,6 +465,11 @@ static void native_machine_restart(char *__unused)

    static void native_machine_halt(void)
    {
    + /* stop other cpus and apics */
    + machine_shutdown();
    +
    + /* stop this cpu */
    + stop_this_cpu(NULL);
    }

    static void native_machine_power_off(void)
    diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
    index 18f9b19..3f92b13 100644
    --- a/arch/x86/kernel/smp.c
    +++ b/arch/x86/kernel/smp.c
    @@ -140,19 +140,6 @@ void native_send_call_func_ipi(cpumask_t mask)
    send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
    }

    -static void stop_this_cpu(void *dummy)
    -{
    - local_irq_disable();
    - /*
    - * Remove this CPU:
    - */
    - cpu_clear(smp_processor_id(), cpu_online_map);
    - disable_local_APIC();
    - if (hlt_works(smp_processor_id()))
    - for (; halt();
    - for (;;
    -}
    -
    /*
    * this function calls the 'stop' function on all other CPUs in the system.
    */
    diff --git a/include/asm-x86/system.h b/include/asm-x86/system.h
    index b20c894..17ac753 100644
    --- a/include/asm-x86/system.h
    +++ b/include/asm-x86/system.h
    @@ -314,6 +314,8 @@ extern void free_init_pages(char *what, unsigned long begin, unsigned long end);

    void default_idle(void);

    +void stop_this_cpu(void *dummy);
    +
    /*
    * Force strict CPU ordering.
    * And yes, this is required on UP too when we're talking
    --
    1.5.6.3
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt

    On Thu, Nov 06, 2008 at 05:10:01PM +0100, Ivan Vecera wrote:
    > Any comments?
    >
    > Ivan
    >

    Acked-by: Neil Horman

    > ---
    > Ivan Vecera wrote:
    > > Ingo Molnar wrote:
    > >> the code in arch/x86/kernel/smp.c::stop_this_cpu() is very similar to
    > >> this and could be shared. You could move the stop_this_cpu() function to
    > >> arch/x86/kernel/process.c (out of smp.c), so that it can be used by
    > >> reboot.c.
    > >>

    > > Yes, this make sense. Here is the patch.
    > >
    > > Ivan
    > >
    > > ---
    > > arch/x86/kernel/process.c | 16 ++++++++++++++++
    > > arch/x86/kernel/reboot.c | 5 +++++
    > > arch/x86/kernel/smp.c | 13 -------------
    > > include/asm-x86/system.h | 2 ++
    > > 4 files changed, 23 insertions(+), 13 deletions(-)
    > >
    > > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
    > > index c622772..af6904e 100644
    > > --- a/arch/x86/kernel/process.c
    > > +++ b/arch/x86/kernel/process.c
    > > @@ -8,6 +8,7 @@
    > > #include
    > > #include
    > > #include
    > > +#include
    > >
    > > unsigned long idle_halt;
    > > EXPORT_SYMBOL(idle_halt);
    > > @@ -122,6 +123,21 @@ void default_idle(void)
    > > EXPORT_SYMBOL(default_idle);
    > > #endif
    > >
    > > +void stop_this_cpu(void *dummy)
    > > +{
    > > + local_irq_disable();
    > > + /*
    > > + * Remove this CPU:
    > > + */
    > > + cpu_clear(smp_processor_id(), cpu_online_map);
    > > +#ifdef CONFIG_X86_LOCAL_APIC
    > > + disable_local_APIC();
    > > +#endif
    > > + if (hlt_works(smp_processor_id()))
    > > + for (; halt();
    > > + for (;;
    > > +}
    > > +
    > > static void do_nothing(void *unused)
    > > {
    > > }
    > > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
    > > index f4c93f1..3113d9e 100644
    > > --- a/arch/x86/kernel/reboot.c
    > > +++ b/arch/x86/kernel/reboot.c
    > > @@ -465,6 +465,11 @@ static void native_machine_restart(char *__unused)
    > >
    > > static void native_machine_halt(void)
    > > {
    > > + /* stop other cpus and apics */
    > > + machine_shutdown();
    > > +
    > > + /* stop this cpu */
    > > + stop_this_cpu(NULL);
    > > }
    > >
    > > static void native_machine_power_off(void)
    > > diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
    > > index 18f9b19..3f92b13 100644
    > > --- a/arch/x86/kernel/smp.c
    > > +++ b/arch/x86/kernel/smp.c
    > > @@ -140,19 +140,6 @@ void native_send_call_func_ipi(cpumask_t mask)
    > > send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
    > > }
    > >
    > > -static void stop_this_cpu(void *dummy)
    > > -{
    > > - local_irq_disable();
    > > - /*
    > > - * Remove this CPU:
    > > - */
    > > - cpu_clear(smp_processor_id(), cpu_online_map);
    > > - disable_local_APIC();
    > > - if (hlt_works(smp_processor_id()))
    > > - for (; halt();
    > > - for (;;
    > > -}
    > > -
    > > /*
    > > * this function calls the 'stop' function on all other CPUs in the system.
    > > */
    > > diff --git a/include/asm-x86/system.h b/include/asm-x86/system.h
    > > index b20c894..17ac753 100644
    > > --- a/include/asm-x86/system.h
    > > +++ b/include/asm-x86/system.h
    > > @@ -314,6 +314,8 @@ extern void free_init_pages(char *what, unsigned long begin, unsigned long end);
    > >
    > > void default_idle(void);
    > >
    > > +void stop_this_cpu(void *dummy);
    > > +
    > > /*
    > > * Force strict CPU ordering.
    > > * And yes, this is required on UP too when we're talking

    >


    --
    /************************************************** *
    *Neil Horman
    *Senior Software Engineer
    *Red Hat, Inc.
    *nhorman@redhat.com
    *gpg keyid: 1024D / 0x92A74FA1
    *http://pgp.mit.edu
    ************************************************** */
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt

    Any comments?

    Ivan

    ---
    Ivan Vecera wrote:
    > Ingo Molnar wrote:
    >> the code in arch/x86/kernel/smp.c::stop_this_cpu() is very similar to
    >> this and could be shared. You could move the stop_this_cpu() function to
    >> arch/x86/kernel/process.c (out of smp.c), so that it can be used by
    >> reboot.c.
    >>

    > Yes, this make sense. Here is the patch.
    >
    > Ivan
    >
    > ---
    > arch/x86/kernel/process.c | 16 ++++++++++++++++
    > arch/x86/kernel/reboot.c | 5 +++++
    > arch/x86/kernel/smp.c | 13 -------------
    > include/asm-x86/system.h | 2 ++
    > 4 files changed, 23 insertions(+), 13 deletions(-)
    >
    > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
    > index c622772..af6904e 100644
    > --- a/arch/x86/kernel/process.c
    > +++ b/arch/x86/kernel/process.c
    > @@ -8,6 +8,7 @@
    > #include
    > #include
    > #include
    > +#include
    >
    > unsigned long idle_halt;
    > EXPORT_SYMBOL(idle_halt);
    > @@ -122,6 +123,21 @@ void default_idle(void)
    > EXPORT_SYMBOL(default_idle);
    > #endif
    >
    > +void stop_this_cpu(void *dummy)
    > +{
    > + local_irq_disable();
    > + /*
    > + * Remove this CPU:
    > + */
    > + cpu_clear(smp_processor_id(), cpu_online_map);
    > +#ifdef CONFIG_X86_LOCAL_APIC
    > + disable_local_APIC();
    > +#endif
    > + if (hlt_works(smp_processor_id()))
    > + for (; halt();
    > + for (;;
    > +}
    > +
    > static void do_nothing(void *unused)
    > {
    > }
    > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
    > index f4c93f1..3113d9e 100644
    > --- a/arch/x86/kernel/reboot.c
    > +++ b/arch/x86/kernel/reboot.c
    > @@ -465,6 +465,11 @@ static void native_machine_restart(char *__unused)
    >
    > static void native_machine_halt(void)
    > {
    > + /* stop other cpus and apics */
    > + machine_shutdown();
    > +
    > + /* stop this cpu */
    > + stop_this_cpu(NULL);
    > }
    >
    > static void native_machine_power_off(void)
    > diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
    > index 18f9b19..3f92b13 100644
    > --- a/arch/x86/kernel/smp.c
    > +++ b/arch/x86/kernel/smp.c
    > @@ -140,19 +140,6 @@ void native_send_call_func_ipi(cpumask_t mask)
    > send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
    > }
    >
    > -static void stop_this_cpu(void *dummy)
    > -{
    > - local_irq_disable();
    > - /*
    > - * Remove this CPU:
    > - */
    > - cpu_clear(smp_processor_id(), cpu_online_map);
    > - disable_local_APIC();
    > - if (hlt_works(smp_processor_id()))
    > - for (; halt();
    > - for (;;
    > -}
    > -
    > /*
    > * this function calls the 'stop' function on all other CPUs in the system.
    > */
    > diff --git a/include/asm-x86/system.h b/include/asm-x86/system.h
    > index b20c894..17ac753 100644
    > --- a/include/asm-x86/system.h
    > +++ b/include/asm-x86/system.h
    > @@ -314,6 +314,8 @@ extern void free_init_pages(char *what, unsigned long begin, unsigned long end);
    >
    > void default_idle(void);
    >
    > +void stop_this_cpu(void *dummy);
    > +
    > /*
    > * Force strict CPU ordering.
    > * And yes, this is required on UP too when we're talking


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

  7. Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt


    * Ivan Vecera wrote:

    > Ingo Molnar wrote:
    > > the code in arch/x86/kernel/smp.c::stop_this_cpu() is very similar to
    > > this and could be shared. You could move the stop_this_cpu() function to
    > > arch/x86/kernel/process.c (out of smp.c), so that it can be used by
    > > reboot.c.
    > >

    > Yes, this make sense. Here is the patch.


    looks good. One small detail:

    > +#ifdef CONFIG_X86_LOCAL_APIC
    > + disable_local_APIC();
    > +#endif


    could you please avoid this #ifdef by putting an inline void function
    for disable_local_APIC() into arch/x86/include/asm/apic.h for the
    !CONFIG_X86_LOCAL_APIC case?

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

  8. Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt

    On Thu 2008-11-06 17:10:01, Ivan Vecera wrote:
    > Any comments?
    >
    > Ivan
    >
    > ---
    > Ivan Vecera wrote:
    > > Ingo Molnar wrote:
    > >> the code in arch/x86/kernel/smp.c::stop_this_cpu() is very similar to
    > >> this and could be shared. You could move the stop_this_cpu() function to
    > >> arch/x86/kernel/process.c (out of smp.c), so that it can be used by
    > >> reboot.c.
    > >>

    > > Yes, this make sense. Here is the patch.
    > >
    > > Ivan
    > >
    > > ---
    > > arch/x86/kernel/process.c | 16 ++++++++++++++++
    > > arch/x86/kernel/reboot.c | 5 +++++
    > > arch/x86/kernel/smp.c | 13 -------------
    > > include/asm-x86/system.h | 2 ++
    > > 4 files changed, 23 insertions(+), 13 deletions(-)
    > >
    > > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
    > > index c622772..af6904e 100644
    > > --- a/arch/x86/kernel/process.c
    > > +++ b/arch/x86/kernel/process.c
    > > @@ -8,6 +8,7 @@
    > > #include
    > > #include
    > > #include
    > > +#include
    > >
    > > unsigned long idle_halt;
    > > EXPORT_SYMBOL(idle_halt);
    > > @@ -122,6 +123,21 @@ void default_idle(void)
    > > EXPORT_SYMBOL(default_idle);
    > > #endif
    > >
    > > +void stop_this_cpu(void *dummy)
    > > +{
    > > + local_irq_disable();
    > > + /*
    > > + * Remove this CPU:
    > > + */
    > > + cpu_clear(smp_processor_id(), cpu_online_map);
    > > +#ifdef CONFIG_X86_LOCAL_APIC
    > > + disable_local_APIC();
    > > +#endif
    > > + if (hlt_works(smp_processor_id()))
    > > + for (; halt();
    > > + for (;;
    > > +}


    Why the new ifdef? And while we are at it, why is it neccessary to
    disable APIC for stopping CPU? (comment in code would be nice)

    > > -static void stop_this_cpu(void *dummy)
    > > -{
    > > - local_irq_disable();
    > > - /*
    > > - * Remove this CPU:
    > > - */
    > > - cpu_clear(smp_processor_id(), cpu_online_map);
    > > - disable_local_APIC();
    > > - if (hlt_works(smp_processor_id()))
    > > - for (; halt();
    > > - for (;;
    > > -}
    > > -
    > > /*
    > > * this function calls the 'stop' function on all other CPUs in the system.
    > > */


    --
    (english) http://www.livejournal.com/~pavelmachek
    (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pav...rses/blog.html
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt

    Pavel Machek wrote:
    > On Thu 2008-11-06 17:10:01, Ivan Vecera wrote:
    >> Any comments?
    >>
    >> Ivan
    >>
    >> ---
    >> Ivan Vecera wrote:
    >>> Ingo Molnar wrote:
    >>>> the code in arch/x86/kernel/smp.c::stop_this_cpu() is very similar to
    >>>> this and could be shared. You could move the stop_this_cpu() function to
    >>>> arch/x86/kernel/process.c (out of smp.c), so that it can be used by
    >>>> reboot.c.
    >>>>
    >>> Yes, this make sense. Here is the patch.
    >>>
    >>> Ivan
    >>>
    >>> ---
    >>> arch/x86/kernel/process.c | 16 ++++++++++++++++
    >>> arch/x86/kernel/reboot.c | 5 +++++
    >>> arch/x86/kernel/smp.c | 13 -------------
    >>> include/asm-x86/system.h | 2 ++
    >>> 4 files changed, 23 insertions(+), 13 deletions(-)
    >>>
    >>> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
    >>> index c622772..af6904e 100644
    >>> --- a/arch/x86/kernel/process.c
    >>> +++ b/arch/x86/kernel/process.c
    >>> @@ -8,6 +8,7 @@
    >>> #include
    >>> #include
    >>> #include
    >>> +#include
    >>>
    >>> unsigned long idle_halt;
    >>> EXPORT_SYMBOL(idle_halt);
    >>> @@ -122,6 +123,21 @@ void default_idle(void)
    >>> EXPORT_SYMBOL(default_idle);
    >>> #endif
    >>>
    >>> +void stop_this_cpu(void *dummy)
    >>> +{
    >>> + local_irq_disable();
    >>> + /*
    >>> + * Remove this CPU:
    >>> + */
    >>> + cpu_clear(smp_processor_id(), cpu_online_map);
    >>> +#ifdef CONFIG_X86_LOCAL_APIC
    >>> + disable_local_APIC();
    >>> +#endif
    >>> + if (hlt_works(smp_processor_id()))
    >>> + for (; halt();
    >>> + for (;;
    >>> +}

    >
    > Why the new ifdef? And while we are at it, why is it neccessary to
    > disable APIC for stopping CPU? (comment in code would be nice)

    Because stop_this_cpu is shared between native_machine_halt and
    native_smp_send_stop. It was only moved from smp.c to process.c.
    native_machine_halt doesn't require disable APIC for stopping the
    current CPU because at this time is already disabled.
    The ifdef is another thing and I sent already corrected patch.
    >
    >>> -static void stop_this_cpu(void *dummy)
    >>> -{
    >>> - local_irq_disable();
    >>> - /*
    >>> - * Remove this CPU:
    >>> - */
    >>> - cpu_clear(smp_processor_id(), cpu_online_map);
    >>> - disable_local_APIC();
    >>> - if (hlt_works(smp_processor_id()))
    >>> - for (; halt();
    >>> - for (;;
    >>> -}
    >>> -
    >>> /*
    >>> * this function calls the 'stop' function on all other CPUs in the system.
    >>> */

    >


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

  10. Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt

    Ingo Molnar wrote:
    > looks good. One small detail:
    >
    >> +#ifdef CONFIG_X86_LOCAL_APIC
    >> + disable_local_APIC();
    >> +#endif

    >
    > could you please avoid this #ifdef by putting an inline void function
    > for disable_local_APIC() into arch/x86/include/asm/apic.h for the
    > !CONFIG_X86_LOCAL_APIC case?
    >
    > Ingo

    OK, here is the result.

    Ivan

    ---
    arch/x86/include/asm/apic.h | 1 +
    arch/x86/include/asm/system.h | 2 ++
    arch/x86/kernel/process.c | 14 ++++++++++++++
    arch/x86/kernel/reboot.c | 5 +++++
    arch/x86/kernel/smp.c | 13 -------------
    5 files changed, 22 insertions(+), 13 deletions(-)

    diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
    index 3b1510b..25caa07 100644
    --- a/arch/x86/include/asm/apic.h
    +++ b/arch/x86/include/asm/apic.h
    @@ -193,6 +193,7 @@ extern u8 setup_APIC_eilvt_ibs(u8 vector, u8 msg_type, u8 mask);
    static inline void lapic_shutdown(void) { }
    #define local_apic_timer_c2_ok 1
    static inline void init_apic_mappings(void) { }
    +static inline void disable_local_APIC(void) { }

    #endif /* !CONFIG_X86_LOCAL_APIC */

    diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h
    index 2ed3f0f..07c3e40 100644
    --- a/arch/x86/include/asm/system.h
    +++ b/arch/x86/include/asm/system.h
    @@ -314,6 +314,8 @@ extern void free_init_pages(char *what, unsigned long begin, unsigned long end);

    void default_idle(void);

    +void stop_this_cpu(void *dummy);
    +
    /*
    * Force strict CPU ordering.
    * And yes, this is required on UP too when we're talking
    diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
    index c622772..3380458 100644
    --- a/arch/x86/kernel/process.c
    +++ b/arch/x86/kernel/process.c
    @@ -8,6 +8,7 @@
    #include
    #include
    #include
    +#include

    unsigned long idle_halt;
    EXPORT_SYMBOL(idle_halt);
    @@ -122,6 +123,19 @@ void default_idle(void)
    EXPORT_SYMBOL(default_idle);
    #endif

    +void stop_this_cpu(void *dummy)
    +{
    + local_irq_disable();
    + /*
    + * Remove this CPU:
    + */
    + cpu_clear(smp_processor_id(), cpu_online_map);
    + disable_local_APIC();
    + if (hlt_works(smp_processor_id()))
    + for (; halt();
    + for (;;
    +}
    +
    static void do_nothing(void *unused)
    {
    }
    diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
    index 724adfc..34f8d37 100644
    --- a/arch/x86/kernel/reboot.c
    +++ b/arch/x86/kernel/reboot.c
    @@ -461,6 +461,11 @@ static void native_machine_restart(char *__unused)

    static void native_machine_halt(void)
    {
    + /* stop other cpus and apics */
    + machine_shutdown();
    +
    + /* stop this cpu */
    + stop_this_cpu(NULL);
    }

    static void native_machine_power_off(void)
    diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
    index 18f9b19..3f92b13 100644
    --- a/arch/x86/kernel/smp.c
    +++ b/arch/x86/kernel/smp.c
    @@ -140,19 +140,6 @@ void native_send_call_func_ipi(cpumask_t mask)
    send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
    }

    -static void stop_this_cpu(void *dummy)
    -{
    - local_irq_disable();
    - /*
    - * Remove this CPU:
    - */
    - cpu_clear(smp_processor_id(), cpu_online_map);
    - disable_local_APIC();
    - if (hlt_works(smp_processor_id()))
    - for (; halt();
    - for (;;
    -}
    -
    /*
    * this function calls the 'stop' function on all other CPUs in the system.
    */
    --
    1.5.6.3

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

  11. Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt


    * Ivan Vecera wrote:

    > Ingo Molnar wrote:
    > > looks good. One small detail:
    > >
    > >> +#ifdef CONFIG_X86_LOCAL_APIC
    > >> + disable_local_APIC();
    > >> +#endif

    > >
    > > could you please avoid this #ifdef by putting an inline void function
    > > for disable_local_APIC() into arch/x86/include/asm/apic.h for the
    > > !CONFIG_X86_LOCAL_APIC case?
    > >
    > > Ingo

    > OK, here is the result.
    >
    > Ivan
    >
    > ---
    > arch/x86/include/asm/apic.h | 1 +
    > arch/x86/include/asm/system.h | 2 ++
    > arch/x86/kernel/process.c | 14 ++++++++++++++
    > arch/x86/kernel/reboot.c | 5 +++++
    > arch/x86/kernel/smp.c | 13 -------------
    > 5 files changed, 22 insertions(+), 13 deletions(-)


    applied to tip/x86/reboot, thanks Ivan!

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

+ Reply to Thread