Re: [PATCH v5 0/7] Convert the intel iommu driver to the dma-iommu api

2020-11-20 Thread Chris Wilson
Quoting Lu Baolu (2020-11-20 10:17:12)
> Lu Baolu (3):
>   iommu: Add quirk for Intel graphic devices in map_sg
>   iommu/vt-d: Update domain geometry in iommu_ops.at(de)tach_dev
>   iommu/vt-d: Cleanup after converting to dma-iommu ops
> 
> Tom Murphy (4):
>   iommu: Handle freelists when using deferred flushing in iommu drivers
>   iommu: Add iommu_dma_free_cpu_cached_iovas()
>   iommu: Allow the dma-iommu api to use bounce buffers
>   iommu/vt-d: Convert intel iommu driver to the iommu ops

Something that may be of interest is that we encounter problems with
using intel-iommu across a PCI remove event. All HW generations fail
with faults like:

DMAR: DRHD: handling fault status reg 3
DMAR: [DMA Write] Request device [00:02.0] PASID  fault addr 4b822000 
[fault reason 02] Present bit in context entry is clear

i.e. they all report missing present bit after re-adding the device to the
iommu group. Forcing an identity map (or disabling iommu) works fine.

I applied this series just on the off-chance it changed the symptoms; it
does not. If you have any ideas on how to chase down this fault, that
would be very useful. We have a few other DMAR faults visible on many
platforms, all "[fault reason 07] Next page table ptr is invalid" that
are again not affected by this series, that we also need to resolve.
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 07/15] iommu/vt-d: Delegate the dma domain to upper layer

2020-08-24 Thread Chris Wilson
Quoting Lu Baolu (2020-08-24 07:31:23)
> Hi Chris,
> 
> On 2020/8/22 2:33, Chris Wilson wrote:
> > Quoting Lu Baolu (2019-05-25 06:41:28)
> >> This allows the iommu generic layer to allocate a dma domain and
> >> attach it to a device through the iommu api's. With all types of
> >> domains being delegated to upper layer, we can remove an internal
> >> flag which was used to distinguish domains mananged internally or
> >> externally.
> > 
> > I'm seeing some really strange behaviour with this patch on a 32b
> > Skylake system (and still present on mainline). Before this patch
> > everything is peaceful and appears to work correctly. Applying this patch,
> > and we fail to initialise the GPU with a few DMAR errors reported, e.g.
> > 
> > [   20.279445] DMAR: DRHD: handling fault status reg 3
> > [   20.279508] DMAR: [DMA Read] Request device [00:02.0] fault addr 
> > 8900a000 [fault reason 05] PTE Write access is not set
> > 
> > Setting an identity map for the igfx made the DMAR errors disappear, but
> > the GPU still failed to initialise.
> > 
> > There's no difference in the DMAR configuration dmesg between working and
> > the upset patch. And the really strange part is that switching to a 64b
> > kernel with this patch, it's working.
> > 
> > Any suggestions on what I should look for?
> 
> Can the patch titled "[PATCH] iommu/intel: Handle 36b addressing for
> x86-32" solve this problem?

It does. Not sure why, but that mystery I can leave for others.
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/intel: Handle 36b addressing for x86-32

2020-08-22 Thread Chris Wilson
Quoting Chris Wilson (2020-08-22 17:02:09)
> Beware that the address size for x86-32 may exceed unsigned long.
> 
> [0.368971] UBSAN: shift-out-of-bounds in 
> drivers/iommu/intel/iommu.c:128:14
> [0.369055] shift exponent 36 is too large for 32-bit type 'long unsigned 
> int'
> 
> If we don't handle the wide addresses, the pages are mismapped and the
> device read/writes go astray, detected as DMAR faults and leading to
> device failure. The behaviour changed (from working to broken) in commit
> fa954e683178 ("iommu/vt-d: Delegate the dma domain to upper layer"), but
> the error looks older.
> 
> Fixes: fa954e683178 ("iommu/vt-d: Delegate the dma domain to upper layer")
> Signed-off-by: Chris Wilson 
> Cc: James Sewart 
> Cc: Lu Baolu 
> Cc: Joerg Roedel 
> Cc:  # v5.3+
> ---
>  drivers/iommu/intel/iommu.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 2e9c8c3d0da4..ba78a2e854f9 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -123,29 +123,29 @@ static inline unsigned int level_to_offset_bits(int 
> level)
> return (level - 1) * LEVEL_STRIDE;
>  }
>  
> -static inline int pfn_level_offset(unsigned long pfn, int level)
> +static inline int pfn_level_offset(u64 pfn, int level)

Maybe s/u64/dma_addr_t/ ? I'm not sure what is the appropriate type,
just that this makes i915 not try and eat itself. :)
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] iommu/intel: Handle 36b addressing for x86-32

2020-08-22 Thread Chris Wilson
Beware that the address size for x86-32 may exceed unsigned long.

[0.368971] UBSAN: shift-out-of-bounds in drivers/iommu/intel/iommu.c:128:14
[0.369055] shift exponent 36 is too large for 32-bit type 'long unsigned 
int'

If we don't handle the wide addresses, the pages are mismapped and the
device read/writes go astray, detected as DMAR faults and leading to
device failure. The behaviour changed (from working to broken) in commit
fa954e683178 ("iommu/vt-d: Delegate the dma domain to upper layer"), but
the error looks older.

Fixes: fa954e683178 ("iommu/vt-d: Delegate the dma domain to upper layer")
Signed-off-by: Chris Wilson 
Cc: James Sewart 
Cc: Lu Baolu 
Cc: Joerg Roedel 
Cc:  # v5.3+
---
 drivers/iommu/intel/iommu.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 2e9c8c3d0da4..ba78a2e854f9 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -123,29 +123,29 @@ static inline unsigned int level_to_offset_bits(int level)
return (level - 1) * LEVEL_STRIDE;
 }
 
-static inline int pfn_level_offset(unsigned long pfn, int level)
+static inline int pfn_level_offset(u64 pfn, int level)
 {
return (pfn >> level_to_offset_bits(level)) & LEVEL_MASK;
 }
 
-static inline unsigned long level_mask(int level)
+static inline u64 level_mask(int level)
 {
-   return -1UL << level_to_offset_bits(level);
+   return -1ULL << level_to_offset_bits(level);
 }
 
-static inline unsigned long level_size(int level)
+static inline u64 level_size(int level)
 {
-   return 1UL << level_to_offset_bits(level);
+   return 1ULL << level_to_offset_bits(level);
 }
 
-static inline unsigned long align_to_level(unsigned long pfn, int level)
+static inline u64 align_to_level(u64 pfn, int level)
 {
return (pfn + level_size(level) - 1) & level_mask(level);
 }
 
 static inline unsigned long lvl_to_nr_pages(unsigned int lvl)
 {
-   return  1 << min_t(int, (lvl - 1) * LEVEL_STRIDE, MAX_AGAW_PFN_WIDTH);
+   return 1UL << min_t(int, (lvl - 1) * LEVEL_STRIDE, MAX_AGAW_PFN_WIDTH);
 }
 
 /* VT-d pages must always be _smaller_ than MM pages. Otherwise things
-- 
2.20.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 07/15] iommu/vt-d: Delegate the dma domain to upper layer

2020-08-21 Thread Chris Wilson
Quoting Lu Baolu (2019-05-25 06:41:28)
> This allows the iommu generic layer to allocate a dma domain and
> attach it to a device through the iommu api's. With all types of
> domains being delegated to upper layer, we can remove an internal
> flag which was used to distinguish domains mananged internally or
> externally.

I'm seeing some really strange behaviour with this patch on a 32b
Skylake system (and still present on mainline). Before this patch
everything is peaceful and appears to work correctly. Applying this patch,
and we fail to initialise the GPU with a few DMAR errors reported, e.g.

[   20.279445] DMAR: DRHD: handling fault status reg 3
[   20.279508] DMAR: [DMA Read] Request device [00:02.0] fault addr 8900a000 
[fault reason 05] PTE Write access is not set

Setting an identity map for the igfx made the DMAR errors disappear, but
the GPU still failed to initialise.

There's no difference in the DMAR configuration dmesg between working and
the upset patch. And the really strange part is that switching to a 64b
kernel with this patch, it's working.

Any suggestions on what I should look for?
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] iommu/intel: Use fallback generic_device_group() for ACPI devices

2019-10-04 Thread Chris Wilson
[2.073922] DMAR: ACPI device "INT33C2:00" under DMAR at fed91000 as 00:15.1
[2.073983] DMAR: ACPI device "INT33C3:00" under DMAR at fed91000 as 00:15.2
[2.074027] DMAR: ACPI device "INT33C0:00" under DMAR at fed91000 as 00:15.3
[2.074072] DMAR: ACPI device "INT33C1:00" under DMAR at fed91000 as 00:15.4
[2.074114] DMAR: Failed to find handle for ACPI object \_SB.PCI0.UA01
[2.074156] DMAR: Failed to find handle for ACPI object \_SB.PCI0.SDHC
[2.074208] DMAR: No ATSR found
[2.074572] DMAR: dmar0: Using Queued invalidation
[2.074629] DMAR: dmar1: Using Queued invalidation
[2.110029] pci :00:00.0: Adding to iommu group 0
[2.115703] pci :00:02.0: Adding to iommu group 1
[2.116221] pci :00:03.0: Adding to iommu group 2
[2.116759] pci :00:14.0: Adding to iommu group 3
[2.117276] pci :00:16.0: Adding to iommu group 4
[2.117762] pci :00:1b.0: Adding to iommu group 5
[2.118264] pci :00:1c.0: Adding to iommu group 6
[2.118733] pci :00:1c.2: Adding to iommu group 7
[2.119289] pci :00:1d.0: Adding to iommu group 8
[2.119846] pci :00:1f.0: Adding to iommu group 9
[2.119960] pci :00:1f.2: Adding to iommu group 9
[2.120073] pci :00:1f.3: Adding to iommu group 9
[2.120549] pci :06:00.0: Adding to iommu group 10
[2.120631] [ cut here ]
[2.120681] WARNING: CPU: 2 PID: 1 at drivers/iommu/iommu.c:1275 
pci_device_group+0x109/0x120
[2.120723] Modules linked in:
[2.120744] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 
5.4.0-rc1-CI-CI_DRM_7000+ #1
[2.120782] Hardware name: Dell Inc. XPS 12-9Q33/XPS 12-9Q33, BIOS A04 
12/03/2013
[2.120821] RIP: 0010:pci_device_group+0x109/0x120
[2.120848] Code: e9 ff ff 48 85 c0 48 89 c5 75 bd 48 8d 74 24 10 4c 89 e7 
e8 49 ea ff ff 48 85 c0 48 89 c5 75 a8 e8 fc ee ff ff 48 89 c5 eb 9e <0f> 0b 48 
c7 c5 ea ff ff ff eb 93 e8 37 5f a7 ff 0f 1f 80 00 00 00
[2.120933] RSP: :c9037cd0 EFLAGS: 00010202
[2.120961] RAX: 81639810 RBX: ffea RCX: 
[2.120996] RDX:  RSI: 403efd19 RDI: 88811c08
[2.121031] RBP: 88811c08 R08: 88811a5188f8 R09: fffe
[2.121066] R10: ca7d066a R11: 2161dc90 R12: 888118320a58
[2.121100] R13: 888119fc1e50 R14: 0001 R15: 888119fc2300
[2.121136] FS:  () GS:88811b90() 
knlGS:
[2.121176] CS:  0010 DS:  ES:  CR0: 80050033
[2.121205] CR2:  CR3: 05210001 CR4: 001606e0
[2.121240] Call Trace:
[2.121264]  iommu_group_get_for_dev+0x77/0x210
[2.121295]  intel_iommu_add_device+0x54/0x1c0
[2.121323]  iommu_probe_device+0x43/0xc0
[2.121350]  intel_iommu_init+0x11fb/0x12c9
[2.121383]  ? set_debug_rodata+0xc/0xc
[2.121410]  ? set_debug_rodata+0xc/0xc
[2.121434]  ? e820__memblock_setup+0x5b/0x5b
[2.121458]  ? pci_iommu_init+0x11/0x3a
[2.121471]  ? rcu_read_lock_sched_held+0x4d/0x80
[2.121471]  pci_iommu_init+0x11/0x3a
[2.121471]  do_one_initcall+0x58/0x2ff
[2.121471]  ? set_debug_rodata+0xc/0xc
[2.121471]  ? rcu_read_lock_sched_held+0x4d/0x80
[2.121471]  kernel_init_freeable+0x137/0x1c7
[2.121471]  ? rest_init+0x250/0x250
[2.121471]  kernel_init+0x5/0x100
[2.121471]  ret_from_fork+0x3a/0x50
[2.121471] irq event stamp: 1252438
[2.121471] hardirqs last  enabled at (1252437): [] 
__slab_alloc.isra.84.constprop.89+0x4d/0x70
[2.121471] hardirqs last disabled at (1252438): [] 
trace_hardirqs_off_thunk+0x1a/0x20
[2.121471] softirqs last  enabled at (1252382): [] 
__do_softirq+0x385/0x47f
[2.121471] softirqs last disabled at (1252375): [] 
irq_exit+0xba/0xc0
[2.121471] ---[ end trace 610717c918cf08f3 ]---
[2.121974] DMAR: ACPI name space devices didn't probe correctly
[2.122069] DMAR: Intel(R) Virtualization Technology for Directed I/O

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111906
Signed-off-by: Chris Wilson 
Cc: Joerg Roedel 
---
Take the patch with a pinch of salt; it seems to be the pattern used by
other iommu backends, but I don't know if it is even suitable for iommu
and what appear to be ACPI devices rather than the expected PCI.
---
 drivers/iommu/intel-iommu.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index ed321b808176..e231be0d0534 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5939,6 +5939,14 @@ static bool intel_iommu_is_attach_deferred(struct 
iommu_domain *domain,
return dev->archdata.iommu == DEFER_DEVICE_DOMAIN_INFO;
 }
 
+static struct iommu_group *intel_iommu_device_group(struct device *dev)
+{
+   if (dev_is_pci(dev))
+

[PATCH] iommu/intel: Declare Broadwell igfx dmar support snafu

2019-09-07 Thread Chris Wilson
Despite the widespread and complete failure of Broadwell integrated
graphics when DMAR is enabled, known over the years, we have never been
able to root cause the issue. Instead, we let the failure undermine our
confidence in the iommu system itself when we should be pushing for it to
be always enabled. Quirk away Broadwell and remove the rotten apple.

References: https://bugs.freedesktop.org/show_bug.cgi?id=89360
Signed-off-by: Chris Wilson 
Cc: Lu Baolu 
Cc: Martin Peres 
Cc: Joerg Roedel 
---
 drivers/iommu/intel-iommu.c | 44 +
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 43f667379a37..c4fb1ddea4dd 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5637,20 +5637,46 @@ const struct iommu_ops intel_iommu_ops = {
.pgsize_bitmap  = INTEL_IOMMU_PGSIZES,
 };
 
-static void quirk_iommu_g4x_gfx(struct pci_dev *dev)
+static void quirk_iommu_igfx(struct pci_dev *dev)
 {
-   /* G4x/GM45 integrated gfx dmar support is totally busted. */
pci_info(dev, "Disabling IOMMU for graphics on this chipset\n");
dmar_map_gfx = 0;
 }
 
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_g4x_gfx);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_g4x_gfx);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e10, quirk_iommu_g4x_gfx);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e20, quirk_iommu_g4x_gfx);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e30, quirk_iommu_g4x_gfx);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e40, quirk_iommu_g4x_gfx);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e90, quirk_iommu_g4x_gfx);
+/* G4x/GM45 integrated gfx dmar support is totally busted. */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e10, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e20, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e30, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e40, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e90, quirk_iommu_igfx);
+
+/* Broadwell igfx malfunctions with dmar */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1606, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x160B, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x160E, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1602, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x160A, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x160D, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1616, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x161B, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x161E, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1612, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x161A, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x161D, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1626, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x162B, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x162E, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1622, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x162A, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x162D, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1636, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163B, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163E, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1632, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163A, quirk_iommu_igfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163D, quirk_iommu_igfx);
 
 static void quirk_iommu_rwbf(struct pci_dev *dev)
 {
-- 
2.23.0

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/iova: Remove stale cached32_node

2019-07-22 Thread Chris Wilson
Quoting Robin Murphy (2019-07-22 11:13:49)
> Hi Chris,
> 
> On 20/07/2019 19:08, Chris Wilson wrote:
> > Since the cached32_node is allowed to be advanced above dma_32bit_pfn
> > (to provide a shortcut into the limited range), we need to be careful to
> > remove the to be freed node if it is the cached32_node.
> 
> Thanks for digging in - just to confirm my understanding, the 
> problematic situation is where both 32-bit and 64-bit nodes have been 
> allocated, the topmost 32-bit node is freed, then the lowest 64-bit node 
> is freed, which leaves the pointer dangling because the second free 
> fails the "free->pfn_hi < iovad->dma_32bit_pfn" check. Does that match 
> your reasoning?

Yes. Once cached32_node is set to the right of dma_32bit_pfn it fails
to be picked up in the next cycle through __cached_rbnode_delete_update
should cached32_node be the next victim.
 
> Assuming I haven't completely misread the problem, I can't think of a 
> more appropriate way to fix it, so;
> 
> Reviewed-by: Robin Murphy 
> 
> I could swear I did consider that corner case at some point, but since 
> Leizhen and I rewrote this stuff about 5 times between us I'm not 
> entirely surprised such a subtlety could have got lost again along the way.

I admit to being impressed that rb_prev() does not appear high in the
profiles -- though I guess that is more an artifact of the scary layers
of caching above it.
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] iommu/iova: Remove stale cached32_node

2019-07-20 Thread Chris Wilson
 mapcount:0 
mapping:181a91c0 index:0x0 compound_mapcount: 0
[   48.484045] flags: 0x80010200(slab|head)
[   48.484096] raw: 80010200 ea001c421a08 ea001c447e88 
181a91c0
[   48.484141] raw:  00120012 0001 

[   48.484188] page dumped because: kasan: bad access detected
[   48.484230]
[   48.484265] Memory state around the buggy address:
[   48.484314]  88870fc18f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 
fc
[   48.484361]  88870fc18f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 
fc
[   48.484406] >88870fc19000: fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc 
fc
[   48.484451]^
[   48.484494]  88870fc19080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 
fc
[   48.484530]  88870fc19100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 
fc

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108602
Fixes: e60aa7b53845 ("iommu/iova: Extend rbtree node caching")
Signed-off-by: Chris Wilson 
Cc: Robin Murphy 
Cc: Joerg Roedel 
Cc: Joerg Roedel 
Cc:  # v4.15+
---
 drivers/iommu/iova.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index d499b2621239..24b9f9b19086 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -127,8 +127,9 @@ __cached_rbnode_delete_update(struct iova_domain *iovad, 
struct iova *free)
struct iova *cached_iova;
 
cached_iova = rb_entry(iovad->cached32_node, struct iova, node);
-   if (free->pfn_hi < iovad->dma_32bit_pfn &&
-   free->pfn_lo >= cached_iova->pfn_lo) {
+   if (free == cached_iova ||
+   (free->pfn_hi < iovad->dma_32bit_pfn &&
+free->pfn_lo >= cached_iova->pfn_lo)) {
iovad->cached32_node = rb_next(>node);
iovad->max32_alloc_size = iovad->dma_32bit_pfn;
}
-- 
2.22.0

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: Use after free from intel_alloc_iova

2019-06-22 Thread Chris Wilson
Quoting Lu Baolu (2019-06-22 09:46:36)
> Hi,
> 
> On 6/22/19 4:09 PM, Chris Wilson wrote:
> > Quoting Lu Baolu (2019-06-22 07:49:22)
> >> Hi Chris,
> >>
> >> Thanks for the test and report.
> >>
> >> On 6/21/19 9:27 PM, Chris Wilson wrote:
> >>> We see a use-after-free in our CI about 20% of the time on a Skylake
> >>> iommu testing host, present since enabling that host. Sadly, it has not
> >>> presented itself while running under KASAN.
> >>>
> >>> <4> [302.391799] general protection fault:  [#1] PREEMPT SMP PTI
> >>> <4> [302.391803] CPU: 7 PID: 4854 Comm: i915_selftest Tainted: G U
> >>> 5.2.0-rc5-CI-CI_DRM_6320+ #1
> >>
> >> Since it's CI-CI_DRM_6320+, what kind of patches have you applied on top
> >> of 5.2.0-rc5?
> > 
> > $ git diff --stat v5.2-rc5..intel/CI_DRM_6320
> > ...
> > 1383 files changed, 62481 insertions(+), 35301 deletions(-)
> > 
> > The usual drivers/gpu churn, and nothing inside drivers/iommu.
> 
> Can this be reproduced with any bare mainline rc's? So that people can
> reproduce and debug it.

Yes. The earlier reports are on code that is all in 5.0.
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: Use after free from intel_alloc_iova

2019-06-22 Thread Chris Wilson
Quoting Lu Baolu (2019-06-22 07:49:22)
> Hi Chris,
> 
> Thanks for the test and report.
> 
> On 6/21/19 9:27 PM, Chris Wilson wrote:
> > We see a use-after-free in our CI about 20% of the time on a Skylake
> > iommu testing host, present since enabling that host. Sadly, it has not
> > presented itself while running under KASAN.
> > 
> > <4> [302.391799] general protection fault:  [#1] PREEMPT SMP PTI
> > <4> [302.391803] CPU: 7 PID: 4854 Comm: i915_selftest Tainted: G U  
> >   5.2.0-rc5-CI-CI_DRM_6320+ #1
> 
> Since it's CI-CI_DRM_6320+, what kind of patches have you applied on top
> of 5.2.0-rc5?

$ git diff --stat v5.2-rc5..intel/CI_DRM_6320
...
1383 files changed, 62481 insertions(+), 35301 deletions(-)

The usual drivers/gpu churn, and nothing inside drivers/iommu.

Our oldest report (when the machine was configured) was with
4.19.0-CI-CI_DRM_5049. The tags are available from
git://git.freedesktop.org/git/gfx-ci/linux
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Use after free from intel_alloc_iova

2019-06-21 Thread Chris Wilson
We see a use-after-free in our CI about 20% of the time on a Skylake
iommu testing host, present since enabling that host. Sadly, it has not
presented itself while running under KASAN.

<4> [302.391799] general protection fault:  [#1] PREEMPT SMP PTI
<4> [302.391803] CPU: 7 PID: 4854 Comm: i915_selftest Tainted: G U  
  5.2.0-rc5-CI-CI_DRM_6320+ #1
<4> [302.391805] Hardware name: System manufacturer System Product Name/Z170I 
PRO GAMING, BIOS 1809 07/11/2016
<4> [302.391809] RIP: 0010:rb_prev+0x16/0x50
<4> [302.391811] Code: d0 e9 a5 fe ff ff 4c 89 49 10 c3 4c 89 41 10 c3 0f 1f 40 
00 48 8b 0f 48 39 cf 74 36 48 8b 47 10 48 85 c0 75 05 eb 1a 48 89 d0 <48> 8b 50 
08 48 85 d2 75 f4 f3 c3 48 3b 79 10 75 15 48 8b 09 48 89
<4> [302.391813] RSP: 0018:c954f850 EFLAGS: 00010002
<4> [302.391816] RAX: 6b6b6b6b6b6b6b6b RBX: 0010 RCX: 
6b6b6b6b6b6b6b6b
<4> [302.391818] RDX: 0001 RSI:  RDI: 
88806504dfc0
<4> [302.391820] RBP: 2000 R08: 0001 R09: 

<4> [302.391821] R10: c954f7d0 R11:  R12: 
88822b1d0370
<4> [302.391823] R13: 000fe000 R14: 88809a48f840 R15: 
88806504dfc0
<4> [302.391825] FS:  7fdec7d6de40() GS:88822eb8() 
knlGS:
<4> [302.391827] CS:  0010 DS:  ES:  CR0: 80050033
<4> [302.391829] CR2: 55e125021b78 CR3: 00011277e004 CR4: 
003606e0
<4> [302.391830] DR0:  DR1:  DR2: 

<4> [302.391832] DR3:  DR6: fffe0ff0 DR7: 
0400
<4> [302.391833] Call Trace:
<4> [302.391838]  alloc_iova+0xb3/0x150
<4> [302.391842]  alloc_iova_fast+0x51/0x270
<4> [302.391846]  intel_alloc_iova+0xa0/0xd0
<4> [302.391849]  intel_map_sg+0xae/0x190
<4> [302.391902]  i915_gem_gtt_prepare_pages+0x3e/0xf0 [i915]
<4> [302.391946]  i915_gem_object_get_pages_internal+0x225/0x2b0 [i915]
<4> [302.391981]  i915_gem_object_get_pages+0x1d/0xa0 [i915]
<4> [302.392027]  i915_gem_object_pin_map+0x1cf/0x2a0 [i915]
<4> [302.392073]  igt_fill_blt+0xdb/0x4e0 [i915]
<4> [302.392130]  __i915_subtests+0x1a4/0x1e0 [i915]
<4> [302.392184]  __run_selftests+0x112/0x170 [i915]
<4> [302.392236]  i915_live_selftests+0x2c/0x60 [i915]
<4> [302.392279]  i915_pci_probe+0x83/0x1a0 [i915]
<4> [302.392282]  ? _raw_spin_unlock_irqrestore+0x39/0x60
<4> [302.392285]  pci_device_probe+0x9e/0x120
<4> [302.392287]  really_probe+0xea/0x3c0
<4> [302.392289]  driver_probe_device+0x10b/0x120
<4> [302.392291]  device_driver_attach+0x4a/0x50
<4> [302.392293]  __driver_attach+0x97/0x130
<4> [302.392295]  ? device_driver_attach+0x50/0x50
<4> [302.392296]  bus_for_each_dev+0x74/0xc0
<4> [302.392298]  bus_add_driver+0x13f/0x210
<4> [302.392300]  ? 0xa01d8000
<4> [302.392302]  driver_register+0x56/0xe0
<4> [302.392303]  ? 0xa01d8000
<4> [302.392305]  do_one_initcall+0x58/0x300
<4> [302.392308]  ? kmem_cache_alloc_trace+0x1e8/0x290
<4> [302.392311]  do_init_module+0x56/0x1f6
<4> [302.392312]  load_module+0x24d1/0x2990
<4> [302.392318]  ? __se_sys_finit_module+0xd3/0xf0
<4> [302.392319]  __se_sys_finit_module+0xd3/0xf0
<4> [302.392323]  do_syscall_64+0x55/0x1c0
<4> [302.392325]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
<4> [302.392326] RIP: 0033:0x7fdec7428839
<4> [302.392329] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 
f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 
f0 ff ff 73 01 c3 48 8b 0d 1f f6 2c 00 f7 d8 64 89 01 48
<4> [302.392331] RSP: 002b:7ffec5007258 EFLAGS: 0246 ORIG_RAX: 
0139
<4> [302.392333] RAX: ffda RBX: 55fcf119cc00 RCX: 
7fdec7428839
<4> [302.392335] RDX:  RSI: 55fcf119e570 RDI: 
0006
<4> [302.392336] RBP: 55fcf119e570 R08: 0004 R09: 
55fcf000bc1b
<4> [302.392338] R10: 7ffec50074a0 R11: 0246 R12: 

<4> [302.392340] R13: 55fcf1197070 R14: 0020 R15: 
0042

https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6320/fi-skl-iommu/igt@i915_selftest@live_blt.html
https://bugs.freedesktop.org/show_bug.cgi?id=108602
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] Revert "iommu/vt-d: Fix lock inversion between iommu->lock and device_domain_lock"

2019-06-21 Thread Chris Wilson
Quoting Peter Xu (2019-06-21 03:32:05)
> This reverts commit 7560cc3ca7d9d11555f80c830544e463fcdb28b8.
> 
> With 5.2.0-rc5 I can easily trigger this with lockdep and iommu=pt:
> 
> ==
> WARNING: possible circular locking dependency detected
> 5.2.0-rc5 #78 Not tainted
> --
> swapper/0/1 is trying to acquire lock:
> ea2b3beb (&(>lock)->rlock){+.+.}, at: 
> domain_context_mapping_one+0xa5/0x4e0
> but task is already holding lock:
> a681907b (device_domain_lock){}, at: 
> domain_context_mapping_one+0x8d/0x4e0
> which lock already depends on the new lock.
> the existing dependency chain (in reverse order) is:
> -> #1 (device_domain_lock){}:
>_raw_spin_lock_irqsave+0x3c/0x50
>dmar_insert_one_dev_info+0xbb/0x510
>domain_add_dev_info+0x50/0x90
>dev_prepare_static_identity_mapping+0x30/0x68
>intel_iommu_init+0xddd/0x1422
>pci_iommu_init+0x16/0x3f
>do_one_initcall+0x5d/0x2b4
>kernel_init_freeable+0x218/0x2c1
>kernel_init+0xa/0x100
>ret_from_fork+0x3a/0x50
> -> #0 (&(>lock)->rlock){+.+.}:
>lock_acquire+0x9e/0x170
>_raw_spin_lock+0x25/0x30
>domain_context_mapping_one+0xa5/0x4e0
>pci_for_each_dma_alias+0x30/0x140
>dmar_insert_one_dev_info+0x3b2/0x510
>domain_add_dev_info+0x50/0x90
>dev_prepare_static_identity_mapping+0x30/0x68
>intel_iommu_init+0xddd/0x1422
>pci_iommu_init+0x16/0x3f
>do_one_initcall+0x5d/0x2b4
>kernel_init_freeable+0x218/0x2c1
>kernel_init+0xa/0x100
>ret_from_fork+0x3a/0x50
> 
> other info that might help us debug this:
>  Possible unsafe locking scenario:
>CPU0CPU1
>
>   lock(device_domain_lock);
>lock(&(>lock)->rlock);
>lock(device_domain_lock);
>   lock(&(>lock)->rlock);
> 
>  *** DEADLOCK ***
> 2 locks held by swapper/0/1:
>  #0: 033eb13d (dmar_global_lock){}, at: 
> intel_iommu_init+0x1e0/0x1422
>  #1: a681907b (device_domain_lock){}, at: 
> domain_context_mapping_one+0x8d/0x4e0
> 
> stack backtrace:
> CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc5 #78
> Hardware name: LENOVO 20KGS35G01/20KGS35G01, BIOS N23ET50W (1.25 ) 
> 06/25/2018
> Call Trace:
>  dump_stack+0x85/0xc0
>  print_circular_bug.cold.57+0x15c/0x195
>  __lock_acquire+0x152a/0x1710
>  lock_acquire+0x9e/0x170
>  ? domain_context_mapping_one+0xa5/0x4e0
>  _raw_spin_lock+0x25/0x30
>  ? domain_context_mapping_one+0xa5/0x4e0
>  domain_context_mapping_one+0xa5/0x4e0
>  ? domain_context_mapping_one+0x4e0/0x4e0
>  pci_for_each_dma_alias+0x30/0x140
>  dmar_insert_one_dev_info+0x3b2/0x510
>  domain_add_dev_info+0x50/0x90
>  dev_prepare_static_identity_mapping+0x30/0x68
>  intel_iommu_init+0xddd/0x1422
>  ? printk+0x58/0x6f
>  ? lockdep_hardirqs_on+0xf0/0x180
>  ? do_early_param+0x8e/0x8e
>  ? e820__memblock_setup+0x63/0x63
>  pci_iommu_init+0x16/0x3f
>  do_one_initcall+0x5d/0x2b4
>  ? do_early_param+0x8e/0x8e
>  ? rcu_read_lock_sched_held+0x55/0x60
>  ? do_early_param+0x8e/0x8e
>  kernel_init_freeable+0x218/0x2c1
>  ? rest_init+0x230/0x230
>  kernel_init+0xa/0x100
>  ret_from_fork+0x3a/0x50
> 
> domain_context_mapping_one() is taking device_domain_lock first then
> iommu lock, while dmar_insert_one_dev_info() is doing the reverse.
> 
> That should be introduced by commit:
> 
> 7560cc3ca7d9 ("iommu/vt-d: Fix lock inversion between iommu->lock and
>   device_domain_lock", 2019-05-27)
> 
> So far I still cannot figure out how the previous deadlock was
> triggered (I cannot find iommu lock taken before calling of
> iommu_flush_dev_iotlb()), however I'm pretty sure that that change
> should be incomplete at least because it does not fix all the places
> so we're still taking the locks in different orders, while reverting
> that commit is very clean to me so far that we should always take
> device_domain_lock first then the iommu lock.
> 
> We can continue to try to find the real culprit mentioned in
> 7560cc3ca7d9, but for now I think we should revert it to fix current
> breakage.
> 
> CC: Joerg Roedel 
> CC: Lu Baolu 
> CC: dave.ji...@intel.com
> Signed-off-by: Peter Xu 

I've run this through our CI which was also reporting the inversion, so
Tested-by: Chris Wilson 
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/2] iommu/vt-d: Fix lock inversion between iommu->lock and device_domain_lock

2019-06-18 Thread Chris Wilson
Quoting Lu Baolu (2019-05-21 08:30:15)
> From: Dave Jiang 
> 
> Lockdep debug reported lock inversion related with the iommu code
> caused by dmar_insert_one_dev_info() grabbing the iommu->lock and
> the device_domain_lock out of order versus the code path in
> iommu_flush_dev_iotlb(). Expanding the scope of the iommu->lock and
> reversing the order of lock acquisition fixes the issue.

Which of course violates the property that device_domain_lock is the
outer lock...

<4>[1.252997] ==
<4>[1.252999] WARNING: possible circular locking dependency detected
<4>[1.253002] 5.2.0-rc5-CI-CI_DRM_6299+ #1 Not tainted
<4>[1.253004] --
<4>[1.253006] swapper/0/1 is trying to acquire lock:
<4>[1.253009] 91462475 (&(>lock)->rlock){+.+.}, at: 
domain_context_mapping_one+0xa0/0x4f0
<4>[1.253015]
  but task is already holding lock:
<4>[1.253017] 69266737 (device_domain_lock){}, at: 
domain_context_mapping_one+0x88/0x4f0
<4>[1.253021]
  which lock already depends on the new lock.

<4>[1.253024]
  the existing dependency chain (in reverse order) is:
<4>[1.253027]
  -> #1 (device_domain_lock){}:
<4>[1.253031]_raw_spin_lock_irqsave+0x33/0x50
<4>[1.253034]dmar_insert_one_dev_info+0xb8/0x520
<4>[1.253036]set_domain_for_dev+0x66/0xf0
<4>[1.253039]iommu_prepare_identity_map+0x48/0x95
<4>[1.253042]intel_iommu_init+0xfd8/0x138d
<4>[1.253045]pci_iommu_init+0x11/0x3a
<4>[1.253048]do_one_initcall+0x58/0x300
<4>[1.253051]kernel_init_freeable+0x2c0/0x359
<4>[1.253054]kernel_init+0x5/0x100
<4>[1.253056]ret_from_fork+0x3a/0x50
<4>[1.253058]
  -> #0 (&(>lock)->rlock){+.+.}:
<4>[1.253062]lock_acquire+0xa6/0x1c0
<4>[1.253064]_raw_spin_lock+0x2a/0x40
<4>[1.253067]domain_context_mapping_one+0xa0/0x4f0
<4>[1.253070]pci_for_each_dma_alias+0x2b/0x160
<4>[1.253072]dmar_insert_one_dev_info+0x44e/0x520
<4>[1.253075]set_domain_for_dev+0x66/0xf0
<4>[1.253077]iommu_prepare_identity_map+0x48/0x95
<4>[1.253080]intel_iommu_init+0xfd8/0x138d
<4>[1.253082]pci_iommu_init+0x11/0x3a
<4>[1.253084]do_one_initcall+0x58/0x300
<4>[1.253086]kernel_init_freeable+0x2c0/0x359
<4>[1.253089]kernel_init+0x5/0x100
<4>[1.253091]ret_from_fork+0x3a/0x50
<4>[1.253093]
  other info that might help us debug this:

<4>[1.253095]  Possible unsafe locking scenario:

<4>[1.253095]CPU0CPU1
<4>[1.253095]
<4>[1.253095]   lock(device_domain_lock);
<4>[1.253095]lock(&(>lock)->rlock);
<4>[1.253095]lock(device_domain_lock);
<4>[1.253095]   lock(&(>lock)->rlock);
<4>[1.253095]
   *** DEADLOCK ***

<4>[1.253095] 2 locks held by swapper/0/1:
<4>[1.253095]  #0: 76465a1e (dmar_global_lock){}, at: 
intel_iommu_init+0x1d3/0x138d
<4>[1.253095]  #1: 69266737 (device_domain_lock){}, at: 
domain_context_mapping_one+0x88/0x4f0
<4>[1.253095]
  stack backtrace:
<4>[1.253095] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
5.2.0-rc5-CI-CI_DRM_6299+ #1
<4>[1.253095] Hardware name:  /NUC5i7RYB, BIOS 
RYBDWi35.86A.0362.2017.0118.0940 01/18/2017
<4>[1.253095] Call Trace:
<4>[1.253095]  dump_stack+0x67/0x9b
<4>[1.253095]  print_circular_bug+0x1c8/0x2b0
<4>[1.253095]  __lock_acquire+0x1ce9/0x24c0
<4>[1.253095]  ? lock_acquire+0xa6/0x1c0
<4>[1.253095]  lock_acquire+0xa6/0x1c0
<4>[1.253095]  ? domain_context_mapping_one+0xa0/0x4f0
<4>[1.253095]  _raw_spin_lock+0x2a/0x40
<4>[1.253095]  ? domain_context_mapping_one+0xa0/0x4f0
<4>[1.253095]  domain_context_mapping_one+0xa0/0x4f0
<4>[1.253095]  ? domain_context_mapping_one+0x4f0/0x4f0
<4>[1.253095]  pci_for_each_dma_alias+0x2b/0x160
<4>[1.253095]  dmar_insert_one_dev_info+0x44e/0x520
<4>[1.253095]  set_domain_for_dev+0x66/0xf0
<4>[1.253095]  iommu_prepare_identity_map+0x48/0x95
<4>[1.253095]  intel_iommu_init+0xfd8/0x138d
<4>[1.253095]  ? set_debug_rodata+0xc/0xc
<4>[1.253095]  ? set_debug_rodata+0xc/0xc
<4>[1.253095]  ? e820__memblock_setup+0x5b/0x5b
<4>[1.253095]  ? pci_iommu_init+0x11/0x3a
<4>[1.253095]  ? set_debug_rodata+0xc/0xc
<4>[1.253095]  pci_iommu_init+0x11/0x3a
<4>[1.253095]  do_one_initcall+0x58/0x300
<4>[1.253095]  kernel_init_freeable+0x2c0/0x359
<4>[1.253095]  ? rest_init+0x250/0x250
<4>[1.253095]  kernel_init+0x5/0x100
<4>[1.253095]  ret_from_fork+0x3a/0x50


Re: [PATCH v2 2/3] drm: Add a drm_gem_objects_lookup helper

2019-04-01 Thread Chris Wilson
Quoting Daniel Vetter (2019-04-01 14:06:48)
> On Mon, Apr 1, 2019 at 9:47 AM Rob Herring  wrote:
> > +{
> > +   int i, ret = 0;
> > +   struct drm_gem_object *obj;
> > +
> > +   spin_lock(>table_lock);
> > +
> > +   for (i = 0; i < count; i++) {
> > +   /* Check if we currently have a reference on the object */
> > +   obj = idr_find(>object_idr, handle[i]);
> > +   if (!obj) {
> > +   ret = -ENOENT;

Unwind previous drm_gem_object_get(), the caller has no idea how many
were processed before the error.
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 3/4] iommu/vt-d: Clean up pasid quirk for pre-production devices

2018-07-07 Thread Chris Wilson
Quoting Lu Baolu (2018-05-04 06:08:18)
> The pasid28 quirk is needed only for some pre-production devices.
> Remove it to make the code concise.

Every skylake mixing iommu and i915 is now inoperable on boot. Reads
by the GPU from iommu map pages return zero, writes disappear into the
void, no errors flagged.

Please revert until the matter is resolved.
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] swiotlb: Suppress "Out of SW-IOMMU" errors for NO_WARN

2018-05-02 Thread Chris Wilson
This extends the warning suppression from commit d0bc0c2a31c9 ("swiotlb:
suppress warning when __GFP_NOWARN is set") to suppress the warnings
when DMA_ATTR_NO_WARN is given by caller. In such cases the caller wants
to handle the error themselves.

Fixes: d0bc0c2a31c9 ("swiotlb: suppress warning when __GFP_NOWARN is set")
Cc: Christian König 
Cc: Mike Galbraith 
Cc: Konrad Rzeszutek Wilk 
Cc: Christoph Hellwig 
---
 lib/swiotlb.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index fece57566d45..2bfb936c5708 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -768,11 +768,14 @@ static bool swiotlb_free_buffer(struct device *dev, 
size_t size,
 
 static void
 swiotlb_full(struct device *dev, size_t size, enum dma_data_direction dir,
-int do_panic)
+unsigned long attrs, int do_panic)
 {
if (swiotlb_force == SWIOTLB_NO_FORCE)
return;
 
+   if (attrs & DMA_ATTR_NO_WARN)
+   return;
+
/*
 * Ran out of IOMMU space for this operation. This is very bad.
 * Unfortunately the drivers cannot handle this operation properly.
@@ -823,7 +826,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page 
*page,
/* Oh well, have to allocate and map a bounce buffer. */
map = map_single(dev, phys, size, dir, attrs);
if (map == SWIOTLB_MAP_ERROR) {
-   swiotlb_full(dev, size, dir, 1);
+   swiotlb_full(dev, size, dir, attrs, 1);
return __phys_to_dma(dev, io_tlb_overflow_buffer);
}
 
@@ -959,7 +962,7 @@ swiotlb_map_sg_attrs(struct device *hwdev, struct 
scatterlist *sgl, int nelems,
if (map == SWIOTLB_MAP_ERROR) {
/* Don't panic here, we expect map_sg users
   to do proper error handling. */
-   swiotlb_full(hwdev, sg->length, dir, 0);
+   swiotlb_full(hwdev, sg->length, dir, attrs, 0);
attrs |= DMA_ATTR_SKIP_CPU_SYNC;
swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
   attrs);
-- 
2.17.0

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes

2017-08-16 Thread Chris Wilson
Quoting Christian König (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> >  1) changed cls_flower to use IDR to manage the filters.
> >  2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
> 
> WOW, wait a second. The idr change is touching a lot of drivers and to 
> be honest doesn't looks correct at all.
> 
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct 
> > device *parent,
> >   
> >   mutex_lock(_mutex);
> >   
> > - ret = idr_alloc(_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> > - if (ret < 0) {
> > + ret = idr_alloc(_minor_idr, bcd, _index, 0, BSG_MAX_DEVS,
> > + GFP_KERNEL);
> > + if (ret) {
> >   if (ret == -ENOSPC) {
> >   printk(KERN_ERR "bsg: too many bsg devices\n");
> >   ret = -EINVAL;
> The condition "if (ret)" will now always be true after the first 
> allocation and so we always run into the error handling after that.

ret is now purely the error code, so it doesn't look that suspicious.

> I've never read the bsg code before, but that's certainly not correct. 
> And that incorrect pattern repeats over and over again in this code.
> 
> Apart from that why the heck do you want to allocate more than 1<<31 
> handles?

And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=0 with end=INT_MAX
to maintain existing behaviour.
-Chris
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [Intel-gfx] [PATCH 7/7] iommu/vt-d: Implement page request handling

2015-10-12 Thread Chris Wilson
On Fri, Oct 09, 2015 at 12:54:07AM +0100, David Woodhouse wrote:
> Signed-off-by: David Woodhouse <david.woodho...@intel.com>
> ---
>  drivers/iommu/intel-svm.c   | 115 
> +++-
>  include/linux/intel-iommu.h |  21 
>  2 files changed, 134 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> index 1260e87..ace1e32 100644
> --- a/drivers/iommu/intel-svm.c
> +++ b/drivers/iommu/intel-svm.c
>  static irqreturn_t prq_event_thread(int irq, void *d)
>  {

> + if (svm) {
> + mutex_lock(_mutex);
> + kref_put(>kref, _mm_free);
> + mutex_unlock(_mutex);

kref_put_mutex(>kref, intel_mm_free, _mutex); ?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu