[PATCH v3 0/2] arm64: Fix CMA/crashkernel reservation

[PATCH v3 0/2] arm64: Fix CMA/crashkernel reservation

[Nicolas Saenz Julienne]

As pointed out by Qian Cai[1] the series enabling ZONE_DMA in arm64
breaks CMA/crashkernel reservations on large devices, as it changed its
default placement. After discussing it with Catalin Marinas we're
restoring the old behavior.

The Raspberry Pi 4, being the only device that needs CMA and crashkernel
in ZONE_DMA will explicitly do so trough it's device tree.

[1] https://lkml.org/lkml/2019/10/21/725

---

Changes since v1:
  - Move CMA area registration into bcm2711.dtsi

Changes since v1:
  - s/Raspberry Pi/bcm2711 in bcm2711.dtsi

Nicolas Saenz Julienne (2):
  ARM: dts: bcm2711: force CMA into first GB of memory
  arm64: mm: reserve CMA and crashkernel in ZONE_DMA32

 arch/arm/boot/dts/bcm2711.dtsi | 20 ++++++++++++++++++++
 arch/arm64/mm/init.c           |  4 ++--
 2 files changed, 22 insertions(+), 2 deletions(-)

-- 
2.23.0

[PATCH v3 1/2] ARM: dts: bcm2711: force CMA into first GB of memory

[Nicolas Saenz Julienne]

arm64 places the CMA in ZONE_DMA32, which is not good enough for the
Raspberry Pi 4 since it contains peripherals that can only address the
first GB of memory. Explicitly place the CMA into that area.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

---

Changes since v1:
  - Move into bcm2711.dtsi

Changes since v1:
  - s/Raspberry Pi/bcm2711

 arch/arm/boot/dts/bcm2711.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index ac83dac2e6ba..667658497898 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -12,6 +12,26 @@
 
 	interrupt-parent = <&gicv2>;
 
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges;
+
+		/*
+		 * arm64 reserves the CMA by default somewhere in ZONE_DMA32,
+		 * that's not good enough for bcm2711 as some devices can
+		 * only address the lower 1G of memory (ZONE_DMA).
+		 */
+		linux,cma {
+			compatible = "shared-dma-pool";
+			size = <0x2000000>; /* 32MB */
+			alloc-ranges = <0x0 0x00000000 0x40000000>;
+			reusable;
+			linux,cma-default;
+		};
+	};
+
+
 	soc {
 		/*
 		 * Defined ranges:
-- 
2.23.0

[PATCH v3 2/2] arm64: mm: reserve CMA and crashkernel in ZONE_DMA32

[Nicolas Saenz Julienne]

With the introduction of ZONE_DMA in arm64 we moved the default CMA and
crashkernel reservation into that area. This caused a regression on big
machines that need big CMA and crashkernel reservations. Note that
ZONE_DMA is only 1GB big.

Restore the previous behavior as the wide majority of devices are OK
with reserving these in ZONE_DMA32. The ones that need them in ZONE_DMA
will configure it explicitly.

Reported-by: Qian Cai <cai@lca.pw>
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 arch/arm64/mm/init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 580d1052ac34..8385d3c0733f 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -88,7 +88,7 @@ static void __init reserve_crashkernel(void)
 
 	if (crash_base == 0) {
 		/* Current arm64 boot protocol requires 2MB alignment */
-		crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT,
+		crash_base = memblock_find_in_range(0, arm64_dma32_phys_limit,
 				crash_size, SZ_2M);
 		if (crash_base == 0) {
 			pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
@@ -454,7 +454,7 @@ void __init arm64_memblock_init(void)
 
 	high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
 
-	dma_contiguous_reserve(arm64_dma_phys_limit ? : arm64_dma32_phys_limit);
+	dma_contiguous_reserve(arm64_dma32_phys_limit);
 }
 
 void __init bootmem_init(void)
-- 
2.23.0

Re: [PATCH v3 1/2] ARM: dts: bcm2711: force CMA into first GB of memory

[Catalin Marinas]

Hi Nicolas,

On Thu, Nov 07, 2019 at 10:56:10AM +0100, Nicolas Saenz Julienne wrote:
> diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
> index ac83dac2e6ba..667658497898 100644
> --- a/arch/arm/boot/dts/bcm2711.dtsi
> +++ b/arch/arm/boot/dts/bcm2711.dtsi
> @@ -12,6 +12,26 @@
>  
>  	interrupt-parent = <&gicv2>;
>  
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		/*
> +		 * arm64 reserves the CMA by default somewhere in ZONE_DMA32,
> +		 * that's not good enough for bcm2711 as some devices can
> +		 * only address the lower 1G of memory (ZONE_DMA).
> +		 */
> +		linux,cma {
> +			compatible = "shared-dma-pool";
> +			size = <0x2000000>; /* 32MB */
> +			alloc-ranges = <0x0 0x00000000 0x40000000>;
> +			reusable;
> +			linux,cma-default;
> +		};
> +	};
> +
> +
>  	soc {
>  		/*
>  		 * Defined ranges:

Sorry, I just realised I can't merge this as it depends on a patch
that's only in -next: 7dbe8c62ceeb ("ARM: dts: Add minimal Raspberry Pi
4 support").

I'll queue the second patch in the series to fix the regression
introduces by the ZONE_DMA patches and, AFAICT, the dts update can be
queued independently.

-- 
Catalin

Re: [PATCH v3 1/2] ARM: dts: bcm2711: force CMA into first GB of memory

[Florian Fainelli]

On 11/7/19 3:20 AM, Catalin Marinas wrote:
> Hi Nicolas,
> 
> On Thu, Nov 07, 2019 at 10:56:10AM +0100, Nicolas Saenz Julienne wrote:
>> diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
>> index ac83dac2e6ba..667658497898 100644
>> --- a/arch/arm/boot/dts/bcm2711.dtsi
>> +++ b/arch/arm/boot/dts/bcm2711.dtsi
>> @@ -12,6 +12,26 @@
>>  
>>  	interrupt-parent = <&gicv2>;
>>  
>> +	reserved-memory {
>> +		#address-cells = <2>;
>> +		#size-cells = <1>;
>> +		ranges;
>> +
>> +		/*
>> +		 * arm64 reserves the CMA by default somewhere in ZONE_DMA32,
>> +		 * that's not good enough for bcm2711 as some devices can
>> +		 * only address the lower 1G of memory (ZONE_DMA).
>> +		 */
>> +		linux,cma {
>> +			compatible = "shared-dma-pool";
>> +			size = <0x2000000>; /* 32MB */
>> +			alloc-ranges = <0x0 0x00000000 0x40000000>;
>> +			reusable;
>> +			linux,cma-default;
>> +		};
>> +	};
>> +
>> +
>>  	soc {
>>  		/*
>>  		 * Defined ranges:
> 
> Sorry, I just realised I can't merge this as it depends on a patch
> that's only in -next: 7dbe8c62ceeb ("ARM: dts: Add minimal Raspberry Pi
> 4 support").
> 
> I'll queue the second patch in the series to fix the regression
> introduces by the ZONE_DMA patches and, AFAICT, the dts update can be
> queued independently.

I will take it directly, unless you have more stuff coming Stefan?
-- 
Florian

Re: [PATCH v3 1/2] ARM: dts: bcm2711: force CMA into first GB of memory

[Stefan Wahren]

Am 07.11.19 um 18:59 schrieb Florian Fainelli:
> On 11/7/19 3:20 AM, Catalin Marinas wrote:
>> Hi Nicolas,
...
>> Sorry, I just realised I can't merge this as it depends on a patch
>> that's only in -next: 7dbe8c62ceeb ("ARM: dts: Add minimal Raspberry Pi
>> 4 support").
>>
>> I'll queue the second patch in the series to fix the regression
>> introduces by the ZONE_DMA patches and, AFAICT, the dts update can be
>> queued independently.
> I will take it directly, unless you have more stuff coming Stefan?
Please take. Thanks

Stefan

Re: [PATCH v3 1/2] ARM: dts: bcm2711: force CMA into first GB of memory

[Florian Fainelli]

On 11/7/19 11:09 AM, Stefan Wahren wrote:
> 
> Am 07.11.19 um 18:59 schrieb Florian Fainelli:
>> On 11/7/19 3:20 AM, Catalin Marinas wrote:
>>> Hi Nicolas,
> ...
>>> Sorry, I just realised I can't merge this as it depends on a patch
>>> that's only in -next: 7dbe8c62ceeb ("ARM: dts: Add minimal Raspberry Pi
>>> 4 support").
>>>
>>> I'll queue the second patch in the series to fix the regression
>>> introduces by the ZONE_DMA patches and, AFAICT, the dts update can be
>>> queued independently.
>> I will take it directly, unless you have more stuff coming Stefan?
> Please take. Thanks

I picked up v2 because it had your explicit Acked-by tag, but amended in
a similar way to what Nicolas did, except s/Raspberry Pi 4/BCM2711/:

https://github.com/Broadcom/stblinux/commit/d98a8dbdaec628f5c993cc711ba9ab98fe909f0f

neither of you will probably mind me having done that.
-- 
Florian

Re: [PATCH v3 1/2] ARM: dts: bcm2711: force CMA into first GB of memory

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]

On Thu, 2019-11-14 at 12:35 -0800, Florian Fainelli wrote:
> On 11/7/19 11:09 AM, Stefan Wahren wrote:
> > Am 07.11.19 um 18:59 schrieb Florian Fainelli:
> > > On 11/7/19 3:20 AM, Catalin Marinas wrote:
> > > > Hi Nicolas,
> > ...
> > > > Sorry, I just realised I can't merge this as it depends on a patch
> > > > that's only in -next: 7dbe8c62ceeb ("ARM: dts: Add minimal Raspberry Pi
> > > > 4 support").
> > > > 
> > > > I'll queue the second patch in the series to fix the regression
> > > > introduces by the ZONE_DMA patches and, AFAICT, the dts update can be
> > > > queued independently.
> > > I will take it directly, unless you have more stuff coming Stefan?
> > Please take. Thanks
> 
> I picked up v2 because it had your explicit Acked-by tag, but amended in
> a similar way to what Nicolas did, except s/Raspberry Pi 4/BCM2711/:
> 
> 
https://github.com/Broadcom/stblinux/commit/d98a8dbdaec628f5c993cc711ba9ab98fe909f0f
> 
> neither of you will probably mind me having done that.

All good, thanks!


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 2/2] arm64: mm: reserve CMA and crashkernel in ZONE_DMA32

[Jon Masters]

Hi Nicolas,

On 11/7/19 4:56 AM, Nicolas Saenz Julienne wrote:
> With the introduction of ZONE_DMA in arm64 we moved the default CMA and
> crashkernel reservation into that area. This caused a regression on big
> machines that need big CMA and crashkernel reservations. Note that
> ZONE_DMA is only 1GB big.
> 
> Restore the previous behavior as the wide majority of devices are OK
> with reserving these in ZONE_DMA32. The ones that need them in ZONE_DMA
> will configure it explicitly.
> 
> Reported-by: Qian Cai <cai@lca.pw>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>   arch/arm64/mm/init.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 580d1052ac34..8385d3c0733f 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -88,7 +88,7 @@ static void __init reserve_crashkernel(void)
>   
>   	if (crash_base == 0) {
>   		/* Current arm64 boot protocol requires 2MB alignment */
> -		crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT,
> +		crash_base = memblock_find_in_range(0, arm64_dma32_phys_limit,
>   				crash_size, SZ_2M);
>   		if (crash_base == 0) {
>   			pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
> @@ -454,7 +454,7 @@ void __init arm64_memblock_init(void)
>   
>   	high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
>   
> -	dma_contiguous_reserve(arm64_dma_phys_limit ? : arm64_dma32_phys_limit);
> +	dma_contiguous_reserve(arm64_dma32_phys_limit);
>   }
>   
>   void __init bootmem_init(void)

Can we get a bit more of a backstory about what the regression was on 
larger machines? If the 32-bit DMA region is too small, but the machine 
otherwise has plenty of memory, the crashkernel reservation will fail. 
Most e.g. enterprise users aren't going to respond to that situation by 
determining the placement manually, they'll just not have a crashkernel.

Jon.

-- 
Computer Architect

Re: [PATCH v3 2/2] arm64: mm: reserve CMA and crashkernel in ZONE_DMA32

[Jon Masters]

On 3/22/21 2:34 PM, Jon Masters wrote:
> Hi Nicolas,
> 
> On 11/7/19 4:56 AM, Nicolas Saenz Julienne wrote:
>> With the introduction of ZONE_DMA in arm64 we moved the default CMA and
>> crashkernel reservation into that area. This caused a regression on big
>> machines that need big CMA and crashkernel reservations. Note that
>> ZONE_DMA is only 1GB big.
>>
>> Restore the previous behavior as the wide majority of devices are OK
>> with reserving these in ZONE_DMA32. The ones that need them in ZONE_DMA
>> will configure it explicitly.
>>
>> Reported-by: Qian Cai <cai@lca.pw>
>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>> ---
>>   arch/arm64/mm/init.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 580d1052ac34..8385d3c0733f 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -88,7 +88,7 @@ static void __init reserve_crashkernel(void)
>>       if (crash_base == 0) {
>>           /* Current arm64 boot protocol requires 2MB alignment */
>> -        crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT,
>> +        crash_base = memblock_find_in_range(0, arm64_dma32_phys_limit,
>>                   crash_size, SZ_2M);
>>           if (crash_base == 0) {
>>               pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
>> @@ -454,7 +454,7 @@ void __init arm64_memblock_init(void)
>>       high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
>> -    dma_contiguous_reserve(arm64_dma_phys_limit ? : 
>> arm64_dma32_phys_limit);
>> +    dma_contiguous_reserve(arm64_dma32_phys_limit);
>>   }
>>   void __init bootmem_init(void)
> 
> Can we get a bit more of a backstory about what the regression was on 
> larger machines? If the 32-bit DMA region is too small, but the machine 
> otherwise has plenty of memory, the crashkernel reservation will fail. 
> Most e.g. enterprise users aren't going to respond to that situation by 
> determining the placement manually, they'll just not have a crashkernel.

Nevermind, looks like Catalin already changed this logic in Jan 2021 by 
removing arm64_dma32_phys_limit and I'm out of date.

Jon.

-- 
Computer Architect

Re: [PATCH v3 2/2] arm64: mm: reserve CMA and crashkernel in ZONE_DMA32

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 2652 bytes --]

On Mon, 2021-03-22 at 14:40 -0400, Jon Masters wrote:
> On 3/22/21 2:34 PM, Jon Masters wrote:
> > Hi Nicolas,
> > 
> > On 11/7/19 4:56 AM, Nicolas Saenz Julienne wrote:
> > > With the introduction of ZONE_DMA in arm64 we moved the default CMA and
> > > crashkernel reservation into that area. This caused a regression on big
> > > machines that need big CMA and crashkernel reservations. Note that
> > > ZONE_DMA is only 1GB big.
> > > 
> > > Restore the previous behavior as the wide majority of devices are OK
> > > with reserving these in ZONE_DMA32. The ones that need them in ZONE_DMA
> > > will configure it explicitly.
> > > 
> > > Reported-by: Qian Cai <cai@lca.pw>
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > ---
> > >   arch/arm64/mm/init.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > > index 580d1052ac34..8385d3c0733f 100644
> > > --- a/arch/arm64/mm/init.c
> > > +++ b/arch/arm64/mm/init.c
> > > @@ -88,7 +88,7 @@ static void __init reserve_crashkernel(void)
> > >       if (crash_base == 0) {
> > >           /* Current arm64 boot protocol requires 2MB alignment */
> > > -        crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT,
> > > +        crash_base = memblock_find_in_range(0, arm64_dma32_phys_limit,
> > >                   crash_size, SZ_2M);
> > >           if (crash_base == 0) {
> > >               pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
> > > @@ -454,7 +454,7 @@ void __init arm64_memblock_init(void)
> > >       high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
> > > -    dma_contiguous_reserve(arm64_dma_phys_limit ? : 
> > > arm64_dma32_phys_limit);
> > > +    dma_contiguous_reserve(arm64_dma32_phys_limit);
> > >   }
> > >   void __init bootmem_init(void)
> > 
> > Can we get a bit more of a backstory about what the regression was on 
> > larger machines? If the 32-bit DMA region is too small, but the machine 
> > otherwise has plenty of memory, the crashkernel reservation will fail. 
> > Most e.g. enterprise users aren't going to respond to that situation by 
> > determining the placement manually, they'll just not have a crashkernel.
> 
> Nevermind, looks like Catalin already changed this logic in Jan 2021 by 
> removing arm64_dma32_phys_limit and I'm out of date.

Also see this series (already merged):

https://lore.kernel.org/linux-arm-kernel/20201119175400.9995-1-nsaenzjulienne@suse.de/

Regads,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]