On Mon, 2014-03-03 at 11:24 +0800, Gavin Shan wrote:
> The problem is specific to the case of BIST issued applied to IPR
> adapter on the guest side. After BIST reset, we lose everything
> in MSIx table and we never have chance update MSIx messages for
> those enabled interrupts to MSIx table.
>
> The patch fixes it by writing MSIx message to MSIx table before
> reenabling them.
That needs a better explanation... the guest does try to restore the
MSI-X (in a way similar to resuming from suspend) by writing the
message value back into the table.
My understanding is that we trap that and turn that into a call to
vfio_msi_set_vector_signal, is that correct ?
In that case, it does indeed make sense to have that function rewrite
the cached message.
(Just trying to understand how this patch fixes the problem we saw)
I suppose other drivers would have similar issues if they have some
kind of internal "reset" path and try to save and restore the MSI-X
table.
Cheers,
Ben.
> Reported-by: Wen Xiong <wenxiong@xxxxxxxxxx>
> Signed-off-by: Gavin Shan <shangw@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/vfio/pci/vfio_pci_intrs.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> index 2103576..279ebd0 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,23 @@ 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.
> + * For example, BIST reset on IPR adapter. The MSIx table
> + * is cleaned out. However, we never get chance to put
> + * MSIx messages to MSIx table because all MSIx stuff is
> + * being cached in QEMU. Here, we had the trick to put the
> + * MSI/MSIx message back.
> + *
> + * Basically, we needn't worry about MSI messages. However,
> + * it's not harmful and there might be cases of PCI config data
> + * lost because of cached PCI config data in QEMU again.
> + *
> + * Note that we should flash the message prior to enabling
> + * the corresponding interrupt by request_irq().
> + */
> + 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) {
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
