On Mon Sep 26, 2022 at 4:18 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 <ganes...@linux.ibm.com>
> ---
> v2: Force inline few more functions.
>
> v3: Adding noinstr to few functions instead of __always_inline.
I would still like to consider doing a realmode annotation, but
as a minimal fix for the next merge window I suppose this is okay.
There's still no indication for why the annotation exists on the
functions which is a bit annoying, maybe not fundamentally worse
than notrace was, but the scope of reasons why it's there gets
bigger.
> ---
> 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 983551859891..c4d542b4a623 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)
> +noinstr static unsigned long irq_soft_mask_return(void)
> {
> unsigned long flags;
Don't uninline the ones in headers.
> @@ -128,7 +128,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)
> +noinstr static void irq_soft_mask_set(unsigned long mask)
> {
> /*
> * The irq mask must always include the STD bit if any are set.
> @@ -155,7 +155,7 @@ static inline notrace void irq_soft_mask_set(unsigned
> long mask)
> : "memory");
> }
>
> -static inline notrace unsigned long irq_soft_mask_set_return(unsigned long
> mask)
> +noinstr static unsigned long irq_soft_mask_set_return(unsigned long mask)
> {
> unsigned long flags;
>
> @@ -191,7 +191,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();
> }
Can we instead add noinstr to this too, the the other ones that were
changed to always inline?
Thanks,
Nick
> 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
