diff mbox series

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

Message ID 159903455305.28509.6834799024577960185.stgit@pasha-ThinkPad-X280
State New
Headers show
Series Reverse debugging | expand

Commit Message

Pavel Dovgalyuk Sept. 2, 2020, 8:15 a.m. UTC
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(-)

Comments

Alex Bennée Sept. 7, 2020, 10:17 a.m. UTC | #1
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>
diff mbox series

Patch

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();
+                }
                 /*
                  * After processing the interrupt, ensure an EXCP_DEBUG is
                  * raised when single-stepping so that GDB doesn't miss the