mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/2] uncompress: use read_full to fill decompression buffer
@ 2021-05-25 18:37 Lucas Stach
  2021-05-25 18:37 ` [PATCH 2/2] bootm: add support for booting compressed images Lucas Stach
  0 siblings, 1 reply; 8+ messages in thread
From: Lucas Stach @ 2021-05-25 18:37 UTC (permalink / raw)
  To: barebox

The decompression algorithms want all of the requested buffer size
to be filled and don't cope with less bytes being returned.
Use read_full to satisfy this requirement.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 lib/uncompress.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/uncompress.c b/lib/uncompress.c
index c47d319dbb5f..5c0d1e9f4d66 100644
--- a/lib/uncompress.c
+++ b/lib/uncompress.c
@@ -24,6 +24,7 @@
 #include <filetype.h>
 #include <malloc.h>
 #include <fs.h>
+#include <libfile.h>
 
 static void *uncompress_buf;
 static unsigned int uncompress_size;
@@ -142,7 +143,7 @@ static int uncompress_infd, uncompress_outfd;
 
 static int fill_fd(void *buf, unsigned int len)
 {
-	return read(uncompress_infd, buf, len);
+	return read_full(uncompress_infd, buf, len);
 }
 
 static int flush_fd(void *buf, unsigned int len)
-- 
2.29.2


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


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

* [PATCH 2/2] bootm: add support for booting compressed images
  2021-05-25 18:37 [PATCH 1/2] uncompress: use read_full to fill decompression buffer Lucas Stach
@ 2021-05-25 18:37 ` Lucas Stach
  2021-05-26  5:42   ` Sascha Hauer
  2021-05-26  9:17   ` Ahmad Fatoum
  0 siblings, 2 replies; 8+ messages in thread
From: Lucas Stach @ 2021-05-25 18:37 UTC (permalink / raw)
  To: barebox

ARM64 does not have a self extracting image format, but relies on the image
being externally compressed with one of the standard compression algorithms.

Add support for decompressing the bootm OS image. It is added in common
code as it may also be useful for other images/architectures.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 common/bootm.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/common/bootm.c b/common/bootm.c
index 092116beb94a..2bfb5cb01593 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -12,6 +12,7 @@
 #include <environment.h>
 #include <linux/stat.h>
 #include <magicvar.h>
+#include <uncompress.h>
 
 static LIST_HEAD(handler_list);
 
@@ -808,6 +809,85 @@ err_out:
 	return ret;
 }
 
+static int do_bootm_compressed(struct image_data *img_data)
+{
+	struct bootm_data bootm_data = {
+		.oftree_file = img_data->oftree_file,
+		.initrd_file = img_data->initrd_file,
+		.tee_file = img_data->tee_file,
+		.verbose = img_data->verbose,
+		.verify = img_data->verify,
+		.force = img_data->force,
+		.dryrun = img_data->dryrun,
+		.initrd_address = img_data->initrd_address,
+		.os_address = img_data->os_address,
+		.os_entry = img_data->os_entry,
+	};
+	int from, to, ret;
+	char *dstpath;
+
+	from = open(img_data->os_file, O_RDONLY);
+	if (from < 0)
+		return -ENODEV;
+
+	dstpath = make_temp("bootm-compressed");
+	if (!dstpath) {
+		ret = -ENOMEM;
+		goto fail_from;
+	}
+
+	to = open(dstpath, O_CREAT | O_WRONLY);
+	if (to < 0) {
+		ret = -ENODEV;
+		goto fail_make_temp;
+	}
+
+	ret = uncompress_fd_to_fd(from, to, uncompress_err_stdout);
+	if (ret)
+		goto fail_to;
+
+	bootm_data.os_file = dstpath;
+	return bootm_boot(&bootm_data);
+
+fail_to:
+	close(to);
+fail_make_temp:
+	free(dstpath);
+fail_from:
+	close(from);
+	return ret;
+}
+
+static struct image_handler bzip2_bootm_handler = {
+	.name = "BZIP2 compressed file",
+	.bootm = do_bootm_compressed,
+	.filetype = filetype_bzip2,
+};
+
+static struct image_handler gzip_bootm_handler = {
+	.name = "GZIP compressed file",
+	.bootm = do_bootm_compressed,
+	.filetype = filetype_gzip,
+};
+
+static struct image_handler lzo_bootm_handler = {
+	.name = "LZO compressed file",
+	.bootm = do_bootm_compressed,
+	.filetype = filetype_lzo_compressed,
+};
+
+static struct image_handler lz4_bootm_handler = {
+	.name = "LZ4 compressed file",
+	.bootm = do_bootm_compressed,
+	.filetype = filetype_lz4_compressed,
+};
+
+static struct image_handler xz_bootm_handler = {
+	.name = "XZ compressed file",
+	.bootm = do_bootm_compressed,
+	.filetype = filetype_xz_compressed,
+};
+
 static int bootm_init(void)
 {
 	globalvar_add_simple("bootm.image", NULL);
@@ -830,6 +910,18 @@ static int bootm_init(void)
 	globalvar_add_simple_enum("bootm.verify", (unsigned int *)&bootm_verify_mode,
 				  bootm_verify_names, ARRAY_SIZE(bootm_verify_names));
 
+
+	if (IS_ENABLED(CONFIG_BZLIB))
+		register_image_handler(&bzip2_bootm_handler);
+	if (IS_ENABLED(CONFIG_ZLIB))
+		register_image_handler(&gzip_bootm_handler);
+	if (IS_ENABLED(CONFIG_LZO_DECOMPRESS))
+		register_image_handler(&lzo_bootm_handler);
+	if (IS_ENABLED(CONFIG_LZ4_DECOMPRESS))
+		register_image_handler(&lz4_bootm_handler);
+	if (IS_ENABLED(CONFIG_XZ_DECOMPRESS))
+		register_image_handler(&xz_bootm_handler);
+
 	return 0;
 }
 late_initcall(bootm_init);
-- 
2.29.2


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


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

* Re: [PATCH 2/2] bootm: add support for booting compressed images
  2021-05-25 18:37 ` [PATCH 2/2] bootm: add support for booting compressed images Lucas Stach
@ 2021-05-26  5:42   ` Sascha Hauer
  2021-05-26  9:17   ` Ahmad Fatoum
  1 sibling, 0 replies; 8+ messages in thread
From: Sascha Hauer @ 2021-05-26  5:42 UTC (permalink / raw)
  To: Lucas Stach; +Cc: barebox

Hi Lucas,

On Tue, May 25, 2021 at 08:37:33PM +0200, Lucas Stach wrote:
> ARM64 does not have a self extracting image format, but relies on the image
> being externally compressed with one of the standard compression algorithms.
> 
> Add support for decompressing the bootm OS image. It is added in common
> code as it may also be useful for other images/architectures.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  common/bootm.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
> 
> diff --git a/common/bootm.c b/common/bootm.c
> index 092116beb94a..2bfb5cb01593 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -12,6 +12,7 @@
>  #include <environment.h>
>  #include <linux/stat.h>
>  #include <magicvar.h>
> +#include <uncompress.h>
>  
>  static LIST_HEAD(handler_list);
>  
> @@ -808,6 +809,85 @@ err_out:
>  	return ret;
>  }
>  
> +static int do_bootm_compressed(struct image_data *img_data)
> +{
> +	struct bootm_data bootm_data = {
> +		.oftree_file = img_data->oftree_file,
> +		.initrd_file = img_data->initrd_file,
> +		.tee_file = img_data->tee_file,
> +		.verbose = img_data->verbose,
> +		.verify = img_data->verify,
> +		.force = img_data->force,
> +		.dryrun = img_data->dryrun,
> +		.initrd_address = img_data->initrd_address,
> +		.os_address = img_data->os_address,
> +		.os_entry = img_data->os_entry,
> +	};
> +	int from, to, ret;
> +	char *dstpath;
> +
> +	from = open(img_data->os_file, O_RDONLY);
> +	if (from < 0)
> +		return -ENODEV;
> +
> +	dstpath = make_temp("bootm-compressed");
> +	if (!dstpath) {
> +		ret = -ENOMEM;
> +		goto fail_from;
> +	}
> +
> +	to = open(dstpath, O_CREAT | O_WRONLY);
> +	if (to < 0) {
> +		ret = -ENODEV;
> +		goto fail_make_temp;
> +	}
> +
> +	ret = uncompress_fd_to_fd(from, to, uncompress_err_stdout);
> +	if (ret)
> +		goto fail_to;
> +
> +	bootm_data.os_file = dstpath;
> +	return bootm_boot(&bootm_data);

	ret = bootm_boot();

and fall through to free the resources please. Also an unlink(dtspath)
is missing.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] 8+ messages in thread

* Re: [PATCH 2/2] bootm: add support for booting compressed images
  2021-05-25 18:37 ` [PATCH 2/2] bootm: add support for booting compressed images Lucas Stach
  2021-05-26  5:42   ` Sascha Hauer
@ 2021-05-26  9:17   ` Ahmad Fatoum
  2021-05-26  9:39     ` Lucas Stach
  1 sibling, 1 reply; 8+ messages in thread
From: Ahmad Fatoum @ 2021-05-26  9:17 UTC (permalink / raw)
  To: Lucas Stach, barebox

Hello Lucas,

On 25.05.21 20:37, Lucas Stach wrote:
> ARM64 does not have a self extracting image format, but relies on the image
> being externally compressed with one of the standard compression algorithms.
> 
> Add support for decompressing the bootm OS image. It is added in common
> code as it may also be useful for other images/architectures.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  common/bootm.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
> 
> diff --git a/common/bootm.c b/common/bootm.c
> index 092116beb94a..2bfb5cb01593 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -12,6 +12,7 @@
>  #include <environment.h>
>  #include <linux/stat.h>
>  #include <magicvar.h>
> +#include <uncompress.h>
>  
>  static LIST_HEAD(handler_list);
>  
> @@ -808,6 +809,85 @@ err_out:
>  	return ret;
>  }
>  
> +static int do_bootm_compressed(struct image_data *img_data)
> +{
> +	struct bootm_data bootm_data = {
> +		.oftree_file = img_data->oftree_file,
> +		.initrd_file = img_data->initrd_file,
> +		.tee_file = img_data->tee_file,
> +		.verbose = img_data->verbose,
> +		.verify = img_data->verify,
> +		.force = img_data->force,
> +		.dryrun = img_data->dryrun,
> +		.initrd_address = img_data->initrd_address,
> +		.os_address = img_data->os_address,

I am wondering whether it makes sense to directly extract
to os_address to avoid the extra copy. Depending on the subsequent
bootm handler, the image may still need to be relocated, but if
we choose a generous alignment here, the copy later on could
be avoided.

> +		.os_entry = img_data->os_entry,
> +	};
> +	int from, to, ret;
> +	char *dstpath;
> +
> +	from = open(img_data->os_file, O_RDONLY);
> +	if (from < 0)
> +		return -ENODEV;
> +
> +	dstpath = make_temp("bootm-compressed");
> +	if (!dstpath) {
> +		ret = -ENOMEM;
> +		goto fail_from;
> +	}
> +
> +	to = open(dstpath, O_CREAT | O_WRONLY);
> +	if (to < 0) {
> +		ret = -ENODEV;
> +		goto fail_make_temp;
> +	}
> +
> +	ret = uncompress_fd_to_fd(from, to, uncompress_err_stdout);
> +	if (ret)
> +		goto fail_to;
> +
> +	bootm_data.os_file = dstpath;
> +	return bootm_boot(&bootm_data);
> +
> +fail_to:
> +	close(to);
> +fail_make_temp:
> +	free(dstpath);
> +fail_from:
> +	close(from);
> +	return ret;
> +}
> +
> +static struct image_handler bzip2_bootm_handler = {
> +	.name = "BZIP2 compressed file",
> +	.bootm = do_bootm_compressed,
> +	.filetype = filetype_bzip2,
> +};
> +
> +static struct image_handler gzip_bootm_handler = {
> +	.name = "GZIP compressed file",
> +	.bootm = do_bootm_compressed,
> +	.filetype = filetype_gzip,
> +};
> +
> +static struct image_handler lzo_bootm_handler = {
> +	.name = "LZO compressed file",
> +	.bootm = do_bootm_compressed,
> +	.filetype = filetype_lzo_compressed,
> +};
> +
> +static struct image_handler lz4_bootm_handler = {
> +	.name = "LZ4 compressed file",
> +	.bootm = do_bootm_compressed,
> +	.filetype = filetype_lz4_compressed,
> +};
> +
> +static struct image_handler xz_bootm_handler = {
> +	.name = "XZ compressed file",
> +	.bootm = do_bootm_compressed,
> +	.filetype = filetype_xz_compressed,
> +};
> +
>  static int bootm_init(void)
>  {
>  	globalvar_add_simple("bootm.image", NULL);
> @@ -830,6 +910,18 @@ static int bootm_init(void)
>  	globalvar_add_simple_enum("bootm.verify", (unsigned int *)&bootm_verify_mode,
>  				  bootm_verify_names, ARRAY_SIZE(bootm_verify_names));
>  
> +
> +	if (IS_ENABLED(CONFIG_BZLIB))
> +		register_image_handler(&bzip2_bootm_handler);
> +	if (IS_ENABLED(CONFIG_ZLIB))
> +		register_image_handler(&gzip_bootm_handler);
> +	if (IS_ENABLED(CONFIG_LZO_DECOMPRESS))
> +		register_image_handler(&lzo_bootm_handler);
> +	if (IS_ENABLED(CONFIG_LZ4_DECOMPRESS))
> +		register_image_handler(&lz4_bootm_handler);
> +	if (IS_ENABLED(CONFIG_XZ_DECOMPRESS))
> +		register_image_handler(&xz_bootm_handler);
> +
>  	return 0;
>  }
>  late_initcall(bootm_init);
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] 8+ messages in thread

* Re: [PATCH 2/2] bootm: add support for booting compressed images
  2021-05-26  9:17   ` Ahmad Fatoum
@ 2021-05-26  9:39     ` Lucas Stach
  2021-05-26  9:41       ` Ahmad Fatoum
  0 siblings, 1 reply; 8+ messages in thread
From: Lucas Stach @ 2021-05-26  9:39 UTC (permalink / raw)
  To: Ahmad Fatoum, barebox

Hi Ahmad,

Am Mittwoch, dem 26.05.2021 um 11:17 +0200 schrieb Ahmad Fatoum:
> Hello Lucas,
> 
> On 25.05.21 20:37, Lucas Stach wrote:
> > ARM64 does not have a self extracting image format, but relies on the image
> > being externally compressed with one of the standard compression algorithms.
> > 
> > Add support for decompressing the bootm OS image. It is added in common
> > code as it may also be useful for other images/architectures.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  common/bootm.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 92 insertions(+)
> > 
> > diff --git a/common/bootm.c b/common/bootm.c
> > index 092116beb94a..2bfb5cb01593 100644
> > --- a/common/bootm.c
> > +++ b/common/bootm.c
> > @@ -12,6 +12,7 @@
> >  #include <environment.h>
> >  #include <linux/stat.h>
> >  #include <magicvar.h>
> > +#include <uncompress.h>
> >  
> >  static LIST_HEAD(handler_list);
> >  
> > @@ -808,6 +809,85 @@ err_out:
> >  	return ret;
> >  }
> >  
> > +static int do_bootm_compressed(struct image_data *img_data)
> > +{
> > +	struct bootm_data bootm_data = {
> > +		.oftree_file = img_data->oftree_file,
> > +		.initrd_file = img_data->initrd_file,
> > +		.tee_file = img_data->tee_file,
> > +		.verbose = img_data->verbose,
> > +		.verify = img_data->verify,
> > +		.force = img_data->force,
> > +		.dryrun = img_data->dryrun,
> > +		.initrd_address = img_data->initrd_address,
> > +		.os_address = img_data->os_address,
> 
> I am wondering whether it makes sense to directly extract
> to os_address to avoid the extra copy. Depending on the subsequent
> bootm handler, the image may still need to be relocated, but if
> we choose a generous alignment here, the copy later on could
> be avoided.

For the common case where we let Barebox/the bootm image handler decide
where to put the OS image, we don't know this address yet when
decompressing the image here. I don't think it makes sense to optimize
the special case where the OS address is already known.

Regards,
Lucas


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


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

* Re: [PATCH 2/2] bootm: add support for booting compressed images
  2021-05-26  9:39     ` Lucas Stach
@ 2021-05-26  9:41       ` Ahmad Fatoum
  2021-05-26  9:58         ` Lucas Stach
  0 siblings, 1 reply; 8+ messages in thread
From: Ahmad Fatoum @ 2021-05-26  9:41 UTC (permalink / raw)
  To: Lucas Stach, barebox

On 26.05.21 11:39, Lucas Stach wrote:
> Hi Ahmad,
> 
> Am Mittwoch, dem 26.05.2021 um 11:17 +0200 schrieb Ahmad Fatoum:
>> Hello Lucas,
>>
>> On 25.05.21 20:37, Lucas Stach wrote:
>>> ARM64 does not have a self extracting image format, but relies on the image
>>> being externally compressed with one of the standard compression algorithms.
>>>
>>> Add support for decompressing the bootm OS image. It is added in common
>>> code as it may also be useful for other images/architectures.
>>>
>>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>>> ---
>>>  common/bootm.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 92 insertions(+)
>>>
>>> diff --git a/common/bootm.c b/common/bootm.c
>>> index 092116beb94a..2bfb5cb01593 100644
>>> --- a/common/bootm.c
>>> +++ b/common/bootm.c
>>> @@ -12,6 +12,7 @@
>>>  #include <environment.h>
>>>  #include <linux/stat.h>
>>>  #include <magicvar.h>
>>> +#include <uncompress.h>
>>>  
>>>  static LIST_HEAD(handler_list);
>>>  
>>> @@ -808,6 +809,85 @@ err_out:
>>>  	return ret;
>>>  }
>>>  
>>> +static int do_bootm_compressed(struct image_data *img_data)
>>> +{
>>> +	struct bootm_data bootm_data = {
>>> +		.oftree_file = img_data->oftree_file,
>>> +		.initrd_file = img_data->initrd_file,
>>> +		.tee_file = img_data->tee_file,
>>> +		.verbose = img_data->verbose,
>>> +		.verify = img_data->verify,
>>> +		.force = img_data->force,
>>> +		.dryrun = img_data->dryrun,
>>> +		.initrd_address = img_data->initrd_address,
>>> +		.os_address = img_data->os_address,
>>
>> I am wondering whether it makes sense to directly extract
>> to os_address to avoid the extra copy. Depending on the subsequent
>> bootm handler, the image may still need to be relocated, but if
>> we choose a generous alignment here, the copy later on could
>> be avoided.
> 
> For the common case where we let Barebox/the bootm image handler decide
> where to put the OS image, we don't know this address yet when
> decompressing the image here. I don't think it makes sense to optimize
> the special case where the OS address is already known.

I see. A debug print for higher verbosity levels, just before invoking
bootm_boot would be nice, so it's easy to find out how long the
extraction took.

Cheers,
Ahmad


> 
> Regards,
> Lucas
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] 8+ messages in thread

* Re: [PATCH 2/2] bootm: add support for booting compressed images
  2021-05-26  9:41       ` Ahmad Fatoum
@ 2021-05-26  9:58         ` Lucas Stach
  2021-05-26 10:04           ` Ahmad Fatoum
  0 siblings, 1 reply; 8+ messages in thread
From: Lucas Stach @ 2021-05-26  9:58 UTC (permalink / raw)
  To: Ahmad Fatoum, barebox

Am Mittwoch, dem 26.05.2021 um 11:41 +0200 schrieb Ahmad Fatoum:
> On 26.05.21 11:39, Lucas Stach wrote:
> > Hi Ahmad,
> > 
> > Am Mittwoch, dem 26.05.2021 um 11:17 +0200 schrieb Ahmad Fatoum:
> > > Hello Lucas,
> > > 
> > > On 25.05.21 20:37, Lucas Stach wrote:
> > > > ARM64 does not have a self extracting image format, but relies on the image
> > > > being externally compressed with one of the standard compression algorithms.
> > > > 
> > > > Add support for decompressing the bootm OS image. It is added in common
> > > > code as it may also be useful for other images/architectures.
> > > > 
> > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > > ---
> > > >  common/bootm.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 92 insertions(+)
> > > > 
> > > > diff --git a/common/bootm.c b/common/bootm.c
> > > > index 092116beb94a..2bfb5cb01593 100644
> > > > --- a/common/bootm.c
> > > > +++ b/common/bootm.c
> > > > @@ -12,6 +12,7 @@
> > > >  #include <environment.h>
> > > >  #include <linux/stat.h>
> > > >  #include <magicvar.h>
> > > > +#include <uncompress.h>
> > > >  
> > > >  static LIST_HEAD(handler_list);
> > > >  
> > > > @@ -808,6 +809,85 @@ err_out:
> > > >  	return ret;
> > > >  }
> > > >  
> > > > +static int do_bootm_compressed(struct image_data *img_data)
> > > > +{
> > > > +	struct bootm_data bootm_data = {
> > > > +		.oftree_file = img_data->oftree_file,
> > > > +		.initrd_file = img_data->initrd_file,
> > > > +		.tee_file = img_data->tee_file,
> > > > +		.verbose = img_data->verbose,
> > > > +		.verify = img_data->verify,
> > > > +		.force = img_data->force,
> > > > +		.dryrun = img_data->dryrun,
> > > > +		.initrd_address = img_data->initrd_address,
> > > > +		.os_address = img_data->os_address,
> > > 
> > > I am wondering whether it makes sense to directly extract
> > > to os_address to avoid the extra copy. Depending on the subsequent
> > > bootm handler, the image may still need to be relocated, but if
> > > we choose a generous alignment here, the copy later on could
> > > be avoided.
> > 
> > For the common case where we let Barebox/the bootm image handler decide
> > where to put the OS image, we don't know this address yet when
> > decompressing the image here. I don't think it makes sense to optimize
> > the special case where the OS address is already known.
> 
> I see. A debug print for higher verbosity levels, just before invoking
> bootm_boot would be nice, so it's easy to find out how long the
> extraction took.

Actually due to the way this is implemented as nested bootm you kind of
get this naturally. The output when booting a compressed image looks
like this:

blspec: booting someboard from somewhere
Loading LZO compressed 'somewhere/Image.lzo'
Loading ARM aarch64 Linux image '/tmp/bootm-compressed-6504dc00'
Loading devicetree from 'somewhere/someboard.dtb'

Regards,
Lucas


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


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

* Re: [PATCH 2/2] bootm: add support for booting compressed images
  2021-05-26  9:58         ` Lucas Stach
@ 2021-05-26 10:04           ` Ahmad Fatoum
  0 siblings, 0 replies; 8+ messages in thread
From: Ahmad Fatoum @ 2021-05-26 10:04 UTC (permalink / raw)
  To: Lucas Stach, barebox



On 26.05.21 11:58, Lucas Stach wrote:
> Am Mittwoch, dem 26.05.2021 um 11:41 +0200 schrieb Ahmad Fatoum:
>> On 26.05.21 11:39, Lucas Stach wrote:
>>> Hi Ahmad,
>>>
>>> Am Mittwoch, dem 26.05.2021 um 11:17 +0200 schrieb Ahmad Fatoum:
>>>> Hello Lucas,
>>>>
>>>> On 25.05.21 20:37, Lucas Stach wrote:
>>>>> ARM64 does not have a self extracting image format, but relies on the image
>>>>> being externally compressed with one of the standard compression algorithms.
>>>>>
>>>>> Add support for decompressing the bootm OS image. It is added in common
>>>>> code as it may also be useful for other images/architectures.
>>>>>
>>>>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>>>>> ---
>>>>>  common/bootm.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 92 insertions(+)
>>>>>
>>>>> diff --git a/common/bootm.c b/common/bootm.c
>>>>> index 092116beb94a..2bfb5cb01593 100644
>>>>> --- a/common/bootm.c
>>>>> +++ b/common/bootm.c
>>>>> @@ -12,6 +12,7 @@
>>>>>  #include <environment.h>
>>>>>  #include <linux/stat.h>
>>>>>  #include <magicvar.h>
>>>>> +#include <uncompress.h>
>>>>>  
>>>>>  static LIST_HEAD(handler_list);
>>>>>  
>>>>> @@ -808,6 +809,85 @@ err_out:
>>>>>  	return ret;
>>>>>  }
>>>>>  
>>>>> +static int do_bootm_compressed(struct image_data *img_data)
>>>>> +{
>>>>> +	struct bootm_data bootm_data = {
>>>>> +		.oftree_file = img_data->oftree_file,
>>>>> +		.initrd_file = img_data->initrd_file,
>>>>> +		.tee_file = img_data->tee_file,
>>>>> +		.verbose = img_data->verbose,
>>>>> +		.verify = img_data->verify,
>>>>> +		.force = img_data->force,
>>>>> +		.dryrun = img_data->dryrun,
>>>>> +		.initrd_address = img_data->initrd_address,
>>>>> +		.os_address = img_data->os_address,
>>>>
>>>> I am wondering whether it makes sense to directly extract
>>>> to os_address to avoid the extra copy. Depending on the subsequent
>>>> bootm handler, the image may still need to be relocated, but if
>>>> we choose a generous alignment here, the copy later on could
>>>> be avoided.
>>>
>>> For the common case where we let Barebox/the bootm image handler decide
>>> where to put the OS image, we don't know this address yet when
>>> decompressing the image here. I don't think it makes sense to optimize
>>> the special case where the OS address is already known.
>>
>> I see. A debug print for higher verbosity levels, just before invoking
>> bootm_boot would be nice, so it's easy to find out how long the
>> extraction took.
> 
> Actually due to the way this is implemented as nested bootm you kind of
> get this naturally. The output when booting a compressed image looks
> like this:
> 
> blspec: booting someboard from somewhere
> Loading LZO compressed 'somewhere/Image.lzo'
> Loading ARM aarch64 Linux image '/tmp/bootm-compressed-6504dc00'
> Loading devicetree from 'somewhere/someboard.dtb'

Great! :-)

> 
> Regards,
> Lucas
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] 8+ messages in thread

end of thread, other threads:[~2021-05-26 12:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25 18:37 [PATCH 1/2] uncompress: use read_full to fill decompression buffer Lucas Stach
2021-05-25 18:37 ` [PATCH 2/2] bootm: add support for booting compressed images Lucas Stach
2021-05-26  5:42   ` Sascha Hauer
2021-05-26  9:17   ` Ahmad Fatoum
2021-05-26  9:39     ` Lucas Stach
2021-05-26  9:41       ` Ahmad Fatoum
2021-05-26  9:58         ` Lucas Stach
2021-05-26 10:04           ` Ahmad Fatoum

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