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

[Gavin Shan]

On Mon, Mar 16, 2015 at 09:05:27AM -0600, Alex Williamson wrote:
>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,
>

Thanks for confirm. I'll send out v2 tomorrow.

Nope, I'm not skipping why the slow path doesn't work. I didn't
understand well on how the QEMU memory model works together with
KVM. I need more time to trace and update with the findings.

Thanks,
Gavin 

>Alex
>
>