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

[Madhavan Srinivasan <maddy@linux.vnet.ibm.com>]

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.

> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 96bf5bd..147a5e98 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -2044,6 +2044,8 @@ static void dump_one_paca(int cpu)
>  	DUMP(p, stab_addr, "lx");
>  #endif
>  	DUMP(p, emergency_sp, "p");
> +	DUMP(p, mc_emergency_sp, "p");
> +	DUMP(p, in_mce, "x");
>  	DUMP(p, data_offset, "lx");
>  	DUMP(p, hw_cpu_id, "x");
>  	DUMP(p, cpu_start, "x");
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>