Re: [Qemu-devel] [RFC PATCH 19/26] cpu-exec: reset exit flag before calling cpu_exec_nocache

[Paolo Bonzini]

On 10/11/2017 13:29, Pavel Dovgalyuk wrote:
>> From: Paolo Bonzini [mailto:address@hidden
>>>>>
>>>>> 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.
>>>
>>> I've tested your variant more thoroughly.
>>> It seems, that iothread calls cpu_exec between 
>>> atomic_set(&cpu->icount_decr.u16.high, 0);
>>> in cpu_handle_interrupt and cpu_exec_nocache in cpu_handle_exception.
>>> I see no other reason, because this happens not for the every time.
>>> And cpu_handle_interrupt is not called again, because cpu_handle_exception 
>>> returns true.
>>> Therefore we have an infinite loop, because no other code here resets cpu-
>>> icount_decr.u16.high.
>>
>> Then returning true unconditionally is wrong in the cpu_exec_nocache
>> case.  What if you do:
>>
>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>> index 61297f8f4a..fb5446be3e 100644
>> --- a/accel/tcg/cpu-exec.c
>> +++ b/accel/tcg/cpu-exec.c
>> @@ -470,7 +470,19 @@ static inline void cpu_handle_debug_exception(CPUState 
>> *cpu)
>>
>>  static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
>>  {
>> -    if (cpu->exception_index >= 0) {
>> +    if (cpu->exception_index < 0) {
>> +#ifndef CONFIG_USER_ONLY
>> +        if (replay_has_exception()
>> +            && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
>> +            /* try to cause an exception pending in the log */
>> +            cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0, curr_cflags()), 
>> true);
>> +        }
>> +#endif
>> +        if (cpu->exception_index < 0) {
>> +            return;
> 
> return false, I guess?
> This approach allows iterating in case of races
> and QEMU does not hangs anymore at replay.

Great, can you put this change the next time you send your series?
There are some parts that can definitely go in for 2.11.

Thanks,

Paolo

>> +        }
>> +    }
>> +
>>          if (cpu->exception_index >= EXCP_INTERRUPT) {
>>              /* exit request from the cpu execution loop */
>>              *ret = cpu->exception_index;
>> @@ -505,16 +517,6 @@ static inline bool cpu_handle_exception(CPUState *cpu, 
>> int *ret)
>>              }
>>  #endif
>>          }
>> -#ifndef CONFIG_USER_ONLY
>> -    } else if (replay_has_exception()
>> -               && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
>> -        /* try to cause an exception pending in the log */
>> -        cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0, curr_cflags()), 
>> true);
>> -        *ret = -1;
>> -        return true;
>> -#endif
>> -    }
>> -
>>      return false;
>>  }
>>
> 
> 
> Pavel Dovgalyuk
>