| Message ID | 20230316082007.652669-1-ardb@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | ARM: vfp: Switch to C API to en/disable softirqs | expand |
On 3/16/23 01:20, Ard Biesheuvel wrote: > As it turns out, enabling or disabling softirqs from asm code is more > tricky than it looks. Simply bumping the associated bit in preempt_count > does the trick for uninstrumented kernels, but with lockdep or preempt > debug enabled, we really need to call the C versions, as replicating > their behavior in asm fully is intractible. > > So let's rework the existing code a little bit so we can interpose a > little C helper 'vfp_entry()' that disables softirqs before calling the > existing vfpstate handling code in asm. Re-enabling softirqs from asm > code is more straight-forward, as we can simply perform a tail call via > a C routine that is guaranteed to be callable as a function. > > However, since calling these APIs from asm code is still not ideal, > let's reimplement the whole thing in C (patch #4) > > > Changes since v2: > - add Rbs and Tbs from Linus and Guenter (thanks!) > - correct reference to local_bh_enable() vs __local_bh_enable_ip() in > commit log of patch #3 > - add patch that reworks the asm VFP exception handling entirely so the > bulk of it is implemented in C. This could be taken into v6.4, while > the preceding patches are fixes for v6.3-rc > > Cc: Frederic Weisbecker <frederic@kernel.org> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Link: https://lore.kernel.org/all/ZBBYCSZUJOWBg1s8@localhost.localdomain/ > > Ard Biesheuvel (4): > ARM: vfp: Pass thread_info pointer to vfp_support_entry > ARM: vfp: Pass successful return address via register R3 > ARM: vfp: Fix broken softirq handling with instrumentation enabled > ARM: vfp: Reimplement VFP exception entry in C code > > arch/arm/include/asm/assembler.h | 13 -- > arch/arm/kernel/entry-armv.S | 12 ++ > arch/arm/vfp/Makefile | 2 +- > arch/arm/vfp/entry.S | 39 ---- > arch/arm/vfp/vfp.h | 1 + > arch/arm/vfp/vfphw.S | 198 ++------------------ > arch/arm/vfp/vfpmodule.c | 125 +++++++++++- > 7 files changed, 142 insertions(+), 248 deletions(-) > delete mode 100644 arch/arm/vfp/entry.S > After applying this series, imx_v4_v5_defconfig and aspeed_g4_defconfig (and maybe others) fail to build: arm-linux-gnueabi-ld: arch/arm/kernel/entry-armv.o: in function `do_vfp': (.entry.text+0x5a4): undefined reference to `vfp_entry' make[1]: *** [scripts/Makefile.vmlinux:35: vmlinux] Error 1 make: *** [Makefile:1250: vmlinux] Error 2 Guenter
On 3/16/23 17:23, Guenter Roeck wrote: > On 3/16/23 01:20, Ard Biesheuvel wrote: >> As it turns out, enabling or disabling softirqs from asm code is more >> tricky than it looks. Simply bumping the associated bit in preempt_count >> does the trick for uninstrumented kernels, but with lockdep or preempt >> debug enabled, we really need to call the C versions, as replicating >> their behavior in asm fully is intractible. >> >> So let's rework the existing code a little bit so we can interpose a >> little C helper 'vfp_entry()' that disables softirqs before calling the >> existing vfpstate handling code in asm. Re-enabling softirqs from asm >> code is more straight-forward, as we can simply perform a tail call via >> a C routine that is guaranteed to be callable as a function. >> >> However, since calling these APIs from asm code is still not ideal, >> let's reimplement the whole thing in C (patch #4) >> >> >> Changes since v2: >> - add Rbs and Tbs from Linus and Guenter (thanks!) >> - correct reference to local_bh_enable() vs __local_bh_enable_ip() in >> commit log of patch #3 >> - add patch that reworks the asm VFP exception handling entirely so the >> bulk of it is implemented in C. This could be taken into v6.4, while >> the preceding patches are fixes for v6.3-rc >> >> Cc: Frederic Weisbecker <frederic@kernel.org> >> Cc: Guenter Roeck <linux@roeck-us.net> >> Cc: Peter Zijlstra <peterz@infradead.org> >> Cc: Linus Walleij <linus.walleij@linaro.org> >> Cc: Arnd Bergmann <arnd@arndb.de> >> Link: https://lore.kernel.org/all/ZBBYCSZUJOWBg1s8@localhost.localdomain/ >> >> Ard Biesheuvel (4): >> ARM: vfp: Pass thread_info pointer to vfp_support_entry >> ARM: vfp: Pass successful return address via register R3 >> ARM: vfp: Fix broken softirq handling with instrumentation enabled >> ARM: vfp: Reimplement VFP exception entry in C code >> >> arch/arm/include/asm/assembler.h | 13 -- >> arch/arm/kernel/entry-armv.S | 12 ++ >> arch/arm/vfp/Makefile | 2 +- >> arch/arm/vfp/entry.S | 39 ---- >> arch/arm/vfp/vfp.h | 1 + >> arch/arm/vfp/vfphw.S | 198 ++------------------ >> arch/arm/vfp/vfpmodule.c | 125 +++++++++++- >> 7 files changed, 142 insertions(+), 248 deletions(-) >> delete mode 100644 arch/arm/vfp/entry.S >> > > After applying this series, imx_v4_v5_defconfig and aspeed_g4_defconfig > (and maybe others) fail to build: > > arm-linux-gnueabi-ld: arch/arm/kernel/entry-armv.o: in function `do_vfp': > (.entry.text+0x5a4): undefined reference to `vfp_entry' > make[1]: *** [scripts/Makefile.vmlinux:35: vmlinux] Error 1 > make: *** [Makefile:1250: vmlinux] Error 2 > ... and reverting the last patch of the series fixes the problem. Guenter
On Fri, 17 Mar 2023 at 01:24, Guenter Roeck <linux@roeck-us.net> wrote: > > On 3/16/23 17:23, Guenter Roeck wrote: > > On 3/16/23 01:20, Ard Biesheuvel wrote: > >> As it turns out, enabling or disabling softirqs from asm code is more > >> tricky than it looks. Simply bumping the associated bit in preempt_count > >> does the trick for uninstrumented kernels, but with lockdep or preempt > >> debug enabled, we really need to call the C versions, as replicating > >> their behavior in asm fully is intractible. > >> > >> So let's rework the existing code a little bit so we can interpose a > >> little C helper 'vfp_entry()' that disables softirqs before calling the > >> existing vfpstate handling code in asm. Re-enabling softirqs from asm > >> code is more straight-forward, as we can simply perform a tail call via > >> a C routine that is guaranteed to be callable as a function. > >> > >> However, since calling these APIs from asm code is still not ideal, > >> let's reimplement the whole thing in C (patch #4) > >> > >> > >> Changes since v2: > >> - add Rbs and Tbs from Linus and Guenter (thanks!) > >> - correct reference to local_bh_enable() vs __local_bh_enable_ip() in > >> commit log of patch #3 > >> - add patch that reworks the asm VFP exception handling entirely so the > >> bulk of it is implemented in C. This could be taken into v6.4, while > >> the preceding patches are fixes for v6.3-rc > >> > >> Cc: Frederic Weisbecker <frederic@kernel.org> > >> Cc: Guenter Roeck <linux@roeck-us.net> > >> Cc: Peter Zijlstra <peterz@infradead.org> > >> Cc: Linus Walleij <linus.walleij@linaro.org> > >> Cc: Arnd Bergmann <arnd@arndb.de> > >> Link: https://lore.kernel.org/all/ZBBYCSZUJOWBg1s8@localhost.localdomain/ > >> > >> Ard Biesheuvel (4): > >> ARM: vfp: Pass thread_info pointer to vfp_support_entry > >> ARM: vfp: Pass successful return address via register R3 > >> ARM: vfp: Fix broken softirq handling with instrumentation enabled > >> ARM: vfp: Reimplement VFP exception entry in C code > >> > >> arch/arm/include/asm/assembler.h | 13 -- > >> arch/arm/kernel/entry-armv.S | 12 ++ > >> arch/arm/vfp/Makefile | 2 +- > >> arch/arm/vfp/entry.S | 39 ---- > >> arch/arm/vfp/vfp.h | 1 + > >> arch/arm/vfp/vfphw.S | 198 ++------------------ > >> arch/arm/vfp/vfpmodule.c | 125 +++++++++++- > >> 7 files changed, 142 insertions(+), 248 deletions(-) > >> delete mode 100644 arch/arm/vfp/entry.S > >> > > > > After applying this series, imx_v4_v5_defconfig and aspeed_g4_defconfig > > (and maybe others) fail to build: > > > > arm-linux-gnueabi-ld: arch/arm/kernel/entry-armv.o: in function `do_vfp': > > (.entry.text+0x5a4): undefined reference to `vfp_entry' > > make[1]: *** [scripts/Makefile.vmlinux:35: vmlinux] Error 1 > > make: *** [Makefile:1250: vmlinux] Error 2 > > > > ... and reverting the last patch of the series fixes the problem. > Thanks Guenter, It was a missing #ifdef CONFIG_VFP - I'll fix that up before I drop it into the patch system.