Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On 07/22/15 01:11, Michael Ellerman wrote:
On Wed, 2015-07-22 at 00:46 -0400, David Long wrote:
On 06/29/15 23:29, Michael Ellerman wrote:
On Wed, 2015-06-17 at 14:30 -0400, David Long wrote:
On 06/16/15 09:17, Rob Herring wrote:
On Mon, Jun 15, 2015 at 11:42 AM, David Long dave.l...@linaro.org wrote:
#define REG_OFFSET_NAME(r) \
{.name = #r, .offset = offsetof(struct pt_regs, ARM_##r)}
#define REG_OFFSET_END {.name = NULL, .offset = 0}
Can't you also move these? ARM is complicated with the ARM_
prefixing, but the others appear to be the same. Maybe you can remove
the prefix or redefine the macro for ARM.
That would mandate that all the architecture-specific pt_regs structures
would have to use a top-level named field for each named register.
Why does it mandate that?
See eg. powerpc where we use REG_OFFSET_NAME for the top-level named fields and
then a different macro for the array elements:
#define REG_OFFSET_NAME(r) {.name = #r, .offset = offsetof(struct pt_regs,
r)}
#define GPR_OFFSET_NAME(num)\
{.name = STR(gpr##num), .offset = offsetof(struct pt_regs, gpr[num])}
static const struct pt_regs_offset regoffset_table[] = {
GPR_OFFSET_NAME(0),
GPR_OFFSET_NAME(1),
GPR_OFFSET_NAME(2),
GPR_OFFSET_NAME(3),
...
REG_OFFSET_NAME(nip),
REG_OFFSET_NAME(msr),
So I don't see why REG_OFFSET_NAME couldn't be common.
Sorry for the delay in responding to this.
OK, so you're saying architectures that don't want this constraint can
make their own macro. Seems to make this whole exercise slightly less
useful, but whatever.
Well yeah.
In fact of the 4 arches that use REG_OFFSET_NAME, 2 already have another macro
for specially named registers (powerpc sh).
I see three ways to go here:
1) Leave it as is.
2) Force all architectures to use a common definition.
3) Provide a common definition that all architectures (except arm)
currently using this functionality will use.
I have a v2 patch to implement #3, ready to post. Do we think this is
the way to go?
Yeah I think it is. How are you making it conditional? Just #ifndef
REG_OFFSET_NAME?
I'm just defining a new macro for arm. The macro is only invoked in one
arm file. Then the REG_OFFSET_NAME macro goes unused for this architecture.
I don't like #2 because I really don't want to rename all
uses of the current register fields for arm since this is
architecture-specific code to begin with and since it affects code in 39
arm source files.
I guess you're talking about renaming all the ARM_x regs to x. That would
likely cause problems because they're implemented as #defines,
eg. #define r0 uregs[0] would probably confuse your assembler.
Yeah, and I had not looked further to the implications of doing that but
I see you've found where it is a genuine problem.
The clean thing to do would be to have the in-kernel struct pt_regs have actual
named members, but that would still be an intrusive change.
cheers
Thanks,
-dl
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On 06/29/15 23:29, Michael Ellerman wrote:
On Wed, 2015-06-17 at 14:30 -0400, David Long wrote:
On 06/16/15 09:17, Rob Herring wrote:
On Mon, Jun 15, 2015 at 11:42 AM, David Long dave.l...@linaro.org wrote:
#define REG_OFFSET_NAME(r) \
{.name = #r, .offset = offsetof(struct pt_regs, ARM_##r)}
#define REG_OFFSET_END {.name = NULL, .offset = 0}
Can't you also move these? ARM is complicated with the ARM_
prefixing, but the others appear to be the same. Maybe you can remove
the prefix or redefine the macro for ARM.
That would mandate that all the architecture-specific pt_regs structures
would have to use a top-level named field for each named register.
Why does it mandate that?
See eg. powerpc where we use REG_OFFSET_NAME for the top-level named fields and
then a different macro for the array elements:
#define REG_OFFSET_NAME(r) {.name = #r, .offset = offsetof(struct pt_regs,
r)}
#define GPR_OFFSET_NAME(num) \
{.name = STR(gpr##num), .offset = offsetof(struct pt_regs, gpr[num])}
static const struct pt_regs_offset regoffset_table[] = {
GPR_OFFSET_NAME(0),
GPR_OFFSET_NAME(1),
GPR_OFFSET_NAME(2),
GPR_OFFSET_NAME(3),
...
REG_OFFSET_NAME(nip),
REG_OFFSET_NAME(msr),
So I don't see why REG_OFFSET_NAME couldn't be common.
Sorry for the delay in responding to this.
OK, so you're saying architectures that don't want this constraint can
make their own macro. Seems to make this whole exercise slightly less
useful, but whatever.
I see three ways to go here:
1) Leave it as is.
2) Force all architectures to use a common definition.
3) Provide a common definition that all architectures (except arm)
currently using this functionality will use.
I have a v2 patch to implement #3, ready to post. Do we think this is
the way to go? I don't like #2 because I really don't want to rename all
uses of the current register fields for arm since this is
architecture-specific code to begin with and since it affects code in 39
arm source files.
cheers
Thanks,
-dl
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On Wed, 2015-07-22 at 00:46 -0400, David Long wrote:
On 06/29/15 23:29, Michael Ellerman wrote:
On Wed, 2015-06-17 at 14:30 -0400, David Long wrote:
On 06/16/15 09:17, Rob Herring wrote:
On Mon, Jun 15, 2015 at 11:42 AM, David Long dave.l...@linaro.org wrote:
#define REG_OFFSET_NAME(r) \
{.name = #r, .offset = offsetof(struct pt_regs, ARM_##r)}
#define REG_OFFSET_END {.name = NULL, .offset = 0}
Can't you also move these? ARM is complicated with the ARM_
prefixing, but the others appear to be the same. Maybe you can remove
the prefix or redefine the macro for ARM.
That would mandate that all the architecture-specific pt_regs structures
would have to use a top-level named field for each named register.
Why does it mandate that?
See eg. powerpc where we use REG_OFFSET_NAME for the top-level named fields
and
then a different macro for the array elements:
#define REG_OFFSET_NAME(r) {.name = #r, .offset = offsetof(struct
pt_regs, r)}
#define GPR_OFFSET_NAME(num) \
{.name = STR(gpr##num), .offset = offsetof(struct pt_regs, gpr[num])}
static const struct pt_regs_offset regoffset_table[] = {
GPR_OFFSET_NAME(0),
GPR_OFFSET_NAME(1),
GPR_OFFSET_NAME(2),
GPR_OFFSET_NAME(3),
...
REG_OFFSET_NAME(nip),
REG_OFFSET_NAME(msr),
So I don't see why REG_OFFSET_NAME couldn't be common.
Sorry for the delay in responding to this.
OK, so you're saying architectures that don't want this constraint can
make their own macro. Seems to make this whole exercise slightly less
useful, but whatever.
Well yeah.
In fact of the 4 arches that use REG_OFFSET_NAME, 2 already have another macro
for specially named registers (powerpc sh).
I see three ways to go here:
1) Leave it as is.
2) Force all architectures to use a common definition.
3) Provide a common definition that all architectures (except arm)
currently using this functionality will use.
I have a v2 patch to implement #3, ready to post. Do we think this is
the way to go?
Yeah I think it is. How are you making it conditional? Just #ifndef
REG_OFFSET_NAME?
I don't like #2 because I really don't want to rename all
uses of the current register fields for arm since this is
architecture-specific code to begin with and since it affects code in 39
arm source files.
I guess you're talking about renaming all the ARM_x regs to x. That would
likely cause problems because they're implemented as #defines,
eg. #define r0 uregs[0] would probably confuse your assembler.
The clean thing to do would be to have the in-kernel struct pt_regs have actual
named members, but that would still be an intrusive change.
cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On Wed, 2015-06-17 at 14:30 -0400, David Long wrote:
On 06/16/15 09:17, Rob Herring wrote:
On Mon, Jun 15, 2015 at 11:42 AM, David Long dave.l...@linaro.org wrote:
#define REG_OFFSET_NAME(r) \
{.name = #r, .offset = offsetof(struct pt_regs, ARM_##r)}
#define REG_OFFSET_END {.name = NULL, .offset = 0}
Can't you also move these? ARM is complicated with the ARM_
prefixing, but the others appear to be the same. Maybe you can remove
the prefix or redefine the macro for ARM.
That would mandate that all the architecture-specific pt_regs structures
would have to use a top-level named field for each named register.
Why does it mandate that?
See eg. powerpc where we use REG_OFFSET_NAME for the top-level named fields and
then a different macro for the array elements:
#define REG_OFFSET_NAME(r) {.name = #r, .offset = offsetof(struct pt_regs, r)}
#define GPR_OFFSET_NAME(num) \
{.name = STR(gpr##num), .offset = offsetof(struct pt_regs, gpr[num])}
static const struct pt_regs_offset regoffset_table[] = {
GPR_OFFSET_NAME(0),
GPR_OFFSET_NAME(1),
GPR_OFFSET_NAME(2),
GPR_OFFSET_NAME(3),
...
REG_OFFSET_NAME(nip),
REG_OFFSET_NAME(msr),
So I don't see why REG_OFFSET_NAME couldn't be common.
cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On 06/19/15 12:58, Kees Cook wrote: On Fri, Jun 19, 2015 at 7:12 AM, David Long dave.l...@linaro.org wrote: On 06/19/15 00:19, Michael Ellerman wrote: On Mon, 2015-06-15 at 12:42 -0400, David Long wrote: From: David A. Long dave.l...@linaro.org The pt_regs_offset structure is used for HAVE_REGS_AND_STACK_ACCESS_API feature and has identical definitions in four different arch ptrace.h include files. It seems unlikely that definition would ever need to be changed regardless of architecture so lets move it into include/linux/ptrace.h. Signed-off-by: David A. Long dave.l...@linaro.org --- arch/powerpc/kernel/ptrace.c | 5 - Built and booted on powerpc, but is there an easy way to actually test the code paths in question? There is an easy way to smoke test it on all archiectures that also implement kprobes (which powerpc does). If I'm understanding the powerpc code correctly (WRT register naming conventions) just do the following: cd /sys/kernel/debug/tracing echo 'p do_fork %gpr0' kprobe_events echo 1 events/kprobes/enable ls cat trace echo 0 events/kprobes/enable Every fork() call done on the system between those two echo commands (hence the ls) should append a line to the trace file. For a more exhaustive test one could repeat this sequence for every register in the architecture. This should work the same on all architectures supporting kprobes. You just have to use the appropriate register names for your architecture after the %. Is this something we could codify into the selftests directory? It seems like a great thing to capture in a single place somewhere (the register lists, that is). e -Kees Due to the architecture-specific naming of registers this would have to be added to the architecture subdirectories. I only see powerpc and x86 subdirs at this time so extending that infrastructure would have to be part of this. Verifying the register contents would also require some change to the kernel, possibly a simple test module, which would have to be unique to each architecture. Without that we could only check for recognition of the register name, although maybe that's good enough. Acked-by: Michael Ellerman m...@ellerman.id.au cheers Thanks, -dl -dl ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On 06/24/15 00:07, Michael Ellerman wrote: On Tue, 2015-06-23 at 09:48 -0400, David Long wrote: On 06/22/15 23:32, Michael Ellerman wrote: On Fri, 2015-06-19 at 10:12 -0400, David Long wrote: On 06/19/15 00:19, Michael Ellerman wrote: On Mon, 2015-06-15 at 12:42 -0400, David Long wrote: From: David A. Long dave.l...@linaro.org The pt_regs_offset structure is used for HAVE_REGS_AND_STACK_ACCESS_API feature and has identical definitions in four different arch ptrace.h include files. It seems unlikely that definition would ever need to be changed regardless of architecture so lets move it into include/linux/ptrace.h. Signed-off-by: David A. Long dave.l...@linaro.org --- arch/powerpc/kernel/ptrace.c | 5 - Built and booted on powerpc, but is there an easy way to actually test the code paths in question? There is an easy way to smoke test it on all archiectures that also implement kprobes (which powerpc does). If I'm understanding the powerpc code correctly (WRT register naming conventions) just do the following: cd /sys/kernel/debug/tracing echo 'p do_fork %gpr0' kprobe_events echo 1 events/kprobes/enable ls cat trace echo 0 events/kprobes/enable Every fork() call done on the system between those two echo commands (hence the ls) should append a line to the trace file. For a more exhaustive test one could repeat this sequence for every register in the architecture. OK, so I went the whole hog and did: $ echo 'p do_fork %gpr0 %gpr1 %gpr2 %gpr3 %gpr4 %gpr5 %gpr6 %gpr7 %gpr8 %gpr9 %gpr10 %gpr11 %gpr12 %gpr13 %gpr14 %gpr15 %gpr16 %gpr17 %gpr18 %gpr19 %gpr20 %gpr21 %gpr22 %gpr23 %gpr24 %gpr25 %gpr26 %gpr27 %gpr28 %gpr29 %gpr30 %gpr31 %nip %msr %ctr %link %xer %ccr %softe %trap %dar %dsisr' kprobe_events And I get: bash-2057 [001] d... 535.433941: p_do_fork_0: (do_fork+0x8/0x490) arg1=0xc00094d0 arg2=0xc001fbe9be30 arg3=0xc1133bb8 arg4=0x1200011 arg5=0x0 arg6=0x0 arg7=0x0 arg8=0x3fff7c885940 arg9=0x1 arg10=0xc001fbe9bea0 arg11=0x0 arg12=0xc01 arg13=0xc00094c8 arg14=0xcfdc0480 arg15=0x0 arg16=0x2200 arg17=0x1016d6e8 arg18=0x0 arg19=0x4400 arg20=0x0 arg21=0x10037c82208 arg22=0x1017b008 arg23=0x10143d18 arg24=0x10178854 arg25=0x10144f90 arg26=0x10037c821e8 arg27=0x0 arg28=0x0 arg29=0x0 arg30=0x0 arg31=0x809 arg32=0x3788c010 arg33=0xc00a7fe8 arg34=0x80029033 arg35=0xc00094c8 arg36=0xc00094d0 arg37=0x0 arg38=0x4844 arg39=0x1 arg40=0x700 arg41=0xc001fbe9bd50 arg42=0xc001fbe9bd30 Which is ugly as hell, but appears unchanged since before your patch. Excellent. Many thanks. No worries. Did I already send you an ack? Have another one in case: Acked-by: Michael Ellerman m...@ellerman.id.au Thanks. I take it it's expected that the names are not decoded in the output? Yes. In fact I don't see anywhere that uses the reverse decoding, ie. regs_query_register_name(). Neither did I. I assumed it was intended to support either future kernel code or custom debug modules. cheers -dl ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On 06/22/15 23:32, Michael Ellerman wrote: On Fri, 2015-06-19 at 10:12 -0400, David Long wrote: On 06/19/15 00:19, Michael Ellerman wrote: On Mon, 2015-06-15 at 12:42 -0400, David Long wrote: From: David A. Long dave.l...@linaro.org The pt_regs_offset structure is used for HAVE_REGS_AND_STACK_ACCESS_API feature and has identical definitions in four different arch ptrace.h include files. It seems unlikely that definition would ever need to be changed regardless of architecture so lets move it into include/linux/ptrace.h. Signed-off-by: David A. Long dave.l...@linaro.org --- arch/powerpc/kernel/ptrace.c | 5 - Built and booted on powerpc, but is there an easy way to actually test the code paths in question? There is an easy way to smoke test it on all archiectures that also implement kprobes (which powerpc does). If I'm understanding the powerpc code correctly (WRT register naming conventions) just do the following: cd /sys/kernel/debug/tracing echo 'p do_fork %gpr0' kprobe_events echo 1 events/kprobes/enable ls cat trace echo 0 events/kprobes/enable Every fork() call done on the system between those two echo commands (hence the ls) should append a line to the trace file. For a more exhaustive test one could repeat this sequence for every register in the architecture. OK, so I went the whole hog and did: $ echo 'p do_fork %gpr0 %gpr1 %gpr2 %gpr3 %gpr4 %gpr5 %gpr6 %gpr7 %gpr8 %gpr9 %gpr10 %gpr11 %gpr12 %gpr13 %gpr14 %gpr15 %gpr16 %gpr17 %gpr18 %gpr19 %gpr20 %gpr21 %gpr22 %gpr23 %gpr24 %gpr25 %gpr26 %gpr27 %gpr28 %gpr29 %gpr30 %gpr31 %nip %msr %ctr %link %xer %ccr %softe %trap %dar %dsisr' kprobe_events And I get: bash-2057 [001] d... 535.433941: p_do_fork_0: (do_fork+0x8/0x490) arg1=0xc00094d0 arg2=0xc001fbe9be30 arg3=0xc1133bb8 arg4=0x1200011 arg5=0x0 arg6=0x0 arg7=0x0 arg8=0x3fff7c885940 arg9=0x1 arg10=0xc001fbe9bea0 arg11=0x0 arg12=0xc01 arg13=0xc00094c8 arg14=0xcfdc0480 arg15=0x0 arg16=0x2200 arg17=0x1016d6e8 arg18=0x0 arg19=0x4400 arg20=0x0 arg21=0x10037c82208 arg22=0x1017b008 arg23=0x10143d18 arg24=0x10178854 arg25=0x10144f90 arg26=0x10037c821e8 arg27=0x0 arg28=0x0 arg29=0x0 arg30=0x0 arg31=0x809 arg32=0x3788c010 arg33=0xc00a7fe8 arg34=0x80029033 arg35=0xc00094c8 arg36=0xc00094d0 arg37=0x0 arg38=0x4844 arg39=0x1 arg40=0x700 arg41=0xc001fbe9bd50 arg42=0xc001fbe9bd30 Which is ugly as hell, but appears unchanged since before your patch. Excellent. Many thanks. I take it it's expected that the names are not decoded in the output? Yes. Also I wonder why we choose gpr when r is the more usual prefix on powerpc. I guess we can add new aliases to the table. Yeah I can't answer that, this is just what the preexisting code is written to do. I believe you could add aliases to the table, perhaps as a step in migrating to supporting only the more common naming. The reverse translation would have to return one or the other though. -dl ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On Tue, 2015-06-23 at 09:48 -0400, David Long wrote: On 06/22/15 23:32, Michael Ellerman wrote: On Fri, 2015-06-19 at 10:12 -0400, David Long wrote: On 06/19/15 00:19, Michael Ellerman wrote: On Mon, 2015-06-15 at 12:42 -0400, David Long wrote: From: David A. Long dave.l...@linaro.org The pt_regs_offset structure is used for HAVE_REGS_AND_STACK_ACCESS_API feature and has identical definitions in four different arch ptrace.h include files. It seems unlikely that definition would ever need to be changed regardless of architecture so lets move it into include/linux/ptrace.h. Signed-off-by: David A. Long dave.l...@linaro.org --- arch/powerpc/kernel/ptrace.c | 5 - Built and booted on powerpc, but is there an easy way to actually test the code paths in question? There is an easy way to smoke test it on all archiectures that also implement kprobes (which powerpc does). If I'm understanding the powerpc code correctly (WRT register naming conventions) just do the following: cd /sys/kernel/debug/tracing echo 'p do_fork %gpr0' kprobe_events echo 1 events/kprobes/enable ls cat trace echo 0 events/kprobes/enable Every fork() call done on the system between those two echo commands (hence the ls) should append a line to the trace file. For a more exhaustive test one could repeat this sequence for every register in the architecture. OK, so I went the whole hog and did: $ echo 'p do_fork %gpr0 %gpr1 %gpr2 %gpr3 %gpr4 %gpr5 %gpr6 %gpr7 %gpr8 %gpr9 %gpr10 %gpr11 %gpr12 %gpr13 %gpr14 %gpr15 %gpr16 %gpr17 %gpr18 %gpr19 %gpr20 %gpr21 %gpr22 %gpr23 %gpr24 %gpr25 %gpr26 %gpr27 %gpr28 %gpr29 %gpr30 %gpr31 %nip %msr %ctr %link %xer %ccr %softe %trap %dar %dsisr' kprobe_events And I get: bash-2057 [001] d... 535.433941: p_do_fork_0: (do_fork+0x8/0x490) arg1=0xc00094d0 arg2=0xc001fbe9be30 arg3=0xc1133bb8 arg4=0x1200011 arg5=0x0 arg6=0x0 arg7=0x0 arg8=0x3fff7c885940 arg9=0x1 arg10=0xc001fbe9bea0 arg11=0x0 arg12=0xc01 arg13=0xc00094c8 arg14=0xcfdc0480 arg15=0x0 arg16=0x2200 arg17=0x1016d6e8 arg18=0x0 arg19=0x4400 arg20=0x0 arg21=0x10037c82208 arg22=0x1017b008 arg23=0x10143d18 arg24=0x10178854 arg25=0x10144f90 arg26=0x10037c821e8 arg27=0x0 arg28=0x0 arg29=0x0 arg30=0x0 arg31=0x809 arg32=0x3788c010 arg33=0xc00a7fe8 arg34=0x80029033 arg35=0xc00094c8 arg36=0xc00094d0 arg37=0x0 arg38=0x4844 arg39=0x1 arg40=0x700 arg41=0xc001fbe9bd50 arg42=0xc001fbe9bd30 Which is ugly as hell, but appears unchanged since before your patch. Excellent. Many thanks. No worries. Did I already send you an ack? Have another one in case: Acked-by: Michael Ellerman m...@ellerman.id.au I take it it's expected that the names are not decoded in the output? Yes. In fact I don't see anywhere that uses the reverse decoding, ie. regs_query_register_name(). cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On Fri, 2015-06-19 at 10:12 -0400, David Long wrote: On 06/19/15 00:19, Michael Ellerman wrote: On Mon, 2015-06-15 at 12:42 -0400, David Long wrote: From: David A. Long dave.l...@linaro.org The pt_regs_offset structure is used for HAVE_REGS_AND_STACK_ACCESS_API feature and has identical definitions in four different arch ptrace.h include files. It seems unlikely that definition would ever need to be changed regardless of architecture so lets move it into include/linux/ptrace.h. Signed-off-by: David A. Long dave.l...@linaro.org --- arch/powerpc/kernel/ptrace.c | 5 - Built and booted on powerpc, but is there an easy way to actually test the code paths in question? There is an easy way to smoke test it on all archiectures that also implement kprobes (which powerpc does). If I'm understanding the powerpc code correctly (WRT register naming conventions) just do the following: cd /sys/kernel/debug/tracing echo 'p do_fork %gpr0' kprobe_events echo 1 events/kprobes/enable ls cat trace echo 0 events/kprobes/enable Every fork() call done on the system between those two echo commands (hence the ls) should append a line to the trace file. For a more exhaustive test one could repeat this sequence for every register in the architecture. OK, so I went the whole hog and did: $ echo 'p do_fork %gpr0 %gpr1 %gpr2 %gpr3 %gpr4 %gpr5 %gpr6 %gpr7 %gpr8 %gpr9 %gpr10 %gpr11 %gpr12 %gpr13 %gpr14 %gpr15 %gpr16 %gpr17 %gpr18 %gpr19 %gpr20 %gpr21 %gpr22 %gpr23 %gpr24 %gpr25 %gpr26 %gpr27 %gpr28 %gpr29 %gpr30 %gpr31 %nip %msr %ctr %link %xer %ccr %softe %trap %dar %dsisr' kprobe_events And I get: bash-2057 [001] d... 535.433941: p_do_fork_0: (do_fork+0x8/0x490) arg1=0xc00094d0 arg2=0xc001fbe9be30 arg3=0xc1133bb8 arg4=0x1200011 arg5=0x0 arg6=0x0 arg7=0x0 arg8=0x3fff7c885940 arg9=0x1 arg10=0xc001fbe9bea0 arg11=0x0 arg12=0xc01 arg13=0xc00094c8 arg14=0xcfdc0480 arg15=0x0 arg16=0x2200 arg17=0x1016d6e8 arg18=0x0 arg19=0x4400 arg20=0x0 arg21=0x10037c82208 arg22=0x1017b008 arg23=0x10143d18 arg24=0x10178854 arg25=0x10144f90 arg26=0x10037c821e8 arg27=0x0 arg28=0x0 arg29=0x0 arg30=0x0 arg31=0x809 arg32=0x3788c010 arg33=0xc00a7fe8 arg34=0x80029033 arg35=0xc00094c8 arg36=0xc00094d0 arg37=0x0 arg38=0x4844 arg39=0x1 arg40=0x700 arg41=0xc001fbe9bd50 arg42=0xc001fbe9bd30 Which is ugly as hell, but appears unchanged since before your patch. I take it it's expected that the names are not decoded in the output? Also I wonder why we choose gpr when r is the more usual prefix on powerpc. I guess we can add new aliases to the table. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On 06/19/15 00:19, Michael Ellerman wrote: On Mon, 2015-06-15 at 12:42 -0400, David Long wrote: From: David A. Long dave.l...@linaro.org The pt_regs_offset structure is used for HAVE_REGS_AND_STACK_ACCESS_API feature and has identical definitions in four different arch ptrace.h include files. It seems unlikely that definition would ever need to be changed regardless of architecture so lets move it into include/linux/ptrace.h. Signed-off-by: David A. Long dave.l...@linaro.org --- arch/powerpc/kernel/ptrace.c | 5 - Built and booted on powerpc, but is there an easy way to actually test the code paths in question? There is an easy way to smoke test it on all archiectures that also implement kprobes (which powerpc does). If I'm understanding the powerpc code correctly (WRT register naming conventions) just do the following: cd /sys/kernel/debug/tracing echo 'p do_fork %gpr0' kprobe_events echo 1 events/kprobes/enable ls cat trace echo 0 events/kprobes/enable Every fork() call done on the system between those two echo commands (hence the ls) should append a line to the trace file. For a more exhaustive test one could repeat this sequence for every register in the architecture. This should work the same on all architectures supporting kprobes. You just have to use the appropriate register names for your architecture after the %. Acked-by: Michael Ellerman m...@ellerman.id.au cheers Thanks, -dl ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On Fri, Jun 19, 2015 at 7:12 AM, David Long dave.l...@linaro.org wrote: On 06/19/15 00:19, Michael Ellerman wrote: On Mon, 2015-06-15 at 12:42 -0400, David Long wrote: From: David A. Long dave.l...@linaro.org The pt_regs_offset structure is used for HAVE_REGS_AND_STACK_ACCESS_API feature and has identical definitions in four different arch ptrace.h include files. It seems unlikely that definition would ever need to be changed regardless of architecture so lets move it into include/linux/ptrace.h. Signed-off-by: David A. Long dave.l...@linaro.org --- arch/powerpc/kernel/ptrace.c | 5 - Built and booted on powerpc, but is there an easy way to actually test the code paths in question? There is an easy way to smoke test it on all archiectures that also implement kprobes (which powerpc does). If I'm understanding the powerpc code correctly (WRT register naming conventions) just do the following: cd /sys/kernel/debug/tracing echo 'p do_fork %gpr0' kprobe_events echo 1 events/kprobes/enable ls cat trace echo 0 events/kprobes/enable Every fork() call done on the system between those two echo commands (hence the ls) should append a line to the trace file. For a more exhaustive test one could repeat this sequence for every register in the architecture. This should work the same on all architectures supporting kprobes. You just have to use the appropriate register names for your architecture after the %. Is this something we could codify into the selftests directory? It seems like a great thing to capture in a single place somewhere (the register lists, that is). -Kees Acked-by: Michael Ellerman m...@ellerman.id.au cheers Thanks, -dl -- Kees Cook Chrome OS Security ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On Mon, 2015-06-15 at 12:42 -0400, David Long wrote: From: David A. Long dave.l...@linaro.org The pt_regs_offset structure is used for HAVE_REGS_AND_STACK_ACCESS_API feature and has identical definitions in four different arch ptrace.h include files. It seems unlikely that definition would ever need to be changed regardless of architecture so lets move it into include/linux/ptrace.h. Signed-off-by: David A. Long dave.l...@linaro.org --- arch/powerpc/kernel/ptrace.c | 5 - Built and booted on powerpc, but is there an easy way to actually test the code paths in question? Acked-by: Michael Ellerman m...@ellerman.id.au cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On 06/16/15 09:17, Rob Herring wrote:
On Mon, Jun 15, 2015 at 11:42 AM, David Long dave.l...@linaro.org wrote:
From: David A. Long dave.l...@linaro.org
The pt_regs_offset structure is used for HAVE_REGS_AND_STACK_ACCESS_API
feature and has identical definitions in four different arch ptrace.h
include files. It seems unlikely that definition would ever need to be
changed regardless of architecture so lets move it into
include/linux/ptrace.h.
Signed-off-by: David A. Long dave.l...@linaro.org
---
arch/arm/kernel/ptrace.c | 5 -
arch/powerpc/kernel/ptrace.c | 5 -
arch/sh/include/asm/ptrace.h | 5 -
arch/x86/kernel/ptrace.c | 5 -
include/linux/ptrace.h | 9 +
5 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index ef9119f..fb45cf1 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -59,11 +59,6 @@
#define BREAKINST_THUMB0xde01
#endif
-struct pt_regs_offset {
- const char *name;
- int offset;
-};
-
#define REG_OFFSET_NAME(r) \
{.name = #r, .offset = offsetof(struct pt_regs, ARM_##r)}
#define REG_OFFSET_END {.name = NULL, .offset = 0}
Can't you also move these? ARM is complicated with the ARM_
prefixing, but the others appear to be the same. Maybe you can remove
the prefix or redefine the macro for ARM.
Rob
That would mandate that all the architecture-specific pt_regs structures
would have to use a top-level named field for each named register. That
seems to me like an unnecessary restriction when the point of
regs_offset_table is to provide the offset of the register inside an
arbitarily complex pt_regs struct. The often redundant definition of
these two macros doesn't seem to me that high a price for that.
-dl
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] Move the pt_regs_offset struct definition from arch to common include file
On Mon, Jun 15, 2015 at 11:42 AM, David Long dave.l...@linaro.org wrote:
From: David A. Long dave.l...@linaro.org
The pt_regs_offset structure is used for HAVE_REGS_AND_STACK_ACCESS_API
feature and has identical definitions in four different arch ptrace.h
include files. It seems unlikely that definition would ever need to be
changed regardless of architecture so lets move it into
include/linux/ptrace.h.
Signed-off-by: David A. Long dave.l...@linaro.org
---
arch/arm/kernel/ptrace.c | 5 -
arch/powerpc/kernel/ptrace.c | 5 -
arch/sh/include/asm/ptrace.h | 5 -
arch/x86/kernel/ptrace.c | 5 -
include/linux/ptrace.h | 9 +
5 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index ef9119f..fb45cf1 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -59,11 +59,6 @@
#define BREAKINST_THUMB0xde01
#endif
-struct pt_regs_offset {
- const char *name;
- int offset;
-};
-
#define REG_OFFSET_NAME(r) \
{.name = #r, .offset = offsetof(struct pt_regs, ARM_##r)}
#define REG_OFFSET_END {.name = NULL, .offset = 0}
Can't you also move these? ARM is complicated with the ARM_
prefixing, but the others appear to be the same. Maybe you can remove
the prefix or redefine the macro for ARM.
Rob
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
