Re: [PATCH v3 01/15] replay: don't record interrupt poll

["Alex Bennée" <alex.bennee@linaro.org>]

Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> Interrupt poll is not a real interrupt event. It is needed only for
> thread safety. This interrupt is used for i386 and converted
> to hardware interrupt by cpu_handle_interrupt function.
> Therefore it is not needed to be recorded, because hardware
> interrupt will be recorded after converting.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>  accel/tcg/cpu-exec.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 66d38f9d85..28aff1bb1e 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -429,8 +429,7 @@ static inline bool cpu_handle_halt(CPUState *cpu)
>  {
>      if (cpu->halted) {
>  #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
> -        if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
> -            && replay_interrupt()) {
> +        if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
>              X86CPU *x86_cpu = X86_CPU(cpu);
>              qemu_mutex_lock_iothread();
>              apic_poll_irq(x86_cpu->apic_state);
> @@ -587,7 +586,13 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>             and via longjmp via cpu_loop_exit.  */
>          else {
>              if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
> -                replay_interrupt();
> +                if (true
> +#if defined(TARGET_I386)
> +                    && !(interrupt_request & CPU_INTERRUPT_POLL)
> +#endif
> +                ) {
> +                    replay_interrupt();
> +                }

The cpu-exec code has enough special casing in it without adding more
ugly ifdefs. You could hoist the check into:

  /*
   * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
   * "real" interrupt event later. It does not need to be recorded for
   * replay purposes.
   */
  static inline bool replay_valid_interrupt(int interrupt) {
  #ifdef TARGET_I386
      return !(interrupt_request & CPU_INTERRUPT_POLL);
  #else
      return true;
  #endif
  }

and then the logic is a cleaner in the function:

  if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
      if (replay_valid_interrupt(interrupt_request)) {
          replay_interrupt();
      }

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée