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

[Udit Kumar <udit.kumar@nxp.com>]

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