From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mx.groups.io with SMTP id smtpd.web10.718.1571714231131133271 for ; Mon, 21 Oct 2019 20:17:11 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: ray.ni@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Oct 2019 18:13:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,325,1566889200"; d="scan'208";a="397510943" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga005.fm.intel.com with ESMTP; 21 Oct 2019 18:13:55 -0700 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 21 Oct 2019 18:13:55 -0700 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Mon, 21 Oct 2019 18:13:55 -0700 Received: from shsmsx107.ccr.corp.intel.com (10.239.4.96) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Mon, 21 Oct 2019 18:13:54 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.166]) by SHSMSX107.ccr.corp.intel.com ([169.254.9.33]) with mapi id 14.03.0439.000; Tue, 22 Oct 2019 09:13:53 +0800 From: "Ni, Ray" To: "Wang, Sunny (HPS SW)" , "devel@edk2.groups.io" , "Gao, Zhichao" , "pjones@redhat.com" , Sean Brogan CC: "Wang, Jian J" , "Wu, Hao A" , "Zeng, Star" , "Gao, Liming" , Michael Turner , Bret Barkelew , "Li, Walon" , "Wei, Kent (HPS SW)" , "Spottswood, Jason" Subject: Re: [edk2-devel] Use a pcd to control PlatformRecovery Thread-Topic: [edk2-devel] Use a pcd to control PlatformRecovery Thread-Index: AdWDK94yxgzpZjqYRC+1b6PvmRyefQAmqTjgAAec3iAABAVasAACE+xQAQJ4ahAAG5/1kA== Date: Tue, 22 Oct 2019 01:13:53 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C32B297@SHSMSX104.ccr.corp.intel.com> References: <3CE959C139B4C44DBEA1810E3AA6F9000B857172@SHSMSX101.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C325246@SHSMSX104.ccr.corp.intel.com> In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Sean, Do you have any comments from MS side? Thanks, Ray > -----Original Message----- > From: Wang, Sunny (HPS SW) > Sent: Monday, October 21, 2019 8:45 PM > To: Ni, Ray ; devel@edk2.groups.io; Gao, Zhichao > ; pjones@redhat.com > Cc: Wang, Jian J ; Wu, Hao A = ; > Zeng, Star ; Gao, Liming ; > Sean Brogan ; Michael Turner > ; Bret Barkelew > ; Li, Walon ; Wei, Kent > (HPS SW) ; Spottswood, Jason > ; Wang, Sunny (HPS SW) > > Subject: RE: [edk2-devel] Use a pcd to control PlatformRecovery >=20 > Sorry for the delay and thanks for checking my question, Ray. >=20 > 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. >=20 > No, I didn't see a real need of using the OS recovery option. If my memo= ry > 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 t= he use > of "persistent" OS recovery. Therefore, if we don't have any concern abo= ut > adding OS recovery option support, I think it is still better to push th= e code > because the UEFI specification mentioned it. Add Peter. He may know more > information about this. >=20 > 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 >=20 >=20 > Regards, > Sunny Wang >=20 > -----Original Message----- > From: Ni, Ray [mailto:ray.ni@intel.com] > Sent: Wednesday, October 16, 2019 4:43 PM > To: Wang, Sunny (HPS SW) ; devel@edk2.groups.io; > Gao, Zhichao > Cc: Wang, Jian J ; Wu, Hao A = ; > Zeng, Star ; Gao, Liming ; > Sean Brogan ; Michael Turner > ; Bret Barkelew > ; Li, Walon ; Wei, Kent > (HPS SW) ; Spottswood, Jason > > Subject: RE: [edk2-devel] Use a pcd to control PlatformRecovery > Importance: High >=20 > Sunny, > For the OS recovery question, I had the code change in > https://github.com/niruiyu/edk2/tree/bds/osrecovery. >=20 > Due to there is no OS support for OS recovery yet, the code was not push= ed. >=20 > Do you see any needs of the OS recovery? >=20 > Thanks, > Ray >=20 > > -----Original Message----- > > From: Wang, Sunny (HPS SW) > > Sent: Wednesday, October 16, 2019 3:42 PM > > To: devel@edk2.groups.io; Gao, Zhichao ; Ni, > > Ray > > Cc: Wang, Jian J ; Wu, Hao A > > ; Zeng, Star ; Gao, Liming > > ; Sean Brogan ; > > Michael Turner ; Bret Barkelew > > ; Li, Walon ; Wei, > Kent > > (HPS SW) ; Spottswood, Jason > > ; Wang, Sunny (HPS SW) > > > 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) ; > devel@edk2.groups.io; > > Ni, Ray > > Cc: Wang, Jian J ; Wu, Hao A > > ; Zeng, Star ; Gao, Liming > > ; Sean Brogan ; > > Michael Turner ; Bret Barkelew > > ; Li, Walon ; Wei, > Kent > > (HPS SW) > > 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)' ; > > devel@edk2.groups.io; > > > Ni, Ray > > > Cc: Wang, Jian J ; Wu, Hao A > > > ; Zeng, Star ; Gao, Liming > > > ; Sean Brogan ; > > > Michael Turner ; Bret Barkelew > > > ; Li, Walon ; Wei, > > Kent > > > (HPS SW) > > > 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 O= S. > > > 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 ; > > > > Ni, Ray > > > > Cc: Wang, Jian J ; Wu, Hao A > > > > ; Zeng, Star ; Gao, > > > > Liming ; Sean Brogan > > > > ; Michael Turner > > > > ; Bret Barkelew > > > > ; Li, Walon ; Wei, > > > Kent > > > > (HPS SW) ; Wang, Sunny (HPS SW) > > > > > > > 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 =3D 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 requiremen= t. > > > > > > > > > > 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 Me= dia > 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 > > > > > > > >=20 > > >=20