qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 4/8] user: Introduce host_interrupt_signal


From: Warner Losh
Subject: Re: [PATCH 4/8] user: Introduce host_interrupt_signal
Date: Tue, 5 Nov 2024 08:39:43 -0700



On Thu, Oct 24, 2024 at 2:00 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
Attaching to the gdbstub of a running process requires stopping its
threads. For threads that run on a CPU, cpu_exit() is enough, but the
only way to grab attention of a thread that is stuck in a long-running
syscall is to interrupt it with a signal.

Reserve a host realtime signal for this, just like it's already done
for TARGET_SIGABRT on Linux. This may reduce the number of available
guest realtime signals by one, but this is acceptable, since there are
quite a lot of them, and it's unlikely that there are apps that need
them all.

Set signal_pending for the safe_sycall machinery to prevent invoking
the syscall. This is a lie, since we don't queue a guest signal, but
process_pending_signals() can handle the absence of pending signals.
The syscall returns with QEMU_ERESTARTSYS errno, which arranges for
the automatic restart. This is important, because it helps avoiding
disturbing poorly written guests.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 bsd-user/signal.c     | 12 ++++++++++++
 include/user/signal.h |  2 ++
 linux-user/signal.c   | 11 +++++++++++
 3 files changed, 25 insertions(+)

diff --git a/bsd-user/signal.c b/bsd-user/signal.c
index a2b11a97131..992736df5c5 100644
--- a/bsd-user/signal.c
+++ b/bsd-user/signal.c
@@ -49,6 +49,8 @@ static inline int sas_ss_flags(TaskState *ts, unsigned long sp)
         on_sig_stack(ts, sp) ? SS_ONSTACK : 0;
 }

+int host_interrupt_signal = SIGRTMAX;
+

I'd be tempted to use SIGRTMAX + 1 or even TARGET_NSIG. 127 or 128 would
work and not overflow any arrays (or hit any bounds tests) I'd likely use SIGRTMAX + 1,
though, since it avoids any edge-cases from sig == NSIG that might be in the code
unnoticed.

Now, having said that, I don't think that there's too many (any?) programs we need
to run as bsd-user that have real-time signals, much less one that uses SIGRTMAX,
but stranger things have happened. But it is a little wiggle room just in case.

Other than that:

Reviewed-by: Warner Losh <imp@bsdimp.com>

 
 /*
  * The BSD ABIs use the same signal numbers across all the CPU architectures, so
  * (unlike Linux) these functions are just the identity mapping. This might not
@@ -489,6 +491,12 @@ static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
     uintptr_t pc = 0;
     bool sync_sig = false;

+    if (host_sig == host_interrupt_signal) {
+        ts->signal_pending = 1;
+        cpu_exit(thread_cpu);
+        return;
+    }
+
     /*
      * Non-spoofed SIGSEGV and SIGBUS are synchronous, and need special
      * handling wrt signal blocking and unwinding.
@@ -852,6 +860,9 @@ void signal_init(void)

     for (i = 1; i <= TARGET_NSIG; i++) {
         host_sig = target_to_host_signal(i);
+        if (host_sig == host_interrupt_signal) {
+            continue;
+        }
         sigaction(host_sig, NULL, &oact);
         if (oact.sa_sigaction == (void *)SIG_IGN) {
             sigact_table[i - 1]._sa_handler = TARGET_SIG_IGN;
@@ -870,6 +881,7 @@ void signal_init(void)
             sigaction(host_sig, &act, NULL);
         }
     }
+    sigaction(host_interrupt_signal, &act, NULL);
 }

 static void handle_pending_signal(CPUArchState *env, int sig,
diff --git a/include/user/signal.h b/include/user/signal.h
index 19b6b9e5ddc..7fa33b05d91 100644
--- a/include/user/signal.h
+++ b/include/user/signal.h
@@ -20,4 +20,6 @@
  */
 int target_to_host_signal(int sig);

+extern int host_interrupt_signal;
+
 #endif
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 84bb8a34808..f0bcbd367d5 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -514,6 +514,8 @@ static int core_dump_signal(int sig)
     }
 }

+int host_interrupt_signal;
+
 static void signal_table_init(void)
 {
     int hsig, tsig, count;
@@ -540,6 +542,7 @@ static void signal_table_init(void)
     hsig = SIGRTMIN;
     host_to_target_signal_table[SIGABRT] = 0;
     host_to_target_signal_table[hsig++] = TARGET_SIGABRT;
+    host_interrupt_signal = hsig++;

     for (tsig = TARGET_SIGRTMIN;
          hsig <= SIGRTMAX && tsig <= TARGET_NSIG;
@@ -619,6 +622,8 @@ void signal_init(void)
         }
         sigact_table[tsig - 1]._sa_handler = thand;
     }
+
+    sigaction(host_interrupt_signal, &act, NULL);
 }

 /* Force a synchronously taken signal. The kernel force_sig() function
@@ -966,6 +971,12 @@ static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
     bool sync_sig = false;
     void *sigmask;

+    if (host_sig == host_interrupt_signal) {
+        ts->signal_pending = 1;
+        cpu_exit(thread_cpu);
+        return;
+    }
+
     /*
      * Non-spoofed SIGSEGV and SIGBUS are synchronous, and need special
      * handling wrt signal blocking and unwinding.  Non-spoofed SIGILL,
--
2.47.0


reply via email to

[Prev in Thread] Current Thread [Next in Thread]