[Qemu-devel] [RFC PATCH for 2.9] gdbstub: remove spurious tb_flush() calls

[Qemu-devel] [RFC PATCH for 2.9] gdbstub: remove spurious tb_flush() calls

[Alex Bennée]

These are (now) mostly harmless but not needed. The correct place to
flush if you want to is in the TCG aware code and not in gdbstub.
Currently flushes are triggered for cpu_single_step() and breakpoint
manipulation.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index de62d26..ea48415 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1274,7 +1274,6 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
             cpu->watchpoint_hit = NULL;
             goto send_packet;
         }
-        tb_flush(cpu);
         ret = GDB_SIGNAL_TRAP;
         break;
     case RUN_STATE_PAUSED:
@@ -1513,7 +1512,6 @@ gdb_handlesig(CPUState *cpu, int sig)
 
     /* disable single step if it was enabled */
     cpu_single_step(cpu, 0);
-    tb_flush(cpu);
 
     if (sig != 0) {
         snprintf(buf, sizeof(buf), "S%02x", target_signal_to_gdb(sig));
-- 
2.10.2

Re: [Qemu-devel] [RFC PATCH for 2.9] gdbstub: remove spurious tb_flush() calls

[Peter Maydell]

On 6 December 2016 at 15:00, Alex Bennée <alex.bennee@linaro.org> wrote:
> These are (now) mostly harmless but not needed. The correct place to
> flush if you want to is in the TCG aware code and not in gdbstub.
> Currently flushes are triggered for cpu_single_step() and breakpoint
> manipulation.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  gdbstub.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index de62d26..ea48415 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1274,7 +1274,6 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
>              cpu->watchpoint_hit = NULL;
>              goto send_packet;
>          }
> -        tb_flush(cpu);
>          ret = GDB_SIGNAL_TRAP;
>          break;
>      case RUN_STATE_PAUSED:
> @@ -1513,7 +1512,6 @@ gdb_handlesig(CPUState *cpu, int sig)
>
>      /* disable single step if it was enabled */
>      cpu_single_step(cpu, 0);
> -    tb_flush(cpu);
>
>      if (sig != 0) {
>          snprintf(buf, sizeof(buf), "S%02x", target_signal_to_gdb(sig));
> --
> 2.10.2

I think these are right in principle but we should probably
try to exercise the gdbstub fairly thoroughly to convince
ourselves they weren't hiding some bug...

I also think that cpu_single_step() doing a flush is wrong:
we should have the singlestep flag in the tbflags instead.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH for 2.9] gdbstub: remove spurious tb_flush() calls

[Richard Henderson]

On 12/06/2016 07:42 AM, Peter Maydell wrote:
> I also think that cpu_single_step() doing a flush is wrong:
> we should have the singlestep flag in the tbflags instead.

Yes.  And the parallel_cpus flag, so we don't have to keep re-translating an
instruction that cannot be handled by the host natively.

Indeed, it occurs to me that we could steal some bits out of cflags to
represent both of these, and then include cflags in the hash.


r~