Re: [PATCH v2 10/23] dmaengine: sdxi: Complete administrative context jump start

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using a non-atomic 64-bit fallback for a hardware doorbell register causes torn writes and device state corruption on 32-bit architectures.
--

commit 1c8a47b1b7cac23ca82b3d7ffe7b54c6cf98827e
Author: Nathan Lynch <nathan.lynch@amd.com>
dmaengine: sdxi: Complete administrative context jump start

This commit completes the SDXI administrative context initialization by
writing to its doorbell once the function is placed in the active state.
It introduces a new helper function for pushing the doorbell, which will
be used across all descriptor submission paths.

> diff --git a/drivers/dma/sdxi/context.h b/drivers/dma/sdxi/context.h
> index 65b773446ba31..8dd6beb7a6424 100644
> --- a/drivers/dma/sdxi/context.h
> +++ b/drivers/dma/sdxi/context.h
> @@ -7,6 +7,7 @@
>  #define DMA_SDXI_CONTEXT_H
>  
>  #include <linux/dma-mapping.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>

Does including linux/io-64-nonatomic-lo-hi.h in a shared header risk
polluting the compilation unit? It appears this might blindly convert all
missing 64-bit I/O operations into non-atomic fallbacks for any file that
includes context.h.

>  #include <linux/types.h>
>  
>  #include "hw.h"

[ ... ]

> @@ -58,4 +59,9 @@ struct sdxi_cxt {
>  
>  int sdxi_admin_cxt_init(struct sdxi_dev *sdxi);
>  
> +static inline void sdxi_cxt_push_doorbell(struct sdxi_cxt *cxt, u64 index)
> +{
> +	iowrite64(index, cxt->db);

On 32-bit architectures without native 64-bit I/O, the non-atomic fallback
splits this 64-bit write into two 32-bit writes (lower 32 bits, then upper
32 bits).

Since this is a doorbell register that triggers hardware actions, could a torn
write expose a transient, invalid index to the device before the upper bits
are written?

For example, if the index crosses a 32-bit boundary, the hardware might
observe a corrupted index and begin processing an invalid ring state.

Should the driver depend on CONFIG_64BIT if atomic 64-bit writes are
mandatory, or use a safe, hardware-specific 32-bit doorbell mechanism
instead?

> +}
> +
>  #endif /* DMA_SDXI_CONTEXT_H */

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