Re: [edk2-devel] Use a pcd to control PlatformRecovery

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

Sorry for the delay and thanks for checking my question, Ray.

Since the OS recovery and platform recovery options were added to UEFI specification at the same time, I thought we will also implement it and push the code regardless of the OS support. 

No, I didn't see a real need of using the OS recovery option. If my memory serves me well, I have only seen a need of using "One-Time" OS recovery, but it can be done by creating, adjusting, and removing a boot option in boot order. 
However, I can still imagine that the OS recovery option is needed for the use of "persistent" OS recovery. Therefore, if we don't have any concern about adding OS recovery option support, I think it is still better to push the code because the UEFI specification mentioned it. Add Peter. He may know more information about this.

By the way, I think if no one works on the task below, there will be no OS or OS tool supporting it. :)
   - https://github.com/rhboot/efibootmgr/issues/41


Regards,
Sunny Wang

-----Original Message-----
From: Ni, Ray [mailto:ray.ni@intel.com] 
Sent: Wednesday, October 16, 2019 4:43 PM
To: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Li, Walon <walon.li@hpe.com>; Wei, Kent (HPS SW) <kent.wei@hpe.com>; Spottswood, Jason <jason.spottswood@hpe.com>
Subject: RE: [edk2-devel] Use a pcd to control PlatformRecovery
Importance: High

Sunny,
For the OS recovery question, I had the code change in https://github.com/niruiyu/edk2/tree/bds/osrecovery.

Due to there is no OS support for OS recovery yet, the code was not pushed.

Do you see any needs of the OS recovery?

Thanks,
Ray

> -----Original Message-----
> From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
> Sent: Wednesday, October 16, 2019 3:42 PM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Ni, 
> Ray <ray.ni@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A 
> <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming 
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; 
> Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew 
> <Bret.Barkelew@microsoft.com>; Li, Walon <walon.li@hpe.com>; Wei, Kent 
> (HPS SW) <kent.wei@hpe.com>; Spottswood, Jason 
> <jason.spottswood@hpe.com>; Wang, Sunny (HPS SW) <sunnywang@hpe.com>
> Subject: RE: [edk2-devel] Use a pcd to control PlatformRecovery
> 
> Thanks for checking this, Zhichao and Ray.
> I just sent a patch to fix it with the subject " [edk2-devel] [PATCH]
> MdeModulePkg/BdsDxe: Make PlatformRecovery work regardless of 
> OsIndications"
> 
> Regards,
> Sunny Wang
> 
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of 
> Gao, Zhichao
> Sent: Wednesday, October 16, 2019 1:57 PM
> To: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; devel@edk2.groups.io; 
> Ni, Ray <ray.ni@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A 
> <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming 
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; 
> Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew 
> <Bret.Barkelew@microsoft.com>; Li, Walon <walon.li@hpe.com>; Wei, Kent 
> (HPS SW) <kent.wei@hpe.com>
> Subject: Re: [edk2-devel] Use a pcd to control PlatformRecovery
> Importance: High
> 
> 
> > -----Original Message-----
> > From: Gao, Zhichao
> > Sent: Wednesday, October 16, 2019 10:09 AM
> > To: 'Wang, Sunny (HPS SW)' <sunnywang@hpe.com>;
> devel@edk2.groups.io;
> > Ni, Ray <ray.ni@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A 
> > <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming 
> > <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; 
> > Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew 
> > <Bret.Barkelew@microsoft.com>; Li, Walon <walon.li@hpe.com>; Wei,
> Kent
> > (HPS SW) <kent.wei@hpe.com>
> > Subject: RE: Use a pcd to control PlatformRecovery
> >
> > First of all, the patch didn't aim to change the other part of the 
> > boot flow except PlatformRecovery.
> >
> > Local variable PlatformRecovery is controlled by OsIndications 
> > variable. When the EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY is
> set,
> > the firmware should try to platform specific recovery. But that 
> > doesn't mean the platform must support the specific recovery. i.e.
> > local PlatformRecovery is controlling the boot flow and the Pcd just 
> > indicated whether the platform support recovery or not.
> > So, on my opinion, I don't agree with your change.
> 
> After discuss with Sunny and Ray, and refer to the spec section 3.4.1 
> and 3.4.2, the OsRecovery and PlatformRecovery should always be 
> operated regardless of the value of OsIndication variable if fail to 
> boot the BootOrder. I am wrong. We should change to use the 
> PcdPlatformRecoverySupport to control the PlatformRecovery. Please 
> help to send a patch to fix it. Thanks a lot.
> 
> >
> > Default Platform Recovery refer to the short file path to boot the OS.
> > If the firmware supports platform recovery, then *short file path* 
> > option would be one part of the PlatformRecovery#### in case there 
> > are no other boot options. If the firmware doesn't support platform 
> > recovery, we still need this default boot thru a short file path and 
> > it should not depend on the PlatformRecovery#### variable for the
> compatibility thinking.
> >
> > Thanks,
> > Zhichao
> >
> > > -----Original Message-----
> > > From: Wang, Sunny (HPS SW) [mailto:sunnywang@hpe.com]
> > > Sent: Tuesday, October 15, 2019 4:53 PM
> > > To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; 
> > > Ni, Ray <ray.ni@intel.com>
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A 
> > > <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, 
> > > Liming <liming.gao@intel.com>; Sean Brogan 
> > > <sean.brogan@microsoft.com>; Michael Turner 
> > > <Michael.Turner@microsoft.com>; Bret Barkelew 
> > > <Bret.Barkelew@microsoft.com>; Li, Walon <walon.li@hpe.com>; Wei,
> > Kent
> > > (HPS SW) <kent.wei@hpe.com>; Wang, Sunny (HPS SW)
> > <sunnywang@hpe.com>
> > > Subject: Use a pcd to control PlatformRecovery
> > >
> > > Hi Zhichao and Ray,
> > >
> > > I have some questions about this code change. Sorry for being late 
> > > to bring my questions here.
> > >
> > > For now, the code block for iterating the PlatformRecovery#### 
> > > variables is controlled by OsIndications variable. However, it 
> > > looks to me like that the PlatformRecovery#### should still be 
> > > attempted for the case of that processing of BootOrder does NOT 
> > > result in success (according to section 3.4 in UEFI 2.8). In other 
> > > words, I think we should check PCD "PcdPlatformRecoverySupport" 
> > > instead of Local variable
> > "PlatformRecovery"
> > > (from OsIndications variable) like the code below. What do you 
> > > guys think? If you need a meeting or short talk to discuss this, 
> > > feel free to let me
> > know.
> > >
> > >   if (!BootSuccess) {
> > > -   if (PlatformRecovery) {
> > > +  if (PcdGetBool (PcdPlatformRecoverySupport)) {
> > >       LoadOptions = EfiBootManagerGetLoadOptions 
> > > (&LoadOptionCount, LoadOptionTypePlatformRecovery);
> > >       ProcessLoadOptions (LoadOptions, LoadOptionCount);
> > >       EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
> > >     } else {
> > >       //
> > >       // When platform recovery is not enabled, still boot to 
> > > platform default file path.
> > >       //
> > >       EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption);
> > >     }
> > >
> > >
> > > In addition, it looks like EDK2 don't have code to process 
> > > OsRecovery#### variables. Do we need to create a Bug on TianoCore
> > Bugzilla system?
> 
> OsRecovery#### doesn't have an implement yet, we should co-work with 
> the OS vendor to define the operation. For now, there is no requirement.
> 
> > >
> > > Moreover, I saw that both of you had a discussion about "Default 
> > > PlatformRecovery", but I can't figure out the connection between 
> > > the discussion and the final code change. Isn't the "Default
> PlatformRecovery"
> > > part of the Platform Recovery feature? At this moment, we don't 
> > > have OS recovery support, so I think that NO platform recovery 
> > > support can be identified as NO boot option recovery support. For 
> > > this case, shouldn't we use PCD "PcdPlatformRecoverySupport" to 
> > > control "Default PlatformRecovery" as well? For the next step, I 
> > > think we need to get further clarification from USWG to either not 
> > > tie "Default Boot Behavior" with PlatformRecovery or update the 
> > > description to the
> > following:
> > >     - If system firmware supports Platform recovery as described 
> > > in Section 3.4.2, system firmware must include a 
> > > PlatformRecovery#### variable specifying a short-form File Path Media Device Path....
> 
> As I said above the 'Default Platform Recovery' refer to the 
> short-form file path boot. When the PcdPlatformRecoverySupport is 
> TRUE, it is part of PlatformRecovery. Ohterwise, it is not part of 
> platform recovery and we still need to support it because of the 
> capability issue. And that isn't conflict with the spec.
> 
> Thanks,
> Zhichao
> 
> > >
> > > Regards,
> > > Sunny Wang
> > >
> 
> 
>