mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* Re: [PATCH] ARM: bootm: recalculate decompression space
       [not found] <alpine.DEB.2.00.1607051136430.23712@blala.de>
@ 2016-07-05  9:47 ` Alexander Kurz
  2016-07-05  9:58   ` Sascha Hauer
  0 siblings, 1 reply; 4+ messages in thread
From: Alexander Kurz @ 2016-07-05  9:47 UTC (permalink / raw)
  To: s.mueller-klieser; +Cc: barebox

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ARM: bootm: recalculate decompression space
  2016-07-05  9:47 ` [PATCH] ARM: bootm: recalculate decompression space Alexander Kurz
@ 2016-07-05  9:58   ` Sascha Hauer
  0 siblings, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2016-07-05  9:58 UTC (permalink / raw)
  To: Alexander Kurz; +Cc: barebox, s.mueller-klieser

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ARM: bootm: recalculate decompression space
  2016-07-01 14:48 Stefan Müller-Klieser
@ 2016-07-04  9:32 ` Sascha Hauer
  0 siblings, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2016-07-04  9:32 UTC (permalink / raw)
  To: Stefan Müller-Klieser; +Cc: barebox

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] ARM: bootm: recalculate decompression space
@ 2016-07-01 14:48 Stefan Müller-Klieser
  2016-07-04  9:32 ` Sascha Hauer
  0 siblings, 1 reply; 4+ messages in thread
From: Stefan Müller-Klieser @ 2016-07-01 14:48 UTC (permalink / raw)
  To: barebox

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-07-05  9:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <alpine.DEB.2.00.1607051136430.23712@blala.de>
2016-07-05  9:47 ` [PATCH] ARM: bootm: recalculate decompression space Alexander Kurz
2016-07-05  9:58   ` Sascha Hauer
2016-07-01 14:48 Stefan Müller-Klieser
2016-07-04  9:32 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox