| Message ID | 20190116131527.2071570-1-arnd@arndb.de |
|---|---|
| Headers | show |
| Series | s390: rework compat wrapper generation | expand |
On Wed, Jan 16, 2019 at 02:15:18PM +0100, Arnd Bergmann wrote: > Hi Heiko and Martin, > > As promised, I gave this a go and changed the SYSCALL_DEFINEx() > infrastructure to always include the wrappers for doing the > 31-bit argument conversion on s390 compat mode. > > This does three main things: > > - The UID16 rework saved a lot of duplicated code, and would > probably make sense by itself, but is also required as > we can no longer call sys_*() functions directly after the > last step. > > - Removing the compat_wrapper.c file is of course the main > goal here, in order to remove the need to maintain the > compat_wrapper.c file when new system calls get added. > Unfortunately, this requires adding some complexity in > syscall_wrapper.h, and trades a small reduction in source > code lines for a small increase in binary size for > unused wrappers. > > - As an added benefit, the use of syscall_wrapper.h now makes > it easy to change the syscall wrappers so they no longer > see all user space register contents, similar to changes > done in commits fa697140f9a2 ("syscalls/x86: Use 'struct pt_regs' > based syscall calling convention for 64-bit syscalls") and > 4378a7d4be30 ("arm64: implement syscall wrappers"). > I leave the actual implementation of this for you, if you > want to do it later. > > I did not test the changes at runtime, but I looked at the > generated object code, which seems fine here and includes > the same conversions as before. This work is highly appreciated! I'll look into this tomorrow. Thanks!
On Wed, Jan 16, 2019 at 02:15:18PM +0100, Arnd Bergmann wrote: > Hi Heiko and Martin, > > As promised, I gave this a go and changed the SYSCALL_DEFINEx() > infrastructure to always include the wrappers for doing the > 31-bit argument conversion on s390 compat mode. > > This does three main things: > > - The UID16 rework saved a lot of duplicated code, and would > probably make sense by itself, but is also required as > we can no longer call sys_*() functions directly after the > last step. > > - Removing the compat_wrapper.c file is of course the main > goal here, in order to remove the need to maintain the > compat_wrapper.c file when new system calls get added. > Unfortunately, this requires adding some complexity in > syscall_wrapper.h, and trades a small reduction in source > code lines for a small increase in binary size for > unused wrappers. > > - As an added benefit, the use of syscall_wrapper.h now makes > it easy to change the syscall wrappers so they no longer > see all user space register contents, similar to changes > done in commits fa697140f9a2 ("syscalls/x86: Use 'struct pt_regs' > based syscall calling convention for 64-bit syscalls") and > 4378a7d4be30 ("arm64: implement syscall wrappers"). > I leave the actual implementation of this for you, if you > want to do it later. > > I did not test the changes at runtime, but I looked at the > generated object code, which seems fine here and includes > the same conversions as before. All looks good and seems to work fine. This is a very nice simplification of our compat code, even if it adds some dead code to the kernel image. I did some tests and it all looks good. Also the generated code looks fine. So, if nothing breaks, this will go upstream with next merge window via the s390 tree. Thanks again for your work!
On Thu, Jan 17, 2019 at 2:36 PM Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > > On Wed, Jan 16, 2019 at 02:15:18PM +0100, Arnd Bergmann wrote: > > I did not test the changes at runtime, but I looked at the > > generated object code, which seems fine here and includes > > the same conversions as before. > > All looks good and seems to work fine. This is a very nice > simplification of our compat code, even if it adds some dead code to > the kernel image. > > I did some tests and it all looks good. Also the generated code looks > fine. So, if nothing breaks, this will go upstream with next merge > window via the s390 tree. > > Thanks again for your work! Awesome, thanks for testing it so quickly and agreeing to merge it! There is a dependency that I now have for my y2038 syscall series of course, so I'd need to have those patches on top of the s390 series. I think we can either have a shared git branch that gets merged both into your s390 tree and my y2038 tree, or we merge it only through my tree, with your Ack. Does that work for you? Arnd
On Thu, Jan 17, 2019 at 05:21:50PM +0100, Arnd Bergmann wrote: > On Thu, Jan 17, 2019 at 2:36 PM Heiko Carstens > <heiko.carstens@de.ibm.com> wrote: > > > > On Wed, Jan 16, 2019 at 02:15:18PM +0100, Arnd Bergmann wrote: > > > > I did not test the changes at runtime, but I looked at the > > > generated object code, which seems fine here and includes > > > the same conversions as before. > > > > All looks good and seems to work fine. This is a very nice > > simplification of our compat code, even if it adds some dead code to > > the kernel image. > > > > I did some tests and it all looks good. Also the generated code looks > > fine. So, if nothing breaks, this will go upstream with next merge > > window via the s390 tree. > > > > Thanks again for your work! > > Awesome, thanks for testing it so quickly and agreeing to merge it! > > There is a dependency that I now have for my y2038 syscall series > of course, so I'd need to have those patches on top of the s390 series. > > I think we can either have a shared git branch that gets merged both > into your s390 tree and my y2038 tree, or we merge it only through > my tree, with your Ack. > > Does that work for you? I'll discuss this with Martin tomorrow, however I think a shared git branch would be the best solution. I want to get as much testing as possible for this patch set, which means this must also be in the s390 tree.
On Thu, 17 Jan 2019 21:19:20 +0100 Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > On Thu, Jan 17, 2019 at 05:21:50PM +0100, Arnd Bergmann wrote: > > On Thu, Jan 17, 2019 at 2:36 PM Heiko Carstens > > <heiko.carstens@de.ibm.com> wrote: > > > > > > On Wed, Jan 16, 2019 at 02:15:18PM +0100, Arnd Bergmann wrote: > > > > > > I did not test the changes at runtime, but I looked at the > > > > generated object code, which seems fine here and includes > > > > the same conversions as before. > > > > > > All looks good and seems to work fine. This is a very nice > > > simplification of our compat code, even if it adds some dead code to > > > the kernel image. > > > > > > I did some tests and it all looks good. Also the generated code looks > > > fine. So, if nothing breaks, this will go upstream with next merge > > > window via the s390 tree. > > > > > > Thanks again for your work! > > > > Awesome, thanks for testing it so quickly and agreeing to merge it! > > > > There is a dependency that I now have for my y2038 syscall series > > of course, so I'd need to have those patches on top of the s390 series. > > > > I think we can either have a shared git branch that gets merged both > > into your s390 tree and my y2038 tree, or we merge it only through > > my tree, with your Ack. > > > > Does that work for you? > > I'll discuss this with Martin tomorrow, however I think a shared git > branch would be the best solution. I want to get as much testing as > possible for this patch set, which means this must also be in the s390 > tree. Yes, a shared git branch seems like the best solutoin, this is now available as a topic branch on the s390/linux: git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git compat The branch has been merged to the features branch for v5.1. Thanks for the work on the compat code, good stuff! -- blue skies, Martin. "Reality continues to ruin my life." - Calvin.
Hi Heiko and Martin, As promised, I gave this a go and changed the SYSCALL_DEFINEx() infrastructure to always include the wrappers for doing the 31-bit argument conversion on s390 compat mode. This does three main things: - The UID16 rework saved a lot of duplicated code, and would probably make sense by itself, but is also required as we can no longer call sys_*() functions directly after the last step. - Removing the compat_wrapper.c file is of course the main goal here, in order to remove the need to maintain the compat_wrapper.c file when new system calls get added. Unfortunately, this requires adding some complexity in syscall_wrapper.h, and trades a small reduction in source code lines for a small increase in binary size for unused wrappers. - As an added benefit, the use of syscall_wrapper.h now makes it easy to change the syscall wrappers so they no longer see all user space register contents, similar to changes done in commits fa697140f9a2 ("syscalls/x86: Use 'struct pt_regs' based syscall calling convention for 64-bit syscalls") and 4378a7d4be30 ("arm64: implement syscall wrappers"). I leave the actual implementation of this for you, if you want to do it later. I did not test the changes at runtime, but I looked at the generated object code, which seems fine here and includes the same conversions as before. Arnd Arnd Bergmann (5): s390: open-code s390_personality syscall ipc: introduce ksys_ipc()/compat_ksys_ipc() for s390 s390: use generic UID16 implementation s390: autogenerate compat syscall wrappers s390: remove compat_wrapper.c arch/s390/Kconfig | 2 + arch/s390/include/asm/syscall_wrapper.h | 136 +++++++++++ arch/s390/include/uapi/asm/posix_types.h | 6 + arch/s390/kernel/Makefile | 2 +- arch/s390/kernel/compat_linux.c | 235 +------------------ arch/s390/kernel/compat_wrapper.c | 186 --------------- arch/s390/kernel/entry.S | 4 +- arch/s390/kernel/sys_s390.c | 14 +- arch/s390/kernel/syscalls/syscall.tbl | 286 +++++++++++------------ include/linux/syscalls.h | 4 + ipc/syscall.c | 20 +- 11 files changed, 322 insertions(+), 573 deletions(-) create mode 100644 arch/s390/include/asm/syscall_wrapper.h delete mode 100644 arch/s390/kernel/compat_wrapper.c -- 2.20.0