Jan Beulich
2013-Aug-22 11:55 UTC
[PATCH] PCI: break MSI-X data out of struct pci_dev_info
Considering that a significant share of PCI devices out there (not the
least the myriad of CPU-exposed ones) don''t support MSI-X at all, and
that the amount of data is well beyond a handful of bytes, break this
out of the common structure, at once allowing the actual data to be
tracked to become architecture specific.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -199,7 +199,7 @@ static void __iomem *msixtbl_addr_to_vir
nr_page = (addr >> PAGE_SHIFT) -
(entry->gtable >> PAGE_SHIFT);
- idx = entry->pdev->msix_table_idx[nr_page];
+ idx = entry->pdev->msix->table_idx[nr_page];
if ( !idx )
return NULL;
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -66,7 +66,7 @@ static void msix_fixmap_free(int idx)
spin_unlock(&msix_fixmap_lock);
}
-static int msix_get_fixmap(struct pci_dev *dev, u64 table_paddr,
+static int msix_get_fixmap(struct arch_msix *msix, u64 table_paddr,
u64 entry_paddr)
{
long nr_page;
@@ -77,48 +77,48 @@ static int msix_get_fixmap(struct pci_de
if ( nr_page < 0 || nr_page >= MAX_MSIX_TABLE_PAGES )
return -EINVAL;
- spin_lock(&dev->msix_table_lock);
- if ( dev->msix_table_refcnt[nr_page]++ == 0 )
+ spin_lock(&msix->table_lock);
+ if ( msix->table_refcnt[nr_page]++ == 0 )
{
idx = msix_fixmap_alloc();
if ( idx < 0 )
{
- dev->msix_table_refcnt[nr_page]--;
+ msix->table_refcnt[nr_page]--;
goto out;
}
set_fixmap_nocache(idx, entry_paddr);
- dev->msix_table_idx[nr_page] = idx;
+ msix->table_idx[nr_page] = idx;
}
else
- idx = dev->msix_table_idx[nr_page];
+ idx = msix->table_idx[nr_page];
out:
- spin_unlock(&dev->msix_table_lock);
+ spin_unlock(&msix->table_lock);
return idx;
}
-static void msix_put_fixmap(struct pci_dev *dev, int idx)
+static void msix_put_fixmap(struct arch_msix *msix, int idx)
{
int i;
- spin_lock(&dev->msix_table_lock);
+ spin_lock(&msix->table_lock);
for ( i = 0; i < MAX_MSIX_TABLE_PAGES; i++ )
{
- if ( dev->msix_table_idx[i] == idx )
+ if ( msix->table_idx[i] == idx )
break;
}
if ( i == MAX_MSIX_TABLE_PAGES )
goto out;
- if ( --dev->msix_table_refcnt[i] == 0 )
+ if ( --msix->table_refcnt[i] == 0 )
{
__set_fixmap(idx, 0, 0);
msix_fixmap_free(idx);
- dev->msix_table_idx[i] = 0;
+ msix->table_idx[i] = 0;
}
out:
- spin_unlock(&dev->msix_table_lock);
+ spin_unlock(&msix->table_lock);
}
/*
@@ -503,7 +503,7 @@ int msi_free_irq(struct msi_desc *entry)
{
unsigned long start;
start = (unsigned long)entry->mask_base & ~(PAGE_SIZE - 1);
- msix_put_fixmap(entry->dev, virt_to_fix(start));
+ msix_put_fixmap(entry->dev->msix, virt_to_fix(start));
nr = 1;
}
@@ -698,6 +698,7 @@ static int msix_capability_init(struct p
struct msi_desc **desc,
unsigned int nr_entries)
{
+ struct arch_msix *msix = dev->msix;
struct msi_desc *entry = NULL;
int pos, vf;
u16 control;
@@ -757,17 +758,17 @@ static int msix_capability_init(struct p
}
table_paddr += table_offset;
- if ( !dev->msix_used_entries )
+ if ( !msix->used_entries )
{
u64 pba_paddr;
u32 pba_offset;
- dev->msix_nr_entries = nr_entries;
- dev->msix_table.first = PFN_DOWN(table_paddr);
- dev->msix_table.last = PFN_DOWN(table_paddr +
- nr_entries * PCI_MSIX_ENTRY_SIZE - 1);
- WARN_ON(rangeset_overlaps_range(mmio_ro_ranges,
dev->msix_table.first,
- dev->msix_table.last));
+ msix->nr_entries = nr_entries;
+ msix->table.first = PFN_DOWN(table_paddr);
+ msix->table.last = PFN_DOWN(table_paddr +
+ nr_entries * PCI_MSIX_ENTRY_SIZE - 1);
+ WARN_ON(rangeset_overlaps_range(mmio_ro_ranges, msix->table.first,
+ msix->table.last));
pba_offset = pci_conf_read32(seg, bus, slot, func,
msix_pba_offset_reg(pos));
@@ -776,18 +777,18 @@ static int msix_capability_init(struct p
WARN_ON(!pba_paddr);
pba_paddr += pba_offset & ~PCI_MSIX_BIRMASK;
- dev->msix_pba.first = PFN_DOWN(pba_paddr);
- dev->msix_pba.last = PFN_DOWN(pba_paddr +
- BITS_TO_LONGS(nr_entries) - 1);
- WARN_ON(rangeset_overlaps_range(mmio_ro_ranges, dev->msix_pba.first,
- dev->msix_pba.last));
+ msix->pba.first = PFN_DOWN(pba_paddr);
+ msix->pba.last = PFN_DOWN(pba_paddr +
+ BITS_TO_LONGS(nr_entries) - 1);
+ WARN_ON(rangeset_overlaps_range(mmio_ro_ranges, msix->pba.first,
+ msix->pba.last));
}
if ( entry )
{
/* Map MSI-X table region */
u64 entry_paddr = table_paddr + msi->entry_nr * PCI_MSIX_ENTRY_SIZE;
- int idx = msix_get_fixmap(dev, table_paddr, entry_paddr);
+ int idx = msix_get_fixmap(msix, table_paddr, entry_paddr);
void __iomem *base;
if ( idx < 0 )
@@ -815,13 +816,13 @@ static int msix_capability_init(struct p
*desc = entry;
}
- if ( !dev->msix_used_entries )
+ if ( !msix->used_entries )
{
- if ( rangeset_add_range(mmio_ro_ranges, dev->msix_table.first,
- dev->msix_table.last) )
+ if ( rangeset_add_range(mmio_ro_ranges, msix->table.first,
+ msix->table.last) )
WARN();
- if ( rangeset_add_range(mmio_ro_ranges, dev->msix_pba.first,
- dev->msix_pba.last) )
+ if ( rangeset_add_range(mmio_ro_ranges, msix->pba.first,
+ msix->pba.last) )
WARN();
if ( dev->domain )
@@ -834,16 +835,16 @@ static int msix_capability_init(struct p
if ( !d )
for_each_domain(d)
if ( !paging_mode_translate(d) &&
- (iomem_access_permitted(d, dev->msix_table.first,
- dev->msix_table.last) ||
- iomem_access_permitted(d, dev->msix_pba.first,
- dev->msix_pba.last)) )
+ (iomem_access_permitted(d, msix->table.first,
+ msix->table.last) ||
+ iomem_access_permitted(d, msix->pba.first,
+ msix->pba.last)) )
break;
if ( d )
{
- if ( !is_hardware_domain(d) && dev->msix_warned !=
d->domain_id )
+ if ( !is_hardware_domain(d) && msix->warned !=
d->domain_id )
{
- dev->msix_warned = d->domain_id;
+ msix->warned = d->domain_id;
printk(XENLOG_ERR
"Potentially insecure use of MSI-X on
%04x:%02x:%02x.%u by Dom%d\n",
seg, bus, slot, func, d->domain_id);
@@ -852,9 +853,9 @@ static int msix_capability_init(struct p
}
}
}
- WARN_ON(dev->msix_nr_entries != nr_entries);
- WARN_ON(dev->msix_table.first != (table_paddr >> PAGE_SHIFT));
- ++dev->msix_used_entries;
+ WARN_ON(msix->nr_entries != nr_entries);
+ WARN_ON(msix->table.first != (table_paddr >> PAGE_SHIFT));
+ ++msix->used_entries;
/* Restore MSI-X enabled bits */
pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), control);
@@ -978,15 +979,15 @@ static int __pci_enable_msix(struct msi_
return status;
}
-static void _pci_cleanup_msix(struct pci_dev *dev)
+static void _pci_cleanup_msix(struct arch_msix *msix)
{
- if ( !--dev->msix_used_entries )
+ if ( !--msix->used_entries )
{
- if ( rangeset_remove_range(mmio_ro_ranges, dev->msix_table.first,
- dev->msix_table.last) )
+ if ( rangeset_remove_range(mmio_ro_ranges, msix->table.first,
+ msix->table.last) )
WARN();
- if ( rangeset_remove_range(mmio_ro_ranges, dev->msix_pba.first,
- dev->msix_pba.last) )
+ if ( rangeset_remove_range(mmio_ro_ranges, msix->pba.first,
+ msix->pba.last) )
WARN();
}
}
@@ -1014,7 +1015,7 @@ static void __pci_disable_msix(struct ms
pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), control);
- _pci_cleanup_msix(dev);
+ _pci_cleanup_msix(dev->msix);
}
int pci_prepare_msix(u16 seg, u8 bus, u8 devfn, bool_t off)
@@ -1035,11 +1036,11 @@ int pci_prepare_msix(u16 seg, u8 bus, u8
pdev = pci_get_pdev(seg, bus, devfn);
if ( !pdev )
rc = -ENODEV;
- else if ( pdev->msix_used_entries != !!off )
+ else if ( pdev->msix->used_entries != !!off )
rc = -EBUSY;
else if ( off )
{
- _pci_cleanup_msix(pdev);
+ _pci_cleanup_msix(pdev->msix);
rc = 0;
}
else
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -179,8 +179,22 @@ static struct pci_dev *alloc_pdev(struct
*((u8*) &pdev->devfn) = devfn;
pdev->domain = NULL;
INIT_LIST_HEAD(&pdev->msi_list);
+
+ if ( pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn),
PCI_FUNC(devfn),
+ PCI_CAP_ID_MSIX) )
+ {
+ struct arch_msix *msix = xzalloc(struct arch_msix);
+
+ if ( !msix )
+ {
+ xfree(pdev);
+ return NULL;
+ }
+ spin_lock_init(&msix->table_lock);
+ pdev->msix = msix;
+ }
+
list_add(&pdev->alldevs_list, &pseg->alldevs_list);
- spin_lock_init(&pdev->msix_table_lock);
/* update bus2bridge */
switch ( pdev->type = pdev_type(pseg->nr, bus, devfn) )
@@ -277,6 +291,7 @@ static void free_pdev(struct pci_seg *ps
}
list_del(&pdev->alldevs_list);
+ xfree(pdev->msix);
xfree(pdev);
}
--- a/xen/include/asm-x86/msi.h
+++ b/xen/include/asm-x86/msi.h
@@ -214,6 +214,22 @@ struct msg_address {
__u32 hi_address;
} __attribute__ ((packed));
+#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_BIRMASK & (PAGE_SIZE -
1)))
+
+struct arch_msix {
+ unsigned int nr_entries, used_entries;
+ struct {
+ unsigned long first, last;
+ } table, pba;
+ int table_refcnt[MAX_MSIX_TABLE_PAGES];
+ int table_idx[MAX_MSIX_TABLE_PAGES];
+ spinlock_t table_lock;
+ domid_t warned;
+};
+
void early_msi_init(void);
void msi_compose_msg(struct irq_desc *, struct msi_msg *);
void __msi_set_enable(u16 seg, u8 bus, u8 slot, u8 func, int pos, int enable);
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -32,10 +32,6 @@
#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 (PCI_MSIX_FLAGS_QSIZE + 1)
-#define MAX_MSIX_TABLE_PAGES PFN_UP(MAX_MSIX_TABLE_ENTRIES * \
- PCI_MSIX_ENTRY_SIZE + \
- (~PCI_MSIX_BIRMASK & (PAGE_SIZE -
1)))
struct pci_dev_info {
bool_t is_extfn;
bool_t is_virtfn;
@@ -50,14 +46,8 @@ struct pci_dev {
struct list_head domain_list;
struct list_head msi_list;
- unsigned int msix_nr_entries, msix_used_entries;
- struct {
- unsigned long first, last;
- } msix_table, msix_pba;
- int msix_table_refcnt[MAX_MSIX_TABLE_PAGES];
- int msix_table_idx[MAX_MSIX_TABLE_PAGES];
- spinlock_t msix_table_lock;
- domid_t msix_warned;
+
+ struct arch_msix *msix;
struct domain *domain;
const u16 seg;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Keir Fraser
2013-Aug-22 18:45 UTC
Re: [PATCH] PCI: break MSI-X data out of struct pci_dev_info
On 22/08/2013 12:55, "Jan Beulich" <JBeulich@suse.com> wrote:> Considering that a significant share of PCI devices out there (not the > least the myriad of CPU-exposed ones) don''t support MSI-X at all, and > that the amount of data is well beyond a handful of bytes, break this > out of the common structure, at once allowing the actual data to be > tracked to become architecture specific. > > Signed-off-by: Jan Beulich <jbeulich@suse.com>Acked-by: Keir Fraser <keir@xen.org>