Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

14 participants
2902 discussions

[PATCH v5 0/2] dma-buf: heaps: Support carved-out heaps

by Maxime Ripard

Hi, This series is the follow-up of the discussion that John and I had some time ago here: https://lore.kernel.org/all/CANDhNCquJn6bH3KxKf65BWiTYLVqSd9892-xtFDHHqqyrr… The initial problem we were discussing was that I'm currently working on a platform which has a memory layout with ECC enabled. However, enabling the ECC has a number of drawbacks on that platform: lower performance, increased memory usage, etc. So for things like framebuffers, the trade-off isn't great and thus there's a memory region with ECC disabled to allocate from for such use cases. After a suggestion from John, I chose to first start using heap allocations flags to allow for userspace to ask for a particular ECC setup. This is then backed by a new heap type that runs from reserved memory chunks flagged as such, and the existing DT properties to specify the ECC properties. After further discussion, it was considered that flags were not the right solution, and relying on the names of the heaps would be enough to let userspace know the kind of buffer it deals with. Thus, even though the uAPI part of it has been dropped in this second version, we still need a driver to create heaps out of carved-out memory regions. In addition to the original usecase, a similar driver can be found in BSPs from most vendors, so I believe it would be a useful addition to the kernel. Let me know what you think, Maxime Signed-off-by: Maxime Ripard <mripard(a)kernel.org> --- Changes in v5: - Rebased on 6.16-rc2 - Switch from property to dedicated binding - Link to v4: https://lore.kernel.org/r/20250520-dma-buf-ecc-heap-v4-1-bd2e1f1bb42c@kerne… Changes in v4: - Rebased on 6.15-rc7 - Map buffers only when map is actually called, not at allocation time - Deal with restricted-dma-pool and shared-dma-pool - Reword Kconfig options - Properly report dma_map_sgtable failures - Link to v3: https://lore.kernel.org/r/20250407-dma-buf-ecc-heap-v3-0-97cdd36a5f29@kerne… Changes in v3: - Reworked global variable patch - Link to v2: https://lore.kernel.org/r/20250401-dma-buf-ecc-heap-v2-0-043fd006a1af@kerne… Changes in v2: - Add vmap/vunmap operations - Drop ECC flags uapi - Rebase on top of 6.14 - Link to v1: https://lore.kernel.org/r/20240515-dma-buf-ecc-heap-v1-0-54cbbd049511@kerne… --- Maxime Ripard (2): dt-bindings: reserved-memory: Introduce carved-out memory region binding dma-buf: heaps: Introduce a new heap for reserved memory .../bindings/reserved-memory/carved-out.yaml | 49 +++ drivers/dma-buf/heaps/Kconfig | 8 + drivers/dma-buf/heaps/Makefile | 1 + drivers/dma-buf/heaps/carveout_heap.c | 362 +++++++++++++++++++++ 4 files changed, 420 insertions(+) --- base-commit: d076bed8cb108ba2236d4d49c92303fda4036893 change-id: 20240515-dma-buf-ecc-heap-28a311d2c94e Best regards, -- Maxime Ripard <mripard(a)kernel.org>

1 week, 1 day

[PATCH v2] Documentation: dma-buf: heaps: Add naming guidelines

by Maxime Ripard

We've discussed a number of times of how some heap names are bad, but not really what makes a good heap name. Let's document what we expect the heap names to look like. Reviewed-by: Bagas Sanjaya <bagasdotme(a)gmail.com> Signed-off-by: Maxime Ripard <mripard(a)kernel.org> --- Changes in v2: - Added justifications for each requirement / suggestions - Added a mention and example of buffer attributes - Link to v1: https://lore.kernel.org/r/20250520-dma-buf-heap-names-doc-v1-1-ab31f74809ee… --- Documentation/userspace-api/dma-buf-heaps.rst | 38 +++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst index 535f49047ce6450796bf4380c989e109355efc05..835ad1c3a65bc07b6f41d387d85c57162909e859 100644 --- a/Documentation/userspace-api/dma-buf-heaps.rst +++ b/Documentation/userspace-api/dma-buf-heaps.rst @@ -21,5 +21,43 @@ following heaps: usually created either through the kernel commandline through the `cma` parameter, a memory region Device-Tree node with the `linux,cma-default` property set, or through the `CMA_SIZE_MBYTES` or `CMA_SIZE_PERCENTAGE` Kconfig options. Depending on the platform, it might be called ``reserved``, ``linux,cma``, or ``default-pool``. + +Naming Convention +================= + +``dma-buf`` heaps name should meet a number of constraints: + +- That name must be stable, and must not change from one version to the + other. Userspace identifies heaps by their name, so if the names ever + changes, we would be likely to introduce regressions. + +- That name must describe the memory region the heap will allocate from, + and must uniquely identify it in a given platform. Since userspace + applications use the heap name as the discriminant, it must be able to + tell which heap it wants to use reliably if there's multiple heaps. + +- That name must not mention implementation details, such as the + allocator. The heap driver will change over time, and implementation + details when it was introduced might not be relevant in the future. + +- The name should describe properties of the buffers that would be + allocated. Doing so will make heap identification easier for + userspace. Such properties are: + + - ``cacheable`` / ``uncacheable`` for buffers with CPU caches enabled + or disabled; + + - ``contiguous`` for physically contiguous buffers; + + - ``protected`` for encrypted buffers not accessible the OS; + +- The name may describe intended usage. Doing so will make heap + identification easier for userspace applications and users. + +For example, assuming a platform with a reserved memory region located +at the RAM address 0x42000000, intended to allocate video framebuffers, +physically contiguous, and backed by the CMA kernel allocator. Good +names would be ``memory@42000000-cacheable-contiguous`` or +``video@42000000``, but ``cma-video`` wouldn't. --- base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 change-id: 20250520-dma-buf-heap-names-doc-31261aa0cfe6 Best regards, -- Maxime Ripard <mripard(a)kernel.org>

1 week, 2 days

[PATCH] Documentation: dma-buf: heaps: Add naming guidelines

by Maxime Ripard

We've discussed a number of times of how some heap names are bad, but not really what makes a good heap name. Let's document what we expect the heap names to look like. Signed-off-by: Maxime Ripard <mripard(a)kernel.org> --- Documentation/userspace-api/dma-buf-heaps.rst | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst index 535f49047ce6450796bf4380c989e109355efc05..b24618e360a9a9ba0bd85135d8c1760776f1a37f 100644 --- a/Documentation/userspace-api/dma-buf-heaps.rst +++ b/Documentation/userspace-api/dma-buf-heaps.rst @@ -21,5 +21,24 @@ following heaps: usually created either through the kernel commandline through the `cma` parameter, a memory region Device-Tree node with the `linux,cma-default` property set, or through the `CMA_SIZE_MBYTES` or `CMA_SIZE_PERCENTAGE` Kconfig options. Depending on the platform, it might be called ``reserved``, ``linux,cma``, or ``default-pool``. + +Naming Convention +================= + +A good heap name is a name that: + +- Is stable, and won't change from one version to the other; + +- Describes the memory region the heap will allocate from, and will + uniquely identify it in a given platform; + +- Doesn't use implementation details, such as the allocator; + +- Can describe intended usage. + +For example, assuming a platform with a reserved memory region located +at the RAM address 0x42000000, intended to allocate video framebuffers, +and backed by the CMA kernel allocator. Good names would be +`memory@42000000` or `video@42000000`, but `cma-video` wouldn't. --- base-commit: 92a09c47464d040866cf2b4cd052bc60555185fb change-id: 20250520-dma-buf-heap-names-doc-31261aa0cfe6 Best regards, -- Maxime Ripard <mripard(a)kernel.org>

1 week, 2 days

Re: [PATCH v4 0/4] Implement dmabuf direct I/O via copy_file_range

by Christoph Hellwig

On Fri, Jun 13, 2025 at 09:33:55AM +0000, wangtao wrote: > > > > > On Mon, Jun 09, 2025 at 09:32:20AM +0000, wangtao wrote: > > > Are you suggesting adding an ITER_DMABUF type to iov_iter, > > > > Yes. > > May I clarify: Do all disk operations require data to pass through > memory (reading into memory or writing from memory)? In the block layer, > the bio structure uses bio_iov_iter_get_pages to convert iter_type > objects into memory-backed bio_vec representations. > However, some dmabufs are not memory-based, making page-to-bio_vec > conversion impossible. This suggests adding a callback function in > dma_buf_ops to handle dmabuf- to-bio_vec conversion. bios do support PCI P2P tranfers. This could be fairly easily extended to other peer to peer transfers if we manage to come up with a coherent model for them. No need for a callback.

1 week, 3 days

Re: [PATCH v4 0/4] Implement dmabuf direct I/O via copy_file_range

by Christoph Hellwig

On Fri, Jun 13, 2025 at 09:43:08AM +0000, wangtao wrote: > Here's my analysis on Linux 6.6 with F2FS/iomap. Linux 6.6 is almost two years old and completely irrelevant. Please provide numbers on 6.16 or current Linus' tree.

1 week, 3 days

Re: [PATCH v5 03/12] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF

by Andrew Davis

On 5/27/25 1:56 AM, Amirreza Zarrabi wrote: > For drivers that can transfer data to the TEE without using shared > memory from client, it is necessary to receive the user address > directly, bypassing any processing by the TEE subsystem. Introduce > TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT/OUTPUT/INOUT to represent > userspace buffers. > Could you expand on this, what is the issue with normal MEMREF? Andrew > Reviewed-by: Sumit Garg <sumit.garg(a)oss.qualcomm.com> > Tested-by: Neil Armstrong <neil.armstrong(a)linaro.org> > Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi(a)oss.qualcomm.com> > --- > drivers/tee/tee_core.c | 33 +++++++++++++++++++++++++++++++++ > include/linux/tee_drv.h | 6 ++++++ > include/uapi/linux/tee.h | 22 ++++++++++++++++------ > 3 files changed, 55 insertions(+), 6 deletions(-) > > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c > index b9ea5a85278c..74e40ed83fa7 100644 > --- a/drivers/tee/tee_core.c > +++ b/drivers/tee/tee_core.c > @@ -387,6 +387,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params, > params[n].u.value.b = ip.b; > params[n].u.value.c = ip.c; > break; > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT: > + params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a); > + params[n].u.ubuf.size = ip.b; > + > + if (!access_ok(params[n].u.ubuf.uaddr, > + params[n].u.ubuf.size)) > + return -EFAULT; > + > + break; > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: > @@ -455,6 +466,11 @@ static int params_to_user(struct tee_ioctl_param __user *uparams, > put_user(p->u.value.c, &up->c)) > return -EFAULT; > break; > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT: > + if (put_user((u64)p->u.ubuf.size, &up->b)) > + return -EFAULT; > + break; > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: > if (put_user((u64)p->u.memref.size, &up->b)) > @@ -655,6 +671,13 @@ static int params_to_supp(struct tee_context *ctx, > ip.b = p->u.value.b; > ip.c = p->u.value.c; > break; > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT: > + ip.a = (u64)p->u.ubuf.uaddr; > + ip.b = p->u.ubuf.size; > + ip.c = 0; > + break; > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: > @@ -757,6 +780,16 @@ static int params_from_supp(struct tee_param *params, size_t num_params, > p->u.value.b = ip.b; > p->u.value.c = ip.c; > break; > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT: > + p->u.ubuf.uaddr = u64_to_user_ptr(ip.a); > + p->u.ubuf.size = ip.b; > + > + if (!access_ok(params[n].u.ubuf.uaddr, > + params[n].u.ubuf.size)) > + return -EFAULT; > + > + break; > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: > /* > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h > index a54c203000ed..78bbf12f02f0 100644 > --- a/include/linux/tee_drv.h > +++ b/include/linux/tee_drv.h > @@ -82,6 +82,11 @@ struct tee_param_memref { > struct tee_shm *shm; > }; > > +struct tee_param_ubuf { > + void * __user uaddr; > + size_t size; > +}; > + > struct tee_param_value { > u64 a; > u64 b; > @@ -92,6 +97,7 @@ struct tee_param { > u64 attr; > union { > struct tee_param_memref memref; > + struct tee_param_ubuf ubuf; > struct tee_param_value value; > } u; > }; > diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h > index d0430bee8292..3e9b1ec5dfde 100644 > --- a/include/uapi/linux/tee.h > +++ b/include/uapi/linux/tee.h > @@ -151,6 +151,13 @@ struct tee_ioctl_buf_data { > #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6 > #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */ > > +/* > + * These defines userspace buffer parameters. > + */ > +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT 8 > +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT 9 > +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT 10 /* input and output */ > + > /* > * Mask for the type part of the attribute, leaves room for more types > */ > @@ -186,14 +193,17 @@ struct tee_ioctl_buf_data { > /** > * struct tee_ioctl_param - parameter > * @attr: attributes > - * @a: if a memref, offset into the shared memory object, else a value parameter > - * @b: if a memref, size of the buffer, else a value parameter > + * @a: if a memref, offset into the shared memory object, > + * else if a ubuf, address of the user buffer, > + * else a value parameter > + * @b: if a memref or ubuf, size of the buffer, else a value parameter > * @c: if a memref, shared memory identifier, else a value parameter > * > - * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in > - * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and > - * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE > - * indicates that none of the members are used. > + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref, ubuf, or value is > + * used in the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value, > + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref, and TEE_PARAM_ATTR_TYPE_UBUF_* > + * indicates ubuf. TEE_PARAM_ATTR_TYPE_NONE indicates that none of the members > + * are used. > * > * Shared memory is allocated with TEE_IOC_SHM_ALLOC which returns an > * identifier representing the shared memory object. A memref can reference >

2 weeks

Re: [PATCH v6 1/4] sync_file: Protect access to driver and timeline name

by Christian König

On 6/10/25 18:42, Tvrtko Ursulin wrote: > Protect the access to driver and timeline name which otherwise could be > freed as dma-fence exported is signalling fences. > > This prepares the code for incoming dma-fence API changes which will start > asserting these accesses are done from a RCU locked section. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin(a)igalia.com> Reviewed-by: Christian König <christian.koenig(a)amd.com> > --- > drivers/dma-buf/sync_file.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c > index 212df4b849fe..747e377fb954 100644 > --- a/drivers/dma-buf/sync_file.c > +++ b/drivers/dma-buf/sync_file.c > @@ -135,12 +135,18 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len) > strscpy(buf, sync_file->user_name, len); > } else { > struct dma_fence *fence = sync_file->fence; > + const char __rcu *timeline; > + const char __rcu *driver; > > + rcu_read_lock(); > + driver = dma_fence_driver_name(fence); > + timeline = dma_fence_timeline_name(fence); > snprintf(buf, len, "%s-%s%llu-%lld", > - dma_fence_driver_name(fence), > - dma_fence_timeline_name(fence), > + rcu_dereference(driver), > + rcu_dereference(timeline), > fence->context, > fence->seqno); > + rcu_read_unlock(); > } > > return buf; > @@ -262,9 +268,17 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, > static int sync_fill_fence_info(struct dma_fence *fence, > struct sync_fence_info *info) > { > - strscpy(info->obj_name, dma_fence_timeline_name(fence), > + const char __rcu *timeline; > + const char __rcu *driver; > + > + rcu_read_lock(); > + > + driver = dma_fence_driver_name(fence); > + timeline = dma_fence_timeline_name(fence); > + > + strscpy(info->obj_name, rcu_dereference(timeline), > sizeof(info->obj_name)); > - strscpy(info->driver_name, dma_fence_driver_name(fence), > + strscpy(info->driver_name, rcu_dereference(driver), > sizeof(info->driver_name)); > > info->status = dma_fence_get_status(fence); > @@ -273,6 +287,8 @@ static int sync_fill_fence_info(struct dma_fence *fence, > ktime_to_ns(dma_fence_timestamp(fence)) : > ktime_set(0, 0); > > + rcu_read_unlock(); > + > return info->status; > } >

2 weeks

Re: [PATCH v6 3/4] dma-fence: Add safe access helpers and document the rules

by Christian König

On 6/10/25 18:42, Tvrtko Ursulin wrote: > Dma-fence objects currently suffer from a potential use after free problem > where fences exported to userspace and other drivers can outlive the > exporting driver, or the associated data structures. > > The discussion on how to address this concluded that adding reference > counting to all the involved objects is not desirable, since it would need > to be very wide reaching and could cause unloadable drivers if another > entity would be holding onto a signaled fence reference potentially > indefinitely. > > This patch enables the safe access by introducing and documenting a > contract between fence exporters and users. It documents a set of > contraints and adds helpers which a) drivers with potential to suffer from > the use after free must use and b) users of the dma-fence API must use as > well. > > Premise of the design has multiple sides: > > 1. Drivers (fence exporters) MUST ensure a RCU grace period between > signalling a fence and freeing the driver private data associated with it. > > The grace period does not have to follow the signalling immediately but > HAS to happen before data is freed. > > 2. Users of the dma-fence API marked with such requirement MUST contain > the complete access to the data within a single code block guarded by > rcu_read_lock() and rcu_read_unlock(). > > The combination of the two ensures that whoever sees the > DMA_FENCE_FLAG_SIGNALED_BIT not set is guaranteed to have access to a > valid fence->lock and valid data potentially accessed by the fence->ops > virtual functions, until the call to rcu_read_unlock(). > > 3. Module unload (fence->ops) disappearing is for now explicitly not > handled. That would required a more complex protection, possibly needing > SRCU instead of RCU to handle callers such as dma_fence_release() and > dma_fence_wait_timeout(), where race between > dma_fence_enable_sw_signaling, signalling, and dereference of > fence->ops->wait() would need a sleeping SRCU context. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin(a)igalia.com> Reviewed-by: Christian König <christian.koenig(a)amd.com> > --- > drivers/dma-buf/dma-fence.c | 111 ++++++++++++++++++++++++++++--- > include/linux/dma-fence.h | 31 ++++++--- > include/trace/events/dma_fence.h | 38 +++++++++-- > 3 files changed, 157 insertions(+), 23 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index 74f9e4b665e3..3f78c56b58dc 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -511,12 +511,20 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) > > dma_fence_enable_sw_signaling(fence); > > - trace_dma_fence_wait_start(fence); > + if (trace_dma_fence_wait_start_enabled()) { > + rcu_read_lock(); > + trace_dma_fence_wait_start(fence); > + rcu_read_unlock(); > + } > if (fence->ops->wait) > ret = fence->ops->wait(fence, intr, timeout); > else > ret = dma_fence_default_wait(fence, intr, timeout); > - trace_dma_fence_wait_end(fence); > + if (trace_dma_fence_wait_end_enabled()) { > + rcu_read_lock(); > + trace_dma_fence_wait_end(fence); > + rcu_read_unlock(); > + } > return ret; > } > EXPORT_SYMBOL(dma_fence_wait_timeout); > @@ -533,16 +541,23 @@ void dma_fence_release(struct kref *kref) > struct dma_fence *fence = > container_of(kref, struct dma_fence, refcount); > > + rcu_read_lock(); > trace_dma_fence_destroy(fence); > > - if (WARN(!list_empty(&fence->cb_list) && > - !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags), > - "Fence %s:%s:%llx:%llx released with pending signals!\n", > - dma_fence_driver_name(fence), > - dma_fence_timeline_name(fence), > - fence->context, fence->seqno)) { > + if (!list_empty(&fence->cb_list) && > + !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) { > + const char __rcu *timeline; > + const char __rcu *driver; > unsigned long flags; > > + driver = dma_fence_driver_name(fence); > + timeline = dma_fence_timeline_name(fence); > + > + WARN(1, > + "Fence %s:%s:%llx:%llx released with pending signals!\n", > + rcu_dereference(driver), rcu_dereference(timeline), > + fence->context, fence->seqno); > + > /* > * Failed to signal before release, likely a refcounting issue. > * > @@ -556,6 +571,8 @@ void dma_fence_release(struct kref *kref) > spin_unlock_irqrestore(fence->lock, flags); > } > > + rcu_read_unlock(); > + > if (fence->ops->release) > fence->ops->release(fence); > else > @@ -982,11 +999,21 @@ EXPORT_SYMBOL(dma_fence_set_deadline); > */ > void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq) > { > + const char __rcu *timeline; > + const char __rcu *driver; > + > + rcu_read_lock(); > + > + timeline = dma_fence_timeline_name(fence); > + driver = dma_fence_driver_name(fence); > + > seq_printf(seq, "%s %s seq %llu %ssignalled\n", > - dma_fence_driver_name(fence), > - dma_fence_timeline_name(fence), > + rcu_dereference(driver), > + rcu_dereference(timeline), > fence->seqno, > dma_fence_is_signaled(fence) ? "" : "un"); > + > + rcu_read_unlock(); > } > EXPORT_SYMBOL(dma_fence_describe); > > @@ -1055,3 +1082,67 @@ dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, > BIT(DMA_FENCE_FLAG_SEQNO64_BIT)); > } > EXPORT_SYMBOL(dma_fence_init64); > + > +/** > + * dma_fence_driver_name - Access the driver name > + * @fence: the fence to query > + * > + * Returns a driver name backing the dma-fence implementation. > + * > + * IMPORTANT CONSIDERATION: > + * Dma-fence contract stipulates that access to driver provided data (data not > + * directly embedded into the object itself), such as the &dma_fence.lock and > + * memory potentially accessed by the &dma_fence.ops functions, is forbidden > + * after the fence has been signalled. Drivers are allowed to free that data, > + * and some do. > + * > + * To allow safe access drivers are mandated to guarantee a RCU grace period > + * between signalling the fence and freeing said data. > + * > + * As such access to the driver name is only valid inside a RCU locked section. > + * The pointer MUST be both queried and USED ONLY WITHIN a SINGLE block guarded > + * by the &rcu_read_lock and &rcu_read_unlock pair. > + */ > +const char __rcu *dma_fence_driver_name(struct dma_fence *fence) > +{ > + RCU_LOCKDEP_WARN(!rcu_read_lock_held(), > + "RCU protection is required for safe access to returned string"); > + > + if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) > + return fence->ops->get_driver_name(fence); > + else > + return "detached-driver"; > +} > +EXPORT_SYMBOL(dma_fence_driver_name); > + > +/** > + * dma_fence_timeline_name - Access the timeline name > + * @fence: the fence to query > + * > + * Returns a timeline name provided by the dma-fence implementation. > + * > + * IMPORTANT CONSIDERATION: > + * Dma-fence contract stipulates that access to driver provided data (data not > + * directly embedded into the object itself), such as the &dma_fence.lock and > + * memory potentially accessed by the &dma_fence.ops functions, is forbidden > + * after the fence has been signalled. Drivers are allowed to free that data, > + * and some do. > + * > + * To allow safe access drivers are mandated to guarantee a RCU grace period > + * between signalling the fence and freeing said data. > + * > + * As such access to the driver name is only valid inside a RCU locked section. > + * The pointer MUST be both queried and USED ONLY WITHIN a SINGLE block guarded > + * by the &rcu_read_lock and &rcu_read_unlock pair. > + */ > +const char __rcu *dma_fence_timeline_name(struct dma_fence *fence) > +{ > + RCU_LOCKDEP_WARN(!rcu_read_lock_held(), > + "RCU protection is required for safe access to returned string"); > + > + if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) > + return fence->ops->get_driver_name(fence); > + else > + return "signaled-timeline"; > +} > +EXPORT_SYMBOL(dma_fence_timeline_name); > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index 10a849cb2d3f..64639e104110 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -378,15 +378,28 @@ bool dma_fence_remove_callback(struct dma_fence *fence, > struct dma_fence_cb *cb); > void dma_fence_enable_sw_signaling(struct dma_fence *fence); > > -static inline const char *dma_fence_driver_name(struct dma_fence *fence) > -{ > - return fence->ops->get_driver_name(fence); > -} > - > -static inline const char *dma_fence_timeline_name(struct dma_fence *fence) > -{ > - return fence->ops->get_timeline_name(fence); > -} > +/** > + * DOC: Safe external access to driver provided object members > + * > + * All data not stored directly in the dma-fence object, such as the > + * &dma_fence.lock and memory potentially accessed by functions in the > + * &dma_fence.ops table, MUST NOT be accessed after the fence has been signalled > + * because after that point drivers are allowed to free it. > + * > + * All code accessing that data via the dma-fence API (or directly, which is > + * discouraged), MUST make sure to contain the complete access within a > + * &rcu_read_lock and &rcu_read_unlock pair. > + * > + * Some dma-fence API handles this automatically, while other, as for example > + * &dma_fence_driver_name and &dma_fence_timeline_name, leave that > + * responsibility to the caller. > + * > + * To enable this scheme to work drivers MUST ensure a RCU grace period elapses > + * between signalling the fence and freeing the said data. > + * > + */ > +const char __rcu *dma_fence_driver_name(struct dma_fence *fence); > +const char __rcu *dma_fence_timeline_name(struct dma_fence *fence); > > /** > * dma_fence_is_signaled_locked - Return an indication if the fence > diff --git a/include/trace/events/dma_fence.h b/include/trace/events/dma_fence.h > index 84c83074ee81..4814a65b68dc 100644 > --- a/include/trace/events/dma_fence.h > +++ b/include/trace/events/dma_fence.h > @@ -34,14 +34,44 @@ DECLARE_EVENT_CLASS(dma_fence, > __entry->seqno) > ); > > -DEFINE_EVENT(dma_fence, dma_fence_emit, > +/* > + * Safe only for call sites which are guaranteed to not race with fence > + * signaling,holding the fence->lock and having checked for not signaled, or the > + * signaling path itself. > + */ > +DECLARE_EVENT_CLASS(dma_fence_unsignaled, > + > + TP_PROTO(struct dma_fence *fence), > + > + TP_ARGS(fence), > + > + TP_STRUCT__entry( > + __string(driver, fence->ops->get_driver_name(fence)) > + __string(timeline, fence->ops->get_timeline_name(fence)) > + __field(unsigned int, context) > + __field(unsigned int, seqno) > + ), > + > + TP_fast_assign( > + __assign_str(driver); > + __assign_str(timeline); > + __entry->context = fence->context; > + __entry->seqno = fence->seqno; > + ), > + > + TP_printk("driver=%s timeline=%s context=%u seqno=%u", > + __get_str(driver), __get_str(timeline), __entry->context, > + __entry->seqno) > +); > + > +DEFINE_EVENT(dma_fence_unsignaled, dma_fence_emit, > > TP_PROTO(struct dma_fence *fence), > > TP_ARGS(fence) > ); > > -DEFINE_EVENT(dma_fence, dma_fence_init, > +DEFINE_EVENT(dma_fence_unsignaled, dma_fence_init, > > TP_PROTO(struct dma_fence *fence), > > @@ -55,14 +85,14 @@ DEFINE_EVENT(dma_fence, dma_fence_destroy, > TP_ARGS(fence) > ); > > -DEFINE_EVENT(dma_fence, dma_fence_enable_signal, > +DEFINE_EVENT(dma_fence_unsignaled, dma_fence_enable_signal, > > TP_PROTO(struct dma_fence *fence), > > TP_ARGS(fence) > ); > > -DEFINE_EVENT(dma_fence, dma_fence_signaled, > +DEFINE_EVENT(dma_fence_unsignaled, dma_fence_signaled, > > TP_PROTO(struct dma_fence *fence), >

2 weeks

[PATCH v3 0/3] media: fix incorrect use of dma_sync_sg_*() calls

by Marek Szyprowski

Dear All, This patchset fixes the incorrect use of dma_sync_sg_*() calls in media and related drivers. They are replaced with much safer dma_sync_sgtable_*() variants, which take care of passing the proper number of elements for the sync operation. Best regards Marek Szyprowski, PhD Samsung R&D Institute Poland Change log: v3: added cc: stable to tags v2: fixes typos and added cc: stable Patch summary: Marek Szyprowski (3): media: videobuf2: use sgtable-based scatterlist wrappers udmabuf: use sgtable-based scatterlist wrappers media: omap3isp: use sgtable-based scatterlist wrappers drivers/dma-buf/udmabuf.c | 5 ++--- drivers/media/common/videobuf2/videobuf2-dma-sg.c | 4 ++-- drivers/media/platform/ti/omap3isp/ispccdc.c | 8 ++++---- drivers/media/platform/ti/omap3isp/ispstat.c | 6 ++---- 4 files changed, 10 insertions(+), 13 deletions(-) -- 2.34.1

2 weeks

[PATCH] dma-buf: fix compare in WARN_ON_ONCE

by Christian König

Smatch pointed out this trivial typo: drivers/dma-buf/dma-buf.c:1123 dma_buf_map_attachment() warn: passing positive error code '16' to 'ERR_PTR' drivers/dma-buf/dma-buf.c 1113 dma_resv_assert_held(attach->dmabuf->resv); 1114 1115 if (dma_buf_pin_on_map(attach)) { 1116 ret = attach->dmabuf->ops->pin(attach); 1117 /* 1118 * Catch exporters making buffers inaccessible even when 1119 * attachments preventing that exist. 1120 */ 1121 WARN_ON_ONCE(ret == EBUSY); ^^^^^ This was probably intended to be -EBUSY? 1122 if (ret) --> 1123 return ERR_PTR(ret); ^^^ Otherwise we will eventually crash. 1124 } 1125 1126 sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); 1127 if (!sg_table) 1128 sg_table = ERR_PTR(-ENOMEM); 1129 if (IS_ERR(sg_table)) 1130 goto error_unpin; 1131 Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/dma-buf/dma-buf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 0c48d41dd5eb..451714008e8a 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1060,7 +1060,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, * Catch exporters making buffers inaccessible even when * attachments preventing that exist. */ - WARN_ON_ONCE(ret == EBUSY); + WARN_ON_ONCE(ret == -EBUSY); if (ret) return ERR_PTR(ret); } -- 2.43.0

2 weeks, 1 day

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig