On 13/05/2019 15:47, Chris Wilson wrote: > Quoting Daniel Vetter (2019-05-13 15:39:21) >> On Mon, May 13, 2019 at 03:32:44PM +0100, Steven Price wrote: >>> panfrost_ioctl_mmap_bo() contains a reimplementation of >>> drm_gem_dump_map_offset() but with a bug - it allows mapping imported >>> objects (without going through the exporter). Fix this by switching to >>> use the generic drm_gem_dump_map_offset() function instead which has the >>> bonus of simplifying the code. >> >> gem_dumb stuff is for kms drivers, panfrost is a render driver. We're >> generally trying to separate these two worlds somewhat cleanly. >> >> I think it'd be good to have a non-dumb version of this in the core, and >> use that. Or upgrade the dumb version to be that helper for everyone (and >> drop the _dumb). I'm slightly confused by what you think is the best course of action here. I can create a patch dropping the '_dumb' from drm_gem_dump_map_offset() if that's the objection. Or do you want a helper function with two callers (the 'dump' and the 'shmem' versions)? > More specifically, since panfrost is using the drm_gem_shmem helper and > vm_ops, it too can provide the wrapper as it is the drm_gem_shmem layer > that implies that trying to mmap an imported object is an issue as that > is not a universal problem. mmap'ing an imported object isn't a problem as such. But rather than going behind the exporter's back we would need to call dma_buf_mmap() to ensure that the exporter can do whatever is necessary. I could add a wrapper in drm_gem_shmem_helper, but I'm not sure what the benefit of a wrapper here is? Steve _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
