Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fiptool: Add --align-reserve command line option #916

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 41 additions & 8 deletions tools/fiptool/fiptool.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#define OPT_TOC_ENTRY 0
#define OPT_PLAT_TOC_FLAGS 1
#define OPT_ALIGN 2
#define OPT_ALIGN_RESERVE 3

static int info_cmd(int argc, char *argv[]);
static void info_usage(void);
Expand Down Expand Up @@ -511,7 +512,8 @@ static void info_usage(void)
exit(1);
}

static int pack_images(const char *filename, uint64_t toc_flags, unsigned long align)
static int pack_images(const char *filename, uint64_t toc_flags,
unsigned long align, unsigned long align_reserve)
{
FILE *fp;
image_desc_t *desc;
Expand Down Expand Up @@ -546,7 +548,7 @@ static int pack_images(const char *filename, uint64_t toc_flags, unsigned long a
if (image == NULL)
continue;
payload_size += image->toc_e.size;
entry_offset = (entry_offset + align - 1) & ~(align - 1);
entry_offset = (entry_offset + align_reserve + align - 1) & ~(align - 1);
image->toc_e.offset_address = entry_offset;
*toc_entry++ = image->toc_e;
entry_offset += image->toc_e.size;
Expand Down Expand Up @@ -653,6 +655,19 @@ static unsigned long get_image_align(char *arg)
return align;
}

static unsigned long get_image_align_reservation(char *arg)
{
char *endptr;
unsigned long align_reserve;

errno = 0;
align_reserve = strtoul(arg, &endptr, 0);
if (*endptr != '\0' || errno != 0)
log_errx("Invalid alignment: %s", arg);

return align_reserve;
}

static void parse_blob_opt(char *arg, uuid_t *uuid, char *filename, size_t len)
{
char *p;
Expand All @@ -673,7 +688,7 @@ static int create_cmd(int argc, char *argv[])
struct option *opts = NULL;
size_t nr_opts = 0;
unsigned long long toc_flags = 0;
unsigned long align = 1;
unsigned long align = 1, align_reserve = 0;

if (argc < 2)
create_usage();
Expand All @@ -682,6 +697,8 @@ static int create_cmd(int argc, char *argv[])
opts = add_opt(opts, &nr_opts, "plat-toc-flags", required_argument,
OPT_PLAT_TOC_FLAGS);
opts = add_opt(opts, &nr_opts, "align", required_argument, OPT_ALIGN);
opts = add_opt(opts, &nr_opts, "align-reserve", required_argument,
OPT_ALIGN_RESERVE);
opts = add_opt(opts, &nr_opts, "blob", required_argument, 'b');
opts = add_opt(opts, &nr_opts, NULL, 0, 0);

Expand All @@ -706,6 +723,9 @@ static int create_cmd(int argc, char *argv[])
case OPT_ALIGN:
align = get_image_align(optarg);
break;
case OPT_ALIGN_RESERVE:
align_reserve = get_image_align_reservation(optarg);
break;
case 'b': {
char name[_UUID_STR_LEN + 1];
char filename[PATH_MAX] = { 0 };
Expand Down Expand Up @@ -741,7 +761,7 @@ static int create_cmd(int argc, char *argv[])

update_fip();

pack_images(argv[0], toc_flags, align);
pack_images(argv[0], toc_flags, align, align_reserve);
return 0;
}

Expand All @@ -753,6 +773,7 @@ static void create_usage(void)
printf("\n");
printf("Options:\n");
printf(" --align <value>\t\tEach image is aligned to <value> (default: 1).\n");
printf(" --align-reserve <value>\t\tEach image is padded by <value> (default: 0).\n");
Copy link

@ghost ghost Apr 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use single \t here and in update_usage() and remove_usage().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you wish.
Can you think of any ways to improve the wording while at it? I clearly copied the --align example, but it might be clearer to append "bytes" to both? I.e., to distinguish it from which byte value is used to pad up to the alignment size, which I assume is always 0x00.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably mention the units indeed.

printf(" --blob uuid=...,file=...\tAdd an image with the given UUID pointed to by file.\n");
printf(" --plat-toc-flags <value>\t16-bit platform specific flag field occupying bits 32-47 in 64-bit ToC header.\n");
printf("\n");
Expand All @@ -770,14 +791,16 @@ static int update_cmd(int argc, char *argv[])
char outfile[PATH_MAX] = { 0 };
fip_toc_header_t toc_header = { 0 };
unsigned long long toc_flags = 0;
unsigned long align = 1;
unsigned long align = 1, align_reserve = 0;
int pflag = 0;

if (argc < 2)
update_usage();

opts = fill_common_opts(opts, &nr_opts, required_argument);
opts = add_opt(opts, &nr_opts, "align", required_argument, OPT_ALIGN);
opts = add_opt(opts, &nr_opts, "align-reserve", required_argument,
OPT_ALIGN_RESERVE);
opts = add_opt(opts, &nr_opts, "blob", required_argument, 'b');
opts = add_opt(opts, &nr_opts, "out", required_argument, 'o');
opts = add_opt(opts, &nr_opts, "plat-toc-flags", required_argument,
Expand Down Expand Up @@ -828,6 +851,9 @@ static int update_cmd(int argc, char *argv[])
case OPT_ALIGN:
align = get_image_align(optarg);
break;
case OPT_ALIGN_RESERVE:
align_reserve = get_image_align_reservation(optarg);
break;
case 'o':
snprintf(outfile, sizeof(outfile), "%s", optarg);
break;
Expand All @@ -854,7 +880,7 @@ static int update_cmd(int argc, char *argv[])

update_fip();

pack_images(outfile, toc_flags, align);
pack_images(outfile, toc_flags, align, align_reserve);
return 0;
}

Expand All @@ -866,6 +892,7 @@ static void update_usage(void)
printf("\n");
printf("Options:\n");
printf(" --align <value>\t\tEach image is aligned to <value> (default: 1).\n");
printf(" --align-reserve <value>\t\tEach image is padded by <value> (default: 0).\n");
printf(" --blob uuid=...,file=...\tAdd or update an image with the given UUID pointed to by file.\n");
printf(" --out FIP_FILENAME\t\tSet an alternative output FIP file.\n");
printf(" --plat-toc-flags <value>\t16-bit platform specific flag field occupying bits 32-47 in 64-bit ToC header.\n");
Expand Down Expand Up @@ -1020,14 +1047,16 @@ static int remove_cmd(int argc, char *argv[])
char outfile[PATH_MAX] = { 0 };
fip_toc_header_t toc_header;
image_desc_t *desc;
unsigned long align = 1;
unsigned long align = 1, align_reserve = 0;
int fflag = 0;

if (argc < 2)
remove_usage();

opts = fill_common_opts(opts, &nr_opts, no_argument);
opts = add_opt(opts, &nr_opts, "align", required_argument, OPT_ALIGN);
opts = add_opt(opts, &nr_opts, "align-reserve", required_argument,
OPT_ALIGN_RESERVE);
opts = add_opt(opts, &nr_opts, "blob", required_argument, 'b');
opts = add_opt(opts, &nr_opts, "force", no_argument, 'f');
opts = add_opt(opts, &nr_opts, "out", required_argument, 'o');
Expand All @@ -1051,6 +1080,9 @@ static int remove_cmd(int argc, char *argv[])
case OPT_ALIGN:
align = get_image_align(optarg);
break;
case OPT_ALIGN_RESERVE:
align_reserve = get_image_align_reservation(optarg);
break;
case 'b': {
char name[_UUID_STR_LEN + 1], filename[PATH_MAX];
uuid_t uuid = { 0 };
Expand Down Expand Up @@ -1113,7 +1145,7 @@ static int remove_cmd(int argc, char *argv[])
}
}

pack_images(outfile, toc_header.flags, align);
pack_images(outfile, toc_header.flags, align, align_reserve);
return 0;
}

Expand All @@ -1125,6 +1157,7 @@ static void remove_usage(void)
printf("\n");
printf("Options:\n");
printf(" --align <value>\tEach image is aligned to <value> (default: 1).\n");
printf(" --align-reserve <value>\t\tEach image is padded by <value> (default: 0).\n");
printf(" --blob uuid=...\tRemove an image with the given UUID.\n");
printf(" --force\t\tIf the output FIP file already exists, use --force to overwrite it.\n");
printf(" --out FIP_FILENAME\tSet an alternative output FIP file.\n");
Expand Down