diff mbox series

[v4,25/26] gdbstub: split out softmmu/user specifics for syscall handling

Message ID 20230302190846.2593720-26-alex.bennee@linaro.org
State New
Headers show
Series gdbstub/next: re-organise and split build | expand

Commit Message

Alex Bennée March 2, 2023, 7:08 p.m. UTC
Most of the syscall code is config agnostic aside from the size of
target_ulong. In preparation for the next patch move the final bits of
specialisation into the appropriate user and softmmu helpers.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v4
  - checkpatch cleanups
---
 gdbstub/internals.h |  5 +++++
 gdbstub/softmmu.c   | 24 ++++++++++++++++++++++++
 gdbstub/syscalls.c  | 35 +++++++++++------------------------
 gdbstub/user.c      | 24 ++++++++++++++++++++++++
 4 files changed, 64 insertions(+), 24 deletions(-)

Comments

Richard Henderson March 2, 2023, 10:21 p.m. UTC | #1
On 3/2/23 09:08, Alex Bennée wrote:
> @@ -104,9 +104,10 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
>       }
>   
>       gdbserver_syscall_state.current_syscall_cb = cb;
> -#ifndef CONFIG_USER_ONLY
> -    vm_stop(RUN_STATE_DEBUG);
> -#endif
> +
> +    /* user/softmmu specific handling */
> +    gdb_pre_syscall_handling();

I think this placement of vm_stop is inconvenient, and that we don't need to continue.  If 
we move it down below the construction of gdbserver_syscall_state.syscall_buf...

>       p = &gdbserver_syscall_state.syscall_buf[0];
>       p_end = &gdbserver_syscall_state.syscall_buf[sizeof(gdbserver_syscall_state.syscall_buf)];
>       *(p++) = 'F';
> @@ -141,27 +142,13 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
>           }
>       }
>       *p = 0;
> -#ifdef CONFIG_USER_ONLY
> -    gdb_put_packet(gdbserver_syscall_state.syscall_buf);
> -    /*
> -     * Return control to gdb for it to process the syscall request.
> -     * Since the protocol requires that gdb hands control back to us
> -     * using a "here are the results" F packet, we don't need to check
> -     * gdb_handlesig's return value (which is the signal to deliver if
> -     * execution was resumed via a continue packet).
> -     */
> -    gdb_handlesig(gdbserver_state.c_cpu, 0);
> -#else
> -    /*
> -     * In this case wait to send the syscall packet until notification that
> -     * the CPU has stopped.  This must be done because if the packet is sent
> -     * now the reply from the syscall request could be received while the CPU
> -     * is still in the running state, which can cause packets to be dropped
> -     * and state transition 'T' packets to be sent while the syscall is still
> -     * being processed.
> -     */
> -    qemu_cpu_kick(gdbserver_state.c_cpu);
> -#endif
> +
> +    if (gdb_send_syscall_now()) { /* true only for *-user */
> +        gdb_put_packet(gdbserver_syscall_state.syscall_buf);
> +    }
> +
> +    /* user/softmmu specific handling */
> +    gdb_post_syscall_handling();

... then we don't need 3 separate hooks for user/softmmu.

softmmu:

void gdb_syscall_handling(const char *syscall_buf)
{
     vm_stop(RUN_STATE_DEBUG);
     qemu_cpu_kick(gdbserver_state.c_cpu);
}

user:

void gdb_syscall_handling(const char *syscall_buf)
{
     gdb_put_packet(syscall_buf);
     gdb_handlesig(gdbserver_state.c_cpu, 0);
}


r~
diff mbox series

Patch

diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 8db61f7fb4..65d75d9435 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -195,6 +195,11 @@  bool gdb_handled_syscall(void);
 void gdb_disable_syscalls(void);
 void gdb_syscall_reset(void);
 
+/* user/softmmu specific signal handling */
+void gdb_pre_syscall_handling(void);
+bool gdb_send_syscall_now(void);
+void gdb_post_syscall_handling(void);
+
 /*
  * Break/Watch point support - there is an implementation for softmmu
  * and user mode.
diff --git a/gdbstub/softmmu.c b/gdbstub/softmmu.c
index d3152fb6e7..02f3b41095 100644
--- a/gdbstub/softmmu.c
+++ b/gdbstub/softmmu.c
@@ -103,6 +103,30 @@  static void gdb_chr_event(void *opaque, QEMUChrEvent event)
     }
 }
 
+/*
+ * In softmmu mode we stop the VM and wait to send the syscall packet
+ * until notification that the CPU has stopped. This must be done
+ * because if the packet is sent now the reply from the syscall
+ * request could be received while the CPU is still in the running
+ * state, which can cause packets to be dropped and state transition
+ * 'T' packets to be sent while the syscall is still being processed.
+ */
+
+void gdb_pre_syscall_handling(void)
+{
+    vm_stop(RUN_STATE_DEBUG);
+}
+
+bool gdb_send_syscall_now(void)
+{
+    return false;
+}
+
+void gdb_post_syscall_handling(void)
+{
+    qemu_cpu_kick(gdbserver_state.c_cpu);
+}
+
 static void gdb_vm_state_change(void *opaque, bool running, RunState state)
 {
     CPUState *cpu = gdbserver_state.c_cpu;
diff --git a/gdbstub/syscalls.c b/gdbstub/syscalls.c
index f15b210958..1ca3d81305 100644
--- a/gdbstub/syscalls.c
+++ b/gdbstub/syscalls.c
@@ -104,9 +104,10 @@  void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
     }
 
     gdbserver_syscall_state.current_syscall_cb = cb;
-#ifndef CONFIG_USER_ONLY
-    vm_stop(RUN_STATE_DEBUG);
-#endif
+
+    /* user/softmmu specific handling */
+    gdb_pre_syscall_handling();
+
     p = &gdbserver_syscall_state.syscall_buf[0];
     p_end = &gdbserver_syscall_state.syscall_buf[sizeof(gdbserver_syscall_state.syscall_buf)];
     *(p++) = 'F';
@@ -141,27 +142,13 @@  void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
         }
     }
     *p = 0;
-#ifdef CONFIG_USER_ONLY
-    gdb_put_packet(gdbserver_syscall_state.syscall_buf);
-    /*
-     * Return control to gdb for it to process the syscall request.
-     * Since the protocol requires that gdb hands control back to us
-     * using a "here are the results" F packet, we don't need to check
-     * gdb_handlesig's return value (which is the signal to deliver if
-     * execution was resumed via a continue packet).
-     */
-    gdb_handlesig(gdbserver_state.c_cpu, 0);
-#else
-    /*
-     * In this case wait to send the syscall packet until notification that
-     * the CPU has stopped.  This must be done because if the packet is sent
-     * now the reply from the syscall request could be received while the CPU
-     * is still in the running state, which can cause packets to be dropped
-     * and state transition 'T' packets to be sent while the syscall is still
-     * being processed.
-     */
-    qemu_cpu_kick(gdbserver_state.c_cpu);
-#endif
+
+    if (gdb_send_syscall_now()) { /* true only for *-user */
+        gdb_put_packet(gdbserver_syscall_state.syscall_buf);
+    }
+
+    /* user/softmmu specific handling */
+    gdb_post_syscall_handling();
 }
 
 void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...)
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 3da410e221..583cc83898 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -472,3 +472,27 @@  void gdb_breakpoint_remove_all(CPUState *cs)
 {
     cpu_breakpoint_remove_all(cs, BP_GDB);
 }
+
+/*
+ * For user-mode syscall support we send the system call immediately
+ * and then return control to gdb for it to process the syscall request.
+ * Since the protocol requires that gdb hands control back to us
+ * using a "here are the results" F packet, we don't need to check
+ * gdb_handlesig's return value (which is the signal to deliver if
+ * execution was resumed via a continue packet).
+ */
+
+void gdb_pre_syscall_handling(void)
+{
+    return;
+}
+
+bool gdb_send_syscall_now(void)
+{
+    return true;
+}
+
+void gdb_post_syscall_handling(void)
+{
+    gdb_handlesig(gdbserver_state.c_cpu, 0);
+}