Re: [Qemu-devel] [RFC PATCH 19/26] cpu-exec: reset exit flag before calling cpu_exec_nocache

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:address@hidden
> >>>
> >>> I tried this approach and it didn't work.
> >>> I think iothread sets u16.high flag after resetting it in 
> >>> cpu_handle_interrupt.
> >>
> >> But why is this a problem?  The TB would exit immediately and go again
> >> to cpu_handle_interrupt.  cpu_handle_interrupt returns true and
> >> cpu_handle_exception causes the exception via cpu_exec_nocache.
> >
> > I've tested your variant more thoroughly.
> > It seems, that iothread calls cpu_exec between 
> > atomic_set(&cpu->icount_decr.u16.high, 0);
> > in cpu_handle_interrupt and cpu_exec_nocache in cpu_handle_exception.
> > I see no other reason, because this happens not for the every time.
> > And cpu_handle_interrupt is not called again, because cpu_handle_exception 
> > returns true.
> > Therefore we have an infinite loop, because no other code here resets cpu-
> >icount_decr.u16.high.
> 
> Then returning true unconditionally is wrong in the cpu_exec_nocache
> case.  What if you do:
> 
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 61297f8f4a..fb5446be3e 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -470,7 +470,19 @@ static inline void cpu_handle_debug_exception(CPUState 
> *cpu)
> 
>  static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
>  {
> -    if (cpu->exception_index >= 0) {
> +    if (cpu->exception_index < 0) {
> +#ifndef CONFIG_USER_ONLY
> +        if (replay_has_exception()
> +            && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
> +            /* try to cause an exception pending in the log */
> +            cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0, curr_cflags()), 
> true);
> +        }
> +#endif
> +        if (cpu->exception_index < 0) {
> +            return;

return false, I guess?
This approach allows iterating in case of races
and QEMU does not hangs anymore at replay.

> +        }
> +    }
> +
>          if (cpu->exception_index >= EXCP_INTERRUPT) {
>              /* exit request from the cpu execution loop */
>              *ret = cpu->exception_index;
> @@ -505,16 +517,6 @@ static inline bool cpu_handle_exception(CPUState *cpu, 
> int *ret)
>              }
>  #endif
>          }
> -#ifndef CONFIG_USER_ONLY
> -    } else if (replay_has_exception()
> -               && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
> -        /* try to cause an exception pending in the log */
> -        cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0, curr_cflags()), true);
> -        *ret = -1;
> -        return true;
> -#endif
> -    }
> -
>      return false;
>  }
> 


Pavel Dovgalyuk