From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=jiewen.yao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8BD08211B698E for ; Mon, 11 Jun 2018 05:37:04 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jun 2018 05:37:04 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,501,1520924400"; d="scan'208";a="207078457" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga004.jf.intel.com with ESMTP; 11 Jun 2018 05:37:03 -0700 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 11 Jun 2018 05:37:03 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 11 Jun 2018 05:37:03 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.223]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.70]) with mapi id 14.03.0319.002; Mon, 11 Jun 2018 20:37:01 +0800 From: "Yao, Jiewen" To: Ard Biesheuvel CC: "Kinney, Michael D" , "edk2-devel@lists.01.org" , "leif.lindholm@linaro.org" , "Zeng, Star" , "Yao, Jiewen" Thread-Topic: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules () to be called once Thread-Index: AQHT/vYbbSADsfB+hEWX7Kwk2kNBmKRWSrdA//98PACAAIb9kIACKYCAgACr9YCAALUdEIAABtFQgABH4wCAANynWw== Date: Mon, 11 Jun 2018 12:37:00 +0000 Message-ID: <87F9382F-EBA0-4715-B323-FB0C6F8DE5A2@intel.com> References: <20180608065811.2065-1-ard.biesheuvel@linaro.org> <20180608065811.2065-3-ard.biesheuvel@linaro.org> <74D8A39837DF1E4DA445A8C0B3885C503AC3515B@shsmsx102.ccr.corp.intel.com> <6534F306-C3E7-40D2-84F4-D409BF1F7B68@linaro.org> <74D8A39837DF1E4DA445A8C0B3885C503AC3A387@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503AC3ADC8@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503AC3ADF5@shsmsx102.ccr.corp.intel.com>, In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: MIME-Version: 1.0 Subject: Re: [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules () to be called once X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Jun 2018 12:37:04 -0000 Content-Language: zh-CN Content-Type: text/plain; charset="iso-2022-jp" Content-Transfer-Encoding: quoted-printable If all fmp can be processed one time=1B$B!$=1B(Byou just need call once. Th= en system reset. thank you! Yao, Jiewen > =1B$B:_=1B(B 2018=1B$BG/=1B(B6=1B$B7n=1B(B11=1B$BF|!$>e8a=1B(B12:27=1B$B!= $=1B(BArd Biesheuvel =1B$B=20 >> On 10 June 2018 at 21:21, Yao, Jiewen wrote: >> Just got some idea since I am also reviewing FmpDevicePkg patch. >>=20 >>=20 >> The key problem we are trying to resolve that is: The core code uses End= OfDxe as the lock event for system firmware, but an ARM platform may want t= o lock system firmware later, maybe in other lock event. >>=20 >> As such, how about we generalize the lock event as PcdSystemFmpLockEvent= Guid? We can use EndOfDxeGuid as default one to keep the compatibility. At = same time this brings the flexibility for platform overriding. >>=20 >>=20 >> We only need update DxeCapsuleLibConstructor() to use the new PcdGuid, i= nstead of hardcode EndOfDxe. >> We don't need update the logic in ProcessCapsules() and ProcessTheseCaps= ules(). >> The policy in current platform is already enforced, if the platform has = a trusted console. >> For ARM platform, which wants to lock system firmware later. It can conf= igure another lock event GUID explicitly in platform DSC. >>=20 >=20 > But that means we are still required to call ProcessCapsules () twice, no= ? >=20 >>> -----Original Message----- >>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of = Yao, >>> Jiewen >>> Sent: Sunday, June 10, 2018 12:02 PM >>> To: Kinney, Michael D ; Ard Biesheuvel >>> >>> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star >>> >>> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permi= t >>> ProcessCapsules () to be called once >>>=20 >>> My concern is that *always allowing* processing SystemCapsule after End= OfDxe >>> has security risk. >>>=20 >>> IMHO, the risk is not *process*, if it is a platform choice. Instead, t= he risk is >>> *allow*, in the core logic. >>>=20 >>> If we really want to do that, I hope we need a way to distinguish the d= ifference >>> between a platform dispatching SystemCapsule after EndOfDxe *purposely*= and >>> a platform dispatching SystemCapsule after EndOfDxe *by mistake*. >>>=20 >>> Maybe some policy enforcement in the core logic. Static policy, at buil= d time. >>>=20 >>> Thank you >>> Yao Jiewen >>>=20 >>>> -----Original Message----- >>>> From: Kinney, Michael D >>>> Sent: Sunday, June 10, 2018 8:57 AM >>>> To: Ard Biesheuvel ; Yao, Jiewen >>>> ; Kinney, Michael D >>>> Cc: edk2-devel@lists.01.org; Zeng, Star ; >>>> leif.lindholm@linaro.org >>>> Subject: RE: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: perm= it >>>> ProcessCapsules () to be called once >>>>=20 >>>> Ard, >>>>=20 >>>> I agree it should be a platform choice to process capsules >>>> before or after End of DXE. It is even allowed in the >>>> UEFI Spec for capsules to be processed immediately >>>> which would include at OS runtime after ExitBootServices. >>>>=20 >>>> There are 2 inputs to these choices: >>>> * The flags set in the Capsule itself. See UEFI 2.7 Spec >>>> Table 38 for the 5 allowed combinations. >>>> * Platform policy for each of these 5 capsule types and >>>> when each of these 5 capsule types are allowed to be >>>> processed. >>>>=20 >>>> Jiewen's comments point to some assumptions that have been >>>> made in the logic. These assumptions may be considered a >>>> safe default platform policy, but the code logic should >>>> allow the platform to easily select alternate policies. >>>>=20 >>>> I think the patch you have provided attempts to add one >>>> additional policy. Perhaps we should look at this from >>>> the UEFI Spec perspective and see how difficult it is to >>>> express policies for the supported capsule types. >>>>=20 >>>> I will review your patch in more detail tomorrow. >>>>=20 >>>> Thanks, >>>>=20 >>>> Mike >>>>=20 >>>>> -----Original Message----- >>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >>>>> Sent: Saturday, June 9, 2018 10:42 PM >>>>> To: Yao, Jiewen >>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D >>>>> ; Zeng, Star >>>>> ; leif.lindholm@linaro.org >>>>> Subject: Re: [edk2] [PATCH v2 2/5] >>>>> MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules >>>>> () to be called once >>>>>=20 >>>>> On 10 June 2018 at 07:38, Yao, Jiewen >>>>> wrote: >>>>>> Hi Ard >>>>>> According to PI spec, "Prior to invoking any UEFI >>>>> drivers, or applications that are not from the platform >>>>> manufacturer, or connecting consoles, the platform >>>>> should signals the event EFI_END_OF_DXE_EVENT_GUID" >>>>>>=20 >>>>>> In brief, EndOfDxe is signaled before 3rd part code >>>>> running. >>>>>>=20 >>>>>> As such, it is legal that a trusted console is >>>>> connected before EndOfDxe. >>>>>> You can report progress to user before EndOfDxe. >>>>>>=20 >>>>>=20 >>>>> So how do I connect a trusted console on a system with >>>>> a plugin graphics card? >>>>> How can I report capsule update progress on such a >>>>> system? >>>>> On a system such as ARM where the actual flash update >>>>> involves calls >>>>> into the standalone MM layer, which makes the >>>>> distinction unnecessary, >>>>> how do you recommend to handle this if it is mandatory >>>>> according to >>>>> you to process the capsule before EndOfDxe? >>>>>=20 >>>>>=20 >>>>>>> -----Original Message----- >>>>>>> From: Ard Biesheuvel >>>>> [mailto:ard.biesheuvel@linaro.org] >>>>>>> Sent: Friday, June 8, 2018 8:38 AM >>>>>>> To: Yao, Jiewen >>>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D >>>>> ; >>>>>>> Zeng, Star ; >>>>> leif.lindholm@linaro.org >>>>>>> Subject: Re: [edk2] [PATCH v2 2/5] >>>>> MdeModulePkg/DxeCapsuleLibFmp: permit >>>>>>> ProcessCapsules () to be called once >>>>>>>=20 >>>>>>>=20 >>>>>>>=20 >>>>>>>> On 8 Jun 2018, at 14:34, Yao, Jiewen >>>>> wrote: >>>>>>>>=20 >>>>>>>> Hi Ard >>>>>>>> We don't allow platform to update system firmware >>>>> *after* EndOfDxe. >>>>>>>>=20 >>>>>>>> According to PI spec, after EndOfDxe, 3rd part >>>>> code start running. It brings >>>>>>> security risk if we allow system firmware after >>>>> EndOfDxe. >>>>>>>>=20 >>>>>>>> In our X86 system design, we lock flash part >>>>> *before* EndOfDxe in any boot >>>>>>> mode. >>>>>>>> Even in CapsuleUpdate boot mode, we also lock >>>>> flash part at EndOfDxe, just in >>>>>>> case the capsule update does not indicate a reset. >>>>>>>>=20 >>>>>>>> Would you please share the info, why your platform >>>>> need update system >>>>>>> firmware after EndOfDxe? >>>>>>>> Is that possible to make it earlier? >>>>>>>>=20 >>>>>>>>=20 >>>>>>>=20 >>>>>>> Because we need some kind of console to report >>>>> progress to the user. >>>>>>>=20 >>>>>>> The secure platform execution context is completely >>>>> separate from UEFI on Arm, >>>>>>> so for us the distinction does not make sense. >>>>>>>=20 >>>>>>>> Thank you >>>>>>>> Yao Jiewen >>>>>>>>=20 >>>>>>>>> -----Original Message----- >>>>>>>>> From: edk2-devel [mailto:edk2-devel- >>>>> bounces@lists.01.org] On Behalf Of >>>>>>> Ard >>>>>>>>> Biesheuvel >>>>>>>>> Sent: Friday, June 8, 2018 2:58 AM >>>>>>>>> To: edk2-devel@lists.01.org >>>>>>>>> Cc: Kinney, Michael D >>>>> ; Yao, Jiewen >>>>>>>>> ; Zeng, Star >>>>> ; >>>>>>>>> leif.lindholm@linaro.org; Ard Biesheuvel >>>>> >>>>>>>>> Subject: [edk2] [PATCH v2 2/5] >>>>> MdeModulePkg/DxeCapsuleLibFmp: permit >>>>>>>>> ProcessCapsules () to be called once >>>>>>>>>=20 >>>>>>>>> Permit ProcessCapsules () to be called only a >>>>> single time, after >>>>>>>>> EndOfDxe. This allows platforms that are able to >>>>> update system >>>>>>>>> firmware after EndOfDxe (e.g., because the flash >>>>> ROM is not locked >>>>>>>>> down) to do so at a time when a non-trusted >>>>> console is up and running, >>>>>>>>> and progress can be reported to the user. >>>>>>>>>=20 >>>>>>>>> Contributed-under: TianoCore Contribution >>>>> Agreement 1.1 >>>>>>>>> Signed-off-by: Ard Biesheuvel >>>>> >>>>>>>>> --- >>>>>>>>>=20 >>>>> MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess >>>>> Lib.c | 18 >>>>>>>>> ++++++++++++------ >>>>>>>>> 1 file changed, 12 insertions(+), 6 deletions(-) >>>>>>>>>=20 >>>>>>>>> diff --git >>>>>>>=20 >>>>> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce >>>>> ssLib.c >>>>>>>>>=20 >>>>> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce >>>>> ssLib.c >>>>>>>>> index 26ca4e295f20..ad83660f1737 100644 >>>>>>>>> --- >>>>> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce >>>>> ssLib.c >>>>>>>>> +++ >>>>> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce >>>>> ssLib.c >>>>>>>>> @@ -100,6 +100,7 @@ IsValidCapsuleHeader ( >>>>>>>>>=20 >>>>>>>>> extern BOOLEAN >>>>> mDxeCapsuleLibEndOfDxe; >>>>>>>>> BOOLEAN mNeedReset; >>>>>>>>> +BOOLEAN mFirstRound =3D >>>>> TRUE; >>>>>>>>>=20 >>>>>>>>> VOID **mCapsulePtr; >>>>>>>>> EFI_STATUS *mCapsuleStatusArray; >>>>>>>>> @@ -364,8 +365,11 @@ >>>>> PopulateCapsuleInConfigurationTable ( >>>>>>>>>=20 >>>>>>>>> Each individual capsule result is recorded in >>>>> capsule record variable. >>>>>>>>>=20 >>>>>>>>> - @param[in] FirstRound TRUE: First >>>>> round. Need skip the >>>>>>> FMP >>>>>>>>> capsules with non zero EmbeddedDriverCount. >>>>>>>>> - FALSE: Process >>>>> rest FMP capsules. >>>>>>>>> + @param[in] FirstRound Whether this is >>>>> the first invocation >>>>>>>>> + @param[in] LastRound Whether this is >>>>> the last invocation >>>>>>>>> + FALSE: First >>>>> of 2 rounds. Need skip >>>>>>> the >>>>>>>>> FMP >>>>>>>>> + >>>>> capsules with non zero >>>>>>>>> EmbeddedDriverCount. >>>>>>>>> + TRUE: Process >>>>> rest FMP capsules. >>>>>>>>>=20 >>>>>>>>> @retval EFI_SUCCESS There is no >>>>> error when processing >>>>>>>>> capsules. >>>>>>>>> @retval EFI_OUT_OF_RESOURCES No enough >>>>> resource to process >>>>>>>>> capsules. >>>>>>>>> @@ -373,7 +377,8 @@ >>>>> PopulateCapsuleInConfigurationTable ( >>>>>>>>> **/ >>>>>>>>> EFI_STATUS >>>>>>>>> ProcessTheseCapsules ( >>>>>>>>> - IN BOOLEAN FirstRound >>>>>>>>> + IN BOOLEAN FirstRound, >>>>>>>>> + IN BOOLEAN LastRound >>>>>>>>> ) >>>>>>>>> { >>>>>>>>> EFI_STATUS Status; >>>>>>>>> @@ -453,7 +458,7 @@ ProcessTheseCapsules ( >>>>>>>>> continue; >>>>>>>>> } >>>>>>>>>=20 >>>>>>>>> - if ((!FirstRound) || (EmbeddedDriverCount >>>>> =3D=3D 0)) { >>>>>>>>> + if (LastRound || (EmbeddedDriverCount =3D=3D >>>>> 0)) { >>>>>>>>> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - >>>>> 0x%x\n", >>>>>>>>> CapsuleHeader)); >>>>>>>>> Status =3D ProcessCapsuleImage >>>>> (CapsuleHeader); >>>>>>>>> mCapsuleStatusArray [Index] =3D Status; >>>>>>>>> @@ -546,7 +551,7 @@ ProcessCapsules ( >>>>>>>>> EFI_STATUS Status; >>>>>>>>>=20 >>>>>>>>> if (!mDxeCapsuleLibEndOfDxe) { >>>>>>>>> - Status =3D ProcessTheseCapsules(TRUE); >>>>>>>>> + Status =3D ProcessTheseCapsules(TRUE, FALSE); >>>>>>>>>=20 >>>>>>>>> // >>>>>>>>> // Reboot System if and only if all capsule >>>>> processed. >>>>>>>>> @@ -555,8 +560,9 @@ ProcessCapsules ( >>>>>>>>> if (mNeedReset && AreAllImagesProcessed()) { >>>>>>>>> DoResetSystem(); >>>>>>>>> } >>>>>>>>> + mFirstRound =3D FALSE; >>>>>>>>> } else { >>>>>>>>> - Status =3D ProcessTheseCapsules(FALSE); >>>>>>>>> + Status =3D ProcessTheseCapsules(mFirstRound, >>>>> TRUE); >>>>>>>>> // >>>>>>>>> // Reboot System if required after all >>>>> capsule processed >>>>>>>>> // >>>>>>>>> -- >>>>>>>>> 2.17.0 >>>>>>>>>=20 >>>>>>>>> _______________________________________________ >>>>>>>>> edk2-devel mailing list >>>>>>>>> edk2-devel@lists.01.org >>>>>>>>> https://lists.01.org/mailman/listinfo/edk2-devel >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel