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

["Nicholas Piggin" <npiggin@gmail.com>]

On Mon Sep 19, 2022 at 4:03 PM AEST, Ganesh wrote:
> On 9/7/22 09:49, Nicholas Piggin wrote:
>
> > 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@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?
>
> Yeah we can add noinstr. Missed to see your comment, Sorry for the delayed reply

Okay that would be good. I would prefer to avoid changing the
inline-ness of things in a fix patch if possible.
>
> >
> > What about adding a 'realmode' function annotation that includes noinstr?
>
> You mean to define a new function annotation?

Yes, a powerpc specific one that has the necessary adjustments. I
think it would be helpful documentation for the code and possibly
something we could use to do additional debug checking with at
some point too.

Thanks,
Nick