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>,
Eric Dong <eric.dong@intel.com>, Star Zeng <star.zeng@intel.com>
Subject: Re: [Patch v3 1/2] MdeModulePkg/DxeCapsuleLibFmp: Add progress bar support
Date: Wed, 6 Jun 2018 19:34:27 +0200 [thread overview]
Message-ID: <CAKv+Gu9d2A93_9nDY7eXKanv3pJWCdDfLScznFtzEvHQ-GNQWg@mail.gmail.com> (raw)
In-Reply-To: <20180529161755.9424-2-michael.d.kinney@intel.com>
On 29 May 2018 at 18:17, 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
>
> Based on content from the following branch/commits:
> https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport
>
> * Change Update_Image_Progress() to UpdateImageProcess()
> * Call DisplayUpdateProgressLib from UpdateImageProgress().
> * Split out a boot service and runtime version of
> UpdateImageProgress() so the DisplayUpdateProgressLib is
> not used at runtime.
> * If gEdkiiFirmwareManagementProgressProtocolGuid is present,
> then use its progress bar color and watchdog timer value.
> * If gEdkiiFirmwareManagementProgressProtocolGuid is not present,
> then use default progress bar color and 5 min watchdog timer.
> * Remove Print() calls during capsule processing. Instead,
> the DisplayUpdateProgressLib is used to inform the user
> of progress during a capsule update.
>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Please don't push this before the edk2-platforms changes are in.
> ---
> .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | 47 +++++++++---
> .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf | 8 ++-
> .../DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 84 ++++++++++++++++------
> .../DxeCapsuleLibFmp/DxeCapsuleProcessLibNull.c | 21 +++++-
> .../DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf | 7 +-
> 5 files changed, 131 insertions(+), 36 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> index 05fcd92deb..f0226eafa5 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> @@ -45,6 +45,7 @@
> #include <Protocol/GraphicsOutput.h>
> #include <Protocol/EsrtManagement.h>
> #include <Protocol/FirmwareManagement.h>
> +#include <Protocol/FirmwareManagementProgress.h>
> #include <Protocol/DevicePath.h>
>
> EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable = NULL;
> @@ -53,6 +54,8 @@ BOOLEAN mIsVirtualAddrConverted = FALSE;
> BOOLEAN mDxeCapsuleLibEndOfDxe = FALSE;
> EFI_EVENT mDxeCapsuleLibEndOfDxeEvent = NULL;
>
> +EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL *mFmpProgress = NULL;
> +
> /**
> Initialize capsule related variables.
> **/
> @@ -101,18 +104,17 @@ RecordFmpCapsuleStatusVariable (
> Function indicate the current completion progress of the firmware
> update. Platform may override with own specific progress function.
>
> - @param[in] Completion A value between 1 and 100 indicating the current completion progress of the firmware update
> + @param[in] Completion A value between 1 and 100 indicating the current
> + completion progress of the firmware update
>
> - @retval EFI_SUCESS Input capsule is a correct FMP capsule.
> + @retval EFI_SUCESS The capsule update progress was updated.
> + @retval EFI_INVALID_PARAMETER Completion is greater than 100%.
> **/
> EFI_STATUS
> EFIAPI
> -Update_Image_Progress (
> +UpdateImageProgress (
> IN UINTN Completion
> - )
> -{
> - return EFI_SUCCESS;
> -}
> + );
>
> /**
> Return if this CapsuleGuid is a FMP capsule GUID or not.
> @@ -849,6 +851,19 @@ SetFmpImageData (
> return Status;
> }
>
> + //
> + // Lookup Firmware Management Progress Protocol before SetImage() is called
> + // This is an optional protocol that may not be present on Handle.
> + //
> + Status = gBS->HandleProtocol (
> + Handle,
> + &gEdkiiFirmwareManagementProgressProtocolGuid,
> + (VOID **)&mFmpProgress
> + );
> + if (EFI_ERROR (Status)) {
> + mFmpProgress = NULL;
> + }
> +
> if (ImageHeader->Version >= EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION) {
> Image = (UINT8 *)(ImageHeader + 1);
> } else {
> @@ -873,21 +888,37 @@ SetFmpImageData (
> DEBUG((DEBUG_INFO, "(UpdateHardwareInstance - 0x%x)", ImageHeader->UpdateHardwareInstance));
> }
> DEBUG((DEBUG_INFO, "\n"));
> +
> + //
> + // Before calling SetImage(), reset the progress bar to 0%
> + //
> + UpdateImageProgress (0);
> +
> Status = Fmp->SetImage(
> Fmp,
> ImageHeader->UpdateImageIndex, // ImageIndex
> Image, // Image
> ImageHeader->UpdateImageSize, // ImageSize
> VendorCode, // VendorCode
> - Update_Image_Progress, // Progress
> + UpdateImageProgress, // Progress
> &AbortReason // AbortReason
> );
> + //
> + // Set the progress bar to 100% after returning from SetImage()
> + //
> + UpdateImageProgress (100);
> +
> DEBUG((DEBUG_INFO, "Fmp->SetImage - %r\n", Status));
> if (AbortReason != NULL) {
> DEBUG ((DEBUG_ERROR, "%s\n", AbortReason));
> FreePool(AbortReason);
> }
>
> + //
> + // Clear mFmpProgress after SetImage() returns
> + //
> + mFmpProgress = NULL;
> +
> return Status;
> }
>
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> index 1d947101d3..8367264f76 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> @@ -52,6 +52,7 @@ [LibraryClasses]
> PrintLib
> HobLib
> BmpSupportLib
> + DisplayUpdateProgressLib
>
> [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleMax ## CONSUMES
> @@ -66,9 +67,10 @@ [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleStatusCodeResettingSystem ## CONSUMES
>
> [Protocols]
> - gEsrtManagementProtocolGuid ## CONSUMES
> - gEfiFirmwareManagementProtocolGuid ## CONSUMES
> - gEdkiiVariableLockProtocolGuid ## SOMETIMES_CONSUMES
> + gEsrtManagementProtocolGuid ## CONSUMES
> + gEfiFirmwareManagementProtocolGuid ## CONSUMES
> + gEdkiiVariableLockProtocolGuid ## SOMETIMES_CONSUMES
> + gEdkiiFirmwareManagementProgressProtocolGuid ## SOMETIMES_CONSUMES
>
> [Guids]
> gEfiFmpCapsuleGuid ## SOMETIMES_CONSUMES ## GUID
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
> index ba3ff90b9f..26ca4e295f 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
> @@ -9,7 +9,7 @@
> ProcessCapsules(), ProcessTheseCapsules() 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
> @@ -22,6 +22,7 @@
>
> #include <PiDxe.h>
> #include <Protocol/EsrtManagement.h>
> +#include <Protocol/FirmwareManagementProgress.h>
>
> #include <Library/BaseLib.h>
> #include <Library/DebugLib.h>
> @@ -34,9 +35,12 @@
> #include <Library/HobLib.h>
> #include <Library/ReportStatusCodeLib.h>
> #include <Library/CapsuleLib.h>
> +#include <Library/DisplayUpdateProgressLib.h>
>
> #include <IndustryStandard/WindowsUxCapsule.h>
>
> +extern EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL *mFmpProgress;
> +
> /**
> Return if this FMP is a system FMP or a device FMP, based upon CapsuleHeader.
>
> @@ -101,6 +105,62 @@ VOID **mCapsulePtr;
> EFI_STATUS *mCapsuleStatusArray;
> UINT32 mCapsuleTotalNumber;
>
> +/**
> + Function indicate the current completion progress of the firmware
> + update. Platform may override with own specific progress function.
> +
> + @param[in] Completion A value between 1 and 100 indicating the current
> + completion progress of the firmware update
> +
> + @retval EFI_SUCESS The capsule update progress was updated.
> + @retval EFI_INVALID_PARAMETER Completion is greater than 100%.
> +**/
> +EFI_STATUS
> +EFIAPI
> +UpdateImageProgress (
> + IN UINTN Completion
> + )
> +{
> + EFI_STATUS Status;
> + UINTN Seconds;
> + EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION *Color;
> +
> + DEBUG((DEBUG_INFO, "Update Progress - %d%%\n", Completion));
> +
> + if (Completion > 100) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + //
> + // Use a default timeout of 5 minutes if there is not FMP Progress Protocol.
> + //
> + Seconds = 5 * 60;
> + Color = NULL;
> + if (mFmpProgress != NULL) {
> + Seconds = mFmpProgress->WatchdogSeconds;
> + Color = &mFmpProgress->ProgressBarForegroundColor;
> + }
> +
> + //
> + // Cancel the watchdog timer
> + //
> + gBS->SetWatchdogTimer (0, 0x0000, 0, NULL);
> +
> + if (Completion != 100) {
> + //
> + // Arm the watchdog timer from PCD setting
> + //
> + if (Seconds != 0) {
> + DEBUG ((DEBUG_VERBOSE, "Arm watchdog timer %d seconds\n", Seconds));
> + gBS->SetWatchdogTimer (Seconds, 0x0000, 0, NULL);
> + }
> + }
> +
> + Status = DisplayUpdateProgress (Completion, Color);
> +
> + return Status;
> +}
> +
> /**
> This function initializes the mCapsulePtr, mCapsuleStatusArray and mCapsuleTotalNumber.
> **/
> @@ -319,7 +379,6 @@ ProcessTheseCapsules (
> EFI_STATUS Status;
> EFI_CAPSULE_HEADER *CapsuleHeader;
> UINT32 Index;
> - BOOLEAN DisplayCapsuleExist;
> ESRT_MANAGEMENT_PROTOCOL *EsrtManagement;
> UINT16 EmbeddedDriverCount;
>
> @@ -354,12 +413,10 @@ ProcessTheseCapsules (
> //
> // If Windows UX capsule exist, process it first
> //
> - DisplayCapsuleExist = FALSE;
> for (Index = 0; Index < mCapsuleTotalNumber; Index++) {
> CapsuleHeader = (EFI_CAPSULE_HEADER*) mCapsulePtr [Index];
> if (CompareGuid (&CapsuleHeader->CapsuleGuid, &gWindowsUxCapsuleGuid)) {
> DEBUG ((DEBUG_INFO, "ProcessCapsuleImage (Ux) - 0x%x\n", CapsuleHeader));
> - DisplayCapsuleExist = TRUE;
> DEBUG ((DEBUG_INFO, "Display logo capsule is found.\n"));
> Status = ProcessCapsuleImage (CapsuleHeader);
> mCapsuleStatusArray [Index] = EFI_SUCCESS;
> @@ -368,12 +425,7 @@ ProcessTheseCapsules (
> }
> }
>
> - if (!DisplayCapsuleExist) {
> - //
> - // Display Capsule not found. Display the default string.
> - //
> - Print (L"Updating the firmware ......\r\n");
> - }
> + DEBUG ((DEBUG_INFO, "Updating the firmware ......\n"));
>
> //
> // All capsules left are recognized by platform.
> @@ -411,7 +463,6 @@ ProcessTheseCapsules (
> if (EFI_ERROR(Status)) {
> REPORT_STATUS_CODE(EFI_ERROR_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeUpdateFirmwareFailed)));
> DEBUG ((DEBUG_ERROR, "Capsule process failed!\n"));
> - Print (L"Firmware update failed...\r\n");
> } else {
> REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeUpdateFirmwareSuccess)));
> }
> @@ -447,18 +498,9 @@ DoResetSystem (
> VOID
> )
> {
> - UINTN Index;
> -
> - REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeResettingSystem)));
> -
> - Print(L"Capsule Request Cold Reboot.\n");
> DEBUG((DEBUG_INFO, "Capsule Request Cold Reboot."));
>
> - for (Index = 5; Index > 0; Index--) {
> - Print(L"\rResetting system in %d seconds ...", Index);
> - DEBUG((DEBUG_INFO, "\rResetting system in %d seconds ...", Index));
> - gBS->Stall(1000000);
> - }
> + REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeResettingSystem)));
>
> gRT->ResetSystem(EfiResetCold, EFI_SUCCESS, 0, NULL);
>
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLibNull.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLibNull.c
> index 07e9e46eae..274c1c4c1c 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLibNull.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLibNull.c
> @@ -3,7 +3,7 @@
> Dummy function for runtime module, because CapsuleDxeRuntime
> does not need call ProcessCapsules().
>
> - 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
> @@ -17,6 +17,25 @@
> #include <PiDxe.h>
> #include <Library/CapsuleLib.h>
>
> +/**
> + Function indicate the current completion progress of the firmware
> + update. Platform may override with own specific progress function.
> +
> + @param[in] Completion A value between 1 and 100 indicating the current
> + completion progress of the firmware update
> +
> + @retval EFI_SUCESS The capsule update progress was updated.
> + @retval EFI_INVALID_PARAMETER Completion is greater than 100%.
> +**/
> +EFI_STATUS
> +EFIAPI
> +UpdateImageProgress (
> + IN UINTN Completion
> + )
> +{
> + return EFI_SUCCESS;
> +}
> +
> /**
>
> This routine is called to process capsules.
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> index 1659b13ef4..342df9e99c 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> @@ -69,9 +69,10 @@ [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleStatusCodeResettingSystem ## CONSUMES
>
> [Protocols]
> - gEsrtManagementProtocolGuid ## CONSUMES
> - gEfiFirmwareManagementProtocolGuid ## CONSUMES
> - gEdkiiVariableLockProtocolGuid ## SOMETIMES_CONSUMES
> + gEsrtManagementProtocolGuid ## CONSUMES
> + gEfiFirmwareManagementProtocolGuid ## CONSUMES
> + gEdkiiVariableLockProtocolGuid ## SOMETIMES_CONSUMES
> + gEdkiiFirmwareManagementProgressProtocolGuid ## SOMETIMES_CONSUMES
>
> [Guids]
> gEfiFmpCapsuleGuid ## SOMETIMES_CONSUMES ## GUID
> --
> 2.14.2.windows.3
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
next prev parent reply other threads:[~2018-06-06 17:34 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-29 16:17 [Patch v3 0/2] Use DisplayUpdateProgressLib and PerformFlashWriteWithProgress() Michael D Kinney
2018-05-29 16:17 ` [Patch v3 1/2] MdeModulePkg/DxeCapsuleLibFmp: Add progress bar support Michael D Kinney
2018-05-31 9:43 ` Zeng, Star
2018-05-31 13:03 ` Ard Biesheuvel
2018-06-06 17:34 ` Ard Biesheuvel [this message]
2018-05-29 16:17 ` [Patch v3 2/2] SignedCapsulePkg/SystemFirmwareUpdateDxe: Use progress API Michael D Kinney
2018-06-06 17:35 ` Ard Biesheuvel
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+Gu9d2A93_9nDY7eXKanv3pJWCdDfLScznFtzEvHQ-GNQWg@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