From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::242; helo=mail-io0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x242.google.com (mail-io0-x242.google.com [IPv6:2607:f8b0:4001:c06::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id AD2FC210DF771 for ; Thu, 7 Jun 2018 23:29:51 -0700 (PDT) Received: by mail-io0-x242.google.com with SMTP id l19-v6so14678153ioj.5 for ; Thu, 07 Jun 2018 23:29:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=vvhK2XxskQfIIdLAguy+keeoGJZkjitgSeGQnwNzsJQ=; b=aew3tIyMEBE3jnbqnEAQ1MIZpWwjJ6p+mNuhj6UR0yMqNarRvBGphhjfaQZo9zhya9 HbktUqfg+Ems2rUppQaTFHkKb7AqBTEKkOEk91be0Kl5Bj4NBG2wZNEBcBiSw/lO2WJk qH4hEvDpDmaPo9GaxWsr9oY6jP7k+2ruwjn38= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=vvhK2XxskQfIIdLAguy+keeoGJZkjitgSeGQnwNzsJQ=; b=mLjI32F8juirmVpJMFtVqcB7x7qzQBwSxXhFcFN0bYG4TCIlAquko1gQC3gER/WxsG 34uGDTLgxnN8bAw30+vBdcHjeVJHrEbf5/qLxK8O4Gz6bgvGFie1UoQp1hKgfsjWoYx0 KO6f0wjIB+8lW2f08O2P6pxhYdl/c94FijKtteob65wIJCFCdZBifHW7yciiz0lpoBFK PvNIUjKdJNtyKFQb6J0YTtYWMxOAXSK7V/6IY5bOdYYez/ghoXA1EfcS8rf96CNDM5Am Ut5JEld359L4Ae8IssRTkCVD76bYzwgdy2iHxlYFcUXTkZdRplTbcyJi8EQTjP29Ornt y/cg== X-Gm-Message-State: APt69E1/U2Loq/TQY6Cce6/j+DryLs2GxXrdm46Z7ZGo762w5WMjynep 26F30mfL7q3W9yRZx44BY+KvQpgbABqDOATORZRU1Q== X-Google-Smtp-Source: ADUXVKJm/Vvu2Uh9Lh3cJh3vgM2YxaAKWXRmpBvEnNPiEcj0u33USh0hCOSaBDvOEE2+xREE+lh07kySY3cUpFkxSdQ= X-Received: by 2002:a6b:520d:: with SMTP id g13-v6mr2983602iob.60.1528439390928; Thu, 07 Jun 2018 23:29:50 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Thu, 7 Jun 2018 23:29:50 -0700 (PDT) In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BB54B6F@shsmsx102.ccr.corp.intel.com> References: <20180607110812.26778-1-ard.biesheuvel@linaro.org> <20180607110812.26778-3-ard.biesheuvel@linaro.org> <0C09AFA07DD0434D9E2A0C6AEB0483103BB54B6F@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Fri, 8 Jun 2018 08:29:50 +0200 Message-ID: To: "Zeng, Star" Cc: "edk2-devel@lists.01.org" , "leif.lindholm@linaro.org" , "Kinney, Michael D" , "Yao, Jiewen" Subject: Re: [PATCH 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules () to be called once X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Jun 2018 06:29:51 -0000 Content-Type: text/plain; charset="UTF-8" On 8 June 2018 at 04:57, Zeng, Star wrote: > Without the patch, PopulateCapsuleInConfigurationTable is only run at first round. > With the patch, PopulateCapsuleInConfigurationTable is only run at last round. > > Is that expected? > Yes. Otherwise, I need two global BOOLEANs to keep track of the state. However, I just noticed that we may now end up in a situation where PopulateCapsuleInConfigurationTable() never gets called if all capsules are processed in the first pass. I will fix and resend. > Jiewen, could you help check whether the patch meets the original design purpose or any security concern? > > > Thanks, > Star > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Thursday, June 7, 2018 7:08 PM > To: edk2-devel@lists.01.org > Cc: leif.lindholm@linaro.org; Kinney, Michael D ; Yao, Jiewen ; Zeng, Star ; Ard Biesheuvel > Subject: [PATCH 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 > --- > MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > index 26ca4e295f20..52691fa68be4 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,10 @@ 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] LastRound FALSE: First of multiple 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 +376,7 @@ PopulateCapsuleInConfigurationTable ( **/ EFI_STATUS ProcessTheseCapsules ( > - IN BOOLEAN FirstRound > + IN BOOLEAN LastRound > ) > { > EFI_STATUS Status; > @@ -384,8 +387,9 @@ ProcessTheseCapsules ( > > REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeProcessCapsulesBegin))); > > - if (FirstRound) { > + if (mFirstRound) { > InitCapsulePtr (); > + mFirstRound = FALSE; > } > > if (mCapsuleTotalNumber == 0) { > @@ -404,7 +408,7 @@ ProcessTheseCapsules ( > // Check the capsule flags,if contains CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE, install > // capsuleTable to configure table with EFI_CAPSULE_GUID > // > - if (FirstRound) { > + if (LastRound) { > PopulateCapsuleInConfigurationTable (); > } > > @@ -453,7 +457,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 +550,7 @@ ProcessCapsules ( > EFI_STATUS Status; > > if (!mDxeCapsuleLibEndOfDxe) { > - Status = ProcessTheseCapsules(TRUE); > + Status = ProcessTheseCapsules(FALSE); > > // > // Reboot System if and only if all capsule processed. > @@ -556,7 +560,7 @@ ProcessCapsules ( > DoResetSystem(); > } > } else { > - Status = ProcessTheseCapsules(FALSE); > + Status = ProcessTheseCapsules(TRUE); > // > // Reboot System if required after all capsule processed > // > -- > 2.17.0 >