| Message ID | 20220324175417.1014562-2-f.fainelli@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: MAX_DMA_ADDRESS fixes | expand |
On Thu, Mar 24, 2022 at 6:54 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > The maximum DMA address is PAGE_OFFSET + arm_dma_zone_size - 1, fix this > by doing the appropriate subtraction. > > Fixes: 650320181a08 ("ARM: change ARM_DMA_ZONE_SIZE into a variable") > Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
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.
On Fri, Apr 29, 2022 at 5:15 PM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > 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. This driver is on its way out and will be removed in 5.19. > 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. Looking at the other definitions, I see it's usually either ULONG_MAX (which is inclusive) or an exclusive limit, same as the existing version on arm: arch/alpha/include/asm/io.h:#define IDENT_ADDR 0xffff800000000000UL arch/alpha/include/asm/dma.h:#define MAX_DMA_ADDRESS (alpha_mv.mv_pci_tbi ? \ arch/alpha/include/asm/dma.h- ~0UL : IDENT_ADDR + 0x01000000) arch/arc/include/asm/dma.h:#define MAX_DMA_ADDRESS 0xC0000000 arch/hexagon/include/asm/dma.h:#define MAX_DMA_ADDRESS (PAGE_OFFSET) arch/ia64/mm/init.c:unsigned long MAX_DMA_ADDRESS = PAGE_OFFSET + 0x100000000UL; arch/m68k/include/asm/dma.h:#define MAX_DMA_ADDRESS PAGE_OFFSET arch/mips/include/asm/dma.h:#define MAX_DMA_ADDRESS PAGE_OFFSET arch/mips/include/asm/dma.h:#define MAX_DMA_ADDRESS (PAGE_OFFSET + 0x01000000) arch/parisc/include/asm/dma.h:#define MAX_DMA_ADDRESS (~0UL) arch/powerpc/include/asm/dma.h:#define MAX_DMA_ADDRESS (~0UL) arch/s390/include/asm/dma.h:#define MAX_DMA_ADDRESS 0x80000000 arch/sparc/include/asm/dma.h:#define MAX_DMA_ADDRESS (~0UL) arch/um/include/asm/dma.h:#define MAX_DMA_ADDRESS (uml_physmem) arch/x86/include/asm/dma.h:#define MAX_DMA_ADDRESS (PAGE_OFFSET + 0x1000000) arch/x86/include/asm/dma.h:#define MAX_DMA_PFN ((16UL * 1024 * 1024) >> PAGE_SHIFT) arch/x86/include/asm/dma.h:#define MAX_DMA_ADDRESS ((unsigned long)__va(MAX_DMA_PFN << PAGE_SHIFT)) The exceptions are: arch/xtensa/include/asm/kmem_layout.h:#define XCHAL_KIO_SIZE 0x10000000 arch/xtensa/include/asm/dma.h:#define MAX_DMA_ADDRESS (PAGE_OFFSET + XCHAL_KIO_SIZE - 1) arch/microblaze/include/asm/dma.h:#define MAX_DMA_ADDRESS (CONFIG_KERNEL_START + memory_size - 1) Arnd
diff --git a/arch/arm/include/asm/dma.h b/arch/arm/include/asm/dma.h index a81dda65c576..f244ee68e814 100644 --- a/arch/arm/include/asm/dma.h +++ b/arch/arm/include/asm/dma.h @@ -11,7 +11,7 @@ #define MAX_DMA_ADDRESS ({ \ extern phys_addr_t arm_dma_zone_size; \ arm_dma_zone_size && arm_dma_zone_size < (0x10000000 - PAGE_OFFSET) ? \ - (PAGE_OFFSET + arm_dma_zone_size) : 0xffffffffUL; }) + (PAGE_OFFSET + arm_dma_zone_size - 1) : 0xffffffffUL; }) #endif #ifdef CONFIG_ISA_DMA_API
The maximum DMA address is PAGE_OFFSET + arm_dma_zone_size - 1, fix this by doing the appropriate subtraction. Fixes: 650320181a08 ("ARM: change ARM_DMA_ZONE_SIZE into a variable") Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- arch/arm/include/asm/dma.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)