Re: [v2][PATCH 4/7] book3e/kexec/kdump: introduce a kexec kernel flag

tiejun.chen
2013-07-09T07:51:19Z

On 07/02/2013 01:37 PM, Bhushan Bharat-R65777 wrote:
>
>
>> -----Original Message-----
>> From: Linuxppc-dev [mailto:linuxppc-dev-
>> bounces+bharat.bhushan=freescale.com@lists.ozlabs.org] On Behalf Of Tiejun Chen
>> Sent: Thursday, June 20, 2013 1:23 PM
>> To: benh@kernel.crashing.org
>> Cc: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
>> Subject: [v2][PATCH 4/7] book3e/kexec/kdump: introduce a kexec kernel flag
>>
>> We need to introduce a flag to indicate we're already running
>> a kexec kernel then we can go proper path. For example, We
>> shouldn't access spin_table from the bootloader to up any secondary
>> cpu for kexec kernel, and kexec kernel already know how to jump to
>> generic_secondary_smp_init.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
>> ---

[snip]

>> +++ b/arch/powerpc/platforms/85xx/smp.c
>> @@ -150,6 +150,9 @@ static int __cpuinit smp_85xx_kick_cpu(int nr)
>> int hw_cpu = get_hard_smp_processor_id(nr);
>> int ioremappable;
>> int ret = 0;
>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_CRASH_DUMP)
>> + unsigned long *ptr;
>> +#endif
>
> What about if we can remove the ifdef around *ptr ...
>
>>
>> WARN_ON(nr < 0 || nr >= NR_CPUS);
>> WARN_ON(hw_cpu < 0 || hw_cpu >= NR_CPUS);
>> @@ -238,11 +241,22 @@ out:
>> #else
>> smp_generic_kick_cpu(nr);
>>
>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_CRASH_DUMP)
>> + ptr = (unsigned long *)((unsigned long)&__run_at_kexec);
>
> ... #endif here ...
>
>> + /* We shouldn't access spin_table from the bootloader to up any
>> + * secondary cpu for kexec kernel, and kexec kernel already
>> + * know how to jump to generic_secondary_smp_init.
>> + */
>> + if (!*ptr) {
>> +#endif
>
> ... remove #endif ...
>
>> flush_spin_table(spin_table);
>> out_be32(&spin_table->pir, hw_cpu);
>> out_be64((u64 *)(&spin_table->addr_h),
>> __pa((u64)*((unsigned long long *)generic_secondary_smp_init)));
>> flush_spin_table(spin_table);
>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_CRASH_DUMP)
>> + }
>> +#endif
>
> --- remove above 3 lines

I'd like to try to address your comments next version.

Thanks

Tiejun