| Message ID | YWSSVwQ9tc9YaC+f@ls3530 (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | parisc: Fix xmpyu usage in 32-bit kernel | expand |
On 2021-10-11 3:36 p.m., Helge Deller wrote: > Dave noticed that the libgcc.a which is linked into the kernel binary > uses the xmpyu assembler statement. This asm statement uses floating > point registers which are forbidden in the kernel, because we don't save > the FP regs at kernel entry. > > Switch the parisc kernel to use the already existing shared copies of > some GCC library routines instead of linking in those from libgcc. > > This patch avoids all uses of xmpyu in the 32-bit kernel, but the 64-bit > kernel still needs fixing. > > Reported-by: John David Anglin <dave.anglin@bell.net> > Signed-off-by: Helge Deller <deller@gmx.de> > > diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig > index fc17285f4da1..e412d5c6c64f 100644 > --- a/arch/parisc/Kconfig > +++ b/arch/parisc/Kconfig > @@ -36,6 +36,12 @@ config PARISC > select GENERIC_SMP_IDLE_THREAD > select GENERIC_CPU_DEVICES > select GENERIC_LIB_DEVMEM_IS_ALLOWED > + select GENERIC_LIB_ASHLDI3 > + select GENERIC_LIB_ASHRDI3 > + select GENERIC_LIB_LSHRDI3 > + select GENERIC_LIB_MULDI3 > + select GENERIC_LIB_CMPDI2 > + select GENERIC_LIB_UCMPDI2 I would avoid using the generic routines in the kernel. The routines from libgcc are better maintained and produce better code. This is really a gcc bug. libgcc needs to be built with -msoft-float. This will prevent the use of the xmpyu instruction in __muldi3, __divdi3, __udivdi3, __moddi3 and __umoddi3, and fix both 32 and 64-bit kernels. The libgcc build also needs to be fixed to provide the full compliment of software floating-point routines. The -msoft-float and -mdisable-fpregs options are equivalent but currently only -msoft-float disables generation of all floating-point instructions. > select SYSCTL_ARCH_UNALIGN_ALLOW > select SYSCTL_EXCEPTION_TRACE > select HAVE_MOD_ARCH_SPECIFIC > diff --git a/arch/parisc/kernel/parisc_ksyms.c b/arch/parisc/kernel/parisc_ksyms.c > index 00297e8e1c88..da8c99c5a8c6 100644 > --- a/arch/parisc/kernel/parisc_ksyms.c > +++ b/arch/parisc/kernel/parisc_ksyms.c > @@ -92,18 +92,6 @@ EXPORT_SYMBOL($$divI_12); > EXPORT_SYMBOL($$divI_14); > EXPORT_SYMBOL($$divI_15); > > -extern void __ashrdi3(void); > -extern void __ashldi3(void); > -extern void __lshrdi3(void); If these are actually being used, pa.md needs to be enhanced to avoid these calls from being generated. It should be possible to handle these using hardware shifts. > -extern void __muldi3(void); > -extern void __ucmpdi2(void); > - > -EXPORT_SYMBOL(__ashrdi3); > -EXPORT_SYMBOL(__ashldi3); > -EXPORT_SYMBOL(__lshrdi3); > -EXPORT_SYMBOL(__muldi3); > -EXPORT_SYMBOL(__ucmpdi2); > - > asmlinkage void * __canonicalize_funcptr_for_compare(void *); > EXPORT_SYMBOL(__canonicalize_funcptr_for_compare); > > diff --git a/arch/parisc/lib/Makefile b/arch/parisc/lib/Makefile > index 7b197667faf6..2bfafb3c9ae0 100644 > --- a/arch/parisc/lib/Makefile > +++ b/arch/parisc/lib/Makefile > @@ -4,6 +4,6 @@ > # > > lib-y := lusercopy.o bitops.o checksum.o io.o memset.o memcpy.o \ > - ucmpdi2.o delay.o > + delay.o > > obj-y := iomap.o > diff --git a/arch/parisc/lib/ucmpdi2.c b/arch/parisc/lib/ucmpdi2.c > deleted file mode 100644 > index 8e6014a142ef..000000000000 > --- a/arch/parisc/lib/ucmpdi2.c > +++ /dev/null > @@ -1,26 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0 > -#include <linux/module.h> > - > -union ull_union { > - unsigned long long ull; > - struct { > - unsigned int high; > - unsigned int low; > - } ui; > -}; > - > -int __ucmpdi2(unsigned long long a, unsigned long long b) > -{ > - union ull_union au = {.ull = a}; > - union ull_union bu = {.ull = b}; > - > - if (au.ui.high < bu.ui.high) > - return 0; > - else if (au.ui.high > bu.ui.high) > - return 2; > - if (au.ui.low < bu.ui.low) > - return 0; > - else if (au.ui.low > bu.ui.low) > - return 2; > - return 1; > -} This should be removed and libgcc routine used. Dave
On 10/12/21 13:43, John David Anglin wrote: > On 2021-10-11 3:36 p.m., Helge Deller wrote: >> Dave noticed that the libgcc.a which is linked into the kernel binary >> uses the xmpyu assembler statement. This asm statement uses floating >> point registers which are forbidden in the kernel, because we don't save >> the FP regs at kernel entry. >> >> Switch the parisc kernel to use the already existing shared copies of >> some GCC library routines instead of linking in those from libgcc. >> >> This patch avoids all uses of xmpyu in the 32-bit kernel, but the 64-bit >> kernel still needs fixing. >> >> Reported-by: John David Anglin <dave.anglin@bell.net> >> Signed-off-by: Helge Deller <deller@gmx.de> >> >> diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig >> index fc17285f4da1..e412d5c6c64f 100644 >> --- a/arch/parisc/Kconfig >> +++ b/arch/parisc/Kconfig >> @@ -36,6 +36,12 @@ config PARISC >> select GENERIC_SMP_IDLE_THREAD >> select GENERIC_CPU_DEVICES >> select GENERIC_LIB_DEVMEM_IS_ALLOWED >> + select GENERIC_LIB_ASHLDI3 >> + select GENERIC_LIB_ASHRDI3 >> + select GENERIC_LIB_LSHRDI3 >> + select GENERIC_LIB_MULDI3 >> + select GENERIC_LIB_CMPDI2 >> + select GENERIC_LIB_UCMPDI2 > I would avoid using the generic routines in the kernel. The routines from libgcc are better maintained > and produce better code. > > This is really a gcc bug. libgcc needs to be built with -msoft-float. This will prevent the use of > the xmpyu instruction in __muldi3, __divdi3, __udivdi3, __moddi3 and __umoddi3, and fix both > 32 and 64-bit kernels. > > The libgcc build also needs to be fixed to provide the full compliment of software floating-point > routines. > > The -msoft-float and -mdisable-fpregs options are equivalent but currently only -msoft-float disables > generation of all floating-point instructions. >> select SYSCTL_ARCH_UNALIGN_ALLOW >> select SYSCTL_EXCEPTION_TRACE >> select HAVE_MOD_ARCH_SPECIFIC >> diff --git a/arch/parisc/kernel/parisc_ksyms.c b/arch/parisc/kernel/parisc_ksyms.c >> index 00297e8e1c88..da8c99c5a8c6 100644 >> --- a/arch/parisc/kernel/parisc_ksyms.c >> +++ b/arch/parisc/kernel/parisc_ksyms.c >> @@ -92,18 +92,6 @@ EXPORT_SYMBOL($$divI_12); >> EXPORT_SYMBOL($$divI_14); >> EXPORT_SYMBOL($$divI_15); >> >> -extern void __ashrdi3(void); >> -extern void __ashldi3(void); >> -extern void __lshrdi3(void); > If these are actually being used, pa.md needs to be enhanced to avoid these calls from being generated. > It should be possible to handle these using hardware shifts. >> -extern void __muldi3(void); >> -extern void __ucmpdi2(void); >> - >> -EXPORT_SYMBOL(__ashrdi3); >> -EXPORT_SYMBOL(__ashldi3); >> -EXPORT_SYMBOL(__lshrdi3); >> -EXPORT_SYMBOL(__muldi3); >> -EXPORT_SYMBOL(__ucmpdi2); >> - >> asmlinkage void * __canonicalize_funcptr_for_compare(void *); >> EXPORT_SYMBOL(__canonicalize_funcptr_for_compare); >> >> diff --git a/arch/parisc/lib/Makefile b/arch/parisc/lib/Makefile >> index 7b197667faf6..2bfafb3c9ae0 100644 >> --- a/arch/parisc/lib/Makefile >> +++ b/arch/parisc/lib/Makefile >> @@ -4,6 +4,6 @@ >> # >> >> lib-y := lusercopy.o bitops.o checksum.o io.o memset.o memcpy.o \ >> - ucmpdi2.o delay.o >> + delay.o >> >> obj-y := iomap.o >> diff --git a/arch/parisc/lib/ucmpdi2.c b/arch/parisc/lib/ucmpdi2.c >> deleted file mode 100644 >> index 8e6014a142ef..000000000000 >> --- a/arch/parisc/lib/ucmpdi2.c >> +++ /dev/null >> @@ -1,26 +0,0 @@ >> -// SPDX-License-Identifier: GPL-2.0 >> -#include <linux/module.h> >> - >> -union ull_union { >> - unsigned long long ull; >> - struct { >> - unsigned int high; >> - unsigned int low; >> - } ui; >> -}; >> - >> -int __ucmpdi2(unsigned long long a, unsigned long long b) >> -{ >> - union ull_union au = {.ull = a}; >> - union ull_union bu = {.ull = b}; >> - >> - if (au.ui.high < bu.ui.high) >> - return 0; >> - else if (au.ui.high > bu.ui.high) >> - return 2; >> - if (au.ui.low < bu.ui.low) >> - return 0; >> - else if (au.ui.low > bu.ui.low) >> - return 2; >> - return 1; >> -} > This should be removed and libgcc routine used. I fully agree with you! The problem is though, that the current kernel is buggy and we need to solve it ASAP. The question is, when a fixed libgcc will become available...? Helge
On 2021-10-12 8:43 a.m., Helge Deller wrote: > I fully agree with you! > The problem is though, that the current kernel is buggy and we > need to solve it ASAP. > The question is, when a fixed libgcc will become available...? I will do an updated patch today and install it after testing. It will take some time to get into Debian, etc. Once that is done, we could generate assembly code for __muldi3, __divdi3, etc. That would remove the kernel dependence. But I think it would still be better to use the libgcc implementations. The compiler might get better, etc. Dave
On 2021-10-12 10:14 a.m., John David Anglin wrote: > On 2021-10-12 8:43 a.m., Helge Deller wrote: >> I fully agree with you! >> The problem is though, that the current kernel is buggy and we >> need to solve it ASAP. >> The question is, when a fixed libgcc will become available...? > I will do an updated patch today and install it after testing. It will take some time to get > into Debian, etc. Bah, -msoft-float and -mdisable-fpregs are broken and libgcc won't build with these options. Dave
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig index fc17285f4da1..e412d5c6c64f 100644 --- a/arch/parisc/Kconfig +++ b/arch/parisc/Kconfig @@ -36,6 +36,12 @@ config PARISC select GENERIC_SMP_IDLE_THREAD select GENERIC_CPU_DEVICES select GENERIC_LIB_DEVMEM_IS_ALLOWED + select GENERIC_LIB_ASHLDI3 + select GENERIC_LIB_ASHRDI3 + select GENERIC_LIB_LSHRDI3 + select GENERIC_LIB_MULDI3 + select GENERIC_LIB_CMPDI2 + select GENERIC_LIB_UCMPDI2 select SYSCTL_ARCH_UNALIGN_ALLOW select SYSCTL_EXCEPTION_TRACE select HAVE_MOD_ARCH_SPECIFIC diff --git a/arch/parisc/kernel/parisc_ksyms.c b/arch/parisc/kernel/parisc_ksyms.c index 00297e8e1c88..da8c99c5a8c6 100644 --- a/arch/parisc/kernel/parisc_ksyms.c +++ b/arch/parisc/kernel/parisc_ksyms.c @@ -92,18 +92,6 @@ EXPORT_SYMBOL($$divI_12); EXPORT_SYMBOL($$divI_14); EXPORT_SYMBOL($$divI_15); -extern void __ashrdi3(void); -extern void __ashldi3(void); -extern void __lshrdi3(void); -extern void __muldi3(void); -extern void __ucmpdi2(void); - -EXPORT_SYMBOL(__ashrdi3); -EXPORT_SYMBOL(__ashldi3); -EXPORT_SYMBOL(__lshrdi3); -EXPORT_SYMBOL(__muldi3); -EXPORT_SYMBOL(__ucmpdi2); - asmlinkage void * __canonicalize_funcptr_for_compare(void *); EXPORT_SYMBOL(__canonicalize_funcptr_for_compare); diff --git a/arch/parisc/lib/Makefile b/arch/parisc/lib/Makefile index 7b197667faf6..2bfafb3c9ae0 100644 --- a/arch/parisc/lib/Makefile +++ b/arch/parisc/lib/Makefile @@ -4,6 +4,6 @@ # lib-y := lusercopy.o bitops.o checksum.o io.o memset.o memcpy.o \ - ucmpdi2.o delay.o + delay.o obj-y := iomap.o diff --git a/arch/parisc/lib/ucmpdi2.c b/arch/parisc/lib/ucmpdi2.c deleted file mode 100644 index 8e6014a142ef..000000000000 --- a/arch/parisc/lib/ucmpdi2.c +++ /dev/null @@ -1,26 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include <linux/module.h> - -union ull_union { - unsigned long long ull; - struct { - unsigned int high; - unsigned int low; - } ui; -}; - -int __ucmpdi2(unsigned long long a, unsigned long long b) -{ - union ull_union au = {.ull = a}; - union ull_union bu = {.ull = b}; - - if (au.ui.high < bu.ui.high) - return 0; - else if (au.ui.high > bu.ui.high) - return 2; - if (au.ui.low < bu.ui.low) - return 0; - else if (au.ui.low > bu.ui.low) - return 2; - return 1; -}
Dave noticed that the libgcc.a which is linked into the kernel binary uses the xmpyu assembler statement. This asm statement uses floating point registers which are forbidden in the kernel, because we don't save the FP regs at kernel entry. Switch the parisc kernel to use the already existing shared copies of some GCC library routines instead of linking in those from libgcc. This patch avoids all uses of xmpyu in the 32-bit kernel, but the 64-bit kernel still needs fixing. Reported-by: John David Anglin <dave.anglin@bell.net> Signed-off-by: Helge Deller <deller@gmx.de>