Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] target/ppc: Fix system lockups caused by interrupt_request state corruption

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Mon, 2017-12-04 at 12:44 +1100, David Gibson wrote:
> On Mon, Dec 04, 2017 at 12:00:40PM +1100, David Gibson wrote:
> > 
> > On Fri, Dec 01, 2017 at 03:49:07PM +0000, Richard Purdie wrote:
> > > 
> > > Occasionally in Linux guests on x86_64 we're seeing logs like:
> > > 
> > > ppc_set_irq: 0x55b4e0d562f0 n_IRQ 8 level 1 => pending
> > > 00000100req 00000004
> > > 
> > > when they should read:
> > > 
> > > ppc_set_irq: 0x55b4e0d562f0 n_IRQ 8 level 1 => pending
> > > 00000100req 00000002
> > > 
> > > The "00000004" is CPU_INTERRUPT_EXITTB yet the code calls
> > > cpu_interrupt(cs, CPU_INTERRUPT_HARD) ("00000002") in this
> > > function
> > > just before the log message. Something is causing the HARD bit
> > > setting
> > > to get lost.
> > > 
> > > The knock on effect of losing that bit is the decrementer timer
> > > interrupts
> > > don't get delivered which causes the guest to sit idle in its
> > > idle handler
> > > and 'hang'.
> > > 
> > > The issue occurs due to races from code which sets
> > > CPU_INTERRUPT_EXITTB.
> > > 
> > > Rather than poking directly into cs->interrupt_request, that code
> > > needs to:
> > > 
> > > a) hold BQL
> > > b) use the cpu_interrupt() helper
> > > 
> > > This patch fixes the call sites to do this, fixing the hang.
> > > 
> > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org
> > > >
> > I strongly suspect there's a better way to do this long term - a
> > lot
> > of that old ppc TCG code is really crufty.  But as best I can tell,
> > this is certainly a fix over what we had.  So, applied to
> > ppc-for-2.11.
> I take that back.  Running make check with this patch results in:
> 
>   GTESTER check-qtest-ppc64
> **
> ERROR:/home/dwg/src/qemu/cpus.c:1582:qemu_mutex_lock_iothread:
> assertion failed: (!qemu_mutex_iothread_locked())
> Broken pipe
> qemu-system-ppc64: RP: Received invalid message 0x0000 length 0x0000
> GTester: last random seed: R02S895b0f4813776bf68c147bf987e73f7b
> make: *** [/home/dwg/src/qemu/tests/Makefile.include:852: check-
> qtest-ppc64] Error 1
> 
> So, I've reverted it.

Sorry about that. I tried to stress the code paths and no issues showed
up in our testing but hadn't realised those tests existed.

Given there do seem to be mixed locked and unlocked code paths I've
taken a different approach and sent a v3 which passes those tests.

I do agree with you that there is probably a better way but that would
need someone with a better understanding of the bigger picture than I
have. This patch does stop our image tests locking up so does seem to
fix a valid/real problem which people can run into.

Cheers,

Richard