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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cde7e2fc-2e13-4b82-98b3-3d3a52c4c185@os.amperecomputing.com>
Date:   Tue, 19 Sep 2023 11:54:10 -0700
From:   Jan Bottorff <janb@...amperecomputing.com>
To:     Catalin Marinas <catalin.marinas@....com>,
        Yann Sionneau <ysionneau@...rayinc.com>
Cc:     Wolfram Sang <wsa@...nel.org>,
        Serge Semin <fancer.lancer@...il.com>,
        Yann Sionneau <yann@...nneau.net>,
        Will Deacon <will@...nel.org>,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Jan Dabros <jsd@...ihalf.com>,
        Andi Shyti <andi.shyti@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] i2c: designware: Fix corrupted memory seen in the ISR

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