xcp-1.6-updates/xen-4.1.hg
changeset 23243:871ed74bbefd
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@suse.com>
Committed-by: Keir Fraser <keir@xen.org>
xen-unstable changeset: 24535:fb81b807c154
xen-unstable date: Mon Jan 23 09:35:17 2012 +0000
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@suse.com>
Committed-by: Keir Fraser <keir@xen.org>
xen-unstable changeset: 24535:fb81b807c154
xen-unstable date: Mon Jan 23 09:35:17 2012 +0000
| author | Jan Beulich <jbeulich@suse.com> |
|---|---|
| date | Wed Mar 07 08:26:25 2012 +0000 (2012-03-07) |
| parents | 96db1984e878 |
| children | d4ae43e71dcf |
| files | xen/arch/x86/hvm/vmsi.c xen/include/asm-x86/msi.h xen/include/xen/pci.h xen/include/xen/pci_regs.h |
line diff
1.1 --- a/xen/arch/x86/hvm/vmsi.c Wed Mar 07 08:22:38 2012 +0000 1.2 +++ b/xen/arch/x86/hvm/vmsi.c Wed Mar 07 08:26:25 2012 +0000 1.3 @@ -158,7 +158,7 @@ struct msixtbl_entry 1.4 struct pci_dev *pdev; 1.5 unsigned long gtable; /* gpa of msix table */ 1.6 unsigned long table_len; 1.7 - unsigned long table_flags[MAX_MSIX_TABLE_ENTRIES / BITS_PER_LONG + 1]; 1.8 + unsigned long table_flags[BITS_TO_LONGS(MAX_MSIX_TABLE_ENTRIES)]; 1.9 #define MAX_MSIX_ACC_ENTRIES 3 1.10 struct { 1.11 uint32_t msi_ad[3]; /* Shadow of address low, high and data */ 1.12 @@ -185,17 +185,14 @@ static struct msixtbl_entry *msixtbl_fin 1.13 static void __iomem *msixtbl_addr_to_virt( 1.14 struct msixtbl_entry *entry, unsigned long addr) 1.15 { 1.16 - int idx, nr_page; 1.17 + unsigned int idx, nr_page; 1.18 1.19 - if ( !entry ) 1.20 + if ( !entry || !entry->pdev ) 1.21 return NULL; 1.22 1.23 nr_page = (addr >> PAGE_SHIFT) - 1.24 (entry->gtable >> PAGE_SHIFT); 1.25 1.26 - if ( !entry->pdev ) 1.27 - return NULL; 1.28 - 1.29 idx = entry->pdev->msix_table_idx[nr_page]; 1.30 if ( !idx ) 1.31 return NULL; 1.32 @@ -208,37 +205,34 @@ static int msixtbl_read( 1.33 struct vcpu *v, unsigned long address, 1.34 unsigned long len, unsigned long *pval) 1.35 { 1.36 - unsigned long offset, val; 1.37 + unsigned long offset; 1.38 struct msixtbl_entry *entry; 1.39 void *virt; 1.40 - int nr_entry, index; 1.41 + unsigned int nr_entry, index; 1.42 int r = X86EMUL_UNHANDLEABLE; 1.43 1.44 + if ( len != 4 || (address & 3) ) 1.45 + return r; 1.46 + 1.47 rcu_read_lock(&msixtbl_rcu_lock); 1.48 1.49 - if ( len != 4 ) 1.50 - goto out; 1.51 - 1.52 entry = msixtbl_find_entry(v, address); 1.53 - virt = msixtbl_addr_to_virt(entry, address); 1.54 - if ( !virt ) 1.55 - goto out; 1.56 + offset = address & (PCI_MSIX_ENTRY_SIZE - 1); 1.57 1.58 - nr_entry = (address - entry->gtable) / PCI_MSIX_ENTRY_SIZE; 1.59 - offset = address & (PCI_MSIX_ENTRY_SIZE - 1); 1.60 - if ( nr_entry >= MAX_MSIX_ACC_ENTRIES && 1.61 - offset != PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET ) 1.62 - goto out; 1.63 - 1.64 - val = readl(virt); 1.65 if ( offset != PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET ) 1.66 { 1.67 + nr_entry = (address - entry->gtable) / PCI_MSIX_ENTRY_SIZE; 1.68 + if ( nr_entry >= MAX_MSIX_ACC_ENTRIES ) 1.69 + goto out; 1.70 index = offset / sizeof(uint32_t); 1.71 *pval = entry->gentries[nr_entry].msi_ad[index]; 1.72 } 1.73 else 1.74 { 1.75 - *pval = val; 1.76 + virt = msixtbl_addr_to_virt(entry, address); 1.77 + if ( !virt ) 1.78 + goto out; 1.79 + *pval = readl(virt); 1.80 } 1.81 1.82 r = X86EMUL_OKAY; 1.83 @@ -253,14 +247,14 @@ static int msixtbl_write(struct vcpu *v, 1.84 unsigned long offset; 1.85 struct msixtbl_entry *entry; 1.86 void *virt; 1.87 - int nr_entry, index; 1.88 + unsigned int nr_entry, index; 1.89 int r = X86EMUL_UNHANDLEABLE; 1.90 1.91 + if ( len != 4 || (address & 3) ) 1.92 + return r; 1.93 + 1.94 rcu_read_lock(&msixtbl_rcu_lock); 1.95 1.96 - if ( len != 4 ) 1.97 - goto out; 1.98 - 1.99 entry = msixtbl_find_entry(v, address); 1.100 nr_entry = (address - entry->gtable) / PCI_MSIX_ENTRY_SIZE; 1.101 1.102 @@ -284,9 +278,22 @@ static int msixtbl_write(struct vcpu *v, 1.103 if ( !virt ) 1.104 goto out; 1.105 1.106 + /* Do not allow the mask bit to be changed. */ 1.107 +#if 0 /* XXX 1.108 + * As the mask bit is the only defined bit in the word, and as the 1.109 + * host MSI-X code doesn't preserve the other bits anyway, doing 1.110 + * this is pointless. So for now just discard the write (also 1.111 + * saving us from having to determine the matching irq_desc). 1.112 + */ 1.113 + spin_lock_irqsave(&desc->lock, flags); 1.114 + orig = readl(virt); 1.115 + val &= ~PCI_MSIX_VECTOR_BITMASK; 1.116 + val |= orig & PCI_MSIX_VECTOR_BITMASK; 1.117 writel(val, virt); 1.118 + spin_unlock_irqrestore(&desc->lock, flags); 1.119 +#endif 1.120 + 1.121 r = X86EMUL_OKAY; 1.122 - 1.123 out: 1.124 rcu_read_unlock(&msixtbl_rcu_lock); 1.125 return r;
2.1 --- a/xen/include/asm-x86/msi.h Wed Mar 07 08:22:38 2012 +0000 2.2 +++ b/xen/include/asm-x86/msi.h Wed Mar 07 08:26:25 2012 +0000 2.3 @@ -119,12 +119,6 @@ int msi_free_irq(struct msi_desc *entry) 2.4 2.5 extern const struct hw_interrupt_type pci_msi_type; 2.6 2.7 -#define PCI_MSIX_ENTRY_SIZE 16 2.8 -#define PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET 0 2.9 -#define PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET 4 2.10 -#define PCI_MSIX_ENTRY_DATA_OFFSET 8 2.11 -#define PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET 12 2.12 - 2.13 #define msi_control_reg(base) (base + PCI_MSI_FLAGS) 2.14 #define msi_lower_address_reg(base) (base + PCI_MSI_ADDRESS_LO) 2.15 #define msi_upper_address_reg(base) (base + PCI_MSI_ADDRESS_HI)
3.1 --- a/xen/include/xen/pci.h Wed Mar 07 08:22:38 2012 +0000 3.2 +++ b/xen/include/xen/pci.h Wed Mar 07 08:26:25 2012 +0000 3.3 @@ -12,6 +12,7 @@ 3.4 #include <xen/list.h> 3.5 #include <xen/spinlock.h> 3.6 #include <xen/irq.h> 3.7 +#include <xen/pci_regs.h> 3.8 3.9 /* 3.10 * The PCI interface treats multi-function devices as independent 3.11 @@ -30,8 +31,10 @@ 3.12 #define PCI_BDF(b,d,f) ((((b) & 0xff) << 8) | PCI_DEVFN(d,f)) 3.13 #define PCI_BDF2(b,df) ((((b) & 0xff) << 8) | ((df) & 0xff)) 3.14 3.15 -#define MAX_MSIX_TABLE_ENTRIES 2048 3.16 -#define MAX_MSIX_TABLE_PAGES 8 3.17 +#define MAX_MSIX_TABLE_ENTRIES (PCI_MSIX_FLAGS_QSIZE + 1) 3.18 +#define MAX_MSIX_TABLE_PAGES PFN_UP(MAX_MSIX_TABLE_ENTRIES * \ 3.19 + PCI_MSIX_ENTRY_SIZE + \ 3.20 + (~PCI_MSIX_BIRMASK & (PAGE_SIZE - 1))) 3.21 struct pci_dev_info { 3.22 unsigned is_extfn; 3.23 unsigned is_virtfn;
4.1 --- a/xen/include/xen/pci_regs.h Wed Mar 07 08:22:38 2012 +0000 4.2 +++ b/xen/include/xen/pci_regs.h Wed Mar 07 08:26:25 2012 +0000 4.3 @@ -305,6 +305,12 @@ 4.4 #define PCI_MSIX_PBA 8 4.5 #define PCI_MSIX_BIRMASK (7 << 0) 4.6 4.7 +#define PCI_MSIX_ENTRY_SIZE 16 4.8 +#define PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET 0 4.9 +#define PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET 4 4.10 +#define PCI_MSIX_ENTRY_DATA_OFFSET 8 4.11 +#define PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET 12 4.12 + 4.13 #define PCI_MSIX_VECTOR_BITMASK (1 << 0) 4.14 4.15 /* CompactPCI Hotswap Register */