public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Michael D Kinney <michael.d.kinney@intel.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	Jiewen Yao <jiewen.yao@intel.com>
Subject: Re: [Patch 9/9] SignedCapsulePkg/SystemFirmwareUpdateDxe: Use progress API
Date: Fri, 6 Apr 2018 15:55:26 +0200	[thread overview]
Message-ID: <CAKv+Gu9xLn5WLrz=d3qhWuud3mKxoE6p=w1o_n9EhBewPKDWVg@mail.gmail.com> (raw)
In-Reply-To: <20180404202554.9568-10-michael.d.kinney@intel.com>

On 4 April 2018 at 22:25, Michael D Kinney <michael.d.kinney@intel.com> wrote:
> From: "Kinney, Michael D" <michael.d.kinney@intel.com>
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=801
>
> Use PlatformFlashWriteWithProgress() instead of PlatformFLashWrite()
> so the user can be informed of the progress as a capsule is used
> to update a firmware image in a firmware device.
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1

Doesn't this break existing platforms that do not implement this in
their PlatformFlashAccessLib?

> ---
>  .../SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c | 92 ++++++++++++++++------
>  1 file changed, 68 insertions(+), 24 deletions(-)
>
> diff --git a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
> index fd6641eb3e..8d2e0df81c 100644
> --- a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
> +++ b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
> @@ -8,7 +8,7 @@
>
>    FmpSetImage() will receive untrusted input and do basic validation.
>
> -  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD License
>    which accompanies this distribution.  The full text of the license may be found at
> @@ -65,11 +65,14 @@ ParseUpdateDataFile (
>  **/
>  EFI_STATUS
>  PerformUpdate (
> -  IN VOID                         *SystemFirmwareImage,
> -  IN UINTN                        SystemFirmwareImageSize,
> -  IN UPDATE_CONFIG_DATA           *ConfigData,
> -  OUT UINT32                      *LastAttemptVersion,
> -  OUT UINT32                      *LastAttemptStatus
> +  IN VOID                                           *SystemFirmwareImage,
> +  IN UINTN                                          SystemFirmwareImageSize,
> +  IN UPDATE_CONFIG_DATA                             *ConfigData,
> +  OUT UINT32                                        *LastAttemptVersion,
> +  OUT UINT32                                        *LastAttemptStatus,
> +  IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS  Progress,
> +  IN UINTN                                          StartPercentage,
> +  IN UINTN                                          EndPercentage
>    )
>  {
>    EFI_STATUS                   Status;
> @@ -78,12 +81,15 @@ PerformUpdate (
>    DEBUG((DEBUG_INFO, "  BaseAddress - 0x%lx,", ConfigData->BaseAddress));
>    DEBUG((DEBUG_INFO, "  ImageOffset - 0x%x,", ConfigData->ImageOffset));
>    DEBUG((DEBUG_INFO, "  Legnth - 0x%x\n", ConfigData->Length));
> -  Status = PerformFlashWrite (
> +  Status = PerformFlashWriteWithProgress (
>               ConfigData->FirmwareType,
>               ConfigData->BaseAddress,
>               ConfigData->AddressType,
>               (VOID *)((UINTN)SystemFirmwareImage + (UINTN)ConfigData->ImageOffset),
> -             ConfigData->Length
> +             ConfigData->Length,
> +             Progress,
> +             StartPercentage,
> +             EndPercentage
>               );
>    if (!EFI_ERROR(Status)) {
>      *LastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS;
> @@ -111,12 +117,13 @@ PerformUpdate (
>  **/
>  EFI_STATUS
>  UpdateImage (
> -  IN VOID                         *SystemFirmwareImage,
> -  IN UINTN                        SystemFirmwareImageSize,
> -  IN VOID                         *ConfigImage,
> -  IN UINTN                        ConfigImageSize,
> -  OUT UINT32                      *LastAttemptVersion,
> -  OUT UINT32                      *LastAttemptStatus
> +  IN VOID                                           *SystemFirmwareImage,
> +  IN UINTN                                          SystemFirmwareImageSize,
> +  IN VOID                                           *ConfigImage,
> +  IN UINTN                                          ConfigImageSize,
> +  OUT UINT32                                        *LastAttemptVersion,
> +  OUT UINT32                                        *LastAttemptStatus,
> +  IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS  Progress
>    )
>  {
>    EFI_STATUS                            Status;
> @@ -124,19 +131,34 @@ UpdateImage (
>    UPDATE_CONFIG_DATA                    *UpdateConfigData;
>    CONFIG_HEADER                         ConfigHeader;
>    UINTN                                 Index;
> +  UINTN                                 TotalSize;
> +  UINTN                                 BytesWritten;
> +  UINTN                                 StartPercentage;
> +  UINTN                                 EndPercentage;
>
>    if (ConfigImage == NULL) {
>      DEBUG((DEBUG_INFO, "PlatformUpdate (NoConfig):"));
>      DEBUG((DEBUG_INFO, "  BaseAddress - 0x%x,", 0));
>      DEBUG((DEBUG_INFO, "  Length - 0x%x\n", SystemFirmwareImageSize));
>      // ASSUME the whole System Firmware include NVRAM region.
> -    Status = PerformFlashWrite (
> +    StartPercentage = 0;
> +    EndPercentage = 100;
> +    if (Progress != NULL) {
> +      Progress (StartPercentage);
> +    }
> +    Status = PerformFlashWriteWithProgress (
>                 PlatformFirmwareTypeNvRam,
>                 0,
>                 FlashAddressTypeRelativeAddress,
>                 SystemFirmwareImage,
> -               SystemFirmwareImageSize
> +               SystemFirmwareImageSize,
> +               Progress,
> +               StartPercentage,
> +               EndPercentage
>                 );
> +    if (Progress != NULL) {
> +      Progress (EndPercentage);
> +    }
>      if (!EFI_ERROR(Status)) {
>        *LastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS;
>        mNvRamUpdated = TRUE;
> @@ -163,18 +185,37 @@ UpdateImage (
>    DEBUG((DEBUG_INFO, "ConfigHeader.NumOfUpdates - 0x%x\n", ConfigHeader.NumOfUpdates));
>    DEBUG((DEBUG_INFO, "PcdEdkiiSystemFirmwareFileGuid - %g\n", PcdGetPtr(PcdEdkiiSystemFirmwareFileGuid)));
>
> +  TotalSize = 0;
> +  for (Index = 0; Index < ConfigHeader.NumOfUpdates; Index++) {
> +    if (CompareGuid(&ConfigData[Index].FileGuid, PcdGetPtr(PcdEdkiiSystemFirmwareFileGuid))) {
> +      TotalSize = TotalSize + ConfigData[Index].Length;
> +    }
> +  }
> +
> +  BytesWritten = 0;
>    Index = 0;
>    UpdateConfigData = ConfigData;
>    while (Index < ConfigHeader.NumOfUpdates) {
>      if (CompareGuid(&UpdateConfigData->FileGuid, PcdGetPtr(PcdEdkiiSystemFirmwareFileGuid))) {
>        DEBUG((DEBUG_INFO, "FileGuid - %g (processing)\n", &UpdateConfigData->FileGuid));
> +      StartPercentage = (BytesWritten * 100) / TotalSize;
> +      EndPercentage   = ((BytesWritten + UpdateConfigData->Length) * 100) / TotalSize;
> +      if (Progress != NULL) {
> +        Progress (StartPercentage);
> +      }
>        Status = PerformUpdate (
>                   SystemFirmwareImage,
>                   SystemFirmwareImageSize,
>                   UpdateConfigData,
>                   LastAttemptVersion,
> -                 LastAttemptStatus
> +                 LastAttemptStatus,
> +                 Progress,
> +                 StartPercentage,
> +                 EndPercentage
>                   );
> +      if (Progress != NULL) {
> +        Progress (EndPercentage);
> +      }
>        //
>        // Shall updates be serialized so that if an update is not successfully completed,
>        // the remaining updates won't be performed.
> @@ -186,6 +227,8 @@ UpdateImage (
>        DEBUG((DEBUG_INFO, "FileGuid - %g (ignored)\n", &UpdateConfigData->FileGuid));
>      }
>
> +    BytesWritten += UpdateConfigData->Length;
> +
>      Index++;
>      UpdateConfigData++;
>    }
> @@ -209,10 +252,11 @@ UpdateImage (
>  **/
>  EFI_STATUS
>  SystemFirmwareAuthenticatedUpdate (
> -  IN VOID                         *Image,
> -  IN UINTN                        ImageSize,
> -  OUT UINT32                      *LastAttemptVersion,
> -  OUT UINT32                      *LastAttemptStatus
> +  IN VOID                                           *Image,
> +  IN UINTN                                          ImageSize,
> +  OUT UINT32                                        *LastAttemptVersion,
> +  OUT UINT32                                        *LastAttemptStatus,
> +  IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS  Progress
>    )
>  {
>    EFI_STATUS                  Status;
> @@ -240,7 +284,7 @@ SystemFirmwareAuthenticatedUpdate (
>    ExtractConfigImage(AuthenticatedImage, AuthenticatedImageSize, &ConfigImage, &ConfigImageSize);
>
>    DEBUG((DEBUG_INFO, "UpdateImage ...\n"));
> -  Status = UpdateImage(SystemFirmwareImage, SystemFirmwareImageSize, ConfigImage, ConfigImageSize, LastAttemptVersion, LastAttemptStatus);
> +  Status = UpdateImage(SystemFirmwareImage, SystemFirmwareImageSize, ConfigImage, ConfigImageSize, LastAttemptVersion, LastAttemptStatus, Progress);
>    if (EFI_ERROR(Status)) {
>      DEBUG((DEBUG_INFO, "UpdateImage - %r\n", Status));
>      return Status;
> @@ -442,8 +486,8 @@ FmpSetImage (
>      return EFI_INVALID_PARAMETER;
>    }
>
> -  Status = SystemFirmwareAuthenticatedUpdate((VOID *)Image, ImageSize, &SystemFmpPrivate->LastAttempt.LastAttemptVersion, &SystemFmpPrivate->LastAttempt.LastAttemptStatus);
> -  DEBUG((DEBUG_INFO, "SetImage - LastAttemp Version - 0x%x, State - 0x%x\n", SystemFmpPrivate->LastAttempt.LastAttemptVersion, SystemFmpPrivate->LastAttempt.LastAttemptStatus));
> +  Status = SystemFirmwareAuthenticatedUpdate((VOID *)Image, ImageSize, &SystemFmpPrivate->LastAttempt.LastAttemptVersion, &SystemFmpPrivate->LastAttempt.LastAttemptStatus, Progress);
> +  DEBUG((DEBUG_INFO, "SetImage - LastAttempt Version - 0x%x, State - 0x%x\n", SystemFmpPrivate->LastAttempt.LastAttemptVersion, SystemFmpPrivate->LastAttempt.LastAttemptStatus));
>
>    //
>    // If NVRAM is updated, we should no longer touch variable services, because
> --
> 2.14.2.windows.3
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2018-04-06 13:55 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-04 20:25 [Patch 0/9] Add DisplayUpdateProgressLib for capsules Michael D Kinney
2018-04-04 20:25 ` [Patch 1/9] MdeModulePkg: Add DisplayUpdateProgressLib class Michael D Kinney
2018-04-04 20:25 ` [Patch 2/9] MdeModulePkg: Add DisplayUpdateProgressLib instances Michael D Kinney
2018-04-04 20:25 ` [Patch 3/9] Vlv2Tbl2DevicePkg: Add DisplayUpdateProgressLib mapping Michael D Kinney
2018-04-08  6:12   ` Wei, David
2018-04-04 20:25 ` [Patch 4/9] QuarkPlatformPkg: " Michael D Kinney
2018-04-04 20:25 ` [Patch 5/9] MdeModulePkg/DxeCapsuleLibFmp: Add progress bar support Michael D Kinney
2018-04-04 20:25 ` [Patch 6/9] SignedCapsulePkg/PlatformFlashAccessLib: Add progress API Michael D Kinney
2018-04-04 20:25 ` [Patch 7/9] Vlv2TbltDevicePkg/PlatformFlashAccessLib: " Michael D Kinney
2018-04-08  6:11   ` Wei, David
2018-04-04 20:25 ` [Patch 8/9] QuarkPlatformPkg/PlatformFlashAccessLib: " Michael D Kinney
2018-04-04 20:25 ` [Patch 9/9] SignedCapsulePkg/SystemFirmwareUpdateDxe: Use " Michael D Kinney
2018-04-06 13:55   ` Ard Biesheuvel [this message]
2018-04-06 15:29     ` Kinney, Michael D
2018-04-04 20:29 ` [Patch 0/9] Add DisplayUpdateProgressLib for capsules Kinney, Michael D
2018-04-05  0:02 ` Yao, Jiewen
2018-04-05  0:37   ` Kinney, Michael D

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKv+Gu9xLn5WLrz=d3qhWuud3mKxoE6p=w1o_n9EhBewPKDWVg@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox