On 11/11/21 12:06, Paolo Bonzini wrote:
> From: Maxim Levitsky <mlevi...@redhat.com>
>
> handle_query_qemu_sstepbits is reporting NOIRQ and NOTIMER bits
> even if they are not supported (as is the case with record/replay).
> Instead, store the supported singlestep flags and reject
> any unsupported bits in handle_set_qemu_sstep. This removes
> the need for the get_sstep_flags() wrapper.
>
> While at it, move the variables in GDBState, instead of using
> global variables.
>
> Signed-off-by: Maxim Levitsky <mlevi...@redhat.com>
> [Extracted from Maxim's patch into a separate commit. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> gdbstub.c | 73 +++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 47 insertions(+), 26 deletions(-)
> static void init_gdbserver_state(void)
> @@ -399,6 +382,24 @@ static void init_gdbserver_state(void)
> gdbserver_state.str_buf = g_string_new(NULL);
> gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH);
> gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH +
> 4);
Simpler:
gdbserver_state.supported_sstep_flags = SSTEP_ENABLE;
> + /*
> + * In replay mode all events written into the log should be replayed.
> + * That is why NOIRQ flag is removed in this mode.
> + */
if (replay_mode == REPLAY_MODE_NONE) {
gdbserver_state.supported_sstep_flags |= SSTEP_NOIRQ | SSTEP_NOTIMER;
}
> + if (replay_mode != REPLAY_MODE_NONE) {
> + gdbserver_state.supported_sstep_flags = SSTEP_ENABLE;
> + } else {
> + gdbserver_state.supported_sstep_flags =
> + SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
> + }
> +
> + /*
> + * By default use no IRQs and no timers while single stepping so as to
> + * make single stepping like an ICE HW step.
> + */
> + gdbserver_state.sstep_flags = gdbserver_state.supported_sstep_flags;
> +
> }
> static void handle_set_qemu_sstep(GArray *params, void *user_ctx)
> {
> + int new_sstep_flags;
> +
> if (!params->len) {
> return;
> }
>
> - sstep_flags = get_param(params, 0)->val_ul;
> + new_sstep_flags = get_param(params, 0)->val_ul;
> +
> + if (new_sstep_flags & ~gdbserver_state.supported_sstep_flags) {
> + put_packet("E22");
> + return;
> + }
Please does this change in a separate patch, after moving
to GDBState.
> + gdbserver_state.sstep_flags = new_sstep_flags;
> put_packet("OK");
> }
