From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=jiewen.yao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6CB31210C6434 for ; Mon, 30 Jul 2018 17:45:21 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Jul 2018 17:45:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,425,1526367600"; d="scan'208";a="58414933" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga007.fm.intel.com with ESMTP; 30 Jul 2018 17:45:16 -0700 Received: from fmsmsx113.amr.corp.intel.com (10.18.116.7) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 30 Jul 2018 17:45:16 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX113.amr.corp.intel.com (10.18.116.7) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 30 Jul 2018 17:45:15 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.124]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.57]) with mapi id 14.03.0319.002; Tue, 31 Jul 2018 08:45:13 +0800 From: "Yao, Jiewen" To: "Zeng, Star" , "edk2-devel@lists.01.org" CC: "Kinney, Michael D" Thread-Topic: [PATCH] MdeModulePkg DxeCapsuleLib: Use Attr to know whether reset is required Thread-Index: AQHUJ+LzHfT8QjidLEWLuWwEPJEJvKSof59g Date: Tue, 31 Jul 2018 00:45:12 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503ACD8219@shsmsx102.ccr.corp.intel.com> References: <1532940864-29952-1-git-send-email-star.zeng@intel.com> In-Reply-To: <1532940864-29952-1-git-send-email-star.zeng@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTdlNTQ0ZTgtNTRiZi00ZDM3LThlZTAtNjJiZjVkY2JkZGE3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUW03Vnc3Y290XC9BSmJBYmZVSDlOTUYyZDNRc2NGTm14Rk0rRmJ4WTVZSTNOemQ1U09wVGFrUTdabmVLNGladW0ifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg DxeCapsuleLib: Use Attr to know whether reset is required X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 Jul 2018 00:45:21 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Jiewen.yao@intel.com > -----Original Message----- > From: Zeng, Star > Sent: Monday, July 30, 2018 4:54 PM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Yao, Jiewen ; > Kinney, Michael D > Subject: [PATCH] MdeModulePkg DxeCapsuleLib: Use Attr to know whether > reset is required >=20 > Current DxeCapsuleLibFmp always do reset for FMP capsule. > Actually, the code should use Attributes from FMP descriptor to know > whether reset is required or not. >=20 > Cc: Jiewen Yao > Cc: Michael D Kinney > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Star Zeng > --- > .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | 134 > ++++++++++++++++----- > .../DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 40 ++++-- > 2 files changed, 134 insertions(+), 40 deletions(-) >=20 > diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c > index e88dab8c7642..d00d7d88ff7d 100644 > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c > @@ -620,11 +620,14 @@ DumpAllFmpInfo ( >=20 > @param[in] UpdateImageTypeId Used to identify device > firmware targeted by this update. > @param[in] UpdateHardwareInstance The HardwareInstance to > target with this update. > - @param[in,out] NoHandles The number of handles > returned in Buffer. > - @param[out] Buffer[out] A pointer to the buffer to retu= rn > the requested array of handles. > - > - @retval EFI_SUCCESS The array of handles was returned in > Buffer, and the number of > - handles in Buffer was returned in > NoHandles. > + @param[out] NoHandles The number of handles > returned in HandleBuf. > + @param[out] HandleBuf A pointer to the buffer to > return the requested array of handles. > + @param[out] ResetRequiredBuf A pointer to the buffer to > return reset required flag for > + the requested array of > handles. > + > + @retval EFI_SUCCESS The array of handles and their reset > required flag were returned in > + HandleBuf and ResetRequiredBuf, and > the number of handles in HandleBuf > + was returned in NoHandles. > @retval EFI_NOT_FOUND No handles match the search. > @retval EFI_OUT_OF_RESOURCES There is not enough pool memory to > store the matching results. > **/ > @@ -632,14 +635,16 @@ EFI_STATUS > GetFmpHandleBufferByType ( > IN EFI_GUID *UpdateImageTypeId, > IN UINT64 UpdateHardwareInstance, > - IN OUT UINTN *NoHandles, > - OUT EFI_HANDLE **Buffer > + OUT UINTN *NoHandles, OPTIONAL > + OUT EFI_HANDLE **HandleBuf, OPTIONAL > + OUT BOOLEAN **ResetRequiredBuf > OPTIONAL > ) > { > EFI_STATUS Status; > EFI_HANDLE *HandleBuffer; > UINTN NumberOfHandles; > EFI_HANDLE > *MatchedHandleBuffer; > + BOOLEAN > *MatchedResetRequiredBuffer; > UINTN > MatchedNumberOfHandles; > EFI_FIRMWARE_MANAGEMENT_PROTOCOL *Fmp; > UINTN Index; > @@ -653,8 +658,15 @@ GetFmpHandleBufferByType ( > UINTN Index2; > EFI_FIRMWARE_IMAGE_DESCRIPTOR > *TempFmpImageInfo; >=20 > - *NoHandles =3D 0; > - *Buffer =3D NULL; > + if (NoHandles !=3D NULL) { > + *NoHandles =3D 0; > + } > + if (HandleBuf !=3D NULL) { > + *HandleBuf =3D NULL; > + } > + if (ResetRequiredBuf !=3D NULL) { > + *ResetRequiredBuf =3D NULL; > + } >=20 > Status =3D gBS->LocateHandleBuffer ( > ByProtocol, > @@ -668,10 +680,26 @@ GetFmpHandleBufferByType ( > } >=20 > MatchedNumberOfHandles =3D 0; > - MatchedHandleBuffer =3D AllocateZeroPool (sizeof(EFI_HANDLE) * > NumberOfHandles); > - if (MatchedHandleBuffer =3D=3D NULL) { > - FreePool (HandleBuffer); > - return EFI_OUT_OF_RESOURCES; > + > + MatchedHandleBuffer =3D NULL; > + if (HandleBuf !=3D NULL) { > + MatchedHandleBuffer =3D AllocateZeroPool (sizeof(EFI_HANDLE) * > NumberOfHandles); > + if (MatchedHandleBuffer =3D=3D NULL) { > + FreePool (HandleBuffer); > + return EFI_OUT_OF_RESOURCES; > + } > + } > + > + MatchedResetRequiredBuffer =3D NULL; > + if (ResetRequiredBuf !=3D NULL) { > + MatchedResetRequiredBuffer =3D AllocateZeroPool (sizeof(BOOLEAN) * > NumberOfHandles); > + if (MatchedResetRequiredBuffer =3D=3D NULL) { > + if (MatchedHandleBuffer !=3D NULL) { > + FreePool (MatchedHandleBuffer); > + } > + FreePool (HandleBuffer); > + return EFI_OUT_OF_RESOURCES; > + } > } >=20 > for (Index =3D 0; Index < NumberOfHandles; Index++) { > @@ -733,7 +761,15 @@ GetFmpHandleBufferByType ( > if ((UpdateHardwareInstance =3D=3D 0) || > ((FmpImageInfoDescriptorVer >=3D > EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION) && > (UpdateHardwareInstance =3D=3D > TempFmpImageInfo->HardwareInstance))) { > - MatchedHandleBuffer[MatchedNumberOfHandles] =3D > HandleBuffer[Index]; > + if (MatchedHandleBuffer !=3D NULL) { > + MatchedHandleBuffer[MatchedNumberOfHandles] =3D > HandleBuffer[Index]; > + } > + if (MatchedResetRequiredBuffer !=3D NULL) { > + MatchedResetRequiredBuffer[MatchedNumberOfHandles] =3D > (((TempFmpImageInfo->AttributesSupported & > + > IMAGE_ATTRIBUTE_RESET_REQUIRED) !=3D 0) && > + > ((TempFmpImageInfo->AttributesSetting & > + > IMAGE_ATTRIBUTE_RESET_REQUIRED) !=3D 0)); > + } > MatchedNumberOfHandles++; > break; > } > @@ -749,8 +785,15 @@ GetFmpHandleBufferByType ( > return EFI_NOT_FOUND; > } >=20 > - *NoHandles =3D MatchedNumberOfHandles; > - *Buffer =3D MatchedHandleBuffer; > + if (NoHandles !=3D NULL) { > + *NoHandles =3D MatchedNumberOfHandles; > + } > + if (HandleBuf !=3D NULL) { > + *HandleBuf =3D MatchedHandleBuffer; > + } > + if (ResetRequiredBuf !=3D NULL) { > + *ResetRequiredBuf =3D MatchedResetRequiredBuffer; > + } >=20 > return EFI_SUCCESS; > } > @@ -1078,7 +1121,8 @@ RecordFmpCapsuleStatus ( >=20 > This function need support nested FMP capsule. >=20 > - @param[in] CapsuleHeader Points to a capsule header. > + @param[in] CapsuleHeader Points to a capsule header. > + @param[out] ResetRequired Indicates whether reset is required > or not. >=20 > @retval EFI_SUCESS Process Capsule Image successfully. > @retval EFI_UNSUPPORTED Capsule image is not supported by the > firmware. > @@ -1088,7 +1132,8 @@ RecordFmpCapsuleStatus ( > **/ > EFI_STATUS > ProcessFmpCapsuleImage ( > - IN EFI_CAPSULE_HEADER *CapsuleHeader > + IN EFI_CAPSULE_HEADER *CapsuleHeader, > + OUT BOOLEAN *ResetRequired OPTIONAL > ) > { > EFI_STATUS Status; > @@ -1098,6 +1143,7 @@ ProcessFmpCapsuleImage ( > UINT32 ItemNum; > UINTN Index; > EFI_HANDLE *HandleBuffer; > + BOOLEAN > *ResetRequiredBuffer; > UINTN NumberOfHandles; > UINTN DriverLen; > UINT64 > UpdateHardwareInstance; > @@ -1106,7 +1152,7 @@ ProcessFmpCapsuleImage ( > BOOLEAN Abort; >=20 > if (!IsFmpCapsuleGuid(&CapsuleHeader->CapsuleGuid)) { > - return ProcessFmpCapsuleImage ((EFI_CAPSULE_HEADER > *)((UINTN)CapsuleHeader + CapsuleHeader->HeaderSize)); > + return ProcessFmpCapsuleImage ((EFI_CAPSULE_HEADER > *)((UINTN)CapsuleHeader + CapsuleHeader->HeaderSize), ResetRequired); > } >=20 > NotReady =3D FALSE; > @@ -1176,7 +1222,8 @@ ProcessFmpCapsuleImage ( > &ImageHeader->UpdateImageTypeId, > UpdateHardwareInstance, > &NumberOfHandles, > - &HandleBuffer > + &HandleBuffer, > + &ResetRequiredBuffer > ); > if (EFI_ERROR(Status)) { > NotReady =3D TRUE; > @@ -1209,6 +1256,10 @@ ProcessFmpCapsuleImage ( > ); > if (Status !=3D EFI_SUCCESS) { > Abort =3D TRUE; > + } else { > + if (ResetRequired !=3D NULL) { > + *ResetRequired |=3D ResetRequiredBuffer[Index2]; > + } > } >=20 > RecordFmpCapsuleStatus ( > @@ -1222,6 +1273,9 @@ ProcessFmpCapsuleImage ( > if (HandleBuffer !=3D NULL) { > FreePool(HandleBuffer); > } > + if (ResetRequiredBuffer !=3D NULL) { > + FreePool(ResetRequiredBuffer); > + } > } >=20 > if (NotReady) { > @@ -1256,8 +1310,6 @@ IsNestedFmpCapsule ( > UINTN NestedCapsuleSize; > ESRT_MANAGEMENT_PROTOCOL *EsrtProtocol; > EFI_SYSTEM_RESOURCE_ENTRY Entry; > - EFI_HANDLE *HandleBuffer; > - UINTN NumberOfHandles; >=20 > EsrtGuidFound =3D FALSE; > if (mIsVirtualAddrConverted) { > @@ -1286,19 +1338,16 @@ IsNestedFmpCapsule ( > // Check Firmware Management Protocols > // > if (!EsrtGuidFound) { > - HandleBuffer =3D NULL; > Status =3D GetFmpHandleBufferByType ( > &CapsuleHeader->CapsuleGuid, > 0, > - &NumberOfHandles, > - &HandleBuffer > + NULL, > + NULL, > + NULL > ); > if (!EFI_ERROR(Status)) { > EsrtGuidFound =3D TRUE; > } > - if (HandleBuffer !=3D NULL) { > - FreePool (HandleBuffer); > - } > } > } > if (!EsrtGuidFound) { > @@ -1386,6 +1435,7 @@ SupportCapsuleImage ( > Caution: This function may receive untrusted input. >=20 > @param[in] CapsuleHeader Points to a capsule header. > + @param[out] ResetRequired Indicates whether reset is required > or not. >=20 > @retval EFI_SUCESS Process Capsule Image successfully. > @retval EFI_UNSUPPORTED Capsule image is not supported by the > firmware. > @@ -1394,8 +1444,9 @@ SupportCapsuleImage ( > **/ > EFI_STATUS > EFIAPI > -ProcessCapsuleImage ( > - IN EFI_CAPSULE_HEADER *CapsuleHeader > +ProcessThisCapsuleImage ( > + IN EFI_CAPSULE_HEADER *CapsuleHeader, > + OUT BOOLEAN *ResetRequired OPTIONAL > ) > { > EFI_STATUS Status; > @@ -1432,7 +1483,7 @@ ProcessCapsuleImage ( > // Process EFI FMP Capsule > // > DEBUG((DEBUG_INFO, "ProcessFmpCapsuleImage ...\n")); > - Status =3D ProcessFmpCapsuleImage(CapsuleHeader); > + Status =3D ProcessFmpCapsuleImage(CapsuleHeader, ResetRequired); > DEBUG((DEBUG_INFO, "ProcessFmpCapsuleImage - %r\n", Status)); >=20 > return Status; > @@ -1442,6 +1493,27 @@ ProcessCapsuleImage ( > } >=20 > /** > + The firmware implements to process the capsule image. > + > + Caution: This function may receive untrusted input. > + > + @param[in] CapsuleHeader Points to a capsule header. > + > + @retval EFI_SUCESS Process Capsule Image successfully. > + @retval EFI_UNSUPPORTED Capsule image is not supported by the > firmware. > + @retval EFI_VOLUME_CORRUPTED FV volume in the capsule is > corrupted. > + @retval EFI_OUT_OF_RESOURCES Not enough memory. > +**/ > +EFI_STATUS > +EFIAPI > +ProcessCapsuleImage ( > + IN EFI_CAPSULE_HEADER *CapsuleHeader > + ) > +{ > + return ProcessThisCapsuleImage (CapsuleHeader, NULL); > +} > + > +/** > Callback function executed when the EndOfDxe event group is signaled. >=20 > @param[in] Event Event whose notification function is being > invoked. > diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > index 26ca4e295f20..176dea196026 100644 > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c > @@ -99,13 +99,33 @@ IsValidCapsuleHeader ( > ); >=20 > extern BOOLEAN mDxeCapsuleLibEndOfDxe; > -BOOLEAN mNeedReset; > +BOOLEAN mNeedReset =3D FALSE; >=20 > VOID **mCapsulePtr; > EFI_STATUS *mCapsuleStatusArray; > UINT32 mCapsuleTotalNumber; >=20 > /** > + The firmware implements to process the capsule image. > + > + Caution: This function may receive untrusted input. > + > + @param[in] CapsuleHeader Points to a capsule header. > + @param[out] ResetRequired Indicates whether reset is required > or not. > + > + @retval EFI_SUCESS Process Capsule Image successfully. > + @retval EFI_UNSUPPORTED Capsule image is not supported by the > firmware. > + @retval EFI_VOLUME_CORRUPTED FV volume in the capsule is > corrupted. > + @retval EFI_OUT_OF_RESOURCES Not enough memory. > +**/ > +EFI_STATUS > +EFIAPI > +ProcessThisCapsuleImage ( > + IN EFI_CAPSULE_HEADER *CapsuleHeader, > + OUT BOOLEAN *ResetRequired OPTIONAL > + ); > + > +/** > Function indicate the current completion progress of the firmware > update. Platform may override with own specific progress function. >=20 > @@ -381,6 +401,7 @@ ProcessTheseCapsules ( > UINT32 Index; > ESRT_MANAGEMENT_PROTOCOL *EsrtManagement; > UINT16 EmbeddedDriverCount; > + BOOLEAN ResetRequired; >=20 > REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | > PcdGet32(PcdStatusCodeSubClassCapsule) | > PcdGet32(PcdCapsuleStatusCodeProcessCapsulesBegin))); >=20 > @@ -416,11 +437,11 @@ ProcessTheseCapsules ( > for (Index =3D 0; Index < mCapsuleTotalNumber; Index++) { > CapsuleHeader =3D (EFI_CAPSULE_HEADER*) mCapsulePtr [Index]; > if (CompareGuid (&CapsuleHeader->CapsuleGuid, > &gWindowsUxCapsuleGuid)) { > - DEBUG ((DEBUG_INFO, "ProcessCapsuleImage (Ux) - 0x%x\n", > CapsuleHeader)); > + DEBUG ((DEBUG_INFO, "ProcessThisCapsuleImage (Ux) - 0x%x\n", > CapsuleHeader)); > DEBUG ((DEBUG_INFO, "Display logo capsule is found.\n")); > - Status =3D ProcessCapsuleImage (CapsuleHeader); > + Status =3D ProcessThisCapsuleImage (CapsuleHeader, NULL); > mCapsuleStatusArray [Index] =3D EFI_SUCCESS; > - DEBUG((DEBUG_INFO, "ProcessCapsuleImage (Ux) - %r\n", Status)); > + DEBUG((DEBUG_INFO, "ProcessThisCapsuleImage (Ux) - %r\n", > Status)); > break; > } > } > @@ -454,10 +475,11 @@ ProcessTheseCapsules ( > } >=20 > if ((!FirstRound) || (EmbeddedDriverCount =3D=3D 0)) { > - DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", > CapsuleHeader)); > - Status =3D ProcessCapsuleImage (CapsuleHeader); > + DEBUG((DEBUG_INFO, "ProcessThisCapsuleImage - 0x%x\n", > CapsuleHeader)); > + ResetRequired =3D FALSE; > + Status =3D ProcessThisCapsuleImage (CapsuleHeader, &ResetRequire= d); > mCapsuleStatusArray [Index] =3D Status; > - DEBUG((DEBUG_INFO, "ProcessCapsuleImage - %r\n", Status)); > + DEBUG((DEBUG_INFO, "ProcessThisCapsuleImage - %r\n", Status)); >=20 > if (Status !=3D EFI_NOT_READY) { > if (EFI_ERROR(Status)) { > @@ -467,8 +489,8 @@ ProcessTheseCapsules ( > REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE > | PcdGet32(PcdStatusCodeSubClassCapsule) | > PcdGet32(PcdCapsuleStatusCodeUpdateFirmwareSuccess))); > } >=20 > - if ((CapsuleHeader->Flags & > PcdGet16(PcdSystemRebootAfterCapsuleProcessFlag)) !=3D 0 || > - IsFmpCapsule(CapsuleHeader)) { > + mNeedReset |=3D ResetRequired; > + if ((CapsuleHeader->Flags & > PcdGet16(PcdSystemRebootAfterCapsuleProcessFlag)) !=3D 0) { > mNeedReset =3D TRUE; > } > } > -- > 2.7.0.windows.1