# HG changeset patch # User Keir Fraser # Date 1229694124 0 # Node ID d238101c1832ba178bfc00a20b461fcebe21d5df # Parent 8c35da364ab39605839869d8eb0ac9b831c370f0 VT-d: Fix PCI-X device assignment When assign PCI device, current code just map its bridge and its secondary bus number and devfn 0. It doesn't work for PCI-x device assignment, because the request may be the source-id in the original PCI-X transaction or the source-id provided by the bridge. It needs to map the device itself, and its upstream bridges till PCIe-to-PCI/PCI-x bridge. In addition, add description for DEV_TYPE_PCIe_BRIDGE and DEV_TYPE_PCI_BRIDGE for understandability. Signed-off-by: Weidong Han # HG changeset patch # User Keir Fraser # Date 1231154002 0 # Node ID b3a9bc72624166a230da74c498154ae2cb45eacc # Parent 9cc632cc6d400685679671b6bbc58dfe4c5e287e vtd: avoid redundant context mapping After changeset 18934 (VT-d: Fix PCI-X device assignment), my assigned PCI E1000 NIC doesn't work in guest. The NIC is 03:00.0. Its parent bridge is: 00:1e.0. In domain_context_mapping(): case DEV_TYPE_PCI: After we domain_context_mapping_one() 03:00.0 and 00:1e.0, the 'secbus' is 3 and 'bus' is 0, so we domain_context_mapping_one() 03:00.0 again -- this redundant invocation returns -EINVAL because we have created the mapping but haven't changed pdev->domain from Dom0 to a new domain at this time and eventually the XEN_DOMCTL_assign_device hypercall returns a failure. The attached patch detects this case and avoids the redundant invocation. Signed-off-by: Dexuan Cui --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1155,8 +1155,8 @@ static int domain_context_mapping_one( enum { DEV_TYPE_PCIe_ENDPOINT, - DEV_TYPE_PCIe_BRIDGE, - DEV_TYPE_PCI_BRIDGE, + DEV_TYPE_PCIe_BRIDGE, // PCIe root port, switch + DEV_TYPE_PCI_BRIDGE, // PCIe-to-PCI/PCIx bridge, PCI-to-PCI bridge DEV_TYPE_PCI, }; @@ -1170,7 +1170,8 @@ int pdev_type(u8 bus, u8 devfn) class_device = pci_conf_read16(bus, d, f, PCI_CLASS_DEVICE); if ( class_device == PCI_CLASS_BRIDGE_PCI ) { - pos = pci_find_next_cap(bus, devfn, PCI_CAPABILITY_LIST, PCI_CAP_ID_EXP); + pos = pci_find_next_cap(bus, devfn, + PCI_CAPABILITY_LIST, PCI_CAP_ID_EXP); if ( !pos ) return DEV_TYPE_PCI_BRIDGE; creg = pci_conf_read16(bus, d, f, pos + PCI_EXP_FLAGS); @@ -1219,9 +1220,9 @@ static int domain_context_mapping(struct { struct acpi_drhd_unit *drhd; int ret = 0; - u16 sec_bus, sub_bus, ob, odf; + u16 sec_bus, sub_bus; u32 type; - u8 secbus; + u8 secbus, secdevfn; drhd = acpi_find_matched_drhd_unit(bus, devfn); if ( !drhd ) @@ -1231,15 +1232,13 @@ static int domain_context_mapping(struct switch ( type ) { case DEV_TYPE_PCIe_BRIDGE: + break; + case DEV_TYPE_PCI_BRIDGE: sec_bus = pci_conf_read8(bus, PCI_SLOT(devfn), PCI_FUNC(devfn), PCI_SECONDARY_BUS); sub_bus = pci_conf_read8(bus, PCI_SLOT(devfn), PCI_FUNC(devfn), PCI_SUBORDINATE_BUS); - /*dmar_scope_add_buses(&drhd->scope, sec_bus, sub_bus);*/ - - if ( type == DEV_TYPE_PCIe_BRIDGE ) - break; for ( sub_bus &= 0xff; sec_bus <= sub_bus; sec_bus++ ) { @@ -1258,26 +1257,28 @@ static int domain_context_mapping(struct case DEV_TYPE_PCI: gdprintk(XENLOG_INFO VTDPREFIX, - "domain_context_mapping:PCI: bdf = %x:%x.%x\n", + "domain_context_mapping:PCI: bdf = %x:%x.%x\n", bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); - ob = bus; odf = devfn; - if ( !find_pcie_endpoint(&bus, &devfn, &secbus) ) + ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn); + if ( ret ) + break; + + secbus = bus; + secdevfn = devfn; + /* dependent devices mapping */ + while ( bus2bridge[bus].map ) { - gdprintk(XENLOG_WARNING VTDPREFIX, - "domain_context_mapping:invalid\n"); - break; + secbus = bus; + secdevfn = devfn; + devfn = bus2bridge[bus].devfn; + bus = bus2bridge[bus].bus; + ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn); + if ( ret ) + return ret; } - if ( ob != bus || odf != devfn ) - gdprintk(XENLOG_INFO VTDPREFIX, - "domain_context_mapping:map: " - "bdf = %x:%x.%x -> %x:%x.%x\n", - ob, PCI_SLOT(odf), PCI_FUNC(odf), - bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); - - ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn); - if ( secbus != bus ) + if ( (secbus != bus) && (secdevfn != 0) ) /* * The source-id for transactions on non-PCIe buses seem * to originate from devfn=0 on the secondary bus behind @@ -1285,7 +1286,7 @@ static int domain_context_mapping(struct * these scanarios is not particularly well documented * anywhere. */ - domain_context_mapping_one(domain, drhd->iommu, secbus, 0); + ret = domain_context_mapping_one(domain, drhd->iommu, secbus, 0); break; default: @@ -1339,10 +1340,9 @@ static int domain_context_unmap_one( static int domain_context_unmap(struct domain *domain, u8 bus, u8 devfn) { struct acpi_drhd_unit *drhd; - u16 sec_bus, sub_bus; int ret = 0; u32 type; - u8 secbus; + u8 secbus, secdevfn; drhd = acpi_find_matched_drhd_unit(bus, devfn); if ( !drhd ) @@ -1353,24 +1353,39 @@ static int domain_context_unmap(struct d { case DEV_TYPE_PCIe_BRIDGE: case DEV_TYPE_PCI_BRIDGE: - sec_bus = pci_conf_read8(bus, PCI_SLOT(devfn), PCI_FUNC(devfn), - PCI_SECONDARY_BUS); - sub_bus = pci_conf_read8(bus, PCI_SLOT(devfn), PCI_FUNC(devfn), - PCI_SUBORDINATE_BUS); - /*dmar_scope_remove_buses(&drhd->scope, sec_bus, sub_bus);*/ - if ( DEV_TYPE_PCI_BRIDGE ) - ret = domain_context_unmap_one(domain, drhd->iommu, bus, devfn); break; case DEV_TYPE_PCIe_ENDPOINT: + gdprintk(XENLOG_INFO VTDPREFIX, + "domain_context_unmap:PCIe: bdf = %x:%x.%x\n", + bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); ret = domain_context_unmap_one(domain, drhd->iommu, bus, devfn); break; case DEV_TYPE_PCI: - if ( find_pcie_endpoint(&bus, &devfn, &secbus) ) + gdprintk(XENLOG_INFO VTDPREFIX, + "domain_context_unmap:PCI: bdf = %x:%x.%x\n", + bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); + ret = domain_context_unmap_one(domain, drhd->iommu, bus, devfn); + if ( ret ) + break; + + secbus = bus; + secdevfn = devfn; + /* dependent devices unmapping */ + while ( bus2bridge[bus].map ) + { + secbus = bus; + secdevfn = devfn; + devfn = bus2bridge[bus].devfn; + bus = bus2bridge[bus].bus; ret = domain_context_unmap_one(domain, drhd->iommu, bus, devfn); - if ( bus != secbus ) - domain_context_unmap_one(domain, drhd->iommu, secbus, 0); + if ( ret ) + return ret; + } + + if ( (secbus != bus) && (secdevfn != 0) ) + ret = domain_context_unmap_one(domain, drhd->iommu, secbus, 0); break; default: