| Message ID | 20220726153935.2272777-1-bob.beckett@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5] drm/i915: stop using swiotlb | expand |
On 28/07/2022 09:01, Patchwork wrote: [snip] > Possible regressions > > * igt@gem_mmap_offset@clear: > o shard-iclb: PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/shard-iclb6/igt@gem_mmap_offset@clear.html> > -> INCOMPLETE > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106589v6/shard-iclb1/igt@gem_mmap_offset@clear.html> What was supposed to be a simple patch.. a storm of errors like: DMAR: ERROR: DMA PTE for vPFN 0x3d00000 already set (to 2fd7ff003 not 2fd7ff003) ------------[ cut here ]------------ WARNING: CPU: 6 PID: 1254 at drivers/iommu/intel/iommu.c:2278 __domain_mapping.cold.93+0x32/0x39<> Modules linked in: vgem drm_shmem_helper snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_cod> CPU: 6 PID: 1254 Comm: gem_mmap_offset Not tainted 5.19.0-rc8-Patchwork_106589v6-g0e9c43d76a14+ #> Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS > RIP: 0010:__domain_mapping.cold.93+0x32/0x39 Code: fe 48 c7 c7 28 32 37 82 4c 89 5c 24 08 e8 e4 61 fd ff 8b 05 bf 8e c9 00 4c 8b 5c 24 08 85 c> RSP: 0000:ffffc9000037f9c0 EFLAGS: 00010202 RAX: 0000000000000004 RBX: ffff8881117b4000 RCX: 0000000000000001 RDX: 0000000000000000 RSI: ffffffff82320b25 RDI: 00000000ffffffff RBP: 0000000000000001 R08: 0000000000000000 R09: c0000000ffff7fff R10: 0000000000000001 R11: 00000000002fd7ff R12: 00000002fd7ff003 R13: 0000000000076c01 R14: ffff8881039ee800 R15: 0000000003d00000 FS: 00007f2863c1d700(0000) GS:ffff88849fd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f2692c53000 CR3: 000000011c440006 CR4: 0000000000770ee0 PKRU: 55555554 Call Trace: <TASK> intel_iommu_map_pages+0xb7/0xe0 __iommu_map+0xe0/0x310 __iommu_map_sg+0xa2/0x140 iommu_dma_map_sg+0x2ef/0x4e0 __dma_map_sg_attrs+0x64/0x70 dma_map_sg_attrs+0x5/0x20 i915_gem_gtt_prepare_pages+0x56/0x70 [i915] shmem_get_pages+0xe3/0x360 [i915] ____i915_gem_object_get_pages+0x32/0x100 [i915] __i915_gem_object_get_pages+0x8d/0xa0 [i915] vm_fault_gtt+0x3d0/0x940 [i915] ? ptlock_alloc+0x15/0x40 ? rt_mutex_debug_task_free+0x91/0xa0 __do_fault+0x30/0x180 do_fault+0x1c4/0x4c0 __handle_mm_fault+0x615/0xbe0 handle_mm_fault+0x75/0x1c0 do_user_addr_fault+0x1e7/0x670 exc_page_fault+0x62/0x230 asm_exc_page_fault+0x22/0x30 No idea. Maybe try CI kernel config on your Tigerlake? Regards, Tvrtko
On 28/07/2022 15:03, Tvrtko Ursulin wrote: > > On 28/07/2022 09:01, Patchwork wrote: > > [snip] > >> Possible regressions >> >> * igt@gem_mmap_offset@clear: >> o shard-iclb: PASS >> >> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/shard-iclb6/igt@gem_mmap_offset@clear.html> >> >> -> INCOMPLETE >> >> <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106589v6/shard-iclb1/igt@gem_mmap_offset@clear.html> >> > > What was supposed to be a simple patch.. a storm of errors like: yeah, them's the breaks sometimes .... > > DMAR: ERROR: DMA PTE for vPFN 0x3d00000 already set (to 2fd7ff003 not > 2fd7ff003) > ------------[ cut here ]------------ > WARNING: CPU: 6 PID: 1254 at drivers/iommu/intel/iommu.c:2278 > __domain_mapping.cold.93+0x32/0x39<> > Modules linked in: vgem drm_shmem_helper snd_hda_codec_hdmi > snd_hda_codec_realtek snd_hda_cod> > CPU: 6 PID: 1254 Comm: gem_mmap_offset Not tainted > 5.19.0-rc8-Patchwork_106589v6-g0e9c43d76a14+ #> > Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U > DDR4 SODIMM PD RVP TLC, BIOS > > RIP: 0010:__domain_mapping.cold.93+0x32/0x39 > Code: fe 48 c7 c7 28 32 37 82 4c 89 5c 24 08 e8 e4 61 fd ff 8b 05 bf > 8e c9 00 4c 8b 5c 24 08 85 c> > RSP: 0000:ffffc9000037f9c0 EFLAGS: 00010202 > RAX: 0000000000000004 RBX: ffff8881117b4000 RCX: 0000000000000001 > RDX: 0000000000000000 RSI: ffffffff82320b25 RDI: 00000000ffffffff > RBP: 0000000000000001 R08: 0000000000000000 R09: c0000000ffff7fff > R10: 0000000000000001 R11: 00000000002fd7ff R12: 00000002fd7ff003 > R13: 0000000000076c01 R14: ffff8881039ee800 R15: 0000000003d00000 > FS: 00007f2863c1d700(0000) GS:ffff88849fd00000(0000) > knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007f2692c53000 CR3: 000000011c440006 CR4: 0000000000770ee0 > PKRU: 55555554 > Call Trace: > <TASK> > intel_iommu_map_pages+0xb7/0xe0 > __iommu_map+0xe0/0x310 > __iommu_map_sg+0xa2/0x140 > iommu_dma_map_sg+0x2ef/0x4e0 > __dma_map_sg_attrs+0x64/0x70 > dma_map_sg_attrs+0x5/0x20 > i915_gem_gtt_prepare_pages+0x56/0x70 [i915] > shmem_get_pages+0xe3/0x360 [i915] > ____i915_gem_object_get_pages+0x32/0x100 [i915] > __i915_gem_object_get_pages+0x8d/0xa0 [i915] > vm_fault_gtt+0x3d0/0x940 [i915] > ? ptlock_alloc+0x15/0x40 > ? rt_mutex_debug_task_free+0x91/0xa0 > __do_fault+0x30/0x180 > do_fault+0x1c4/0x4c0 > __handle_mm_fault+0x615/0xbe0 > handle_mm_fault+0x75/0x1c0 > do_user_addr_fault+0x1e7/0x670 > exc_page_fault+0x62/0x230 > asm_exc_page_fault+0x22/0x30 > > No idea. Maybe try CI kernel config on your Tigerlake? I have an idea of what could be happening: The warning is due to a pte already existing. We can see from the warning that it is the same value, which indicates that the same page has been mapped to the same iova before. This map shrink loop will keep mapping the same sg, shrinking if it fails to hopefully free up iova space. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/i915/i915_gem_gtt.c?h=v5.19-rc8#n32 If we now look at the intel iommu driver's mapping function: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/intel/iommu.c?h=v5.19-rc8#n2248 If that -ENOMEM loop breaking return is hit (presumably running out of pte space, though I have not delved deeper), then it will return back up the stack, eventually returning 0 from dma_map_sg_attrs() indicating the error. This will cause a shrink and retry. The problem is that the iommu does not undo it's partial mapping on error. So the next time round, it will map the same page to the same address giving the same pte encoding, which would give the warning observed. I would need to get some time to try to repro and debug to confirm, but this looks like it might be exposing an iommu driver issue due to us changing our mapping patterns because the segment sizes are now different. I'll see if I can get some time allotted to debug it further, but for now, I don't have the bandwidth, so this may need to go on hold until I or someone else can get time to look in to it. > > Regards, > > Tvrtko
On 28/07/2022 16:54, Robert Beckett wrote: > On 28/07/2022 15:03, Tvrtko Ursulin wrote: >> >> On 28/07/2022 09:01, Patchwork wrote: >> >> [snip] >> >>> Possible regressions >>> >>> * igt@gem_mmap_offset@clear: >>> o shard-iclb: PASS >>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/shard-iclb6/igt@gem_mmap_offset@clear.html> >>> >>> -> INCOMPLETE >>> <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106589v6/shard-iclb1/igt@gem_mmap_offset@clear.html> >>> >> >> What was supposed to be a simple patch.. a storm of errors like: > > yeah, them's the breaks sometimes .... > >> >> DMAR: ERROR: DMA PTE for vPFN 0x3d00000 already set (to 2fd7ff003 >> not 2fd7ff003) >> ------------[ cut here ]------------ >> WARNING: CPU: 6 PID: 1254 at drivers/iommu/intel/iommu.c:2278 >> __domain_mapping.cold.93+0x32/0x39<> >> Modules linked in: vgem drm_shmem_helper snd_hda_codec_hdmi >> snd_hda_codec_realtek snd_hda_cod> >> CPU: 6 PID: 1254 Comm: gem_mmap_offset Not tainted >> 5.19.0-rc8-Patchwork_106589v6-g0e9c43d76a14+ #> >> Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U >> DDR4 SODIMM PD RVP TLC, BIOS > >> RIP: 0010:__domain_mapping.cold.93+0x32/0x39 >> Code: fe 48 c7 c7 28 32 37 82 4c 89 5c 24 08 e8 e4 61 fd ff 8b 05 bf >> 8e c9 00 4c 8b 5c 24 08 85 c> >> RSP: 0000:ffffc9000037f9c0 EFLAGS: 00010202 >> RAX: 0000000000000004 RBX: ffff8881117b4000 RCX: 0000000000000001 >> RDX: 0000000000000000 RSI: ffffffff82320b25 RDI: 00000000ffffffff >> RBP: 0000000000000001 R08: 0000000000000000 R09: c0000000ffff7fff >> R10: 0000000000000001 R11: 00000000002fd7ff R12: 00000002fd7ff003 >> R13: 0000000000076c01 R14: ffff8881039ee800 R15: 0000000003d00000 >> FS: 00007f2863c1d700(0000) GS:ffff88849fd00000(0000) >> knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 00007f2692c53000 CR3: 000000011c440006 CR4: 0000000000770ee0 >> PKRU: 55555554 >> Call Trace: >> <TASK> >> intel_iommu_map_pages+0xb7/0xe0 >> __iommu_map+0xe0/0x310 >> __iommu_map_sg+0xa2/0x140 >> iommu_dma_map_sg+0x2ef/0x4e0 >> __dma_map_sg_attrs+0x64/0x70 >> dma_map_sg_attrs+0x5/0x20 >> i915_gem_gtt_prepare_pages+0x56/0x70 [i915] >> shmem_get_pages+0xe3/0x360 [i915] >> ____i915_gem_object_get_pages+0x32/0x100 [i915] >> __i915_gem_object_get_pages+0x8d/0xa0 [i915] >> vm_fault_gtt+0x3d0/0x940 [i915] >> ? ptlock_alloc+0x15/0x40 >> ? rt_mutex_debug_task_free+0x91/0xa0 >> __do_fault+0x30/0x180 >> do_fault+0x1c4/0x4c0 >> __handle_mm_fault+0x615/0xbe0 >> handle_mm_fault+0x75/0x1c0 >> do_user_addr_fault+0x1e7/0x670 >> exc_page_fault+0x62/0x230 >> asm_exc_page_fault+0x22/0x30 >> >> No idea. Maybe try CI kernel config on your Tigerlake? > > I have an idea of what could be happening: > > The warning is due to a pte already existing. We can see from the > warning that it is the same value, which indicates that the same page > has been mapped to the same iova before. > > This map shrink loop will keep mapping the same sg, shrinking if it > fails to hopefully free up iova space. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/i915/i915_gem_gtt.c?h=v5.19-rc8#n32 > > > If we now look at the intel iommu driver's mapping function: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/intel/iommu.c?h=v5.19-rc8#n2248 > > > If that -ENOMEM loop breaking return is hit (presumably running out of > pte space, though I have not delved deeper), then it will return back up > the stack, eventually returning 0 from dma_map_sg_attrs() indicating the > error. This will cause a shrink and retry. > > The problem is that the iommu does not undo it's partial mapping on > error. So the next time round, it will map the same page to the same > address giving the same pte encoding, which would give the warning > observed. > > I would need to get some time to try to repro and debug to confirm, but > this looks like it might be exposing an iommu driver issue due to us > changing our mapping patterns because the segment sizes are now different. > > I'll see if I can get some time allotted to debug it further, but for > now, I don't have the bandwidth, so this may need to go on hold until I > or someone else can get time to look in to it. Yeah that's understandable. I also currently don't have any free bandwidth unfortunately. + Christoph FYI, as per above, swiotlb API usage removal is currently a bit stuck until we find someone with some spare time to debug this further. Regards, Tvrtko
Hi, [back from vacation] On Tue, 2022-07-26 at 16:39 +0100, Robert Beckett wrote: > Calling swiotlb functions directly is nowadays considered harmful. > See > https://lore.kernel.org/intel-gfx/20220711082614.GA29487@lst.de/ > > Replace swiotlb_max_segment() calls with dma_max_mapping_size(). > In i915_gem_object_get_pages_internal() no longer consider > max_segment > only if CONFIG_SWIOTLB is enabled. There can be other (iommu related) > causes of specific max segment sizes. > > Cc: Christoph Hellwig <hch@lst.de> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Cc: Thomas Hellstrom <thomas.hellstrom@intel.com> > Cc: Matthew Auld <matthew.auld@intel.com> > > v2: - restore UINT_MAX clamp in i915_sg_segment_size() > - drop PAGE_SIZE check as it will always be >= PAGE_SIZE > v3: - actually clamp to UINT_MAX in i915_sg_segment_size() > v4: - round down max segment size to PAGE_SIZE > v5: - fix checkpatch whitespace issue > > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Signed-off-by: Robert Beckett <bob.beckett@collabora.com> Hmm, This whole thing looks a bit strange to me since with SWIOTLB actually used for i915, the driver should malfunction anyway as it doesn't do any dma_sync_sg_for_cpu() or dma_sync_sg_for_device(), and the driver assumes all coherent dma. Is that SWIOTLB=force kernel option still available? Also, correct me if I'm wrong, but the original driver segment size appears to mean "the largest contiguous area that can be handled either by the device or the dma mapping layer" rather than the total space available for dma mappings? Not completely sure what dma_max_mapping_size() is returning, though? /Thomas
On 08/08/2022 16:48, Hellstrom, Thomas wrote: > Hi, [back from vacation] > > On Tue, 2022-07-26 at 16:39 +0100, Robert Beckett wrote: >> Calling swiotlb functions directly is nowadays considered harmful. >> See >> https://lore.kernel.org/intel-gfx/20220711082614.GA29487@lst.de/ >> >> Replace swiotlb_max_segment() calls with dma_max_mapping_size(). >> In i915_gem_object_get_pages_internal() no longer consider >> max_segment >> only if CONFIG_SWIOTLB is enabled. There can be other (iommu related) >> causes of specific max segment sizes. >> >> Cc: Christoph Hellwig <hch@lst.de> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> >> Cc: Thomas Hellstrom <thomas.hellstrom@intel.com> >> Cc: Matthew Auld <matthew.auld@intel.com> >> >> v2: - restore UINT_MAX clamp in i915_sg_segment_size() >> - drop PAGE_SIZE check as it will always be >= PAGE_SIZE >> v3: - actually clamp to UINT_MAX in i915_sg_segment_size() >> v4: - round down max segment size to PAGE_SIZE >> v5: - fix checkpatch whitespace issue >> >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Signed-off-by: Robert Beckett <bob.beckett@collabora.com> > > Hmm, > > This whole thing looks a bit strange to me since with SWIOTLB actually > used for i915, the driver should malfunction anyway as it doesn't do > any dma_sync_sg_for_cpu() or dma_sync_sg_for_device(), and the driver > assumes all coherent dma. Is that SWIOTLB=force kernel option still > available? Don't know about these - but pretty sure in the past we had i915 break if we did not respect swiotlb_max_segment. Digging through git history at least running as Xen dom0 looks to have been impacted, but commits such as abb0deacb5a6 ("drm/i915: Fallback to single PAGE_SIZE segments for DMA remapping") are older and suggest problem was generic. 1625e7e549c5 ("drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.") as well. So it looks it did work behind swiotlb despite those missing calls you highlighted. > Also, correct me if I'm wrong, but the original driver segment size > appears to mean "the largest contiguous area that can be handled either > by the device or the dma mapping layer" rather than the total space > available for dma mappings? Not completely sure what > dma_max_mapping_size() is returning, though? AFAIU looks to be compatible on paper at least.: dma_max_mapping_size -> "Returns the maximum size of a mapping for the device." So an individual mapping. But then in case of swiotlb is implemented in swiotlb_max_mapping_size, and not the same code as swiotlb_max_segment. I agree, ideally if someone could clarify they are returning the same thing or there is a miss somewhere. Regards, Tvrtko
On Mon, Aug 08, 2022 at 03:48:02PM +0000, Hellstrom, Thomas wrote: > This whole thing looks a bit strange to me since with SWIOTLB actually > used for i915, the driver should malfunction anyway as it doesn't do > any dma_sync_sg_for_cpu() or dma_sync_sg_for_device(), Yeah, I can't actually see any syncs in i915. > and the driver > assumes all coherent dma. Is that SWIOTLB=force kernel option still > available? Yes, although it is (and always has been) lower case swiotlb for the option. > Also, correct me if I'm wrong, but the original driver segment size > appears to mean "the largest contiguous area that can be handled either > by the device or the dma mapping layer" rather than the total space > available for dma mappings? Not completely sure what > dma_max_mapping_size() is returning, though? dma_max_mapping_size is sort of both. It is is the largest contigous size, but we really should not go above that to avoid starvation.
On Tue, Aug 09, 2022 at 12:36:50PM +0100, Tvrtko Ursulin wrote: > > Digging through git history at least running as Xen dom0 looks to have been > impacted, but commits such as abb0deacb5a6 ("drm/i915: Fallback to single > PAGE_SIZE segments for DMA remapping") are older and suggest problem was > generic. 1625e7e549c5 ("drm/i915: make compact dma scatter lists creation > work with SWIOTLB backend.") as well. So it looks it did work behind > swiotlb despite those missing calls you highlighted. Hmm. xen-swiotlb bounce buffers as soon as any single mapping that straddles a Xen page size boundary. Could the magic value there somehow made all mappings small enough to just avoid bounce buffering for Xen by more or less accident?
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c b/drivers/gpu/drm/i915/gem/i915_gem_internal.c index c698f95af15f..24f37658f1bb 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c @@ -6,7 +6,6 @@ #include <linux/scatterlist.h> #include <linux/slab.h> -#include <linux/swiotlb.h> #include "i915_drv.h" #include "i915_gem.h" @@ -38,22 +37,12 @@ static int i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj) struct scatterlist *sg; unsigned int sg_page_sizes; unsigned int npages; - int max_order; + int max_order = MAX_ORDER; + unsigned int max_segment; gfp_t gfp; - max_order = MAX_ORDER; -#ifdef CONFIG_SWIOTLB - if (is_swiotlb_active(obj->base.dev->dev)) { - unsigned int max_segment; - - max_segment = swiotlb_max_segment(); - if (max_segment) { - max_segment = max_t(unsigned int, max_segment, - PAGE_SIZE) >> PAGE_SHIFT; - max_order = min(max_order, ilog2(max_segment)); - } - } -#endif + max_segment = i915_sg_segment_size(i915->drm.dev) >> PAGE_SHIFT; + max_order = min(max_order, ilog2(max_segment)); gfp = GFP_KERNEL | __GFP_HIGHMEM | __GFP_RECLAIMABLE; if (IS_I965GM(i915) || IS_I965G(i915)) { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 4eed3dd90ba8..34b9c76cd8e6 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -194,7 +194,7 @@ static int shmem_get_pages(struct drm_i915_gem_object *obj) struct intel_memory_region *mem = obj->mm.region; struct address_space *mapping = obj->base.filp->f_mapping; const unsigned long page_count = obj->base.size / PAGE_SIZE; - unsigned int max_segment = i915_sg_segment_size(); + unsigned int max_segment = i915_sg_segment_size(i915->drm.dev); struct sg_table *st; struct sgt_iter sgt_iter; struct page *page; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 5a5cf332d8a5..7a828c9c0f6d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -189,7 +189,7 @@ static int i915_ttm_tt_shmem_populate(struct ttm_device *bdev, struct drm_i915_private *i915 = container_of(bdev, typeof(*i915), bdev); struct intel_memory_region *mr = i915->mm.regions[INTEL_MEMORY_SYSTEM]; struct i915_ttm_tt *i915_tt = container_of(ttm, typeof(*i915_tt), ttm); - const unsigned int max_segment = i915_sg_segment_size(); + const unsigned int max_segment = i915_sg_segment_size(i915->drm.dev); const size_t size = (size_t)ttm->num_pages << PAGE_SHIFT; struct file *filp = i915_tt->filp; struct sgt_iter sgt_iter; @@ -568,7 +568,7 @@ static struct i915_refct_sgt *i915_ttm_tt_get_st(struct ttm_tt *ttm) ret = sg_alloc_table_from_pages_segment(st, ttm->pages, ttm->num_pages, 0, (unsigned long)ttm->num_pages << PAGE_SHIFT, - i915_sg_segment_size(), GFP_KERNEL); + i915_sg_segment_size(i915_tt->dev), GFP_KERNEL); if (ret) { st->sgl = NULL; return ERR_PTR(ret); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 094f06b4ce33..dfc35905dba2 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -129,7 +129,7 @@ static void i915_gem_object_userptr_drop_ref(struct drm_i915_gem_object *obj) static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) { const unsigned long num_pages = obj->base.size >> PAGE_SHIFT; - unsigned int max_segment = i915_sg_segment_size(); + unsigned int max_segment = i915_sg_segment_size(obj->base.dev->dev); struct sg_table *st; unsigned int sg_page_sizes; struct page **pvec; diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h b/drivers/gpu/drm/i915/i915_scatterlist.h index 9ddb3e743a3e..3dd40012e901 100644 --- a/drivers/gpu/drm/i915/i915_scatterlist.h +++ b/drivers/gpu/drm/i915/i915_scatterlist.h @@ -9,7 +9,7 @@ #include <linux/pfn.h> #include <linux/scatterlist.h> -#include <linux/swiotlb.h> +#include <linux/dma-mapping.h> #include "i915_gem.h" @@ -127,19 +127,11 @@ static inline unsigned int i915_sg_dma_sizes(struct scatterlist *sg) return page_sizes; } -static inline unsigned int i915_sg_segment_size(void) +static inline unsigned int i915_sg_segment_size(struct device *dev) { - unsigned int size = swiotlb_max_segment(); + size_t max = min_t(size_t, UINT_MAX, dma_max_mapping_size(dev)); - if (size == 0) - size = UINT_MAX; - - size = rounddown(size, PAGE_SIZE); - /* swiotlb_max_segment_size can return 1 byte when it means one page. */ - if (size < PAGE_SIZE) - size = PAGE_SIZE; - - return size; + return round_down(max, PAGE_SIZE); } bool i915_sg_trim(struct sg_table *orig_st);
Calling swiotlb functions directly is nowadays considered harmful. See https://lore.kernel.org/intel-gfx/20220711082614.GA29487@lst.de/ Replace swiotlb_max_segment() calls with dma_max_mapping_size(). In i915_gem_object_get_pages_internal() no longer consider max_segment only if CONFIG_SWIOTLB is enabled. There can be other (iommu related) causes of specific max segment sizes. Cc: Christoph Hellwig <hch@lst.de> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Thomas Hellstrom <thomas.hellstrom@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> v2: - restore UINT_MAX clamp in i915_sg_segment_size() - drop PAGE_SIZE check as it will always be >= PAGE_SIZE v3: - actually clamp to UINT_MAX in i915_sg_segment_size() v4: - round down max segment size to PAGE_SIZE v5: - fix checkpatch whitespace issue Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Robert Beckett <bob.beckett@collabora.com> --- drivers/gpu/drm/i915/gem/i915_gem_internal.c | 19 ++++--------------- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 4 ++-- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 2 +- drivers/gpu/drm/i915/i915_scatterlist.h | 16 ++++------------ 5 files changed, 12 insertions(+), 31 deletions(-)