score: add regset support
Subsystems (3)
PATCH v2 1/1 score: add regset support
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.
---
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(-)
arch/score/include/asm/elf.h +0 -0
@@ -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
arch/score/include/asm/ptrace.h +0 -0
@@ -74,6 +74,8 @@ struct pt_regs {
#ifdef __KERNEL__
+struct task_struct;
+
/*
* Does the process account for user or for system time? */
arch/score/kernel/ptrace.c +1 -0
@@ -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:Signature · 1 line
1.6.2
Arnd Bergmann
maintainerQuoted 14 lines · liqin.chen
(quoted diff) +14
-0+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,
Review
Quoted 10 lines
(quoted diff) +10
-0+ 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.
Review
Quoted 7 lines
(quoted diff) +7
-0+ + 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
};
Quoted 20 lines
(quoted diff) +8
-2@@ -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;
Quoted 1 line
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.
kernel/ptrace.c +15 -0
@@ -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:
Quoted 46 lines
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];
Quoted 3 lines
#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
Arnd Bergmann
maintainerQuoted 8 lines · On Monday 13 July 2009, [email protected] wrote:
unsigned long pad0[6];
was place at the start of
pt_regsuse
to hold the arguments from userland.
caller(exception handler) not need to
change stack pointer.
Quoted 4 lines
unsigned long
orig_r4;
unsigned longorig_r7;
move to end of thept_regshaven'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:
Quoted 35 lines
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; };
Quoted 3 lines
#endif
That would keep the user struct relatively clean.
OK, I will send this patch soon.
Thanks,
liqin
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