public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
@ 2020-06-19  2:40 KrishnadasX Veliyathuparambil Prakashan
  2020-06-22  8:32 ` [edk2-devel] " Wang, Sunny (HPS SW)
  2020-07-03  6:02 ` Liming Gao
  0 siblings, 2 replies; 10+ messages in thread
From: KrishnadasX Veliyathuparambil Prakashan @ 2020-06-19  2:40 UTC (permalink / raw)
  To: devel; +Cc: Gao, Zhichao, Ni, Ray

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
  2020-06-19  2:40 [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC KrishnadasX Veliyathuparambil Prakashan
@ 2020-06-22  8:32 ` Wang, Sunny (HPS SW)
  2020-07-03  6:02 ` Liming Gao
  1 sibling, 0 replies; 10+ messages in thread
From: Wang, Sunny (HPS SW) @ 2020-06-22  8:32 UTC (permalink / raw)
  To: devel@edk2.groups.io,
	krishnadasx.veliyathuparambil.prakashan@intel.com
  Cc: Gao, Zhichao, Ni, Ray, Wang, Sunny (HPS SW)

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





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
  2020-06-19  2:40 [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC KrishnadasX Veliyathuparambil Prakashan
  2020-06-22  8:32 ` [edk2-devel] " Wang, Sunny (HPS SW)
@ 2020-07-03  6:02 ` Liming Gao
  2020-07-03  9:30   ` Veliyathuparambil Prakashan, KrishnadasX
  2020-07-03 10:43   ` Leif Lindholm
  1 sibling, 2 replies; 10+ messages in thread
From: Liming Gao @ 2020-07-03  6:02 UTC (permalink / raw)
  To: devel@edk2.groups.io, Veliyathuparambil Prakashan, KrishnadasX,
	leif@nuviainc.com, Laszlo Ersek, afish@apple.com,
	Kinney, Michael D
  Cc: Gao, Zhichao, Ni, Ray

Signed-off-by line is too long and exceeds 80 characters requirement. But, it is valid. 

So, I suggest to enhance PatchCheck.py and skip the check for the lines with Signed-off-by, Ack-by:, Reviewed-by:, and Tested-By:. 

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io <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
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#61517): https://edk2.groups.io/g/devel/message/61517
> Mute This Topic: https://groups.io/mt/74978785/1759384
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [liming.gao@intel.com]
> -=-=-=-=-=-=


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
  2020-07-03  6:02 ` Liming Gao
@ 2020-07-03  9:30   ` Veliyathuparambil Prakashan, KrishnadasX
  2020-07-17  4:09     ` Veliyathuparambil Prakashan, KrishnadasX
  2020-07-03 10:43   ` Leif Lindholm
  1 sibling, 1 reply; 10+ messages in thread
From: Veliyathuparambil Prakashan, KrishnadasX @ 2020-07-03  9:30 UTC (permalink / raw)
  To: Gao, Liming, Gao, Zhichao
  Cc: Ni, Ray, T V, Krishnamoorthy, devel@edk2.groups.io,
	leif@nuviainc.com, Laszlo Ersek, afish@apple.com,
	Kinney, Michael D

Thank you very much Liming  and Zhichao for your time to discuss this case.

Hello Liming, 

As discussed, please help to raise the BZ to enhance PatchCheck.py and kindly help to submit our Edk2 patch to Edk2 Repo.
[EDK2 Change BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2818]

Also, please give us an update on next week regarding the ETA , as per our discussion.

Thanks,
Krishnadas

> -----Original Message-----
> From: Gao, Liming <liming.gao@intel.com>
> Sent: Friday, July 3, 2020 11:32 AM
> To: devel@edk2.groups.io; Veliyathuparambil Prakashan, KrishnadasX
> <krishnadasx.veliyathuparambil.prakashan@intel.com>; leif@nuviainc.com;
> Laszlo Ersek <lersek@redhat.com>; afish@apple.com; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure,
> Destroy RamDisk memory before RSC.
> 
> Signed-off-by line is too long and exceeds 80 characters requirement. But, it is
> valid.
> 
> So, I suggest to enhance PatchCheck.py and skip the check for the lines with
> Signed-off-by, Ack-by:, Reviewed-by:, and Tested-By:.
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: devel@edk2.groups.io <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
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> >
> > View/Reply Online (#61517):
> > https://edk2.groups.io/g/devel/message/61517
> > Mute This Topic: https://groups.io/mt/74978785/1759384
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [liming.gao@intel.com] -=-=-=-=-=-=


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
  2020-07-03  6:02 ` Liming Gao
  2020-07-03  9:30   ` Veliyathuparambil Prakashan, KrishnadasX
@ 2020-07-03 10:43   ` Leif Lindholm
  2020-07-03 15:02     ` Liming Gao
  1 sibling, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2020-07-03 10:43 UTC (permalink / raw)
  To: Gao, Liming
  Cc: devel@edk2.groups.io, Veliyathuparambil Prakashan, KrishnadasX,
	Laszlo Ersek, afish@apple.com, Kinney, Michael D, Gao, Zhichao,
	Ni, Ray

On Fri, Jul 03, 2020 at 06:02:12 +0000, Gao, Liming wrote:
> Signed-off-by line is too long and exceeds 80 characters requirement. But, it is valid. 
> 
> So, I suggest to enhance PatchCheck.py and skip the check for the lines with Signed-off-by, Ack-by:, Reviewed-by:, and Tested-By:. 

Acked-by, not Ack-by, but yes, I completely agree. Restricting the
lenght of these tag lines is not correct.
We may want to consider adding "Cc:" to the list also.

Regards,

Leif

> Thanks
> Liming
> > -----Original Message-----
> > From: devel@edk2.groups.io <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
> > 
> > 
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > 
> > View/Reply Online (#61517): https://edk2.groups.io/g/devel/message/61517
> > Mute This Topic: https://groups.io/mt/74978785/1759384
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub  [liming.gao@intel.com]
> > -=-=-=-=-=-=
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
  2020-07-03 10:43   ` Leif Lindholm
@ 2020-07-03 15:02     ` Liming Gao
  0 siblings, 0 replies; 10+ messages in thread
From: Liming Gao @ 2020-07-03 15:02 UTC (permalink / raw)
  To: devel@edk2.groups.io, leif@nuviainc.com
  Cc: Veliyathuparambil Prakashan, KrishnadasX, Laszlo Ersek,
	afish@apple.com, Kinney, Michael D, Gao, Zhichao, Ni, Ray

BZ to enhance PatchCheck is submitted. https://bugzilla.tianocore.org/show_bug.cgi?id=2836

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm
> Sent: Friday, July 3, 2020 6:43 PM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: devel@edk2.groups.io; Veliyathuparambil Prakashan, KrishnadasX <krishnadasx.veliyathuparambil.prakashan@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; afish@apple.com; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
> 
> On Fri, Jul 03, 2020 at 06:02:12 +0000, Gao, Liming wrote:
> > Signed-off-by line is too long and exceeds 80 characters requirement. But, it is valid.
> >
> > So, I suggest to enhance PatchCheck.py and skip the check for the lines with Signed-off-by, Ack-by:, Reviewed-by:, and Tested-By:.
> 
> Acked-by, not Ack-by, but yes, I completely agree. Restricting the
> lenght of these tag lines is not correct.
> We may want to consider adding "Cc:" to the list also.
> 
> Regards,
> 
> Leif
> 
> > Thanks
> > Liming
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <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
> > >
> > >
> > > -=-=-=-=-=-=
> > > Groups.io Links: You receive all messages sent to this group.
> > >
> > > View/Reply Online (#61517): https://edk2.groups.io/g/devel/message/61517
> > > Mute This Topic: https://groups.io/mt/74978785/1759384
> > > Group Owner: devel+owner@edk2.groups.io
> > > Unsubscribe: https://edk2.groups.io/g/devel/unsub  [liming.gao@intel.com]
> > > -=-=-=-=-=-=
> >
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
  2020-07-03  9:30   ` Veliyathuparambil Prakashan, KrishnadasX
@ 2020-07-17  4:09     ` Veliyathuparambil Prakashan, KrishnadasX
  2020-07-17  4:54       ` Gao, Zhichao
                         ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Veliyathuparambil Prakashan, KrishnadasX @ 2020-07-17  4:09 UTC (permalink / raw)
  To: Gao, Liming, Gao, Zhichao
  Cc: Ni, Ray, T V, Krishnamoorthy, devel@edk2.groups.io,
	Kinney, Michael D

Hello Liming,

Gentle Reminder.
As discussed before, please let us know when we can expect our changes (below BZ) to get pushed in to Edk2Repo.
Please help to give an ETA.

BZ Details:
[edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC
https://bugzilla.tianocore.org/show_bug.cgi?id=2818

Dependency : https://bugzilla.tianocore.org/show_bug.cgi?id=2836

Thanks,
Krishnadas

>-----Original Message-----
> From: Veliyathuparambil Prakashan, KrishnadasX
> Sent: Friday, July 3, 2020 3:00 PM
> To: Gao, Liming <liming.gao@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; T V, Krishnamoorthy
> <Krishnamoorthy.T.V@intel.com>; devel@edk2.groups.io; leif@nuviainc.com;
> Laszlo Ersek <lersek@redhat.com>; afish@apple.com; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure,
> Destroy RamDisk memory before RSC.
> 
> Thank you very much Liming  and Zhichao for your time to discuss this case.
> 
> Hello Liming,
> 
> As discussed, please help to raise the BZ to enhance PatchCheck.py and kindly
> help to submit our Edk2 patch to Edk2 Repo.
> [EDK2 Change BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2818]
> 
> Also, please give us an update on next week regarding the ETA , as per our
> discussion.
> 
> Thanks,
> Krishnadas
> 
> > -----Original Message-----
> > From: Gao, Liming <liming.gao@intel.com>
> > Sent: Friday, July 3, 2020 11:32 AM
> > To: devel@edk2.groups.io; Veliyathuparambil Prakashan, KrishnadasX
> > <krishnadasx.veliyathuparambil.prakashan@intel.com>;
> > leif@nuviainc.com; Laszlo Ersek <lersek@redhat.com>; afish@apple.com;
> > Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption
> > failure, Destroy RamDisk memory before RSC.
> >
> > Signed-off-by line is too long and exceeds 80 characters requirement.
> > But, it is valid.
> >
> > So, I suggest to enhance PatchCheck.py and skip the check for the
> > lines with Signed-off-by, Ack-by:, Reviewed-by:, and Tested-By:.
> >
> > Thanks
> > Liming
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <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
> > >
> > >
> > > -=-=-=-=-=-=
> > > Groups.io Links: You receive all messages sent to this group.
> > >
> > > View/Reply Online (#61517):
> > > https://edk2.groups.io/g/devel/message/61517
> > > Mute This Topic: https://groups.io/mt/74978785/1759384
> > > Group Owner: devel+owner@edk2.groups.io
> > > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > > [liming.gao@intel.com] -=-=-=-=-=-=


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
  2020-07-17  4:09     ` Veliyathuparambil Prakashan, KrishnadasX
@ 2020-07-17  4:54       ` Gao, Zhichao
  2020-07-17 15:06       ` Liming Gao
       [not found]       ` <162292995B2D9EDC.3435@groups.io>
  2 siblings, 0 replies; 10+ messages in thread
From: Gao, Zhichao @ 2020-07-17  4:54 UTC (permalink / raw)
  To: Veliyathuparambil Prakashan, KrishnadasX, Gao, Liming
  Cc: Ni, Ray, T V, Krishnamoorthy, devel@edk2.groups.io,
	Kinney, Michael D

With the patchcheck issue fixed. Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Veliyathuparambil Prakashan, KrishnadasX
> <krishnadasx.veliyathuparambil.prakashan@intel.com>
> Sent: Friday, July 17, 2020 12:09 PM
> To: Gao, Liming <liming.gao@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; T V, Krishnamoorthy
> <krishnamoorthy.t.v@intel.com>; devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure,
> Destroy RamDisk memory before RSC.
> 
> Hello Liming,
> 
> Gentle Reminder.
> As discussed before, please let us know when we can expect our changes (below
> BZ) to get pushed in to Edk2Repo.
> Please help to give an ETA.
> 
> BZ Details:
> [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk
> memory before RSC
> https://bugzilla.tianocore.org/show_bug.cgi?id=2818
> 
> Dependency : https://bugzilla.tianocore.org/show_bug.cgi?id=2836
> 
> Thanks,
> Krishnadas
> 
> >-----Original Message-----
> > From: Veliyathuparambil Prakashan, KrishnadasX
> > Sent: Friday, July 3, 2020 3:00 PM
> > To: Gao, Liming <liming.gao@intel.com>; Gao, Zhichao
> ><zhichao.gao@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; T V, Krishnamoorthy
> ><Krishnamoorthy.T.V@intel.com>; devel@edk2.groups.io;
> >leif@nuviainc.com;  Laszlo Ersek <lersek@redhat.com>; afish@apple.com;
> >Kinney, Michael D  <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption
> >failure,  Destroy RamDisk memory before RSC.
> >
> > Thank you very much Liming  and Zhichao for your time to discuss this case.
> >
> > Hello Liming,
> >
> > As discussed, please help to raise the BZ to enhance PatchCheck.py and
> > kindly help to submit our Edk2 patch to Edk2 Repo.
> > [EDK2 Change BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2818]
> >
> > Also, please give us an update on next week regarding the ETA , as per
> > our discussion.
> >
> > Thanks,
> > Krishnadas
> >
> > > -----Original Message-----
> > > From: Gao, Liming <liming.gao@intel.com>
> > > Sent: Friday, July 3, 2020 11:32 AM
> > > To: devel@edk2.groups.io; Veliyathuparambil Prakashan, KrishnadasX
> > > <krishnadasx.veliyathuparambil.prakashan@intel.com>;
> > > leif@nuviainc.com; Laszlo Ersek <lersek@redhat.com>;
> > > afish@apple.com; Kinney, Michael D <michael.d.kinney@intel.com>
> > > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption
> > > failure, Destroy RamDisk memory before RSC.
> > >
> > > Signed-off-by line is too long and exceeds 80 characters requirement.
> > > But, it is valid.
> > >
> > > So, I suggest to enhance PatchCheck.py and skip the check for the
> > > lines with Signed-off-by, Ack-by:, Reviewed-by:, and Tested-By:.
> > >
> > > Thanks
> > > Liming
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <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
> > > >
> > > >
> > > > -=-=-=-=-=-=
> > > > Groups.io Links: You receive all messages sent to this group.
> > > >
> > > > View/Reply Online (#61517):
> > > > https://edk2.groups.io/g/devel/message/61517
> > > > Mute This Topic: https://groups.io/mt/74978785/1759384
> > > > Group Owner: devel+owner@edk2.groups.io
> > > > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > > > [liming.gao@intel.com] -=-=-=-=-=-=


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
  2020-07-17  4:09     ` Veliyathuparambil Prakashan, KrishnadasX
  2020-07-17  4:54       ` Gao, Zhichao
@ 2020-07-17 15:06       ` Liming Gao
       [not found]       ` <162292995B2D9EDC.3435@groups.io>
  2 siblings, 0 replies; 10+ messages in thread
From: Liming Gao @ 2020-07-17 15:06 UTC (permalink / raw)
  To: Veliyathuparambil Prakashan, KrishnadasX, Gao, Zhichao
  Cc: Ni, Ray, T V, Krishnamoorthy, devel@edk2.groups.io,
	Kinney, Michael D

Krishnadas:
  The patch for BZ 2836 has passed reviewed. It will be merged early next week. Then, your patch will also be merged next week.

Thanks
Liming
> -----Original Message-----
> From: Veliyathuparambil Prakashan, KrishnadasX <krishnadasx.veliyathuparambil.prakashan@intel.com>
> Sent: Friday, July 17, 2020 12:09 PM
> To: Gao, Liming <liming.gao@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; T V, Krishnamoorthy <krishnamoorthy.t.v@intel.com>; devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
> 
> Hello Liming,
> 
> Gentle Reminder.
> As discussed before, please let us know when we can expect our changes (below BZ) to get pushed in to Edk2Repo.
> Please help to give an ETA.
> 
> BZ Details:
> [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC
> https://bugzilla.tianocore.org/show_bug.cgi?id=2818
> 
> Dependency : https://bugzilla.tianocore.org/show_bug.cgi?id=2836
> 
> Thanks,
> Krishnadas
> 
> >-----Original Message-----
> > From: Veliyathuparambil Prakashan, KrishnadasX
> > Sent: Friday, July 3, 2020 3:00 PM
> > To: Gao, Liming <liming.gao@intel.com>; Gao, Zhichao
> > <zhichao.gao@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; T V, Krishnamoorthy
> > <Krishnamoorthy.T.V@intel.com>; devel@edk2.groups.io; leif@nuviainc.com;
> > Laszlo Ersek <lersek@redhat.com>; afish@apple.com; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure,
> > Destroy RamDisk memory before RSC.
> >
> > Thank you very much Liming  and Zhichao for your time to discuss this case.
> >
> > Hello Liming,
> >
> > As discussed, please help to raise the BZ to enhance PatchCheck.py and kindly
> > help to submit our Edk2 patch to Edk2 Repo.
> > [EDK2 Change BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2818]
> >
> > Also, please give us an update on next week regarding the ETA , as per our
> > discussion.
> >
> > Thanks,
> > Krishnadas
> >
> > > -----Original Message-----
> > > From: Gao, Liming <liming.gao@intel.com>
> > > Sent: Friday, July 3, 2020 11:32 AM
> > > To: devel@edk2.groups.io; Veliyathuparambil Prakashan, KrishnadasX
> > > <krishnadasx.veliyathuparambil.prakashan@intel.com>;
> > > leif@nuviainc.com; Laszlo Ersek <lersek@redhat.com>; afish@apple.com;
> > > Kinney, Michael D <michael.d.kinney@intel.com>
> > > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption
> > > failure, Destroy RamDisk memory before RSC.
> > >
> > > Signed-off-by line is too long and exceeds 80 characters requirement.
> > > But, it is valid.
> > >
> > > So, I suggest to enhance PatchCheck.py and skip the check for the
> > > lines with Signed-off-by, Ack-by:, Reviewed-by:, and Tested-By:.
> > >
> > > Thanks
> > > Liming
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <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
> > > >
> > > >
> > > > -=-=-=-=-=-=
> > > > Groups.io Links: You receive all messages sent to this group.
> > > >
> > > > View/Reply Online (#61517):
> > > > https://edk2.groups.io/g/devel/message/61517
> > > > Mute This Topic: https://groups.io/mt/74978785/1759384
> > > > Group Owner: devel+owner@edk2.groups.io
> > > > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > > > [liming.gao@intel.com] -=-=-=-=-=-=


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
       [not found]       ` <162292995B2D9EDC.3435@groups.io>
@ 2020-07-21  0:46         ` Liming Gao
  0 siblings, 0 replies; 10+ messages in thread
From: Liming Gao @ 2020-07-21  0:46 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Liming,
	Veliyathuparambil Prakashan, KrishnadasX, Gao, Zhichao
  Cc: Ni, Ray, T V, Krishnamoorthy, Kinney, Michael D, Gao, Liming

This patch has been merged @ cb38ace647231076acfc0c5bdd21d3aff43e4f83.

Thanks
Liming

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming Gao
> Sent: Friday, July 17, 2020 11:07 PM
> To: Veliyathuparambil Prakashan, KrishnadasX <krishnadasx.veliyathuparambil.prakashan@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; T V, Krishnamoorthy <krishnamoorthy.t.v@intel.com>; devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
> 
> Krishnadas:
>   The patch for BZ 2836 has passed reviewed. It will be merged early next week. Then, your patch will also be merged next week.
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: Veliyathuparambil Prakashan, KrishnadasX <krishnadasx.veliyathuparambil.prakashan@intel.com>
> > Sent: Friday, July 17, 2020 12:09 PM
> > To: Gao, Liming <liming.gao@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; T V, Krishnamoorthy <krishnamoorthy.t.v@intel.com>; devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC.
> >
> > Hello Liming,
> >
> > Gentle Reminder.
> > As discussed before, please let us know when we can expect our changes (below BZ) to get pushed in to Edk2Repo.
> > Please help to give an ETA.
> >
> > BZ Details:
> > [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2818
> >
> > Dependency : https://bugzilla.tianocore.org/show_bug.cgi?id=2836
> >
> > Thanks,
> > Krishnadas
> >
> > >-----Original Message-----
> > > From: Veliyathuparambil Prakashan, KrishnadasX
> > > Sent: Friday, July 3, 2020 3:00 PM
> > > To: Gao, Liming <liming.gao@intel.com>; Gao, Zhichao
> > > <zhichao.gao@intel.com>
> > > Cc: Ni, Ray <ray.ni@intel.com>; T V, Krishnamoorthy
> > > <Krishnamoorthy.T.V@intel.com>; devel@edk2.groups.io; leif@nuviainc.com;
> > > Laszlo Ersek <lersek@redhat.com>; afish@apple.com; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption failure,
> > > Destroy RamDisk memory before RSC.
> > >
> > > Thank you very much Liming  and Zhichao for your time to discuss this case.
> > >
> > > Hello Liming,
> > >
> > > As discussed, please help to raise the BZ to enhance PatchCheck.py and kindly
> > > help to submit our Edk2 patch to Edk2 Repo.
> > > [EDK2 Change BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2818]
> > >
> > > Also, please give us an update on next week regarding the ETA , as per our
> > > discussion.
> > >
> > > Thanks,
> > > Krishnadas
> > >
> > > > -----Original Message-----
> > > > From: Gao, Liming <liming.gao@intel.com>
> > > > Sent: Friday, July 3, 2020 11:32 AM
> > > > To: devel@edk2.groups.io; Veliyathuparambil Prakashan, KrishnadasX
> > > > <krishnadasx.veliyathuparambil.prakashan@intel.com>;
> > > > leif@nuviainc.com; Laszlo Ersek <lersek@redhat.com>; afish@apple.com;
> > > > Kinney, Michael D <michael.d.kinney@intel.com>
> > > > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> > > > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Upon BootOption
> > > > failure, Destroy RamDisk memory before RSC.
> > > >
> > > > Signed-off-by line is too long and exceeds 80 characters requirement.
> > > > But, it is valid.
> > > >
> > > > So, I suggest to enhance PatchCheck.py and skip the check for the
> > > > lines with Signed-off-by, Ack-by:, Reviewed-by:, and Tested-By:.
> > > >
> > > > Thanks
> > > > Liming
> > > > > -----Original Message-----
> > > > > From: devel@edk2.groups.io <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
> > > > >
> > > > >
> > > > > -=-=-=-=-=-=
> > > > > Groups.io Links: You receive all messages sent to this group.
> > > > >
> > > > > View/Reply Online (#61517):
> > > > > https://edk2.groups.io/g/devel/message/61517
> > > > > Mute This Topic: https://groups.io/mt/74978785/1759384
> > > > > Group Owner: devel+owner@edk2.groups.io
> > > > > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > > > > [liming.gao@intel.com] -=-=-=-=-=-=
> 
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-07-21  0:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-19  2:40 [PATCH] MdeModulePkg: Upon BootOption failure, Destroy RamDisk memory before RSC KrishnadasX Veliyathuparambil Prakashan
2020-06-22  8:32 ` [edk2-devel] " Wang, Sunny (HPS SW)
2020-07-03  6:02 ` Liming Gao
2020-07-03  9:30   ` Veliyathuparambil Prakashan, KrishnadasX
2020-07-17  4:09     ` Veliyathuparambil Prakashan, KrishnadasX
2020-07-17  4:54       ` Gao, Zhichao
2020-07-17 15:06       ` Liming Gao
     [not found]       ` <162292995B2D9EDC.3435@groups.io>
2020-07-21  0:46         ` Liming Gao
2020-07-03 10:43   ` Leif Lindholm
2020-07-03 15:02     ` Liming Gao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox