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

Jan Bottorff
2023-09-19T18:54:25Z

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