[PATCH 3/8] arm64: stacktrace: move SDEI stack helpers to stacktrace code

[Kalesh Singh]

On Tue, Aug 2, 2022 at 5:26 AM Mark Rutland <mark.rutland at arm.com> wrote:
>
> Hi Kalesh,
>
> On Mon, Aug 01, 2022 at 09:53:35PM -0700, Kalesh Singh wrote:
> > On Mon, Aug 1, 2022 at 5:12 AM Mark Rutland <mark.rutland at arm.com> wrote:
> > > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> > > index 4c8865e495fea..04a9b56b114c1 100644
> > > --- a/arch/arm64/kernel/stacktrace.c
> > > +++ b/arch/arm64/kernel/stacktrace.c
> > > @@ -86,8 +86,17 @@ static bool on_accessible_stack(const struct task_struct *tsk,
> > >                 return true;
> > >         if (on_overflow_stack(sp, size, info))
> > >                 return true;
> > > -       if (on_sdei_stack(sp, size, info))
> > > -               return true;
> > > +
> > > +       if (IS_ENABLED(CONFIG_VMAP_STACK) &&
> > > +           IS_ENABLED(CONFIG_ARM_SDE_INTERFACE) &&
> > > +           in_nmi())
> >
> > I think we can remove the IS_ENABLED() checks since it's handled by
> > ifdefs in asm/stacktrace.h
> >
> > Otherwise, Reviewed-by: Kalesh Singh <kaleshsingh at google.com>
>
> I'd kept the IS_ENABLED() checks here to avoid code being generated of in_nmi()
> when SDEI is not configured in. Since in_nmi() uses preempt_count() and that
> uses READ_ONCE(), the compiler can't optimize the read away in case there are
> side effects (but I imagine will discard the value immediately).
>
> With that in mind, are you happy if I leave this as is, and take your
> Reviewed-by?

Hi Mark,

Feel free to keep the Reviewed-by. Thanks for the improvements.

--Kalesh


>
> Thanks,
> Mark.
>
> >
> > > +       {
> > > +               if (on_sdei_critical_stack(sp, size, info))
> > > +                       return true;
> > > +
> > > +               if (on_sdei_normal_stack(sp, size, info))
> > > +                       return true;
> > > +       }
> > >
> > >         return false;
> > >  }
> > > --
> > > 2.30.2
> > >