Re: [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt

[Luc Michel <lmichel@kalray.eu>]

On 14:26 Mon 14 Feb     , Luc Michel wrote:
> In some cases, cpu->exit_request can be false after handling the
> interrupt, leading to another TB being executed instead of returning
> to the main loop.
> 
> Fix this by returning true unconditionally when in single-step mode.
> 
> Fixes: ba3c35d9c4026361fd380b269dc6def9510b7166
> 
> Signed-off-by: Luc Michel <lmichel@kalray.eu>

Hi Richard, did you have time to have a look at this patch?
Thanks,
 
Luc

> ---
> Coming back on this issue I worked on with Richard in 2020. The issue is
> that when debugging the guest with GDB, the first instruction of the IRQ
> handler is missed by GDB (it's still executed though).
> 
> It happened to me again in TCG RR mode (but not in MTTCG). It seems that
> cpu->exit_request can be false in RR mode when returning from
> cc->tcg_ops->cpu_exec_interrupt, leading to cpu_handle_interrupt
> returning false and the next TB being executed, instead of the EXCP_DEBUG
> being handled.
> ---
>  accel/tcg/cpu-exec.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 8b4cd6c59d..74d7f83f34 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -796,13 +796,17 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>                  /*
>                   * After processing the interrupt, ensure an EXCP_DEBUG is
>                   * raised when single-stepping so that GDB doesn't miss the
>                   * next instruction.
>                   */
> -                cpu->exception_index =
> -                    (cpu->singlestep_enabled ? EXCP_DEBUG : -1);
> -                *last_tb = NULL;
> +                if (unlikely(cpu->singlestep_enabled)) {
> +                    cpu->exception_index = EXCP_DEBUG;
> +                    return true;
> +                } else {
> +                    cpu->exception_index = -1;
> +                    *last_tb = NULL;
> +                }
>              }
>              /* The target hook may have updated the 'cpu->interrupt_request';
>               * reload the 'interrupt_request' value */
>              interrupt_request = cpu->interrupt_request;
>          }
> -- 
> 2.17.1
> 

--