Re: [PATCH] ARM: mm: Fix ARCH_LOW_ADDRESS_LIMIT when CONFIG_ZONE_DMA

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Stefan Wahren <stefan.wahren@i2se.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Christoph Hellwig <hch@lst.de>,
	Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
	linux-arm-kernel@lists.infradead.org,
	Lukas Wunner <lukas@wunner.de>
Subject: Re: [PATCH] ARM: mm: Fix ARCH_LOW_ADDRESS_LIMIT when CONFIG_ZONE_DMA
Date: Sat, 3 Jun 2023 14:52:31 +0100	[thread overview]
Message-ID: <ZHtFn+cPKGJ81gr7@shell.armlinux.org.uk> (raw)
In-Reply-To: <728053e0-5cc5-4449-a6ea-b224c7ff2af2@app.fastmail.com>

On Sat, Jun 03, 2023 at 02:59:53PM +0200, Arnd Bergmann wrote:
> On Sat, Jun 3, 2023, at 14:38, Russell King (Oracle) wrote:
> > On Sat, Jun 03, 2023 at 02:33:49PM +0200, Arnd Bergmann wrote:
> >> Changing the low address limit by itself makes the swiotlb work
> >> correctly, but for performance we also want to avoid having to
> >> use the swiotlb at all when dealing with the zero page.
> >
> > How often is the zero page used for DMA _to_ a device (it should never
> > be used for DMA _from_ a device.) ? It doesn't sound that useful to me,
> > since it can only be used for writing zeros.
> 
> It's possible that this doesn't happen all that much at all, the
> bcm2835-dma driver is the only one that has a specific optimization
> for this case, introduced in commit bf75703d0912d ("dmaengine:
> bcm2835: Avoid accessing memory when copying zeroes").
> 
> If the ZERO page can only be mapped through swiotlb, that
> optimization is clearly counterproductive because it requires
> making a copy of the zero page but doesn't actually skip
> any transfers.
> 
> I don't really understand what the spi driver is doing here,
> but I can see that it still contains the code that originally
> started the discussion about the change to the DMA driver,
> so I assume it's still used that way.

With many SPI interfaces, if you want to read something, you need to
have an active "write" stream (because when you clock the bus, bits
need to be clocked out while bits are clocked in.)

A common way to do that is to clock out zeros, and to DMA them from
somewhere.

Note that in the commit you pointed to:

+               /* non-lite channels can write zeroes w/o accessing memory */
+               if (buf_addr == od->zero_page && !c->is_lite_channel)
+                       info |= BCM2835_DMA_S_IGNORE;

So, if it's not a "lite" channel, then even mapping the zero page is
a waste, because according to the comment, it won't even access
memory.

Note also that the driver is also relying on dma_map_*() of zero-page
_always_ producing the same result, which afaik the DMA API has never
guaranteed. Also note that the buffer passed into
bcm2835_dma_prep_dma_cyclic() must also have been dma-mapped (since
it takes a dma_addr_t, not the virtual address of the buffer.)

So this all looks rather dodgy to me.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel