Replace fip_create with fiptool

[ghost]

fiptool provides a more consistent and intuitive interface compared to
the fip_create program. It serves as a better base to build on more
features in the future.

fiptool supports various subcommands. Below are the currently
supported subcommands:

1. info - List the images contained in a FIP file.
2. create - Create a new FIP file with the given images.
3. update - Update an existing FIP with the given images.
4. unpack - Extract a selected set or all the images from a FIP file.
5. remove - Remove images from a FIP file. This is a new command that
   was not present in fip_create.

To create a new FIP file, replace "fip_create" with "fiptool create".

To update a FIP file, replace "fip_create" with "fiptool update".

To dump the contents of a FIP file, replace "fip_create --dump" with
"fiptool info".

A compatibility script that emulates the basic functionality of
fip_create is provided. Existing scripts might or might not work with
the compatibility script. Users are strongly encouraged to migrate to
fiptool.

Fixes ARM-software/tf-issues#87
Fixes ARM-software/tf-issues#108
Fixes ARM-software/tf-issues#361

[ssg-bot]

Can one of the admins verify this patch?

[danh-arm]

jenkins: add to whitelist

[danh-arm]

jenkins: test this please

[afaerber]

How do you plan to handle plat specifics? Amlogic has patched fip_create to add padding in between entries for 16k alignment, which - assuming we don't want multiple plat-specific builds of fiptool with the same name - will need to be user-supplied for both packing and info operations.

[danh-arm]

Hi @afaerber

How do you plan to handle plat specifics?

That's quite an open question. Platforms can currently extend fiptool in 2 ways:

1. They can provide their own set of FIP entries by replacing tbbr_config.c.
2. They can override the platform specific ToC flags via the option --plat-toc-flags.

Anything else is a new requirement. We can only provide compatibility for code/platforms that are upstream, or if we at least know about their requirements.

It sounds like this requirement could be added as a build option on top of the this PR. We're happy to accept contributions once the CLA is signed.

[afaerber]

Hi @danh-arm

Very specifically I am commenting on the above hard check on argc == 2, whereas pack_cmd() further below checks for argc < 2 and has getopt handling. Further, main() only has rudimentary -v/--verbose handling. fip_create by contrast had all options handled in a central place, so I was pointing out that this refactoring seems to be making the use case of adding options harder here. Can be fixed as follow-up of course.

Coming from a Linux distribution background, I need to package our, e.g., U-Boot bootloader into a FIP in order to build bootable SD card images (e.g., for Hardkernel Odroid-C2). Therefore I would like to package one official, maintained version of fip_create / fiptool and use that to combine vendor's ATF (where not yet upstream) and our U-Boot. In general, having a tool with the same name multiple times is a no-go. For example, we have one /usr/bin/mkimage tool from mainline U-Boot (packaged as u-boot-tools), not hundreds from different vendors that would all conflict and likely not get the same fixes. Note that in our packaging workflow, the time of building the tool and using it are distinct.

So while you are right that vendors can fork and patch your software, I would really like to have one official tool with suitable command line options. Be it --pad=0x4000 or --target=amlogic/gxbb or anything like that, defaulting to current vanilla behavior. It would be a global option then, applying to all commands - either after the subcommand or before it.
Writing local patches is not the problem - how to get them mainline is, to not have to maintain patches for dozens of vendors myself. And the commit message here specifically promises a better base for future features, which I'm seeing this as. A uniform SBBR world is not what we're seeing in practice, so we'll need to handle variances. [EDIT: I realize that I misread TBBR as SBBR... still a valid point.]

Restricting bugfix contributions for a tool that does not directly touch on vendor IP is counter-productive. Would you seriously rather have us fork the tool into U-Boot (like Amlogic/Hardkernel did) or as a separate project to get around your contribution hurdles? That can't be in your best interest, unless you plan to drop it here then.

[afaerber]

Regarding --plat-toc-flags= this is what I see with this branch here:

DEBUG: toc_header[flags]: 0x0

So I might use that argument to create FIP files and then use it as a hint in info_cmd(). But that wouldn't work with foreign-created FIP files without such custom flag. And it doesn't solve creating custom entries yet.

Apart from replacing tbbr_config.c, are you open to extending it? I doubt that you want a fake UUID like this in tbbr_config.c:toc_entries[]? (Yes, that's what Amlogic is using!)

{0xAABBCCDD, 0xABCD, 0xEFEF, 0xAB, 0xCD, {0x12, 0x34, 0x56, 0x78, 0xAB, 0xCD} }

Most functions appear to assume that there is one global list that can be iterated over (which could be duplicated theoretically) or even accessed by index.

[danh-arm]

Hi @afaerber

Very specifically I am commenting on the above hard check on argc == 2, whereas pack_cmd() further below checks for argc < 2 and has getopt handling.

Yes, if support were needed for options on the info sub-command, that would need changing. I'm sure we'd be open to that.

Further, main() only has rudimentary -v/--verbose handling.

fip_create doesn't have that option at all. We can always build on this.

fip_create by contrast had all options handled in a central place, so I was pointing out that this refactoring seems to be making the use case of adding options harder here. Can be fixed as follow-up of course.

Processing options centrally makes it more difficult for the sub-command to handle unsupported/incompatible options for that sub-command. This way, it's clearer which options apply to which sub-command. There could be better code sharing in the handling of options that apply to multiple sub-commands, and the mapping of sub-commands to valid options could be represented more centrally in the code (e.g. via some data structure). I agree this can be done later if needed, but the current set of options / sub-commands don't really justify it.

... Therefore I would like to package one official, maintained version of fip_create / fiptool and use that to combine vendor's ATF (where not yet upstream) and our U-Boot...

Sounds good to me.

So while you are right that vendors can fork and patch your software, I would really like to have one official tool with suitable command line options.

I never said that; I agree it's much better if vendors contribute their changes back upstream into 1 official tool.

Be it --pad=0x4000 or --target=amlogic/gxbb or anything like that, defaulting to current vanilla behavior. It would be a global option then, applying to all commands - either after the subcommand or before it.

Although those commands sounds reasonable and would apply to multiple sub-commands, I'm not sure they're truly global. Anyway, that's just an implementation detail.

Writing local patches is not the problem - how to get them mainline is, to not have to maintain patches for dozens of vendors myself. And the commit message here specifically promises a better base for future features, which I'm seeing this as. A uniform SBBR world is not what we're seeing in practice, so we'll need to handle variances. [EDIT: I realize that I misread TBBR as SBBR... still a valid point.]

Hmm, it sounds like you're at the hard end of handling platform variation! Up to now, we haven't been very aware of platform specific requirements like this. We'd be grateful of any code contributions or raising of requirements on our issue tracker.

Restricting bugfix contributions for a tool that does not directly touch on vendor IP is counter-productive. Would you seriously rather have us fork the tool into U-Boot (like Amlogic/Hardkernel did) or as a separate project to get around your contribution hurdles? That can't be in your best interest, unless you plan to drop it here then.

I'm not trying to introduce any contribution hurdles for you; I'm just saying what the current process is. Of course I'd rather avoid forks, but this is the first time I heard there was a fork, or that the the TF contribution process is causing a problem.

If required I can go back to our legal team and try to improve the current process, but I can't unilaterally change it. Are you only concerned about bug fix changes or contributing more generally (including feature enhancements)? Are the CLA terms an issue for you, or the fact that there is one at all? We can take this offline if you want.

[danh-arm]

Unless there are any futher comments, I will merge this as-is. Dimitris has addressed the immediate code concerns. The other code improvements can be addressed later. We are separately pursuing improvements to the contribution process.

[danh-arm]

We've had some offline comments about this PR, which we're going to address. To summarise:

• We're going to remove the .sh extension from the fip_create.sh script so that most existing command invocations will work without have to update to fiptool. This causes some checkpatch and CI issues but we'll resolve those too.
• We're going to improve the user guide and commit message to provide a bit more helpful migration info.
• We're going to split "fiptool pack --create" and "fiptool pack" into 2 separate sub-commands, "fiptool create" and "fiptool update". Internally they will share mostly the same code. The --force option for "fiptool update" will become unnecessary, since it is clear to the user that any existing fip will be overwritten.

[danh-arm]

Perhaps just need a little more context here. Suggest:
"The previous version of fiptool was called fip_create. A compatibility script that emulates the basic functionality of
the previous fip_create is provided. However, users are strongly encouraged to migrate to fiptool."