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

["Leif Lindholm" <leif@nuviainc.com>]

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]
> > -=-=-=-=-=-=
>