> From: Paolo Bonzini [mailto:pbonz...@redhat.com] > On 26/01/2017 15:32, Pavel Dovgalyuk wrote: > >> From: Paolo Bonzini [mailto:pbonz...@redhat.com] > >> On 26/01/2017 14:37, Pavel Dovgalyuk wrote: > >>>> Simpler: > >>>> > >>>> use_icount && > >>>> ((int32_t)cpu->icount_decr.u32 < 0 || > >>>> cpu->icount_decr.u16.low + cpu->icount_extra == 0) > >>> Right. > >>> > >>>> But I'm not sure that you need to test u32. After all you're not > >>> Checking u32 is needed, because sometimes it is less than zero. > >> > >> If cpu->icount_decr.u32 is less than zero, the next translation block > >> would immediately exit with TB_EXIT_ICOUNT_EXPIRED, causing > >> > >> cpu->exception_index = EXCP_INTERRUPT; > >> *last_tb = NULL; > >> cpu_loop_exit(cpu); > >> > >> from cpu_loop_exec_tb's "case TB_EXIT_ICOUNT_EXPIRED". > >> > >> And the same is true for cpu->icount_decr.u16.low + cpu->icount_extra == > >> 0, so I don't understand why this part of the patch is necessary. > > > > I removed that lines because we have to check icount=0 not only when it is > > expired, > > but also when all instructions were executed successfully. > > If there are no instructions to execute, calling tb_find (and translation > > then) > > may cause an exception at the wrong moment. > > Ok, that makes sense for cpu->icount_decr.u16.low + cpu->icount_extra == 0. > > But for decr.u32 < 0, the same reasoning of this comment is also true: > > /* Something asked us to stop executing > * chained TBs; just continue round the main > * loop. Whatever requested the exit will also > * have set something else (eg exit_request or > * interrupt_request) which we will handle > * next time around the loop. But we need to > * ensure the tcg_exit_req read in generated code > * comes before the next read of cpu->exit_request > * or cpu->interrupt_request. > */
Right. If the following lines will not be removed (as opposite to my patch) then checking decr.u32 < 0 will not be needed. - cpu->exception_index = EXCP_INTERRUPT; - *last_tb = NULL; - cpu_loop_exit(cpu); What is your point about the new version of that patch? Pavel Dovgalyuk