Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755148Ab0DIRwy (ORCPT ); Fri, 9 Apr 2010 13:52:54 -0400 Received: from rcsinet12.oracle.com ([148.87.113.124]:31120 "EHLO rcsinet12.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755008Ab0DIRwv (ORCPT ); Fri, 9 Apr 2010 13:52:51 -0400 Message-ID: <4BBF68AD.9010405@oracle.com> Date: Fri, 09 Apr 2010 10:49:33 -0700 From: Yinghai User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.8) Gecko/20100228 SUSE/3.0.3-1.1.1 Thunderbird/3.0.3 MIME-Version: 1.0 To: David Woodhouse CC: Jesse Barnes , Chris Wright , "Yu, Fenghua" , "linux-kernel@vger.kernel.org" , "linux-pci@vger.kernel.org" , "iommu@lists.linux-foundation.org" Subject: Re: [PATCH 6/6] intel-iommu: Don't call domain_exit if can not attach with iommu References: <1270753107-15881-1-git-send-email-yinghai@kernel.org> <1270753107-15881-7-git-send-email-yinghai@kernel.org> <1270827783.4478.10459.camel@macbook.infradead.org> In-Reply-To: <1270827783.4478.10459.camel@macbook.infradead.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Source-IP: acsmt353.oracle.com [141.146.40.153] X-Auth-Type: Internal IP X-CT-RefId: str=0001.0A090201.4BBF6964.00C8:SCFMA4539814,ss=1,fgs=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6102 Lines: 158 On 04/09/2010 08:43 AM, David Woodhouse wrote: > On Thu, 2010-04-08 at 19:58 +0100, Yinghai Lu wrote: >> >> --- a/drivers/pci/intel-iommu.c >> +++ b/drivers/pci/intel-iommu.c >> @@ -1853,7 +1853,7 @@ static struct dmar_domain >> *get_domain_for_dev(struct pci_dev *pdev, int gaw) >> >> ret = iommu_attach_domain(domain, iommu); >> if (ret) { >> - domain_exit(domain); >> + free_domain_mem(domain); >> goto error; >> } >> > > Um, shouldn't the next error have the same change? If domain_init() > fails before it gets round to initialising the domain->devices list, the > same oops in domain_exit() will occur, won't it? It's because > domain->devices.{next,prev} are both NULL? > thanks. please check Subject: [PATCH] intel-iommu: Don't call domain_exit if can not attach with iommu found when a lot of ixgbevf are used with intel iommu, will get crash > > > > eth304 ip=192.171.178.102 mac=56:95:16:88:4C:C1 pci=0000:0e:18.3 drv=ixgbevf > > eth305 ip=192.171.179.102 mac=D6:41:8C:4A:87:B3 pci=0000:0e:18.5 drv=ixgbevf > > [ 9534.886519] BUG: unable to handle kernel NULL pointer dereference at > > 0000000000000008 > > [ 9534.889775] Call Trace: > > [ 9534.889775] [] domain_remove_dev_info+0x34/0xcf > > [ 9534.889775] [] domain_exit+0x23/0xc4 > > [ 9534.889775] [] T.953+0x173/0x342 > > [ 9534.889775] [] __intel_map_single+0x63/0x1b3 > > [ 9534.889775] [] intel_alloc_coherent+0xc7/0xee > > [ 9534.889775] [] ? ixgbevf_setup_tx_resources+0x2f/0x12c > > [ 9534.889775] [] ixgbevf_setup_tx_resources+0xe8/0x12c > > [ 9534.889775] [] ixgbevf_open+0x7a/0x160 > > [ 9534.889775] [] __dev_open+0x8e/0xbc > > [ 9534.889775] [] __dev_change_flags+0xad/0x130 > > [ 9534.889775] [] dev_change_flags+0x21/0x57 > > [ 9534.889775] [] devinet_ioctl+0x29d/0x541 > > [ 9534.889775] [] ? trace_hardirqs_off_caller+0x1f/0xa9 > > [ 9534.889775] [] inet_ioctl+0x8f/0xa7 > > [ 9534.889775] [] sock_do_ioctl+0x29/0x48 > > [ 9534.889775] [] sock_ioctl+0x1fe/0x20d > > [ 9534.889775] [] vfs_ioctl+0x32/0xa6 > > [ 9534.889775] [] do_vfs_ioctl+0x2b0/0x2cb > > [ 9534.889775] [] ? sysret_check+0x27/0x62 > > [ 9534.889775] [] sys_ioctl+0x47/0x6a > > [ 9534.889775] [] system_call_fastpath+0x16/0x1b > > [ 9534.889775] Code: cd 7f cc ff 41 54 9d 48 83 c4 20 5b 41 5c 41 5d 41 > > 5e c9 c3 55 48 89 e5 e8 11 ff ff ff c9 c3 55 48 89 e5 53 48 89 fb 48 83 > > ec 08 <48> 8b 47 08 4c 8b 00 49 39 f8 74 1d 48 89 f9 48 c7 c2 7f e9 18 > > [ 9534.889775] RIP [] list_del+0xc/0x8b > > [ 9534.889775] RSP > > [ 9534.889775] CR2: 0000000000000008 > > [ 9534.889775] ---[ end trace 072bd8cdb08a760c ]--- > > xifconfig8_2x_vf.sh: line 10: 28555 Killed ifconfig > > $DEV $IP > > when intel_iommu=off or iommu=pt is used, will work well. root cause: domain is initialized after trying attached it, if it can not be attached, don't call domain_exit yet. it will cause kernel crash in domain_exit() after patch will will get error instead of crash. [ 1781.910241] IOMMU: no free domain ids [ 1781.910244] Allocating domain for 0000:d0:1f.5 failed SIOCSIFFLAGS: Cannot allocate memory -v2: DavidW pointed out next error with domain_init could have the same problem. So try to adjust sequence in domain_init() to make sure domain->devices get initialized properly Signed-off-by: Yinghai Lu --- drivers/pci/intel-iommu.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) Index: linux-2.6/drivers/pci/intel-iommu.c =================================================================== --- linux-2.6.orig/drivers/pci/intel-iommu.c +++ linux-2.6/drivers/pci/intel-iommu.c @@ -1379,24 +1379,10 @@ static int domain_init(struct dmar_domai domain_reserve_special_ranges(domain); - /* calculate AGAW */ - iommu = domain_get_iommu(domain); - if (guest_width > cap_mgaw(iommu->cap)) - guest_width = cap_mgaw(iommu->cap); - domain->gaw = guest_width; - adjust_width = guestwidth_to_adjustwidth(guest_width); - agaw = width_to_agaw(adjust_width); - sagaw = cap_sagaw(iommu->cap); - if (!test_bit(agaw, &sagaw)) { - /* hardware doesn't support it, choose a bigger one */ - pr_debug("IOMMU: hardware doesn't support agaw %d\n", agaw); - agaw = find_next_bit(&sagaw, 5, agaw); - if (agaw >= 5) - return -ENODEV; - } - domain->agaw = agaw; INIT_LIST_HEAD(&domain->devices); + iommu = domain_get_iommu(domain); + if (ecap_coherent(iommu->ecap)) domain->iommu_coherency = 1; else @@ -1415,6 +1401,23 @@ static int domain_init(struct dmar_domai if (!domain->pgd) return -ENOMEM; __iommu_flush_cache(iommu, domain->pgd, PAGE_SIZE); + + /* calculate AGAW */ + if (guest_width > cap_mgaw(iommu->cap)) + guest_width = cap_mgaw(iommu->cap); + domain->gaw = guest_width; + adjust_width = guestwidth_to_adjustwidth(guest_width); + agaw = width_to_agaw(adjust_width); + sagaw = cap_sagaw(iommu->cap); + if (!test_bit(agaw, &sagaw)) { + /* hardware doesn't support it, choose a bigger one */ + pr_debug("IOMMU: hardware doesn't support agaw %d\n", agaw); + agaw = find_next_bit(&sagaw, 5, agaw); + if (agaw >= 5) + return -ENODEV; + } + domain->agaw = agaw; + return 0; } @@ -1853,7 +1856,7 @@ static struct dmar_domain *get_domain_fo ret = iommu_attach_domain(domain, iommu); if (ret) { - domain_exit(domain); + free_domain_mem(domain); goto error; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/