Re: [PATCH 1/2] ARM: Fix off by one in checking DMA zone size

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Nicolas Pitre <nico@fluxnic.net>,
	Catalin Marinas <catalin.marinas@arm.com>,
	open list <linux-kernel@vger.kernel.org>,
	linus.walleij@linaro.org, Rob Herring <robh+dt@kernel.org>,
	Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH 1/2] ARM: Fix off by one in checking DMA zone size
Date: Fri, 29 Apr 2022 16:15:38 +0100	[thread overview]
Message-ID: <YmwBGh5mvuNIgyRY@shell.armlinux.org.uk> (raw)
In-Reply-To: <20220324175417.1014562-2-f.fainelli@gmail.com>

On Thu, Mar 24, 2022 at 10:54:16AM -0700, Florian Fainelli wrote:
> The maximum DMA address is PAGE_OFFSET + arm_dma_zone_size - 1, fix this
> by doing the appropriate subtraction.

The question is... is MAX_DMA_ADDRESS inclusive or exclusive. The answer
is rather indeterminant, unfortunately.

drivers/net/wan/cosa.c: if (b + len >= MAX_DMA_ADDRESS)

So, if the buffer address + buffer length is equal to MAX_DMA_ADDRESS,
then the buffer is not DMA-able. To me this looks completely wrong.
It's also completely wrong because 'b' is a "unsigned long" which
means on 32-bit, this can wrap.

drivers/parport/parport_pc.c:
	unsigned long start = (unsigned long) buf;
	unsigned long end = (unsigned long) buf + length - 1;

	if (end < MAX_DMA_ADDRESS) {

So, "end" is the last byte of the buffer. If MAX_DMA_ADDRESS is the last
byte of the virtual address space that supports DMA, then if the two are
equal, we do not use DMA. This seems wrong to me, and points to it being
an _exclusive_ value - it's the last byte plus one.

there's a bunch of memblock allocation functions that use
__pa(MAX_DMA_ADDRESS) as the "min_addr" and if this is a minimum
address, then surely this means that it is also an exclusive value.

So, I came to the conclusion that MAX_DMA_ADDRESS is supposed to be
exclusive. In other words, where PAGE_OFFSET + sizeof(ram) if
sizeof(ram) is the size in bytes of the RAM, limited to the maximum
addressable virtual address that RAM can be mapped to.

I don't see an inclusive value being correct, at least not for the cases
I've looekd at.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps 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