diff mbox series

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

Message ID 20240224-sam-fix-sparc32-all-builds-v2-4-1f186603c5c4@ravnborg.org
State New
Headers show
Series sparc32: build fixes for all{yes,mod}config builds | expand

Commit Message

Sam Ravnborg via B4 Relay Feb. 24, 2024, 5:42 p.m. UTC 
From: Sam Ravnborg <sam@ravnborg.org>

sparc32 has no limited DMA zone so there is no need to select ZONE_DMA.

Based on analysis from Marciej:
"
    Actually I think ZONE_DMA should go too (it's linked to GENERIC_ISA_DMA,
    isn't it? -- cf. commit 5ac6da669e24 ("[PATCH] Set CONFIG_ZONE_DMA for
    arches with GENERIC_ISA_DMA")), and the whole thing use:

    Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

    The GENERIC_ISA_DMA option itself was added to arch/sparc/config.in with
    2.5.31 as:

        define_bool CONFIG_GENERIC_ISA_DMA y

    despite of:

        define_bool CONFIG_ISA n
"

The sparc32 code did not differ between ZONE_NORMAL and ZONE_DMA,
which confirms the above. This patch drop ZONE_DMA.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Reported-by: "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: Andreas Larsson <andreas@gaisler.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Maciej W. Rozycki <macro@orcam.me.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 arch/sparc/Kconfig    | 1 -
 arch/sparc/mm/srmmu.c | 1 -
 2 files changed, 2 deletions(-)

Comments

Maciej W. Rozycki Feb. 25, 2024, 12:57 a.m. UTC | #1 
On Sat, 24 Feb 2024, Sam Ravnborg via B4 Relay wrote:

> sparc32 has no limited DMA zone so there is no need to select ZONE_DMA.
> 
> Based on analysis from Marciej:

 Can you please use the correct spelling of my name in change descriptions 
(and preferably everywhere)?  Thank you.

  Maciej
Sam Ravnborg Feb. 25, 2024, 7:08 a.m. UTC | #2 
On Sun, Feb 25, 2024 at 12:57:42AM +0000, Maciej W. Rozycki wrote:
> On Sat, 24 Feb 2024, Sam Ravnborg via B4 Relay wrote:
> 
> > sparc32 has no limited DMA zone so there is no need to select ZONE_DMA.
> > 
> > Based on analysis from Marciej:
> 
>  Can you please use the correct spelling of my name in change descriptions 
> (and preferably everywhere)?  Thank you.

Sorry, will fix in v3.

	Sam
Randy Dunlap March 2, 2024, 11:51 p.m. UTC | #3 
On 2/24/24 09:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <sam@ravnborg.org>
> 
> sparc32 has no limited DMA zone so there is no need to select ZONE_DMA.
> 
> Based on analysis from Marciej:
> "
>     Actually I think ZONE_DMA should go too (it's linked to GENERIC_ISA_DMA,
>     isn't it? -- cf. commit 5ac6da669e24 ("[PATCH] Set CONFIG_ZONE_DMA for
>     arches with GENERIC_ISA_DMA")), and the whole thing use:
> 
>     Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> 
>     The GENERIC_ISA_DMA option itself was added to arch/sparc/config.in with
>     2.5.31 as:
> 
>         define_bool CONFIG_GENERIC_ISA_DMA y
> 
>     despite of:
> 
>         define_bool CONFIG_ISA n
> "
> 
> The sparc32 code did not differ between ZONE_NORMAL and ZONE_DMA,
> which confirms the above. This patch drop ZONE_DMA.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Reported-by: "Maciej W. Rozycki" <macro@orcam.me.uk>
> Cc: Andreas Larsson <andreas@gaisler.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Maciej W. Rozycki <macro@orcam.me.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>

Tested-by: Randy Dunlap <rdunlap@infradead.org> # build-tested

Thanks.

> ---
>  arch/sparc/Kconfig    | 1 -
>  arch/sparc/mm/srmmu.c | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index 734f23daecca..bdbde506c01e 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -62,7 +62,6 @@ config SPARC32
>  	select HAVE_UID16
>  	select LOCK_MM_AND_FIND_VMA
>  	select OLD_SIGACTION
> -	select ZONE_DMA
>  
>  config SPARC64
>  	def_bool 64BIT
> diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c
> index 852085ada368..7aae2f6f4973 100644
> --- a/arch/sparc/mm/srmmu.c
> +++ b/arch/sparc/mm/srmmu.c
> @@ -975,7 +975,6 @@ void __init srmmu_paging_init(void)
>  	{
>  		unsigned long max_zone_pfn[MAX_NR_ZONES] = { 0 };
>  
> -		max_zone_pfn[ZONE_DMA] = max_low_pfn;
>  		max_zone_pfn[ZONE_NORMAL] = max_low_pfn;
>  		max_zone_pfn[ZONE_HIGHMEM] = highend_pfn;
>  
>
Andreas Larsson March 5, 2024, 3:06 p.m. UTC | #4 
On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <sam@ravnborg.org>
> 
> sparc32 has no limited DMA zone so there is no need to select ZONE_DMA.
> 
> Based on analysis from Marciej:
> "
>     Actually I think ZONE_DMA should go too (it's linked to GENERIC_ISA_DMA,
>     isn't it? -- cf. commit 5ac6da669e24 ("[PATCH] Set CONFIG_ZONE_DMA for
>     arches with GENERIC_ISA_DMA")), and the whole thing use:
> 
>     Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> 
>     The GENERIC_ISA_DMA option itself was added to arch/sparc/config.in with
>     2.5.31 as:
> 
>         define_bool CONFIG_GENERIC_ISA_DMA y
> 
>     despite of:
> 
>         define_bool CONFIG_ISA n
> "
> 
> The sparc32 code did not differ between ZONE_NORMAL and ZONE_DMA,
> which confirms the above. This patch drop ZONE_DMA.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Reported-by: "Maciej W. Rozycki" <macro@orcam.me.uk>
> Cc: Andreas Larsson <andreas@gaisler.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Maciej W. Rozycki <macro@orcam.me.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/sparc/Kconfig    | 1 -
>  arch/sparc/mm/srmmu.c | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index 734f23daecca..bdbde506c01e 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -62,7 +62,6 @@ config SPARC32
>  	select HAVE_UID16
>  	select LOCK_MM_AND_FIND_VMA
>  	select OLD_SIGACTION
> -	select ZONE_DMA

This however makes a number of PCI drivers that depend on
ZONE_DMA unselectable.

>  
>  config SPARC64
>  	def_bool 64BIT
> diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c
> index 852085ada368..7aae2f6f4973 100644
> --- a/arch/sparc/mm/srmmu.c
> +++ b/arch/sparc/mm/srmmu.c
> @@ -975,7 +975,6 @@ void __init srmmu_paging_init(void)
>  	{
>  		unsigned long max_zone_pfn[MAX_NR_ZONES] = { 0 };
>  
> -		max_zone_pfn[ZONE_DMA] = max_low_pfn;
>  		max_zone_pfn[ZONE_NORMAL] = max_low_pfn;
>  		max_zone_pfn[ZONE_HIGHMEM] = highend_pfn;
>  
> 
Thanks,
Andreas
Arnd Bergmann March 5, 2024, 3:26 p.m. UTC | #5 
On Tue, Mar 5, 2024, at 16:06, Andreas Larsson wrote:
> On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:

>> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
>> index 734f23daecca..bdbde506c01e 100644
>> --- a/arch/sparc/Kconfig
>> +++ b/arch/sparc/Kconfig
>> @@ -62,7 +62,6 @@ config SPARC32
>>  	select HAVE_UID16
>>  	select LOCK_MM_AND_FIND_VMA
>>  	select OLD_SIGACTION
>> -	select ZONE_DMA
>
> This however makes a number of PCI drivers that depend on
> ZONE_DMA unselectable.

I think that is the correct thing to do then: the only
drivers that I see with this dependency are PCI sound cards
that apparently rely on DMA to the 16MB ISA range, which is
not provided by sparc.

      Arnd
Andreas Larsson March 6, 2024, 2:19 p.m. UTC | #6 
On 2024-03-05 16:26, Arnd Bergmann wrote:
> On Tue, Mar 5, 2024, at 16:06, Andreas Larsson wrote:
>> On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
> 
>>> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
>>> index 734f23daecca..bdbde506c01e 100644
>>> --- a/arch/sparc/Kconfig
>>> +++ b/arch/sparc/Kconfig
>>> @@ -62,7 +62,6 @@ config SPARC32
>>>  	select HAVE_UID16
>>>  	select LOCK_MM_AND_FIND_VMA
>>>  	select OLD_SIGACTION
>>> -	select ZONE_DMA
>>
>> This however makes a number of PCI drivers that depend on
>> ZONE_DMA unselectable.
> 
> I think that is the correct thing to do then: the only
> drivers that I see with this dependency are PCI sound cards
> that apparently rely on DMA to the 16MB ISA range, which is
> not provided by sparc.

The ZONE_DMA dependency does not seem related to ISA per se. Commit
80ab8eae70e5 ("ALSA: Enable CONFIG_ZONE_DMA for smaller PCI DMA masks")
that started to introduce it did were about ensuring 32-bit masks.

Some of those sound card drivers sets a 24 bit mask, i.e. a 0-16MB
range, but some among those sets a 28, and 30 bit DMA mask with
dma_set_mask_and_coherent. 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 am also a bit concerned if removing ZONE_DMA will let DMA be allocated
in highmem and what that could lead to.

Cheers,
Andreas
Arnd Bergmann March 6, 2024, 2:45 p.m. UTC | #7 
On Wed, Mar 6, 2024, at 15:19, Andreas Larsson wrote:
> On 2024-03-05 16:26, Arnd Bergmann wrote:
>> On Tue, Mar 5, 2024, at 16:06, Andreas Larsson wrote:
>>> On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
>> 
>>>> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
>>>> index 734f23daecca..bdbde506c01e 100644
>>>> --- a/arch/sparc/Kconfig
>>>> +++ b/arch/sparc/Kconfig
>>>> @@ -62,7 +62,6 @@ config SPARC32
>>>>  	select HAVE_UID16
>>>>  	select LOCK_MM_AND_FIND_VMA
>>>>  	select OLD_SIGACTION
>>>> -	select ZONE_DMA
>>>
>>> This however makes a number of PCI drivers that depend on
>>> ZONE_DMA unselectable.
>> 
>> I think that is the correct thing to do then: the only
>> drivers that I see with this dependency are PCI sound cards
>> that apparently rely on DMA to the 16MB ISA range, which is
>> not provided by sparc.
>
> The ZONE_DMA dependency does not seem related to ISA per se. Commit
> 80ab8eae70e5 ("ALSA: Enable CONFIG_ZONE_DMA for smaller PCI DMA masks")
> that started to introduce it did were about ensuring 32-bit masks.
>
> Some of those sound card drivers sets a 24 bit mask, i.e. a 0-16MB
> range, but some among those sets a 28, and 30 bit DMA mask with
> dma_set_mask_and_coherent.

Ah right, I see it now.

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

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

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

      Arnd
Christoph Hellwig March 6, 2024, 3:04 p.m. UTC | #8 
On Wed, Mar 06, 2024 at 03:19:52PM +0100, Andreas Larsson wrote:
> > I think that is the correct thing to do then: the only
> > drivers that I see with this dependency are PCI sound cards
> > that apparently rely on DMA to the 16MB ISA range, which is
> > not provided by sparc.
> 
> The ZONE_DMA dependency does not seem related to ISA per se. Commit
> 80ab8eae70e5 ("ALSA: Enable CONFIG_ZONE_DMA for smaller PCI DMA masks")
> that started to introduce it did were about ensuring 32-bit masks.

Yikes!  That commit is just unbelievable buggy.  CONFIG_ZONE_DMA
is only for architetures to select, not drivers.  A driver randomly
enabling such an arch zone is just crazy.

I've been wondering for a while if we need some Kconfig magic
so that certain symbols can only be select from arch/* and not
elsewhere to prevent this (we had a few other similar cases like
DMA_MAP_OPS).
Arnd Bergmann March 6, 2024, 3:24 p.m. UTC | #9 
On Wed, Mar 6, 2024, at 16:04, Christoph Hellwig wrote:
> On Wed, Mar 06, 2024 at 03:19:52PM +0100, Andreas Larsson wrote:
>> > I think that is the correct thing to do then: the only
>> > drivers that I see with this dependency are PCI sound cards
>> > that apparently rely on DMA to the 16MB ISA range, which is
>> > not provided by sparc.
>> 
>> The ZONE_DMA dependency does not seem related to ISA per se. Commit
>> 80ab8eae70e5 ("ALSA: Enable CONFIG_ZONE_DMA for smaller PCI DMA masks")
>> that started to introduce it did were about ensuring 32-bit masks.
>
> Yikes!  That commit is just unbelievable buggy.  CONFIG_ZONE_DMA
> is only for architetures to select, not drivers.  A driver randomly
> enabling such an arch zone is just crazy.

It looks like it did not remain that way for long, as
2db1a57986d3 ("ALSA: pci: depend on ZONE_DMA") removed
the broken select again.

> I've been wondering for a while if we need some Kconfig magic
> so that certain symbols can only be select from arch/* and not
> elsewhere to prevent this (we had a few other similar cases like
> DMA_MAP_OPS).

It's a nice idea, but it would require a lot of reworks to get right,
with things like drivers/platform/x86 or drivers/{parisc,s390,sh}
that are still somewhat architecture code, and subsystems like
drivers/gpu/drm that typically just select whatever they want
instead of using dependencies.

      Arnd
Andreas Larsson March 6, 2024, 3:31 p.m. UTC | #10 
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
Arnd Bergmann March 6, 2024, 4:22 p.m. UTC | #11 
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
Arnd Bergmann March 6, 2024, 6:19 p.m. UTC | #12 
On Wed, Mar 6, 2024, at 17:22, Arnd Bergmann wrote:
> On Wed, Mar 6, 2024, at 16:31, Andreas Larsson wrote:

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

I had another look and found that this is a result of

arch/sparc/include/asm/page_32.h:#define PAGE_OFFSET    0xf0000000

which gives 3840MiB to userspace addresses, leaving only
256MiB for kernel lowmem and vmalloc space, which is
less than any other architectures.

I still don't know the history behind this choice, but I
see this was already configured the same when arch/sparc/
was originally merged. You can probably change it to a more
sensible 0xc0000000 or 0x80000000 like on other
architectures and run without highmem on anything with
less than 2GB of total RAM.

How much RAM do Leon machines have typically, or at the
maximum?

     Arnd
Andreas Larsson March 7, 2024, 12:42 p.m. UTC | #13 
On 2024-03-06 19:19, Arnd Bergmann wrote:
> On Wed, Mar 6, 2024, at 17:22, Arnd Bergmann wrote:
>> On Wed, Mar 6, 2024, at 16:31, Andreas Larsson wrote:
> 
>> 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.
> 
> I had another look and found that this is a result of
> 
> arch/sparc/include/asm/page_32.h:#define PAGE_OFFSET    0xf0000000
> 
> which gives 3840MiB to userspace addresses, leaving only
> 256MiB for kernel lowmem and vmalloc space, which is
> less than any other architectures.
> 
> I still don't know the history behind this choice, but I
> see this was already configured the same when arch/sparc/
> was originally merged. You can probably change it to a more
> sensible 0xc0000000 or 0x80000000 like on other
> architectures and run without highmem on anything with
> less than 2GB of total RAM.
> 
> How much RAM do Leon machines have typically, or at the
> maximum?
The amount of RAM can vary greatly between systems, from less that
128 MiB up to 2 GiB. An upcoming design uses the entire 36-bit
physical address space and have the possibility of having up to 60
GiB memory.

Thanks,
Andreas
Arnd Bergmann March 7, 2024, 1:06 p.m. UTC | #14 
On Thu, Mar 7, 2024, at 13:42, Andreas Larsson wrote:
> On 2024-03-06 19:19, Arnd Bergmann wrote:
>
>> I still don't know the history behind this choice, but I
>> see this was already configured the same when arch/sparc/
>> was originally merged. You can probably change it to a more
>> sensible 0xc0000000 or 0x80000000 like on other
>> architectures and run without highmem on anything with
>> less than 2GB of total RAM.
>> 
>> How much RAM do Leon machines have typically, or at the
>> maximum?
> The amount of RAM can vary greatly between systems, from less that
> 128 MiB up to 2 GiB. An upcoming design uses the entire 36-bit
> physical address space and have the possibility of having up to 60
> GiB memory.

If the current maximum is 2GiB, that would be easily handled
by making PAGE_OFFSET configurable the same way that x86 or
arm do, where the common CONFIG_VMSPLIT_3G would extend lowmem
from 192MiB to a little under 1GiB, with PAGE_OFFSET 0xc0000000,
and VMSPLIT_2G_OPT allows exactly 2GiB of lowmem, plus a little
under 2GiB of user addressing.

This would also be an improvement over using highmem for
most applications today, and it means you can keep using the
same setup once highmem support gets removed from the kernel.

For a new design, I would strongly advise against advertising
support for more than 2GiB on a 32-bit Linux kernel, as this
is likely to not be supportable in the future. Note that
in the best case today (40 byte struct page), the mem_map[]
array uses 10MiB of lowmem per GiB installed memory, so you
quickly run out of lowmem when you allow more RAM.

     Arnd
diff mbox series

Patch

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 734f23daecca..bdbde506c01e 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -62,7 +62,6 @@  config SPARC32
 	select HAVE_UID16
 	select LOCK_MM_AND_FIND_VMA
 	select OLD_SIGACTION
-	select ZONE_DMA
 
 config SPARC64
 	def_bool 64BIT
diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c
index 852085ada368..7aae2f6f4973 100644
--- a/arch/sparc/mm/srmmu.c
+++ b/arch/sparc/mm/srmmu.c
@@ -975,7 +975,6 @@  void __init srmmu_paging_init(void)
 	{
 		unsigned long max_zone_pfn[MAX_NR_ZONES] = { 0 };
 
-		max_zone_pfn[ZONE_DMA] = max_low_pfn;
 		max_zone_pfn[ZONE_NORMAL] = max_low_pfn;
 		max_zone_pfn[ZONE_HIGHMEM] = highend_pfn;