Re: [patch v3 19/36] Hexagon: Add ptrace support

Jonas Bonn
2011-09-09T20:15:22Z

On 9 September 2011 03:09, Richard Kuo <rkuo@codeaurora.org> wrote:
> +long arch_ptrace(struct task_struct *child, long request,
> +                unsigned long addr, unsigned long data)
> +{
> +       void __user *udata = (void __user *) data;
> +
> +       switch (request) {
> +       case PTRACE_POKETEXT:   /* write the word at location addr. */
> +       case PTRACE_POKEDATA:
> +               return generic_ptrace_pokedata(child, addr, data);
> +       case PTRACE_POKEUSR:    /* write register specified by addr. */
> +               return ptrace_pokeusr(child, addr, data);
> +       case PTRACE_GETREGS:
> +               return copy_regset_to_user(child, &hexagon_user_view,
> +                                          REGSET_GPR, 0,
> +                                          sizeof(struct user_regs_struct),
> +                                          udata);
> +       case PTRACE_SETREGS:
> +               return copy_regset_from_user(child, &hexagon_user_view,
> +                                            REGSET_GPR, 0,
> +                                            sizeof(struct user_regs_struct),
> +                                            udata);
> +       default:
> +               return ptrace_request(child, request, addr, data);
> +       }
> +
> +       return 0;
> +}
>

I think that... other than for compatibility with legacy userspace
code, you shouldn't need any of these cases other than the 'default'.

PTRACE_GETREGS is replaced by PTRACE_GETREGSET and this is handled in
generic code. POKETEXT, POKEDATA are handled in the generic code.
POKEUSR can also be done via the SETREGSET mechanism.

That said, I don't think gdbserver has been updated to use
GETREGSET/SETREGSET. This is a bit like the uClibc/glibc issue,
though... somebody just needs to do the work to make userspace work
with the new, "clean" kernel interface.

Given all that, you shouldn't need to export pt_regs to userspace
either. You already have a user_regs_struct which is userspace's view
of the process registers. For OpenRISC we made pt_regs a kernel
private structure, giving us the freedom to change the stack layout of
the registers in the future while providing a stable view for
userspace. I would suggest doing the same here.

If you do decide to make pt_regs private, you'll need to change your
struct sigcontext to use user_regs_struct, as well.

/Jonas