Re: [Qemu-devel] [RFC/RFT PATCH 0/7] cpu-exec: simplify cpu_exec and remove some icount special cases

[Paolo Bonzini]

----- Original Message -----
> From: "Pavel Dovgalyuk" <address@hidden>
> To: "Paolo Bonzini" <address@hidden>, address@hidden
> Cc: "serge fdrv" <address@hidden>, "pavel dovgaluk" <address@hidden>, "peter 
> maydell"
> <address@hidden>
> Sent: Thursday, February 2, 2017 11:07:12 PM
> Subject: RE: [RFC/RFT PATCH 0/7] cpu-exec: simplify cpu_exec and remove some 
> icount special cases
> 
> > From: Paolo Bonzini [mailto:address@hidden On Behalf Of Paolo
> > Bonzini
> > On 31/01/2017 01:05, Pavel Dovgalyuk wrote:
> > > Hi, Paolo!
> > >
> > > Thanks for refactoring.
> > > I tested these patches with icount record/replay on i386 machine.
> > > It works, but the following changes should be applied.
> > > I also removed call to replay_has_interrupt, because now it is not needed
> > > here.
> > > It seems, that this call is an artifact of an older record/replay
> > > revision.
> > >
> > > diff --git a/cpu-exec.c b/cpu-exec.c
> > > index 3838eb8..5cef8bc 100644
> > > --- a/cpu-exec.c
> > > +++ b/cpu-exec.c
> > > @@ -519,7 +519,8 @@ static inline bool cpu_handle_interrupt(CPUState
> > > *cpu,
> > >      }
> > >
> > >      /* Finally, check if we need to exit to the main loop.  */
> > > -    if (unlikely(atomic_read(&cpu->exit_request) ||
> > > replay_has_interrupt())) {
> > > +    if (unlikely(atomic_read(&cpu->exit_request)
> > > +        || (use_icount && cpu->icount_decr.u16.low + cpu->icount_extra
> > > == 0))) {
> > >          atomic_set(&cpu->exit_request, 0);
> > >          cpu->exception_index = EXCP_INTERRUPT;
> > >          return true;
> > 
> > So is this needed to avoid exceptions in tb_find?  Please add a comment
> > about this
> 
> This code comes from my last patch, that was not applied.
> Here is the comment:
> 
> It adds check to break cpu loop when icount expires without
> setting the TB_EXIT_ICOUNT_EXPIRED flag. It happens when there is no
> available translated blocks and all instructions were executed.
> In icount replay mode unnecessary tb_find will be called (which may
> cause an exception) and execution will be non-deterministic.
> 
> > and check if you can also replace:
> > 
> >     atomic_set(&cpu->exit_request, 1);
> > 
> > in cpu_loop_exec_tb with
> > 
> >     cpu->icount_decr.u16.low = 0;
> > 
> > ?
> > 
> 
> This line is not needed at all, because the following code decrements
> icount automatically.
> 
>         if (insns_left > 0) {
>             cpu_exec_nocache(cpu, insns_left, tb, false);
>         }

Right, so please send a patch and we can apply my series + yours.

Paolo