Re: [Qemu-devel] [PATCH] qom/cpu: Remove cpu->exit_request from reset state

[Andreas Färber]

Am 20.02.2014 16:58, schrieb Peter Maydell:
> On 16 February 2014 02:07, Edgar E. Iglesias <address@hidden> wrote:
>> On Sat, Feb 15, 2014 at 03:42:56PM +0000, Peter Maydell wrote:
>>> On 13 February 2014 05:07,  <address@hidden> wrote:
>>>> From: "Edgar E. Iglesias" <address@hidden>
>>>>
>>>> cpu->exit_request is part of the execution environment and should
>>>> not be cleared when a CPU resets.
>>>>
>>>> Otherwise, we might deadlock QEMU if a CPU resets while there is
>>>> I/O going on.
>>>>
>>>> Signed-off-by: Edgar E. Iglesias <address@hidden>
>>>> ---
>>>>  qom/cpu.c | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/qom/cpu.c b/qom/cpu.c
>>>> index 9d62479..40d82dd 100644
>>>> --- a/qom/cpu.c
>>>> +++ b/qom/cpu.c
>>>> @@ -195,7 +195,6 @@ static void cpu_common_reset(CPUState *cpu)
>>>>          log_cpu_state(cpu, cc->reset_dump_flags);
>>>>      }
>>>>
>>>> -    cpu->exit_request = 0;
>>>>      cpu->interrupt_request = 0;
>>>>      cpu->current_tb = NULL;
>>>>      cpu->halted = 0;
>>>
>>> This looks kind of odd to me. What's the situation you see where
>>> this matters -- is the CPU resetting itself, or is some other device
>>> in another thread triggering the CPU reset? TCG or KVM?
>>
>> Seeing this in TCG. The CPU gets signaled by the IO thread while the
>> CPU is resetting itself. If the CPU looses the race, it clears its
>> exit_request leaving the IO thread waiting for the global lock
>> potentially forever.
>>
>> The CPU actually exits generated code but goes right back in because
>> there is no exit_request pending.
> 
> Yes, having looked at the code I agree with you, so:
> 
> Reviewed-by: Peter Maydell <address@hidden>
> 
> However, doesn't this also apply to interrupt_request ?

I was wondering the same thing but didn't find time to investigate yet.

Is it possible that we rather need to register some reset hook or bottom
half to process the exit_request *before* this reset code runs?

Regards,
Andreas

> If we have a pending asserted interrupt on the CPU
> (ie the IRQ line into the chip is being held high)
> this should result in an interrupt as soon as the
> CPU reenables interrupts after reset, I would have
> thought. Clearing cpu->interrupt_request here will
> make us drop it on the floor.
> 
> thanks
> -- PMM
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg