Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

22 participants
3660 discussions

Re: [PATCH RFC 01/26] dma-buf: Introduce DMA-buf mapping types

by Christian König

On 10/22/26 08:52, Alistair Popple wrote: > On 2026-10-21 at 23:37 +1100, Jason Gunthorpe <jgg(a)nvidia.com> wrote... >> On Wed, Oct 21, 2026 at 08:24:40AM +0200, Thomas Hellström wrote: >>>> struct dma_buf_mapping_exp_ops >>>> Base type for the per-mapping type exporter provided functions. >>>> This >>>> would be the map/unmap callbacks. Each mapping type can provide >>>> its own >>>> functions for map/unmap type operations with optimal type >>>> signatures. >>> >>> Does it make sense to try to land this patch first, plus a conversion >>> of pcie_p2p so we could agree on what the contract should look like? >> >> I think it would be good to focus on this patch as the main 'crux' of >> the series. The API contract is the main agreement. I'm relucant to >> merge code that has no users, so it woudl be ebtter to go up to at >> least through SGT. >> >>> s/mapping/interconnect/? My gut feeling is that "interconnect" best >>> describes what's typically going on, but "mapping" perhaps better >>> describes all cases. >> >> My intention was to cover more than just the phyiscal transport. We do >> really want to get away from scatterlist, so having PCIe map with >> scatterlist and PCIe map with <something else> still ends up with two >> mapping types in the system. >> >> This is why I choose the name over interconnect. >> >>> Location of common protocols which typically carry cross-subsystem >>> dependencies? >>> drivers/dma-buf/mapping_types? drivers/dma-buf/interconnects? >> >> I'd encourage people to design more like PCI where the actual >> *interconnect* has its own home and there is just some minor call outs >> from dmabuf to help the matching logic. >> >>> As discussed previously: >>> Agree on theoretically possible matches at attach time, Match at >>> mapping time and guarantee the match is valid until >>> invalidate_mappings(). >> >> I was thinking about this some more, and I'm less sure now. >> >> If I think about mlx5, in a multi-path PCIe configuration, it can move >> the translation around within a single PCIe path, but it cannot move >> it to another path without creating disruption for the >> application. This is a HW limitation in how the internal routing >> works.. >> >> I strongly suspect this will be the norm for any hardware that doesn't >> rely on decorated page tables to select the IO path.. >> >> So, while I agree we should support hardware that can fully change the >> path on each and every map, we also need to suport hardware that must >> pin the path during attach. >> >> Probably this points to needing a 'pin' operation for the mapping >> selection after attach. > > If I'm understanding things correctly we're expecting a change in path between > attach and map time to be rare in practice. Therefore the need to handle this > case is mostly as a fallback to not break things. So instead of pinning could > new attachments just invalidate all existing incompatible attachments on the > list by setting eg. attach->valid = 0? No, that would result in ping/pong behavior which is something we explicitly try to avoid. This is also not something uncommon, it happens multiple times a second on a standard X11 Linux desktop when a new buffer is started to be scanned out on an A+A or A+I laptop. Regards, Christian. > > Attachment could also scan existing attachments to preferentially pick matching > mapping_types that are compatible with existing attachments, thus avoiding > invalidation where possible. > > Map time functions (eg. dma_buf_sgt_map_attachment) would check attach->valid > and return a failure if a subsequent attachment/event had invalidated the > mapping type. > > Importers with invalidated attachments would then have to retry creating the > attachment, probably getting a different mapping_type. Would this be acceptable? > Or do some importers absolutely require the mapping_type to never change once > the intial match/attachment has happened? > > - Alistair > >>>> +/* >>>> + * Match items are generated by the importer using the >>>> DMA_BUF_IMAPPING_*() and >>>> + * the exporter using the DMA_BUF_EMAPPING_*() functions. Each >>>> mapping type >>>> + * defines its own signature with its own data to make the match and >>>> attachment. >>>> + */ >>>> +struct dma_buf_mapping_match { >>>> + const struct dma_buf_mapping_type *type; >>>> + const struct dma_buf_mapping_exp_ops *exp_ops; >>>> + union { >>>> + /* Each mapping_type has unique match parameters >>>> here */ >>>> + }; >>> >>> Do we really need to add all global mapping types here? Then we'd need >>> something for driver-private mapping types as well. >> >> IIRC it is fixed size up front like this because of how the >> structure's memory is managed in this design.. >> >> If you want a driver private thing then I'd add a 'u64 >> driver_private[2]' to the union and the driver can cast over it. >> >> But this is the kernel, if you want to add an xe_mapping_type, then >> just add it. I don't know we need to do driver private things. >> >> Jason

8 hours, 20 minutes

Re: [PATCH RFC 01/26] dma-buf: Introduce DMA-buf mapping types

by Thomas Hellström

* On Thu, 2026-10-22 at 22:17 +1100, Alistair Popple wrote: > On 2026-10-22 at 18:25 +1100, Thomas Hellström > <thomas.hellstrom(a)linux.intel.com> wrote... > > On Thu, 2026-10-22 at 17:52 +1100, Alistair Popple wrote: > > > On 2026-10-21 at 23:37 +1100, Jason Gunthorpe <jgg(a)nvidia.com> > > > wrote... > > > > On Wed, Oct 21, 2026 at 08:24:40AM +0200, Thomas Hellström > > > > wrote: > > > > > > struct dma_buf_mapping_exp_ops > > > > > > Base type for the per-mapping type exporter provided > > > > > > functions. > > > > > > This > > > > > > would be the map/unmap callbacks. Each mapping type can > > > > > > provide > > > > > > its own > > > > > > functions for map/unmap type operations with optimal > > > > > > type > > > > > > signatures. > > > > > > > > > > Does it make sense to try to land this patch first, plus a > > > > > conversion > > > > > of pcie_p2p so we could agree on what the contract should > > > > > look > > > > > like? > > > > > > > > I think it would be good to focus on this patch as the main > > > > 'crux' > > > > of > > > > the series. The API contract is the main agreement. I'm > > > > relucant to > > > > merge code that has no users, so it woudl be ebtter to go up to > > > > at > > > > least through SGT. > > > > > > > > > s/mapping/interconnect/? My gut feeling is that > > > > > "interconnect" > > > > > best > > > > > describes what's typically going on, but "mapping" perhaps > > > > > better > > > > > describes all cases. > > > > > > > > My intention was to cover more than just the phyiscal > > > > transport. We > > > > do > > > > really want to get away from scatterlist, so having PCIe map > > > > with > > > > scatterlist and PCIe map with <something else> still ends up > > > > with > > > > two > > > > mapping types in the system. > > > > > > > > This is why I choose the name over interconnect. > > > > > > > > > Location of common protocols which typically carry cross- > > > > > subsystem > > > > > dependencies? > > > > > drivers/dma-buf/mapping_types? drivers/dma-buf/interconnects? > > > > > > > > I'd encourage people to design more like PCI where the actual > > > > *interconnect* has its own home and there is just some minor > > > > call > > > > outs > > > > from dmabuf to help the matching logic. > > > > > > > > > As discussed previously: > > > > > Agree on theoretically possible matches at attach time, Match > > > > > at > > > > > mapping time and guarantee the match is valid until > > > > > invalidate_mappings(). > > > > > > > > I was thinking about this some more, and I'm less sure now. > > > > > > > > If I think about mlx5, in a multi-path PCIe configuration, it > > > > can > > > > move > > > > the translation around within a single PCIe path, but it cannot > > > > move > > > > it to another path without creating disruption for the > > > > application. This is a HW limitation in how the internal > > > > routing > > > > works.. > > > > > > > > I strongly suspect this will be the norm for any hardware that > > > > doesn't > > > > rely on decorated page tables to select the IO path.. > > > > > > > > So, while I agree we should support hardware that can fully > > > > change > > > > the > > > > path on each and every map, we also need to suport hardware > > > > that > > > > must > > > > pin the path during attach. > > > > > > > > Probably this points to needing a 'pin' operation for the > > > > mapping > > > > selection after attach. > > > > > > If I'm understanding things correctly we're expecting a change in > > > path between > > > attach and map time to be rare in practice. Therefore the need to > > > handle this > > > case is mostly as a fallback to not break things. So instead of > > > pinning could > > > new attachments just invalidate all existing incompatible > > > attachments > > > on the > > > list by setting eg. attach->valid = 0? > > > > I agree, (although I think the "invalidate_attachment" needs to be > > discussed). In the end the importer should be given a choice, > > either > > pin and accept that subsequent attachers might see errors or > > accept > > app disruption. > > > > > > > > Attachment could also scan existing attachments to preferentially > > > pick matching > > > mapping_types that are compatible with existing attachments, thus > > > avoiding > > > invalidation where possible. > > > > Current design appears to be that attach only determines the set of > > theoretically possible mapping types, disregarding other > > attachments. > > Taking other attachments into account is also problematic because > > the > > dma-resv lock is not held. > > Oh I see. Do you foresee any problems just taking the dma-resv lock > to walk the > current attachments? If so we could probably resolve it with RCU, but > I'd rather > keep it simple if we can. It looks like dma_buf_dynamic_attach() actually takes the dma_resv lock to update the attachment list, so that should work. Provided we want to take existing attachments into account already at this stage. Need to take care, though, not to have the resv lock held over the ops- >attach callback into the driver. > > > Looking at current GPU drivers, what you describe above is > > typically > > done at pin() time, but if some importers want an early very likely > > decision on what mapping_type to use, one could introduce a > > separate > > function that match the mapping_types, but that would be very > > similar > > to the map() function but without returning a sgt/mapping, or a > > pin() > > without actually pinning. One could reuse the invalidate_mappings() > > callback to indicate the mapping type is no longer valid? > > I did wonder about that, but a callback for attachment invalidation > seems like > it might be a pain for importers to syncrhonize with. What I'm > thinking, which I > think is similar to what you describe, is something like the > following: > > 1. Importer gets the likley attachment by passing a list of mapping > types in > prefered order to dma_buf_mapping_attach() which returns the > dma_buf_attachment > that would work based on current attachments and what the exporter > provides: > > Something like this sounds OK, although given Jason's use-case, I added some comments: > attach = dma_buf_mapping_attach(...) > Just filter out impossible mappings. > 2. Takes dmabuf->resv lock. dma_buf_match_mapping_type(); Determines a mapping that is valid until the next invalidate_mappings(). NOP if a valid mapping already exists. Allows masking mapping_types. This allows Jason's use-case while keeping the attachment as a callback anchor. > > 3. Pins the attachment (if required): dma_buf_match_mapping_type(); Determines a mapping that is valid until the next invalidate_mappings(). NOP if a valid mapping already exists. Allows masking mapping_types. This allows Jason's use-case while keeping the attachment as a callback anchor. > > rc = dma_buf_pin(attach) This would implicitly pin also the mapping_type, but question is what happens if a link is severed? Then we'd want an invalidate_mappings() to re-negotiate the mapping type, but a pinned dma-buf that receives an invalidate_mappings() is revoked by definition. > > DMA-BUF core revalidates the attachment is still valid (ie. by > checking the > match again or via some other mechanism to detect the attachment > path/location > has changed such as a sequence/generation number). Since the resv is locked, nothing could have changed since dma_buf_match_mapping_type(); /Thomas > > On error the importer can either fail or try to create a new > attachment. > > 4. Map > > rc = dma_buf_[sgt|pcie_p2p]_map(attach, ...) > > Follows the same revalidation process as dma_buf_pin(). > > Thoughts? > > - Alistair > > > /Thomas > > > > > > > > > > Map time functions (eg. dma_buf_sgt_map_attachment) would check > > > attach->valid > > > and return a failure if a subsequent attachment/event had > > > invalidated > > > the > > > mapping type. > > > > > > Importers with invalidated attachments would then have to retry > > > creating the > > > attachment, probably getting a different mapping_type. Would this > > > be > > > acceptable? > > > Or do some importers absolutely require the mapping_type to never > > > change once > > > the intial match/attachment has happened? > > > > > > - Alistair > > > > > > > > > +/* > > > > > > + * Match items are generated by the importer using the > > > > > > DMA_BUF_IMAPPING_*() and > > > > > > + * the exporter using the DMA_BUF_EMAPPING_*() functions. > > > > > > Each > > > > > > mapping type > > > > > > + * defines its own signature with its own data to make the > > > > > > match and > > > > > > attachment. > > > > > > + */ > > > > > > +struct dma_buf_mapping_match { > > > > > > + const struct dma_buf_mapping_type *type; > > > > > > + const struct dma_buf_mapping_exp_ops *exp_ops; > > > > > > + union { > > > > > > + /* Each mapping_type has unique match > > > > > > parameters > > > > > > here */ > > > > > > + }; > > > > > > > > > > Do we really need to add all global mapping types here? Then > > > > > we'd > > > > > need > > > > > something for driver-private mapping types as well. > > > > > > > > IIRC it is fixed size up front like this because of how the > > > > structure's memory is managed in this design.. > > > > > > > > If you want a driver private thing then I'd add a 'u64 > > > > driver_private[2]' to the union and the driver can cast over > > > > it. > > > > > > > > But this is the kernel, if you want to add an xe_mapping_type, > > > > then > > > > just add it. I don't know we need to do driver private things. > > > > > > > > Jason

8 hours, 23 minutes

Re: [PATCH] dma-buf: Add DMA_BUF_IOCTL_SET_RELEASE_EVENTFD

by Christian König

On 10/21/26 22:16, Val Packett wrote: > > On 10/21/26 10:43 AM, Christian König wrote: >> [..] >>> The buffer lifecycle coordination we need this for applies to KVM as well >>> and it generally doesn't make sense to have this as a provider-specific >>> thing since it's not related to the provider in any way, it's just >>> something we want from dmabufs in general. >> I wouldn't put that on the DMA-buf file descriptor where basically anybody can mess with it. That is just an recipe for trouble. >> >> The better approach seems to be putting it into the udmabuf_ioctl(), so that from the very beginning you add the eventfd which signals when the udmabuf is released. >> >> If we need that for more implementations than just udmabuf, we can potentially add it to dma_buf_export_info so that exporters can provide the eventfd file descriptor directly on creation. > > Well, we already need it with both udmabuf and gntdev-dmabuf.. > > Involving the exporter sounds really annoying when it's not something exporter dependent really.. What if the policy is just "only one eventfd ever" i.e. the first call of the ioctl succeeds but later calls on the same dmabuf return an error? No, exactly that is what I'm trying to avoid. Nobody except the original creater should be able to insert an eventfd here. Regards, Christian. > That should remove the concern about one user messing with another's events (and avoid the concurrent replacement issue sashiko-bot warned about).. > > Thanks, > ~val >

9 hours, 54 minutes

Re: [PATCH RFC 01/26] dma-buf: Introduce DMA-buf mapping types

by Thomas Hellström

On Thu, 2026-10-22 at 17:52 +1100, Alistair Popple wrote: > On 2026-10-21 at 23:37 +1100, Jason Gunthorpe <jgg(a)nvidia.com> > wrote... > > On Wed, Oct 21, 2026 at 08:24:40AM +0200, Thomas Hellström wrote: > > > > struct dma_buf_mapping_exp_ops > > > > Base type for the per-mapping type exporter provided > > > > functions. > > > > This > > > > would be the map/unmap callbacks. Each mapping type can > > > > provide > > > > its own > > > > functions for map/unmap type operations with optimal type > > > > signatures. > > > > > > Does it make sense to try to land this patch first, plus a > > > conversion > > > of pcie_p2p so we could agree on what the contract should look > > > like? > > > > I think it would be good to focus on this patch as the main 'crux' > > of > > the series. The API contract is the main agreement. I'm relucant to > > merge code that has no users, so it woudl be ebtter to go up to at > > least through SGT. > > > > > s/mapping/interconnect/? My gut feeling is that "interconnect" > > > best > > > describes what's typically going on, but "mapping" perhaps better > > > describes all cases. > > > > My intention was to cover more than just the phyiscal transport. We > > do > > really want to get away from scatterlist, so having PCIe map with > > scatterlist and PCIe map with <something else> still ends up with > > two > > mapping types in the system. > > > > This is why I choose the name over interconnect. > > > > > Location of common protocols which typically carry cross- > > > subsystem > > > dependencies? > > > drivers/dma-buf/mapping_types? drivers/dma-buf/interconnects? > > > > I'd encourage people to design more like PCI where the actual > > *interconnect* has its own home and there is just some minor call > > outs > > from dmabuf to help the matching logic. > > > > > As discussed previously: > > > Agree on theoretically possible matches at attach time, Match at > > > mapping time and guarantee the match is valid until > > > invalidate_mappings(). > > > > I was thinking about this some more, and I'm less sure now. > > > > If I think about mlx5, in a multi-path PCIe configuration, it can > > move > > the translation around within a single PCIe path, but it cannot > > move > > it to another path without creating disruption for the > > application. This is a HW limitation in how the internal routing > > works.. > > > > I strongly suspect this will be the norm for any hardware that > > doesn't > > rely on decorated page tables to select the IO path.. > > > > So, while I agree we should support hardware that can fully change > > the > > path on each and every map, we also need to suport hardware that > > must > > pin the path during attach. > > > > Probably this points to needing a 'pin' operation for the mapping > > selection after attach. > > If I'm understanding things correctly we're expecting a change in > path between > attach and map time to be rare in practice. Therefore the need to > handle this > case is mostly as a fallback to not break things. So instead of > pinning could > new attachments just invalidate all existing incompatible attachments > on the > list by setting eg. attach->valid = 0? I agree, (although I think the "invalidate_attachment" needs to be discussed). In the end the importer should be given a choice, either pin and accept that subsequent attachers might see errors or accept app disruption. > > Attachment could also scan existing attachments to preferentially > pick matching > mapping_types that are compatible with existing attachments, thus > avoiding > invalidation where possible. Current design appears to be that attach only determines the set of theoretically possible mapping types, disregarding other attachments. Taking other attachments into account is also problematic because the dma-resv lock is not held. Looking at current GPU drivers, what you describe above is typically done at pin() time, but if some importers want an early very likely decision on what mapping_type to use, one could introduce a separate function that match the mapping_types, but that would be very similar to the map() function but without returning a sgt/mapping, or a pin() without actually pinning. One could reuse the invalidate_mappings() callback to indicate the mapping type is no longer valid? /Thomas > > Map time functions (eg. dma_buf_sgt_map_attachment) would check > attach->valid > and return a failure if a subsequent attachment/event had invalidated > the > mapping type. > > Importers with invalidated attachments would then have to retry > creating the > attachment, probably getting a different mapping_type. Would this be > acceptable? > Or do some importers absolutely require the mapping_type to never > change once > the intial match/attachment has happened? > > - Alistair > > > > > +/* > > > > + * Match items are generated by the importer using the > > > > DMA_BUF_IMAPPING_*() and > > > > + * the exporter using the DMA_BUF_EMAPPING_*() functions. Each > > > > mapping type > > > > + * defines its own signature with its own data to make the > > > > match and > > > > attachment. > > > > + */ > > > > +struct dma_buf_mapping_match { > > > > + const struct dma_buf_mapping_type *type; > > > > + const struct dma_buf_mapping_exp_ops *exp_ops; > > > > + union { > > > > + /* Each mapping_type has unique match > > > > parameters > > > > here */ > > > > + }; > > > > > > Do we really need to add all global mapping types here? Then we'd > > > need > > > something for driver-private mapping types as well. > > > > IIRC it is fixed size up front like this because of how the > > structure's memory is managed in this design.. > > > > If you want a driver private thing then I'd add a 'u64 > > driver_private[2]' to the union and the driver can cast over it. > > > > But this is the kernel, if you want to add an xe_mapping_type, then > > just add it. I don't know we need to do driver private things. > > > > Jason

10 hours, 9 minutes

[PATCH RFC v2 0/8] memcg: dma-buf cgroup accounting and charge transfer

by Albert Esteve

This series adds memory cgroup accounting for dma-buf allocations, building on T.J. Mercier's earlier series [1] and incorporating feedback from the v1 and LPC discussions. Patch 1 introduces the MEMCG_DMABUF stat and charge/uncharge helpers. Rather than charging at the page level via __GFP_ACCOUNT (which would require every dma-buf exporter to implement charging, and makes cross-cgroup transfer impractical without access to backing pages), we track charges externally on struct dma_buf via a MEMCG_DMABUF stat counter and route all accounting through a single path, removing the __GFP_ACCOUNT flag from system-heap allocations to avoid double counting. Patch 2 adds a charge_fd field to DMA_HEAP_IOCTL_ALLOC, accepting either a pidfd or a cgroupfd to identify the target cgroup at allocation time. If the target cgroup cannot accommodate the charge, the allocation returns -ENOMEM. Patches 3-4 add LSM hooks for the two charge_fd flavours: security_dma_buf_charge_task() gates the pidfd path (target identified by task credentials) and security_dma_buf_charge_cgroup() gates the cgroupfd path (target identified by cgroup directory label). Patches 5-6 implement the SELinux side under a dma_buf object class with charge_task and charge_cgroup permissions. For charge_cgroup, the target SID is derived from the cgroup directory's SELinux label. Patch 7 adds DMA_BUF_IOCTL_XFER_CHARGE, a general ioctl on any dma-buf fd to transfer charges (or charge) post-allocation. This covers the double-hop allocator case (SurfaceFlinger -> gralloc) and works for buffers from any exporter (DRM, V4L2, etc.). If the receiving cgroup lacks capacity for the charge, the ioctl returns -ENOMEM and the original charge is preserved. Patch 8 adds selftests. Known limitations: - Single-owner model: a dma-buf carries one memcg charge regardless of how many processes share it. - Only memcg accounting. CMA heaps would additionally need dmem controller integration. - Charging CMA-backed heaps to memcg charges unreclaimable memory: when a cgroup hits its limit, try_charge_memcg() may trigger a reclaim pass that cannot free CMA pages. A future version could move charging into per-heap callbacks. - If a transfer of an uncharged buffer fails, the buffer remains uncharged. The allocator (userspace) is expected to handle this (e.g., retry, free, ...). - The LSM hook is intentionally narrow. Broader cgroup access controls could be a separate effort. [1] https://lore.kernel.org/cgroups/20230109213809.418135-1-tjmercier@google.co… Signed-off-by: Albert Esteve <aesteve(a)redhat.com> --- Changes in v2: - Move charge_pid_fd to charge_fd, accepting cgroupfd in addition to pidfd per Christian König. - New DMA_BUF_IOCTL_XFER_CHARGE ioctl for post-allocation charge transfer on any dma-buf, not just dma-heap buffers per Christian König and T.J. Mercier. - Moved __GFP_ACCOUNT removal to patch 1 per Barry Song. - dma-buf charge/uncharge helpers take struct dma_buf * directly to avoid the need for caller to manage size calculations. - Added test_xfer_charge_cgroupfd_self to dmabuf-heap selftests. - Added LSM and SELinux hooks for cgroupfd-based charging. - Rebased to v7.3-rc7. - Link to v1: https://lore.kernel.org/r/20260512-v2_20230123_tjmercier_google_com-v1-0-63… --- Albert Esteve (7): dma-heap: charge dma-buf memory via explicit memcg security: dma-buf: Add dma_buf_charge_task LSM hook security: dma-buf: Add dma_buf_charge_cgroup LSM hook selinux: Restrict cross-cgroup dma-buf charging selinux: Restrict dma-buf direct cgroup charging dma-buf: transfer memcg charges via DMA_BUF_IOCTL_XFER_CHARGE selftests/dmabuf-heaps: Add dma-buf memcg accounting tests T.J. Mercier (1): memcg: Add MEMCG_DMABUF accounting infrastructure Documentation/admin-guide/cgroup-v2.rst | 5 + drivers/dma-buf/dma-buf.c | 80 ++++++++ drivers/dma-buf/dma-heap.c | 72 ++++++- drivers/dma-buf/heaps/system_heap.c | 2 - include/linux/dma-buf.h | 4 + include/linux/lsm_hook_defs.h | 2 + include/linux/memcontrol.h | 29 +++ include/linux/security.h | 14 ++ include/uapi/linux/dma-buf.h | 26 +++ include/uapi/linux/dma-heap.h | 7 + mm/memcontrol.c | 94 +++++++++ security/security.c | 32 +++ security/selinux/hooks.c | 40 ++++ security/selinux/include/classmap.h | 1 + tools/testing/selftests/cgroup/Makefile | 2 +- tools/testing/selftests/cgroup/test_memcontrol.c | 145 +++++++++++++- tools/testing/selftests/dmabuf-heaps/config | 1 + tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c | 222 ++++++++++++++++++++- tools/testing/selftests/dmabuf-heaps/vmtest.sh | 205 +++++++++++++++++++ 19 files changed, 974 insertions(+), 9 deletions(-) --- base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907 change-id: 20260508-v2_20230123_tjmercier_google_com-f44fcfb16530 Best regards, -- Albert Esteve <aesteve(a)redhat.com>

11 hours, 57 minutes

Re: [PATCH RFC 01/26] dma-buf: Introduce DMA-buf mapping types

by Thomas Hellström

On Thu, 2026-10-22 at 18:20 +1100, Alistair Popple wrote: > On 2026-10-22 at 17:59 +1100, Thomas Hellström > <thomas.hellstrom(a)linux.intel.com> wrote... > > On Wed, 2026-10-21 at 09:37 -0300, Jason Gunthorpe wrote: > > > On Wed, Oct 21, 2026 at 08:24:40AM +0200, Thomas Hellström wrote: > > > > > struct dma_buf_mapping_exp_ops > > > > > Base type for the per-mapping type exporter provided > > > > > functions. > > > > > This > > > > > would be the map/unmap callbacks. Each mapping type can > > > > > provide > > > > > its own > > > > > functions for map/unmap type operations with optimal type > > > > > signatures. > > > > > > > > Does it make sense to try to land this patch first, plus a > > > > conversion > > > > of pcie_p2p so we could agree on what the contract should look > > > > like? > > > > > > I think it would be good to focus on this patch as the main > > > 'crux' of > > > the series. The API contract is the main agreement. I'm relucant > > > to > > > merge code that has no users, so it woudl be ebtter to go up to > > > at > > > least through SGT. > > > > I agree about unused code, but wouldn't a pcie_p2p conversion to a > > mapping type be a valid use-case? > > In some ways I think having both sgt and the pcie_p2p conversion is > helpful as > it demonstrates how different mapping types are used. I agree the > important bit > is settling on the contract though. Adding a pcie_p2p conversion on > top doesn't > look too hard though. Thomas would you be ok if I rebased and posted > some of > your patches[1] adding pcie_p2p on top of the mapping types + SGT > conversion > v2 I'm getting ready to send to demonstrate usage? Or would you > rather do that > after I send? Please go ahead and use them as you wish. That was only a quick mostly AI-generated POC. Thanks, Thomas > > [1] - > https://gitlab.freedesktop.org/thomash/xe-vibe/-/commit/7452df0fc858a7dc5cc… > > - Alistair > > > > > > > > s/mapping/interconnect/? My gut feeling is that "interconnect" > > > > best > > > > describes what's typically going on, but "mapping" perhaps > > > > better > > > > describes all cases. > > > > > > My intention was to cover more than just the phyiscal transport. > > > We > > > do > > > really want to get away from scatterlist, so having PCIe map with > > > scatterlist and PCIe map with <something else> still ends up with > > > two > > > mapping types in the system. > > > > > > This is why I choose the name over interconnect. > > > > > > > Location of common protocols which typically carry cross- > > > > subsystem > > > > dependencies? > > > > drivers/dma-buf/mapping_types? drivers/dma-buf/interconnects? > > > > > > I'd encourage people to design more like PCI where the actual > > > *interconnect* has its own home and there is just some minor call > > > outs > > > from dmabuf to help the matching logic. > > > > > > > As discussed previously: > > > > Agree on theoretically possible matches at attach time, Match > > > > at > > > > mapping time and guarantee the match is valid until > > > > invalidate_mappings(). > > > > > > I was thinking about this some more, and I'm less sure now. > > > > > > If I think about mlx5, in a multi-path PCIe configuration, it can > > > move > > > the translation around within a single PCIe path, but it cannot > > > move > > > it to another path without creating disruption for the > > > application. This is a HW limitation in how the internal routing > > > works.. > > > > > > I strongly suspect this will be the norm for any hardware that > > > doesn't > > > rely on decorated page tables to select the IO path.. > > > > > > So, while I agree we should support hardware that can fully > > > change > > > the > > > path on each and every map, we also need to suport hardware that > > > must > > > pin the path during attach. > > > > > > Probably this points to needing a 'pin' operation for the mapping > > > selection after attach. > > > > > > > > +/* > > > > > + * Match items are generated by the importer using the > > > > > DMA_BUF_IMAPPING_*() and > > > > > + * the exporter using the DMA_BUF_EMAPPING_*() functions. > > > > > Each > > > > > mapping type > > > > > + * defines its own signature with its own data to make the > > > > > match > > > > > and > > > > > attachment. > > > > > + */ > > > > > +struct dma_buf_mapping_match { > > > > > + const struct dma_buf_mapping_type *type; > > > > > + const struct dma_buf_mapping_exp_ops *exp_ops; > > > > > + union { > > > > > + /* Each mapping_type has unique match > > > > > parameters > > > > > here */ > > > > > + }; > > > > > > > > Do we really need to add all global mapping types here? Then > > > > we'd > > > > need > > > > something for driver-private mapping types as well. > > > > > > IIRC it is fixed size up front like this because of how the > > > structure's memory is managed in this design.. > > > > > > If you want a driver private thing then I'd add a 'u64 > > > driver_private[2]' to the union and the driver can cast over it. > > > > > > But this is the kernel, if you want to add an xe_mapping_type, > > > then > > > just add it. I don't know we need to do driver private things. > > > > I guess it's my wish to separate components. Couldn't this lead to > > excessive includes and unnecessary dma-buf recompilations when > > mostly > > unrelated headers are touched? Except possibly for memory size, > > core > > dma-buf shouldn't need to know about this, it's a mapping private > > thing? > > > > /Thomas > > > > > > > > > > Jason

14 hours, 51 minutes

[PATCH RFC 00/26] Add DMA-buf mapping types and convert vfio/iommufd to use them

by Jason Gunthorpe

Since its introduction, DMA-buf has only supported using scatterlist for the exporter and importer to exchange address information. This is not sufficient for all use cases as dma_addr_t is a very specific and limited type that should not be abused for things unrelated to the DMA API. There are several motivations for addressing this now: 1) VFIO to IOMMUFD and KVM requires a physical address, not a dma_addr_t scatterlist, it cannot be represented in the scatterlist structure 2) xe vGPU requires the host driver to accept a DMABUF from VFIO of its own VF and convert it into an internal VRAM address on the PF 3) We are starting to look at replacement datastructures for scatterlist 4) Ideas around UALink/etc are suggesting not using the DMA API None of these can sanely be achieved using scatterlist. Introduce a new mechanism called "mapping types" which allows DMA-buf to work with more map/unmap options than scatterlist. Each mapping type encompasses a full set of functions and data unique to itself. The core code provides a match-making system to select the best type offered by the exporter and importer to be the active mapping type for the attachment. Everything related to scatterlist is moved into a DMA-buf SGT mapping type, and into the "dma_buf_sgt_*" namespace for clarity. Existing exporters are moved over to explicitly declare SGT mapping types and importers are adjusted to use the dma_buf_sgt_* named importer helpers. Mapping types are designed to be extendable, a driver can declare its own mapping type for its internal private interconnect and use that without having to adjust the core code. The new attachment sequence starts with the importing driver declaring what mapping types it can accept: struct dma_buf_mapping_match imp_match[] = { DMA_BUF_IMAPPING_MY_DRIVER(dev, ...), DMA_BUF_IMAPPING_SGT(dev, false), }; attach = dma_buf_mapping_attach(dmabuf, imp_match, ...) Most drivers will do this via a dma_buf_sgt_*attach() helper. The exporting driver can then declare what mapping types it can supply: int exporter_match_mapping(struct dma_buf_match_args *args) { struct dma_buf_mapping_match exp_match[] = { DMA_BUF_EMAPPING_MY_DRIVER(my_ops, dev, ...), DMA_BUF_EMAPPING_SGT(sgt_ops, dev, false), DMA_BUF_EMAPPING_PAL(PAL_ops), }; return dma_buf_match_mapping(args, exp_match, ...); } Most drivers will do this via a helper: static const struct dma_buf_ops ops = { DMA_BUF_SIMPLE_SGT_EXP_MATCH(map_func, unmap_func) }; During dma_buf_mapping_attach() the core code will select a mutual match between the importer and exporter and record it as the active match in the attach->map_type. Each mapping type has its own types/function calls for mapping/unmapping, and storage in the attach->map_type for its information. As such each mapping type can offer function signatures and data that exactly matches its needs. This series goes through a sequence of: 1) Introduce the basic mapping type framework and the main components of the SGT mapping type 2) Automatically make all existing exporters and importers use core generated SGT mapping types so every attachment has a SGT mapping type 3) Convert all exporter drivers to natively create a SGT mapping type 4) Move all dma_buf_* functions and types that are related to SGT into dma_buf_sgt_* 5) Remove all the now-unused items that have been moved into SGT specific structures. 6) Demonstrate adding a new Physical Address List alongside SGT. Due to the high number of files touched I would expect this to be broken into phases, but this shows the entire picture. This is on github: https://github.com/jgunthorpe/linux/commits/dmabuf_map_type It is a followup to the discussion here: https://lore.kernel.org/dri-devel/20251027044712.1676175-1-vivek.kasireddy@… Jason Gunthorpe (26): dma-buf: Introduce DMA-buf mapping types dma-buf: Add the SGT DMA mapping type dma-buf: Add dma_buf_mapping_attach() dma-buf: Route SGT related actions through attach->map_type dma-buf: Allow single exporter drivers to avoid the match_mapping function drm: Check the SGT ops for drm_gem_map_dma_buf() dma-buf: Convert all the simple exporters to use SGT mapping type drm/vmwgfx: Use match_mapping instead of dummy calls accel/habanalabs: Use the SGT mapping type drm/xe/dma-buf: Use the SGT mapping type drm/amdgpu: Use the SGT mapping type vfio/pci: Change the DMA-buf exporter to use mapping_type dma-buf: Update dma_buf_phys_vec_to_sgt() to use the SGT mapping type iio: buffer: convert to use the SGT mapping type functionfs: convert to use the SGT mapping type dma-buf: Remove unused SGT stuff from the common structures treewide: Rename dma_buf_map_attachment(_unlocked) to dma_buf_sgt_ treewide: Rename dma_buf_unmap_attachment(_unlocked) to dma_buf_sgt_* treewide: Rename dma_buf_attach() to dma_buf_sgt_attach() treewide: Rename dma_buf_dynamic_attach() to dma_buf_sgt_dynamic_attach() dma-buf: Add the Physical Address List DMA mapping type vfio/pci: Add physical address list support to DMABUF iommufd: Use the PAL mapping type instead of a vfio function iommufd: Support DMA-bufs with multiple physical ranges iommufd/selftest: Check multi-phys DMA-buf scenarios dma-buf: Add kunit tests for mapping type Documentation/gpu/todo.rst | 2 +- drivers/accel/amdxdna/amdxdna_gem.c | 14 +- drivers/accel/amdxdna/amdxdna_ubuf.c | 10 +- drivers/accel/habanalabs/common/memory.c | 54 ++- drivers/accel/ivpu/ivpu_gem.c | 10 +- drivers/accel/ivpu/ivpu_gem_userptr.c | 11 +- drivers/accel/qaic/qaic_data.c | 8 +- drivers/dma-buf/Makefile | 1 + drivers/dma-buf/dma-buf-mapping.c | 186 ++++++++- drivers/dma-buf/dma-buf.c | 180 ++++++--- drivers/dma-buf/heaps/cma_heap.c | 12 +- drivers/dma-buf/heaps/system_heap.c | 13 +- drivers/dma-buf/st-dma-mapping.c | 373 ++++++++++++++++++ drivers/dma-buf/udmabuf.c | 8 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 98 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 +- drivers/gpu/drm/armada/armada_gem.c | 33 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/drm_prime.c | 31 +- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 18 +- drivers/gpu/drm/i915/gem/i915_gem_object.c | 2 +- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 8 +- .../gpu/drm/i915/gem/selftests/mock_dmabuf.c | 8 +- drivers/gpu/drm/msm/msm_gem_prime.c | 7 +- drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 11 +- drivers/gpu/drm/tegra/gem.c | 33 +- drivers/gpu/drm/virtio/virtgpu_prime.c | 23 +- drivers/gpu/drm/vmwgfx/vmwgfx_prime.c | 32 +- drivers/gpu/drm/xe/xe_bo.c | 18 +- drivers/gpu/drm/xe/xe_dma_buf.c | 61 +-- drivers/iio/industrialio-buffer.c | 15 +- drivers/infiniband/core/umem_dmabuf.c | 15 +- drivers/iommu/iommufd/io_pagetable.h | 4 +- drivers/iommu/iommufd/iommufd_private.h | 8 - drivers/iommu/iommufd/iommufd_test.h | 7 + drivers/iommu/iommufd/pages.c | 85 ++-- drivers/iommu/iommufd/selftest.c | 177 ++++++--- .../media/common/videobuf2/videobuf2-core.c | 2 +- .../common/videobuf2/videobuf2-dma-contig.c | 26 +- .../media/common/videobuf2/videobuf2-dma-sg.c | 21 +- .../common/videobuf2/videobuf2-vmalloc.c | 13 +- .../platform/nvidia/tegra-vde/dmabuf-cache.c | 9 +- drivers/misc/fastrpc.c | 21 +- drivers/tee/tee_heap.c | 13 +- drivers/usb/gadget/function/f_fs.c | 11 +- drivers/vfio/pci/vfio_pci_dmabuf.c | 79 ++-- drivers/xen/gntdev-dmabuf.c | 29 +- include/linux/dma-buf-mapping.h | 297 ++++++++++++++ include/linux/dma-buf.h | 168 ++++---- io_uring/zcrx.c | 9 +- net/core/devmem.c | 14 +- samples/vfio-mdev/mbochs.c | 10 +- sound/soc/fsl/fsl_asrc_m2m.c | 12 +- tools/testing/selftests/iommu/iommufd.c | 43 ++ tools/testing/selftests/iommu/iommufd_utils.h | 17 + 55 files changed, 1764 insertions(+), 614 deletions(-) create mode 100644 drivers/dma-buf/st-dma-mapping.c base-commit: c63e5a50e1dd291cd95b04291b028fdcaba4c534 -- 2.43.0

15 hours, 21 minutes

Re: [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access

by Tomasz Figa

On Mon, Oct 19, 2026 at 4:14 PM Karl Mehltretter <kmehltretter(a)gmail.com> wrote: > > Drivers must bracket CPU access to imported DMA-BUFs with > dma_buf_begin_cpu_access() and dma_buf_end_cpu_access(). > > Add a queue flag to do so from buf_prepare() through buf_finish(). Use > read access for output and write access for capture. Map imports > bidirectionally because CPU and device access directions are opposite. > Pass the mapping direction to attach_dmabuf() so USERPTR and MMAP keep > the queue's normal direction. This isn't right. For the regular vb2 mem ops (dma-sg, dma-contig), a buffer in the prepared state is expected to be ready for hardware usage, meaning only the hardware can access it until the buffer is finished. The vmalloc ops are special in a way that CPU is the "hardware". > > Keep this opt-in because some vb2-vmalloc drivers pass buffers to > hardware. End access in process context because DMA-BUF callbacks may > sleep. That is at least unexpected, but also probably wrong (I haven't checked all the drivers) - the vmalloc mem ops are expected to be used by drivers that only access the buffer contents from the CPU. Putting all the above together, what I think we need is: 1) prepare/finish ops in vb2_vmalloc_memops to invoke the dma_buf_begin/end_cpu_access() API if the buffer is backed by a DMA-buf, 2) some vb2 API for drivers using the regular (dma-) memops to invoke the dma_buf_begin/end_cpu_access() API for them when they need to perform some CPU accesses to the buffers - typically they would do that in their vb2_ops::buf_prepare/finish callbacks, but definitelly must not after the buffer is already prepared. It sounds like this series focuses on uvcvideo and vivid, which likely means that 1) alone would be enough to fix the original problem. WDYT? Best, Tomasz > > Assisted-by: LLM > Signed-off-by: Karl Mehltretter <kmehltretter(a)gmail.com> > --- > .../media/common/videobuf2/videobuf2-core.c | 83 ++++++++++++++++++- > .../common/videobuf2/videobuf2-dma-contig.c | 5 +- > .../media/common/videobuf2/videobuf2-dma-sg.c | 5 +- > .../common/videobuf2/videobuf2-vmalloc.c | 5 +- > include/media/videobuf2-core.h | 12 ++- > 5 files changed, 101 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index 460eaf4e82f78..d2a78cd0a86bb 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -371,6 +371,65 @@ static void __vb2_buf_mem_finish(struct vb2_buffer *vb) > call_void_memop(vb, finish, vb->planes[plane].mem_priv); > } > > +/* Output buffers are read by the driver; capture buffers are written. */ > +static enum dma_data_direction vb2_cpu_access_dir(const struct vb2_queue *q) > +{ > + return q->is_output ? DMA_FROM_DEVICE : DMA_TO_DEVICE; > +} > + > +static bool __vb2_dmabuf_is_duplicate(const struct vb2_buffer *vb, > + unsigned int plane) > +{ > + unsigned int i; > + > + for (i = 0; i < plane; i++) > + if (vb->planes[i].dbuf == vb->planes[plane].dbuf) > + return true; > + > + return false; > +} > + > +static void __vb2_buf_end_cpu_access(struct vb2_buffer *vb, > + unsigned int num_planes) > +{ > + struct vb2_queue *q = vb->vb2_queue; > + enum dma_data_direction dir = vb2_cpu_access_dir(q); > + unsigned int plane; > + > + for (plane = 0; plane < num_planes; plane++) { > + int ret; > + > + if (__vb2_dmabuf_is_duplicate(vb, plane)) > + continue; > + > + ret = dma_buf_end_cpu_access(vb->planes[plane].dbuf, dir); > + if (ret) > + pr_warn_ratelimited("[%s] failed to end CPU access to dmabuf plane %u: %d\n", > + q->name, plane, ret); > + } > +} > + > +static int __vb2_buf_begin_cpu_access(struct vb2_buffer *vb) > +{ > + struct vb2_queue *q = vb->vb2_queue; > + enum dma_data_direction dir = vb2_cpu_access_dir(q); > + unsigned int plane; > + int ret; > + > + for (plane = 0; plane < vb->num_planes; plane++) { > + if (__vb2_dmabuf_is_duplicate(vb, plane)) > + continue; > + > + ret = dma_buf_begin_cpu_access(vb->planes[plane].dbuf, dir); > + if (ret) { > + __vb2_buf_end_cpu_access(vb, plane); > + return ret; > + } > + } > + > + return 0; > +} > + > /* > * __setup_offsets() - setup unique offsets ("cookies") for every plane in > * the buffer. > @@ -1423,6 +1482,9 @@ static int __prepare_dmabuf(struct vb2_buffer *vb) > } > > if (reacquired) { > + enum dma_data_direction attach_dir = q->uses_cpu_access ? > + DMA_BIDIRECTIONAL : q->dma_dir; > + > if (vb->planes[0].mem_priv) { > vb->copied_timestamp = 0; > call_void_vb_qop(vb, buf_cleanup, vb); > @@ -1453,7 +1515,8 @@ static int __prepare_dmabuf(struct vb2_buffer *vb) > vb, > q->alloc_devs[plane] ? : q->dev, > planes[plane].dbuf, > - planes[plane].length); > + planes[plane].length, > + attach_dir); > if (IS_ERR(mem_priv)) { > dprintk(q, 1, "failed to attach dmabuf\n"); > ret = PTR_ERR(mem_priv); > @@ -1504,9 +1567,21 @@ static int __prepare_dmabuf(struct vb2_buffer *vb) > } > } > > + if (q->uses_cpu_access) { > + ret = __vb2_buf_begin_cpu_access(vb); > + if (ret) { > + dprintk(q, 1, "failed to begin CPU access to dmabuf: %d\n", > + ret); > + call_void_vb_qop(vb, buf_cleanup, vb); > + goto err_put_vb2_buf; > + } > + } > + > ret = call_vb_qop(vb, buf_prepare, vb); > if (ret) { > dprintk(q, 1, "buffer preparation failed\n"); > + if (q->uses_cpu_access) > + __vb2_buf_end_cpu_access(vb, vb->num_planes); > call_void_vb_qop(vb, buf_cleanup, vb); > goto err_put_vb2_buf; > } > @@ -1756,6 +1831,8 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c > > __vb2_buf_mem_finish(vb); > call_void_vb_qop(vb, buf_finish, vb); > + if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access) > + __vb2_buf_end_cpu_access(vb, vb->num_planes); > vb->prepared = 0; > } > > @@ -2160,6 +2237,8 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb, > } > > call_void_vb_qop(vb, buf_finish, vb); > + if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access) > + __vb2_buf_end_cpu_access(vb, vb->num_planes); > vb->prepared = 0; > > if (pindex) > @@ -2296,6 +2375,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q) > > if (vb->prepared) { > call_void_vb_qop(vb, buf_finish, vb); > + if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access) > + __vb2_buf_end_cpu_access(vb, vb->num_planes); > vb->prepared = 0; > } > __vb2_dqbuf(vb); > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > index 9ce6284cd5f26..68c146c043fb1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > @@ -768,7 +768,8 @@ static void vb2_dc_detach_dmabuf(void *mem_priv) > } > > static void *vb2_dc_attach_dmabuf(struct vb2_buffer *vb, struct device *dev, > - struct dma_buf *dbuf, unsigned long size) > + struct dma_buf *dbuf, unsigned long size, > + enum dma_data_direction dma_dir) > { > struct vb2_dc_buf *buf; > struct dma_buf_attachment *dba; > @@ -794,7 +795,7 @@ static void *vb2_dc_attach_dmabuf(struct vb2_buffer *vb, struct device *dev, > return dba; > } > > - buf->dma_dir = vb->vb2_queue->dma_dir; > + buf->dma_dir = dma_dir; > buf->size = size; > buf->db_attach = dba; > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > index b1d0695cda260..ab03f9d31391a 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > @@ -616,7 +616,8 @@ static void vb2_dma_sg_detach_dmabuf(void *mem_priv) > } > > static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev, > - struct dma_buf *dbuf, unsigned long size) > + struct dma_buf *dbuf, unsigned long size, > + enum dma_data_direction dma_dir) > { > struct vb2_dma_sg_buf *buf; > struct dma_buf_attachment *dba; > @@ -640,7 +641,7 @@ static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev, > return dba; > } > > - buf->dma_dir = vb->vb2_queue->dma_dir; > + buf->dma_dir = dma_dir; > buf->size = size; > buf->db_attach = dba; > buf->vb = vb; > diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c > index 63f023696f3ee..c38faa0188bc9 100644 > --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c > +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c > @@ -402,7 +402,8 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv) > static void *vb2_vmalloc_attach_dmabuf(struct vb2_buffer *vb, > struct device *dev, > struct dma_buf *dbuf, > - unsigned long size) > + unsigned long size, > + enum dma_data_direction dma_dir) > { > struct vb2_vmalloc_buf *buf; > > @@ -414,7 +415,7 @@ static void *vb2_vmalloc_attach_dmabuf(struct vb2_buffer *vb, > return ERR_PTR(-ENOMEM); > > buf->dbuf = dbuf; > - buf->dma_dir = vb->vb2_queue->dma_dir; > + buf->dma_dir = dma_dir; > buf->size = size; > > return buf; > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 4b4f4c15c53a7..8dfe22fa4423c 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -78,7 +78,8 @@ struct vb2_buffer; > * to the userspace, also optional. > * @attach_dmabuf: attach a shared &struct dma_buf for a hardware operation; > * used for DMABUF memory types; dev is the alloc device > - * dbuf is the shared dma_buf; returns ERR_PTR() on failure; > + * dbuf is the shared dma_buf; dma_dir is the DMA mapping > + * direction; returns ERR_PTR() on failure; > * allocator private per-buffer structure on success; > * this needs to be used for further accesses to the buffer. > * @detach_dmabuf: inform the exporter of the buffer that the current DMABUF > @@ -136,7 +137,8 @@ struct vb2_mem_ops { > void *(*attach_dmabuf)(struct vb2_buffer *vb, > struct device *dev, > struct dma_buf *dbuf, > - unsigned long size); > + unsigned long size, > + enum dma_data_direction dma_dir); > void (*detach_dmabuf)(void *buf_priv); > int (*map_dmabuf)(void *buf_priv); > void (*unmap_dmabuf)(void *buf_priv); > @@ -510,6 +512,11 @@ struct vb2_buf_ops { > * ->finish(). > * @non_coherent_mem: when set queue will attempt to allocate buffers using > * non-coherent memory. > + * @uses_cpu_access: the driver transfers buffer data with the CPU instead of > + * DMA. For imported dma-bufs, videobuf2 brackets the interval from > + * buf_prepare through buf_finish with dma_buf_begin_cpu_access() > + * and dma_buf_end_cpu_access(). Imported attachments use > + * bidirectional DMA mappings. > * @lock: pointer to a mutex that protects the &struct vb2_queue. The > * driver must set this to a mutex to let the v4l2 core serialize > * the queuing ioctls. This lock is used when waiting for a new > @@ -614,6 +621,7 @@ struct vb2_queue { > unsigned int uses_requests:1; > unsigned int allow_cache_hints:1; > unsigned int non_coherent_mem:1; > + unsigned int uses_cpu_access:1; > > struct mutex *lock; > void *owner; > -- > 2.47.3 >

1 day, 1 hour

Re: [PATCH 1/4] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS

by Tomasz Figa

Hi Karl, On Mon, Oct 19, 2026 at 4:14 PM Karl Mehltretter <kmehltretter(a)gmail.com> wrote: > > VIDIOC_PREPARE_BUF leaves a buffer DEQUEUED after calling buf_prepare() > and the prepare() memory operation. VIDIOC_REMOVE_BUFS accepts that state > but frees the buffer without calling the matching finish operations. > > With VIDEO_ADV_DEBUG, vivid reports: > > unbalanced counters for queue 000000009fa716f8, buffer 0: > buf_prepare: 1 buf_finish: 0 > prepare: 1 finish: 0 > > Finish prepared buffers before freeing them, as queue cancellation does. > > Fixes: a3293a85381e ("media: v4l2: Add REMOVE_BUFS ioctl") > Assisted-by: LLM > Signed-off-by: Karl Mehltretter <kmehltretter(a)gmail.com> > --- > drivers/media/common/videobuf2/videobuf2-core.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) Thanks for the patch. Please take a look at my comments inline. > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index b0a6084f17573..460eaf4e82f78 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -1746,6 +1746,19 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c > goto unlock; > } > } > + > + /* A buffer can be prepared without being queued. Finish it first. */ > + for (i = start; i < start + count; i++) { > + struct vb2_buffer *vb = vb2_get_buffer(q, i); > + > + if (!vb->prepared) > + continue; > + > + __vb2_buf_mem_finish(vb); > + call_void_vb_qop(vb, buf_finish, vb); > + vb->prepared = 0; > + } > + __vb2_queue_cancel() does much more than that, especially all the media request handling. In addition, this open coded handling just causes code duplication. Would it make more sense to refactor the relevant part of the buffer cancelation loop from __vb2_queue_cancel() into a helper function and call it here instead? Best, Tomasz > __vb2_queue_free(q, start, count); > dprintk(q, 2, "%u buffers removed\n", count); > > -- > 2.47.3 >

1 day, 6 hours

[PATCH] dma-buf: Restore the scatterlist entry count before freeing

by Leon Romanovsky

From: Leon Romanovsky <leonro(a)nvidia.com> dma_buf_phys_vec_to_sgt() clears orig_nents to mark the returned table as DMA-only. Since sg_free_table() uses orig_nents to walk and free the scatterlist, dma_buf_free_sgt() leaks the allocation on every successful map/unmap cycle. Restore orig_nents from nents before freeing the table. The helper allocates the DMA list directly, so nents retains the allocation count, including for chained scatterlists. The reported kmemleak output was: KMEMLEAK LEAK: 2 driver-related leak(s) KMEMLEAK LEAK: unreferenced object 0xff11000125ceff00 (size 32): KMEMLEAK LEAK: comm dtr, pid 4432, jiffies 4295031027 KMEMLEAK LEAK: hex dump (first 32 bytes): KMEMLEAK LEAK: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ KMEMLEAK LEAK: 00 00 01 00 28 38 00 00 00 10 00 00 00 00 00 00 ....(8.......... KMEMLEAK LEAK: backtrace (crc 4e8077f2): KMEMLEAK LEAK: __kmalloc_noprof+0x451/0x8b0 KMEMLEAK LEAK: __sg_alloc_table+0x2a0/0x4d0 KMEMLEAK LEAK: sg_alloc_table+0x24/0x170 KMEMLEAK LEAK: dma_buf_phys_vec_to_sgt+0x84f/0xed0 KMEMLEAK LEAK: 0xffffffffa1ad21a8 KMEMLEAK LEAK: dma_buf_map_attachment+0x138/0x690 KMEMLEAK LEAK: ib_umem_dmabuf_map_pages+0x249/0xc90 [ib_core] KMEMLEAK LEAK: 0xffffffffa138f8d2 KMEMLEAK LEAK: 0xffffffffa1396abe KMEMLEAK LEAK: 0xffffffffa1331a5f KMEMLEAK LEAK: 0xffffffffa13320dd KMEMLEAK LEAK: 0xffffffffa1ad696d KMEMLEAK LEAK: 0xffffffffa1acecc7 KMEMLEAK LEAK: 0xffffffffa1acfd17 KMEMLEAK LEAK: __x64_sys_ioctl+0x72c/0x11f0 KMEMLEAK LEAK: do_syscall_64+0xa1/0x4f0 KMEMLEAK LEAK: unreferenced object 0xff1100016dbf3a80 (size 32): KMEMLEAK LEAK: comm dtr, pid 4432, jiffies 4295034705 KMEMLEAK LEAK: hex dump (first 32 bytes): KMEMLEAK LEAK: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ KMEMLEAK LEAK: 00 20 10 00 28 38 00 00 00 10 00 00 00 00 00 00 . ..(8.......... Fixes: 3aa31a8bb11e ("dma-buf: provide phys_vec to scatter-gather mapping routine") Signed-off-by: Leon Romanovsky <leonro(a)nvidia.com> --- drivers/dma-buf/dma-buf-mapping.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c index 833be519e1e6..cbdf99cb0e05 100644 --- a/drivers/dma-buf/dma-buf-mapping.c +++ b/drivers/dma-buf/dma-buf-mapping.c @@ -251,6 +251,8 @@ void dma_buf_free_sgt(struct dma_buf_attachment *attach, struct sg_table *sgt, sg_dma_len(sgl), dir, DMA_ATTR_MMIO); } + /* Restore the allocation count cleared for the DMA-only table. */ + sgt->orig_nents = sgt->nents; sg_free_table(sgt); kfree(dma->state); kfree(dma); --- base-commit: 39d3758108bb0ef9da484062d65288bbe9cada0e change-id: 20261013-dma-buf-fixes-99dd3fd00f48 Best regards, -- Leon Romanovsky <leonro(a)nvidia.com>

1 day, 7 hours

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig