Re: [edk2-devel] [PATCH] MdeModulePkg/BdsDxe: Make PlatformRecovery work regardless of OsIndications

["Wang, Jian J" <jian.j.wang@intel.com>]

I'm not familiar this part of code. Ray is expert.

Acked-by: Jian J Wang <jian.j.wang@intel.com>

> -----Original Message-----
> From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
> Sent: Wednesday, October 16, 2019 5:43 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io;
> lersek@redhat.com; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Li, Walon <walon.li@hpe.com>; Wang, Sunny
> (HPS SW) <sunnywang@hpe.com>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/BdsDxe: Make
> PlatformRecovery work regardless of OsIndications
> 
> Thanks for catching this, Laszlo.
> Thanks for the suggestion and information, Zhichao. Next time, I will definitely
> take care of this in the first place.
> 
> Hi Jian and Hao,
> I forgot to add both of you in Cc. Sorry about that. I will send you guys a new
> patch with the updated commit message below. If you guys need me to resend
> an email to edk2-devel for the new patch, feel free to let me know.
> 
> ===============================================
> MdeModulePkg/BdsDxe: Fix PlatformRecovery issue
> 
> For now, PlatformRecovery doesn't work if OsIndications variable
> doesn't exist, which is wrong.
> According to the UEFI specification section 3.4.1 and 3.4.2, if
> processing of BootOrder does not result in success, the OsRecovery
> and PlatformRecovery options should still be processed regardless of
> the existence of the OsIndications variable.
> Therefore, update the code to check PcdPlatformRecoverySupport instead
> of the value of OsIndications variable (PlatformRecovery) to fix
> this issue.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Walon Li <walon.li@hpe.com>
> Signed-off-by: Sunny Wang <sunnywang@hpe.com>
> =================================================
> 
> Regards,
> Sunny Wang
> 
> -----Original Message-----
> From: Gao, Zhichao [mailto:zhichao.gao@intel.com]
> Sent: Wednesday, October 16, 2019 4:36 PM
> To: devel@edk2.groups.io; lersek@redhat.com; Wang, Sunny (HPS SW)
> <sunnywang@hpe.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Li, Walon <walon.li@hpe.com>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/BdsDxe: Make
> PlatformRecovery work regardless of OsIndications
> Importance: High
> 
> MdeModulePkg/BdsDxe: Do PlatformRecovery regardless of OsIndications
> 
> According to the UEFI specification section 3.4.1 and 3.4.2, the OsRecovery and
> PlatformRecovery options should still be processed regardless of the value of
> OsIndications variable if processing of BootOrder does not result in success.
> Therefore, update the code to check PcdPlatformRecoverySupport instead of
> the value of OsIndications variable (PlatformRecovery).
> 
> I suggest to use the above title because of the length limitation. And I also break
> the commit message into serval lines.
> Here is the development process for edk open source link:
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-
> Process. That would help you to send a patch in a basic correct format.
> With the commit massage issue addressed, Reviewed-by: Zhichao Gao
> <zhichao.gao@intel.com>
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Laszlo Ersek
> > Sent: Wednesday, October 16, 2019 4:02 PM
> > To: devel@edk2.groups.io; sunnywang@hpe.com
> > Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> > Walon Li <walon.li@hpe.com>
> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/BdsDxe: Make
> > PlatformRecovery work regardless of OsIndications
> >
> > On 10/16/19 09:40, Wang, Sunny (HPS SW) wrote:
> > > According to the UEFI specification section 3.4.1 and 3.4.2, the
> > > OsRecovery
> > and PlatformRecovery options should still be processed regardless of
> > the value of OsIndications variable if processing of BootOrder does
> > not result in success. Therefore, update the code to check
> > PcdPlatformRecoverySupport instead of the value of OsIndications variable
> (PlatformRecovery).
> >
> > Please wrap the commit message to 74 characters.
> >
> > (Can be done on push, if the maintainer accepts the patch.)
> >
> > Thanks
> > Laszlo
> >
> > >
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > Cc: Walon Li <walon.li@hpe.com>
> > > Signed-off-by: Sunny Wang <sunnywang@hpe.com>
> > > ---
> > >  MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > > index d6ec31118c..d387dbe7ac 100644
> > > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > > @@ -6,7 +6,7 @@
> > >    to enter BDS phase.
> > >
> > >  Copyright (c) 2004 - 2019, Intel Corporation. All rights
> > > reserved.<BR>
> > > -(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > > +(C) Copyright 2016-2019 Hewlett Packard Enterprise Development
> > > +LP<BR>
> > >  (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > @@ -1069,7 +1069,7 @@ BdsEntry (
> > >    }
> > >
> > >    if (!BootSuccess) {
> > > -    if (PlatformRecovery) {
> > > +    if (PcdGetBool (PcdPlatformRecoverySupport)) {
> > >        LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount,
> > LoadOptionTypePlatformRecovery);
> > >        ProcessLoadOptions (LoadOptions, LoadOptionCount);
> > >        EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
> > >
> >
> >
> > 
>