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] [<ffffffff81406a3d>] domain_remove_dev_info+0x34/0xcf
> > [ 9534.889775] [<ffffffff81408a1d>] domain_exit+0x23/0xc4
> > [ 9534.889775] [<ffffffff81409c1c>] T.953+0x173/0x342
> > [ 9534.889775] [<ffffffff81409fd0>] __intel_map_single+0x63/0x1b3
> > [ 9534.889775] [<ffffffff8140a22a>] intel_alloc_coherent+0xc7/0xee
> > [ 9534.889775] [<ffffffff816c3e7d>] ? ixgbevf_setup_tx_resources+0x2f/0x12c
> > [ 9534.889775] [<ffffffff816c3f36>] ixgbevf_setup_tx_resources+0xe8/0x12c
> > [ 9534.889775] [<ffffffff816c42d2>] ixgbevf_open+0x7a/0x160
> > [ 9534.889775] [<ffffffff81adf21e>] __dev_open+0x8e/0xbc
> > [ 9534.889775] [<ffffffff81adb716>] __dev_change_flags+0xad/0x130
> > [ 9534.889775] [<ffffffff81adf15a>] dev_change_flags+0x21/0x57
> > [ 9534.889775] [<ffffffff81b211e5>] devinet_ioctl+0x29d/0x541
> > [ 9534.889775] [<ffffffff810a4b92>] ? trace_hardirqs_off_caller+0x1f/0xa9
> > [ 9534.889775] [<ffffffff81b22801>] inet_ioctl+0x8f/0xa7
> > [ 9534.889775] [<ffffffff81acc258>] sock_do_ioctl+0x29/0x48
> > [ 9534.889775] [<ffffffff81acca64>] sock_ioctl+0x1fe/0x20d
> > [ 9534.889775] [<ffffffff8113b86a>] vfs_ioctl+0x32/0xa6
> > [ 9534.889775] [<ffffffff8113bd04>] do_vfs_ioctl+0x2b0/0x2cb
> > [ 9534.889775] [<ffffffff81033c4c>] ? sysret_check+0x27/0x62
> > [ 9534.889775] [<ffffffff8113bd66>] sys_ioctl+0x47/0x6a
> > [ 9534.889775] [<ffffffff81033c1b>] 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 [<ffffffff813ddf94>] list_del+0xc/0x8b
> > [ 9534.889775] RSP <ffff88c070373af8>
> > [ 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 <yinghai@xxxxxxxxxx>
---
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-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
