| Message ID | 20220412130447.300574-9-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | efi: capsule: Capsule Update fixes and enhancements | expand |
On Tue, Apr 12, 2022 at 06:34:47PM +0530, Sughosh Ganu wrote: > Update the capsule update functionality related documentation to > refect the additional definitions that need to be made per platform > for supporting the capsule update feature. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > > Changes since V5: None > > doc/develop/uefi/uefi.rst | 51 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 49 insertions(+), 2 deletions(-) > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst > index fe337c88bd..b766aecf67 100644 > --- a/doc/develop/uefi/uefi.rst > +++ b/doc/develop/uefi/uefi.rst > @@ -312,8 +312,8 @@ Run the following command > .. code-block:: console > > $ mkeficapsule \ > - --index 1 --instance 0 \ > - [--fit <FIT image> | --raw <raw image>] \ > + --index <index> --instance 0 \ > + --guid <image GUID> \ > <capsule_file_name> > > Performing the update > @@ -333,6 +333,53 @@ won't be taken over across the reboot. If this is the case, you can skip > this feature check with the Kconfig option (CONFIG_EFI_IGNORE_OSINDICATIONS) > set. > > +A few values need to be defined in the board file for performing the > +capsule update. These values are defined in the board file by > +initialisation of a structure which provides information needed for > +capsule updates. The following structures have been defined for > +containing the image related information > + > +.. code-block:: c > + > + struct efi_fw_images { Why "images" (in the plural)? > + efi_guid_t image_type_id; > + u16 *fw_name; > + u8 image_index; > + }; Why not add "version" and "last_attempt_version" which is expected to be easily implemented in this structure. > + struct efi_capsule_update_info { > + const char *dfu_string; > + struct efi_fw_images *images; > + }; > + > + > +A string is defined which is to be used for populating the > +dfu_alt_info variable. This string is used by the function > +set_dfu_alt_info. Instead of taking the variable from the environment, > +the capsule update feature requires that the variable be set through > +the function, since that is more robust. Allowing the user to change > +the location of the firmware updates is not a very secure > +practice. Getting this information from the firmware itself is more > +secure, assuming the firmware has been verified by a previous stage > +boot loader. > + > +The firmware images structure defines the GUID values, image index > +values and the name of the images that are to be updated through > +the capsule update feature. These values are to be defined as part of > +an array. These GUID values would be used by the Firmware Management > +Protocol(FMP) to populate the image descriptor array and also > +displayed as part of the ESRT table. The image index values defined in > +the array should be one greater than the dfu alt number that > +corresponds to the firmware image. So, if the dfu alt number for an > +image is 2, the value of image index in the fw_images array for that > +image should be 3. The dfu alt number can be obtained by running the > +following command:: > + > + dfu list > + > +When using the FMP for FIT images, the image index value needs to be > +set to 1. The explanation would be correct, but it's not quite easy to understand, in particular, index in case of raw. You should add some examples here. -Takahiro Akashi > + > Finally, the capsule update can be initiated by rebooting the board. > > Enabling Capsule Authentication > -- > 2.25.1 >
On Wed, 13 Apr 2022 at 11:48, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > On Tue, Apr 12, 2022 at 06:34:47PM +0530, Sughosh Ganu wrote: > > Update the capsule update functionality related documentation to > > refect the additional definitions that need to be made per platform > > for supporting the capsule update feature. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > > > Changes since V5: None > > > > doc/develop/uefi/uefi.rst | 51 +++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 49 insertions(+), 2 deletions(-) > > > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst > > index fe337c88bd..b766aecf67 100644 > > --- a/doc/develop/uefi/uefi.rst > > +++ b/doc/develop/uefi/uefi.rst > > @@ -312,8 +312,8 @@ Run the following command > > .. code-block:: console > > > > $ mkeficapsule \ > > - --index 1 --instance 0 \ > > - [--fit <FIT image> | --raw <raw image>] \ > > + --index <index> --instance 0 \ > > + --guid <image GUID> \ > > <capsule_file_name> > > > > Performing the update > > @@ -333,6 +333,53 @@ won't be taken over across the reboot. If this is the case, you can skip > > this feature check with the Kconfig option (CONFIG_EFI_IGNORE_OSINDICATIONS) > > set. > > > > +A few values need to be defined in the board file for performing the > > +capsule update. These values are defined in the board file by > > +initialisation of a structure which provides information needed for > > +capsule updates. The following structures have been defined for > > +containing the image related information > > + > > +.. code-block:: c > > + > > + struct efi_fw_images { > > Why "images" (in the plural)? Hmm, since this is to be an array of firmware images which should be handled by the capsule update code, I used a plural form. Do you prefer efi_fw_image instead? > > > + efi_guid_t image_type_id; > > + u16 *fw_name; > > + u8 image_index; > > + }; > > Why not add "version" and "last_attempt_version" which is expected > to be easily implemented in this structure. It can be added to this structure, yes. But we will also need to add code in the capsule driver to update these fields accordingly. I can take this up as a follow up task once the FWU series upstreaming is done. > > > > + struct efi_capsule_update_info { > > + const char *dfu_string; > > + struct efi_fw_images *images; > > + }; > > + > > + > > +A string is defined which is to be used for populating the > > +dfu_alt_info variable. This string is used by the function > > +set_dfu_alt_info. Instead of taking the variable from the environment, > > +the capsule update feature requires that the variable be set through > > +the function, since that is more robust. Allowing the user to change > > +the location of the firmware updates is not a very secure > > +practice. Getting this information from the firmware itself is more > > +secure, assuming the firmware has been verified by a previous stage > > +boot loader. > > + > > +The firmware images structure defines the GUID values, image index > > +values and the name of the images that are to be updated through > > +the capsule update feature. These values are to be defined as part of > > +an array. These GUID values would be used by the Firmware Management > > +Protocol(FMP) to populate the image descriptor array and also > > +displayed as part of the ESRT table. The image index values defined in > > +the array should be one greater than the dfu alt number that > > +corresponds to the firmware image. So, if the dfu alt number for an > > +image is 2, the value of image index in the fw_images array for that > > +image should be 3. The dfu alt number can be obtained by running the > > +following command:: > > + > > + dfu list > > + > > +When using the FMP for FIT images, the image index value needs to be > > +set to 1. > > The explanation would be correct, but it's not quite easy to understand, > in particular, index in case of raw. > You should add some examples here. I have added an example above for the raw images, as to how the image index corresponds with the dfu alt number. Does it not suffice? -sughosh > > -Takahiro Akashi > > > + > > Finally, the capsule update can be initiated by rebooting the board. > > > > Enabling Capsule Authentication > > -- > > 2.25.1 > >
On Wed, Apr 13, 2022 at 12:38:05PM +0530, Sughosh Ganu wrote: > On Wed, 13 Apr 2022 at 11:48, AKASHI Takahiro > <takahiro.akashi@linaro.org> wrote: > > > > On Tue, Apr 12, 2022 at 06:34:47PM +0530, Sughosh Ganu wrote: > > > Update the capsule update functionality related documentation to > > > refect the additional definitions that need to be made per platform > > > for supporting the capsule update feature. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > --- > > > > > > Changes since V5: None > > > > > > doc/develop/uefi/uefi.rst | 51 +++++++++++++++++++++++++++++++++++++-- > > > 1 file changed, 49 insertions(+), 2 deletions(-) > > > > > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst > > > index fe337c88bd..b766aecf67 100644 > > > --- a/doc/develop/uefi/uefi.rst > > > +++ b/doc/develop/uefi/uefi.rst > > > @@ -312,8 +312,8 @@ Run the following command > > > .. code-block:: console > > > > > > $ mkeficapsule \ > > > - --index 1 --instance 0 \ > > > - [--fit <FIT image> | --raw <raw image>] \ > > > + --index <index> --instance 0 \ > > > + --guid <image GUID> \ > > > <capsule_file_name> > > > > > > Performing the update > > > @@ -333,6 +333,53 @@ won't be taken over across the reboot. If this is the case, you can skip > > > this feature check with the Kconfig option (CONFIG_EFI_IGNORE_OSINDICATIONS) > > > set. > > > > > > +A few values need to be defined in the board file for performing the > > > +capsule update. These values are defined in the board file by > > > +initialisation of a structure which provides information needed for > > > +capsule updates. The following structures have been defined for > > > +containing the image related information > > > + > > > +.. code-block:: c > > > + > > > + struct efi_fw_images { > > > > Why "images" (in the plural)? > > Hmm, since this is to be an array of firmware images which should be > handled by the capsule update code, I used a plural form. Do you > prefer efi_fw_image instead? This structure (not a variable like struct efi_fw_image foo_images[]) can only hold/represent one firmware image, right? If so, efi_fw_image looks better. > > > > > + efi_guid_t image_type_id; > > > + u16 *fw_name; > > > + u8 image_index; > > > + }; > > > > Why not add "version" and "last_attempt_version" which is expected > > to be easily implemented in this structure. > > It can be added to this structure, yes. But we will also need to add > code in the capsule driver to update these fields accordingly. I can > take this up as a follow up task once the FWU series upstreaming is > done. > > > > > > > > + struct efi_capsule_update_info { > > > + const char *dfu_string; > > > + struct efi_fw_images *images; > > > + }; > > > + > > > + > > > +A string is defined which is to be used for populating the > > > +dfu_alt_info variable. This string is used by the function > > > +set_dfu_alt_info. Instead of taking the variable from the environment, > > > +the capsule update feature requires that the variable be set through > > > +the function, since that is more robust. Allowing the user to change > > > +the location of the firmware updates is not a very secure > > > +practice. Getting this information from the firmware itself is more > > > +secure, assuming the firmware has been verified by a previous stage > > > +boot loader. > > > + > > > +The firmware images structure defines the GUID values, image index > > > +values and the name of the images that are to be updated through > > > +the capsule update feature. These values are to be defined as part of > > > +an array. These GUID values would be used by the Firmware Management > > > +Protocol(FMP) to populate the image descriptor array and also > > > +displayed as part of the ESRT table. The image index values defined in > > > +the array should be one greater than the dfu alt number that > > > +corresponds to the firmware image. So, if the dfu alt number for an > > > +image is 2, the value of image index in the fw_images array for that > > > +image should be 3. The dfu alt number can be obtained by running the > > > +following command:: > > > + > > > + dfu list > > > + > > > +When using the FMP for FIT images, the image index value needs to be > > > +set to 1. > > > > The explanation would be correct, but it's not quite easy to understand, > > in particular, index in case of raw. > > You should add some examples here. > > I have added an example above for the raw images, as to how the image > index corresponds with the dfu alt number. Does it not suffice? I hope that it will cover not only index, but example arrays of struct efi_fw_image and struct efi_capsule_update_info as well as an example "dfu_alto_info". -Takahiro Akashi > -sughosh > > > > > -Takahiro Akashi > > > > > + > > > Finally, the capsule update can be initiated by rebooting the board. > > > > > > Enabling Capsule Authentication > > > -- > > > 2.25.1 > > >
Hi 2022年4月13日(水) 16:33 AKASHI Takahiro <takahiro.akashi@linaro.org>: > > On Wed, Apr 13, 2022 at 12:38:05PM +0530, Sughosh Ganu wrote: > > On Wed, 13 Apr 2022 at 11:48, AKASHI Takahiro > > <takahiro.akashi@linaro.org> wrote: > > > > > > On Tue, Apr 12, 2022 at 06:34:47PM +0530, Sughosh Ganu wrote: > > > > Update the capsule update functionality related documentation to > > > > refect the additional definitions that need to be made per platform > > > > for supporting the capsule update feature. > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > --- > > > > > > > > Changes since V5: None > > > > > > > > doc/develop/uefi/uefi.rst | 51 +++++++++++++++++++++++++++++++++++++-- > > > > 1 file changed, 49 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst > > > > index fe337c88bd..b766aecf67 100644 > > > > --- a/doc/develop/uefi/uefi.rst > > > > +++ b/doc/develop/uefi/uefi.rst > > > > @@ -312,8 +312,8 @@ Run the following command > > > > .. code-block:: console > > > > > > > > $ mkeficapsule \ > > > > - --index 1 --instance 0 \ > > > > - [--fit <FIT image> | --raw <raw image>] \ > > > > + --index <index> --instance 0 \ > > > > + --guid <image GUID> \ > > > > <capsule_file_name> > > > > > > > > Performing the update > > > > @@ -333,6 +333,53 @@ won't be taken over across the reboot. If this is the case, you can skip > > > > this feature check with the Kconfig option (CONFIG_EFI_IGNORE_OSINDICATIONS) > > > > set. > > > > > > > > +A few values need to be defined in the board file for performing the > > > > +capsule update. These values are defined in the board file by > > > > +initialisation of a structure which provides information needed for > > > > +capsule updates. The following structures have been defined for > > > > +containing the image related information > > > > + > > > > +.. code-block:: c > > > > + > > > > + struct efi_fw_images { > > > > > > Why "images" (in the plural)? > > > > Hmm, since this is to be an array of firmware images which should be > > handled by the capsule update code, I used a plural form. Do you > > prefer efi_fw_image instead? > > This structure (not a variable like struct efi_fw_image foo_images[]) > can only hold/represent one firmware image, right? > If so, efi_fw_image looks better. > > > > > > > > + efi_guid_t image_type_id; > > > > + u16 *fw_name; > > > > + u8 image_index; > > > > + }; > > > > > > Why not add "version" and "last_attempt_version" which is expected > > > to be easily implemented in this structure. > > > > It can be added to this structure, yes. But we will also need to add > > code in the capsule driver to update these fields accordingly. I can > > take this up as a follow up task once the FWU series upstreaming is > > done. > > > > > > > > > > > > + struct efi_capsule_update_info { > > > > + const char *dfu_string; > > > > + struct efi_fw_images *images; > > > > + }; > > > > + > > > > + > > > > +A string is defined which is to be used for populating the > > > > +dfu_alt_info variable. This string is used by the function > > > > +set_dfu_alt_info. Instead of taking the variable from the environment, > > > > +the capsule update feature requires that the variable be set through > > > > +the function, since that is more robust. Allowing the user to change > > > > +the location of the firmware updates is not a very secure > > > > +practice. Getting this information from the firmware itself is more > > > > +secure, assuming the firmware has been verified by a previous stage > > > > +boot loader. > > > > + > > > > +The firmware images structure defines the GUID values, image index > > > > +values and the name of the images that are to be updated through > > > > +the capsule update feature. These values are to be defined as part of > > > > +an array. These GUID values would be used by the Firmware Management > > > > +Protocol(FMP) to populate the image descriptor array and also > > > > +displayed as part of the ESRT table. The image index values defined in > > > > +the array should be one greater than the dfu alt number that > > > > +corresponds to the firmware image. So, if the dfu alt number for an > > > > +image is 2, the value of image index in the fw_images array for that > > > > +image should be 3. The dfu alt number can be obtained by running the > > > > +following command:: > > > > + > > > > + dfu list > > > > + > > > > +When using the FMP for FIT images, the image index value needs to be > > > > +set to 1. > > > > > > The explanation would be correct, but it's not quite easy to understand, > > > in particular, index in case of raw. > > > You should add some examples here. > > > > I have added an example above for the raw images, as to how the image > > index corresponds with the dfu alt number. Does it not suffice? > > I hope that it will cover not only index, but example arrays of > struct efi_fw_image and struct efi_capsule_update_info > as well as an example "dfu_alto_info". +1. If there is an example of "dfu_alt_info", the platform setting of image-type and index list, and how to make a capsule file in that case, people can easily understand the relationship of them. Can we use Qemu case for example? Thank you, > > -Takahiro Akashi > > > -sughosh > > > > > > > > -Takahiro Akashi > > > > > > > + > > > > Finally, the capsule update can be initiated by rebooting the board. > > > > > > > > Enabling Capsule Authentication > > > > -- > > > > 2.25.1 > > > >
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index fe337c88bd..b766aecf67 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -312,8 +312,8 @@ Run the following command .. code-block:: console $ mkeficapsule \ - --index 1 --instance 0 \ - [--fit <FIT image> | --raw <raw image>] \ + --index <index> --instance 0 \ + --guid <image GUID> \ <capsule_file_name> Performing the update @@ -333,6 +333,53 @@ won't be taken over across the reboot. If this is the case, you can skip this feature check with the Kconfig option (CONFIG_EFI_IGNORE_OSINDICATIONS) set. +A few values need to be defined in the board file for performing the +capsule update. These values are defined in the board file by +initialisation of a structure which provides information needed for +capsule updates. The following structures have been defined for +containing the image related information + +.. code-block:: c + + struct efi_fw_images { + efi_guid_t image_type_id; + u16 *fw_name; + u8 image_index; + }; + + struct efi_capsule_update_info { + const char *dfu_string; + struct efi_fw_images *images; + }; + + +A string is defined which is to be used for populating the +dfu_alt_info variable. This string is used by the function +set_dfu_alt_info. Instead of taking the variable from the environment, +the capsule update feature requires that the variable be set through +the function, since that is more robust. Allowing the user to change +the location of the firmware updates is not a very secure +practice. Getting this information from the firmware itself is more +secure, assuming the firmware has been verified by a previous stage +boot loader. + +The firmware images structure defines the GUID values, image index +values and the name of the images that are to be updated through +the capsule update feature. These values are to be defined as part of +an array. These GUID values would be used by the Firmware Management +Protocol(FMP) to populate the image descriptor array and also +displayed as part of the ESRT table. The image index values defined in +the array should be one greater than the dfu alt number that +corresponds to the firmware image. So, if the dfu alt number for an +image is 2, the value of image index in the fw_images array for that +image should be 3. The dfu alt number can be obtained by running the +following command:: + + dfu list + +When using the FMP for FIT images, the image index value needs to be +set to 1. + Finally, the capsule update can be initiated by rebooting the board. Enabling Capsule Authentication
Update the capsule update functionality related documentation to refect the additional definitions that need to be made per platform for supporting the capsule update feature. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V5: None doc/develop/uefi/uefi.rst | 51 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-)