Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

25 participants
3680 discussions

[PATCH 0/9] vfio/pci: Add mmap() for DMABUFs

by Matt Evans

Hi all, This series is based on previous RFCs/discussions: Tech topic: https://lore.kernel.org/linux-iommu/20250918214425.2677057-1-amastro@fb.com/ RFCv1: https://lore.kernel.org/all/20260226202211.929005-1-mattev@meta.com/ RFCv2: https://lore.kernel.org/kvm/20260312184613.3710705-1-mattev@meta.com/ The background/rationale is covered in more detail in the RFC cover letters. The TL;DR is: The goal is to enable userspace driver designs that use VFIO to export DMABUFs representing subsets of PCI device BARs, and "vend" those buffers from a primary process to other subordinate processes by fd. These processes then mmap() the buffers and their access to the device is isolated to the exported ranges. This is an improvement on sharing the VFIO device fd to subordinate processes, which would allow unfettered access . This is achieved by enabling mmap() of vfio-pci DMABUFs. Second, a new ioctl()-based revocation mechanism is added to allow the primary process to forcibly revoke access to previously-shared BAR spans, even if the subordinate processes haven't cleanly exited. (The related topic of safe delegation of iommufd control to the subordinate processes is not addressed here, and is follow-up work.) As well as isolation and revocation, another advantage to accessing a BAR through a VMA backed by a DMABUF is that it's straightforward to create the buffer with access attributes, such as write-combining. Notes on patches ================ Feedback from the RFCs requested that, instead of creating DMABUF-specific vm_ops and .fault paths, to go the whole way and migrate the existing VFIO PCI BAR mmap() to be backed by a DMABUF too, resulting in a common vm_ops and fault handler for mmap()s of both the VFIO device and explicitly-exported DMABUFs. This has been done for vfio-pci, but not sub-drivers (nvgrace-gpu's special-case mappings are unchanged). vfio/pci: Fix vfio_pci_dma_buf_cleanup() double-put A bug fix to a related are, whose context is a depdency for later patches. vfio/pci: Add a helper to look up PFNs for DMABUFs vfio/pci: Add a helper to create a DMABUF for a BAR-map VMA The first is for a DMABUF VMA fault handler to determine arbitrary-sized PFNs from ranges in DMABUF. Secondly, refactor DMABUF export for use by the existing export feature and a new helper that creates a DMABUF corresponding to a VFIO BAR mmap() request. vfio/pci: Convert BAR mmap() to use a DMABUF The vfio-pci core mmap() creates a DMABUF with the helper, and the vm_ops fault handler uses the other helper to resolve the fault. Because this depends on DMABUF structs/code, CONFIG_VFIO_PCI_CORE needs to depend on CONFIG_DMA_SHARED_BUFFER. The CONFIG_VFIO_PCI_DMABUF still conditionally enables the export support code. NOTE: The user mmap()s a device fd, but the resulting VMA's vm_file becomes that of the DMABUF which takes ownership of the device and puts it on release. This maintains the existing behaviour of a VMA keeping the VFIO device open. BAR zapping then happens via the existing vfio_pci_dma_buf_move() path, which now needs to unmap PTEs in the DMABUF's address_space. vfio/pci: Provide a user-facing name for BAR mappings There was a request for decent debug naming in /proc/<pid>/maps etc. comparable to the existing VFIO names: since the VMAs are DMABUFs, they have a "dmabuf:" prefix and can't be 100% identical to before. This is a user-visible change, but this patch at least now gives us extra info on the BDF & BAR being mapped. vfio/pci: Clean up BAR zap and revocation In general (see NOTE!) the vfio_pci_zap_bars() is now obsolete, since it unmaps PTEs in the VFIO device address_space which is now unused. This consolidates all calls (e.g. around reset) with the neighbouring vfio_pci_dma_buf_move()s into new functions, to revoke-zap/unrevoke. NOTE: the nvgrace-gpu driver continues to use its own private vm_ops, fault handler, etc. for its special memregions, and these DO still add PTEs to the VFIO device address_space. So, a temporary flag, vdev->bar_needs_zap, maintains the old behaviour for this use. At least this patch's consolidation makes it easy to remove the remaining zap when this need goes away. A FIXME is added: if nvgrace-gpu is converted to DMABUFs, remove the flag and final zap. vfio/pci: Support mmap() of a VFIO DMABUF Adds mmap() for a DMABUF fd exported from vfio-pci. It was a goal to keep the VFIO device fd lifetime behaviour unchanged with respect to the DMABUFs. An application can close all device fds, and this will revoke/clean up all DMABUFs; no mappings or other access can be performed now. When enabling mmap() of the DMABUFs, this means access through the VMA is also revoked. This complicates the fault handler because whilst the DMABUF exists, it has no guarantee that the corresponding VFIO device is still alive. Adds synchronisation ensuring the vdev is available before vdev->memory_lock is touched. (I decided against the alternative of preventing cleanup by holding the VFIO device open if any DMABUFs exist, because it's both a change of behaviour and less clean overall.) I've added a chonky comment in place, happy to clarify more if you have ideas. vfio/pci: Permanently revoke a DMABUF on request By weight, this is mostly a rename of revoked to an enum, status. There are now 3 states for a buffer, usable and revoked temporary/permanent. A new VFIO device ioctl is added, VFIO_DEVICE_PCI_DMABUF_REVOKE, which passes a DMABUF (exported from that device) and permanently revokes it. Thus a userspace driver can guarantee any downstream consumers of a shared fd are prevented from accessing a BAR range, and that range can be reused. The code doing revocation in vfio_pci_dma_buf_move() is moved, unchanged, to a common function for use by _move() and the new ioctl path. Q: I can't think of a good reason to temporarily revoke/unrevoke buffers from userspace, so didn't add a 'flags' field to the ioctl struct. Easy to add if people think it's worthwhile for future use. vfio/pci: Add mmap() attributes to DMABUF feature Reserves bits [31:28] in vfio_device_feature_dma_buf to allow a (CPU) mapping attribute to be specified for an exported set of ranges. The default is the current UC, and a new flag can specify CPU access as WC. Q: I've taken 4 bits; the intention is for this field to be a scalar not a bitmap (i.e. mutually-exclusive access properties). Perhaps 4 is a bit too many? Testing ======= (The [RFC ONLY] userspace test program, for QEMU edu-plus, has been dropped, but can be found in the GitHub branch below.) This code has been tested in mapping DMABUFs of single/multiple ranges, aliasing mmap()s, aliasing ranges across DMABUFs, vm_pgoff > 0, revocation, shutdown/cleanup scenarios, and hugepage mappings seem to work correctly. I've lightly tested WC mappings also (by observing resulting PTEs as having the correct attributes...). No regressions observed on the VFIO selftests, or on our internal vfio-pci applications. End === This is based on -next (next-20260414 but will merge earlier), as it depends on Leon's series "vfio: Wait for dma-buf invalidation to complete": https://lore.kernel.org/linux-iommu/20260205-nocturnal-poetic-chamois-f566a… These commits are on GitHub, along with "[RFC ONLY] selftests: vfio: Add standalone vfio_dmabuf_mmap_test": https://github.com/metamev/linux/compare/next-20260414...metamev:linux:dev/… Thanks for reading, Matt ================================================================================ Change log: v1: - Cleanup of the common DMABUF-aware VMA vm_ops fault handler and export code. - Fixed a lot of races, particularly faults racing with DMABUF cleanup (if the VFIO device fds close, for example). - Added nicer human-readable names for VFIO mmap() VMAs RFCv2: Respin based on the feedback/suggestions: https://lore.kernel.org/kvm/20260312184613.3710705-1-mattev@meta.com/ - Transform the existing VFIO BAR mmap path to also use DMABUFs behind the scenes, and then simply share that code for explicitly-mapped DMABUFs. Jason wanted to go that direction to enable iommufd VFIO type 1 emulation to pick up a DMABUF for an IO mapping. - Revoke buffers using a VFIO device fd ioctl RFCv1: https://lore.kernel.org/all/20260226202211.929005-1-mattev@meta.com/ Matt Evans (9): vfio/pci: Fix vfio_pci_dma_buf_cleanup() double-put vfio/pci: Add a helper to look up PFNs for DMABUFs vfio/pci: Add a helper to create a DMABUF for a BAR-map VMA vfio/pci: Convert BAR mmap() to use a DMABUF vfio/pci: Provide a user-facing name for BAR mappings vfio/pci: Clean up BAR zap and revocation vfio/pci: Support mmap() of a VFIO DMABUF vfio/pci: Permanently revoke a DMABUF on request vfio/pci: Add mmap() attributes to DMABUF feature drivers/vfio/pci/Kconfig | 3 +- drivers/vfio/pci/Makefile | 3 +- drivers/vfio/pci/nvgrace-gpu/main.c | 5 + drivers/vfio/pci/vfio_pci_config.c | 30 +- drivers/vfio/pci/vfio_pci_core.c | 224 ++++++++++--- drivers/vfio/pci/vfio_pci_dmabuf.c | 500 +++++++++++++++++++++++----- drivers/vfio/pci/vfio_pci_priv.h | 49 ++- include/linux/vfio_pci_core.h | 1 + include/uapi/linux/vfio.h | 42 ++- 9 files changed, 690 insertions(+), 167 deletions(-) -- 2.47.3

5 months, 2 weeks

[PATCH v3] drm/virtio: use uninterruptible resv lock for plane updates

by Deepanshu Kartikey

virtio_gpu_cursor_plane_update() and virtio_gpu_resource_flush() lock the framebuffer BO's dma_resv via virtio_gpu_array_lock_resv() and ignore its return value. The function can fail with -EINTR from dma_resv_lock_interruptible() (signal during lock wait) or with -ENOMEM from dma_resv_reserve_fences() (fence slot allocation), leaving the resv lock not held. The queue path then walks the object array and calls dma_resv_add_fence(), which requires the lock held; with lockdep enabled this trips dma_resv_assert_held(): WARNING: drivers/dma-buf/dma-resv.c:296 at dma_resv_add_fence+0x71e/0x840 Call Trace: virtio_gpu_array_add_fence virtio_gpu_queue_ctrl_sgs virtio_gpu_queue_fenced_ctrl_buffer virtio_gpu_cursor_plane_update drm_atomic_helper_commit_planes drm_atomic_helper_commit_tail commit_tail drm_atomic_helper_commit drm_atomic_commit drm_atomic_helper_update_plane __setplane_atomic drm_mode_cursor_universal drm_mode_cursor_common drm_mode_cursor_ioctl drm_ioctl __x64_sys_ioctl Beyond the WARN, mutating the dma_resv fence list without the lock races with concurrent readers/writers and can corrupt the list. Both call sites run inside the .atomic_update plane callback, which DRM atomic helpers do not allow to fail (by the time it runs, the commit has been signed off to userspace and there is no clean rollback path). Moving the lock acquisition to .prepare_fb (v2) was rejected because the broader lock scope deadlocks against other BO locking paths in the same atomic commit. Introduce virtio_gpu_array_lock_resv_uninterruptible() that uses dma_resv_lock() instead of dma_resv_lock_interruptible() on the nents==1 path. This eliminates the -EINTR failure mode -- the realistic syzbot trigger -- without extending the lock hold across the commit. Use it from both virtio_gpu_cursor_plane_update() and virtio_gpu_resource_flush(); check the return value to handle the remaining -ENOMEM case by freeing the objs and skipping the plane update for that frame. The framebuffer BOs touched here are not shared with other contexts and lock contention is expected to be brief, so the loss of signal-interruptibility is acceptable. Other callers of virtio_gpu_array_lock_resv() (the ioctl paths) continue to use the interruptible variant. The bug was reported by syzbot, triggered via fault injection (fail_nth) on the DRM_IOCTL_MODE_CURSOR path, which forces the -ENOMEM branch in dma_resv_reserve_fences(). Reported-by: syzbot+72bd3dd3a5d5f39a0271(a)syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=72bd3dd3a5d5f39a0271 Fixes: 5cfd31c5b3a3 ("drm/virtio: fix virtio_gpu_cursor_plane_update().") Cc: stable(a)vger.kernel.org Signed-off-by: Deepanshu Kartikey <kartikey406(a)gmail.com> --- v3: Per maintainer feedback on v2 (lockup caused by the broader lock scope in prepare_fb conflicting with other BO locking in the same atomic commit): drop the prepare_fb/cleanup_fb approach, introduce an uninterruptible variant of virtio_gpu_array_lock_resv(), and use it in both virtio_gpu_cursor_plane_update() and virtio_gpu_resource_flush(). v2: Move resv lock acquisition from .atomic_update (which must not fail) to .prepare_fb (which may), per maintainer review of v1. The previous approach of silently skipping the cursor update on lock failure violated the atomic-commit contract with userspace. --- drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + drivers/gpu/drm/virtio/virtgpu_gem.c | 24 ++++++++++++++++++++++++ drivers/gpu/drm/virtio/virtgpu_plane.c | 10 ++++++++-- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index f17660a71a3e..43a7eb568e15 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -317,6 +317,7 @@ virtio_gpu_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents void virtio_gpu_array_add_obj(struct virtio_gpu_object_array *objs, struct drm_gem_object *obj); int virtio_gpu_array_lock_resv(struct virtio_gpu_object_array *objs); +int virtio_gpu_array_lock_resv_uninterruptible(struct virtio_gpu_object_array *objs); void virtio_gpu_array_unlock_resv(struct virtio_gpu_object_array *objs); void virtio_gpu_array_add_fence(struct virtio_gpu_object_array *objs, struct dma_fence *fence); diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c index f22dc5c21cd4..08c4b7ef8d44 100644 --- a/drivers/gpu/drm/virtio/virtgpu_gem.c +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c @@ -238,6 +238,30 @@ int virtio_gpu_array_lock_resv(struct virtio_gpu_object_array *objs) return ret; } +int virtio_gpu_array_lock_resv_uninterruptible(struct virtio_gpu_object_array *objs) +{ + unsigned int i; + int ret = 0; + + if (objs->nents == 1) { + dma_resv_lock(objs->objs[0]->resv, NULL); + } else { + ret = drm_gem_lock_reservations(objs->objs, objs->nents, + &objs->ticket); + if (ret) + return ret; + } + + for (i = 0; i < objs->nents; ++i) { + ret = dma_resv_reserve_fences(objs->objs[i]->resv, 1); + if (ret) { + virtio_gpu_array_unlock_resv(objs); + return ret; + } + } + return 0; +} + void virtio_gpu_array_unlock_resv(struct virtio_gpu_object_array *objs) { if (objs->nents == 1) { diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index a126d1b25f46..ef118cb4f0fa 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -215,7 +215,10 @@ static void virtio_gpu_resource_flush(struct drm_plane *plane, if (!objs) return; virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]); - virtio_gpu_array_lock_resv(objs); + if (virtio_gpu_array_lock_resv_uninterruptible(objs)) { + virtio_gpu_array_put_free(objs); + return; + } virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y, width, height, objs, vgplane_st->fence); @@ -459,7 +462,10 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, if (!objs) return; virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]); - virtio_gpu_array_lock_resv(objs); + if (virtio_gpu_array_lock_resv_uninterruptible(objs)) { + virtio_gpu_array_put_free(objs); + return; + } virtio_gpu_cmd_transfer_to_host_2d (vgdev, 0, plane->state->crtc_w, -- 2.43.0

5 months, 2 weeks

[bug report] dma-buf: heaps: Rework heap allocation hooks to return struct dma_buf instead of fd

by Dan Carpenter

I'm not sure exactly who to report this bug too. Probably the mm devs? drivers/dma-buf/heaps/system_heap.c:499 system_heap_allocate() warn: passing positive error code 's32min-(-1),1' to 'ERR_PTR' drivers/dma-buf/heaps/system_heap.c 459 if (cc_shared) { 460 for_each_sgtable_sg(table, sg, i) { 461 ret = system_heap_set_page_decrypted(sg_page(sg)); 462 if (ret) 463 goto free_pages; It kind of looks like system_heap_set_page_decrypted() can return 1. 464 } 465 } 466 467 /* create the dmabuf */ 468 exp_info.exp_name = dma_heap_get_name(heap); 469 exp_info.ops = &system_heap_buf_ops; 470 exp_info.size = buffer->len; 471 exp_info.flags = fd_flags; 472 exp_info.priv = buffer; 473 dmabuf = dma_buf_export(&exp_info); 474 if (IS_ERR(dmabuf)) { 475 ret = PTR_ERR(dmabuf); 476 goto free_pages; 477 } 478 return dmabuf; 479 480 free_pages: 481 for_each_sgtable_sg(table, sg, i) { 482 struct page *p = sg_page(sg); 483 484 /* 485 * Intentionally leak pages that cannot be re-encrypted 486 * to prevent shared memory from being reused. 487 */ 488 if (buffer->cc_shared && 489 system_heap_set_page_encrypted(p)) 490 continue; 491 __free_pages(p, compound_order(p)); 492 } 493 sg_free_table(table); 494 free_buffer: 495 list_for_each_entry_safe(page, tmp_page, &pages, lru) 496 __free_pages(page, compound_order(page)); 497 kfree(buffer); 498 --> 499 return ERR_PTR(ret); 500 } The problem is that add_to_pagemap() returns PM_END_OF_BUFFER (1) which is used by pagemap_read() and nowhere else. The call tree is: system_heap_allocate() system_heap_set_page_decrypted() set_memory_decrypted() realm_set_memory_decrypted() __set_memory_enc_dec() __change_memory_common() update_range_prot() walk_kernel_page_table_range_lockless() walk_pgd_range() pagemap_pte_hole() add_to_pagemap() This code seems sort of old and I guess no one has reported the bug so maybe it's a false positive, but it feels like it's asking for problems to return the PM_END_OF_BUFFER. There aren't any comments on any of those functions above explaining what return values are expected. This email is a free service from the Smatch-CI project [smatch.sf.net]. regards, dan carpenter

5 months, 2 weeks

[PATCH V1] accel/amdxdna: Remove mmap and export support for ubuf

by Lizhi Hou

Ubuf pages should not be mmaped or exported. Remove the ubuf mmap callback and return -EOPNOTSUPP when exporting ubuf objects. ubuf vmap is also removed for there is not a real use case yet. Fixes: bd72d4acda10 ("accel/amdxdna: Support user space allocated buffer") Cc: Christian Koenig <christian.koenig(a)amd.com> Signed-off-by: Lizhi Hou <lizhi.hou(a)amd.com> --- drivers/accel/amdxdna/amdxdna_gem.c | 9 +++++++- drivers/accel/amdxdna/amdxdna_gem.h | 2 ++ drivers/accel/amdxdna/amdxdna_ubuf.c | 32 ---------------------------- 3 files changed, 10 insertions(+), 33 deletions(-) diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c index 319d2064fafa..6087264ba1b5 100644 --- a/drivers/accel/amdxdna/amdxdna_gem.c +++ b/drivers/accel/amdxdna/amdxdna_gem.c @@ -492,6 +492,9 @@ static struct dma_buf *amdxdna_gem_prime_export(struct drm_gem_object *gobj, int struct amdxdna_gem_obj *abo = to_xdna_obj(gobj); DEFINE_DMA_BUF_EXPORT_INFO(exp_info); + if (abo->pri) + return ERR_PTR(-EOPNOTSUPP); + if (abo->dma_buf) { get_dma_buf(abo->dma_buf); return abo->dma_buf; @@ -716,6 +719,7 @@ amdxdna_gem_create_ubuf_object(struct drm_device *dev, struct amdxdna_drm_create { struct amdxdna_dev *xdna = to_xdna_dev(dev); struct amdxdna_drm_va_tbl va_tbl; + struct amdxdna_gem_obj *abo; struct drm_gem_object *gobj; struct dma_buf *dma_buf; @@ -742,7 +746,10 @@ amdxdna_gem_create_ubuf_object(struct drm_device *dev, struct amdxdna_drm_create dma_buf_put(dma_buf); - return to_xdna_obj(gobj); + abo = to_xdna_obj(gobj); + abo->pri = true; + + return abo; } static struct amdxdna_gem_obj * diff --git a/drivers/accel/amdxdna/amdxdna_gem.h b/drivers/accel/amdxdna/amdxdna_gem.h index 4fc48a1189d2..162e5499f5e0 100644 --- a/drivers/accel/amdxdna/amdxdna_gem.h +++ b/drivers/accel/amdxdna/amdxdna_gem.h @@ -54,6 +54,8 @@ struct amdxdna_gem_obj { /* True, if BO is managed by XRT, not application */ bool internal; + /* True, if BO is not exportable */ + bool pri; }; #define to_gobj(obj) (&(obj)->base.base) diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c index 3769210c55cc..df4ab225fbf9 100644 --- a/drivers/accel/amdxdna/amdxdna_ubuf.c +++ b/drivers/accel/amdxdna/amdxdna_ubuf.c @@ -87,42 +87,10 @@ static const struct vm_operations_struct amdxdna_ubuf_vm_ops = { .fault = amdxdna_ubuf_vm_fault, }; -static int amdxdna_ubuf_mmap(struct dma_buf *dbuf, struct vm_area_struct *vma) -{ - struct amdxdna_ubuf_priv *ubuf = dbuf->priv; - - vma->vm_ops = &amdxdna_ubuf_vm_ops; - vma->vm_private_data = ubuf; - vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP); - - return 0; -} - -static int amdxdna_ubuf_vmap(struct dma_buf *dbuf, struct iosys_map *map) -{ - struct amdxdna_ubuf_priv *ubuf = dbuf->priv; - void *kva; - - kva = vmap(ubuf->pages, ubuf->nr_pages, VM_MAP, PAGE_KERNEL); - if (!kva) - return -EINVAL; - - iosys_map_set_vaddr(map, kva); - return 0; -} - -static void amdxdna_ubuf_vunmap(struct dma_buf *dbuf, struct iosys_map *map) -{ - vunmap(map->vaddr); -} - static const struct dma_buf_ops amdxdna_ubuf_dmabuf_ops = { .map_dma_buf = amdxdna_ubuf_map, .unmap_dma_buf = amdxdna_ubuf_unmap, .release = amdxdna_ubuf_release, - .mmap = amdxdna_ubuf_mmap, - .vmap = amdxdna_ubuf_vmap, - .vunmap = amdxdna_ubuf_vunmap, }; static int readonly_va_entry(struct amdxdna_drm_va_entry *va_ent) -- 2.34.1

5 months, 2 weeks

[PATCH v2] dma-fence: Clarify external lock use case in dma_fence_init() docs

by Maíra Canal

The kerneldoc comment on dma_fence_init() and dma_fence_init64() describe the legacy reason to pass an external lock as a need to prevent multiple fences "from signaling out of order". However, this wording is a bit misleading: a shared spinlock does not (and cannot) prevent the signaler from signaling out of order. Signaling order is the driver's responsibility regardless of whether the lock is shared or per-fence. What a shared lock actually provides is serialization of signaling and observation across fences in a given context, so that observers never see a later fence signaled while an earlier one is not. Reword both comments to describe this more accurately. Signed-off-by: Maíra Canal <mcanal(a)igalia.com> --- v1 -> v2: https://lore.kernel.org/dri-devel/20260411185756.1887119-4-mcanal@igalia.co… - Be more explicit about not allowing new users to use an external lock. - De-duplicate the explanation in dma_fence_init64() by pointing to the dma_fence_init() documentation. --- drivers/dma-buf/dma-fence.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 1c1eaecaf1b0..63b349ba9a34 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -1102,9 +1102,11 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, * context and seqno are used for easy comparison between fences, allowing * to check which fence is later by simply using dma_fence_later(). * - * It is strongly discouraged to provide an external lock because this couples - * lock and fence life time. This is only allowed for legacy use cases when - * multiple fences need to be prevented from signaling out of order. + * External locks are a relic from legacy use cases that needed a shared lock + * to serialize signaling and observation of fences within a context, so that + * observers never see a later fence signaled while an earlier one isn't. New + * users MUST NOT use external locks, as they force the issuer to outlive all + * fences that reference the lock. */ void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, @@ -1129,9 +1131,8 @@ EXPORT_SYMBOL(dma_fence_init); * Context and seqno are used for easy comparison between fences, allowing * to check which fence is later by simply using dma_fence_later(). * - * It is strongly discouraged to provide an external lock because this couples - * lock and fence life time. This is only allowed for legacy use cases when - * multiple fences need to be prevented from signaling out of order. + * New users MUST NOT use external locks. Check the documentation in + * dma_fence_init() to understand the motives behind the legacy use cases. */ void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, -- 2.53.0

5 months, 2 weeks

[PATCH] drm/amdgpu: fix lock leak on ENOMEM in AMDGPU_GEM_OP_GET_MAPPING_INFO

by Michael Bommarito

The AMDGPU_GEM_OP_GET_MAPPING_INFO branch of amdgpu_gem_op_ioctl() holds three cleanup-tracked resources before calling kvcalloc(): the drm_gem_object reference from drm_gem_object_lookup(), the drm_exec lock on the looked-up GEM via drm_exec_lock_obj(), and the drm_exec lock on the per-process VM root page directory via amdgpu_vm_lock_pd(). All three are released by the out_exec label that every other error path in this function jumps to. The kvcalloc() failure path returns -ENOMEM directly, skipping out_exec and leaking all three. The leaked per-process VM root PD dma_resv lock is the load-bearing leak: any subsequent operation on the same VM (further GEM ops, command-submission, eviction, TTM shrinker callbacks) blocks on the held lock. DRM_IOCTL_AMDGPU_GEM_OP is DRM_AUTH | DRM_RENDER_ALLOW, so this is an unprivileged-local denial of service against the caller's GPU context, reachable by any process with /dev/dri/renderD* access. Route the failure through out_exec so drm_exec_fini() and drm_gem_object_put() run. Reproduced on stock 7.0.0-10, Ryzen 7 5700U / Radeon Vega (Lucienne): the failing ioctl returns -ENOMEM and a second GET_MAPPING_INFO on the same fd then blocks in drm_exec_lock_obj() on the leaked dma_resv. SIGKILL on the caller does not reap the task; the fd-release path during process exit goes through amdgpu_gem_object_close() -> drm_exec_prepare_obj() on the same lock, leaving the task in D state until the box is rebooted. The patched kernel was not rebuilt and re-tested on this hardware; the fix is mechanical. Tested on a single Lucienne / Vega box only. Ziyi Guo posted an independent INT_MAX-bound check for args->num_entries in the same branch [1]; the two patches are complementary and can land in either order. Fixes: 4d82724f7f2b ("drm/amdgpu: Add mapping info option for GEM_OP ioctl") Cc: stable(a)vger.kernel.org Link: https://lore.kernel.org/all/20260208000255.4073363-1-n7l8m4@u.northwestern.… # [1] Signed-off-by: Michael Bommarito <michael.bommarito(a)gmail.com> Assisted-by: Claude:claude-opus-4-7 --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 9ef80bca4102..8224fb499fdf 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -1091,8 +1091,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data, * be retried. */ vm_entries = kvcalloc(args->num_entries, sizeof(*vm_entries), GFP_KERNEL); - if (!vm_entries) - return -ENOMEM; + if (!vm_entries) { + r = -ENOMEM; + goto out_exec; + } amdgpu_vm_bo_va_for_each_valid_mapping(bo_va, mapping) { if (num_mappings < args->num_entries) { -- 2.53.0

5 months, 2 weeks

[PATCH 00/13] Add DMA support for LINFlexD UART driver

by Larisa Grigore

This patchset enhances the LINFlexD UART driver and its device tree bindings to support DMA transfers, configurable clock inputs, dynamic baudrate changes, and termios features. It also includes a series of fixes and improvements to ensure reliable operation across various modes and configurations. The changes added can be summarized as follows: 1. Fixes with respect to FIFO handling, locking, interrupt related registers and INITM mode transition. 2. Removal of the earlycon workaround, as proper FIFO handling and INITM transitions now ensure stable behavior. 3. Support for configurable stop bits and dynamic baudrate changes based on clock inputs and termios settings. 4. Optional DMA support for RX and TX paths, preventing character loss during high-throughput operations like copy-paste. Cyclic DMA is used for RX to avoid gaps between transactions. Larisa Grigore (8): serial: linflexuart: Clean SLEEP bit in LINCR1 after suspend serial: linflexuart: Check FIFO full before writing serial: linflexuart: Correctly clear UARTSR in buffer mode serial: linflexuart: Update RXEN/TXEN outside INITM mode serial: linflexuart: Ensure FIFO is empty when entering INITM serial: linflexuart: Revert earlycon workaround serial: linflexuart: Add support for configurable stop bits serial: linflexuart: Add DMA support Radu Pirea (5): serial: linflexuart: Fix locking in set_termios dt-bindings: serial: fsl-linflexuart: add clock input properties dt-bindings: serial: fsl-linflexuart: add dma properties serial: linflexuart: Add support for changing baudrate serial: linflexuart: Avoid stopping DMA during receive operations .../bindings/serial/fsl,s32-linflexuart.yaml | 31 + drivers/tty/serial/fsl_linflexuart.c | 972 +++++++++++++++--- 2 files changed, 846 insertions(+), 157 deletions(-) -- 2.47.0

5 months, 2 weeks

[PATCH V1] accel/amdxdna: Fix user buffer VMA checking and page pinning locking

by Lizhi Hou

From: Max Zhen <max.zhen(a)amd.com> Holding mmap_read_lock across user buffer VMA checking and page pinning. This keeps VMA validation and user page pinning synchronized with mmap updates. Also adding the check for VM_MAYWRITE to make sure the mapping can't be updated to writable after pinning of the pages. Fixes: f649e63d4a64 ("accel/amdxdna: Support read-only user-pointer BO mappings") Signed-off-by: Max Zhen <max.zhen(a)amd.com> Signed-off-by: Lizhi Hou <lizhi.hou(a)amd.com> --- drivers/accel/amdxdna/amdxdna_ubuf.c | 34 ++++++++++++++++------------ 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c index df4ab225fbf9..85c3d6f46845 100644 --- a/drivers/accel/amdxdna/amdxdna_ubuf.c +++ b/drivers/accel/amdxdna/amdxdna_ubuf.c @@ -99,17 +99,13 @@ static int readonly_va_entry(struct amdxdna_drm_va_entry *va_ent) struct vm_area_struct *vma; int ret; - mmap_read_lock(mm); - vma = find_vma(mm, va_ent->vaddr); if (!vma || vma->vm_start > va_ent->vaddr || vma->vm_end - va_ent->vaddr < va_ent->len) ret = -ENOENT; else - ret = vma->vm_flags & VM_WRITE ? 0 : 1; - - mmap_read_unlock(mm); + ret = vma->vm_flags & (VM_WRITE | VM_MAYWRITE) ? 0 : 1; return ret; } @@ -123,7 +119,7 @@ struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev, u32 npages, start = 0; struct dma_buf *dbuf; bool readonly = true; - int i, ret; + int i, ret = 0; DEFINE_DMA_BUF_EXPORT_INFO(exp_info); if (!can_do_mlock()) @@ -161,10 +157,6 @@ struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev, ret = -EINVAL; goto free_ent; } - - /* Pin pages as writable as long as not all entries are read-only. */ - if (readonly && readonly_va_entry(&va_ent[i]) != 1) - readonly = false; } ubuf->nr_pages = exp_info.size >> PAGE_SHIFT; @@ -183,25 +175,37 @@ struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev, goto sub_pin_cnt; } + mmap_read_lock(current->mm); + + for (i = 0; i < num_entries; i++) { + /* Pin pages as writable as long as not all entries are read-only. */ + if (readonly && readonly_va_entry(&va_ent[i]) != 1) + readonly = false; + } for (i = 0; i < num_entries; i++) { npages = va_ent[i].len >> PAGE_SHIFT; - ret = pin_user_pages_fast(va_ent[i].vaddr, npages, - (readonly ? 0 : FOLL_WRITE) | FOLL_LONGTERM, - &ubuf->pages[start]); + ret = pin_user_pages(va_ent[i].vaddr, npages, + (readonly ? 0 : FOLL_WRITE) | FOLL_LONGTERM, + &ubuf->pages[start]); if (ret >= 0) { start += ret; if (ret != npages) { XDNA_ERR(xdna, "Partially pinned pages %d/%u", ret, npages); ret = -ENOMEM; - goto destroy_pages; + break; } } else { XDNA_ERR(xdna, "Failed to pin pages ret %d", ret); - goto destroy_pages; + break; } } + mmap_read_unlock(current->mm); + + if (ret < 0) + goto destroy_pages; + exp_info.ops = &amdxdna_ubuf_dmabuf_ops; exp_info.priv = ubuf; exp_info.flags = (readonly ? O_RDONLY : O_RDWR) | O_CLOEXEC; -- 2.34.1

5 months, 2 weeks

[PATCH v3 05/11] dma-buf: Use trace_call__##name() at guarded tracepoint call sites

by Vineeth Pillai (Google)

From: Vineeth Pillai <vineeth(a)bitbyteword.org> Replace trace_foo() with the new trace_call__foo() at sites already guarded by trace_foo_enabled(), avoiding a redundant static_branch_unlikely() re-evaluation inside the tracepoint. trace_call__foo() calls the tracepoint callbacks directly without utilizing the static branch again. Original v2 series: https://lore.kernel.org/linux-trace-kernel/20260323160052.17528-1-vineeth@b… Parts of the original v2 series have already been merged in mainline. This patch is being reposted as a follow-up cleanup for the remaining unmerged pieces. Suggested-by: Steven Rostedt <rostedt(a)goodmis.org> Suggested-by: Peter Zijlstra <peterz(a)infradead.org> Signed-off-by: Vineeth Pillai (Google) <vineeth(a)bitbyteword.org> Assisted-by: Claude:claude-sonnet-4-6 --- drivers/dma-buf/dma-fence.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index a2aa82f4eedd..a41cdd9c9343 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -553,7 +553,7 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) } if (trace_dma_fence_wait_end_enabled()) { rcu_read_lock(); - trace_dma_fence_wait_end(fence); + trace_call__dma_fence_wait_end(fence); rcu_read_unlock(); } return ret; -- 2.54.0

5 months, 2 weeks

[PATCH v2] drm/virtio: move cursor resv lock acquisition to prepare_fb

by Deepanshu Kartikey

virtio_gpu_cursor_plane_update() allocates a virtio_gpu_object_array, locks its dma_resv, and queues a fenced transfer to the host. The lock acquisition can fail in two ways: - dma_resv_lock_interruptible() returns -EINTR when a signal is delivered while waiting for the reservation lock. - dma_resv_reserve_fences() returns -ENOMEM if it fails to allocate a fence slot; in this case lock_resv unlocks before returning. The return value was ignored, so the cursor path could proceed with the resv lock not held. The queue path then walks the object array and calls dma_resv_add_fence(), which requires the lock; with lockdep enabled this trips dma_resv_assert_held(): WARNING: drivers/dma-buf/dma-resv.c:296 at dma_resv_add_fence+0x71e/0x840 Call Trace: virtio_gpu_array_add_fence virtio_gpu_queue_ctrl_sgs virtio_gpu_queue_fenced_ctrl_buffer virtio_gpu_cursor_plane_update drm_atomic_helper_commit_planes drm_atomic_helper_commit_tail commit_tail drm_atomic_helper_commit drm_atomic_commit drm_atomic_helper_update_plane __setplane_atomic drm_mode_cursor_universal drm_mode_cursor_common drm_mode_cursor_ioctl drm_ioctl __x64_sys_ioctl Beyond the WARN, mutating the dma_resv fence list without the lock races with concurrent readers/writers and can corrupt the list. The DRM atomic helpers do not allow .atomic_update to fail: by the time it runs, the commit has been signed off to userspace and there is no clean rollback path. Move the fallible work -- objs allocation, dma_resv locking, and fence slot reservation -- into virtio_gpu_plane_prepare_fb, which is the designated callback for resource acquisition and may return errors that the framework handles by rolling back the commit. Stash the prepared object array on virtio_gpu_plane_state so the update step can consume it. Make virtio_gpu_plane_cleanup_fb release the objs if the commit was rolled back before update ran (i.e., objs not consumed). The queue path already unlocks the resv after attaching the fence (vq.c:411) and frees the array via put_free_delayed after host completion (vq.c:271), so the update step only needs to clear vgplane_st->objs to transfer ownership. Simplify virtio_gpu_cursor_plane_update to a no-fail queue submission that hands the prepared, locked objs to the queue path. The bug was reported by syzbot, triggered via fault injection (fail_nth) on the DRM_IOCTL_MODE_CURSOR path, which forces the -ENOMEM branch in dma_resv_reserve_fences(). Reported-by: syzbot+72bd3dd3a5d5f39a0271(a)syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=72bd3dd3a5d5f39a0271 Fixes: 5cfd31c5b3a3 ("drm/virtio: fix virtio_gpu_cursor_plane_update().") Cc: stable(a)vger.kernel.org Link: https://lore.kernel.org/all/20260510053025.100224-1-kartikey406@gmail.com/T/ [v1] Signed-off-by: Deepanshu Kartikey <kartikey406(a)gmail.com> --- v2: Move resv lock acquisition from .atomic_update (which must not fail) to .prepare_fb (which may), per maintainer review of v1. The previous approach of silently skipping the cursor update on lock failure violated the atomic-commit contract with userspace. --- drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + drivers/gpu/drm/virtio/virtgpu_plane.c | 38 ++++++++++++++++++++------ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index f17660a71a3e..e51f959dce46 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -198,6 +198,7 @@ struct virtio_gpu_framebuffer { struct virtio_gpu_plane_state { struct drm_plane_state base; struct virtio_gpu_fence *fence; + struct virtio_gpu_object_array *objs; }; #define to_virtio_gpu_plane_state(x) \ container_of(x, struct virtio_gpu_plane_state, base) diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index a126d1b25f46..b0511ace89e6 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -381,6 +381,23 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane, goto err_fence; } + if (plane->type == DRM_PLANE_TYPE_CURSOR && bo->dumb) { + struct virtio_gpu_object_array *objs; + + objs = virtio_gpu_array_alloc(1); + if (!objs) { + ret = -ENOMEM; + goto err_fence; + } + virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]); + ret = virtio_gpu_array_lock_resv(objs); + if (ret) { + virtio_gpu_array_put_free(objs); + goto err_fence; + } + vgplane_st->objs = objs; + } + return 0; err_fence: @@ -417,6 +434,12 @@ static void virtio_gpu_plane_cleanup_fb(struct drm_plane *plane, vgplane_st->fence = NULL; } + if (vgplane_st->objs) { + virtio_gpu_array_unlock_resv(vgplane_st->objs); + virtio_gpu_array_put_free(vgplane_st->objs); + vgplane_st->objs = NULL; + } + obj = state->fb->obj[0]; if (drm_gem_is_imported(obj)) virtio_gpu_cleanup_imported_obj(obj); @@ -452,21 +475,18 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, } if (bo && bo->dumb && (plane->state->fb != old_state->fb)) { - /* new cursor -- update & wait */ - struct virtio_gpu_object_array *objs; - - objs = virtio_gpu_array_alloc(1); - if (!objs) - return; - virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]); - virtio_gpu_array_lock_resv(objs); + /* objs and fence were prepared in virtio_gpu_plane_prepare_fb; + * the resv is already locked. The queue path takes ownership + * of objs and unlocks the resv after attaching the fence. + */ virtio_gpu_cmd_transfer_to_host_2d (vgdev, 0, plane->state->crtc_w, plane->state->crtc_h, - 0, 0, objs, vgplane_st->fence); + 0, 0, vgplane_st->objs, vgplane_st->fence); virtio_gpu_notify(vgdev); dma_fence_wait(&vgplane_st->fence->f, true); + vgplane_st->objs = NULL; } if (plane->state->fb != old_state->fb) { -- 2.43.0

5 months, 2 weeks

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig