Re: [PATCH v5 0/7] Convert the intel iommu driver to the dma-iommu api
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
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
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
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
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
[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
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
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
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
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
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
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"
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
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
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
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
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önigCc: 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
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
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