[PACTH v2] powerpc/pseries/mce: Avoid instrumentation in realmode

Nicholas Piggin npiggin at gmail.com
Wed Sep 7 14:19:21 AEST 2022


On Mon Sep 5, 2022 at 4:38 PM AEST, Ganesh Goudar wrote:
> Part of machine check error handling is done in realmode,
> As of now instrumentation is not possible for any code that
> runs in realmode.
> When MCE is injected on KASAN enabled kernel, crash is
> observed, Hence force inline or mark no instrumentation
> for functions which can run in realmode, to avoid KASAN
> instrumentation.
>
> Signed-off-by: Ganesh Goudar <ganeshgr at linux.ibm.com>
> ---
> v2: Force inline few more functions.
> ---
>  arch/powerpc/include/asm/hw_irq.h    | 8 ++++----
>  arch/powerpc/include/asm/interrupt.h | 2 +-
>  arch/powerpc/include/asm/rtas.h      | 4 ++--
>  arch/powerpc/kernel/rtas.c           | 4 ++--
>  4 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index 26ede09c521d..3264991fe524 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -111,7 +111,7 @@ static inline void __hard_RI_enable(void)
>  #ifdef CONFIG_PPC64
>  #include <asm/paca.h>
>  
> -static inline notrace unsigned long irq_soft_mask_return(void)
> +static __always_inline notrace unsigned long irq_soft_mask_return(void)
>  {
>  	return READ_ONCE(local_paca->irq_soft_mask);
>  }
> @@ -121,7 +121,7 @@ static inline notrace unsigned long irq_soft_mask_return(void)
>   * for the critical section and as a clobber because
>   * we changed paca->irq_soft_mask
>   */
> -static inline notrace void irq_soft_mask_set(unsigned long mask)
> +static __always_inline notrace void irq_soft_mask_set(unsigned long mask)
>  {
>  	/*
>  	 * The irq mask must always include the STD bit if any are set.

This doesn't give a reason why it's __always_inline, and having the
notrace attribute makes it possibly confusing. I think it would be easy
for someone to break without realising. Could you add a noinstr to these
instead / as well?

What about adding a 'realmode' function annotation that includes noinstr?

Thanks,
Nick

> @@ -144,7 +144,7 @@ static inline notrace void irq_soft_mask_set(unsigned long mask)
>  	barrier();
>  }
>  
> -static inline notrace unsigned long irq_soft_mask_set_return(unsigned long mask)
> +static __always_inline notrace unsigned long irq_soft_mask_set_return(unsigned long mask)
>  {
>  	unsigned long flags = irq_soft_mask_return();
>  
> @@ -162,7 +162,7 @@ static inline notrace unsigned long irq_soft_mask_or_return(unsigned long mask)
>  	return flags;
>  }
>  
> -static inline unsigned long arch_local_save_flags(void)
> +static __always_inline unsigned long arch_local_save_flags(void)
>  {
>  	return irq_soft_mask_return();
>  }
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 8069dbc4b8d1..090895051712 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -92,7 +92,7 @@ static inline bool is_implicit_soft_masked(struct pt_regs *regs)
>  	return search_kernel_soft_mask_table(regs->nip);
>  }
>  
> -static inline void srr_regs_clobbered(void)
> +static __always_inline void srr_regs_clobbered(void)
>  {
>  	local_paca->srr_valid = 0;
>  	local_paca->hsrr_valid = 0;
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 00531af17ce0..52d29d664fdf 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -201,13 +201,13 @@ inline uint32_t rtas_ext_event_company_id(struct rtas_ext_event_log_v6 *ext_log)
>  #define PSERIES_ELOG_SECT_ID_MCE		(('M' << 8) | 'C')
>  
>  static
> -inline uint16_t pseries_errorlog_id(struct pseries_errorlog *sect)
> +__always_inline uint16_t pseries_errorlog_id(struct pseries_errorlog *sect)
>  {
>  	return be16_to_cpu(sect->id);
>  }
>  
>  static
> -inline uint16_t pseries_errorlog_length(struct pseries_errorlog *sect)
> +__always_inline uint16_t pseries_errorlog_length(struct pseries_errorlog *sect)
>  {
>  	return be16_to_cpu(sect->length);
>  }
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 693133972294..f9d78245c0e8 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -48,7 +48,7 @@
>  /* This is here deliberately so it's only used in this file */
>  void enter_rtas(unsigned long);
>  
> -static inline void do_enter_rtas(unsigned long args)
> +static __always_inline void do_enter_rtas(unsigned long args)
>  {
>  	unsigned long msr;
>  
> @@ -435,7 +435,7 @@ static char *__fetch_rtas_last_error(char *altbuf)
>  #endif
>  
>  
> -static void
> +noinstr static void
>  va_rtas_call_unlocked(struct rtas_args *args, int token, int nargs, int nret,
>  		      va_list list)
>  {
> -- 
> 2.37.1



More information about the Linuxppc-dev mailing list