Hi Teddy,
On 5 June 2016 at 15:18, 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.
Why does SPL require that? Is it because it is at an external address, not inside the FIT?
It is considered an error if the requested absolute position overlaps with the initial data required for the compact FIT.
Cc: Simon Glass sjg@chromium.org
Changes in v2:
- Add -p argument to mkimage.1
- Add -E, -p arguments to mkimage usage text
- Add notes for static position within uImage.FIT docs
- Rebased onto master
doc/mkimage.1 | 6 ++++++ doc/uImage.FIT/source_file_format.txt | 4 ++++ tools/fit_image.c | 19 ++++++++++++++++++- tools/imagetool.h | 1 + tools/mkimage.c | 13 +++++++++++-- 5 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/doc/mkimage.1 b/doc/mkimage.1 index 4b3a255..ffa7d60 100644 --- a/doc/mkimage.1 +++ b/doc/mkimage.1 @@ -152,6 +152,12 @@ verification. Typically the file here is the device tree binary used by CONFIG_OF_CONTROL in U-Boot.
.TP +.BI "-p [" "external position" "]" +Place external data at a static external position. See -E. Instead of writing +a 'data-offset' property defining the offset from the end of the FIT, -p will +use 'data-position' as the absolute position from the base of the FIT.
+.TP .BI "-r Specifies that keys used to sign the FIT are required. This means that they must be verified for the image to boot. Without this option, the verification diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt index 3f54180..ee72740 100644 --- a/doc/uImage.FIT/source_file_format.txt +++ b/doc/uImage.FIT/source_file_format.txt @@ -282,6 +282,10 @@ In this case the 'data' property is omitted. Instead you can use: aligned to a 4-byte boundary.
- data-size : size of the data in bytes
+The 'data-offset' property can be substituted with 'data-position', which +defines a static position or address from the base of the FIT as the offset. +A static position is helpful when booting U-Boot proper before performing +relocation.
This confuses me since it mentions a static address, but then references the base of the FIT, suggesting it is relative.
Can you use the word 'absolute' instead of 'static'?
- Examples
diff --git a/tools/fit_image.c b/tools/fit_image.c index 0551572..76a6de4 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -416,7 +416,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;@@ -437,6 +443,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 a3ed0f4..1f2161c 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -74,6 +74,7 @@ struct image_tool_params { struct content_info *content_tail; bool external_data; /* Store data outside the FIT */ bool quiet; /* Don't output text in normal operation */
unsigned int external_offset; /* Add padding to external data */};
/* diff --git a/tools/mkimage.c b/tools/mkimage.c index aefe22f..ff3024a 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -93,11 +93,13 @@ static void usage(const char *msg) " -f => input filename for FIT source\n"); #ifdef CONFIG_FIT_SIGNATURE fprintf(stderr,
"Signing / verified boot options: [-k keydir] [-K dtb] [ -c <comment>] [-r]\n"
"Signing / verified boot options: [-E] [-k keydir] [-K dtb] [ -c <comment>] [-p addr] [-r]\n"" -E => place data outside of the FIT structure\n" " -k => set directory containing private keys\n" " -K => write public keys to this .dtb file\n" " -c => add comment in signature node\n" " -F => re-sign existing FIT image\n"" -p => place external data at a static position\n" " -r => mark keys used as 'required' in dtb\n");#else fprintf(stderr, @@ -136,7 +138,7 @@ static void process_args(int argc, char **argv) int opt;
while ((opt = getopt(argc, argv,
"a:A:b:cC:d:D:e:Ef:Fk:K:ln:O:rR:qsT:vVx")) != -1) {
"a:A:b:cC:d:D:e:Ef:Fk:K:ln:p:O:rR:qsT:vVx")) != -1) { switch (opt) { case 'a': params.addr = strtoull(optarg, &ptr, 16);@@ -216,6 +218,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 'q': params.quiet = 1; break;-- 2.7.4
Regards, Simon