Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

2 participants
2906 discussions

Re: [PATCH v3 03/18] ASoC: dt-bindings: mt6357: Add audio codec document

by Krzysztof Kozlowski

On 23/04/2024 19:07, Alexandre Mergnat wrote: > > > On 09/04/2024 17:55, Krzysztof Kozlowski wrote: >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + codec { >>> + mediatek,micbias0-microvolt = <1900000>; >>> + mediatek,micbias1-microvolt = <1700000>; >>> + mediatek,vaud28-supply = <&mt6357_vaud28_reg>; >> Sorry, this does not work. Change voltage to 1111111 and check the results. > > Actually it's worst ! I've removed the required property (vaud28-supply) but the dt check pass. > Same behavior for some other docs like mt6359.yaml Yeah, the schema is not applied. There is nothing selecting it, so this is no-op schema. I don't know what exactly you want to describe, but usually either you miss compatible or this should be just part of parent node. > > The at24.yaml doc works as expected, then I tried compare an find the issue, without success... > > I've replaced "codec" by "audio-codec", according to [1]. > I've tried multiple manner to implement the example code, without success. I'm wondering if what I > try to do is the correct way or parse-able by the dt_check. > > If I drop this file and implement all these new properties into the MFD PMIC documentation directly, > I've the expected dt_check result (function to good or wrong parameters) Yes. Best regards, Krzysztof

1 year, 2 months

Re: [PATCH] drm/panfrost: Fix dma_resv deadlock at drm object pin time

by Dmitry Osipenko

On 4/21/24 19:39, Adrián Larumbe wrote: > When Panfrost must pin an object that is being prepared a dma-buf > attachment for on behalf of another driver, the core drm gem object pinning > code already takes a lock on the object's dma reservation. > > However, Panfrost GEM object's pinning callback would eventually try taking > the lock on the same dma reservation when delegating pinning of the object > onto the shmem subsystem, which led to a deadlock. > > This can be shown by enabling CONFIG_DEBUG_WW_MUTEX_SLOWPATH, which throws > the following recursive locking situation: > > weston/3440 is trying to acquire lock: > ffff000000e235a0 (reservation_ww_class_mutex){+.+.}-{3:3}, at: drm_gem_shmem_pin+0x34/0xb8 [drm_shmem_helper] > but task is already holding lock: > ffff000000e235a0 (reservation_ww_class_mutex){+.+.}-{3:3}, at: drm_gem_pin+0x2c/0x80 [drm] > > Fix it by assuming the object's reservation had already been locked by the > time we reach panfrost_gem_pin. > > Cc: Thomas Zimmermann <tzimmermann(a)suse.de> > Cc: Dmitry Osipenko <dmitry.osipenko(a)collabora.com> > Cc: Boris Brezillon <boris.brezillon(a)collabora.com> > Cc: Steven Price <steven.price(a)arm.com> > Fixes: a78027847226 ("drm/gem: Acquire reservation lock in drm_gem_{pin/unpin}()") > Signed-off-by: Adrián Larumbe <adrian.larumbe(a)collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_gem.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > index d47b40b82b0b..6c26652d425d 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -192,7 +192,12 @@ static int panfrost_gem_pin(struct drm_gem_object *obj) > if (bo->is_heap) > return -EINVAL; > > - return drm_gem_shmem_pin(&bo->base); > + /* > + * Pinning can only happen in response to a prime attachment request from > + * another driver, but that's already being handled by drm_gem_pin > + */ > + drm_WARN_ON(obj->dev, obj->import_attach); > + return drm_gem_shmem_pin_locked(&bo->base); > } Will be better to use drm_gem_shmem_object_pin() to avoid such problem in future Please also fix the Lima driver -- Best regards, Dmitry

1 year, 2 months

Re: [PATCH v3 2/2] misc: sram: Add DMA-BUF Heap exporting of SRAM areas

by Greg Kroah-Hartman

On Tue, Apr 09, 2024 at 02:06:05PM +0200, Pascal FONTAIN wrote: > From: Andrew Davis <afd(a)ti.com> > > This new export type exposes to userspace the SRAM area as a DMA-BUF > Heap, > this allows for allocations of DMA-BUFs that can be consumed by various > DMA-BUF supporting devices. > > Signed-off-by: Andrew Davis <afd(a)ti.com> > Tested-by: Pascal Fontain <pascal.fontain(a)bachmann.info> When sending on a patch from someone else, you too must sign off on it as per our documenation. Please read it and understand what you are agreeing to when you do that for a new version please. > --- > drivers/misc/Kconfig | 7 + > drivers/misc/Makefile | 1 + > drivers/misc/sram-dma-heap.c | 246 +++++++++++++++++++++++++++++++++++ > drivers/misc/sram.c | 6 + > drivers/misc/sram.h | 16 +++ > 5 files changed, 276 insertions(+) > create mode 100644 drivers/misc/sram-dma-heap.c > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index 9e4ad4d61f06..e6674e913168 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -448,6 +448,13 @@ config SRAM > config SRAM_EXEC > bool > > +config SRAM_DMA_HEAP > + bool "Export on-chip SRAM pools using DMA-Heaps" > + depends on DMABUF_HEAPS && SRAM > + help > + This driver allows the export of on-chip SRAM marked as both pool > + and exportable to userspace using the DMA-Heaps interface. What will use this in userspace? thanks, greg k-h

1 year, 2 months

Re: [PATCH v2] doc: dma-buf: fix grammar typo

by Christian König

Well checkpatch.pl still complained: ERROR: trailing whitespace #157: FILE: Documentation/driver-api/dma-buf.rst:80: + If llseek on dma-buf FDs isn't supported the kernel will report -ESPIPE for all^M$ That actually looks like you used a Windows line ending. I fixed it up and pushed the patch, but please take a bit more care next time. Thanks, Christian. Am 17.04.24 um 06:59 schrieb Baruch Siach: > Use 'supported' instead of 'support'. 'support' makes no sense in this > context. > > Signed-off-by: Baruch Siach <baruch(a)tkos.co.il> > --- > v2: Add commit log message (Christian König) > --- > Documentation/driver-api/dma-buf.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst > index 0c153d79ccc4..29abf1eebf9f 100644 > --- a/Documentation/driver-api/dma-buf.rst > +++ b/Documentation/driver-api/dma-buf.rst > @@ -77,7 +77,7 @@ consider though: > the usual size discover pattern size = SEEK_END(0); SEEK_SET(0). Every other > llseek operation will report -EINVAL. > > - If llseek on dma-buf FDs isn't support the kernel will report -ESPIPE for all > + If llseek on dma-buf FDs isn't supported the kernel will report -ESPIPE for all > cases. Userspace can use this to detect support for discovering the dma-buf > size using llseek. >

1 year, 2 months

Re: [PATCH v5 1/9] drm/mediatek/uapi: Add DRM_MTK_GEM_CREATE_ENCRYPTED flag

by Nicolas Dufresne

Hi, Le mercredi 03 avril 2024 à 18:26 +0800, Shawn Sung a écrit : > From: "Jason-JH.Lin" <jason-jh.lin(a)mediatek.com> > > Add DRM_MTK_GEM_CREATE_ENCRYPTED flag to allow user to allocate Is "ENCRYPTED" a proper naming ? My expectation is that this would hold data in a PROTECTED memory region but that no cryptographic algorithm will be involved. Nicolas > a secure buffer to support secure video path feature. > > Signed-off-by: Jason-JH.Lin <jason-jh.lin(a)mediatek.com> > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.com> > --- > include/uapi/drm/mediatek_drm.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/uapi/drm/mediatek_drm.h b/include/uapi/drm/mediatek_drm.h > index b0dea00bacbc4..e9125de3a24ad 100644 > --- a/include/uapi/drm/mediatek_drm.h > +++ b/include/uapi/drm/mediatek_drm.h > @@ -54,6 +54,7 @@ struct drm_mtk_gem_map_off { > > #define DRM_MTK_GEM_CREATE 0x00 > #define DRM_MTK_GEM_MAP_OFFSET 0x01 > +#define DRM_MTK_GEM_CREATE_ENCRYPTED 0x02 > > #define DRM_IOCTL_MTK_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + \ > DRM_MTK_GEM_CREATE, struct drm_mtk_gem_create)

1 year, 2 months

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

by Maíra Canal

Hi Tvrtko, On 4/1/24 10:21, Tvrtko Ursulin wrote: > > On 01/04/2024 13:45, Christian König wrote: >> Am 01.04.24 um 14:39 schrieb Tvrtko Ursulin: >>> >>> On 29/03/2024 00:00, T.J. Mercier wrote: >>>> 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> >>> >>> Thanks! >>> >>> How would patches to dma-buf be typically landed? Via what tree I >>> mean? drm-misc-next? >> >> That should go through drm-misc-next. >> >> And feel free to add Reviewed-by: Christian König >> <christian.koenig(a)amd.com> as well. > > Thanks! > > Maarten if I got it right you are handling the next drm-misc-next pull - > could you merge this one please? Applied to drm-misc/drm-misc-next! Best Regards, - Maíra > > Regards, > > Tvrtko

1 year, 2 months

Re: [PATCH] doc: dma-buf: fix grammar typo

by Christian König

Am 14.04.24 um 15:07 schrieb Baruch Siach: > Signed-off-by: Baruch Siach <baruch(a)tkos.co.il> > --- > Documentation/driver-api/dma-buf.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst > index 0c153d79ccc4..29abf1eebf9f 100644 > --- a/Documentation/driver-api/dma-buf.rst > +++ b/Documentation/driver-api/dma-buf.rst > @@ -77,7 +77,7 @@ consider though: > the usual size discover pattern size = SEEK_END(0); SEEK_SET(0). Every other > llseek operation will report -EINVAL. > > - If llseek on dma-buf FDs isn't support the kernel will report -ESPIPE for all > + If llseek on dma-buf FDs isn't supported the kernel will report -ESPIPE for all Looks valid of hand, but checkpatch.pl complains about 2 errors (missing commit message for example) and a warning. Please fix and resend. Thanks, Christian. > cases. Userspace can use this to detect support for discovering the dma-buf > size using llseek. >

1 year, 2 months

Re: [PATCH net-next v9 0/3] Add minimal XDP support to TI AM65 CPSW Ethernet driver

by patchwork-bot+netdevbpf＠kernel.org

Hello: This series was applied to netdev/net-next.git (main) by David S. Miller <davem(a)davemloft.net>: On Fri, 12 Apr 2024 17:38:31 +0200 you wrote: > This patch adds XDP support to TI AM65 CPSW Ethernet driver. > > The following features are implemented: NETDEV_XDP_ACT_BASIC, > NETDEV_XDP_ACT_REDIRECT, and NETDEV_XDP_ACT_NDO_XMIT. > > Zero-copy and non-linear XDP buffer supports are NOT implemented. > > [...] Here is the summary with links: - [net-next,v9,1/3] net: ethernet: ti: Add accessors for struct k3_cppi_desc_pool members https://git.kernel.org/netdev/net-next/c/cd8ff81f747f - [net-next,v9,2/3] net: ethernet: ti: Add desc_infos member to struct k3_cppi_desc_pool https://git.kernel.org/netdev/net-next/c/84d767a3c0b5 - [net-next,v9,3/3] net: ethernet: ti: am65-cpsw: Add minimal XDP support https://git.kernel.org/netdev/net-next/c/8acacc40f733 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html

1 year, 2 months

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

by Christian König

Am 15.04.24 um 12:35 schrieb zhiguojiang: > 在 2024/4/12 14:39, Christian König 写道: >> [Some people who received this message don't often get email from >> christian.koenig(a)amd.com. Learn why this is important at >> https://aka.ms/LearnAboutSenderIdentification ] >> >> Am 12.04.24 um 08:19 schrieb zhiguojiang: >>> [SNIP] >>> -> Here task 2220 do epoll again where internally it will get/put then >>> start to free twice and lead to final crash. >>> >>> Here is the basic flow: >>> >>> 1. Thread A install the dma_buf_fd via dma_buf_export, the fd refcount >>> is 1 >>> >>> 2. Thread A add the fd to epoll list via epoll_ctl(EPOLL_CTL_ADD) >>> >>> 3. After use the dma buf, Thread A close the fd, then here fd refcount >>> is 0, >>> and it will run __fput finally to release the file >> >> Stop, that isn't correct. >> >> The fs layer which calls dma_buf_poll() should make sure that the file >> can't go away until the function returns. >> >> Then inside dma_buf_poll() we add another reference to the file while >> installing the callback: >> >> /* Paired with fput in dma_buf_poll_cb */ >> get_file(dmabuf->file); > Hi, > > The problem may just occurred here. > > Is it possible file reference count already decreased to 0 and fput > already being in progressing just before calling > get_file(dmabuf->file) in dma_buf_poll()? No, exactly that isn't possible. If a function gets a dma_buf pointer or even more general any reference counted pointer which has already decreased to 0 then that is a major bug in the caller of that function. BTW: It's completely illegal to work around such issues by using file_count() or RCU functions. So when you suggest stuff like that it will immediately face rejection. Regards, Christian. > >> >> This reference is only dropped after the callback is completed in >> dma_buf_poll_cb(): >> >> /* Paired with get_file in dma_buf_poll */ >> fput(dmabuf->file); >> >> So your explanation for the issue just seems to be incorrect. >> >>> >>> 4. Here Thread A not do epoll_ctl(EPOLL_CTL_DEL) manunally, so it >>> still resides in one epoll_list. >>> Although __fput will call eventpoll_release to remove the file from >>> binded epoll list, >>> but it has small time window where Thread B jumps in. >> >> Well if that is really the case then that would then be a bug in >> epoll_ctl(). >> >>> >>> 5. During the small window, Thread B do the poll action for >>> dma_buf_fd, where it will fget/fput for the file, >>> this means the fd refcount will be 0 -> 1 -> 0, and it will call >>> __fput again. >>> This will lead to __fput twice for the same file. >>> >>> 6. So the potenial fix is use get_file_rcu which to check if file >>> refcount already zero which means under free. >>> If so, we just return and no need to do the dma_buf_poll. >> >> Well to say it bluntly as far as I can see this suggestion is completely >> nonsense. >> >> When the reference to the file goes away while dma_buf_poll() is >> executed then that's a massive bug in the caller of that function. >> >> Regards, >> Christian. >> >>> >>> Here is the race condition: >>> >>> Thread A Thread B >>> dma_buf_export >>> fd_refcount is 1 >>> epoll_ctl(EPOLL_ADD) >>> add dma_buf_fd to epoll list >>> close(dma_buf_fd) >>> fd_refcount is 0 >>> __fput >>> dma_buf_poll >>> fget >>> fput >>> fd_refcount is zero again >>> >>> Thanks >>> >> >

1 year, 2 months

Re: [PATCH v5 2/9] drm/mediatek: Add secure buffer control flow to mtk_drm_gem

by Maxime Ripard

On Wed, Apr 03, 2024 at 06:26:54PM +0800, Shawn Sung wrote: > From: "Jason-JH.Lin" <jason-jh.lin(a)mediatek.com> > > Add secure buffer control flow to mtk_drm_gem. > > When user space takes DRM_MTK_GEM_CREATE_ENCRYPTED flag and size > to create a mtk_drm_gem object, mtk_drm_gem will find a matched size > dma buffer from secure dma-heap and bind it to mtk_drm_gem object. > > Signed-off-by: Jason-JH.Lin <jason-jh.lin(a)mediatek.com> > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_gem.c | 85 +++++++++++++++++++++++++++++- > drivers/gpu/drm/mediatek/mtk_gem.h | 4 ++ > 2 files changed, 88 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_gem.c b/drivers/gpu/drm/mediatek/mtk_gem.c > index e59e0727717b7..ec34d02c14377 100644 > --- a/drivers/gpu/drm/mediatek/mtk_gem.c > +++ b/drivers/gpu/drm/mediatek/mtk_gem.c > @@ -4,6 +4,8 @@ > */ > > #include <linux/dma-buf.h> > +#include <linux/dma-heap.h> > +#include <uapi/linux/dma-heap.h> > #include <drm/mediatek_drm.h> > > #include <drm/drm.h> > @@ -102,6 +104,81 @@ struct mtk_gem_obj *mtk_gem_create(struct drm_device *dev, > return ERR_PTR(ret); > } > > +struct mtk_gem_obj *mtk_gem_create_from_heap(struct drm_device *dev, > + const char *heap, size_t size) > +{ > + struct mtk_drm_private *priv = dev->dev_private; > + struct mtk_gem_obj *mtk_gem; > + struct drm_gem_object *obj; > + struct dma_heap *dma_heap; > + struct dma_buf *dma_buf; > + struct dma_buf_attachment *attach; > + struct sg_table *sgt; > + struct iosys_map map = {}; > + int ret; > + > + mtk_gem = mtk_gem_init(dev, size); > + if (IS_ERR(mtk_gem)) > + return ERR_CAST(mtk_gem); > + > + obj = &mtk_gem->base; > + > + dma_heap = dma_heap_find(heap); > + if (!dma_heap) { > + DRM_ERROR("heap find fail\n"); > + goto err_gem_free; > + } > + dma_buf = dma_heap_buffer_alloc(dma_heap, size, > + O_RDWR | O_CLOEXEC, DMA_HEAP_VALID_HEAP_FLAGS); > + if (IS_ERR(dma_buf)) { > + DRM_ERROR("buffer alloc fail\n"); > + dma_heap_put(dma_heap); > + goto err_gem_free; > + } > + dma_heap_put(dma_heap); > + > + attach = dma_buf_attach(dma_buf, priv->dma_dev); > + if (IS_ERR(attach)) { > + DRM_ERROR("attach fail, return\n"); > + dma_buf_put(dma_buf); > + goto err_gem_free; > + } > + > + sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); > + if (IS_ERR(sgt)) { > + DRM_ERROR("map failed, detach and return\n"); > + dma_buf_detach(dma_buf, attach); > + dma_buf_put(dma_buf); > + goto err_gem_free; > + } > + obj->import_attach = attach; > + mtk_gem->dma_addr = sg_dma_address(sgt->sgl); > + mtk_gem->sg = sgt; > + mtk_gem->size = dma_buf->size; > + > + if (!strcmp(heap, "mtk_svp") || !strcmp(heap, "mtk_svp_cma")) { > + /* secure buffer can not be mapped */ > + mtk_gem->secure = true; > + } else { > + ret = dma_buf_vmap(dma_buf, &map); > + mtk_gem->kvaddr = map.vaddr; > + if (ret) { > + DRM_ERROR("map failed, ret=%d\n", ret); > + dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL); > + dma_buf_detach(dma_buf, attach); > + dma_buf_put(dma_buf); > + mtk_gem->kvaddr = NULL; > + } > + } > + > + return mtk_gem; > + > +err_gem_free: > + drm_gem_object_release(obj); > + kfree(mtk_gem); > + return ERR_PTR(-ENOMEM); > +} > + > void mtk_gem_free_object(struct drm_gem_object *obj) > { > struct mtk_gem_obj *mtk_gem = to_mtk_gem_obj(obj); > @@ -229,7 +306,9 @@ struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev, > if (IS_ERR(mtk_gem)) > return ERR_CAST(mtk_gem); > > + mtk_gem->secure = !sg_page(sg->sgl); > mtk_gem->dma_addr = sg_dma_address(sg->sgl); > + mtk_gem->size = attach->dmabuf->size; > mtk_gem->sg = sg; > > return &mtk_gem->base; > @@ -304,7 +383,11 @@ int mtk_gem_create_ioctl(struct drm_device *dev, void *data, > struct drm_mtk_gem_create *args = data; > int ret; > > - mtk_gem = mtk_gem_create(dev, args->size, false); > + if (args->flags & DRM_MTK_GEM_CREATE_ENCRYPTED) > + mtk_gem = mtk_gem_create_from_heap(dev, "mtk_svp_cma", args->size); That heap doesn't exist upstream either. Also, I'm wondering if it's the right solution there. From what I can tell, you want to allow to create encrypted buffers from the TEE. Why do we need this as a DRM ioctl at all? A heap seems like the perfect solution to do so, and then you just have to import it into DRM. I'm also not entirely sure that not having a SG list is enough to consider the buffer secure. Wouldn't a buffer allocated without a kernel mapping also be in that situation? Maxime

1 year, 2 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig