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
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 */