Re: [PATCH 3/3] drivers/vfio/pci: Fix MSIx message lost

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 03/10/2014 04:46 PM, Gavin Shan wrote:
> The problem is specific to the case of BIST reset issued to IPR
> adapter on the guest side. The IPR driver calls pci_save_state(),
> issues BIST reset and then pci_restore_state(). The issued BIST
> cleans out MSIx table and pci_restore_state() doesn't restore
> the MSIx table as expected. Eventually, MSIx messages with all
> zeros are sent and causes EEH error on Power platform.
> 
> The patch fixes it by writing MSIx message to MSIx table before
> reenabling the individual interrupts in the following path:
> 
> qemu/hw/pci/msix.c::msix_table_mmio_write
>                     msix_handle_mask_update
>                     msix_fire_vector_notifier
> qemu/hw/misc/vfio.c::vfio_msix_vector_use
>                     vfio_msix_vector_do_use
> 
> IOCTL Command VFIO_DEVICE_SET_IRQS to VFIO-PCI
> vfio/pci/vfio_pci.c::vfio_pci_ioctl
> vfio/pci/vfio_pci_intrs.c::vfio_pci_set_irqs_ioctl
>                     vfio_pci_set_msi_trigger
>                     vfio_msi_set_block
>                     vfio_msi_set_vector_signal
> 
> Reported-by: Wen Xiong <wenxiong@us.ibm.com>
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  drivers/vfio/pci/vfio_pci_intrs.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> index 2103576..83e0638 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -17,6 +17,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/eventfd.h>
>  #include <linux/pci.h>
> +#include <linux/msi.h>
>  #include <linux/file.h>
>  #include <linux/poll.h>
>  #include <linux/vfio.h>
> @@ -517,6 +518,7 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
>  	struct pci_dev *pdev = vdev->pdev;
>  	int irq = msix ? vdev->msix[vector].vector : pdev->irq + vector;
>  	char *name = msix ? "vfio-msix" : "vfio-msi";
> +	struct msi_msg msg;
>  	struct eventfd_ctx *trigger;
>  	int ret;
>  
> @@ -544,6 +546,15 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
>  		return PTR_ERR(trigger);
>  	}
>  
> +	/* We possiblly lose the MSI/MSIx message in some cases, one
> +	 * of which is pci_save_state(), BIST reset and pci_restore_state()
> +	 * for IPR adapter. The BIST reset cleans out MSIx table and we
> +	 * don't have chance to restore it. Here, we have the trick to
> +	 * restore it before enabling individual interrupts. For MSI messages,
> +	 * it's harmless to write them back.
> +	 */
> +	get_cached_msi_msg(irq, &msg);
> +	write_msi_msg(irq, &msg);
>  	ret = request_irq(irq, vfio_msihandler, 0,
>  			  vdev->ctx[vector].name, trigger);
>  	if (ret) {
> 


This is missing to successfully compile VFIO as modules:


diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 2c10752..80ba2e9 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -283,6 +283,7 @@ void get_cached_msi_msg(unsigned int irq, struct
msi_msg *msg)

        __get_cached_msi_msg(entry, msg);
 }
+EXPORT_SYMBOL_GPL(get_cached_msi_msg);

 void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 {
@@ -327,6 +328,7 @@ void write_msi_msg(unsigned int irq, struct msi_msg *msg)

        __write_msi_msg(entry, msg);
 }
+EXPORT_SYMBOL_GPL(write_msi_msg);

 static void free_msi_irqs(struct pci_dev *dev)
 {




-- 
Alexey