Reviewed-by: Michael D Kinney <michael.d.kin...@intel.com>
Mike
> -----Original Message-----
> From: Zeng, Star
> Sent: Tuesday, August 7, 2018 3:28 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.z...@intel.com>; Kinney, Michael D
> <michael.d.kin...@intel.com>
> Subject: [PATCH] FmpDevicePkg FmpDxe: Lock variables in
> entrypoint instead of callback
>
> Current code locks variables in PcdFmpDeviceLockEventGuid
> callback by
> VariableLock protocol whose interface will be closed at
> EndOfDxe.
> So the PcdFmpDeviceLockEventGuid callback needs be
> executed before
> the EndOfDxe callback in Variable driver.
> When PcdFmpDeviceLockEventGuid =
> gEfiEndOfDxeEventGroupGuid, the
> callback's execution sequence depends on the callback's
> TPL and
> registration sequence.
> When PcdFmpDeviceLockEventGuid =
> gEfiEventReadyToBootGuid, the
> PcdFmpDeviceLockEventGuid callback will be executed after
> the
> EndOfDxe callback in Variable driver, the locking will
> fail.
>
> The patch moves the variables locking logic to
> entrypoint.
> The patch also moves the
> IsLockFmpDeviceAtLockEventGuidRequired ()
> checking to entrypoint.
>
> The entrypoint's final return status should be better to
> depend on
> the return status of
> RegisterFmpInstaller/InstallFmpInstance, but not
> gBS->CreateEventEx.
> So the patch also moves the
> RegisterFmpInstaller/InstallFmpInstance
> calling to the end of entrypoint.
>
> Cc: Michael D Kinney <michael.d.kin...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.z...@intel.com>
> ---
> FmpDevicePkg/FmpDxe/FmpDxe.c | 96
> ++++++++++++++++++++++----------------------
> 1 file changed, 48 insertions(+), 48 deletions(-)
>
> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> b/FmpDevicePkg/FmpDxe/FmpDxe.c
> index 3794ac5008f9..57eac5ac147f 100644
> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> @@ -1248,32 +1248,18 @@ FmpDxeLockEventNotify (
> EFI_STATUS Status;
>
> if (!mFmpDeviceLocked) {
> - if (IsLockFmpDeviceAtLockEventGuidRequired ()) {
> - //
> - // Lock all UEFI Variables used by this module.
> - //
> - Status = LockAllFmpVariables ();
> - if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "FmpDxe: Failed to lock
> variables. Status = %r.\n"));
> + //
> + // Lock the firmware device
> + //
> + Status = FmpDeviceLock();
> + if (EFI_ERROR (Status)) {
> + if (Status != EFI_UNSUPPORTED) {
> + DEBUG ((DEBUG_ERROR, "FmpDxe: FmpDeviceLock()
> returned error. Status = %r\n", Status));
> } else {
> - DEBUG ((DEBUG_INFO, "FmpDxe: All variables
> locked\n"));
> - }
> -
> - //
> - // Lock the firmware device
> - //
> - Status = FmpDeviceLock();
> - if (EFI_ERROR (Status)) {
> - if (Status != EFI_UNSUPPORTED) {
> - DEBUG ((DEBUG_ERROR, "FmpDxe: FmpDeviceLock()
> returned error. Status = %r\n", Status));
> - } else {
> - DEBUG ((DEBUG_WARN, "FmpDxe: FmpDeviceLock()
> returned error. Status = %r\n", Status));
> - }
> + DEBUG ((DEBUG_WARN, "FmpDxe: FmpDeviceLock()
> returned error. Status = %r\n", Status));
> }
> - mFmpDeviceLocked = TRUE;
> - } else {
> - DEBUG ((DEBUG_VERBOSE, "FmpDxe: Not calling
> FmpDeviceLock() because mfg mode\n"));
> }
> + mFmpDeviceLocked = TRUE;
> }
> }
>
> @@ -1417,6 +1403,45 @@ FmpDxeEntryPoint (
> //
> DetectTestKey ();
>
> + if (IsLockFmpDeviceAtLockEventGuidRequired ()) {
> + //
> + // Lock all UEFI Variables used by this module.
> + //
> + Status = LockAllFmpVariables ();
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "FmpDxe: Failed to lock
> variables. Status = %r.\n", Status));
> + } else {
> + DEBUG ((DEBUG_INFO, "FmpDxe: All variables
> locked\n"));
> + }
> +
> + //
> + // Register notify function to lock the FMP device.
> + // The lock event GUID is retrieved from
> PcdFmpDeviceLockEventGuid.
> + // If PcdFmpDeviceLockEventGuid is not the size of
> an EFI_GUID, then
> + // gEfiEndOfDxeEventGroupGuid is used.
> + //
> + LockGuid = &gEfiEndOfDxeEventGroupGuid;
> + if (PcdGetSize (PcdFmpDeviceLockEventGuid) == sizeof
> (EFI_GUID)) {
> + LockGuid = (EFI_GUID *)PcdGetPtr
> (PcdFmpDeviceLockEventGuid);
> + }
> + DEBUG ((DEBUG_INFO, "FmpDxe: Lock GUID: %g\n",
> LockGuid));
> +
> + Status = gBS->CreateEventEx (
> + EVT_NOTIFY_SIGNAL,
> + TPL_CALLBACK,
> + FmpDxeLockEventNotify,
> + NULL,
> + LockGuid,
> + &mFmpDeviceLockEvent
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "FmpDxe: Failed to register
> notification. Status = %r\n", Status));
> + }
> + ASSERT_EFI_ERROR (Status);
> + } else {
> + DEBUG ((DEBUG_VERBOSE, "FmpDxe: Not registering
> notification to call FmpDeviceLock() because mfg
> mode\n"));
> + }
> +
> //
> // Register with library the install function so if
> the library uses
> // UEFI driver model/driver binding protocol it can
> install FMP on its device handle
> @@ -1436,30 +1461,5 @@ FmpDxeEntryPoint (
> ));
> }
>
> - //
> - // Register notify function to lock the FMP device.
> - // The lock event GUID is retrieved from
> PcdFmpDeviceLockEventGuid.
> - // If PcdFmpDeviceLockEventGuid is not the size of an
> EFI_GUID, then
> - // gEfiEndOfDxeEventGroupGuid is used.
> - //
> - LockGuid = &gEfiEndOfDxeEventGroupGuid;
> - if (PcdGetSize (PcdFmpDeviceLockEventGuid) == sizeof
> (EFI_GUID)) {
> - LockGuid = (EFI_GUID *)PcdGetPtr
> (PcdFmpDeviceLockEventGuid);
> - }
> - DEBUG ((DEBUG_INFO, "FmpDxe: Lock GUID: %g\n",
> LockGuid));
> -
> - Status = gBS->CreateEventEx (
> - EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> - FmpDxeLockEventNotify,
> - NULL,
> - LockGuid,
> - &mFmpDeviceLockEvent
> - );
> - if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "FmpDxe: Failed to register for
> ready to boot. Status = %r\n", Status));
> - }
> - ASSERT_EFI_ERROR (Status);
> -
> return Status;
> }
> --
> 2.7.0.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
