| Message ID | 20210219134014.7775-1-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/prime: Only call dma_map_sgtable() for devices with DMA support | expand |
Well as far as I can see this is a relative clear NAK. When a device can't do DMA and has no DMA mask then why it is requesting an sg-table in the first place? The problem is rather in drm_gem_prime_import_dev() which always tries to create an sg-table to get a GEM object, even when the device doesn't support DMA at all. Regards, Christian. Am 19.02.21 um 14:40 schrieb Thomas Zimmermann: > Fixes a regression for udl and probably other USB-based drivers where > joining and mirroring displays fails. > > Joining displays requires importing a dma_buf from another DRM driver. > These devices don't support DMA and therefore have no DMA mask. Trying > to map imported buffers from a DMA-able memory zone fails with an error. > An example is shown below. > > [ 60.050199] ------------[ cut here ]------------ > [ 60.055092] WARNING: CPU: 0 PID: 1403 at kernel/dma/mapping.c:190 dma_map_sg_attrs+0x8f/0xc0 > [ 60.064331] Modules linked in: af_packet(E) rfkill(E) dmi_sysfs(E) intel_rapl_msr(E) intel_rapl_common(E) snd_hda_codec_realtek(E) snd_hda_codec_generic(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) x86_pkg_temp_thermal(E) intel_powerclam) > [ 60.064801] wmi(E) video(E) btrfs(E) blake2b_generic(E) libcrc32c(E) crc32c_intel(E) xor(E) raid6_pq(E) sg(E) dm_multipath(E) dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) msr(E) efivarfs(E) > [ 60.170778] CPU: 0 PID: 1403 Comm: Xorg.bin Tainted: G E 5.11.0-rc5-1-default+ #747 > [ 60.179841] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS A24 10/24/2018 > [ 60.187145] RIP: 0010:dma_map_sg_attrs+0x8f/0xc0 > [ 60.191822] Code: 4d 85 ff 75 2b 49 89 d8 44 89 e1 44 89 f2 4c 89 ee 48 89 ef e8 62 30 00 00 85 c0 78 36 5b 5d 41 5c 41 5d 41 5e 41 5f c3 0f 0b <0f> 0b 31 c0 eb ed 49 8d 7f 50 e8 72 f5 2a 00 49 8b 47 50 49 89 d8 > [ 60.210770] RSP: 0018:ffffc90001d6fc18 EFLAGS: 00010246 > [ 60.216062] RAX: 0000000000000000 RBX: 0000000000000020 RCX: ffffffffb31e677b > [ 60.223274] RDX: dffffc0000000000 RSI: ffff888212c10600 RDI: ffff8881b08a9488 > [ 60.230501] RBP: ffff8881b08a9030 R08: 0000000000000020 R09: ffff888212c10600 > [ 60.237710] R10: ffffed10425820df R11: 0000000000000001 R12: 0000000000000000 > [ 60.244939] R13: ffff888212c10600 R14: 0000000000000008 R15: 0000000000000000 > [ 60.252155] FS: 00007f08ac2b2f00(0000) GS:ffff8887cbe00000(0000) knlGS:0000000000000000 > [ 60.260333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 60.266150] CR2: 000055831c899be8 CR3: 000000015372e006 CR4: 00000000001706f0 > [ 60.273369] Call Trace: > [ 60.275845] drm_gem_map_dma_buf+0xb4/0x120 > [ 60.280089] dma_buf_map_attachment+0x15d/0x1e0 > [ 60.284688] drm_gem_prime_import_dev.part.0+0x5d/0x140 > [ 60.289984] drm_gem_prime_fd_to_handle+0x213/0x280 > [ 60.294931] ? drm_prime_destroy_file_private+0x30/0x30 > [ 60.300224] drm_ioctl_kernel+0x131/0x180 > [ 60.304291] ? drm_setversion+0x230/0x230 > [ 60.308366] ? drm_prime_destroy_file_private+0x30/0x30 > [ 60.313659] drm_ioctl+0x2f1/0x500 > [ 60.317118] ? drm_version+0x150/0x150 > [ 60.320903] ? lock_downgrade+0xa0/0xa0 > [ 60.324806] ? do_vfs_ioctl+0x5f4/0x680 > [ 60.328694] ? __fget_files+0x13e/0x210 > [ 60.332591] ? ioctl_fiemap.isra.0+0x1a0/0x1a0 > [ 60.337102] ? __fget_files+0x15d/0x210 > [ 60.340990] __x64_sys_ioctl+0xb9/0xf0 > [ 60.344795] do_syscall_64+0x33/0x80 > [ 60.348427] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [ 60.353542] RIP: 0033:0x7f08ac7b53cb > [ 60.357168] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 ba 0c 00 f7 d8 64 89 01 48 > [ 60.376108] RSP: 002b:00007ffeabc89fc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 > [ 60.383758] RAX: ffffffffffffffda RBX: 00007ffeabc8a00c RCX: 00007f08ac7b53cb > [ 60.390970] RDX: 00007ffeabc8a00c RSI: 00000000c00c642e RDI: 000000000000000d > [ 60.398221] RBP: 00000000c00c642e R08: 000055831c691d00 R09: 000055831b2ec010 > [ 60.405446] R10: 00007f08acf6ada0 R11: 0000000000000246 R12: 000055831c691d00 > [ 60.412667] R13: 000000000000000d R14: 00000000007e9000 R15: 0000000000000000 > [ 60.419903] irq event stamp: 672893 > [ 60.423441] hardirqs last enabled at (672913): [<ffffffffb31b796d>] console_unlock+0x44d/0x500 > [ 60.432230] hardirqs last disabled at (672922): [<ffffffffb31b7963>] console_unlock+0x443/0x500 > [ 60.441021] softirqs last enabled at (672940): [<ffffffffb46003dd>] __do_softirq+0x3dd/0x554 > [ 60.449634] softirqs last disabled at (672931): [<ffffffffb44010f2>] asm_call_irq_on_stack+0x12/0x20 > [ 60.458871] ---[ end trace f2f88696eb17806c ]--- > > For the fix, we don't call dma_map_sgtable() for devices without the > DMA mask. Drivers for such devices have to map the imported buffer into > kernel address space and perfom the copy operation in software. > > Tested by joining/mirroring displays of udl and radeon un der Gnome/X11. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices") > Cc: Christoph Hellwig <hch@lst.de> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Alan Stern <stern@rowland.harvard.edu> > Cc: Johan Hovold <johan@kernel.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Cc: "Ahmed S. Darwish" <a.darwish@linutronix.de> > Cc: Mathias Nyman <mathias.nyman@linux.intel.com> > Cc: Oliver Neukum <oneukum@suse.com> > Cc: Felipe Balbi <balbi@kernel.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: <stable@vger.kernel.org> # v5.10+ > --- > drivers/gpu/drm/drm_prime.c | 27 ++++++++++++++++++--------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 2a54f86856af..d5a39fe76b78 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -603,10 +603,15 @@ EXPORT_SYMBOL(drm_gem_map_detach); > * @attach: attachment whose scatterlist is to be returned > * @dir: direction of DMA transfer > * > - * Calls &drm_gem_object_funcs.get_sg_table and then maps the scatterlist. This > - * can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together > + * Calls &drm_gem_object_funcs.get_sg_table and, if possible, maps the scatterlist. > + * This can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together > * with drm_gem_unmap_dma_buf(). > * > + * Devices on some peripheral busses, such as USB, cannot use DMA. In this case, > + * pages in the scatterlist remain unmapped. Drivers for such devices should acquire > + * a mapping with dma_buf_vmap() and implement copy operation via bus-specific > + * interfaces. > + * > * Returns:sg_table containing the scatterlist to be returned; returns ERR_PTR > * on error. May return -EINTR if it is interrupted by a signal. > */ > @@ -627,12 +632,14 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, > if (IS_ERR(sgt)) > return sgt; > > - ret = dma_map_sgtable(attach->dev, sgt, dir, > - DMA_ATTR_SKIP_CPU_SYNC); > - if (ret) { > - sg_free_table(sgt); > - kfree(sgt); > - sgt = ERR_PTR(ret); > + if (attach->dev->dma_mask) { > + ret = dma_map_sgtable(attach->dev, sgt, dir, > + DMA_ATTR_SKIP_CPU_SYNC); > + if (ret) { > + sg_free_table(sgt); > + kfree(sgt); > + sgt = ERR_PTR(ret); > + } > } > > return sgt; > @@ -654,7 +661,9 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach, > if (!sgt) > return; > > - dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); > + if (attach->dev->dma_mask) > + dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); > + > sg_free_table(sgt); > kfree(sgt); > } > -- > 2.30.0 >
Hi Am 19.02.21 um 14:45 schrieb Christian König: > Well as far as I can see this is a relative clear NAK. > > When a device can't do DMA and has no DMA mask then why it is requesting > an sg-table in the first place? > > The problem is rather in drm_gem_prime_import_dev() which always tries > to create an sg-table to get a GEM object, even when the device doesn't > support DMA at all. Sure, that's the clean solution. But it also needs a bit of work on the way the importing works. Our generic code assumes that an SG table is being used. Most driver's memory managers as well, I guess. The regressing patch has already been in upstream kernels for a while. The fix at hand is backport-able at least. Can we add a TODO item for the real thing? Best regards Thomas > > Regards, > Christian. > > Am 19.02.21 um 14:40 schrieb Thomas Zimmermann: >> Fixes a regression for udl and probably other USB-based drivers where >> joining and mirroring displays fails. >> >> Joining displays requires importing a dma_buf from another DRM driver. >> These devices don't support DMA and therefore have no DMA mask. Trying >> to map imported buffers from a DMA-able memory zone fails with an error. >> An example is shown below. >> >> [ 60.050199] ------------[ cut here ]------------ >> [ 60.055092] WARNING: CPU: 0 PID: 1403 at kernel/dma/mapping.c:190 >> dma_map_sg_attrs+0x8f/0xc0 >> [ 60.064331] Modules linked in: af_packet(E) rfkill(E) dmi_sysfs(E) >> intel_rapl_msr(E) intel_rapl_common(E) snd_hda_codec_realtek(E) >> snd_hda_codec_generic(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) >> x86_pkg_temp_thermal(E) intel_powerclam) >> [ 60.064801] wmi(E) video(E) btrfs(E) blake2b_generic(E) >> libcrc32c(E) crc32c_intel(E) xor(E) raid6_pq(E) sg(E) dm_multipath(E) >> dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) msr(E) >> efivarfs(E) >> [ 60.170778] CPU: 0 PID: 1403 Comm: Xorg.bin Tainted: G >> E 5.11.0-rc5-1-default+ #747 >> [ 60.179841] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS A24 >> 10/24/2018 >> [ 60.187145] RIP: 0010:dma_map_sg_attrs+0x8f/0xc0 >> [ 60.191822] Code: 4d 85 ff 75 2b 49 89 d8 44 89 e1 44 89 f2 4c 89 >> ee 48 89 ef e8 62 30 00 00 85 c0 78 36 5b 5d 41 5c 41 5d 41 5e 41 5f >> c3 0f 0b <0f> 0b 31 c0 eb ed 49 8d 7f 50 e8 72 f5 2a 00 49 8b 47 50 49 >> 89 d8 >> [ 60.210770] RSP: 0018:ffffc90001d6fc18 EFLAGS: 00010246 >> [ 60.216062] RAX: 0000000000000000 RBX: 0000000000000020 RCX: >> ffffffffb31e677b >> [ 60.223274] RDX: dffffc0000000000 RSI: ffff888212c10600 RDI: >> ffff8881b08a9488 >> [ 60.230501] RBP: ffff8881b08a9030 R08: 0000000000000020 R09: >> ffff888212c10600 >> [ 60.237710] R10: ffffed10425820df R11: 0000000000000001 R12: >> 0000000000000000 >> [ 60.244939] R13: ffff888212c10600 R14: 0000000000000008 R15: >> 0000000000000000 >> [ 60.252155] FS: 00007f08ac2b2f00(0000) GS:ffff8887cbe00000(0000) >> knlGS:0000000000000000 >> [ 60.260333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> [ 60.266150] CR2: 000055831c899be8 CR3: 000000015372e006 CR4: >> 00000000001706f0 >> [ 60.273369] Call Trace: >> [ 60.275845] drm_gem_map_dma_buf+0xb4/0x120 >> [ 60.280089] dma_buf_map_attachment+0x15d/0x1e0 >> [ 60.284688] drm_gem_prime_import_dev.part.0+0x5d/0x140 >> [ 60.289984] drm_gem_prime_fd_to_handle+0x213/0x280 >> [ 60.294931] ? drm_prime_destroy_file_private+0x30/0x30 >> [ 60.300224] drm_ioctl_kernel+0x131/0x180 >> [ 60.304291] ? drm_setversion+0x230/0x230 >> [ 60.308366] ? drm_prime_destroy_file_private+0x30/0x30 >> [ 60.313659] drm_ioctl+0x2f1/0x500 >> [ 60.317118] ? drm_version+0x150/0x150 >> [ 60.320903] ? lock_downgrade+0xa0/0xa0 >> [ 60.324806] ? do_vfs_ioctl+0x5f4/0x680 >> [ 60.328694] ? __fget_files+0x13e/0x210 >> [ 60.332591] ? ioctl_fiemap.isra.0+0x1a0/0x1a0 >> [ 60.337102] ? __fget_files+0x15d/0x210 >> [ 60.340990] __x64_sys_ioctl+0xb9/0xf0 >> [ 60.344795] do_syscall_64+0x33/0x80 >> [ 60.348427] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> [ 60.353542] RIP: 0033:0x7f08ac7b53cb >> [ 60.357168] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c >> ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 >> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 ba 0c 00 f7 d8 64 89 >> 01 48 >> [ 60.376108] RSP: 002b:00007ffeabc89fc8 EFLAGS: 00000246 ORIG_RAX: >> 0000000000000010 >> [ 60.383758] RAX: ffffffffffffffda RBX: 00007ffeabc8a00c RCX: >> 00007f08ac7b53cb >> [ 60.390970] RDX: 00007ffeabc8a00c RSI: 00000000c00c642e RDI: >> 000000000000000d >> [ 60.398221] RBP: 00000000c00c642e R08: 000055831c691d00 R09: >> 000055831b2ec010 >> [ 60.405446] R10: 00007f08acf6ada0 R11: 0000000000000246 R12: >> 000055831c691d00 >> [ 60.412667] R13: 000000000000000d R14: 00000000007e9000 R15: >> 0000000000000000 >> [ 60.419903] irq event stamp: 672893 >> [ 60.423441] hardirqs last enabled at (672913): >> [<ffffffffb31b796d>] console_unlock+0x44d/0x500 >> [ 60.432230] hardirqs last disabled at (672922): >> [<ffffffffb31b7963>] console_unlock+0x443/0x500 >> [ 60.441021] softirqs last enabled at (672940): >> [<ffffffffb46003dd>] __do_softirq+0x3dd/0x554 >> [ 60.449634] softirqs last disabled at (672931): >> [<ffffffffb44010f2>] asm_call_irq_on_stack+0x12/0x20 >> [ 60.458871] ---[ end trace f2f88696eb17806c ]--- >> >> For the fix, we don't call dma_map_sgtable() for devices without the >> DMA mask. Drivers for such devices have to map the imported buffer into >> kernel address space and perfom the copy operation in software. >> >> Tested by joining/mirroring displays of udl and radeon un der Gnome/X11. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB >> devices") >> Cc: Christoph Hellwig <hch@lst.de> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Alan Stern <stern@rowland.harvard.edu> >> Cc: Johan Hovold <johan@kernel.org> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> >> Cc: "Ahmed S. Darwish" <a.darwish@linutronix.de> >> Cc: Mathias Nyman <mathias.nyman@linux.intel.com> >> Cc: Oliver Neukum <oneukum@suse.com> >> Cc: Felipe Balbi <balbi@kernel.org> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: <stable@vger.kernel.org> # v5.10+ >> --- >> drivers/gpu/drm/drm_prime.c | 27 ++++++++++++++++++--------- >> 1 file changed, 18 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >> index 2a54f86856af..d5a39fe76b78 100644 >> --- a/drivers/gpu/drm/drm_prime.c >> +++ b/drivers/gpu/drm/drm_prime.c >> @@ -603,10 +603,15 @@ EXPORT_SYMBOL(drm_gem_map_detach); >> * @attach: attachment whose scatterlist is to be returned >> * @dir: direction of DMA transfer >> * >> - * Calls &drm_gem_object_funcs.get_sg_table and then maps the >> scatterlist. This >> - * can be used as the &dma_buf_ops.map_dma_buf callback. Should be >> used together >> + * Calls &drm_gem_object_funcs.get_sg_table and, if possible, maps >> the scatterlist. >> + * This can be used as the &dma_buf_ops.map_dma_buf callback. Should >> be used together >> * with drm_gem_unmap_dma_buf(). >> * >> + * Devices on some peripheral busses, such as USB, cannot use DMA. In >> this case, >> + * pages in the scatterlist remain unmapped. Drivers for such devices >> should acquire >> + * a mapping with dma_buf_vmap() and implement copy operation via >> bus-specific >> + * interfaces. >> + * >> * Returns:sg_table containing the scatterlist to be returned; >> returns ERR_PTR >> * on error. May return -EINTR if it is interrupted by a signal. >> */ >> @@ -627,12 +632,14 @@ struct sg_table *drm_gem_map_dma_buf(struct >> dma_buf_attachment *attach, >> if (IS_ERR(sgt)) >> return sgt; >> >> - ret = dma_map_sgtable(attach->dev, sgt, dir, >> - DMA_ATTR_SKIP_CPU_SYNC); >> - if (ret) { >> - sg_free_table(sgt); >> - kfree(sgt); >> - sgt = ERR_PTR(ret); >> + if (attach->dev->dma_mask) { >> + ret = dma_map_sgtable(attach->dev, sgt, dir, >> + DMA_ATTR_SKIP_CPU_SYNC); >> + if (ret) { >> + sg_free_table(sgt); >> + kfree(sgt); >> + sgt = ERR_PTR(ret); >> + } >> } >> >> return sgt; >> @@ -654,7 +661,9 @@ void drm_gem_unmap_dma_buf(struct >> dma_buf_attachment *attach, >> if (!sgt) >> return; >> >> - dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); >> + if (attach->dev->dma_mask) >> + dma_unmap_sgtable(attach->dev, sgt, dir, >> DMA_ATTR_SKIP_CPU_SYNC); >> + >> sg_free_table(sgt); >> kfree(sgt); >> } >> -- >> 2.30.0 >> > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Am 19.02.21 um 15:34 schrieb Thomas Zimmermann: > Hi > > Am 19.02.21 um 14:45 schrieb Christian König: >> Well as far as I can see this is a relative clear NAK. >> >> When a device can't do DMA and has no DMA mask then why it is >> requesting an sg-table in the first place? >> >> The problem is rather in drm_gem_prime_import_dev() which always >> tries to create an sg-table to get a GEM object, even when the device >> doesn't support DMA at all. > > Sure, that's the clean solution. But it also needs a bit of work on > the way the importing works. Our generic code assumes that an SG table > is being used. Most driver's memory managers as well, I guess. > > The regressing patch has already been in upstream kernels for a while. > The fix at hand is backport-able at least. > > Can we add a TODO item for the real thing? Not really. The "fix" here is a hack for the export handling code in DRM. That only works because the USB device tries to communicate with another DRM driver, but is not a general solution you could backport. What you can do instead for example to fix this is to implement the gem_prime_import callback in the USB device so that the GEM object is created without trying to get an sg-table without being DMA capable. Regards, Christian. > > Best regards > Thomas > >> >> Regards, >> Christian. >> >> Am 19.02.21 um 14:40 schrieb Thomas Zimmermann: >>> Fixes a regression for udl and probably other USB-based drivers where >>> joining and mirroring displays fails. >>> >>> Joining displays requires importing a dma_buf from another DRM driver. >>> These devices don't support DMA and therefore have no DMA mask. Trying >>> to map imported buffers from a DMA-able memory zone fails with an >>> error. >>> An example is shown below. >>> >>> [ 60.050199] ------------[ cut here ]------------ >>> [ 60.055092] WARNING: CPU: 0 PID: 1403 at kernel/dma/mapping.c:190 >>> dma_map_sg_attrs+0x8f/0xc0 >>> [ 60.064331] Modules linked in: af_packet(E) rfkill(E) >>> dmi_sysfs(E) intel_rapl_msr(E) intel_rapl_common(E) >>> snd_hda_codec_realtek(E) snd_hda_codec_generic(E) ledtrig_audio(E) >>> snd_hda_codec_hdmi(E) x86_pkg_temp_thermal(E) intel_powerclam) >>> [ 60.064801] wmi(E) video(E) btrfs(E) blake2b_generic(E) >>> libcrc32c(E) crc32c_intel(E) xor(E) raid6_pq(E) sg(E) >>> dm_multipath(E) dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) >>> scsi_dh_alua(E) msr(E) efivarfs(E) >>> [ 60.170778] CPU: 0 PID: 1403 Comm: Xorg.bin Tainted: G >>> E 5.11.0-rc5-1-default+ #747 >>> [ 60.179841] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS >>> A24 10/24/2018 >>> [ 60.187145] RIP: 0010:dma_map_sg_attrs+0x8f/0xc0 >>> [ 60.191822] Code: 4d 85 ff 75 2b 49 89 d8 44 89 e1 44 89 f2 4c 89 >>> ee 48 89 ef e8 62 30 00 00 85 c0 78 36 5b 5d 41 5c 41 5d 41 5e 41 5f >>> c3 0f 0b <0f> 0b 31 c0 eb ed 49 8d 7f 50 e8 72 f5 2a 00 49 8b 47 50 >>> 49 89 d8 >>> [ 60.210770] RSP: 0018:ffffc90001d6fc18 EFLAGS: 00010246 >>> [ 60.216062] RAX: 0000000000000000 RBX: 0000000000000020 RCX: >>> ffffffffb31e677b >>> [ 60.223274] RDX: dffffc0000000000 RSI: ffff888212c10600 RDI: >>> ffff8881b08a9488 >>> [ 60.230501] RBP: ffff8881b08a9030 R08: 0000000000000020 R09: >>> ffff888212c10600 >>> [ 60.237710] R10: ffffed10425820df R11: 0000000000000001 R12: >>> 0000000000000000 >>> [ 60.244939] R13: ffff888212c10600 R14: 0000000000000008 R15: >>> 0000000000000000 >>> [ 60.252155] FS: 00007f08ac2b2f00(0000) GS:ffff8887cbe00000(0000) >>> knlGS:0000000000000000 >>> [ 60.260333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> [ 60.266150] CR2: 000055831c899be8 CR3: 000000015372e006 CR4: >>> 00000000001706f0 >>> [ 60.273369] Call Trace: >>> [ 60.275845] drm_gem_map_dma_buf+0xb4/0x120 >>> [ 60.280089] dma_buf_map_attachment+0x15d/0x1e0 >>> [ 60.284688] drm_gem_prime_import_dev.part.0+0x5d/0x140 >>> [ 60.289984] drm_gem_prime_fd_to_handle+0x213/0x280 >>> [ 60.294931] ? drm_prime_destroy_file_private+0x30/0x30 >>> [ 60.300224] drm_ioctl_kernel+0x131/0x180 >>> [ 60.304291] ? drm_setversion+0x230/0x230 >>> [ 60.308366] ? drm_prime_destroy_file_private+0x30/0x30 >>> [ 60.313659] drm_ioctl+0x2f1/0x500 >>> [ 60.317118] ? drm_version+0x150/0x150 >>> [ 60.320903] ? lock_downgrade+0xa0/0xa0 >>> [ 60.324806] ? do_vfs_ioctl+0x5f4/0x680 >>> [ 60.328694] ? __fget_files+0x13e/0x210 >>> [ 60.332591] ? ioctl_fiemap.isra.0+0x1a0/0x1a0 >>> [ 60.337102] ? __fget_files+0x15d/0x210 >>> [ 60.340990] __x64_sys_ioctl+0xb9/0xf0 >>> [ 60.344795] do_syscall_64+0x33/0x80 >>> [ 60.348427] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >>> [ 60.353542] RIP: 0033:0x7f08ac7b53cb >>> [ 60.357168] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c >>> ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 >>> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 ba 0c 00 f7 d8 64 >>> 89 01 48 >>> [ 60.376108] RSP: 002b:00007ffeabc89fc8 EFLAGS: 00000246 ORIG_RAX: >>> 0000000000000010 >>> [ 60.383758] RAX: ffffffffffffffda RBX: 00007ffeabc8a00c RCX: >>> 00007f08ac7b53cb >>> [ 60.390970] RDX: 00007ffeabc8a00c RSI: 00000000c00c642e RDI: >>> 000000000000000d >>> [ 60.398221] RBP: 00000000c00c642e R08: 000055831c691d00 R09: >>> 000055831b2ec010 >>> [ 60.405446] R10: 00007f08acf6ada0 R11: 0000000000000246 R12: >>> 000055831c691d00 >>> [ 60.412667] R13: 000000000000000d R14: 00000000007e9000 R15: >>> 0000000000000000 >>> [ 60.419903] irq event stamp: 672893 >>> [ 60.423441] hardirqs last enabled at (672913): >>> [<ffffffffb31b796d>] console_unlock+0x44d/0x500 >>> [ 60.432230] hardirqs last disabled at (672922): >>> [<ffffffffb31b7963>] console_unlock+0x443/0x500 >>> [ 60.441021] softirqs last enabled at (672940): >>> [<ffffffffb46003dd>] __do_softirq+0x3dd/0x554 >>> [ 60.449634] softirqs last disabled at (672931): >>> [<ffffffffb44010f2>] asm_call_irq_on_stack+0x12/0x20 >>> [ 60.458871] ---[ end trace f2f88696eb17806c ]--- >>> >>> For the fix, we don't call dma_map_sgtable() for devices without the >>> DMA mask. Drivers for such devices have to map the imported buffer into >>> kernel address space and perfom the copy operation in software. >>> >>> Tested by joining/mirroring displays of udl and radeon un der >>> Gnome/X11. >>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB >>> devices") >>> Cc: Christoph Hellwig <hch@lst.de> >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> Cc: Alan Stern <stern@rowland.harvard.edu> >>> Cc: Johan Hovold <johan@kernel.org> >>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> >>> Cc: "Ahmed S. Darwish" <a.darwish@linutronix.de> >>> Cc: Mathias Nyman <mathias.nyman@linux.intel.com> >>> Cc: Oliver Neukum <oneukum@suse.com> >>> Cc: Felipe Balbi <balbi@kernel.org> >>> Cc: Thomas Gleixner <tglx@linutronix.de> >>> Cc: <stable@vger.kernel.org> # v5.10+ >>> --- >>> drivers/gpu/drm/drm_prime.c | 27 ++++++++++++++++++--------- >>> 1 file changed, 18 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >>> index 2a54f86856af..d5a39fe76b78 100644 >>> --- a/drivers/gpu/drm/drm_prime.c >>> +++ b/drivers/gpu/drm/drm_prime.c >>> @@ -603,10 +603,15 @@ EXPORT_SYMBOL(drm_gem_map_detach); >>> * @attach: attachment whose scatterlist is to be returned >>> * @dir: direction of DMA transfer >>> * >>> - * Calls &drm_gem_object_funcs.get_sg_table and then maps the >>> scatterlist. This >>> - * can be used as the &dma_buf_ops.map_dma_buf callback. Should be >>> used together >>> + * Calls &drm_gem_object_funcs.get_sg_table and, if possible, maps >>> the scatterlist. >>> + * This can be used as the &dma_buf_ops.map_dma_buf callback. >>> Should be used together >>> * with drm_gem_unmap_dma_buf(). >>> * >>> + * Devices on some peripheral busses, such as USB, cannot use DMA. >>> In this case, >>> + * pages in the scatterlist remain unmapped. Drivers for such >>> devices should acquire >>> + * a mapping with dma_buf_vmap() and implement copy operation via >>> bus-specific >>> + * interfaces. >>> + * >>> * Returns:sg_table containing the scatterlist to be returned; >>> returns ERR_PTR >>> * on error. May return -EINTR if it is interrupted by a signal. >>> */ >>> @@ -627,12 +632,14 @@ struct sg_table *drm_gem_map_dma_buf(struct >>> dma_buf_attachment *attach, >>> if (IS_ERR(sgt)) >>> return sgt; >>> >>> - ret = dma_map_sgtable(attach->dev, sgt, dir, >>> - DMA_ATTR_SKIP_CPU_SYNC); >>> - if (ret) { >>> - sg_free_table(sgt); >>> - kfree(sgt); >>> - sgt = ERR_PTR(ret); >>> + if (attach->dev->dma_mask) { >>> + ret = dma_map_sgtable(attach->dev, sgt, dir, >>> + DMA_ATTR_SKIP_CPU_SYNC); >>> + if (ret) { >>> + sg_free_table(sgt); >>> + kfree(sgt); >>> + sgt = ERR_PTR(ret); >>> + } >>> } >>> >>> return sgt; >>> @@ -654,7 +661,9 @@ void drm_gem_unmap_dma_buf(struct >>> dma_buf_attachment *attach, >>> if (!sgt) >>> return; >>> >>> - dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); >>> + if (attach->dev->dma_mask) >>> + dma_unmap_sgtable(attach->dev, sgt, dir, >>> DMA_ATTR_SKIP_CPU_SYNC); >>> + >>> sg_free_table(sgt); >>> kfree(sgt); >>> } >>> -- >>> 2.30.0 >>> >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >
On Fri, Feb 19, 2021 at 3:35 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Hi > > Am 19.02.21 um 14:45 schrieb Christian König: > > Well as far as I can see this is a relative clear NAK. > > > > When a device can't do DMA and has no DMA mask then why it is requesting > > an sg-table in the first place? > > > > The problem is rather in drm_gem_prime_import_dev() which always tries > > to create an sg-table to get a GEM object, even when the device doesn't > > support DMA at all. > > Sure, that's the clean solution. But it also needs a bit of work on the > way the importing works. Our generic code assumes that an SG table is > being used. Most driver's memory managers as well, I guess. > > The regressing patch has already been in upstream kernels for a while. > The fix at hand is backport-able at least. > > Can we add a TODO item for the real thing? I'm not against a quick&dirt fix for this regression, but this is kinda on the wrong side: The issue is that importers did the wrong thing, and now that the dma-api is a bit more strict, this stopped working. So if we throw a hack at the problem, it needs to be on the importer's side. If we now add a hack on the exporter side, we risk that it gets used in more places, and it's rather hard to get the cat back into the bag. E.g. all the importers relying on a struct page being there is really hard to fix. We shouldn't make the same mistake here. -Daniel > > Best regards > Thomas > > > > > Regards, > > Christian. > > > > Am 19.02.21 um 14:40 schrieb Thomas Zimmermann: > >> Fixes a regression for udl and probably other USB-based drivers where > >> joining and mirroring displays fails. > >> > >> Joining displays requires importing a dma_buf from another DRM driver. > >> These devices don't support DMA and therefore have no DMA mask. Trying > >> to map imported buffers from a DMA-able memory zone fails with an error. > >> An example is shown below. > >> > >> [ 60.050199] ------------[ cut here ]------------ > >> [ 60.055092] WARNING: CPU: 0 PID: 1403 at kernel/dma/mapping.c:190 > >> dma_map_sg_attrs+0x8f/0xc0 > >> [ 60.064331] Modules linked in: af_packet(E) rfkill(E) dmi_sysfs(E) > >> intel_rapl_msr(E) intel_rapl_common(E) snd_hda_codec_realtek(E) > >> snd_hda_codec_generic(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) > >> x86_pkg_temp_thermal(E) intel_powerclam) > >> [ 60.064801] wmi(E) video(E) btrfs(E) blake2b_generic(E) > >> libcrc32c(E) crc32c_intel(E) xor(E) raid6_pq(E) sg(E) dm_multipath(E) > >> dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) msr(E) > >> efivarfs(E) > >> [ 60.170778] CPU: 0 PID: 1403 Comm: Xorg.bin Tainted: G > >> E 5.11.0-rc5-1-default+ #747 > >> [ 60.179841] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS A24 > >> 10/24/2018 > >> [ 60.187145] RIP: 0010:dma_map_sg_attrs+0x8f/0xc0 > >> [ 60.191822] Code: 4d 85 ff 75 2b 49 89 d8 44 89 e1 44 89 f2 4c 89 > >> ee 48 89 ef e8 62 30 00 00 85 c0 78 36 5b 5d 41 5c 41 5d 41 5e 41 5f > >> c3 0f 0b <0f> 0b 31 c0 eb ed 49 8d 7f 50 e8 72 f5 2a 00 49 8b 47 50 49 > >> 89 d8 > >> [ 60.210770] RSP: 0018:ffffc90001d6fc18 EFLAGS: 00010246 > >> [ 60.216062] RAX: 0000000000000000 RBX: 0000000000000020 RCX: > >> ffffffffb31e677b > >> [ 60.223274] RDX: dffffc0000000000 RSI: ffff888212c10600 RDI: > >> ffff8881b08a9488 > >> [ 60.230501] RBP: ffff8881b08a9030 R08: 0000000000000020 R09: > >> ffff888212c10600 > >> [ 60.237710] R10: ffffed10425820df R11: 0000000000000001 R12: > >> 0000000000000000 > >> [ 60.244939] R13: ffff888212c10600 R14: 0000000000000008 R15: > >> 0000000000000000 > >> [ 60.252155] FS: 00007f08ac2b2f00(0000) GS:ffff8887cbe00000(0000) > >> knlGS:0000000000000000 > >> [ 60.260333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > >> [ 60.266150] CR2: 000055831c899be8 CR3: 000000015372e006 CR4: > >> 00000000001706f0 > >> [ 60.273369] Call Trace: > >> [ 60.275845] drm_gem_map_dma_buf+0xb4/0x120 > >> [ 60.280089] dma_buf_map_attachment+0x15d/0x1e0 > >> [ 60.284688] drm_gem_prime_import_dev.part.0+0x5d/0x140 > >> [ 60.289984] drm_gem_prime_fd_to_handle+0x213/0x280 > >> [ 60.294931] ? drm_prime_destroy_file_private+0x30/0x30 > >> [ 60.300224] drm_ioctl_kernel+0x131/0x180 > >> [ 60.304291] ? drm_setversion+0x230/0x230 > >> [ 60.308366] ? drm_prime_destroy_file_private+0x30/0x30 > >> [ 60.313659] drm_ioctl+0x2f1/0x500 > >> [ 60.317118] ? drm_version+0x150/0x150 > >> [ 60.320903] ? lock_downgrade+0xa0/0xa0 > >> [ 60.324806] ? do_vfs_ioctl+0x5f4/0x680 > >> [ 60.328694] ? __fget_files+0x13e/0x210 > >> [ 60.332591] ? ioctl_fiemap.isra.0+0x1a0/0x1a0 > >> [ 60.337102] ? __fget_files+0x15d/0x210 > >> [ 60.340990] __x64_sys_ioctl+0xb9/0xf0 > >> [ 60.344795] do_syscall_64+0x33/0x80 > >> [ 60.348427] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > >> [ 60.353542] RIP: 0033:0x7f08ac7b53cb > >> [ 60.357168] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c > >> ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 > >> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 ba 0c 00 f7 d8 64 89 > >> 01 48 > >> [ 60.376108] RSP: 002b:00007ffeabc89fc8 EFLAGS: 00000246 ORIG_RAX: > >> 0000000000000010 > >> [ 60.383758] RAX: ffffffffffffffda RBX: 00007ffeabc8a00c RCX: > >> 00007f08ac7b53cb > >> [ 60.390970] RDX: 00007ffeabc8a00c RSI: 00000000c00c642e RDI: > >> 000000000000000d > >> [ 60.398221] RBP: 00000000c00c642e R08: 000055831c691d00 R09: > >> 000055831b2ec010 > >> [ 60.405446] R10: 00007f08acf6ada0 R11: 0000000000000246 R12: > >> 000055831c691d00 > >> [ 60.412667] R13: 000000000000000d R14: 00000000007e9000 R15: > >> 0000000000000000 > >> [ 60.419903] irq event stamp: 672893 > >> [ 60.423441] hardirqs last enabled at (672913): > >> [<ffffffffb31b796d>] console_unlock+0x44d/0x500 > >> [ 60.432230] hardirqs last disabled at (672922): > >> [<ffffffffb31b7963>] console_unlock+0x443/0x500 > >> [ 60.441021] softirqs last enabled at (672940): > >> [<ffffffffb46003dd>] __do_softirq+0x3dd/0x554 > >> [ 60.449634] softirqs last disabled at (672931): > >> [<ffffffffb44010f2>] asm_call_irq_on_stack+0x12/0x20 > >> [ 60.458871] ---[ end trace f2f88696eb17806c ]--- > >> > >> For the fix, we don't call dma_map_sgtable() for devices without the > >> DMA mask. Drivers for such devices have to map the imported buffer into > >> kernel address space and perfom the copy operation in software. > >> > >> Tested by joining/mirroring displays of udl and radeon un der Gnome/X11. > >> > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > >> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB > >> devices") > >> Cc: Christoph Hellwig <hch@lst.de> > >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >> Cc: Alan Stern <stern@rowland.harvard.edu> > >> Cc: Johan Hovold <johan@kernel.org> > >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > >> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > >> Cc: "Ahmed S. Darwish" <a.darwish@linutronix.de> > >> Cc: Mathias Nyman <mathias.nyman@linux.intel.com> > >> Cc: Oliver Neukum <oneukum@suse.com> > >> Cc: Felipe Balbi <balbi@kernel.org> > >> Cc: Thomas Gleixner <tglx@linutronix.de> > >> Cc: <stable@vger.kernel.org> # v5.10+ > >> --- > >> drivers/gpu/drm/drm_prime.c | 27 ++++++++++++++++++--------- > >> 1 file changed, 18 insertions(+), 9 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > >> index 2a54f86856af..d5a39fe76b78 100644 > >> --- a/drivers/gpu/drm/drm_prime.c > >> +++ b/drivers/gpu/drm/drm_prime.c > >> @@ -603,10 +603,15 @@ EXPORT_SYMBOL(drm_gem_map_detach); > >> * @attach: attachment whose scatterlist is to be returned > >> * @dir: direction of DMA transfer > >> * > >> - * Calls &drm_gem_object_funcs.get_sg_table and then maps the > >> scatterlist. This > >> - * can be used as the &dma_buf_ops.map_dma_buf callback. Should be > >> used together > >> + * Calls &drm_gem_object_funcs.get_sg_table and, if possible, maps > >> the scatterlist. > >> + * This can be used as the &dma_buf_ops.map_dma_buf callback. Should > >> be used together > >> * with drm_gem_unmap_dma_buf(). > >> * > >> + * Devices on some peripheral busses, such as USB, cannot use DMA. In > >> this case, > >> + * pages in the scatterlist remain unmapped. Drivers for such devices > >> should acquire > >> + * a mapping with dma_buf_vmap() and implement copy operation via > >> bus-specific > >> + * interfaces. > >> + * > >> * Returns:sg_table containing the scatterlist to be returned; > >> returns ERR_PTR > >> * on error. May return -EINTR if it is interrupted by a signal. > >> */ > >> @@ -627,12 +632,14 @@ struct sg_table *drm_gem_map_dma_buf(struct > >> dma_buf_attachment *attach, > >> if (IS_ERR(sgt)) > >> return sgt; > >> > >> - ret = dma_map_sgtable(attach->dev, sgt, dir, > >> - DMA_ATTR_SKIP_CPU_SYNC); > >> - if (ret) { > >> - sg_free_table(sgt); > >> - kfree(sgt); > >> - sgt = ERR_PTR(ret); > >> + if (attach->dev->dma_mask) { > >> + ret = dma_map_sgtable(attach->dev, sgt, dir, > >> + DMA_ATTR_SKIP_CPU_SYNC); > >> + if (ret) { > >> + sg_free_table(sgt); > >> + kfree(sgt); > >> + sgt = ERR_PTR(ret); > >> + } > >> } > >> > >> return sgt; > >> @@ -654,7 +661,9 @@ void drm_gem_unmap_dma_buf(struct > >> dma_buf_attachment *attach, > >> if (!sgt) > >> return; > >> > >> - dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); > >> + if (attach->dev->dma_mask) > >> + dma_unmap_sgtable(attach->dev, sgt, dir, > >> DMA_ATTR_SKIP_CPU_SYNC); > >> + > >> sg_free_table(sgt); > >> kfree(sgt); > >> } > >> -- > >> 2.30.0 > >> > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer >
On Fri, Feb 19, 2021 at 02:45:54PM +0100, Christian König wrote: > Well as far as I can see this is a relative clear NAK. > > When a device can't do DMA and has no DMA mask then why it is requesting an > sg-table in the first place? This may not be important for your discussion, but I'd like to give an answer to the question -- at least, for the case of USB. A USB device cannot do DMA and has no DMA mask. Nevertheless, if you want to send large amounts of bulk data to/from a USB device then using an SG table is often a good way to do it. The reason is simple: All communication with a USB device has to go through a USB host controller, and many (though not all) host controllers _can_ do DMA and _do_ have a DMA mask. The USB mass-storage and uas drivers in particular make heavy use of this mechanism. Alan Stern
Am 19.02.21 um 16:53 schrieb Alan Stern: > On Fri, Feb 19, 2021 at 02:45:54PM +0100, Christian König wrote: >> Well as far as I can see this is a relative clear NAK. >> >> When a device can't do DMA and has no DMA mask then why it is requesting an >> sg-table in the first place? > This may not be important for your discussion, but I'd like to give an > answer to the question -- at least, for the case of USB. > > A USB device cannot do DMA and has no DMA mask. Nevertheless, if you > want to send large amounts of bulk data to/from a USB device then using > an SG table is often a good way to do it. The reason is simple: All > communication with a USB device has to go through a USB host controller, > and many (though not all) host controllers _can_ do DMA and _do_ have a > DMA mask. > > The USB mass-storage and uas drivers in particular make heavy use of > this mechanism. Yeah, I was assuming something like that would work. But in this case the USB device should give the host controllers device structure to the dma_buf_attach function so that the sg_table can be filled in with DMA addresses properly. Regards, Christian. > > Alan Stern
On Fri, Feb 19, 2021 at 04:56:16PM +0100, Christian König wrote: > > > Am 19.02.21 um 16:53 schrieb Alan Stern: > > On Fri, Feb 19, 2021 at 02:45:54PM +0100, Christian König wrote: > > > Well as far as I can see this is a relative clear NAK. > > > > > > When a device can't do DMA and has no DMA mask then why it is requesting an > > > sg-table in the first place? > > This may not be important for your discussion, but I'd like to give an > > answer to the question -- at least, for the case of USB. > > > > A USB device cannot do DMA and has no DMA mask. Nevertheless, if you > > want to send large amounts of bulk data to/from a USB device then using > > an SG table is often a good way to do it. The reason is simple: All > > communication with a USB device has to go through a USB host controller, > > and many (though not all) host controllers _can_ do DMA and _do_ have a > > DMA mask. > > > > The USB mass-storage and uas drivers in particular make heavy use of > > this mechanism. > > Yeah, I was assuming something like that would work. > > But in this case the USB device should give the host controllers device > structure to the dma_buf_attach function so that the sg_table can be filled > in with DMA addresses properly. Indeed. Although in the contexts I'm familiar with, the host controller device is actually passed to routines like dma_pool_create, dma_alloc_coherent, dma_map_sg, or dma_map_single. Alan Stern
Den 19.02.2021 14.40, skrev Thomas Zimmermann: > Fixes a regression for udl and probably other USB-based drivers where > joining and mirroring displays fails. > > Joining displays requires importing a dma_buf from another DRM driver. > These devices don't support DMA and therefore have no DMA mask. Trying > to map imported buffers from a DMA-able memory zone fails with an error. > An example is shown below. > > [ 60.050199] ------------[ cut here ]------------ > [ 60.055092] WARNING: CPU: 0 PID: 1403 at kernel/dma/mapping.c:190 dma_map_sg_attrs+0x8f/0xc0 > [ 60.064331] Modules linked in: af_packet(E) rfkill(E) dmi_sysfs(E) intel_rapl_msr(E) intel_rapl_common(E) snd_hda_codec_realtek(E) snd_hda_codec_generic(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) x86_pkg_temp_thermal(E) intel_powerclam) > [ 60.064801] wmi(E) video(E) btrfs(E) blake2b_generic(E) libcrc32c(E) crc32c_intel(E) xor(E) raid6_pq(E) sg(E) dm_multipath(E) dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) msr(E) efivarfs(E) > [ 60.170778] CPU: 0 PID: 1403 Comm: Xorg.bin Tainted: G E 5.11.0-rc5-1-default+ #747 > [ 60.179841] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS A24 10/24/2018 > [ 60.187145] RIP: 0010:dma_map_sg_attrs+0x8f/0xc0 > [ 60.191822] Code: 4d 85 ff 75 2b 49 89 d8 44 89 e1 44 89 f2 4c 89 ee 48 89 ef e8 62 30 00 00 85 c0 78 36 5b 5d 41 5c 41 5d 41 5e 41 5f c3 0f 0b <0f> 0b 31 c0 eb ed 49 8d 7f 50 e8 72 f5 2a 00 49 8b 47 50 49 89 d8 > [ 60.210770] RSP: 0018:ffffc90001d6fc18 EFLAGS: 00010246 > [ 60.216062] RAX: 0000000000000000 RBX: 0000000000000020 RCX: ffffffffb31e677b > [ 60.223274] RDX: dffffc0000000000 RSI: ffff888212c10600 RDI: ffff8881b08a9488 > [ 60.230501] RBP: ffff8881b08a9030 R08: 0000000000000020 R09: ffff888212c10600 > [ 60.237710] R10: ffffed10425820df R11: 0000000000000001 R12: 0000000000000000 > [ 60.244939] R13: ffff888212c10600 R14: 0000000000000008 R15: 0000000000000000 > [ 60.252155] FS: 00007f08ac2b2f00(0000) GS:ffff8887cbe00000(0000) knlGS:0000000000000000 > [ 60.260333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 60.266150] CR2: 000055831c899be8 CR3: 000000015372e006 CR4: 00000000001706f0 > [ 60.273369] Call Trace: > [ 60.275845] drm_gem_map_dma_buf+0xb4/0x120 > [ 60.280089] dma_buf_map_attachment+0x15d/0x1e0 > [ 60.284688] drm_gem_prime_import_dev.part.0+0x5d/0x140 > [ 60.289984] drm_gem_prime_fd_to_handle+0x213/0x280 > [ 60.294931] ? drm_prime_destroy_file_private+0x30/0x30 > [ 60.300224] drm_ioctl_kernel+0x131/0x180 > [ 60.304291] ? drm_setversion+0x230/0x230 > [ 60.308366] ? drm_prime_destroy_file_private+0x30/0x30 > [ 60.313659] drm_ioctl+0x2f1/0x500 > [ 60.317118] ? drm_version+0x150/0x150 > [ 60.320903] ? lock_downgrade+0xa0/0xa0 > [ 60.324806] ? do_vfs_ioctl+0x5f4/0x680 > [ 60.328694] ? __fget_files+0x13e/0x210 > [ 60.332591] ? ioctl_fiemap.isra.0+0x1a0/0x1a0 > [ 60.337102] ? __fget_files+0x15d/0x210 > [ 60.340990] __x64_sys_ioctl+0xb9/0xf0 > [ 60.344795] do_syscall_64+0x33/0x80 > [ 60.348427] entry_SYSCALL_64_after_hwframe+0x44/0xa9 I'm working on a USB display driver and got the same splat (although from i915_gem_map_dma_buf) and silenced it using: ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); But I understand now that this is not the solution. Since the USB host controller can do DMA, would the following be an acceptable solution? static struct drm_gem_object *gud_gem_prime_import(struct drm_device *drm, struct dma_buf *dma_buf) { struct usb_device *usb = gud_to_usb_device(to_gud_device(drm)); drm_dbg_prime(drm, "usb->bus->controller=%s\n", dev_name(usb->bus->controller)); return drm_gem_prime_import_dev(drm, dma_buf, usb->bus->controller); } static const struct drm_driver gud_drm_driver = { .gem_prime_import = gud_gem_prime_import, }; Noralf. > [ 60.353542] RIP: 0033:0x7f08ac7b53cb > [ 60.357168] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 ba 0c 00 f7 d8 64 89 01 48 > [ 60.376108] RSP: 002b:00007ffeabc89fc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 > [ 60.383758] RAX: ffffffffffffffda RBX: 00007ffeabc8a00c RCX: 00007f08ac7b53cb > [ 60.390970] RDX: 00007ffeabc8a00c RSI: 00000000c00c642e RDI: 000000000000000d > [ 60.398221] RBP: 00000000c00c642e R08: 000055831c691d00 R09: 000055831b2ec010 > [ 60.405446] R10: 00007f08acf6ada0 R11: 0000000000000246 R12: 000055831c691d00 > [ 60.412667] R13: 000000000000000d R14: 00000000007e9000 R15: 0000000000000000 > [ 60.419903] irq event stamp: 672893 > [ 60.423441] hardirqs last enabled at (672913): [<ffffffffb31b796d>] console_unlock+0x44d/0x500 > [ 60.432230] hardirqs last disabled at (672922): [<ffffffffb31b7963>] console_unlock+0x443/0x500 > [ 60.441021] softirqs last enabled at (672940): [<ffffffffb46003dd>] __do_softirq+0x3dd/0x554 > [ 60.449634] softirqs last disabled at (672931): [<ffffffffb44010f2>] asm_call_irq_on_stack+0x12/0x20 > [ 60.458871] ---[ end trace f2f88696eb17806c ]--- > > For the fix, we don't call dma_map_sgtable() for devices without the > DMA mask. Drivers for such devices have to map the imported buffer into > kernel address space and perfom the copy operation in software. > > Tested by joining/mirroring displays of udl and radeon un der Gnome/X11. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices") > Cc: Christoph Hellwig <hch@lst.de> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Alan Stern <stern@rowland.harvard.edu> > Cc: Johan Hovold <johan@kernel.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Cc: "Ahmed S. Darwish" <a.darwish@linutronix.de> > Cc: Mathias Nyman <mathias.nyman@linux.intel.com> > Cc: Oliver Neukum <oneukum@suse.com> > Cc: Felipe Balbi <balbi@kernel.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: <stable@vger.kernel.org> # v5.10+ > --- > drivers/gpu/drm/drm_prime.c | 27 ++++++++++++++++++--------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 2a54f86856af..d5a39fe76b78 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -603,10 +603,15 @@ EXPORT_SYMBOL(drm_gem_map_detach); > * @attach: attachment whose scatterlist is to be returned > * @dir: direction of DMA transfer > * > - * Calls &drm_gem_object_funcs.get_sg_table and then maps the scatterlist. This > - * can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together > + * Calls &drm_gem_object_funcs.get_sg_table and, if possible, maps the scatterlist. > + * This can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together > * with drm_gem_unmap_dma_buf(). > * > + * Devices on some peripheral busses, such as USB, cannot use DMA. In this case, > + * pages in the scatterlist remain unmapped. Drivers for such devices should acquire > + * a mapping with dma_buf_vmap() and implement copy operation via bus-specific > + * interfaces. > + * > * Returns:sg_table containing the scatterlist to be returned; returns ERR_PTR > * on error. May return -EINTR if it is interrupted by a signal. > */ > @@ -627,12 +632,14 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, > if (IS_ERR(sgt)) > return sgt; > > - ret = dma_map_sgtable(attach->dev, sgt, dir, > - DMA_ATTR_SKIP_CPU_SYNC); > - if (ret) { > - sg_free_table(sgt); > - kfree(sgt); > - sgt = ERR_PTR(ret); > + if (attach->dev->dma_mask) { > + ret = dma_map_sgtable(attach->dev, sgt, dir, > + DMA_ATTR_SKIP_CPU_SYNC); > + if (ret) { > + sg_free_table(sgt); > + kfree(sgt); > + sgt = ERR_PTR(ret); > + } > } > > return sgt; > @@ -654,7 +661,9 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach, > if (!sgt) > return; > > - dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); > + if (attach->dev->dma_mask) > + dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); > + > sg_free_table(sgt); > kfree(sgt); > } > -- > 2.30.0 >
Am 20.02.21 um 14:02 schrieb Noralf Trønnes: > > Den 19.02.2021 14.40, skrev Thomas Zimmermann: >> Fixes a regression for udl and probably other USB-based drivers where >> joining and mirroring displays fails. >> >> Joining displays requires importing a dma_buf from another DRM driver. >> These devices don't support DMA and therefore have no DMA mask. Trying >> to map imported buffers from a DMA-able memory zone fails with an error. >> An example is shown below. >> >> [ 60.050199] ------------[ cut here ]------------ >> [ 60.055092] WARNING: CPU: 0 PID: 1403 at kernel/dma/mapping.c:190 dma_map_sg_attrs+0x8f/0xc0 >> [ 60.064331] Modules linked in: af_packet(E) rfkill(E) dmi_sysfs(E) intel_rapl_msr(E) intel_rapl_common(E) snd_hda_codec_realtek(E) snd_hda_codec_generic(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) x86_pkg_temp_thermal(E) intel_powerclam) >> [ 60.064801] wmi(E) video(E) btrfs(E) blake2b_generic(E) libcrc32c(E) crc32c_intel(E) xor(E) raid6_pq(E) sg(E) dm_multipath(E) dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) msr(E) efivarfs(E) >> [ 60.170778] CPU: 0 PID: 1403 Comm: Xorg.bin Tainted: G E 5.11.0-rc5-1-default+ #747 >> [ 60.179841] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS A24 10/24/2018 >> [ 60.187145] RIP: 0010:dma_map_sg_attrs+0x8f/0xc0 >> [ 60.191822] Code: 4d 85 ff 75 2b 49 89 d8 44 89 e1 44 89 f2 4c 89 ee 48 89 ef e8 62 30 00 00 85 c0 78 36 5b 5d 41 5c 41 5d 41 5e 41 5f c3 0f 0b <0f> 0b 31 c0 eb ed 49 8d 7f 50 e8 72 f5 2a 00 49 8b 47 50 49 89 d8 >> [ 60.210770] RSP: 0018:ffffc90001d6fc18 EFLAGS: 00010246 >> [ 60.216062] RAX: 0000000000000000 RBX: 0000000000000020 RCX: ffffffffb31e677b >> [ 60.223274] RDX: dffffc0000000000 RSI: ffff888212c10600 RDI: ffff8881b08a9488 >> [ 60.230501] RBP: ffff8881b08a9030 R08: 0000000000000020 R09: ffff888212c10600 >> [ 60.237710] R10: ffffed10425820df R11: 0000000000000001 R12: 0000000000000000 >> [ 60.244939] R13: ffff888212c10600 R14: 0000000000000008 R15: 0000000000000000 >> [ 60.252155] FS: 00007f08ac2b2f00(0000) GS:ffff8887cbe00000(0000) knlGS:0000000000000000 >> [ 60.260333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> [ 60.266150] CR2: 000055831c899be8 CR3: 000000015372e006 CR4: 00000000001706f0 >> [ 60.273369] Call Trace: >> [ 60.275845] drm_gem_map_dma_buf+0xb4/0x120 >> [ 60.280089] dma_buf_map_attachment+0x15d/0x1e0 >> [ 60.284688] drm_gem_prime_import_dev.part.0+0x5d/0x140 >> [ 60.289984] drm_gem_prime_fd_to_handle+0x213/0x280 >> [ 60.294931] ? drm_prime_destroy_file_private+0x30/0x30 >> [ 60.300224] drm_ioctl_kernel+0x131/0x180 >> [ 60.304291] ? drm_setversion+0x230/0x230 >> [ 60.308366] ? drm_prime_destroy_file_private+0x30/0x30 >> [ 60.313659] drm_ioctl+0x2f1/0x500 >> [ 60.317118] ? drm_version+0x150/0x150 >> [ 60.320903] ? lock_downgrade+0xa0/0xa0 >> [ 60.324806] ? do_vfs_ioctl+0x5f4/0x680 >> [ 60.328694] ? __fget_files+0x13e/0x210 >> [ 60.332591] ? ioctl_fiemap.isra.0+0x1a0/0x1a0 >> [ 60.337102] ? __fget_files+0x15d/0x210 >> [ 60.340990] __x64_sys_ioctl+0xb9/0xf0 >> [ 60.344795] do_syscall_64+0x33/0x80 >> [ 60.348427] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > I'm working on a USB display driver and got the same splat (although > from i915_gem_map_dma_buf) and silenced it using: > > ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); > > But I understand now that this is not the solution. > > Since the USB host controller can do DMA, would the following be an > acceptable solution? I'm not an expert for the USB stuff. But from a 10 mile high view I would say that this is the right approach, yes. Regards, Christian. > > static struct drm_gem_object *gud_gem_prime_import(struct drm_device > *drm, struct dma_buf *dma_buf) > { > struct usb_device *usb = gud_to_usb_device(to_gud_device(drm)); > > drm_dbg_prime(drm, "usb->bus->controller=%s\n", > dev_name(usb->bus->controller)); > > return drm_gem_prime_import_dev(drm, dma_buf, usb->bus->controller); > } > > static const struct drm_driver gud_drm_driver = { > .gem_prime_import = gud_gem_prime_import, > }; > > > Noralf. > >> [ 60.353542] RIP: 0033:0x7f08ac7b53cb >> [ 60.357168] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 ba 0c 00 f7 d8 64 89 01 48 >> [ 60.376108] RSP: 002b:00007ffeabc89fc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 >> [ 60.383758] RAX: ffffffffffffffda RBX: 00007ffeabc8a00c RCX: 00007f08ac7b53cb >> [ 60.390970] RDX: 00007ffeabc8a00c RSI: 00000000c00c642e RDI: 000000000000000d >> [ 60.398221] RBP: 00000000c00c642e R08: 000055831c691d00 R09: 000055831b2ec010 >> [ 60.405446] R10: 00007f08acf6ada0 R11: 0000000000000246 R12: 000055831c691d00 >> [ 60.412667] R13: 000000000000000d R14: 00000000007e9000 R15: 0000000000000000 >> [ 60.419903] irq event stamp: 672893 >> [ 60.423441] hardirqs last enabled at (672913): [<ffffffffb31b796d>] console_unlock+0x44d/0x500 >> [ 60.432230] hardirqs last disabled at (672922): [<ffffffffb31b7963>] console_unlock+0x443/0x500 >> [ 60.441021] softirqs last enabled at (672940): [<ffffffffb46003dd>] __do_softirq+0x3dd/0x554 >> [ 60.449634] softirqs last disabled at (672931): [<ffffffffb44010f2>] asm_call_irq_on_stack+0x12/0x20 >> [ 60.458871] ---[ end trace f2f88696eb17806c ]--- >> >> For the fix, we don't call dma_map_sgtable() for devices without the >> DMA mask. Drivers for such devices have to map the imported buffer into >> kernel address space and perfom the copy operation in software. >> >> Tested by joining/mirroring displays of udl and radeon un der Gnome/X11. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices") >> Cc: Christoph Hellwig <hch@lst.de> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Alan Stern <stern@rowland.harvard.edu> >> Cc: Johan Hovold <johan@kernel.org> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> >> Cc: "Ahmed S. Darwish" <a.darwish@linutronix.de> >> Cc: Mathias Nyman <mathias.nyman@linux.intel.com> >> Cc: Oliver Neukum <oneukum@suse.com> >> Cc: Felipe Balbi <balbi@kernel.org> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: <stable@vger.kernel.org> # v5.10+ >> --- >> drivers/gpu/drm/drm_prime.c | 27 ++++++++++++++++++--------- >> 1 file changed, 18 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >> index 2a54f86856af..d5a39fe76b78 100644 >> --- a/drivers/gpu/drm/drm_prime.c >> +++ b/drivers/gpu/drm/drm_prime.c >> @@ -603,10 +603,15 @@ EXPORT_SYMBOL(drm_gem_map_detach); >> * @attach: attachment whose scatterlist is to be returned >> * @dir: direction of DMA transfer >> * >> - * Calls &drm_gem_object_funcs.get_sg_table and then maps the scatterlist. This >> - * can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together >> + * Calls &drm_gem_object_funcs.get_sg_table and, if possible, maps the scatterlist. >> + * This can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together >> * with drm_gem_unmap_dma_buf(). >> * >> + * Devices on some peripheral busses, such as USB, cannot use DMA. In this case, >> + * pages in the scatterlist remain unmapped. Drivers for such devices should acquire >> + * a mapping with dma_buf_vmap() and implement copy operation via bus-specific >> + * interfaces. >> + * >> * Returns:sg_table containing the scatterlist to be returned; returns ERR_PTR >> * on error. May return -EINTR if it is interrupted by a signal. >> */ >> @@ -627,12 +632,14 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, >> if (IS_ERR(sgt)) >> return sgt; >> >> - ret = dma_map_sgtable(attach->dev, sgt, dir, >> - DMA_ATTR_SKIP_CPU_SYNC); >> - if (ret) { >> - sg_free_table(sgt); >> - kfree(sgt); >> - sgt = ERR_PTR(ret); >> + if (attach->dev->dma_mask) { >> + ret = dma_map_sgtable(attach->dev, sgt, dir, >> + DMA_ATTR_SKIP_CPU_SYNC); >> + if (ret) { >> + sg_free_table(sgt); >> + kfree(sgt); >> + sgt = ERR_PTR(ret); >> + } >> } >> >> return sgt; >> @@ -654,7 +661,9 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach, >> if (!sgt) >> return; >> >> - dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); >> + if (attach->dev->dma_mask) >> + dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); >> + >> sg_free_table(sgt); >> kfree(sgt); >> } >> -- >> 2.30.0 >>
Hi Am 19.02.21 um 16:53 schrieb Alan Stern: > On Fri, Feb 19, 2021 at 02:45:54PM +0100, Christian König wrote: >> Well as far as I can see this is a relative clear NAK. >> >> When a device can't do DMA and has no DMA mask then why it is requesting an >> sg-table in the first place? > > This may not be important for your discussion, but I'd like to give an > answer to the question -- at least, for the case of USB. > > A USB device cannot do DMA and has no DMA mask. Nevertheless, if you > want to send large amounts of bulk data to/from a USB device then using > an SG table is often a good way to do it. The reason is simple: All > communication with a USB device has to go through a USB host controller, > and many (though not all) host controllers _can_ do DMA and _do_ have a > DMA mask. It's semi-related. One idea we had for speeding up transfers is to use the USB controller's DMA functionality and allocate framebuffers accordingly. It needs a bit of work in the DRM memory-management code IIRC. And we do some internal modifications if the frambuffer's data, so direct forwarding from renderer to USB controller is not always possible. Best regards Thomas > > The USB mass-storage and uas drivers in particular make heavy use of > this mechanism. > > Alan Stern >
Hi Am 20.02.21 um 14:02 schrieb Noralf Trønnes: > > > Den 19.02.2021 14.40, skrev Thomas Zimmermann: >> Fixes a regression for udl and probably other USB-based drivers where >> joining and mirroring displays fails. >> >> Joining displays requires importing a dma_buf from another DRM driver. >> These devices don't support DMA and therefore have no DMA mask. Trying >> to map imported buffers from a DMA-able memory zone fails with an error. >> An example is shown below. >> >> [ 60.050199] ------------[ cut here ]------------ >> [ 60.055092] WARNING: CPU: 0 PID: 1403 at kernel/dma/mapping.c:190 dma_map_sg_attrs+0x8f/0xc0 >> [ 60.064331] Modules linked in: af_packet(E) rfkill(E) dmi_sysfs(E) intel_rapl_msr(E) intel_rapl_common(E) snd_hda_codec_realtek(E) snd_hda_codec_generic(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) x86_pkg_temp_thermal(E) intel_powerclam) >> [ 60.064801] wmi(E) video(E) btrfs(E) blake2b_generic(E) libcrc32c(E) crc32c_intel(E) xor(E) raid6_pq(E) sg(E) dm_multipath(E) dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) msr(E) efivarfs(E) >> [ 60.170778] CPU: 0 PID: 1403 Comm: Xorg.bin Tainted: G E 5.11.0-rc5-1-default+ #747 >> [ 60.179841] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS A24 10/24/2018 >> [ 60.187145] RIP: 0010:dma_map_sg_attrs+0x8f/0xc0 >> [ 60.191822] Code: 4d 85 ff 75 2b 49 89 d8 44 89 e1 44 89 f2 4c 89 ee 48 89 ef e8 62 30 00 00 85 c0 78 36 5b 5d 41 5c 41 5d 41 5e 41 5f c3 0f 0b <0f> 0b 31 c0 eb ed 49 8d 7f 50 e8 72 f5 2a 00 49 8b 47 50 49 89 d8 >> [ 60.210770] RSP: 0018:ffffc90001d6fc18 EFLAGS: 00010246 >> [ 60.216062] RAX: 0000000000000000 RBX: 0000000000000020 RCX: ffffffffb31e677b >> [ 60.223274] RDX: dffffc0000000000 RSI: ffff888212c10600 RDI: ffff8881b08a9488 >> [ 60.230501] RBP: ffff8881b08a9030 R08: 0000000000000020 R09: ffff888212c10600 >> [ 60.237710] R10: ffffed10425820df R11: 0000000000000001 R12: 0000000000000000 >> [ 60.244939] R13: ffff888212c10600 R14: 0000000000000008 R15: 0000000000000000 >> [ 60.252155] FS: 00007f08ac2b2f00(0000) GS:ffff8887cbe00000(0000) knlGS:0000000000000000 >> [ 60.260333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> [ 60.266150] CR2: 000055831c899be8 CR3: 000000015372e006 CR4: 00000000001706f0 >> [ 60.273369] Call Trace: >> [ 60.275845] drm_gem_map_dma_buf+0xb4/0x120 >> [ 60.280089] dma_buf_map_attachment+0x15d/0x1e0 >> [ 60.284688] drm_gem_prime_import_dev.part.0+0x5d/0x140 >> [ 60.289984] drm_gem_prime_fd_to_handle+0x213/0x280 >> [ 60.294931] ? drm_prime_destroy_file_private+0x30/0x30 >> [ 60.300224] drm_ioctl_kernel+0x131/0x180 >> [ 60.304291] ? drm_setversion+0x230/0x230 >> [ 60.308366] ? drm_prime_destroy_file_private+0x30/0x30 >> [ 60.313659] drm_ioctl+0x2f1/0x500 >> [ 60.317118] ? drm_version+0x150/0x150 >> [ 60.320903] ? lock_downgrade+0xa0/0xa0 >> [ 60.324806] ? do_vfs_ioctl+0x5f4/0x680 >> [ 60.328694] ? __fget_files+0x13e/0x210 >> [ 60.332591] ? ioctl_fiemap.isra.0+0x1a0/0x1a0 >> [ 60.337102] ? __fget_files+0x15d/0x210 >> [ 60.340990] __x64_sys_ioctl+0xb9/0xf0 >> [ 60.344795] do_syscall_64+0x33/0x80 >> [ 60.348427] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > I'm working on a USB display driver and got the same splat (although > from i915_gem_map_dma_buf) and silenced it using: > > ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); > > But I understand now that this is not the solution. > > Since the USB host controller can do DMA, would the following be an > acceptable solution? Not all controllers can do DMA AFAIK, so it's maybe not reliable. I'll send out another patch to address the issue later today. Best regards Thomas > > static struct drm_gem_object *gud_gem_prime_import(struct drm_device > *drm, struct dma_buf *dma_buf) > { > struct usb_device *usb = gud_to_usb_device(to_gud_device(drm)); > > drm_dbg_prime(drm, "usb->bus->controller=%s\n", > dev_name(usb->bus->controller)); > > return drm_gem_prime_import_dev(drm, dma_buf, usb->bus->controller); > } > > static const struct drm_driver gud_drm_driver = { > .gem_prime_import = gud_gem_prime_import, > }; > > > Noralf. > >> [ 60.353542] RIP: 0033:0x7f08ac7b53cb >> [ 60.357168] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 ba 0c 00 f7 d8 64 89 01 48 >> [ 60.376108] RSP: 002b:00007ffeabc89fc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 >> [ 60.383758] RAX: ffffffffffffffda RBX: 00007ffeabc8a00c RCX: 00007f08ac7b53cb >> [ 60.390970] RDX: 00007ffeabc8a00c RSI: 00000000c00c642e RDI: 000000000000000d >> [ 60.398221] RBP: 00000000c00c642e R08: 000055831c691d00 R09: 000055831b2ec010 >> [ 60.405446] R10: 00007f08acf6ada0 R11: 0000000000000246 R12: 000055831c691d00 >> [ 60.412667] R13: 000000000000000d R14: 00000000007e9000 R15: 0000000000000000 >> [ 60.419903] irq event stamp: 672893 >> [ 60.423441] hardirqs last enabled at (672913): [<ffffffffb31b796d>] console_unlock+0x44d/0x500 >> [ 60.432230] hardirqs last disabled at (672922): [<ffffffffb31b7963>] console_unlock+0x443/0x500 >> [ 60.441021] softirqs last enabled at (672940): [<ffffffffb46003dd>] __do_softirq+0x3dd/0x554 >> [ 60.449634] softirqs last disabled at (672931): [<ffffffffb44010f2>] asm_call_irq_on_stack+0x12/0x20 >> [ 60.458871] ---[ end trace f2f88696eb17806c ]--- >> >> For the fix, we don't call dma_map_sgtable() for devices without the >> DMA mask. Drivers for such devices have to map the imported buffer into >> kernel address space and perfom the copy operation in software. >> >> Tested by joining/mirroring displays of udl and radeon un der Gnome/X11. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices") >> Cc: Christoph Hellwig <hch@lst.de> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Alan Stern <stern@rowland.harvard.edu> >> Cc: Johan Hovold <johan@kernel.org> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> >> Cc: "Ahmed S. Darwish" <a.darwish@linutronix.de> >> Cc: Mathias Nyman <mathias.nyman@linux.intel.com> >> Cc: Oliver Neukum <oneukum@suse.com> >> Cc: Felipe Balbi <balbi@kernel.org> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: <stable@vger.kernel.org> # v5.10+ >> --- >> drivers/gpu/drm/drm_prime.c | 27 ++++++++++++++++++--------- >> 1 file changed, 18 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >> index 2a54f86856af..d5a39fe76b78 100644 >> --- a/drivers/gpu/drm/drm_prime.c >> +++ b/drivers/gpu/drm/drm_prime.c >> @@ -603,10 +603,15 @@ EXPORT_SYMBOL(drm_gem_map_detach); >> * @attach: attachment whose scatterlist is to be returned >> * @dir: direction of DMA transfer >> * >> - * Calls &drm_gem_object_funcs.get_sg_table and then maps the scatterlist. This >> - * can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together >> + * Calls &drm_gem_object_funcs.get_sg_table and, if possible, maps the scatterlist. >> + * This can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together >> * with drm_gem_unmap_dma_buf(). >> * >> + * Devices on some peripheral busses, such as USB, cannot use DMA. In this case, >> + * pages in the scatterlist remain unmapped. Drivers for such devices should acquire >> + * a mapping with dma_buf_vmap() and implement copy operation via bus-specific >> + * interfaces. >> + * >> * Returns:sg_table containing the scatterlist to be returned; returns ERR_PTR >> * on error. May return -EINTR if it is interrupted by a signal. >> */ >> @@ -627,12 +632,14 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, >> if (IS_ERR(sgt)) >> return sgt; >> >> - ret = dma_map_sgtable(attach->dev, sgt, dir, >> - DMA_ATTR_SKIP_CPU_SYNC); >> - if (ret) { >> - sg_free_table(sgt); >> - kfree(sgt); >> - sgt = ERR_PTR(ret); >> + if (attach->dev->dma_mask) { >> + ret = dma_map_sgtable(attach->dev, sgt, dir, >> + DMA_ATTR_SKIP_CPU_SYNC); >> + if (ret) { >> + sg_free_table(sgt); >> + kfree(sgt); >> + sgt = ERR_PTR(ret); >> + } >> } >> >> return sgt; >> @@ -654,7 +661,9 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach, >> if (!sgt) >> return; >> >> - dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); >> + if (attach->dev->dma_mask) >> + dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); >> + >> sg_free_table(sgt); >> kfree(sgt); >> } >> -- >> 2.30.0 >> > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
Den 22.02.2021 08.54, skrev Thomas Zimmermann: > Hi > > Am 20.02.21 um 14:02 schrieb Noralf Trønnes: >> >> >> Den 19.02.2021 14.40, skrev Thomas Zimmermann: >>> Fixes a regression for udl and probably other USB-based drivers where >>> joining and mirroring displays fails. >>> >>> Joining displays requires importing a dma_buf from another DRM driver. >>> These devices don't support DMA and therefore have no DMA mask. Trying >>> to map imported buffers from a DMA-able memory zone fails with an error. >>> An example is shown below. >>> >>> [ 60.050199] ------------[ cut here ]------------ >>> [ 60.055092] WARNING: CPU: 0 PID: 1403 at kernel/dma/mapping.c:190 >>> dma_map_sg_attrs+0x8f/0xc0 >>> [ 60.064331] Modules linked in: af_packet(E) rfkill(E) dmi_sysfs(E) >>> intel_rapl_msr(E) intel_rapl_common(E) snd_hda_codec_realtek(E) >>> snd_hda_codec_generic(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) >>> x86_pkg_temp_thermal(E) intel_powerclam) >>> [ 60.064801] wmi(E) video(E) btrfs(E) blake2b_generic(E) >>> libcrc32c(E) crc32c_intel(E) xor(E) raid6_pq(E) sg(E) dm_multipath(E) >>> dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) msr(E) >>> efivarfs(E) >>> [ 60.170778] CPU: 0 PID: 1403 Comm: Xorg.bin Tainted: G >>> E 5.11.0-rc5-1-default+ #747 >>> [ 60.179841] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS >>> A24 10/24/2018 >>> [ 60.187145] RIP: 0010:dma_map_sg_attrs+0x8f/0xc0 >>> [ 60.191822] Code: 4d 85 ff 75 2b 49 89 d8 44 89 e1 44 89 f2 4c 89 >>> ee 48 89 ef e8 62 30 00 00 85 c0 78 36 5b 5d 41 5c 41 5d 41 5e 41 5f >>> c3 0f 0b <0f> 0b 31 c0 eb ed 49 8d 7f 50 e8 72 f5 2a 00 49 8b 47 50 >>> 49 89 d8 >>> [ 60.210770] RSP: 0018:ffffc90001d6fc18 EFLAGS: 00010246 >>> [ 60.216062] RAX: 0000000000000000 RBX: 0000000000000020 RCX: >>> ffffffffb31e677b >>> [ 60.223274] RDX: dffffc0000000000 RSI: ffff888212c10600 RDI: >>> ffff8881b08a9488 >>> [ 60.230501] RBP: ffff8881b08a9030 R08: 0000000000000020 R09: >>> ffff888212c10600 >>> [ 60.237710] R10: ffffed10425820df R11: 0000000000000001 R12: >>> 0000000000000000 >>> [ 60.244939] R13: ffff888212c10600 R14: 0000000000000008 R15: >>> 0000000000000000 >>> [ 60.252155] FS: 00007f08ac2b2f00(0000) GS:ffff8887cbe00000(0000) >>> knlGS:0000000000000000 >>> [ 60.260333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> [ 60.266150] CR2: 000055831c899be8 CR3: 000000015372e006 CR4: >>> 00000000001706f0 >>> [ 60.273369] Call Trace: >>> [ 60.275845] drm_gem_map_dma_buf+0xb4/0x120 >>> [ 60.280089] dma_buf_map_attachment+0x15d/0x1e0 >>> [ 60.284688] drm_gem_prime_import_dev.part.0+0x5d/0x140 >>> [ 60.289984] drm_gem_prime_fd_to_handle+0x213/0x280 >>> [ 60.294931] ? drm_prime_destroy_file_private+0x30/0x30 >>> [ 60.300224] drm_ioctl_kernel+0x131/0x180 >>> [ 60.304291] ? drm_setversion+0x230/0x230 >>> [ 60.308366] ? drm_prime_destroy_file_private+0x30/0x30 >>> [ 60.313659] drm_ioctl+0x2f1/0x500 >>> [ 60.317118] ? drm_version+0x150/0x150 >>> [ 60.320903] ? lock_downgrade+0xa0/0xa0 >>> [ 60.324806] ? do_vfs_ioctl+0x5f4/0x680 >>> [ 60.328694] ? __fget_files+0x13e/0x210 >>> [ 60.332591] ? ioctl_fiemap.isra.0+0x1a0/0x1a0 >>> [ 60.337102] ? __fget_files+0x15d/0x210 >>> [ 60.340990] __x64_sys_ioctl+0xb9/0xf0 >>> [ 60.344795] do_syscall_64+0x33/0x80 >>> [ 60.348427] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> >> I'm working on a USB display driver and got the same splat (although >> from i915_gem_map_dma_buf) and silenced it using: >> >> ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); >> >> But I understand now that this is not the solution. >> >> Since the USB host controller can do DMA, would the following be an >> acceptable solution? > > Not all controllers can do DMA AFAIK, so it's maybe not reliable. I'll > send out another patch to address the issue later today. > I looked at your other attempt at fixing this and the discussion about how much work it is to fix this properly. For my use case I think I just won't support import on a USB controller without DMA: if (!usb->bus->controller->dma_mask) return -ENOSYS; A USB controller without DMA must be rare and I'll deal with that later should the need arise. Thanks for bringing this issue to my attention. Noralf. > Best regards > Thomas > >> >> static struct drm_gem_object *gud_gem_prime_import(struct drm_device >> *drm, struct dma_buf *dma_buf) >> { >> struct usb_device *usb = gud_to_usb_device(to_gud_device(drm)); >> >> drm_dbg_prime(drm, "usb->bus->controller=%s\n", >> dev_name(usb->bus->controller)); >> >> return drm_gem_prime_import_dev(drm, dma_buf, usb->bus->controller); >> } >> >> static const struct drm_driver gud_drm_driver = { >> .gem_prime_import = gud_gem_prime_import, >> }; >> >> >> Noralf. >> >>> [ 60.353542] RIP: 0033:0x7f08ac7b53cb >>> [ 60.357168] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c >>> ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 >>> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 ba 0c 00 f7 d8 64 >>> 89 01 48 >>> [ 60.376108] RSP: 002b:00007ffeabc89fc8 EFLAGS: 00000246 ORIG_RAX: >>> 0000000000000010 >>> [ 60.383758] RAX: ffffffffffffffda RBX: 00007ffeabc8a00c RCX: >>> 00007f08ac7b53cb >>> [ 60.390970] RDX: 00007ffeabc8a00c RSI: 00000000c00c642e RDI: >>> 000000000000000d >>> [ 60.398221] RBP: 00000000c00c642e R08: 000055831c691d00 R09: >>> 000055831b2ec010 >>> [ 60.405446] R10: 00007f08acf6ada0 R11: 0000000000000246 R12: >>> 000055831c691d00 >>> [ 60.412667] R13: 000000000000000d R14: 00000000007e9000 R15: >>> 0000000000000000 >>> [ 60.419903] irq event stamp: 672893 >>> [ 60.423441] hardirqs last enabled at (672913): >>> [<ffffffffb31b796d>] console_unlock+0x44d/0x500 >>> [ 60.432230] hardirqs last disabled at (672922): >>> [<ffffffffb31b7963>] console_unlock+0x443/0x500 >>> [ 60.441021] softirqs last enabled at (672940): >>> [<ffffffffb46003dd>] __do_softirq+0x3dd/0x554 >>> [ 60.449634] softirqs last disabled at (672931): >>> [<ffffffffb44010f2>] asm_call_irq_on_stack+0x12/0x20 >>> [ 60.458871] ---[ end trace f2f88696eb17806c ]--- >>> >>> For the fix, we don't call dma_map_sgtable() for devices without the >>> DMA mask. Drivers for such devices have to map the imported buffer into >>> kernel address space and perfom the copy operation in software. >>> >>> Tested by joining/mirroring displays of udl and radeon un der Gnome/X11. >>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB >>> devices") >>> Cc: Christoph Hellwig <hch@lst.de> >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> Cc: Alan Stern <stern@rowland.harvard.edu> >>> Cc: Johan Hovold <johan@kernel.org> >>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> >>> Cc: "Ahmed S. Darwish" <a.darwish@linutronix.de> >>> Cc: Mathias Nyman <mathias.nyman@linux.intel.com> >>> Cc: Oliver Neukum <oneukum@suse.com> >>> Cc: Felipe Balbi <balbi@kernel.org> >>> Cc: Thomas Gleixner <tglx@linutronix.de> >>> Cc: <stable@vger.kernel.org> # v5.10+ >>> --- >>> drivers/gpu/drm/drm_prime.c | 27 ++++++++++++++++++--------- >>> 1 file changed, 18 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >>> index 2a54f86856af..d5a39fe76b78 100644 >>> --- a/drivers/gpu/drm/drm_prime.c >>> +++ b/drivers/gpu/drm/drm_prime.c >>> @@ -603,10 +603,15 @@ EXPORT_SYMBOL(drm_gem_map_detach); >>> * @attach: attachment whose scatterlist is to be returned >>> * @dir: direction of DMA transfer >>> * >>> - * Calls &drm_gem_object_funcs.get_sg_table and then maps the >>> scatterlist. This >>> - * can be used as the &dma_buf_ops.map_dma_buf callback. Should be >>> used together >>> + * Calls &drm_gem_object_funcs.get_sg_table and, if possible, maps >>> the scatterlist. >>> + * This can be used as the &dma_buf_ops.map_dma_buf callback. Should >>> be used together >>> * with drm_gem_unmap_dma_buf(). >>> * >>> + * Devices on some peripheral busses, such as USB, cannot use DMA. >>> In this case, >>> + * pages in the scatterlist remain unmapped. Drivers for such >>> devices should acquire >>> + * a mapping with dma_buf_vmap() and implement copy operation via >>> bus-specific >>> + * interfaces. >>> + * >>> * Returns:sg_table containing the scatterlist to be returned; >>> returns ERR_PTR >>> * on error. May return -EINTR if it is interrupted by a signal. >>> */ >>> @@ -627,12 +632,14 @@ struct sg_table *drm_gem_map_dma_buf(struct >>> dma_buf_attachment *attach, >>> if (IS_ERR(sgt)) >>> return sgt; >>> >>> - ret = dma_map_sgtable(attach->dev, sgt, dir, >>> - DMA_ATTR_SKIP_CPU_SYNC); >>> - if (ret) { >>> - sg_free_table(sgt); >>> - kfree(sgt); >>> - sgt = ERR_PTR(ret); >>> + if (attach->dev->dma_mask) { >>> + ret = dma_map_sgtable(attach->dev, sgt, dir, >>> + DMA_ATTR_SKIP_CPU_SYNC); >>> + if (ret) { >>> + sg_free_table(sgt); >>> + kfree(sgt); >>> + sgt = ERR_PTR(ret); >>> + } >>> } >>> >>> return sgt; >>> @@ -654,7 +661,9 @@ void drm_gem_unmap_dma_buf(struct >>> dma_buf_attachment *attach, >>> if (!sgt) >>> return; >>> >>> - dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); >>> + if (attach->dev->dma_mask) >>> + dma_unmap_sgtable(attach->dev, sgt, dir, >>> DMA_ATTR_SKIP_CPU_SYNC); >>> + >>> sg_free_table(sgt); >>> kfree(sgt); >>> } >>> -- >>> 2.30.0 >>> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> >
On Mon, Feb 22, 2021 at 06:30:35PM +0100, Noralf Trønnes wrote: > > > Den 22.02.2021 08.54, skrev Thomas Zimmermann: > > Hi > > > > Am 20.02.21 um 14:02 schrieb Noralf Trønnes: > >> > >> > >> Den 19.02.2021 14.40, skrev Thomas Zimmermann: > >>> Fixes a regression for udl and probably other USB-based drivers where > >>> joining and mirroring displays fails. > >>> > >>> Joining displays requires importing a dma_buf from another DRM driver. > >>> These devices don't support DMA and therefore have no DMA mask. Trying > >>> to map imported buffers from a DMA-able memory zone fails with an error. > >>> An example is shown below. > >>> > >>> [ 60.050199] ------------[ cut here ]------------ > >>> [ 60.055092] WARNING: CPU: 0 PID: 1403 at kernel/dma/mapping.c:190 > >>> dma_map_sg_attrs+0x8f/0xc0 > >>> [ 60.064331] Modules linked in: af_packet(E) rfkill(E) dmi_sysfs(E) > >>> intel_rapl_msr(E) intel_rapl_common(E) snd_hda_codec_realtek(E) > >>> snd_hda_codec_generic(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) > >>> x86_pkg_temp_thermal(E) intel_powerclam) > >>> [ 60.064801] wmi(E) video(E) btrfs(E) blake2b_generic(E) > >>> libcrc32c(E) crc32c_intel(E) xor(E) raid6_pq(E) sg(E) dm_multipath(E) > >>> dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) msr(E) > >>> efivarfs(E) > >>> [ 60.170778] CPU: 0 PID: 1403 Comm: Xorg.bin Tainted: G > >>> E 5.11.0-rc5-1-default+ #747 > >>> [ 60.179841] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS > >>> A24 10/24/2018 > >>> [ 60.187145] RIP: 0010:dma_map_sg_attrs+0x8f/0xc0 > >>> [ 60.191822] Code: 4d 85 ff 75 2b 49 89 d8 44 89 e1 44 89 f2 4c 89 > >>> ee 48 89 ef e8 62 30 00 00 85 c0 78 36 5b 5d 41 5c 41 5d 41 5e 41 5f > >>> c3 0f 0b <0f> 0b 31 c0 eb ed 49 8d 7f 50 e8 72 f5 2a 00 49 8b 47 50 > >>> 49 89 d8 > >>> [ 60.210770] RSP: 0018:ffffc90001d6fc18 EFLAGS: 00010246 > >>> [ 60.216062] RAX: 0000000000000000 RBX: 0000000000000020 RCX: > >>> ffffffffb31e677b > >>> [ 60.223274] RDX: dffffc0000000000 RSI: ffff888212c10600 RDI: > >>> ffff8881b08a9488 > >>> [ 60.230501] RBP: ffff8881b08a9030 R08: 0000000000000020 R09: > >>> ffff888212c10600 > >>> [ 60.237710] R10: ffffed10425820df R11: 0000000000000001 R12: > >>> 0000000000000000 > >>> [ 60.244939] R13: ffff888212c10600 R14: 0000000000000008 R15: > >>> 0000000000000000 > >>> [ 60.252155] FS: 00007f08ac2b2f00(0000) GS:ffff8887cbe00000(0000) > >>> knlGS:0000000000000000 > >>> [ 60.260333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > >>> [ 60.266150] CR2: 000055831c899be8 CR3: 000000015372e006 CR4: > >>> 00000000001706f0 > >>> [ 60.273369] Call Trace: > >>> [ 60.275845] drm_gem_map_dma_buf+0xb4/0x120 > >>> [ 60.280089] dma_buf_map_attachment+0x15d/0x1e0 > >>> [ 60.284688] drm_gem_prime_import_dev.part.0+0x5d/0x140 > >>> [ 60.289984] drm_gem_prime_fd_to_handle+0x213/0x280 > >>> [ 60.294931] ? drm_prime_destroy_file_private+0x30/0x30 > >>> [ 60.300224] drm_ioctl_kernel+0x131/0x180 > >>> [ 60.304291] ? drm_setversion+0x230/0x230 > >>> [ 60.308366] ? drm_prime_destroy_file_private+0x30/0x30 > >>> [ 60.313659] drm_ioctl+0x2f1/0x500 > >>> [ 60.317118] ? drm_version+0x150/0x150 > >>> [ 60.320903] ? lock_downgrade+0xa0/0xa0 > >>> [ 60.324806] ? do_vfs_ioctl+0x5f4/0x680 > >>> [ 60.328694] ? __fget_files+0x13e/0x210 > >>> [ 60.332591] ? ioctl_fiemap.isra.0+0x1a0/0x1a0 > >>> [ 60.337102] ? __fget_files+0x15d/0x210 > >>> [ 60.340990] __x64_sys_ioctl+0xb9/0xf0 > >>> [ 60.344795] do_syscall_64+0x33/0x80 > >>> [ 60.348427] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > >> > >> I'm working on a USB display driver and got the same splat (although > >> from i915_gem_map_dma_buf) and silenced it using: > >> > >> ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); > >> > >> But I understand now that this is not the solution. > >> > >> Since the USB host controller can do DMA, would the following be an > >> acceptable solution? > > > > Not all controllers can do DMA AFAIK, so it's maybe not reliable. I'll > > send out another patch to address the issue later today. > > > > I looked at your other attempt at fixing this and the discussion about > how much work it is to fix this properly. > > For my use case I think I just won't support import on a USB controller > without DMA: > > if (!usb->bus->controller->dma_mask) > return -ENOSYS; > > A USB controller without DMA must be rare and I'll deal with that later > should the need arise. > > Thanks for bringing this issue to my attention. Yeah I think that's a reasonable short-term fix. Not perfect, but good enough for years most likely :-) -Daniel > > Noralf. > > > Best regards > > Thomas > > > >> > >> static struct drm_gem_object *gud_gem_prime_import(struct drm_device > >> *drm, struct dma_buf *dma_buf) > >> { > >> struct usb_device *usb = gud_to_usb_device(to_gud_device(drm)); > >> > >> drm_dbg_prime(drm, "usb->bus->controller=%s\n", > >> dev_name(usb->bus->controller)); > >> > >> return drm_gem_prime_import_dev(drm, dma_buf, usb->bus->controller); > >> } > >> > >> static const struct drm_driver gud_drm_driver = { > >> .gem_prime_import = gud_gem_prime_import, > >> }; > >> > >> > >> Noralf. > >> > >>> [ 60.353542] RIP: 0033:0x7f08ac7b53cb > >>> [ 60.357168] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c > >>> ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 > >>> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 ba 0c 00 f7 d8 64 > >>> 89 01 48 > >>> [ 60.376108] RSP: 002b:00007ffeabc89fc8 EFLAGS: 00000246 ORIG_RAX: > >>> 0000000000000010 > >>> [ 60.383758] RAX: ffffffffffffffda RBX: 00007ffeabc8a00c RCX: > >>> 00007f08ac7b53cb > >>> [ 60.390970] RDX: 00007ffeabc8a00c RSI: 00000000c00c642e RDI: > >>> 000000000000000d > >>> [ 60.398221] RBP: 00000000c00c642e R08: 000055831c691d00 R09: > >>> 000055831b2ec010 > >>> [ 60.405446] R10: 00007f08acf6ada0 R11: 0000000000000246 R12: > >>> 000055831c691d00 > >>> [ 60.412667] R13: 000000000000000d R14: 00000000007e9000 R15: > >>> 0000000000000000 > >>> [ 60.419903] irq event stamp: 672893 > >>> [ 60.423441] hardirqs last enabled at (672913): > >>> [<ffffffffb31b796d>] console_unlock+0x44d/0x500 > >>> [ 60.432230] hardirqs last disabled at (672922): > >>> [<ffffffffb31b7963>] console_unlock+0x443/0x500 > >>> [ 60.441021] softirqs last enabled at (672940): > >>> [<ffffffffb46003dd>] __do_softirq+0x3dd/0x554 > >>> [ 60.449634] softirqs last disabled at (672931): > >>> [<ffffffffb44010f2>] asm_call_irq_on_stack+0x12/0x20 > >>> [ 60.458871] ---[ end trace f2f88696eb17806c ]--- > >>> > >>> For the fix, we don't call dma_map_sgtable() for devices without the > >>> DMA mask. Drivers for such devices have to map the imported buffer into > >>> kernel address space and perfom the copy operation in software. > >>> > >>> Tested by joining/mirroring displays of udl and radeon un der Gnome/X11. > >>> > >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > >>> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB > >>> devices") > >>> Cc: Christoph Hellwig <hch@lst.de> > >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >>> Cc: Alan Stern <stern@rowland.harvard.edu> > >>> Cc: Johan Hovold <johan@kernel.org> > >>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > >>> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > >>> Cc: "Ahmed S. Darwish" <a.darwish@linutronix.de> > >>> Cc: Mathias Nyman <mathias.nyman@linux.intel.com> > >>> Cc: Oliver Neukum <oneukum@suse.com> > >>> Cc: Felipe Balbi <balbi@kernel.org> > >>> Cc: Thomas Gleixner <tglx@linutronix.de> > >>> Cc: <stable@vger.kernel.org> # v5.10+ > >>> --- > >>> drivers/gpu/drm/drm_prime.c | 27 ++++++++++++++++++--------- > >>> 1 file changed, 18 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > >>> index 2a54f86856af..d5a39fe76b78 100644 > >>> --- a/drivers/gpu/drm/drm_prime.c > >>> +++ b/drivers/gpu/drm/drm_prime.c > >>> @@ -603,10 +603,15 @@ EXPORT_SYMBOL(drm_gem_map_detach); > >>> * @attach: attachment whose scatterlist is to be returned > >>> * @dir: direction of DMA transfer > >>> * > >>> - * Calls &drm_gem_object_funcs.get_sg_table and then maps the > >>> scatterlist. This > >>> - * can be used as the &dma_buf_ops.map_dma_buf callback. Should be > >>> used together > >>> + * Calls &drm_gem_object_funcs.get_sg_table and, if possible, maps > >>> the scatterlist. > >>> + * This can be used as the &dma_buf_ops.map_dma_buf callback. Should > >>> be used together > >>> * with drm_gem_unmap_dma_buf(). > >>> * > >>> + * Devices on some peripheral busses, such as USB, cannot use DMA. > >>> In this case, > >>> + * pages in the scatterlist remain unmapped. Drivers for such > >>> devices should acquire > >>> + * a mapping with dma_buf_vmap() and implement copy operation via > >>> bus-specific > >>> + * interfaces. > >>> + * > >>> * Returns:sg_table containing the scatterlist to be returned; > >>> returns ERR_PTR > >>> * on error. May return -EINTR if it is interrupted by a signal. > >>> */ > >>> @@ -627,12 +632,14 @@ struct sg_table *drm_gem_map_dma_buf(struct > >>> dma_buf_attachment *attach, > >>> if (IS_ERR(sgt)) > >>> return sgt; > >>> > >>> - ret = dma_map_sgtable(attach->dev, sgt, dir, > >>> - DMA_ATTR_SKIP_CPU_SYNC); > >>> - if (ret) { > >>> - sg_free_table(sgt); > >>> - kfree(sgt); > >>> - sgt = ERR_PTR(ret); > >>> + if (attach->dev->dma_mask) { > >>> + ret = dma_map_sgtable(attach->dev, sgt, dir, > >>> + DMA_ATTR_SKIP_CPU_SYNC); > >>> + if (ret) { > >>> + sg_free_table(sgt); > >>> + kfree(sgt); > >>> + sgt = ERR_PTR(ret); > >>> + } > >>> } > >>> > >>> return sgt; > >>> @@ -654,7 +661,9 @@ void drm_gem_unmap_dma_buf(struct > >>> dma_buf_attachment *attach, > >>> if (!sgt) > >>> return; > >>> > >>> - dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); > >>> + if (attach->dev->dma_mask) > >>> + dma_unmap_sgtable(attach->dev, sgt, dir, > >>> DMA_ATTR_SKIP_CPU_SYNC); > >>> + > >>> sg_free_table(sgt); > >>> kfree(sgt); > >>> } > >>> -- > >>> 2.30.0 > >>> > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >> > >
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 2a54f86856af..d5a39fe76b78 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -603,10 +603,15 @@ EXPORT_SYMBOL(drm_gem_map_detach); * @attach: attachment whose scatterlist is to be returned * @dir: direction of DMA transfer * - * Calls &drm_gem_object_funcs.get_sg_table and then maps the scatterlist. This - * can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together + * Calls &drm_gem_object_funcs.get_sg_table and, if possible, maps the scatterlist. + * This can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together * with drm_gem_unmap_dma_buf(). * + * Devices on some peripheral busses, such as USB, cannot use DMA. In this case, + * pages in the scatterlist remain unmapped. Drivers for such devices should acquire + * a mapping with dma_buf_vmap() and implement copy operation via bus-specific + * interfaces. + * * Returns:sg_table containing the scatterlist to be returned; returns ERR_PTR * on error. May return -EINTR if it is interrupted by a signal. */ @@ -627,12 +632,14 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, if (IS_ERR(sgt)) return sgt; - ret = dma_map_sgtable(attach->dev, sgt, dir, - DMA_ATTR_SKIP_CPU_SYNC); - if (ret) { - sg_free_table(sgt); - kfree(sgt); - sgt = ERR_PTR(ret); + if (attach->dev->dma_mask) { + ret = dma_map_sgtable(attach->dev, sgt, dir, + DMA_ATTR_SKIP_CPU_SYNC); + if (ret) { + sg_free_table(sgt); + kfree(sgt); + sgt = ERR_PTR(ret); + } } return sgt; @@ -654,7 +661,9 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach, if (!sgt) return; - dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); + if (attach->dev->dma_mask) + dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); + sg_free_table(sgt); kfree(sgt); }
Fixes a regression for udl and probably other USB-based drivers where joining and mirroring displays fails. Joining displays requires importing a dma_buf from another DRM driver. These devices don't support DMA and therefore have no DMA mask. Trying to map imported buffers from a DMA-able memory zone fails with an error. An example is shown below. [ 60.050199] ------------[ cut here ]------------ [ 60.055092] WARNING: CPU: 0 PID: 1403 at kernel/dma/mapping.c:190 dma_map_sg_attrs+0x8f/0xc0 [ 60.064331] Modules linked in: af_packet(E) rfkill(E) dmi_sysfs(E) intel_rapl_msr(E) intel_rapl_common(E) snd_hda_codec_realtek(E) snd_hda_codec_generic(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) x86_pkg_temp_thermal(E) intel_powerclam) [ 60.064801] wmi(E) video(E) btrfs(E) blake2b_generic(E) libcrc32c(E) crc32c_intel(E) xor(E) raid6_pq(E) sg(E) dm_multipath(E) dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) msr(E) efivarfs(E) [ 60.170778] CPU: 0 PID: 1403 Comm: Xorg.bin Tainted: G E 5.11.0-rc5-1-default+ #747 [ 60.179841] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS A24 10/24/2018 [ 60.187145] RIP: 0010:dma_map_sg_attrs+0x8f/0xc0 [ 60.191822] Code: 4d 85 ff 75 2b 49 89 d8 44 89 e1 44 89 f2 4c 89 ee 48 89 ef e8 62 30 00 00 85 c0 78 36 5b 5d 41 5c 41 5d 41 5e 41 5f c3 0f 0b <0f> 0b 31 c0 eb ed 49 8d 7f 50 e8 72 f5 2a 00 49 8b 47 50 49 89 d8 [ 60.210770] RSP: 0018:ffffc90001d6fc18 EFLAGS: 00010246 [ 60.216062] RAX: 0000000000000000 RBX: 0000000000000020 RCX: ffffffffb31e677b [ 60.223274] RDX: dffffc0000000000 RSI: ffff888212c10600 RDI: ffff8881b08a9488 [ 60.230501] RBP: ffff8881b08a9030 R08: 0000000000000020 R09: ffff888212c10600 [ 60.237710] R10: ffffed10425820df R11: 0000000000000001 R12: 0000000000000000 [ 60.244939] R13: ffff888212c10600 R14: 0000000000000008 R15: 0000000000000000 [ 60.252155] FS: 00007f08ac2b2f00(0000) GS:ffff8887cbe00000(0000) knlGS:0000000000000000 [ 60.260333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 60.266150] CR2: 000055831c899be8 CR3: 000000015372e006 CR4: 00000000001706f0 [ 60.273369] Call Trace: [ 60.275845] drm_gem_map_dma_buf+0xb4/0x120 [ 60.280089] dma_buf_map_attachment+0x15d/0x1e0 [ 60.284688] drm_gem_prime_import_dev.part.0+0x5d/0x140 [ 60.289984] drm_gem_prime_fd_to_handle+0x213/0x280 [ 60.294931] ? drm_prime_destroy_file_private+0x30/0x30 [ 60.300224] drm_ioctl_kernel+0x131/0x180 [ 60.304291] ? drm_setversion+0x230/0x230 [ 60.308366] ? drm_prime_destroy_file_private+0x30/0x30 [ 60.313659] drm_ioctl+0x2f1/0x500 [ 60.317118] ? drm_version+0x150/0x150 [ 60.320903] ? lock_downgrade+0xa0/0xa0 [ 60.324806] ? do_vfs_ioctl+0x5f4/0x680 [ 60.328694] ? __fget_files+0x13e/0x210 [ 60.332591] ? ioctl_fiemap.isra.0+0x1a0/0x1a0 [ 60.337102] ? __fget_files+0x15d/0x210 [ 60.340990] __x64_sys_ioctl+0xb9/0xf0 [ 60.344795] do_syscall_64+0x33/0x80 [ 60.348427] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 60.353542] RIP: 0033:0x7f08ac7b53cb [ 60.357168] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 ba 0c 00 f7 d8 64 89 01 48 [ 60.376108] RSP: 002b:00007ffeabc89fc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 60.383758] RAX: ffffffffffffffda RBX: 00007ffeabc8a00c RCX: 00007f08ac7b53cb [ 60.390970] RDX: 00007ffeabc8a00c RSI: 00000000c00c642e RDI: 000000000000000d [ 60.398221] RBP: 00000000c00c642e R08: 000055831c691d00 R09: 000055831b2ec010 [ 60.405446] R10: 00007f08acf6ada0 R11: 0000000000000246 R12: 000055831c691d00 [ 60.412667] R13: 000000000000000d R14: 00000000007e9000 R15: 0000000000000000 [ 60.419903] irq event stamp: 672893 [ 60.423441] hardirqs last enabled at (672913): [<ffffffffb31b796d>] console_unlock+0x44d/0x500 [ 60.432230] hardirqs last disabled at (672922): [<ffffffffb31b7963>] console_unlock+0x443/0x500 [ 60.441021] softirqs last enabled at (672940): [<ffffffffb46003dd>] __do_softirq+0x3dd/0x554 [ 60.449634] softirqs last disabled at (672931): [<ffffffffb44010f2>] asm_call_irq_on_stack+0x12/0x20 [ 60.458871] ---[ end trace f2f88696eb17806c ]--- For the fix, we don't call dma_map_sgtable() for devices without the DMA mask. Drivers for such devices have to map the imported buffer into kernel address space and perfom the copy operation in software. Tested by joining/mirroring displays of udl and radeon un der Gnome/X11. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices") Cc: Christoph Hellwig <hch@lst.de> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Alan Stern <stern@rowland.harvard.edu> Cc: Johan Hovold <johan@kernel.org> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Cc: "Ahmed S. Darwish" <a.darwish@linutronix.de> Cc: Mathias Nyman <mathias.nyman@linux.intel.com> Cc: Oliver Neukum <oneukum@suse.com> Cc: Felipe Balbi <balbi@kernel.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: <stable@vger.kernel.org> # v5.10+ --- drivers/gpu/drm/drm_prime.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) -- 2.30.0