Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/3] VFIO: Clear INTx pending state on EEH reset

[Alex Williamson]

On Tue, 2015-03-17 at 01:34 +1100, Gavin Shan wrote:
> On Mon, Mar 16, 2015 at 03:05:32PM +1100, Benjamin Herrenschmidt wrote:
> >On Mon, 2015-03-16 at 12:04 +1100, Gavin Shan wrote:
> >> 
> >> 
> >> (2) QEMU sends IOCTL commands to host to disable MSIx and enable INTx. At
> >> this stage the INTx is still masked. At later point, the guest is 
> >> requesting
> >> unmasking INTx, which is captured by host. Host checks and founds pending
> >> INTx, which is sent to QEMU. In QEMU INTx handler (vfio_intx_interrupt()),
> >> the mmap'ed regions are disabled, "intx.pending" is set and a timer is 
> >> started
> >> to reenable mmap'ed regions if "intx.pending" is cleared there. However,
> >> "intx.pending" is only cleared upon BAR access in slow path, which is never
> >> happing.
> >> 
> >> (3) After guest disables MSIx and issue EEH reset, the device driver starts
> >> to check its firmware state by reading MMIO register, which isn't completed
> >> by QEMU VFIO BAR slow path (Note: fast path supported by mmaped regions 
> >> have
> >> been disabled). Eventually, the guest hangs on reading MMIO register. With
> >> this patch applied to QEMU, I didn't see the problem again. 
> >
> >Note that it might be a good idea to disable INTx (and synchronize with a cfg
> >read of some sort) around resetting a device.
> >
> >Otherwise, you may hit a known issue if the device is behind a switch and has
> >sent the INTx "assert" message, and not the "deassert" one before it gets 
> >reset.
> >
> >That can cause the INTx to effectively be "stuck" in the switch preventing a
> >subsequent one from being delivered.
> >
> 
> Yeah, It makes more sense to disable INTx before issuing EEH reset. I verified
> that disabling INTx interrupt upon EEH reset can avoid the issue as well. I'll
> post updated patch accordingly if Alex Williamson doesn't object.

That sounds like a cleaner approach, but you seem to be skipping
something around why the slow-path clearing of intx.pending isn't
working for you.  Step (2) says "... is only cleared upon BAR access in
slow path, which is never happening."  Step (3) "the device driver
starts to check its firmware state by reading MMIO register, which isn't
completed by QEMU VFIO BAR slow path".   So it sounds like (3) is doing
exactly what should allow the QEMU path INTx state machine to advance,
so why doesn't it work?  Thanks,

Alex