lkml.org 
[lkml]   [2012]   [Feb]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] ARM: Wire up HAVE_SYSCALL_TRACEPOINTS
From
On Thu, February 2, 2012 10:21, Takuo Koguchi wrote:
> Hello,
> I am glad to start getting responses.
> (2012/02/01 10:47), Indan Zupancic wrote:
>> Hello,
>>
>> CC'ing Roland.
>>
>> On Thu, December 1, 2011 12:01, takuo.koguchi.sw@hitachi.com wrote:
>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> index 44789ef..84181b3 100644
>>> --- a/arch/arm/Kconfig
>>> +++ b/arch/arm/Kconfig
>>> @@ -13,6 +13,7 @@ config ARM
>>> select HAVE_KPROBES if !XIP_KERNEL
>>> select HAVE_KRETPROBES if (HAVE_KPROBES)
>>> select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
>>> + select HAVE_SYSCALL_TRACEPOINTS
>> Your patch totally ignores OABI, it should at least depend on CONFIG_AEABI
>> and on !CONFIG_OABI_COMPAT.
> Right. As Russel King suggested, this patch depends on those configs
> until very large NR_syscalls is properly handled by ftrace.

It has nothing to do with large NR_syscalls. Supporting OABI is hard,
e.g. it doesn't put the syscall nr in r7, it's encoded as part of the
syscall instruction. Also the ABI for some system calls is different,
with different arg layouts (alignment of 64 bit args is different).

>
>>> select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
>>> select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL)
>>> select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
>>> diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
>>> new file mode 100644
>>> index 0000000..cabeb67
>>> --- /dev/null
>>> +++ b/arch/arm/include/asm/syscall.h
>>> @@ -0,0 +1,45 @@
>>> +#ifndef _ASM_ARM_SYSCALL_H
>>> +#define _ASM_ARM_SYSCALL_H
>>> +
>>> +extern const unsigned long sys_call_table[];
>>> +
>>> +#include<linux/sched.h>
>>> +
>>> +static inline long syscall_get_nr(struct task_struct *task,
>>> + struct pt_regs *regs)
>>> +{
>>> + return regs->ARM_r7;
>>> +}
>>> +
>>> +static inline long syscall_get_return_value(struct task_struct *task,
>>> + struct pt_regs *regs)
>>> +{
>>> + return regs->ARM_r0;
>>> +}
>>> +
>>> +static inline void syscall_get_arguments(struct task_struct *task,
>>> + struct pt_regs *regs,
>>> + unsigned int i, unsigned int n,
>>> + unsigned long *args)
>>> +{
>>> + BUG_ON(i);
>> This is unacceptable, that would make this function useless.
>>
>> See Rolands patch:
>>
>> https://lkml.org/lkml/2009/4/24/399 for a better implementation.
> Thanks. My apology for the lack of investigation.

I got both links from Will Drewry, I'm just jumping in and passing it on.

>>> +#endif /* _ASM_ARM_SYSCALL_H */
>>> diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
>>> index 7b5cc8d..2509028 100644
>>> --- a/arch/arm/include/asm/thread_info.h
>>> +++ b/arch/arm/include/asm/thread_info.h
>>> @@ -139,6 +139,7 @@ extern void vfp_flush_hwstate(struct thread_info *);
>>> #define TIF_NEED_RESCHED 1
>>> #define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
>>> #define TIF_SYSCALL_TRACE 8
>>> +#define TIF_SYSCALL_TRACEPOINT 15
>>> #define TIF_POLLING_NRFLAG 16
>>> #define TIF_USING_IWMMXT 17
>>> #define TIF_MEMDIE 18 /* is terminating due to OOM killer */
>>> @@ -150,11 +151,13 @@ extern void vfp_flush_hwstate(struct thread_info *);
>>> #define _TIF_NEED_RESCHED (1<< TIF_NEED_RESCHED)
>>> #define _TIF_NOTIFY_RESUME (1<< TIF_NOTIFY_RESUME)
>>> #define _TIF_SYSCALL_TRACE (1<< TIF_SYSCALL_TRACE)
>>> +#define _TIF_SYSCALL_TRACEPOINT (1<< TIF_SYSCALL_TRACEPOINT)
>>> #define _TIF_POLLING_NRFLAG (1<< TIF_POLLING_NRFLAG)
>>> #define _TIF_USING_IWMMXT (1<< TIF_USING_IWMMXT)
>>> #define _TIF_FREEZE (1<< TIF_FREEZE)
>>> #define _TIF_RESTORE_SIGMASK (1<< TIF_RESTORE_SIGMASK)
>>> #define _TIF_SECCOMP (1<< TIF_SECCOMP)
>>> +#define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_TRACEPOINT)
>> What does 'A' stand for?
> 'A' originally stands for AUDIT. I should have used a better name as
> there is no _TIF_SYSCALL_AUDIT for ARM.
> Probably it is better to define _TIF_WORK_SYSCALL_ENTRY and
> _TIF_WORK_SYSCALL_EXIT properly.

That would be much better.

>>> /*
>>> * Change these and you break ASM code in entry-common.S
>>> diff --git a/arch/arm/include/asm/unistd.h b/arch/arm/include/asm/unistd.h
>>> index 4a11237..f4eac2d 100644
>>> --- a/arch/arm/include/asm/unistd.h
>>> +++ b/arch/arm/include/asm/unistd.h
>>> @@ -405,6 +405,9 @@
>>> #define __NR_process_vm_readv (__NR_SYSCALL_BASE+376)
>>> #define __NR_process_vm_writev (__NR_SYSCALL_BASE+377)
>>>
>>> +#ifndef __ASSEMBLY__
>>> +#define NR_syscalls 378
>>> +#endif
>>> /*
>>> * The following SWIs are ARM private.
>>> */
>>> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
>>> index b2a27b6..a1577c2 100644
>>> --- a/arch/arm/kernel/entry-common.S
>>> +++ b/arch/arm/kernel/entry-common.S
>>> @@ -87,7 +87,7 @@ ENTRY(ret_from_fork)
>>> get_thread_info tsk
>>> ldr r1, [tsk, #TI_FLAGS] @ check for syscall tracing
>>> mov why, #1
>>> - tst r1, #_TIF_SYSCALL_TRACE @ are we tracing syscalls?
>>> + tst r1, #_TIF_SYSCALL_T_OR_A @ are we tracing syscalls?
>>> beq ret_slow_syscall
>>> mov r1, sp
>>> mov r0, #1 @ trace exit [IP = 1]
>>> @@ -443,7 +443,7 @@ ENTRY(vector_swi)
>>> 1:
>>> #endif
>>>
>>> - tst r10, #_TIF_SYSCALL_TRACE @ are we tracing syscalls?
>>> + tst r10, #_TIF_SYSCALL_T_OR_A @ are we tracing syscalls?
>>> bne __sys_trace
>>>
>>> cmp scno, #NR_syscalls @ check upper syscall limit
>>> diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
>>> index 483727a..a690c9f 100644
>>> --- a/arch/arm/kernel/ptrace.c
>>> +++ b/arch/arm/kernel/ptrace.c
>>> @@ -28,6 +28,9 @@
>>> #include<asm/system.h>
>>> #include<asm/traps.h>
>>>
>>> +#define CREATE_TRACE_POINTS
>>> +#include<trace/events/syscalls.h>
>>> +
>>> #define REG_PC 15
>>> #define REG_PSR 16
>>> /*
>>> @@ -906,6 +909,13 @@ asmlinkage int syscall_trace(int why, struct pt_regs *regs, int
>>> scno)
>>> {
>>> unsigned long ip;
>> Not used, you get the same info from 'why'.
> Sorry. But I do not understand what you suggest here. This is the
> existing code.

Sorry, I missed that.

>>
>>> + if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) {
>>> + if (why)
>>> + trace_sys_exit(regs, regs->ARM_r0);
>>> + else
>>> + trace_sys_enter(regs, scno);
>>> + }
>>> +
>>> if (!test_thread_flag(TIF_SYSCALL_TRACE))
>>> return scno;
>>> if (!(current->ptrace& PT_PTRACED))
>>
>
> Thanks,
>
> Takuo
>

Greetings,

Indan




\
 
 \ /
  Last update: 2012-02-02 12:03    [W:0.214 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site