Re: [PATCH for-6.2 10/23] target/m68k: Drop checks for singlestep_enabled

From: Laurent Vivier <laurent@vivier.eu>
To: Richard Henderson <richard.henderson@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [PATCH for-6.2 10/23] target/m68k: Drop checks for singlestep_enabled
Date: Wed, 21 Jul 2021 10:54:56 +0200	[thread overview]
Message-ID: <cdaf5484-a7e1-5bf3-0906-9db3d16df52e@vivier.eu> (raw)
In-Reply-To: <20210721064155.645508-11-richard.henderson@linaro.org>

Le 21/07/2021 à 08:41, Richard Henderson a écrit :
> GDB single-stepping is now handled generically.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/m68k/translate.c | 44 +++++++++--------------------------------
>  1 file changed, 9 insertions(+), 35 deletions(-)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index c34d9aed61..c3d281a5f2 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -194,18 +194,6 @@ static void do_writebacks(DisasContext *s)
>      }
>  }
>  
> -static bool is_singlestepping(DisasContext *s)
> -{
> -    /*
> -     * Return true if we are singlestepping either because of
> -     * architectural singlestep or QEMU gdbstub singlestep. This does
> -     * not include the command line '-singlestep' mode which is rather
> -     * misnamed as it only means "one instruction per TB" and doesn't
> -     * affect the code we generate.
> -     */
> -    return s->base.singlestep_enabled || s->ss_active;
> -}
> -
>  /* is_jmp field values */
>  #define DISAS_JUMP      DISAS_TARGET_0 /* only pc was modified dynamically */
>  #define DISAS_EXIT      DISAS_TARGET_1 /* cpu state was modified dynamically */
> @@ -320,20 +308,6 @@ static void gen_exception(DisasContext *s, uint32_t dest, int nr)
>      s->base.is_jmp = DISAS_NORETURN;
>  }
>  
> -static void gen_singlestep_exception(DisasContext *s)
> -{
> -    /*
> -     * Generate the right kind of exception for singlestep, which is
> -     * either the architectural singlestep or EXCP_DEBUG for QEMU's
> -     * gdb singlestepping.
> -     */
> -    if (s->ss_active) {
> -        gen_raise_exception(EXCP_TRACE);
> -    } else {
> -        gen_raise_exception(EXCP_DEBUG);
> -    }
> -}
> -
>  static inline void gen_addr_fault(DisasContext *s)
>  {
>      gen_exception(s, s->base.pc_next, EXCP_ADDRESS);
> @@ -1522,10 +1496,10 @@ static void gen_exit_tb(DisasContext *s)
>  /* Generate a jump to an immediate address.  */
>  static void gen_jmp_tb(DisasContext *s, int n, uint32_t dest)
>  {
> -    if (unlikely(is_singlestepping(s))) {
> +    if (unlikely(s->ss_active)) {
>          update_cc_op(s);
>          tcg_gen_movi_i32(QREG_PC, dest);
> -        gen_singlestep_exception(s);
> +        gen_raise_exception(EXCP_TRACE);
>      } else if (translator_use_goto_tb(&s->base, dest)) {
>          tcg_gen_goto_tb(n);
>          tcg_gen_movi_i32(QREG_PC, dest);
> @@ -6193,7 +6167,7 @@ static void m68k_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
>  
>      dc->ss_active = (M68K_SR_TRACE(env->sr) == M68K_SR_TRACE_ANY_INS);
>      /* If architectural single step active, limit to 1 */
> -    if (is_singlestepping(dc)) {
> +    if (dc->ss_active) {
>          dc->base.max_insns = 1;
>      }
>  }
> @@ -6252,17 +6226,17 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
>          break;
>      case DISAS_TOO_MANY:
>          update_cc_op(dc);
> -        if (is_singlestepping(dc)) {
> +        if (dc->ss_active) {
>              tcg_gen_movi_i32(QREG_PC, dc->pc);
> -            gen_singlestep_exception(dc);
> +            gen_raise_exception(EXCP_TRACE);
>          } else {
>              gen_jmp_tb(dc, 0, dc->pc);
>          }
>          break;
>      case DISAS_JUMP:
>          /* We updated CC_OP and PC in gen_jmp/gen_jmp_im.  */
> -        if (is_singlestepping(dc)) {
> -            gen_singlestep_exception(dc);
> +        if (dc->ss_active) {
> +            gen_raise_exception(EXCP_TRACE);
>          } else {
>              tcg_gen_lookup_and_goto_ptr();
>          }
> @@ -6272,8 +6246,8 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
>           * We updated CC_OP and PC in gen_exit_tb, but also modified
>           * other state that may require returning to the main loop.
>           */
> -        if (is_singlestepping(dc)) {
> -            gen_singlestep_exception(dc);
> +        if (dc->ss_active) {
> +            gen_raise_exception(EXCP_TRACE);
>          } else {
>              tcg_gen_exit_tb(NULL, 0);
>          }
> 

Acked-by: Laurent Vivier <laurent@vivier.eu>