From e94b9f01543cc6a83538c2c2cc645a424d3015ca Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Wed, 15 Apr 2026 09:32:07 +0100 Subject: dma-fence: Fix potential tracepoint null pointer dereferences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trace_dma_fence_signaled, trace_dma_fence_wait_end and trace_dma_fence_destroy can all currently dereference a null fence->ops pointer after it has been reset on fence signalling. Lets use the safe string getters for most tracepoints to avoid this class of a problem, while for the signal tracepoint we move it to before ops are cleared to avoid losing the driver and timeline name information. Apart from moving it we also need to add a new tracepoint class to bypass the safe name getters since the signaled bit is already set. For dma_fence_init we also need to use the new tracepoint class since the rcu read lock is not held there, and we can do the same for the enable signaling since there we are certain the fence cannot be signaled while we are holding the lock and have even validated the fence->ops. Signed-off-by: Tvrtko Ursulin Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3") Cc: Christian König Cc: Philipp Stanner Cc: Boris Brezillon Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@lists.linaro.org Reviewed-by: Christian König Signed-off-by: Tvrtko Ursulin Link: https://lore.kernel.org/r/20260415083207.40513-2-tvrtko.ursulin@igalia.com --- include/trace/events/dma_fence.h | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) (limited to 'include/trace') diff --git a/include/trace/events/dma_fence.h b/include/trace/events/dma_fence.h index 3abba45c0601..5b10a9e06fb4 100644 --- a/include/trace/events/dma_fence.h +++ b/include/trace/events/dma_fence.h @@ -9,12 +9,40 @@ struct dma_fence; +DECLARE_EVENT_CLASS(dma_fence, + + TP_PROTO(struct dma_fence *fence), + + TP_ARGS(fence), + + TP_STRUCT__entry( + __string(driver, dma_fence_driver_name(fence)) + __string(timeline, dma_fence_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) +); + /* * 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. + * signaling, holding the fence->lock and having checked for not signaled, or + * the signaling path itself. + * + * TODO: Remove the need for this event class when drivers switch to independent + * fences. */ -DECLARE_EVENT_CLASS(dma_fence, +DECLARE_EVENT_CLASS(dma_fence_ops, TP_PROTO(struct dma_fence *fence), @@ -46,7 +74,7 @@ DEFINE_EVENT(dma_fence, dma_fence_emit, TP_ARGS(fence) ); -DEFINE_EVENT(dma_fence, dma_fence_init, +DEFINE_EVENT(dma_fence_ops, dma_fence_init, TP_PROTO(struct dma_fence *fence), @@ -60,14 +88,14 @@ DEFINE_EVENT(dma_fence, dma_fence_destroy, TP_ARGS(fence) ); -DEFINE_EVENT(dma_fence, dma_fence_enable_signal, +DEFINE_EVENT(dma_fence_ops, dma_fence_enable_signal, TP_PROTO(struct dma_fence *fence), TP_ARGS(fence) ); -DEFINE_EVENT(dma_fence, dma_fence_signaled, +DEFINE_EVENT(dma_fence_ops, dma_fence_signaled, TP_PROTO(struct dma_fence *fence), -- cgit From 8711eb2dde2ed44c98714b875dcf7329950c71ba Mon Sep 17 00:00:00 2001 From: Max Zhen Date: Tue, 21 Apr 2026 11:15:02 -0700 Subject: accel/amdxdna: Improve tracing for job lifecycle and mailbox RX worker Add more trace coverage to amdxdna job handling and mailbox receive processing to make driver execution easier to debug. Extend the xdna_job trace event to record the command opcode in addition to the job sequence number. Use the enhanced tracepoint in the job run, sent-to-device, signaled-fence, and job-free paths so that trace output can be correlated with the command being executed. Also add debug-point tracing when a command is received through the submit ioctl path, and add a trace event when the mailbox RX worker runs. These changes improve visibility into job lifetime transitions and mailbox activity, which helps debug command flow and scheduler issues. Signed-off-by: Max Zhen Reviewed-by: Mario Limonciello (AMD) Signed-off-by: Lizhi Hou Link: https://patch.msgid.link/20260421181502.1970263-1-lizhi.hou@amd.com --- drivers/accel/amdxdna/aie2_ctx.c | 14 +++++++---- drivers/accel/amdxdna/amdxdna_ctx.c | 3 ++- drivers/accel/amdxdna/amdxdna_ctx.h | 1 + drivers/accel/amdxdna/amdxdna_mailbox.c | 1 + include/trace/events/amdxdna.h | 42 ++++++++++++++++++++++----------- 5 files changed, 42 insertions(+), 19 deletions(-) (limited to 'include/trace') diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c index d37123d925b6..3b0feba448c4 100644 --- a/drivers/accel/amdxdna/aie2_ctx.c +++ b/drivers/accel/amdxdna/aie2_ctx.c @@ -64,6 +64,7 @@ static void aie2_job_release(struct kref *ref) struct amdxdna_sched_job *job; job = container_of(ref, struct amdxdna_sched_job, refcnt); + amdxdna_sched_job_cleanup(job); atomic64_inc(&job->hwctx->job_free_cnt); wake_up(&job->hwctx->priv->job_free_wq); @@ -195,7 +196,8 @@ aie2_sched_notify(struct amdxdna_sched_job *job) { struct dma_fence *fence = job->fence; - trace_xdna_job(&job->base, job->hwctx->name, "signaled fence", job->seq); + trace_xdna_job(&job->base, job->hwctx->name, "signaling fence", + job->seq, job->drv_cmd ? job->drv_cmd->opcode : DEFAULT_IO); aie2_tdr_signal(job->hwctx->client->xdna); job->hwctx->priv->completed++; @@ -366,6 +368,9 @@ aie2_sched_job_run(struct drm_sched_job *sched_job) struct dma_fence *fence; int ret; + trace_xdna_job(sched_job, hwctx->name, "job run", + job->seq, job->drv_cmd ? job->drv_cmd->opcode : DEFAULT_IO); + if (!hwctx->priv->mbox_chann) return NULL; @@ -409,7 +414,8 @@ out: } else { aie2_tdr_signal(hwctx->client->xdna); } - trace_xdna_job(sched_job, hwctx->name, "sent to device", job->seq); + trace_xdna_job(sched_job, hwctx->name, "sent to device", + job->seq, job->drv_cmd ? job->drv_cmd->opcode : DEFAULT_IO); return fence; } @@ -419,7 +425,8 @@ static void aie2_sched_job_free(struct drm_sched_job *sched_job) struct amdxdna_sched_job *job = drm_job_to_xdna_job(sched_job); struct amdxdna_hwctx *hwctx = job->hwctx; - trace_xdna_job(sched_job, hwctx->name, "job free", job->seq); + trace_xdna_job(sched_job, hwctx->name, "job free", + job->seq, job->drv_cmd ? job->drv_cmd->opcode : DEFAULT_IO); if (!job->job_done) up(&hwctx->priv->job_sem); @@ -437,7 +444,6 @@ aie2_sched_job_timedout(struct drm_sched_job *sched_job) int ret; xdna = hwctx->client->xdna; - trace_xdna_job(sched_job, hwctx->name, "job timedout", job->seq); guard(mutex)(&xdna->dev_lock); diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c index ff6c3e8e5a15..2c2c21992c87 100644 --- a/drivers/accel/amdxdna/amdxdna_ctx.c +++ b/drivers/accel/amdxdna/amdxdna_ctx.c @@ -514,7 +514,6 @@ int amdxdna_cmd_submit(struct amdxdna_client *client, goto unlock_srcu; } - job->hwctx = hwctx; job->mm = current->mm; @@ -612,6 +611,8 @@ int amdxdna_drm_submit_cmd_ioctl(struct drm_device *dev, void *data, struct drm_ if (args->ext || args->ext_flags) return -EINVAL; + trace_amdxdna_debug_point(current->comm, args->type, "job received"); + switch (args->type) { case AMDXDNA_CMD_SUBMIT_EXEC_BUF: return amdxdna_drm_submit_execbuf(client, args); diff --git a/drivers/accel/amdxdna/amdxdna_ctx.h b/drivers/accel/amdxdna/amdxdna_ctx.h index a8557d7e8923..355798687376 100644 --- a/drivers/accel/amdxdna/amdxdna_ctx.h +++ b/drivers/accel/amdxdna/amdxdna_ctx.h @@ -119,6 +119,7 @@ struct amdxdna_hwctx { container_of(j, struct amdxdna_sched_job, base) enum amdxdna_job_opcode { + DEFAULT_IO, SYNC_DEBUG_BO, ATTACH_DEBUG_BO, DETACH_DEBUG_BO, diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c b/drivers/accel/amdxdna/amdxdna_mailbox.c index 37771bdb24a1..cc8865f4e79c 100644 --- a/drivers/accel/amdxdna/amdxdna_mailbox.c +++ b/drivers/accel/amdxdna/amdxdna_mailbox.c @@ -361,6 +361,7 @@ static void mailbox_rx_worker(struct work_struct *rx_work) int ret; mb_chann = container_of(rx_work, struct mailbox_channel, rx_work); + trace_mbox_rx_worker(MAILBOX_NAME, mb_chann->msix_irq); if (READ_ONCE(mb_chann->bad_state)) { MB_ERR(mb_chann, "Channel in bad state, work aborted"); diff --git a/include/trace/events/amdxdna.h b/include/trace/events/amdxdna.h index c6cb2da7b706..71da24267e52 100644 --- a/include/trace/events/amdxdna.h +++ b/include/trace/events/amdxdna.h @@ -30,26 +30,30 @@ TRACE_EVENT(amdxdna_debug_point, ); TRACE_EVENT(xdna_job, - TP_PROTO(struct drm_sched_job *sched_job, const char *name, const char *str, u64 seq), + TP_PROTO(struct drm_sched_job *sched_job, const char *name, + const char *str, u64 seq, u32 op), - TP_ARGS(sched_job, name, str, seq), + TP_ARGS(sched_job, name, str, seq, op), TP_STRUCT__entry(__string(name, name) __string(str, str) __field(u64, fence_context) __field(u64, fence_seqno) - __field(u64, seq)), + __field(u64, seq) + __field(u32, op)), TP_fast_assign(__assign_str(name); __assign_str(str); __entry->fence_context = sched_job->s_fence->finished.context; __entry->fence_seqno = sched_job->s_fence->finished.seqno; - __entry->seq = seq;), + __entry->seq = seq; + __entry->op = op;), - TP_printk("fence=(context:%llu, seqno:%lld), %s seq#:%lld %s", + TP_printk("fence=(context:%llu, seqno:%llu), %s seq#:%llu %s, op=%u", __entry->fence_context, __entry->fence_seqno, __get_str(name), __entry->seq, - __get_str(str)) + __get_str(str), + __entry->op) ); DECLARE_EVENT_CLASS(xdna_mbox_msg, @@ -81,18 +85,28 @@ DEFINE_EVENT(xdna_mbox_msg, mbox_set_head, TP_ARGS(name, chann_id, opcode, id) ); -TRACE_EVENT(mbox_irq_handle, - TP_PROTO(char *name, int irq), +DECLARE_EVENT_CLASS(xdna_mbox_name_id, + TP_PROTO(char *name, int irq), - TP_ARGS(name, irq), + TP_ARGS(name, irq), - TP_STRUCT__entry(__string(name, name) - __field(int, irq)), + TP_STRUCT__entry(__string(name, name) + __field(int, irq)), - TP_fast_assign(__assign_str(name); - __entry->irq = irq;), + TP_fast_assign(__assign_str(name); + __entry->irq = irq;), + + TP_printk("%s.%d", __get_str(name), __entry->irq) +); + +DEFINE_EVENT(xdna_mbox_name_id, mbox_irq_handle, + TP_PROTO(char *name, int irq), + TP_ARGS(name, irq) +); - TP_printk("%s.%d", __get_str(name), __entry->irq) +DEFINE_EVENT(xdna_mbox_name_id, mbox_rx_worker, + TP_PROTO(char *name, int irq), + TP_ARGS(name, irq) ); #endif /* !defined(_TRACE_AMDXDNA_H) || defined(TRACE_HEADER_MULTI_READ) */ -- cgit From 697d6de37650f9c362e02efcf49df2ec774796d6 Mon Sep 17 00:00:00 2001 From: Artur Kowalski Date: Tue, 19 May 2026 12:16:43 +0200 Subject: gpu: host1x: trace: fix string fields in host1x traces Use __assign_str and __get_str as required by tracing subsystem. Fixes string fields being rejected by the verifier and unreadable from userspace. Tested on v6.18.21. Signed-off-by: Artur Kowalski Acked-by: Steven Rostedt Signed-off-by: Thierry Reding Link: https://patch.msgid.link/20260519-host1x-tracing-v1-1-55afb8cbd186@gmail.com --- include/trace/events/host1x.h | 50 ++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 24 deletions(-) (limited to 'include/trace') diff --git a/include/trace/events/host1x.h b/include/trace/events/host1x.h index 1ba84b738e46..1b6aeb7b177b 100644 --- a/include/trace/events/host1x.h +++ b/include/trace/events/host1x.h @@ -21,9 +21,11 @@ struct host1x_bo; DECLARE_EVENT_CLASS(host1x, TP_PROTO(const char *name), TP_ARGS(name), - TP_STRUCT__entry(__field(const char *, name)), - TP_fast_assign(__entry->name = name;), - TP_printk("name=%s", __entry->name) + TP_STRUCT__entry(__string(name, name)), + TP_fast_assign( + __assign_str(name); + ), + TP_printk("name=%s", __get_str(name)) ); DEFINE_EVENT(host1x, host1x_channel_open, @@ -52,19 +54,19 @@ TRACE_EVENT(host1x_cdma_push, TP_ARGS(name, op1, op2), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(u32, op1) __field(u32, op2) ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->op1 = op1; __entry->op2 = op2; ), TP_printk("name=%s, op1=%08x, op2=%08x", - __entry->name, __entry->op1, __entry->op2) + __get_str(name), __entry->op1, __entry->op2) ); TRACE_EVENT(host1x_cdma_push_wide, @@ -73,7 +75,7 @@ TRACE_EVENT(host1x_cdma_push_wide, TP_ARGS(name, op1, op2, op3, op4), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(u32, op1) __field(u32, op2) __field(u32, op3) @@ -81,7 +83,7 @@ TRACE_EVENT(host1x_cdma_push_wide, ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->op1 = op1; __entry->op2 = op2; __entry->op3 = op3; @@ -89,7 +91,7 @@ TRACE_EVENT(host1x_cdma_push_wide, ), TP_printk("name=%s, op1=%08x, op2=%08x, op3=%08x op4=%08x", - __entry->name, __entry->op1, __entry->op2, __entry->op3, + __get_str(name), __entry->op1, __entry->op2, __entry->op3, __entry->op4) ); @@ -100,7 +102,7 @@ TRACE_EVENT(host1x_cdma_push_gather, TP_ARGS(name, bo, words, offset, cmdbuf), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(struct host1x_bo *, bo) __field(u32, words) __field(u32, offset) @@ -114,14 +116,14 @@ TRACE_EVENT(host1x_cdma_push_gather, words * sizeof(u32)); } __entry->cmdbuf = cmdbuf; - __entry->name = name; + __assign_str(name); __entry->bo = bo; __entry->words = words; __entry->offset = offset; ), TP_printk("name=%s, bo=%p, words=%u, offset=%d, contents=[%s]", - __entry->name, __entry->bo, + __get_str(name), __entry->bo, __entry->words, __entry->offset, __print_hex(__get_dynamic_array(cmdbuf), __entry->cmdbuf ? __entry->words * 4 : 0)) @@ -134,7 +136,7 @@ TRACE_EVENT(host1x_channel_submit, TP_ARGS(name, cmdbufs, relocs, syncpt_id, syncpt_incrs), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(u32, cmdbufs) __field(u32, relocs) __field(u32, syncpt_id) @@ -142,7 +144,7 @@ TRACE_EVENT(host1x_channel_submit, ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->cmdbufs = cmdbufs; __entry->relocs = relocs; __entry->syncpt_id = syncpt_id; @@ -151,7 +153,7 @@ TRACE_EVENT(host1x_channel_submit, TP_printk("name=%s, cmdbufs=%u, relocs=%u, syncpt_id=%u, " "syncpt_incrs=%u", - __entry->name, __entry->cmdbufs, __entry->relocs, + __get_str(name), __entry->cmdbufs, __entry->relocs, __entry->syncpt_id, __entry->syncpt_incrs) ); @@ -161,19 +163,19 @@ TRACE_EVENT(host1x_channel_submitted, TP_ARGS(name, syncpt_base, syncpt_max), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(u32, syncpt_base) __field(u32, syncpt_max) ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->syncpt_base = syncpt_base; __entry->syncpt_max = syncpt_max; ), TP_printk("name=%s, syncpt_base=%d, syncpt_max=%d", - __entry->name, __entry->syncpt_base, __entry->syncpt_max) + __get_str(name), __entry->syncpt_base, __entry->syncpt_max) ); TRACE_EVENT(host1x_channel_submit_complete, @@ -182,19 +184,19 @@ TRACE_EVENT(host1x_channel_submit_complete, TP_ARGS(name, count, thresh), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(int, count) __field(u32, thresh) ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->count = count; __entry->thresh = thresh; ), TP_printk("name=%s, count=%d, thresh=%d", - __entry->name, __entry->count, __entry->thresh) + __get_str(name), __entry->count, __entry->thresh) ); TRACE_EVENT(host1x_wait_cdma, @@ -203,16 +205,16 @@ TRACE_EVENT(host1x_wait_cdma, TP_ARGS(name, eventid), TP_STRUCT__entry( - __field(const char *, name) + __string(name, name) __field(u32, eventid) ), TP_fast_assign( - __entry->name = name; + __assign_str(name); __entry->eventid = eventid; ), - TP_printk("name=%s, event=%d", __entry->name, __entry->eventid) + TP_printk("name=%s, event=%d", __get_str(name), __entry->eventid) ); TRACE_EVENT(host1x_syncpt_load_min, -- cgit