Message ID | 20160501101011.72db83f05e19477399fd669e@gmail.com |
---|---|
State | Superseded |
Delegated to: | Tom Rini |
Headers | show |
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 --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;
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(-)