[PATCH 0/7] Intel pci: Fix various problems with Intel IOMMU code

[PATCH 0/7] Intel pci: Fix various problems with Intel IOMMU code

[Mike Travis]

	Various problems exist in the Intel IOMMU PCI driver when
	using DMA remapping in 1:1 identity mode on UV systems and
	devices cannot address all of physical memory.

	This patchset addresses those problems.

-- 

[PATCH 1/7] Intel pci: Check for identity mapping candidate using system dma mask

[Mike Travis]

[-- Attachment #1: check-against-reqd-mask.patch --]
[-- Type: text/plain, Size: 1447 bytes --]

	The identity mapping code appears to make the assumption that
	if the devices dma_mask is greater than 32bits the device can
	use identity mapping.  But that is not true, take the case 
	where we have a 40bit device in a 44bit architecture.  The
	device can potentially receive a physical address that it
	will truncate and cause incorrect addresses to be used.

	Instead check to see if the device's dma_mask is large enough
	to address the system's dma_mask.

From: Chris Wright <chrisw@sous-sol.org>
Signed-off-by: Mike Travis <travis@sgi.com>
Reviewed-by: Mike Habeck <habeck@sgi.com>
Cc: stable@kernel.org
---
 drivers/pci/intel-iommu.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

--- linux.orig/drivers/pci/intel-iommu.c
+++ linux/drivers/pci/intel-iommu.c
@@ -2187,8 +2187,19 @@ static int iommu_should_identity_map(str
 	 * Assume that they will -- if they turn out not to be, then we can 
 	 * take them out of the 1:1 domain later.
 	 */
-	if (!startup)
-		return pdev->dma_mask > DMA_BIT_MASK(32);
+	if (!startup) {
+		/*
+		 * If the device's dma_mask is less than the system's memory
+		 * size then this is not a candidate for identity mapping.
+		 */
+		u64 dma_mask = pdev->dma_mask;
+
+		if (pdev->dev.coherent_dma_mask &&
+		    pdev->dev.coherent_dma_mask < dma_mask)
+			dma_mask = pdev->dev.coherent_dma_mask;
+
+		return dma_mask >= dma_get_required_mask(&pdev->dev);
+	}
 
 	return 1;
 }

-- 

[PATCH 2/7] Intel pci: Speed up processing of the identity_mapping function

[Mike Travis]

[-- Attachment #1: speed-up-iommu_no_mapping.patch --]
[-- Type: text/plain, Size: 997 bytes --]

    When there are a large count of PCI devices, and the pass
    through option for iommu is set, much time is spent in the
    identity_mapping function hunting though the iommu domains to
    check if a specific device is "identity mapped".

    Speed up the function by checking the cached info to see if
    it's mapped to the static identity domain.

Signed-off-by: Mike Travis <travis@sgi.com>
Reviewed-by: Mike Habeck <habeck@sgi.com>
Cc: stable@kernel.org
---
 drivers/pci/intel-iommu.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- linux.orig/drivers/pci/intel-iommu.c
+++ linux/drivers/pci/intel-iommu.c
@@ -2106,10 +2106,10 @@ static int identity_mapping(struct pci_d
 	if (likely(!iommu_identity_mapping))
 		return 0;
 
+	info = pdev->dev.archdata.iommu;
+	if (info && info != DUMMY_DEVICE_DOMAIN_INFO)
+		return (info->domain == si_domain);
 
-	list_for_each_entry(info, &si_domain->devices, link)
-		if (info->dev == pdev)
-			return 1;
 	return 0;
 }
 

-- 

[PATCH 3/7] Intel pci: Dont cache iova above 32bit

[Mike Travis]

[-- Attachment #1: dont-cache-iova-above-32bit.patch --]
[-- Type: text/plain, Size: 2193 bytes --]

Mike Travis and Mike Habeck reported an issue where iova allocation
would return a range that was larger than a device's dma mask.

https://lkml.org/lkml/2011/3/29/423

The dmar initialization code will reserve all PCI MMIO regions and copy
those reservations into a domain specific iova tree.  It is possible for
one of those regions to be above the dma mask of a device.  It is typical
to allocate iovas with a 32bit mask (despite device's dma mask possibly
being larger) and cache the result until it exhausts the lower 32bit
address space.  Freeing the iova range that is >= the last iova in the
lower 32bit range when there is still an iova above the 32bit range will
corrupt the cached iova by pointing it to a region that is above 32bit.
If that region is also larger than the device's dma mask, a subsequent
allocation will return an unusable iova and cause dma failure.

Simply don't cache an iova that is above the 32bit caching boundary.

From: Chris Wright <chrisw@sous-sol.org>
Reported-by: Mike Travis <travis@sgi.com>
Reported-by: Mike Habeck <habeck@sgi.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: stable@kernel.org
Acked-by: Mike Travis <travis@sgi.com>
Tested-by: Mike Habeck <habeck@sgi.com>
Signed-off-by: Chris Wright <chrisw@sous-sol.org>
---

v3: rb_next() can return NULL, found when testing on my hw

David, Mike Travis will collect and resumbit full series when he's back.

 drivers/pci/iova.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

--- linux.orig/drivers/pci/iova.c
+++ linux/drivers/pci/iova.c
@@ -63,8 +63,16 @@ __cached_rbnode_delete_update(struct iov
 	curr = iovad->cached32_node;
 	cached_iova = container_of(curr, struct iova, node);
 
-	if (free->pfn_lo >= cached_iova->pfn_lo)
-		iovad->cached32_node = rb_next(&free->node);
+	if (free->pfn_lo >= cached_iova->pfn_lo) {
+		struct rb_node *node = rb_next(&free->node);
+		struct iova *iova = container_of(node, struct iova, node);
+
+		/* only cache if it's below 32bit pfn */
+		if (node && iova->pfn_lo < iovad->dma_32bit_pfn)
+			iovad->cached32_node = node;
+		else
+			iovad->cached32_node = NULL;
+	}
 }
 
 /* Computes the padding size required, to make the

-- 

[PATCH 4/7] Intel pci: Use coherent DMA mask when requested

[Mike Travis]

[-- Attachment #1: use-coherent-dma-mask.patch --]
[-- Type: text/plain, Size: 828 bytes --]

    The __intel_map_single function is not honoring the passed in
    DMA mask.  This results in not using the coherent DMA mask when
    called from intel_alloc_coherent().

Signed-off-by: Mike Travis <travis@sgi.com>
Acked-by: Chris Wright <chrisw@sous-sol.org>
Reviewed-by: Mike Habeck <habeck@sgi.com>
Cc: stable@kernel.org
---
 drivers/pci/intel-iommu.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- linux.orig/drivers/pci/intel-iommu.c
+++ linux/drivers/pci/intel-iommu.c
@@ -2603,8 +2603,7 @@ static dma_addr_t __intel_map_single(str
 	iommu = domain_get_iommu(domain);
 	size = aligned_nrpages(paddr, size);
 
-	iova = intel_alloc_iova(hwdev, domain, dma_to_mm_pfn(size),
-				pdev->dma_mask);
+	iova = intel_alloc_iova(hwdev, domain, dma_to_mm_pfn(size), dma_mask);
 	if (!iova)
 		goto error;
 

-- 

[PATCH 5/7] Intel pci: Remove Host Bridge devices from identity mapping

[Mike Travis]

[-- Attachment #1: remove-pci-host-bridge-devices.patch --]
[-- Type: text/plain, Size: 5850 bytes --]

    When using the 1:1 (identity) PCI DMA remapping, PCI Host Bridge
    devices that do not use the IOMMU causes a kernel panic.  Fix that
    by not inserting those devices into the si_domain.

Signed-off-by: Mike Travis <travis@sgi.com>
Reviewed-by: Mike Habeck <habeck@sgi.com>
Cc: stable@kernel.org
---

Specific panic message:

 IOMMU 0 0xfd020000: using Queued invalidation
 IOMMU 1 0xfd040000: using Queued invalidation
 IOMMU 2 0xfd060000: using Queued invalidation
 IOMMU 3 0xfd080000: using Queued invalidation
 IOMMU 4 0xfd0a0000: using Queued invalidation
 IOMMU 5 0xfd0c0000: using Queued invalidation
 IOMMU: hardware identity mapping for device 1000:3e:00.0
 Failed to setup IOMMU pass-through
 BUG: unable to handle kernel NULL pointer dereference at 000000000000001c
 IP: [<ffffffff816378fd>] _raw_spin_lock_irqsave+0xc/0x23
 PGD 0
 Oops: 0002 [#1] SMP
 last sysfs file:
 CPU 512
 Modules linked in:

 Pid: 1, comm: swapper Not tainted 2.6.39-rc7+ #2 Intel Corp. Stoutland Platform
 RIP: 0010:[<ffffffff816378fd>]  [<ffffffff816378fd>] _raw_spin_lock_irqsave+0xc/0x23
 RSP: 0000:ffff88105fd99d00  EFLAGS: 00010046
 RAX: 0000000000000046 RBX: 0000000000000000 RCX: 0000000000000007
 RDX: 0000000000010000 RSI: 00000000000000b8 RDI: 000000000000001c
 RBP: ffff88105fd99d00 R08: 0000000000000000 R09: ffff88107d400470
 R10: 000000000000000a R11: 0000000000000086 R12: 000000000000001c
 R13: 00000000000000b8 R14: ffff88107d82947c R15: 0000000000000286
 FS:  0000000000000000(0000) GS:ffff88107fd00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
 CR2: 000000000000001c CR3: 0000000001c03000 CR4: 00000000000006e0
 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
 Process swapper (pid: 1, threadinfo ffff88105fd98000, task ffff88105fd90000)
 Stack:
  ffff88105fd99d20 ffffffff812ad179 ffff88107d829400 ffff8b6ffe820f00
  ffff88105fd99d40 ffffffff8108eb2f ffff88105fd99d40 ffff88107d829400
  ffff88105fd99d80 ffffffff8108e396 ffff8beffa854c00 ffff88107d829400
 Call Trace:
  [<ffffffff812ad179>] dmar_msi_mask+0x17/0x3b
  [<ffffffff8108eb2f>] irq_shutdown+0x43/0x4e
  [<ffffffff8108e396>] __free_irq+0xd1/0x15d
  [<ffffffff8108e47b>] free_irq+0x59/0x72
  [<ffffffff812b09b3>] free_dmar_iommu+0x10a/0x1cb
  [<ffffffff812acc7f>] free_iommu+0x16/0x30
  [<ffffffff81e85564>] intel_iommu_init+0x8c1/0x9de
  [<ffffffff81e608c7>] ? dma32_reserve_bootmem+0x6/0x6
  [<ffffffff81e608dd>] pci_iommu_init+0x16/0x41
  [<ffffffff81000218>] do_one_initcall+0x7c/0x130
  [<ffffffff81e5b8a2>] kernel_init+0xf9/0x17e
  [<ffffffff8163eab4>] kernel_thread_helper+0x4/0x10
  [<ffffffff81e5b7a9>] ? do_early_param+0x89/0x89
  [<ffffffff8163eab0>] ? gs_change+0xb/0xb
 Code: b8 00 00 01 00 48 89 e5 f0 0f c1 07 0f b7 d0 c1 e8 10 39 c2 74 07 f3 90 0f b7 17 eb f5 c9 c3 55 48 89 e5 9c 58 fa ba 00 00 01 00 <f0> 0f c1 17 0f b7 ca c1 ea 10 39 d1 74 07 f3 90 0f b7 0f eb f5
 RIP  [<ffffffff816378fd>] _raw_spin_lock_irqsave+0xc/0x23
  RSP <ffff88105fd99d00>
 CR2: 000000000000001c
 ---[ end trace 32b2ebdae113b4c3 ]---
 swapper used greatest stack depth: 3952 bytes left
 Kernel panic - not syncing: Attempted to kill init!
 Pid: 1, comm: swapper Tainted: G      D     2.6.39-rc7+ #2
 Call Trace:
  [<ffffffff81040993>] panic+0xb7/0x1be
  [<ffffffff810338f5>] ? task_rq_unlock+0xc/0xe
  [<ffffffff810340d4>] ? sched_move_task+0xb0/0xbb
  [<ffffffff8107c479>] ? __put_css_set+0x24/0x150
  [<ffffffff81043d0c>] do_exit+0xa9/0x775
  [<ffffffff81040ccb>] ? spin_unlock_irqrestore+0x9/0xb
  [<ffffffff81638931>] oops_end+0xb3/0xbb
  [<ffffffff81027a01>] no_context+0x1f5/0x204
  [<ffffffff81027b92>] __bad_area_nosemaphore+0x182/0x1a5
  [<ffffffff81290b55>] ? __delay+0xa/0xc
  [<ffffffff81290b90>] ? __const_udelay+0x39/0x3b
  [<ffffffff81027c2b>] bad_area_nosemaphore+0xe/0x10
  [<ffffffff8163a65c>] do_page_fault+0x170/0x320
  [<ffffffff81290b55>] ? __delay+0xa/0xc
  [<ffffffff8130f8ec>] ? wait_for_xmitr+0x24/0x89
  [<ffffffff81040b9a>] ? __call_console_drivers+0x78/0x8a
  [<ffffffff8163e36e>] ? apic_timer_interrupt+0xe/0x20
  [<ffffffff81637edf>] page_fault+0x1f/0x30
  [<ffffffff816378fd>] ? _raw_spin_lock_irqsave+0xc/0x23
  [<ffffffff812ad179>] dmar_msi_mask+0x17/0x3b
  [<ffffffff8108eb2f>] irq_shutdown+0x43/0x4e
  [<ffffffff8108e396>] __free_irq+0xd1/0x15d
  [<ffffffff8108e47b>] free_irq+0x59/0x72
  [<ffffffff812b09b3>] free_dmar_iommu+0x10a/0x1cb
  [<ffffffff812acc7f>] free_iommu+0x16/0x30
  [<ffffffff81e85564>] intel_iommu_init+0x8c1/0x9de
  [<ffffffff81e608c7>] ? dma32_reserve_bootmem+0x6/0x6
  [<ffffffff81e608dd>] pci_iommu_init+0x16/0x41
  [<ffffffff81000218>] do_one_initcall+0x7c/0x130
  [<ffffffff81e5b8a2>] kernel_init+0xf9/0x17e
  [<ffffffff8163eab4>] kernel_thread_helper+0x4/0x10
  [<ffffffff81e5b7a9>] ? do_early_param+0x89/0x89
  [<ffffffff8163eab0>] ? gs_change+0xb/0xb

---
 drivers/pci/intel-iommu.c |    5 +++++
 1 file changed, 5 insertions(+)

--- linux.orig/drivers/pci/intel-iommu.c
+++ linux/drivers/pci/intel-iommu.c
@@ -47,6 +47,8 @@
 #define ROOT_SIZE		VTD_PAGE_SIZE
 #define CONTEXT_SIZE		VTD_PAGE_SIZE
 
+#define IS_BRIDGE_HOST_DEVICE(pdev) \
+			    ((pdev->class >> 8) == PCI_CLASS_BRIDGE_HOST)
 #define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
 #define IS_ISA_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA)
 #define IS_AZALIA(pdev) ((pdev)->vendor == 0x8086 && (pdev)->device == 0x3a3e)
@@ -2214,6 +2216,9 @@ static int __init iommu_prepare_static_i
 		return -EFAULT;
 
 	for_each_pci_dev(pdev) {
+		/* Skip Host/PCI Bridge devices */
+		if (IS_BRIDGE_HOST_DEVICE(pdev))
+			continue;
 		if (iommu_should_identity_map(pdev, 1)) {
 			printk(KERN_INFO "IOMMU: %s identity mapping for device %s\n",
 			       hw ? "hardware" : "software", pci_name(pdev));

-- 

[PATCH 6/7] Intel pci: Add domain check in domain_remove_one_dev_info

[Mike Travis]

[-- Attachment #1: fix-domain_remove_one_dev_info.patch --]
[-- Type: text/plain, Size: 1158 bytes --]

	The comment in domain_remove_one_dev_info() states "No need
	to compare PCI domain; it has to be the same".	But for the
	si_domain that isn't going to be true, as it consists of all the
	PCI devices that are identity mapped thus multiple PCI domains can
	be in si_domain.  The code needs to validate the PCI domain too.

From: Mike Habeck <habeck@sgi.com>
Signed-off-by: Mike Habeck <habeck@sgi.com>
Signed-off-by: Mike Travis <travis@sgi.com>
Cc: stable@kernel.org
---
 drivers/pci/intel-iommu.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- linux.orig/drivers/pci/intel-iommu.c
+++ linux/drivers/pci/intel-iommu.c
@@ -3395,8 +3395,8 @@ static void domain_remove_one_dev_info(s
 	spin_lock_irqsave(&device_domain_lock, flags);
 	list_for_each_safe(entry, tmp, &domain->devices) {
 		info = list_entry(entry, struct device_domain_info, link);
-		/* No need to compare PCI domain; it has to be the same */
-		if (info->bus == pdev->bus->number &&
+		if (info->segment == pci_domain_nr(pdev->bus) &&
+		    info->bus == pdev->bus->number &&
 		    info->devfn == pdev->devfn) {
 			list_del(&info->link);
 			list_del(&info->global);

-- 

[PATCH 7/7] Intel pci: Provide option to enable 64-bit IOMMU pass through mode

[Mike Travis]

[-- Attachment #1: indicate-iommu-fixed.patch --]
[-- Type: text/plain, Size: 3018 bytes --]

v2: Use kernel option to implement pass through mode based on this
feedback from David Woodhouse <dwmw2@infradead.org>:

	Alternatively, let your version check just be '>=2.6.40', and
	your backports to the archaeological trees can include a *new*
	kernel command line parameter like 'iommu=pt64' which unfixed
	kernels won't understand and will ignore. So your SCU will provide
	'iommu=pt' for 2.6.40 or newer, and 'iommu=pt64' for older
	kernels. Then at least your hack doesn't affect the *current*
	kernels, and will eventually be lost in the mists of time.

    Prior to these IOMMU patches, the pass through option could not
    be used because it would cause the kernel to panic in certain
    circumstances (primarily having devices with more than 32 bits of
    DMA addressing, but not enough to handle the system's address range).
    In addition, the "force double address cycle" was required to work
    around similar issues which causes a significant reduction in DMA
    performance.

    This patch provides an option to enable the 64bit pass through mode if
    the kernel has the capability of enabling that mode without panicing.
    It also disables "forcedac" unless that option follows the "pt64" option.

Signed-off-by: Mike Travis <travis@sgi.com>
Acked-by: Dimitri Sivanich <sivanich@sgi.com>
Cc: stable@kernel.org
---
 Documentation/kernel-parameters.txt |    8 +++++++-
 drivers/pci/intel-iommu.c           |    5 +++++
 2 files changed, 12 insertions(+), 1 deletion(-)

--- linux.orig/Documentation/kernel-parameters.txt
+++ linux/Documentation/kernel-parameters.txt
@@ -988,13 +988,19 @@ bytes respectively. Such letter suffixes
 			bypassed by not enabling DMAR with this option. In
 			this case, gfx device will use physical address for
 			DMA.
+		pt64 [Default Off]
+			Enable 64 bit pass through mode if the kernel has the
+			capability.  Also disables forcedac, unless that option
+			follows this option.
 		forcedac [x86_64]
 			With this option iommu will not optimize to look
 			for io virtual address below 32 bit forcing dual
 			address cycle on pci bus for cards supporting greater
 			than 32 bit addressing. The default is to look
 			for translation below 32 bit and if not available
-			then look in the higher range.
+			then look in the higher range.  Note that pt64 disables
+			this option, so this must follow that option to be
+			effective.
 		strict [Default Off]
 			With this option on every unmap_single operation will
 			result in a hardware IOTLB flush operation as opposed
--- linux.orig/drivers/pci/intel-iommu.c
+++ linux/drivers/pci/intel-iommu.c
@@ -419,6 +419,11 @@ static int __init intel_iommu_setup(char
 			printk(KERN_INFO
 				"Intel-IOMMU: disable batched IOTLB flush\n");
 			intel_iommu_strict = 1;
+		} else if (!strncmp(str, "pt64", 6)) {
+			pr_info("Intel-IOMMU: enable 64bit passthrough mode, "
+				"disable Forcing DAC for PCI devices\n");
+			iommu_pass_through = 1;
+			dmar_forcedac = 0;
 		}
 
 		str += strcspn(str, ",");

-- 

Re: [PATCH 0/7] Intel pci: Fix various problems with Intel IOMMU code

[David Woodhouse]

On Sat, 2011-05-28 at 13:15 -0500, Mike Travis wrote:
> 	Various problems exist in the Intel IOMMU PCI driver when
> 	using DMA remapping in 1:1 identity mode on UV systems and
> 	devices cannot address all of physical memory.
> 
> 	This patchset addresses those problems.

Patches 1-6 applied to iommu-2.6.git. Please test. Thanks.

Please note for future reference that 'From:' lines such as the one in
your patch 1/7 should be *first* in the body of your email. The git-am
tool doesn't find them and set the authorship if you put them with the
Signed-off-bys. Also please avoid putting whitespace at the beginning of
every line of your commit comments.

I still hate patch 7, but the point was that I wouldn't have to care
about it. Once your patches 1-6 are included in known stable releases
that your SCU can recognise, it can just pass 'iommu=pt' for those.

It's only the "probably buggy" kernels with older release numbers that
you'll pass 'forcedac,pt64' to, so your mission is to get patch 7 or
some variant of it accepted into *those* kernels.

-- 
dwmw2

Re: [PATCH 0/7] Intel pci: Fix various problems with Intel IOMMU code

[Mike Travis]

On 05/28/2011 05:12 PM, David Woodhouse wrote:
> On Sat, 2011-05-28 at 13:15 -0500, Mike Travis wrote:
>> 	Various problems exist in the Intel IOMMU PCI driver when
>> 	using DMA remapping in 1:1 identity mode on UV systems and
>> 	devices cannot address all of physical memory.
>>
>> 	This patchset addresses those problems.
>
> Patches 1-6 applied to iommu-2.6.git. Please test. Thanks.
>
> Please note for future reference that 'From:' lines such as the one in
> your patch 1/7 should be *first* in the body of your email. The git-am
> tool doesn't find them and set the authorship if you put them with the
> Signed-off-bys. Also please avoid putting whitespace at the beginning of
> every line of your commit comments.
>
> I still hate patch 7, but the point was that I wouldn't have to care
> about it. Once your patches 1-6 are included in known stable releases
> that your SCU can recognise, it can just pass 'iommu=pt' for those.
>
> It's only the "probably buggy" kernels with older release numbers that
> you'll pass 'forcedac,pt64' to, so your mission is to get patch 7 or
> some variant of it accepted into *those* kernels.
>

Thanks David, all noted.  The From: thing is swallowed by the
quilt mail command if it's first.  I'll see if I can't fix that.

Cheers.

Re: [PATCH 0/7] Intel pci: Fix various problems with Intel IOMMU code

[David Woodhouse]

On Sun, 2011-05-29 at 01:12 +0100, David Woodhouse wrote:
> Patches 1-6 applied to iommu-2.6.git. Please test. Thanks.

This is all now in Linus' tree for 3.0-rc2, and since you included
'Cc: stable' in the commit comments it should magically turn up in a
stable tree near you.

I will leave you to deal with the question of how to mark 'old' kernels
as actually having had later bugs fixed.

-- 
dwmw2

[PATCH 0/7] Intel pci: Fix various problems with Intel IOMMU code

[Mike Travis]

	Various problems exist in the Intel IOMMU PCI driver when
	using DMA remapping in 1:1 identity mode on UV systems and
	devices cannot address all of physical memory.

	This patchset addresses those problems.

--