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<ganes...@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
