rimage: enable key to be override by ENV

[xiulipan]

To use the key from ENV instead of args.

Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com

some enhancement to #1239
usage:

export SOF_RIMAGE_KEY=path/to/you/key
make bin

...
using ENV key path/to/you/key
...
Firmware file size 0x2d000 page count 39
 pkcs: signing with key path/to/you/key
...

Fix #1238

[jajanusz]

I'm against passing it through env variables, just use cmdline arugments.

[jajanusz]

I guess you can just fix -k arguments if it doesn't work - @plbossart thinks that's the case in #1238 , but I think he just changed key in the wrong place and it never got passed to the rimage.

If -k works ok, I'll close this PR, unless you have another strong reason to start passing arguments via env variables.

[plbossart]

On 4/9/19 6:34 AM, Janusz Jankowski wrote: I guess you can just fix |-k| arguments if it doesn't work - @plbossart <https://github.com/plbossart> thinks that the case in #1238 <#1238> , but I think he just changed key in the wrong place and it never got passed to the rimage. If |-k| works ok, I'll close this PR, unless you have another strong reason to start passing arguments via env variables.

I don't get your point. What happens when you want to build all firmware with different keys, e.g. APL with the OTC key and ICL with a debug key? You'd want to just use the existing build system and just update a variable. We already have dependencies on tools location and core, I don't see the issue with having a pointer to a specific key? The last thing I want is a manual command I have to run after the build is done with the OTC public key.

[jajanusz]

I don't get your point. What happens when you want to build all firmware
with different keys, e.g. APL with the OTC key and ICL with a debug key?
You'd want to just use the existing build system and just update a
variable.

Just use cmake variable, you have to make different build/config for each of them anyway so you'll have something like that for APL:

cmake -Bbuild_apl -DRIMAGE_KEY=/path/to/apl/key ...

For ICL smth like:

cmake -Bbuild_icl -DRIMAGE_KEY=/path/to/icl/key ...

etc.

We already have dependencies on tools location and core, I
don't see the issue with having a pointer to a specific key?

The fact that w had to comply to bad practice in one place, doesn't mean we have to continue doing it.

The last thing I want is a manual command I have to run after the build
is done with the OTC public key.

You don't have to run any manual command after the build, you just do configuration step once for each config (set of cmake variables) and rebuild it with 1 command: make bin

[plbossart]

I don't get your point. What happens when you want to build all firmware with different keys, e.g. APL with the OTC key and ICL with a debug key? You'd want to just use the existing build system and just update a variable. Just use cmake variable, you have to make different build/config for each of them anyway so you'll have something like that for APL: |cmake -Bbuild_apl -DRIMAGE_KEY=/path/to/apl/key ... | For ICL smth like: |cmake -Bbuild_icl -DRIMAGE_KEY=/path/to/icl/key ... |

so I guess you have no objection to having an RIMAGE_KEY environment variable defined in xtensa-build-all.sh and using it to generate the makefiles? cmake -Bbuild_icl -DRIMAGE_KEY=${RIMAGE_KEY} ..

[jajanusz]

I don't get your point. What happens when you want to build all firmware with different keys, e.g. APL with the OTC key and ICL with a debug key? You'd want to just use the existing build system and just update a variable. Just use cmake variable, you have to make different build/config for each of them anyway so you'll have something like that for APL: |cmake -Bbuild_apl -DRIMAGE_KEY=/path/to/apl/key ... | For ICL smth like: |cmake -Bbuild_icl -DRIMAGE_KEY=/path/to/icl/key ... |
so I guess you have no objection to having an RIMAGE_KEY environment variable defined in xtensa-build-all.sh and using it to generate the makefiles? cmake -Bbuild_icl -DRIMAGE_KEY=${RIMAGE_KEY} ..

Yea, it's better to handle it in shell script

[xiulipan]

@jajanusz @plbossart I will close this and update #1239