ptrace.c add register sets support for score architecture.
- genregs_get(struct task_struct *target,
const struct user_regset *regset,
unsigned int pos, unsigned int count,
void *kbuf, void __user *ubuf)
Retrieve the contents of score userspace general registers.
- genregs_set(struct task_struct *target,
const struct user_regset *regset,
unsigned int pos, unsigned int count,
const void *kbuf, const void __user *ubuf)
Update the contents of the score userspace general registers.
Signed-off-by: Chen Liqin <[email protected]>
---
arch/score/include/asm/elf.h | 18 +++--
arch/score/include/asm/ptrace.h | 2 +
arch/score/kernel/ptrace.c | 140
++++++++++++++++++++++++++++++--------
3 files changed, 123 insertions(+), 37 deletions(-)
diff --git a/arch/score/include/asm/elf.h b/arch/score/include/asm/elf.h
index 8324363..43526d9 100644
--- a/arch/score/include/asm/elf.h
+++ b/arch/score/include/asm/elf.h
@@ -1,9 +1,8 @@
#ifndef _ASM_SCORE_ELF_H
#define _ASM_SCORE_ELF_H
-/* ELF register definitions */
-#define ELF_NGREG 45
-#define ELF_NFPREG 33
+#include <linux/ptrace.h>
+
#define EM_SCORE7 135
/* Relocation types. */
@@ -30,11 +29,15 @@
#define R_SCORE_IMM30 20
#define R_SCORE_IMM32 21
-typedef unsigned long elf_greg_t;
-typedef elf_greg_t elf_gregset_t[ELF_NGREG];
+/* ELF register definitions */
+typedef unsigned long elf_greg_t;
+
+#define ELF_NGREG (sizeof(struct pt_regs) / sizeof(elf_greg_t))
+typedef elf_greg_t elf_gregset_t[ELF_NGREG];
-typedef double elf_fpreg_t;
-typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
+/* Score does not have fp regs. */
+typedef double elf_fpreg_t;
+typedef elf_fpreg_t elf_fpregset_t;
#define elf_check_arch(x) ((x)->e_machine == EM_SCORE7)
@@ -57,6 +60,7 @@ do { \
struct task_struct;
struct pt_regs;
+#define CORE_DUMP_USE_REGSET
#define USE_ELF_CORE_DUMP
#define ELF_EXEC_PAGESIZE PAGE_SIZE
diff --git a/arch/score/include/asm/ptrace.h
b/arch/score/include/asm/ptrace.h
index 66b14c8..19ce850 100644
--- a/arch/score/include/asm/ptrace.h
+++ b/arch/score/include/asm/ptrace.h
@@ -74,6 +74,8 @@ struct pt_regs {
#ifdef __KERNEL__
+struct task_struct;
+
/*
* Does the process account for user or for system time?
*/
diff --git a/arch/score/kernel/ptrace.c b/arch/score/kernel/ptrace.c
index 1db876b..43784d8 100644
--- a/arch/score/kernel/ptrace.c
+++ b/arch/score/kernel/ptrace.c
@@ -23,11 +23,113 @@
* 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include <linux/elf.h>
#include <linux/kernel.h>
#include <linux/ptrace.h>
+#include <linux/regset.h>
#include <asm/uaccess.h>
+/*
+ * retrieve the contents of SCORE userspace general registers
+ */
+static int genregs_get(struct task_struct *target,
+ const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ void *kbuf, void __user *ubuf)
+{
+ const struct pt_regs *regs = task_pt_regs(target);
+ int ret;
+
+ /* skip 8 * sizeof(unsigned long) not use for pt_regs */
+ ret = user_regset_copyout_zero(&pos, &count, &kbuf, &ubuf,
+ 0, offsetof(struct pt_regs, regs));
+
+ /* r0 - r31 */
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ regs->regs,
+ offsetof(struct pt_regs, regs),
+ offsetof(struct pt_regs, cel));
+
+ if (!ret)
+ /* cel, ceh, sr0, sr1, sr2, epc, ema, psr, ecr, condition */
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ ®s->cel,
+ offsetof(struct pt_regs, cel),
+ offsetof(struct pt_regs, is_syscall));
+
+ if (!ret)
+ ret = user_regset_copyout_zero(&pos, &count, &kbuf, &ubuf,
+ sizeof(struct pt_regs), -1);
+
+ return ret;
+}
+
+/*
+ * update the contents of the SCORE userspace general registers
+ */
+static int genregs_set(struct task_struct *target,
+ const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf)
+{
+ struct pt_regs *regs = task_pt_regs(target);
+ int ret;
+
+ /* skip 8 * sizeof(unsigned long) */
+ ret = user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
+ 0, offsetof(struct pt_regs, regs));
+
+ /* r0 - r31 */
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ regs->regs,
+ offsetof(struct pt_regs, regs),
+ offsetof(struct pt_regs, cel));
+
+ if (!ret)
+ /* cel, ceh, sr0, sr1, sr2, epc, ema, psr, ecr, condition */
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ ®s->cel,
+ offsetof(struct pt_regs, cel),
+ offsetof(struct pt_regs, is_syscall));
+
+ if (!ret)
+ ret = user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
+ sizeof(struct pt_regs), -1);
+
+ return ret;
+}
+
+/*
+ * Define the register sets available on the score7 under Linux
+ */
+enum score7_regset {
+ REGSET_GENERAL,
+};
+
+static const struct user_regset score7_regsets[] = {
+ [REGSET_GENERAL] = {
+ .core_note_type = NT_PRSTATUS,
+ .n = ELF_NGREG,
+ .size = sizeof(long),
+ .align = sizeof(long),
+ .get = genregs_get,
+ .set = genregs_set,
+ },
+};
+
+static const struct user_regset_view user_score_native_view = {
+ .name = "score7",
+ .e_machine = EM_SCORE7,
+ .regsets = score7_regsets,
+ .n = ARRAY_SIZE(score7_regsets),
+};
+
+const struct user_regset_view *task_user_regset_view(struct task_struct
*task)
+{
+ return &user_score_native_view;
+}
+
static int is_16bitinsn(unsigned long insn)
{
if ((insn & INSN32_MASK) == INSN32_MASK)
@@ -80,34 +182,6 @@ write_tsk_long(struct task_struct *child,
return copied != sizeof(val) ? -EIO : 0;
}
-/*
- * Get all user integer registers.
- */
-static int ptrace_getregs(struct task_struct *tsk, void __user *uregs)
-{
- struct pt_regs *regs = task_pt_regs(tsk);
-
- return copy_to_user(uregs, regs, sizeof(struct pt_regs)) ? -EFAULT : 0;
-}
-
-/*
- * Set all user integer registers.
- */
-static int ptrace_setregs(struct task_struct *tsk, void __user *uregs)
-{
- struct pt_regs newregs;
- int ret;
-
- ret = -EFAULT;
- if (copy_from_user(&newregs, uregs, sizeof(struct pt_regs)) == 0) {
- struct pt_regs *regs = task_pt_regs(tsk);
- *regs = newregs;
- ret = 0;
- }
-
- return ret;
-}
-
void user_enable_single_step(struct task_struct *child)
{
/* far_epc is the target of branch */
@@ -356,11 +430,17 @@ arch_ptrace(struct task_struct *child, long
request, long addr, long data)
}
case PTRACE_GETREGS:
- ret = ptrace_getregs(child, (void __user *)datap);
+ return copy_regset_to_user(child, &user_score_native_view,
+ REGSET_GENERAL,
+ 0, sizeof(struct pt_regs),
+ (void __user *)datap);
break;
case PTRACE_SETREGS:
- ret = ptrace_setregs(child, (void __user *)datap);
+ return copy_regset_from_user(child, &user_score_native_view,
+ REGSET_GENERAL,
+ 0, sizeof(struct pt_regs),
+ (const void __user *)datap);
break;
default:
--
1.6.2
On Friday 10 July 2009, liqin.chen wrote:
> +static int genregs_get(struct task_struct *target,
> + const struct user_regset *regset,
> + unsigned int pos, unsigned int count,
> + void *kbuf, void __user *ubuf)
> +{
> + const struct pt_regs *regs = task_pt_regs(target);
> + int ret;
> +
> + /* skip 8 * sizeof(unsigned long) not use for pt_regs */
> + ret = user_regset_copyout_zero(&pos, &count, &kbuf, &ubuf,
> + 0, offsetof(struct pt_regs, regs));
> +
> + /* r0 - r31 */
> + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> + regs->regs,
> + offsetof(struct pt_regs, regs),
> + offsetof(struct pt_regs, cel));
> +
> + if (!ret)
> + /* cel, ceh, sr0, sr1, sr2, epc, ema, psr, ecr, condition */
> + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> + ®s->cel,
> + offsetof(struct pt_regs, cel),
> + offsetof(struct pt_regs, is_syscall));
The two user_regset_copyout are consecutive, so AFAICT they can
be combined into a single function call.
> +
> + if (!ret)
> + ret = user_regset_copyout_zero(&pos, &count, &kbuf, &ubuf,
> + sizeof(struct pt_regs), -1);
> +
> + return ret;
> +}
While the code looks correct to me now based on Rolands comments,
I think it would be nicer to define separate pt_regs and user_regset
data structures to make the two independent and give you more
flexibility with the kernel stack layout in the future.
Maybe you could change
struct pt_regs {
unsigned long pad0[6];
unsigned long orig_r4;
unsigned long orig_r7;
unsigned long regs[32];
unsigned long cel;
unsigned long ceh;
unsigned long sr0; /* cnt */
unsigned long sr1; /* lcr */
unsigned long sr2; /* scr */
unsigned long cp0_epc;
unsigned long cp0_ema;
unsigned long cp0_psr;
unsigned long cp0_ecr;
unsigned long cp0_condition;
long is_syscall;
};
to
struct pt_regs {
unsigned long regs[32];
unsigned long cel;
unsigned long ceh;
unsigned long sr0; /* cnt */
unsigned long sr1; /* lcr */
unsigned long sr2; /* scr */
unsigned long cp0_epc;
unsigned long cp0_ema;
unsigned long cp0_psr;
unsigned long cp0_ecr;
unsigned long cp0_condition;
#ifdef __KERNEL__
unsigned long orig_r4;
unsigned long orig_r7;
long is_syscall;
unsigned long pad0[3];
#else
unsigned long pad0[6];
#endif
};
> @@ -356,11 +430,17 @@ arch_ptrace(struct task_struct *child, long
> request, long addr, long data)
> }
>
> case PTRACE_GETREGS:
> - ret = ptrace_getregs(child, (void __user *)datap);
> + return copy_regset_to_user(child, &user_score_native_view,
> + REGSET_GENERAL,
> + 0, sizeof(struct pt_regs),
> + (void __user *)datap);
> break;
>
> case PTRACE_SETREGS:
> - ret = ptrace_setregs(child, (void __user *)datap);
> + return copy_regset_from_user(child, &user_score_native_view,
> + REGSET_GENERAL,
> + 0, sizeof(struct pt_regs),
> + (const void __user *)datap);
> break;
>
> default:
I guess you still need to remove the PTRACE_PEEKUSR and PTRACE_POKEUSR
code, as mentioned by Roland.
Roland, Christoph: Do you think it would be reasonable to implement
this in common code? That would make it possible to have an empty
arch_ptrace() function.
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 61c78b2..a6b7862 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -570,6 +570,21 @@ int ptrace_request(struct task_struct *child, long request,
return 0;
return ptrace_resume(child, request, SIGKILL);
+#if defined(PTRACE_GENERIC_GETREGS) && defined(REGSET_GENERAL)
+ case PTRACE_GETREGS:
+ return copy_regset_to_user(child,
+ task_user_regset_view(child),
+ REGSET_GENERAL, 0,
+ sizeof(struct pt_regs),
+ (void __user *)datap);
+ case PTRACE_GETREGS:
+ return copy_regset_from_user(child,
+ task_user_regset_view(child),
+ REGSET_GENERAL, 0,
+ sizeof(struct pt_regs),
+ (void __user *)datap);
+#endif
+
default:
break;
}
[email protected] д?? 2009-07-10 20:49:44:
> struct pt_regs {
> unsigned long pad0[6];
> unsigned long orig_r4;
> unsigned long orig_r7;
> unsigned long regs[32];
>
> unsigned long cel;
> unsigned long ceh;
>
> unsigned long sr0; /* cnt */
> unsigned long sr1; /* lcr */
> unsigned long sr2; /* scr */
>
> unsigned long cp0_epc;
> unsigned long cp0_ema;
> unsigned long cp0_psr;
> unsigned long cp0_ecr;
> unsigned long cp0_condition;
>
> long is_syscall;
> };
>
> to
>
> struct pt_regs {
> unsigned long regs[32];
>
> unsigned long cel;
> unsigned long ceh;
>
> unsigned long sr0; /* cnt */
> unsigned long sr1; /* lcr */
> unsigned long sr2; /* scr */
>
> unsigned long cp0_epc;
> unsigned long cp0_ema;
> unsigned long cp0_psr;
> unsigned long cp0_ecr;
> unsigned long cp0_condition;
> #ifdef __KERNEL__
> unsigned long orig_r4;
> unsigned long orig_r7;
> long is_syscall;
> unsigned long pad0[3];
> #else
> unsigned long pad0[6];
> #endif
> };
>
unsigned long pad0[6];
was place at the start of pt_regs use
to hold the arguments from userland.
caller(exception handler) not need to
change stack pointer.
unsigned long orig_r4;
unsigned long orig_r7;
move to end of the pt_regs haven't
problem.
Thanks,
Liqin
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Monday 13 July 2009, [email protected] wrote:
>
> unsigned long pad0[6];
>
> was place at the start of pt_regs use
> to hold the arguments from userland.
> caller(exception handler) not need to
> change stack pointer.
>
> unsigned long orig_r4;
> unsigned long orig_r7;
> move to end of the pt_regs haven't
> problem.
Ok, I see.
Maybe to get a nicer layout, you can define separate
structures then:
struct user_regs_struct {
unsigned long regs[32];
unsigned long cel;
unsigned long ceh;
unsigned long sr0; /* cnt */
unsigned long sr1; /* lcr */
unsigned long sr2; /* scr */
unsigned long cp0_epc;
unsigned long cp0_ema;
unsigned long cp0_psr;
unsigned long cp0_ecr;
unsigned long cp0_condition;
unsigned long reserved[6]; /* future extensions */
};
#ifdef __KERNEL__
struct pt_regs {
/* stack arguments */
unsigned long pad0[6];
/* internal use */
unsigned long orig_r4;
unsigned long orig_r7;
long is_syscall;
/* saved registers */
struct user_regs_struct uregs;
};
#endif
That would keep the user struct relatively clean.
Arnd <><
Arnd Bergmann <[email protected]> д?? 2009-07-13 22:24:28:
> Maybe to get a nicer layout, you can define separate
> structures then:
>
> struct user_regs_struct {
> unsigned long regs[32];
>
> unsigned long cel;
> unsigned long ceh;
>
> unsigned long sr0; /* cnt */
> unsigned long sr1; /* lcr */
> unsigned long sr2; /* scr */
>
> unsigned long cp0_epc;
> unsigned long cp0_ema;
> unsigned long cp0_psr;
> unsigned long cp0_ecr;
> unsigned long cp0_condition;
>
> unsigned long reserved[6]; /* future extensions */
> };
>
> #ifdef __KERNEL__
> struct pt_regs {
> /* stack arguments */
> unsigned long pad0[6];
>
> /* internal use */
> unsigned long orig_r4;
> unsigned long orig_r7;
> long is_syscall;
>
> /* saved registers */
> struct user_regs_struct uregs;
> };
> #endif
>
> That would keep the user struct relatively clean.
OK, I will send this patch soon.
Thanks,
liqin
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
IMHO it is certainly a wise choice to have some struct (be it called
user_regs_struct or whatever) that describes the exact userland layout.
(I think it's also fine to just call that elf_gregset_t and have it be an
array, with REG_* defines/enum somewhere to document its indices. It's
just a matter of taste for the arch folks.)
Given such a struct, the regset code can refer to it (with offsetof, e.g.)
and that will make it far easier to read, understand, and review.
Thanks,
Roland