Re: [PATCH] ARM: bootm: recalculate decompression space

Re: [PATCH] ARM: bootm: recalculate decompression space

[Alexander Kurz]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1777 bytes --]

Hi Stefan,
FYI, I just got a compiler warning obviously related to your patch
On Fri Jul 1 07:48:39 PDT 2016, Stefan Müller-Klieser wrote:

> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 803aa94..be1259b 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -81,10 +81,15 @@ static int get_kernel_addresses(size_t image_size,
>  		return ret;
>  
>  	/*
> -	 * We don't know the exact decompressed size so just use a 
> conservative
> -	 * default of 4 times the size of the compressed image.
> +	 * The kernel documentation "Documentation/arm/Booting" advises
> +	 * to place the compressed image outside of the lowest 32 MiB to
> +	 * avoid relocation. We should do this if we have at least 64 MiB
> +	 * of ram. If we have less space, we assume a maximum
> +	 * compression factor of 5.
>  	 */
> -	image_decomp_size = PAGE_ALIGN(image_size * 4);
> +	image_decomp_size = PAGE_ALIGN(image_size * 5);
> +	if (mem_size >= SZ_64M)
> +		image_decomp_size = max(image_decomp_size, SZ_32M);
>  
>  	/*
>  	 * By default put oftree/initrd close behind compressed kernel 
> image to

this one:
>   CC      arch/arm/lib/bootm.o
> In file included from include/common.h:30:0,
>                  from arch/arm/lib/bootm.c:2:
> arch/arm/lib/bootm.c: In function 'get_kernel_addresses':
> include/linux/kernel.h:110:17: warning: comparison of distinct pointer 
> types lacks a cast
>   (void) (&_max1 == &_max2);  \
>                 ^
> arch/arm/lib/bootm.c:92:23: note: in expansion of macro 'max'
>    image_decomp_size = max(image_decomp_size, SZ_32M);
>                        ^

I am using
$ gcc --version
arm-arm1136jfs-linux-gnueabi-gcc (crosstool-NG crosstool-ng-1.22.0) 5.2.0

Regards, Alexander

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: bootm: recalculate decompression space

[Sascha Hauer]

On Tue, Jul 05, 2016 at 11:47:08AM +0200, Alexander Kurz wrote:
> Hi Stefan,
> FYI, I just got a compiler warning obviously related to your patch
> On Fri Jul 1 07:48:39 PDT 2016, Stefan Müller-Klieser wrote:
> 
> > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> > index 803aa94..be1259b 100644
> > --- a/arch/arm/lib/bootm.c
> > +++ b/arch/arm/lib/bootm.c
> > @@ -81,10 +81,15 @@ static int get_kernel_addresses(size_t image_size,
> >  		return ret;
> >  
> >  	/*
> > -	 * We don't know the exact decompressed size so just use a 
> > conservative
> > -	 * default of 4 times the size of the compressed image.
> > +	 * The kernel documentation "Documentation/arm/Booting" advises
> > +	 * to place the compressed image outside of the lowest 32 MiB to
> > +	 * avoid relocation. We should do this if we have at least 64 MiB
> > +	 * of ram. If we have less space, we assume a maximum
> > +	 * compression factor of 5.
> >  	 */
> > -	image_decomp_size = PAGE_ALIGN(image_size * 4);
> > +	image_decomp_size = PAGE_ALIGN(image_size * 5);
> > +	if (mem_size >= SZ_64M)
> > +		image_decomp_size = max(image_decomp_size, SZ_32M);
> >  
> >  	/*
> >  	 * By default put oftree/initrd close behind compressed kernel 
> > image to
> 
> this one:
> >   CC      arch/arm/lib/bootm.o
> > In file included from include/common.h:30:0,
> >                  from arch/arm/lib/bootm.c:2:
> > arch/arm/lib/bootm.c: In function 'get_kernel_addresses':
> > include/linux/kernel.h:110:17: warning: comparison of distinct pointer 
> > types lacks a cast
> >   (void) (&_max1 == &_max2);  \

I get that aswell. Will change to:

	max_t(size_t, image_decomp_size, SZ_32M);

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH] ARM: bootm: recalculate decompression space

[Stefan Müller-Klieser]

According to the kernel documentation it is recommended to place the
compressed image between 32 MiB and 128 MiB. The DTB and initrd should
be placed above 128 MiB. We will follow the recommendation as long as we
have enough RAM. If this is not the case, we fall back to the scheme.
This change is required because of the ARM default kernel config changes
regarding RODATA layout, which lead to an increased compression factor
of the kernel image.
This should be regarded as an intermediate solution until there is a
mechanism for the kernel image to report the decompressed layout
requirements to the bootloader.

Signed-off-by: Stefan Müller-Klieser <s.mueller-klieser@phytec.de>
---
 arch/arm/lib/bootm.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 803aa94..be1259b 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -81,10 +81,15 @@ static int get_kernel_addresses(size_t image_size,
 		return ret;
 
 	/*
-	 * We don't know the exact decompressed size so just use a conservative
-	 * default of 4 times the size of the compressed image.
+	 * The kernel documentation "Documentation/arm/Booting" advises
+	 * to place the compressed image outside of the lowest 32 MiB to
+	 * avoid relocation. We should do this if we have at least 64 MiB
+	 * of ram. If we have less space, we assume a maximum
+	 * compression factor of 5.
 	 */
-	image_decomp_size = PAGE_ALIGN(image_size * 4);
+	image_decomp_size = PAGE_ALIGN(image_size * 5);
+	if (mem_size >= SZ_64M)
+		image_decomp_size = max(image_decomp_size, SZ_32M);
 
 	/*
 	 * By default put oftree/initrd close behind compressed kernel image to
@@ -113,6 +118,13 @@ static int get_kernel_addresses(size_t image_size,
 
 	*mem_free = PAGE_ALIGN(*load_address + image_size + spacing);
 
+	/*
+	 * Place oftree/initrd outside of the first 128 MiB, if we have space
+	 * for it. This avoids potential conflicts with the kernel decompressor.
+	 */
+	if (mem_size > SZ_256M)
+		*mem_free = max(*mem_free, mem_start + SZ_128M);
+
 	return 0;
 }
 
-- 
1.9.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: bootm: recalculate decompression space

[Sascha Hauer]

On Fri, Jul 01, 2016 at 04:48:39PM +0200, Stefan Müller-Klieser wrote:
> According to the kernel documentation it is recommended to place the
> compressed image between 32 MiB and 128 MiB. The DTB and initrd should
> be placed above 128 MiB. We will follow the recommendation as long as we
> have enough RAM. If this is not the case, we fall back to the scheme.
> This change is required because of the ARM default kernel config changes
> regarding RODATA layout, which lead to an increased compression factor
> of the kernel image.
> This should be regarded as an intermediate solution until there is a
> mechanism for the kernel image to report the decompressed layout
> requirements to the bootloader.
> 
> Signed-off-by: Stefan Müller-Klieser <s.mueller-klieser@phytec.de>
> ---
>  arch/arm/lib/bootm.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

Applied, thanks.

Let's hope we don't have to revisit this too soon.

Sascha

> 
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 803aa94..be1259b 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -81,10 +81,15 @@ static int get_kernel_addresses(size_t image_size,
>  		return ret;
>  
>  	/*
> -	 * We don't know the exact decompressed size so just use a conservative
> -	 * default of 4 times the size of the compressed image.
> +	 * The kernel documentation "Documentation/arm/Booting" advises
> +	 * to place the compressed image outside of the lowest 32 MiB to
> +	 * avoid relocation. We should do this if we have at least 64 MiB
> +	 * of ram. If we have less space, we assume a maximum
> +	 * compression factor of 5.
>  	 */
> -	image_decomp_size = PAGE_ALIGN(image_size * 4);
> +	image_decomp_size = PAGE_ALIGN(image_size * 5);
> +	if (mem_size >= SZ_64M)
> +		image_decomp_size = max(image_decomp_size, SZ_32M);
>  
>  	/*
>  	 * By default put oftree/initrd close behind compressed kernel image to
> @@ -113,6 +118,13 @@ static int get_kernel_addresses(size_t image_size,
>  
>  	*mem_free = PAGE_ALIGN(*load_address + image_size + spacing);
>  
> +	/*
> +	 * Place oftree/initrd outside of the first 128 MiB, if we have space
> +	 * for it. This avoids potential conflicts with the kernel decompressor.
> +	 */
> +	if (mem_size > SZ_256M)
> +		*mem_free = max(*mem_free, mem_start + SZ_128M);
> +
>  	return 0;
>  }
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox