----- Original Message -----
> On Fri, Dec 18, 2015 at 11:55:07PM +0100, Andrew Jones wrote:
> > compat user mode prstatus already just works, almost. This missing
> > pieces are that pt_regs->sp and pt_regs->fp are not in their usual
> > locations. We need to pull them out of their architecturally mapped
> > general purpose registers.
> > ---
> >  arm64.c | 42 ++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 34 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arm64.c b/arm64.c
> > index 183e768498fe8..3a82d20cdd465 100644
> > --- a/arm64.c
> > +++ b/arm64.c
> > @@ -993,6 +993,25 @@ arm64_stackframe_init(void)
> >  #define PSR_MODE_EL3h   0x0000000d
> >  #define PSR_MODE_MASK   0x0000000f
> >  
> > +/* Architecturally defined mapping between AArch32 and AArch64 registers
> > */
> > +#define compat_usr(x)   regs[(x)]
> > +#define compat_fp       regs[11]
> > +#define compat_sp       regs[13]
> > +#define compat_lr       regs[14]
> > +
> > +#define user_mode(ptregs) \
> > +   (((ptregs)->pstate & PSR_MODE_MASK) == PSR_MODE_EL0t)
> > +
> > +#define compat_user_mode(ptregs)  \
> > +   (((ptregs)->pstate & (PSR_MODE32_BIT | PSR_MODE_MASK)) == \
> > +    (PSR_MODE32_BIT | PSR_MODE_EL0t))
> > +
> > +#define user_stack_pointer(ptregs) \
> > +   (!compat_user_mode(ptregs) ? (ptregs)->sp : (ptregs)->compat_sp)
> > +
> > +#define user_frame_pointer(ptregs) \
> > +   (!compat_user_mode(ptregs) ? (ptregs)->regs[29] : (ptregs)->compat_fp)
> > +
> >  static int
> >  arm64_is_kernel_exception_frame(struct bt_info *bt, ulong stkptr)
> >  {
> > @@ -1340,21 +1359,28 @@ arm64_get_dumpfile_stackframe(struct bt_info *bt,
> > struct arm64_stackframe *frame
> >     struct machine_specific *ms = machdep->machspec;
> >     struct arm64_pt_regs *ptregs;
> >  
> > -   if (!ms->panic_task_regs ||
> > -       (!ms->panic_task_regs[bt->tc->processor].sp &&
> > -        !ms->panic_task_regs[bt->tc->processor].pc)) {
> > +   if (!ms->panic_task_regs || !ms->panic_task_regs[bt->tc->processor].pc) 
> > {
> 
> Hi Dave,
> 
> I'm having second thoughts about keeping the
> !ms->panic_task_regs[bt->tc->processor].pc test. pc being zero sounds
> like a good reason to crash, or a potential status of a pc after a crash.
> I don't think we should be too hasty to reject the rest of the registers.
> Is there anyway we can relax the sanity checking, but still keep the crash
> utility happy?
> 
> Thanks,
> drew

Hi Drew,

OK, refresh my mind -- I'm not sure what the reasoning behind the patch snippet 
above?

Originally the code checked whether *both* the sp and pc are NULL, and if so, 
returns
BT_REGS_NOT_FOUD (which seems fair to me).  Your patch stops checking for a 
NULL sp, but
still checks for a NULL pc.  In this QEMU dump, the patch doesn't really 
accomplish anything, 
since both incoming registers are non-NULL (albeit with a sp pointing to 
16-bytes below the
top of the stack).  It seems like we could leave it as-is.

Dave
 


> >             bt->flags |= BT_REGS_NOT_FOUND;
> >             return FALSE;
> >     }
> >  
> >     ptregs = &ms->panic_task_regs[bt->tc->processor];
> > -   frame->sp = ptregs->sp;
> >     frame->pc = ptregs->pc;
> > -   frame->fp = ptregs->regs[29];
> > -
> > -   if (!is_kernel_text(frame->pc) &&
> > -       in_user_stack(bt->tc->task, frame->sp))
> > +   if (user_mode(ptregs)) {
> > +           frame->sp = user_stack_pointer(ptregs);
> > +           frame->fp = user_frame_pointer(ptregs);
> > +           if (is_kernel_text(frame->pc) ||
> > +               !in_user_stack(bt->tc->task, frame->sp)) {
> > +                   error(WARNING, "Corrupt prstatus? pstate=0x%lx, but no 
> > user frame
> > found\n",
> > +                                                                           
> > ptregs->pstate);
> > +                   bt->flags |= BT_REGS_NOT_FOUND;
> > +                   return FALSE;
> > +           }
> >             bt->flags |= BT_USER_SPACE;
> > +   } else {
> > +           frame->sp = ptregs->sp;
> > +           frame->fp = ptregs->regs[29];
> > +   }
> >  
> >     if (arm64_in_kdump_text(bt, frame))
> >             bt->flags |= BT_KDUMP_ADJUST;
> > --
> > 1.8.3.1
> > 
> > --
> > Crash-utility mailing list
> > Crash-utility@redhat.com
> > https://www.redhat.com/mailman/listinfo/crash-utility
> 
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
> 

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Reply via email to