QubesOS
/
qubes-linux-kernel
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Apply pciback/pcifront patches fixing MSI-X enabled devices handling

Browse Source 
Fixes QubesOS/qubes-issues#1734
pull/3/head mm_8cf04490
Marek Marczykowski-Górecki 8 years ago
parent c9318689a6
commit 8cf0449067
No known key found for this signature in database
GPG Key ID: 063938BA42CFA724
11 changed files with 765 additions and 0 deletions
Show Stats Download Patch File Download Diff File

46
patches.xen/0001-xen-pciback-Check-PF-instead-of-VF-for-PCI_COMMAND_M.patch
Unescape View File

@ -0,0 +1,46 @@
From 9b8c92f9535ca9af672facd05360570730a33e05 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 3 Feb 2016 10:18:18 -0500
Subject: [PATCH 1/4] xen/pciback: Check PF instead of VF for
PCI_COMMAND_MEMORY
c/s 408fb0e5aa7fda0059db282ff58c3b2a4278baa0
"xen/pciback: Don't allow MSI-X ops if PCI_COMMAND_MEMORY is not set."
would check the device for PCI_COMMAND_MEMORY which is great.
Except that VF devices are unique - for example they have no
legacy interrupts, and also any writes to PCI_COMMAND_MEMORY
are silently ignored (by the hardware).
CC: stable@vger.kernel.org
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index 73dafdc..8c86a53 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -213,6 +213,7 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
int i, result;
struct msix_entry *entries;
u16 cmd;
+ struct pci_dev *phys_dev;
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: enable MSI-X\n",
@@ -227,8 +228,10 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
/*
* PCI_COMMAND_MEMORY must be enabled, otherwise we may not be able
* to access the BARs where the MSI-X entries reside.
+ * But VF devices are unique in which the PF needs to be checked.
*/
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
+ phys_dev = pci_physfn(dev);
+ pci_read_config_word(phys_dev, PCI_COMMAND, &cmd);
if (dev->msi_enabled || !(cmd & PCI_COMMAND_MEMORY))
return -ENXIO;
--
2.1.0

56
patches.xen/0002-xen-pciback-Save-the-number-of-MSI-X-entries-to-be-c.patch
Unescape View File

@ -0,0 +1,56 @@
From 0f8901117800fc2f1a87cc5468f1ab7e4288cc5f Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 3 Feb 2016 10:22:02 -0500
Subject: [PATCH 2/4] xen/pciback: Save the number of MSI-X entries to be
copied later.
c/s 8135cf8b092723dbfcc611fe6fdcb3a36c9951c5
"xen/pciback: Save xen_pci_op commands before processing it"
would copyback the processed values - which was great.
However it missed the case that xen_pcibk_enable_msix - when
completing would overwrite op->value (which had the number
of MSI-X vectors requested) with the return value (which for
success was zero). Hence the copy-back routine (which would use
op->value) would copy exactly zero MSI-X vectors back.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index 8c86a53..2fc5880 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -335,7 +335,9 @@ void xen_pcibk_do_op(struct work_struct *data)
struct xen_pcibk_dev_data *dev_data = NULL;
struct xen_pci_op *op = &pdev->op;
int test_intx = 0;
-
+#ifdef CONFIG_PCI_MSI
+ unsigned int nr = 0;
+#endif
*op = pdev->sh_info->op;
barrier();
dev = xen_pcibk_get_pci_dev(pdev, op->domain, op->bus, op->devfn);
@@ -363,6 +365,7 @@ void xen_pcibk_do_op(struct work_struct *data)
op->err = xen_pcibk_disable_msi(pdev, dev, op);
break;
case XEN_PCI_OP_enable_msix:
+ nr = op->value;
op->err = xen_pcibk_enable_msix(pdev, dev, op);
break;
case XEN_PCI_OP_disable_msix:
@@ -385,7 +388,7 @@ void xen_pcibk_do_op(struct work_struct *data)
if (op->cmd == XEN_PCI_OP_enable_msix && op->err == 0) {
unsigned int i;
- for (i = 0; i < op->value; i++)
+ for (i = 0; i < nr; i++)
pdev->sh_info->op.msix_entries[i].vector =
op->msix_entries[i].vector;
}
--
2.1.0

54
patches.xen/0003-xen-pcifront-Report-the-errors-better.patch
Unescape View File

@ -0,0 +1,54 @@
From 3a0d57b60a61eb461504f8ed1845afd5084b7889 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 3 Feb 2016 16:39:21 -0500
Subject: [PATCH 3/4] xen/pcifront: Report the errors better.
The messages should be different depending on the type of error.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
arch/x86/include/asm/xen/pci.h | 4 ++--
arch/x86/pci/xen.c | 5 ++++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/xen/pci.h b/arch/x86/include/asm/xen/pci.h
index 968d57d..f320ee3 100644
--- a/arch/x86/include/asm/xen/pci.h
+++ b/arch/x86/include/asm/xen/pci.h
@@ -57,7 +57,7 @@ static inline int xen_pci_frontend_enable_msi(struct pci_dev *dev,
{
if (xen_pci_frontend && xen_pci_frontend->enable_msi)
return xen_pci_frontend->enable_msi(dev, vectors);
- return -ENODEV;
+ return -ENOSYS;
}
static inline void xen_pci_frontend_disable_msi(struct pci_dev *dev)
{
@@ -69,7 +69,7 @@ static inline int xen_pci_frontend_enable_msix(struct pci_dev *dev,
{
if (xen_pci_frontend && xen_pci_frontend->enable_msix)
return xen_pci_frontend->enable_msix(dev, vectors, nvec);
- return -ENODEV;
+ return -ENOSYS;
}
static inline void xen_pci_frontend_disable_msix(struct pci_dev *dev)
{
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index ff31ab4..beac4df 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -196,7 +196,10 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
return 0;
error:
- dev_err(&dev->dev, "Xen PCI frontend has not registered MSI/MSI-X support!\n");
+ if (ret == -ENOSYS)
+ dev_err(&dev->dev, "Xen PCI frontend has not registered MSI/MSI-X support!\n");
+ else if (ret)
+ dev_err(&dev->dev, "Xen PCI frontend error: %d!\n", ret);
free:
kfree(v);
return ret;
--
2.1.0

117
patches.xen/0004-xen-pcifront-Fix-mysterious-crashes-when-NUMA-locali.patch
Unescape View File

@ -0,0 +1,117 @@
From 25af39d59d2ff4a5e5bc872b8d4c451bbeffa312 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 3 Feb 2016 16:43:56 -0500
Subject: [PATCH 4/4] xen/pcifront: Fix mysterious crashes when NUMA locality
information was extracted.
Occasionaly PV guests would crash with:
pciback 0000:00:00.1: Xen PCI mapped GSI0 to IRQ16
BUG: unable to handle kernel paging request at 0000000d1a8c0be0
.. snip..
<ffffffff8139ce1b>] find_next_bit+0xb/0x10
[<ffffffff81387f22>] cpumask_next_and+0x22/0x40
[<ffffffff813c1ef8>] pci_device_probe+0xb8/0x120
[<ffffffff81529097>] ? driver_sysfs_add+0x77/0xa0
[<ffffffff815293e4>] driver_probe_device+0x1a4/0x2d0
[<ffffffff813c1ddd>] ? pci_match_device+0xdd/0x110
[<ffffffff81529657>] __device_attach_driver+0xa7/0xb0
[<ffffffff815295b0>] ? __driver_attach+0xa0/0xa0
[<ffffffff81527622>] bus_for_each_drv+0x62/0x90
[<ffffffff8152978d>] __device_attach+0xbd/0x110
[<ffffffff815297fb>] device_attach+0xb/0x10
[<ffffffff813b75ac>] pci_bus_add_device+0x3c/0x70
[<ffffffff813b7618>] pci_bus_add_devices+0x38/0x80
[<ffffffff813dc34e>] pcifront_scan_root+0x13e/0x1a0
[<ffffffff817a0692>] pcifront_backend_changed+0x262/0x60b
[<ffffffff814644c6>] ? xenbus_gather+0xd6/0x160
[<ffffffff8120900f>] ? put_object+0x2f/0x50
[<ffffffff81465c1d>] xenbus_otherend_changed+0x9d/0xa0
[<ffffffff814678ee>] backend_changed+0xe/0x10
[<ffffffff81463a28>] xenwatch_thread+0xc8/0x190
[<ffffffff810f22f0>] ? woken_wake_function+0x10/0x10
which was the result of two things:
When we call pci_scan_root_bus we would pass in 'sd' (sysdata)
pointer which was an 'pcifront_sd' structure. However in the
pci_device_add it expects that the 'sd' is 'struct sysdata' and
sets the dev->node to what is in sd->node (offset 4):
set_dev_node(&dev->dev, pcibus_to_node(bus));
__pcibus_to_node(const struct pci_bus *bus)
{
const struct pci_sysdata *sd = bus->sysdata;
return sd->node;
}
However our structure was pcifront_sd which had nothing at that
offset:
struct pcifront_sd {
int domain; /* 0 4 */
/* XXX 4 bytes hole, try to pack */
struct pcifront_device * pdev; /* 8 8 */
}
That is an hole - filled with garbage as we used kmalloc instead of
kzalloc (the second problem).
This patch fixes the issue by:
1) Use kzalloc to initialize to a well known state.
2) Put 'struct pci_sysdata' at the start of 'pcifront_sd'. That
way access to the 'node' will access the right offset.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/pci/xen-pcifront.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index c777b97..66d197d 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -53,7 +53,7 @@ struct pcifront_device {
};
struct pcifront_sd {
- int domain;
+ struct pci_sysdata sd;
struct pcifront_device *pdev;
};
@@ -67,7 +67,9 @@ static inline void pcifront_init_sd(struct pcifront_sd *sd,
unsigned int domain, unsigned int bus,
struct pcifront_device *pdev)
{
- sd->domain = domain;
+ /* Because we do not expose that information via XenBus. */
+ sd->sd.node = first_online_node;
+ sd->sd.domain = domain;
sd->pdev = pdev;
}
@@ -468,8 +470,8 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
dev_info(&pdev->xdev->dev, "Creating PCI Frontend Bus %04x:%02x\n",
domain, bus);
- bus_entry = kmalloc(sizeof(*bus_entry), GFP_KERNEL);
- sd = kmalloc(sizeof(*sd), GFP_KERNEL);
+ bus_entry = kzalloc(sizeof(*bus_entry), GFP_KERNEL);
+ sd = kzalloc(sizeof(*sd), GFP_KERNEL);
if (!bus_entry || !sd) {
err = -ENOMEM;
goto err_out;
@@ -576,6 +578,7 @@ static void pcifront_free_roots(struct pcifront_device *pdev)
free_root_bus_devs(bus_entry->bus);
kfree(bus_entry->bus->sysdata);
+ bus_entry->bus->sysdata = NULL;
device_unregister(bus_entry->bus->bridge);
pci_remove_bus(bus_entry->bus);
--
2.1.0

104
patches.xen/pci_op-cleanup.patch
Unescape View File

@ -0,0 +1,104 @@
From xen-devel-bounces@lists.xen.org Tue Feb 9 06:00:36 2016
Date: Mon, 8 Feb 2016 23:59:27 -0500
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Marek =?iso-8859-1?Q?Marczykowski-G=F3recki?=
<marmarek@invisiblethingslab.com>
Message-ID: <20160209045927.GC3853@localhost.localdomain>
References: <CANQMFx4YULqKctKZqeESesTQjLQun7rQ0ZjGzq96TXTtUw6VWA@mail.gmail.com>
<20160127183005.GB3134@char.us.oracle.com>
<CANQMFx5macG2AbNWtrKjs6o445_Jo7+twMaDg6ozE=0DSD_n7A@mail.gmail.com>
<1454323426.28781.73.camel@citrix.com>
<20160201145053.GA21826@char.us.oracle.com>
<20160203142230.GC24446@mail-itl>
<20160203152657.GE20732@char.us.oracle.com>
<20160208173917.GD24446@mail-itl>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <20160208173917.GD24446@mail-itl>
User-Agent: Mutt/1.5.24 (2015-08-30)
Cc: Tommi Airikka <tommi@airikka.net>, Ian Campbell <ian.campbell@citrix.com>,
810379@bugs.debian.org, xen-devel@lists.xen.org
Subject: Re: [Xen-devel] [BUG] pci-passthrough generates "xen:events: Failed
to obtain physical IRQ" for some devices
I posted it at some point. It was that the MSI-X enable op stashes the
error value in op->value. But 'op->value' is an unsigned int so the
value ends up being 0xfffffe or such. And the other PV frontends only
check for !0 - and manufacture their own value (-EINVAL).
Hence I want to update the pciff.h .. Oh here is the patch:
Oh man. A year?!
Anyhow this can be posted as a cleanup patch seperately of the
bug-fixes.
commit 393be47782bca7a24d3e365448d4d3d1a303abfe
Author: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed Apr 1 17:01:26 2015 -0400
xen/pcifront/pciback: Update pciif.h with ->err and ->result values.
The '->err' should contain only the XEN_PCI_ERR_* type values.
The '->result' may contain -EXX values or any other value
that the XEN_PCI_OP_* deems appropiate.
As such update the header and also the implementations.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Conflicts:
drivers/xen/xen-pciback/pciback_ops.c
Conflicts:
drivers/xen/xen-pciback/pciback_ops.c
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index b1ffebe..353c8a2 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -297,7 +297,7 @@ static int pci_frontend_enable_msix(struct pci_dev *dev,
} else {
dev_err(&dev->dev, "enable msix get err %x\n", err);
}
- return err;
+ return err ? -EINVAL : 0;
}
static void pci_frontend_disable_msix(struct pci_dev *dev)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index fa2b222..4db6c19 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -266,7 +266,7 @@ error:
if (dev_data)
dev_data->ack_intr = 0;
- return result > 0 ? 0 : result;
+ return result >= 0 ? 0 : XEN_PCI_ERR_op_failed;
}
static
diff --git a/include/xen/interface/io/pciif.h b/include/xen/interface/io/pciif.h
index d9922ae..c8b674f 100644
--- a/include/xen/interface/io/pciif.h
+++ b/include/xen/interface/io/pciif.h
@@ -70,7 +70,7 @@ struct xen_pci_op {
/* IN: what action to perform: XEN_PCI_OP_* */
uint32_t cmd;
- /* OUT: will contain an error number (if any) from errno.h */
+ /* OUT: will contain an XEN_PCI_ERR_* number. */
int32_t err;
/* IN: which device to touch */
@@ -82,7 +82,9 @@ struct xen_pci_op {
int32_t offset;
int32_t size;
- /* IN/OUT: Contains the result after a READ or the value to WRITE */
+ /* IN/OUT: Contains the result after a READ or the value to WRITE.
+ * If the err does not have XEN_PCI_ERR_success, depending on
+ * XEN_PCI_OP_* might have the errno value. */
uint32_t value;
/* IN: Contains extra infor for this operation */
uint32_t info;

61
patches.xen/xsa157-0001-xen-pciback-Return-error-on-XEN_PCI_OP_enable_msi-wh.patch
Unescape View File

@ -0,0 +1,61 @@
From e3de4a44cfe196e162ddeffd6379e5c4e75ff1d7 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Fri, 3 Apr 2015 11:08:22 -0400
Subject: [PATCH v2 XSA157 1/5] xen/pciback: Return error on
XEN_PCI_OP_enable_msi when device has MSI or MSI-X enabled
The guest sequence of:
a) XEN_PCI_OP_enable_msi
b) XEN_PCI_OP_enable_msi
c) XEN_PCI_OP_disable_msi
results in hitting an BUG_ON condition in the msi.c code.
The MSI code uses an dev->msi_list to which it adds MSI entries.
Under the above conditions an BUG_ON() can be hit. The device
passed in the guest MUST have MSI capability.
The a) adds the entry to the dev->msi_list and sets msi_enabled.
The b) adds a second entry but adding in to SysFS fails (duplicate entry)
and deletes all of the entries from msi_list and returns (with msi_enabled
is still set). c) pci_disable_msi passes the msi_enabled checks and hits:
BUG_ON(list_empty(dev_to_msi_list(&dev->dev)));
and blows up.
The patch adds a simple check in the XEN_PCI_OP_enable_msi to guard
against that. The check for msix_enabled is not stricly neccessary.
This is part of XSA-157.
CC: stable@vger.kernel.org
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index c4a0666..5ce573a 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -144,7 +144,12 @@ int xen_pcibk_enable_msi(struct xen_pcibk_device *pdev,
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: enable MSI\n", pci_name(dev));
- status = pci_enable_msi(dev);
+ if (dev->msi_enabled)
+ status = -EALREADY;
+ else if (dev->msix_enabled)
+ status = -ENXIO;
+ else
+ status = pci_enable_msi(dev);
if (status) {
pr_warn_ratelimited("%s: error enabling MSI for guest %u: err %d\n",
--
2.1.0

63
patches.xen/xsa157-0002-xen-pciback-Return-error-on-XEN_PCI_OP_enable_msix-w.patch
Unescape View File

@ -0,0 +1,63 @@
From 19b33b70d423ddfea1daf7615eb7f605371a1841 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 2 Nov 2015 18:07:44 -0500
Subject: [PATCH v2 XSA157 2/5] xen/pciback: Return error on
XEN_PCI_OP_enable_msix when device has MSI or MSI-X enabled
The guest sequence of:
a) XEN_PCI_OP_enable_msix
b) XEN_PCI_OP_enable_msix
results in hitting an NULL pointer due to using freed pointers.
The device passed in the guest MUST have MSI-X capability.
The a) constructs and SysFS representation of MSI and MSI groups.
The b) adds a second set of them but adding in to SysFS fails (duplicate entry).
'populate_msi_sysfs' frees the newly allocated msi_irq_groups (note that
in a) pdev->msi_irq_groups is still set) and also free's ALL of the
MSI-X entries of the device (the ones allocated in step a) and b)).
The unwind code: 'free_msi_irqs' deletes all the entries and tries to
delete the pdev->msi_irq_groups (which hasn't been set to NULL).
However the pointers in the SysFS are already freed and we hit an
NULL pointer further on when 'strlen' is attempted on a freed pointer.
The patch adds a simple check in the XEN_PCI_OP_enable_msix to guard
against that. The check for msi_enabled is not stricly neccessary.
This is part of XSA-157
CC: stable@vger.kernel.org
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index 5ce573a..a107928 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -206,9 +206,16 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: enable MSI-X\n",
pci_name(dev));
+
if (op->value > SH_INFO_MAX_VEC)
return -EINVAL;
+ if (dev->msix_enabled)
+ return -EALREADY;
+
+ if (dev->msi_enabled)
+ return -ENXIO;
+
entries = kmalloc(op->value * sizeof(*entries), GFP_KERNEL);
if (entries == NULL)
return -ENOMEM;
--
2.1.0

81
patches.xen/xsa157-0003-xen-pciback-Do-not-install-an-IRQ-handler-for-MSI-in.patch
Unescape View File

@ -0,0 +1,81 @@
From aa48314c60da1035a8e6cc05bec12838a074de98 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 2 Nov 2015 17:24:08 -0500
Subject: [PATCH v2 XSA157 3/5] xen/pciback: Do not install an IRQ handler for
MSI interrupts.
Otherwise an guest can subvert the generic MSI code to trigger
an BUG_ON condition during MSI interrupt freeing:
for (i = 0; i < entry->nvec_used; i++)
BUG_ON(irq_has_action(entry->irq + i));
Xen PCI backed installs an IRQ handler (request_irq) for
the dev->irq whenever the guest writes PCI_COMMAND_MEMORY
(or PCI_COMMAND_IO) to the PCI_COMMAND register. This is
done in case the device has legacy interrupts the GSI line
is shared by the backend devices.
To subvert the backend the guest needs to make the backend
to change the dev->irq from the GSI to the MSI interrupt line,
make the backend allocate an interrupt handler, and then command
the backend to free the MSI interrupt and hit the BUG_ON.
Since the backend only calls 'request_irq' when the guest
writes to the PCI_COMMAND register the guest needs to call
XEN_PCI_OP_enable_msi before any other operation. This will
cause the generic MSI code to setup an MSI entry and
populate dev->irq with the new PIRQ value.
Then the guest can write to PCI_COMMAND PCI_COMMAND_MEMORY
and cause the backend to setup an IRQ handler for dev->irq
(which instead of the GSI value has the MSI pirq). See
'xen_pcibk_control_isr'.
Then the guest disables the MSI: XEN_PCI_OP_disable_msi
which ends up triggering the BUG_ON condition in 'free_msi_irqs'
as there is an IRQ handler for the entry->irq (dev->irq).
Note that this cannot be done using MSI-X as the generic
code does not over-write dev->irq with the MSI-X PIRQ values.
The patch inhibits setting up the IRQ handler if MSI or
MSI-X (for symmetry reasons) code had been called successfully.
P.S.
Xen PCIBack when it sets up the device for the guest consumption
ends up writting 0 to the PCI_COMMAND (see xen_pcibk_reset_device).
XSA-120 addendum patch removed that - however when upstreaming said
addendum we found that it caused issues with qemu upstream. That
has now been fixed in qemu upstream.
This is part of XSA-157
CC: stable@vger.kernel.org
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index a107928..5bb76c0 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -70,6 +70,13 @@ static void xen_pcibk_control_isr(struct pci_dev *dev, int reset)
enable ? "enable" : "disable");
if (enable) {
+ /*
+ * The MSI or MSI-X should not have an IRQ handler. Otherwise
+ * if the guest terminates we BUG_ON in free_msi_irqs.
+ */
+ if (dev->msi_enabled || dev->msix_enabled)
+ goto out;
+
rc = request_irq(dev_data->irq,
xen_pcibk_guest_interrupt, IRQF_SHARED,
dev_data->irq_name, dev);
--
2.1.0

105
patches.xen/xsa157-0004-xen-pciback-For-XEN_PCI_OP_disable_msi-x-only-disabl.patch
Unescape View File

@ -0,0 +1,105 @@
From 59a403750d3796b45376041a4843fcde436ae37e Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 1 Apr 2015 10:49:47 -0400
Subject: [PATCH v2 XSA157 4/5] xen/pciback: For XEN_PCI_OP_disable_msi[|x]
only disable if device has MSI(X) enabled.
Otherwise just continue on, returning the same values as
previously (return of 0, and op->result has the PIRQ value).
This does not change the behavior of XEN_PCI_OP_disable_msi[|x].
The pci_disable_msi or pci_disable_msix have the checks for
msi_enabled or msix_enabled so they will error out immediately.
However the guest can still call these operations and cause
us to disable the 'ack_intr'. That means the backend IRQ handler
for the legacy interrupt will not respond to interrupts anymore.
This will lead to (if the device is causing an interrupt storm)
for the Linux generic code to disable the interrupt line.
Naturally this will only happen if the device in question
is plugged in on the motherboard on shared level interrupt GSI.
This is part of XSA-157
CC: stable@vger.kernel.org
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index 5bb76c0..648c09c 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -185,20 +185,23 @@ static
int xen_pcibk_disable_msi(struct xen_pcibk_device *pdev,
struct pci_dev *dev, struct xen_pci_op *op)
{
- struct xen_pcibk_dev_data *dev_data;
-
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: disable MSI\n",
pci_name(dev));
- pci_disable_msi(dev);
+ if (dev->msi_enabled) {
+ struct xen_pcibk_dev_data *dev_data;
+
+ pci_disable_msi(dev);
+
+ dev_data = pci_get_drvdata(dev);
+ if (dev_data)
+ dev_data->ack_intr = 1;
+ }
op->value = dev->irq ? xen_pirq_from_irq(dev->irq) : 0;
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: MSI: %d\n", pci_name(dev),
op->value);
- dev_data = pci_get_drvdata(dev);
- if (dev_data)
- dev_data->ack_intr = 1;
return 0;
}
@@ -264,23 +267,27 @@ static
int xen_pcibk_disable_msix(struct xen_pcibk_device *pdev,
struct pci_dev *dev, struct xen_pci_op *op)
{
- struct xen_pcibk_dev_data *dev_data;
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: disable MSI-X\n",
pci_name(dev));
- pci_disable_msix(dev);
+ if (dev->msix_enabled) {
+ struct xen_pcibk_dev_data *dev_data;
+
+ pci_disable_msix(dev);
+
+ dev_data = pci_get_drvdata(dev);
+ if (dev_data)
+ dev_data->ack_intr = 1;
+ }
/*
* SR-IOV devices (which don't have any legacy IRQ) have
* an undefined IRQ value of zero.
*/
op->value = dev->irq ? xen_pirq_from_irq(dev->irq) : 0;
if (unlikely(verbose_request))
- printk(KERN_DEBUG DRV_NAME ": %s: MSI-X: %d\n", pci_name(dev),
- op->value);
- dev_data = pci_get_drvdata(dev);
- if (dev_data)
- dev_data->ack_intr = 1;
+ printk(KERN_DEBUG DRV_NAME ": %s: MSI-X: %d\n",
+ pci_name(dev), op->value);
return 0;
}
#endif
--
2.1.0

64
patches.xen/xsa157-0005-xen-pciback-Don-t-allow-MSI-X-ops-if-PCI_COMMAND_MEM.patch
Unescape View File

@ -0,0 +1,64 @@
From e9ab9e04ed76ef06b4ba9a30b3724ca563fdf1fa Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 2 Nov 2015 18:13:27 -0500
Subject: [PATCH v2 XSA157 5/5] xen/pciback: Don't allow MSI-X ops if
PCI_COMMAND_MEMORY is not set.
commit f598282f51 ("PCI: Fix the NIU MSI-X problem in a better way")
teaches us that dealing with MSI-X can be troublesome.
Further checks in the MSI-X architecture shows that if the
PCI_COMMAND_MEMORY bit is turned of in the PCI_COMMAND we
may not be able to access the BAR (since they are memory regions).
Since the MSI-X tables are located in there.. that can lead
to us causing PCIe errors. Inhibit us performing any
operation on the MSI-X unless the MEMORY bit is set.
Note that Xen hypervisor with:
"x86/MSI-X: access MSI-X table only after having enabled MSI-X"
will return:
xen_pciback: 0000:0a:00.1: error -6 enabling MSI-X for guest 3!
When the generic MSI code tries to setup the PIRQ without
MEMORY bit set. Which means with later versions of Xen
(4.6) this patch is not neccessary.
This is part of XSA-157
CC: stable@vger.kernel.org
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index 648c09c..edb9357 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -212,6 +212,7 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
struct xen_pcibk_dev_data *dev_data;
int i, result;
struct msix_entry *entries;
+ u16 cmd;
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: enable MSI-X\n",
@@ -223,7 +224,12 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
if (dev->msix_enabled)
return -EALREADY;
- if (dev->msi_enabled)
+ /*
+ * PCI_COMMAND_MEMORY must be enabled, otherwise we may not be able
+ * to access the BARs where the MSI-X entries reside.
+ */
+ pci_read_config_word(dev, PCI_COMMAND, &cmd);
+ if (dev->msi_enabled || !(cmd & PCI_COMMAND_MEMORY))
return -ENXIO;
entries = kmalloc(op->value * sizeof(*entries), GFP_KERNEL);
--
2.1.0

14
series.conf
Unescape View File

@ -25,3 +25,17 @@ patches.xen/xsa155-linux41-0010-xen-netfront-do-not-use-data-already-exposed-to-
patches.xen/xsa155-linux-0011-xen-netfront-add-range-check-for-Tx-response-id.patch
patches.xen/xsa155-linux312-0012-xen-blkfront-make-local-copy-of-response-before-usin.patch
patches.xen/xsa155-linux319-0013-xen-blkfront-prepare-request-locally-only-then-put-i.patch
patches.xen/xsa157-0001-xen-pciback-Return-error-on-XEN_PCI_OP_enable_msi-wh.patch
patches.xen/xsa157-0002-xen-pciback-Return-error-on-XEN_PCI_OP_enable_msix-w.patch
patches.xen/xsa157-0003-xen-pciback-Do-not-install-an-IRQ-handler-for-MSI-in.patch
patches.xen/xsa157-0004-xen-pciback-For-XEN_PCI_OP_disable_msi-x-only-disabl.patch
patches.xen/xsa157-0005-xen-pciback-Don-t-allow-MSI-X-ops-if-PCI_COMMAND_MEM.patch
# MSI-X enabled device passthrough fix (#1734)
patches.xen/0001-xen-pciback-Check-PF-instead-of-VF-for-PCI_COMMAND_M.patch
patches.xen/0002-xen-pciback-Save-the-number-of-MSI-X-entries-to-be-c.patch
patches.xen/0003-xen-pcifront-Report-the-errors-better.patch
patches.xen/0004-xen-pcifront-Fix-mysterious-crashes-when-NUMA-locali.patch
patches.xen/pci_op-cleanup.patch

Write Preview
Loading…
Cancel
Save