Re: [PATCH v2 17/23] dmaengine: sdxi: Add completion status block API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Misplaced memory barrier in `sdxi_completion_signaled()` allows speculative stale reads.
- [High] Unsafe use of `jiffies` for an excessive 1-second busy-wait loop in potentially atomic contexts.
- [Medium] False timeout failure in `sdxi_completion_poll()` due to missing final check (TOCTOU race).
--

commit 06424605658df30b37e1f10fa36bddfa9f3b20e8
Author: Nathan Lynch <nathan.lynch@amd.com>

    dmaengine: sdxi: Add completion status block API

This commit introduces an API for managing DMA-coherent completion status
blocks for SDXI descriptors. It includes functions for allocating, polling,
and querying the status of these completion blocks.

> diff --git a/drivers/dma/sdxi/completion.c b/drivers/dma/sdxi/completion.c
> new file mode 100644
> index 0000000000000..7ffd034b129b8
> --- /dev/null
> +++ b/drivers/dma/sdxi/completion.c
[ ... ]
> +int sdxi_completion_poll(const struct sdxi_completion *sc)
> +{
> +	unsigned long deadline = jiffies + msecs_to_jiffies(1000);
> +
> +	while (le64_to_cpu(READ_ONCE(sc->cst_blk->signal)) != 0) {
> +		if (time_after(jiffies, deadline))
> +			return -ETIMEDOUT;

Could this cause a spurious timeout if the thread is preempted or delayed
between the signal check and the timeout check? If the hardware completes the
DMA request during the delay, jiffies will have advanced past the deadline
when the thread resumes, causing the function to incorrectly return
-ETIMEDOUT. A final read of the signal field after the timeout condition
might prevent this race.

Also, is it safe to busy-wait using jiffies for up to 1 second here? The
use of GFP_NOWAIT in sdxi_completion_alloc() suggests this API might be
called from atomic or interrupt-disabled contexts. Busy-waiting for 1 second
could cause severe latency or soft lockups. Furthermore, if interrupts are
disabled on the CPU responsible for the timer tick, jiffies will not
increment, potentially resulting in an infinite loop.

> +		cpu_relax();
> +	}
> +
> +	return sdxi_completion_errored(sc) ? -EIO : 0;
> +}
> +
> +bool sdxi_completion_signaled(const struct sdxi_completion *sc)
> +{
> +	dma_rmb();
> +	return (sc->cst_blk->signal == 0);

Does this read memory barrier need to be executed after checking the signal
field instead of before? Placing dma_rmb() before evaluating the
DMA-coherent signal allows the CPU to speculatively fetch payload data
before confirming the device has actually signaled completion. Moving the
barrier after a successful check (and using READ_ONCE) would ensure the
payload reads are properly ordered.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-sdxi-base-v2-0-889cfed17e3f@amd.com?part=17