Re: [PATCH v5 4/7] firmware: samsung: acpm: Add memory barrier before advancing RX pointer

[Arnd Bergmann]

On Tue, May 5, 2026, at 15:13, Tudor Ambarus wrote:
> Sashiko identified a silent data corruption in [1].
>
> In acpm_get_rx(), the driver reads the response payload from SRAM using
> __ioread32_copy() and subsequently updates the hardware RX rear pointer
> via writel().
>
> On weakly ordered architectures like ARM64, writel() provides a write
> memory barrier (wmb()), which strictly orders prior writes against
> subsequent writes. However, it does not order prior reads against
> subsequent writes. Consequently, the CPU is permitted to reorder the
> writel() store to become globally visible before the payload reads
> have completed.

I am very confused by this after seeing it in the Exynos fixes pull
request. How would anything get reordered here? What I see is that

- The SRAM is device memory, so any access to it is architecturally
  ordered against other accesses to the same device. Even on
  architectures that don't guarantee this, Linux I/O accessors
  do.

- The __ioread32_copy() writes data from MMIO into main memory,
  and the store into main memory is guaranteed to be both before
  the final writel() (because of the implied __iowmb()) and
  after the read (because of the data dependency).

- The smp_store_release() in addition orders the write into
  rx_data->completed after the previous memory asccesses and
  before the writel().

> If this reordering occurs, the firmware may observe the updated rear
> pointer, assume the queue slot is available, and overwrite the SRAM
> payload while the kernel is still actively reading from it, leading
> to silent data corruption.

It is possible that I'm still missing the point here, but it
very much sounds like you trusted a chatbot over trying to
understand what is actually going on. Can you explain a
sceneario where the barrier actually makes a difference,
and what the corresponding barrier operation on the other
side is?

     Arnd