All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chao Gao <chao.gao@intel.com>
To: linux-kernel@vger.kernel.org
Cc: "Chao Gao" <chao.gao@intel.com>,
	"Boris Ostrovsky" <boris.ostrovsky@oracle.com>,
	"Juergen Gross" <jgross@suse.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Jia-Ju Bai" <baijiaju1990@gmail.com>,
	xen-devel@lists.xenproject.org,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>
Subject: [PATCH] xen: xen-pciback: Reset MSI-X state when exposing a device
Date: Wed,  5 Dec 2018 10:19:17 +0800	[thread overview]
Message-ID: <1543976357-1053-1-git-send-email-chao.gao@intel.com> (raw)

I find some pass-thru devices don't work any more across guest reboot.
Assigning it to another guest also meets the same issue. And the only
way to make it work again is un-binding and binding it to pciback.
Someone reported this issue one year ago [1]. More detail also can be
found in [2].

The root-cause is Xen's internal MSI-X state isn't reset properly
during reboot or re-assignment. In the above case, Xen set maskall bit
to mask all MSI interrupts after it detected a potential security
issue. Even after device reset, Xen didn't reset its internal maskall
bit. As a result, maskall bit would be set again in next write to
MSI-X message control register.

Given that PHYSDEVOPS_prepare_msix() also triggers Xen resetting MSI-X
internal state of a device, we employ it to fix this issue rather than
introducing another dedicated sub-hypercall.

Note that PHYSDEVOPS_release_msix() will fail if the mapping between
the device's msix and pirq has been created. This limitation prevents
us calling this function when detaching a device from a guest during
guest shutdown. Thus it is called right before calling
PHYSDEVOPS_prepare_msix().

[1]: https://lists.xenproject.org/archives/html/xen-devel/2017-09/
     msg02520.html
[2]: https://lists.xen.org/archives/html/xen-devel/2018-11/msg01616.html

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 drivers/xen/xen-pciback/pci_stub.c | 49 ++++++++++++++++++++++++++++++++++++++
 drivers/xen/xen-pciback/pciback.h  |  1 +
 drivers/xen/xen-pciback/xenbus.c   | 10 ++++++++
 3 files changed, 60 insertions(+)

diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
index 59661db..f8623d0 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -87,6 +87,55 @@ static struct pcistub_device *pcistub_device_alloc(struct pci_dev *dev)
 	return psdev;
 }
 
+/*
+ * Reset Xen internal MSI-X state by invoking PHYSDEVOP_{release, prepare}_msix.
+ */
+int pcistub_msix_reset(struct pci_dev *dev)
+{
+#ifdef CONFIG_PCI_MSI
+	if (dev->msix_cap) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+		int err;
+		u16 val;
+
+		/*
+		 * Do a write first to flush Xen's internal state to hardware
+		 * such that the following read can infer whether MSI-X maskall
+		 * bit is set by Xen.
+		 */
+		pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &val);
+		pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, val);
+
+		pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &val);
+		if (!(val & PCI_MSIX_FLAGS_MASKALL))
+			return 0;
+
+		pr_info("Reset MSI-X state for device %04x:%02x:%02x.%d\n",
+			ppdev.seg, ppdev.bus, PCI_SLOT(ppdev.devfn),
+			PCI_FUNC(ppdev.devfn));
+
+		err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix, &ppdev);
+		if (err) {
+			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
+				 err);
+			return err;
+		}
+
+		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
+		if (err) {
+			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
+				err);
+			return err;
+		}
+	}
+#endif
+	return 0;
+}
+
 /* Don't call this directly as it's called by pcistub_device_put */
 static void pcistub_device_release(struct kref *kref)
 {
diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h
index 263c059..9046154 100644
--- a/drivers/xen/xen-pciback/pciback.h
+++ b/drivers/xen/xen-pciback/pciback.h
@@ -66,6 +66,7 @@ struct pci_dev *pcistub_get_pci_dev_by_slot(struct xen_pcibk_device *pdev,
 struct pci_dev *pcistub_get_pci_dev(struct xen_pcibk_device *pdev,
 				    struct pci_dev *dev);
 void pcistub_put_pci_dev(struct pci_dev *dev);
+int pcistub_msix_reset(struct pci_dev *dev);
 
 /* Ensure a device is turned off or reset */
 void xen_pcibk_reset_device(struct pci_dev *pdev);
diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
index 581c4e1..2f71f26 100644
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -243,6 +243,16 @@ static int xen_pcibk_export_device(struct xen_pcibk_device *pdev,
 		goto out;
 	}
 
+	/*
+	 * Reset Xen's internal MSI-X state before exposing a device.
+	 *
+	 * In some cases, Xen's internal MSI-X state is not clean, which would
+	 * incur the new guest cannot receive MSIs.
+	 */
+	err = pcistub_msix_reset(dev);
+	if (err)
+		goto out;
+
 	err = xen_pcibk_add_pci_dev(pdev, dev, devid,
 				    xen_pcibk_publish_pci_dev);
 	if (err)
-- 
1.8.3.1


             reply	other threads:[~2018-12-05  2:15 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-05  2:19 Chao Gao [this message]
2018-12-05  9:32 ` [PATCH] xen: xen-pciback: Reset MSI-X state when exposing a device Roger Pau Monné
2018-12-05  9:32 ` Roger Pau Monné
2018-12-05 14:01   ` Boris Ostrovsky
2018-12-12  7:06     ` Chao Gao
2018-12-12  8:51       ` Jan Beulich
2018-12-12  8:51       ` Jan Beulich
2018-12-12 15:18         ` Chao Gao
2018-12-12 15:18         ` Chao Gao
2018-12-12 15:21           ` Jan Beulich
2018-12-12 15:21           ` Jan Beulich
2018-12-13  3:46             ` Chao Gao
2018-12-13  7:54               ` Jan Beulich
2018-12-13  7:54               ` Jan Beulich
2018-12-13 13:17                 ` Chao Gao
2018-12-13 13:17                 ` Chao Gao
2018-12-13  3:46             ` Chao Gao
2018-12-12  7:06     ` Chao Gao
2018-12-05 14:01   ` Boris Ostrovsky
2018-12-06  2:18   ` Chao Gao
2018-12-06  2:18   ` Chao Gao
2018-12-05 16:26 ` Jan Beulich
2018-12-05 16:26 ` Jan Beulich
2019-09-13 10:02 ` [Xen-devel] " Spassov, Stanislav
2019-09-13 10:02   ` Spassov, Stanislav
2019-09-13 15:28   ` [Xen-devel] " Chao Gao
2019-09-13 15:28     ` Chao Gao
2019-09-26 10:13     ` [Xen-devel] " Pasi Kärkkäinen
2019-09-26 10:13       ` Pasi Kärkkäinen
2019-09-26 10:54       ` Spassov, Stanislav
2019-09-26 10:54         ` Spassov, Stanislav
2020-01-17 18:57       ` Rich Persaud
2020-01-17 18:57         ` Rich Persaud
2020-01-18  1:13         ` Chao Gao
2020-01-18  1:13           ` Chao Gao
  -- strict thread matches above, loose matches on Subject: below --
2018-12-05  2:19 Chao Gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1543976357-1053-1-git-send-email-chao.gao@intel.com \
    --to=chao.gao@intel.com \
    --cc=baijiaju1990@gmail.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.