Re: [Qemu-devel] [RFC PATCH v7 08/21] replay: interrupts and exceptions

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:address@hidden
> On 12/01/2015 13:00, Pavel Dovgalyuk wrote:
> >
> > +                    if (replay_exception()) {
> > +                        cc->do_interrupt(cpu);
> > +                        cpu->exception_index = -1;
> 
> I cannot see replay_exception() in the series?

It is in the patch number 8.

> 
> > @@ -419,21 +434,24 @@ int cpu_exec(CPUArchState *env)
> >                          cpu->exception_index = EXCP_DEBUG;
> >                          cpu_loop_exit(cpu);
> 
> Why not for EXCP_DEBUG?

As far as I understand, EXCP_DEBUG is used for external debugger (like gdb).
Debugger is usually connected to VM during replay.
That is why we do not need to replay EXCP_DEBUG - it may appear in different
places, and it does not affect on behavior of the virtual machine.

> > -                    if (interrupt_request & CPU_INTERRUPT_HALT) {
> > +                    if ((interrupt_request & CPU_INTERRUPT_HALT)
> > +                        && replay_interrupt()) {
> >                          cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;
> >                          cpu->halted = 1;
> >                          cpu->exception_index = EXCP_HLT;
> >                          cpu_loop_exit(cpu);
> >                      }
> >  #if defined(TARGET_I386)
> > -                    if (interrupt_request & CPU_INTERRUPT_INIT) {
> > +                    if ((interrupt_request & CPU_INTERRUPT_INIT)
> > +                        && replay_interrupt()) {
> >                          cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 
> > 0);
> >                          do_cpu_init(x86_cpu);
> >                          cpu->exception_index = EXCP_HALTED;
> >                          cpu_loop_exit(cpu);
> >                      }
> >  #else
> > -                    if (interrupt_request & CPU_INTERRUPT_RESET) {
> > +                    if ((interrupt_request & CPU_INTERRUPT_RESET)
> > +                        && replay_interrupt()) {
> >                          cpu_reset(cpu);
> >                      }
> >  #endif
> 
> Perhaps check the replay_interrupt() outside, in an && with "if
> (unlikely(interrupt_request))"?

You mean that I should wrap whole condition into "unlikely"?

> > @@ -441,7 +459,10 @@ int cpu_exec(CPUArchState *env)
> >                         False when the interrupt isn't processed,
> >                         True when it is, and we should restart on a new TB,
> >                         and via longjmp via cpu_loop_exit.  */
> > -                    if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
> > +                    if ((replay_mode != REPLAY_MODE_PLAY
> > +                            || replay_has_interrupt())
> > +                        && cc->cpu_exec_interrupt(cpu, interrupt_request)) 
> > {
> > +                        replay_interrupt();
> 
> Please put this in a separate function like:
> 
>     if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {
>         return false;
>     }
>     ret = cc->cpu_exec_interrupt(cpu, interrupt_request);
>     if (ret) {
>         replay_interrupt();
>     }
>     return ret;

Ok.

Pavel Dovgalyuk