[PATCH] ARM: mm: Fix ARCH_LOW_ADDRESS_LIMIT when CONFIG_ZONE_DMA

[PATCH] ARM: mm: Fix ARCH_LOW_ADDRESS_LIMIT when CONFIG_ZONE_DMA

[Stefan Wahren]

Configuring VMSPLIT_2G + LPAE on Raspberry Pi 4 leads to SWIOTLB
buffer allocation beyond platform dma_zone_size of SZ_1G, which
results in broken SD card boot.

So fix this be setting ARCH_LOW_ADDRESS_LIMIT in CONFIG_ZONE_DMA
case.

Suggested-by: Russell King <linux@armlinux.org.uk>
Fixes: e9faf9b0b07a ("ARM: add multi_v7_lpae_defconfig")
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 arch/arm/include/asm/dma.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/include/asm/dma.h b/arch/arm/include/asm/dma.h
index c6aded1b069c..e2a1916013e7 100644
--- a/arch/arm/include/asm/dma.h
+++ b/arch/arm/include/asm/dma.h
@@ -12,6 +12,9 @@
 	extern phys_addr_t arm_dma_zone_size; \
 	arm_dma_zone_size && arm_dma_zone_size < (0x100000000ULL - PAGE_OFFSET) ? \
 		(PAGE_OFFSET + arm_dma_zone_size) : 0xffffffffUL; })
+
+extern phys_addr_t arm_dma_limit;
+#define ARCH_LOW_ADDRESS_LIMIT arm_dma_limit
 #endif
 
 #ifdef CONFIG_ISA_DMA_API
-- 
2.34.1


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

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

[Arnd Bergmann]

On Sat, Jun 3, 2023, at 11:06, Stefan Wahren wrote:
> Configuring VMSPLIT_2G + LPAE on Raspberry Pi 4 leads to SWIOTLB
> buffer allocation beyond platform dma_zone_size of SZ_1G, which
> results in broken SD card boot.
>
> So fix this be setting ARCH_LOW_ADDRESS_LIMIT in CONFIG_ZONE_DMA
> case.
>
> Suggested-by: Russell King <linux@armlinux.org.uk>
> Fixes: e9faf9b0b07a ("ARM: add multi_v7_lpae_defconfig")
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

This looks correct and required to me, but I think we also
want the change to paging_init() that you tried earlier before
this patch.

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.

      Arnd

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

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

[Russell King (Oracle)]

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.

-- 
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

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

[Arnd Bergmann]

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.

     Arnd


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

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

[Russell King (Oracle)]

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

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

[Stefan Wahren]

Hi,

Am 03.06.23 um 15:58 schrieb Lukas Wunner:
> 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.
>
> SPI is a bidirectional bus, i.e. you receive as many bytes as you write.
>
> When you're receiving an Ethernet frame from an SPI-attached network chip,
> you need to clock out bytes in order to clock in the Ethernet frame.
> The content of those bytes you clock out doesn't matter.  They can be
> zeroes or anything else.
>
> The Raspberry Pi's BCM2835 DMA controller is capable of synthesizing zeroes
> and writing them to the SPI controller's FIFO.  This reduces traffic on the
> memory bus because you don't need to copy data from memory to the FIFO.
>
> Unfortunately only a subset of the DMA channels has that capability.
> If the SPI controller happens to get only one of the inferior "lite"
> channels, it needs to fall back to copying data from memory.  To avoid
> having to map a page for that, it simply reuses the zero page.
>
> Hope that sheds some light on what the driver is doing there.

i'm bit a lost in this old discussion. Arnd sent a Reviewed-By to the patch.

Shall i send this to RMK's patch system?

Or is the patch not sufficient enough?

>
> Thanks,
>
> Lukas

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

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

[Robin Murphy]

On 2023-09-06 11:31, Stefan Wahren wrote:
> Hi,
> 
> Am 03.06.23 um 15:58 schrieb Lukas Wunner:
>> 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.
>>
>> SPI is a bidirectional bus, i.e. you receive as many bytes as you write.
>>
>> When you're receiving an Ethernet frame from an SPI-attached network 
>> chip,
>> you need to clock out bytes in order to clock in the Ethernet frame.
>> The content of those bytes you clock out doesn't matter.  They can be
>> zeroes or anything else.
>>
>> The Raspberry Pi's BCM2835 DMA controller is capable of synthesizing 
>> zeroes
>> and writing them to the SPI controller's FIFO.  This reduces traffic 
>> on the
>> memory bus because you don't need to copy data from memory to the FIFO.
>>
>> Unfortunately only a subset of the DMA channels has that capability.
>> If the SPI controller happens to get only one of the inferior "lite"
>> channels, it needs to fall back to copying data from memory.  To avoid
>> having to map a page for that, it simply reuses the zero page.
>>
>> Hope that sheds some light on what the driver is doing there.
> 
> i'm bit a lost in this old discussion. Arnd sent a Reviewed-By to the 
> patch.
> 
> Shall i send this to RMK's patch system?

I think so - I don't remember how we got into the SPI thing, but that 
looks somewhat bogus and broken, and doesn't matter to the correctness 
of this patch in itself.

Thanks,
Robin.

> 
> Or is the patch not sufficient enough?
> 
>>
>> Thanks,
>>
>> Lukas

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