This patch series clamps the MAX_DMA_ADDRESS to 32-bit in order to fit
within a virtual address, and also fixes the off by one which was
suggested by Geert.
Florian Fainelli (2):
ARM: Fix off by one in checking DMA zone size
ARM: Clamp MAX_DMA_ADDRESS to 32-bit
arch/arm/include/asm/dma.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
--
2.25.1
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 <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm/include/asm/dma.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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
--
2.25.1
On Thu, Mar 24, 2022 at 6:54 PM Florian Fainelli <[email protected]> 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 <[email protected]>
> Signed-off-by: Florian Fainelli <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
MAX_DMA_ADDRESS is a virtual address, therefore it needs to fit within a
32-bit unsigned quantity. Platforms defining a DMA zone size in
their machine descriptor can easily overflow this quantity depending on
the DMA zone size and/or the PAGE_OFFSET setting.
In most cases this is harmless, however in the case of a
CONFIG_DEBUG_VIRTUAL enabled, __virt_addr_valid() will be unable to
return that MAX_DMA_ADDRESS is valid because the value passed to that
function is an unsigned long which has already overflowed.
Fixes: e377cd8221eb ("ARM: 8640/1: Add support for CONFIG_DEBUG_VIRTUAL")
Fixes: 2fb3ec5c9503 ("ARM: Replace platform definition of ISA_DMA_THRESHOLD/MAX_DMA_ADDRESS")
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm/include/asm/dma.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/dma.h b/arch/arm/include/asm/dma.h
index f244ee68e814..ea47420babd4 100644
--- a/arch/arm/include/asm/dma.h
+++ b/arch/arm/include/asm/dma.h
@@ -8,10 +8,12 @@
#ifndef CONFIG_ZONE_DMA
#define MAX_DMA_ADDRESS 0xffffffffUL
#else
+#include <linux/minmax.h>
#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 - 1) : 0xffffffffUL; })
+ min_t(phys_addr_t, (PAGE_OFFSET + arm_dma_zone_size - 1), 0xffffffffUL) : \
+ 0xffffffffUL; })
#endif
#ifdef CONFIG_ISA_DMA_API
--
2.25.1
On 3/24/22 10:54, Florian Fainelli wrote:
> This patch series clamps the MAX_DMA_ADDRESS to 32-bit in order to fit
> within a virtual address, and also fixes the off by one which was
> suggested by Geert.
>
> Florian Fainelli (2):
> ARM: Fix off by one in checking DMA zone size
> ARM: Clamp MAX_DMA_ADDRESS to 32-bit
Russell, do you have any feedback on these two patches?
--
Florian
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!
On Fri, Apr 29, 2022 at 5:15 PM Russell King (Oracle)
<[email protected]> 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