From: Chris Z. <ch...@za...> - 2013-03-23 02:07:40
Attachments:
strace.patch
|
Hi, It would be great if you could apply the attached patch to add support for the Xtensa architecture. Xtensa was added to the kernel a long time ago, and we have kept local patches for strace for a long time, but it would be easier to support it in the mainline repository. Thanks, -Chris |
From: Dmitry V. L. <ld...@al...> - 2013-03-23 01:12:54
|
Hi, On Fri, Mar 22, 2013 at 05:09:23PM -0700, Chris Zankel wrote: > > It would be great if you could apply the attached patch to add support > for the Xtensa architecture. Xtensa was added to the kernel a long time > ago, and we have kept local patches for strace for a long time, but it > would be easier to support it in the mainline repository. [...] > diff --git a/linux/xtensa/syscallent.h b/linux/xtensa/syscallent.h > new file mode 100644 > index 0000000..2ef4fd8 > --- /dev/null > +++ b/linux/xtensa/syscallent.h > @@ -0,0 +1,434 @@ > + { 0, 0, printargs, "spill" }, /* 0 */ > + { 0, 0, printargs, "xtensa" }, /* 1 */ > + { MA, 0, printargs, "SYS_2" }, /* 2 */ > + { MA, 0, printargs, "SYS_3" }, /* 3 */ > + { MA, 0, printargs, "SYS_4" }, /* 4 */ > + { MA, 0, printargs, "SYS_5" }, /* 5 */ > + { MA, 0, printargs, "SYS_6" }, /* 6 */ > + { MA, 0, printargs, "SYS_7" }, /* 7 */ In this and other similar places, please collapse blocks of undefined syscall entries using "[N1 ... N2] = {}" notation. [...] > --- a/syscall.c > +++ b/syscall.c > @@ -76,6 +76,10 @@ > # include <asm/ptrace.h> > #endif > > +#if defined(XTENSA) > +# include <asm/ptrace.h> > +#endif > + > #ifndef ERESTARTSYS > # define ERESTARTSYS 512 > #endif > @@ -799,6 +803,8 @@ static struct user_regs_struct or1k_regs; > #elif defined(METAG) > static struct user_gp_regs metag_regs; > # define ARCH_REGS_FOR_GETREGSET metag_regs > +#elif defined(XTENSA) > +static long xtensa_a2; > #endif > > void > @@ -936,6 +942,13 @@ printcall(struct tcb *tcp) > tprintf("[%08lx] ", or1k_regs.pc); > #elif defined(METAG) > tprintf("[%08lx] ", metag_regs.pc); > +#elif defined(XTENSA) > + long pc; > + if (upeek(tcp, REG_PC, &pc) < 0) { > + PRINTBADPC; > + return; > + } > + tprintf("[%08lx] ", pc); > #endif /* architecture */ > } > > @@ -1516,6 +1529,9 @@ get_scno(struct tcb *tcp) > scno = or1k_regs.gpr[11]; > #elif defined(METAG) > scno = metag_regs.dx[0][1]; /* syscall number in D1Re0 (D1.0) */ > +#elif defined(XTENSA) > + if (upeek(tcp, SYSCALL_NR, &scno) < 0) > + return -1; > #endif > > tcp->scno = scno; > @@ -1916,6 +1932,12 @@ get_syscall_args(struct tcb *tcp) > for (i = 0; i < nargs; i++) > /* arguments go backwards from D1Ar1 (D1.3) */ > tcp->u_arg[i] = ((unsigned long *)&metag_regs.dx[3][1])[-i]; > +#elif defined(XTENSA) > + /* arg0: a6, arg1: a3, arg2: a4, arg3: a5, arg4: a8, arg5: a9 */ > + static const int xtensaregs[MAX_ARGS] = { 6, 3, 4, 5, 8, 9 }; > + for (i = 0; i < nargs; ++i) > + if (upeek(tcp, REG_A_BASE + xtensaregs[i], &tcp->u_arg[i]) < 0) > + return -1; > #else /* Other architecture (32bits specific) */ > for (i = 0; i < nargs; ++i) > if (upeek(tcp, i*4, &tcp->u_arg[i]) < 0) > @@ -2112,6 +2134,9 @@ get_syscall_result(struct tcb *tcp) > /* already done by get_regs */ > #elif defined(METAG) > /* already done by get_regs */ > +#elif defined(XTENSA) > + if (upeek(tcp, REG_A_BASE + 2, &xtensa_a2) < 0) > + return -1; > #endif > return 1; > } > @@ -2411,6 +2436,14 @@ get_error(struct tcb *tcp) > else { > tcp->u_rval = or1k_regs.gpr[11]; > } > +#elif defined(XTENSA) > + if (check_errno && is_negated_errno(xtensa_a2)) { > + tcp->u_rval = -1; > + u_error = -xtensa_a2; > + } > + else { > + tcp->u_rval = xtensa_a2; > + } > #endif > tcp->u_error = u_error; > } Is there any reason to upeek one register at a time instead of fetching the whole set using PTRACE_GETREGS or PTRACE_GETREGSET? -- ldv |
From: Chris Z. <ch...@za...> - 2013-03-23 18:08:47
|
Hi Dmitry, Thanks for your quick response. On 03/22/2013 06:12 PM, Dmitry V. Levin wrote: > > + { MA, 0, printargs, "SYS_2" }, /* 2 */ > + { MA, 0, printargs, "SYS_3" }, /* 3 */ > + { MA, 0, printargs, "SYS_4" }, /* 4 */ > + { MA, 0, printargs, "SYS_5" }, /* 5 */ > + { MA, 0, printargs, "SYS_6" }, /* 6 */ > + { MA, 0, printargs, "SYS_7" }, /* 7 */ > In this and other similar places, please collapse blocks of undefined > syscall entries using "[N1 ... N2] = {}" notation. Will do. > +#elif defined(XTENSA) > + if (check_errno && is_negated_errno(xtensa_a2)) { > + tcp->u_rval = -1; > + u_error = -xtensa_a2; > + } > + else { > + tcp->u_rval = xtensa_a2; > + } > #endif > tcp->u_error = u_error; > } > Is there any reason to upeek one register at a time instead of fetching > the whole set using PTRACE_GETREGS or PTRACE_GETREGSET? We don't yet support GETREGSET and I noticed a flaw with GETREGS. Xtensa is a configurable processor architecture with 'windowed' registers, where the output registers of the caller become the input registers of the callee. At any particular time, you can access 16 out of N registers, where N is configurable to be 32 or 64 registers. GETREGS returns a structure that includes all 32 or 64 registers, but no information of how many there are. So, the syscall arguments could either be in registers 30,31,32,33,34, etc. (for 64 registers), or they wrap around at 32 and be in 30, 31, 0, 1, 2, etc. Hopefully, we can fix that when supporting GETREGSET, but until then, we probably have to use the slower upeek method. I'll send an updated patch with the unused syscall entries removed and others collapsed. Thanks, -Chris |
From: Chris Z. <ch...@za...> - 2013-03-25 17:29:19
Attachments:
xtensa-strace.patch
|
Hi Dimitry, On 03/22/2013 06:12 PM, Dmitry V. Levin wrote: > On Fri, Mar 22, 2013 at 05:09:23PM -0700, Chris Zankel wrote: >> It would be great if you could apply the attached patch to add support >> for the Xtensa architecture. Xtensa was added to the kernel a long time >> ago, and we have kept local patches for strace for a long time, but it >> would be easier to support it in the mainline repository. > [...] >> diff --git a/linux/xtensa/syscallent.h b/linux/xtensa/syscallent.h >> new file mode 100644 >> index 0000000..2ef4fd8 >> --- /dev/null >> +++ b/linux/xtensa/syscallent.h >> @@ -0,0 +1,434 @@ >> + { 0, 0, printargs, "spill" }, /* 0 */ >> + { 0, 0, printargs, "xtensa" }, /* 1 */ >> + { MA, 0, printargs, "SYS_2" }, /* 2 */ >> + { MA, 0, printargs, "SYS_3" }, /* 3 */ >> + { MA, 0, printargs, "SYS_4" }, /* 4 */ >> + { MA, 0, printargs, "SYS_5" }, /* 5 */ >> + { MA, 0, printargs, "SYS_6" }, /* 6 */ >> + { MA, 0, printargs, "SYS_7" }, /* 7 */ > In this and other similar places, please collapse blocks of undefined > syscall entries using "[N1 ... N2] = {}" notation. In this patch, I removed the trailing unused entries and change the above and one other location to the [N1...N2] = {} notation. > > [...] >> tcp->u_arg[i] = ((unsigned long *)&metag_regs.dx[3][1])[-i]; >> +#elif defined(XTENSA) >> + /* arg0: a6, arg1: a3, arg2: a4, arg3: a5, arg4: a8, arg5: a9 */ >> + static const int xtensaregs[MAX_ARGS] = { 6, 3, 4, 5, 8, 9 }; >> + for (i = 0; i < nargs; ++i) >> + if (upeek(tcp, REG_A_BASE + xtensaregs[i], &tcp->u_arg[i]) < 0) >> + return -1; >> >> Is there any reason to upeek one register at a time instead of fetching >> the whole set using PTRACE_GETREGS or PTRACE_GETREGSET? >> >> I checked again, but couldn't find a good way to use the regset directly. As a recap, Xtensa is a configurable processor with a variable number of internal registers, and from the register set, it is not always clear where the syscall arguments are without knowing the configuration dependent register count. Goal is to fix it with getregset, which is currently not supported, so we can eventually deprecate the upeek method. Thanks, -Chris |
From: Denys V. <dvl...@re...> - 2013-03-26 11:25:12
|
On 03/25/2013 06:29 PM, Chris Zankel wrote: > Hi Dimitry, > > On 03/22/2013 06:12 PM, Dmitry V. Levin wrote: >> On Fri, Mar 22, 2013 at 05:09:23PM -0700, Chris Zankel wrote: >>> It would be great if you could apply the attached patch to add support >>> for the Xtensa architecture. Xtensa was added to the kernel a long time >>> ago, and we have kept local patches for strace for a long time, but it >>> would be easier to support it in the mainline repository. >> [...] >>> diff --git a/linux/xtensa/syscallent.h b/linux/xtensa/syscallent.h >>> new file mode 100644 >>> index 0000000..2ef4fd8 >>> --- /dev/null >>> +++ b/linux/xtensa/syscallent.h >>> @@ -0,0 +1,434 @@ >>> + { 0, 0, printargs, "spill" }, /* 0 */ >>> + { 0, 0, printargs, "xtensa" }, /* 1 */ >>> + { MA, 0, printargs, "SYS_2" }, /* 2 */ >>> + { MA, 0, printargs, "SYS_3" }, /* 3 */ >>> + { MA, 0, printargs, "SYS_4" }, /* 4 */ >>> + { MA, 0, printargs, "SYS_5" }, /* 5 */ >>> + { MA, 0, printargs, "SYS_6" }, /* 6 */ >>> + { MA, 0, printargs, "SYS_7" }, /* 7 */ >> In this and other similar places, please collapse blocks of undefined >> syscall entries using "[N1 ... N2] = {}" notation. > In this patch, I removed the trailing unused entries and change the > above and one other location to the [N1...N2] = {} notation. > >> >> [...] >>> tcp->u_arg[i] = ((unsigned long *)&metag_regs.dx[3][1])[-i]; >>> +#elif defined(XTENSA) >>> + /* arg0: a6, arg1: a3, arg2: a4, arg3: a5, arg4: a8, arg5: a9 */ >>> + static const int xtensaregs[MAX_ARGS] = { 6, 3, 4, 5, 8, 9 }; >>> + for (i = 0; i < nargs; ++i) >>> + if (upeek(tcp, REG_A_BASE + xtensaregs[i], &tcp->u_arg[i]) < 0) >>> + return -1; >>> >>> Is there any reason to upeek one register at a time instead of fetching >>> the whole set using PTRACE_GETREGS or PTRACE_GETREGSET? >>> >>> > I checked again, but couldn't find a good way to use the regset > directly. As a recap, Xtensa is a configurable processor with a > variable number of internal registers, and from the register set, it is > not always clear where the syscall arguments are without knowing the > configuration dependent register count. Goal is to fix it with > getregset, which is currently not supported, so we can eventually > deprecate the upeek method. Is there a plan to implement PTRACE_GETREGSET (in kernel) for this arch? -- vda |
From: Chris Z. <ch...@za...> - 2013-03-26 17:36:31
|
Hi Denys, On 03/26/2013 04:25 AM, Denys Vlasenko wrote: > On 03/25/2013 06:29 PM, Chris Zankel wrote: >> I checked again, but couldn't find a good way to use the regset >> directly. As a recap, Xtensa is a configurable processor with a >> variable number of internal registers, and from the register set, it >> is not always clear where the syscall arguments are without knowing >> the configuration dependent register count. Goal is to fix it with >> getregset, which is currently not supported, so we can eventually >> deprecate the upeek method. > Is there a plan to implement PTRACE_GETREGSET (in kernel) > for this arch? It's definitely on the plan, but not necessarily highest priority right now. It's not so much a problem for the base registers (which strace needs), but the complication for Xtensa is its configurable architecture, which allows to define any number of custom registers. The current implementation to get those registers (getxregs, setxregs) requires the client (gdb, etc.) to know the exact processor configuration and register layout. Ideally, that tool would adapt to the configuration at run-time instead of compile time, and regsets might be just the right vehicle for such implementation. Thanks, -Chris |
From: Dmitry V. L. <ld...@al...> - 2013-03-31 01:44:20
|
On Mon, Mar 25, 2013 at 10:29:05AM -0700, Chris Zankel wrote: > On 03/22/2013 06:12 PM, Dmitry V. Levin wrote: > > On Fri, Mar 22, 2013 at 05:09:23PM -0700, Chris Zankel wrote: > >> It would be great if you could apply the attached patch to add support > >> for the Xtensa architecture. Xtensa was added to the kernel a long time > >> ago, and we have kept local patches for strace for a long time, but it > >> would be easier to support it in the mainline repository. > > [...] > >> diff --git a/linux/xtensa/syscallent.h b/linux/xtensa/syscallent.h > >> new file mode 100644 > >> index 0000000..2ef4fd8 > >> --- /dev/null > >> +++ b/linux/xtensa/syscallent.h > >> @@ -0,0 +1,434 @@ > >> + { 0, 0, printargs, "spill" }, /* 0 */ > >> + { 0, 0, printargs, "xtensa" }, /* 1 */ > >> + { MA, 0, printargs, "SYS_2" }, /* 2 */ > >> + { MA, 0, printargs, "SYS_3" }, /* 3 */ > >> + { MA, 0, printargs, "SYS_4" }, /* 4 */ > >> + { MA, 0, printargs, "SYS_5" }, /* 5 */ > >> + { MA, 0, printargs, "SYS_6" }, /* 6 */ > >> + { MA, 0, printargs, "SYS_7" }, /* 7 */ > > In this and other similar places, please collapse blocks of undefined > > syscall entries using "[N1 ... N2] = {}" notation. > In this patch, I removed the trailing unused entries and change the > above and one other location to the [N1...N2] = {} notation. I took the liberty of converting remaining single undefined syscall entries to [N] = {} notation as well. Thanks, -- ldv |
From: Chris Z. <ch...@za...> - 2013-03-31 02:22:45
|
Hi Dmitry, On 3/30/13 6:44 PM, Dmitry V. Levin wrote: > On Mon, Mar 25, 2013 at 10:29:05AM -0700, Chris Zankel wrote: >> On 03/22/2013 06:12 PM, Dmitry V. Levin wrote: >> >>> In this and other similar places, please collapse blocks of undefined >>> syscall entries using "[N1 ... N2] = {}" notation. >> In this patch, I removed the trailing unused entries and change the >> above and one other location to the [N1...N2] = {} notation. > I took the liberty of converting remaining single undefined syscall > entries to [N] = {} notation as well. Thanks for adding the patch (sorry, misunderstood that you wanted me to change also single-line undefined syscalls, so thanks for making the modifications) -Chris |