[RFC,1/4] efidebug: capsule: Add a command to update capsule on disk

Message ID 20200323071201.5992-2-sughosh.ganu@linaro.org
State New
Headers show
Series
  • qemu: arm64: Add support for uefi firmware management protocol routines
Related show

Commit Message

Sughosh Ganu March 23, 2020, 7:11 a.m.
Add a efidebug subcommand to initiate a firmware update using the efi
firmware management protocol(fmp) set_image routine.

The firmware update can be initiated through

'efidebug capsule disk-update'

This would locate the efi capsule file on the efi system partition,
and call the platform's set_image fmp routine to initiate the firmware
update.

Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
---
 cmd/efidebug.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Heinrich Schuchardt March 23, 2020, 11:50 a.m. | #1
On 3/23/20 8:11 AM, Sughosh Ganu wrote:
> Add a efidebug subcommand to initiate a firmware update using the efi
> firmware management protocol(fmp) set_image routine.
>
> The firmware update can be initiated through
>
> 'efidebug capsule disk-update'
>
> This would locate the efi capsule file on the efi system partition,
> and call the platform's set_image fmp routine to initiate the firmware
> update.

Hello Sughosh,

why do we need this command? Shouldn't a simple reset do the job?

See chapter 8.5.5 "Delivery of Capsules via file on Mass Storage device"
of UEFI spec 2.8.

What might be of interest is a command to start a capsule from a memory
location. This would allow to load capsules from other locations than
directory \EFI\UpdateCapsule.

Best regards

Heinrich

>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
>  cmd/efidebug.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index 4a7661d0ac..fd8366dc90 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -77,6 +77,16 @@ static int do_efi_capsule_update(cmd_tbl_t *cmdtp, int flag,
>  	return CMD_RET_SUCCESS;
>  }
>
> +static int do_efi_capsule_on_disk_update(cmd_tbl_t *cmdtp, int flag,
> +					 int argc, char * const argv[])
> +{
> +	efi_status_t ret;
> +
> +	ret = efi_launch_capsules();
> +
> +	return ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> +}
> +
>  /**
>   * do_efi_capsule_show() - show capsule information
>   *
> @@ -205,6 +215,8 @@ static cmd_tbl_t cmd_efidebug_capsule_sub[] = {
>  			 "", ""),
>  	U_BOOT_CMD_MKENT(show, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_show,
>  			 "", ""),
> +	U_BOOT_CMD_MKENT(disk-update, 0, 0, do_efi_capsule_on_disk_update,
> +			 "", ""),
>  	U_BOOT_CMD_MKENT(result, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_res,
>  			 "", ""),
>  };
> @@ -1387,6 +1399,8 @@ static char efidebug_help_text[] =
>  #ifdef CONFIG_EFI_CAPSULE_UPDATE
>  	"efidebug capsule update [-v] <capsule address>\n"
>  	"  - process a capsule\n"
> +	"efidebug capsule disk-update\n"
> +	"  - update a capsule from disk\n"
>  	"efidebug capsule show <capsule address>\n"
>  	"  - show capsule information\n"
>  	"efidebug capsule result [<capsule result var>]\n"
>
Sughosh Ganu March 24, 2020, 3:49 a.m. | #2
On Mon, 23 Mar 2020 at 17:20, Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:

> On 3/23/20 8:11 AM, Sughosh Ganu wrote:
> > Add a efidebug subcommand to initiate a firmware update using the efi
> > firmware management protocol(fmp) set_image routine.
> >
> > The firmware update can be initiated through
> >
> > 'efidebug capsule disk-update'
> >
> > This would locate the efi capsule file on the efi system partition,
> > and call the platform's set_image fmp routine to initiate the firmware
> > update.
>
> Hello Sughosh,
>
> why do we need this command? Shouldn't a simple reset do the job?
>
> See chapter 8.5.5 "Delivery of Capsules via file on Mass Storage device"
> of UEFI spec 2.8.
>

Like i have mentioned in the cover letter, we need to invoke
capsule-on-disk update during the platform boot -- that is on the Todo
list. The spec requires setting the
EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED flag in the
OsIndications variable. But we would not be supporting that, at least for
now, since we do not have support of runtime variables on our platforms.
The idea is to invoke a capsule-on-disk update during the platform boot. I
think Takahiro will be working on this.


>
> What might be of interest is a command to start a capsule from a memory
> location. This would allow to load capsules from other locations than
> directory \EFI\UpdateCapsule.
>

Takahiro has indeed added this command which updates a capsule which has
been loaded into the memory. I thought it might be easier to invoke a
command where the user is not required to load the capsule into memory
first, but that is taken care of directly by the capsule update logic. That
said, I can drop the patch if you prefer just having the version which
updates the capsule from memory.

-sughosh


>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> >  cmd/efidebug.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> > index 4a7661d0ac..fd8366dc90 100644
> > --- a/cmd/efidebug.c
> > +++ b/cmd/efidebug.c
> > @@ -77,6 +77,16 @@ static int do_efi_capsule_update(cmd_tbl_t *cmdtp,
> int flag,
> >       return CMD_RET_SUCCESS;
> >  }
> >
> > +static int do_efi_capsule_on_disk_update(cmd_tbl_t *cmdtp, int flag,
> > +                                      int argc, char * const argv[])
> > +{
> > +     efi_status_t ret;
> > +
> > +     ret = efi_launch_capsules();
> > +
> > +     return ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> > +}
> > +
> >  /**
> >   * do_efi_capsule_show() - show capsule information
> >   *
> > @@ -205,6 +215,8 @@ static cmd_tbl_t cmd_efidebug_capsule_sub[] = {
> >                        "", ""),
> >       U_BOOT_CMD_MKENT(show, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_show,
> >                        "", ""),
> > +     U_BOOT_CMD_MKENT(disk-update, 0, 0, do_efi_capsule_on_disk_update,
> > +                      "", ""),
> >       U_BOOT_CMD_MKENT(result, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_res,
> >                        "", ""),
> >  };
> > @@ -1387,6 +1399,8 @@ static char efidebug_help_text[] =
> >  #ifdef CONFIG_EFI_CAPSULE_UPDATE
> >       "efidebug capsule update [-v] <capsule address>\n"
> >       "  - process a capsule\n"
> > +     "efidebug capsule disk-update\n"
> > +     "  - update a capsule from disk\n"
> >       "efidebug capsule show <capsule address>\n"
> >       "  - show capsule information\n"
> >       "efidebug capsule result [<capsule result var>]\n"
> >
>
>
AKASHI Takahiro March 31, 2020, 3:25 a.m. | #3
On Tue, Mar 24, 2020 at 09:19:55AM +0530, Sughosh Ganu wrote:
> On Mon, 23 Mar 2020 at 17:20, Heinrich Schuchardt <xypron.glpk at gmx.de>
> wrote:
> 
> > On 3/23/20 8:11 AM, Sughosh Ganu wrote:
> > > Add a efidebug subcommand to initiate a firmware update using the efi
> > > firmware management protocol(fmp) set_image routine.
> > >
> > > The firmware update can be initiated through
> > >
> > > 'efidebug capsule disk-update'
> > >
> > > This would locate the efi capsule file on the efi system partition,
> > > and call the platform's set_image fmp routine to initiate the firmware
> > > update.
> >
> > Hello Sughosh,
> >
> > why do we need this command? Shouldn't a simple reset do the job?
> >
> > See chapter 8.5.5 "Delivery of Capsules via file on Mass Storage device"
> > of UEFI spec 2.8.
> >
> 
> Like i have mentioned in the cover letter, we need to invoke
> capsule-on-disk update during the platform boot -- that is on the Todo
> list. The spec requires setting the
> EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED flag in the
> OsIndications variable. But we would not be supporting that, at least for
> now, since we do not have support of runtime variables on our platforms.
> The idea is to invoke a capsule-on-disk update during the platform boot. I
> think Takahiro will be working on this.

To be strict,
under my current implementation, all the capsule files under
\EFI\UpdateCapsule will be applied only at the first time when
any of UEFI-related commands is invoked.
This is the way how the UEFI subsystem is initialized on U-Boot.

I mentioned this in my cover letter, and how we should comply
with UEFI specification and meet users' expectation is a point
of discussion as RFC. 

UpdateCapsule API is provided, but it is more or less an internal
function in the current form so as to directly initiate capsule
handling. It currently ignores CAPSULE_FLAGS_PERSIST_ACROSS_RESET
and CAPSULE_FLAGS_INITIATE_RESET in a capsule.
(I mentioned this in TODO list.)

> 
> >
> > What might be of interest is a command to start a capsule from a memory
> > location. This would allow to load capsules from other locations than
> > directory \EFI\UpdateCapsule.
> >
> 
> Takahiro has indeed added this command which updates a capsule which has
> been loaded into the memory. I thought it might be easier to invoke a
> command where the user is not required to load the capsule into memory
> first, but that is taken care of directly by the capsule update logic. That
> said, I can drop the patch if you prefer just having the version which
> updates the capsule from memory.

"efidebug capsule" command, like other subcommands of efidebug,
is a tool mainly for test/debug purpose.
Keep the functionality in the command won't bother anyone
in that sense.

-Takahiro Akashi


> -sughosh
> 
> 
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > ---
> > >  cmd/efidebug.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> > > index 4a7661d0ac..fd8366dc90 100644
> > > --- a/cmd/efidebug.c
> > > +++ b/cmd/efidebug.c
> > > @@ -77,6 +77,16 @@ static int do_efi_capsule_update(cmd_tbl_t *cmdtp,
> > int flag,
> > >       return CMD_RET_SUCCESS;
> > >  }
> > >
> > > +static int do_efi_capsule_on_disk_update(cmd_tbl_t *cmdtp, int flag,
> > > +                                      int argc, char * const argv[])
> > > +{
> > > +     efi_status_t ret;
> > > +
> > > +     ret = efi_launch_capsules();
> > > +
> > > +     return ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> > > +}
> > > +
> > >  /**
> > >   * do_efi_capsule_show() - show capsule information
> > >   *
> > > @@ -205,6 +215,8 @@ static cmd_tbl_t cmd_efidebug_capsule_sub[] = {
> > >                        "", ""),
> > >       U_BOOT_CMD_MKENT(show, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_show,
> > >                        "", ""),
> > > +     U_BOOT_CMD_MKENT(disk-update, 0, 0, do_efi_capsule_on_disk_update,
> > > +                      "", ""),
> > >       U_BOOT_CMD_MKENT(result, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_res,
> > >                        "", ""),
> > >  };
> > > @@ -1387,6 +1399,8 @@ static char efidebug_help_text[] =
> > >  #ifdef CONFIG_EFI_CAPSULE_UPDATE
> > >       "efidebug capsule update [-v] <capsule address>\n"
> > >       "  - process a capsule\n"
> > > +     "efidebug capsule disk-update\n"
> > > +     "  - update a capsule from disk\n"
> > >       "efidebug capsule show <capsule address>\n"
> > >       "  - show capsule information\n"
> > >       "efidebug capsule result [<capsule result var>]\n"
> > >
> >
> >

Patch

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 4a7661d0ac..fd8366dc90 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -77,6 +77,16 @@  static int do_efi_capsule_update(cmd_tbl_t *cmdtp, int flag,
 	return CMD_RET_SUCCESS;
 }
 
+static int do_efi_capsule_on_disk_update(cmd_tbl_t *cmdtp, int flag,
+					 int argc, char * const argv[])
+{
+	efi_status_t ret;
+
+	ret = efi_launch_capsules();
+
+	return ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
+}
+
 /**
  * do_efi_capsule_show() - show capsule information
  *
@@ -205,6 +215,8 @@  static cmd_tbl_t cmd_efidebug_capsule_sub[] = {
 			 "", ""),
 	U_BOOT_CMD_MKENT(show, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_show,
 			 "", ""),
+	U_BOOT_CMD_MKENT(disk-update, 0, 0, do_efi_capsule_on_disk_update,
+			 "", ""),
 	U_BOOT_CMD_MKENT(result, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_res,
 			 "", ""),
 };
@@ -1387,6 +1399,8 @@  static char efidebug_help_text[] =
 #ifdef CONFIG_EFI_CAPSULE_UPDATE
 	"efidebug capsule update [-v] <capsule address>\n"
 	"  - process a capsule\n"
+	"efidebug capsule disk-update\n"
+	"  - update a capsule from disk\n"
 	"efidebug capsule show <capsule address>\n"
 	"  - show capsule information\n"
 	"efidebug capsule result [<capsule result var>]\n"