| Message ID | 20191118103536.17675-11-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Retire dma_buf_k(un)map | expand |
On Mon, 2019-11-18 at 11:35 +0100, Daniel Vetter wrote: > No need for stubs, dma-buf.c takes care of that. > > Aside, not having a ->release callback smelled like refcounting leak > somewhere. It will also score you a WARN_ON backtrace in dma-buf.c on > every export. But then I found that ttm_device_object_init overwrites > it. Overwriting const memory is not going to go down well in recent > kernels. It's actually taking a non-const copy and updating it. Not that that's much better, but at least it won't crash due to writing to wp memory. I'll add a backlog item to revisit this. > > One more aside: The (un)map_dma_buf can't ever be called because > ->attach rejects everything. Might want to drop a BUG_ON(1) in there. > Same for ->detach. And this. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> > Cc: Thomas Hellstrom <thellstrom@vmware.com> > --- > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com> Will you be taking this through drm-misc? Thanks, Thomas
On Mon, Nov 18, 2019 at 6:25 PM Thomas Hellstrom <thellstrom@vmware.com> wrote: > > On Mon, 2019-11-18 at 11:35 +0100, Daniel Vetter wrote: > > No need for stubs, dma-buf.c takes care of that. > > > > Aside, not having a ->release callback smelled like refcounting leak > > somewhere. It will also score you a WARN_ON backtrace in dma-buf.c on > > every export. But then I found that ttm_device_object_init overwrites > > it. Overwriting const memory is not going to go down well in recent > > kernels. > > It's actually taking a non-const copy and updating it. Not that that's > much better, but at least it won't crash due to writing to wp memory. > I'll add a backlog item to revisit this. Hm I was looking for that, but didn't spot where it does that. I'll update the commit message when merging. > > One more aside: The (un)map_dma_buf can't ever be called because > > ->attach rejects everything. Might want to drop a BUG_ON(1) in there. > > Same for ->detach. > > And this. > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> > > Cc: Thomas Hellstrom <thellstrom@vmware.com> > > --- > > > > > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com> Thanks for your review. > Will you be taking this through drm-misc? Yup, that's the plan. -Daniel
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c index e420675e8db3..d9552a1efd13 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c @@ -62,45 +62,12 @@ static void vmw_prime_unmap_dma_buf(struct dma_buf_attachment *attach, { } -static void *vmw_prime_dmabuf_vmap(struct dma_buf *dma_buf) -{ - return NULL; -} - -static void vmw_prime_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr) -{ -} - -static void *vmw_prime_dmabuf_kmap(struct dma_buf *dma_buf, - unsigned long page_num) -{ - return NULL; -} - -static void vmw_prime_dmabuf_kunmap(struct dma_buf *dma_buf, - unsigned long page_num, void *addr) -{ - -} - -static int vmw_prime_dmabuf_mmap(struct dma_buf *dma_buf, - struct vm_area_struct *vma) -{ - WARN_ONCE(true, "Attempted use of dmabuf mmap. Bad.\n"); - return -ENOSYS; -} - const struct dma_buf_ops vmw_prime_dmabuf_ops = { .attach = vmw_prime_map_attach, .detach = vmw_prime_map_detach, .map_dma_buf = vmw_prime_map_dma_buf, .unmap_dma_buf = vmw_prime_unmap_dma_buf, .release = NULL, - .map = vmw_prime_dmabuf_kmap, - .unmap = vmw_prime_dmabuf_kunmap, - .mmap = vmw_prime_dmabuf_mmap, - .vmap = vmw_prime_dmabuf_vmap, - .vunmap = vmw_prime_dmabuf_vunmap, }; int vmw_prime_fd_to_handle(struct drm_device *dev,
No need for stubs, dma-buf.c takes care of that. Aside, not having a ->release callback smelled like refcounting leak somewhere. It will also score you a WARN_ON backtrace in dma-buf.c on every export. But then I found that ttm_device_object_init overwrites it. Overwriting const memory is not going to go down well in recent kernels. One more aside: The (un)map_dma_buf can't ever be called because ->attach rejects everything. Might want to drop a BUG_ON(1) in there. Same for ->detach. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> Cc: Thomas Hellstrom <thellstrom@vmware.com> --- drivers/gpu/drm/vmwgfx/vmwgfx_prime.c | 33 --------------------------- 1 file changed, 33 deletions(-)