Re: [PATCH v2 2/9] MdeModulePkg/UefiBootManager: Add EfiBootManagerDispatchDeferredImages

["Wang, Sunny (HPS SW)" <sunnywang@hpe.com>]

[-- Attachment #1: Type: text/plain, Size: 7206 bytes --]

Thanks for addressing my offline comments. This patch looks good to me. 
Reviewed-by: Sunny Wang <sunnywang@hpe.com>

Regards,
Sunny Wang
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, November 08, 2016 8:29 PM
To: edk2-devel@lists.01.org
Cc: Chao B Zhang <chao.b.zhang@intel.com>; Liming Gao <liming.gao@intel.com>
Subject: [edk2] [PATCH v2 2/9] MdeModulePkg/UefiBootManager: Add EfiBootManagerDispatchDeferredImages

The API dispatches the deferred images that are returned from all DeferredImageLoad instances.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Chao B Zhang <chao.b.zhang@intel.com>
---
 MdeModulePkg/Include/Library/UefiBootManagerLib.h  |  13 +++
 MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c   | 113 +++++++++++++++++++++
 .../Library/UefiBootManagerLib/InternalBm.h        |   1 +
 .../UefiBootManagerLib/UefiBootManagerLib.inf      |   1 +
 4 files changed, 128 insertions(+)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index e333ffd..97ac1f2 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -777,4 +777,17 @@ EfiBootManagerIsValidLoadOptionVariableName (
   OUT UINT16                            *OptionNumber OPTIONAL
   );
 
+
+/**
+  Dispatch the deferred images that are returned from all DeferredImageLoad instances.
+
+  @retval EFI_SUCCESS       At least one deferred image is loaded successfully and started.
+  @retval EFI_NOT_FOUND     There is no deferred image.
+  @retval EFI_ACCESS_DENIED There are deferred images but all of them are failed to load.
+**/
+EFI_STATUS
+EFIAPI
+EfiBootManagerDispatchDeferredImages (
+  VOID
+  );
 #endif
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
index 2a60f06..09e4211 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
@@ -417,3 +417,116 @@ BmCharToUint (
   return (UINTN) -1;
 }
 
+/**
+  Dispatch the deferred images that are returned from all DeferredImageLoad instances.
+
+  @retval EFI_SUCCESS       At least one deferred image is loaded successfully and started.
+  @retval EFI_NOT_FOUND     There is no deferred image.
+  @retval EFI_ACCESS_DENIED There are deferred images but all of them are failed to load.
+**/
+EFI_STATUS
+EFIAPI
+EfiBootManagerDispatchDeferredImages (
+  VOID
+  )
+{
+  EFI_STATUS                         Status;
+  EFI_DEFERRED_IMAGE_LOAD_PROTOCOL   *DeferredImage;
+  UINTN                              HandleCount;
+  EFI_HANDLE                         *Handles;
+  UINTN                              Index;
+  UINTN                              ImageIndex;
+  EFI_DEVICE_PATH_PROTOCOL           *ImageDevicePath;
+  VOID                               *Image;
+  UINTN                              ImageSize;
+  BOOLEAN                            BootOption;
+  EFI_HANDLE                         ImageHandle;
+  UINTN                              ExitDataSize;
+  CHAR16                             *ExitData;
+  UINTN                              ImageCount;
+  UINTN                              LoadCount;
+
+  //
+  // Find all the deferred image load protocols.
+  //
+  HandleCount = 0;
+  Handles = NULL;
+  Status = gBS->LocateHandleBuffer (
+    ByProtocol,
+    &gEfiDeferredImageLoadProtocolGuid,
+    NULL,
+    &HandleCount,
+    &Handles
+  );
+  if (EFI_ERROR (Status)) {
+    return EFI_NOT_FOUND;
+  }
+
+  ImageCount = 0;
+  LoadCount  = 0;
+  for (Index = 0; Index < HandleCount; Index++) {
+    Status = gBS->HandleProtocol (Handles[Index], &gEfiDeferredImageLoadProtocolGuid, (VOID **) &DeferredImage);
+    if (EFI_ERROR (Status)) {
+      continue;
+    }
+
+    for (ImageIndex = 0; ;ImageIndex++) {
+      //
+      // Load all the deferred images in this protocol instance.
+      //
+      Status = DeferredImage->GetImageInfo (
+                                DeferredImage,
+                                ImageIndex,
+                                &ImageDevicePath,
+                                (VOID **) &Image,
+                                &ImageSize,
+                                &BootOption
+                                );
+      if (EFI_ERROR (Status)) {
+        break;
+      }
+      ImageCount++;
+      //
+      // Load and start the image.
+      //
+      Status = gBS->LoadImage (
+        BootOption,
+        gImageHandle,
+        ImageDevicePath,
+        NULL,
+        0,
+        &ImageHandle
+      );
+      if (!EFI_ERROR (Status)) {
+        LoadCount++;
+        //
+        // Before calling the image, enable the Watchdog Timer for
+        // a 5 Minute period
+        //
+        gBS->SetWatchdogTimer (5 * 60, 0x0000, 0x00, NULL);
+        Status = gBS->StartImage (ImageHandle, &ExitDataSize, &ExitData);
+        if (ExitData != NULL) {
+          FreePool (ExitData);
+        }
+
+        //
+        // Clear the Watchdog Timer after the image returns.
+        //
+        gBS->SetWatchdogTimer (0x0000, 0x0000, 0x0000, NULL);
+      }
+    }
+  }
+  if (Handles != NULL) {
+    FreePool (Handles);
+  }
+
+  if (ImageCount == 0) {
+    return EFI_NOT_FOUND;
+  } else {
+    if (LoadCount == 0) {
+      return EFI_ACCESS_DENIED;
+    } else {
+      return EFI_SUCCESS;
+    }
+  }
+}
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index cb719e9..444d4a5 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -44,6 +44,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Protocol/FormBrowser2.h>
 #include <Protocol/VariableLock.h>
 #include <Protocol/RamDisk.h>
+#include <Protocol/DeferredImageLoad.h>
 
 #include <Guid/MemoryTypeInformation.h>  #include <Guid/FileInfo.h> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index 8c3fd7f..bb7c00d 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -109,6 +109,7 @@ [Protocols]
   gEfiDriverHealthProtocolGuid                  ## SOMETIMES_CONSUMES
   gEfiFormBrowser2ProtocolGuid                  ## SOMETIMES_CONSUMES
   gEfiRamDiskProtocolGuid                       ## SOMETIMES_CONSUMES
+  gEfiDeferredImageLoadProtocolGuid             ## CONSUMES
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange      ## SOMETIMES_CONSUMES
--
2.9.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[-- Attachment #2: Type: message/rfc822, Size: 14762 bytes --]

From: "Ni, Ruiyu" <ruiyu.ni@intel.com>
To: "Wang, Sunny (HPS SW)" <sunnywang@hpe.com>
Cc: "Haskell, Darrell" <darrell.haskell@hpe.com>
Subject: RE: [edk2] [PATCH 2/4] MdeModulePkg/UefiBootManager: Add EfiBootManagerDispatchDeferredImages
Date: Mon, 7 Nov 2016 02:37:59 +0000
Message-ID: <734D49CCEBEEF84792F5B80ED585239D58E5679E@SHSMSX104.ccr.corp.intel.com>



Thanks/Ray

> -----Original Message-----
> From: Wang, Sunny (HPS SW) [mailto:sunnywang@hpe.com]
> Sent: Friday, November 4, 2016 7:42 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; Haskell, Darrell
> <darrell.haskell@hpe.com>
> Subject: RE: [edk2] [PATCH 2/4] MdeModulePkg/UefiBootManager: Add
> EfiBootManagerDispatchDeferredImages
>
> Hi Ray,
> Remove others. Feel free to add some people back.
> EfiBootManagerDispatchDeferredImages ()  is almost the same as
> LoadDeferredImage () in UserIdentifyManagerDxe\LoadDeferredImage.c, so
> It looks good to me.
> However, I still have a question from my curiosity. Why don't you just simply
> call USER_MANAGER_PROTOCOL->Identify to trigger LoadDeferredImage()
> right after signaling EndOfDxe in platform BDS code instead of this patch
> series?

UID (User Identification) is a standalone feature and I don't think it is
actively used by the industry. I don't want to combine this feature with
the UID.

>
> By the way, besides the question above, I still have two minor questions
> below, but I think I will see the answer when you send your platform patch.
> 1. Will EfiBootManagerDispatchDeferredImages() only be consumed/called
> by BDS platform code? In other words, we will not call
> EfiBootManagerDispatchDeferredImages() in BdsDxe module, right?


Yes It has to be called from PlatformBDS because it should be called after
platform signals the EndOfDxe event (and Install ReadyToLock protocol).

> 2. Since EfiBootManagerDispatchDeferredImages() is almost the same as
> LoadDeferredImage(), will you remove LoadDeferredImage() from
> UserIdentifyManagerDxe module and call this new API right after calling
> USER_MANAGER_PROTOCOL->Identify in BDS platform code instead?

Sounds like a good idea. I will do it after this patches are committed.

>
> Regards,
> Sunny Wang
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ruiyu Ni
> Sent: Friday, November 04, 2016 9:00 AM
> To: edk2-devel@lists.01.org
> Cc: Chao B Zhang <chao.b.zhang@intel.com>; Liming Gao
> <liming.gao@intel.com>
> Subject: [edk2] [PATCH 2/4] MdeModulePkg/UefiBootManager: Add
> EfiBootManagerDispatchDeferredImages
>
> The API dispatches the deferred images that are returned from all
> DeferredImageLoad instances.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Chao B Zhang <chao.b.zhang@intel.com>
> ---
>  MdeModulePkg/Include/Library/UefiBootManagerLib.h  |  13 +++
>  MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c   | 113
> +++++++++++++++++++++
>  .../Library/UefiBootManagerLib/InternalBm.h        |   1 +
>  .../UefiBootManagerLib/UefiBootManagerLib.inf      |   1 +
>  4 files changed, 128 insertions(+)
>
> diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> index e333ffd..97ac1f2 100644
> --- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> +++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> @@ -777,4 +777,17 @@ EfiBootManagerIsValidLoadOptionVariableName (
>    OUT UINT16                            *OptionNumber OPTIONAL
>    );
>
> +
> +/**
> +  Dispatch the deferred images that are returned from all
> DeferredImageLoad instances.
> +
> +  @retval EFI_SUCCESS       At least one deferred image is loaded successfully
> and started.
> +  @retval EFI_NOT_FOUND     There is no deferred image.
> +  @retval EFI_ACCESS_DENIED There are deferred images but all of them
> are failed to load.
> +**/
> +EFI_STATUS
> +EFIAPI
> +EfiBootManagerDispatchDeferredImages (
> +  VOID
> +  );
>  #endif
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
> index 2a60f06..09e4211 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
> @@ -417,3 +417,116 @@ BmCharToUint (
>    return (UINTN) -1;
>  }
>
> +/**
> +  Dispatch the deferred images that are returned from all
> DeferredImageLoad instances.
> +
> +  @retval EFI_SUCCESS       At least one deferred image is loaded successfully
> and started.
> +  @retval EFI_NOT_FOUND     There is no deferred image.
> +  @retval EFI_ACCESS_DENIED There are deferred images but all of them
> are failed to load.
> +**/
> +EFI_STATUS
> +EFIAPI
> +EfiBootManagerDispatchDeferredImages (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                         Status;
> +  EFI_DEFERRED_IMAGE_LOAD_PROTOCOL   *DeferredImage;
> +  UINTN                              HandleCount;
> +  EFI_HANDLE                         *Handles;
> +  UINTN                              Index;
> +  UINTN                              ImageIndex;
> +  EFI_DEVICE_PATH_PROTOCOL           *ImageDevicePath;
> +  VOID                               *Image;
> +  UINTN                              ImageSize;
> +  BOOLEAN                            BootOption;
> +  EFI_HANDLE                         ImageHandle;
> +  UINTN                              ExitDataSize;
> +  CHAR16                             *ExitData;
> +  UINTN                              ImageCount;
> +  UINTN                              LoadCount;
> +
> +  //
> +  // Find all the deferred image load protocols.
> +  //
> +  HandleCount = 0;
> +  Handles = NULL;
> +  Status = gBS->LocateHandleBuffer (
> +    ByProtocol,
> +    &gEfiDeferredImageLoadProtocolGuid,
> +    NULL,
> +    &HandleCount,
> +    &Handles
> +  );
> +  if (EFI_ERROR (Status)) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  ImageCount = 0;
> +  LoadCount  = 0;
> +  for (Index = 0; Index < HandleCount; Index++) {
> +    Status = gBS->HandleProtocol (Handles[Index],
> &gEfiDeferredImageLoadProtocolGuid, (VOID **) &DeferredImage);
> +    if (EFI_ERROR (Status)) {
> +      continue;
> +    }
> +
> +    for (ImageIndex = 0; ;ImageIndex++) {
> +      //
> +      // Load all the deferred images in this protocol instance.
> +      //
> +      Status = DeferredImage->GetImageInfo (
> +                                DeferredImage,
> +                                ImageIndex,
> +                                &ImageDevicePath,
> +                                (VOID **) &Image,
> +                                &ImageSize,
> +                                &BootOption
> +                                );
> +      if (EFI_ERROR (Status)) {
> +        break;
> +      }
> +      ImageCount++;
> +      //
> +      // Load and start the image.
> +      //
> +      Status = gBS->LoadImage (
> +        BootOption,
> +        gImageHandle,
> +        ImageDevicePath,
> +        NULL,
> +        0,
> +        &ImageHandle
> +      );
> +      if (!EFI_ERROR (Status)) {
> +        LoadCount++;
> +        //
> +        // Before calling the image, enable the Watchdog Timer for
> +        // a 5 Minute period
> +        //
> +        gBS->SetWatchdogTimer (5 * 60, 0x0000, 0x00, NULL);
> +        Status = gBS->StartImage (ImageHandle, &ExitDataSize, &ExitData);
> +        if (ExitData != NULL) {
> +          FreePool (ExitData);
> +        }
> +
> +        //
> +        // Clear the Watchdog Timer after the image returns.
> +        //
> +        gBS->SetWatchdogTimer (0x0000, 0x0000, 0x0000, NULL);
> +      }
> +    }
> +  }
> +  if (Handles != NULL) {
> +    FreePool (Handles);
> +  }
> +
> +  if (ImageCount == 0) {
> +    return EFI_NOT_FOUND;
> +  } else {
> +    if (LoadCount == 0) {
> +      return EFI_ACCESS_DENIED;
> +    } else {
> +      return EFI_SUCCESS;
> +    }
> +  }
> +}
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> index cb719e9..444d4a5 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> @@ -44,6 +44,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #include <Protocol/FormBrowser2.h>
>  #include <Protocol/VariableLock.h>
>  #include <Protocol/RamDisk.h>
> +#include <Protocol/DeferredImageLoad.h>
>
>  #include <Guid/MemoryTypeInformation.h>  #include <Guid/FileInfo.h>
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> index 8c3fd7f..bb7c00d 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +++
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> @@ -109,6 +109,7 @@ [Protocols]
>    gEfiDriverHealthProtocolGuid                  ## SOMETIMES_CONSUMES
>    gEfiFormBrowser2ProtocolGuid                  ## SOMETIMES_CONSUMES
>    gEfiRamDiskProtocolGuid                       ## SOMETIMES_CONSUMES
> +  gEfiDeferredImageLoadProtocolGuid             ## CONSUMES
>
>  [Pcd]
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC
> hange      ## SOMETIMES_CONSUMES
> --
> 2.9.0.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel