Re: [Qemu-devel] [PATCH v2 2/3] VFIO: Disable INTx interrupt on EEH reset

[Alex Williamson <alex.williamson@redhat.com>]

On Tue, 2015-03-17 at 03:31 +1100, Gavin Shan wrote:
> When Linux guest recovers from EEH error on the following Emulex
> adapter, the MSIx interrupts are disabled and the INTx emulation
> is enabled. One INTx interrupt is injected to the guest by host
> because of detected pending INTx interrupts on the adapter. QEMU
> disables mmap'ed BAR regions and starts a timer to enable those
> regions at later point the INTx interrupt handler. Unfortunately,
> "VFIOPCIDevice->intx.pending" isn't cleared, meaning those disabled
> mapp'ed BAR regions won't be reenabled properly. It leads to EEH
> recovery failure at guest side because of hanged MMIO access.
> 
>  # lspci | grep Emulex
>  0000:01:00.0 Ethernet controller: Emulex Corporation \
>               OneConnect 10Gb NIC (be3) (rev 02)
>  0000:01:00.1 Ethernet controller: Emulex Corporation \
>               OneConnect 10Gb NIC (be3) (rev 02)
> 
> The patch disables INTx interrupt before doing EEH reset to avoid
> the issue.
> 
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>  hw/vfio/pci.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index fca1edc..bfa3d0c 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3340,6 +3340,14 @@ int vfio_container_eeh_event(AddressSpace *as, int32_t groupid,
>           * disable it so that it can be reenabled properly. Also,
>           * the cached MSIx table should be cleared as it's not
>           * reflecting the contents in hardware.
> +         *
> +         * We might have INTx interrupt whose handler disables the
> +         * memory mapped BARs. The INTx pending state can't be
> +         * cleared with memory BAR access in slow path. The timer
> +         * kicked by the INTx interrupt handler won't enable those
> +         * disabled memory mapped BARs, which leads to hanged MMIO
> +         * register access and EEH recovery failure. We simply disable
> +         * INTx if it has been enabled.
>           */

This feels like a quick hack for a problem we don't really understand.
Why is INTx being fired through QEMU rather than KVM?  Why isn't the
INTx re-enabling happening since this is exactly the scenario where it's
supposed to work (ie. INTx occurs, BAR mmap disabled, guest accesses
BAR, mmap re-enabled, INTx unmasked)?

>          QLIST_FOREACH(vbasedev, &group->device_list, next) {
>              vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
> @@ -3348,6 +3356,11 @@ int vfio_container_eeh_event(AddressSpace *as, int32_t groupid,
>              }
>  
>              msix_reset(&vdev->pdev);
> +
> +            /* Disable INTx */
> +            if (vdev->interrupt == VFIO_INT_INTx) {
> +                vfio_disable_intx(vdev);
> +            }
>          }
>  
>          break;