[PATCH] spufs: change ppc_rtas declaration to weak

[Geoff Levand]

Arnd Bergmann wrote:
> On Thursday 05 October 2006 20:35, Geoff Levand wrote:
>> Index: cell--common--5/include/asm-powerpc/syscalls.h
>> ===================================================================
>> --- cell--common--5.orig/include/asm-powerpc/syscalls.h
>> +++ cell--common--5/include/asm-powerpc/syscalls.h
>> @@ -37,7 +37,7 @@
>> �asmlinkage int sys_ipc(uint call, int first, unsigned long second,
>> ����������������long third, void __user *ptr, long fifth);
>> �asmlinkage long ppc64_personality(unsigned long personality);
>> -asmlinkage int ppc_rtas(struct rtas_args __user *uargs);
>> +asmlinkage int ppc_rtas(struct rtas_args __user *uargs) __attribute__((weak));
>> �asmlinkage time_t sys64_time(time_t __user * tloc);
>> �asmlinkage long ppc_newuname(struct new_utsname __user * name);
>> �
> 
> Hmm, I can't see why this does the right thing. __attribute__((weak)) should
> normally be put only into the definition of a function, not into the common
> declaration. This looks like it makes _both_ definitions (kernel/sys.c and
> arch/powerpc/kernel/rtas.c) weak, so on pseries it becomes unspecific which
> one is actually used.


You're right, I see now that that fix just worked by pure chance.


> The problem that this is trying to work around is probably caused by the
> dot-symbols: cond_syscall defines a ".ppc_rtas", but not a "ppc_rtas" symbol,
> which spufs tries to resolve.


Yes, on studying the code produced I found that the problem was that
cond_syscall() didn't create a non-dot symbol that the C linkage of
spu_syscall_table[] expected, as seen here:

./arch/powerpc/platforms/cell/spu_callbacks.o
                 U ppc_rtas
./kernel/sys_ni.o
0000000000000000 W .ppc_rtas

As you suggested to try, the method used by ia64 works properly, creating both
a dot and a non-dot weak symbol:

./kernel/sys_ni.o
0000000000000000 W .ppc_rtas
0000000000000000 W ppc_rtas

Updated patch follows.

-Geoff



Change the definition of powerpc's cond_syscall() to use the standard gcc
weak attribute specifier which provides proper support for C linkage as
needed by spu_syscall_table[].

Fixes this powerpc build error with CONFIG_SPU_FS=y, CONFIG_PPC_RTAS=n:

 arch/powerpc/platforms/built-in.o: undefined reference to `ppc_rtas'


Signed-off-by: Geoff Levand <geoffrey.levand at am.sony.com>

---

Build tests done with pseries_defconfig, cell_defconfig and ebony_defconfig.

>From what I could determine, the toolchain troubles are no longer relevant
with the recent change to a minimum version of gcc 3.2 for kernel builds.
Confirmation of this point would be appreciated.

 
Index: cell--common--5/include/asm-powerpc/unistd.h
===================================================================
--- cell--common--5.orig/include/asm-powerpc/unistd.h
+++ cell--common--5/include/asm-powerpc/unistd.h
@@ -445,7 +445,6 @@
 #include <linux/types.h>
 #include <linux/compiler.h>
 #include <linux/linkage.h>
-#include <asm/syscalls.h>
 
 #define __ARCH_WANT_IPC_PARSE_VERSION
 #define __ARCH_WANT_OLD_READDIR
@@ -487,16 +486,9 @@
 
 /*
  * "Conditional" syscalls
- *
- * What we want is __attribute__((weak,alias("sys_ni_syscall"))),
- * but it doesn't work on all toolchains, so we just do it by hand
  */
-#ifdef CONFIG_PPC32
-#define cond_syscall(x) asm(".weak\t" #x "\n\t.set\t" #x ",sys_ni_syscall")
-#else
-#define cond_syscall(x) asm(".weak\t." #x "\n\t.set\t." #x ",.sys_ni_syscall")
-#endif
-
+#define cond_syscall(x) \
+	asmlinkage long x (void) __attribute__((weak,alias("sys_ni_syscall")))
 
 #endif		/* __ASSEMBLY__ */
 #endif		/* __KERNEL__ */