[PATCH 0/3] firmware: Add support for compressed images

[PATCH 0/3] firmware: Add support for compressed images

[Sascha Hauer]

Steffen already sent this. This version has some improvements like
better error reporting. Also we uncompress only files we detect as
compressed files, not just everything we do not detect as unknown file.

Sascha Hauer (2):
  filetype: Add function to check if a filetype is a compressed file
  libfile: Add copy_fd()

Steffen Trumtrar (1):
  firmware: add support for compressed images

 common/firmware.c  | 50 ++++++++++++++++++++++++++++++++++++++++++----
 include/filetype.h | 14 +++++++++++++
 include/libfile.h  |  1 +
 lib/libfile.c      | 34 +++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 4 deletions(-)

-- 
2.29.2


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

[PATCH 1/3] filetype: Add function to check if a filetype is a compressed file

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/filetype.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/filetype.h b/include/filetype.h
index fd339f9564..ae0920320e 100644
--- a/include/filetype.h
+++ b/include/filetype.h
@@ -70,6 +70,20 @@ enum filetype is_fat_or_mbr(const unsigned char *sector, unsigned long *bootsec)
 int is_fat_boot_sector(const void *_buf);
 bool filetype_is_barebox_image(enum filetype ft);
 
+static inline bool file_is_compressed_file(enum filetype ft)
+{
+	switch (ft) {
+	case filetype_lzo_compressed:
+	case filetype_lz4_compressed:
+	case filetype_gzip:
+	case filetype_bzip2:
+	case filetype_xz_compressed:
+		return true;
+	default:
+		return false;
+	}
+}
+
 #define ARM_HEAD_SIZE			0x30
 #define ARM_HEAD_MAGICWORD_OFFSET	0x20
 #define ARM_HEAD_SIZE_OFFSET		0x2C
-- 
2.29.2


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

[PATCH 2/3] libfile: Add copy_fd()

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/libfile.h |  1 +
 lib/libfile.c     | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/include/libfile.h b/include/libfile.h
index 350ddddf70..3c2fe1714d 100644
--- a/include/libfile.h
+++ b/include/libfile.h
@@ -5,6 +5,7 @@
 int pwrite_full(int fd, const void *buf, size_t size, loff_t offset);
 int write_full(int fd, const void *buf, size_t size);
 int read_full(int fd, void *buf, size_t size);
+int copy_fd(int in, int out);
 
 char *read_file_line(const char *fmt, ...);
 
diff --git a/lib/libfile.c b/lib/libfile.c
index 4ab8db11ad..e42126017d 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -100,6 +100,40 @@ int read_full(int fd, void *buf, size_t size)
 }
 EXPORT_SYMBOL(read_full);
 
+int copy_fd(int in, int out)
+{
+	int bs = 4096, ret;
+	void *buf = malloc(bs);
+
+	if (!buf)
+		return -ENOMEM;
+
+	while (1) {
+		int now, wr;
+
+		now = read(in, buf, bs);
+		if (now < 0) {
+			ret = now;
+			goto err;
+		}
+
+		if (!now)
+			break;
+
+		wr = write_full(out, buf, now);
+		if (wr < 0) {
+			ret = wr;
+			goto err;
+		}
+	}
+
+	ret = 0;
+err:
+        free(buf);
+
+	return ret;
+}
+
 /*
  * read_file_line - read a line from a file
  *
-- 
2.29.2


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

[PATCH 3/3] firmware: add support for compressed images

[Sascha Hauer]

From: Steffen Trumtrar <s.trumtrar@pengutronix.de>

At least bitstreams for FPGAs can consist of a lot of zeros depending on
device utilization. These bitstreams can be compressed very effectively.

Let the firmware code accept these images and decompress them before
handing it to the firmware-manager in question.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Link: https://lore.barebox.org/20210616063246.14900-10-s.trumtrar@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/firmware.c | 50 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 4 deletions(-)

diff --git a/common/firmware.c b/common/firmware.c
index 58509d5da6..e4b5025ab1 100644
--- a/common/firmware.c
+++ b/common/firmware.c
@@ -14,6 +14,8 @@
 #include <linux/list.h>
 #include <linux/stat.h>
 #include <linux/err.h>
+#include <uncompress.h>
+#include <filetype.h>
 
 #define BUFSIZ 4096
 
@@ -211,12 +213,52 @@ out:
  */
 int firmwaremgr_load_file(struct firmware_mgr *mgr, const char *firmware)
 {
-	int ret;
-	char *name = basprintf("/dev/%s", mgr->handler->id);
+	char *dst;
+	enum filetype type;
+	int ret = 0;
+	int firmwarefd = 0;
+	int devicefd = 0;
+
+	if (!firmware)
+		return -EINVAL;
+
+	if (!mgr->handler->id) {
+		pr_err("id not defined for handler\n");
+		return -ENODEV;
+	}
+
+	dst = basprintf("/dev/%s", mgr->handler->id);
+
+	firmwarefd = open(firmware, O_RDONLY);
+	if (firmwarefd < 0) {
+		printf("could not open %s: %s\n", firmware,
+		       errno_str());
+		ret = firmwarefd;
+		goto out;
+	}
 
-	ret = copy_file(firmware, name, 0);
+	type = file_name_detect_type(firmware);
+
+	devicefd = open(dst, O_WRONLY);
+	if (devicefd < 0) {
+		printf("could not open %s: %s\n", dst, errno_str());
+		ret = devicefd;
+		goto out;
+	}
+
+	if (file_is_compressed_file(type))
+		ret = uncompress_fd_to_fd(firmwarefd, devicefd,
+					  uncompress_err_stdout);
+	else
+		ret = copy_fd(firmwarefd, devicefd);
+
+out:
+	free(dst);
 
-	free(name);
+	if (firmwarefd > 0)
+		close(firmwarefd);
+	if (devicefd > 0)
+		close(devicefd);
 
 	return ret;
 }
-- 
2.29.2


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

Re: [PATCH 2/3] libfile: Add copy_fd()

[Trent Piepho]

On Tue, Jun 22, 2021 at 9:34 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> +       while (1) {
> +               int now, wr;
> +
> +               now = read(in, buf, bs);
> +               if (now < 0) {
> +                       ret = now;
> +                       goto err;
> +               }
> +
> +               if (!now)
> +                       break;
> +
> +               wr = write_full(out, buf, now);
> +               if (wr < 0) {
> +                       ret = wr;
> +                       goto err;
> +               }
> +       }
> +
> +       ret = 0;
> +err:
> +        free(buf);
> +
> +       return ret;

This can be quite a bit shorter:

while (1) {
       ret = read(in, buf, bs);
       if (ret <= 0)
               break;
       ret = write_full(out, buf, ret);
       if (ret < 0)
               break;
}
free(buf)
return ret;

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

Re: [PATCH 2/3] libfile: Add copy_fd()

[Sascha Hauer]

On Tue, Jun 22, 2021 at 11:24:57PM -0700, Trent Piepho wrote:
> On Tue, Jun 22, 2021 at 9:34 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > +       while (1) {
> > +               int now, wr;
> > +
> > +               now = read(in, buf, bs);
> > +               if (now < 0) {
> > +                       ret = now;
> > +                       goto err;
> > +               }
> > +
> > +               if (!now)
> > +                       break;
> > +
> > +               wr = write_full(out, buf, now);
> > +               if (wr < 0) {
> > +                       ret = wr;
> > +                       goto err;
> > +               }
> > +       }
> > +
> > +       ret = 0;
> > +err:
> > +        free(buf);
> > +
> > +       return ret;
> 
> This can be quite a bit shorter:
> 
> while (1) {
>        ret = read(in, buf, bs);
>        if (ret <= 0)
>                break;
>        ret = write_full(out, buf, ret);
>        if (ret < 0)
>                break;
> }
> free(buf)
> return ret;

Indeed, thanks. I'll change it like that.

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

Re: [PATCH 3/3] firmware: add support for compressed images

[Trent Piepho]

On Fri, 2016-05-20 at 14:21 +0200, Steffen Trumtrar wrote:
> Allow using compressed firmware images with the firmware framework.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  common/firmware.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 78 insertions(+), 4 deletions(-)
> 
> diff --git a/common/firmware.c b/common/firmware.c
> index 664f9107d0f8..6437005bf813 100644
> --- a/common/firmware.c
> +++ b/common/firmware.c
> @@ -22,6 +22,8 @@
>  #include <linux/list.h>
>  #include <linux/stat.h>
>  #include <linux/err.h>
> +#include <uncompress.h>
> +#include <filetype.h>
>  
>  #define BUFSIZ 4096
>  
> @@ -197,16 +199,88 @@ out:
>  }
>  
>  /*
> + * firmware_load_compressed - load a compressed firmware to a device
> + */
> +int firmwaremgr_load_compressed(const char *firmware, const char *dst)
> +{
> +	int srcfd = 0;
> +	int dstfd = 0;
> +	int ret;
> +
> +	srcfd = open(firmware, O_RDONLY);
> +	if (srcfd < 0) {
> +		printf("could not open %s: %s\n", firmware, errno_str());
> +		ret = srcfd;
> +		goto out;
> +	}

Since the firmware was already opened, could the fd be passed to this
function so it doesn't need to be opened again?

> +
> +	dstfd = open(dst, O_WRONLY | O_TRUNC);
> +	if (dstfd < 0) {
> +		printf("could not open %s: %s\n", dst, errno_str());
> +		ret = dstfd;
> +		goto out;
> +	}
> +
> +	ret = uncompress_fd_to_fd(srcfd, dstfd, uncompress_err_stdout);
> +
> +out:
> +	if (dstfd > 0)
> +		close(dstfd);
> +
> +	return ret;
> +}
> +
> +/*
>   * firmware_load_file - load a firmware to a device
>   */
>  int firmwaremgr_load_file(struct firmware_mgr *mgr, const char *firmware)
>  {
> -	int ret;
> -	char *name = basprintf("/dev/%s", mgr->handler->id);
> +	char *dst = basprintf("/dev/%s", mgr->handler->id);
> +	enum filetype type;
> +	int ret = -ENOENT;
> +	int srcfd = 0;
> +	char buf[32];
> +
> +	if (firmware) {
> +		srcfd = open(firmware, O_RDONLY);
> +		if (srcfd < 0) {
> +			printf("could not open %s: %s\n", firmware, errno_str());
> +			ret = srcfd;
> +			goto out;
> +		}
>  
> -	ret = copy_file(firmware, name, 0);
> +		ret = read(srcfd, buf, sizeof(buf));
> +		if (ret < sizeof(buf))
> +			goto out;
>  
> -	free(name);
> +		type = file_detect_type(buf, 32);
> +		if ((int)type < 0) {
> +			printf("could not open %s: %s\n", firmware,
> +					strerror(-type));
> +			ret = (int)type;
> +			goto out;

srcfd is not closed on the error path here.

> +		}
> +
> +		close(srcfd);
> +
> +		switch (type) {
> +		case filetype_lzo_compressed:
> +		case filetype_lz4_compressed:
> +		case filetype_bzip2:
> +		case filetype_gzip:

This is missing xz compression.

Instead of trying to list all compressed types here, and also find the
compressed type of the file, would it make more sense to have the
uncompression code do this?  This way it's not necessary to have a list
that matches what compression algorithms are supported and compiled in.
And uncompress() already reads the file header and determines if the
file type and if it can be uncompressed.  So this code is basically an
incomplete copy of code already in uncompress.c.


> +			ret = firmwaremgr_load_compressed(firmware, dst);
> +			break;
> +		case filetype_unknown:
> +			ret = copy_file(firmware, dst, 0);
> +			break;
> +		default:
> +			ret = -ENOSYS;
> +			printf("unsupported filetype (%d)\n", (int) type);
> +		}
> +	}
> +
> +out:
> +	free(dst);
>  
>  	return ret;
>  }

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

[PATCH 3/3] firmware: add support for compressed images

[Steffen Trumtrar]

Allow using compressed firmware images with the firmware framework.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 common/firmware.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 4 deletions(-)

diff --git a/common/firmware.c b/common/firmware.c
index 664f9107d0f8..6437005bf813 100644
--- a/common/firmware.c
+++ b/common/firmware.c
@@ -22,6 +22,8 @@
 #include <linux/list.h>
 #include <linux/stat.h>
 #include <linux/err.h>
+#include <uncompress.h>
+#include <filetype.h>
 
 #define BUFSIZ 4096
 
@@ -197,16 +199,88 @@ out:
 }
 
 /*
+ * firmware_load_compressed - load a compressed firmware to a device
+ */
+int firmwaremgr_load_compressed(const char *firmware, const char *dst)
+{
+	int srcfd = 0;
+	int dstfd = 0;
+	int ret;
+
+	srcfd = open(firmware, O_RDONLY);
+	if (srcfd < 0) {
+		printf("could not open %s: %s\n", firmware, errno_str());
+		ret = srcfd;
+		goto out;
+	}
+
+	dstfd = open(dst, O_WRONLY | O_TRUNC);
+	if (dstfd < 0) {
+		printf("could not open %s: %s\n", dst, errno_str());
+		ret = dstfd;
+		goto out;
+	}
+
+	ret = uncompress_fd_to_fd(srcfd, dstfd, uncompress_err_stdout);
+
+out:
+	if (dstfd > 0)
+		close(dstfd);
+
+	return ret;
+}
+
+/*
  * firmware_load_file - load a firmware to a device
  */
 int firmwaremgr_load_file(struct firmware_mgr *mgr, const char *firmware)
 {
-	int ret;
-	char *name = basprintf("/dev/%s", mgr->handler->id);
+	char *dst = basprintf("/dev/%s", mgr->handler->id);
+	enum filetype type;
+	int ret = -ENOENT;
+	int srcfd = 0;
+	char buf[32];
+
+	if (firmware) {
+		srcfd = open(firmware, O_RDONLY);
+		if (srcfd < 0) {
+			printf("could not open %s: %s\n", firmware, errno_str());
+			ret = srcfd;
+			goto out;
+		}
 
-	ret = copy_file(firmware, name, 0);
+		ret = read(srcfd, buf, sizeof(buf));
+		if (ret < sizeof(buf))
+			goto out;
 
-	free(name);
+		type = file_detect_type(buf, 32);
+		if ((int)type < 0) {
+			printf("could not open %s: %s\n", firmware,
+					strerror(-type));
+			ret = (int)type;
+			goto out;
+		}
+
+		close(srcfd);
+
+		switch (type) {
+		case filetype_lzo_compressed:
+		case filetype_lz4_compressed:
+		case filetype_bzip2:
+		case filetype_gzip:
+			ret = firmwaremgr_load_compressed(firmware, dst);
+			break;
+		case filetype_unknown:
+			ret = copy_file(firmware, dst, 0);
+			break;
+		default:
+			ret = -ENOSYS;
+			printf("unsupported filetype (%d)\n", (int) type);
+		}
+	}
+
+out:
+	free(dst);
 
 	return ret;
 }
-- 
2.8.0.rc3


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