From mboxrd@z Thu Jan 1 00:00:00 1970 From: Weidong Han Subject: Re: VT-d device assignment may fail (regression from Xen c/s 19805:2f1fa2215e60) Date: Thu, 28 Oct 2010 08:31:29 +0800 Message-ID: <4CC8C461.9000303@intel.com> References: <4CC17FE5020000780001E91F@vpn.id2.novell.com> <4CC7BBFC.8010802@intel.com> <4CC82A74020000780001F6FC@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4CC82A74020000780001F6FC@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: "Jiang, Yunhong" , "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org Jan Beulich wrote: >>>> On 27.10.10 at 07:43, Weidong Han wrote: >>>> >> Jan Beulich wrote: >> >>> The question now is whether some similar check should be restored, >>> or whether pdev->domain should get updated earlier. This may >>> >>> >> I prefer to add the check. >> > > Like this (not tested yet, simplifying the code a little at once): > > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -1371,23 +1371,16 @@ static int domain_context_mapping(struct > if ( find_upstream_bridge(&bus, &devfn, &secbus) < 1 ) > break; > > - /* PCIe to PCI/PCIx bridge */ > - if ( pdev_type(bus, devfn) == DEV_TYPE_PCIe2PCI_BRIDGE ) > - { > - ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn); > - if ( ret ) > - return ret; > + ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn); > > - /* > - * Devices behind PCIe-to-PCI/PCIx bridge may generate > - * different requester-id. It may originate from devfn=0 > - * on the secondary bus behind the bridge. Map that id > - * as well. > - */ > + /* > + * Devices behind PCIe-to-PCI/PCIx bridge may generate different > + * requester-id. It may originate from devfn=0 on the secondary bus > + * behind the bridge. Map that id as well if we didn't already. > + */ > + if ( !ret && pdev_type(bus, devfn) == DEV_TYPE_PCIe2PCI_BRIDGE && > + (secbus != pdev->bus || pdev->devfn != 0) ) > ret = domain_context_mapping_one(domain, drhd->iommu, secbus, 0); > - } > - else /* Legacy PCI bridge */ > - ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn); > > break; > > > > Looks good. Regards, Weidong