Re: [RFC PATCH v2 02/10] powerpc/book3s: Introduce exclusive emergency stack for machine check exception.

[Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com>]

On 08/16/2013 02:29 PM, Madhavan Srinivasan wrote:
> On Friday 16 August 2013 01:34 PM, Mahesh J Salgaonkar wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> This patch introduces exclusive emergency stack for machine check exception.
>> We use emergency stack to handle machine check exception so that we can save
>> MCE information (srr1, srr0, dar and dsisr) before turning on ME bit and be
>> ready for re-entrancy. This helps us to prevent clobbering of MCE information
>> in case of nested machine checks.
>>
>> The reason for using emergency stack over normal kernel stack is that the
>> machine check might occur in the middle of setting up a stack frame which may
>> result into improper use of kernel stack.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/paca.h |    9 +++++++++
>>  arch/powerpc/kernel/setup_64.c  |    8 +++++++-
>>  arch/powerpc/xmon/xmon.c        |    2 ++
>>  3 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
>> index 77c91e7..b4ca4e9 100644
>> --- a/arch/powerpc/include/asm/paca.h
>> +++ b/arch/powerpc/include/asm/paca.h
>> @@ -147,6 +147,15 @@ struct paca_struct {
>>  	 */
>>  	struct opal_machine_check_event *opal_mc_evt;
>>  #endif
>> +#ifdef CONFIG_PPC_BOOK3S_64
>> +	/* Exclusive emergency stack pointer for machine check exception. */
>> +	void *mc_emergency_sp;
>> +	/*
>> +	 * Flag to check whether we are in machine check early handler
>> +	 * and already using emergency stack.
>> +	 */
>> +	u16 in_mce;
>> +#endif
>>
>>  	/* Stuff for accurate time accounting */
>>  	u64 user_time;			/* accumulated usermode TB ticks */
>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>> index 389fb807..3fdbdb0 100644
>> --- a/arch/powerpc/kernel/setup_64.c
>> +++ b/arch/powerpc/kernel/setup_64.c
>> @@ -529,7 +529,8 @@ static void __init exc_lvl_early_init(void)
>>
>>  /*
>>   * Stack space used when we detect a bad kernel stack pointer, and
>> - * early in SMP boots before relocation is enabled.
>> + * early in SMP boots before relocation is enabled. Exclusive emergency
>> + * stack for machine checks.
>>   */
>>  static void __init emergency_stack_init(void)
>>  {
>> @@ -552,6 +553,11 @@ static void __init emergency_stack_init(void)
>>  		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
>>  		sp += THREAD_SIZE;
>>  		paca[i].emergency_sp = __va(sp);
>> +
>> +		/* emergency stack for machine check exception handling. */
>> +		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
>> +		sp += THREAD_SIZE;
>> +		paca[i].mc_emergency_sp = __va(sp);
>>  	}
>>  }
>>
> Just a concern, kindly ignore it if it is irrelevant.
> you have defined mc_emergency_sp under CONFIG_PPC_BOOK3S_64, but
> assigning memory in a common code. This may break build for other
> configs such as like ppc64e_defconfig.

Nice catch. Will fix it in next spin.

Thanks,
-Mahesh.