On 03/11/2017 09:27, Pavel Dovgalyuk wrote: >> From: Paolo Bonzini [mailto:pbonz...@redhat.com] >> On 02/11/2017 12:33, Paolo Bonzini wrote: >>> On 02/11/2017 12:24, Pavel Dovgalyuk wrote: >>>>> I am not sure about this. I think if instead you should return false >>>>> from here and EXCP_INTERRUPT from cpu_exec. >>>> The problem is inside the TB. It checks cpu->icount_decr.u16.high which is >>>> -1. >>>> And we have to enter the TB to cause an exception (because it exists in >>>> replay log). >>>> That is why we reset this flag and try to execute the TB. >>> >>> But if u16.high is -1, shouldn't you return EXCP_INTERRUPT first (via >>> "Finally, check if we need to exit to the main loop" in >>> cpu_handle_interrupt)? Then only cause the exception when that one is >>> processed. >> >> ... indeed, you probably need something like: >> >> /* Clear the interrupt flag now since we're processing >> * cpu->interrupt_request and cpu->exit_request. >> */ >> insns_left = atomic_read(&cpu->icount_decr.u32); >> atomic_set(&cpu->icount_decr.u16.high, 0); >> if (unlikely(insns_left < 0) { >> /* Ensure the zeroing of icount_decr comes before the next read >> * of cpu->exit_request or cpu->interrupt_request. >> */ >> smb_mb(); >> } >> >> at the top of cpu_handle_interrupt. Then you can remove the same >> atomic_set+smp_mb in cpu_loop_exec_tb, like >> >> *last_tb = NULL; >> insns_left = atomic_read(&cpu->icount_decr.u32); >> if (insns_left < 0) { >> /* 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 will be handled by >> * cpu_handle_interrupt. cpu_handle_interrupt will also >> * clear cpu->icount_decr.u16.high. >> */ >> return; >> } > > I tried this approach and it didn't work. > I think iothread sets u16.high flag after resetting it in > cpu_handle_interrupt.
But why is this a problem? The TB would exit immediately and go again to cpu_handle_interrupt. cpu_handle_interrupt returns true and cpu_handle_exception causes the exception via cpu_exec_nocache. > But there is another possible approach: set new tcg flag, which disables > checking > the exit_request at the entry to the TB. No, I don't want to add complication. I want to understand what's going on---it's always worked very well whenever you pushed new rr bits, overall we ended up with a *simpler* CPU loop I think. :) Paolo