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

[Arnd Bergmann]

On Wed, Mar 6, 2024, at 16:31, Andreas Larsson wrote:
> On 2024-03-06 15:45, Arnd Bergmann wrote:
>> 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]


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

That sounds like a bug somewhere else. As Sam explained in
the patch description, ZONE_NORMAL and ZONE_DMA always have
the same limit, which explains that without the patch you
only have DMA and highmem, but not normal.

What we expected to happen here is that anything that asks
for ZONE_DMA memory just uses ZONE_NORMAL instead and the
behavior never changes.

It looks like this is not how dma_direct_supported()
works though:

        u64 min_mask = (max_pfn - 1) << PAGE_SHIFT;
        [...]
        /*
         * This check needs to be against the actual bit mask value, so use
         * phys_to_dma_unencrypted() here so that the SME encryption mask isn't
         * part of the check.
         */     
        if (IS_ENABLED(CONFIG_ZONE_DMA))
                min_mask = min_t(u64, min_mask, DMA_BIT_MASK(zone_dma_bits));
        return mask >= phys_to_dma_unencrypted(dev, min_mask);

Without ZONE_DMA, it checks for the highest page of any
zone, but that is ZONE_HIGHMEM in your case, which apparently
is outside of the device's mask, while ZONE_NORMAL is inside
the mask.

Not sure if it's worth changing the generic code for this,
or if we want to just keep the existing version without
Sam's patch now that we understand the issue.

On a relate note, it does seem odd to have such a small
lowmem area, and I wonder if that could be extended.
The 192MB lowmem limit comes from 

#define SRMMU_MAXMEM            0x0c000000

but I don't understand if that is a hardware limitation
or a design choice that can be changed, and if it is
even valid on leon or only on the old sun machines.

There is a recurring discussion about eventually
killing off support for CONFIG_HIGHMEM in the kernel,
so if you have a hardware limit of 192MB of lowmem,
this would hit you particularly hard.

      Arnd