Re: [PATCH v2] i2c: designware: Fix corrupted memory seen in the ISR

[Jan Bottorff]

On 9/19/2023 7:51 AM, Catalin Marinas wrote:
> While smp_* is ok, it really depends on what the regmap_write() does. Is
> it a write to a shared peripheral (if not, you may need a DSB)? Does the
> regmap_write() caller know this? That's why I think having the barrier
> in dw_reg_write() is better.
>
> If you do want to stick to a fix in i2c_dw_xfer_init(), you could go for
> dma_wmb(). While this is not strictly DMA, it's sharing data with
> another coherent agent (a different CPU in this instance). The smp_wmb()
> is more about communication via memory not involving I/O. But this still
> assumes that the caller knows regmap_write() ends up with an I/O
> write*() (potentially relaxed).

If we wanted maximum correctness wouldn't we need something like 
writel_triggers_interrupt/regmap_write_triggers_interrupt or maybe 
preinterrupt_wmb?

The ARM docs do have a specific example case where the device write 
triggers an interrupt, and that example specifically says a DSB barrier 
is needed.

If I look at the ARM GIC IPI send function gic_ipi_send_mask in 
https://elixir.bootlin.com/linux/v6.6-rc2/source/drivers/irqchip/irq-gic-v3.c#L1354 
is says:

        /*
	 * Ensure that stores to Normal memory are visible to the
	 * other CPUs before issuing the IPI.
	 */
	dsb(ishst);

I would think the IPI send code is very carefully tuned for performance, 
and would not use a barrier any stronger than required.

I believe dma_wmb maps to DMB on ARM64.

- Jan