Hi Sughosh, On Mon, Dec 4, 2023 at 7:15 AM Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > hi Simon, > > On Sat, 2 Dec 2023 at 00:02, Simon Glass <s...@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > > > > > hi Simon, > > > > > > On Thu, 30 Nov 2023 at 08:16, Simon Glass <s...@chromium.org> wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu <sughosh.g...@linaro.org> > > > > wrote: > > > > > > > > > > hi Ilias, > > > > > > > > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas > > > > > <ilias.apalodi...@linaro.org> wrote: > > > > > > > > > > > > Hi all, > > > > > > > > > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu > > > > > > <sughosh.g...@linaro.org> wrote: > > > > > > > > > > > > > > hi Simon, > > > > > > > > > > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass <s...@chromium.org> > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu > > > > > > > > <sughosh.g...@linaro.org> wrote: > > > > > > > > > > > > > > > > > > Add support for specifying the parameters needed for capsule > > > > > > > > > generation through a config file, instead of passing them > > > > > > > > > through > > > > > > > > > command-line. Parameters for more than a single capsule file > > > > > > > > > can be > > > > > > > > > specified, resulting in generation of multiple capsules > > > > > > > > > through a > > > > > > > > > single invocation of the command. > > > > > > > > > > > > > > > > > > The config file can then be passed to the mkeficapsule tool > > > > > > > > > in such > > > > > > > > > manner > > > > > > > > > > > > > > > > > > $ ./tools/mkeficapsule -f <path/to/the/config/file> > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > > > > > > > > > --- > > > > > > > > > tools/Kconfig | 15 ++ > > > > > > > > > tools/Makefile | 1 + > > > > > > > > > tools/eficapsule.h | 114 ++++++++++++ > > > > > > > > > tools/mkeficapsule.c | 87 +++++---- > > > > > > > > > tools/mkeficapsule_parse.c | 352 > > > > > > > > > +++++++++++++++++++++++++++++++++++++ > > > > > > > > > 5 files changed, 538 insertions(+), 31 deletions(-) > > > > > > > > > create mode 100644 tools/mkeficapsule_parse.c > > > > > > > > > > > > > > > > This patch keeps coming back :-) > > > > > > > > > > > > > > > > Can we not add multiple capsules in the binman description? Why > > > > > > > > do we > > > > > > > > need a new file format? How can binman decode images produced > > > > > > > > in this > > > > > > > > way? > > > > > > > > > > > > > > So as Tom mentions, this brings parity with respect to the other > > > > > > > capsule generation tool in EDKII that generates capsules. IIRC, > > > > > > > this > > > > > > > is something which even Xilix was interested in, and Michal had > > > > > > > kind > > > > > > > of gone through these patches earlier. Lastly, it would be good to > > > > > > > have support in U-Boot's mkeficapsule tool for generating a single > > > > > > > capsule file with multiple payloads, and having support for this > > > > > > > functionality helps in that goal. > > > > > > > > > > > > > > Also, you might have noticed that, since your objection to the > > > > > > > last > > > > > > > series, I have removed putting this in binman. So now, this > > > > > > > aspect of > > > > > > > the capsule generation would only be supported through the > > > > > > > command-line invocation of the tool. > > > > > > > > > > > > I think that overall the approach is sane. mkeficapsule is currently > > > > > > supported and compiled for distros, so the multiple payload support > > > > > > is > > > > > > useful. If we want to add support to binman, instead of rewriting > > > > > > this > > > > > > in python, we could just call that tool for parsing and creating > > > > > > capsules > > > > > > > > > > Given the amount of time these patches have been under review(also > > > > > number of iterations), I would request that this series be reviewed > > > > > and merged first. I think there is general consensus that there is > > > > > value to have this functionality in the mkeficapsule tool. If it is > > > > > deemed fit to support this through binman as well, that task can be > > > > > taken up separately. Thanks. > > > > > > > > The point you are missing is that it is the entire goal of 'skirting > > > > around' binman which is suspect. > > > > > > > > If there is a need to generate an output file from the build, we > > > > should support it in binman. If people start creating configuration > > > > files all over the place, then they are not using binman, right? > > > > > > > > I understand that there are pre-existing vendor-specific config files, > > > > etc. that the EFI thing is a grey area, but I cannot imagine that this > > > > patch would lead to a good outcome. > > > > > > > > The goal of binman is to bring order to the chaos of firmware > > > > packaging...we cannot do that if it is not actually used. > > > > > > > > So let's figure out what is missing from binman's capsule generation > > > > (multiple capsules? accept/reject capsules?) and how best to add it. > > > > > > I think I need to jog your memory back a bit. For context, I have > > > jotted down the points. > > > > > > * The mkeficapsule tool generates capsules in U-Boot. > > > * Currently, when the tool is invoked from the command-line, the > > > capsules are generated by passing the capsule parameters as cmd-line > > > options. > > > * I had earlier added support for generating the capsules as part of > > > U-Boot build, through binman. This support has been merged. > > > * I had followed these patches up with another series [1] which > > > generates capsules by parsing the capsule parameters through a config > > > file instead of cmd-line options. > > > * This series also had patches which were attempting to integrate this > > > functionality into binman [2]. > > > * As part of reviewing the patch series, you had objected to adding > > > this support in binman, primarily because this way of specifying the > > > capsule parameters goes against the normal way of image description in > > > binman [3]. > > > * I have described in this mail thread about why we need to have > > > support for generating capsules through config files [4]. > > > > Thanks for that. Yes this fits with my memory, such as it is. > > > > I regard the tool as a binman tool, though, in the sense that binman > > is responsible for generating the images. For example, binman calls > > mkimage to handle FIT images. People are free to write a script to > > call mkimage manually, but the in-tree use of mkimage is supposed to > > be with binman. > > No issues with supporting the functionality through binman. However, I > think that if binman is to be used as the encompassing packaging tool > which in turn calls other tools like mkimage or mkeficapsule, it > should make allowances for accepting formats, or ways of describing > payloads that might not sit with it's core design of "each image is a > DT node". > > > > > > > > > So, in essence, this functionality is needed to be added to the tool. > > > I have earlier tried integrating this in binman, but that was > > > rejected. So, the way I see this moving ahead is to first add support > > > for this feature in the tool, and then see if this can also be added > > > in binman. As I mentioned earlier, I am fine with this functionality > > > not getting integrated with binman, if this contravenes the idea of > > > describing images in binman, and if no exceptions can be made on that > > > regard. But please do understand that no attempt is being made at > > > 'skirting around binman'. In fact, I had worked on your earlier > > > objection of using absolute paths in testing this functionality in > > > binman. But then you had put the other objection of how this does not > > > follow the idea or concept of image description in binman. Hence this > > > approach. > > > > It would be easier to talk this through f2f, but I will make an attempt > > here. > > > > The goal here is for people to be able to create an EFI capsule (or > > more than one?) automatically as part of the build. > > That is one of the goals, yes. But the other goal is also to add > functionality to the base capsule generation tool which extends the > tool to support generating capsules through the config file. I would > be more than happy if this can be plumbed in as part of the build. > > > > Since the config > > is in the binman description, putting it somewhere else doesn't help > > anyone. It will lead to .cfg files in U-Boot and Binman will just be a > > side show. That is my concern. > > Okay. But I don't think that is the main issue here. For example, we > can require the .cfg files to be placed in the board dir, similar to > how the capsule keys are placed. The issue here is that can binman > make allowances(IMO it should) to have the capsule payloads be > specified through a .cfg file parameter, instead of the usual way of > specifying it as a DT node. I think if this allowance can be made, it > solves all the issues here. > > > > > I would really like to see a capsule generated for sandbox so I can > > understand this better. > > I will work on the capsule generation for sandbox. I will generate > signed capsules, as you suggested in this mail thread as part of the > sandbox U-Boot build. However, this will be using the currently merged > capsule generation logic, where all the capsule parameters, it's > payload included, are specified in binman nodes. I suspect this won't > give you a picture of capsule generation through config files.
Is there is progress on this? I would still really like to see a single capsule generated by the sandbox build. > > > > > Anyway, I hope that makes sense? If not, I think we should discuss it > > f2f or VC or somehow. > > Sure. I will be happy to have a VC anytime if you think that would > help. Let me know and we can sync up over IRC. Thanks. Perhaps we can do this in the new year. I think it would help to do a patch for the above first, though, since it might answer many of my questions. Regards, Simon