Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-12-28 Thread Simon Glass
Hi Sughosh,

On Fri, Dec 29, 2023 at 6:53 AM Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Wed, 27 Dec 2023 at 23:19, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Mon, Dec 4, 2023 at 7:15 AM Sughosh Ganu  wrote:
> > >
> > > hi Simon,
> > >
> > > On Sat, 2 Dec 2023 at 00:02, Simon Glass  wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu  
> > > > wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Thu, 30 Nov 2023 at 08:16, Simon Glass  wrote:
> > > > > >
> > > > > > Hi Sughosh,
> > > > > >
> > > > > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu 
> > > > > >  wrote:
> > > > > > >
> > > > > > > hi Ilias,
> > > > > > >
> > > > > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > Hi all,
> > > > > > > >
> > > > > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > hi Simon,
> > > > > > > > >
> > > > > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass  
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Sughosh,
> > > > > > > > > >
> > > > > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu 
> > > > > > > > > >  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 
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > > > > > ---
> > > > > > > > > > >  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 

Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-12-28 Thread Sughosh Ganu
hi Simon,

On Wed, 27 Dec 2023 at 23:19, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Mon, Dec 4, 2023 at 7:15 AM Sughosh Ganu  wrote:
> >
> > hi Simon,
> >
> > On Sat, 2 Dec 2023 at 00:02, Simon Glass  wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu  
> > > wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Thu, 30 Nov 2023 at 08:16, Simon Glass  wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu  
> > > > > wrote:
> > > > > >
> > > > > > hi Ilias,
> > > > > >
> > > > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > > > >  wrote:
> > > > > > >
> > > > > > > Hi all,
> > > > > > >
> > > > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > hi Simon,
> > > > > > > >
> > > > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Sughosh,
> > > > > > > > >
> > > > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu 
> > > > > > > > >  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 
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > > > > ---
> > > > > > > > > >  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. 

Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-12-27 Thread Simon Glass
Hi Sughosh,

On Mon, Dec 4, 2023 at 7:15 AM Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Sat, 2 Dec 2023 at 00:02, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu  wrote:
> > >
> > > hi Simon,
> > >
> > > On Thu, 30 Nov 2023 at 08:16, Simon Glass  wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu  
> > > > wrote:
> > > > >
> > > > > hi Ilias,
> > > > >
> > > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > > >  wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu 
> > > > > >  wrote:
> > > > > > >
> > > > > > > hi Simon,
> > > > > > >
> > > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Sughosh,
> > > > > > > >
> > > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu 
> > > > > > > >  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 
> > > > > > > > >
> > > > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > > > ---
> > > > > > > > >  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
> > > > 

Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-12-03 Thread Sughosh Ganu
hi Simon,

On Sat, 2 Dec 2023 at 00:02, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu  wrote:
> >
> > hi Simon,
> >
> > On Thu, 30 Nov 2023 at 08:16, Simon Glass  wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu  
> > > wrote:
> > > >
> > > > hi Ilias,
> > > >
> > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > >  wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu  
> > > > > wrote:
> > > > > >
> > > > > > hi Simon,
> > > > > >
> > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass  wrote:
> > > > > > >
> > > > > > > Hi Sughosh,
> > > > > > >
> > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu 
> > > > > > >  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 
> > > > > > > >
> > > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > > ---
> > > > > > > >  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
> > 

Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-12-01 Thread Simon Glass
Hi Sughosh,

On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Thu, 30 Nov 2023 at 08:16, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu  wrote:
> > >
> > > hi Ilias,
> > >
> > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > >  wrote:
> > > >
> > > > Hi all,
> > > >
> > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu  
> > > > wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass  wrote:
> > > > > >
> > > > > > Hi Sughosh,
> > > > > >
> > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu 
> > > > > >  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 
> > > > > > >
> > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > ---
> > > > > > >  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 

Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-11-30 Thread Sughosh Ganu
hi Simon,

On Thu, 30 Nov 2023 at 08:16, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu  wrote:
> >
> > hi Ilias,
> >
> > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> >  wrote:
> > >
> > > Hi all,
> > >
> > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu  
> > > wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass  wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu  
> > > > > 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 
> > > > > >
> > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > ---
> > > > > >  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 

Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-11-29 Thread Simon Glass
Hi Sughosh,

On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu  wrote:
>
> hi Ilias,
>
> On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
>  wrote:
> >
> > Hi all,
> >
> > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu  wrote:
> > >
> > > hi Simon,
> > >
> > > On Wed, 22 Nov 2023 at 03:42, Simon Glass  wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu  
> > > > 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 
> > > > >
> > > > > Signed-off-by: Sughosh Ganu 
> > > > > ---
> > > > >  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.

>
> -sughosh
>
> >
> > Thanks
> > /Ilias
> > >
> > > >
> > > > Also, could we get sandbox to produce one EFI capsule as part of the
> > > > normal build? I think that discussion trailed off.
> > >
> > > Yes, apologies for missing out on this. Slipped my mind. Would you
> > > want, say, all the non-signed capsules to be generated as part of the
> > > sandbox build?
> > >
> > > -sughosh

Regards,
Simon


Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-11-29 Thread Simon Glass
Hi Sughosh,

On Tue, 21 Nov 2023 at 22:23, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Wed, 22 Nov 2023 at 03:42, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu  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 
> > >
> > > Signed-off-by: Sughosh Ganu 
> > > ---
> > >  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.

That sounds like the opposite of what I was asking for...

>
> >
> > Also, could we get sandbox to produce one EFI capsule as part of the
> > normal build? I think that discussion trailed off.
>
> Yes, apologies for missing out on this. Slipped my mind. Would you
> want, say, all the non-signed capsules to be generated as part of the
> sandbox build?

Just one would be good. Probably using a signed one makes more sense
since it is more realistic?

You don't need to change any of the existing tests...I just mean to
add a single capsule generated as part of the sandbox binman
invocation.

The problem is that, at the moment, I cannot make much sense of the
build/binman integration since nothing actually uses it. Once sandbox
uses it, I will be able to understand it better and suggest ways to
expand the binman support.

Regards,
Simon


Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-11-21 Thread Sughosh Ganu
hi Ilias,

On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
 wrote:
>
> Hi all,
>
> On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu  wrote:
> >
> > hi Simon,
> >
> > On Wed, 22 Nov 2023 at 03:42, Simon Glass  wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu  
> > > 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 
> > > >
> > > > Signed-off-by: Sughosh Ganu 
> > > > ---
> > > >  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.

-sughosh

>
> Thanks
> /Ilias
> >
> > >
> > > Also, could we get sandbox to produce one EFI capsule as part of the
> > > normal build? I think that discussion trailed off.
> >
> > Yes, apologies for missing out on this. Slipped my mind. Would you
> > want, say, all the non-signed capsules to be generated as part of the
> > sandbox build?
> >
> > -sughosh


Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-11-21 Thread Ilias Apalodimas
Hi all,

On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Wed, 22 Nov 2023 at 03:42, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu  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 
> > >
> > > Signed-off-by: Sughosh Ganu 
> > > ---
> > >  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

Thanks
/Ilias
>
> >
> > Also, could we get sandbox to produce one EFI capsule as part of the
> > normal build? I think that discussion trailed off.
>
> Yes, apologies for missing out on this. Slipped my mind. Would you
> want, say, all the non-signed capsules to be generated as part of the
> sandbox build?
>
> -sughosh


Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-11-21 Thread Sughosh Ganu
hi Tom,

On Wed, 22 Nov 2023 at 03:54, Tom Rini  wrote:
>
> On Tue, Nov 21, 2023 at 03:11:50PM -0700, Simon Glass wrote:
> > Hi Sughosh,
> >
> > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu  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 
> > >
> > > Signed-off-by: Sughosh Ganu 
> > > ---
> > >  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?
> >
> > Also, could we get sandbox to produce one EFI capsule as part of the
> > normal build? I think that discussion trailed off.
>
> I believe part of the answer here is to support the config file format
> that other capsule generators use. Do I recall this all right, Sughosh?

Yes, that is correct. There are some other reasons as well, which I
have mentioned in the reply to Simon's email.

-sughosh


Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-11-21 Thread Sughosh Ganu
hi Simon,

On Wed, 22 Nov 2023 at 03:42, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu  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 
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> >  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.

>
> Also, could we get sandbox to produce one EFI capsule as part of the
> normal build? I think that discussion trailed off.

Yes, apologies for missing out on this. Slipped my mind. Would you
want, say, all the non-signed capsules to be generated as part of the
sandbox build?

-sughosh


Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-11-21 Thread Tom Rini
On Tue, Nov 21, 2023 at 03:11:50PM -0700, Simon Glass wrote:
> Hi Sughosh,
> 
> On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu  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 
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> >  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?
> 
> Also, could we get sandbox to produce one EFI capsule as part of the
> normal build? I think that discussion trailed off.

I believe part of the answer here is to support the config file format
that other capsule generators use. Do I recall this all right, Sughosh?

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-11-21 Thread Simon Glass
Hi Sughosh,

On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu  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 
>
> Signed-off-by: Sughosh Ganu 
> ---
>  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?

Also, could we get sandbox to produce one EFI capsule as part of the
normal build? I think that discussion trailed off.

Regards,
Simon


[PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

2023-11-20 Thread Sughosh Ganu
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 

Signed-off-by: Sughosh Ganu 
---
 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

diff --git a/tools/Kconfig b/tools/Kconfig
index 6e23f44d55..9408b60fac 100644
--- a/tools/Kconfig
+++ b/tools/Kconfig
@@ -98,6 +98,21 @@ config TOOLS_MKEFICAPSULE
  optionally sign that file. If you want to enable UEFI capsule
  update feature on your target, you certainly need this.
 
+config EFI_CAPSULE_CFG_FILE
+   string "Path to the EFI Capsule Config File"
+   help
+ Path to the EFI capsule config file which provides the
+ parameters needed to build capsule(s). Parameters can be
+ provided for multiple payloads resulting in corresponding
+ capsule images being generated.
+
+config EFI_USE_CAPSULE_CFG_FILE
+   bool "Use the config file for generating capsules"
+   help
+ Boolean option used to specify if the EFI capsules are to
+ be generated through parameters specified via the config
+ file or through command line.
+
 menuconfig FSPI_CONF_HEADER
bool "FlexSPI Header Configuration"
help
diff --git a/tools/Makefile b/tools/Makefile
index 1aa1e36137..ce7217e623 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -251,6 +251,7 @@ HOSTLDLIBS_mkeficapsule += \
 HOSTLDLIBS_mkeficapsule += \
$(shell pkg-config --libs uuid 2> /dev/null || echo "-luuid")
 hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
+mkeficapsule-objs := mkeficapsule.o mkeficapsule_parse.o
 
 mkfwumdata-objs := mkfwumdata.o generated/lib/crc32.o
 HOSTLDLIBS_mkfwumdata += -luuid
diff --git a/tools/eficapsule.h b/tools/eficapsule.h
index 6efd07d2eb..71a08b62e6 100644
--- a/tools/eficapsule.h
+++ b/tools/eficapsule.h
@@ -54,6 +54,12 @@ typedef struct {
 /* flags */
 #define CAPSULE_FLAGS_PERSIST_ACROSS_RESET  0x0001
 
+enum capsule_type {
+   CAPSULE_NORMAL_BLOB = 0,
+   CAPSULE_ACCEPT,
+   CAPSULE_REVERT,
+};
+
 struct efi_capsule_header {
efi_guid_t capsule_guid;
uint32_t header_size;
@@ -145,4 +151,112 @@ struct fmp_payload_header_params {
uint32_t fw_version;
 };
 
+/**
+ * struct efi_capsule_params - Capsule parameters
+ * @image_guid: Guid value of the payload input image
+ * @image_index: Image index value
+ * @hardware_instance: Hardware instance to be used for the image
+ * @fmp: FMP payload header used for storing firmware version
+ * @monotonic_count: Monotonic count value to be used for signed capsule
+ * @privkey_file: Path to private key used in capsule signing
+ * @cert_file: Path to public key certificate used in capsule signing
+ * @input_file: Path to payload input image
+ * @capsule_file: Path to the output capsule file
+ * @oemflags: Oemflags to be populated in the capsule header
+ * @capsule: Capsule Type, normal or accept or revert
+ */
+struct efi_capsule_params {
+   efi_guid_t *image_guid;
+   unsigned long image_index;
+   unsigned long hardware_instance;
+   struct fmp_payload_header_params fmp;
+   uint64_t monotonic_count;
+   char *privkey_file;
+   char *cert_file;
+   char *input_file;
+   char *capsule_file;
+   unsigned long oemflags;
+   enum capsule_type capsule;
+};
+
+/**
+ * capsule_with_cfg_file() - Generate capsule from config file
+ * @cfg_file: Path to the config file
+ *
+ * Parse the capsule parameters from the config file and use the
+ * parameters for generating one or more capsules.
+ *
+ * Return: None
+ *
+ */
+void capsule_with_cfg_file(const char *cfg_file);
+
+/**
+ * convert_uuid_to_guid() - convert UUID to GUID
+ * @buf:   UUID binary
+ *
+ * UUID and GUID have the same data structure, but their binary
+ * formats are different due to the endianness. See lib/uuid.c.
+ * Since uuid_parse() can handle only UUID, this function must
+ * be called to get correct data for GUID when parsing a string.
+ *
+ * The correct data will be returned in @buf.
+ */
+void convert_uuid_to_guid(unsigned char *buf);
+
+/**
+ * create_empty_capsule() - Generate an empty capsule
+ * @path: Path to the empty capsule file to be generated
+ * @guid: Guid value of the image for which empty capsule is generated
+ * @fw_accept: Flag to specify whether to generate accept or revert capsule
+ *
+ * Generate an empty capsule, either an accept or a revert