Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

15 participants
3025 discussions

[PATCH] dma-buf/dma-resv: check if the new fence is really later

by Christian König

Previously when we added a fence to a dma_resv object we always assumed the the newer than all the existing fences. With Jason's work to add an UAPI to explicit export/import that's not necessary the case any more. So without this check we would allow userspace to force the kernel into an use after free error. Since the change is very small and defensive it's probably a good idea to backport this to stable kernels as well just in case others are using the dma_resv object in the same way. Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/dma-buf/dma-resv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 205acb2c744d..e3885c90a3ac 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -295,7 +295,8 @@ void dma_resv_add_fence(struct dma_resv *obj, struct dma_fence *fence, enum dma_resv_usage old_usage; dma_resv_list_entry(fobj, i, obj, &old, &old_usage); - if ((old->context == fence->context && old_usage >= usage) || + if ((old->context == fence->context && old_usage >= usage && + dma_fence_is_later(fence, old)) || dma_fence_is_signaled(old)) { dma_resv_list_set(fobj, i, fence, usage); dma_fence_put(old); -- 2.25.1

3 years

Re: [EXT] Re: [PATCH 1/3] dma-buf: heaps: add Linaro secure dmabuf heap support

by cyrille.fleury＠nxp.com

The content of this message was lost. It was probably cross-posted to multiple lists and previously handled on another list.

3 years

[PATCH v2] dma-buf: cma_heap: Check for device max segment size when attaching

by Andrew Davis

Although there is usually not such a limitation (and when there is it is often only because the driver forgot to change the super small default), it is still correct here to break scatterlist element into chunks of dma_max_mapping_size(). This might cause some issues for users with misbehaving drivers. If bisecting has landed you on this commit, make sure your drivers both set dma_set_max_seg_size() and are checking for contiguousness correctly. Signed-off-by: Andrew Davis <afd(a)ti.com> --- Changes from v1: - Fixed mixed declarations and code warning drivers/dma-buf/heaps/cma_heap.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 28fb04eccdd0..a927b248c045 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -52,16 +52,18 @@ static int cma_heap_attach(struct dma_buf *dmabuf, { struct cma_heap_buffer *buffer = dmabuf->priv; struct dma_heap_attachment *a; + size_t max_segment; int ret; a = kzalloc(sizeof(*a), GFP_KERNEL); if (!a) return -ENOMEM; - ret = sg_alloc_table_from_pages(&a->table, buffer->pages, - buffer->pagecount, 0, - buffer->pagecount << PAGE_SHIFT, - GFP_KERNEL); + max_segment = dma_get_max_seg_size(attachment->dev); + ret = sg_alloc_table_from_pages_segment(&a->table, buffer->pages, + buffer->pagecount, 0, + buffer->pagecount << PAGE_SHIFT, + max_segment, GFP_KERNEL); if (ret) { kfree(a); return ret; -- 2.36.1

3 years

[PATCH] dma-buf: cma_heap: Check for device max segment size when attaching

by Andrew Davis

Although there is usually not such a limitation (and when there is it is often only because the driver forgot to change the super small default), it is still correct here to break scatterlist element into chunks of dma_max_mapping_size(). This might cause some issues for users with misbehaving drivers. If bisecting has landed you on this commit, make sure your drivers both set dma_set_max_seg_size() and are checking for contiguousness correctly. Signed-off-by: Andrew Davis <afd(a)ti.com> --- drivers/dma-buf/heaps/cma_heap.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 28fb04eccdd0..cacc84cb5ece 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -58,10 +58,11 @@ static int cma_heap_attach(struct dma_buf *dmabuf, if (!a) return -ENOMEM; - ret = sg_alloc_table_from_pages(&a->table, buffer->pages, - buffer->pagecount, 0, - buffer->pagecount << PAGE_SHIFT, - GFP_KERNEL); + size_t max_segment = dma_get_max_seg_size(attachment->dev); + ret = sg_alloc_table_from_pages_segment(&a->table, buffer->pages, + buffer->pagecount, 0, + buffer->pagecount << PAGE_SHIFT, + max_segment, GFP_KERNEL); if (ret) { kfree(a); return ret; -- 2.36.1

3 years

[PATCH 0/3] Add dma-buf secure-heap

by Olivier Masse

Purpose of these patches is to add a new dma-buf heap: linaro,secure-heap Linaro OPTEE OS Secure Data Path feature is relying on a reserved memory defined at Linux Kernel level and OPTEE OS level. From Linux Kernel side, heap management is using dma-buf heaps interface. Olivier Masse (3): dma-buf: heaps: add Linaro secure dmabuf heap support dt-bindings: reserved-memory: add linaro,secure-heap plat-hikey: Add linaro,secure-heap compatible .../reserved-memory/linaro,secure-heap.yaml | 56 +++ .../arm64/boot/dts/hisilicon/hi6220-hikey.dts | 11 + arch/arm64/configs/defconfig | 2 + drivers/dma-buf/heaps/Kconfig | 9 + drivers/dma-buf/heaps/Makefile | 1 + drivers/dma-buf/heaps/secure_heap.c | 357 ++++++++++++++++++ 6 files changed, 436 insertions(+) create mode 100644 Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml create mode 100644 drivers/dma-buf/heaps/secure_heap.c -- 2.25.0

3 years

[PATCH v3] drm/gem: Fix GEM handle release errors

by Jeffy Chen

Currently we are assuming a one to one mapping between dmabuf and GEM handle when releasing GEM handles. But that is not always true, since we would create extra handles for the GEM obj in cases like gem_open() and getfb{,2}(). A similar issue was reported at: https://lore.kernel.org/all/20211105083308.392156-1-jay.xu@rock-chips.com/ Another problem is that the imported dmabuf might not always have gem_obj->dma_buf set, which would cause leaks in drm_gem_remove_prime_handles(). Let's fix these for now by using handle to find the exact map to remove. Signed-off-by: Jeffy Chen <jeffy.chen(a)rock-chips.com> Reviewed-by: Christian König <christian.koenig(a)amd.com> --- Changes in v3: Rewrite commit message a bit. Changes in v2: Fix a typo of rbtree. drivers/gpu/drm/drm_gem.c | 17 +---------------- drivers/gpu/drm/drm_internal.h | 4 ++-- drivers/gpu/drm/drm_prime.c | 20 ++++++++++++-------- 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index eb0c2d041f13..ed39da383570 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -168,21 +168,6 @@ void drm_gem_private_object_init(struct drm_device *dev, } EXPORT_SYMBOL(drm_gem_private_object_init); -static void -drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) -{ - /* - * Note: obj->dma_buf can't disappear as long as we still hold a - * handle reference in obj->handle_count. - */ - mutex_lock(&filp->prime.lock); - if (obj->dma_buf) { - drm_prime_remove_buf_handle_locked(&filp->prime, - obj->dma_buf); - } - mutex_unlock(&filp->prime.lock); -} - /** * drm_gem_object_handle_free - release resources bound to userspace handles * @obj: GEM object to clean up. @@ -253,7 +238,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) if (obj->funcs->close) obj->funcs->close(obj, file_priv); - drm_gem_remove_prime_handles(obj, file_priv); + drm_prime_remove_buf_handle(&file_priv->prime, id); drm_vma_node_revoke(&obj->vma_node, file_priv); drm_gem_object_handle_put_unlocked(obj); diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 1fbbc19f1ac0..7bb98e6a446d 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -74,8 +74,8 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv); void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv); -void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf); +void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, + uint32_t handle); /* drm_drv.c */ struct drm_minor *drm_minor_acquire(unsigned int minor_id); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index e3f09f18110c..bd5366b16381 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -190,29 +190,33 @@ static int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpri return -ENOENT; } -void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf) +void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, + uint32_t handle) { struct rb_node *rb; - rb = prime_fpriv->dmabufs.rb_node; + mutex_lock(&prime_fpriv->lock); + + rb = prime_fpriv->handles.rb_node; while (rb) { struct drm_prime_member *member; - member = rb_entry(rb, struct drm_prime_member, dmabuf_rb); - if (member->dma_buf == dma_buf) { + member = rb_entry(rb, struct drm_prime_member, handle_rb); + if (member->handle == handle) { rb_erase(&member->handle_rb, &prime_fpriv->handles); rb_erase(&member->dmabuf_rb, &prime_fpriv->dmabufs); - dma_buf_put(dma_buf); + dma_buf_put(member->dma_buf); kfree(member); - return; - } else if (member->dma_buf < dma_buf) { + break; + } else if (member->handle < handle) { rb = rb->rb_right; } else { rb = rb->rb_left; } } + + mutex_unlock(&prime_fpriv->lock); } void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv) -- 2.20.1

3 years

[PATCH] dma-buf: move from strlcpy with unused retval to strscpy

by Wolfram Sang

Follow the advice of the below link and prefer 'strscpy' in this subsystem. Conversion is 1:1 because the return value is not used. Generated by a coccinelle script. Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmk… Signed-off-by: Wolfram Sang <wsa+renesas(a)sang-engineering.com> --- drivers/dma-buf/sw_sync.c | 2 +- drivers/dma-buf/sync_file.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 348b3a9170fa..63f0aeb66db6 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -85,7 +85,7 @@ static struct sync_timeline *sync_timeline_create(const char *name) kref_init(&obj->kref); obj->context = dma_fence_context_alloc(1); - strlcpy(obj->name, name, sizeof(obj->name)); + strscpy(obj->name, name, sizeof(obj->name)); obj->pt_tree = RB_ROOT; INIT_LIST_HEAD(&obj->pt_list); diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 3ebec19a8e02..af57799c86ce 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -132,7 +132,7 @@ EXPORT_SYMBOL(sync_file_get_fence); char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len) { if (sync_file->user_name[0]) { - strlcpy(buf, sync_file->user_name, len); + strscpy(buf, sync_file->user_name, len); } else { struct dma_fence *fence = sync_file->fence; @@ -172,7 +172,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, return NULL; } sync_file->fence = fence; - strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name)); + strscpy(sync_file->user_name, name, sizeof(sync_file->user_name)); return sync_file; } @@ -262,9 +262,9 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, static int sync_fill_fence_info(struct dma_fence *fence, struct sync_fence_info *info) { - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), + strscpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); - strlcpy(info->driver_name, fence->ops->get_driver_name(fence), + strscpy(info->driver_name, fence->ops->get_driver_name(fence), sizeof(info->driver_name)); info->status = dma_fence_get_status(fence); -- 2.35.1

3 years

ACCOUNT WARNING: Undelivered mails linaro-mm-sig@lists.linaro.org Kindly rectify

by Noreply＠lists.linaro.org

3 years

[PATCH v2] drm/ast: add dmabuf/prime buffer sharing support

by oushixiong

This patch adds ast specific codes for DRM prime feature, this is to allow for offloading of rending in one direction and outputs in other. v1->v2: - Fix the comment. Signed-off-by: oushixiong <oushixiong(a)kylinos.cn> --- drivers/gpu/drm/ast/ast_drv.c | 22 ++++++ drivers/gpu/drm/ast/ast_mode.c | 125 ++++++++++++++++++++++++++++++++- 2 files changed, 146 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c index 7465c4f0156a..6c1f75174368 100644 --- a/drivers/gpu/drm/ast/ast_drv.c +++ b/drivers/gpu/drm/ast/ast_drv.c @@ -28,6 +28,7 @@ #include <linux/module.h> #include <linux/pci.h> +#include <linux/dma-buf.h> #include <drm/drm_aperture.h> #include <drm/drm_atomic_helper.h> @@ -50,6 +51,23 @@ module_param_named(modeset, ast_modeset, int, 0400); DEFINE_DRM_GEM_FOPS(ast_fops); +static struct drm_gem_object *ast_gem_prime_import_sg_table(struct drm_device *dev, + struct dma_buf_attachment *attach, + struct sg_table *sg) +{ + struct drm_gem_vram_object *gbo; + struct dma_resv *resv = attach->dmabuf->resv; + + ww_mutex_lock(&resv->lock, NULL); + gbo = drm_gem_vram_create(dev, attach->dmabuf->size, 0); + ww_mutex_unlock(&resv->lock); + + if (IS_ERR(gbo)) + return NULL; + + return &gbo->bo.base; +} + static const struct drm_driver ast_driver = { .driver_features = DRIVER_ATOMIC | DRIVER_GEM | @@ -63,6 +81,10 @@ static const struct drm_driver ast_driver = { .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .gem_prime_import_sg_table = ast_gem_prime_import_sg_table, + DRM_GEM_VRAM_DRIVER }; diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 45b56b39ad47..ebe732705e34 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -48,6 +48,8 @@ #include "ast_drv.h" #include "ast_tables.h" +MODULE_IMPORT_NS(DMA_BUF); + static inline void ast_load_palette_index(struct ast_private *ast, u8 index, u8 red, u8 green, u8 blue) @@ -1535,8 +1537,129 @@ static const struct drm_mode_config_helper_funcs ast_mode_config_helper_funcs = .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm, }; +static int ast_handle_damage(struct drm_framebuffer *fb, int x, int y, + int width, int height) +{ + struct drm_gem_vram_object *dst_bo = NULL; + void *dst = NULL; + int ret = 0, i; + unsigned long offset = 0; + bool unmap = false; + unsigned int bytesPerPixel; + struct iosys_map map; + struct iosys_map dmabuf_map; + + bytesPerPixel = fb->format->cpp[0]; + + if (!fb->obj[0]->import_attach) + return -EINVAL; + + if (!fb->obj[0]->import_attach->dmabuf->vmap_ptr.vaddr) { + ret = dma_buf_vmap(fb->obj[0]->import_attach->dmabuf, &dmabuf_map); + if (ret) + return 0; + } else + dmabuf_map.vaddr = fb->obj[0]->import_attach->dmabuf->vmap_ptr.vaddr; + + dst_bo = drm_gem_vram_of_gem(fb->obj[0]); + + ret = drm_gem_vram_pin(dst_bo, 0); + if (ret) { + DRM_ERROR("ast_bo_pin failed\n"); + goto error; + } + + if (!dst_bo->map.vaddr) { + ret = drm_gem_vram_vmap(dst_bo, &map); + if (ret) { + DRM_ERROR("failed to vmap fbcon\n"); + drm_gem_vram_unpin(dst_bo); + goto error; + } + unmap = true; + } + dst = dst_bo->map.vaddr; + + for (i = y; i < y + height; i++) { + offset = i * fb->pitches[0] + (x * bytesPerPixel); + memcpy_toio(dst + offset, dmabuf_map.vaddr + offset, + width * bytesPerPixel); + } + + if (unmap) + drm_gem_vram_vunmap(dst_bo, &map); + + drm_gem_vram_unpin(dst_bo); +error: + return 0; +} + + +static int ast_user_framebuffer_dirty(struct drm_framebuffer *fb, + struct drm_file *file, + unsigned int flags, + unsigned int color, + struct drm_clip_rect *clips, + unsigned int num_clips) +{ + int i, ret = 0; + + drm_modeset_lock_all(fb->dev); + if (fb->obj[0]->import_attach) { + ret = dma_buf_begin_cpu_access(fb->obj[0]->import_attach->dmabuf, + DMA_FROM_DEVICE); + if (ret) + goto unlock; + } + + for (i = 0; i < num_clips; i++) { + ret = ast_handle_damage(fb, clips[i].x1, clips[i].y1, + clips[i].x2 - clips[i].x1, clips[i].y2 - clips[i].y1); + if (ret) + break; + } + + if (fb->obj[0]->import_attach) { + dma_buf_end_cpu_access(fb->obj[0]->import_attach->dmabuf, + DMA_FROM_DEVICE); + } + +unlock: + drm_modeset_unlock_all(fb->dev); + + return ret; +} + +static void ast_user_framebuffer_destroy(struct drm_framebuffer *fb) +{ + struct iosys_map dmabuf_map; + + if (fb->obj[0]->import_attach) { + dmabuf_map.vaddr = fb->obj[0]->import_attach->dmabuf->vmap_ptr.vaddr; + if (dmabuf_map.vaddr) + dma_buf_vunmap(fb->obj[0]->import_attach->dmabuf, + &dmabuf_map); + } + + drm_gem_fb_destroy(fb); +} + +static const struct drm_framebuffer_funcs ast_gem_fb_funcs_dirtyfb = { + .destroy = ast_user_framebuffer_destroy, + .create_handle = drm_gem_fb_create_handle, + .dirty = ast_user_framebuffer_dirty, +}; + +static struct drm_framebuffer * +ast_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file, + const struct drm_mode_fb_cmd2 *mode_cmd) +{ + return drm_gem_fb_create_with_funcs(dev, file, mode_cmd, + &ast_gem_fb_funcs_dirtyfb); +} + static const struct drm_mode_config_funcs ast_mode_config_funcs = { - .fb_create = drm_gem_fb_create, + .fb_create = ast_gem_fb_create_with_dirty, .mode_valid = drm_vram_helper_mode_valid, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, -- 2.17.1

3 years

Re: [PATCH] [Draft]: media: videobuf2-dma-heap: add a vendor defined memory runtine

by ayaka

Sent from my iPad > On Aug 1, 2022, at 5:46 PM, Tomasz Figa <tfiga(a)chromium.org> wrote: > > CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe. > > >> On Mon, Aug 1, 2022 at 3:44 PM Hsia-Jun Li <Randy.Li(a)synaptics.com> wrote: >>> On 8/1/22 14:19, Tomasz Figa wrote: >> Hello Tomasz >>> ?Hi Randy, >>> On Mon, Aug 1, 2022 at 5:21 AM <ayaka(a)soulik.info> wrote: >>>> From: Randy Li <ayaka(a)soulik.info> >>>> This module is still at a early stage, I wrote this for showing what >>>> APIs we need here. >>>> Let me explain why we need such a module here. >>>> If you won't allocate buffers from a V4L2 M2M device, this module >>>> may not be very useful. I am sure the most of users won't know a >>>> device would require them allocate buffers from a DMA-Heap then >>>> import those buffers into a V4L2's queue. >>>> Then the question goes back to why DMA-Heap. From the Android's >>>> description, we know it is about the copyright's DRM. >>>> When we allocate a buffer in a DMA-Heap, it may register that buffer >>>> in the trusted execution environment so the firmware which is running >>>> or could only be acccesed from there could use that buffer later. >>>> The answer above leads to another thing which is not done in this >>>> version, the DMA mapping. Although in some platforms, a DMA-Heap >>>> responses a IOMMU device as well. For the genernal purpose, we would >>>> be better assuming the device mapping should be done for each device >>>> itself. The problem here we only know alloc_devs in those DMAbuf >>>> methods, which are DMA-heaps in my design, the device from the queue >>>> is not enough, a plane may requests another IOMMU device or table >>>> for mapping. >>>> Signed-off-by: Randy Li <ayaka(a)soulik.info> >>>> --- >>>> drivers/media/common/videobuf2/Kconfig | 6 + >>>> drivers/media/common/videobuf2/Makefile | 1 + >>>> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++ >>>> include/media/videobuf2-dma-heap.h | 30 ++ >>>> 4 files changed, 387 insertions(+) >>>> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c >>>> create mode 100644 include/media/videobuf2-dma-heap.h >>> First of all, thanks for the series. >>> Possibly a stupid question, but why not just allocate the DMA-bufs >>> directly from the DMA-buf heap device in the userspace and just import >>> the buffers to the V4L2 device using V4L2_MEMORY_DMABUF? >> Sometimes the allocation policy could be very complex, let's suppose a >> multiple planes pixel format enabling with frame buffer compression. >> Its luma, chroma data could be allocated from a pool which is delegated >> for large buffers while its metadata would come from a pool which many >> users could take some few slices from it(likes system pool). >> Then when we have a new users knowing nothing about this platform, if we >> just configure the alloc_devs in each queues well. The user won't need >> to know those complex rules. >> The real situation could be more complex, Samsung MFC's left and right >> banks could be regarded as two pools, many devices would benefit from >> this either from the allocation times or the security buffers policy. >> In our design, when we need to do some security decoding(DRM video), >> codecs2 would allocate buffers from the pool delegated for that. While >> the non-DRM video, users could not care about this. > > I'm a little bit surprised about this, because on Android all the > graphics buffers are allocated from the system IAllocator and imported > to the specific devices. In the non-tunnel mode, yes it is. While the tunnel mode is completely vendor defined. Neither HWC nor codec2 cares about where the buffers coming from, you could do what ever you want. Besides there are DRM video in GNU Linux platform, I heard the webkit has made huge effort here and Playready is one could work in non-Android Linux. > Would it make sense to instead extend the UAPI to expose enough > information about the allocation requirements to the userspace, so it > can allocate correctly? Yes, it could. But as I said it would need the users to do more works. > My reasoning here is that it's not a driver's decision to allocate > from a DMA-buf heap (and which one) or not. It's the userspace which > knows that, based on the specific use case that it wants to fulfill. Although I would like to let the users decide that, users just can’t do that which would violate the security rules in some platforms. For example, video codec and display device could only access a region of memory, any other device or trusted apps can’t access it. Users have to allocate the buffer from the pool the vendor decided. So why not we offer a quick way that users don’t need to try and error. > Also, FWIW, dma_heap_ioctl_allocate() is a static function not exposed > to other kernel modules: > https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_lin… I may forget to mention that you need two extra patches from Linaro that export those API(original version is actually out of time). Besides Android kernel did have the two kAPI I need here. Actually I need more APIs from DMA-heap to archive those things in TODO list. > By the way, the MFC left/right port requirement was gone long ago, it > was only one of the earliest Exynos SoCs which required that. Yes, MFCv5 or v6 right. I just want mention that the world has any possible, vendor always has its own reason. > Best regards, > Tomasz > >>> Best regards, >>> Tomasz >>>> diff --git a/drivers/media/common/videobuf2/Kconfig b/drivers/media/common/videobuf2/Kconfig >>>> index d2223a12c95f..02235077f07e 100644 >>>> --- a/drivers/media/common/videobuf2/Kconfig >>>> +++ b/drivers/media/common/videobuf2/Kconfig >>>> @@ -30,3 +30,9 @@ config VIDEOBUF2_DMA_SG >>>> config VIDEOBUF2_DVB >>>> tristate >>>> select VIDEOBUF2_CORE >>>> + >>>> +config VIDEOBUF2_DMA_HEAP >>>> + tristate >>>> + select VIDEOBUF2_CORE >>>> + select VIDEOBUF2_MEMOPS >>>> + select DMABUF_HEAPS >>>> diff --git a/drivers/media/common/videobuf2/Makefile b/drivers/media/common/videobuf2/Makefile >>>> index a6fe3f304685..7fe65f93117f 100644 >>>> --- a/drivers/media/common/videobuf2/Makefile >>>> +++ b/drivers/media/common/videobuf2/Makefile >>>> @@ -10,6 +10,7 @@ endif >>>> # (e. g. LC_ALL=C sort Makefile) >>>> obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-common.o >>>> obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o >>>> +obj-$(CONFIG_VIDEOBUF2_DMA_HEAP) += videobuf2-dma-heap.o >>>> obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o >>>> obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o >>>> obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o >>>> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-heap.c b/drivers/media/common/videobuf2/videobuf2-dma-heap.c >>>> new file mode 100644 >>>> index 000000000000..377b82ab8f5a >>>> --- /dev/null >>>> +++ b/drivers/media/common/videobuf2/videobuf2-dma-heap.c >>>> @@ -0,0 +1,350 @@ >>>> +/* >>>> + * Copyright (C) 2022 Randy Li <ayaka(a)soulik.info> >>>> + * >>>> + * This software is licensed under the terms of the GNU General Public >>>> + * License version 2, as published by the Free Software Foundation, and >>>> + * may be copied, distributed, and modified under those terms. >>>> + * >>>> + * This program is distributed in the hope that it will be useful, >>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>> + * GNU General Public License for more details. >>>> + * >>>> + */ >>>> + >>>> +#include <linux/dma-buf.h> >>>> +#include <linux/dma-heap.h> >>>> +#include <linux/refcount.h> >>>> +#include <linux/scatterlist.h> >>>> +#include <linux/sched.h> >>>> +#include <linux/slab.h> >>>> +#include <linux/dma-mapping.h> >>>> + >>>> +#include <media/videobuf2-v4l2.h> >>>> +#include <media/videobuf2-memops.h> >>>> +#include <media/videobuf2-dma-heap.h> >>>> + >>>> +struct vb2_dmaheap_buf { >>>> + struct device *dev; >>>> + void *vaddr; >>>> + unsigned long size; >>>> + struct dma_buf *dmabuf; >>>> + dma_addr_t dma_addr; >>>> + unsigned long attrs; >>>> + enum dma_data_direction dma_dir; >>>> + struct sg_table *dma_sgt; >>>> + >>>> + /* MMAP related */ >>>> + struct vb2_vmarea_handler handler; >>>> + refcount_t refcount; >>>> + >>>> + /* DMABUF related */ >>>> + struct dma_buf_attachment *db_attach; >>>> +}; >>>> + >>>> +/*********************************************/ >>>> +/* callbacks for all buffers */ >>>> +/*********************************************/ >>>> + >>>> +void *vb2_dmaheap_cookie(struct vb2_buffer *vb, void *buf_priv) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = buf_priv; >>>> + >>>> + return &buf->dma_addr; >>>> +} >>>> + >>>> +static void *vb2_dmaheap_vaddr(struct vb2_buffer *vb, void *buf_priv) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = buf_priv; >>>> + struct iosys_map map; >>>> + >>>> + if (buf->vaddr) >>>> + return buf->vaddr; >>>> + >>>> + if (buf->db_attach) { >>>> + if (!dma_buf_vmap(buf->db_attach->dmabuf, &map)) >>>> + buf->vaddr = map.vaddr; >>>> + } >>>> + >>>> + return buf->vaddr; >>>> +} >>>> + >>>> +static unsigned int vb2_dmaheap_num_users(void *buf_priv) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = buf_priv; >>>> + >>>> + return refcount_read(&buf->refcount); >>>> +} >>>> + >>>> +static void vb2_dmaheap_prepare(void *buf_priv) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = buf_priv; >>>> + >>>> + /* TODO: DMABUF exporter will flush the cache for us */ >>>> + if (buf->db_attach) >>>> + return; >>>> + >>>> + dma_buf_end_cpu_access(buf->dmabuf, buf->dma_dir); >>>> +} >>>> + >>>> +static void vb2_dmaheap_finish(void *buf_priv) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = buf_priv; >>>> + >>>> + /* TODO: DMABUF exporter will flush the cache for us */ >>>> + if (buf->db_attach) >>>> + return; >>>> + >>>> + dma_buf_begin_cpu_access(buf->dmabuf, buf->dma_dir); >>>> +} >>>> + >>>> +/*********************************************/ >>>> +/* callbacks for MMAP buffers */ >>>> +/*********************************************/ >>>> + >>>> +void vb2_dmaheap_put(void *buf_priv) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = buf_priv; >>>> + >>>> + if (!refcount_dec_and_test(&buf->refcount)) >>>> + return; >>>> + >>>> + dma_buf_put(buf->dmabuf); >>>> + >>>> + put_device(buf->dev); >>>> + kfree(buf); >>>> +} >>>> + >>>> +static void *vb2_dmaheap_alloc(struct vb2_buffer *vb, >>>> + struct device *dev, >>>> + unsigned long size) >>>> +{ >>>> + struct vb2_queue *q = vb->vb2_queue; >>>> + struct dma_heap *heap; >>>> + struct vb2_dmaheap_buf *buf; >>>> + const char *heap_name; >>>> + int ret; >>>> + >>>> + if (WARN_ON(!dev)) >>>> + return ERR_PTR(-EINVAL); >>>> + >>>> + heap_name = dev_name(dev); >>>> + if (!heap_name) >>>> + return ERR_PTR(-EINVAL); >>>> + >>>> + heap = dma_heap_find(heap_name); >>>> + if (!heap) { >>>> + dev_err(dev, "is not a DMA-heap device\n"); >>>> + return ERR_PTR(-EINVAL); >>>> + } >>>> + >>>> + buf = kzalloc(sizeof *buf, GFP_KERNEL); >>>> + if (!buf) >>>> + return ERR_PTR(-ENOMEM); >>>> + >>>> + /* Prevent the device from being released while the buffer is used */ >>>> + buf->dev = get_device(dev); >>>> + buf->attrs = vb->vb2_queue->dma_attrs; >>>> + buf->dma_dir = vb->vb2_queue->dma_dir; >>>> + >>>> + /* TODO: heap flags */ >>>> + ret = dma_heap_buffer_alloc(heap, size, 0, 0); >>>> + if (ret < 0) { >>>> + dev_err(dev, "is not a DMA-heap device\n"); >>>> + put_device(buf->dev); >>>> + kfree(buf); >>>> + return ERR_PTR(ret); >>>> + } >>>> + buf->dmabuf = dma_buf_get(ret); >>>> + >>>> + /* FIXME */ >>>> + buf->dma_addr = 0; >>>> + >>>> + if ((q->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0) >>>> + buf->vaddr = buf->dmabuf; >>>> + >>>> + buf->handler.refcount = &buf->refcount; >>>> + buf->handler.put = vb2_dmaheap_put; >>>> + buf->handler.arg = buf; >>>> + >>>> + refcount_set(&buf->refcount, 1); >>>> + >>>> + return buf; >>>> +} >>>> + >>>> +static int vb2_dmaheap_mmap(void *buf_priv, struct vm_area_struct *vma) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = buf_priv; >>>> + int ret; >>>> + >>>> + if (!buf) { >>>> + printk(KERN_ERR "No buffer to map\n"); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + vma->vm_flags &= ~VM_PFNMAP; >>>> + >>>> + ret = dma_buf_mmap(buf->dmabuf, vma, 0); >>>> + if (ret) { >>>> + pr_err("Remapping memory failed, error: %d\n", ret); >>>> + return ret; >>>> + } >>>> + vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP; >>>> + vma->vm_private_data = &buf->handler; >>>> + vma->vm_ops = &vb2_common_vm_ops; >>>> + >>>> + vma->vm_ops->open(vma); >>>> + >>>> + pr_debug("%s: mapped memid 0x%08lx at 0x%08lx, size %ld\n", >>>> + __func__, (unsigned long)buf->dma_addr, vma->vm_start, >>>> + buf->size); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +/*********************************************/ >>>> +/* DMABUF ops for exporters */ >>>> +/*********************************************/ >>>> + >>>> +static struct dma_buf *vb2_dmaheap_get_dmabuf(struct vb2_buffer *vb, >>>> + void *buf_priv, >>>> + unsigned long flags) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = buf_priv; >>>> + struct dma_buf *dbuf; >>>> + >>>> + dbuf = buf->dmabuf; >>>> + >>>> + return dbuf; >>>> +} >>>> + >>>> +/*********************************************/ >>>> +/* callbacks for DMABUF buffers */ >>>> +/*********************************************/ >>>> + >>>> +static int vb2_dmaheap_map_dmabuf(void *mem_priv) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = mem_priv; >>>> + struct sg_table *sgt; >>>> + >>>> + if (WARN_ON(!buf->db_attach)) { >>>> + pr_err("trying to pin a non attached buffer\n"); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + if (WARN_ON(buf->dma_sgt)) { >>>> + pr_err("dmabuf buffer is already pinned\n"); >>>> + return 0; >>>> + } >>>> + >>>> + /* get the associated scatterlist for this buffer */ >>>> + sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir); >>>> + if (IS_ERR(sgt)) { >>>> + pr_err("Error getting dmabuf scatterlist\n"); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + buf->dma_addr = sg_dma_address(sgt->sgl); >>>> + buf->dma_sgt = sgt; >>>> + buf->vaddr = NULL; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static void vb2_dmaheap_unmap_dmabuf(void *mem_priv) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = mem_priv; >>>> + struct sg_table *sgt = buf->dma_sgt; >>>> + struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr); >>>> + >>>> + if (WARN_ON(!buf->db_attach)) { >>>> + pr_err("trying to unpin a not attached buffer\n"); >>>> + return; >>>> + } >>>> + >>>> + if (WARN_ON(!sgt)) { >>>> + pr_err("dmabuf buffer is already unpinned\n"); >>>> + return; >>>> + } >>>> + >>>> + if (buf->vaddr) { >>>> + dma_buf_vunmap(buf->db_attach->dmabuf, &map); >>>> + buf->vaddr = NULL; >>>> + } >>>> + dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir); >>>> + >>>> + buf->dma_addr = 0; >>>> + buf->dma_sgt = NULL; >>>> +} >>>> + >>>> +static void vb2_dmaheap_detach_dmabuf(void *mem_priv) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf = mem_priv; >>>> + >>>> + /* if vb2 works correctly you should never detach mapped buffer */ >>>> + if (WARN_ON(buf->dma_addr)) >>>> + vb2_dmaheap_unmap_dmabuf(buf); >>>> + >>>> + /* detach this attachment */ >>>> + dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach); >>>> + kfree(buf); >>>> +} >>>> + >>>> +static void *vb2_dmaheap_attach_dmabuf(struct vb2_buffer *vb, struct device *dev, >>>> + struct dma_buf *dbuf, unsigned long size) >>>> +{ >>>> + struct vb2_dmaheap_buf *buf; >>>> + struct dma_buf_attachment *dba; >>>> + >>>> + if (dbuf->size < size) >>>> + return ERR_PTR(-EFAULT); >>>> + >>>> + if (WARN_ON(!dev)) >>>> + return ERR_PTR(-EINVAL); >>>> + /* >>>> + * TODO: A better way to check whether the buffer is coming >>>> + * from this heap or this heap could accept this buffer >>>> + */ >>>> + if (strcmp(dbuf->exp_name, dev_name(dev))) >>>> + return ERR_PTR(-EINVAL); >>>> + >>>> + buf = kzalloc(sizeof(*buf), GFP_KERNEL); >>>> + if (!buf) >>>> + return ERR_PTR(-ENOMEM); >>>> + >>>> + buf->dev = dev; >>>> + /* create attachment for the dmabuf with the user device */ >>>> + dba = dma_buf_attach(dbuf, buf->dev); >>>> + if (IS_ERR(dba)) { >>>> + pr_err("failed to attach dmabuf\n"); >>>> + kfree(buf); >>>> + return dba; >>>> + } >>>> + >>>> + buf->dma_dir = vb->vb2_queue->dma_dir; >>>> + buf->size = size; >>>> + buf->db_attach = dba; >>>> + >>>> + return buf; >>>> +} >>>> + >>>> +const struct vb2_mem_ops vb2_dmaheap_memops = { >>>> + .alloc = vb2_dmaheap_alloc, >>>> + .put = vb2_dmaheap_put, >>>> + .get_dmabuf = vb2_dmaheap_get_dmabuf, >>>> + .cookie = vb2_dmaheap_cookie, >>>> + .vaddr = vb2_dmaheap_vaddr, >>>> + .prepare = vb2_dmaheap_prepare, >>>> + .finish = vb2_dmaheap_finish, >>>> + .map_dmabuf = vb2_dmaheap_map_dmabuf, >>>> + .unmap_dmabuf = vb2_dmaheap_unmap_dmabuf, >>>> + .attach_dmabuf = vb2_dmaheap_attach_dmabuf, >>>> + .detach_dmabuf = vb2_dmaheap_detach_dmabuf, >>>> + .num_users = vb2_dmaheap_num_users, >>>> + .mmap = vb2_dmaheap_mmap, >>>> +}; >>>> + >>>> +MODULE_DESCRIPTION("DMA-Heap memory handling routines for videobuf2"); >>>> +MODULE_AUTHOR("Randy Li <ayaka(a)soulik.info>"); >>>> +MODULE_LICENSE("GPL"); >>>> +MODULE_IMPORT_NS(DMA_BUF); >>>> diff --git a/include/media/videobuf2-dma-heap.h b/include/media/videobuf2-dma-heap.h >>>> new file mode 100644 >>>> index 000000000000..fa057f67d6e9 >>>> --- /dev/null >>>> +++ b/include/media/videobuf2-dma-heap.h >>>> @@ -0,0 +1,30 @@ >>>> +/* >>>> + * Copyright (C) 2022 Randy Li <ayaka(a)soulik.info> >>>> + * >>>> + * This software is licensed under the terms of the GNU General Public >>>> + * License version 2, as published by the Free Software Foundation, and >>>> + * may be copied, distributed, and modified under those terms. >>>> + * >>>> + * This program is distributed in the hope that it will be useful, >>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>> + * GNU General Public License for more details. >>>> + * >>>> + */ >>>> + >>>> +#ifndef _MEDIA_VIDEOBUF2_DMA_HEAP_H >>>> +#define _MEDIA_VIDEOBUF2_DMA_HEAP_H >>>> + >>>> +#include <media/videobuf2-v4l2.h> >>>> +#include <linux/dma-mapping.h> >>>> + >>>> +static inline dma_addr_t >>>> +vb2_dmaheap_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no) >>>> +{ >>>> + dma_addr_t *addr = vb2_plane_cookie(vb, plane_no); >>>> + >>>> + return *addr; >>>> +} >>>> + >>>> +extern const struct vb2_mem_ops vb2_dmaheap_memops; >>>> +#endif >>>> -- >>>> 2.17.1 >> -- >> Hsia-Jun(Randy) Li

3 years

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig