| Message ID | 20180608065811.2065-3-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | MdeModulePkg ArmPkg: support for persistent capsules and progress reporting | expand |
Hi Ard We don't allow platform to update system firmware *after* EndOfDxe. According to PI spec, after EndOfDxe, 3rd part code start running. It brings security risk if we allow system firmware after EndOfDxe. 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. Would you please share the info, why your platform need update system firmware after EndOfDxe? Is that possible to make it earlier? Thank you Yao Jiewen > -----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 <michael.d.kinney@intel.com>; Yao, Jiewen > <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; > leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org> > Subject: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit > ProcessCapsules () to be called once > > 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. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 18 > ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > index 26ca4e295f20..ad83660f1737 100644 > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > @@ -100,6 +100,7 @@ IsValidCapsuleHeader ( > > extern BOOLEAN mDxeCapsuleLibEndOfDxe; > BOOLEAN mNeedReset; > +BOOLEAN mFirstRound = TRUE; > > VOID **mCapsulePtr; > EFI_STATUS *mCapsuleStatusArray; > @@ -364,8 +365,11 @@ PopulateCapsuleInConfigurationTable ( > > Each individual capsule result is recorded in capsule record variable. > > - @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. > > @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; > } > > - if ((!FirstRound) || (EmbeddedDriverCount == 0)) { > + if (LastRound || (EmbeddedDriverCount == 0)) { > DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", > CapsuleHeader)); > Status = ProcessCapsuleImage (CapsuleHeader); > mCapsuleStatusArray [Index] = Status; > @@ -546,7 +551,7 @@ ProcessCapsules ( > EFI_STATUS Status; > > if (!mDxeCapsuleLibEndOfDxe) { > - Status = ProcessTheseCapsules(TRUE); > + Status = ProcessTheseCapsules(TRUE, FALSE); > > // > // Reboot System if and only if all capsule processed. > @@ -555,8 +560,9 @@ ProcessCapsules ( > if (mNeedReset && AreAllImagesProcessed()) { > DoResetSystem(); > } > + mFirstRound = FALSE; > } else { > - Status = ProcessTheseCapsules(FALSE); > + Status = ProcessTheseCapsules(mFirstRound, TRUE); > // > // Reboot System if required after all capsule processed > // > -- > 2.17.0 > > _______________________________________________ > 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
> On 8 Jun 2018, at 14:34, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > Hi Ard > We don't allow platform to update system firmware *after* EndOfDxe. > > According to PI spec, after EndOfDxe, 3rd part code start running. It brings security risk if we allow system firmware after EndOfDxe. > > 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. > > Would you please share the info, why your platform need update system firmware after EndOfDxe? > Is that possible to make it earlier? > > Because we need some kind of console to report progress to the user. The secure platform execution context is completely separate from UEFI on Arm, so for us the distinction does not make sense. > Thank you > Yao Jiewen > >> -----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 <michael.d.kinney@intel.com>; Yao, Jiewen >> <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; >> leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Subject: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit >> ProcessCapsules () to be called once >> >> 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. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 18 >> ++++++++++++------ >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c >> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c >> index 26ca4e295f20..ad83660f1737 100644 >> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c >> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c >> @@ -100,6 +100,7 @@ IsValidCapsuleHeader ( >> >> extern BOOLEAN mDxeCapsuleLibEndOfDxe; >> BOOLEAN mNeedReset; >> +BOOLEAN mFirstRound = TRUE; >> >> VOID **mCapsulePtr; >> EFI_STATUS *mCapsuleStatusArray; >> @@ -364,8 +365,11 @@ PopulateCapsuleInConfigurationTable ( >> >> Each individual capsule result is recorded in capsule record variable. >> >> - @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. >> >> @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; >> } >> >> - if ((!FirstRound) || (EmbeddedDriverCount == 0)) { >> + if (LastRound || (EmbeddedDriverCount == 0)) { >> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", >> CapsuleHeader)); >> Status = ProcessCapsuleImage (CapsuleHeader); >> mCapsuleStatusArray [Index] = Status; >> @@ -546,7 +551,7 @@ ProcessCapsules ( >> EFI_STATUS Status; >> >> if (!mDxeCapsuleLibEndOfDxe) { >> - Status = ProcessTheseCapsules(TRUE); >> + Status = ProcessTheseCapsules(TRUE, FALSE); >> >> // >> // Reboot System if and only if all capsule processed. >> @@ -555,8 +560,9 @@ ProcessCapsules ( >> if (mNeedReset && AreAllImagesProcessed()) { >> DoResetSystem(); >> } >> + mFirstRound = FALSE; >> } else { >> - Status = ProcessTheseCapsules(FALSE); >> + Status = ProcessTheseCapsules(mFirstRound, TRUE); >> // >> // Reboot System if required after all capsule processed >> // >> -- >> 2.17.0 >> >> _______________________________________________ >> 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
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" In brief, EndOfDxe is signaled before 3rd part code running. As such, it is legal that a trusted console is connected before EndOfDxe. You can report progress to user before EndOfDxe. Thank you Yao Jiewen > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Friday, June 8, 2018 8:38 AM > To: Yao, Jiewen <jiewen.yao@intel.com> > Cc: edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>; > Zeng, Star <star.zeng@intel.com>; leif.lindholm@linaro.org > Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit > ProcessCapsules () to be called once > > > > > On 8 Jun 2018, at 14:34, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > > > Hi Ard > > We don't allow platform to update system firmware *after* EndOfDxe. > > > > According to PI spec, after EndOfDxe, 3rd part code start running. It brings > security risk if we allow system firmware after EndOfDxe. > > > > 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. > > > > Would you please share the info, why your platform need update system > firmware after EndOfDxe? > > Is that possible to make it earlier? > > > > > > Because we need some kind of console to report progress to the user. > > The secure platform execution context is completely separate from UEFI on Arm, > so for us the distinction does not make sense. > > > Thank you > > Yao Jiewen > > > >> -----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 <michael.d.kinney@intel.com>; Yao, Jiewen > >> <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; > >> leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> Subject: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit > >> ProcessCapsules () to be called once > >> > >> 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. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> --- > >> MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 18 > >> ++++++++++++------ > >> 1 file changed, 12 insertions(+), 6 deletions(-) > >> > >> diff --git > a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > >> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > >> index 26ca4e295f20..ad83660f1737 100644 > >> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > >> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > >> @@ -100,6 +100,7 @@ IsValidCapsuleHeader ( > >> > >> extern BOOLEAN mDxeCapsuleLibEndOfDxe; > >> BOOLEAN mNeedReset; > >> +BOOLEAN mFirstRound = TRUE; > >> > >> VOID **mCapsulePtr; > >> EFI_STATUS *mCapsuleStatusArray; > >> @@ -364,8 +365,11 @@ PopulateCapsuleInConfigurationTable ( > >> > >> Each individual capsule result is recorded in capsule record variable. > >> > >> - @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. > >> > >> @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; > >> } > >> > >> - if ((!FirstRound) || (EmbeddedDriverCount == 0)) { > >> + if (LastRound || (EmbeddedDriverCount == 0)) { > >> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", > >> CapsuleHeader)); > >> Status = ProcessCapsuleImage (CapsuleHeader); > >> mCapsuleStatusArray [Index] = Status; > >> @@ -546,7 +551,7 @@ ProcessCapsules ( > >> EFI_STATUS Status; > >> > >> if (!mDxeCapsuleLibEndOfDxe) { > >> - Status = ProcessTheseCapsules(TRUE); > >> + Status = ProcessTheseCapsules(TRUE, FALSE); > >> > >> // > >> // Reboot System if and only if all capsule processed. > >> @@ -555,8 +560,9 @@ ProcessCapsules ( > >> if (mNeedReset && AreAllImagesProcessed()) { > >> DoResetSystem(); > >> } > >> + mFirstRound = FALSE; > >> } else { > >> - Status = ProcessTheseCapsules(FALSE); > >> + Status = ProcessTheseCapsules(mFirstRound, TRUE); > >> // > >> // Reboot System if required after all capsule processed > >> // > >> -- > >> 2.17.0 > >> > >> _______________________________________________ > >> 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
On 10 June 2018 at 07:38, Yao, Jiewen <jiewen.yao@intel.com> 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" > > In brief, EndOfDxe is signaled before 3rd part code running. > > As such, it is legal that a trusted console is connected before EndOfDxe. > You can report progress to user before EndOfDxe. > 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? >> -----Original Message----- >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> Sent: Friday, June 8, 2018 8:38 AM >> To: Yao, Jiewen <jiewen.yao@intel.com> >> Cc: edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>; >> Zeng, Star <star.zeng@intel.com>; leif.lindholm@linaro.org >> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit >> ProcessCapsules () to be called once >> >> >> >> > On 8 Jun 2018, at 14:34, Yao, Jiewen <jiewen.yao@intel.com> wrote: >> > >> > Hi Ard >> > We don't allow platform to update system firmware *after* EndOfDxe. >> > >> > According to PI spec, after EndOfDxe, 3rd part code start running. It brings >> security risk if we allow system firmware after EndOfDxe. >> > >> > 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. >> > >> > Would you please share the info, why your platform need update system >> firmware after EndOfDxe? >> > Is that possible to make it earlier? >> > >> > >> >> Because we need some kind of console to report progress to the user. >> >> The secure platform execution context is completely separate from UEFI on Arm, >> so for us the distinction does not make sense. >> >> > Thank you >> > Yao Jiewen >> > >> >> -----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 <michael.d.kinney@intel.com>; Yao, Jiewen >> >> <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; >> >> leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> Subject: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit >> >> ProcessCapsules () to be called once >> >> >> >> 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. >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> --- >> >> MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 18 >> >> ++++++++++++------ >> >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> >> >> diff --git >> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c >> >> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c >> >> index 26ca4e295f20..ad83660f1737 100644 >> >> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c >> >> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c >> >> @@ -100,6 +100,7 @@ IsValidCapsuleHeader ( >> >> >> >> extern BOOLEAN mDxeCapsuleLibEndOfDxe; >> >> BOOLEAN mNeedReset; >> >> +BOOLEAN mFirstRound = TRUE; >> >> >> >> VOID **mCapsulePtr; >> >> EFI_STATUS *mCapsuleStatusArray; >> >> @@ -364,8 +365,11 @@ PopulateCapsuleInConfigurationTable ( >> >> >> >> Each individual capsule result is recorded in capsule record variable. >> >> >> >> - @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. >> >> >> >> @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; >> >> } >> >> >> >> - if ((!FirstRound) || (EmbeddedDriverCount == 0)) { >> >> + if (LastRound || (EmbeddedDriverCount == 0)) { >> >> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", >> >> CapsuleHeader)); >> >> Status = ProcessCapsuleImage (CapsuleHeader); >> >> mCapsuleStatusArray [Index] = Status; >> >> @@ -546,7 +551,7 @@ ProcessCapsules ( >> >> EFI_STATUS Status; >> >> >> >> if (!mDxeCapsuleLibEndOfDxe) { >> >> - Status = ProcessTheseCapsules(TRUE); >> >> + Status = ProcessTheseCapsules(TRUE, FALSE); >> >> >> >> // >> >> // Reboot System if and only if all capsule processed. >> >> @@ -555,8 +560,9 @@ ProcessCapsules ( >> >> if (mNeedReset && AreAllImagesProcessed()) { >> >> DoResetSystem(); >> >> } >> >> + mFirstRound = FALSE; >> >> } else { >> >> - Status = ProcessTheseCapsules(FALSE); >> >> + Status = ProcessTheseCapsules(mFirstRound, TRUE); >> >> // >> >> // Reboot System if required after all capsule processed >> >> // >> >> -- >> >> 2.17.0 >> >> >> >> _______________________________________________ >> >> 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
Ard, 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. 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. 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. 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. I will review your patch in more detail tomorrow. Thanks, Mike > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Saturday, June 9, 2018 10:42 PM > To: Yao, Jiewen <jiewen.yao@intel.com> > Cc: edk2-devel@lists.01.org; Kinney, Michael D > <michael.d.kinney@intel.com>; Zeng, Star > <star.zeng@intel.com>; leif.lindholm@linaro.org > Subject: Re: [edk2] [PATCH v2 2/5] > MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules > () to be called once > > On 10 June 2018 at 07:38, Yao, Jiewen > <jiewen.yao@intel.com> 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" > > > > In brief, EndOfDxe is signaled before 3rd part code > running. > > > > As such, it is legal that a trusted console is > connected before EndOfDxe. > > You can report progress to user before EndOfDxe. > > > > 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? > > > >> -----Original Message----- > >> From: Ard Biesheuvel > [mailto:ard.biesheuvel@linaro.org] > >> Sent: Friday, June 8, 2018 8:38 AM > >> To: Yao, Jiewen <jiewen.yao@intel.com> > >> Cc: edk2-devel@lists.01.org; Kinney, Michael D > <michael.d.kinney@intel.com>; > >> Zeng, Star <star.zeng@intel.com>; > leif.lindholm@linaro.org > >> Subject: Re: [edk2] [PATCH v2 2/5] > MdeModulePkg/DxeCapsuleLibFmp: permit > >> ProcessCapsules () to be called once > >> > >> > >> > >> > On 8 Jun 2018, at 14:34, Yao, Jiewen > <jiewen.yao@intel.com> wrote: > >> > > >> > Hi Ard > >> > We don't allow platform to update system firmware > *after* EndOfDxe. > >> > > >> > According to PI spec, after EndOfDxe, 3rd part > code start running. It brings > >> security risk if we allow system firmware after > EndOfDxe. > >> > > >> > 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. > >> > > >> > Would you please share the info, why your platform > need update system > >> firmware after EndOfDxe? > >> > Is that possible to make it earlier? > >> > > >> > > >> > >> Because we need some kind of console to report > progress to the user. > >> > >> The secure platform execution context is completely > separate from UEFI on Arm, > >> so for us the distinction does not make sense. > >> > >> > Thank you > >> > Yao Jiewen > >> > > >> >> -----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 > <michael.d.kinney@intel.com>; Yao, Jiewen > >> >> <jiewen.yao@intel.com>; Zeng, Star > <star.zeng@intel.com>; > >> >> leif.lindholm@linaro.org; Ard Biesheuvel > <ard.biesheuvel@linaro.org> > >> >> Subject: [edk2] [PATCH v2 2/5] > MdeModulePkg/DxeCapsuleLibFmp: permit > >> >> ProcessCapsules () to be called once > >> >> > >> >> 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. > >> >> > >> >> Contributed-under: TianoCore Contribution > Agreement 1.1 > >> >> Signed-off-by: Ard Biesheuvel > <ard.biesheuvel@linaro.org> > >> >> --- > >> >> > MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess > Lib.c | 18 > >> >> ++++++++++++------ > >> >> 1 file changed, 12 insertions(+), 6 deletions(-) > >> >> > >> >> diff --git > >> > a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce > ssLib.c > >> >> > 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 ( > >> >> > >> >> extern BOOLEAN > mDxeCapsuleLibEndOfDxe; > >> >> BOOLEAN mNeedReset; > >> >> +BOOLEAN mFirstRound = > TRUE; > >> >> > >> >> VOID **mCapsulePtr; > >> >> EFI_STATUS *mCapsuleStatusArray; > >> >> @@ -364,8 +365,11 @@ > PopulateCapsuleInConfigurationTable ( > >> >> > >> >> Each individual capsule result is recorded in > capsule record variable. > >> >> > >> >> - @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. > >> >> > >> >> @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; > >> >> } > >> >> > >> >> - if ((!FirstRound) || (EmbeddedDriverCount > == 0)) { > >> >> + if (LastRound || (EmbeddedDriverCount == > 0)) { > >> >> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - > 0x%x\n", > >> >> CapsuleHeader)); > >> >> Status = ProcessCapsuleImage > (CapsuleHeader); > >> >> mCapsuleStatusArray [Index] = Status; > >> >> @@ -546,7 +551,7 @@ ProcessCapsules ( > >> >> EFI_STATUS Status; > >> >> > >> >> if (!mDxeCapsuleLibEndOfDxe) { > >> >> - Status = ProcessTheseCapsules(TRUE); > >> >> + Status = ProcessTheseCapsules(TRUE, FALSE); > >> >> > >> >> // > >> >> // Reboot System if and only if all capsule > processed. > >> >> @@ -555,8 +560,9 @@ ProcessCapsules ( > >> >> if (mNeedReset && AreAllImagesProcessed()) { > >> >> DoResetSystem(); > >> >> } > >> >> + mFirstRound = FALSE; > >> >> } else { > >> >> - Status = ProcessTheseCapsules(FALSE); > >> >> + Status = ProcessTheseCapsules(mFirstRound, > TRUE); > >> >> // > >> >> // Reboot System if required after all > capsule processed > >> >> // > >> >> -- > >> >> 2.17.0 > >> >> > >> >> _______________________________________________ > >> >> 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
My concern is that *always allowing* processing SystemCapsule after EndOfDxe has security risk. IMHO, the risk is not *process*, if it is a platform choice. Instead, the risk is *allow*, in the core logic. If we really want to do that, I hope we need a way to distinguish the difference between a platform dispatching SystemCapsule after EndOfDxe *purposely* and a platform dispatching SystemCapsule after EndOfDxe *by mistake*. Maybe some policy enforcement in the core logic. Static policy, at build time. Thank you Yao Jiewen > -----Original Message----- > From: Kinney, Michael D > Sent: Sunday, June 10, 2018 8:57 AM > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Yao, Jiewen > <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> > Cc: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; > leif.lindholm@linaro.org > Subject: RE: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit > ProcessCapsules () to be called once > > Ard, > > 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. > > 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. > > 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. > > 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. > > I will review your patch in more detail tomorrow. > > Thanks, > > Mike > > > -----Original Message----- > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > Sent: Saturday, June 9, 2018 10:42 PM > > To: Yao, Jiewen <jiewen.yao@intel.com> > > Cc: edk2-devel@lists.01.org; Kinney, Michael D > > <michael.d.kinney@intel.com>; Zeng, Star > > <star.zeng@intel.com>; leif.lindholm@linaro.org > > Subject: Re: [edk2] [PATCH v2 2/5] > > MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules > > () to be called once > > > > On 10 June 2018 at 07:38, Yao, Jiewen > > <jiewen.yao@intel.com> 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" > > > > > > In brief, EndOfDxe is signaled before 3rd part code > > running. > > > > > > As such, it is legal that a trusted console is > > connected before EndOfDxe. > > > You can report progress to user before EndOfDxe. > > > > > > > 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? > > > > > > >> -----Original Message----- > > >> From: Ard Biesheuvel > > [mailto:ard.biesheuvel@linaro.org] > > >> Sent: Friday, June 8, 2018 8:38 AM > > >> To: Yao, Jiewen <jiewen.yao@intel.com> > > >> Cc: edk2-devel@lists.01.org; Kinney, Michael D > > <michael.d.kinney@intel.com>; > > >> Zeng, Star <star.zeng@intel.com>; > > leif.lindholm@linaro.org > > >> Subject: Re: [edk2] [PATCH v2 2/5] > > MdeModulePkg/DxeCapsuleLibFmp: permit > > >> ProcessCapsules () to be called once > > >> > > >> > > >> > > >> > On 8 Jun 2018, at 14:34, Yao, Jiewen > > <jiewen.yao@intel.com> wrote: > > >> > > > >> > Hi Ard > > >> > We don't allow platform to update system firmware > > *after* EndOfDxe. > > >> > > > >> > According to PI spec, after EndOfDxe, 3rd part > > code start running. It brings > > >> security risk if we allow system firmware after > > EndOfDxe. > > >> > > > >> > 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. > > >> > > > >> > Would you please share the info, why your platform > > need update system > > >> firmware after EndOfDxe? > > >> > Is that possible to make it earlier? > > >> > > > >> > > > >> > > >> Because we need some kind of console to report > > progress to the user. > > >> > > >> The secure platform execution context is completely > > separate from UEFI on Arm, > > >> so for us the distinction does not make sense. > > >> > > >> > Thank you > > >> > Yao Jiewen > > >> > > > >> >> -----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 > > <michael.d.kinney@intel.com>; Yao, Jiewen > > >> >> <jiewen.yao@intel.com>; Zeng, Star > > <star.zeng@intel.com>; > > >> >> leif.lindholm@linaro.org; Ard Biesheuvel > > <ard.biesheuvel@linaro.org> > > >> >> Subject: [edk2] [PATCH v2 2/5] > > MdeModulePkg/DxeCapsuleLibFmp: permit > > >> >> ProcessCapsules () to be called once > > >> >> > > >> >> 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. > > >> >> > > >> >> Contributed-under: TianoCore Contribution > > Agreement 1.1 > > >> >> Signed-off-by: Ard Biesheuvel > > <ard.biesheuvel@linaro.org> > > >> >> --- > > >> >> > > MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess > > Lib.c | 18 > > >> >> ++++++++++++------ > > >> >> 1 file changed, 12 insertions(+), 6 deletions(-) > > >> >> > > >> >> diff --git > > >> > > a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce > > ssLib.c > > >> >> > > 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 ( > > >> >> > > >> >> extern BOOLEAN > > mDxeCapsuleLibEndOfDxe; > > >> >> BOOLEAN mNeedReset; > > >> >> +BOOLEAN mFirstRound = > > TRUE; > > >> >> > > >> >> VOID **mCapsulePtr; > > >> >> EFI_STATUS *mCapsuleStatusArray; > > >> >> @@ -364,8 +365,11 @@ > > PopulateCapsuleInConfigurationTable ( > > >> >> > > >> >> Each individual capsule result is recorded in > > capsule record variable. > > >> >> > > >> >> - @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. > > >> >> > > >> >> @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; > > >> >> } > > >> >> > > >> >> - if ((!FirstRound) || (EmbeddedDriverCount > > == 0)) { > > >> >> + if (LastRound || (EmbeddedDriverCount == > > 0)) { > > >> >> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - > > 0x%x\n", > > >> >> CapsuleHeader)); > > >> >> Status = ProcessCapsuleImage > > (CapsuleHeader); > > >> >> mCapsuleStatusArray [Index] = Status; > > >> >> @@ -546,7 +551,7 @@ ProcessCapsules ( > > >> >> EFI_STATUS Status; > > >> >> > > >> >> if (!mDxeCapsuleLibEndOfDxe) { > > >> >> - Status = ProcessTheseCapsules(TRUE); > > >> >> + Status = ProcessTheseCapsules(TRUE, FALSE); > > >> >> > > >> >> // > > >> >> // Reboot System if and only if all capsule > > processed. > > >> >> @@ -555,8 +560,9 @@ ProcessCapsules ( > > >> >> if (mNeedReset && AreAllImagesProcessed()) { > > >> >> DoResetSystem(); > > >> >> } > > >> >> + mFirstRound = FALSE; > > >> >> } else { > > >> >> - Status = ProcessTheseCapsules(FALSE); > > >> >> + Status = ProcessTheseCapsules(mFirstRound, > > TRUE); > > >> >> // > > >> >> // Reboot System if required after all > > capsule processed > > >> >> // > > >> >> -- > > >> >> 2.17.0 > > >> >> > > >> >> _______________________________________________ > > >> >> 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
Just got some idea since I am also reviewing FmpDevicePkg patch. The key problem we are trying to resolve that is: The core code uses EndOfDxe as the lock event for system firmware, but an ARM platform may want to lock system firmware later, maybe in other lock event. As such, how about we generalize the lock event as PcdSystemFmpLockEventGuid? We can use EndOfDxeGuid as default one to keep the compatibility. At same time this brings the flexibility for platform overriding. We only need update DxeCapsuleLibConstructor() to use the new PcdGuid, instead of hardcode EndOfDxe. We don't need update the logic in ProcessCapsules() and ProcessTheseCapsules(). 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 configure another lock event GUID explicitly in platform DSC. Thank you Yao Jiewen > -----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 <michael.d.kinney@intel.com>; Ard Biesheuvel > <ard.biesheuvel@linaro.org> > Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star > <star.zeng@intel.com> > Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit > ProcessCapsules () to be called once > > My concern is that *always allowing* processing SystemCapsule after EndOfDxe > has security risk. > > IMHO, the risk is not *process*, if it is a platform choice. Instead, the risk is > *allow*, in the core logic. > > If we really want to do that, I hope we need a way to distinguish the difference > between a platform dispatching SystemCapsule after EndOfDxe *purposely* and > a platform dispatching SystemCapsule after EndOfDxe *by mistake*. > > Maybe some policy enforcement in the core logic. Static policy, at build time. > > Thank you > Yao Jiewen > > > -----Original Message----- > > From: Kinney, Michael D > > Sent: Sunday, June 10, 2018 8:57 AM > > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Yao, Jiewen > > <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> > > Cc: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; > > leif.lindholm@linaro.org > > Subject: RE: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit > > ProcessCapsules () to be called once > > > > Ard, > > > > 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. > > > > 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. > > > > 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. > > > > 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. > > > > I will review your patch in more detail tomorrow. > > > > Thanks, > > > > Mike > > > > > -----Original Message----- > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > > Sent: Saturday, June 9, 2018 10:42 PM > > > To: Yao, Jiewen <jiewen.yao@intel.com> > > > Cc: edk2-devel@lists.01.org; Kinney, Michael D > > > <michael.d.kinney@intel.com>; Zeng, Star > > > <star.zeng@intel.com>; leif.lindholm@linaro.org > > > Subject: Re: [edk2] [PATCH v2 2/5] > > > MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules > > > () to be called once > > > > > > On 10 June 2018 at 07:38, Yao, Jiewen > > > <jiewen.yao@intel.com> 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" > > > > > > > > In brief, EndOfDxe is signaled before 3rd part code > > > running. > > > > > > > > As such, it is legal that a trusted console is > > > connected before EndOfDxe. > > > > You can report progress to user before EndOfDxe. > > > > > > > > > > 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? > > > > > > > > > >> -----Original Message----- > > > >> From: Ard Biesheuvel > > > [mailto:ard.biesheuvel@linaro.org] > > > >> Sent: Friday, June 8, 2018 8:38 AM > > > >> To: Yao, Jiewen <jiewen.yao@intel.com> > > > >> Cc: edk2-devel@lists.01.org; Kinney, Michael D > > > <michael.d.kinney@intel.com>; > > > >> Zeng, Star <star.zeng@intel.com>; > > > leif.lindholm@linaro.org > > > >> Subject: Re: [edk2] [PATCH v2 2/5] > > > MdeModulePkg/DxeCapsuleLibFmp: permit > > > >> ProcessCapsules () to be called once > > > >> > > > >> > > > >> > > > >> > On 8 Jun 2018, at 14:34, Yao, Jiewen > > > <jiewen.yao@intel.com> wrote: > > > >> > > > > >> > Hi Ard > > > >> > We don't allow platform to update system firmware > > > *after* EndOfDxe. > > > >> > > > > >> > According to PI spec, after EndOfDxe, 3rd part > > > code start running. It brings > > > >> security risk if we allow system firmware after > > > EndOfDxe. > > > >> > > > > >> > 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. > > > >> > > > > >> > Would you please share the info, why your platform > > > need update system > > > >> firmware after EndOfDxe? > > > >> > Is that possible to make it earlier? > > > >> > > > > >> > > > > >> > > > >> Because we need some kind of console to report > > > progress to the user. > > > >> > > > >> The secure platform execution context is completely > > > separate from UEFI on Arm, > > > >> so for us the distinction does not make sense. > > > >> > > > >> > Thank you > > > >> > Yao Jiewen > > > >> > > > > >> >> -----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 > > > <michael.d.kinney@intel.com>; Yao, Jiewen > > > >> >> <jiewen.yao@intel.com>; Zeng, Star > > > <star.zeng@intel.com>; > > > >> >> leif.lindholm@linaro.org; Ard Biesheuvel > > > <ard.biesheuvel@linaro.org> > > > >> >> Subject: [edk2] [PATCH v2 2/5] > > > MdeModulePkg/DxeCapsuleLibFmp: permit > > > >> >> ProcessCapsules () to be called once > > > >> >> > > > >> >> 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. > > > >> >> > > > >> >> Contributed-under: TianoCore Contribution > > > Agreement 1.1 > > > >> >> Signed-off-by: Ard Biesheuvel > > > <ard.biesheuvel@linaro.org> > > > >> >> --- > > > >> >> > > > MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess > > > Lib.c | 18 > > > >> >> ++++++++++++------ > > > >> >> 1 file changed, 12 insertions(+), 6 deletions(-) > > > >> >> > > > >> >> diff --git > > > >> > > > a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce > > > ssLib.c > > > >> >> > > > 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 ( > > > >> >> > > > >> >> extern BOOLEAN > > > mDxeCapsuleLibEndOfDxe; > > > >> >> BOOLEAN mNeedReset; > > > >> >> +BOOLEAN mFirstRound = > > > TRUE; > > > >> >> > > > >> >> VOID **mCapsulePtr; > > > >> >> EFI_STATUS *mCapsuleStatusArray; > > > >> >> @@ -364,8 +365,11 @@ > > > PopulateCapsuleInConfigurationTable ( > > > >> >> > > > >> >> Each individual capsule result is recorded in > > > capsule record variable. > > > >> >> > > > >> >> - @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. > > > >> >> > > > >> >> @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; > > > >> >> } > > > >> >> > > > >> >> - if ((!FirstRound) || (EmbeddedDriverCount > > > == 0)) { > > > >> >> + if (LastRound || (EmbeddedDriverCount == > > > 0)) { > > > >> >> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - > > > 0x%x\n", > > > >> >> CapsuleHeader)); > > > >> >> Status = ProcessCapsuleImage > > > (CapsuleHeader); > > > >> >> mCapsuleStatusArray [Index] = Status; > > > >> >> @@ -546,7 +551,7 @@ ProcessCapsules ( > > > >> >> EFI_STATUS Status; > > > >> >> > > > >> >> if (!mDxeCapsuleLibEndOfDxe) { > > > >> >> - Status = ProcessTheseCapsules(TRUE); > > > >> >> + Status = ProcessTheseCapsules(TRUE, FALSE); > > > >> >> > > > >> >> // > > > >> >> // Reboot System if and only if all capsule > > > processed. > > > >> >> @@ -555,8 +560,9 @@ ProcessCapsules ( > > > >> >> if (mNeedReset && AreAllImagesProcessed()) { > > > >> >> DoResetSystem(); > > > >> >> } > > > >> >> + mFirstRound = FALSE; > > > >> >> } else { > > > >> >> - Status = ProcessTheseCapsules(FALSE); > > > >> >> + Status = ProcessTheseCapsules(mFirstRound, > > > TRUE); > > > >> >> // > > > >> >> // Reboot System if required after all > > > capsule processed > > > >> >> // > > > >> >> -- > > > >> >> 2.17.0 > > > >> >> > > > >> >> _______________________________________________ > > > >> >> 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 10 June 2018 at 21:21, Yao, Jiewen <jiewen.yao@intel.com> wrote: > Just got some idea since I am also reviewing FmpDevicePkg patch. > > > The key problem we are trying to resolve that is: The core code uses EndOfDxe as the lock event for system firmware, but an ARM platform may want to lock system firmware later, maybe in other lock event. > > As such, how about we generalize the lock event as PcdSystemFmpLockEventGuid? We can use EndOfDxeGuid as default one to keep the compatibility. At same time this brings the flexibility for platform overriding. > > > We only need update DxeCapsuleLibConstructor() to use the new PcdGuid, instead of hardcode EndOfDxe. > We don't need update the logic in ProcessCapsules() and ProcessTheseCapsules(). > 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 configure another lock event GUID explicitly in platform DSC. > But that means we are still required to call ProcessCapsules () twice, no? >> -----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 <michael.d.kinney@intel.com>; Ard Biesheuvel >> <ard.biesheuvel@linaro.org> >> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star >> <star.zeng@intel.com> >> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit >> ProcessCapsules () to be called once >> >> My concern is that *always allowing* processing SystemCapsule after EndOfDxe >> has security risk. >> >> IMHO, the risk is not *process*, if it is a platform choice. Instead, the risk is >> *allow*, in the core logic. >> >> If we really want to do that, I hope we need a way to distinguish the difference >> between a platform dispatching SystemCapsule after EndOfDxe *purposely* and >> a platform dispatching SystemCapsule after EndOfDxe *by mistake*. >> >> Maybe some policy enforcement in the core logic. Static policy, at build time. >> >> Thank you >> Yao Jiewen >> >> > -----Original Message----- >> > From: Kinney, Michael D >> > Sent: Sunday, June 10, 2018 8:57 AM >> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Yao, Jiewen >> > <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> >> > Cc: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; >> > leif.lindholm@linaro.org >> > Subject: RE: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit >> > ProcessCapsules () to be called once >> > >> > Ard, >> > >> > 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. >> > >> > 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. >> > >> > 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. >> > >> > 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. >> > >> > I will review your patch in more detail tomorrow. >> > >> > Thanks, >> > >> > Mike >> > >> > > -----Original Message----- >> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> > > Sent: Saturday, June 9, 2018 10:42 PM >> > > To: Yao, Jiewen <jiewen.yao@intel.com> >> > > Cc: edk2-devel@lists.01.org; Kinney, Michael D >> > > <michael.d.kinney@intel.com>; Zeng, Star >> > > <star.zeng@intel.com>; leif.lindholm@linaro.org >> > > Subject: Re: [edk2] [PATCH v2 2/5] >> > > MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules >> > > () to be called once >> > > >> > > On 10 June 2018 at 07:38, Yao, Jiewen >> > > <jiewen.yao@intel.com> 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" >> > > > >> > > > In brief, EndOfDxe is signaled before 3rd part code >> > > running. >> > > > >> > > > As such, it is legal that a trusted console is >> > > connected before EndOfDxe. >> > > > You can report progress to user before EndOfDxe. >> > > > >> > > >> > > 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? >> > > >> > > >> > > >> -----Original Message----- >> > > >> From: Ard Biesheuvel >> > > [mailto:ard.biesheuvel@linaro.org] >> > > >> Sent: Friday, June 8, 2018 8:38 AM >> > > >> To: Yao, Jiewen <jiewen.yao@intel.com> >> > > >> Cc: edk2-devel@lists.01.org; Kinney, Michael D >> > > <michael.d.kinney@intel.com>; >> > > >> Zeng, Star <star.zeng@intel.com>; >> > > leif.lindholm@linaro.org >> > > >> Subject: Re: [edk2] [PATCH v2 2/5] >> > > MdeModulePkg/DxeCapsuleLibFmp: permit >> > > >> ProcessCapsules () to be called once >> > > >> >> > > >> >> > > >> >> > > >> > On 8 Jun 2018, at 14:34, Yao, Jiewen >> > > <jiewen.yao@intel.com> wrote: >> > > >> > >> > > >> > Hi Ard >> > > >> > We don't allow platform to update system firmware >> > > *after* EndOfDxe. >> > > >> > >> > > >> > According to PI spec, after EndOfDxe, 3rd part >> > > code start running. It brings >> > > >> security risk if we allow system firmware after >> > > EndOfDxe. >> > > >> > >> > > >> > 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. >> > > >> > >> > > >> > Would you please share the info, why your platform >> > > need update system >> > > >> firmware after EndOfDxe? >> > > >> > Is that possible to make it earlier? >> > > >> > >> > > >> > >> > > >> >> > > >> Because we need some kind of console to report >> > > progress to the user. >> > > >> >> > > >> The secure platform execution context is completely >> > > separate from UEFI on Arm, >> > > >> so for us the distinction does not make sense. >> > > >> >> > > >> > Thank you >> > > >> > Yao Jiewen >> > > >> > >> > > >> >> -----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 >> > > <michael.d.kinney@intel.com>; Yao, Jiewen >> > > >> >> <jiewen.yao@intel.com>; Zeng, Star >> > > <star.zeng@intel.com>; >> > > >> >> leif.lindholm@linaro.org; Ard Biesheuvel >> > > <ard.biesheuvel@linaro.org> >> > > >> >> Subject: [edk2] [PATCH v2 2/5] >> > > MdeModulePkg/DxeCapsuleLibFmp: permit >> > > >> >> ProcessCapsules () to be called once >> > > >> >> >> > > >> >> 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. >> > > >> >> >> > > >> >> Contributed-under: TianoCore Contribution >> > > Agreement 1.1 >> > > >> >> Signed-off-by: Ard Biesheuvel >> > > <ard.biesheuvel@linaro.org> >> > > >> >> --- >> > > >> >> >> > > MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess >> > > Lib.c | 18 >> > > >> >> ++++++++++++------ >> > > >> >> 1 file changed, 12 insertions(+), 6 deletions(-) >> > > >> >> >> > > >> >> diff --git >> > > >> >> > > a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce >> > > ssLib.c >> > > >> >> >> > > 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 ( >> > > >> >> >> > > >> >> extern BOOLEAN >> > > mDxeCapsuleLibEndOfDxe; >> > > >> >> BOOLEAN mNeedReset; >> > > >> >> +BOOLEAN mFirstRound = >> > > TRUE; >> > > >> >> >> > > >> >> VOID **mCapsulePtr; >> > > >> >> EFI_STATUS *mCapsuleStatusArray; >> > > >> >> @@ -364,8 +365,11 @@ >> > > PopulateCapsuleInConfigurationTable ( >> > > >> >> >> > > >> >> Each individual capsule result is recorded in >> > > capsule record variable. >> > > >> >> >> > > >> >> - @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. >> > > >> >> >> > > >> >> @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; >> > > >> >> } >> > > >> >> >> > > >> >> - if ((!FirstRound) || (EmbeddedDriverCount >> > > == 0)) { >> > > >> >> + if (LastRound || (EmbeddedDriverCount == >> > > 0)) { >> > > >> >> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - >> > > 0x%x\n", >> > > >> >> CapsuleHeader)); >> > > >> >> Status = ProcessCapsuleImage >> > > (CapsuleHeader); >> > > >> >> mCapsuleStatusArray [Index] = Status; >> > > >> >> @@ -546,7 +551,7 @@ ProcessCapsules ( >> > > >> >> EFI_STATUS Status; >> > > >> >> >> > > >> >> if (!mDxeCapsuleLibEndOfDxe) { >> > > >> >> - Status = ProcessTheseCapsules(TRUE); >> > > >> >> + Status = ProcessTheseCapsules(TRUE, FALSE); >> > > >> >> >> > > >> >> // >> > > >> >> // Reboot System if and only if all capsule >> > > processed. >> > > >> >> @@ -555,8 +560,9 @@ ProcessCapsules ( >> > > >> >> if (mNeedReset && AreAllImagesProcessed()) { >> > > >> >> DoResetSystem(); >> > > >> >> } >> > > >> >> + mFirstRound = FALSE; >> > > >> >> } else { >> > > >> >> - Status = ProcessTheseCapsules(FALSE); >> > > >> >> + Status = ProcessTheseCapsules(mFirstRound, >> > > TRUE); >> > > >> >> // >> > > >> >> // Reboot System if required after all >> > > capsule processed >> > > >> >> // >> > > >> >> -- >> > > >> >> 2.17.0 >> > > >> >> >> > > >> >> _______________________________________________ >> > > >> >> 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
If all fmp can be processed one time，you just need call once. Then system reset. thank you! Yao, Jiewen > 在 2018年6月11日，上午12:27，Ard Biesheuvel <ard.biesheuvel@linaro.org> 写道： > >> On 10 June 2018 at 21:21, Yao, Jiewen <jiewen.yao@intel.com> wrote: >> Just got some idea since I am also reviewing FmpDevicePkg patch. >> >> >> The key problem we are trying to resolve that is: The core code uses EndOfDxe as the lock event for system firmware, but an ARM platform may want to lock system firmware later, maybe in other lock event. >> >> As such, how about we generalize the lock event as PcdSystemFmpLockEventGuid? We can use EndOfDxeGuid as default one to keep the compatibility. At same time this brings the flexibility for platform overriding. >> >> >> We only need update DxeCapsuleLibConstructor() to use the new PcdGuid, instead of hardcode EndOfDxe. >> We don't need update the logic in ProcessCapsules() and ProcessTheseCapsules(). >> 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 configure another lock event GUID explicitly in platform DSC. >> > > But that means we are still required to call ProcessCapsules () twice, no? > >>> -----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 <michael.d.kinney@intel.com>; Ard Biesheuvel >>> <ard.biesheuvel@linaro.org> >>> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star >>> <star.zeng@intel.com> >>> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit >>> ProcessCapsules () to be called once >>> >>> My concern is that *always allowing* processing SystemCapsule after EndOfDxe >>> has security risk. >>> >>> IMHO, the risk is not *process*, if it is a platform choice. Instead, the risk is >>> *allow*, in the core logic. >>> >>> If we really want to do that, I hope we need a way to distinguish the difference >>> between a platform dispatching SystemCapsule after EndOfDxe *purposely* and >>> a platform dispatching SystemCapsule after EndOfDxe *by mistake*. >>> >>> Maybe some policy enforcement in the core logic. Static policy, at build time. >>> >>> Thank you >>> Yao Jiewen >>> >>>> -----Original Message----- >>>> From: Kinney, Michael D >>>> Sent: Sunday, June 10, 2018 8:57 AM >>>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Yao, Jiewen >>>> <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> >>>> Cc: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; >>>> leif.lindholm@linaro.org >>>> Subject: RE: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit >>>> ProcessCapsules () to be called once >>>> >>>> Ard, >>>> >>>> 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. >>>> >>>> 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. >>>> >>>> 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. >>>> >>>> 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. >>>> >>>> I will review your patch in more detail tomorrow. >>>> >>>> Thanks, >>>> >>>> Mike >>>> >>>>> -----Original Message----- >>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >>>>> Sent: Saturday, June 9, 2018 10:42 PM >>>>> To: Yao, Jiewen <jiewen.yao@intel.com> >>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D >>>>> <michael.d.kinney@intel.com>; Zeng, Star >>>>> <star.zeng@intel.com>; leif.lindholm@linaro.org >>>>> Subject: Re: [edk2] [PATCH v2 2/5] >>>>> MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules >>>>> () to be called once >>>>> >>>>> On 10 June 2018 at 07:38, Yao, Jiewen >>>>> <jiewen.yao@intel.com> 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" >>>>>> >>>>>> In brief, EndOfDxe is signaled before 3rd part code >>>>> running. >>>>>> >>>>>> As such, it is legal that a trusted console is >>>>> connected before EndOfDxe. >>>>>> You can report progress to user before EndOfDxe. >>>>>> >>>>> >>>>> 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? >>>>> >>>>> >>>>>>> -----Original Message----- >>>>>>> From: Ard Biesheuvel >>>>> [mailto:ard.biesheuvel@linaro.org] >>>>>>> Sent: Friday, June 8, 2018 8:38 AM >>>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> >>>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D >>>>> <michael.d.kinney@intel.com>; >>>>>>> Zeng, Star <star.zeng@intel.com>; >>>>> leif.lindholm@linaro.org >>>>>>> Subject: Re: [edk2] [PATCH v2 2/5] >>>>> MdeModulePkg/DxeCapsuleLibFmp: permit >>>>>>> ProcessCapsules () to be called once >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On 8 Jun 2018, at 14:34, Yao, Jiewen >>>>> <jiewen.yao@intel.com> wrote: >>>>>>>> >>>>>>>> Hi Ard >>>>>>>> We don't allow platform to update system firmware >>>>> *after* EndOfDxe. >>>>>>>> >>>>>>>> According to PI spec, after EndOfDxe, 3rd part >>>>> code start running. It brings >>>>>>> security risk if we allow system firmware after >>>>> EndOfDxe. >>>>>>>> >>>>>>>> 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. >>>>>>>> >>>>>>>> Would you please share the info, why your platform >>>>> need update system >>>>>>> firmware after EndOfDxe? >>>>>>>> Is that possible to make it earlier? >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> Because we need some kind of console to report >>>>> progress to the user. >>>>>>> >>>>>>> The secure platform execution context is completely >>>>> separate from UEFI on Arm, >>>>>>> so for us the distinction does not make sense. >>>>>>> >>>>>>>> Thank you >>>>>>>> Yao Jiewen >>>>>>>> >>>>>>>>> -----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 >>>>> <michael.d.kinney@intel.com>; Yao, Jiewen >>>>>>>>> <jiewen.yao@intel.com>; Zeng, Star >>>>> <star.zeng@intel.com>; >>>>>>>>> leif.lindholm@linaro.org; Ard Biesheuvel >>>>> <ard.biesheuvel@linaro.org> >>>>>>>>> Subject: [edk2] [PATCH v2 2/5] >>>>> MdeModulePkg/DxeCapsuleLibFmp: permit >>>>>>>>> ProcessCapsules () to be called once >>>>>>>>> >>>>>>>>> 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. >>>>>>>>> >>>>>>>>> Contributed-under: TianoCore Contribution >>>>> Agreement 1.1 >>>>>>>>> Signed-off-by: Ard Biesheuvel >>>>> <ard.biesheuvel@linaro.org> >>>>>>>>> --- >>>>>>>>> >>>>> MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess >>>>> Lib.c | 18 >>>>>>>>> ++++++++++++------ >>>>>>>>> 1 file changed, 12 insertions(+), 6 deletions(-) >>>>>>>>> >>>>>>>>> diff --git >>>>>>> >>>>> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce >>>>> ssLib.c >>>>>>>>> >>>>> 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 ( >>>>>>>>> >>>>>>>>> extern BOOLEAN >>>>> mDxeCapsuleLibEndOfDxe; >>>>>>>>> BOOLEAN mNeedReset; >>>>>>>>> +BOOLEAN mFirstRound = >>>>> TRUE; >>>>>>>>> >>>>>>>>> VOID **mCapsulePtr; >>>>>>>>> EFI_STATUS *mCapsuleStatusArray; >>>>>>>>> @@ -364,8 +365,11 @@ >>>>> PopulateCapsuleInConfigurationTable ( >>>>>>>>> >>>>>>>>> Each individual capsule result is recorded in >>>>> capsule record variable. >>>>>>>>> >>>>>>>>> - @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. >>>>>>>>> >>>>>>>>> @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; >>>>>>>>> } >>>>>>>>> >>>>>>>>> - if ((!FirstRound) || (EmbeddedDriverCount >>>>> == 0)) { >>>>>>>>> + if (LastRound || (EmbeddedDriverCount == >>>>> 0)) { >>>>>>>>> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - >>>>> 0x%x\n", >>>>>>>>> CapsuleHeader)); >>>>>>>>> Status = ProcessCapsuleImage >>>>> (CapsuleHeader); >>>>>>>>> mCapsuleStatusArray [Index] = Status; >>>>>>>>> @@ -546,7 +551,7 @@ ProcessCapsules ( >>>>>>>>> EFI_STATUS Status; >>>>>>>>> >>>>>>>>> if (!mDxeCapsuleLibEndOfDxe) { >>>>>>>>> - Status = ProcessTheseCapsules(TRUE); >>>>>>>>> + Status = ProcessTheseCapsules(TRUE, FALSE); >>>>>>>>> >>>>>>>>> // >>>>>>>>> // Reboot System if and only if all capsule >>>>> processed. >>>>>>>>> @@ -555,8 +560,9 @@ ProcessCapsules ( >>>>>>>>> if (mNeedReset && AreAllImagesProcessed()) { >>>>>>>>> DoResetSystem(); >>>>>>>>> } >>>>>>>>> + mFirstRound = FALSE; >>>>>>>>> } else { >>>>>>>>> - Status = ProcessTheseCapsules(FALSE); >>>>>>>>> + Status = ProcessTheseCapsules(mFirstRound, >>>>> TRUE); >>>>>>>>> // >>>>>>>>> // Reboot System if required after all >>>>> capsule processed >>>>>>>>> // >>>>>>>>> -- >>>>>>>>> 2.17.0 >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 11 June 2018 at 14:37, Yao, Jiewen <jiewen.yao@intel.com> wrote: > If all fmp can be processed one time，you just need call once. Then system reset. > I understand that. But if the capsule has embedded drivers, the library will only dispatch them on the second call, even if the platform does not require making that distinction. It seems to me that the phasing between the first and the second call is closely tied to the semantics of EndOfDxe and the associated policy on X86 not to allow third party code before it has been signalled. Could we not parameterize this in some way as well? > >> 在 2018年6月11日，上午12:27，Ard Biesheuvel <ard.biesheuvel@linaro.org> 写道： >> >>> On 10 June 2018 at 21:21, Yao, Jiewen <jiewen.yao@intel.com> wrote: >>> Just got some idea since I am also reviewing FmpDevicePkg patch. >>> >>> >>> The key problem we are trying to resolve that is: The core code uses EndOfDxe as the lock event for system firmware, but an ARM platform may want to lock system firmware later, maybe in other lock event. >>> >>> As such, how about we generalize the lock event as PcdSystemFmpLockEventGuid? We can use EndOfDxeGuid as default one to keep the compatibility. At same time this brings the flexibility for platform overriding. >>> >>> >>> We only need update DxeCapsuleLibConstructor() to use the new PcdGuid, instead of hardcode EndOfDxe. >>> We don't need update the logic in ProcessCapsules() and ProcessTheseCapsules(). >>> 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 configure another lock event GUID explicitly in platform DSC. >>> >> >> But that means we are still required to call ProcessCapsules () twice, no? >> >>>> -----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 <michael.d.kinney@intel.com>; Ard Biesheuvel >>>> <ard.biesheuvel@linaro.org> >>>> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star >>>> <star.zeng@intel.com> >>>> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit >>>> ProcessCapsules () to be called once >>>> >>>> My concern is that *always allowing* processing SystemCapsule after EndOfDxe >>>> has security risk. >>>> >>>> IMHO, the risk is not *process*, if it is a platform choice. Instead, the risk is >>>> *allow*, in the core logic. >>>> >>>> If we really want to do that, I hope we need a way to distinguish the difference >>>> between a platform dispatching SystemCapsule after EndOfDxe *purposely* and >>>> a platform dispatching SystemCapsule after EndOfDxe *by mistake*. >>>> >>>> Maybe some policy enforcement in the core logic. Static policy, at build time. >>>> >>>> Thank you >>>> Yao Jiewen >>>> >>>>> -----Original Message----- >>>>> From: Kinney, Michael D >>>>> Sent: Sunday, June 10, 2018 8:57 AM >>>>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Yao, Jiewen >>>>> <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> >>>>> Cc: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; >>>>> leif.lindholm@linaro.org >>>>> Subject: RE: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit >>>>> ProcessCapsules () to be called once >>>>> >>>>> Ard, >>>>> >>>>> 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. >>>>> >>>>> 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. >>>>> >>>>> 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. >>>>> >>>>> 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. >>>>> >>>>> I will review your patch in more detail tomorrow. >>>>> >>>>> Thanks, >>>>> >>>>> Mike >>>>> >>>>>> -----Original Message----- >>>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >>>>>> Sent: Saturday, June 9, 2018 10:42 PM >>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> >>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D >>>>>> <michael.d.kinney@intel.com>; Zeng, Star >>>>>> <star.zeng@intel.com>; leif.lindholm@linaro.org >>>>>> Subject: Re: [edk2] [PATCH v2 2/5] >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules >>>>>> () to be called once >>>>>> >>>>>> On 10 June 2018 at 07:38, Yao, Jiewen >>>>>> <jiewen.yao@intel.com> 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" >>>>>>> >>>>>>> In brief, EndOfDxe is signaled before 3rd part code >>>>>> running. >>>>>>> >>>>>>> As such, it is legal that a trusted console is >>>>>> connected before EndOfDxe. >>>>>>> You can report progress to user before EndOfDxe. >>>>>>> >>>>>> >>>>>> 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? >>>>>> >>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Ard Biesheuvel >>>>>> [mailto:ard.biesheuvel@linaro.org] >>>>>>>> Sent: Friday, June 8, 2018 8:38 AM >>>>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> >>>>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D >>>>>> <michael.d.kinney@intel.com>; >>>>>>>> Zeng, Star <star.zeng@intel.com>; >>>>>> leif.lindholm@linaro.org >>>>>>>> Subject: Re: [edk2] [PATCH v2 2/5] >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit >>>>>>>> ProcessCapsules () to be called once >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> On 8 Jun 2018, at 14:34, Yao, Jiewen >>>>>> <jiewen.yao@intel.com> wrote: >>>>>>>>> >>>>>>>>> Hi Ard >>>>>>>>> We don't allow platform to update system firmware >>>>>> *after* EndOfDxe. >>>>>>>>> >>>>>>>>> According to PI spec, after EndOfDxe, 3rd part >>>>>> code start running. It brings >>>>>>>> security risk if we allow system firmware after >>>>>> EndOfDxe. >>>>>>>>> >>>>>>>>> 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. >>>>>>>>> >>>>>>>>> Would you please share the info, why your platform >>>>>> need update system >>>>>>>> firmware after EndOfDxe? >>>>>>>>> Is that possible to make it earlier? >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> Because we need some kind of console to report >>>>>> progress to the user. >>>>>>>> >>>>>>>> The secure platform execution context is completely >>>>>> separate from UEFI on Arm, >>>>>>>> so for us the distinction does not make sense. >>>>>>>> >>>>>>>>> Thank you >>>>>>>>> Yao Jiewen >>>>>>>>> >>>>>>>>>> -----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 >>>>>> <michael.d.kinney@intel.com>; Yao, Jiewen >>>>>>>>>> <jiewen.yao@intel.com>; Zeng, Star >>>>>> <star.zeng@intel.com>; >>>>>>>>>> leif.lindholm@linaro.org; Ard Biesheuvel >>>>>> <ard.biesheuvel@linaro.org> >>>>>>>>>> Subject: [edk2] [PATCH v2 2/5] >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit >>>>>>>>>> ProcessCapsules () to be called once >>>>>>>>>> >>>>>>>>>> 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. >>>>>>>>>> >>>>>>>>>> Contributed-under: TianoCore Contribution >>>>>> Agreement 1.1 >>>>>>>>>> Signed-off-by: Ard Biesheuvel >>>>>> <ard.biesheuvel@linaro.org> >>>>>>>>>> --- >>>>>>>>>> >>>>>> MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess >>>>>> Lib.c | 18 >>>>>>>>>> ++++++++++++------ >>>>>>>>>> 1 file changed, 12 insertions(+), 6 deletions(-) >>>>>>>>>> >>>>>>>>>> diff --git >>>>>>>> >>>>>> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce >>>>>> ssLib.c >>>>>>>>>> >>>>>> 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 ( >>>>>>>>>> >>>>>>>>>> extern BOOLEAN >>>>>> mDxeCapsuleLibEndOfDxe; >>>>>>>>>> BOOLEAN mNeedReset; >>>>>>>>>> +BOOLEAN mFirstRound = >>>>>> TRUE; >>>>>>>>>> >>>>>>>>>> VOID **mCapsulePtr; >>>>>>>>>> EFI_STATUS *mCapsuleStatusArray; >>>>>>>>>> @@ -364,8 +365,11 @@ >>>>>> PopulateCapsuleInConfigurationTable ( >>>>>>>>>> >>>>>>>>>> Each individual capsule result is recorded in >>>>>> capsule record variable. >>>>>>>>>> >>>>>>>>>> - @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. >>>>>>>>>> >>>>>>>>>> @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; >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> - if ((!FirstRound) || (EmbeddedDriverCount >>>>>> == 0)) { >>>>>>>>>> + if (LastRound || (EmbeddedDriverCount == >>>>>> 0)) { >>>>>>>>>> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - >>>>>> 0x%x\n", >>>>>>>>>> CapsuleHeader)); >>>>>>>>>> Status = ProcessCapsuleImage >>>>>> (CapsuleHeader); >>>>>>>>>> mCapsuleStatusArray [Index] = Status; >>>>>>>>>> @@ -546,7 +551,7 @@ ProcessCapsules ( >>>>>>>>>> EFI_STATUS Status; >>>>>>>>>> >>>>>>>>>> if (!mDxeCapsuleLibEndOfDxe) { >>>>>>>>>> - Status = ProcessTheseCapsules(TRUE); >>>>>>>>>> + Status = ProcessTheseCapsules(TRUE, FALSE); >>>>>>>>>> >>>>>>>>>> // >>>>>>>>>> // Reboot System if and only if all capsule >>>>>> processed. >>>>>>>>>> @@ -555,8 +560,9 @@ ProcessCapsules ( >>>>>>>>>> if (mNeedReset && AreAllImagesProcessed()) { >>>>>>>>>> DoResetSystem(); >>>>>>>>>> } >>>>>>>>>> + mFirstRound = FALSE; >>>>>>>>>> } else { >>>>>>>>>> - Status = ProcessTheseCapsules(FALSE); >>>>>>>>>> + Status = ProcessTheseCapsules(mFirstRound, >>>>>> TRUE); >>>>>>>>>> // >>>>>>>>>> // Reboot System if required after all >>>>>> capsule processed >>>>>>>>>> // >>>>>>>>>> -- >>>>>>>>>> 2.17.0 >>>>>>>>>> >>>>>>>>>> _______________________________________________ >>>>>>>>>> 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
Ah. Good point. I think the embedded driver dispatch can be controlled by EndOfDxe, which is similar to MdeModulePkg\Universal\SecurityStubDxe. So we can have both EndOfDxe and PcdSystemFmpLockEventGuid. EndOfDxe controls embedded driver dispatch. PcdSystemFmpLockEventGuid controls 1st call and 2nd call. Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard > Biesheuvel > Sent: Monday, June 11, 2018 5:40 AM > To: Yao, Jiewen <jiewen.yao@intel.com> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; > Zeng, Star <star.zeng@intel.com>; leif.lindholm@linaro.org > Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit > ProcessCapsules () to be called once > > On 11 June 2018 at 14:37, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > If all fmp can be processed one time，you just need call once. Then system > reset. > > > > I understand that. But if the capsule has embedded drivers, the > library will only dispatch them on the second call, even if the > platform does not require making that distinction. It seems to me that > the phasing between the first and the second call is closely tied to > the semantics of EndOfDxe and the associated policy on X86 not to > allow third party code before it has been signalled. > > Could we not parameterize this in some way as well? > > > > > >> 在 2018年6月11日，上午12:27，Ard Biesheuvel > <ard.biesheuvel@linaro.org> 写道： > >> > >>> On 10 June 2018 at 21:21, Yao, Jiewen <jiewen.yao@intel.com> wrote: > >>> Just got some idea since I am also reviewing FmpDevicePkg patch. > >>> > >>> > >>> The key problem we are trying to resolve that is: The core code uses > EndOfDxe as the lock event for system firmware, but an ARM platform may want > to lock system firmware later, maybe in other lock event. > >>> > >>> As such, how about we generalize the lock event as > PcdSystemFmpLockEventGuid? We can use EndOfDxeGuid as default one to keep > the compatibility. At same time this brings the flexibility for platform overriding. > >>> > >>> > >>> We only need update DxeCapsuleLibConstructor() to use the new PcdGuid, > instead of hardcode EndOfDxe. > >>> We don't need update the logic in ProcessCapsules() and > ProcessTheseCapsules(). > >>> 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 > configure another lock event GUID explicitly in platform DSC. > >>> > >> > >> But that means we are still required to call ProcessCapsules () twice, no? > >> > >>>> -----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 <michael.d.kinney@intel.com>; Ard Biesheuvel > >>>> <ard.biesheuvel@linaro.org> > >>>> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star > >>>> <star.zeng@intel.com> > >>>> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > permit > >>>> ProcessCapsules () to be called once > >>>> > >>>> My concern is that *always allowing* processing SystemCapsule after > EndOfDxe > >>>> has security risk. > >>>> > >>>> IMHO, the risk is not *process*, if it is a platform choice. Instead, the risk is > >>>> *allow*, in the core logic. > >>>> > >>>> If we really want to do that, I hope we need a way to distinguish the > difference > >>>> between a platform dispatching SystemCapsule after EndOfDxe > *purposely* and > >>>> a platform dispatching SystemCapsule after EndOfDxe *by mistake*. > >>>> > >>>> Maybe some policy enforcement in the core logic. Static policy, at build > time. > >>>> > >>>> Thank you > >>>> Yao Jiewen > >>>> > >>>>> -----Original Message----- > >>>>> From: Kinney, Michael D > >>>>> Sent: Sunday, June 10, 2018 8:57 AM > >>>>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Yao, Jiewen > >>>>> <jiewen.yao@intel.com>; Kinney, Michael D > <michael.d.kinney@intel.com> > >>>>> Cc: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; > >>>>> leif.lindholm@linaro.org > >>>>> Subject: RE: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > permit > >>>>> ProcessCapsules () to be called once > >>>>> > >>>>> Ard, > >>>>> > >>>>> 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. > >>>>> > >>>>> 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. > >>>>> > >>>>> 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. > >>>>> > >>>>> 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. > >>>>> > >>>>> I will review your patch in more detail tomorrow. > >>>>> > >>>>> Thanks, > >>>>> > >>>>> Mike > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > >>>>>> Sent: Saturday, June 9, 2018 10:42 PM > >>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> > >>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D > >>>>>> <michael.d.kinney@intel.com>; Zeng, Star > >>>>>> <star.zeng@intel.com>; leif.lindholm@linaro.org > >>>>>> Subject: Re: [edk2] [PATCH v2 2/5] > >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules > >>>>>> () to be called once > >>>>>> > >>>>>> On 10 June 2018 at 07:38, Yao, Jiewen > >>>>>> <jiewen.yao@intel.com> 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" > >>>>>>> > >>>>>>> In brief, EndOfDxe is signaled before 3rd part code > >>>>>> running. > >>>>>>> > >>>>>>> As such, it is legal that a trusted console is > >>>>>> connected before EndOfDxe. > >>>>>>> You can report progress to user before EndOfDxe. > >>>>>>> > >>>>>> > >>>>>> 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? > >>>>>> > >>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: Ard Biesheuvel > >>>>>> [mailto:ard.biesheuvel@linaro.org] > >>>>>>>> Sent: Friday, June 8, 2018 8:38 AM > >>>>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> > >>>>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D > >>>>>> <michael.d.kinney@intel.com>; > >>>>>>>> Zeng, Star <star.zeng@intel.com>; > >>>>>> leif.lindholm@linaro.org > >>>>>>>> Subject: Re: [edk2] [PATCH v2 2/5] > >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit > >>>>>>>> ProcessCapsules () to be called once > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>>> On 8 Jun 2018, at 14:34, Yao, Jiewen > >>>>>> <jiewen.yao@intel.com> wrote: > >>>>>>>>> > >>>>>>>>> Hi Ard > >>>>>>>>> We don't allow platform to update system firmware > >>>>>> *after* EndOfDxe. > >>>>>>>>> > >>>>>>>>> According to PI spec, after EndOfDxe, 3rd part > >>>>>> code start running. It brings > >>>>>>>> security risk if we allow system firmware after > >>>>>> EndOfDxe. > >>>>>>>>> > >>>>>>>>> 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. > >>>>>>>>> > >>>>>>>>> Would you please share the info, why your platform > >>>>>> need update system > >>>>>>>> firmware after EndOfDxe? > >>>>>>>>> Is that possible to make it earlier? > >>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>>> Because we need some kind of console to report > >>>>>> progress to the user. > >>>>>>>> > >>>>>>>> The secure platform execution context is completely > >>>>>> separate from UEFI on Arm, > >>>>>>>> so for us the distinction does not make sense. > >>>>>>>> > >>>>>>>>> Thank you > >>>>>>>>> Yao Jiewen > >>>>>>>>> > >>>>>>>>>> -----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 > >>>>>> <michael.d.kinney@intel.com>; Yao, Jiewen > >>>>>>>>>> <jiewen.yao@intel.com>; Zeng, Star > >>>>>> <star.zeng@intel.com>; > >>>>>>>>>> leif.lindholm@linaro.org; Ard Biesheuvel > >>>>>> <ard.biesheuvel@linaro.org> > >>>>>>>>>> Subject: [edk2] [PATCH v2 2/5] > >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit > >>>>>>>>>> ProcessCapsules () to be called once > >>>>>>>>>> > >>>>>>>>>> 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. > >>>>>>>>>> > >>>>>>>>>> Contributed-under: TianoCore Contribution > >>>>>> Agreement 1.1 > >>>>>>>>>> Signed-off-by: Ard Biesheuvel > >>>>>> <ard.biesheuvel@linaro.org> > >>>>>>>>>> --- > >>>>>>>>>> > >>>>>> MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess > >>>>>> Lib.c | 18 > >>>>>>>>>> ++++++++++++------ > >>>>>>>>>> 1 file changed, 12 insertions(+), 6 deletions(-) > >>>>>>>>>> > >>>>>>>>>> diff --git > >>>>>>>> > >>>>>> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce > >>>>>> ssLib.c > >>>>>>>>>> > >>>>>> 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 ( > >>>>>>>>>> > >>>>>>>>>> extern BOOLEAN > >>>>>> mDxeCapsuleLibEndOfDxe; > >>>>>>>>>> BOOLEAN mNeedReset; > >>>>>>>>>> +BOOLEAN mFirstRound = > >>>>>> TRUE; > >>>>>>>>>> > >>>>>>>>>> VOID **mCapsulePtr; > >>>>>>>>>> EFI_STATUS *mCapsuleStatusArray; > >>>>>>>>>> @@ -364,8 +365,11 @@ > >>>>>> PopulateCapsuleInConfigurationTable ( > >>>>>>>>>> > >>>>>>>>>> Each individual capsule result is recorded in > >>>>>> capsule record variable. > >>>>>>>>>> > >>>>>>>>>> - @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. > >>>>>>>>>> > >>>>>>>>>> @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; > >>>>>>>>>> } > >>>>>>>>>> > >>>>>>>>>> - if ((!FirstRound) || (EmbeddedDriverCount > >>>>>> == 0)) { > >>>>>>>>>> + if (LastRound || (EmbeddedDriverCount == > >>>>>> 0)) { > >>>>>>>>>> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - > >>>>>> 0x%x\n", > >>>>>>>>>> CapsuleHeader)); > >>>>>>>>>> Status = ProcessCapsuleImage > >>>>>> (CapsuleHeader); > >>>>>>>>>> mCapsuleStatusArray [Index] = Status; > >>>>>>>>>> @@ -546,7 +551,7 @@ ProcessCapsules ( > >>>>>>>>>> EFI_STATUS Status; > >>>>>>>>>> > >>>>>>>>>> if (!mDxeCapsuleLibEndOfDxe) { > >>>>>>>>>> - Status = ProcessTheseCapsules(TRUE); > >>>>>>>>>> + Status = ProcessTheseCapsules(TRUE, FALSE); > >>>>>>>>>> > >>>>>>>>>> // > >>>>>>>>>> // Reboot System if and only if all capsule > >>>>>> processed. > >>>>>>>>>> @@ -555,8 +560,9 @@ ProcessCapsules ( > >>>>>>>>>> if (mNeedReset && AreAllImagesProcessed()) { > >>>>>>>>>> DoResetSystem(); > >>>>>>>>>> } > >>>>>>>>>> + mFirstRound = FALSE; > >>>>>>>>>> } else { > >>>>>>>>>> - Status = ProcessTheseCapsules(FALSE); > >>>>>>>>>> + Status = ProcessTheseCapsules(mFirstRound, > >>>>>> TRUE); > >>>>>>>>>> // > >>>>>>>>>> // Reboot System if required after all > >>>>>> capsule processed > >>>>>>>>>> // > >>>>>>>>>> -- > >>>>>>>>>> 2.17.0 > >>>>>>>>>> > >>>>>>>>>> _______________________________________________ > >>>>>>>>>> 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 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel
On 11 June 2018 at 15:55, Yao, Jiewen <jiewen.yao@intel.com> wrote: > Ah. Good point. > > I think the embedded driver dispatch can be controlled by EndOfDxe, which is similar to MdeModulePkg\Universal\SecurityStubDxe. > > So we can have both EndOfDxe and PcdSystemFmpLockEventGuid. > EndOfDxe controls embedded driver dispatch. > PcdSystemFmpLockEventGuid controls 1st call and 2nd call. > But in our case, we don't need an event at all. The BDS just calls ProcessCapsules() whenever it wants to. I'm rather relucant to complicate the code with an additional event that we don't really need, while retaining some of the EndOfDxe handling as well. Could you think of another way to handle this use case? Thanks, Ard. >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard >> Biesheuvel >> Sent: Monday, June 11, 2018 5:40 AM >> To: Yao, Jiewen <jiewen.yao@intel.com> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; >> Zeng, Star <star.zeng@intel.com>; leif.lindholm@linaro.org >> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit >> ProcessCapsules () to be called once >> >> On 11 June 2018 at 14:37, Yao, Jiewen <jiewen.yao@intel.com> wrote: >> > If all fmp can be processed one time，you just need call once. Then system >> reset. >> > >> >> I understand that. But if the capsule has embedded drivers, the >> library will only dispatch them on the second call, even if the >> platform does not require making that distinction. It seems to me that >> the phasing between the first and the second call is closely tied to >> the semantics of EndOfDxe and the associated policy on X86 not to >> allow third party code before it has been signalled. >> >> Could we not parameterize this in some way as well? >> >> >> > >> >> 在 2018年6月11日，上午12:27，Ard Biesheuvel >> <ard.biesheuvel@linaro.org> 写道： >> >> >> >>> On 10 June 2018 at 21:21, Yao, Jiewen <jiewen.yao@intel.com> wrote: >> >>> Just got some idea since I am also reviewing FmpDevicePkg patch. >> >>> >> >>> >> >>> The key problem we are trying to resolve that is: The core code uses >> EndOfDxe as the lock event for system firmware, but an ARM platform may want >> to lock system firmware later, maybe in other lock event. >> >>> >> >>> As such, how about we generalize the lock event as >> PcdSystemFmpLockEventGuid? We can use EndOfDxeGuid as default one to keep >> the compatibility. At same time this brings the flexibility for platform overriding. >> >>> >> >>> >> >>> We only need update DxeCapsuleLibConstructor() to use the new PcdGuid, >> instead of hardcode EndOfDxe. >> >>> We don't need update the logic in ProcessCapsules() and >> ProcessTheseCapsules(). >> >>> 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 >> configure another lock event GUID explicitly in platform DSC. >> >>> >> >> >> >> But that means we are still required to call ProcessCapsules () twice, no? >> >> >> >>>> -----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 <michael.d.kinney@intel.com>; Ard Biesheuvel >> >>>> <ard.biesheuvel@linaro.org> >> >>>> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star >> >>>> <star.zeng@intel.com> >> >>>> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: >> permit >> >>>> ProcessCapsules () to be called once >> >>>> >> >>>> My concern is that *always allowing* processing SystemCapsule after >> EndOfDxe >> >>>> has security risk. >> >>>> >> >>>> IMHO, the risk is not *process*, if it is a platform choice. Instead, the risk is >> >>>> *allow*, in the core logic. >> >>>> >> >>>> If we really want to do that, I hope we need a way to distinguish the >> difference >> >>>> between a platform dispatching SystemCapsule after EndOfDxe >> *purposely* and >> >>>> a platform dispatching SystemCapsule after EndOfDxe *by mistake*. >> >>>> >> >>>> Maybe some policy enforcement in the core logic. Static policy, at build >> time. >> >>>> >> >>>> Thank you >> >>>> Yao Jiewen >> >>>> >> >>>>> -----Original Message----- >> >>>>> From: Kinney, Michael D >> >>>>> Sent: Sunday, June 10, 2018 8:57 AM >> >>>>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Yao, Jiewen >> >>>>> <jiewen.yao@intel.com>; Kinney, Michael D >> <michael.d.kinney@intel.com> >> >>>>> Cc: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; >> >>>>> leif.lindholm@linaro.org >> >>>>> Subject: RE: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: >> permit >> >>>>> ProcessCapsules () to be called once >> >>>>> >> >>>>> Ard, >> >>>>> >> >>>>> 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. >> >>>>> >> >>>>> 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. >> >>>>> >> >>>>> 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. >> >>>>> >> >>>>> 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. >> >>>>> >> >>>>> I will review your patch in more detail tomorrow. >> >>>>> >> >>>>> Thanks, >> >>>>> >> >>>>> Mike >> >>>>> >> >>>>>> -----Original Message----- >> >>>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> >>>>>> Sent: Saturday, June 9, 2018 10:42 PM >> >>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> >> >>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D >> >>>>>> <michael.d.kinney@intel.com>; Zeng, Star >> >>>>>> <star.zeng@intel.com>; leif.lindholm@linaro.org >> >>>>>> Subject: Re: [edk2] [PATCH v2 2/5] >> >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules >> >>>>>> () to be called once >> >>>>>> >> >>>>>> On 10 June 2018 at 07:38, Yao, Jiewen >> >>>>>> <jiewen.yao@intel.com> 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" >> >>>>>>> >> >>>>>>> In brief, EndOfDxe is signaled before 3rd part code >> >>>>>> running. >> >>>>>>> >> >>>>>>> As such, it is legal that a trusted console is >> >>>>>> connected before EndOfDxe. >> >>>>>>> You can report progress to user before EndOfDxe. >> >>>>>>> >> >>>>>> >> >>>>>> 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? >> >>>>>> >> >>>>>> >> >>>>>>>> -----Original Message----- >> >>>>>>>> From: Ard Biesheuvel >> >>>>>> [mailto:ard.biesheuvel@linaro.org] >> >>>>>>>> Sent: Friday, June 8, 2018 8:38 AM >> >>>>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> >> >>>>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D >> >>>>>> <michael.d.kinney@intel.com>; >> >>>>>>>> Zeng, Star <star.zeng@intel.com>; >> >>>>>> leif.lindholm@linaro.org >> >>>>>>>> Subject: Re: [edk2] [PATCH v2 2/5] >> >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit >> >>>>>>>> ProcessCapsules () to be called once >> >>>>>>>> >> >>>>>>>> >> >>>>>>>> >> >>>>>>>>> On 8 Jun 2018, at 14:34, Yao, Jiewen >> >>>>>> <jiewen.yao@intel.com> wrote: >> >>>>>>>>> >> >>>>>>>>> Hi Ard >> >>>>>>>>> We don't allow platform to update system firmware >> >>>>>> *after* EndOfDxe. >> >>>>>>>>> >> >>>>>>>>> According to PI spec, after EndOfDxe, 3rd part >> >>>>>> code start running. It brings >> >>>>>>>> security risk if we allow system firmware after >> >>>>>> EndOfDxe. >> >>>>>>>>> >> >>>>>>>>> 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. >> >>>>>>>>> >> >>>>>>>>> Would you please share the info, why your platform >> >>>>>> need update system >> >>>>>>>> firmware after EndOfDxe? >> >>>>>>>>> Is that possible to make it earlier? >> >>>>>>>>> >> >>>>>>>>> >> >>>>>>>> >> >>>>>>>> Because we need some kind of console to report >> >>>>>> progress to the user. >> >>>>>>>> >> >>>>>>>> The secure platform execution context is completely >> >>>>>> separate from UEFI on Arm, >> >>>>>>>> so for us the distinction does not make sense. >> >>>>>>>> >> >>>>>>>>> Thank you >> >>>>>>>>> Yao Jiewen >> >>>>>>>>> >> >>>>>>>>>> -----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 >> >>>>>> <michael.d.kinney@intel.com>; Yao, Jiewen >> >>>>>>>>>> <jiewen.yao@intel.com>; Zeng, Star >> >>>>>> <star.zeng@intel.com>; >> >>>>>>>>>> leif.lindholm@linaro.org; Ard Biesheuvel >> >>>>>> <ard.biesheuvel@linaro.org> >> >>>>>>>>>> Subject: [edk2] [PATCH v2 2/5] >> >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit >> >>>>>>>>>> ProcessCapsules () to be called once >> >>>>>>>>>> >> >>>>>>>>>> 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. >> >>>>>>>>>> >> >>>>>>>>>> Contributed-under: TianoCore Contribution >> >>>>>> Agreement 1.1 >> >>>>>>>>>> Signed-off-by: Ard Biesheuvel >> >>>>>> <ard.biesheuvel@linaro.org> >> >>>>>>>>>> --- >> >>>>>>>>>> >> >>>>>> MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess >> >>>>>> Lib.c | 18 >> >>>>>>>>>> ++++++++++++------ >> >>>>>>>>>> 1 file changed, 12 insertions(+), 6 deletions(-) >> >>>>>>>>>> >> >>>>>>>>>> diff --git >> >>>>>>>> >> >>>>>> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce >> >>>>>> ssLib.c >> >>>>>>>>>> >> >>>>>> 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 ( >> >>>>>>>>>> >> >>>>>>>>>> extern BOOLEAN >> >>>>>> mDxeCapsuleLibEndOfDxe; >> >>>>>>>>>> BOOLEAN mNeedReset; >> >>>>>>>>>> +BOOLEAN mFirstRound = >> >>>>>> TRUE; >> >>>>>>>>>> >> >>>>>>>>>> VOID **mCapsulePtr; >> >>>>>>>>>> EFI_STATUS *mCapsuleStatusArray; >> >>>>>>>>>> @@ -364,8 +365,11 @@ >> >>>>>> PopulateCapsuleInConfigurationTable ( >> >>>>>>>>>> >> >>>>>>>>>> Each individual capsule result is recorded in >> >>>>>> capsule record variable. >> >>>>>>>>>> >> >>>>>>>>>> - @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. >> >>>>>>>>>> >> >>>>>>>>>> @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; >> >>>>>>>>>> } >> >>>>>>>>>> >> >>>>>>>>>> - if ((!FirstRound) || (EmbeddedDriverCount >> >>>>>> == 0)) { >> >>>>>>>>>> + if (LastRound || (EmbeddedDriverCount == >> >>>>>> 0)) { >> >>>>>>>>>> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - >> >>>>>> 0x%x\n", >> >>>>>>>>>> CapsuleHeader)); >> >>>>>>>>>> Status = ProcessCapsuleImage >> >>>>>> (CapsuleHeader); >> >>>>>>>>>> mCapsuleStatusArray [Index] = Status; >> >>>>>>>>>> @@ -546,7 +551,7 @@ ProcessCapsules ( >> >>>>>>>>>> EFI_STATUS Status; >> >>>>>>>>>> >> >>>>>>>>>> if (!mDxeCapsuleLibEndOfDxe) { >> >>>>>>>>>> - Status = ProcessTheseCapsules(TRUE); >> >>>>>>>>>> + Status = ProcessTheseCapsules(TRUE, FALSE); >> >>>>>>>>>> >> >>>>>>>>>> // >> >>>>>>>>>> // Reboot System if and only if all capsule >> >>>>>> processed. >> >>>>>>>>>> @@ -555,8 +560,9 @@ ProcessCapsules ( >> >>>>>>>>>> if (mNeedReset && AreAllImagesProcessed()) { >> >>>>>>>>>> DoResetSystem(); >> >>>>>>>>>> } >> >>>>>>>>>> + mFirstRound = FALSE; >> >>>>>>>>>> } else { >> >>>>>>>>>> - Status = ProcessTheseCapsules(FALSE); >> >>>>>>>>>> + Status = ProcessTheseCapsules(mFirstRound, >> >>>>>> TRUE); >> >>>>>>>>>> // >> >>>>>>>>>> // Reboot System if required after all >> >>>>>> capsule processed >> >>>>>>>>>> // >> >>>>>>>>>> -- >> >>>>>>>>>> 2.17.0 >> >>>>>>>>>> >> >>>>>>>>>> _______________________________________________ >> >>>>>>>>>> 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 >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel
Jiewen, A platform can have multiple FMP protocols to update system firmware and may be mapped to different FW storage devices with different security attributes. I think your proposal would treat them all the same. Right? Mike > -----Original Message----- > From: Yao, Jiewen > Sent: Monday, June 11, 2018 6:56 AM > To: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > edk2-devel@lists.01.org; Zeng, Star > <star.zeng@intel.com>; leif.lindholm@linaro.org > Subject: RE: [edk2] [PATCH v2 2/5] > MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules > () to be called once > > Ah. Good point. > > I think the embedded driver dispatch can be controlled > by EndOfDxe, which is similar to > MdeModulePkg\Universal\SecurityStubDxe. > > So we can have both EndOfDxe and > PcdSystemFmpLockEventGuid. > EndOfDxe controls embedded driver dispatch. > PcdSystemFmpLockEventGuid controls 1st call and 2nd > call. > > Thank you > Yao Jiewen > > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel- > bounces@lists.01.org] On Behalf Of Ard > > Biesheuvel > > Sent: Monday, June 11, 2018 5:40 AM > > To: Yao, Jiewen <jiewen.yao@intel.com> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > edk2-devel@lists.01.org; > > Zeng, Star <star.zeng@intel.com>; > leif.lindholm@linaro.org > > Subject: Re: [edk2] [PATCH v2 2/5] > MdeModulePkg/DxeCapsuleLibFmp: permit > > ProcessCapsules () to be called once > > > > On 11 June 2018 at 14:37, Yao, Jiewen > <jiewen.yao@intel.com> wrote: > > > If all fmp can be processed one time，you just need > call once. Then system > > reset. > > > > > > > I understand that. But if the capsule has embedded > drivers, the > > library will only dispatch them on the second call, > even if the > > platform does not require making that distinction. It > seems to me that > > the phasing between the first and the second call is > closely tied to > > the semantics of EndOfDxe and the associated policy > on X86 not to > > allow third party code before it has been signalled. > > > > Could we not parameterize this in some way as well? > > > > > > > > > >> 在 2018年6月11日，上午12:27，Ard Biesheuvel > > <ard.biesheuvel@linaro.org> 写道： > > >> > > >>> On 10 June 2018 at 21:21, Yao, Jiewen > <jiewen.yao@intel.com> wrote: > > >>> Just got some idea since I am also reviewing > FmpDevicePkg patch. > > >>> > > >>> > > >>> The key problem we are trying to resolve that is: > The core code uses > > EndOfDxe as the lock event for system firmware, but > an ARM platform may want > > to lock system firmware later, maybe in other lock > event. > > >>> > > >>> As such, how about we generalize the lock event > as > > PcdSystemFmpLockEventGuid? We can use EndOfDxeGuid as > default one to keep > > the compatibility. At same time this brings the > flexibility for platform overriding. > > >>> > > >>> > > >>> We only need update DxeCapsuleLibConstructor() to > use the new PcdGuid, > > instead of hardcode EndOfDxe. > > >>> We don't need update the logic in > ProcessCapsules() and > > ProcessTheseCapsules(). > > >>> 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 > > configure another lock event GUID explicitly in > platform DSC. > > >>> > > >> > > >> But that means we are still required to call > ProcessCapsules () twice, no? > > >> > > >>>> -----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 > <michael.d.kinney@intel.com>; Ard Biesheuvel > > >>>> <ard.biesheuvel@linaro.org> > > >>>> Cc: edk2-devel@lists.01.org; > leif.lindholm@linaro.org; Zeng, Star > > >>>> <star.zeng@intel.com> > > >>>> Subject: Re: [edk2] [PATCH v2 2/5] > MdeModulePkg/DxeCapsuleLibFmp: > > permit > > >>>> ProcessCapsules () to be called once > > >>>> > > >>>> My concern is that *always allowing* processing > SystemCapsule after > > EndOfDxe > > >>>> has security risk. > > >>>> > > >>>> IMHO, the risk is not *process*, if it is a > platform choice. Instead, the risk is > > >>>> *allow*, in the core logic. > > >>>> > > >>>> If we really want to do that, I hope we need a > way to distinguish the > > difference > > >>>> between a platform dispatching SystemCapsule > after EndOfDxe > > *purposely* and > > >>>> a platform dispatching SystemCapsule after > EndOfDxe *by mistake*. > > >>>> > > >>>> Maybe some policy enforcement in the core logic. > Static policy, at build > > time. > > >>>> > > >>>> Thank you > > >>>> Yao Jiewen > > >>>> > > >>>>> -----Original Message----- > > >>>>> From: Kinney, Michael D > > >>>>> Sent: Sunday, June 10, 2018 8:57 AM > > >>>>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; > Yao, Jiewen > > >>>>> <jiewen.yao@intel.com>; Kinney, Michael D > > <michael.d.kinney@intel.com> > > >>>>> Cc: edk2-devel@lists.01.org; Zeng, Star > <star.zeng@intel.com>; > > >>>>> leif.lindholm@linaro.org > > >>>>> Subject: RE: [edk2] [PATCH v2 2/5] > MdeModulePkg/DxeCapsuleLibFmp: > > permit > > >>>>> ProcessCapsules () to be called once > > >>>>> > > >>>>> Ard, > > >>>>> > > >>>>> 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. > > >>>>> > > >>>>> 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. > > >>>>> > > >>>>> 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. > > >>>>> > > >>>>> 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. > > >>>>> > > >>>>> I will review your patch in more detail > tomorrow. > > >>>>> > > >>>>> Thanks, > > >>>>> > > >>>>> Mike > > >>>>> > > >>>>>> -----Original Message----- > > >>>>>> From: Ard Biesheuvel > [mailto:ard.biesheuvel@linaro.org] > > >>>>>> Sent: Saturday, June 9, 2018 10:42 PM > > >>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> > > >>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D > > >>>>>> <michael.d.kinney@intel.com>; Zeng, Star > > >>>>>> <star.zeng@intel.com>; > leif.lindholm@linaro.org > > >>>>>> Subject: Re: [edk2] [PATCH v2 2/5] > > >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit > ProcessCapsules > > >>>>>> () to be called once > > >>>>>> > > >>>>>> On 10 June 2018 at 07:38, Yao, Jiewen > > >>>>>> <jiewen.yao@intel.com> 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" > > >>>>>>> > > >>>>>>> In brief, EndOfDxe is signaled before 3rd > part code > > >>>>>> running. > > >>>>>>> > > >>>>>>> As such, it is legal that a trusted console > is > > >>>>>> connected before EndOfDxe. > > >>>>>>> You can report progress to user before > EndOfDxe. > > >>>>>>> > > >>>>>> > > >>>>>> 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? > > >>>>>> > > >>>>>> > > >>>>>>>> -----Original Message----- > > >>>>>>>> From: Ard Biesheuvel > > >>>>>> [mailto:ard.biesheuvel@linaro.org] > > >>>>>>>> Sent: Friday, June 8, 2018 8:38 AM > > >>>>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> > > >>>>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael > D > > >>>>>> <michael.d.kinney@intel.com>; > > >>>>>>>> Zeng, Star <star.zeng@intel.com>; > > >>>>>> leif.lindholm@linaro.org > > >>>>>>>> Subject: Re: [edk2] [PATCH v2 2/5] > > >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit > > >>>>>>>> ProcessCapsules () to be called once > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> > > >>>>>>>>> On 8 Jun 2018, at 14:34, Yao, Jiewen > > >>>>>> <jiewen.yao@intel.com> wrote: > > >>>>>>>>> > > >>>>>>>>> Hi Ard > > >>>>>>>>> We don't allow platform to update system > firmware > > >>>>>> *after* EndOfDxe. > > >>>>>>>>> > > >>>>>>>>> According to PI spec, after EndOfDxe, 3rd > part > > >>>>>> code start running. It brings > > >>>>>>>> security risk if we allow system firmware > after > > >>>>>> EndOfDxe. > > >>>>>>>>> > > >>>>>>>>> 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. > > >>>>>>>>> > > >>>>>>>>> Would you please share the info, why your > platform > > >>>>>> need update system > > >>>>>>>> firmware after EndOfDxe? > > >>>>>>>>> Is that possible to make it earlier? > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>>> > > >>>>>>>> Because we need some kind of console to > report > > >>>>>> progress to the user. > > >>>>>>>> > > >>>>>>>> The secure platform execution context is > completely > > >>>>>> separate from UEFI on Arm, > > >>>>>>>> so for us the distinction does not make > sense. > > >>>>>>>> > > >>>>>>>>> Thank you > > >>>>>>>>> Yao Jiewen > > >>>>>>>>> > > >>>>>>>>>> -----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 > > >>>>>> <michael.d.kinney@intel.com>; Yao, Jiewen > > >>>>>>>>>> <jiewen.yao@intel.com>; Zeng, Star > > >>>>>> <star.zeng@intel.com>; > > >>>>>>>>>> leif.lindholm@linaro.org; Ard Biesheuvel > > >>>>>> <ard.biesheuvel@linaro.org> > > >>>>>>>>>> Subject: [edk2] [PATCH v2 2/5] > > >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit > > >>>>>>>>>> ProcessCapsules () to be called once > > >>>>>>>>>> > > >>>>>>>>>> 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. > > >>>>>>>>>> > > >>>>>>>>>> Contributed-under: TianoCore Contribution > > >>>>>> Agreement 1.1 > > >>>>>>>>>> Signed-off-by: Ard Biesheuvel > > >>>>>> <ard.biesheuvel@linaro.org> > > >>>>>>>>>> --- > > >>>>>>>>>> > > >>>>>> > MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess > > >>>>>> Lib.c | 18 > > >>>>>>>>>> ++++++++++++------ > > >>>>>>>>>> 1 file changed, 12 insertions(+), 6 > deletions(-) > > >>>>>>>>>> > > >>>>>>>>>> diff --git > > >>>>>>>> > > >>>>>> > a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce > > >>>>>> ssLib.c > > >>>>>>>>>> > > >>>>>> > 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 ( > > >>>>>>>>>> > > >>>>>>>>>> extern BOOLEAN > > >>>>>> mDxeCapsuleLibEndOfDxe; > > >>>>>>>>>> BOOLEAN > mNeedReset; > > >>>>>>>>>> +BOOLEAN > mFirstRound = > > >>>>>> TRUE; > > >>>>>>>>>> > > >>>>>>>>>> VOID **mCapsulePtr; > > >>>>>>>>>> EFI_STATUS > *mCapsuleStatusArray; > > >>>>>>>>>> @@ -364,8 +365,11 @@ > > >>>>>> PopulateCapsuleInConfigurationTable ( > > >>>>>>>>>> > > >>>>>>>>>> Each individual capsule result is > recorded in > > >>>>>> capsule record variable. > > >>>>>>>>>> > > >>>>>>>>>> - @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. > > >>>>>>>>>> > > >>>>>>>>>> @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; > > >>>>>>>>>> } > > >>>>>>>>>> > > >>>>>>>>>> - if ((!FirstRound) || > (EmbeddedDriverCount > > >>>>>> == 0)) { > > >>>>>>>>>> + if (LastRound || > (EmbeddedDriverCount == > > >>>>>> 0)) { > > >>>>>>>>>> DEBUG((DEBUG_INFO, > "ProcessCapsuleImage - > > >>>>>> 0x%x\n", > > >>>>>>>>>> CapsuleHeader)); > > >>>>>>>>>> Status = ProcessCapsuleImage > > >>>>>> (CapsuleHeader); > > >>>>>>>>>> mCapsuleStatusArray [Index] = > Status; > > >>>>>>>>>> @@ -546,7 +551,7 @@ ProcessCapsules ( > > >>>>>>>>>> EFI_STATUS Status; > > >>>>>>>>>> > > >>>>>>>>>> if (!mDxeCapsuleLibEndOfDxe) { > > >>>>>>>>>> - Status = ProcessTheseCapsules(TRUE); > > >>>>>>>>>> + Status = ProcessTheseCapsules(TRUE, > FALSE); > > >>>>>>>>>> > > >>>>>>>>>> // > > >>>>>>>>>> // Reboot System if and only if all > capsule > > >>>>>> processed. > > >>>>>>>>>> @@ -555,8 +560,9 @@ ProcessCapsules ( > > >>>>>>>>>> if (mNeedReset && > AreAllImagesProcessed()) { > > >>>>>>>>>> DoResetSystem(); > > >>>>>>>>>> } > > >>>>>>>>>> + mFirstRound = FALSE; > > >>>>>>>>>> } else { > > >>>>>>>>>> - Status = ProcessTheseCapsules(FALSE); > > >>>>>>>>>> + Status = > ProcessTheseCapsules(mFirstRound, > > >>>>>> TRUE); > > >>>>>>>>>> // > > >>>>>>>>>> // Reboot System if required after all > > >>>>>> capsule processed > > >>>>>>>>>> // > > >>>>>>>>>> -- > > >>>>>>>>>> 2.17.0 > > >>>>>>>>>> > > >>>>>>>>>> > _______________________________________________ > > >>>>>>>>>> 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 > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel
The difference between 1st call and 2nd call is that: The system will reset even there are capsules not dispatched in 2nd call. If a platform just calls once and there is 1 device FMP not dispatched yet, how the core lib knows it should reset the system or it should wait for the second call after connect all? If you have any thought, please share with us. Summarize some requirement: 1) Embedded driver must be dispatched after EndOfDxe. 2) System Capsule must be dispatched before system firmware lock. (Ideally it should be EndOfDxe, a platform may choose others) 3) When the system capsule is dispatched, the driver FMP might be ready or might not be ready yet. 4) The core code need make sure the platform code dispatch the capsule in a right time. Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard > Biesheuvel > Sent: Monday, June 11, 2018 7:07 AM > To: Yao, Jiewen <jiewen.yao@intel.com> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; > leif.lindholm@linaro.org; Zeng, Star <star.zeng@intel.com> > Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit > ProcessCapsules () to be called once > > On 11 June 2018 at 15:55, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > Ah. Good point. > > > > I think the embedded driver dispatch can be controlled by EndOfDxe, which is > similar to MdeModulePkg\Universal\SecurityStubDxe. > > > > So we can have both EndOfDxe and PcdSystemFmpLockEventGuid. > > EndOfDxe controls embedded driver dispatch. > > PcdSystemFmpLockEventGuid controls 1st call and 2nd call. > > > > But in our case, we don't need an event at all. The BDS just calls > ProcessCapsules() whenever it wants to. > > I'm rather relucant to complicate the code with an additional event > that we don't really need, while retaining some of the EndOfDxe > handling as well. Could you think of another way to handle this use > case? > > Thanks, > Ard. > > >> -----Original Message----- > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Ard > >> Biesheuvel > >> Sent: Monday, June 11, 2018 5:40 AM > >> To: Yao, Jiewen <jiewen.yao@intel.com> > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > edk2-devel@lists.01.org; > >> Zeng, Star <star.zeng@intel.com>; leif.lindholm@linaro.org > >> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > permit > >> ProcessCapsules () to be called once > >> > >> On 11 June 2018 at 14:37, Yao, Jiewen <jiewen.yao@intel.com> wrote: > >> > If all fmp can be processed one time，you just need call once. Then system > >> reset. > >> > > >> > >> I understand that. But if the capsule has embedded drivers, the > >> library will only dispatch them on the second call, even if the > >> platform does not require making that distinction. It seems to me that > >> the phasing between the first and the second call is closely tied to > >> the semantics of EndOfDxe and the associated policy on X86 not to > >> allow third party code before it has been signalled. > >> > >> Could we not parameterize this in some way as well? > >> > >> > >> > > >> >> 在 2018年6月11日，上午12:27，Ard Biesheuvel > >> <ard.biesheuvel@linaro.org> 写道： > >> >> > >> >>> On 10 June 2018 at 21:21, Yao, Jiewen <jiewen.yao@intel.com> wrote: > >> >>> Just got some idea since I am also reviewing FmpDevicePkg patch. > >> >>> > >> >>> > >> >>> The key problem we are trying to resolve that is: The core code uses > >> EndOfDxe as the lock event for system firmware, but an ARM platform may > want > >> to lock system firmware later, maybe in other lock event. > >> >>> > >> >>> As such, how about we generalize the lock event as > >> PcdSystemFmpLockEventGuid? We can use EndOfDxeGuid as default one to > keep > >> the compatibility. At same time this brings the flexibility for platform > overriding. > >> >>> > >> >>> > >> >>> We only need update DxeCapsuleLibConstructor() to use the new > PcdGuid, > >> instead of hardcode EndOfDxe. > >> >>> We don't need update the logic in ProcessCapsules() and > >> ProcessTheseCapsules(). > >> >>> 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 > >> configure another lock event GUID explicitly in platform DSC. > >> >>> > >> >> > >> >> But that means we are still required to call ProcessCapsules () twice, no? > >> >> > >> >>>> -----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 <michael.d.kinney@intel.com>; Ard Biesheuvel > >> >>>> <ard.biesheuvel@linaro.org> > >> >>>> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star > >> >>>> <star.zeng@intel.com> > >> >>>> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > >> permit > >> >>>> ProcessCapsules () to be called once > >> >>>> > >> >>>> My concern is that *always allowing* processing SystemCapsule after > >> EndOfDxe > >> >>>> has security risk. > >> >>>> > >> >>>> IMHO, the risk is not *process*, if it is a platform choice. Instead, the > risk is > >> >>>> *allow*, in the core logic. > >> >>>> > >> >>>> If we really want to do that, I hope we need a way to distinguish the > >> difference > >> >>>> between a platform dispatching SystemCapsule after EndOfDxe > >> *purposely* and > >> >>>> a platform dispatching SystemCapsule after EndOfDxe *by mistake*. > >> >>>> > >> >>>> Maybe some policy enforcement in the core logic. Static policy, at build > >> time. > >> >>>> > >> >>>> Thank you > >> >>>> Yao Jiewen > >> >>>> > >> >>>>> -----Original Message----- > >> >>>>> From: Kinney, Michael D > >> >>>>> Sent: Sunday, June 10, 2018 8:57 AM > >> >>>>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Yao, Jiewen > >> >>>>> <jiewen.yao@intel.com>; Kinney, Michael D > >> <michael.d.kinney@intel.com> > >> >>>>> Cc: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; > >> >>>>> leif.lindholm@linaro.org > >> >>>>> Subject: RE: [edk2] [PATCH v2 2/5] > MdeModulePkg/DxeCapsuleLibFmp: > >> permit > >> >>>>> ProcessCapsules () to be called once > >> >>>>> > >> >>>>> Ard, > >> >>>>> > >> >>>>> 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. > >> >>>>> > >> >>>>> 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. > >> >>>>> > >> >>>>> 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. > >> >>>>> > >> >>>>> 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. > >> >>>>> > >> >>>>> I will review your patch in more detail tomorrow. > >> >>>>> > >> >>>>> Thanks, > >> >>>>> > >> >>>>> Mike > >> >>>>> > >> >>>>>> -----Original Message----- > >> >>>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > >> >>>>>> Sent: Saturday, June 9, 2018 10:42 PM > >> >>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> > >> >>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D > >> >>>>>> <michael.d.kinney@intel.com>; Zeng, Star > >> >>>>>> <star.zeng@intel.com>; leif.lindholm@linaro.org > >> >>>>>> Subject: Re: [edk2] [PATCH v2 2/5] > >> >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules > >> >>>>>> () to be called once > >> >>>>>> > >> >>>>>> On 10 June 2018 at 07:38, Yao, Jiewen > >> >>>>>> <jiewen.yao@intel.com> 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" > >> >>>>>>> > >> >>>>>>> In brief, EndOfDxe is signaled before 3rd part code > >> >>>>>> running. > >> >>>>>>> > >> >>>>>>> As such, it is legal that a trusted console is > >> >>>>>> connected before EndOfDxe. > >> >>>>>>> You can report progress to user before EndOfDxe. > >> >>>>>>> > >> >>>>>> > >> >>>>>> 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? > >> >>>>>> > >> >>>>>> > >> >>>>>>>> -----Original Message----- > >> >>>>>>>> From: Ard Biesheuvel > >> >>>>>> [mailto:ard.biesheuvel@linaro.org] > >> >>>>>>>> Sent: Friday, June 8, 2018 8:38 AM > >> >>>>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> > >> >>>>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D > >> >>>>>> <michael.d.kinney@intel.com>; > >> >>>>>>>> Zeng, Star <star.zeng@intel.com>; > >> >>>>>> leif.lindholm@linaro.org > >> >>>>>>>> Subject: Re: [edk2] [PATCH v2 2/5] > >> >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit > >> >>>>>>>> ProcessCapsules () to be called once > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>>> On 8 Jun 2018, at 14:34, Yao, Jiewen > >> >>>>>> <jiewen.yao@intel.com> wrote: > >> >>>>>>>>> > >> >>>>>>>>> Hi Ard > >> >>>>>>>>> We don't allow platform to update system firmware > >> >>>>>> *after* EndOfDxe. > >> >>>>>>>>> > >> >>>>>>>>> According to PI spec, after EndOfDxe, 3rd part > >> >>>>>> code start running. It brings > >> >>>>>>>> security risk if we allow system firmware after > >> >>>>>> EndOfDxe. > >> >>>>>>>>> > >> >>>>>>>>> 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. > >> >>>>>>>>> > >> >>>>>>>>> Would you please share the info, why your platform > >> >>>>>> need update system > >> >>>>>>>> firmware after EndOfDxe? > >> >>>>>>>>> Is that possible to make it earlier? > >> >>>>>>>>> > >> >>>>>>>>> > >> >>>>>>>> > >> >>>>>>>> Because we need some kind of console to report > >> >>>>>> progress to the user. > >> >>>>>>>> > >> >>>>>>>> The secure platform execution context is completely > >> >>>>>> separate from UEFI on Arm, > >> >>>>>>>> so for us the distinction does not make sense. > >> >>>>>>>> > >> >>>>>>>>> Thank you > >> >>>>>>>>> Yao Jiewen > >> >>>>>>>>> > >> >>>>>>>>>> -----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 > >> >>>>>> <michael.d.kinney@intel.com>; Yao, Jiewen > >> >>>>>>>>>> <jiewen.yao@intel.com>; Zeng, Star > >> >>>>>> <star.zeng@intel.com>; > >> >>>>>>>>>> leif.lindholm@linaro.org; Ard Biesheuvel > >> >>>>>> <ard.biesheuvel@linaro.org> > >> >>>>>>>>>> Subject: [edk2] [PATCH v2 2/5] > >> >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit > >> >>>>>>>>>> ProcessCapsules () to be called once > >> >>>>>>>>>> > >> >>>>>>>>>> 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. > >> >>>>>>>>>> > >> >>>>>>>>>> Contributed-under: TianoCore Contribution > >> >>>>>> Agreement 1.1 > >> >>>>>>>>>> Signed-off-by: Ard Biesheuvel > >> >>>>>> <ard.biesheuvel@linaro.org> > >> >>>>>>>>>> --- > >> >>>>>>>>>> > >> >>>>>> MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess > >> >>>>>> Lib.c | 18 > >> >>>>>>>>>> ++++++++++++------ > >> >>>>>>>>>> 1 file changed, 12 insertions(+), 6 deletions(-) > >> >>>>>>>>>> > >> >>>>>>>>>> diff --git > >> >>>>>>>> > >> >>>>>> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce > >> >>>>>> ssLib.c > >> >>>>>>>>>> > >> >>>>>> 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 ( > >> >>>>>>>>>> > >> >>>>>>>>>> extern BOOLEAN > >> >>>>>> mDxeCapsuleLibEndOfDxe; > >> >>>>>>>>>> BOOLEAN mNeedReset; > >> >>>>>>>>>> +BOOLEAN mFirstRound = > >> >>>>>> TRUE; > >> >>>>>>>>>> > >> >>>>>>>>>> VOID **mCapsulePtr; > >> >>>>>>>>>> EFI_STATUS *mCapsuleStatusArray; > >> >>>>>>>>>> @@ -364,8 +365,11 @@ > >> >>>>>> PopulateCapsuleInConfigurationTable ( > >> >>>>>>>>>> > >> >>>>>>>>>> Each individual capsule result is recorded in > >> >>>>>> capsule record variable. > >> >>>>>>>>>> > >> >>>>>>>>>> - @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. > >> >>>>>>>>>> > >> >>>>>>>>>> @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; > >> >>>>>>>>>> } > >> >>>>>>>>>> > >> >>>>>>>>>> - if ((!FirstRound) || (EmbeddedDriverCount > >> >>>>>> == 0)) { > >> >>>>>>>>>> + if (LastRound || (EmbeddedDriverCount == > >> >>>>>> 0)) { > >> >>>>>>>>>> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - > >> >>>>>> 0x%x\n", > >> >>>>>>>>>> CapsuleHeader)); > >> >>>>>>>>>> Status = ProcessCapsuleImage > >> >>>>>> (CapsuleHeader); > >> >>>>>>>>>> mCapsuleStatusArray [Index] = Status; > >> >>>>>>>>>> @@ -546,7 +551,7 @@ ProcessCapsules ( > >> >>>>>>>>>> EFI_STATUS Status; > >> >>>>>>>>>> > >> >>>>>>>>>> if (!mDxeCapsuleLibEndOfDxe) { > >> >>>>>>>>>> - Status = ProcessTheseCapsules(TRUE); > >> >>>>>>>>>> + Status = ProcessTheseCapsules(TRUE, FALSE); > >> >>>>>>>>>> > >> >>>>>>>>>> // > >> >>>>>>>>>> // Reboot System if and only if all capsule > >> >>>>>> processed. > >> >>>>>>>>>> @@ -555,8 +560,9 @@ ProcessCapsules ( > >> >>>>>>>>>> if (mNeedReset && AreAllImagesProcessed()) { > >> >>>>>>>>>> DoResetSystem(); > >> >>>>>>>>>> } > >> >>>>>>>>>> + mFirstRound = FALSE; > >> >>>>>>>>>> } else { > >> >>>>>>>>>> - Status = ProcessTheseCapsules(FALSE); > >> >>>>>>>>>> + Status = ProcessTheseCapsules(mFirstRound, > >> >>>>>> TRUE); > >> >>>>>>>>>> // > >> >>>>>>>>>> // Reboot System if required after all > >> >>>>>> capsule processed > >> >>>>>>>>>> // > >> >>>>>>>>>> -- > >> >>>>>>>>>> 2.17.0 > >> >>>>>>>>>> > >> >>>>>>>>>> _______________________________________________ > >> >>>>>>>>>> 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 > >> _______________________________________________ > >> 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
I am thinking that could platform BDS parse CAPSULE HOB and call ProcessCapsuleImage by itself in its willing phase with flexibility for the case? Thanks, Star -----Original Message----- From: Yao, Jiewen Sent: Monday, June 11, 2018 11:12 PM To: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star <star.zeng@intel.com> Subject: RE: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules () to be called once The difference between 1st call and 2nd call is that: The system will reset even there are capsules not dispatched in 2nd call. If a platform just calls once and there is 1 device FMP not dispatched yet, how the core lib knows it should reset the system or it should wait for the second call after connect all? If you have any thought, please share with us. Summarize some requirement: 1) Embedded driver must be dispatched after EndOfDxe. 2) System Capsule must be dispatched before system firmware lock. (Ideally it should be EndOfDxe, a platform may choose others) 3) When the system capsule is dispatched, the driver FMP might be ready or might not be ready yet. 4) The core code need make sure the platform code dispatch the capsule in a right time. Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: Monday, June 11, 2018 7:07 AM > To: Yao, Jiewen <jiewen.yao@intel.com> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star > <star.zeng@intel.com> > Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > permit ProcessCapsules () to be called once > > On 11 June 2018 at 15:55, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > Ah. Good point. > > > > I think the embedded driver dispatch can be controlled by EndOfDxe, > > which is > similar to MdeModulePkg\Universal\SecurityStubDxe. > > > > So we can have both EndOfDxe and PcdSystemFmpLockEventGuid. > > EndOfDxe controls embedded driver dispatch. > > PcdSystemFmpLockEventGuid controls 1st call and 2nd call. > > > > But in our case, we don't need an event at all. The BDS just calls > ProcessCapsules() whenever it wants to. > > I'm rather relucant to complicate the code with an additional event > that we don't really need, while retaining some of the EndOfDxe > handling as well. Could you think of another way to handle this use > case? > > Thanks, > Ard. > > >> -----Original Message----- > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf > >> Of > Ard > >> Biesheuvel > >> Sent: Monday, June 11, 2018 5:40 AM > >> To: Yao, Jiewen <jiewen.yao@intel.com> > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > edk2-devel@lists.01.org; > >> Zeng, Star <star.zeng@intel.com>; leif.lindholm@linaro.org > >> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > permit > >> ProcessCapsules () to be called once > >> > >> On 11 June 2018 at 14:37, Yao, Jiewen <jiewen.yao@intel.com> wrote: > >> > If all fmp can be processed one time，you just need call once. > >> > Then system > >> reset. > >> > > >> > >> I understand that. But if the capsule has embedded drivers, the > >> library will only dispatch them on the second call, even if the > >> platform does not require making that distinction. It seems to me > >> that the phasing between the first and the second call is closely > >> tied to the semantics of EndOfDxe and the associated policy on X86 > >> not to allow third party code before it has been signalled. > >> > >> Could we not parameterize this in some way as well? > >> > >> > >> > > >> >> 在 2018年6月11日，上午12:27，Ard Biesheuvel > >> <ard.biesheuvel@linaro.org> 写道： > >> >> > >> >>> On 10 June 2018 at 21:21, Yao, Jiewen <jiewen.yao@intel.com> wrote: > >> >>> Just got some idea since I am also reviewing FmpDevicePkg patch. > >> >>> > >> >>> > >> >>> The key problem we are trying to resolve that is: The core code > >> >>> uses > >> EndOfDxe as the lock event for system firmware, but an ARM platform > >> may > want > >> to lock system firmware later, maybe in other lock event. > >> >>> > >> >>> As such, how about we generalize the lock event as > >> PcdSystemFmpLockEventGuid? We can use EndOfDxeGuid as default one > >> to > keep > >> the compatibility. At same time this brings the flexibility for > >> platform > overriding. > >> >>> > >> >>> > >> >>> We only need update DxeCapsuleLibConstructor() to use the new > PcdGuid, > >> instead of hardcode EndOfDxe. > >> >>> We don't need update the logic in ProcessCapsules() and > >> ProcessTheseCapsules(). > >> >>> 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 > >> configure another lock event GUID explicitly in platform DSC. > >> >>> > >> >> > >> >> But that means we are still required to call ProcessCapsules () twice, no? > >> >> > >> >>>> -----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 <michael.d.kinney@intel.com>; Ard > >> >>>> Biesheuvel <ard.biesheuvel@linaro.org> > >> >>>> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, > >> >>>> Star <star.zeng@intel.com> > >> >>>> Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > >> permit > >> >>>> ProcessCapsules () to be called once > >> >>>> > >> >>>> My concern is that *always allowing* processing SystemCapsule > >> >>>> after > >> EndOfDxe > >> >>>> has security risk. > >> >>>> > >> >>>> IMHO, the risk is not *process*, if it is a platform choice. > >> >>>> Instead, the > risk is > >> >>>> *allow*, in the core logic. > >> >>>> > >> >>>> If we really want to do that, I hope we need a way to > >> >>>> distinguish the > >> difference > >> >>>> between a platform dispatching SystemCapsule after EndOfDxe > >> *purposely* and > >> >>>> a platform dispatching SystemCapsule after EndOfDxe *by mistake*. > >> >>>> > >> >>>> Maybe some policy enforcement in the core logic. Static > >> >>>> policy, at build > >> time. > >> >>>> > >> >>>> Thank you > >> >>>> Yao Jiewen > >> >>>> > >> >>>>> -----Original Message----- > >> >>>>> From: Kinney, Michael D > >> >>>>> Sent: Sunday, June 10, 2018 8:57 AM > >> >>>>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Yao, Jiewen > >> >>>>> <jiewen.yao@intel.com>; Kinney, Michael D > >> <michael.d.kinney@intel.com> > >> >>>>> Cc: edk2-devel@lists.01.org; Zeng, Star > >> >>>>> <star.zeng@intel.com>; leif.lindholm@linaro.org > >> >>>>> Subject: RE: [edk2] [PATCH v2 2/5] > MdeModulePkg/DxeCapsuleLibFmp: > >> permit > >> >>>>> ProcessCapsules () to be called once > >> >>>>> > >> >>>>> Ard, > >> >>>>> > >> >>>>> 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. > >> >>>>> > >> >>>>> 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. > >> >>>>> > >> >>>>> 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. > >> >>>>> > >> >>>>> 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. > >> >>>>> > >> >>>>> I will review your patch in more detail tomorrow. > >> >>>>> > >> >>>>> Thanks, > >> >>>>> > >> >>>>> Mike > >> >>>>> > >> >>>>>> -----Original Message----- > >> >>>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > >> >>>>>> Sent: Saturday, June 9, 2018 10:42 PM > >> >>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> > >> >>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D > >> >>>>>> <michael.d.kinney@intel.com>; Zeng, Star > >> >>>>>> <star.zeng@intel.com>; leif.lindholm@linaro.org > >> >>>>>> Subject: Re: [edk2] [PATCH v2 2/5] > >> >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules > >> >>>>>> () to be called once > >> >>>>>> > >> >>>>>> On 10 June 2018 at 07:38, Yao, Jiewen <jiewen.yao@intel.com> > >> >>>>>> 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" > >> >>>>>>> > >> >>>>>>> In brief, EndOfDxe is signaled before 3rd part code > >> >>>>>> running. > >> >>>>>>> > >> >>>>>>> As such, it is legal that a trusted console is > >> >>>>>> connected before EndOfDxe. > >> >>>>>>> You can report progress to user before EndOfDxe. > >> >>>>>>> > >> >>>>>> > >> >>>>>> 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? > >> >>>>>> > >> >>>>>> > >> >>>>>>>> -----Original Message----- > >> >>>>>>>> From: Ard Biesheuvel > >> >>>>>> [mailto:ard.biesheuvel@linaro.org] > >> >>>>>>>> Sent: Friday, June 8, 2018 8:38 AM > >> >>>>>>>> To: Yao, Jiewen <jiewen.yao@intel.com> > >> >>>>>>>> Cc: edk2-devel@lists.01.org; Kinney, Michael D > >> >>>>>> <michael.d.kinney@intel.com>; > >> >>>>>>>> Zeng, Star <star.zeng@intel.com>; > >> >>>>>> leif.lindholm@linaro.org > >> >>>>>>>> Subject: Re: [edk2] [PATCH v2 2/5] > >> >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit > >> >>>>>>>> ProcessCapsules () to be called once > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>>> On 8 Jun 2018, at 14:34, Yao, Jiewen > >> >>>>>> <jiewen.yao@intel.com> wrote: > >> >>>>>>>>> > >> >>>>>>>>> Hi Ard > >> >>>>>>>>> We don't allow platform to update system firmware > >> >>>>>> *after* EndOfDxe. > >> >>>>>>>>> > >> >>>>>>>>> According to PI spec, after EndOfDxe, 3rd part > >> >>>>>> code start running. It brings > >> >>>>>>>> security risk if we allow system firmware after > >> >>>>>> EndOfDxe. > >> >>>>>>>>> > >> >>>>>>>>> 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. > >> >>>>>>>>> > >> >>>>>>>>> Would you please share the info, why your platform > >> >>>>>> need update system > >> >>>>>>>> firmware after EndOfDxe? > >> >>>>>>>>> Is that possible to make it earlier? > >> >>>>>>>>> > >> >>>>>>>>> > >> >>>>>>>> > >> >>>>>>>> Because we need some kind of console to report > >> >>>>>> progress to the user. > >> >>>>>>>> > >> >>>>>>>> The secure platform execution context is completely > >> >>>>>> separate from UEFI on Arm, > >> >>>>>>>> so for us the distinction does not make sense. > >> >>>>>>>> > >> >>>>>>>>> Thank you > >> >>>>>>>>> Yao Jiewen > >> >>>>>>>>> > >> >>>>>>>>>> -----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 > >> >>>>>> <michael.d.kinney@intel.com>; Yao, Jiewen > >> >>>>>>>>>> <jiewen.yao@intel.com>; Zeng, Star > >> >>>>>> <star.zeng@intel.com>; > >> >>>>>>>>>> leif.lindholm@linaro.org; Ard Biesheuvel > >> >>>>>> <ard.biesheuvel@linaro.org> > >> >>>>>>>>>> Subject: [edk2] [PATCH v2 2/5] > >> >>>>>> MdeModulePkg/DxeCapsuleLibFmp: permit > >> >>>>>>>>>> ProcessCapsules () to be called once > >> >>>>>>>>>> > >> >>>>>>>>>> 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. > >> >>>>>>>>>> > >> >>>>>>>>>> Contributed-under: TianoCore Contribution > >> >>>>>> Agreement 1.1 > >> >>>>>>>>>> Signed-off-by: Ard Biesheuvel > >> >>>>>> <ard.biesheuvel@linaro.org> > >> >>>>>>>>>> --- > >> >>>>>>>>>> > >> >>>>>> MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcess > >> >>>>>> Lib.c | 18 > >> >>>>>>>>>> ++++++++++++------ > >> >>>>>>>>>> 1 file changed, 12 insertions(+), 6 deletions(-) > >> >>>>>>>>>> > >> >>>>>>>>>> diff --git > >> >>>>>>>> > >> >>>>>> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProce > >> >>>>>> ssLib.c > >> >>>>>>>>>> > >> >>>>>> 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 ( > >> >>>>>>>>>> > >> >>>>>>>>>> extern BOOLEAN > >> >>>>>> mDxeCapsuleLibEndOfDxe; > >> >>>>>>>>>> BOOLEAN mNeedReset; > >> >>>>>>>>>> +BOOLEAN mFirstRound = > >> >>>>>> TRUE; > >> >>>>>>>>>> > >> >>>>>>>>>> VOID **mCapsulePtr; > >> >>>>>>>>>> EFI_STATUS *mCapsuleStatusArray; > >> >>>>>>>>>> @@ -364,8 +365,11 @@ > >> >>>>>> PopulateCapsuleInConfigurationTable ( > >> >>>>>>>>>> > >> >>>>>>>>>> Each individual capsule result is recorded in > >> >>>>>> capsule record variable. > >> >>>>>>>>>> > >> >>>>>>>>>> - @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. > >> >>>>>>>>>> > >> >>>>>>>>>> @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; > >> >>>>>>>>>> } > >> >>>>>>>>>> > >> >>>>>>>>>> - if ((!FirstRound) || (EmbeddedDriverCount > >> >>>>>> == 0)) { > >> >>>>>>>>>> + if (LastRound || (EmbeddedDriverCount == > >> >>>>>> 0)) { > >> >>>>>>>>>> DEBUG((DEBUG_INFO, "ProcessCapsuleImage - > >> >>>>>> 0x%x\n", > >> >>>>>>>>>> CapsuleHeader)); > >> >>>>>>>>>> Status = ProcessCapsuleImage > >> >>>>>> (CapsuleHeader); > >> >>>>>>>>>> mCapsuleStatusArray [Index] = Status; > >> >>>>>>>>>> @@ -546,7 +551,7 @@ ProcessCapsules ( > >> >>>>>>>>>> EFI_STATUS Status; > >> >>>>>>>>>> > >> >>>>>>>>>> if (!mDxeCapsuleLibEndOfDxe) { > >> >>>>>>>>>> - Status = ProcessTheseCapsules(TRUE); > >> >>>>>>>>>> + Status = ProcessTheseCapsules(TRUE, FALSE); > >> >>>>>>>>>> > >> >>>>>>>>>> // > >> >>>>>>>>>> // Reboot System if and only if all capsule > >> >>>>>> processed. > >> >>>>>>>>>> @@ -555,8 +560,9 @@ ProcessCapsules ( > >> >>>>>>>>>> if (mNeedReset && AreAllImagesProcessed()) { > >> >>>>>>>>>> DoResetSystem(); > >> >>>>>>>>>> } > >> >>>>>>>>>> + mFirstRound = FALSE; > >> >>>>>>>>>> } else { > >> >>>>>>>>>> - Status = ProcessTheseCapsules(FALSE); > >> >>>>>>>>>> + Status = ProcessTheseCapsules(mFirstRound, > >> >>>>>> TRUE); > >> >>>>>>>>>> // > >> >>>>>>>>>> // Reboot System if required after all > >> >>>>>> capsule processed > >> >>>>>>>>>> // > >> >>>>>>>>>> -- > >> >>>>>>>>>> 2.17.0 > >> >>>>>>>>>> > >> >>>>>>>>>> _______________________________________________ > >> >>>>>>>>>> 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 > >> _______________________________________________ > >> 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
Hi Jiewen, I don't know x86 in details , so ignore stupid question, Also thanks to bear with unrelated question. > 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. On x86, we have SMM mode and I think this is assumed to be secured . What is restriction that flash update is done after reset. This could be done in same call of OS in SMM mode. Regards Udit > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Yao, Jiewen > Sent: Friday, June 8, 2018 6:05 PM > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > leif.lindholm@linaro.org; Zeng, Star <star.zeng@intel.com> > Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > permit ProcessCapsules () to be called once > > Hi Ard > We don't allow platform to update system firmware *after* EndOfDxe. > > According to PI spec, after EndOfDxe, 3rd part code start running. It brings > security risk if we allow system firmware after EndOfDxe. > > 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. > > Would you please share the info, why your platform need update system > firmware after EndOfDxe? > Is that possible to make it earlier? > > > Thank you > Yao Jiewen > > > -----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 <michael.d.kinney@intel.com>; Yao, Jiewen > > <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; > > leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Subject: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > permit > > ProcessCapsules () to be called once > > > > 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. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 18 > > ++++++++++++------ > > 1 file changed, 12 insertions(+), 6 deletions(-) > > > > diff --git > > a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > > index 26ca4e295f20..ad83660f1737 100644 > > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > > +++ > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > > @@ -100,6 +100,7 @@ IsValidCapsuleHeader ( > > > > extern BOOLEAN mDxeCapsuleLibEndOfDxe; > > BOOLEAN mNeedReset; > > +BOOLEAN mFirstRound = TRUE; > > > > VOID **mCapsulePtr; > > EFI_STATUS *mCapsuleStatusArray; > > @@ -364,8 +365,11 @@ PopulateCapsuleInConfigurationTable ( > > > > Each individual capsule result is recorded in capsule record variable. > > > > - @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. > > > > @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; > > } > > > > - if ((!FirstRound) || (EmbeddedDriverCount == 0)) { > > + if (LastRound || (EmbeddedDriverCount == 0)) { > > DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", > > CapsuleHeader)); > > Status = ProcessCapsuleImage (CapsuleHeader); > > mCapsuleStatusArray [Index] = Status; @@ -546,7 +551,7 @@ > > ProcessCapsules ( > > EFI_STATUS Status; > > > > if (!mDxeCapsuleLibEndOfDxe) { > > - Status = ProcessTheseCapsules(TRUE); > > + Status = ProcessTheseCapsules(TRUE, FALSE); > > > > // > > // Reboot System if and only if all capsule processed. > > @@ -555,8 +560,9 @@ ProcessCapsules ( > > if (mNeedReset && AreAllImagesProcessed()) { > > DoResetSystem(); > > } > > + mFirstRound = FALSE; > > } else { > > - Status = ProcessTheseCapsules(FALSE); > > + Status = ProcessTheseCapsules(mFirstRound, TRUE); > > // > > // Reboot System if required after all capsule processed > > // > > -- > > 2.17.0 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis > > ts.01.org%2Fmailman%2Flistinfo%2Fedk2- > devel&data=02%7C01%7Cudit.kumar% > > > 40nxp.com%7C0701a8fc1bd5448675df08d5cd3c3827%7C686ea1d3bc2b4 > c6fa92cd99 > > > c5c301635%7C0%7C0%7C636640580906249469&sdata=Fi56Xg%2B1p5e6 > 9qbD5ITsw8v > > ve397ThhomLr9wcY9Ljc%3D&reserved=0 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis > ts.01.org%2Fmailman%2Flistinfo%2Fedk2- > devel&data=02%7C01%7Cudit.kumar%40nxp.com%7C0701a8fc1bd544867 > 5df08d5cd3c3827%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > C636640580906249469&sdata=Fi56Xg%2B1p5e69qbD5ITsw8vve397Thho > mLr9wcY9Ljc%3D&reserved=0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Good question. That is because there may be some flash hardware feature to lock the flash part early, and unlockable even in SMM. As defense in depth, SMM protection can still be enabled. And the flash update must happen before EndOfDxe. Thank You Yao Jiewen > -----Original Message----- > From: Udit Kumar [mailto:udit.kumar@nxp.com] > Sent: Monday, June 18, 2018 3:36 AM > To: Yao, Jiewen <jiewen.yao@intel.com> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Kinney, Michael D > <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; > leif.lindholm@linaro.org; Zeng, Star <star.zeng@intel.com> > Subject: RE: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit > ProcessCapsules () to be called once > > Hi Jiewen, > > I don't know x86 in details , so ignore stupid question, > Also thanks to bear with unrelated question. > > > 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. > > On x86, we have SMM mode and I think this is assumed to be secured . > What is restriction that flash update is done after reset. This could be > done in same call of OS in SMM mode. > > Regards > Udit > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > > Yao, Jiewen > > Sent: Friday, June 8, 2018 6:05 PM > > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > > leif.lindholm@linaro.org; Zeng, Star <star.zeng@intel.com> > > Subject: Re: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > > permit ProcessCapsules () to be called once > > > > Hi Ard > > We don't allow platform to update system firmware *after* EndOfDxe. > > > > According to PI spec, after EndOfDxe, 3rd part code start running. It brings > > security risk if we allow system firmware after EndOfDxe. > > > > 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. > > > > Would you please share the info, why your platform need update system > > firmware after EndOfDxe? > > Is that possible to make it earlier? > > > > > > Thank you > > Yao Jiewen > > > > > -----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 <michael.d.kinney@intel.com>; Yao, Jiewen > > > <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; > > > leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > Subject: [edk2] [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: > > permit > > > ProcessCapsules () to be called once > > > > > > 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. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > --- > > > MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 18 > > > ++++++++++++------ > > > 1 file changed, 12 insertions(+), 6 deletions(-) > > > > > > diff --git > > > a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > > > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > > > index 26ca4e295f20..ad83660f1737 100644 > > > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > > > +++ > > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > > > @@ -100,6 +100,7 @@ IsValidCapsuleHeader ( > > > > > > extern BOOLEAN mDxeCapsuleLibEndOfDxe; > > > BOOLEAN mNeedReset; > > > +BOOLEAN mFirstRound = TRUE; > > > > > > VOID **mCapsulePtr; > > > EFI_STATUS *mCapsuleStatusArray; > > > @@ -364,8 +365,11 @@ PopulateCapsuleInConfigurationTable ( > > > > > > Each individual capsule result is recorded in capsule record variable. > > > > > > - @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. > > > > > > @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; > > > } > > > > > > - if ((!FirstRound) || (EmbeddedDriverCount == 0)) { > > > + if (LastRound || (EmbeddedDriverCount == 0)) { > > > DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", > > > CapsuleHeader)); > > > Status = ProcessCapsuleImage (CapsuleHeader); > > > mCapsuleStatusArray [Index] = Status; @@ -546,7 +551,7 @@ > > > ProcessCapsules ( > > > EFI_STATUS Status; > > > > > > if (!mDxeCapsuleLibEndOfDxe) { > > > - Status = ProcessTheseCapsules(TRUE); > > > + Status = ProcessTheseCapsules(TRUE, FALSE); > > > > > > // > > > // Reboot System if and only if all capsule processed. > > > @@ -555,8 +560,9 @@ ProcessCapsules ( > > > if (mNeedReset && AreAllImagesProcessed()) { > > > DoResetSystem(); > > > } > > > + mFirstRound = FALSE; > > > } else { > > > - Status = ProcessTheseCapsules(FALSE); > > > + Status = ProcessTheseCapsules(mFirstRound, TRUE); > > > // > > > // Reboot System if required after all capsule processed > > > // > > > -- > > > 2.17.0 > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.01.org > > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis > > > ts.01.org%2Fmailman%2Flistinfo%2Fedk2- > > devel&data=02%7C01%7Cudit.kumar% > > > > > 40nxp.com%7C0701a8fc1bd5448675df08d5cd3c3827%7C686ea1d3bc2b4 > > c6fa92cd99 > > > > > c5c301635%7C0%7C0%7C636640580906249469&sdata=Fi56Xg%2B1p5e6 > > 9qbD5ITsw8v > > > ve397ThhomLr9wcY9Ljc%3D&reserved=0 > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis > > ts.01.org%2Fmailman%2Flistinfo%2Fedk2- > > devel&data=02%7C01%7Cudit.kumar%40nxp.com%7C0701a8fc1bd544867 > > 5df08d5cd3c3827%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > > C636640580906249469&sdata=Fi56Xg%2B1p5e69qbD5ITsw8vve397Thho > > mLr9wcY9Ljc%3D&reserved=0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c index 26ca4e295f20..ad83660f1737 100644 --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c @@ -100,6 +100,7 @@ IsValidCapsuleHeader ( extern BOOLEAN mDxeCapsuleLibEndOfDxe; BOOLEAN mNeedReset; +BOOLEAN mFirstRound = TRUE; VOID **mCapsulePtr; EFI_STATUS *mCapsuleStatusArray; @@ -364,8 +365,11 @@ PopulateCapsuleInConfigurationTable ( Each individual capsule result is recorded in capsule record variable. - @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. @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; } - if ((!FirstRound) || (EmbeddedDriverCount == 0)) { + if (LastRound || (EmbeddedDriverCount == 0)) { DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", CapsuleHeader)); Status = ProcessCapsuleImage (CapsuleHeader); mCapsuleStatusArray [Index] = Status; @@ -546,7 +551,7 @@ ProcessCapsules ( EFI_STATUS Status; if (!mDxeCapsuleLibEndOfDxe) { - Status = ProcessTheseCapsules(TRUE); + Status = ProcessTheseCapsules(TRUE, FALSE); // // Reboot System if and only if all capsule processed. @@ -555,8 +560,9 @@ ProcessCapsules ( if (mNeedReset && AreAllImagesProcessed()) { DoResetSystem(); } + mFirstRound = FALSE; } else { - Status = ProcessTheseCapsules(FALSE); + Status = ProcessTheseCapsules(mFirstRound, TRUE); // // Reboot System if required after all capsule processed //
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. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) -- 2.17.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel