Re: [PATCH V2 14/20] accel/amdxdna: Implement AIE4 command packet building and submission

[Eva Crystal]

On Mon, Oct 05, 2026 at 09:22:24PM -0700, David Zhang wrote:

> +/* Job timeout detection (TDR) will guarantee the fence signalling */
> +static void job_worker(struct work_struct *work)
> +{
> +	struct amdxdna_hwctx_priv *priv =
> +		container_of(work, struct amdxdna_hwctx_priv, job_work);
> +	struct amdxdna_hwctx *hwctx = priv->hwctx;
> +	struct amdxdna_sched_job *job;
> +
> +	while ((job = peek_running_job(hwctx))) {
> +		wait_till_seq_completed(hwctx, job->seq);
> +		if (get_read_index(hwctx) > job->seq) {
> +			dequeue_running_job(hwctx, job);
> +			/* Abort partially submitted jobs; complete fully submitted ones. */
> +			if (job->aie4_job_state != AIE4_JOB_STATE_SUBMITTED)
> +				job_abort(job);
> +			else
> +				job_complete(job);

> +static void job_done(struct amdxdna_sched_job *job)
> +{
> +	job->aie4_job_state = AIE4_JOB_STATE_DONE;
> +	dma_fence_signal(job->fence);
> +	/* Release submitter mm reference taken at submit. */
> +	mmput_async(job->mm);
> +	kref_put(&job->refcnt, aie4_job_release);
> +}

The get_read_index(hwctx) > job->seq test here reads a value userspace can write, and this patch is the first to let it release resources rather than just end a wait.

priv->umq_read_index = &qhdr->read_index is already in drm-misc-next (drivers/accel/amdxdna/aie4_ctx.c:212 at 34e9ab018249), but there its only consumer was check_cmd_done() from aie4_cmd_wait() and aie4_vf_ops had no .cmd_submit, so forging it only ended your own wait early. Here it decides dma_fence_signal(), mmput_async() and the BO reference drops in aie4_job_release().

The queue is userspace's own BO: hwctx->umq_bo_hdl is args->umq_bo from CREATE_HWCTX (drivers/accel/amdxdna/amdxdna_ctx.c:252) and is mmappable read-write (drivers/accel/amdxdna/amdxdna_gem.c:1409), so the submitter shares the pages the driver vmaps. valid_queue_index() bounds it only against the kernel copy priv->write_index, so any value in [write_index - 32, write_index] passes and one store retires every outstanding job. Nothing else is consulted: cert_comp_isr() (drivers/accel/amdxdna/aie4_pci.c:114) only calls wake_up_all(), and aie4 has no per-job mailbox handler.

I may be overstating the impact. I found no kernel memory corruption: no driver allocation's free is gated on a job fence, and the queue BO reference drops in aie4_hwctx_fini(), after hwctx_stop() has done the synchronous aie4_msg_destroy_context(). User pages look covered, since SVA is the default and the core invalidates device TLBs on every mm invalidation (drivers/iommu/iommu-sva.c:341). What is left is cross-process integrity: job->out_fence sits in every argument BO's reservation as DMA_RESV_USAGE_WRITE and those export as dma-buf (drivers/accel/amdxdna/amdxdna_gem.c:680), so an importer is told the NPU is done when it is not. I cannot tell from source whether CERT keeps executing packets it already fetched once the host moves read_index past them; if it stops, this is self inflicted only. You have the hardware.

Would a driver owned completion word work, device mapped but not user mapped? Or could CERT report the count in a register or mailbox message the ISR reads?

Separately: abo->mem.map_invalid is set in the MMU notifier (drivers/accel/amdxdna/amdxdna_gem.c:247) and at mmap time for imported BOs (drivers/accel/amdxdna/amdxdna_gem.c:505), but cleared only in aie2_populate_range() (drivers/accel/amdxdna/aie2_ctx.c:1145), static and called only from aie2_cmd_submit(). On aie4 it is never cleared, so aie4_cmd_submit() returns -EINVAL for that BO's remaining life. An imported dma-buf argument BO that userspace mmapped starts with the flag set and can never be submitted.

Eva Crystal (0xiviel)
XSource Security
https://xsourcesec.com