Linaro-mm-sig March 2024

linaro-mm-sig@lists.linaro.org

19 participants
34 discussions

[RFC PATCH] dma-buf/dma-fence: Use a successful read_trylock() annotation for dma_fence_begin_signalling()

by Thomas Hellström

Condsider the following call sequence: /* Upper layer */ dma_fence_begin_signalling(); lock(tainted_shared_lock); /* Driver callback */ dma_fence_begin_signalling(); ... The driver might here use a utility that is annotated as intended for the dma-fence signalling critical path. Now if the upper layer isn't correctly annotated yet for whatever reason, resulting in /* Upper layer */ lock(tainted_shared_lock); /* Driver callback */ dma_fence_begin_signalling(); We will receive a false lockdep locking order violation notification from dma_fence_begin_signalling(). However entering a dma-fence signalling critical section itself doesn't block and could not cause a deadlock. So use a successful read_trylock() annotation instead for dma_fence_begin_signalling(). That will make sure that the locking order is correctly registered in the first case, and doesn't register any locking order in the second case. The alternative is of course to make sure that the "Upper layer" is always correctly annotated. But experience shows that's not easily achievable in all cases. Signed-off-by: Thomas Hellström <thomas.hellstrom(a)linux.intel.com> --- drivers/dma-buf/dma-fence.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index f177c56269bb..17f632768ef9 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -308,8 +308,8 @@ bool dma_fence_begin_signalling(void) if (in_atomic()) return true; - /* ... and non-recursive readlock */ - lock_acquire(&dma_fence_lockdep_map, 0, 0, 1, 1, NULL, _RET_IP_); + /* ... and non-recursive successful read_trylock */ + lock_acquire(&dma_fence_lockdep_map, 0, 1, 1, 1, NULL, _RET_IP_); return false; } @@ -340,7 +340,7 @@ void __dma_fence_might_wait(void) lock_map_acquire(&dma_fence_lockdep_map); lock_map_release(&dma_fence_lockdep_map); if (tmp) - lock_acquire(&dma_fence_lockdep_map, 0, 0, 1, 1, NULL, _THIS_IP_); + lock_acquire(&dma_fence_lockdep_map, 0, 1, 1, 1, NULL, _THIS_IP_); } #endif -- 2.39.2

2 weeks, 5 days

[PATCH v4 0/7] dma-buf: heaps: Add restricted heap

by Yong Wu

The purpose of this patchset is for MediaTek secure video playback, and also to enable other potential uses of this in the future. The 'restricted dma-heap' will be used to allocate dma_buf objects that reference memory in the secure world that is inaccessible/unmappable by the non-secure (i.e. kernel/userspace) world. That memory will be used by the secure/ trusted world to store secure information (i.e. decrypted media content). The dma_bufs allocated from the kernel will be passed to V4L2 for video decoding (as input and output). They will also be used by the drm system for rendering of the content. This patchset adds two MediaTek restricted heaps and they will be used in v4l2[1] and drm[2]. 1) restricted_mtk_cm: secure chunk memory for MediaTek SVP (Secure Video Path). The buffer is reserved for the secure world after bootup and it is used for vcodec's ES/working buffer; 2) restricted_mtk_cma: secure CMA memory for MediaTek SVP. This buffer is dynamically reserved for the secure world and will be got when we start playing secure videos. Once the security video playing is complete, the CMA will be released. This heap is used for the vcodec's frame buffer. [1] https://lore.kernel.org/linux-mediatek/20231206081538.17056-1-yunfei.dong@m… [2] https://lore.kernel.org/all/20231223182932.27683-1-jason-jh.lin@mediatek.co… Change note: v4: 1) Rename the heap name from "secure" to "restricted". suggested from Simon/Pekka. There are still several "secure" string in MTK file since we use ARM platform in which we call this "secure world"/ "secure command". v3: https://lore.kernel.org/linux-mediatek/20231212024607.3681-1-yong.wu@mediat… 1) Separate the secure heap to a common file(secure_heap.c) and mtk special file (secure_heap_mtk.c), and put all the tee related code into our special file. 2) About dt-binding, Add "mediatek," prefix since this is Mediatek TEE firmware definition. 3) Remove the normal CMA heap which is a draft for qcom. Rebase on v6.7-rc1. v2: https://lore.kernel.org/linux-mediatek/20231111111559.8218-1-yong.wu@mediat… 1) Move John's patches into the vcodec patchset since they use the new dma heap interface directly. https://lore.kernel.org/linux-mediatek/20231106120423.23364-1-yunfei.dong@m… 2) Reword the dt-binding description. 3) Rename the heap name from mtk_svp to secure_mtk_cm. This means the current vcodec/DRM upstream code doesn't match this. 4) Add a normal CMA heap. currently it should be a draft version. 5) Regarding the UUID, I still use hard code, but put it in a private data which allow the others could set their own UUID. What's more, UUID is necessary for the session with TEE. If we don't have it, we can't communicate with the TEE, including the get_uuid interface, which tries to make uuid more generic, not working. If there is other way to make UUID more general, please free to tell me. v1: https://lore.kernel.org/linux-mediatek/20230911023038.30649-1-yong.wu@media… Base on v6.6-rc1. Yong Wu (7): dt-bindings: reserved-memory: Add mediatek,dynamic-restricted-region dma-buf: heaps: Initialize a restricted heap dma-buf: heaps: restricted_heap: Add private heap ops dma-buf: heaps: restricted_heap: Add dma_ops dma-buf: heaps: restricted_heap: Add MediaTek restricted heap and heap_init dma-buf: heaps: restricted_heap_mtk: Add TEE memory service call dma_buf: heaps: restricted_heap_mtk: Add a new CMA heap .../mediatek,dynamic-restricted-region.yaml | 43 +++ drivers/dma-buf/heaps/Kconfig | 16 + drivers/dma-buf/heaps/Makefile | 4 +- drivers/dma-buf/heaps/restricted_heap.c | 237 +++++++++++++ drivers/dma-buf/heaps/restricted_heap.h | 43 +++ drivers/dma-buf/heaps/restricted_heap_mtk.c | 322 ++++++++++++++++++ 6 files changed, 664 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/reserved-memory/mediatek,dynamic-restricted-region.yaml create mode 100644 drivers/dma-buf/heaps/restricted_heap.c create mode 100644 drivers/dma-buf/heaps/restricted_heap.h create mode 100644 drivers/dma-buf/heaps/restricted_heap_mtk.c -- 2.18.0

4 months, 3 weeks

[PATCH v9 0/6] iio: new DMABUF based API

by Paul Cercueil

Hi Jonathan, Here's the final-er version of the IIO DMABUF patchset. This v9 fixes the few issues reported by the kernel bot. This was based on next-20240308. Changelog: - [3/6]: - Select DMA_SHARED_BUFFER in Kconfig - Remove useless forward declaration of 'iio_dma_fence' - Import DMA-BUF namespace - Add missing __user tag to iio_buffer_detach_dmabuf() argument Cheers, -Paul Paul Cercueil (6): dmaengine: Add API function dmaengine_prep_peripheral_dma_vec() dmaengine: dma-axi-dmac: Implement device_prep_peripheral_dma_vec iio: core: Add new DMABUF interface infrastructure iio: buffer-dma: Enable support for DMABUFs iio: buffer-dmaengine: Support new DMABUF based userspace API Documentation: iio: Document high-speed DMABUF based API Documentation/iio/iio_dmabuf_api.rst | 54 ++ Documentation/iio/index.rst | 1 + drivers/dma/dma-axi-dmac.c | 40 ++ drivers/iio/Kconfig | 1 + drivers/iio/buffer/industrialio-buffer-dma.c | 181 ++++++- .../buffer/industrialio-buffer-dmaengine.c | 59 ++- drivers/iio/industrialio-buffer.c | 462 ++++++++++++++++++ include/linux/dmaengine.h | 27 + include/linux/iio/buffer-dma.h | 31 ++ include/linux/iio/buffer_impl.h | 30 ++ include/uapi/linux/iio/buffer.h | 22 + 11 files changed, 891 insertions(+), 17 deletions(-) create mode 100644 Documentation/iio/iio_dmabuf_api.rst -- 2.43.0

6 months

Re: [PATCH net-next v5 1/3] net: ethernet: ti: Add accessors for struct k3_cppi_desc_pool members

by Russell King (Oracle)

On Thu, Mar 28, 2024 at 12:06:56PM +0000, Naveen Mamindlapalli wrote: > > diff --git a/drivers/net/ethernet/ti/k3-cppi-desc-pool.c b/drivers/net/ethernet/ti/k3- > > cppi-desc-pool.c > > index 05cc7aab1ec8..fe8203c05731 100644 > > --- a/drivers/net/ethernet/ti/k3-cppi-desc-pool.c > > +++ b/drivers/net/ethernet/ti/k3-cppi-desc-pool.c > > @@ -132,5 +132,17 @@ size_t k3_cppi_desc_pool_avail(struct > > k3_cppi_desc_pool *pool) } EXPORT_SYMBOL_GPL(k3_cppi_desc_pool_avail); > > > > +size_t k3_cppi_desc_pool_desc_size(struct k3_cppi_desc_pool *pool) { > > + return pool->desc_size; > > Don't you need to add NULL check on pool ptr since this function is exported? What bearing does exporting a function have on whether it should check for NULL? Given that this function returns size_t, it can't return an error number. So what value would it return if "pool" were NULL? It can only return a positive integer or zero. Also, the argument should be const as the function doesn't modify the contents of "pool". -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

6 months, 1 week

Re: [PATCH] dmabuf: fix dmabuf file poll uaf issue

by T.J. Mercier

On Tue, Mar 26, 2024 at 7:29 PM Zhiguo Jiang <justinjiang(a)vivo.com> wrote: > > The issue is a UAF issue of dmabuf file fd. Throght debugging, we found > that the dmabuf file fd is added to the epoll event listener list, and > when it is released, it is not removed from the epoll list, which leads > to the UAF(Use-After-Free) issue. > > The UAF issue can be solved by checking dmabuf file->f_count value and > skipping the poll operation for the closed dmabuf file in the > dma_buf_poll(). We have tested this solved patch multiple times and > have not reproduced the uaf issue. > Hi Zhiguo, What is the most recent kernel version you've seen the bug on? You are closing the dmabuf fd from another thread while it is still part of the epoll interest list? Thanks, T.J.

6 months, 1 week

Re: [PATCH] dma-buf: Do not build debugfs related code when !CONFIG_DEBUG_FS

by T.J. Mercier

On Thu, Mar 28, 2024 at 7:53 AM Tvrtko Ursulin <tursulin(a)igalia.com> wrote: > > From: Tvrtko Ursulin <tursulin(a)ursulin.net> > > There is no point in compiling in the list and mutex operations which are > only used from the dma-buf debugfs code, if debugfs is not compiled in. > > Put the code in questions behind some kconfig guards and so save some text > and maybe even a pointer per object at runtime when not enabled. > > Signed-off-by: Tvrtko Ursulin <tursulin(a)ursulin.net> Reviewed-by: T.J. Mercier <tjmercier(a)google.com>

6 months, 2 weeks

[PATCH] drm/prime: Unbreak virtgpu dma-buf export

by Rob Clark

From: Rob Clark <robdclark(a)chromium.org> virtgpu "vram" GEM objects do not implement obj->get_sg_table(). But they also don't use drm_gem_map_dma_buf(). In fact they may not even have guest visible pages. But it is perfectly fine to export and share with other virtual devices. Reported-by: Dominik Behr <dbehr(a)chromium.org> Fixes: 207395da5a97 ("drm/prime: reject DMA-BUF attach when get_sg_table is missing") Signed-off-by: Rob Clark <robdclark(a)chromium.org> --- drivers/gpu/drm/drm_prime.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 7352bde299d5..64dd6276e828 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -582,7 +582,12 @@ int drm_gem_map_attach(struct dma_buf *dma_buf, { struct drm_gem_object *obj = dma_buf->priv; - if (!obj->funcs->get_sg_table) + /* + * drm_gem_map_dma_buf() requires obj->get_sg_table(), but drivers + * that implement their own ->map_dma_buf() do not. + */ + if ((dma_buf->ops->map_dma_buf == drm_gem_map_dma_buf) && + !obj->funcs->get_sg_table) return -ENOSYS; return drm_gem_pin(obj); -- 2.44.0

6 months, 2 weeks

Re: [PATCH] drm/prime: Unbreak virtgpu dma-buf export

by Rob Clark

This is actually a bit concerning.. importing a host page backed buffer without guest mapping into a passthru device probably doesn't work and should be rejected earlier. I do think we should relax the restriction (either taking my patch or reverting the commit it fixes) until we work this out properly (because the original patch is a regression), but importing a buffer without guest pages into a passthru device can't possibly work properly. Maybe it works by chance if the host buffer is mapped to the guest, but that is not guaranteed. BR, -R On Mon, Mar 25, 2024 at 3:35 PM Dominik Behr <dbehr(a)chromium.org> wrote: > > It also fixes importing virtgpu blobs into real hardware, for instance amdgpu for DRI_PRIME rendering. > > On Fri, Mar 22, 2024 at 2:48 PM Rob Clark <robdclark(a)gmail.com> wrote: >> >> From: Rob Clark <robdclark(a)chromium.org> >> >> virtgpu "vram" GEM objects do not implement obj->get_sg_table(). But >> they also don't use drm_gem_map_dma_buf(). In fact they may not even >> have guest visible pages. But it is perfectly fine to export and share >> with other virtual devices. >> >> Reported-by: Dominik Behr <dbehr(a)chromium.org> >> Fixes: 207395da5a97 ("drm/prime: reject DMA-BUF attach when get_sg_table is missing") >> Signed-off-by: Rob Clark <robdclark(a)chromium.org> >> --- >> drivers/gpu/drm/drm_prime.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >> index 7352bde299d5..64dd6276e828 100644 >> --- a/drivers/gpu/drm/drm_prime.c >> +++ b/drivers/gpu/drm/drm_prime.c >> @@ -582,7 +582,12 @@ int drm_gem_map_attach(struct dma_buf *dma_buf, >> { >> struct drm_gem_object *obj = dma_buf->priv; >> >> - if (!obj->funcs->get_sg_table) >> + /* >> + * drm_gem_map_dma_buf() requires obj->get_sg_table(), but drivers >> + * that implement their own ->map_dma_buf() do not. >> + */ >> + if ((dma_buf->ops->map_dma_buf == drm_gem_map_dma_buf) && >> + !obj->funcs->get_sg_table) >> return -ENOSYS; >> >> return drm_gem_pin(obj); >> -- >> 2.44.0 >>

6 months, 2 weeks

Re: [PATCH v4 03/14] drm/mediatek: Rename "mtk_drm_plane" to "mtk_plane"

by AngeloGioacchino Del Regno

Il 22/03/24 02:27, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename all "mtk_drm_plane" to "mtk_plane": > - To align the naming rule > - To reduce the code size > > Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com> Shawn - please - can you fix my typo'ed name also here and on all of the patches of this series? Thanks. > Reviewed-by: CK Hu <ck.hu(a)mediatek.com> > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > --- > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 6 +++--- > drivers/gpu/drm/mediatek/mtk_drm_plane.h | 4 ++-- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > index cbdb70677d305..43137c46fc148 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > @@ -93,8 +93,8 @@ static bool mtk_plane_format_mod_supported(struct drm_plane *plane, > return true; > } > > -static void mtk_drm_plane_destroy_state(struct drm_plane *plane, > - struct drm_plane_state *state) > +static void mtk_plane_destroy_state(struct drm_plane *plane, > + struct drm_plane_state *state) > { > __drm_atomic_helper_plane_destroy_state(state); > kfree(to_mtk_plane_state(state)); > @@ -241,7 +241,7 @@ static const struct drm_plane_funcs mtk_plane_funcs = { > .destroy = drm_plane_cleanup, > .reset = mtk_plane_reset, > .atomic_duplicate_state = mtk_plane_duplicate_state, > - .atomic_destroy_state = mtk_drm_plane_destroy_state, > + .atomic_destroy_state = mtk_plane_destroy_state, > .format_mod_supported = mtk_plane_format_mod_supported, > }; > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h > index 99aff7da0831d..231bb7aac9473 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h > @@ -4,8 +4,8 @@ > * Author: CK Hu <ck.hu(a)mediatek.com> > */ > > -#ifndef _MTK_DRM_PLANE_H_ > -#define _MTK_DRM_PLANE_H_ > +#ifndef _MTK_PLANE_H_ > +#define _MTK_PLANE_H_ > > #include <drm/drm_crtc.h> > #include <linux/types.h>

6 months, 3 weeks

Re: [PATCH v3 09/14] drm/mediatek: Rename files "mtk_drm_ddp_comp.c" to "mtk_ddp_comp.c"

by AngeloGioacchino Del Regno

Il 20/03/24 03:42, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename files mtk_drm_ddp_comp.c to mtk_ddp_comp.c and > modify the Makefile accordingly. > > Reviewed-by: CK Hu <ck.hu(a)mediatek.com> > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno(a)collabora.com>

6 months, 3 weeks

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig March 2024