* [PATCH] powerpc: fix sys_call_table declaration @ 2014-10-02 14:41 ` Romeo Cane 0 siblings, 0 replies; 8+ messages in thread From: Romeo Cane @ 2014-10-02 14:41 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman Cc: linuxppc-dev, linux-kernel Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr Signed-off-by: Romeo Cane <romeo.cane.ext@coriant.com> --- arch/powerpc/include/asm/syscall.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h index b54b2ad..528ba9d 100644 --- a/arch/powerpc/include/asm/syscall.h +++ b/arch/powerpc/include/asm/syscall.h @@ -17,7 +17,7 @@ /* ftrace syscalls requires exporting the sys_call_table */ #ifdef CONFIG_FTRACE_SYSCALLS -extern const unsigned long *sys_call_table; +extern const unsigned long sys_call_table[]; #endif /* CONFIG_FTRACE_SYSCALLS */ static inline long syscall_get_nr(struct task_struct *task, -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] powerpc: fix sys_call_table declaration @ 2014-10-02 14:41 ` Romeo Cane 0 siblings, 0 replies; 8+ messages in thread From: Romeo Cane @ 2014-10-02 14:41 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman Cc: linuxppc-dev, linux-kernel Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr Signed-off-by: Romeo Cane <romeo.cane.ext@coriant.com> --- arch/powerpc/include/asm/syscall.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h index b54b2ad..528ba9d 100644 --- a/arch/powerpc/include/asm/syscall.h +++ b/arch/powerpc/include/asm/syscall.h @@ -17,7 +17,7 @@ /* ftrace syscalls requires exporting the sys_call_table */ #ifdef CONFIG_FTRACE_SYSCALLS -extern const unsigned long *sys_call_table; +extern const unsigned long sys_call_table[]; #endif /* CONFIG_FTRACE_SYSCALLS */ static inline long syscall_get_nr(struct task_struct *task, -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: fix sys_call_table declaration 2014-10-02 14:41 ` Romeo Cane @ 2014-10-02 21:34 ` Benjamin Herrenschmidt -1 siblings, 0 replies; 8+ messages in thread From: Benjamin Herrenschmidt @ 2014-10-02 21:34 UTC (permalink / raw) To: Romeo Cane; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel On Thu, 2014-10-02 at 15:41 +0100, Romeo Cane wrote: > Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr Care to elaborate ? Ben. > Signed-off-by: Romeo Cane <romeo.cane.ext@coriant.com> > --- > arch/powerpc/include/asm/syscall.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h > index b54b2ad..528ba9d 100644 > --- a/arch/powerpc/include/asm/syscall.h > +++ b/arch/powerpc/include/asm/syscall.h > @@ -17,7 +17,7 @@ > > /* ftrace syscalls requires exporting the sys_call_table */ > #ifdef CONFIG_FTRACE_SYSCALLS > -extern const unsigned long *sys_call_table; > +extern const unsigned long sys_call_table[]; > #endif /* CONFIG_FTRACE_SYSCALLS */ > > static inline long syscall_get_nr(struct task_struct *task, ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: fix sys_call_table declaration @ 2014-10-02 21:34 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 8+ messages in thread From: Benjamin Herrenschmidt @ 2014-10-02 21:34 UTC (permalink / raw) To: Romeo Cane; +Cc: Paul Mackerras, Michael Ellerman, linuxppc-dev, linux-kernel On Thu, 2014-10-02 at 15:41 +0100, Romeo Cane wrote: > Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr Care to elaborate ? Ben. > Signed-off-by: Romeo Cane <romeo.cane.ext@coriant.com> > --- > arch/powerpc/include/asm/syscall.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h > index b54b2ad..528ba9d 100644 > --- a/arch/powerpc/include/asm/syscall.h > +++ b/arch/powerpc/include/asm/syscall.h > @@ -17,7 +17,7 @@ > > /* ftrace syscalls requires exporting the sys_call_table */ > #ifdef CONFIG_FTRACE_SYSCALLS > -extern const unsigned long *sys_call_table; > +extern const unsigned long sys_call_table[]; > #endif /* CONFIG_FTRACE_SYSCALLS */ > > static inline long syscall_get_nr(struct task_struct *task, ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: fix sys_call_table declaration 2014-10-02 21:34 ` Benjamin Herrenschmidt @ 2014-10-03 10:00 ` Romeo Cane -1 siblings, 0 replies; 8+ messages in thread From: Romeo Cane @ 2014-10-03 10:00 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel On Fri, Oct 03, 2014 at 07:34:34AM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2014-10-02 at 15:41 +0100, Romeo Cane wrote: > > Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr > > Care to elaborate ? > > Ben. > > > Signed-off-by: Romeo Cane <romeo.cane.ext@coriant.com> > > --- > > arch/powerpc/include/asm/syscall.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h > > index b54b2ad..528ba9d 100644 > > --- a/arch/powerpc/include/asm/syscall.h > > +++ b/arch/powerpc/include/asm/syscall.h > > @@ -17,7 +17,7 @@ > > > > /* ftrace syscalls requires exporting the sys_call_table */ > > #ifdef CONFIG_FTRACE_SYSCALLS > > -extern const unsigned long *sys_call_table; > > +extern const unsigned long sys_call_table[]; > > #endif /* CONFIG_FTRACE_SYSCALLS */ > > > > static inline long syscall_get_nr(struct task_struct *task, > > Hi Ben, this is the arch_syscall_addr function from kernel/trace/trace_syscalls.c: unsigned long __init __weak arch_syscall_addr(int nr) { return (unsigned long)sys_call_table[nr]; } on my platform (E500MC) the generated assembly code is as follows: without the patch: <arch_syscall_addr>: lis r9,-16384 rlwinm r3,r3,2,0,29 lwz r11,30640(r9) lwzx r3,r11,r3 blr with the patch: <arch_syscall_addr>: lis r9,-16384 rlwinm r3,r3,2,0,29 addi r9,r9,30640 lwzx r3,r9,r3 blr the goal of the function is to retrieve the n-th element of the table (i.e. the address of a syscall) Without the patch, the returned value is in fact the memory content pointed by the address of the first syscall plus an offset, that is not what we want. The consequence is that ftrace of syscalls doesn't work. That table has always been declared as a pointer since the support for syscalls tracing has been introduced for powerpc years ago, so I'm wondering why nobody else had this problem before. Other architectures are not affected since in their includes the table is already declared as an array. Romeo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: fix sys_call_table declaration @ 2014-10-03 10:00 ` Romeo Cane 0 siblings, 0 replies; 8+ messages in thread From: Romeo Cane @ 2014-10-03 10:00 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Paul Mackerras, Michael Ellerman, linuxppc-dev, linux-kernel On Fri, Oct 03, 2014 at 07:34:34AM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2014-10-02 at 15:41 +0100, Romeo Cane wrote: > > Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr > > Care to elaborate ? > > Ben. > > > Signed-off-by: Romeo Cane <romeo.cane.ext@coriant.com> > > --- > > arch/powerpc/include/asm/syscall.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h > > index b54b2ad..528ba9d 100644 > > --- a/arch/powerpc/include/asm/syscall.h > > +++ b/arch/powerpc/include/asm/syscall.h > > @@ -17,7 +17,7 @@ > > > > /* ftrace syscalls requires exporting the sys_call_table */ > > #ifdef CONFIG_FTRACE_SYSCALLS > > -extern const unsigned long *sys_call_table; > > +extern const unsigned long sys_call_table[]; > > #endif /* CONFIG_FTRACE_SYSCALLS */ > > > > static inline long syscall_get_nr(struct task_struct *task, > > Hi Ben, this is the arch_syscall_addr function from kernel/trace/trace_syscalls.c: unsigned long __init __weak arch_syscall_addr(int nr) { return (unsigned long)sys_call_table[nr]; } on my platform (E500MC) the generated assembly code is as follows: without the patch: <arch_syscall_addr>: lis r9,-16384 rlwinm r3,r3,2,0,29 lwz r11,30640(r9) lwzx r3,r11,r3 blr with the patch: <arch_syscall_addr>: lis r9,-16384 rlwinm r3,r3,2,0,29 addi r9,r9,30640 lwzx r3,r9,r3 blr the goal of the function is to retrieve the n-th element of the table (i.e. the address of a syscall) Without the patch, the returned value is in fact the memory content pointed by the address of the first syscall plus an offset, that is not what we want. The consequence is that ftrace of syscalls doesn't work. That table has always been declared as a pointer since the support for syscalls tracing has been introduced for powerpc years ago, so I'm wondering why nobody else had this problem before. Other architectures are not affected since in their includes the table is already declared as an array. Romeo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: fix sys_call_table declaration 2014-10-03 10:00 ` Romeo Cane @ 2014-10-07 12:15 ` Michael Ellerman -1 siblings, 0 replies; 8+ messages in thread From: Michael Ellerman @ 2014-10-07 12:15 UTC (permalink / raw) To: Romeo Cane; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel On Fri, 2014-10-03 at 11:00 +0100, Romeo Cane wrote: > On Fri, Oct 03, 2014 at 07:34:34AM +1000, Benjamin Herrenschmidt wrote: > > On Thu, 2014-10-02 at 15:41 +0100, Romeo Cane wrote: > > > Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr > > > > Care to elaborate ? > > > > > diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h > > > index b54b2ad..528ba9d 100644 > > > --- a/arch/powerpc/include/asm/syscall.h > > > +++ b/arch/powerpc/include/asm/syscall.h > > > @@ -17,7 +17,7 @@ > > > > > > /* ftrace syscalls requires exporting the sys_call_table */ > > > #ifdef CONFIG_FTRACE_SYSCALLS > > > -extern const unsigned long *sys_call_table; > > > +extern const unsigned long sys_call_table[]; > > > #endif /* CONFIG_FTRACE_SYSCALLS */ > > > > > > static inline long syscall_get_nr(struct task_struct *task, > > Hi Ben, > > this is the arch_syscall_addr function from kernel/trace/trace_syscalls.c: > > unsigned long __init __weak arch_syscall_addr(int nr) > { > return (unsigned long)sys_call_table[nr]; > } > > on my platform (E500MC) the generated assembly code is as follows: > > without the patch: > <arch_syscall_addr>: > lis r9,-16384 > rlwinm r3,r3,2,0,29 > lwz r11,30640(r9) > lwzx r3,r11,r3 > blr > > with the patch: > <arch_syscall_addr>: > lis r9,-16384 > rlwinm r3,r3,2,0,29 > addi r9,r9,30640 > lwzx r3,r9,r3 > blr > > > the goal of the function is to retrieve the n-th element of the table (i.e. > the address of a syscall) > Without the patch, the returned value is in fact the memory content pointed > by the address of the first syscall plus an offset, that is not what we want. > The consequence is that ftrace of syscalls doesn't work. > > That table has always been declared as a pointer since the support for > syscalls tracing has been introduced for powerpc years ago, so I'm wondering > why nobody else had this problem before. > Other architectures are not affected since in their includes the table is > already declared as an array. Yeah looks like you're right. I've only ever used the raw_syscall tracing, which does work. Worringly we also use sys_call_table as extern unsigned long * in vdso.c, so I wonder if that is also broken. cheers ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: fix sys_call_table declaration @ 2014-10-07 12:15 ` Michael Ellerman 0 siblings, 0 replies; 8+ messages in thread From: Michael Ellerman @ 2014-10-07 12:15 UTC (permalink / raw) To: Romeo Cane Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, linux-kernel On Fri, 2014-10-03 at 11:00 +0100, Romeo Cane wrote: > On Fri, Oct 03, 2014 at 07:34:34AM +1000, Benjamin Herrenschmidt wrote: > > On Thu, 2014-10-02 at 15:41 +0100, Romeo Cane wrote: > > > Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr > > > > Care to elaborate ? > > > > > diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h > > > index b54b2ad..528ba9d 100644 > > > --- a/arch/powerpc/include/asm/syscall.h > > > +++ b/arch/powerpc/include/asm/syscall.h > > > @@ -17,7 +17,7 @@ > > > > > > /* ftrace syscalls requires exporting the sys_call_table */ > > > #ifdef CONFIG_FTRACE_SYSCALLS > > > -extern const unsigned long *sys_call_table; > > > +extern const unsigned long sys_call_table[]; > > > #endif /* CONFIG_FTRACE_SYSCALLS */ > > > > > > static inline long syscall_get_nr(struct task_struct *task, > > Hi Ben, > > this is the arch_syscall_addr function from kernel/trace/trace_syscalls.c: > > unsigned long __init __weak arch_syscall_addr(int nr) > { > return (unsigned long)sys_call_table[nr]; > } > > on my platform (E500MC) the generated assembly code is as follows: > > without the patch: > <arch_syscall_addr>: > lis r9,-16384 > rlwinm r3,r3,2,0,29 > lwz r11,30640(r9) > lwzx r3,r11,r3 > blr > > with the patch: > <arch_syscall_addr>: > lis r9,-16384 > rlwinm r3,r3,2,0,29 > addi r9,r9,30640 > lwzx r3,r9,r3 > blr > > > the goal of the function is to retrieve the n-th element of the table (i.e. > the address of a syscall) > Without the patch, the returned value is in fact the memory content pointed > by the address of the first syscall plus an offset, that is not what we want. > The consequence is that ftrace of syscalls doesn't work. > > That table has always been declared as a pointer since the support for > syscalls tracing has been introduced for powerpc years ago, so I'm wondering > why nobody else had this problem before. > Other architectures are not affected since in their includes the table is > already declared as an array. Yeah looks like you're right. I've only ever used the raw_syscall tracing, which does work. Worringly we also use sys_call_table as extern unsigned long * in vdso.c, so I wonder if that is also broken. cheers ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-10-07 12:16 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-02 14:41 [PATCH] powerpc: fix sys_call_table declaration Romeo Cane 2014-10-02 14:41 ` Romeo Cane 2014-10-02 21:34 ` Benjamin Herrenschmidt 2014-10-02 21:34 ` Benjamin Herrenschmidt 2014-10-03 10:00 ` Romeo Cane 2014-10-03 10:00 ` Romeo Cane 2014-10-07 12:15 ` Michael Ellerman 2014-10-07 12:15 ` Michael Ellerman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.