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

[Richard Henderson]

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~