[Crash-utility] arm64: Support overflow stack panic

[HAGIO KAZUHITO(萩尾 一仁)]

-----Original Message-----
> Hi Kazu
> 
> Thanks for your time to review the patches and all the detailed comments, the 3rd patch attached with all
> the refinement.

Thanks, it looks good.

> 
> By the way, do I need to squash all the 3 patches as one before them get accepted?

yes, that'll be better.

Thanks,
Kazu

> 
> Thanks
> Hong
> ________________________________
> 
> From: HAGIO KAZUHITO(萩尾　一仁) <k-hagio-ab at nec.com>
> Sent: Friday, November 19, 2021 16:21
> To: Hong Yang3 杨红 <hong.yang3 at nio.com>
> Cc: Discussion list for crash utility usage, maintenance and development <crash-utility at redhat.com>
> Subject: RE: arm64: Support overflow stack panic
> 
> 注意：此封邮件来自于公司外部，请注意信息安全！
> Attention: This email comes from outside of the company, please pay attention to the information security!
> 
> Hi Hong,
> 
> Thanks for the refinement.
> 
> > +     if (symbol_exists("overflow_stack") &&
> > +         (sp = per_cpu_symbol_search("overflow_stack")) &&
> > +         get_symbol_type("overflow_stack", NULL, req)) {
> > +             if (CRASHDEBUG(1)) {
> > +                     fprintf(fp, "overflow_stack: \n");
> > +                     fprintf(fp, "  type: %x, %s\n",
> > +                             (int)req->typecode,
> > +                             (req->typecode == TYPE_CODE_PTR) ?
> > +                                             "TYPE_CODE_PTR" : "other");
> 
> Given that overflow_stack is array, TYPE_CODE_ARRAY would be better.
> 
> > @@ -2673,6 +2724,12 @@ arm64_back_trace_cmd(struct bt_info *bt)
> >                       bt->hp->eip : GET_STACK_ULONG(bt->hp->esp);
> >               stackframe.sp = bt->hp->esp + 8;
> >               bt->flags &= ~BT_REGS_NOT_FOUND;
> > +     } else if (arm64_on_overflow_stack(bt->tc->processor, bt->frameptr)) {
> > +             arm64_set_overflow_stack(bt);
> > +             bt->flags |= BT_OVERFLOW_STACK;
> 
> I would prefer to place this into the else block below
> 
> > +             stackframe.sp = bt->stkptr;
> > +             stackframe.pc = bt->instptr;
> > +             stackframe.fp = bt->frameptr;
> 
> then we can remove these three lines.
> 
> >       } else {
> >               if (arm64_on_irq_stack(bt->tc->processor, bt->frameptr)) {
> >                       arm64_set_irq_stack(bt);
> 
> > +static int
> > +arm64_in_alternate_stack(int cpu, ulong stkptr)
> > +{
> > +     struct machine_specific *ms = machdep->machspec;
> > +
> > +     return arm64_in_alternate_stackv(cpu, stkptr,
> > +                     ms->irq_stacks, ms->irq_stack_size);
> > +}
> 
> Given that in_alternate_stack() should check if it's in non-process stacks,
> I think we should check also for overflow_stacks here.  So how about making
> these function like this?
> 
> arm64_in_alternate_stack
>   arm64_on_irq_stack
>   arm64_on_overflow_stack
> 
> arm64_on_irq_stack
>   arm64_in_alternate_stackv(irq_stacks)
> 
> arm64_on_overflow_stack
>   arm64_in_alternate_stackv(overflow_stacks)
> 
> Thanks,
> Kazu
> 
> -----Original Message-----
> > Hi Kazu
> >
> > Here are the latest patches for supporting to run bt command against a core dump with kernel stack overflow
> > exception for arm64.
> >
> > Please help to review and advise if any further change needed.
> >
> > Tested bt command with options:
> >
> > bt
> > bt -a
> > bt -c 3
> >
> > By the way, 'mach' command also updated to show overflow stacks info as same as IRQ stacks.
> >
> > Thanks
> > Hong
> > ________________________________
> >
> > From: HAGIO KAZUHITO(萩尾　一仁) <k-hagio-ab at nec.com>
> > Sent: Wednesday, November 17, 2021 15:23
> > To: Hong Yang3 杨红 <hong.yang3 at nio.com>
> > Cc: Discussion list for crash utility usage, maintenance and development <crash-utility at redhat.com>
> > Subject: RE: arm64: Support overflow stack panic
> >
> > 注意：此封邮件来自于公司外部，请注意信息安全！
> > Attention: This email comes from outside of the company, please pay attention to the information security!
> >
> > Hi Hong,
> >
> > Thank you for the patch and sending it to this list.
> >
> > -----Original Message-----
> > > Hi Crash
> > >
> > > I'll keep refining the patch before it get approved:
> >
> > OK, so we will wait for the refined patch.
> >
> > Thanks,
> > Kazu
> >
> > >
> > >
> > > 1.    Fix the error in arm64_overflow_stack_init() which saved the overflow stack address into
> > > ms->irqstacks[], which would cause bt command crash on other cpus. The normal IRQ stacks should be used
> > > for bt command for other cpus.
> > > 2.    In addition to unwind on the overflow stack, try to go through the IRQ stack to find more useful
> > > information
> > >
> > > Kernel stack overflow case would be rarely but I'd like to sharp the crash to cover this kind of issue.
> > >
> > > Best regards
> > > Hong
> > > ________________________________
> > >
> > > From: Hong Yang3 杨红
> > > Sent: Tuesday, November 16, 2021 9:55
> > > To: crash-utility at redhat.com <crash-utility at redhat.com>
> > > Subject: arm64: Support overflow stack panic
> > >
> > > Hi All
> > >
> > > When I was trying to open a core of an overflow stack panic result, the bt command caused a segment
> fault,
> > > after a while I figured out the overflow stack is not supported by crash utility.
> > >
> > > This patch is trying to initialize the overflow stack information on startup stage, and the bt command
> > works
> > > as expected to dump the correct call trace in the overflow stack, currently  it only apply to arm64
> target.
> > >
> > > I'm not sure if any other sub command also need to be fixed for full support for the overflow stack,
> please
> > > advise and I'll try to improve the patch.
> > >
> > > Thanks
> > > Hong YANG
>