Re: [edk2] [PATCH v4 1/9] MdeModulePkg/PlatformBootManager: Add PlatformBootManagerUnableToBoot

[Wang, Sunny (HPS SW)]

Looks good to me. 
Reviewed-by: Sunny Wang 

-Original Message-
From: Ruiyu Ni [mailto:ruiyu...@intel.com] 
Sent: Wednesday, July 04, 2018 9:51 AM
To: edk2-devel@lists.01.org
Cc: Sean Brogan ; Michael Turner 
; Wang, Sunny (HPS SW) 
Subject: [PATCH v4 1/9] MdeModulePkg/PlatformBootManager: Add 
PlatformBootManagerUnableToBoot

The patch adds a new API PlatformBootManagerUnableToBoot() to 
PlatformBootManagerLib.
The new API is provided by platform bds library and is called when no boot 
option could be launched.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Sean Brogan 
Cc: Michael Turner 
Reviewed-by: Laszlo Ersek 
Cc: Sunny Wang 
---
 MdeModulePkg/Include/Library/PlatformBootManagerLib.h | 13 +  
.../PlatformBootManagerLibNull/PlatformBootManager.c  | 19 ++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Include/Library/PlatformBootManagerLib.h 
b/MdeModulePkg/Include/Library/PlatformBootManagerLib.h
index 65630ce2bb..6e26329043 100644
--- a/MdeModulePkg/Include/Library/PlatformBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/PlatformBootManagerLib.h
@@ -59,4 +59,17 @@ PlatformBootManagerWaitCallback (
   UINT16  TimeoutRemain
   );
 
+/**
+  The function is called when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  If this function returns, BDS attempts to enter an infinite loop.
+**/
+VOID
+EFIAPI
+PlatformBootManagerUnableToBoot (
+  VOID
+  );
+
 #endif
diff --git 
a/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c 
b/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c
index 1390e19097..5a4455ef23 100644
--- a/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c
+++ b/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManage
+++ r.c
@@ -2,7 +2,7 @@
   This file include all platform action which can be customized
   by IBV/OEM.
 
-Copyright (c) 2012 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
 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,3 +65,20 @@ 
PlatformBootManagerWaitCallback (  {
   return;
 }
+
+/**
+  The function is called when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  If this function returns, BDS attempts to enter an infinite loop.
+**/
+VOID
+EFIAPI
+PlatformBootManagerUnableToBoot (
+  VOID
+  )
+{
+  return;
+}
+
--
2.16.1.windows.1

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

Re: [edk2] [PATCH v4 9/9] MdeModulePkg/BdsDxe: Call PlatformBootManagerUnableToBoot()

[Wang, Sunny (HPS SW)]

Looks good to me. This is clearer. 
Reviewed-by: Sunny Wang 

-Original Message-
From: Ruiyu Ni [mailto:ruiyu...@intel.com] 
Sent: Wednesday, July 04, 2018 9:51 AM
To: edk2-devel@lists.01.org
Cc: Sean Brogan ; Michael Turner 
; Wang, Sunny (HPS SW) 
Subject: [PATCH v4 9/9] MdeModulePkg/BdsDxe: Call 
PlatformBootManagerUnableToBoot()

When no boot option can be launched, BDS core calls
PlatformBootManagerUnableToBoot() to let platform BdsDxe handle it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Reviewed-by: Laszlo Ersek 
Cc: Sean Brogan 
Cc: Michael Turner 
Cc: Sunny Wang 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 39b643c77a..a25663ea43 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -1043,6 +1043,7 @@ BdsEntry (
   }
 
   DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
+  PlatformBootManagerUnableToBoot ();
   CpuDeadLoop ();
 }
 
-- 
2.16.1.windows.1

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

Re: [edk2] [PATCH v4 8/9] MdeModulePkg/BdsDxe: Revert "fall back to UI loop before hanging"

[Wang, Sunny (HPS SW)]

Looks good to me. This is clearer. 
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Wednesday, July 04, 2018 9:51 AM
To: edk2-devel@lists.01.org
Cc: Eric Dong 
Subject: [edk2] [PATCH v4 8/9] MdeModulePkg/BdsDxe: Revert "fall back to UI 
loop before hanging"

Commit d1de487dd2e77f4741abcbd71d19a8c93971fda0
"MdeModulePkg/BdsDxe: fall back to a Boot Manager Menu loop before  hanging"
changed BDS core to fall back to UI loop when no bootable option can be 
launched.
Now since PlatformBootManagerUnableToBoot() is added, the commit can be 
reverted.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Eric Dong 
Reviewed-by: Laszlo Ersek 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 60 +++-
 1 file changed, 4 insertions(+), 56 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 49e403e181..39b643c77a 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -634,55 +634,6 @@ BdsFormalizeEfiGlobalVariable (
   BdsFormalizeOSIndicationVariable ();
 }
 
-/**
-  Enter an infinite loop of calling the Boot Manager Menu.
-
-  This is a last resort alternative to BdsEntry() giving up for good. This
-  function never returns.
-
-  @param[in] BootManagerMenu  The EFI_BOOT_MANAGER_LOAD_OPTION located and/or
-  created by the EfiBootManagerGetBootManagerMenu()
-  call in BdsEntry().
-**/
-VOID
-BdsBootManagerMenuLoop (
-  IN EFI_BOOT_MANAGER_LOAD_OPTION *BootManagerMenu
-  )
-{
-  EFI_INPUT_KEY Key;
-
-  //
-  // Normally BdsDxe does not print anything to the system console, but this is
-  // a last resort -- the end-user will likely not see any DEBUG messages
-  // logged in this situation.
-  //
-  // AsciiPrint() will NULL-check gST->ConOut internally. We check gST->ConIn
-  // here to see if it makes sense to request and wait for a keypress.
-  //
-  if (gST->ConIn != NULL) {
-AsciiPrint (
-  "%a: No bootable option or device was found.\n"
-  "%a: Press any key to enter the Boot Manager Menu.\n",
-  gEfiCallerBaseName,
-  gEfiCallerBaseName
-  );
-BdsWaitForSingleEvent (gST->ConIn->WaitForKey, 0);
-
-//
-// Drain any queued keys.
-//
-while (!EFI_ERROR (gST->ConIn->ReadKeyStroke (gST->ConIn, ))) {
-  //
-  // just throw away Key
-  //
-}
-  }
-
-  for (;;) {
-EfiBootManagerBoot (BootManagerMenu);
-  }
-}
-
 /**
 
   Service routine for BdsInstance->Entry(). Devices are connected, the @@ 
-1081,19 +1032,16 @@ BdsEntry (
 } while (BootSuccess);
   }
 
+  if (BootManagerMenuStatus != EFI_NOT_FOUND) {
+EfiBootManagerFreeLoadOption ();  }
+
   if (!BootSuccess) {
 LoadOptions = EfiBootManagerGetLoadOptions (, 
LoadOptionTypePlatformRecovery);
 ProcessLoadOptions (LoadOptions, LoadOptionCount);
 EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
   }
 
-  //
-  // If BootManagerMenu is available, fall back to it indefinitely.
-  //
-  if (BootManagerMenuStatus != EFI_NOT_FOUND) {
-BdsBootManagerMenuLoop ();
-  }
-
   DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
   CpuDeadLoop ();
 }
--
2.16.1.windows.1

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

Re: [edk2] [PATCH v2 2/2] MdeModulePkg/BdsDxe: Call *UnableToBoot() for ultimate boot failure

[Wang, Sunny (HPS SW)]

Looks good to me. Thanks for addressing my comment, Ray. 
Reviewed-by: Sunny Wang 

-Original Message-
From: Ruiyu Ni [mailto:ruiyu...@intel.com] 
Sent: Friday, June 29, 2018 2:04 PM
To: edk2-devel@lists.01.org
Cc: Sean Brogan ; Michael Turner 
; Laszlo Ersek ; Wang, Sunny 
(HPS SW) 
Subject: [PATCH v2 2/2] MdeModulePkg/BdsDxe: Call *UnableToBoot() for ultimate 
boot failure

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=982

The patch changes the default core behavior of ultimate boot failure to launch 
Boot Manager Menu.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Sean Brogan 
Cc: Michael Turner 
Cc: Laszlo Ersek 
Cc: Sunny Wang 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 65 +++-
 1 file changed, 13 insertions(+), 52 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 49e403e181..db2a4cf208 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -634,55 +634,6 @@ BdsFormalizeEfiGlobalVariable (
   BdsFormalizeOSIndicationVariable ();
 }
 
-/**
-  Enter an infinite loop of calling the Boot Manager Menu.
-
-  This is a last resort alternative to BdsEntry() giving up for good. This
-  function never returns.
-
-  @param[in] BootManagerMenu  The EFI_BOOT_MANAGER_LOAD_OPTION located and/or
-  created by the EfiBootManagerGetBootManagerMenu()
-  call in BdsEntry().
-**/
-VOID
-BdsBootManagerMenuLoop (
-  IN EFI_BOOT_MANAGER_LOAD_OPTION *BootManagerMenu
-  )
-{
-  EFI_INPUT_KEY Key;
-
-  //
-  // Normally BdsDxe does not print anything to the system console, but this is
-  // a last resort -- the end-user will likely not see any DEBUG messages
-  // logged in this situation.
-  //
-  // AsciiPrint() will NULL-check gST->ConOut internally. We check gST->ConIn
-  // here to see if it makes sense to request and wait for a keypress.
-  //
-  if (gST->ConIn != NULL) {
-AsciiPrint (
-  "%a: No bootable option or device was found.\n"
-  "%a: Press any key to enter the Boot Manager Menu.\n",
-  gEfiCallerBaseName,
-  gEfiCallerBaseName
-  );
-BdsWaitForSingleEvent (gST->ConIn->WaitForKey, 0);
-
-//
-// Drain any queued keys.
-//
-while (!EFI_ERROR (gST->ConIn->ReadKeyStroke (gST->ConIn, ))) {
-  //
-  // just throw away Key
-  //
-}
-  }
-
-  for (;;) {
-EfiBootManagerBoot (BootManagerMenu);
-  }
-}
-
 /**
 
   Service routine for BdsInstance->Entry(). Devices are connected, the @@ 
-1088,10 +1039,20 @@ BdsEntry (
   }
 
   //
-  // If BootManagerMenu is available, fall back to it indefinitely.
+  // Inform the platform that we're unable to boot. The platform may 
+ enter  // BootManagerMenu with the public EfiBootManagerBoot() 
+ interface, if so  // desired.
   //
-  if (BootManagerMenuStatus != EFI_NOT_FOUND) {
-BdsBootManagerMenuLoop ();
+  Status = EfiBootManagerUnableToBoot ();  if (EFI_ERROR (Status) && 
+ (BootManagerMenuStatus != EFI_NOT_FOUND)) {
+//
+// The platform didn't register a callback; fall back to BootManagerMenu
+// internally, indefinitely.
+//
+
+while (TRUE) {
+  EfiBootManagerBoot ();
+}
   }
 
   DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
--
2.16.1.windows.1

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

Re: [edk2] [PATCH v2 1/2] MdeModulePkg/UefiBootManagerLib: new APIs for ultimate boot failure

[Wang, Sunny (HPS SW)]

Looks good to me. Thanks for addressing my comment, Ray. 

Reviewed-by: Sunny Wang 

-Original Message-
From: Ruiyu Ni [mailto:ruiyu...@intel.com] 
Sent: Friday, June 29, 2018 2:04 PM
To: edk2-devel@lists.01.org
Cc: Sean Brogan ; Michael Turner 
; Laszlo Ersek ; Wang, Sunny 
(HPS SW) 
Subject: [PATCH v2 1/2] MdeModulePkg/UefiBootManagerLib: new APIs for ultimate 
boot failure

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=982

When no boot option could be launched including platform recovery options and 
options pointing to applications built into firmware volumes, a platform 
callback registered through
EfiBootManagerRegisterUnableToBootHandler() is called.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Sean Brogan 
Cc: Michael Turner 
Cc: Laszlo Ersek 
Cc: Sunny Wang 
---
 MdeModulePkg/Include/Library/UefiBootManagerLib.h | 48 +++ 
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c  | 41 +++
 2 files changed, 89 insertions(+)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h 
b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index bfc0cb86f8..453893e030 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -800,4 +800,52 @@ EFIAPI
 EfiBootManagerDispatchDeferredImages (
   VOID
   );
+
+/**
+  The function is called when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  The platform may register this function to inform the user about the above 
fact.
+  If this function returns, BDS core attempts to enter an infinite loop 
+of pulling
+  up the Boot Manager Menu (if the platform includes the Boot Manager Menu).
+  If the Boot Manager Menu is unavailable, BDS will hang.
+**/
+typedef
+VOID
+(EFIAPI *EFI_BOOT_MANAGER_UNABLE_TO_BOOT) (
+  VOID
+  );
+
+/**
+  Register the callback function when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  @param Handler  The callback function.
+**/
+VOID
+EFIAPI
+EfiBootManagerRegisterUnableToBootHandler (
+  EFI_BOOT_MANAGER_UNABLE_TO_BOOT  Handler
+  );
+
+/**
+  The function is called when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  If this function returns, BDS core attempts to enter an infinite loop 
+ of pulling  up the Boot Manager Menu (if the platform includes the Boot 
Manager Menu).
+  If the Boot Manager Menu is unavailable, BDS will hang.
+
+  @retval EFI_SUCCESS The unable-to-boot callback is called successfully.
+  @retval EFI_UNSUPPORTED There is no unable-to-boot callback registered.
+**/
+EFI_STATUS
+EFIAPI
+EfiBootManagerUnableToBoot (
+  VOID
+  );
+
 #endif
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 6a23477eb8..e59d790122 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -19,6 +19,7 @@ EFI_RAM_DISK_PROTOCOL*mRamDisk
  = NULL;
 
 EFI_BOOT_MANAGER_REFRESH_LEGACY_BOOT_OPTION  mBmRefreshLegacyBootOption = NULL;
 EFI_BOOT_MANAGER_LEGACY_BOOT mBmLegacyBoot  = NULL;
+EFI_BOOT_MANAGER_UNABLE_TO_BOOT  mBmUnableToBoot= NULL;
 
 ///
 /// This GUID is used for an EFI Variable that stores the front device pathes 
@@ -2461,3 +2462,43 @@ EfiBootManagerGetBootManagerMenu (
   }
 }
 
+/**
+  Register the callback function when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  @param Handler  The callback function.
+**/
+VOID
+EFIAPI
+EfiBootManagerRegisterUnableToBootHandler (
+  EFI_BOOT_MANAGER_UNABLE_TO_BOOT  Handler
+  )
+{
+  mBmUnableToBoot = Handler;
+}
+
+/**
+  The function is called when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  If this function returns, BDS core attempts to enter an infinite loop 
+ of pulling  up the Boot Manager Menu (if the platform includes the Boot 
Manager Menu).
+  If the Boot Manager Menu is unavailable, BDS will hang.
+
+  @retval EFI_SUCCESS The unable-to-boot callback is called successfully.
+  @retval EFI_UNSUPPORTED There is no unable-to-boot callback registered.
+**/
+EFI_STATUS
+EFIAPI
+EfiBootManagerUnableToBoot (
+  VOID
+  )
+{
+  if (mBmUnableToBoot != NULL) {
+mBmUnableToBoot ();
+return EFI_SUCCESS;
+  }
+  return EFI_UNSUPPORTED;
+}
--
2.16.1.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: handle ultimate boot failure

[Wang, Sunny (HPS SW)]

Thanks, Laszlo. 

Hi Ray, 
After looking into  <https://bugzilla.tianocore.org/show_bug.cgi?id=982>, I 
just realized that I think too deep about the purpose of adding the 
EfiBootManagerUnableToBoot (). 
EfiBootManagerUnableToBoot () is added for solving the problem that user don't 
want to see the messages " No bootable option..." printed by BDS core code. The 
design is based on the concept that "print in a platformbds lib, loop in core 
code", so your current patch already solved that problem. Therefore, if there 
is no need to deal with the case where system doesn't have BootManagerMenu 
application, I'm totally fine with your current patch. 

Regards,
Sunny Wang

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Thursday, June 28, 2018 11:04 PM
To: Wang, Sunny (HPS SW) ; Ruiyu Ni ; 
edk2-devel@lists.01.org
Cc: Michael Turner 
Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: handle ultimate 
boot failure
Importance: High

On 06/28/18 11:02, Wang, Sunny (HPS SW) wrote:
> Hi Ray,
>
> Does the ultimate boot failure include the case where the system 
> doesn't have "BootManagerMenu" application?  If so, I think we should 
> move EfiBootManagerUnableBoot() call out of BdsBootManagerMenuLoop().
> The BdsBootManagerMenuLoop() is only called when system has 
> BootManagerMenu application. Therefore, if the system doesn't have 
> BootManagerMenu application, the EfiBootManagerUnableBoot() will have 
> no chance to be called for handling the ultimate boot failure case.

Personally I'd be very happy with the current version of the patch as well, but 
I agree Sunny's request makes sense. How about this, for
BdsDxe:

> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
> b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index 3191a986304b..cb4196a56c87 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -1088,11 +1088,26 @@ BdsEntry (
>  EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
>}
>
> -  //
> -  // If BootManagerMenu is available, fall back to it indefinitely.
> -  //
> -  if (BootManagerMenuStatus != EFI_NOT_FOUND) {
> -BdsBootManagerMenuLoop ();
> +  if (BootManagerMenuStatus == EFI_NOT_FOUND) {
> +//
> +// Inform the platform that we're unable to boot, and that there's no
> +// BootManagerMenu.
> +//
> +EfiBootManagerUnableToBoot (NULL);  } else {
> +//
> +// Inform the platform that we're unable to boot. The platform may enter
> +// BootManagerMenu with the public EfiBootManagerBoot() interface, if so
> +// desired.
> +//
> +Status = EfiBootManagerUnableToBoot ();
> +if (EFI_ERROR (Status)) {
> +  //
> +  // The platform didn't register a callback; fall back to 
> BootManagerMenu
> +  // internally, indefinitely.
> +  //
> +  BdsBootManagerMenuLoop ();
> +}
>}
>
>DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));

Note that this requires changing the declaration of EfiBootManagerUnableBoot(), 
so that it takes the parameter

  IN EFI_BOOT_MANAGER_LOAD_OPTION *BootManagerMenu OPTIONAL

The structure EFI_BOOT_MANAGER_LOAD_OPTION is from 
"MdeModulePkg/Include/Library/UefiBootManagerLib.h", so it is OK to expose to 
platforms.

Just an idea, of course.

--*--

Anyway, for a v2, I have some superficial reuqests / questions for Ray:

* Please replace "UNABLE_BOOT" with "UNABLE_TO_BOOT". Same for
  "UnableBoot" and "UnableToBoot".

  (Compare: READY_TO_BOOT, ReadyToBoot.)

  Note that this affects the commit message too.

* Should we split the BdsDxe modification to a separate patch?

* Can you please reference
  <https://bugzilla.tianocore.org/show_bug.cgi?id=982> in the commit
  message?

Thank you very much Ray for doing this!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: handle ultimate boot failure

[Wang, Sunny (HPS SW)]

Hi Ray,

Does the ultimate boot failure include the case where the system doesn't have 
"BootManagerMenu" application?  If so, I think we should move 
EfiBootManagerUnableBoot() call out of BdsBootManagerMenuLoop(). The 
BdsBootManagerMenuLoop() is only called when system has BootManagerMenu 
application. Therefore, if the system doesn't have BootManagerMenu application, 
the EfiBootManagerUnableBoot() will have no chance to be called for handling 
the ultimate boot failure case.  

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Thursday, June 28, 2018 3:40 PM
To: edk2-devel@lists.01.org
Cc: Laszlo Ersek ; Michael Turner 

Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: handle ultimate boot 
failure

When no boot option could be launched including platform recovery options and 
options pointing to applications built into firmware volumes, a platform 
callback registered through
EfiBootManagerRegisterUnableBootHandler() is called.

If there is no platform callback registered, default behavior is to print an 
error message to the screen and wait for user input.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Sean Brogan 
Cc: Michael Turner 
Cc: Laszlo Ersek 
---
 MdeModulePkg/Include/Library/UefiBootManagerLib.h | 48 +++ 
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c  | 41 +++
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c  | 44 +++--
 3 files changed, 113 insertions(+), 20 deletions(-)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h 
b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index bfc0cb86f8..0035c41082 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -800,4 +800,52 @@ EFIAPI
 EfiBootManagerDispatchDeferredImages (
   VOID
   );
+
+/**
+  The function is called when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  The platform may register this function to inform the user about the above 
fact.
+  If this function returns, BDS core attempts to enter an infinite loop 
+of pulling
+  up the Boot Manager Menu (if the platform includes the Boot Manager Menu).
+  If the Boot Manager Menu is unavailable, BDS will hang.
+**/
+typedef
+VOID
+(EFIAPI *EFI_BOOT_MANAGER_UNABLE_BOOT) (
+  VOID
+  );
+
+/**
+  Register the callback function when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  @param Handler  The callback function.
+**/
+VOID
+EFIAPI
+EfiBootManagerRegisterUnableBootHandler (
+  EFI_BOOT_MANAGER_UNABLE_BOOT  Handler
+  );
+
+/**
+  The function is called when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  If this function returns, BDS core attempts to enter an infinite loop 
+ of pulling  up the Boot Manager Menu (if the platform includes the Boot 
Manager Menu).
+  If the Boot Manager Menu is unavailable, BDS will hang.
+
+  @retval EFI_SUCCESS The unable-boot callback is called successfully.
+  @retval EFI_UNSUPPORTED There is no unable-boot callback registered.
+**/
+EFI_STATUS
+EFIAPI
+EfiBootManagerUnableBoot (
+  VOID
+  );
+
 #endif
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 6a23477eb8..122068267d 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -19,6 +19,7 @@ EFI_RAM_DISK_PROTOCOL*mRamDisk
  = NULL;
 
 EFI_BOOT_MANAGER_REFRESH_LEGACY_BOOT_OPTION  mBmRefreshLegacyBootOption = NULL;
 EFI_BOOT_MANAGER_LEGACY_BOOT mBmLegacyBoot  = NULL;
+EFI_BOOT_MANAGER_UNABLE_BOOT mBmUnableBoot  = NULL;
 
 ///
 /// This GUID is used for an EFI Variable that stores the front device pathes 
@@ -2461,3 +2462,43 @@ EfiBootManagerGetBootManagerMenu (
   }
 }
 
+/**
+  Register the callback function when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  @param Handler  The callback function.
+**/
+VOID
+EFIAPI
+EfiBootManagerRegisterUnableBootHandler (
+  EFI_BOOT_MANAGER_UNABLE_BOOT  Handler
+  )
+{
+  mBmUnableBoot = Handler;
+}
+
+/**
+  The function is called when no boot option could be launched,
+  including platform recovery options and options pointing to 
+applications
+  built into firmware volumes.
+
+  If this function returns, BDS core attempts to enter an infinite loop 
+ of pulling  up the Boot Manager Menu (if the platform includes the Boot 
Manager Menu).
+  If the Boot 

Re: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recursive call depth

[Wang, Sunny (HPS SW)]

Hi Heyi, 
Thanks for looking into my suggestion. You already mentioned the value I 
thought. :) The value is to get better boot performance by setting the 
MAX_RECONNECT_REPAIR to a smaller number. 10 times reconnect may be suitable 
for consumer products like laptop. However, it may be not suitable for server. 
For example, user installs a problematic NIC 4-port card and its OPROM produced 
4 driver handles to manage 4 ports. For this case, 10 times reconnect may take 
much longer time.  
If you think it's still not worth adding a PCD for this or have other concern 
about adding a PCD, I'm fine with dropping this suggestion.   

Regards,
Sunny Wang

-Original Message-
From: Guo Heyi [mailto:heyi@linaro.org] 
Sent: Monday, February 26, 2018 7:34 PM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>
Cc: Heyi Guo <heyi@linaro.org>; edk2-devel@lists.01.org; Ruiyu Ni 
<ruiyu...@intel.com>; Eric Dong <eric.d...@intel.com>; Star Zeng 
<star.z...@intel.com>
Subject: Re: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit 
recursive call depth
Importance: High

Hi Sunny,

I didn't consider it as a value necessary for platform override, for the retry 
count should only have some impact on boot performance and it only happens when 
there is something wrong.

May I know what value you will use for your platform and why?

Thanks and regards,

Gary (Heyi Guo)

On Mon, Feb 26, 2018 at 08:56:50AM +, Wang, Sunny (HPS SW) wrote:
> Hi Heyi,
> Just a suggestion. 
> Is it better to use a PCD instead of a define for MAX_RECONNECT_REPAIR? So 
> that we can easily override it in our platform dsc file.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Heyi Guo
> Sent: Monday, February 26, 2018 4:30 PM
> To: edk2-devel@lists.01.org
> Cc: Ruiyu Ni <ruiyu...@intel.com>; Heyi Guo <heyi@linaro.org>; 
> Eric Dong <eric.d...@intel.com>; Star Zeng <star.z...@intel.com>
> Subject: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit 
> recursive call depth
> 
> Function BmRepairAllControllers may recursively call itself if some driver 
> health protocol returns EfiDriverHealthStatusReconnectRequired.
> However, driver health protocol of some buggy third party driver may always 
> return such status even after one and another reconnect. The endless 
> iteration will cause stack overflow and then system exception, and it may be 
> not easy to find that the exception is actually caused by stack overflow.
> 
> So we limit the number of reconnect retry to 10 to improve code robustness.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Heyi Guo <heyi@linaro.org>
> Cc: Star Zeng <star.z...@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Ruiyu Ni <ruiyu...@intel.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h |  6 ++
>  MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 
> -
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
> b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> index 25a1d522fe84..9aa86b096525 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> @@ -108,6 +108,12 @@ CHAR16 *
>  #define BM_OPTION_NAME_LEN  sizeof 
> ("PlatformRecovery")
>  extern CHAR16  *mBmLoadOptionName[];
>  
> +//
> +// Maximum number of reconnect retry to repair controller; it is to 
> +limit the // number of recursive call of BmRepairAllControllers.
> +//
> +#define MAX_RECONNECT_REPAIR10
> +
>  /**
>Visitor function to be called by BmForEachVariable for each variable
>in variable storage.
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
> b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> index ddcee8b0676f..30d70f32af84 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> @@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED
>  L"Reboot Required"
>  };
>  
> +//
> +// Counter of reconnect retry to repair controller; it is to limit 
> +the // number of recursive call of BmRepairAllControllers.
> +//
> +STATIC UINTN mReconnectRepairCount;
> +
>  /**
>Return the controller name.
>  
> @@ -549,7 +555,16 @@ BmRepairAllControllers (
>  
>  
>if (ReconnectRequired) {
> -BmRepairAllControllers ();
> +if (mReconnectRepairCount < MAX_RECONNECT_REPAIR) {
> +  mReconne

Re: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recursive call depth

[Wang, Sunny (HPS SW)]

Hi Heyi, 
Just a suggestion. 
Is it better to use a PCD instead of a define for MAX_RECONNECT_REPAIR? So that 
we can easily override it in our platform dsc file.

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Heyi Guo
Sent: Monday, February 26, 2018 4:30 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Heyi Guo ; Eric Dong 
; Star Zeng 
Subject: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recursive 
call depth

Function BmRepairAllControllers may recursively call itself if some driver 
health protocol returns EfiDriverHealthStatusReconnectRequired.
However, driver health protocol of some buggy third party driver may always 
return such status even after one and another reconnect. The endless iteration 
will cause stack overflow and then system exception, and it may be not easy to 
find that the exception is actually caused by stack overflow.

So we limit the number of reconnect retry to 10 to improve code robustness.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Ruiyu Ni 
---
 MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h |  6 ++
 MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 -
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 25a1d522fe84..9aa86b096525 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -108,6 +108,12 @@ CHAR16 *
 #define BM_OPTION_NAME_LEN  sizeof 
("PlatformRecovery")
 extern CHAR16  *mBmLoadOptionName[];
 
+//
+// Maximum number of reconnect retry to repair controller; it is to 
+limit the // number of recursive call of BmRepairAllControllers.
+//
+#define MAX_RECONNECT_REPAIR10
+
 /**
   Visitor function to be called by BmForEachVariable for each variable
   in variable storage.
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
index ddcee8b0676f..30d70f32af84 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
@@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED
 L"Reboot Required"
 };
 
+//
+// Counter of reconnect retry to repair controller; it is to limit the 
+// number of recursive call of BmRepairAllControllers.
+//
+STATIC UINTN mReconnectRepairCount;
+
 /**
   Return the controller name.
 
@@ -549,7 +555,16 @@ BmRepairAllControllers (
 
 
   if (ReconnectRequired) {
-BmRepairAllControllers ();
+if (mReconnectRepairCount < MAX_RECONNECT_REPAIR) {
+  mReconnectRepairCount++;
+  BmRepairAllControllers ();
+} else {
+  DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n",
+__FUNCTION__, __LINE__, mReconnectRepairCount));
+  // Reset counter so that it will not affect calling
+  // BmRepairAllControllers() somewhere else
+  mReconnectRepairCount = 0;
+}
   }
 
   DEBUG_CODE (
--
2.7.4

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

Re: [edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it

[Wang, Sunny (HPS SW)]

The change Looks good to me. 
Also, Good to see you adding the comment about why we need to cache BootNext 
before all the platform hook function calls. :)

Reviewed-by: Sunny Wang 

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Thursday, September 28, 2017 1:50 PM
To: edk2-devel@lists.01.org
Cc: Eric Dong ; Star Zeng 
Subject: [edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until 
booting it

Current implementation deletes the "BootNext" before calling any 
PlatformBootManagerLib APIs, but if system resets in PlatformBootManagerLib 
APIs, "BootNext" is not consumed but lost.

The patch defers the deletion of "BootNext" to before booting it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Eric Dong 
Cc: Star Zeng 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 ++--
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index ac5f9088dd..a6fe617b56 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -808,7 +808,8 @@ BdsEntry (
   ASSERT_EFI_ERROR (Status);
 
   //
-  // Cache and remove the "BootNext" NV variable.
+  // Cache the "BootNext" NV variable before calling any 
+ PlatformBootManagerLib APIs  // This could avoid the "BootNext" set by 
PlatformBootManagerLib be consumed in this boot.
   //
   GetEfiGlobalVariable2 (EFI_BOOT_NEXT_VARIABLE_NAME, (VOID **) , 
);
   if (DataSize != sizeof (UINT16)) {
@@ -817,17 +818,6 @@ BdsEntry (
 }
 BootNext = NULL;
   }
-  Status = gRT->SetVariable (
-  EFI_BOOT_NEXT_VARIABLE_NAME,
-  ,
-  0,
-  0,
-  NULL
-  );
-  //
-  // Deleting NV variable shouldn't fail unless it doesn't exist.
-  //
-  ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND);
 
   //
   // Initialize the platform language variables @@ -1052,10 +1042,25 @@ 
BdsEntry (
 
 EfiBootManagerHotkeyBoot ();
 
-//
-// Boot to "BootNext"
-//
 if (BootNext != NULL) {
+  //
+  // Delete "BootNext" NV variable before transferring control to it to 
prevent loops.
+  //
+  Status = gRT->SetVariable (
+  EFI_BOOT_NEXT_VARIABLE_NAME,
+  ,
+  0,
+  0,
+  NULL
+  );
+  //
+  // Deleting NV variable shouldn't fail unless it doesn't exist.
+  //
+  ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND);
+
+  //
+  // Boot to "BootNext"
+  //
   UnicodeSPrint (BootNextVariableName, sizeof (BootNextVariableName), 
L"Boot%04x", *BootNext);
   Status = EfiBootManagerVariableToLoadOption (BootNextVariableName, 
);
   if (!EFI_ERROR (Status)) {
--
2.12.2.windows.2

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

Re: [edk2] [Patch] Nt32/PlatformBootManagerLib: Enable STD_ERROR on all consoles

[Wang, Sunny (HPS SW)]

Reviewed and tested it. Looks good to me. Thanks for addressing my offline 
comment, Michael. :)
Reviewed-by: Sunny Wang 

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Michael 
D Kinney
Sent: Tuesday, August 15, 2017 6:30 AM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Hao Wu ; Michael Kinney 

Subject: [edk2] [Patch] Nt32/PlatformBootManagerLib: Enable STD_ERROR on all 
consoles

Add STD_ERROR flag to all output consoles that the Nt32 platform supports so 
all messages sent to the standard error console device(s) are visible by 
default.

The Boot Maintenance Manager can be used to manually disable standard error 
output to specific console devices.

UEFI Applications and UEFI Drivers are recommended to be built with DEBUG() and 
ASSERT() messages sent to the standard error device using 
MdePkg/Library/UefiDebugLibStdErr. Prior to this change, a user would have to 
use the Boot Maintenance Manager to configure a standard error console device 
to make these messages visible.

Cc: Ruiyu Ni 
Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Kinney 
---
 Nt32Pkg/Library/PlatformBootManagerLib/PlatformData.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformData.c 
b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformData.c
index e92f377bd1..e4b400b4a3 100644
--- a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformData.c
+++ b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformData.c
@@ -2,7 +2,7 @@
   Defined the platform specific device path which will be filled to
   ConIn/ConOut variables.

-Copyright (c) 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
 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
@@ -137,19 +137,19 @@ NT_ISA_SERIAL_DEVICE_PATH   gNtSerialDevicePath1 = {
 PLATFORM_CONSOLE_CONNECT_ENTRY   gPlatformConsole[] = {
   {
 (EFI_DEVICE_PATH_PROTOCOL *) ,
-(CONSOLE_OUT | CONSOLE_IN)
+(CONSOLE_OUT | CONSOLE_IN | STD_ERROR)
   },
   {
 (EFI_DEVICE_PATH_PROTOCOL *) ,
-(CONSOLE_OUT | CONSOLE_IN)
+(CONSOLE_OUT | CONSOLE_IN | STD_ERROR)
   },
   {
 (EFI_DEVICE_PATH_PROTOCOL *) ,
-(CONSOLE_OUT | CONSOLE_IN)
+(CONSOLE_OUT | CONSOLE_IN | STD_ERROR)
   },
   {
 (EFI_DEVICE_PATH_PROTOCOL *) ,
-(CONSOLE_OUT | CONSOLE_IN)
+(CONSOLE_OUT | CONSOLE_IN | STD_ERROR)
   },
   {
 NULL,
--
2.13.1.windows.2

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

Re: [edk2] [patch 2/2] MdeModulePkg/BdsDxe: Report Status Code when booting from BootOrder list

[Wang, Sunny (HPS SW)]

Looks good. This is what we discussed with Ray for adding BDS platform hook 
function. Thanks for implementing this, Dandan.
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dandan Bi
Sent: Monday, June 26, 2017 1:51 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni 
Subject: [edk2] [patch 2/2] MdeModulePkg/BdsDxe: Report Status Code when 
booting from BootOrder list

Report Status Code to indicate BDS starts attempting booting from the UEFI 
BootOrder list.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index b5e6ef6..ac5f908 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -3,11 +3,11 @@
   When this module was dispatched by DxeCore, gEfiBdsArchProtocolGuid will be 
installed
   which contains interface of BdsEntry.
   After DxeCore finish DXE phase, gEfiBdsArchProtocolGuid->BdsEntry will be 
invoked
   to enter BDS phase.
 
-Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.
 (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.  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 @@ -368,10 +368,15 @@ BootBootOptions (
   )
 {
   UINTN  Index;
 
   //
+  // Report Status Code to indicate BDS starts attempting booting from the 
UEFI BootOrder list.
+  //
+  REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_DXE_BS_DRIVER | 
+ EFI_SW_DXE_BS_PC_ATTEMPT_BOOT_ORDER_EVENT));
+
+  //
   // Attempt boot each boot option
   //
   for (Index = 0; Index < BootOptionCount; Index++) {
 //
 // According to EFI Specification, if a load option is not marked
--
1.9.5.msysgit.1

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

Re: [edk2] [patch 1/2] MdePkg/PiStatusCode: Add new Status Code for BDS when attempting BootOrder

[Wang, Sunny (HPS SW)]

Looks good. This is what we discussed with Ray for adding BDS platform hook 
function. Thanks for implementing this, Dandan.
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dandan Bi
Sent: Monday, June 26, 2017 1:51 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni 
Subject: [edk2] [patch 1/2] MdePkg/PiStatusCode: Add new Status Code for BDS 
when attempting BootOrder

According to new PI spec, add new Status Code to indicate BDS starts attempting 
booting from the UEFI BootOrder list.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 MdePkg/Include/Pi/PiStatusCode.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Pi/PiStatusCode.h b/MdePkg/Include/Pi/PiStatusCode.h
index 8a5e040..953585c 100644
--- a/MdePkg/Include/Pi/PiStatusCode.h
+++ b/MdePkg/Include/Pi/PiStatusCode.h
@@ -1,9 +1,9 @@
 /** @file
   StatusCode related definitions in PI.
 
-Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials are licensed and made available 
under  the terms and conditions of the BSD License that accompanies this 
distribution.  
 The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php.

 
@@ -788,10 +788,11 @@ typedef struct {
 #define EFI_SW_DXE_BS_PC_LEGACY_OPROM_INIT(EFI_SUBCLASS_SPECIFIC | 
0x)
 #define EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT  (EFI_SUBCLASS_SPECIFIC | 
0x0001)
 #define EFI_SW_DXE_BS_PC_LEGACY_BOOT_EVENT(EFI_SUBCLASS_SPECIFIC | 
0x0002)
 #define EFI_SW_DXE_BS_PC_EXIT_BOOT_SERVICES_EVENT (EFI_SUBCLASS_SPECIFIC | 
0x0003)
 #define EFI_SW_DXE_BS_PC_VIRTUAL_ADDRESS_CHANGE_EVENT (EFI_SUBCLASS_SPECIFIC | 
0x0004)
+#define EFI_SW_DXE_BS_PC_ATTEMPT_BOOT_ORDER_EVENT (EFI_SUBCLASS_SPECIFIC | 
0x0007)
 ///@}
 
 //
 // Software Class SMM Driver Subclass Progress Code definitions.
 //
--
1.9.5.msysgit.1

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot description for NVME

[Wang, Sunny (HPS SW)]

Never mind. I overlooked something. Thanks for clarifying this. 
Looks good to me. 
Reviewed-by: Sunny Wang <sunnyw...@hpe.com>

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Monday, March 13, 2017 10:24 AM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org
Cc: Tian, Feng <feng.t...@intel.com>; Haskell, Darrell <darrell.hask...@hpe.com>
Subject: RE: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot 
description for NVME
Importance: High

I didn't find this library in EDKII.
When I firstly saw your comments, I was a little bit unwilling to change 
UefiBootManagerLib to depend on UefiNvmExpressLib. Because it would require all 
platforms to include UefiNvmExpressLib in DSC.
But since now I find no such library, I feel much better

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Wang, Sunny (HPS SW)
> Sent: Friday, March 10, 2017 6:11 PM
> To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.t...@intel.com>; Haskell, Darrell 
> <darrell.hask...@hpe.com>
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib:
> Generate boot description for NVME
> 
> Hi Ray,
> Just a question out of curiosity. Why don't we use UefiNvmExpressLib's
> NvmExpressIdentify() for sending ADMIN_IDENTIFY command to NVME 
> controller?
> Others look good to me.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Ruiyu Ni
> Sent: Friday, March 10, 2017 3:58 PM
> To: edk2-devel@lists.01.org
> Cc: Feng Tian <feng.t...@intel.com>
> Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot 
> description for NVME
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Feng Tian <feng.t...@intel.com>
> ---
>  .../Library/UefiBootManagerLib/BmBootDescription.c | 104
> -
>  .../Library/UefiBootManagerLib/InternalBm.h|   4 +-
>  .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
>  3 files changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> index 050647d..501a0cc 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> @@ -1,7 +1,7 @@
>  /** @file
>Library functions which relate with boot option description.
> 
> -Copyright (c) 2011 - 2016, Intel Corporation. All rights 
> reserved.
> +Copyright (c) 2011 - 2017, Intel Corporation. All rights 
> +reserved.
>  (C) Copyright 2015 Hewlett Packard Enterprise Development LP  
> This program and the accompanying materials  are licensed and made 
> available under the terms and conditions of the BSD License @@ -501,6 
> +501,107 @@ BmGetLoadFileDescription (  }
> 
>  /**
> +  Return the boot description for NVME boot device.
> +
> +  @param HandleController handle.
> +
> +  @return  The description string.
> +**/
> +CHAR16 *
> +BmGetNvmeDescription (
> +  IN EFI_HANDLE  Handle
> +  )
> +{
> +  EFI_STATUS   Status;
> +  EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL   *NvmePassthru;
> +  EFI_DEV_PATH_PTR DevicePath;
> +  EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET CommandPacket;
> +  EFI_NVM_EXPRESS_COMMAND  Command;
> +  EFI_NVM_EXPRESS_COMPLETION   Completion;
> +  NVME_ADMIN_CONTROLLER_DATA   ControllerData;
> +  CHAR16   *Description;
> +  CHAR16   *Char;
> +  UINTNIndex;
> +
> +  Status = gBS->HandleProtocol (Handle, , 
> + (VOID **) );  if (EFI_ERROR (Status)) {
> +return NULL;
> +  }
> +
> +  Status = gBS->LocateDevicePath 
> + (,
> + , );  if (EFI_ERROR (Status) ||
> +  (DevicePathType (DevicePath.DevPath) != MESSAGING_DEVICE_PATH)
> ||
> +  (DevicePathSubType (DevicePath.DevPath) !=
> MSG_NVME_NAMESPACE_DP)) {
> +//
> +// Do not return description when the Handle is not a child of 
> + NVME
> controller.
> +//
> +return NULL;
> +  }
> +
> +  //
> +  // Send ADMIN_IDENTIFY command to NVME controller to get the model
> and serial number.
> +  //
> +  Status = gBS->HandleProtocol (Handle, 
> + , (VOID **) ); 
> + ASSERT_EFI_ERROR (Status);
> +
> +  ZeroMem (,
> + sizeof(EFI_NVM_EXPRESS_

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot description for NVME

[Wang, Sunny (HPS SW)]

Hi Ray,
Just a question out of curiosity. Why don't we use UefiNvmExpressLib's 
NvmExpressIdentify() for sending ADMIN_IDENTIFY command to NVME controller? 
Others look good to me. 

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Friday, March 10, 2017 3:58 PM
To: edk2-devel@lists.01.org
Cc: Feng Tian 
Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot 
description for NVME

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 .../Library/UefiBootManagerLib/BmBootDescription.c | 104 -
 .../Library/UefiBootManagerLib/InternalBm.h|   4 +-
 .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
 3 files changed, 107 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
index 050647d..501a0cc 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
@@ -1,7 +1,7 @@
 /** @file
   Library functions which relate with boot option description.
 
-Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP  This program 
and the accompanying materials  are licensed and made available under the terms 
and conditions of the BSD License @@ -501,6 +501,107 @@ 
BmGetLoadFileDescription (  }
 
 /**
+  Return the boot description for NVME boot device.
+
+  @param HandleController handle.
+
+  @return  The description string.
+**/
+CHAR16 *
+BmGetNvmeDescription (
+  IN EFI_HANDLE  Handle
+  )
+{
+  EFI_STATUS   Status;
+  EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL   *NvmePassthru;
+  EFI_DEV_PATH_PTR DevicePath;
+  EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET CommandPacket;
+  EFI_NVM_EXPRESS_COMMAND  Command;
+  EFI_NVM_EXPRESS_COMPLETION   Completion;
+  NVME_ADMIN_CONTROLLER_DATA   ControllerData;
+  CHAR16   *Description;
+  CHAR16   *Char;
+  UINTNIndex;
+
+  Status = gBS->HandleProtocol (Handle, , 
+ (VOID **) );  if (EFI_ERROR (Status)) {
+return NULL;
+  }
+
+  Status = gBS->LocateDevicePath (, 
+ , );  if (EFI_ERROR (Status) ||
+  (DevicePathType (DevicePath.DevPath) != MESSAGING_DEVICE_PATH) ||
+  (DevicePathSubType (DevicePath.DevPath) != MSG_NVME_NAMESPACE_DP)) {
+//
+// Do not return description when the Handle is not a child of NVME 
controller.
+//
+return NULL;
+  }
+
+  //
+  // Send ADMIN_IDENTIFY command to NVME controller to get the model and 
serial number.
+  //
+  Status = gBS->HandleProtocol (Handle, 
+ , (VOID **) );  
+ ASSERT_EFI_ERROR (Status);
+
+  ZeroMem (, 
+ sizeof(EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET));
+  ZeroMem (, sizeof(EFI_NVM_EXPRESS_COMMAND));  ZeroMem 
+ (, sizeof(EFI_NVM_EXPRESS_COMPLETION));
+
+  Command.Cdw0.Opcode = NVME_ADMIN_IDENTIFY_CMD;  //  // According to 
+ Nvm Express 1.1 spec Figure 38, When not used, the field shall be cleared to 
0h.
+  // For the Identify command, the Namespace Identifier is only used for the 
Namespace data structure.
+  //
+  Command.Nsid= 0;
+  CommandPacket.NvmeCmd= 
+  CommandPacket.NvmeCompletion =   
+ CommandPacket.TransferBuffer =   
+ CommandPacket.TransferLength = sizeof (ControllerData);  
+ CommandPacket.CommandTimeout = EFI_TIMER_PERIOD_SECONDS (5);
+  CommandPacket.QueueType  = NVME_ADMIN_QUEUE;
+  //
+  // Set bit 0 (Cns bit) to 1 to identify a controller  //
+  Command.Cdw10= 1;
+  Command.Flags= CDW10_VALID;
+
+  Status = NvmePassthru->PassThru (
+   NvmePassthru,
+   0,
+   ,
+   NULL
+   );
+  if (EFI_ERROR (Status)) {
+return NULL;
+  }
+
+  Description = AllocateZeroPool (
+  (ARRAY_SIZE (ControllerData.Mn) + 1
+   + ARRAY_SIZE (ControllerData.Sn) + 1
+   + MAXIMUM_VALUE_CHARACTERS + 1
+   ) * sizeof (CHAR16));  if (Description != NULL) {
+Char = Description;
+for (Index = 0; Index < ARRAY_SIZE (ControllerData.Mn); Index++) {
+  *(Char++) = (CHAR16) ControllerData.Mn[Index];
+}
+*(Char++) = L' ';
+for (Index = 0; Index < ARRAY_SIZE (ControllerData.Sn); Index++) {
+  *(Char++) = (CHAR16) ControllerData.Sn[Index];
+}
+*(Char++) = L' ';
+UnicodeValueToStringS (
+  Char, sizeof (CHAR16) * (MAXIMUM_VALUE_CHARACTERS + 

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize Handle before using it

[Wang, Sunny (HPS SW)]

Yeah, I misread the commit message and had a misunderstanding on what situation 
BmExpandMediaDevicePath() is called . 
Ray and Lazlo, thanks for both of you clarifying this. :)

Regards,
Sunny Wang

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, February 08, 2017 5:18 PM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; Ruiyu Ni <ruiyu...@intel.com>; 
edk2-devel@lists.01.org <edk2-de...@ml01.01.org>
Cc: Jeff Fan <jeff@intel.com>
Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize Handle 
before using it
Importance: High

On 02/08/17 09:13, Wang, Sunny (HPS SW) wrote:
> Looks good to me. 
> Reviewed-by: Sunny Wang <sunnyw...@hpe.com>
> 
> However, I saw the other potential issue below. If you also think the 
> potential issue is valid, you can fix this in the other patch.
> We need to consider the case where MDEPKG_NDEBUG is defined. We're 
> using ASSERT_EFI_ERROR to handle the error status from
> LocateDevicePath() in the following code block, which contains 
> potential bug that we still use uninitialized Handle for the case 
> where LocateDevicePath returns error status.
> 
> Status = gBS->LocateDevicePath (, 
> , ); ASSERT_EFI_ERROR (Status);

I think that Ray made a sufficient argument in the commit message why the 
assertion was correct -- it is indeed an assertion, and not error checking.

Since the function is called when the Handle supports BlockIo or
SimpleFileSystem, when there is no SimpleFileSystem installed on
the Handle, BlockIo should be installed on the Handle.

Perhaps the wording could be clarified, as in:

... BlockIo is *guaranteed* to be installed on the Handle.

Just my two cents anyway.
Laszlo

> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Ruiyu Ni
> Sent: Tuesday, February 07, 2017 10:54 AM
> To: edk2-devel@lists.01.org
> Cc: Jeff Fan <jeff@intel.com>
> Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize 
> Handle before using it
> 
> BmExpandMediaDevicePath contains a bug that it uses the uninitialized Handle.
> 
> Since the function is called when the Handle supports BlockIo or 
> SimpleFileSystem, when there is no SimpleFileSystem installed on the Handle, 
> BlockIo should be installed on the Handle.
> The fix initializes the Handle by locating the BlockIo protocol from the 
> device path.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Jeff Fan <jeff@intel.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 75bd5dc..9f99122 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -991,9 +991,13 @@ BmExpandMediaDevicePath (
>  return FileBuffer;
>}
>  
> +  Status = gBS->LocateDevicePath (, 
> + , );  ASSERT_EFI_ERROR (Status);
> +
>//
>// For device boot option only pointing to the removable device 
> handle,
> -  // should make sure all its children handles (its child partion or media 
> handles) are created and connected. 
> +  // should make sure all its children handles (its child partion or 
> + media handles)  // are created and connected.
>//
>gBS->ConnectController (Handle, NULL, NULL, TRUE);
>  
> @@ -1004,8 +1008,6 @@ BmExpandMediaDevicePath (
>// returned. After the Block IO protocol is reinstalled, subsequent
>// Block IO read/write will success.
>//
> -  Status = gBS->LocateDevicePath (, 
> , );
> -  ASSERT_EFI_ERROR (Status);
>Status = gBS->HandleProtocol (Handle, , (VOID **) 
> );
>ASSERT_EFI_ERROR (Status);
>Buffer = AllocatePool (BlockIo->Media->BlockSize);
> --
> 2.9.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize Handle before using it

[Wang, Sunny (HPS SW)]

Looks good to me. 
Reviewed-by: Sunny Wang 

However, I saw the other potential issue below. If you also think the potential 
issue is valid, you can fix this in the other patch.
We need to consider the case where MDEPKG_NDEBUG is defined. We're using 
ASSERT_EFI_ERROR to handle the error status from LocateDevicePath() in the 
following code block, which contains potential bug that we still use 
uninitialized Handle for the case where LocateDevicePath returns error status. 

Status = gBS->LocateDevicePath (, , 
);  
ASSERT_EFI_ERROR (Status);

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, February 07, 2017 10:54 AM
To: edk2-devel@lists.01.org
Cc: Jeff Fan 
Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize Handle 
before using it

BmExpandMediaDevicePath contains a bug that it uses the uninitialized Handle.

Since the function is called when the Handle supports BlockIo or 
SimpleFileSystem, when there is no SimpleFileSystem installed on the Handle, 
BlockIo should be installed on the Handle.
The fix initializes the Handle by locating the BlockIo protocol from the device 
path.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jeff Fan 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 75bd5dc..9f99122 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -991,9 +991,13 @@ BmExpandMediaDevicePath (
 return FileBuffer;
   }
 
+  Status = gBS->LocateDevicePath (, 
+ , );  ASSERT_EFI_ERROR (Status);
+
   //
   // For device boot option only pointing to the removable device handle,
-  // should make sure all its children handles (its child partion or media 
handles) are created and connected. 
+  // should make sure all its children handles (its child partion or 
+ media handles)  // are created and connected.
   //
   gBS->ConnectController (Handle, NULL, NULL, TRUE);
 
@@ -1004,8 +1008,6 @@ BmExpandMediaDevicePath (
   // returned. After the Block IO protocol is reinstalled, subsequent
   // Block IO read/write will success.
   //
-  Status = gBS->LocateDevicePath (, , 
);
-  ASSERT_EFI_ERROR (Status);
   Status = gBS->HandleProtocol (Handle, , (VOID **) 
);
   ASSERT_EFI_ERROR (Status);
   Buffer = AllocatePool (BlockIo->Media->BlockSize);
--
2.9.0.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Remove redundant type cast

[Wang, Sunny (HPS SW)]

Looks good to me. 
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao Wu
Sent: Friday, January 06, 2017 4:53 PM
To: edk2-devel@lists.01.org
Cc: Hao Wu ; Ruiyu Ni 
Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Remove redundant type 
cast

The type of return value for function EfiBootManagerFindLoadOption() is INTN. 
When checking its return value, it is unnecessary to type cast -1 to type UINTN.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 6b84b85..75bd5dc 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1,7 +1,7 @@
 /** @file
   Library functions which relates with booting.
 
-Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
 (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP  This 
program and the accompanying materials  are licensed and made available under 
the terms and conditions of the BSD License @@ -2152,7 +2152,7 @@ 
EfiBootManagerRefreshAllBootOption (
   // Only check those added by BDS
   // so that the boot options added by end-user or OS installer won't be 
deleted
   //
-  if (EfiBootManagerFindLoadOption ([Index], BootOptions, 
BootOptionCount) == (UINTN) -1) {
+  if (EfiBootManagerFindLoadOption ([Index], 
+ BootOptions, BootOptionCount) == -1) {
 Status = EfiBootManagerDeleteLoadOptionVariable 
(NvBootOptions[Index].OptionNumber, LoadOptionTypeBoot);
 //
 // Deleting variable with current variable implementation shouldn't 
fail.
@@ -2166,7 +2166,7 @@ EfiBootManagerRefreshAllBootOption (
   // Add new EFI boot options to NV
   //
   for (Index = 0; Index < BootOptionCount; Index++) {
-if (EfiBootManagerFindLoadOption ([Index], NvBootOptions, 
NvBootOptionCount) == (UINTN) -1) {
+if (EfiBootManagerFindLoadOption ([Index], 
+ NvBootOptions, NvBootOptionCount) == -1) {
   EfiBootManagerAddLoadOptionVariable ([Index], (UINTN) -1);
   //
   // Try best to add the boot options so continue upon failure.
--
1.9.5.msysgit.0

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

Re: [edk2] [PATCH] MdeModulePkg/Bds: Fix a bug that may causes S4 fails to resume

[Wang, Sunny (HPS SW)]

Looks good to me. 
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Thursday, January 05, 2017 11:18 AM
To: edk2-devel@lists.01.org
Cc: Jiewen Yao 
Subject: [edk2] [PATCH] MdeModulePkg/Bds: Fix a bug that may causes S4 fails to 
resume

When firmware boots to UiApp, the memory type information settings are saved to 
NV storage and the settings in HOB are changed as well.
Because UiApp is an APPLICATION type of boot option, system doesn't reset when 
settings change.
But when user selects OS to boot in UiApp, because the settings in HOB was 
updated when booting to UiApp, the BDS doesn't think the settings change, 
expected reset doesn't happen.

The patch fixes this issue to not update the settings in HOB.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jiewen Yao 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
index 09e4211..cf0455f 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
@@ -1,7 +1,7 @@
 /** @file
   Misc library functions.
 
-Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
 (C) Copyright 2016 Hewlett Packard Enterprise Development LP  This program 
and the accompanying materials  are licensed and made available under the terms 
and conditions of the BSD License @@ -205,8 +205,11 @@ 
BmSetMemoryTypeInformationVariable (
 //
 return;
   }
-  PreviousMemoryTypeInformation = GET_GUID_HOB_DATA (GuidHob);
-  VariableSize = GET_GUID_HOB_DATA_SIZE (GuidHob);
+  VariableSize  = GET_GUID_HOB_DATA_SIZE (GuidHob);
+  PreviousMemoryTypeInformation = AllocateCopyPool (VariableSize, 
+ GET_GUID_HOB_DATA (GuidHob));  if (PreviousMemoryTypeInformation == NULL) {
+return;
+  }
 
   //
   // Use a heuristic to adjust the Memory Type Information for the next boot 
@@ -278,14 +281,18 @@ BmSetMemoryTypeInformationVariable (
   // then reset the platform so the new Memory Type Information setting 
will be used to guarantee that an S4
   // entry/resume cycle will not fail.
   //
-  if (MemoryTypeInformationModified && Boot && PcdGetBool 
(PcdResetOnMemoryTypeInformationChange)) {
-DEBUG ((EFI_D_INFO, "Memory Type Information settings change. Warm 
Reset!!!\n"));
-gRT->ResetSystem (EfiResetWarm, EFI_SUCCESS, 0, NULL);
+  if (MemoryTypeInformationModified) {
+DEBUG ((EFI_D_INFO, "Memory Type Information settings change.\n"));
+if (Boot && PcdGetBool (PcdResetOnMemoryTypeInformationChange)) {
+  DEBUG ((EFI_D_INFO, "...Warm Reset!!!\n"));
+  gRT->ResetSystem (EfiResetWarm, EFI_SUCCESS, 0, NULL);
+}
   }
 } else {
   DEBUG ((EFI_D_ERROR, "Memory Type Information settings cannot be saved. 
OS S4 may fail!\n"));
 }
   }
+  FreePool (PreviousMemoryTypeInformation);
 }
 
 /**
--
2.9.0.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg/BdsDxe: Initialize gConnectConInEvent earlier

[Wang, Sunny (HPS SW)]

Looks good to me. 
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, December 05, 2016 9:57 AM
To: edk2-devel@lists.01.org
Cc: Eric Dong ; Chao B Zhang 
Subject: [edk2] [PATCH] MdeModulePkg/BdsDxe: Initialize gConnectConInEvent 
earlier

PlatformBootManagerBeforeConsole() might want to display UI and ReadKeyStroke() 
call from UI depends on this event to connect ConIn so BdsDxe initializes this 
event before PlatformBootManagerBeforeConsole().

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Eric Dong 
Cc: Chao B Zhang 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 31 ++-
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 98b3931..b5e6ef6 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -874,6 +874,23 @@ BdsEntry (
 );
 
   //
+  // Initialize ConnectConIn event before calling platform code.
+  //
+  if (PcdGetBool (PcdConInConnectOnDemand)) {
+Status = gBS->CreateEventEx (
+EVT_NOTIFY_SIGNAL,
+TPL_CALLBACK,
+BdsDxeOnConnectConInCallBack,
+NULL,
+,
+
+);
+if (EFI_ERROR (Status)) {
+  gConnectConInEvent = NULL;
+}
+  }
+
+  //
   // Do the platform init, can be customized by OEM/IBV
   // Possible things that can be done in PlatformBootManagerBeforeConsole:
   // > Update console variable: 1. include hot-plug devices; 2. Clear ConIn 
and add SOL for AMT @@ -905,21 +922,9 @@ BdsEntry (
   if (PcdGetBool (PcdConInConnectOnDemand)) {
 EfiBootManagerConnectConsoleVariable (ConOut);
 EfiBootManagerConnectConsoleVariable (ErrOut);
-
 //
-// Initialize ConnectConIn event
+// Do not connect ConIn devices when lazy ConIn feature is ON.
 //
-Status = gBS->CreateEventEx (
-EVT_NOTIFY_SIGNAL,
-TPL_CALLBACK,
-BdsDxeOnConnectConInCallBack,
-NULL,
-,
-
-);
-if (EFI_ERROR (Status)) {
-  gConnectConInEvent = NULL;
-}
   } else {
 EfiBootManagerConnectAllDefaultConsoles ();
   }
--
2.9.0.windows.1

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

Re: [edk2] [PATCH 3/3] MdeModulePkg/BdsDxe: Avoid overwriting PlatformRecovery####

[Wang, Sunny (HPS SW)]

Hi Ray,
This patch looks good to me. 
Reviewed-by: Sunny Wang 

Just a suggestion. If we can add the code block below into 
BmGetFreeOptionNumber(), I think it would be better.

+  LoadOptions = EfiBootManagerGetLoadOptions (, 
+ LoadOptionTypePlatformRecovery);  if (EfiBootManagerFindLoadOption 
(, LoadOptions, LoadOptionCount) == -1) {
+for (Index = 0; Index < LoadOptionCount; Index++) {
+  //
+  // The PlatformRecovery options are sorted by OptionNumber.
+  // Find the the smallest unused number as the new OptionNumber.
+  //
+  if (LoadOptions[Index].OptionNumber != Index) {
+break;
+  }
+}
+LoadOption.OptionNumber = Index;
+Status = EfiBootManagerLoadOptionToVariable ();
+ASSERT_EFI_ERROR (Status);
+  }

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, November 15, 2016 6:04 PM
To: edk2-devel@lists.01.org
Cc: Jie Lin 
Subject: [edk2] [PATCH 3/3] MdeModulePkg/BdsDxe: Avoid overwriting 
PlatformRecovery

Current implementation always creates PlatformRecovery pointing to 
\EFI\BOOT\BOOT$(ARCH).efi but it may overwrite PlatformRecovery created 
before (maybe by a DXE driver).

The patch only uses the smallest unused option number for the 
\EFI\BOOT\BOOT$(ARCH).efi PlatformRecovery to avoid overwriting 
already-created PlatformRecovery.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jie Lin 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 48e5351..56034f5 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -834,7 +834,7 @@ BdsEntry (
   FilePath = FileDevicePath (NULL, EFI_REMOVABLE_MEDIA_FILE_NAME);
   Status = EfiBootManagerInitializeLoadOption (
  ,
- 0,
+ LoadOptionNumberUnassigned,
  LoadOptionTypePlatformRecovery,
  LOAD_OPTION_ACTIVE,
  L"Default PlatformRecovery", @@ -843,9 +843,24 @@ BdsEntry (
  0
  );
   ASSERT_EFI_ERROR (Status);
-  EfiBootManagerLoadOptionToVariable ();
+  LoadOptions = EfiBootManagerGetLoadOptions (, 
+ LoadOptionTypePlatformRecovery);  if (EfiBootManagerFindLoadOption 
(, LoadOptions, LoadOptionCount) == -1) {
+for (Index = 0; Index < LoadOptionCount; Index++) {
+  //
+  // The PlatformRecovery options are sorted by OptionNumber.
+  // Find the the smallest unused number as the new OptionNumber.
+  //
+  if (LoadOptions[Index].OptionNumber != Index) {
+break;
+  }
+}
+LoadOption.OptionNumber = Index;
+Status = EfiBootManagerLoadOptionToVariable ();
+ASSERT_EFI_ERROR (Status);
+  }
   EfiBootManagerFreeLoadOption ();
   FreePool (FilePath);
+  EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
 
   //
   // Report Status Code to indicate connecting drivers will happen
--
2.9.0.windows.1

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

Re: [edk2] [PATCH 2/3] MdeModulePkg/BdsDxe: Fix bug to run non-first PlatformRecovery####

[Wang, Sunny (HPS SW)]

Looks good to me. 
Reviewed-by: Sunny Wang 

Just have a suggestion below. 
For the change below, it is related to Driver rather than 
PlatformRecovery. Could you add more comment for this Driver related 
change into your log? It's fine to not update your patch. You can directly 
update it when you check in your code change.
+
+  if (!EFI_ERROR (LoadOptions[Index].Status) &&
+  (LoadOptions[Index].Attributes & LOAD_OPTION_FORCE_RECONNECT) 
+ != 0) {
 ReconnectAll = TRUE;
   }

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, November 15, 2016 6:04 PM
To: edk2-devel@lists.01.org
Cc: Jie Lin 
Subject: [edk2] [PATCH 2/3] MdeModulePkg/BdsDxe: Fix bug to run non-first 
PlatformRecovery

The implementation doesn't check the LoadOptions[Index].Status but only depends 
on the Status returned from EfiBootManagerProcessLoadOption(), which results 
only the first PlatformRecovery runs.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jie Lin 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 72adc51..48e5351 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -443,17 +443,25 @@ ProcessLoadOptions (
   LoadOptionType = LoadOptions[Index].OptionType;
 }
 ASSERT (LoadOptionType == LoadOptions[Index].OptionType);
+ASSERT (LoadOptionType != LoadOptionTypeBoot);
 
 Status = EfiBootManagerProcessLoadOption ([Index]);
 
+//
+// Status indicates whether the load option is loaded and executed
+// LoadOptions[Index].Status is what the load option returns
+//
 if (!EFI_ERROR (Status)) {
-  if (LoadOptionType == LoadOptionTypePlatformRecovery) {
-//
-// Stop processing if any entry is successful
-//
+  //
+  // Stop processing if any PlatformRecovery returns success.
+  //
+  if ((LoadOptions[Index].Status == EFI_SUCCESS) &&
+  (LoadOptionType == LoadOptionTypePlatformRecovery)) {
 break;
   }
-  if ((LoadOptions[Index].Attributes & LOAD_OPTION_FORCE_RECONNECT) != 0) {
+
+  if (!EFI_ERROR (LoadOptions[Index].Status) &&
+  (LoadOptions[Index].Attributes & LOAD_OPTION_FORCE_RECONNECT) 
+ != 0) {
 ReconnectAll = TRUE;
   }
 }
--
2.9.0.windows.1

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

Re: [edk2] [PATCH 1/3] MdeModulePkg/UefiBootManagerLib: Refine the debug message

[Wang, Sunny (HPS SW)]

Looks good to me. 
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, November 15, 2016 6:04 PM
To: edk2-devel@lists.01.org
Cc: Eric Dong 
Subject: [edk2] [PATCH 1/3] MdeModulePkg/UefiBootManagerLib: Refine the debug 
message

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Eric Dong 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index e638e5f..6f705bd 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1277,8 +1277,9 @@ EfiBootManagerProcessLoadOption (
   // Load and start the load option.
   //
   DEBUG ((
-DEBUG_INFO | DEBUG_LOAD, "Process Load Option (%s%04x) ...\n",
-mBmLoadOptionName[LoadOption->OptionType], LoadOption->OptionNumber
+DEBUG_INFO | DEBUG_LOAD, "Process %s%04x (%s) ...\n",
+mBmLoadOptionName[LoadOption->OptionType], LoadOption->OptionNumber,
+LoadOption->Description
 ));
   ImageHandle = NULL;
   FileBuffer = EfiBootManagerGetLoadOptionBuffer (LoadOption->FilePath, 
, ); @@ -1321,7 +1322,7 @@ EfiBootManagerProcessLoadOption (
 
 LoadOption->Status = gBS->StartImage (ImageHandle, 
>ExitDataSize, >ExitData);
 DEBUG ((
-  DEBUG_INFO | DEBUG_LOAD, "Load Option (%s%04x) Return Status = %r\n",
+  DEBUG_INFO | DEBUG_LOAD, "%s%04x Return Status = %r\n",
   mBmLoadOptionName[LoadOption->OptionType], LoadOption->OptionNumber, 
LoadOption->Status
   ));
 
--
2.9.0.windows.1

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

Re: [edk2] [PATCH v2 0/9] Defer 3rd party images loading to after EndOfDxe

[Wang, Sunny (HPS SW)]

Reviewed all BDS related patches including PlatformBds libraries. All look good 
to me.  
Thanks for offline addressing my PlatformBds related questions.

Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, November 08, 2016 8:29 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v2 0/9] Defer 3rd party images loading to after EndOfDxe

The patches change the default image loading policy by deferring 3rd party 
images loading to after EndOfDxe and add a new BDS API to dispatch the deferred 
images.

Platform needs to call the new BDS API
EfiBootManagerDispatchDeferredImages after EndOfDxe to ensure that any deferred 
images are loaded.

v2 puts the deferred images checking in DEBUG_CODE macro, corrects a typo of 
function name, and puts CpuDeadLoop() after ASSERT(FALSE).
v2 includes all the platform changes.

Ruiyu Ni (9):
  MdeModulePkg/SecurityStubDxe: Defer 3rd party image before EndOfDxe
  MdeModulePkg/UefiBootManager: Add EfiBootManagerDispatchDeferredImages
  MdeModulePkg/BdsDxe: Check deferred images before booting to OS
  MdeModulePkg/SecurityStubDxe: Report failure if image is load earlier
  ArmVirPkg/PlatformBds: Dispatch deferred images after EndOfDxe
  OvmfPkg/PlatformBds: Dispatch deferred images after EndOfDxe
  CorebootPayload/PlatformBds: Dispatch deferred images after EndOfDxe
  QuarkPlatformPkg/PlatformBds: Dispatch deferred images after EndOfDxe
  Nt32Pkg/PlatformBds: Dispatch deferred images after EndOfDxe

 .../Library/PlatformBootManagerLib/PlatformBm.c|   5 +
 .../PlatformBootManagerLib/PlatformBootManager.c   |   5 +
 MdeModulePkg/Include/Library/UefiBootManagerLib.h  |  13 +
 MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c   | 113 ++
 .../Library/UefiBootManagerLib/InternalBm.h|   1 +
 .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
 MdeModulePkg/Universal/BdsDxe/Bds.h|   4 +-
 MdeModulePkg/Universal/BdsDxe/BdsDxe.inf   |   2 +
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c   |  90 +
 .../SecurityStubDxe/Defer3rdPartyImageLoad.c   | 414 +
 .../SecurityStubDxe/Defer3rdPartyImageLoad.h   |  95 +
 .../Universal/SecurityStubDxe/SecurityStub.c   |  14 +-
 .../Universal/SecurityStubDxe/SecurityStubDxe.inf  |  11 +-
 .../PlatformBootManagerLib/PlatformBootManager.c   |   5 +
 .../Library/PlatformBootManagerLib/BdsPlatform.c   |   5 +
 .../PlatformBootManagerLib/PlatformBootManager.c   |   7 +-
 16 files changed, 781 insertions(+), 4 deletions(-)  create mode 100644 
MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c
 create mode 100644 
MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.h

--
2.9.0.windows.1

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

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

[Wang, Sunny (HPS SW)]

ol (ExitData);
+}
+
+//
+// Clear the Watchdog Timer after the image returns.
+//
+gBS->SetWatchdogTimer (0x, 0x, 0x, 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 
 #include 
 #include 
+#include 
 
 #include   #include  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
--- Begin Message ---


Thanks/Ray

> -Original Message-
> From: Wang, Sunny (HPS SW) [mailto:sunnyw...@hpe.com]
> Sent: Friday, November 4, 2016 7:42 PM
> To: Ni, Ruiyu <ruiyu...@intel.com>
> Cc: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; Haskell, Darrell
> <darrell.hask...@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-boun...@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.zh...@intel.com>; Liming Gao
> <liming@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...@intel.com>
> Cc: Liming Gao <liming@intel.com>
> Cc: Chao B Zhang <chao.b.zh...@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/UefiBootMana

Re: [edk2] [PATCH v2 3/9] MdeModulePkg/BdsDxe: Check deferred images before booting to OS

[Wang, Sunny (HPS SW)]

 DEBUG ((DEBUG_LOAD, "[Bds] Image was deferred but not loaded: %s.\n", 
DevicePathStr));
+  if (DevicePathStr != NULL) {
+FreePool (DevicePathStr);
+  }
+}
+  }
+  if (Handles != NULL) {
+FreePool (Handles);
+  }
+}
 
 /**
 
@@ -119,6 +194,21 @@ BdsInitialize (
   );
   ASSERT_EFI_ERROR (Status);
 
+  DEBUG_CODE (
+EFI_EVENT   Event;
+//
+// Register notify function to check deferred images on ReadyToBoot Event.
+//
+Status = gBS->CreateEventEx (
+EVT_NOTIFY_SIGNAL,
+TPL_CALLBACK,
+CheckDeferredLoadImageOnReadyToBoot,
+NULL,
+,
+
+);
+ASSERT_EFI_ERROR (Status);
+  );
   return Status;
 }
 
--
2.9.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
--- Begin Message ---


Thanks/Ray

> -Original Message-
> From: Wang, Sunny (HPS SW) [mailto:sunnyw...@hpe.com]
> Sent: Friday, November 4, 2016 7:34 PM
> To: Ni, Ruiyu <ruiyu...@intel.com>
> Cc: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; Haskell, Darrell
> <darrell.hask...@hpe.com>
> Subject: RE: [edk2] [PATCH 3/4] MdeModulePkg/BdsDxe: Check deferred
> images before booting to OS
>
> Hi Ray,
> Remove others. Feel free to add some people back.
> CheckDeferredLoadImageOnReadyToBoot()  is almost the same as
> CheckDeferredImage() in Intel Central park reference code, so It looks good
> to me.
> However, I have a question and a suggestion below:
> Question:
>   - ReadyToBoot event is possibly signaled more than once during a 
> boot.
> Why do we need to call CheckDeferredLoadImageOnReadyToBoot() more
> than once during a boot? If it is related to use cases of deferred image, 
> could
> you give more information? I'm not familiar with the use cases of the
> deferred image, but I would like to know if it is worth for HPE sever to take
> little additional boot time to call CheckDeferredLoadImageOnReadyToBoot()
> more than once during a boot. Thanks in advance.

Yes I agree calling this more than once is unnecessary. I will change the patch
to call it only once.

>
> Suggestions:
>  - It looks like CheckDeferredLoadImageOnReadyToBoot() is only for
> printing DEBUG messages. Is there anything which I overlook?  If not, is it a
> good idea to add this CheckDeferredLoadImage() as a UefiBootManagerLib
> API instead of adding this as a Notify function in BdsDxe?  So that all the
> companies can decide whether/when they want to call
> CheckDeferredLoadImage() in BDS platform code instead of being forced
> calling it with ReadyToBoot event signaled in BDS core code.

I see your needs now. So how about I only register this callback in DEBUG
build, surrounding the code with DEBUG_CODE() macro?
Make this as an API requires platform manually register this callback, which
I think is not very convenient to platform.

>
> Regards,
> Sunny Wang
>
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@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.zh...@intel.com>; Liming Gao
> <liming@intel.com>
> Subject: [edk2] [PATCH 3/4] MdeModulePkg/BdsDxe: Check deferred
> images before booting to OS
>
> The patch adds check of deferred images before booting to OS.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Liming Gao <liming@intel.com>
> Cc: Chao B Zhang <chao.b.zh...@intel.com>
> ---
>  MdeModulePkg/Universal/BdsDxe/Bds.h  |  4 +-
>  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf |  2 +
> MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 89
> 
>  3 files changed, 94 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Universal/BdsDxe/Bds.h
> b/MdeModulePkg/Universal/BdsDxe/Bds.h
> index d243932..1f8a192 100644
> --- a/MdeModulePkg/Universal/BdsDxe/Bds.h
> +++ b/MdeModulePkg/Universal/BdsDxe/Bds.h
> @@ -1,7 +1,7 @@
>  /** @file
>Head file for BDS Architectural Protocol implementation
>
> -Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
>  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
> @@ -20,10 +20,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #include 
>  #inclu

Re: [edk2] [PATCH v2] MdeModulePkg UefiBootManagerLib: Rename BootMenuApp to BootManagerMenu

[Wang, Sunny (HPS SW)]

Looks good to me.
Reviewed-by: Sunny Wang 

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Thursday, September 01, 2016 2:05 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v2] MdeModulePkg UefiBootManagerLib: Rename BootMenuApp 
to BootManagerMenu

Rename local function name BootMenuApp to BootManagerMenu to align to other 
public function name.

In V2, use "BootManagerMenu" instead of "BootMenuApp".

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
Reviewed-by: Ruiyu Ni 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index ecd0ae3..fe09a04 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1530,15 +1530,15 @@ EfiBootManagerGetLoadOptionBuffer (  }
 
 /**
-  Check if it's a Device Path pointing to BootMenuApp.
+  Check if it's a Device Path pointing to BootManagerMenu.
 
   @param  DevicePath Input device path.
 
-  @retval TRUE   The device path is BootMenuApp File Device Path.
-  @retval FALSE  The device path is NOT BootMenuApp File Device Path.
+  @retval TRUE   The device path is BootManagerMenu File Device Path.
+  @retval FALSE  The device path is NOT BootManagerMenu File Device Path.
 **/
 BOOLEAN
-BmIsBootMenuAppFilePath (
+BmIsBootManagerMenuFilePath (
   EFI_DEVICE_PATH_PROTOCOL *DevicePath
 )
 {
@@ -1645,7 +1645,7 @@ EfiBootManagerBoot (
   // 3. Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to load 
and execute
   //the boot option.
   //
-  if (BmIsBootMenuAppFilePath (BootOption->FilePath)) {
+  if (BmIsBootManagerMenuFilePath (BootOption->FilePath)) {
 DEBUG ((EFI_D_INFO, "[Bds] Booting Boot Manager Menu.\n"));
 BmStopHotkeyService (NULL, NULL);
   } else {
@@ -2080,10 +2080,10 @@ BmEnumerateBootOptions (
 ASSERT (BootOptions != NULL);
 
 //
-// If LoadFile includes BootMenuApp, its boot attribue will be set to APP 
and HIDDEN.
+// If LoadFile includes BootManagerMenu, its boot attribue will be set to 
APP and HIDDEN.
 //
 BootAttributes = LOAD_OPTION_ACTIVE;
-if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
+if (BmIsBootManagerMenuFilePath (DevicePathFromHandle 
+ (Handles[Index]))) {
   BootAttributes = LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | 
LOAD_OPTION_HIDDEN;
 }
 
@@ -2215,7 +2215,7 @@ BmRegisterBootManagerMenu (
   DevicePath = NULL;
   Description = NULL;
   //
-  // Try to find BootMenuApp from LoadFile protocol
+  // Try to find BootManagerMenu from LoadFile protocol
   //
   gBS->LocateHandleBuffer (
  ByProtocol,
@@ -2225,7 +2225,7 @@ BmRegisterBootManagerMenu (
  
  );
   for (Index = 0; Index < HandleCount; Index++) {
-if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
+if (BmIsBootManagerMenuFilePath (DevicePathFromHandle 
+ (Handles[Index]))) {
   DevicePath  = DuplicateDevicePath (DevicePathFromHandle 
(Handles[Index]));
   Description = BmGetBootDescription (Handles[Index]);
   break;
@@ -2334,7 +2334,7 @@ EfiBootManagerGetBootManagerMenu (
   BootOptions = EfiBootManagerGetLoadOptions (, 
LoadOptionTypeBoot);
 
   for (Index = 0; Index < BootOptionCount; Index++) {
-if (BmIsBootMenuAppFilePath (BootOptions[Index].FilePath)) {
+if (BmIsBootManagerMenuFilePath (BootOptions[Index].FilePath)) {
 Status = EfiBootManagerInitializeLoadOption (
BootOption,
BootOptions[Index].OptionNumber,
--
2.8.0.windows.1

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

Re: [edk2] [PATCH v2] MdeModulePkg UefiBootManagerLib: Ignore BootManagerMenu from LoadFile

[Wang, Sunny (HPS SW)]

Looks good to me.
Reviewed-by: Sunny Wang 

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Thursday, September 01, 2016 2:05 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v2] MdeModulePkg UefiBootManagerLib: Ignore 
BootManagerMenu from LoadFile

BootManagerMenu boot option is handled by EfiBootManagerGetBootManagerMenu.
Don't need to handle it again when parse LoadFile protocol.

In V2, use "BootManagerMenu" instead of "BootMenuApp".

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
Reviewed-by: Ruiyu Ni 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index fe09a04..6b84b85 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1940,7 +1940,6 @@ BmEnumerateBootOptions (
   UINTN Removable;
   UINTN Index;
   CHAR16*Description;
-  UINT32BootAttributes;
 
   ASSERT (BootOptionCount != NULL);
 
@@ -2070,6 +2069,12 @@ BmEnumerateBootOptions (
  
  );
   for (Index = 0; Index < HandleCount; Index++) {
+//
+// Ignore BootManagerMenu. its boot option will be created by 
EfiBootManagerGetBootManagerMenu().
+//
+if (BmIsBootManagerMenuFilePath (DevicePathFromHandle (Handles[Index]))) {
+  continue;
+}
 
 Description = BmGetBootDescription (Handles[Index]);
 BootOptions = ReallocatePool (
@@ -2079,19 +2084,11 @@ BmEnumerateBootOptions (
 );
 ASSERT (BootOptions != NULL);
 
-//
-// If LoadFile includes BootManagerMenu, its boot attribue will be set to 
APP and HIDDEN.
-//
-BootAttributes = LOAD_OPTION_ACTIVE;
-if (BmIsBootManagerMenuFilePath (DevicePathFromHandle (Handles[Index]))) {
-  BootAttributes = LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | 
LOAD_OPTION_HIDDEN;
-}
-
 Status = EfiBootManagerInitializeLoadOption (
[(*BootOptionCount)++],
LoadOptionNumberUnassigned,
LoadOptionTypeBoot,
-   BootAttributes,
+   LOAD_OPTION_ACTIVE,
Description,
DevicePathFromHandle (Handles[Index]),
NULL,
-- 
2.8.0.windows.1

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

Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore BootManagerMenuApp from LoadFile

[Wang, Sunny (HPS SW)]

Got it. Thanks for confirming this and working on the replacement.

Regards,
Sunny Wang

-Original Message-
From: Gao, Liming [mailto:liming@intel.com] 
Sent: Thursday, September 01, 2016 10:20 AM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu...@intel.com>; Dong, Eric <eric.d...@intel.com>
Subject: RE: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore 
BootManagerMenuApp from LoadFile
Importance: High

Sunny:
  Yes. This is the replacement of Eric patch. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Wang, Sunny (HPS SW)
> Sent: Thursday, September 01, 2016 10:18 AM
> To: Gao, Liming <liming@intel.com>; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Dong, Eric <eric.d...@intel.com>
> Subject: Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore 
> BootManagerMenuApp from LoadFile
> 
> Hi Liming,
> Except Ray's comment, others Look good to me.
> Reviewed-by: Sunny Wang <sunnyw...@hpe.com>
> 
> Hi Ray and Eric,
> It looks like this code change is used for replacing the one which we 
> offline discussed to fix the duplicated Boot Manager menu issue, isn't it?
>  - [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Add OptioalData for 
> boot option.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Liming Gao
> Sent: Wednesday, August 31, 2016 4:19 PM
> To: edk2-devel@lists.01.org
> Cc: Ruiyu Ni <ruiyu...@intel.com>; Eric Dong <eric.d...@intel.com>
> Subject: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore 
> BootManagerMenuApp from LoadFile
> 
> BootManagerMenuApp boot option is handled by 
> EfiBootManagerGetBootManagerMenu.
> Don't need to handle it again when parse LoadFile protocol.
> 
> Cc: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao <liming@intel.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 17 +++---
> ---
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index ecd0ae3..f8a3988 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1940,7 +1940,6 @@ BmEnumerateBootOptions (
>UINTN Removable;
>UINTN Index;
>CHAR16*Description;
> -  UINT32BootAttributes;
> 
>ASSERT (BootOptionCount != NULL);
> 
> @@ -2070,6 +2069,12 @@ BmEnumerateBootOptions (
>   
>   );
>for (Index = 0; Index < HandleCount; Index++) {
> +//
> +// Ignore BootMenuApp. its boot option will be created by
> BmRegisterBootManagerMenu().
> +//
> +if (BmIsBootMenuAppFilePath (DevicePathFromHandle 
> + (Handles[Index])))
> {
> +  continue;
> +}
> 
>  Description = BmGetBootDescription (Handles[Index]);
>  BootOptions = ReallocatePool (
> @@ -2079,19 +2084,11 @@ BmEnumerateBootOptions (
>  );
>  ASSERT (BootOptions != NULL);
> 
> -//
> -// If LoadFile includes BootMenuApp, its boot attribue will be set to APP
> and HIDDEN.
> -//
> -BootAttributes = LOAD_OPTION_ACTIVE;
> -if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index])))
> {
> -  BootAttributes = LOAD_OPTION_CATEGORY_APP |
> LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN;
> -}
> -
>  Status = EfiBootManagerInitializeLoadOption (
> [(*BootOptionCount)++],
> LoadOptionNumberUnassigned,
> LoadOptionTypeBoot,
> -   BootAttributes,
> +   LOAD_OPTION_ACTIVE,
> Description,
> DevicePathFromHandle (Handles[Index]),
> NULL,
> --
> 2.8.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore BootManagerMenuApp from LoadFile

[Wang, Sunny (HPS SW)]

Hi Liming, 
Except Ray's comment, others Look good to me. 
Reviewed-by: Sunny Wang 

Hi Ray and Eric, 
It looks like this code change is used for replacing the one which we offline 
discussed to fix the duplicated Boot Manager menu issue, isn't it?  
 - [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Add OptioalData for boot 
option.

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Wednesday, August 31, 2016 4:19 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Eric Dong 
Subject: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore 
BootManagerMenuApp from LoadFile

BootManagerMenuApp boot option is handled by EfiBootManagerGetBootManagerMenu.
Don't need to handle it again when parse LoadFile protocol.

Cc: Ruiyu Ni 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index ecd0ae3..f8a3988 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1940,7 +1940,6 @@ BmEnumerateBootOptions (
   UINTN Removable;
   UINTN Index;
   CHAR16*Description;
-  UINT32BootAttributes;
 
   ASSERT (BootOptionCount != NULL);
 
@@ -2070,6 +2069,12 @@ BmEnumerateBootOptions (
  
  );
   for (Index = 0; Index < HandleCount; Index++) {
+//
+// Ignore BootMenuApp. its boot option will be created by 
BmRegisterBootManagerMenu().
+//
+if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
+  continue;
+}
 
 Description = BmGetBootDescription (Handles[Index]);
 BootOptions = ReallocatePool (
@@ -2079,19 +2084,11 @@ BmEnumerateBootOptions (
 );
 ASSERT (BootOptions != NULL);
 
-//
-// If LoadFile includes BootMenuApp, its boot attribue will be set to APP 
and HIDDEN.
-//
-BootAttributes = LOAD_OPTION_ACTIVE;
-if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
-  BootAttributes = LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | 
LOAD_OPTION_HIDDEN;
-}
-
 Status = EfiBootManagerInitializeLoadOption (
[(*BootOptionCount)++],
LoadOptionNumberUnassigned,
LoadOptionTypeBoot,
-   BootAttributes,
+   LOAD_OPTION_ACTIVE,
Description,
DevicePathFromHandle (Handles[Index]),
NULL,
-- 
2.8.0.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg/Bds: MemoryTypeInformation excludes boot option mem use

[Wang, Sunny (HPS SW)]

The patch looks good. I also tested this patch on my system to boot a large 
size HTTP image (.iso), and it works well. Thanks for quickly working out the 
permanent fix.

Reviewed-by: Sunny Wang <sunnyw...@hpe.com>

Regards,
Sunny Wang

-Original Message-
From: Ruiyu Ni [mailto:ruiyu...@intel.com] 
Sent: Tuesday, July 05, 2016 5:47 PM
To: edk2-devel@lists.01.org
Cc: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; Star Zeng <star.z...@intel.com>
Subject: [PATCH] MdeModulePkg/Bds: MemoryTypeInformation excludes boot option 
mem use

The patch re-orders the sequences by putting updating memory type information 
before loading the boot option so that the reserved memory usage by HTTP RAM 
disk boot can be excluded by the memory type information updating.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
Cc: Sunny Wang <sunnyw...@hpe.com>
Cc: Star Zeng <star.z...@intel.com>
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 41 +-
 MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c   | 13 +--
 .../Library/UefiBootManagerLib/InternalBm.h|  5 +--
 3 files changed, 19 insertions(+), 40 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 4da401d..bb38f00 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1569,7 +1569,6 @@ EfiBootManagerBoot (
   UINTN FileSize;
   EFI_BOOT_LOGO_PROTOCOL*BootLogo;
   EFI_EVENT LegacyBootEvent;
-  UINTN RamDiskSizeInPages;
 
   if (BootOption == NULL) {
 return;
@@ -1643,8 +1642,24 @@ EfiBootManagerBoot (
   PERF_START_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) OptionNumber);
 
   //
-  // 5. Load EFI boot option to ImageHandle
+  // 5. Adjust the different type memory page number just before booting
+  //and save the updated info into the variable for next boot to use
+  //
+  BmSetMemoryTypeInformationVariable (
+(BOOLEAN) ((BootOption->Attributes & LOAD_OPTION_CATEGORY) == 
+ LOAD_OPTION_CATEGORY_BOOT)  );
+
   //
+  // 6. Load EFI boot option to ImageHandle  //  DEBUG_CODE_BEGIN ();  
+ if (BootOption->Description == NULL) {
+DEBUG ((DEBUG_INFO | DEBUG_LOAD, "[Bds]Booting from unknown device 
+ path\n"));  } else {
+DEBUG ((DEBUG_INFO | DEBUG_LOAD, "[Bds]Booting %s\n", 
+ BootOption->Description));  }  DEBUG_CODE_END ();
+
   ImageHandle   = NULL;
   RamDiskDevicePath = NULL;
   if (DevicePathType (BootOption->FilePath) != BBS_DEVICE_PATH) { @@ -1701,28 
+1716,6 @@ EfiBootManagerBoot (
   }
 
   //
-  // 6. Adjust the different type memory page number just before booting
-  //and save the updated info into the variable for next boot to use
-  //
-  if (RamDiskDevicePath == NULL) {
-RamDiskSizeInPages = 0;
-  } else {
-BmGetRamDiskMemoryInfo (RamDiskDevicePath, );
-  }
-  BmSetMemoryTypeInformationVariable (
-(BOOLEAN) ((BootOption->Attributes & LOAD_OPTION_CATEGORY) == 
LOAD_OPTION_CATEGORY_BOOT),
-RamDiskSizeInPages
-);
-
-  DEBUG_CODE_BEGIN();
-if (BootOption->Description == NULL) {
-  DEBUG ((DEBUG_INFO | DEBUG_LOAD, "[Bds]Booting from unknown device 
path\n"));
-} else {
-  DEBUG ((DEBUG_INFO | DEBUG_LOAD, "[Bds]Booting %s\n", 
BootOption->Description));
-}
-  DEBUG_CODE_END();
-
-  //
   // Check to see if we should legacy BOOT. If yes then do the legacy boot
   // Write boot to OS performance data for Legacy boot
   //
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
index 93502fe..2a60f06 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
@@ -130,13 +130,10 @@ BmMatchDevicePaths (
 
   @param Boot   TRUE if current boot option belongs to boot
 category instead of application category.
-  @param RamDiskSizeInPages Reserved memory size in pages occupied by
-RAM Disk.
 **/
 VOID
 BmSetMemoryTypeInformationVariable (
-  IN BOOLEANBoot,
-  IN UINTN  RamDiskSizeInPages
+  IN BOOLEANBoot
   )
 {
   EFI_STATUS   Status;
@@ -230,14 +227,6 @@ BmSetMemoryTypeInformationVariable (
 }
 
 //
-// Do not count the reserved memory occupied by RAM Disk.
-//
-if ((CurrentMemoryTypeInformation[Index1].Type == EfiReservedMemoryType) &&
-(CurrentMemoryTypeInformation[Index1].NumberOfPages > ((UINT32) 
RamDiskSizeInPages))) {
-  CurrentMemoryTypeInformation[Index1].NumberOfPages -= (UINT32) 
RamDiskSizeInPages;
-}
-
-//
 // Previous is the number of pages pre-allocated
 // Current is

Re: [edk2] [PATCH v2] MdeModulePkg: Skip registering BootManagerMenu if its FFS is not found

[Wang, Sunny (HPS SW)]

Just a update. After offline discussing with Ray, I will send patch v3 based on 
some of his comments listed below and some further updates coming from our 
offline discussion.

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Wednesday, June 29, 2016 10:26 AM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v2] MdeModulePkg: Skip registering BootManagerMenu 
if its FFS is not found
Importance: High

Sunny,
I have 7 minor comments. Please check below.

Regards,
Ray

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>Sunny Wang
>Sent: Tuesday, June 28, 2016 10:27 AM
>To: edk2-devel@lists.01.org
>Cc: Ni, Ruiyu <ruiyu...@intel.com>
>Subject: [edk2] [PATCH v2] MdeModulePkg: Skip registering 
>BootManagerMenu if its FFS is not found
>
>This is a enhancement to support the case when platform firmware doesn’t 
>support Boot Manager Menu.
>For now, if BootManagerMenu FFS can not be retrieved from FV, BDS core 
>code will still register a boot option for it. Then, this 
>non-functional boot option will still be booted by user's request (like 
>HotKey or Exit from shell) to cause additional boot time and error 
>status code reported. Therefore, it would be good to skip BootManagerMenu boot 
>option registration and then return error status and Invalid BootOption data 
>for this case so that the BootManagerBoot() or other consumers can directly 
>return without doing anything.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Sunny Wang <sunnyw...@hpe.com>
>---
> MdeModulePkg/Include/Library/UefiBootManagerLib.h  |  7 ++--
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 39 ++
> MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c |  9 +++--
> MdeModulePkg/Universal/BdsDxe/BdsEntry.c   | 29 +++-
> 4 files changed, 64 insertions(+), 20 deletions(-)
>
>diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h 
>b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
>index 0fdb23d..649af9a 100644
>--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
>+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
>@@ -418,12 +418,13 @@ EfiBootManagerBoot (
>   );
>
> /**
>-  Return the Boot Manager Menu.
>-
>+  Return the boot option corresponding to the Boot Manager Menu.
>+  It may automatically create one if the boot option hasn't been created yet.
>+
>   @param BootOptionReturn the Boot Manager Menu.
>
>   @retval EFI_SUCCESS   The Boot Manager Menu is successfully returned.
>-  @retval EFI_NOT_FOUND The Boot Manager Menu is not found.
>+  @retval StatusReturn status of either gRT->SetVariable () or 
>GetSectionFromFv().
> **/

1. How about we change the return status as following:
  @retval EFI_SUCCESS   The Boot Manager Menu is successfully returned.
  @retval EFI_NOT_FOUND The Boot Manager Menu cannot be found.
  @retval others Return status of gRT->SetVariable (). BootOption still 
points
to the Boot Manager Menu even the Status is not 
EFI_SUCCESS
and EFI_NOT_FOUND.

> EFI_STATUS
> EFIAPI
>diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>index d016517..17e416a 100644
>--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>@@ -2,7 +2,7 @@
>   Library functions which relates with booting.
>
> Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
>-(C) Copyright 2015 Hewlett Packard Enterprise Development LP
>+(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
> 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 @@ -2159,16 +2159,19 @@ EfiBootManagerRefreshAllBootOption (  
>}
>
> /**
>-  This function is called to create the boot option for the Boot Manager Menu.
>+  This function is called to get or create the boot option for the Boot 
>Manager Menu.
>
>   The Boot Manager Menu is shown after successfully booting a boot option.
>   Assume the BootManagerMenuFile is in the same FV as the module links to 
> this library.
>
>-  @param  BootOptionReturn the boot option of the Boot Manager Menu
>+  @param  BootOptionReturn the boot option of the Boot Manager Menu.
>+If BootManagerMenu fails to be set as a Boot 
>variables, BootOption
>+still points to the Boot Manager Menu.
>+ 

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Fix for wrong data updated into MemoryTypeInformation variable

[Wang, Sunny (HPS SW)]

Thanks, Ray. I will update patch to add the commit message you mentioned. 

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Wednesday, June 22, 2016 5:51 PM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org
Cc: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>
Subject: RE: [PATCH] MdeModulePkg/UefiBootManagerLib: Fix for wrong data 
updated into MemoryTypeInformation variable
Importance: High

Sunny,
I agree with this patch. After all the patch is an improvement though it cannot 
fix all the bugs.
Please mention in the commit message that:
But there is a corner case even when the memory bins don't include the RAM disk 
memory, the memory used by all other modules exceeds RamDiskSizeInPages. 

Reviewed-by: Ruiyu Ni <ruiyu...@intel.com>

I will think about how to fix that corner case.

Regards,
Ray

>-Original Message-
>From: Sunny Wang [mailto:sunnyw...@hpe.com]
>Sent: Monday, June 20, 2016 4:03 PM
>To: edk2-devel@lists.01.org
>Cc: el...@hpe.com; Ni, Ruiyu <ruiyu...@intel.com>; Sunny Wang 
><sunnyw...@hpe.com>
>Subject: [PATCH] MdeModulePkg/UefiBootManagerLib: Fix for wrong data 
>updated into MemoryTypeInformation variable
>
>After booting a large-size ISO RAM disk (HTTP boot option pointing to a 
>ISO file) and rebooting the system, system will possibly run into the 
>following ASSERT because the BDS core code doesn't consider the case 
>that Memory page management
>(Page.c) would possibly skip updating current memory usage statistics 
>(CurrentMemoryTypeInformation) if system allocates a memory buffer with 
>a large number of pages. For this case, the problematic BDS code block 
>in
>BmSetMemoryTypeInformationVariable() function will get a 
>CurrentMemoryTypeInformation[Index1].NumberOfPages value which is not 
>updated with large-size ISO RAM disk usage by Memory page management 
>(it keeps original value without RAM disk size. For example, 0x9000) 
>and gets RamDiskSizeInPages (ISO RAM disk size) in a big value (For example, 
>0xC). Then, the result will become a very big value (0xFFF49000) to be 
>updated into MemoryTypeInformation variable to cause the ASSERT in next boot.
>
>ASSERT [DxeCore] u:\MdeModulePkg\Core\Dxe\Gcd\Gcd.c(2273): Length >= 
>MinimalMemorySizeNeeded
>
>For fixing this issue, we need to add a check to the problematic BDS 
>code block for checking that NumberOfPages is bigger than 
>RamDiskSizeInPages to prevent BDS core code updating wrong data (very big 
>value) to MemoryTypeInformation variable in this case.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Sunny Wang <sunnyw...@hpe.com>
>---
> MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
>b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
>index 29c1bfa..93502fe 100644
>--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
>+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
>@@ -2,6 +2,7 @@
>   Misc library functions.
>
> Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
>+(C) Copyright 2016 Hewlett Packard Enterprise Development LP
> 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 @@ -231,7 +232,8 @@ BmSetMemoryTypeInformationVariable (
> //
> // Do not count the reserved memory occupied by RAM Disk.
> //
>-if (CurrentMemoryTypeInformation[Index1].Type == EfiReservedMemoryType) {
>+if ((CurrentMemoryTypeInformation[Index1].Type == EfiReservedMemoryType) 
>&&
>+(CurrentMemoryTypeInformation[Index1].NumberOfPages > 
>+ ((UINT32) RamDiskSizeInPages))) {
>   CurrentMemoryTypeInformation[Index1].NumberOfPages -= (UINT32) 
> RamDiskSizeInPages;
> }
>
>--
>2.5.0.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Skip registering BootManagerMenu if its FFS is not found

[Wang, Sunny (HPS SW)]

After offline discussing with Ray and Samer, I will update patch to completely 
support the case where platform doesn't support BootManagerMenu 
(BootManagerMenu doesn't be built into firmware). Thanks, Ray and Samer.

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Monday, June 20, 2016 6:01 PM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org
Cc: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>
Subject: RE: [PATCH] MdeModulePkg/UefiBootManagerLib: Skip registering 
BootManagerMenu if its FFS is not found
Importance: High

Sunny,
Failure of GetSectionFromFv() only means the BootManagerMenu FFS doesn't have 
UI section.
So it cannot be the indicator of the absence of BootManagerMenu FFS.

Secondly, BDS core assumes firmware would have the Boot Manager Menu. Absence 
is a fatal error I think.
Lots of code calls EfiBootManagerGetBootManagerMenu without checking the 
status, and then directly use EfiBootManagerBoot to boot to Boot Manager Menu.

Do you meet real case that the Boot Manager Menu needs to be removed from 
firmware?

Regards,
Ray

>-Original Message-
>From: Sunny Wang [mailto:sunnyw...@hpe.com]
>Sent: Monday, June 20, 2016 5:44 PM
>To: edk2-devel@lists.01.org
>Cc: el...@hpe.com; Ni, Ruiyu <ruiyu...@intel.com>; Sunny Wang 
><sunnyw...@hpe.com>
>Subject: [PATCH] MdeModulePkg/UefiBootManagerLib: Skip registering 
>BootManagerMenu if its FFS is not found
>
>This is a enhancement.
>For now, if BootManagerMenu FFS can not be found from FV, BDS core code 
>will still register a boot option for it. Then, this non-functional 
>boot option will still be booted by user's request (like HotKey or Exit 
>from shell) to cause additional boot time and error status code reported. 
>Therefore, it would be good to return EFI_NOT_FOUND status and NULL BootOption 
>pointer to skip registering and booting BootManagerMenu boot option for this 
>case.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Sunny Wang <sunnyw...@hpe.com>
>---
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
>diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>index d016517..74b2acc 100644
>--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>@@ -2,7 +2,7 @@
>   Library functions which relates with booting.
>
> Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
>-(C) Copyright 2015 Hewlett Packard Enterprise Development LP
>+(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
> 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 @@ -2164,9 +2164,11 @@ EfiBootManagerRefreshAllBootOption (
>   The Boot Manager Menu is shown after successfully booting a boot option.
>   Assume the BootManagerMenuFile is in the same FV as the module links to 
> this library.
>
>-  @param  BootOptionReturn the boot option of the Boot Manager Menu
>+  @param  BootOptionReturn the boot option of the Boot Manager Menu. 
>Return NULL if
>+BootManagerMenu FFS section can not be found.
>
>   @retval EFI_SUCCESS   Successfully register the Boot Manager Menu.
>+  @retval EFI_NOT_FOUND BootManagerMenu FFS section can not be found.
>   @retval StatusReturn status of gRT->SetVariable (). BootOption 
> still points
> to the Boot Manager Menu even the Status is not 
> EFI_SUCCESS.
> **/
>@@ -2190,7 +2192,9 @@ BmRegisterBootManagerMenu (
>  
>  );
>   if (EFI_ERROR (Status)) {
>-Description = NULL;
>+BootOption = NULL;
>+DEBUG ((EFI_D_ERROR, "[Bds]BootManagerMenu FFS section can not be 
>+ found, skip to register
>BootManagerMenu\n"));
>+return EFI_NOT_FOUND;
>   }
>
>   EfiInitializeFwVolDevicepathNode (, PcdGetPtr 
>(PcdBootManagerMenuFile));
>--
>2.5.0.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg:Prevent the BmRepairAllControllers routine in an infinite loop

[Wang, Sunny (HPS SW)]

Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Li, Gary 
(HPS SW)
Sent: Thursday, June 02, 2016 2:00 PM
To: edk2-devel@lists.01.org
Cc: ruiyu...@intel.com; El-Haj-Mahmoud, Samer 
Subject: [edk2] [PATCH] MdeModulePkg:Prevent the BmRepairAllControllers routine 
in an infinite loop

Prevent the BmRepairAllControllers routine in an infinite loop

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Gary Li 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c  | 11 +--
 .../Library/UefiBootManagerLib/UefiBootManagerLib.inf |  3 ++-
 MdeModulePkg/MdeModulePkg.dec |  4 
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
index bffbeda..1e68cbc 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
@@ -1,8 +1,9 @@
 /** @file
   Library functions which relates with driver health.
 
-(C) Copyright 2015 Hewlett-Packard Development Company, L.P.  Copyright 
(c) 2011 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett-Packard Development Company, L.P.
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP
 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 @@ -439,6 +440,8 @@ 
BmRepairAllControllers (
   BOOLEAN RebootRequired;
   EFI_HII_HANDLE  *HiiHandles;
   EFI_FORM_BROWSER2_PROTOCOL  *FormBrowser2;
+  UINT32  MaxRepairCount;
+  UINT32  RepairCount;
 
   //
   // Configure PcdDriverHealthConfigureForm to ZeroGuid to disable driver 
health check.
@@ -450,6 +453,9 @@ BmRepairAllControllers (
   Status = gBS->LocateProtocol (, NULL, (VOID **) 
);
   ASSERT_EFI_ERROR (Status);
 
+  MaxRepairCount = PcdGet32 (PcdMaxRepairCount);  RepairCount = 0;
+
   do {
 RepairRequired= FALSE;
 ConfigurationRequired = FALSE;
@@ -512,7 +518,8 @@ BmRepairAllControllers (
 }
   
 EfiBootManagerFreeDriverHealthInfo (DriverHealthInfo, Count);
-  } while (RepairRequired || ConfigurationRequired);
+RepairCount++;
+  } while ((RepairRequired || ConfigurationRequired) && 
+ ((MaxRepairCount == 0) || (RepairCount < MaxRepairCount)));
 
   RebootRequired= FALSE;
   ReconnectRequired = FALSE;
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf 
b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index 9d62d3d..078f272 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -6,6 +6,7 @@
 #  manipulation, driver health checking and etc.
 #
 #  Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
+#  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 #  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 @@ 
-117,4 +118,4 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdErrorCodeSetVariable## 
SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm   ## 
SOMETIMES_CONSUMES
-
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount  ## 
CONSUMES
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec 
index a133824..38576ee 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1105,6 +1105,10 @@
   # @Prompt Exposed ACPI table versions.
   
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x3E|UINT32|0x0001004c
 
+  ## This PCD defines the MAX repair count.
+  #  The default value is 0 that means infinite.
+  
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount|0x00|UINT32|0x0001007
+ 6
+
 [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## This PCD defines the Console output row. The default value is 25 
according to UEFI spec.
   #  This PCD could be set to 0 then console output would be at max column and 
max row.
--
2.7.2.windows.1

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

Re: [edk2] [Patch] NetworkPkg: Fix a memory leak in HTTP boot driver.

[Wang, Sunny (HPS SW)]

Looks good to me. Thanks for addressing my comemnt. (Sorry for late to review 
this. I was not aware of that you send the other review for my comment)
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu Siyuan
Sent: Thursday, May 05, 2016 10:16 AM
To: edk2-devel@lists.01.org
Cc: Ye Ting ; Wu Jiaxin 
Subject: [edk2] [Patch] NetworkPkg: Fix a memory leak in HTTP boot driver.

We always need to call EfiBootManagerFreeLoadOption because the memory 
allocated for NewOption (description and device path) is no longer needed.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/HttpBootDxe/HttpBootConfig.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c 
b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
index 2ca38b5..04c2f3e 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
@@ -142,9 +142,7 @@ HttpBootAddBootOption (
   }
 
   Status = EfiBootManagerAddLoadOptionVariable (, (UINTN) -1);
-  if (EFI_ERROR (Status)) {
-EfiBootManagerFreeLoadOption ();
-  }
+  EfiBootManagerFreeLoadOption ();
 
 ON_EXIT:
 
--
2.7.4.windows.1

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

Re: [edk2] [PATCH v2] NetworkPkg: Use UefiBootManagerLib API to create load option.

[Wang, Sunny (HPS SW)]

Hi Siyuan, 
I just found a possible memory leak issue with the following code block. Others 
look good to me. 
+  Status = EfiBootManagerAddLoadOptionVariable (, (UINTN) -1);  
+  if (EFI_ERROR (Status)) {
+EfiBootManagerFreeLoadOption ();
+   }

I think we always need to call EfiBootManagerFreeLoadOption no matter 
EfiBootManagerAddLoadOptionVariable returns error or not because the memory 
allocated for NewOption (description and device path) is no longer needed. We 
already set NewOption to variable, so the data in memory will not be used 
anymore. Therefore, we can modify the code block above to the following, what 
do you think? 
+  EfiBootManagerAddLoadOptionVariable (, (UINTN) -1);  
+  EfiBootManagerFreeLoadOption ();
 
By the way, thanks for addressing my comment in the other review.

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu Siyuan
Sent: Wednesday, May 04, 2016 1:58 PM
To: edk2-devel@lists.01.org
Cc: Ye Ting ; Ni Ruiyu ; Wu Jiaxin 

Subject: [edk2] [PATCH v2] NetworkPkg: Use UefiBootManagerLib API to create 
load option.

V2:
Remove unnecessary ZeroMem and free load option.

This patch updates the HTTP boot driver to use the API in UefiBootManagerLib to 
create new load option, to avoid duplicate code.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Cc: Ni Ruiyu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/HttpBootDxe/HttpBootConfig.c | 136 ++--
 NetworkPkg/HttpBootDxe/HttpBootDxe.inf  |   1 +
 NetworkPkg/NetworkPkg.dsc   |   8 ++
 3 files changed, 33 insertions(+), 112 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c 
b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
index 00e4782..2ca38b5 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
@@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 **/
 
 #include "HttpBootDxe.h"
+#include 
 
 CHAR16  mHttpBootConfigStorageName[] = L"HTTP_BOOT_CONFIG_IFR_NVDATA";
 
@@ -36,31 +37,18 @@ HttpBootAddBootOption (
   IN   CHAR16   *Uri
   )
 {
-  EFI_DEV_PATH   *Node;
-  EFI_DEVICE_PATH_PROTOCOL   *TmpDevicePath;
-  EFI_DEVICE_PATH_PROTOCOL   *NewDevicePath;
-  UINTN  Length;
-  CHAR8  AsciiUri[URI_STR_MAX_SIZE];
-  CHAR16 *CurrentOrder;
-  EFI_STATUS Status;
-  UINTN  OrderCount;
-  UINTN  TargetLocation;
-  BOOLEANFound;
-  UINT8  *TempByteBuffer;
-  UINT8  *TempByteStart;
-  UINTN  DescSize;
-  UINTN  FilePathSize;
-  CHAR16 OptionStr[10];
-  UINT16 *NewOrder;
-  UINTN  Index;
-
-  NewOrder  = NULL;
-  TempByteStart = NULL;
+  EFI_DEV_PATH  *Node;
+  EFI_DEVICE_PATH_PROTOCOL  *TmpDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL  *NewDevicePath;
+  UINTN Length;
+  CHAR8 AsciiUri[URI_STR_MAX_SIZE];
+  EFI_STATUSStatus;
+  UINTN Index;
+  EFI_BOOT_MANAGER_LOAD_OPTION  NewOption;
+
   NewDevicePath = NULL;
-  NewOrder  = NULL;
   Node  = NULL;
   TmpDevicePath = NULL;
-  CurrentOrder  = NULL;
 
   if (StrLen (Description) == 0) {
 return EFI_INVALID_PARAMETER;
@@ -137,105 +125,29 @@ HttpBootAddBootOption (
   }
 
   //
-  // Get current "BootOrder" variable and find a free target.
-  //
-  Length = 0;
-  Status = GetVariable2 (
- L"BootOrder",
- ,
- (VOID **),
-  
- );
-  if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
-goto ON_EXIT;
-  }
-  OrderCount = Length / sizeof (UINT16);
-  Found = FALSE;
-  for (TargetLocation=0; TargetLocation < 0x; TargetLocation++) {
-Found = TRUE;
-for (Index = 0; Index < OrderCount; Index++) {
-  if (CurrentOrder[Index] == TargetLocation) {
-Found = FALSE;
-break;
-  }
-}
-if (Found) {
-  break;
-}
-  }
-
-  if (TargetLocation == 0x) {
-DEBUG ((EFI_D_ERROR, "Could not find unused target index.\n"));
-Status = EFI_OUT_OF_RESOURCES;
-goto ON_EXIT;
-  } else {
-DEBUG ((EFI_D_INFO, "TargetIndex = %04x.\n", TargetLocation));
-  }
-
-  //
-  // Construct and set the "Boot" variable
+  // Add a new load option.
   //
-  DescSize = StrSize(Description);
-  FilePathSize = GetDevicePathSize (NewDevicePath);
-  TempByteBuffer = AllocateZeroPool(sizeof(EFI_LOAD_OPTION) + DescSize + 
FilePathSize);
-  if (TempByteBuffer == NULL) {
-Status 

Re: [edk2] [Patch] MdeModulePkg/Bds: Fix build failures of VS tool chain

[Wang, Sunny (HPS SW)]

Thanks for fixing this.
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Thursday, April 07, 2016 10:14 AM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Shumin Qiu 
Subject: [edk2] [Patch] MdeModulePkg/Bds: Fix build failures of VS tool chain

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Shumin Qiu 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 1 +  
MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 2eb8971..beb2eaf 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -699,6 +699,7 @@ BmExpandUriDevicePath (
 Handles = NULL;
   }
 
+  FileBuffer = NULL;
   for (Index = 0; Index < HandleCount; Index++) {
 FileBuffer = BmGetFileBufferFromLoadFile (Handles[Index], FilePath, 
FullPath, FileSize);
 if (FileBuffer != NULL) {
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
index c6ac242..29c1bfa 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
@@ -232,7 +232,7 @@ BmSetMemoryTypeInformationVariable (
 // Do not count the reserved memory occupied by RAM Disk.
 //
 if (CurrentMemoryTypeInformation[Index1].Type == EfiReservedMemoryType) {
-  CurrentMemoryTypeInformation[Index1].NumberOfPages -= RamDiskSizeInPages;
+  CurrentMemoryTypeInformation[Index1].NumberOfPages -= (UINT32) 
+ RamDiskSizeInPages;
 }
 
 //
--
2.7.0.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: API BmIsValidLoadOptionVariableName

[Wang, Sunny (HPS SW)]

Looks good to me.
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Palmer, 
Thomas
Sent: Tuesday, April 05, 2016 3:52 AM
To: edk2-devel@lists.01.org
Cc: ruiyu...@intel.com; feng.t...@intel.com; star.z...@intel.com
Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: API 
BmIsValidLoadOptionVariableName

Redfine the BmIsValidLoadOptionVariableName function to allow public use. 
Change name to EfiBootManagerIsValidLoadOptionVariableName to match naming 
scheme. Check that VariableName is never NULL and allow OptionType and 
OptionNumber to be optional.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Thomas Palmer 
---
 MdeModulePkg/Include/Library/UefiBootManagerLib.h  | 23 -
 .../Library/UefiBootManagerLib/BmLoadOption.c  | 38 ++
 2 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h 
b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index afb4271..e3555b5 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -2,7 +2,7 @@
   Provide Boot Manager related library APIs.
 
 Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
-(C) Copyright 2015 Hewlett Packard Enterprise Development LP
+(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
 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 @@ -731,4 +731,25 
@@ EFIAPI  EfiBootManagerProcessLoadOption (
   EFI_BOOT_MANAGER_LOAD_OPTION   *LoadOption
   );
+
+/**
+  Check whether the VariableName is a valid load option variable name
+  and return the load option type and option number.
+
+  @param VariableName The name of the load option variable.
+  @param OptionType   Return the load option type.
+  @param OptionNumber Return the load option number.
+
+  @retval TRUE  The variable name is valid; The load option type and
+load option number is returned.
+  @retval FALSE The variable name is NOT valid.
+**/
+BOOLEAN
+EFIAPI
+EfiBootManagerIsValidLoadOptionVariableName (
+  IN CHAR16 *VariableName,
+  OUT EFI_BOOT_MANAGER_LOAD_OPTION_TYPE *OptionType   OPTIONAL,
+  OUT UINT16*OptionNumber OPTIONAL
+  );
+
 #endif
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 696e995..8201255 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -2,7 +2,7 @@
   Load option library functions which relate with creating and processing load 
options.
 
 Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
-(C) Copyright 2015 Hewlett Packard Enterprise Development LP
+(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
 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 @@ -775,16 +775,21 
@@ BmValidateOption (
   @retval FALSE The variable name is NOT valid.
 **/
 BOOLEAN
-BmIsValidLoadOptionVariableName (
+EFIAPI
+EfiBootManagerIsValidLoadOptionVariableName (
   IN CHAR16 *VariableName,
-  OUT EFI_BOOT_MANAGER_LOAD_OPTION_TYPE *OptionType,
-  OUT UINT16*OptionNumber
+  OUT EFI_BOOT_MANAGER_LOAD_OPTION_TYPE *OptionType   OPTIONAL,
+  OUT UINT16*OptionNumber OPTIONAL
   )
 {
   UINTN VariableNameLen;
   UINTN Index;
   UINTN Uint;
 
+  if (VariableName == NULL) {
+return FALSE;
+  }
+
   VariableNameLen = StrLen (VariableName);
 
   if (VariableNameLen <= 4) {
@@ -803,14 +808,19 @@ BmIsValidLoadOptionVariableName (
 return FALSE;
   }
 
-  *OptionType = (EFI_BOOT_MANAGER_LOAD_OPTION_TYPE) Index;
-  *OptionNumber = 0;
-  for (Index = VariableNameLen - 4; Index < VariableNameLen; Index++) {
-Uint = BmCharToUint (VariableName[Index]);
-if (Uint == -1) {
-  break;
-} else {
-  *OptionNumber = (UINT16) Uint + *OptionNumber * 0x10;
+  if (OptionType != NULL) {
+*OptionType = (EFI_BOOT_MANAGER_LOAD_OPTION_TYPE) Index;  }
+
+  if (OptionNumber != NULL) {
+*OptionNumber = 0;
+for (Index = VariableNameLen - 4; Index < VariableNameLen; Index++) {
+  Uint = BmCharToUint (VariableName[Index]);
+  if (Uint == -1) {
+break;
+  } else {
+*OptionNumber = (UINT16) Uint + *OptionNumber * 0x10;
+  }
 }
   }
 
@@ -853,7 +863,7 @@ 

Re: [edk2] [Patch v4 0/3] HTTP Boot to RAM Disk

[Wang, Sunny (HPS SW)]

Thanks for addressing my review comments in patch2. The updated patch (v4) 
looks good!  
Series Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, April 05, 2016 3:54 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni 
Subject: [edk2] [Patch v4 0/3] HTTP Boot to RAM Disk

The existing code already supports HTTP boot to RAM Disk.
The patch 1/3 is to follow the ACPI spec to use reserved memory instead of BS 
memory.
Patch 2/3 is to fix the memory leak issue when Http Boot finishes or fails.
Patch 3/3 is to update memory type information check algorithm to support Ram 
Disk case.

v4 takes Sunny's suggestion to store the ram disk device path instead of handle.

Ruiyu Ni (3):
  MdeModulePkg/Bds: Allocate reserved memory for RAM Disk boot media
  MdeModulePkg/Bds: Free resources after ram disk boot finishes
  MdeModulePkg/Bds: Memory Bins don't count the memory used by RAM Disk

 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 305 -
 MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c   |  19 +-
 .../Library/UefiBootManagerLib/InternalBm.h|  48 ++--
 .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
 4 files changed, 272 insertions(+), 101 deletions(-)

--
2.7.0.windows.1

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

Re: [edk2] [Patch v2 3/3] MdeModulePkg/Bds: Memory Bins don't count the memory used by RAM Disk

[Wang, Sunny (HPS SW)]

Just a update to others in this email. 
After offline clarifying with Ray, this build failure turns out to be with 
either VS2008 or VS2013. VS2015 works fine.  Ray is working on this now.

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Tuesday, April 05, 2016 12:46 PM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>
Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Fu, Siyuan 
<siyuan...@intel.com>; El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>; 
edk2-devel@lists.01.org
Subject: RE: [edk2] [Patch v2 3/3] MdeModulePkg/Bds: Memory Bins don't count 
the memory used by RAM Disk
Importance: High

Sunny,
Did you get real build failure? I checked our internal build test report but 
didn't find any failures.

Regards,
Ray


>-Original Message-----
>From: Wang, Sunny (HPS SW) [mailto:sunnyw...@hpe.com]
>Sent: Friday, April 1, 2016 7:32 PM
>To: Ni, Ruiyu <ruiyu...@intel.com>
>Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Fu, Siyuan 
><siyuan...@intel.com>; Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; 
>El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>; 
>edk2-devel@lists.01.org
>Subject: RE: [edk2] [Patch v2 3/3] MdeModulePkg/Bds: Memory Bins don't 
>count the memory used by RAM Disk
>
>Hi Ray,
>
>For the code block below in BmSetMemoryTypeInformationVariable() in the 
>BmMisc.c,
>+if (CurrentMemoryTypeInformation[Index1].Type == EfiReservedMemoryType) {
>+  CurrentMemoryTypeInformation[Index1].NumberOfPages -= 
>RamDiskSizeInPages;
>+}
>Since NumberOfPages is UINT32 but RamDiskSizeInPages is UINTN, we need 
>to either change RamDiskSizeInPages to
>UINT32 or modify code to the following to prevent the build warning.
>CurrentMemoryTypeInformation[Index1].NumberOfPages -=  (UINT32) 
>RamDiskSizeInPages;
>
>Regards,
>Sunny Wang
>
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>Ruiyu Ni
>Sent: Tuesday, March 29, 2016 6:32 PM
>To: edk2-devel@lists.01.org
>Cc: Ruiyu Ni <ruiyu...@intel.com>; Michael Kinney 
><michael.d.kin...@intel.com>; Siyuan Fu <siyuan...@intel.com>
>Subject: [edk2] [Patch v2 3/3] MdeModulePkg/Bds: Memory Bins don't 
>count the memory used by RAM Disk
>
>MemoryTypeInformation don't count the reserved memory used by RAM Disk, 
>but it still check all types of memory and do reset when any type of memory 
>size changes.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
>Cc: Siyuan Fu <siyuan...@intel.com>
>Cc: Michael Kinney <michael.d.kin...@intel.com>
>---
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c |  5 -
> MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 19 +++
> MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 12 
> 3 files changed, 27 insertions(+), 9 deletions(-)
>
>diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>index 4572644..35aada8 100644
>--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>@@ -1571,6 +1571,7 @@ EfiBootManagerBoot (
>   UINTN FileSize;
>   EFI_BOOT_LOGO_PROTOCOL*BootLogo;
>   EFI_EVENT LegacyBootEvent;
>+  UINTN RamDiskSizeInPages;
>
>   if (BootOption == NULL) {
> return;
>@@ -1702,8 +1703,10 @@ EfiBootManagerBoot (
>   // 6. Adjust the different type memory page number just before booting
>   //and save the updated info into the variable for next boot to use
>   //
>+  BmGetRamDiskMemoryInfo (RamDiskHandle, );
>   BmSetMemoryTypeInformationVariable (
>-(BOOLEAN) ((BootOption->Attributes & LOAD_OPTION_CATEGORY) == 
>LOAD_OPTION_CATEGORY_BOOT)
>+(BOOLEAN) ((BootOption->Attributes & LOAD_OPTION_CATEGORY) == 
>LOAD_OPTION_CATEGORY_BOOT),
>+RamDiskSizeInPages
> );
>
>   DEBUG_CODE_BEGIN();
>diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
>b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
>index 1768781..c6ac242 100644
>--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
>+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
>@@ -124,14 +124,18 @@ BmMatchDevicePaths (
>   This routine adjust the memory information for different memory type and
>   save them into the variables for next boot. It resets the system when
>   memory information is updated and the current boot option belongs to
>-  boot category instead of application category.
>+  boot category instead of application category. It doesn't count the 
>+ reserved memory occupied by RAM Di

Re: [edk2] [Patch v3 1/3] MdeModulePkg/Bds: Allocate reserved memory for RAM Disk boot media

[Wang, Sunny (HPS SW)]

Sorry for late to reply you. (Both Monday and Tuesday are public holidays in my 
location)
Yeah, you concern is more effective than preventing potential issue.  Thanks 
for the clarifying this.
Reviewed-by: Sunny Wang <sunnyw...@hpe.com>
 
-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Tuesday, April 05, 2016 11:11 AM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>
Cc: Fu, Siyuan <siyuan...@intel.com>; El-Haj-Mahmoud, Samer 
<samer.el-haj-mahm...@hpe.com>; edk2-devel@lists.01.org
Subject: RE: [edk2] [Patch v3 1/3] MdeModulePkg/Bds: Allocate reserved memory 
for RAM Disk boot media
Importance: High

Sunny,
Thanks for your comments.
ZeroMem I think is very useful to initialize a structure where the default 
value of many fields are 0.
But in such case, ZeroMem() is not optimal because only when code contains bug 
the ZeroMem() is useful.
Otherwise, it costs time to zero the memory.
For a RAM disk boot, the memory allocated may be 1GB or larger. Zero such big 
memory will need lots of time.

Regards,
Ray

>-Original Message-----
>From: Wang, Sunny (HPS SW) [mailto:sunnyw...@hpe.com]
>Sent: Friday, April 1, 2016 7:35 PM
>To: Ni, Ruiyu <ruiyu...@intel.com>
>Cc: Fu, Siyuan <siyuan...@intel.com>; El-Haj-Mahmoud, Samer 
><samer.el-haj-mahm...@hpe.com>; Wang, Sunny (HPS SW) 
><sunnyw...@hpe.com>; edk2-devel@lists.01.org
>Subject: RE: [edk2] [Patch v3 1/3] MdeModulePkg/Bds: Allocate reserved 
>memory for RAM Disk boot media
>
>Hi Ray,
>
>For this patch, I just have a suggestion below. Others look good to me.
>For the memory allocation code block below, It looks better to add a 
>ZeroMem() call right after it like the following so that we can avoid running 
>into some problems caused by remaining garbage data in memory. What do you 
>think?
>+ FileBuffer = LoadFileSystem ? AllocateReservedPages 
>+ (EFI_SIZE_TO_PAGES
>+ (BufferSize)) : AllocatePool (BufferSize);  if (FileBuffer == NULL) {
>+return NULL;
>+  }
>ZeroMem (FileBuffer, BufferSize);
>
>Regards,
>Sunny Wang
>
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>Ruiyu Ni
>Sent: Thursday, March 31, 2016 10:37 AM
>To: edk2-devel@lists.01.org
>Cc: Ruiyu Ni <ruiyu...@intel.com>; Siyuan Fu <siyuan...@intel.com>
>Subject: [edk2] [Patch v3 1/3] MdeModulePkg/Bds: Allocate reserved 
>memory for RAM Disk boot media
>
>Use reserved memory to hold the buffer for the RAM disk to follow the ACPI 
>spec requirement.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
>Cc: Siyuan Fu <siyuan...@intel.com>
>---
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 149 +++--
> .../Library/UefiBootManagerLib/InternalBm.h|  35 ++---
> 2 files changed, 95 insertions(+), 89 deletions(-)
>
>diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>index f6c6845..a582b90 100644
>--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>@@ -688,7 +688,6 @@ BmExpandUriDevicePath (
>   UINTN   Index;
>   UINTN   HandleCount;
>   EFI_HANDLE  *Handles;
>-  EFI_LOAD_FILE_PROTOCOL  *LoadFile;
>   VOID*FileBuffer;
>
>   EfiBootManagerConnectAll ();
>@@ -698,37 +697,10 @@ BmExpandUriDevicePath (
> Handles = NULL;
>   }
>
>-  FileBuffer = NULL;
>-
>   for (Index = 0; Index < HandleCount; Index++) {
>-Status = gBS->HandleProtocol (Handles[Index], , 
>(VOID *) );
>-ASSERT_EFI_ERROR (Status);
>-
>-FileBuffer = NULL;
>-Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, FileSize, 
>FileBuffer);
>-if (Status == EFI_BUFFER_TOO_SMALL) {
>-  FileBuffer = AllocatePool (*FileSize);
>-  if (FileBuffer == NULL) {
>-break;
>-  }
>-  Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, FileSize, 
>FileBuffer);
>-}
>-
>-if (!EFI_ERROR (Status)) {
>-  //
>-  // LoadFile() returns a file buffer mapping to a file system.
>-  //
>-  if (Status == EFI_WARN_FILE_SYSTEM) {
>-return BmGetFileBufferFromLoadFileFileSystem (Handles[Index], 
>FullPath, FileSize);
>-  }
>-
>-  ASSERT (Status == EFI_SUCCESS);
>-  *FullPath = DuplicateDevicePath (DevicePathFromHandle (Handles[Index]));
>-  break;
>-}
>-
>+FileBuffer = BmGetFileBufferFromLoadFile (Handles[Index], 
>+ FilePath, FullPath, FileSize);
> if (FileBuffer != NULL) {
>-  FreePool (FileBuffe

Re: [edk2] [Patch] MdeModulePkg/Bds: Fix a boot hang due to Ram Disk boot support

[Wang, Sunny (HPS SW)]

Tested it in my side as well. Looks good! Thanks for the quick fix. 
Reviewed-by: Sunny Wang <sunnyw...@hpe.com>

-Original Message-
From: Ruiyu Ni [mailto:ruiyu...@intel.com] 
Sent: Wednesday, April 06, 2016 2:37 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni <ruiyu...@intel.com>; Wang, Sunny (HPS SW) <sunnyw...@hpe.com>
Subject: [Patch] MdeModulePkg/Bds: Fix a boot hang due to Ram Disk boot support

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
Cc: Sunny Wang <sunnyw...@hpe.com>
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 5cae901..2eb8971 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1702,7 +1702,11 @@ EfiBootManagerBoot (
   // 6. Adjust the different type memory page number just before booting
   //and save the updated info into the variable for next boot to use
   //
-  BmGetRamDiskMemoryInfo (RamDiskDevicePath, );
+  if (RamDiskDevicePath == NULL) {
+RamDiskSizeInPages = 0;
+  } else {
+BmGetRamDiskMemoryInfo (RamDiskDevicePath, );  }
   BmSetMemoryTypeInformationVariable (
 (BOOLEAN) ((BootOption->Attributes & LOAD_OPTION_CATEGORY) == 
LOAD_OPTION_CATEGORY_BOOT),
 RamDiskSizeInPages
--
2.7.0.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: BmGetActiveConsoleIn code cleanup

[Wang, Sunny (HPS SW)]

Reviewed-by: Sunny Wang 


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Palmer, 
Thomas
Sent: Thursday, March 31, 2016 1:17 AM
To: edk2-devel@lists.01.org
Cc: ruiyu...@intel.com; feng.t...@intel.com; star.z...@intel.com
Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: BmGetActiveConsoleIn 
code cleanup

Check for NULL from AllocateCopyPool before setting Count to 1. Also change 
sizeof (EFI_HANDLE*) to sizeof (EFI_HANDLE).  Handles is a EFI_HANDLE pointer, 
so the allocated memory must be the size of EFI_HANDLE.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Thomas Palmer 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
index b49758b..f8cb69c 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
@@ -2,6 +2,7 @@
   Hotkey library functions.
 
 Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP
 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 @@ -471,8 +472,10 
@@ BmGetActiveConsoleIn (
 EFI_OPEN_PROTOCOL_TEST_PROTOCOL
 );
 if (!EFI_ERROR (Status)) {
-  Handles = AllocateCopyPool (sizeof (EFI_HANDLE *), 
>ConsoleInHandle);
-  *Count  = 1;
+  Handles = AllocateCopyPool (sizeof (EFI_HANDLE), >ConsoleInHandle);
+  if (Handles != NULL) {
+*Count  = 1;
+  }
 }
   } else {
 Status = gBS->LocateHandleBuffer (
--
1.9.1

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

Re: [edk2] [Patch v3 2/3] MdeModulePkg/Bds: Free resources after ram disk boot finishes

[Wang, Sunny (HPS SW)]

Hi Ray,
For this patch, I just found a minor issue and also have a suggestion below. 
Others look good to me.  

Minor issue: 
The following newly created functions' parameter lack IN and/or OUT modifiers:
1. BmGetLoadFileRamDisk
2. BmGetRamDiskMemoryInfo
3. BmDestroyRamDisk

Suggestion:
It looks like both BmGetLoadFileRamDisk and BmGetFileBufferFromLoadFileSystem) 
should be better to return RamDiskDevicePath instead of RamDiskHandle because 
both BmDestroyRamDisk and BmGetRamDiskMemoryInfo only need RamDisk's device 
path. In other words, if we change to return RamDiskDevicePath, we will NOT 
need to have DevicePathFromHandle function calls in newly added code. What do 
you think? 
By the way, if you agree with my suggestion above, we may also consider to 
directly use FullPath instead of adding new output parameter 
RamDiskHandle/RamDiskDevicePath to BmGetFileBufferFromLoadFileSystem(). For 
this one, I didn't take a deep look, so it maybe is not feasible. Just mention 
it here for your reference. 

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Wednesday, March 30, 2016 8:29 AM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Siyuan Fu ; Feng Tian 

Subject: [edk2] [Patch v3 2/3] MdeModulePkg/Bds: Free resources after ram disk 
boot finishes

The resource free includes to un-register the ram disk device and free the 
memory occupied by the ram disk.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Siyuan Fu 
Cc: Feng Tian 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 155 -
 .../Library/UefiBootManagerLib/InternalBm.h|   1 +
 .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
 3 files changed, 153 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 6e5c13b..598de26 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1095,6 +1095,7 @@ BmMatchHttpBootDevicePath (
   @param LoadFileHandle The handle of LoadFile instance.
   @param FullPath   Return the full device path pointing to the load 
option.
   @param FileSize   Return the size of the load option.
+  @param RamDiskHandle  Return the RAM Disk handle.
 
   @return  The load option buffer.
 **/
@@ -1102,7 +1103,8 @@ VOID *
 BmGetFileBufferFromLoadFileSystem (
   IN  EFI_HANDLE  LoadFileHandle,
   OUT EFI_DEVICE_PATH_PROTOCOL**FullPath,
-  OUT UINTN   *FileSize
+  OUT UINTN   *FileSize,
+  OUT EFI_HANDLE  *RamDiskHandle
   )
 {
   EFI_STATUS  Status;
@@ -1140,7 +1142,13 @@ BmGetFileBufferFromLoadFileSystem (
 FreePool (Handles);
   }
 
-  if (Index != HandleCount) {
+  if (Index == HandleCount) {
+Handle = NULL;
+  }
+
+  *RamDiskHandle = Handle;
+
+  if (Handle != NULL) {
 return BmExpandMediaDevicePath (DevicePathFromHandle (Handle), FullPath, 
FileSize);
   } else {
 return NULL;
@@ -1149,6 +1157,124 @@ BmGetFileBufferFromLoadFileSystem (
 
 
 /**
+  Return the RAM DISK handle created by LoadFile.
+
+  @param FilePath  The source file path.
+
+  @return RAM DISK handle.
+**/
+EFI_HANDLE
+BmGetLoadFileRamDisk (
+  EFI_DEVICE_PATH_PROTOCOL*FilePath
+  )
+{
+  EFI_STATUS  Status;
+  EFI_DEVICE_PATH_PROTOCOL*RamDiskDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL*Node;
+  EFI_HANDLE  Handle;
+
+  Node = FilePath;
+  Status = gBS->LocateDevicePath (, , 
+ );  if (!EFI_ERROR (Status) &&
+  (DevicePathType (Node) == MEDIA_DEVICE_PATH) &&
+  (DevicePathSubType (Node) == MEDIA_RAM_DISK_DP)
+  ) {
+
+//
+// Construct the device path pointing to RAM Disk
+//
+Node = NextDevicePathNode (Node);
+RamDiskDevicePath = DuplicateDevicePath (FilePath);
+ASSERT (RamDiskDevicePath != NULL);
+SetDevicePathEndNode ((VOID *) ((UINTN) RamDiskDevicePath + 
+ ((UINTN) Node - (UINTN) FilePath)));
+
+//
+// Query the RAM Disk handle using the constructed device path.
+//
+Node = RamDiskDevicePath;
+Status = gBS->LocateDevicePath (, , );
+ASSERT ((Status == EFI_SUCCESS) && IsDevicePathEnd (Node));
+FreePool (RamDiskDevicePath);
+return Handle;
+  }
+
+  return NULL;
+}
+
+/**
+  Return the buffer and buffer size occupied by the RAM Disk.
+
+  @param RamDiskHandle  RAM Disk handle.
+  @param RamDiskSizeInPages Return RAM Disk size in pages.
+
+  @retval RAM Disk buffer.
+**/
+VOID *
+BmGetRamDiskMemoryInfo (
+  EFI_HANDLE  RamDiskHandle,
+  UINTN   *RamDiskSizeInPages
+  )
+{
+
+  EFI_STATUS  Status;
+  EFI_DEVICE_PATH_PROTOCOL

Re: [edk2] [Patch v3 1/3] MdeModulePkg/Bds: Allocate reserved memory for RAM Disk boot media

[Wang, Sunny (HPS SW)]

Hi Ray,

For this patch, I just have a suggestion below. Others look good to me.  
For the memory allocation code block below, It looks better to add a ZeroMem() 
call right after it like the following so that we can avoid running into some 
problems caused by remaining garbage data in memory. What do you think? 
+ FileBuffer = LoadFileSystem ? AllocateReservedPages (EFI_SIZE_TO_PAGES 
+ (BufferSize)) : AllocatePool (BufferSize);  if (FileBuffer == NULL) {
+return NULL;
+  }
ZeroMem (FileBuffer, BufferSize);

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Thursday, March 31, 2016 10:37 AM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Siyuan Fu 
Subject: [edk2] [Patch v3 1/3] MdeModulePkg/Bds: Allocate reserved memory for 
RAM Disk boot media

Use reserved memory to hold the buffer for the RAM disk to follow the ACPI spec 
requirement.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Siyuan Fu 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 149 +++--
 .../Library/UefiBootManagerLib/InternalBm.h|  35 ++---
 2 files changed, 95 insertions(+), 89 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index f6c6845..a582b90 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -688,7 +688,6 @@ BmExpandUriDevicePath (
   UINTN   Index;
   UINTN   HandleCount;
   EFI_HANDLE  *Handles;
-  EFI_LOAD_FILE_PROTOCOL  *LoadFile;
   VOID*FileBuffer;
 
   EfiBootManagerConnectAll ();
@@ -698,37 +697,10 @@ BmExpandUriDevicePath (
 Handles = NULL;
   }
 
-  FileBuffer = NULL;
-
   for (Index = 0; Index < HandleCount; Index++) {
-Status = gBS->HandleProtocol (Handles[Index], , 
(VOID *) );
-ASSERT_EFI_ERROR (Status);
-
-FileBuffer = NULL;
-Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, FileSize, 
FileBuffer);
-if (Status == EFI_BUFFER_TOO_SMALL) {
-  FileBuffer = AllocatePool (*FileSize);
-  if (FileBuffer == NULL) {
-break;
-  }
-  Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, FileSize, 
FileBuffer);
-}
-
-if (!EFI_ERROR (Status)) {
-  //
-  // LoadFile() returns a file buffer mapping to a file system.
-  //
-  if (Status == EFI_WARN_FILE_SYSTEM) {
-return BmGetFileBufferFromLoadFileFileSystem (Handles[Index], 
FullPath, FileSize);
-  }
-
-  ASSERT (Status == EFI_SUCCESS);
-  *FullPath = DuplicateDevicePath (DevicePathFromHandle (Handles[Index]));
-  break;
-}
-
+FileBuffer = BmGetFileBufferFromLoadFile (Handles[Index], FilePath, 
+ FullPath, FileSize);
 if (FileBuffer != NULL) {
-  FreePool (FileBuffer);
+  break;
 }
   }
 
@@ -1127,7 +1099,7 @@ BmMatchHttpBootDevicePath (
   @return  The load option buffer.
 **/
 VOID *
-BmGetFileBufferFromLoadFileFileSystem (
+BmGetFileBufferFromLoadFileSystem (
   IN  EFI_HANDLE  LoadFileHandle,
   OUT EFI_DEVICE_PATH_PROTOCOL**FullPath,
   OUT UINTN   *FileSize
@@ -1175,8 +1147,78 @@ BmGetFileBufferFromLoadFileFileSystem (
   }
 }
 
+
 /**
-  Get the file buffer from Load File instance.
+  Get the file buffer from the specified Load File instance.
+
+  @param LoadFileHandle The specified Load File instance.
+  @param FilePath   The file path which will pass to LoadFile().
+  @param FullPath   Return the full device path pointing to the load 
option.
+  @param FileSize   Return the size of the load option.
+
+  @return  The load option buffer or NULL if fails.
+**/
+VOID *
+BmGetFileBufferFromLoadFile (
+  EFI_HANDLE  LoadFileHandle,
+  IN  EFI_DEVICE_PATH_PROTOCOL*FilePath,
+  OUT EFI_DEVICE_PATH_PROTOCOL**FullPath,
+  OUT UINTN   *FileSize
+  )
+{
+  EFI_STATUS  Status;
+  EFI_LOAD_FILE_PROTOCOL  *LoadFile;
+  VOID*FileBuffer;
+  BOOLEAN LoadFileSystem;
+  UINTN   BufferSize;
+
+  *FileSize = 0;
+
+  Status = gBS->OpenProtocol (
+  LoadFileHandle,
+  ,
+  (VOID **) ,
+  gImageHandle,
+  NULL,
+  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+  );
+  ASSERT_EFI_ERROR (Status);
+
+  FileBuffer = NULL;
+  BufferSize = 0;
+  Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, , 
+ FileBuffer);  if ((Status != EFI_WARN_FILE_SYSTEM) && (Status != 
EFI_BUFFER_TOO_SMALL)) {
+return NULL;
+  }
+
+  LoadFileSystem = (BOOLEAN) (Status 

Re: [edk2] [Patch v2 3/3] MdeModulePkg/Bds: Memory Bins don't count the memory used by RAM Disk

[Wang, Sunny (HPS SW)]

Hi Ray,

For the code block below in BmSetMemoryTypeInformationVariable() in the 
BmMisc.c,
+if (CurrentMemoryTypeInformation[Index1].Type == EfiReservedMemoryType) {
+  CurrentMemoryTypeInformation[Index1].NumberOfPages -= RamDiskSizeInPages;
+}
Since NumberOfPages is UINT32 but RamDiskSizeInPages is UINTN, we need to 
either change RamDiskSizeInPages to UINT32 or modify code to the following to 
prevent the build warning.
CurrentMemoryTypeInformation[Index1].NumberOfPages -=  (UINT32) 
RamDiskSizeInPages;

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, March 29, 2016 6:32 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Michael Kinney ; 
Siyuan Fu 
Subject: [edk2] [Patch v2 3/3] MdeModulePkg/Bds: Memory Bins don't count the 
memory used by RAM Disk

MemoryTypeInformation don't count the reserved memory used by RAM Disk, but it 
still check all types of memory and do reset when any type of memory size 
changes.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Siyuan Fu 
Cc: Michael Kinney 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c |  5 -
 MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 19 +++
 MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 12 
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 4572644..35aada8 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1571,6 +1571,7 @@ EfiBootManagerBoot (
   UINTN FileSize;
   EFI_BOOT_LOGO_PROTOCOL*BootLogo;
   EFI_EVENT LegacyBootEvent;
+  UINTN RamDiskSizeInPages;
 
   if (BootOption == NULL) {
 return;
@@ -1702,8 +1703,10 @@ EfiBootManagerBoot (
   // 6. Adjust the different type memory page number just before booting
   //and save the updated info into the variable for next boot to use
   //
+  BmGetRamDiskMemoryInfo (RamDiskHandle, );
   BmSetMemoryTypeInformationVariable (
-(BOOLEAN) ((BootOption->Attributes & LOAD_OPTION_CATEGORY) == 
LOAD_OPTION_CATEGORY_BOOT)
+(BOOLEAN) ((BootOption->Attributes & LOAD_OPTION_CATEGORY) == 
LOAD_OPTION_CATEGORY_BOOT),
+RamDiskSizeInPages
 );
 
   DEBUG_CODE_BEGIN();
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
index 1768781..c6ac242 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
@@ -124,14 +124,18 @@ BmMatchDevicePaths (
   This routine adjust the memory information for different memory type and 
   save them into the variables for next boot. It resets the system when
   memory information is updated and the current boot option belongs to
-  boot category instead of application category.
+  boot category instead of application category. It doesn't count the  
+ reserved memory occupied by RAM Disk.
 
-  @param Boot  TRUE if current boot option belongs to boot category instead of
-   application category.
+  @param Boot   TRUE if current boot option belongs to boot
+category instead of application category.
+  @param RamDiskSizeInPages Reserved memory size in pages occupied by
+RAM Disk.
 **/
 VOID
 BmSetMemoryTypeInformationVariable (
-  IN BOOLEANBoot
+  IN BOOLEANBoot,
+  IN UINTN  RamDiskSizeInPages
   )
 {
   EFI_STATUS   Status;
@@ -225,6 +229,13 @@ BmSetMemoryTypeInformationVariable (
 }
 
 //
+// Do not count the reserved memory occupied by RAM Disk.
+//
+if (CurrentMemoryTypeInformation[Index1].Type == EfiReservedMemoryType) {
+  CurrentMemoryTypeInformation[Index1].NumberOfPages -= RamDiskSizeInPages;
+}
+
+//
 // Previous is the number of pages pre-allocated
 // Current is the number of pages actually needed
 //
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 4660323..7466719 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -202,14 +202,18 @@ BmWriteBootToOsPerformanceData (
   This routine adjust the memory information for different memory type and 
   save them into the variables for next boot. It resets the system when
   memory information is updated and the current boot option belongs to
-  boot category instead of application category.
+  boot category instead of application category. It doesn't count the  
+ 

Re: [edk2] [Patch] MdeModulePkg/Bds: BDS hotkey shouldn't work on inactive consoles

[Wang, Sunny (HPS SW)]

Looks good. It is good for the system having ConSplitter.
Reviewed-by: Sunny Wang 

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, March 22, 2016 11:09 AM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Feng Tian 
Subject: [edk2] [Patch] MdeModulePkg/Bds: BDS hotkey shouldn't work on inactive 
consoles

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c | 96 --
 1 file changed, 73 insertions(+), 23 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
index af303e8..4cc4fb4 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
@@ -1,7 +1,7 @@
 /** @file
   Hotkey library functions.
 
-Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
 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 @@ -441,6 +441,54 
@@ BmHotkeyCallback (  }
 
 /**
+  Return the active Simple Text Input Ex handle array.
+  If the SystemTable.ConsoleInHandle is NULL, the function returns all  
+ founded Simple Text Input Ex handles.
+  Otherwise, it just returns the ConsoleInHandle.
+
+  @param Count  Return the handle count.
+
+  @retval The active console handles.
+**/
+EFI_HANDLE *
+BmGetActiveConsoleIn (
+  OUT UINTN *Count
+  )
+{
+  EFI_STATUSStatus;
+  EFI_HANDLE*Handles;
+
+  if (gST->ConsoleInHandle != NULL) {
+Status = gBS->OpenProtocol (
+gST->ConsoleInHandle,
+,
+NULL,
+gImageHandle,
+NULL,
+EFI_OPEN_PROTOCOL_TEST_PROTOCOL
+);
+if (!EFI_ERROR (Status)) {
+  Handles = AllocateCopyPool (sizeof (EFI_HANDLE *), 
>ConsoleInHandle);
+  *Count  = 1;
+}
+  } else {
+Status = gBS->LocateHandleBuffer (
+ByProtocol,
+,
+NULL,
+Count,
+
+);
+  }
+  if (EFI_ERROR (Status)) {
+Handles = NULL;
+*Count  = 0;
+  }
+
+  return Handles;
+}
+
+/**
   Unregister hotkey notify list.
 
   @paramHotkeyHotkey list.
@@ -461,13 +509,7 @@ BmUnregisterHotkeyNotify (
   EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *TxtInEx;
   VOID  *NotifyHandle;
 
-  gBS->LocateHandleBuffer (
-  ByProtocol,
-  ,
-  NULL,
-  ,
-  
-  );
+  Handles = BmGetActiveConsoleIn ();
   for (Index = 0; Index < HandleCount; Index++) {
 Status = gBS->HandleProtocol (Handles[Index], 
, (VOID **) );
 ASSERT_EFI_ERROR (Status);
@@ -485,6 +527,10 @@ BmUnregisterHotkeyNotify (
 }
   }
 
+  if (Handles != NULL) {
+FreePool (Handles);
+  }
+
   return EFI_SUCCESS;
 }
 
@@ -636,6 +682,8 @@ BmProcessKeyOption (
 
   EfiAcquireLock ();
 
+  Handles = BmGetActiveConsoleIn ();
+
   for (Index = 0; Index < KeyShiftStateCount; Index++) {
 Hotkey = AllocateZeroPool (sizeof (BM_HOTKEY));
 ASSERT (Hotkey != NULL);
@@ -651,13 +699,6 @@ BmProcessKeyOption (
 }
 InsertTailList (, >Link);
 
-gBS->LocateHandleBuffer (
-ByProtocol,
-,
-NULL,
-,
-
-);
 for (HandleIndex = 0; HandleIndex < HandleCount; HandleIndex++) {
   Status = gBS->HandleProtocol (Handles[HandleIndex], 
, (VOID **) );
   ASSERT_EFI_ERROR (Status);
@@ -665,6 +706,9 @@ BmProcessKeyOption (
 }
   }
 
+  if (Handles != NULL) {
+FreePool (Handles);
+  }
   EfiReleaseLock ();
 
   return EFI_SUCCESS;
@@ -875,13 +919,20 @@ EfiBootManagerStartHotkeyService (
 BmProcessKeyOption (mBmContinueKeyOption);
   }
 
-  EfiCreateProtocolNotifyEvent (
-,
-TPL_CALLBACK,
-BmTxtInExCallback,
-NULL,
-
-);
+  //
+  // Hook hotkey on every future SimpleTextInputEx instance when  // 
+ SystemTable.ConsoleInHandle == NULL, which means the console  // 
+ manager (ConSplitter) is absent.
+  //
+  if (gST->ConsoleInHandle == NULL) {
+EfiCreateProtocolNotifyEvent (
+  ,
+  TPL_CALLBACK,
+  BmTxtInExCallback,
+  NULL,
+  
+  );
+  }
 
   Status = EfiCreateEventReadyToBootEx (
  TPL_CALLBACK,
@@ -891,7 +942,6 @@ EfiBootManagerStartHotkeyService (
  );
   ASSERT_EFI_ERROR (Status);
 
-
   mBmHotkeyServiceStarted = 

Re: [edk2] [Patch] MdeModulePkg/Bds: Fix VS2010/VS2012 build failure.

[Wang, Sunny (HPS SW)]

Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, March 07, 2016 1:30 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni; Shumin Qiu
Subject: [edk2] [Patch] MdeModulePkg/Bds: Fix VS2010/VS2012 build failure.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Shumin Qiu 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 18f835a..4225e80 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1667,6 +1667,8 @@ BmGetFileBufferFromLoadFileFileSystem (
 Handles = NULL;
 HandleCount = 0;
   }
+
+  Handle = NULL;
   for (Index = 0; Index < HandleCount; Index++) {
 Node = DevicePathFromHandle (Handles[Index]);
 Status = gBS->LocateDevicePath (, , );
-- 
2.7.0.windows.1

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

Re: [edk2] [Patch 1/4] MdeModulePkg/Bds: Refine the code to load file from FV.

[Wang, Sunny (HPS SW)]

This one looks good to me.
Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Wednesday, February 24, 2016 3:40 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni; Feng Tian; Eric Dong
Subject: [edk2] [Patch 1/4] MdeModulePkg/Bds: Refine the code to load file from 
FV.

Change BmGetFileBufferByMemmapFv to BmGetFileBufferByFvFilePath.
The original function gets the file buffer only from memory mapped FV device 
path and leaves GUIDed FV device path to the code below; The new function gets 
the file buffer from both formats of FV device paths.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
Cc: Eric Dong 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 63 +++-
 1 file changed, 40 insertions(+), 23 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 7297a1d..a6826f6 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1,7 +1,7 @@
 /** @file
   Library functions which relates with booting.
 
-Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP  This program 
and the accompanying materials  are licensed and made available under the terms 
and conditions of the BSD License @@ -244,15 +244,15 @@ 
BmFindBootOptionInVariable (
 
   FV address may change across reboot. This routine promises the FV file 
device path is right.
 
-  @param  DevicePath   The Memory Mapped Device Path to get the file buffer.
+  @param  FilePath The Memory Mapped Device Path to get the file buffer.
   @param  FullPath Receive the updated FV Device Path pointint to the file.
   @param  FileSize Receive the file buffer size.
 
   @return  The file buffer.
 **/
 VOID *
-BmGetFileBufferByMemmapFv (
-  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath,
+BmGetFileBufferByFvFilePath (
+  IN EFI_DEVICE_PATH_PROTOCOL  *FilePath,
   OUT EFI_DEVICE_PATH_PROTOCOL **FullPath,
   OUT UINTN*FileSize
   )
@@ -267,18 +267,28 @@ BmGetFileBufferByMemmapFv (
   EFI_HANDLE*FvHandles;
   EFI_DEVICE_PATH_PROTOCOL  *NewDevicePath;
   VOID  *FileBuffer;
-
-  FvFileNode = DevicePath;
+
+  //
+  // Get the file buffer by using the exactly FilePath.
+  //
+  FvFileNode = FilePath;
   Status = gBS->LocateDevicePath (, 
, );
   if (!EFI_ERROR (Status)) {
-FileBuffer = GetFileBufferByFilePath (TRUE, DevicePath, FileSize, 
);
+FileBuffer = GetFileBufferByFilePath (TRUE, FilePath, FileSize, 
+ );
 if (FileBuffer != NULL) {
-  *FullPath = DuplicateDevicePath (DevicePath);
+  *FullPath = DuplicateDevicePath (FilePath);
 }
 return FileBuffer;
   }
 
-  FvFileNode = NextDevicePathNode (DevicePath);
+  //
+  // Only wide match other FVs if it's a memory mapped FV file path.
+  //
+  if ((DevicePathType (FilePath) != HARDWARE_DEVICE_PATH) || 
(DevicePathSubType (FilePath) != HW_MEMMAP_DP)) {
+return NULL;
+  }
+
+  FvFileNode = NextDevicePathNode (FilePath);
 
   //
   // Firstly find the FV file in current FV @@ -289,7 +299,7 @@ 
BmGetFileBufferByMemmapFv (
  (VOID **) 
  );
   NewDevicePath = AppendDevicePathNode (DevicePathFromHandle 
(LoadedImage->DeviceHandle), FvFileNode);
-  FileBuffer = BmGetFileBufferByMemmapFv (NewDevicePath, FullPath, FileSize);
+  FileBuffer = BmGetFileBufferByFvFilePath (NewDevicePath, FullPath, 
+ FileSize);
   FreePool (NewDevicePath);
 
   if (FileBuffer != NULL) {
@@ -314,7 +324,7 @@ BmGetFileBufferByMemmapFv (
   continue;
 }
 NewDevicePath = AppendDevicePathNode (DevicePathFromHandle 
(FvHandles[Index]), FvFileNode);
-FileBuffer = BmGetFileBufferByMemmapFv (NewDevicePath, FullPath, FileSize);
+FileBuffer = BmGetFileBufferByFvFilePath (NewDevicePath, FullPath, 
+ FileSize);
 FreePool (NewDevicePath);
   }
   
@@ -325,29 +335,36 @@ BmGetFileBufferByMemmapFv (  }
 
 /**
-  Check if it's a Memory Mapped FV Device Path.
+  Check if it's a Device Path pointing to FV file.
   
   The function doesn't garentee the device path points to existing FV file.
 
   @param  DevicePath Input device path.
 
-  @retval TRUE   The device path is a Memory Mapped FV Device Path.
-  @retval FALSE  The device path is NOT a Memory Mapped FV Device Path.
+  @retval TRUE   The device path is a FV File Device Path.
+  @retval FALSE  The device path is NOT a FV File Device Path.
 **/
 BOOLEAN
-BmIsMemmapFvFilePath (
+BmIsFvFilePath (
   IN EFI_DEVICE_PATH_PROTOCOL*DevicePath
   )
 {
-  EFI_DEVICE_PATH_PROTOCOL   *FileNode;
+  EFI_STATUS Status;
+  EFI_HANDLE 

Re: [edk2] [Patch 3/4] MdeModulePkg/Bds: Support short-form URI boot.

[Wang, Sunny (HPS SW)]

I was not aware that there is possible to have no EfiBootManagerConnectAll() 
before booting the http boot option with a short-form URI device path. 
Thanks for pointing this out. 

Reviewed-by: Sunny Wang <sunnyw...@hpe.com>

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Friday, February 26, 2016 8:28 PM
To: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Cc: Fu, Siyuan; El-Haj-Mahmoud, Samer
Subject: RE: [edk2] [Patch 3/4] MdeModulePkg/Bds: Support short-form URI boot.
Importance: High

Sunny,
For a short-form URI device path, it's like Uri(...)\End.
We cannot get the parent handle from such short-form URI device path.
So we have to use EfiBootManagerConnectAll().

Regards,
Ray


>-Original Message-
>From: Wang, Sunny (HPS SW) [mailto:sunnyw...@hpe.com]
>Sent: Friday, February 26, 2016 5:28 PM
>To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org
>Cc: Fu, Siyuan <siyuan...@intel.com>; Wang, Sunny (HPS SW) 
><sunnyw...@hpe.com>; El-Haj-Mahmoud, Samer 
><samer.el-haj-mahm...@hpe.com>
>Subject: RE: [edk2] [Patch 3/4] MdeModulePkg/Bds: Support short-form URI boot.
>
>Hi Ray,
>   For this patch, I just have a question:
>   - Why do we need to call EfiBootManagerConnectAll in 
>BmExpandUriDevicePath? It would cause additional and visible boot time. 
>If we just want to connect URI boot option's parent handle, we can use 
>EfiBootManagerConnectDevicePath instead.
>
>Regards,
>Sunny Wang
>
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>Ruiyu Ni
>Sent: Wednesday, February 24, 2016 3:40 PM
>To: edk2-devel@lists.01.org
>Cc: Ruiyu Ni; Siyuan Fu
>Subject: [edk2] [Patch 3/4] MdeModulePkg/Bds: Support short-form URI boot.
>
>The patch adds short-form URI boot support to follow UEFI Spec.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
>Cc: Siyuan Fu <siyuan...@intel.com>
>---
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 72 
>
> 1 file changed, 72 insertions(+)
>
>diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>index 35234ba..9ce841e 100644
>--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>@@ -1182,6 +1182,72 @@ BmExpandFileDevicePath (  }
>
> /**
>+  Expand URI device path node to be full device path in platform.
>+
>+  @param FilePath  The device path pointing to a load option.
>+   It could be a short-form device path.
>+  @param FullPath  Return the full device path of the load option after
>+   short-form device path expanding.
>+   Caller is responsible to free it.
>+  @param FileSize  Return the load option size.
>+
>+  @return The load option buffer. Caller is responsible to free the memory.
>+**/
>+VOID *
>+BmExpandUriDevicePath (
>+  IN  EFI_DEVICE_PATH_PROTOCOL*FilePath,
>+  OUT EFI_DEVICE_PATH_PROTOCOL**FullPath,
>+  OUT UINTN   *FileSize
>+  )
>+{
>+  EFI_STATUS  Status;
>+  UINTN   Index;
>+  UINTN   HandleCount;
>+  EFI_HANDLE  *Handles;
>+  EFI_LOAD_FILE_PROTOCOL  *LoadFile;
>+  VOID*FileBuffer;
>+
>+  EfiBootManagerConnectAll ();
>+  Status = gBS->LocateHandleBuffer (ByProtocol, 
>+ , NULL, , );  if (EFI_ERROR 
>(Status)) {
>+HandleCount = 0;
>+Handles = NULL;
>+  }
>+
>+  FileBuffer = NULL;
>+
>+  for (Index = 0; Index < HandleCount; Index++) {
>+Status = gBS->HandleProtocol (Handles[Index], , 
>(VOID *) );
>+ASSERT_EFI_ERROR (Status);
>+
>+FileBuffer = NULL;
>+Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, FileSize, 
>FileBuffer);
>+if (Status == EFI_BUFFER_TOO_SMALL) {
>+  FileBuffer = AllocatePool (*FileSize);
>+  if (FileBuffer == NULL) {
>+break;
>+  }
>+  Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, FileSize, 
>FileBuffer);
>+}
>+
>+if (!EFI_ERROR (Status)) {
>+  *FullPath = DuplicateDevicePath (DevicePathFromHandle (Handles[Index]));
>+  break;
>+}
>+
>+if (FileBuffer != NULL) {
>+  FreePool (FileBuffer);
>+}
>+  }
>+
>+  if (Handles != NULL) {
>+FreePool (Handles);
>+  }
>+
>+  return FileBuffer;
>+}
>+
>+/**
>   Save the partition DevicePath to the CachedDevicePath as the first instance.
>
>   @param CachedDevicePath  The device path cache

Re: [edk2] [Patch 2/4] MdeModulePkg/Bds: Wide match HTTP boot option.

[Wang, Sunny (HPS SW)]

Thanks for your explanation which makes sense to me.  I agree that if we don't 
have a need to support loading file buffer from a HTTP Boot driver mounted 
RAMDISK for non-BDS core module, this implementation is good and clearer. 

Reviewed-by: Sunny Wang <sunnyw...@hpe.com>

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Friday, February 26, 2016 8:25 PM
To: Wang, Sunny (HPS SW)
Cc: Fu, Siyuan; edk2-devel@lists.01.org
Subject: RE: [edk2] [Patch 2/4] MdeModulePkg/Bds: Wide match HTTP boot option.
Importance: High

Comments below.

Regards,
Ray


>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>Wang, Sunny (HPS SW)
>Sent: Friday, February 26, 2016 5:18 PM
>To: Ni, Ruiyu <ruiyu...@intel.com>
>Cc: Fu, Siyuan <siyuan...@intel.com>; edk2-devel@lists.01.org
>Subject: Re: [edk2] [Patch 2/4] MdeModulePkg/Bds: Wide match HTTP boot option.
>
>Hi Ray,
>   It looks like you replace GetFileBufferByFilePath function call with 
>both gEfiSimpleFileSystemProtocolGuid handling code block and 
>BmGetFileBufferFromLoadFile in BmGetLoadOptionBuffer, and the 
>BmGetFileBufferFromLoadFile is for replacing  gEfiLoadFileProtocolGuid related 
>code block in GetFileBufferByFilePath with the additional code for wide 
>matching HTTP boot option. Based on the changes I mentioned, I have two 
>questions below:
>   1. Why don't we just add the additional code for wide matching HTTP 
>boot option into GetFileBufferByFilePath to continue using 
>GetFileBufferByFilePath? This way seems easier and simpler.

GetFileBufferByFilePath() is a core library function. And even we enhance it, 
it still cannot support the ram disk boot (implemented in Patch 4/4). So I 
chose to not make the GetFileBufferByFilePath complex.

>   2. The GetFileBufferByFilePath function includes a code block for 
>gEfiLoadFile2ProtocolGuid, but the replacement in your code change 
>(BmGetFileBufferFromLoadFile) doesn't have code to handle 
>gEfiLoadFile2ProtocolGuid. Is it an issue or something I overlooked?

Per UFEI spec, boot option only maps to LoadFile protocol. LoadFile2 protocol 
is for PCI option rom loading, not for booting.

>
>Regards,
>Sunny Wang
>
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>Ruiyu Ni
>Sent: Wednesday, February 24, 2016 3:40 PM
>To: edk2-devel@lists.01.org
>Cc: Ruiyu Ni; Siyuan Fu
>Subject: [edk2] [Patch 2/4] MdeModulePkg/Bds: Wide match HTTP boot option.
>
>Enhance BDS to wide match the HTTP boot option without matching the 
>specific device path data in IP device path and URI device path node.
>It's to follow UEFI Spec 2.6.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
>Cc: Siyuan Fu <siyuan...@intel.com>
>---
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 149 
>+--
> 1 file changed, 137 insertions(+), 12 deletions(-)
>
>diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>index a6826f6..35234ba 100644
>--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>@@ -1528,6 +1528,136 @@ BmExpandMediaDevicePath (  }
>
> /**
>+  Check whether Left and Right are the same without matching the 
>+ specific  device path data in IP device path and URI device path node.
>+
>+  @retval TRUE  Left and Right are the same.
>+  @retval FALSE Left and Right are the different.
>+**/
>+BOOLEAN
>+BmMatchHttpBootDevicePath (
>+  IN EFI_DEVICE_PATH_PROTOCOL *Left,
>+  IN EFI_DEVICE_PATH_PROTOCOL *Right
>+  )
>+{
>+  for (;  !IsDevicePathEnd (Left) && !IsDevicePathEnd (Right)
>+   ;  Left = NextDevicePathNode (Left), Right = NextDevicePathNode (Right)
>+   ) {
>+if (CompareMem (Left, Right, DevicePathNodeLength (Left)) != 0) {
>+  if ((DevicePathType (Left) != MESSAGING_DEVICE_PATH) || 
>+(DevicePathType (Right) != MESSAGING_DEVICE_PATH))
>{
>+return FALSE;
>+  }
>+
>+  if (((DevicePathSubType (Left) != MSG_IPv4_DP) || (DevicePathSubType 
>(Right) != MSG_IPv4_DP)) &&
>+  ((DevicePathSubType (Left) != MSG_IPv6_DP) || (DevicePathSubType 
>(Right) != MSG_IPv6_DP)) &&
>+  ((DevicePathSubType (Left) != MSG_URI_DP)  || (DevicePathSubType 
>(Right) != MSG_URI_DP))
>+  ) {
>+return FALSE;
>+  }
>+}
>+  }
>+  return (BOOLEAN) (IsDevicePathEnd (Left) && IsDevicePathEnd 
>+(Right)); }
>+
>+/**
>+  Get the file buffer from Load File instance.
>+
>+  @param FilePathThe media device path pointing to a LoadFile

Re: [edk2] [Patch] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Wang, Sunny (HPS SW)]

Hi Siyuan, 
I just have two suggestions and a question below. Others look good to 
me.
1. It looks like the code blocks in the HttpBootAddBootOption function 
can be replaced with the UefiBootManagerLib functions (like 
EfiBootManagerInitializeLoadOption and EfiBootManagerAddLoadOptionVariable), 
which can save the maintenance effort on the duplicate code. What do you think? 
2. Is it better to change options' string from both "IP4" and "IP6" to 
"IPv4" and "IPv6"? I mean both STR_HTTP_BOOT_IP_VERSION_4 and  
STR_HTTP_BOOT_IP_VERSION_6.
3. Will you send the other patch for HTTP Boot driver RAMDISK related 
part?  From Ray's implementation (patch 4: MdeModulePkg/Bds: Support booting 
from remote file system.) and UEFI spec section 23.7.3.1, the HTTP Boot driver 
will register RAM disk for binary image file which contains UEFI-compliant file 
system and return EFI_WARN_FILE_SYSTEM status to BDS code.

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu Siyuan
Sent: Wednesday, February 24, 2016 1:27 PM
To: edk2-devel@lists.01.org
Cc: Ye Ting; Wu Jiaxin
Subject: [edk2] [Patch] NetworkPkg: Add URI configuration form to HTTP boot 
driver.

This patch updates the HTTP boot driver to produce a setup page for the boot
file URI configuration. A new boot option will be created for the manual
configured URI address. This change is made to support the HTTP boot usage
in home environment.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Wu Jiaxin 
Cc: Ye Ting 
---
 NetworkPkg/HttpBootDxe/HttpBootClient.c|  99 +--
 NetworkPkg/HttpBootDxe/HttpBootConfig.c| 711 +
 NetworkPkg/HttpBootDxe/HttpBootConfig.h|  78 +++
 NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h |  43 ++
 NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni   | Bin 0 -> 2926 bytes
 NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr   |  53 ++
 NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 111 ++--
 NetworkPkg/HttpBootDxe/HttpBootDhcp4.h |   9 +-
 NetworkPkg/HttpBootDxe/HttpBootDhcp6.c |   6 +-
 NetworkPkg/HttpBootDxe/HttpBootDxe.c   |  44 +-
 NetworkPkg/HttpBootDxe/HttpBootDxe.h   |  33 +-
 NetworkPkg/HttpBootDxe/HttpBootDxe.inf |  17 +-
 NetworkPkg/HttpBootDxe/HttpBootImpl.c  |  71 +-
 NetworkPkg/HttpBootDxe/HttpBootSupport.c   |  55 ++
 NetworkPkg/HttpBootDxe/HttpBootSupport.h   |  18 +
 NetworkPkg/Include/Guid/HttpBootConfigHii.h|  25 +
 NetworkPkg/NetworkPkg.dec  |   5 +-
 17 files changed, 1268 insertions(+), 110 deletions(-)
 create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.c
 create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.h
 create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h
 create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni
 create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr
 create mode 100644 NetworkPkg/Include/Guid/HttpBootConfigHii.h

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c 
b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index dd835c4..37333ee 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -167,18 +167,35 @@ HttpBootDhcp4ExtractUriInfo (
   // HttpOffer contains the boot file URL.
   //
   SelectOffer = >OfferBuffer[SelectIndex].Dhcp4;
-  if ((SelectOffer->OfferType == HttpOfferTypeDhcpIpUri) || 
(SelectOffer->OfferType == HttpOfferTypeDhcpNameUriDns)) {
-HttpOffer = SelectOffer;
+  if (Private->FilePathUri == NULL) {
+//
+// In Corporate environment, we need a HttpOffer.
+//
+if ((SelectOffer->OfferType == HttpOfferTypeDhcpIpUri) || 
+(SelectOffer->OfferType == HttpOfferTypeDhcpIpUriDns) ||
+(SelectOffer->OfferType == HttpOfferTypeDhcpNameUriDns)) {
+  HttpOffer = SelectOffer;
+} else {
+  ASSERT (Private->SelectProxyType != HttpOfferTypeMax);
+  ProxyIndex = Private->OfferIndex[Private->SelectProxyType][0];
+  HttpOffer = >OfferBuffer[ProxyIndex].Dhcp4;
+}
+Private->BootFileUriParser = HttpOffer->UriParser;
+Private->BootFileUri = (CHAR8*) 
HttpOffer->OptList[HTTP_BOOT_DHCP4_TAG_INDEX_BOOTFILE]->Data;
   } else {
-ASSERT (Private->SelectProxyType != HttpOfferTypeMax);
-ProxyIndex = Private->OfferIndex[Private->SelectProxyType][0];
-HttpOffer = >OfferBuffer[ProxyIndex].Dhcp4;
+//
+// In Home environment the BootFileUri comes from the FilePath.
+//
+Private->BootFileUriParser = Private->FilePathUriParser;
+Private->BootFileUri = Private->FilePathUri;
   }
 
   //
   // Configure the default DNS server if server assigned.
   //
-  if ((SelectOffer->OfferType == HttpOfferTypeDhcpNameUriDns) || 
(SelectOffer->OfferType == 

Re: [edk2] [Patch 4/4] MdeModulePkg/Bds: Support booting from remote file system.

[Wang, Sunny (HPS SW)]

This one looks good to me.

Reviewed-by: Sunny Wang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Wednesday, February 24, 2016 3:40 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni; Siyuan Fu
Subject: [edk2] [Patch 4/4] MdeModulePkg/Bds: Support booting from remote file 
system.

Enhance BDS to support booting from a remote file system exposed by a HTTP boot 
option.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Siyuan Fu 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 72 ++
 .../Library/UefiBootManagerLib/InternalBm.h| 18 +-
 2 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 9ce841e..12868df 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1231,6 +1231,14 @@ BmExpandUriDevicePath (
 }
 
 if (!EFI_ERROR (Status)) {
+  //
+  // LoadFile() returns a file buffer mapping to a file system.
+  //
+  if (Status == EFI_WARN_FILE_SYSTEM) {
+return BmGetFileBufferFromLoadFileFileSystem (Handles[Index], 
FullPath, FileSize);
+  }
+
+  ASSERT (Status == EFI_SUCCESS);
   *FullPath = DuplicateDevicePath (DevicePathFromHandle (Handles[Index]));
   break;
 }
@@ -1626,6 +1634,62 @@ BmMatchHttpBootDevicePath (  }
 
 /**
+  Get the file buffer from the file system produced by Load File instance.
+
+  @param LoadFileHandle The handle of LoadFile instance.
+  @param FullPath   Return the full device path pointing to the load 
option.
+  @param FileSize   Return the size of the load option.
+
+  @return  The load option buffer.
+**/
+VOID *
+BmGetFileBufferFromLoadFileFileSystem (
+  IN  EFI_HANDLE  LoadFileHandle,
+  OUT EFI_DEVICE_PATH_PROTOCOL**FullPath,
+  OUT UINTN   *FileSize
+  )
+{
+  EFI_STATUS  Status;
+  EFI_HANDLE  Handle;
+  EFI_HANDLE  *Handles;
+  UINTN   HandleCount;
+  UINTN   Index;
+  EFI_DEVICE_PATH_PROTOCOL*Node;
+
+  Status = gBS->LocateHandleBuffer (
+  ByProtocol,
+  ,
+  NULL,
+  ,
+  
+  );
+  if (EFI_ERROR (Status)) {
+Handles = NULL;
+HandleCount = 0;
+  }
+  for (Index = 0; Index < HandleCount; Index++) {
+Node = DevicePathFromHandle (Handles[Index]);
+Status = gBS->LocateDevicePath (, , );
+if (!EFI_ERROR (Status) &&
+(Handle == LoadFileHandle) &&
+(DevicePathType (Node) == MEDIA_DEVICE_PATH) && (DevicePathSubType 
(Node) == MEDIA_RAM_DISK_DP)) {
+  Handle = Handles[Index];
+  break;
+}
+  }
+
+  if (Handles != NULL) {
+FreePool (Handles);
+  }
+
+  if (Index != HandleCount) {
+return BmExpandMediaDevicePath (DevicePathFromHandle (Handle), 
+FullPath, FileSize);
+  } else {
+return NULL;
+  }
+}
+
+/**
   Get the file buffer from Load File instance.
 
   @param FilePathThe media device path pointing to a LoadFile instance.
@@ -1713,6 +1777,14 @@ BmGetFileBufferFromLoadFile (
 
   if (!EFI_ERROR (Status)) {
 //
+// LoadFile() returns a file buffer mapping to a file system.
+//
+if (Status == EFI_WARN_FILE_SYSTEM) {
+  return BmGetFileBufferFromLoadFileFileSystem (Handle, FullPath, 
FileSize);
+}
+
+ASSERT (Status == EFI_SUCCESS);
+//
 // LoadFile () may cause the device path of the Handle be updated.
 //
 *FullPath = DuplicateDevicePath (DevicePathFromHandle (Handle)); diff 
--git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index fa4d5af..cfaeefe 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -1,7 +1,7 @@
 /** @file
   BDS library definition, include the file and data structure
 
-Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP  This program 
and the accompanying materials  are licensed and made available under the terms 
and conditions of the BSD License @@ -456,4 +456,20 @@ BmCharToUint (
   IN CHAR16   Char
   );
 
+
+/**
+  Get the file buffer from the file system produced by Load File instance.
+
+  @param LoadFileHandle The handle of LoadFile instance.
+  @param FullPath   Return the full device path pointing to the load 
option.
+  @param FileSize   Return the size of the load option.
+
+  @return  The load option buffer.
+**/
+VOID *

Re: [edk2] [Patch 07/11] MdeModulePkg: Add Platform recovery support

[Wang, Sunny (HPS SW)]

Hi Ray,
Yes, you are right! Thanks for pointing it out. :)
I took an inappropriate reference (in ArmPlatformPkg\Bds\Bds.c) which caused my 
misunderstanding. 

Regards,
Sunny Wang


-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Monday, November 09, 2015 7:25 PM
To: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Cc: Dong, Eric
Subject: RE: [edk2] [Patch 07/11] MdeModulePkg: Add Platform recovery support
Importance: High

Sunny,
I guess you may have misunderstanding about the EndOfDxe signal time. This 
event is not signaled immediately before calling Bds->Entry(). This event is 
signaled by platform (in PlatformBootManagerBeforeConsole).
So it means all the PlatformRecovery variables need to be created in core 
(before PlatformBootManagerBeforeConsole()) or in 
PlatformBootManagerBeforeConsole().

I agree with your other comments.

Regards,
Ray

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Thursday, November 5, 2015 9:17 PM
To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org
Cc: Dong, Eric <eric.d...@intel.com>
Subject: Re: [edk2] [Patch 07/11] MdeModulePkg: Add Platform recovery support

Hi Ray,
I made four comments in this patch. You can search "[Sunny]" to find 
them out. 

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, November 02, 2015 7:34 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni; Eric Dong
Subject: [edk2] [Patch 07/11] MdeModulePkg: Add Platform recovery support

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
Cc: Eric Dong <eric.d...@intel.com>
---
 MdeModulePkg/Include/Library/UefiBootManagerLib.h  |   1 +
 .../Library/UefiBootManagerLib/BmLoadOption.c  | 130 ++---
 .../Library/UefiBootManagerLib/InternalBm.h|   3 +-
 .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
 4 files changed, 118 insertions(+), 17 deletions(-)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h 
b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index 5538d90..1b04a8c 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -30,6 +30,7 @@ typedef enum {
   LoadOptionTypeDriver,
   LoadOptionTypeSysPrep,
   LoadOptionTypeBoot,
+  LoadOptionTypePlatformRecovery,
   LoadOptionTypeMax
 } EFI_BOOT_MANAGER_LOAD_OPTION_TYPE;
 
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index fbd7830..454fe20 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -18,14 +18,16 @@ GLOBAL_REMOVE_IF_UNREFERENCED
   CHAR16 *mBmLoadOptionName[] = {
 L"Driver",
 L"SysPrep",
-L"Boot"
+L"Boot",
+L"PlatformRecovery"
   };
 
 GLOBAL_REMOVE_IF_UNREFERENCED
   CHAR16 *mBmLoadOptionOrderName[] = {
 EFI_DRIVER_ORDER_VARIABLE_NAME,
 EFI_SYS_PREP_ORDER_VARIABLE_NAME,
-EFI_BOOT_ORDER_VARIABLE_NAME
+EFI_BOOT_ORDER_VARIABLE_NAME,
+NULL  // PlatformRecovery doesn't have associated *Order 
+ variable
   };
 
 /**
@@ -153,8 +155,9 @@ BmGetFreeOptionNumber (  }
 
 /**
-  Create the Boot, Driver, SysPrep, variable from the load option.
-  
+  Create the Boot, Driver, SysPrep, PlatformRecovery 
+ variable  from the load option.
+
   @param  LoadOption  Pointer to the load option.
 
   @retval EFI_SUCCESS The variable was created.
@@ -166,12 +169,14 @@ EfiBootManagerLoadOptionToVariable (
   IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Option
   )
 {
+  EFI_STATUS   Status;
   UINTNVariableSize;
   UINT8*Variable;
   UINT8*Ptr;
   CHAR16   OptionName[BM_OPTION_NAME_LEN];
   CHAR16   *Description;
   CHAR16   NullChar;
+  EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;
   UINT32   VariableAttributes;
 
   if ((Option->OptionNumber == LoadOptionNumberUnassigned) || @@ -232,6 
+237,17 @@ structure.
   UnicodeSPrint (OptionName, sizeof (OptionName), L"%s%04x", 
mBmLoadOptionName[Option->OptionType], Option->OptionNumber);
 
   VariableAttributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE;
+  if (Option->OptionType == LoadOptionTypePlatformRecovery) {
+//
+// Lock the PlatformRecovery
+//
+Status = gBS->LocateProtocol (, NULL, (VOID 
**) );
+if (!EFI_ERROR (Status)) {
+  Status = VariableLock->RequestToLock (VariableLock, OptionName, 
);
+  ASSERT_EFI_ERROR (St

Re: [edk2] [Patch 03/11] MdeModulePkg: Use BmCharToUint in BmIsKeyOptionVariable

[Wang, Sunny (HPS SW)]

Hi Ray,

The patches 1, 2, 3, 5  look good to me as well. Thanks for making these 
changes. 

Reviewed by: Sunny Wang 

Regards,
Sunny Wang
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, November 02, 2015 7:33 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni; Eric Dong
Subject: [edk2] [Patch 03/11] MdeModulePkg: Use BmCharToUint in 
BmIsKeyOptionVariable

The patch also moves the BmCharToUint to BmMisc.c because it belongs to misc 
functions.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Eric Dong 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c | 10 -
 .../Library/UefiBootManagerLib/BmLoadOption.c  | 23 ---
 MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c   | 26 ++
 .../Library/UefiBootManagerLib/InternalBm.h| 13 +++
 4 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
index 8d398fb..ff1cbe9 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
@@ -88,6 +88,7 @@ BmIsKeyOptionVariable (
   )
 {
   UINTN Index;
+  UINTN Uint;
   
   if (!CompareGuid (Guid, ) ||
   (StrSize (Name) != sizeof (L"Key")) || @@ -98,12 +99,11 @@ 
BmIsKeyOptionVariable (
 
   *OptionNumber = 0;
   for (Index = 3; Index < 7; Index++) {
-if ((Name[Index] >= L'0') && (Name[Index] <= L'9')) {
-  *OptionNumber = *OptionNumber * 16 + Name[Index] - L'0';
-} else if ((Name[Index] >= L'A') && (Name[Index] <= L'F')) {
-  *OptionNumber = *OptionNumber * 16 + Name[Index] - L'A' + 10;
-} else {
+Uint = BmCharToUint (Name[Index]);
+if (Uint == -1) {
   return FALSE;
+} else {
+  *OptionNumber = (UINT16) Uint + *OptionNumber * 0x10;
 }
   }
 
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index a348e81..fbd7830 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -585,29 +585,6 @@ EfiBootManagerDeleteLoadOptionVariable (  }
 
 /**
-  Convert a single character to number.
-  It assumes the input Char is in the scope of L'0' ~ L'9' and L'A' ~ L'F'
-
-  @paramChar   The input char which need to convert to int.
-**/
-UINTN
-BmCharToUint (
-  IN CHAR16   Char
-  )
-{
-  if ((Char >= L'0') && (Char <= L'9')) {
-return (UINTN) (Char - L'0');
-  }
-
-  if ((Char >= L'A') && (Char <= L'F')) {
-return (UINTN) (Char - L'A' + 0xA);
-  }
-
-  ASSERT (FALSE);
-  return (UINTN) -1;
-}
-
-/**
   Returns the size of a device path in bytes.
 
   This function returns the size, in bytes, of the device path data structure 
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
index 97d1a48..cc2032c 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
@@ -384,3 +384,29 @@ BmPrintDp (
 FreePool (Str);
   }
 }
+
+/**
+  Convert a single character to number.
+  It assumes the input Char is in the scope of L'0' ~ L'9' and L'A' ~ L'F'
+
+  @paramChar   The input char which need to convert to int.
+
+  @return  The converted 8-bit number or (UINTN) -1 if conversion failed.
+**/
+UINTN
+BmCharToUint (
+  IN CHAR16   Char
+  )
+{
+  if ((Char >= L'0') && (Char <= L'9')) {
+return (UINTN) (Char - L'0');
+  }
+
+  if ((Char >= L'A') && (Char <= L'F')) {
+return (UINTN) (Char - L'A' + 0xA);  }
+
+  ASSERT (FALSE);
+  return (UINTN) -1;
+}
+
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 9a80da9..7f52b13 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -454,4 +454,17 @@ BmPrintDp (
   EFI_DEVICE_PATH_PROTOCOL*DevicePath
   );
 
+/**
+  Convert a single character to number.
+  It assumes the input Char is in the scope of L'0' ~ L'9' and L'A' ~ L'F'
+
+  @paramChar   The input char which need to convert to int.
+
+  @return  The converted 8-bit number or (UINTN) -1 if conversion failed.
+**/
+UINTN
+BmCharToUint (
+  IN CHAR16   Char
+  );
+
 #endif // _INTERNAL_BM_H_
--
1.9.5.msysgit.1

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

Re: [edk2] [Patch 08/11] MdeModulePkg: Add missing PrintLib to BdsDxe.inf

[Wang, Sunny (HPS SW)]

This patch looks good to me. 
Reviewed by: Sunny Wang 

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, November 02, 2015 7:34 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni; Eric Dong
Subject: [edk2] [Patch 08/11] MdeModulePkg: Add missing PrintLib to BdsDxe.inf

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Eric Dong 
---
 MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf 
b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
index 12d61e7..bdbc83e 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
@@ -58,6 +58,7 @@
   UefiBootManagerLib
   PlatformBootManagerLib
   PcdLib
+  PrintLib
 
 [Guids]
   gEfiGlobalVariableGuid## SOMETIMES_PRODUCES ## 
Variable:L"BootNext" (The number of next boot option)
-- 
1.9.5.msysgit.1

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

Re: [edk2] [Patch 11/11] MdeModulePkg: Enable PlatformRecovery in BdsDxe driver

[Wang, Sunny (HPS SW)]

Hi Ray,
The places for launching PlatformRecovery options and boot menu option 
(for Successful boot) in this code change look good to me. I think this patch 
can solve problem I mentioned in our previous discussion for 
PlatformBootManagerDefaultBootFail related patch. Thanks for making this code 
change. 
In addition, I made four review comments in this patch. You can search 
"[Sunny]" to find my comments out.
 
Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, November 02, 2015 7:34 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni; Eric Dong
Subject: [edk2] [Patch 11/11] MdeModulePkg: Enable PlatformRecovery in BdsDxe 
driver

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Eric Dong 
---
 MdeModulePkg/Universal/BdsDxe/Bds.h  |   3 -
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 405 ---
 2 files changed, 95 insertions(+), 313 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/Bds.h 
b/MdeModulePkg/Universal/BdsDxe/Bds.h
index 2171d14..ac54482 100644
--- a/MdeModulePkg/Universal/BdsDxe/Bds.h
+++ b/MdeModulePkg/Universal/BdsDxe/Bds.h
@@ -24,9 +24,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
-#include 
-#include 
-#include 
 
 #include   #include  diff 
--git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index b632ada..05cdb73 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -51,10 +51,10 @@ CHAR16  *mReadOnlyVariables[] = {
 CHAR16 *mBdsLoadOptionName[] = {
   L"Driver",
   L"SysPrep",
-  L"Boot"
+  L"Boot",
+  L"PlatformRecovery"
 };
 
-CHAR16  mRecoveryBoot[] = L"Recovery Boot";
 /**
   Event to Connect ConIn.
 
@@ -122,187 +122,6 @@ BdsInitialize (
 }
 
 /**
-  Emuerate all possible bootable medias in the following order:
-  1. Removable BlockIo- The boot option only points to the 
removable media
-device, like USB key, DVD, Floppy etc.
-  2. Fixed BlockIo- The boot option only points to a Fixed 
blockIo device,
-like HardDisk.
-  3. Non-BlockIo SimpleFileSystem - The boot option points to a device 
supporting
-SimpleFileSystem Protocol, but not 
supporting BlockIo
-protocol.
-  4. LoadFile - The boot option points to the media 
supporting 
-LoadFile protocol.
-  Reference: UEFI Spec chapter 3.3 Boot Option Variables Default Boot Behavior
-
-  @param BootOptionCount   Return the boot option count which has been found.
-
-  @retval   Pointer to the boot option array.
-**/
-EFI_BOOT_MANAGER_LOAD_OPTION *
-BdsEnumerateBootOptions (
-  UINTN *BootOptionCount
-  )
-{
-  EFI_STATUSStatus;
-  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
-  UINTN HandleCount;
-  EFI_HANDLE*Handles;
-  EFI_BLOCK_IO_PROTOCOL *BlkIo;
-  UINTN Removable;
-  UINTN Index;
-
-  ASSERT (BootOptionCount != NULL);
-
-  *BootOptionCount = 0;
-  BootOptions  = NULL;
-
-  //
-  // Parse removable block io followed by fixed block io
-  //
-  gBS->LocateHandleBuffer (
- ByProtocol,
- ,
- NULL,
- ,
- 
- );
-
-  for (Removable = 0; Removable < 2; Removable++) {
-for (Index = 0; Index < HandleCount; Index++) {
-  Status = gBS->HandleProtocol (
-  Handles[Index],
-  ,
-  (VOID **) 
-  );
-  if (EFI_ERROR (Status)) {
-continue;
-  }
-
-  //
-  // Skip the logical partitions
-  //
-  if (BlkIo->Media->LogicalPartition) {
-continue;
-  }
-
-  //
-  // Skip the fixed block io then the removable block io
-  //
-  if (BlkIo->Media->RemovableMedia == ((Removable == 0) ? FALSE : TRUE)) {
-continue;
-  }
-
-  BootOptions = ReallocatePool (
-  sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * 
(*BootOptionCount),
-  sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * 
(*BootOptionCount + 1),
-  BootOptions
-  );
-  ASSERT (BootOptions != NULL);
-
-  Status = EfiBootManagerInitializeLoadOption (
- [(*BootOptionCount)++],
- LoadOptionNumberUnassigned,
- LoadOptionTypeBoot,
- LOAD_OPTION_ACTIVE,
- mRecoveryBoot,
- DevicePathFromHandle (Handles[Index]),
- 

Re: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

[Wang, Sunny (HPS SW)]

Hi Jordan, 
Yes, you are right! It could make commit message subject clearer. I saw 
the related information mentioned in 
https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format
This is my mistake. Thanks for the reminder. I will do that next time.

Regards,
Sunny Wang

-Original Message-
From: Jordan Justen [mailto:jordan.l.jus...@intel.com] 
Sent: Tuesday, November 03, 2015 3:57 PM
To: Wang, Sunny (HPS SW); Ni, Ruiyu
Cc: Tian, Feng; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues
Importance: High

Although this was already committed, I had one comment for the future.
In the patch commit message subject, I think you should have included 
UefiBootManagerLib.

MdeModulePkg UefiBootManagerLib: Fix memory leak issues

-Jordan

On 2015-11-02 02:56:14, Wang, Sunny (HPS SW) wrote:
> Hi Ray, 
> Good catch! Thanks. :)
> I updated patch for this. Please help to review it again.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
> Sent: Saturday, October 31, 2015 6:59 AM
> To: Wang, Sunny (HPS SW)
> Cc: Tian, Feng; El-Haj-Mahmoud, Samer; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues
> Importance: High
> 
> Sunny,
> You could move the two FreePool (FullInstance) to one place which is under 
> the LocateDevicePath() call to make the change smaller a bit.
> What do you think?
> 
> Thanks,
> Ray
> 
> -Original Message-
> From: Wang, Sunny (HPS SW) [mailto:sunnyw...@hpe.com]
> Sent: Friday, October 30, 2015 6:21 PM
> To: Ni, Ruiyu <ruiyu...@intel.com>
> Cc: Tian, Feng <feng.t...@intel.com>; El-Haj-Mahmoud, Samer 
> <samer.el-haj-mahm...@hpe.com>; Wang, Sunny (HPS SW) 
> <sunnyw...@hpe.com>; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues
> 
> Hi Ray,
> Are you the owner of this module (UefiBootManagerLib)? If so, could 
> you help to review and commit it?  If not, could you tell me who is the 
> owner? 
> In addition, the attached patch would also need your help to get 
> committed. Thanks!
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: Tian, Feng [mailto:feng.t...@intel.com]
> Sent: Thursday, October 29, 2015 8:21 AM
> To: El-Haj-Mahmoud, Samer; Wang, Sunny (HPS SW); 
> edk2-devel@lists.01.org
> Cc: Tian, Feng
> Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues
> Importance: High
> 
> I will let module owner review and commit it.
> 
> Thanks
> Feng
> 
> -Original Message-
> From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
> Sent: Wednesday, October 28, 2015 22:58
> To: Wang, Sunny (HPS SW); edk2-devel@lists.01.org; Tian, Feng
> Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues
> 
> + MdeModuklePkg maintainers.
> 
> Can someone help by committing this please?
> 
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> El-Haj-Mahmoud, Samer
> Sent: Tuesday, October 27, 2015 10:14 AM
> To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues
> 
> Reviewed-by: Samer El-Haj-Mahmoud <el...@hpe.com>
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Wang, Sunny (HPS SW)
> Sent: Tuesday, October 27, 2015 3:47 AM
> To: edk2-devel@lists.01.org
> Cc: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>
> Subject: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues
> 
> Fix memory leak issues
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> 
> Signed-off-by: Sunny Wang <sunnyw...@hpe.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c 
> b/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c
> index 86b4fac..0830166 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c
> @@ -2,6 +2,7 @@
>Library functions which contain all the code to connect console device.
>  
>  Copyright (c) 2011 - 2015, Intel Corporation. All rights 
> reserved.
> +(C) Copyright 2015 Hewlett Packard Enterprise Development LP
>  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 lic

Re: [edk2] [Patch 09/11] MdeModulePkg: Use UefiSpec.h defined macro to replace L"xxx" string

[Wang, Sunny (HPS SW)]

Hi Ray,
This patch overall looks good to me. I just made a minor comment in 
this patch. You can search "[Sunny]" to find it out.

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, November 02, 2015 7:34 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni; Eric Dong
Subject: [edk2] [Patch 09/11] MdeModulePkg: Use UefiSpec.h defined macro to 
replace L"xxx" string

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Eric Dong 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index c889892..4154d41 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -672,7 +672,7 @@ BdsFormalizeOSIndicationVariable (
   //
   OsIndicationSupport = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
   Status = gRT->SetVariable (
-  L"OsIndicationsSupported",
+  EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME,
   ,
   EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS,
   sizeof(UINT64),
@@ -694,7 +694,7 @@ BdsFormalizeOSIndicationVariable (
   Attributes = 0;
   DataSize = sizeof(UINT64);
   Status = gRT->GetVariable (
-  L"OsIndications",
+  EFI_OS_INDICATIONS_VARIABLE_NAME,
   ,
   ,
   ,
@@ -711,7 +711,7 @@ BdsFormalizeOSIndicationVariable (
 
 DEBUG ((EFI_D_ERROR, "[Bds] Unformalized OsIndications variable exists. 
Delete it\n"));
 Status = gRT->SetVariable (
-L"OsIndications",
+EFI_OS_INDICATIONS_VARIABLE_NAME,
 ,
 0,
 0,
@@ -737,9 +737,9 @@ BdsFormalizeEfiGlobalVariable (
   //
   // Validate Console variable.
   //
-  BdsFormalizeConsoleVariable (L"ConIn");
-  BdsFormalizeConsoleVariable (L"ConOut");
-  BdsFormalizeConsoleVariable (L"ErrOut");
+  BdsFormalizeConsoleVariable (EFI_CON_IN_VARIABLE_NAME);  
+ BdsFormalizeConsoleVariable (EFI_CON_OUT_VARIABLE_NAME);  
+ BdsFormalizeConsoleVariable (EFI_ERR_OUT_VARIABLE_NAME);
[Sunny] Are these two lines above having the wrong indent?  
 
   //
   // Validate OSIndication related variable.
@@ -1045,7 +1045,7 @@ BdsEntry (
   //
   DataSize = sizeof (UINT64);
   Status = gRT->GetVariable (
-  L"OsIndications",
+  EFI_OS_INDICATIONS_VARIABLE_NAME,
   ,
   NULL,
   ,
@@ -1062,7 +1062,7 @@ BdsEntry (
   if (BootFwUi) {
 OsIndication &= ~((UINT64) EFI_OS_INDICATIONS_BOOT_TO_FW_UI);
 Status = gRT->SetVariable (
-   L"OsIndications",
+   EFI_OS_INDICATIONS_VARIABLE_NAME,
,
EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | 
EFI_VARIABLE_NON_VOLATILE,
sizeof(UINT64),
--
1.9.5.msysgit.1

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

Re: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

[Wang, Sunny (HPS SW)]

Hi Ray, 
Good catch! Thanks. :)
I updated patch for this. Please help to review it again.

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Saturday, October 31, 2015 6:59 AM
To: Wang, Sunny (HPS SW)
Cc: Tian, Feng; El-Haj-Mahmoud, Samer; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues
Importance: High

Sunny,
You could move the two FreePool (FullInstance) to one place which is under the 
LocateDevicePath() call to make the change smaller a bit.
What do you think?

Thanks,
Ray

-Original Message-
From: Wang, Sunny (HPS SW) [mailto:sunnyw...@hpe.com]
Sent: Friday, October 30, 2015 6:21 PM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: Tian, Feng <feng.t...@intel.com>; El-Haj-Mahmoud, Samer 
<samer.el-haj-mahm...@hpe.com>; Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; 
edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

Hi Ray,
Are you the owner of this module (UefiBootManagerLib)? If so, could you 
help to review and commit it?  If not, could you tell me who is the owner? 
In addition, the attached patch would also need your help to get 
committed. Thanks!

Regards,
Sunny Wang

-Original Message-
From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Thursday, October 29, 2015 8:21 AM
To: El-Haj-Mahmoud, Samer; Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Cc: Tian, Feng
Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues
Importance: High

I will let module owner review and commit it.

Thanks
Feng

-Original Message-
From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
Sent: Wednesday, October 28, 2015 22:58
To: Wang, Sunny (HPS SW); edk2-devel@lists.01.org; Tian, Feng
Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

+ MdeModuklePkg maintainers.

Can someone help by committing this please?



-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
El-Haj-Mahmoud, Samer
Sent: Tuesday, October 27, 2015 10:14 AM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

Reviewed-by: Samer El-Haj-Mahmoud <el...@hpe.com>

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Tuesday, October 27, 2015 3:47 AM
To: edk2-devel@lists.01.org
Cc: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>
Subject: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

Fix memory leak issues

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Sunny Wang <sunnyw...@hpe.com>
---
 MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c
index 86b4fac..0830166 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c
@@ -2,6 +2,7 @@
   Library functions which contain all the code to connect console device.
 
 Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 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 @@ -301,6 +302,7 @@ 
BmUpdateSystemTableConsole (
   EFI_DEVICE_PATH_PROTOCOL*FullDevicePath;
   EFI_DEVICE_PATH_PROTOCOL*VarConsole;
   EFI_DEVICE_PATH_PROTOCOL*Instance;
+  EFI_DEVICE_PATH_PROTOCOL*FullInstance;
   VOID*Interface;
   EFI_HANDLE  NewHandle;
   EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL *TextOut; @@ -354,6 +356,7 @@ 
BmUpdateSystemTableConsole (
 //
 // Find console device handle by device path instance
 //
+FullInstance = Instance;
 Status = gBS->LocateDevicePath (
 ConsoleGuid,
 ,
@@ -383,15 +386,18 @@ BmUpdateSystemTableConsole (
 TextOut->SetMode (TextOut, 0);
   }
 }
+FreePool (FullDevicePath);
+FreePool (FullInstance);
 return TRUE;
   }
 }
-
+FreePool (FullInstance);
   } while (Instance != NULL);
 
   //
   // No any available console devcie found.
   //
+  FreePool (FullDevicePath);
   return FALSE;
 }
 
--
2.5.0.windows.1

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


--- Begin Message ---
Fix memory leak issues

C

Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

[Wang, Sunny (HPS SW)]

Hi Ray,
That's great. If we can directly use your current platform recovery 
implementation, that can save our effort to replace the short-term fix with the 
permanent fix in the future. 
Thanks for your help! :) 
In addition, could you help to take a look into the attached patch 
which adds the other BDS hook platform function 
PlatformBootManagerBeforeDefaultBoot() as well? For 
PlatformBootManagerBeforeDefaultBoot(), do you also prefer to add a 
REPORT_STATUS_CODE before doing DefaultBootBehavior() while-loop or some other 
way? Or adding PlatformBootManagerBeforeDefaultBoot() is acceptable for this 
case?

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Saturday, October 31, 2015 6:56 AM
To: Wang, Sunny (HPS SW)
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib
Importance: High

Sunny,
I don't like to add a library interface as a short term fix. I will provide a 
patch next week to enable platform recovery (We are still working on OS 
recovery) so that you don't need PlatformBootManagerDefaultBootFail() interface.

Thanks,
Ray

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Friday, October 30, 2015 6:14 PM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Hi Ray,
Yes, I think OS/Platform recovery feature can meet our requirement.
However, OS/Platform recovery feature has not yet been completely implemented, 
so we would like to have a short-term solution for this.
Just like Samer said, if you are OK with introducing a new StatusCode in either 
BootAllBootOptions() or DefaultBootBehavior(), I can update patch for this. 
Of cause, If you think my current patch (adding hook function) could be used 
for short-term, it would be great.
By the way, I have the other similar need as the attached email.  If you have 
time, please help to review it. Thanks! 

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Friday, October 30, 2015 2:41 PM
To: Wang, Sunny (HPS SW)
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib
Importance: High

Sunny,
Can the OS/Platform recovery feature in UEFI spec can meet your requirement?

Thanks,
Ray

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Friday, October 30, 2015 11:46 AM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Hi Ray,
Thanks for the information.
However, the Status codes you mentioned in EfiBootManagerBoot() is for 
indicating booting a specific boot option failure rather than booting ALL boot 
options failure, so I can't use these Status codes for my purpose. The 
following two are detailed reasons:
-  I can't figure out how to know that all the boot options are booted in the 
Status code handler which is triggered by the REPORT_STATUS_CODE in 
EfiBootManagerBoot()
-  If there is no boot option in the boot order, this status code will not work 
either because BDS code doesn't call EfiBootManagerBoot().

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Thursday, October 29, 2015 1:07 PM
To: El-Haj-Mahmoud, Samer
Cc: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Samer,
EfiBootManagerBoot() function reports the failure to load boot option and the 
failure to start boot option through status code.

//
// Report Status Code to indicate that the failure to load boot option // 
REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR)
  );
...

//
// Report Status Code to indicate that boot failure // REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
  );

Platform can hook the above two status code so that the 
PlatformBootManagerDefaultBootFail() can be avoided.

We are working on the Platform Recovery feature right now. But not sure whether 
it can be done by the end of this year.
Some spec clarification/discussion are on-going.

Thanks,
Ray

-Original Message-
From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
Sent: Wednesday, October 28, 2015 10:49 PM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: W

Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

[Wang, Sunny (HPS SW)]

Hi Ray,
Yes, I think OS/Platform recovery feature can meet our requirement.
However, OS/Platform recovery feature has not yet been completely implemented, 
so we would like to have a short-term solution for this.
Just like Samer said, if you are OK with introducing a new StatusCode in either 
BootAllBootOptions() or DefaultBootBehavior(), I can update patch for this. 
Of cause, If you think my current patch (adding hook function) could be used 
for short-term, it would be great.
By the way, I have the other similar need as the attached email.  If you have 
time, please help to review it. Thanks! 

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Friday, October 30, 2015 2:41 PM
To: Wang, Sunny (HPS SW)
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib
Importance: High

Sunny,
Can the OS/Platform recovery feature in UEFI spec can meet your requirement?

Thanks,
Ray

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Friday, October 30, 2015 11:46 AM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Hi Ray,
Thanks for the information.
However, the Status codes you mentioned in EfiBootManagerBoot() is for 
indicating booting a specific boot option failure rather than booting ALL boot 
options failure, so I can't use these Status codes for my purpose. The 
following two are detailed reasons:
-  I can't figure out how to know that all the boot options are booted in the 
Status code handler which is triggered by the REPORT_STATUS_CODE in 
EfiBootManagerBoot()
-  If there is no boot option in the boot order, this status code will not work 
either because BDS code doesn't call EfiBootManagerBoot().

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Thursday, October 29, 2015 1:07 PM
To: El-Haj-Mahmoud, Samer
Cc: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Samer,
EfiBootManagerBoot() function reports the failure to load boot option and the 
failure to start boot option through status code.

//
// Report Status Code to indicate that the failure to load boot option // 
REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR)
  );
...

//
// Report Status Code to indicate that boot failure // REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
  );

Platform can hook the above two status code so that the 
PlatformBootManagerDefaultBootFail() can be avoided.

We are working on the Platform Recovery feature right now. But not sure whether 
it can be done by the end of this year.
Some spec clarification/discussion are on-going.

Thanks,
Ray

-Original Message-
From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
Sent: Wednesday, October 28, 2015 10:49 PM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org; 
El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Ray,

The use case is in taking some platform specific action when all boot options 
have failed. I guess this is more in line with UEFI 2.5 Platform Recovery 
(section 3.4.3). Since EDK2 has not implemented that yet, and there is no 
processing of PlatformRecovery variables, a hook function seems to solve 
the issue for now.

I guess a Status Code could solve this as well, if you are OK with introducting 
a new StatusCode in BootAllBootOptions() or DefaultBootBehavior(). If so, Sunny 
can update the patch with this proposal.

On a separate note, what is the plan for implementing UEFI 2.5 Platform 
Recovery in EDK2?

Thanks,
--Samer






-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Wednesday, October 28, 2015 8:18 AM
To: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>
Cc: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Do you have real requirement about the boot failure notify?
Have you considered to use report status handler?

Thanks,
Ray

> 在 2015年10月27日，23:18，El-Haj-Mahmoud, Samer 
> <samer.el-haj-mahm...@hpe.com> 写道：
> 
> Reviewed-by: Samer El-Haj-Mahmoud <el...@hpe.com>
> 
> -

Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

[Wang, Sunny (HPS SW)]

Hi Ray,
Thanks for the information.
However, the Status codes you mentioned in EfiBootManagerBoot() is for 
indicating booting a specific boot option failure rather than booting ALL boot 
options failure, so I can't use these Status codes for my purpose. The 
following two are detailed reasons:
-  I can't figure out how to know that all the boot options are booted in the 
Status code handler which is triggered by the REPORT_STATUS_CODE in 
EfiBootManagerBoot()
-  If there is no boot option in the boot order, this status code will not work 
either because BDS code doesn't call EfiBootManagerBoot().

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Thursday, October 29, 2015 1:07 PM
To: El-Haj-Mahmoud, Samer
Cc: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Samer,
EfiBootManagerBoot() function reports the failure to load boot option and the 
failure to start boot option through status code.

//
// Report Status Code to indicate that the failure to load boot option // 
REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR)
  );
...

//
// Report Status Code to indicate that boot failure // REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
  );

Platform can hook the above two status code so that the 
PlatformBootManagerDefaultBootFail() can be avoided.

We are working on the Platform Recovery feature right now. But not sure whether 
it can be done by the end of this year.
Some spec clarification/discussion are on-going.

Thanks,
Ray

-Original Message-
From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
Sent: Wednesday, October 28, 2015 10:49 PM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org; 
El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Ray,

The use case is in taking some platform specific action when all boot options 
have failed. I guess this is more in line with UEFI 2.5 Platform Recovery 
(section 3.4.3). Since EDK2 has not implemented that yet, and there is no 
processing of PlatformRecovery variables, a hook function seems to solve 
the issue for now.

I guess a Status Code could solve this as well, if you are OK with introducting 
a new StatusCode in BootAllBootOptions() or DefaultBootBehavior(). If so, Sunny 
can update the patch with this proposal.

On a separate note, what is the plan for implementing UEFI 2.5 Platform 
Recovery in EDK2?

Thanks,
--Samer






-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Wednesday, October 28, 2015 8:18 AM
To: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>
Cc: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Do you have real requirement about the boot failure notify?
Have you considered to use report status handler?

Thanks,
Ray

> 在 2015年10月27日，23:18，El-Haj-Mahmoud, Samer 
> <samer.el-haj-mahm...@hpe.com> 写道：
> 
> Reviewed-by: Samer El-Haj-Mahmoud <el...@hpe.com>
> 
> -Original Message-
> From: Wang, Sunny (HPS SW)
> Sent: Tuesday, October 27, 2015 4:21 AM
> To: edk2-devel@lists.01.org
> Cc: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>; Wang, Sunny 
> (HPS SW) <sunnyw...@hpe.com>
> Subject: [PATCH] MdeModulePkg: Add a BDS platform hook function 
> PlatformBootManagerDefaultBootFail to PlatformBootManagerLib
> 
> Add a BDS platform hook function PlatformBootManagerDefaultBootFail to 
> PlatformBootManagerLib
> 
> Why we need this hook function :
> 1) According to UEFI 2.5 Section 3.4.3, the 3rd paragraph, It seems we need 
> to have a hook function (PlatformBootManagerDefaultBootFail) for platform 
> firmware to do platform action like recovering or setting to a known set of 
> boot options.
> 2) This hook function is similar to IntelFrameworkModulePkg's 
> PlatformBdsLib's PlatformBdsBootFail function which was used in several 
> platforms in EDK2, so it would be helpful for other platforms to change their 
> BDS platform library from IntelFrameworkModulePkg's PlatformBdsLib to 
> MdeModulePkg's PlatformBootManagerLib in the future.
> 
> Where we call/run this hook function:
> 1) According to UEFI 2.5 Section 3.1.1, the 5th paragraph, we need to present 
> a boot manager menu to the user for the successful boot attempt, so we only 
>

Re: [edk2] [PATCH v2] MdeModulePkg: Make the BmFindLoadOption function public

[Wang, Sunny (HPS SW)]

Hi Ray, 
Could you help to review the updated patch? 

Regards,
Sunny Wang

-Original Message-
From: Wang, Sunny (HPS SW) 
Sent: Tuesday, October 13, 2015 6:10 PM
To: edk2-devel@lists.01.org
Cc: El-Haj-Mahmoud, Samer; Wang, Sunny (HPS SW)
Subject: [PATCH v2] MdeModulePkg: Make the BmFindLoadOption function public

Make the BmFindLoadOption function public

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Sunny Wang <sunnyw...@hpe.com>
Reviewed-by: Samer El-Haj-Mahmoud <el...@hpe.com>
---
 MdeModulePkg/Include/Library/UefiBootManagerLib.h  | 22 ++
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 10 +-
 .../Library/UefiBootManagerLib/BmLoadOption.c  |  4 +++-
 .../Library/UefiBootManagerLib/InternalBm.h| 22 +-
 4 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h 
b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index 5538d90..54a6713 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -2,6 +2,7 @@
   Provide Boot Manager related library APIs.
 
 Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 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 @@ -222,6 +223,27 
@@ EfiBootManagerSortLoadOptionVariable (
   IN SORT_COMPARE  CompareFunction
   );
 
+/**
+  Return the index of the load option in the load option array.
+
+  The function consider two load options are equal when the  
+ OptionType, Attributes, Description, FilePath and OptionalData are equal.
+
+  @param KeyPointer to the load option to be found.
+  @param Array  Pointer to the array of load options to be found.
+  @param Count  Number of entries in the Array.
+
+  @retval -1  Key wasn't found in the Array.
+  @retval 0 ~ Count-1 The index of the Key in the Array.
+**/
+INTN
+EFIAPI
+EfiBootManagerFindLoadOption (
+  IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Key,
+  IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Array,
+  IN UINTN  Count
+  );
+
 //
 // Boot Manager hot key library functions.
 //
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 8f14cf6..aef2e7b 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1,8 +1,8 @@
 /** @file
   Library functions which relates with booting.
 
-(C) Copyright 2015 Hewlett-Packard Development Company, L.P.  Copyright 
(c) 2011 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 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 @@ -206,7 +206,7 @@ 
BmFindBootOptionInVariable (
   if (OptionNumber == LoadOptionNumberUnassigned) {
 BootOptions = EfiBootManagerGetLoadOptions (, 
LoadOptionTypeBoot);
 
-Index = BmFindLoadOption (OptionToFind, BootOptions, BootOptionCount);
+Index = EfiBootManagerFindLoadOption (OptionToFind, BootOptions, 
+ BootOptionCount);
 if (Index != -1) {
   OptionNumber = BootOptions[Index].OptionNumber;
 }
@@ -2165,7 +2165,7 @@ EfiBootManagerRefreshAllBootOption (
   // Only check those added by BDS
   // so that the boot options added by end-user or OS installer won't be 
deleted
   //
-  if (BmFindLoadOption ([Index], BootOptions, 
BootOptionCount) == (UINTN) -1) {
+  if (EfiBootManagerFindLoadOption ([Index], 
+ BootOptions, BootOptionCount) == (UINTN) -1) {
 Status = EfiBootManagerDeleteLoadOptionVariable 
(NvBootOptions[Index].OptionNumber, LoadOptionTypeBoot);
 //
 // Deleting variable with current variable implementation shouldn't 
fail.
@@ -2179,7 +2179,7 @@ EfiBootManagerRefreshAllBootOption (
   // Add new EFI boot options to NV
   //
   for (Index = 0; Index < BootOptionCount; Index++) {
-if (BmFindLoadOption ([Index], NvBootOptions, 
NvBootOptionCount) == (UINTN) -1) {
+if (EfiBootManagerFindLoadOption ([Index], 
+ NvBootOptions, NvBootOptionCount) == (UINTN) -1) {
   EfiBootManagerAddLoadOptionVariable ([Index], (UINTN) -1);
   //
   // Try best to add the boot options so continue upon failure.
@@ -2260,7 +2260,7 @@ BmRegisterBootManagerMenu (
 UINTN   BootOptionCount;
 
 BootOptions = EfiBootManagerGetLoadOptions (, 
LoadOptionTypeBoot);
-ASSERT (BmFindLoadOption (BootOption, BootOptions, BootOptionCount) == -1);
+ASSERT (EfiBootManagerFindLoadOpt

Re: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function public

[Wang, Sunny (HPS SW)]

OK, got it. 

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Tuesday, October 13, 2015 1:06 PM
To: Wang, Sunny (HPS SW); Laszlo Ersek
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function 
public
Importance: High

Sunny,
Your bitmask idea works. But I think it might introduce unnecessary complexity. 
Keeping current implementation is good enough I think.

Thanks,
Ray

-Original Message-
From: Wang, Sunny (HPS SW) [mailto:sunnyw...@hpe.com]
Sent: Tuesday, October 13, 2015 12:15 PM
To: Ni, Ruiyu <ruiyu...@intel.com>; Laszlo Ersek <ler...@redhat.com>
Cc: edk2-devel@lists.01.org <edk2-de...@ml01.01.org>; Wang, Sunny (HPS SW) 
<sunnyw...@hpe.com>
Subject: RE: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function 
public

Laszlo,
Would you like to make GetLoadOptionBuffer() public as well with my 
next patch?  I can help to do the passing change and add you into Signed-off-by 
as well. If you need it, let me know by end of today.  
Ruiyu,
OK, got it. I will update the patch and send it out. 
I just get an idea from your reply for enhancing the 
EfiBootManagerFindLoadOption, we may add a input parameter called IgnoreMember 
like the following for all the use cases so that all the consumers of this API 
don't need to create their own version. What do you think? If it works, I will 
send the other patch for this. 

typedef enum {
  IgnoreOptionType   = 0x01,// BIT0
  IgnoreAttributes  = 0x02,// BIT1
  IgnoreDescription   = 0x04, // BIT2
  IgnoreFilePath  = 0x08, // BIT3
  IgnoreOptionalData= 0x10, // BIT4
  IgnoreOptionalDataSize = 0x20 // BIT5
} IGNORE_MEMBER_OF_ LOAD_OPTION;

EfiBootManagerFindLoadOption (
  IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Key,
  IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Array,
  IN UINTN  
  Count,
  IN IGNORE_MEMBER_OF_ LOAD_OPTION IgnoreMember
  )

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Tuesday, October 13, 2015 10:35 AM
To: Laszlo Ersek; Wang, Sunny (HPS SW)
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function 
public
Importance: High

Sunny,
Yes it's acceptable. Currently the implementation of *FindLoadOption does an 
exactly match. If other consumers of this API need to just match partially, 
they need to provide their own version instead of using this exposed one.

Laszlo,
Sorry I missed your comments. I am thinking about whether exposing the 
*GetLoadOptionBuffer() breaks the orthogonality because we already have 
EfiBootManagerProcessLoadOption() API. The latter one not only loads the buffer 
but also treats the buffer as a PECOFF buffer to start it.
The *GetLoadOptionBuffer() seems to do part of the job.
Anyway I think your request also makes sense. Just want to know if there is 
other better idea to not break the orthogonality.

Thanks,
Ray

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Monday, October 12, 2015 8:11 PM
To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; Ni, Ruiyu <ruiyu...@intel.com>
Cc: edk2-devel@lists.01.org <edk2-de...@ml01.01.org>
Subject: Re: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function 
public

On 10/12/15 09:45, Wang, Sunny (HPS SW) wrote:
> Hi Ruiyu,
> 
> Thanks for the quick response.
> It is because of that we need to do same thing as what 
> BmFindLoadOption function did in our PlatformBootManagerLib to 
> check/find specific boot option from the boot options array which is 
> got from EfiBootManagerGetLoadOptions function. I also think other 
> developers may have the same need.  Therefore, it would be better to 
> make BmFindLoadOption public to reduce the maintenance effort caused 
> by duplicating the function into other BDS libraries like 
> PlatformBootManagerLib. Is it reasonable and acceptable?  If so, I 
> will rename the BmFindLoadOption function to 
> EfiBootManagerFindLoadOption and resend the patch.

On a similar note: in the message

http://thread.gmane.org/gmane.comp.bios.edk2.devel/759/focus=1153

point (10), I requested that the function BmGetLoadOptionBuffer() be made 
public.

Looks like there are now at least two Bm*() functions that see outside demand! 
:)

Thanks
Laszlo

> 
> Regards,
> Sunny Wang
> 
> -Original Message-----
> From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
> Sent: Monday, October 12, 2015 3:29 PM
> To: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption 
> function public
> Importance: High
> 
> Sunny,
> Why do you want to expose this function?
>

Re: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function public

[Wang, Sunny (HPS SW)]

Hi Ruiyu, 

Thanks for the quick response.  
It is because of that we need to do same thing as what BmFindLoadOption 
function did in our PlatformBootManagerLib to check/find specific boot option 
from the boot options array which is got from EfiBootManagerGetLoadOptions 
function. I also think other developers may have the same need.  Therefore, it 
would be better to make BmFindLoadOption public to reduce the maintenance 
effort caused by duplicating the function into other BDS libraries like 
PlatformBootManagerLib. Is it reasonable and acceptable?  If so, I will rename 
the BmFindLoadOption function to EfiBootManagerFindLoadOption and resend the 
patch.   

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Monday, October 12, 2015 3:29 PM
To: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function 
public
Importance: High

Sunny,
Why do you want to expose this function?
Bm* needs to change to EfiBootManager* if it's public.

Thanks,
Ray

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Sunny 
Wang
Sent: Monday, October 12, 2015 3:15 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function public

Make the BmFindLoadOption function public

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Sunny Wang <sunnyw...@hpe.com>
---
 MdeModulePkg/Include/Library/UefiBootManagerLib.h | 21 +
 1 file changed, 21 insertions(+)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h 
b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index 5538d90..e86b589 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -2,6 +2,7 @@
   Provide Boot Manager related library APIs.
 
 Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 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 @@ -222,6 +223,26 
@@ EfiBootManagerSortLoadOptionVariable (
   IN SORT_COMPARE  CompareFunction
   );
 
+/**
+  Return the index of the load option in the load option array.
+
+  The function consider two load options are equal when the  
+ OptionType, Attributes, Description, FilePath and OptionalData are equal.
+
+  @param KeyPointer to the load option to be found.
+  @param Array  Pointer to the array of load options to be found.
+  @param Count  Number of entries in the Array.
+
+  @retval -1  Key wasn't found in the Array.
+  @retval 0 ~ Count-1 The index of the Key in the Array.
+**/
+INTN
+BmFindLoadOption (
+  IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Key,
+  IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Array,
+  IN UINTN  Count
+  );
+
 //
 // Boot Manager hot key library functions.
 //
--
2.5.0.windows.1

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