[RFC v3 0/5] linux-user: simplify safe signal handling

* [RFC v3 0/5] linux-user: simplify safe signal handling
@ 2021-11-13  4:55 Warner Losh
  2021-11-13  4:55 ` [RFC v3 1/5] linux-user: Add host_signal_set_pc to set pc in mcontext Warner Losh
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Warner Losh @ 2021-11-13  4:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Philippe Mathieu-Daude, Laurent Vivier,
	Konrad Witaszczyk, Warner Losh

This is a quick RFC to see if something like this is worth doing.

I've created a new interface host_signal_set_pc. This allows us to move all the
nearly identical copies of rewind_if_in_safe_syscall into signal.c.  This
reduces the amount of code that needs to be rewritten for bsd-user's adaptation
of both the safe signal handling and the sigsegv/sigbus changes that have
happened. Since BSD's mcontext_t differs in some cases, we wouldn't be able to
share this between platforms, but it reduces the number of nearly identical
routines I'd have to write.

In addition, the assembler glue for the safe system calls is almost identical
between linux and bsd-user's fork. The only difference is inverting the system
call return to comply with the -ERRNO convention *-user uses in the rest of the
code which is native to Linux, but differs for the BSDs and other traditional
unix targets.

I know the patches may not be sliced and diced in the typical desired
fashion. This is a RFC, and the changes are short enough to be easily digested
though since it's quite repetitive. They do now pass a push to gitlab and
the default CI (see note in v2 section about one ugly kludge that likely
needs discussion).

These were extracted from the 'blitz' branch we have in the bsd-user fork and
then that was adapted to use the common code. I've pushed a branch to gitlab
(viewable at https://gitlab.com/bsdimp/qemu/-/tree/blitz if you prefer that to
fetching) that shows how these will be used. I'm working on upstreaming
bsd-user/signal.c next which will take a little bit of time to work into a place
where it can be reviewed here. I wanted to get feedback because this is
one chunk I can cleave off and make landing that easier.

v3:	o Make arm and aarch64 fixes as suggested in the review
	o Fix a stray & that remained after some churn for 32-bit sparc,
	  clearly not compiled in our CI pipeline...
	o Fix the comments to be more descriptive as to the errno convetion
	  and not characterize it as the Linux way.

v2:
	o move the externs for the system call setup to safe-syscall.h
	o move to using the #ifdef __FreeBSD__ code for FreeBSD's adjustment
	  to return value from system calls.
	o move safe-syscall.inc to common-user so bsd-user can use it too
	o create a kludge for mips to allow CI to pass (but maybe we should
	  remove mips hosts as a supported platform instead)
	o side note: the blitz bsd-user branch hasn't been updated yet since
	  I think the first two of this series may be merged early to solve
	  a different problem.

Warner Losh (5):
  linux-user: Add host_signal_set_pc to set pc in mcontext
  linux-user/signal.c: Create a common rewind_if_in_safe_syscall
  linux-user/safe-syscall.inc.S: Move to common-user
  common-user: Adjust system call return on FreeBSD
  *-user: move safe-syscall.* to common-user

 common-user/common-safe-syscall.S             | 30 ++++++++++++++++++
 .../host/aarch64/safe-syscall.inc.S           |  8 +++++
 .../host/arm/safe-syscall.inc.S               |  7 +++++
 .../host/i386/safe-syscall.inc.S              |  9 ++++++
 .../host/ppc64/safe-syscall.inc.S             |  0
 .../host/riscv/safe-syscall.inc.S             |  0
 .../host/s390x/safe-syscall.inc.S             |  0
 .../host/x86_64/safe-syscall.inc.S            |  9 ++++++
 {linux-user => common-user}/safe-syscall.h    |  3 ++
 linux-user/host/aarch64/host-signal.h         |  5 +++
 linux-user/host/aarch64/hostdep.h             | 20 ------------
 linux-user/host/alpha/host-signal.h           |  5 +++
 linux-user/host/arm/host-signal.h             |  5 +++
 linux-user/host/arm/hostdep.h                 | 20 ------------
 linux-user/host/i386/host-signal.h            |  5 +++
 linux-user/host/i386/hostdep.h                | 20 ------------
 linux-user/host/mips/host-signal.h            |  5 +++
 linux-user/host/ppc/host-signal.h             |  5 +++
 linux-user/host/ppc64/hostdep.h               | 20 ------------
 linux-user/host/riscv/host-signal.h           |  5 +++
 linux-user/host/riscv/hostdep.h               | 20 ------------
 linux-user/host/s390/host-signal.h            |  5 +++
 linux-user/host/s390x/hostdep.h               | 20 ------------
 linux-user/host/sparc/host-signal.h           |  9 ++++++
 linux-user/host/x86_64/host-signal.h          |  5 +++
 linux-user/host/x86_64/hostdep.h              | 20 ------------
 linux-user/safe-syscall.S                     | 31 +------------------
 linux-user/signal.c                           | 15 ++++++++-
 meson.build                                   |  2 ++
 29 files changed, 137 insertions(+), 171 deletions(-)
 create mode 100644 common-user/common-safe-syscall.S
 rename {linux-user => common-user}/host/aarch64/safe-syscall.inc.S (92%)
 rename {linux-user => common-user}/host/arm/safe-syscall.inc.S (93%)
 rename {linux-user => common-user}/host/i386/safe-syscall.inc.S (93%)
 rename {linux-user => common-user}/host/ppc64/safe-syscall.inc.S (100%)
 rename {linux-user => common-user}/host/riscv/safe-syscall.inc.S (100%)
 rename {linux-user => common-user}/host/s390x/safe-syscall.inc.S (100%)
 rename {linux-user => common-user}/host/x86_64/safe-syscall.inc.S (94%)
 rename {linux-user => common-user}/safe-syscall.h (98%)

-- 
2.33.0

^ permalink raw reply	[flat|nested] 7+ messages in thread