Re: [PATCH v2 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules () to be called once

["Yao, Jiewen" <jiewen.yao@intel.com>]

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