- Linaro-mm-sig - lists.linaro.org

[PATCH v2] udmabuf: Fix a potential (and unlikely) access to unallocated memory

by Christophe JAILLET

If 'list_limit' is set to a very high value, 'lsize' computation could overflow if 'head.count' is big enough. In such a case, udmabuf_create() would access to memory beyond 'list'. Use memdup_array_user() which checks for overflow. While at it, include <linux/string.h>. Fixes: fbb0de795078 ("Add udmabuf misc device") Signed-off-by: Christophe JAILLET <christophe.jaillet(a)wanadoo.fr> --- v2: - Use memdup_array_user() [Kees Cook] - Use sizeof(*list) [Gustavo A. R. Silva] - Add include <linux/string.h> v1: https://lore.kernel.org/all/3e37f05c7593f1016f0a46de188b3357cbbd0c0b.169506… Signed-off-by: Christophe JAILLET <christophe.jaillet(a)wanadoo.fr> --- drivers/dma-buf/udmabuf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c40645999648..5728948ea6f2 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -11,6 +11,7 @@ #include <linux/module.h> #include <linux/shmem_fs.h> #include <linux/slab.h> +#include <linux/string.h> #include <linux/udmabuf.h> #include <linux/vmalloc.h> #include <linux/iosys-map.h> @@ -314,14 +315,13 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) struct udmabuf_create_list head; struct udmabuf_create_item *list; int ret = -EINVAL; - u32 lsize; if (copy_from_user(&head, (void __user *)arg, sizeof(head))) return -EFAULT; if (head.count > list_limit) return -EINVAL; - lsize = sizeof(struct udmabuf_create_item) * head.count; - list = memdup_user((void __user *)(arg + sizeof(head)), lsize); + list = memdup_array_user((void __user *)(arg + sizeof(head)), + sizeof(*list), head.count); if (IS_ERR(list)) return PTR_ERR(list); -- 2.43.2

1 year, 11 months

3
4
0 0

Re: [PATCH v5] drm/test: add a test suite for GEM objects backed by shmem

by Guenter Roeck

Hi, On Thu, Nov 30, 2023 at 06:14:16PM +0100, Marco Pagani wrote: > This patch introduces an initial KUnit test suite for GEM objects > backed by shmem buffers. > > Suggested-by: Javier Martinez Canillas <javierm(a)redhat.com> > Signed-off-by: Marco Pagani <marpagan(a)redhat.com> When running this in qemu, I get lots of warnings backtraces in the drm core. WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:327 WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:173 WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:385 WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:211 WARNING: CPU: 0 PID: 1345 at kernel/dma/mapping.c:194 WARNING: CPU: 0 PID: 1347 at drivers/gpu/drm/drm_gem_shmem_helper.c:429 WARNING: CPU: 0 PID: 1349 at drivers/gpu/drm/drm_gem_shmem_helper.c:445 It looks like dma_resv_assert_held() asserts each time it is executed. The backtrace in kernel/dma/mapping.c is triggered by if (WARN_ON_ONCE(!dev->dma_mask)) return 0; in __dma_map_sg_attrs(). Is this a possible problem in the test code, or can it be caused by some limitations or bugs in the qemu emulation ? If so, do you have any thoughts or ideas what those limitations / bugs might be ? Thanks, Guenter

1 year, 11 months

1
0
0 0

[PATCH] dma-buf: try to catch swiotlb bounce buffers

by Daniel Vetter

They rather fundamentally break the entire concept of zero copy, so if an exporter manages to hand these out things will break all over. Luckily there's not too many case that use swiotlb_sync_single_for_device/cpu(): - The generic iommu dma-api code in drivers/iommu/dma-iommu.c. We can catch that with sg_dma_is_swiotlb() reliably. - The generic direct dma code in kernel/dma/direct.c. We can mostly catch that with looking for a NULL dma_ops, except for some powerpc special cases. - Xen, which I don't bother to catch here. Implement these checks in dma_buf_map_attachment when CONFIG_DMA_API_DEBUG is enabled. Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: Paul Cercueil <paul(a)crapouillou.net> --- Entirely untested, but since I sent the mail with the idea I figured I might as well type it up after I realized there's a lot fewer cases to check. That is, if I haven't completely misread the dma-api and swiotlb code. -Sima --- drivers/dma-buf/dma-buf.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index d1e7f823fbdb..d6f95523f995 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -28,6 +28,12 @@ #include <linux/mount.h> #include <linux/pseudo_fs.h> +#ifdef CONFIG_DMA_API_DEBUG +#include <linux/dma-direct.h> +#include <linux/dma-map-ops.h> +#include <linux/swiotlb.h> +#endif + #include <uapi/linux/dma-buf.h> #include <uapi/linux/magic.h> @@ -1149,10 +1155,13 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, #ifdef CONFIG_DMA_API_DEBUG if (!IS_ERR(sg_table)) { struct scatterlist *sg; + struct device *dev = attach->dev; u64 addr; int len; int i; + bool is_direct_dma = !get_dma_ops(dev); + for_each_sgtable_dma_sg(sg_table, sg, i) { addr = sg_dma_address(sg); len = sg_dma_len(sg); @@ -1160,7 +1169,15 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, pr_debug("%s: addr %llx or len %x is not page aligned!\n", __func__, addr, len); } + + if (is_direct_dma) { + phys_addr_t paddr = dma_to_phys(dev, addr); + + WARN_ON_ONCE(is_swiotlb_buffer(dev, paddr)); + } } + + WARN_ON_ONCE(sg_dma_is_swiotlb(sg)); } #endif /* CONFIG_DMA_API_DEBUG */ return sg_table; -- 2.43.0

2 years

1
0
0 0

[PATCH v5 0/6] usb: gadget: functionfs: DMABUF import interface

by Paul Cercueil

Hi, This is the v5 of my patchset that adds a new DMABUF import interface to FunctionFS. Daniel / Sima suggested that I should cache the dma_buf_attachment while the DMABUF is attached to the interface, instead of mapping/unmapping the DMABUF for every transfer (also because unmapping is not possible in the dma_fence's critical section). This meant having to add new dma_buf_begin_access() / dma_buf_end_access() functions that the driver can call to ensure cache coherency. These two functions are provided by the new patch [1/6], and an implementation for udmabuf was added in [2/6] - see the changelog below. This patchset was successfully tested with CONFIG_LOCKDEP, no errors were reported in dmesg while using the interface. This interface is being used at Analog Devices, to transfer data from high-speed transceivers to USB in a zero-copy fashion, using also the DMABUF import interface to the IIO subsystem which is being upstreamed in parallel [1]. The two are used by the Libiio software [2]. On a ZCU102 board with a FMComms3 daughter board, using the combination of these two new interfaces yields a drastic improvement of the throughput, from about 127 MiB/s using IIO's buffer read/write interface + read/write to the FunctionFS endpoints, to about 274 MiB/s when passing around DMABUFs, for a lower CPU usage (0.85 load avg. before, vs. 0.65 after). Right now, *technically* there are no users of this interface, as Analog Devices wants to wait until both interfaces are accepted upstream to merge the DMABUF code in Libiio into the main branch, and Jonathan wants to wait and see if this patchset is accepted to greenlight the DMABUF interface in IIO as well. I think this isn't really a problem; once everybody is happy with its part of the cake, we can merge them all at once. This is obviously for 5.9, and based on next-20240119. Changelog: - [1/6]: New patch - [2/6]: New patch - [5/6]: - Cache the dma_buf_attachment while the DMABUF is attached. - Use dma_buf_begin/end_access() to ensure that the DMABUF data will be coherent to the hardware. - Remove comment about cache-management and dma_buf_unmap_attachment(), since we now use dma_buf_begin/end_access(). - Select DMA_SHARED_BUFFER in Kconfig entry - Add Christian's ACK Cheers, -Paul [1] https://lore.kernel.org/linux-iio/219abc43b4fdd4a13b307ed2efaa0e6869e68e3f.… [2] https://github.com/analogdevicesinc/libiio/tree/pcercuei/dev-new-dmabuf-api Paul Cercueil (6): dma-buf: Add dma_buf_{begin,end}_access() dma-buf: udmabuf: Implement .{begin,end}_access usb: gadget: Support already-mapped DMA SGs usb: gadget: functionfs: Factorize wait-for-endpoint code usb: gadget: functionfs: Add DMABUF import interface Documentation: usb: Document FunctionFS DMABUF API Documentation/usb/functionfs.rst | 36 ++ drivers/dma-buf/dma-buf.c | 66 ++++ drivers/dma-buf/udmabuf.c | 27 ++ drivers/usb/gadget/Kconfig | 1 + drivers/usb/gadget/function/f_fs.c | 502 ++++++++++++++++++++++++++-- drivers/usb/gadget/udc/core.c | 7 +- include/linux/dma-buf.h | 37 ++ include/linux/usb/gadget.h | 2 + include/uapi/linux/usb/functionfs.h | 41 +++ 9 files changed, 698 insertions(+), 21 deletions(-) -- 2.43.0

2 years

6
31
0 0

Re: [PATCH] mm/cma: Drop cma_get_name()

by Andrew Morton

On Tue, 6 Feb 2024 09:45:18 +0530 Anshuman Khandual <anshuman.khandual(a)arm.com> wrote: > cma_get_name() just returns cma->name without any additional transformation > unlike other helpers such as cma_get_base() and cma_get_size(). This helper > is not worth the additional indirection, and can be dropped after replacing > directly with cma->name in the sole caller __add_cma_heap(). drivers/dma-buf/heaps/cma_heap.c: In function '__add_cma_heap': drivers/dma-buf/heaps/cma_heap.c:379:28: error: invalid use of undefined type 'struct cma' 379 | exp_info.name = cma->name; | ^~ Fixing this would require moving the `struct cma' definition into cma.h. I don't think that's worthwhile.

2 years

1
0
0 0

[PATCH v3 00/11] Add mediate-drm secure flow for SVP

by Jason-JH.Lin

From: Jason-jh Lin <jason-jh.lin(a)mediatek.corp-partner.google.com> Memory Definitions: secure memory - Memory allocated in the TEE (Trusted Execution Environment) which is inaccessible in the REE (Rich Execution Environment, i.e. linux kernel/userspace). secure handle - Integer value which acts as reference to 'secure memory'. Used in communication between TEE and REE to reference 'secure memory'. secure buffer - 'secure memory' that is used to store decrypted, compressed video or for other general purposes in the TEE. secure surface - 'secure memory' that is used to store graphic buffers. Memory Usage in SVP: The overall flow of SVP starts with encrypted video coming in from an outside source into the REE. The REE will then allocate a 'secure buffer' and send the corresponding 'secure handle' along with the encrypted, compressed video data to the TEE. The TEE will then decrypt the video and store the result in the 'secure buffer'. The REE will then allocate a 'secure surface'. The REE will pass the 'secure handles' for both the 'secure buffer' and 'secure surface' into the TEE for video decoding. The video decoder HW will then decode the contents of the 'secure buffer' and place the result in the 'secure surface'. The REE will then attach the 'secure surface' to the overlay plane for rendering of the video. Everything relating to ensuring security of the actual contents of the 'secure buffer' and 'secure surface' is out of scope for the REE and is the responsibility of the TEE. DRM driver handles allocation of gem objects that are backed by a 'secure surface' and for displaying a 'secure surface' on the overlay plane. This introduces a new flag for object creation called DRM_MTK_GEM_CREATE_ENCRYPTED which indicates it should be a 'secure surface'. All changes here are in MediaTek specific code. --- TODO: 1) Remove get sec larb port interface in ddp_comp, ovl and ovl_adaptor. 2) Verify instruction for enabling/disabling dapc and larb port in TEE drop the sec_engine flags in normal world and. 3) Move DISP_REG_OVL_SECURE setting to secure world for mtk_disp_ovl.c. 4) Change the parameter register address in mtk_ddp_sec_write() from "u32 addr" to "struct cmdq_client_reg *cmdq_reg". 5) Implement setting mmsys routing table in the secure world series. --- Based on 5 series and 1 patch: [1] v3 dma-buf: heaps: Add MediaTek secure heap - https://patchwork.kernel.org/project/linux-mediatek/list/?series=809023 [2] v3 add driver to support secure video decoder - https://patchwork.kernel.org/project/linux-mediatek/list/?series=807308 [3] v4 soc: mediatek: Add register definitions for GCE - https://patchwork.kernel.org/project/linux-mediatek/patch/20231212121957.19… [4] v2 Add CMDQ driver support for mt8188 - https://patchwork.kernel.org/project/linux-mediatek/list/?series=810302 [5] Add mediatek,gce-events definition to mediatek,gce-mailbox bindings - https://patchwork.kernel.org/project/linux-mediatek/list/?series=810938 [6] v3 Add CMDQ secure driver for SVP - https://patchwork.kernel.org/project/linux-mediatek/list/?series=812379 --- Change in v3: 1. fix kerneldoc problems 2. fix typo in title and commit message 3. adjust naming for secure variable 4. add the missing part for is_suecure plane implementation 5. use BIT_ULL macro to replace bit shifting 6. move modification of ovl_adaptor part to the correct patch 7. add TODO list in commit message 8. add commit message for using share memory to store execute count Change in v2: 1. remove the DRIVER_RDNDER flag for mtk_drm_ioctl 2. move cmdq_insert_backup_cookie into client driver 3. move secure gce node define from mt8195-cherry.dtsi to mt8195.dtsi --- CK Hu (1): drm/mediatek: Add interface to allocate MediaTek GEM buffer. Jason-JH.Lin (10): drm/mediatek/uapi: Add DRM_MTK_GEM_CREATE_ENCRYPTED flag drm/mediatek: Add secure buffer control flow to mtk_drm_gem drm/mediatek: Add secure identify flag and funcution to mtk_drm_plane drm/mediatek: Add mtk_ddp_sec_write to config secure buffer info drm/mediatek: Add get_sec_port interface to mtk_ddp_comp drm/mediatek: Add secure layer config support for ovl drm/mediatek: Add secure layer config support for ovl_adaptor drm/mediatek: Add secure flow support to mediatek-drm drm/mediatek: Add cmdq_insert_backup_cookie before secure pkt finalize arm64: dts: mt8195: Add secure mbox settings for vdosys arch/arm64/boot/dts/mediatek/mt8195.dtsi | 6 +- drivers/gpu/drm/mediatek/mtk_disp_drv.h | 3 + drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 31 +- .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 15 + drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 274 +++++++++++++++++- drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 1 + drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 30 ++ drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 14 + drivers/gpu/drm/mediatek/mtk_drm_drv.c | 13 + drivers/gpu/drm/mediatek/mtk_drm_gem.c | 122 ++++++++ drivers/gpu/drm/mediatek/mtk_drm_gem.h | 16 + drivers/gpu/drm/mediatek/mtk_drm_plane.c | 26 ++ drivers/gpu/drm/mediatek/mtk_drm_plane.h | 2 + drivers/gpu/drm/mediatek/mtk_mdp_rdma.c | 11 +- drivers/gpu/drm/mediatek/mtk_mdp_rdma.h | 2 + include/uapi/drm/mediatek_drm.h | 59 ++++ 16 files changed, 607 insertions(+), 18 deletions(-) create mode 100644 include/uapi/drm/mediatek_drm.h -- 2.18.0

2 years

3
13
0 0

[PATCH] dma-buf: heaps: Don't track CMA dma-buf pages under RssFile

by T.J. Mercier

DMA buffers allocated from the CMA dma-buf heap get counted under RssFile for processes that map them and trigger page faults. In addition to the incorrect accounting reported to userspace, reclaim behavior was influenced by the MM_FILEPAGES counter until linux 6.8, but this memory is not reclaimable. [1] Change the CMA dma-buf heap to set VM_PFNMAP on the VMA so MM does not poke at the memory managed by this dma-buf heap, and use vmf_insert_pfn to correct the RSS accounting. The system dma-buf heap does not suffer from this issue since remap_pfn_range is used during the mmap of the buffer, which also sets VM_PFNMAP on the VMA. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/m… Fixes: b61614ec318a ("dma-buf: heaps: Add CMA heap to dmabuf heaps") Signed-off-by: T.J. Mercier <tjmercier(a)google.com> --- drivers/dma-buf/heaps/cma_heap.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index ee899f8e6721..4a63567e93ba 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -168,10 +168,7 @@ static vm_fault_t cma_heap_vm_fault(struct vm_fault *vmf) if (vmf->pgoff > buffer->pagecount) return VM_FAULT_SIGBUS; - vmf->page = buffer->pages[vmf->pgoff]; - get_page(vmf->page); - - return 0; + return vmf_insert_pfn(vma, vmf->address, page_to_pfn(buffer->pages[vmf->pgoff])); } static const struct vm_operations_struct dma_heap_vm_ops = { @@ -185,6 +182,8 @@ static int cma_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) == 0) return -EINVAL; + vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP); + vma->vm_ops = &dma_heap_vm_ops; vma->vm_private_data = buffer; -- 2.43.0.381.gb435a96ce8-goog

2 years

4
5
0 0

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

by Paul Cercueil

Hi Jonathan, This is the v6 of my patchset that introduces a new interface based on DMABUF objects. The code was updated quite a bit, using the feedback on the list for this patchset but also the feedback I received on the FunctionFS patchset that I'm working on upstreaming in parallel [1] where the DMABUF handling code is very similar. See below for the full changelog. I decided to drop the scope-based memory management for dma_buf and I hope you are OK with that. Christian wants the patch(es) to support scope-based memory management in dma-buf as a separate patchset; once it's in, I will gladly send a follow-up patch to use __free() where it makes sense. For performance numbers, I'll point you to the cover letter for my v5 patchset [2]. This patchset was based on next-20240129. Cheers, -Paul [1] https://lore.kernel.org/all/20230322092118.9213-1-paul@crapouillou.net/ [2] https://lore.kernel.org/linux-iio/219abc43b4fdd4a13b307ed2efaa0e6869e68e3f.… --- Changelog: * [2/6]: - Use new prototype for axi_dmac_alloc_desc() as it changed upstream * [3/6]: - Remove dead code in iio_dma_resv_lock() - Fix non-block actually blocking - Cache dma_buf_attachment instead of mapping/unmapping it for every transfer - Return -EINVAL instead of IIO_IOCTL_UNHANDLED for unknown ioctl - Make .block_enqueue() callback take a dma_fence pointer, which will be passed to iio_buffer_signal_dmabuf_done() instead of the dma_buf_attachment; and remove the backpointer from the priv structure to the dma_fence. - Use dma_fence_begin/end_signalling in the dma_fence critical sections - Unref dma_fence and dma_buf_attachment in worker, because they might try to lock the dma_resv, which would deadlock. - Add buffer ops to lock/unlock the queue. This is motivated by the fact that once the dma_fence has been installed, we cannot lock anything anymore - so the queue must be locked before the dma_fence is installed. - Use 'long retl' variable to handle the return value of dma_resv_wait_timeout() - Protect dmabufs list access with a mutex - Rework iio_buffer_find_attachment() to use the internal dmabufs list, instead of messing with dmabufs private data. - Add an atomically-increasing sequence number for fences * [4/6]: - Update iio_dma_buffer_enqueue_dmabuf() to take a dma_fence pointer - Pass that dma_fence pointer along to iio_buffer_signal_dmabuf_done() - Add iio_dma_buffer_lock_queue() / iio_dma_buffer_unlock_queue() - Do not lock the queue in iio_dma_buffer_enqueue_dmabuf(). The caller will ensure that it has been locked already. - Replace "int += bool;" by "if (bool) int++;" - Use dma_fence_begin/end_signalling in the dma_fence critical sections - Use one "num_dmabufs" fields instead of one "num_blocks" and one "num_fileio_blocks". Make it an atomic_t, which makes it possible to decrement it atomically in iio_buffer_block_release() without having to lock the queue mutex; and in turn, it means that we don't need to use iio_buffer_block_put_atomic() everywhere to avoid locking the queue mutex twice. - Use cleanup.h guard(mutex) when possible - Explicitely list all states in the switch in iio_dma_can_enqueue_block() - Rename iio_dma_buffer_fileio_mode() to iio_dma_buffer_can_use_fileio(), and add a comment explaining why it cannot race vs. DMABUF. * [5/6]: - Populate .lock_queue / .unlock_queue callbacks - Switch to atomic memory allocations in .submit_queue, because of the dma_fence critical section - Make sure that the size of the scatterlist is enough --- Paul Cercueil (6): dmaengine: Add API function dmaengine_prep_slave_dma_vec() dmaengine: dma-axi-dmac: Implement device_prep_slave_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/dmabuf_api.rst | 54 ++ Documentation/iio/index.rst | 2 + drivers/dma/dma-axi-dmac.c | 40 ++ drivers/iio/buffer/industrialio-buffer-dma.c | 181 ++++++- .../buffer/industrialio-buffer-dmaengine.c | 58 ++- drivers/iio/industrialio-buffer.c | 462 ++++++++++++++++++ include/linux/dmaengine.h | 25 + include/linux/iio/buffer-dma.h | 31 ++ include/linux/iio/buffer_impl.h | 33 ++ include/uapi/linux/iio/buffer.h | 22 + 10 files changed, 891 insertions(+), 17 deletions(-) create mode 100644 Documentation/iio/dmabuf_api.rst -- 2.43.0

2 years

2
8
0 0

[PATCH v6 0/4] usb: gadget: functionfs: DMABUF import interface

by Paul Cercueil

Hi, This is the v6 of my patchset that adds a new DMABUF import interface to FunctionFS. Given that the cache coherency issue that has been discussed after my v5 is a tangential problem and not directly related to this new interface, I decided to drop the dma_buf_begin/end_access() functions for now - but I'm open to the idea of re-introducing them in a subsequent patchset. The patchset was rebased on next-20240129. Cheers, -Paul --- Changelog: * Drop v5's patches [1/6] and [2/6]. * [3/4]: - Drop use of dma_buf_begin/end_access(). We now make the assumption that the devices attached to the DMABUFs must be coherent between themselves. The cache coherency issue is a tangential problem, and those functions can be re-introduced in a subsequent patchset. - Unqueue pending requests on detach. Otherwise, when closing the data endpoint the DMABUF will never be signaled. - Use list_for_each_entry_safe() in ffs_dmabuf_detach(), because there is a list_del() in there. - use pr_vdebug() instead of pr_debug() - Rename ffs_dmabuf_unmap_work() -> ffs_dmabuf_cleanup() --- Paul Cercueil (4): usb: gadget: Support already-mapped DMA SGs usb: gadget: functionfs: Factorize wait-for-endpoint code usb: gadget: functionfs: Add DMABUF import interface Documentation: usb: Document FunctionFS DMABUF API Documentation/usb/functionfs.rst | 36 ++ drivers/usb/gadget/Kconfig | 1 + drivers/usb/gadget/function/f_fs.c | 513 ++++++++++++++++++++++++++-- drivers/usb/gadget/udc/core.c | 7 +- include/linux/usb/gadget.h | 2 + include/uapi/linux/usb/functionfs.h | 41 +++ 6 files changed, 579 insertions(+), 21 deletions(-) -- 2.43.0

2 years

1
4
0 0

[PATCH v5 0/8] iio: new DMABUF based API, v5

by Paul Cercueil

[V4 was: "iio: Add buffer write() support"][1] Hi Jonathan, This is a respin of the V3 of my patchset that introduced a new interface based on DMABUF objects [2]. The V4 was a split of the patchset, to attempt to upstream buffer write() support first. But since there is no current user upstream, it was not merged. This V5 is about doing the opposite, and contains the new DMABUF interface, without adding the buffer write() support. It can already be used with the upstream adi-axi-adc driver. In user-space, Libiio uses it to transfer back and forth blocks of samples between the hardware and the applications, without having to copy the data. On a ZCU102 with a FMComms3 daughter board, running Libiio from the pcercuei/dev-new-dmabuf-api branch [3], compiled with WITH_LOCAL_DMABUF_API=OFF (so that it uses fileio): sudo utils/iio_rwdev -b 4096 -B cf-ad9361-lpc Throughput: 116 MiB/s Same hardware, with the DMABUF API (WITH_LOCAL_DMABUF_API=ON): sudo utils/iio_rwdev -b 4096 -B cf-ad9361-lpc Throughput: 475 MiB/s This benchmark only measures the speed at which the data can be fetched to iio_rwdev's internal buffers, and does not actually try to read the data (e.g. to pipe it to stdout). It shows that fetching the data is more than 4x faster using the new interface. When actually reading the data, the performance difference isn't that impressive (maybe because in case of DMABUF the data is not in cache): WITH_LOCAL_DMABUF_API=OFF (so that it uses fileio): sudo utils/iio_rwdev -b 4096 cf-ad9361-lpc | dd of=/dev/zero status=progress 2446422528 bytes (2.4 GB, 2.3 GiB) copied, 22 s, 111 MB/s WITH_LOCAL_DMABUF_API=ON: sudo utils/iio_rwdev -b 4096 cf-ad9361-lpc | dd of=/dev/zero status=progress 2334388736 bytes (2.3 GB, 2.2 GiB) copied, 21 s, 114 MB/s One interesting thing to note is that fileio is (currently) actually faster than the DMABUF interface if you increase a lot the buffer size. My explanation is that the cache invalidation routine takes more and more time the bigger the DMABUF gets. This is because the DMABUF is backed by small-size pages, so a (e.g.) 64 MiB DMABUF is backed by up to 16 thousands pages, that have to be invalidated one by one. This can be addressed by using huge pages, but the udmabuf driver does not (yet) support creating DMABUFs backed by huge pages. Anyway, the real benefits happen when the DMABUFs are either shared between IIO devices, or between the IIO subsystem and another filesystem. In that case, the DMABUFs are simply passed around drivers, without the data being copied at any moment. We use that feature to transfer samples from our transceivers to USB, using a DMABUF interface to FunctionFS [4]. This drastically increases the throughput, to about 274 MiB/s over a USB3 link, vs. 127 MiB/s using IIO's fileio interface + write() to the FunctionFS endpoints, for a lower CPU usage (0.85 vs. 0.65 load avg.). Based on linux-next/next-20231219. Cheers, -Paul [1] https://lore.kernel.org/all/20230807112113.47157-1-paul@crapouillou.net/ [2] https://lore.kernel.org/all/20230403154800.215924-1-paul@crapouillou.net/ [3] https://github.com/analogdevicesinc/libiio/tree/pcercuei/dev-new-dmabuf-api [4] https://lore.kernel.org/all/20230322092118.9213-1-paul@crapouillou.net/ --- Changelog: - [3/8]: Replace V3's dmaengine_prep_slave_dma_array() with a new dmaengine_prep_slave_dma_vec(), which uses a new 'dma_vec' struct. Note that at some point we will need to support cyclic transfers using dmaengine_prep_slave_dma_vec(). Maybe with a new "flags" parameter to the function? - [4/8]: Implement .device_prep_slave_dma_vec() instead of V3's .device_prep_slave_dma_array(). @Vinod: this patch will cause a small conflict with my other patchset adding scatter-gather support to the axi-dmac driver. This patch adds a call to axi_dmac_alloc_desc(num_sgs), but the prototype of this function changed in my other patchset - it would have to be passed the "chan" variable. I don't know how you prefer it to be resolved. Worst case scenario (and if @Jonathan is okay with that) this one patch can be re-sent later, but it would make this patchset less "atomic". - [5/8]: - Use dev_err() instead of pr_err() - Inline to_iio_dma_fence() - Add comment to explain why we unref twice when detaching dmabuf - Remove TODO comment. It is actually safe to free the file's private data even when transfers are still pending because it won't be accessed. - Fix documentation of new fields in struct iio_buffer_access_funcs - iio_dma_resv_lock() does not need to be exported, make it static - [7/8]: - Use the new dmaengine_prep_slave_dma_vec(). - Restrict to input buffers, since output buffers are not yet supported by IIO buffers. - [8/8]: Use description lists for the documentation of the three new IOCTLs instead of abusing subsections. --- Alexandru Ardelean (1): iio: buffer-dma: split iio_dma_buffer_fileio_free() function Paul Cercueil (7): iio: buffer-dma: Get rid of outgoing queue dmaengine: Add API function dmaengine_prep_slave_dma_vec() dmaengine: dma-axi-dmac: Implement device_prep_slave_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/dmabuf_api.rst | 54 +++ Documentation/iio/index.rst | 2 + drivers/dma/dma-axi-dmac.c | 40 ++ drivers/iio/buffer/industrialio-buffer-dma.c | 242 ++++++++--- .../buffer/industrialio-buffer-dmaengine.c | 52 ++- drivers/iio/industrialio-buffer.c | 402 ++++++++++++++++++ include/linux/dmaengine.h | 25 ++ include/linux/iio/buffer-dma.h | 33 +- include/linux/iio/buffer_impl.h | 26 ++ include/uapi/linux/iio/buffer.h | 22 + 10 files changed, 836 insertions(+), 62 deletions(-) create mode 100644 Documentation/iio/dmabuf_api.rst -- 2.43.0

2 years

7
40
0 0

[PATCH 1/3] udmabuf: Keep track current device mappings

by Andrew Davis

When a device attaches to and maps our buffer we need to keep track of this mapping/device. This is needed for synchronization with these devices when beginning and ending CPU access for instance. Add a list that tracks device mappings as part of {map,unmap}_udmabuf(). Signed-off-by: Andrew Davis <afd(a)ti.com> --- drivers/dma-buf/udmabuf.c | 43 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c406459996489..3a23f0a7d112a 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -28,6 +28,14 @@ struct udmabuf { struct page **pages; struct sg_table *sg; struct miscdevice *device; + struct list_head attachments; + struct mutex lock; +}; + +struct udmabuf_attachment { + struct device *dev; + struct sg_table *table; + struct list_head list; }; static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) @@ -120,14 +128,42 @@ static void put_sg_table(struct device *dev, struct sg_table *sg, static struct sg_table *map_udmabuf(struct dma_buf_attachment *at, enum dma_data_direction direction) { - return get_sg_table(at->dev, at->dmabuf, direction); + struct udmabuf *ubuf = at->dmabuf->priv; + struct udmabuf_attachment *a; + + a = kzalloc(sizeof(*a), GFP_KERNEL); + if (!a) + return ERR_PTR(-ENOMEM); + + a->table = get_sg_table(at->dev, at->dmabuf, direction); + if (IS_ERR(a->table)) { + kfree(a); + return a->table; + } + + a->dev = at->dev; + + mutex_lock(&ubuf->lock); + list_add(&a->list, &ubuf->attachments); + mutex_unlock(&ubuf->lock); + + return a->table; } static void unmap_udmabuf(struct dma_buf_attachment *at, struct sg_table *sg, enum dma_data_direction direction) { - return put_sg_table(at->dev, sg, direction); + struct udmabuf_attachment *a = at->priv; + struct udmabuf *ubuf = at->dmabuf->priv; + + mutex_lock(&ubuf->lock); + list_del(&a->list); + mutex_unlock(&ubuf->lock); + + put_sg_table(at->dev, sg, direction); + + kfree(a); } static void release_udmabuf(struct dma_buf *buf) @@ -263,6 +299,9 @@ static long udmabuf_create(struct miscdevice *device, memfd = NULL; } + INIT_LIST_HEAD(&ubuf->attachments); + mutex_init(&ubuf->lock); + exp_info.ops = &udmabuf_ops; exp_info.size = ubuf->pagecount << PAGE_SHIFT; exp_info.priv = ubuf; -- 2.39.2

2 years

3
5
0 0

Re: [PATCH 2/3] udmabuf: Sync buffer mappings for attached devices

by Andrew Davis

On 1/26/24 1:25 AM, Kasireddy, Vivek wrote: >>>> Currently this driver creates a SGT table using the CPU as the >>>> target device, then performs the dma_sync operations against >>>> that SGT. This is backwards to how DMA-BUFs are supposed to behave. >>>> This may have worked for the case where these buffers were given >>>> only back to the same CPU that produced them as in the QEMU case. >>>> And only then because the original author had the dma_sync >>>> operations also backwards, syncing for the "device" on begin_cpu. >>>> This was noticed and "fixed" in this patch[0]. >>>> >>>> That then meant we were sync'ing from the CPU to the CPU using >>>> a pseudo-device "miscdevice". Which then caused another issue >>>> due to the miscdevice not having a proper DMA mask (and why should >>>> it, the CPU is not a DMA device). The fix for that was an even >>>> more egregious hack[1] that declares the CPU is coherent with >>>> itself and can access its own memory space.. >>>> >>>> Unwind all this and perform the correct action by doing the dma_sync >>>> operations for each device currently attached to the backing buffer. >>> Makes sense. >>> >>>> >>>> [0] commit 1ffe09590121 ("udmabuf: fix dma-buf cpu access") >>>> [1] commit 9e9fa6a9198b ("udmabuf: Set the DMA mask for the udmabuf >>>> device (v2)") >>>> >>>> Signed-off-by: Andrew Davis <afd(a)ti.com> >>>> --- >>>> drivers/dma-buf/udmabuf.c | 41 +++++++++++++++------------------------ >>>> 1 file changed, 16 insertions(+), 25 deletions(-) >>>> >>>> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >>>> index 3a23f0a7d112a..ab6764322523c 100644 >>>> --- a/drivers/dma-buf/udmabuf.c >>>> +++ b/drivers/dma-buf/udmabuf.c >>>> @@ -26,8 +26,6 @@ MODULE_PARM_DESC(size_limit_mb, "Max size of a >>>> dmabuf, in megabytes. Default is >>>> struct udmabuf { >>>> pgoff_t pagecount; >>>> struct page **pages; >>>> - struct sg_table *sg; >>>> - struct miscdevice *device; >>>> struct list_head attachments; >>>> struct mutex lock; >>>> }; >>>> @@ -169,12 +167,8 @@ static void unmap_udmabuf(struct >>>> dma_buf_attachment *at, >>>> static void release_udmabuf(struct dma_buf *buf) >>>> { >>>> struct udmabuf *ubuf = buf->priv; >>>> - struct device *dev = ubuf->device->this_device; >>>> pgoff_t pg; >>>> >>>> - if (ubuf->sg) >>>> - put_sg_table(dev, ubuf->sg, DMA_BIDIRECTIONAL); >>> What happens if the last importer maps the dmabuf but erroneously >>> closes it immediately? Would unmap somehow get called in this case? >>> >> >> Good question, had to scan the framework code a bit here. I thought >> closing a DMABUF handle would automatically unwind any current >> attachments/mappings, but it seems nothing in the framework does that. >> >> Looks like that is up to the importing drivers[0]: >> >>> Once a driver is done with a shared buffer it needs to call >>> dma_buf_detach() (after cleaning up any mappings) and then >>> release the reference acquired with dma_buf_get() by >>> calling dma_buf_put(). >> >> So closing a DMABUF after mapping without first unmapping it would >> be a bug in the importer, it is not the exporters problem to check > It may be a bug in the importer but wouldn't the memory associated > with the sg table and attachment get leaked if unmap doesn't get called > in this scenario? > Yes the attachment data would be leaked if unattach was not called, but that is true for all DMABUF exporters. The .release() callback is meant to be the mirror of the export function and it only cleans up that. Same for attach/unattach, map/unmap, etc.. If these calls are not balanced then yes they can leak memory. Since balance is guaranteed by the API, checking the balance should be done at that level, not in each and every exporter. If your comment is that we should add those checks into the DMABUF framework layer then I would agree. Andrew > Thanks, > Vivek > >> for (although some more warnings in the framework checking for that >> might not be a bad idea..). >> >> Andrew >> >> [0] https://www.kernel.org/doc/html/v6.7/driver-api/dma-buf.html >> >>> Thanks, >>> Vivek >>> >>>> - >>>> for (pg = 0; pg < ubuf->pagecount; pg++) >>>> put_page(ubuf->pages[pg]); >>>> kfree(ubuf->pages); >>>> @@ -185,33 +179,31 @@ static int begin_cpu_udmabuf(struct dma_buf >>>> *buf, >>>> enum dma_data_direction direction) >>>> { >>>> struct udmabuf *ubuf = buf->priv; >>>> - struct device *dev = ubuf->device->this_device; >>>> - int ret = 0; >>>> - >>>> - if (!ubuf->sg) { >>>> - ubuf->sg = get_sg_table(dev, buf, direction); >>>> - if (IS_ERR(ubuf->sg)) { >>>> - ret = PTR_ERR(ubuf->sg); >>>> - ubuf->sg = NULL; >>>> - } >>>> - } else { >>>> - dma_sync_sg_for_cpu(dev, ubuf->sg->sgl, ubuf->sg->nents, >>>> - direction); >>>> - } >>>> + struct udmabuf_attachment *a; >>>> >>>> - return ret; >>>> + mutex_lock(&ubuf->lock); >>>> + >>>> + list_for_each_entry(a, &ubuf->attachments, list) >>>> + dma_sync_sgtable_for_cpu(a->dev, a->table, direction); >>>> + >>>> + mutex_unlock(&ubuf->lock); >>>> + >>>> + return 0; >>>> } >>>> >>>> static int end_cpu_udmabuf(struct dma_buf *buf, >>>> enum dma_data_direction direction) >>>> { >>>> struct udmabuf *ubuf = buf->priv; >>>> - struct device *dev = ubuf->device->this_device; >>>> + struct udmabuf_attachment *a; >>>> >>>> - if (!ubuf->sg) >>>> - return -EINVAL; >>>> + mutex_lock(&ubuf->lock); >>>> + >>>> + list_for_each_entry(a, &ubuf->attachments, list) >>>> + dma_sync_sgtable_for_device(a->dev, a->table, direction); >>>> + >>>> + mutex_unlock(&ubuf->lock); >>>> >>>> - dma_sync_sg_for_device(dev, ubuf->sg->sgl, ubuf->sg->nents, >>>> direction); >>>> return 0; >>>> } >>>> >>>> @@ -307,7 +299,6 @@ static long udmabuf_create(struct miscdevice >>>> *device, >>>> exp_info.priv = ubuf; >>>> exp_info.flags = O_RDWR; >>>> >>>> - ubuf->device = device; >>>> buf = dma_buf_export(&exp_info); >>>> if (IS_ERR(buf)) { >>>> ret = PTR_ERR(buf); >>>> -- >>>> 2.39.2 >>>

2 years

1
0
0 0

Re: [PATCH 2/3] udmabuf: Sync buffer mappings for attached devices

by Andrew Davis

On 1/24/24 5:05 PM, Kasireddy, Vivek wrote: > Hi Andrew, > >> Currently this driver creates a SGT table using the CPU as the >> target device, then performs the dma_sync operations against >> that SGT. This is backwards to how DMA-BUFs are supposed to behave. >> This may have worked for the case where these buffers were given >> only back to the same CPU that produced them as in the QEMU case. >> And only then because the original author had the dma_sync >> operations also backwards, syncing for the "device" on begin_cpu. >> This was noticed and "fixed" in this patch[0]. >> >> That then meant we were sync'ing from the CPU to the CPU using >> a pseudo-device "miscdevice". Which then caused another issue >> due to the miscdevice not having a proper DMA mask (and why should >> it, the CPU is not a DMA device). The fix for that was an even >> more egregious hack[1] that declares the CPU is coherent with >> itself and can access its own memory space.. >> >> Unwind all this and perform the correct action by doing the dma_sync >> operations for each device currently attached to the backing buffer. > Makes sense. > >> >> [0] commit 1ffe09590121 ("udmabuf: fix dma-buf cpu access") >> [1] commit 9e9fa6a9198b ("udmabuf: Set the DMA mask for the udmabuf >> device (v2)") >> >> Signed-off-by: Andrew Davis <afd(a)ti.com> >> --- >> drivers/dma-buf/udmabuf.c | 41 +++++++++++++++------------------------ >> 1 file changed, 16 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >> index 3a23f0a7d112a..ab6764322523c 100644 >> --- a/drivers/dma-buf/udmabuf.c >> +++ b/drivers/dma-buf/udmabuf.c >> @@ -26,8 +26,6 @@ MODULE_PARM_DESC(size_limit_mb, "Max size of a >> dmabuf, in megabytes. Default is >> struct udmabuf { >> pgoff_t pagecount; >> struct page **pages; >> - struct sg_table *sg; >> - struct miscdevice *device; >> struct list_head attachments; >> struct mutex lock; >> }; >> @@ -169,12 +167,8 @@ static void unmap_udmabuf(struct >> dma_buf_attachment *at, >> static void release_udmabuf(struct dma_buf *buf) >> { >> struct udmabuf *ubuf = buf->priv; >> - struct device *dev = ubuf->device->this_device; >> pgoff_t pg; >> >> - if (ubuf->sg) >> - put_sg_table(dev, ubuf->sg, DMA_BIDIRECTIONAL); > What happens if the last importer maps the dmabuf but erroneously > closes it immediately? Would unmap somehow get called in this case? > Good question, had to scan the framework code a bit here. I thought closing a DMABUF handle would automatically unwind any current attachments/mappings, but it seems nothing in the framework does that. Looks like that is up to the importing drivers[0]: > Once a driver is done with a shared buffer it needs to call > dma_buf_detach() (after cleaning up any mappings) and then > release the reference acquired with dma_buf_get() by > calling dma_buf_put(). So closing a DMABUF after mapping without first unmapping it would be a bug in the importer, it is not the exporters problem to check for (although some more warnings in the framework checking for that might not be a bad idea..). Andrew [0] https://www.kernel.org/doc/html/v6.7/driver-api/dma-buf.html > Thanks, > Vivek > >> - >> for (pg = 0; pg < ubuf->pagecount; pg++) >> put_page(ubuf->pages[pg]); >> kfree(ubuf->pages); >> @@ -185,33 +179,31 @@ static int begin_cpu_udmabuf(struct dma_buf >> *buf, >> enum dma_data_direction direction) >> { >> struct udmabuf *ubuf = buf->priv; >> - struct device *dev = ubuf->device->this_device; >> - int ret = 0; >> - >> - if (!ubuf->sg) { >> - ubuf->sg = get_sg_table(dev, buf, direction); >> - if (IS_ERR(ubuf->sg)) { >> - ret = PTR_ERR(ubuf->sg); >> - ubuf->sg = NULL; >> - } >> - } else { >> - dma_sync_sg_for_cpu(dev, ubuf->sg->sgl, ubuf->sg->nents, >> - direction); >> - } >> + struct udmabuf_attachment *a; >> >> - return ret; >> + mutex_lock(&ubuf->lock); >> + >> + list_for_each_entry(a, &ubuf->attachments, list) >> + dma_sync_sgtable_for_cpu(a->dev, a->table, direction); >> + >> + mutex_unlock(&ubuf->lock); >> + >> + return 0; >> } >> >> static int end_cpu_udmabuf(struct dma_buf *buf, >> enum dma_data_direction direction) >> { >> struct udmabuf *ubuf = buf->priv; >> - struct device *dev = ubuf->device->this_device; >> + struct udmabuf_attachment *a; >> >> - if (!ubuf->sg) >> - return -EINVAL; >> + mutex_lock(&ubuf->lock); >> + >> + list_for_each_entry(a, &ubuf->attachments, list) >> + dma_sync_sgtable_for_device(a->dev, a->table, direction); >> + >> + mutex_unlock(&ubuf->lock); >> >> - dma_sync_sg_for_device(dev, ubuf->sg->sgl, ubuf->sg->nents, >> direction); >> return 0; >> } >> >> @@ -307,7 +299,6 @@ static long udmabuf_create(struct miscdevice >> *device, >> exp_info.priv = ubuf; >> exp_info.flags = O_RDWR; >> >> - ubuf->device = device; >> buf = dma_buf_export(&exp_info); >> if (IS_ERR(buf)) { >> ret = PTR_ERR(buf); >> -- >> 2.39.2 >

2 years

1
0
0 0

Re: [PATCH 1/3] udmabuf: Keep track current device mappings

by Andrew Davis

On 1/24/24 4:36 PM, Kasireddy, Vivek wrote: > Hi Andrew, > >> When a device attaches to and maps our buffer we need to keep track >> of this mapping/device. This is needed for synchronization with these >> devices when beginning and ending CPU access for instance. Add a list >> that tracks device mappings as part of {map,unmap}_udmabuf(). >> >> Signed-off-by: Andrew Davis <afd(a)ti.com> >> --- >> drivers/dma-buf/udmabuf.c | 43 >> +++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 41 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >> index c406459996489..3a23f0a7d112a 100644 >> --- a/drivers/dma-buf/udmabuf.c >> +++ b/drivers/dma-buf/udmabuf.c >> @@ -28,6 +28,14 @@ struct udmabuf { >> struct page **pages; >> struct sg_table *sg; >> struct miscdevice *device; >> + struct list_head attachments; >> + struct mutex lock; >> +}; >> + >> +struct udmabuf_attachment { >> + struct device *dev; >> + struct sg_table *table; >> + struct list_head list; >> }; >> >> static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) >> @@ -120,14 +128,42 @@ static void put_sg_table(struct device *dev, struct >> sg_table *sg, >> static struct sg_table *map_udmabuf(struct dma_buf_attachment *at, >> enum dma_data_direction direction) >> { >> - return get_sg_table(at->dev, at->dmabuf, direction); >> + struct udmabuf *ubuf = at->dmabuf->priv; >> + struct udmabuf_attachment *a; >> + >> + a = kzalloc(sizeof(*a), GFP_KERNEL); >> + if (!a) >> + return ERR_PTR(-ENOMEM); >> + >> + a->table = get_sg_table(at->dev, at->dmabuf, direction); >> + if (IS_ERR(a->table)) { >> + kfree(a); >> + return a->table; > Isn't that a use-after-free bug? Indeed it is, will fix. Seems coccicheck also caught this but I missed it when reviewing its output, my bad :( Andrew > Rest of the patch lgtm. > > Thanks, > Vivek > >> + } >> + >> + a->dev = at->dev; >> + >> + mutex_lock(&ubuf->lock); >> + list_add(&a->list, &ubuf->attachments); >> + mutex_unlock(&ubuf->lock); >> + >> + return a->table; >> } >> >> static void unmap_udmabuf(struct dma_buf_attachment *at, >> struct sg_table *sg, >> enum dma_data_direction direction) >> { >> - return put_sg_table(at->dev, sg, direction); >> + struct udmabuf_attachment *a = at->priv; >> + struct udmabuf *ubuf = at->dmabuf->priv; >> + >> + mutex_lock(&ubuf->lock); >> + list_del(&a->list); >> + mutex_unlock(&ubuf->lock); >> + >> + put_sg_table(at->dev, sg, direction); >> + >> + kfree(a); >> } >> >> static void release_udmabuf(struct dma_buf *buf) >> @@ -263,6 +299,9 @@ static long udmabuf_create(struct miscdevice >> *device, >> memfd = NULL; >> } >> >> + INIT_LIST_HEAD(&ubuf->attachments); >> + mutex_init(&ubuf->lock); >> + >> exp_info.ops = &udmabuf_ops; >> exp_info.size = ubuf->pagecount << PAGE_SHIFT; >> exp_info.priv = ubuf; >> -- >> 2.39.2 >

2 years

1
0
0 0

[PATCH v4 0/3] Add mediatek,gce-props.yaml for other bindings reference

by Jason-JH.Lin

From: Jason-jh Lin <jason-jh.lin(a)mediatek.corp-partner.google.com> The property "mediatek,gce-events" is used for GCE event ID corresponding to a hardware event signal sent by the hardware or a software driver. If the mailbox providers or consumers want to manipulate the value of the event ID, they need to know the specific event ID. Since mediatek,gce-events property is used for both mailbox producers and consumers, we add a mediatek,gce-props.yaml to place the common GCE properties like mediatek,gce-events. Change in v4: 1. Fix some typo. 2. Change maxItems of gce-events from 1024 to 32. Change in v3: 1. Add more description and fix typo and grammar. 2. Fix $ref as full path. Change in v2: 1. Add mediatek,gce-props.yaml for other binding reference. Jason-JH.Lin (3): dt-bindings: mailbox: Add mediatek,gce-props.yaml dt-bindings: media: mediatek: mdp: Change mediatek,gce-events to reference dt-bindings: soc: mediatek: Change mediatek,gce-events to refernece .../bindings/mailbox/mediatek,gce-props.yaml | 52 +++++++++++++++++++ .../bindings/media/mediatek,mdp3-rdma.yaml | 11 ++-- .../bindings/media/mediatek,mdp3-rsz.yaml | 12 ++--- .../bindings/media/mediatek,mdp3-wrot.yaml | 12 ++--- .../bindings/soc/mediatek/mediatek,ccorr.yaml | 12 ++--- .../bindings/soc/mediatek/mediatek,mutex.yaml | 11 ++-- .../bindings/soc/mediatek/mediatek,wdma.yaml | 12 ++--- 7 files changed, 74 insertions(+), 48 deletions(-) create mode 100644 Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml -- 2.18.0

2 years

2
6
0 0

[bug report] drm/amdkfd: Export DMABufs from KFD using GEM handles

by Dan Carpenter

Hello Felix Kuehling, The patch 1819200166ce: "drm/amdkfd: Export DMABufs from KFD using GEM handles" from Aug 24, 2023 (linux-next), leads to the following Smatch static checker warning: drivers/dma-buf/dma-buf.c:729 dma_buf_get() warn: fd used after fd_install() 'fd' drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 809 static int kfd_mem_export_dmabuf(struct kgd_mem *mem) 810 { 811 if (!mem->dmabuf) { 812 struct amdgpu_device *bo_adev; 813 struct dma_buf *dmabuf; 814 int r, fd; 815 816 bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev); 817 r = drm_gem_prime_handle_to_fd(&bo_adev->ddev, bo_adev->kfd.client.file, 818 mem->gem_handle, 819 mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ? 820 DRM_RDWR : 0, &fd); ^^^ The drm_gem_prime_handle_to_fd() function does an fd_install() and returns the result as "fd". 821 if (r) 822 return r; 823 dmabuf = dma_buf_get(fd); ^^ Then we do another fget() inside dma_buf_get(). I'm not an expert, but this looks wrong. We can't assume that the dmabuf here is the same one from drm_gem_prime_handle_to_fd() because the user could change it after the fd_install(). I suspect drm_gem_prime_handle_to_fd() should pass the dmabuf back instead. We had several CVEs similar to this such as CVE-2022-1998. 824 close_fd(fd); 825 if (WARN_ON_ONCE(IS_ERR(dmabuf))) 826 return PTR_ERR(dmabuf); 827 mem->dmabuf = dmabuf; 828 } 829 830 return 0; 831 } regards, dan carpenter

2 years

2
1
0 0

[PATCH v3 0/3] Add mediatek,gce-props.yaml for other bindings reference

by Jason-JH.Lin

From: Jason-jh Lin <jason-jh.lin(a)mediatek.corp-partner.google.com> The property "mediatek,gce-events" is used for GCE event ID corresponding to a hardware event signal sent by the hardware or a sofware driver. If the mailbox providers or consumers want to manipulate the value of the event ID, they need to know the specific event ID. Since mediatek,gce-events property is used for both mailbox producers and consumers, we add a mediatek,gce-props.yaml to place the common GCE properties like mediatek,gce-events. Change in v3: 1. Add more description and fix typo and grammar. 2. Fix $ref as full path. Change in v2: 1. Add mediatek,gce-props.yaml for other binding reference. Jason-JH.Lin (3): dt-bindings: mailbox: Add mediatek,gce-props.yaml dt-bindings: media: mediatek: mdp: Change mediatek,gce-events to reference dt-bindings: soc: mediatek: Change mediatek,gce-events to refernece .../bindings/mailbox/mediatek,gce-props.yaml | 52 +++++++++++++++++++ .../bindings/media/mediatek,mdp3-rdma.yaml | 11 ++-- .../bindings/media/mediatek,mdp3-rsz.yaml | 12 ++--- .../bindings/media/mediatek,mdp3-wrot.yaml | 12 ++--- .../bindings/soc/mediatek/mediatek,ccorr.yaml | 12 ++--- .../bindings/soc/mediatek/mediatek,mutex.yaml | 11 ++-- .../bindings/soc/mediatek/mediatek,wdma.yaml | 12 ++--- 7 files changed, 74 insertions(+), 48 deletions(-) create mode 100644 Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml -- 2.18.0

2 years

3
9
0 0

[PATCH 15/82] dma-buf: Refactor intentional wrap-around calculation

by Kees Cook

In an effort to separate intentional arithmetic wrap-around from unexpected wrap-around, we need to refactor places that depend on this kind of math. One of the most common code patterns of this is: VAR + value < VAR Notably, this is considered "undefined behavior" for signed and pointer types, which the kernel works around by using the -fno-strict-overflow option in the build[1] (which used to just be -fwrapv). Regardless, we want to get the kernel source to the position where we can meaningfully instrument arithmetic wrap-around conditions and catch them when they are unexpected, regardless of whether they are signed[2], unsigned[3], or pointer[4] types. Refactor open-coded unsigned wrap-around addition test to use check_add_overflow(), retaining the result for later usage (which removes the redundant open-coded addition). This paves the way to enabling the wrap-around sanitizers in the future. Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] Link: https://github.com/KSPP/linux/issues/26 [2] Link: https://github.com/KSPP/linux/issues/27 [3] Link: https://github.com/KSPP/linux/issues/344 [4] Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Christian König <christian.koenig(a)amd.com> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Signed-off-by: Kees Cook <keescook(a)chromium.org> --- drivers/dma-buf/dma-buf.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 8fe5aa67b167..3743c63a9b59 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1458,6 +1458,8 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_end_cpu_access, DMA_BUF); int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, unsigned long pgoff) { + unsigned long sum; + if (WARN_ON(!dmabuf || !vma)) return -EINVAL; @@ -1466,12 +1468,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, return -EINVAL; /* check for offset overflow */ - if (pgoff + vma_pages(vma) < pgoff) + if (check_add_overflow(pgoff, vma_pages(vma), &sum)) return -EOVERFLOW; /* check for overflowing the buffer's size */ - if (pgoff + vma_pages(vma) > - dmabuf->size >> PAGE_SHIFT) + if (sum > dmabuf->size >> PAGE_SHIFT) return -EINVAL; /* readjust the vma */ -- 2.34.1

2 years

1
0
0 0

Re: [PATCH v4 1/4] usb: gadget: Support already-mapped DMA SGs

by Paul Cercueil

Hi Frank, Le vendredi 19 janvier 2024 à 16:49 -0500, Frank Li a écrit : > On Wed, Jan 17, 2024 at 01:26:43PM +0100, Paul Cercueil wrote: > > Add a new 'sg_was_mapped' field to the struct usb_request. This > > field > > can be used to indicate that the scatterlist associated to the USB > > transfer has already been mapped into the DMA space, and it does > > not > > have to be done internally. > > > > Signed-off-by: Paul Cercueil <paul(a)crapouillou.net> > > --- > > drivers/usb/gadget/udc/core.c | 7 ++++++- > > include/linux/usb/gadget.h | 2 ++ > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/gadget/udc/core.c > > b/drivers/usb/gadget/udc/core.c > > index d59f94464b87..9d4150124fdb 100644 > > --- a/drivers/usb/gadget/udc/core.c > > +++ b/drivers/usb/gadget/udc/core.c > > @@ -903,6 +903,11 @@ int usb_gadget_map_request_by_dev(struct > > device *dev, > > if (req->length == 0) > > return 0; > > > > + if (req->sg_was_mapped) { > > + req->num_mapped_sgs = req->num_sgs; > > + return 0; > > + } > > + > > if (req->num_sgs) { > > int mapped; > > > > @@ -948,7 +953,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_map_request); > > void usb_gadget_unmap_request_by_dev(struct device *dev, > > struct usb_request *req, int is_in) > > { > > - if (req->length == 0) > > + if (req->length == 0 || req->sg_was_mapped) > > return; > > > > if (req->num_mapped_sgs) { > > diff --git a/include/linux/usb/gadget.h > > b/include/linux/usb/gadget.h > > index a771ccc038ac..c529e4e06997 100644 > > --- a/include/linux/usb/gadget.h > > +++ b/include/linux/usb/gadget.h > > @@ -52,6 +52,7 @@ struct usb_ep; > > * @short_not_ok: When reading data, makes short packets be > > * treated as errors (queue stops advancing till cleanup). > > * @dma_mapped: Indicates if request has been mapped to DMA > > (internal) > > + * @sg_was_mapped: Set if the scatterlist has been mapped before > > the request > > * @complete: Function called when request completes, so this > > request and > > * its buffer may be re-used. The function will always be > > called with > > * interrupts disabled, and it must not sleep. > > @@ -111,6 +112,7 @@ struct usb_request { > > unsigned zero:1; > > unsigned short_not_ok:1; > > unsigned dma_mapped:1; > > + unsigned sg_was_mapped:1; > > why not use dma_mapped direclty? Because of the unmap case. We want to know whether we should unmap or not. > > Frank Cheers, -Paul > > > > > void (*complete)(struct usb_ep *ep, > > struct usb_request *req); > > -- > > 2.43.0 > >

2 years

1
0
0 0

[PATCH v4 0/4] usb: gadget: functionfs: DMABUF import interface

by Paul Cercueil

Hi, This is the v4 of my patchset that adds a new DMABUF import interface to FunctionFS. It addresses the points that Daniel raised on the v3 - see changelog below. This interface is being used at Analog Devices, to transfer data from high-speed transceivers to USB in a zero-copy fashion, using also the DMABUF import interface to the IIO subsystem which is being upstreamed in parallel [1]. The two are used by the Libiio software [2]. On a ZCU102 board with a FMComms3 daughter board, using the combination of these two new interfaces yields a drastic improvement of the throughput, from about 127 MiB/s using IIO's buffer read/write interface + read/write to the FunctionFS endpoints, to about 274 MiB/s when passing around DMABUFs, for a lower CPU usage (0.85 load avg. before, vs. 0.65 after). Right now, *technically* there are no users of this interface, as Analog Devices wants to wait until both interfaces are accepted upstream to merge the DMABUF code in Libiio into the main branch, and Jonathan wants to wait and see if this patchset is accepted to greenlight the DMABUF interface in IIO as well. I think this isn't really a problem; once everybody is happy with its part of the cake, we can merge them all at once. This is obviously for 5.9, and based on next-20240117. Changelog: - [3/4]: - Protect the dmabufs list with a mutex - Use incremental sequence number for the dma_fences - Unref attachments and DMABUFs in workers - Remove dead code in ffs_dma_resv_lock() - Fix non-block actually blocking - Use dma_fence_begin/end_signalling() - Add comment about cache-management and dma_buf_unmap_attachment() - Make sure dma_buf_map_attachment() is called with the dma-resv locked Cheers, -Paul [1] https://lore.kernel.org/linux-iio/219abc43b4fdd4a13b307ed2efaa0e6869e68e3f.… [2] https://github.com/analogdevicesinc/libiio/tree/pcercuei/dev-new-dmabuf-api Paul Cercueil (4): usb: gadget: Support already-mapped DMA SGs usb: gadget: functionfs: Factorize wait-for-endpoint code usb: gadget: functionfs: Add DMABUF import interface Documentation: usb: Document FunctionFS DMABUF API Documentation/usb/functionfs.rst | 36 ++ drivers/usb/gadget/function/f_fs.c | 500 ++++++++++++++++++++++++++-- drivers/usb/gadget/udc/core.c | 7 +- include/linux/usb/gadget.h | 2 + include/uapi/linux/usb/functionfs.h | 41 +++ 5 files changed, 565 insertions(+), 21 deletions(-) -- 2.43.0

2 years

3
7
0 0

[PATCH v3 0/4] usb: gadget: functionfs: DMABUF import interface

by Paul Cercueil

Hi, This small patchset adds three new IOCTLs that can be used to attach, detach, or transfer from/to a DMABUF object. This interface is being used at Analog Devices, to transfer data from high-speed transceivers to USB in a zero-copy fashion, using also the DMABUF import interface to the IIO subsystem which is being upstreamed in parallel [1] (and has high chances to be accepted for 5.9, I think Jonathan can confirm). The two are used by the Libiio software [2]. On a ZCU102 board with a FMComms3 daughter board, using the combination of these two new interfaces yields a drastic improvement of the throughput, from about 127 MiB/s using IIO's buffer read/write interface + read/write to the FunctionFS endpoints, to about 274 MiB/s when passing around DMABUFs, for a lower CPU usage (0.85 load avg. before, vs. 0.65 after). Right now, *technically* there are no users of this interface, as Analog Devices wants to wait until both interfaces are accepted upstream to merge the DMABUF code in Libiio into the main branch, and Jonathan wants to wait and see if this patchset is accepted to greenlight the DMABUF interface in IIO as well. I think this isn't really a problem; once everybody is happy with its part of the cake, we can merge them all at once. This is obviously for 5.9, and based on next-20240108. Changelog: - [3/4]: - Inline to_ffs_dma_fence() which was called only once. - Simplify ffs_dma_resv_lock() - Add comment explaining why we unref twice in ffs_dmabuf_detach() - Document uapi struct usb_ffs_dmabuf_transfer_req and IOCTLs - [4/4]: New patch, as I figured out having documentation wouldn't hurt. Cheers, -Paul [1] https://lore.kernel.org/linux-iio/219abc43b4fdd4a13b307ed2efaa0e6869e68e3f.… [2] https://github.com/analogdevicesinc/libiio/tree/pcercuei/dev-new-dmabuf-api Paul Cercueil (4): usb: gadget: Support already-mapped DMA SGs usb: gadget: functionfs: Factorize wait-for-endpoint code usb: gadget: functionfs: Add DMABUF import interface Documentation: usb: Document FunctionFS DMABUF API Documentation/usb/functionfs.rst | 36 +++ drivers/usb/gadget/function/f_fs.c | 463 ++++++++++++++++++++++++++-- drivers/usb/gadget/udc/core.c | 7 +- include/linux/usb/gadget.h | 2 + include/uapi/linux/usb/functionfs.h | 41 +++ 5 files changed, 528 insertions(+), 21 deletions(-) -- 2.43.0

2 years

3
17
0 0

Re: [PATCH v3 4/4] Documentation: usb: Document FunctionFS DMABUF API

by Paul Cercueil

Hi Vegard, Le mardi 09 janvier 2024 à 14:08 +0100, Vegard Nossum a écrit : > On 08/01/2024 13:00, Paul Cercueil wrote: > > Add documentation for the three ioctls used to attach or detach > > externally-created DMABUFs, and to request transfers from/to > > previously > > attached DMABUFs. > > > > Signed-off-by: Paul Cercueil <paul(a)crapouillou.net> > > > > --- > > v3: New patch > > --- > > Documentation/usb/functionfs.rst | 36 > > ++++++++++++++++++++++++++++++++ > > 1 file changed, 36 insertions(+) > > Hi, > > I'd like to point out that this file (usb/functionfs.rst) is > currently > included by Documentation/subsystem-apis.rst, the top-level file for > the > "Kernel subsystem documentation" set of books, which describe > internal > APIs: "These books get into the details of how specific kernel > subsystems work from the point of view of a kernel developer". > > However, functionfs.rst (and especially your new additions) are > documenting a userspace API, so it really belongs somewhere in > Documentation/userspace-api/ -- that's where /proc, /sys, /dev and > ioctl > descriptions for userspace programmers belong. Agreed. Even the original content prior to my additions describe a userspace API. > > I'm not NAKing the patch -- I just want to draw attention to this > discrepancy. Maybe we can separate the kernel-implementation details > (stuff about __init sections and stuff) from the new ioctl() info? > > Looking at <https://docs.kernel.org/usb/> I see that there are many > other adjacent documents that are also not really documenting kernel > implementation details, rough categorization as follows: > > USB support > ----------- > > - Linux ACM driver v0.16 ==> admin/user info > - Authorizing (or not) your USB devices to connect to the system ==> > admin/user info > - ChipIdea Highspeed Dual Role Controller Driver => admin/user info > - DWC3 driver ==> driver TODOs (can be moved into source code?) > - EHCI driver ==> technical info + driver details > - How FunctionFS works > - Linux USB gadget configured through configfs ==> userspace API + > implementation > - Linux USB HID gadget driver ==> implementation + userspace API > - Multifunction Composite Gadget ==> technical + user info > - Linux USB Printer Gadget Driver ==> userspace API > - Linux Gadget Serial Driver v2.0 ==> user/admin + userspace API > - Linux UVC Gadget Driver ==> user/admin + userspace API > - Gadget Testing ==> user/admin + userspace API > - Infinity Usb Unlimited Readme ==> user/admin > - Mass Storage Gadget (MSG) ==> user/admin > - USB 7-Segment Numeric Display ==> user/admin > - mtouchusb driver ==> user/admin > - OHCI ==> technical info > - USB Raw Gadget ==> userspace API > - USB/IP protocol ==> technical info > - usbmon ==> user/admin + userspace API > - USB serial ==> user/admin + technical info > - USB references > - Linux CDC ACM inf > - Linux inf > - USB devfs drop permissions source > - Credits > > By "admin/user info", I mean things that a user would have to do or > run > (e.g. modprobe + flags) to make use of a driver; "technical info" is > more like device specifications (transfer speeds, modes of operation, > etc.); "userspace API" is stuff like configfs and ioctls; "driver > details" is really implementation details and internal > considerations. > > The last ones I don't even really know how to categorize. > > I'm guessing nobody is really enthralled by the idea of splitting > Documentation/usb/ up like this? > > Documentation/admin-guide/usb/ > Documentation/driver-api/usb/ (this one actually exists already) > Documentation/userspace-api/usb/ > > For the stuff that is _actually_ internal to a specific driver (so > not > useful for end users, not useful for admins, not generic USB info, > and > not useful for userspace programmers), I would honestly propose to > just > move it directly into the driver's source code, or, if the text is > obsolete, just get rid of it completely. > > The distinction between user/admin and userspace API is pretty clear > (one is for end users, the other is for userspace _programmers_), but > it > can sometimes be hard to determine whether something falls in one or > the > other category. > > In any case -- it looks like almost all of the usb/ directory does > not > document "how specific kernel subsystems work from the point of view > of > a kernel developer" so maybe we should just move the include to > userspace-api/ for now as an obvious improvement (if still not 100% > correct): > > diff --git a/Documentation/subsystem-apis.rst > b/Documentation/subsystem-apis.rst > index 2d353fb8ea26..fe972f57bf4c 100644 > --- a/Documentation/subsystem-apis.rst > +++ b/Documentation/subsystem-apis.rst > @@ -81,7 +81,6 @@ Storage interfaces > security/index > crypto/index > bpf/index > - usb/index > PCI/index > misc-devices/index > peci/index > diff --git a/Documentation/userspace-api/index.rst > b/Documentation/userspace-api/index.rst > index 82f9dbd228f5..e60cd9174ada 100644 > --- a/Documentation/userspace-api/index.rst > +++ b/Documentation/userspace-api/index.rst > @@ -41,6 +41,7 @@ Subsystem-specific documentation: > tee > isapnp > dcdbas > + ../usb/index > > Kernel ABIs: These documents describe the the ABI between the Linux > kernel and userspace, and the relative stability of these > interfaces. > > > Thoughts? Makes sense to me. There's definitely some cleanup to be done in the USB documentation. > Vegard Cheers, -Paul

2 years

1
0
0 0

[PATCH] dma-buf/dma-resv: fix spelling

by Randy Dunlap

Fix spelling mistakes as reported by codespell. Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: Christian König <christian.koenig(a)amd.com> Cc: linaro-mm-sig(a)lists.linaro.org --- drivers/dma-buf/dma-resv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff -- a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -405,7 +405,7 @@ static void dma_resv_iter_walk_unlocked( * * Beware that the iterator can be restarted. Code which accumulates statistics * or similar needs to check for this with dma_resv_iter_is_restarted(). For - * this reason prefer the locked dma_resv_iter_first() whenver possible. + * this reason prefer the locked dma_resv_iter_first() whenever possible. * * Returns the first fence from an unlocked dma_resv obj. */ @@ -428,7 +428,7 @@ EXPORT_SYMBOL(dma_resv_iter_first_unlock * * Beware that the iterator can be restarted. Code which accumulates statistics * or similar needs to check for this with dma_resv_iter_is_restarted(). For - * this reason prefer the locked dma_resv_iter_next() whenver possible. + * this reason prefer the locked dma_resv_iter_next() whenever possible. * * Returns the next fence from an unlocked dma_resv obj. */

2 years

3
2
0 0

[PATCH] dma-buf/dma-fence: fix spelling

by Randy Dunlap

Fix spelling mistakes as reported by codespell. Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Gustavo Padovan <gustavo(a)padovan.org> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: Christian König <christian.koenig(a)amd.com> Cc: linaro-mm-sig(a)lists.linaro.org --- drivers/dma-buf/dma-fence.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff -- a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -102,7 +102,7 @@ static atomic64_t dma_fence_context_coun * * * Drivers are allowed to call dma_fence_wait() from their &mmu_notifier * respectively &mmu_interval_notifier callbacks. This means any code required - * for fence completeion cannot allocate memory with GFP_NOFS or GFP_NOIO. + * for fence completion cannot allocate memory with GFP_NOFS or GFP_NOIO. * Only GFP_ATOMIC is permissible, which might fail. * * Note that only GPU drivers have a reasonable excuse for both requiring @@ -522,7 +522,7 @@ dma_fence_wait_timeout(struct dma_fence EXPORT_SYMBOL(dma_fence_wait_timeout); /** - * dma_fence_release - default relese function for fences + * dma_fence_release - default release function for fences * @kref: &dma_fence.recfount * * This is the default release functions for &dma_fence. Drivers shouldn't call @@ -974,8 +974,8 @@ void dma_fence_set_deadline(struct dma_f EXPORT_SYMBOL(dma_fence_set_deadline); /** - * dma_fence_describe - Dump fence describtion into seq_file - * @fence: the 6fence to describe + * dma_fence_describe - Dump fence description into seq_file + * @fence: the fence to describe * @seq: the seq_file to put the textual description into * * Dump a textual description of the fence and it's state into the seq_file.

2 years

1
0
0 0

[PATCH v3 0/7] dma-buf: heaps: Add secure heap

by Yong Wu

This patchset is for secure video playback and enables other potential uses in the future. The 'secure 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 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 secure heaps and they will be used v4l2[1] and drm[2]. 1) secure_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) secure_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/linux-mediatek/20231023044549.21412-1-jason-jh.lin@… Change note: v3: Base on v6.7-rc1. 1) Separate the secure heap into a common file(secure_heap.c) and a mtk special file (secure_heap_mtk.c), and put all tee related code into our special file. 2) About dt-binding, a) Add "mediatek," prefix since this is Mediatek TEE firmware definition. b) Mute dt-binding check waring. 3) Remove the normal CMA heap which is a draft for qcom. 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-secure-region dma-buf: heaps: Initialize a secure heap dma-buf: heaps: secure_heap: Add private heap ops dma-buf: heaps: secure_heap: Add dma_ops dma-buf: heaps: secure_heap: Add MediaTek secure heap and heap_init dma-buf: heaps: secure_heap_mtk: Add tee memory service call dma_buf: heaps: secure_heap_mtk: Add a new CMA heap .../mediatek,dynamic-secure-region.yaml | 43 +++ drivers/dma-buf/heaps/Kconfig | 13 + drivers/dma-buf/heaps/Makefile | 2 + drivers/dma-buf/heaps/secure_heap.c | 234 +++++++++++++ drivers/dma-buf/heaps/secure_heap.h | 43 +++ drivers/dma-buf/heaps/secure_heap_mtk.c | 321 ++++++++++++++++++ 6 files changed, 656 insertions(+) create mode 100644 Documentation/devicetree/bindings/reserved-memory/mediatek,dynamic-secure-region.yaml create mode 100644 drivers/dma-buf/heaps/secure_heap.c create mode 100644 drivers/dma-buf/heaps/secure_heap.h create mode 100644 drivers/dma-buf/heaps/secure_heap_mtk.c -- 2.18.0

2 years

8
18
0 0

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig