|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen-4.0-testing] x86/vMSI: miscellaneous fixes
# HG changeset patch
# User Jan Beulich <jbeulich@xxxxxxxx>
# Date 1331204417 0
# Node ID c41ab909f08eef8ecf9cfa975e991a767695aed8
# Parent c62e9965b3950a423b5e5dc2fb9b9495d535dc35
x86/vMSI: miscellaneous fixes
This addresses a number of problems in msixtbl_{read,write}():
- address alignment was not checked, allowing for memory corruption in
the hypervisor (write case) or returning of hypervisor private data
to the guest (read case)
- the interrupt mask bit was permitted to be written by the guest
(while Xen's interrupt flow control routines need to control it)
- MAX_MSIX_TABLE_{ENTRIES,PAGES} were pointlessly defined to plain
numbers (making it unobvious why they have these values, and making
the latter non-portable)
- MAX_MSIX_TABLE_PAGES was also off by one (failing to account for a
non-zero table offset); this was also affecting host MSI-X code
- struct msixtbl_entry's table_flags[] was one element larger than
necessary due to improper open-coding of BITS_TO_LONGS()
- msixtbl_read() unconditionally accessed the physical table, even
though the data was only needed in a quarter of all cases
- various calculations were done unnecessarily for both of the rather
distinct code paths in msixtbl_read()
Additionally it is unclear on what basis MAX_MSIX_ACC_ENTRIES was
chosen to be 3.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Committed-by: Keir Fraser <keir@xxxxxxx>
xen-unstable changeset: 24535:fb81b807c154
xen-unstable date: Mon Jan 23 09:35:17 2012 +0000
---
diff -r c62e9965b395 -r c41ab909f08e xen/arch/x86/hvm/vmsi.c
--- a/xen/arch/x86/hvm/vmsi.c Wed Mar 07 09:09:05 2012 +0000
+++ b/xen/arch/x86/hvm/vmsi.c Thu Mar 08 11:00:17 2012 +0000
@@ -157,7 +157,7 @@
struct pci_dev *pdev;
unsigned long gtable; /* gpa of msix table */
unsigned long table_len;
- unsigned long table_flags[MAX_MSIX_TABLE_ENTRIES / BITS_PER_LONG + 1];
+ unsigned long table_flags[BITS_TO_LONGS(MAX_MSIX_TABLE_ENTRIES)];
struct rcu_head rcu;
};
@@ -179,17 +179,14 @@
static void __iomem *msixtbl_addr_to_virt(
struct msixtbl_entry *entry, unsigned long addr)
{
- int idx, nr_page;
+ unsigned int idx, nr_page;
- if ( !entry )
+ if ( !entry || !entry->pdev )
return NULL;
nr_page = (addr >> PAGE_SHIFT) -
(entry->gtable >> PAGE_SHIFT);
- if ( !entry->pdev )
- return NULL;
-
idx = entry->pdev->msix_table_idx[nr_page];
if ( !idx )
return NULL;
@@ -207,11 +204,11 @@
void *virt;
int r = X86EMUL_UNHANDLEABLE;
+ if ( len != 4 || (address & 3) )
+ return r;
+
rcu_read_lock();
- if ( len != 4 )
- goto out;
-
offset = address & (PCI_MSIX_ENTRY_SIZE - 1);
if ( offset != PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET)
goto out;
@@ -235,14 +232,14 @@
unsigned long offset;
struct msixtbl_entry *entry;
void *virt;
- int nr_entry;
+ unsigned int nr_entry;
int r = X86EMUL_UNHANDLEABLE;
+ if ( len != 4 || (address & 3) )
+ return r;
+
rcu_read_lock();
- if ( len != 4 )
- goto out;
-
entry = msixtbl_find_entry(v, address);
nr_entry = (address - entry->gtable) / PCI_MSIX_ENTRY_SIZE;
@@ -261,9 +258,22 @@
if ( !virt )
goto out;
+ /* Do not allow the mask bit to be changed. */
+#if 0 /* XXX
+ * As the mask bit is the only defined bit in the word, and as the
+ * host MSI-X code doesn't preserve the other bits anyway, doing
+ * this is pointless. So for now just discard the write (also
+ * saving us from having to determine the matching irq_desc).
+ */
+ spin_lock_irqsave(&desc->lock, flags);
+ orig = readl(virt);
+ val &= ~PCI_MSIX_VECTOR_BITMASK;
+ val |= orig & PCI_MSIX_VECTOR_BITMASK;
writel(val, virt);
+ spin_unlock_irqrestore(&desc->lock, flags);
+#endif
+
r = X86EMUL_OKAY;
-
out:
rcu_read_unlock();
return r;
diff -r c62e9965b395 -r c41ab909f08e xen/include/asm-x86/msi.h
--- a/xen/include/asm-x86/msi.h Wed Mar 07 09:09:05 2012 +0000
+++ b/xen/include/asm-x86/msi.h Thu Mar 08 11:00:17 2012 +0000
@@ -127,12 +127,6 @@
#define PCI_MSIX_FLAGS_BIRMASK (7 << 0)
#define PCI_MSIX_FLAGS_BITMASK (1 << 0)
-#define PCI_MSIX_ENTRY_SIZE 16
-#define PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET 0
-#define PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET 4
-#define PCI_MSIX_ENTRY_DATA_OFFSET 8
-#define PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET 12
-
#define msi_control_reg(base) (base + PCI_MSI_FLAGS)
#define msi_lower_address_reg(base) (base + PCI_MSI_ADDRESS_LO)
#define msi_upper_address_reg(base) (base + PCI_MSI_ADDRESS_HI)
diff -r c62e9965b395 -r c41ab909f08e xen/include/xen/pci.h
--- a/xen/include/xen/pci.h Wed Mar 07 09:09:05 2012 +0000
+++ b/xen/include/xen/pci.h Thu Mar 08 11:00:17 2012 +0000
@@ -11,6 +11,8 @@
#include <xen/types.h>
#include <xen/list.h>
#include <xen/spinlock.h>
+#include <xen/pci_regs.h>
+#include <asm/page.h>
/*
* The PCI interface treats multi-function devices as independent
@@ -29,8 +31,10 @@
#define PCI_BDF(b,d,f) ((((b) & 0xff) << 8) | PCI_DEVFN(d,f))
#define PCI_BDF2(b,df) ((((b) & 0xff) << 8) | ((df) & 0xff))
-#define MAX_MSIX_TABLE_ENTRIES 2048
-#define MAX_MSIX_TABLE_PAGES 8
+#define MAX_MSIX_TABLE_ENTRIES (PCI_MSIX_FLAGS_QSIZE + 1)
+#define MAX_MSIX_TABLE_PAGES PFN_UP(MAX_MSIX_TABLE_ENTRIES * \
+ PCI_MSIX_ENTRY_SIZE + \
+ (~PCI_MSIX_FLAGS_BIRMASK & (PAGE_SIZE -
1)))
struct pci_dev_info {
unsigned is_extfn;
unsigned is_virtfn;
diff -r c62e9965b395 -r c41ab909f08e xen/include/xen/pci_regs.h
--- a/xen/include/xen/pci_regs.h Wed Mar 07 09:09:05 2012 +0000
+++ b/xen/include/xen/pci_regs.h Thu Mar 08 11:00:17 2012 +0000
@@ -302,6 +302,13 @@
#define PCI_MSIX_FLAGS_ENABLE (1 << 15)
#define PCI_MSIX_FLAGS_MASKALL (1 << 14)
#define PCI_MSIX_FLAGS_BIRMASK (7 << 0)
+
+#define PCI_MSIX_ENTRY_SIZE 16
+#define PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET 0
+#define PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET 4
+#define PCI_MSIX_ENTRY_DATA_OFFSET 8
+#define PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET 12
+
#define PCI_MSIX_FLAGS_BITMASK (1 << 0)
/* CompactPCI Hotswap Register */
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |