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

[Edgar E. Iglesias]

On Thu, Feb 20, 2014 at 03:58:15PM +0000, Peter Maydell wrote:
> 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 ?
> 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.

Hi,

I'm not sure about interrupt_request, seems to be a bit of
a mix. For example, I' not sure it's safe to keep all
the CPU_INTERRUPT_TGT_INT_X bits alive across a reset?
Agree with you about the interrupt hard line though.

Cheers,
Edgar