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

[Ivan Vecera <ivecera@redhat.com>]

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 <linux/pm.h>
>>>  #include <linux/clockchips.h>
>>>  #include <asm/system.h>
>>> +#include <asm/apic.h>
>>>  
>>>  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.
>>>   */
>