Re: [PATCH v2 4/7] sparc32: Do not select ZONE_DMA

From: Andreas Larsson <andreas@gaisler.com>
To: Arnd Bergmann <arnd@arndb.de>, Sam Ravnborg <sam@ravnborg.org>,
	"Maciej W. Rozycki" <macro@orcam.me.uk>,
	sparclinux@vger.kernel.org, Randy Dunlap <rdunlap@infradead.org>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,
	linux-parport@lists.infradead.org,
	"David S . Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/7] sparc32: Do not select ZONE_DMA
Date: Wed, 6 Mar 2024 16:31:35 +0100	[thread overview]
Message-ID: <5d97d50a-9d40-4651-8071-073dee5f9aa8@gaisler.com> (raw)
In-Reply-To: <75a4a08d-85c2-4a60-9cbd-90dd50f765a8@app.fastmail.com>

On 2024-03-06 15:45, Arnd Bergmann wrote:
>> Testing, in a different driver, setting and
>> allocating under a 30-bit DMA mask (or even a 28-bit DMA mask depending
>> on where the physical memory resides) is possible before removing
>> ZONE_DMA, but not after. 
> 
> I still don't see how that changes anything if
> max_zone_pfn[ZONE_DMA] and max_zone_pfn[ZONE_NORMAL] are set
> to the same value. Did you test this on a mainline kernel, or
> do you have any patches on top that might have set up
> the zones differently?

This was tested on my for-next plus this patch series, i.e. v6.8-rc1
plus these ones:

(local)       sparc32: Fix section mismatch in leon_pci_grpci
(local)       sparc32: Fix parport build with sparc32
(local)       sparc32: Do not select GENERIC_ISA_DMA
(local)       sparc32: Do not select ZONE_DMA
(local)       mtd: maps: sun_uflash: Declare uflash_devinit static
(local)       sparc32: Fix build with trapbase
(local)       sparc32: Use generic cmpdi2/ucmpdi2 variants
626db6ee8ee1e sparc: select FRAME_POINTER instead of redefining it
5378f00c935be sparc: vDSO: fix return value of __setup handler
3ed7c61e49d65 sparc64: NMI watchdog: fix return value of __setup handler
079431ea9ed3e sparc: vio: make vio_bus_type const
3cc208ffa84a7 sparc: Fix typos
0f1991949d9bd sparc: Use shared font data
0955723ef9358 sparc: remove obsolete config ARCH_ATU

so nothing else that should affect zones.

> More specifically, what do you see in the boot log for the
> size of each zone? 

dma_set_mask_and_coherent(dev, DMA_BIT_MASK(28)) fails with
the ZONE_DMA removed, and no other differences. Boot log:

831MB HIGHMEM available.
Zone ranges:
  Normal   [mem 0x0000000000000000-0x000000000bffffff]
  HighMem  [mem 0x000000000c000000-0x000000003fff7fff]
Movable zone start for each node
Early memory node ranges
  node   0: [mem 0x0000000000000000-0x000000003fff7fff]
Initmem setup node 0 [mem 0x0000000000000000-0x000000003fff7fff]

dma_set_mask_and_coherent(dev, DMA_BIT_MASK(28)) succeeds with
ZONE_DMA still in place (i.e. the above plus the ZONE_DMA patch
reverted and no other differences). Boot log:

831MB HIGHMEM available.
Zone ranges:
  DMA      [mem 0x0000000000000000-0x000000000bffffff]
  Normal   empty
  HighMem  [mem 0x000000000c000000-0x000000003fff7fff]
Movable zone start for each node
Early memory node ranges
  node   0: [mem 0x0000000000000000-0x000000003fff7fff]
Initmem setup node 0 [mem 0x0000000000000000-0x000000003fff7fff]

> 
>> I am also a bit concerned if removing ZONE_DMA will let DMA be allocated
>> in highmem and what that could lead to.
> 
> It's not supposed to make a difference, but this is a bit
> more complex:
> 
> - Any kernel allocation (kmalloc etc) by definition comes from
>   lowmem, regardless of GFP_DMA/GFP_KERNEL.
> 
> - user pages that get mapped using dma_map_sg() or dma_map_page()
>   can be in highmem, but this should not depend on the presence
>   of ZONE_DMA
> 
> - If you have devices that can only access a subset of the RAM
>   and there is no IOMMU, you really need to use swiotlb to
>   make it use bounce buffers, at least for the streaming
>   (dma_map_*) API. A driver using only the coherent (dma_alloc_*)
>   API should use ZONE_DMA if ZONE_NORMAL goes beyond the
>   dma mask.
Thanks for the explanation!

Cheers,
Andreas