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;
}
Thanks,
Paolo
