diff mbox

[U-Boot] mkimage: fit: spl: Add an optional static offset for external data

Message ID 20160501101011.72db83f05e19477399fd669e@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Teddy Reed May 1, 2016, 5:10 p.m. UTC
When building a FIT with external data (-E), an SPL may require absolute
positioning for executing the external firmware. To acheive this use the
(-p) switch which will replace the amended "data-offset" with "data-position"
indicating the absolute position of external data.

It is considered an error if the requested absolute position overlaps with the
initial data required for the compact FIT.

Signed-off-by: Teddy Reed <teddy.reed@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
---
 tools/fit_image.c | 19 ++++++++++++++++++-
 tools/imagetool.h |  1 +
 tools/mkimage.c   |  9 ++++++++-
 3 files changed, 27 insertions(+), 2 deletions(-)

Comments

Simon Glass May 1, 2016, 7:32 p.m. UTC | #1
Hi Teddy,

On 1 May 2016 at 11:10, Teddy Reed <teddy.reed@gmail.com> wrote:
> When building a FIT with external data (-E), an SPL may require absolute
> positioning for executing the external firmware. To acheive this use the
> (-p) switch which will replace the amended "data-offset" with "data-position"
> indicating the absolute position of external data.
>
> It is considered an error if the requested absolute position overlaps with the
> initial data required for the compact FIT.

Can you explain why this is useful? I'd like to understand that
clearly for your use case.

I have considered this as a general feature, but I was thinking of
being more explicit, e.g. add a property like:

data-source
    - source of data, valid values are:
        - "internal" - internal to the FIT, with data-offset providing
the offset from the start of the FIT to the data
        - "device" - a separate device, details in property TBD
        - "address" - a memory address

What do you think?

Also can you please update the docs? See uImage.FIT and the mkimage man page.

>
> Signed-off-by: Teddy Reed <teddy.reed@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  tools/fit_image.c | 19 ++++++++++++++++++-
>  tools/imagetool.h |  1 +
>  tools/mkimage.c   |  9 ++++++++-
>  3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> index ddefa72..98610bf 100644
> --- a/tools/fit_image.c
> +++ b/tools/fit_image.c
> @@ -417,7 +417,13 @@ static int fit_extract_data(struct image_tool_params *params, const char *fname)
>                         ret = -EPERM;
>                         goto err_munmap;
>                 }
> -               fdt_setprop_u32(fdt, node, "data-offset", buf_ptr);
> +               if (params->external_offset > 0) {
> +                       /* An external offset positions the data absolutely. */
> +                       fdt_setprop_u32(fdt, node, "data-position",
> +                                       params->external_offset + buf_ptr);
> +               } else {
> +                       fdt_setprop_u32(fdt, node, "data-offset", buf_ptr);
> +               }
>                 fdt_setprop_u32(fdt, node, "data-size", len);
>
>                 buf_ptr += (len + 3) & ~3;
> @@ -438,6 +444,17 @@ static int fit_extract_data(struct image_tool_params *params, const char *fname)
>                 ret = -EIO;
>                 goto err;
>         }
> +
> +       /* Check if an offset for the external data was set. */
> +       if (params->external_offset > 0) {
> +               if (params->external_offset < new_size) {
> +                       debug("External offset %x overlaps FIT length %x",
> +                             params->external_offset, new_size);
> +                       ret = -EINVAL;
> +                       goto err;
> +               }
> +               new_size = params->external_offset;
> +       }
>         if (lseek(fd, new_size, SEEK_SET) < 0) {
>                 debug("%s: Failed to seek to end of file: %s\n", __func__,
>                       strerror(errno));
> diff --git a/tools/imagetool.h b/tools/imagetool.h
> index 24f8f4b..7862fa3 100644
> --- a/tools/imagetool.h
> +++ b/tools/imagetool.h
> @@ -73,6 +73,7 @@ struct image_tool_params {
>         struct content_info *content_head;      /* List of files to include */
>         struct content_info *content_tail;
>         bool external_data;     /* Store data outside the FIT */
> +       unsigned int external_offset;   /* Add padding to external data */
>  };
>
>  /*
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index 2931783..85e4781 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -138,7 +138,7 @@ static void process_args(int argc, char **argv)
>
>         expecting = IH_TYPE_COUNT;      /* Unknown */
>         while ((opt = getopt(argc, argv,
> -                            "-a:A:bcC:d:D:e:Ef:Fk:K:ln:O:rR:sT:vVx")) != -1) {
> +                            "-a:A:bcC:d:D:e:Ef:Fk:K:ln:p:O:rR:sT:vVx")) != -1) {
>                 switch (opt) {
>                 case 'a':
>                         params.addr = strtoull(optarg, &ptr, 16);
> @@ -213,6 +213,13 @@ static void process_args(int argc, char **argv)
>                         if (params.os < 0)
>                                 usage("Invalid operating system");
>                         break;
> +               case 'p':
> +                       params.external_offset = strtoull(optarg, &ptr, 16);
> +                       if (*ptr) {
> +                               fprintf(stderr, "%s: invalid offset size %s\n",
> +                                       params.cmdname, optarg);
> +                               exit(EXIT_FAILURE);
> +                       }
>                 case 'r':
>                         params.require_keys = 1;
>                         break;
> --
> 1.9.1

Regards,
Simon
diff mbox

Patch

diff --git a/tools/fit_image.c b/tools/fit_image.c
index ddefa72..98610bf 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -417,7 +417,13 @@  static int fit_extract_data(struct image_tool_params *params, const char *fname)
 			ret = -EPERM;
 			goto err_munmap;
 		}
-		fdt_setprop_u32(fdt, node, "data-offset", buf_ptr);
+		if (params->external_offset > 0) {
+			/* An external offset positions the data absolutely. */
+			fdt_setprop_u32(fdt, node, "data-position",
+					params->external_offset + buf_ptr);
+		} else {
+			fdt_setprop_u32(fdt, node, "data-offset", buf_ptr);
+		}
 		fdt_setprop_u32(fdt, node, "data-size", len);
 
 		buf_ptr += (len + 3) & ~3;
@@ -438,6 +444,17 @@  static int fit_extract_data(struct image_tool_params *params, const char *fname)
 		ret = -EIO;
 		goto err;
 	}
+
+	/* Check if an offset for the external data was set. */
+	if (params->external_offset > 0) {
+		if (params->external_offset < new_size) {
+			debug("External offset %x overlaps FIT length %x",
+			      params->external_offset, new_size);
+			ret = -EINVAL;
+			goto err;
+		}
+		new_size = params->external_offset;
+	}
 	if (lseek(fd, new_size, SEEK_SET) < 0) {
 		debug("%s: Failed to seek to end of file: %s\n", __func__,
 		      strerror(errno));
diff --git a/tools/imagetool.h b/tools/imagetool.h
index 24f8f4b..7862fa3 100644
--- a/tools/imagetool.h
+++ b/tools/imagetool.h
@@ -73,6 +73,7 @@  struct image_tool_params {
 	struct content_info *content_head;	/* List of files to include */
 	struct content_info *content_tail;
 	bool external_data;	/* Store data outside the FIT */
+	unsigned int external_offset;	/* Add padding to external data */
 };
 
 /*
diff --git a/tools/mkimage.c b/tools/mkimage.c
index 2931783..85e4781 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -138,7 +138,7 @@  static void process_args(int argc, char **argv)
 
 	expecting = IH_TYPE_COUNT;	/* Unknown */
 	while ((opt = getopt(argc, argv,
-			     "-a:A:bcC:d:D:e:Ef:Fk:K:ln:O:rR:sT:vVx")) != -1) {
+			     "-a:A:bcC:d:D:e:Ef:Fk:K:ln:p:O:rR:sT:vVx")) != -1) {
 		switch (opt) {
 		case 'a':
 			params.addr = strtoull(optarg, &ptr, 16);
@@ -213,6 +213,13 @@  static void process_args(int argc, char **argv)
 			if (params.os < 0)
 				usage("Invalid operating system");
 			break;
+		case 'p':
+			params.external_offset = strtoull(optarg, &ptr, 16);
+			if (*ptr) {
+				fprintf(stderr, "%s: invalid offset size %s\n",
+					params.cmdname, optarg);
+				exit(EXIT_FAILURE);
+			}
 		case 'r':
 			params.require_keys = 1;
 			break;