[Xen-devel] [PATCH, RFC 5/7] IOMMU: consolidate pdev_type() and cache its result for a given device

["Jan Beulich" <JBeulich@xxxxxxxx>]

Add an "unknown" device types as well as one for PCI-to-PCIe bridges
(the latter of which other IOMMU code with or without this patch
doesn't appear to handle properly).

Make sure we don't mistake a device for which we can't access its
config space as a legacy PCI device (after all we in fact don't know
how to deal with such a device, and hence shouldn't try to).

--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -142,7 +142,7 @@ static struct pci_dev *alloc_pdev(struct
     spin_lock_init(&pdev->msix_table_lock);
 
     /* update bus2bridge */
-    switch ( pdev_type(pseg->nr, bus, devfn) )
+    switch ( pdev->type = pdev_type(pseg->nr, bus, devfn) )
     {
         u8 sec_bus, sub_bus;
 
@@ -182,7 +182,7 @@ static struct pci_dev *alloc_pdev(struct
 static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
 {
     /* update bus2bridge */
-    switch ( pdev_type(pseg->nr, pdev->bus, pdev->devfn) )
+    switch ( pdev->type )
     {
         u8 dev, func, sec_bus, sub_bus;
 
@@ -200,6 +200,9 @@ static void free_pdev(struct pci_seg *ps
                 pseg->bus2bridge[sec_bus] = pseg->bus2bridge[pdev->bus];
             spin_unlock(&pseg->bus2bridge_lock);
             break;
+
+        default:
+            break;
     }
 
     list_del(&pdev->alldevs_list);
@@ -587,20 +590,30 @@ void pci_release_devices(struct domain *
 
 #define PCI_CLASS_BRIDGE_PCI     0x0604
 
-int pdev_type(u16 seg, u8 bus, u8 devfn)
+enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
 {
     u16 class_device, creg;
     u8 d = PCI_SLOT(devfn), f = PCI_FUNC(devfn);
     int pos = pci_find_cap_offset(seg, bus, d, f, PCI_CAP_ID_EXP);
 
     class_device = pci_conf_read16(seg, bus, d, f, PCI_CLASS_DEVICE);
-    if ( class_device == PCI_CLASS_BRIDGE_PCI )
+    switch ( class_device )
     {
+    case PCI_CLASS_BRIDGE_PCI:
         if ( !pos )
             return DEV_TYPE_LEGACY_PCI_BRIDGE;
         creg = pci_conf_read16(seg, bus, d, f, pos + PCI_EXP_FLAGS);
-        return ((creg & PCI_EXP_FLAGS_TYPE) >> 4) == PCI_EXP_TYPE_PCI_BRIDGE ?
-            DEV_TYPE_PCIe2PCI_BRIDGE : DEV_TYPE_PCIe_BRIDGE;
+        switch ( (creg & PCI_EXP_FLAGS_TYPE) >> 4 )
+        {
+        case PCI_EXP_TYPE_PCI_BRIDGE:
+            return DEV_TYPE_PCIe2PCI_BRIDGE;
+        case PCI_EXP_TYPE_PCIE_BRIDGE:
+            return DEV_TYPE_PCI2PCIe_BRIDGE;
+        }
+        return DEV_TYPE_PCIe_BRIDGE;
+
+    case 0x0000: case 0xffff:
+        return DEV_TYPE_PCI_UNKNOWN;
     }
 
     return pos ? DEV_TYPE_PCIe_ENDPOINT : DEV_TYPE_PCI;
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -430,7 +430,6 @@ void io_apic_write_remap_rte(
 
 static void set_msi_source_id(struct pci_dev *pdev, struct iremap_entry *ire)
 {
-    int type;
     u16 seg;
     u8 bus, devfn, secbus;
     int ret;
@@ -441,8 +440,7 @@ static void set_msi_source_id(struct pci
     seg = pdev->seg;
     bus = pdev->bus;
     devfn = pdev->devfn;
-    type = pdev_type(seg, bus, devfn);
-    switch ( type )
+    switch ( pdev->type )
     {
     case DEV_TYPE_PCIe_BRIDGE:
     case DEV_TYPE_PCIe2PCI_BRIDGE:
@@ -474,7 +472,7 @@ static void set_msi_source_id(struct pci
     default:
         dprintk(XENLOG_WARNING VTDPREFIX,
                 "d%d: unknown(%u): %04x:%02x:%02x.%u\n",
-                pdev->domain->domain_id, type,
+                pdev->domain->domain_id, pdev->type,
                 seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
         break;
    }
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1419,7 +1419,6 @@ static int domain_context_mapping(
 {
     struct acpi_drhd_unit *drhd;
     int ret = 0;
-    u32 type;
     u8 seg = pdev->seg, bus = pdev->bus, secbus;
 
     drhd = acpi_find_matched_drhd_unit(pdev);
@@ -1428,8 +1427,7 @@ static int domain_context_mapping(
 
     ASSERT(spin_is_locked(&pcidevs_lock));
 
-    type = pdev_type(seg, bus, devfn);
-    switch ( type )
+    switch ( pdev->type )
     {
     case DEV_TYPE_PCIe_BRIDGE:
     case DEV_TYPE_PCIe2PCI_BRIDGE:
@@ -1479,7 +1477,7 @@ static int domain_context_mapping(
 
     default:
         dprintk(XENLOG_ERR VTDPREFIX, "d%d:unknown(%u): %04x:%02x:%02x.%u\n",
-                domain->domain_id, type,
+                domain->domain_id, pdev->type,
                 seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
         ret = -EINVAL;
         break;
@@ -1551,7 +1549,6 @@ static int domain_context_unmap(
     struct acpi_drhd_unit *drhd;
     struct iommu *iommu;
     int ret = 0;
-    u32 type;
     u8 seg = pdev->seg, bus = pdev->bus, tmp_bus, tmp_devfn, secbus;
     int found = 0;
 
@@ -1560,8 +1557,7 @@ static int domain_context_unmap(
         return -ENODEV;
     iommu = drhd->iommu;
 
-    type = pdev_type(seg, bus, devfn);
-    switch ( type )
+    switch ( pdev->type )
     {
     case DEV_TYPE_PCIe_BRIDGE:
     case DEV_TYPE_PCIe2PCI_BRIDGE:
@@ -1608,7 +1604,7 @@ static int domain_context_unmap(
 
     default:
         dprintk(XENLOG_ERR VTDPREFIX, "d%d:unknown(%u): %04x:%02x:%02x.%u\n",
-                domain->domain_id, type,
+                domain->domain_id, pdev->type,
                 seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
         ret = -EINVAL;
         goto out;
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -62,6 +62,17 @@ struct pci_dev {
     const u16 seg;
     const u8 bus;
     const u8 devfn;
+
+    enum pdev_type {
+        DEV_TYPE_PCI_UNKNOWN,
+        DEV_TYPE_PCIe_ENDPOINT,
+        DEV_TYPE_PCIe_BRIDGE,       // PCIe root port, switch
+        DEV_TYPE_PCIe2PCI_BRIDGE,   // PCIe-to-PCI/PCIx bridge
+        DEV_TYPE_PCI2PCIe_BRIDGE,   // PCI/PCIx-to-PCIe bridge
+        DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
+        DEV_TYPE_PCI,
+    } type;
+
     struct pci_dev_info info;
     struct arch_pci_dev arch;
     struct {
@@ -83,18 +94,10 @@ struct pci_dev {
 
 extern spinlock_t pcidevs_lock;
 
-enum {
-    DEV_TYPE_PCIe_ENDPOINT,
-    DEV_TYPE_PCIe_BRIDGE,       // PCIe root port, switch
-    DEV_TYPE_PCIe2PCI_BRIDGE,   // PCIe-to-PCI/PCIx bridge
-    DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
-    DEV_TYPE_PCI,
-};
-
 bool_t pci_known_segment(u16 seg);
 int pci_device_detect(u16 seg, u8 bus, u8 dev, u8 func);
 int scan_pci_devices(void);
-int pdev_type(u16 seg, u8 bus, u8 devfn);
+enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn);
 int find_upstream_bridge(u16 seg, u8 *bus, u8 *devfn, u8 *secbus);
 struct pci_dev *pci_lock_pdev(int seg, int bus, int devfn);
 struct pci_dev *pci_lock_domain_pdev(
--- a/xen/include/xen/pci_regs.h
+++ b/xen/include/xen/pci_regs.h
@@ -371,6 +371,9 @@
 #define  PCI_EXP_TYPE_UPSTREAM 0x5     /* Upstream Port */
 #define  PCI_EXP_TYPE_DOWNSTREAM 0x6   /* Downstream Port */
 #define  PCI_EXP_TYPE_PCI_BRIDGE 0x7   /* PCI/PCI-X Bridge */
+#define  PCI_EXP_TYPE_PCIE_BRIDGE 0x8  /* PCI/PCI-X to PCIE Bridge */
+#define  PCI_EXP_TYPE_RC_END   0x9     /* Root Complex Integrated Endpoint */
+#define  PCI_EXP_TYPE_RC_EC    0xa     /* Root Complex Event Collector */
 #define PCI_EXP_FLAGS_SLOT     0x0100  /* Slot implemented */
 #define PCI_EXP_FLAGS_IRQ      0x3e00  /* Interrupt message number */
 #define PCI_EXP_DEVCAP         4       /* Device capabilities */

Attachment: IOMMU-pdev-type.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel