Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.

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

Looks good to me. Not sure if there was a reason to call BmDestroyRamDisk before ReportStatusCode. 
By the way, it is an interesting case that there is a custom boot option or application that needs the memory that was occupied by the RAM disk. It looks to me like the custom boot option or application would like to create the other RAM disk with allocating large memory for recovering the failed boot option. 

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

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of KrishnadasX Veliyathuparambil Prakashan
Sent: Friday, June 19, 2020 10:40 AM
To: devel@edk2.groups.io
Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.

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

For better memory management, re-ordered the DestroyRamDisk and ReportStatusCode calls inside the EfiBootManagerBoot() function.

This will help to clean the unused memory before reporting the failure status, so that OEMs can use RSC Listener to launch custom boot option or application for recovering the failed hard drive.

This change will help to ensure that the allocated pool of memory for the failed boot option is freed before executing OEM's RSC listener callback to handle every boot option failure.

Signed-off-by: KrishnadasX Veliyathuparambil Prakashan <krishnadasx.veliyathuparambil.prakashan@intel.com>
Cc: "Gao, Zhichao" <zhichao.gao@intel.com>
Cc: "Ni, Ray" <ray.ni@intel.com>
---
 .../Library/UefiBootManagerLib/BmBoot.c       | 28 ++++++++++---------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 540d169ec1..aff620ad52 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) 2019, NVIDIA CORPORATION. All rights reserved.
-Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2020, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -1903,17 +1903,17 @@ EfiBootManagerBoot (
         gBS->UnloadImage (ImageHandle);
       }
       //
-      // Report Status Code with the failure status to indicate that the failure to load boot option
-      //
-      BmReportLoadFailure (EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR, Status);
-      BootOption->Status = Status;
-      //
       // Destroy the RAM disk
       //
       if (RamDiskDevicePath != NULL) {
         BmDestroyRamDisk (RamDiskDevicePath);
         FreePool (RamDiskDevicePath);
       }
+      //
+      // Report Status Code with the failure status to indicate that the failure to load boot option
+      //
+      BmReportLoadFailure (EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR, Status);
+      BootOption->Status = Status;
       return;
     }
   }
@@ -1982,13 +1982,6 @@ EfiBootManagerBoot (
   Status = gBS->StartImage (ImageHandle, &BootOption->ExitDataSize, &BootOption->ExitData);
   DEBUG ((DEBUG_INFO | DEBUG_LOAD, "Image Return Status = %r\n", Status));
   BootOption->Status = Status;
-  if (EFI_ERROR (Status)) {
-    //
-    // Report Status Code with the failure status to indicate that boot failure
-    //
-    BmReportLoadFailure (EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED, Status);
-  }
-  PERF_END_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) OptionNumber);
 
   //
   // Destroy the RAM disk
@@ -1998,6 +1991,15 @@ EfiBootManagerBoot (
     FreePool (RamDiskDevicePath);
   }
 
+  if (EFI_ERROR (Status)) {
+    //
+    // Report Status Code with the failure status to indicate that boot failure
+    //
+    BmReportLoadFailure (EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED, Status);
+ }  PERF_END_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) 
+ OptionNumber);
+
+
   //
   // Clear the Watchdog Timer after the image returns
   //
--
2.27.0.windows.1