[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/1 V3] x86/AMD: Fix setup ssss:bb:dd:f for d0 failed
On 9/10/2013 7:47 AM, Jan Beulich wrote: On 09.09.13 at 17:52, <suravee.suthikulpanit@xxxxxxx> wrote:--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -147,9 +147,10 @@ static void amd_iommu_setup_domain_device(amd_iommu_flush_device(iommu, req_id); - AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, "+ AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, type = %x, "Consistency please: Mixing hex and dec numbers in a single message almost requires disambiguating them. Please add the missing #. Ok @@ -175,6 +176,15 @@ static int __init amd_iommu_setup_dom0_device(u8 devfn, struct pci_dev *pdev)if ( unlikely(!iommu) ){ + /* Filter the bridge devices */ + if ( (pdev->type == DEV_TYPE_PCI_HOST_BRIDGE) )Superfluous parentheses. Ok + { + AMD_IOMMU_DEBUG("Skipping host bridge %04x:%02x:%02x.%u (type %x)\n", + pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf), + pdev->type);Saying "host bridge" _and_ being in a conditional only allowing host bridges in makes printing the type redundant. Ok --- a/xen/drivers/passthrough/vtd/intremap.c +++ b/xen/drivers/passthrough/vtd/intremap.c @@ -439,6 +439,9 @@ static void set_msi_source_id(struct pci_dev *pdev, struct iremap_entry *ire) { unsigned int sq;+ case DEV_TYPE_PCI_HOST_BRIDGE:+ break; +This can't be right, as it re-introduces the issue XSA-49 was about. The easiest is perhaps to simply have this handled together with the immediately subsequent cases. Ok, I'll merge them. --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1433,6 +1433,12 @@ static int domain_context_mapping(switch ( pdev->type ){ + case DEV_TYPE_PCI_HOST_BRIDGE: + if (iommu_verbose)Hard tab and missing spaces. Ok + dprintk(VTDPREFIX, "d%d:Hostbridge map skip %04x:%02x:%02x.%u\n", + domain->domain_id, seg, bus, + PCI_SLOT(devfn), PCI_FUNC(devfn)); +Fall-through in a switch statement needs to be annotated, to make clear it is intentional. Ok. I'll put break here. case DEV_TYPE_PCIe_BRIDGE: case DEV_TYPE_PCIe2PCI_BRIDGE: case DEV_TYPE_LEGACY_PCI_BRIDGE: @@ -1563,6 +1569,12 @@ static int domain_context_unmap(switch ( pdev->type ){ + case DEV_TYPE_PCI_HOST_BRIDGE: + if (iommu_verbose)See above. OK + dprintk(VTDPREFIX, "d%d:Hostbridge unmap skip %04x:%02x:%02x.%u\n", + domain->domain_id, seg, bus, + PCI_SLOT(devfn), PCI_FUNC(devfn)); +See above. OK. I'll put "goto out" here. Sorry, I forgot to take this one out. I was going to return -EPERM before for the case when we skipping the host bridges. But I don't think this is needed since I already printing message about skipping hostbridge for when iommu_verbose.case DEV_TYPE_PCIe_BRIDGE: case DEV_TYPE_PCIe2PCI_BRIDGE: case DEV_TYPE_LEGACY_PCI_BRIDGE: @@ -1885,7 +1897,8 @@ static int intel_iommu_add_device(u8 devfn, struct pci_dev *pdev) ret = domain_context_mapping(pdev->domain, devfn, pdev); if ( ret ) { - dprintk(XENLOG_ERR VTDPREFIX, "d%d: context mapping failed\n", + if ( ret != -EPERM ) + dprintk(XENLOG_ERR VTDPREFIX, "d%d: context mapping failed\n", pdev->domain->domain_id);Where's that -EPERM coming from, and what's the reason for not printing the message in that case? Thanks, Suravee _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |