From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=ray.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (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 BAB4121B02822 for ; Thu, 14 Feb 2019 00:25:32 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Feb 2019 00:25:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,367,1544515200"; d="scan'208";a="124397497" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga008.fm.intel.com with ESMTP; 14 Feb 2019 00:25:31 -0800 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 14 Feb 2019 00:25:31 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 14 Feb 2019 00:25:31 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.102]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.172]) with mapi id 14.03.0415.000; Thu, 14 Feb 2019 16:25:28 +0800 From: "Ni, Ray" To: "Chiu, Chasel" , "edk2-devel@lists.01.org" CC: "Dong, Eric" , Laszlo Ersek Thread-Topic: [PATCH v2 3/3] UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI Thread-Index: AQHUw4EXuFDFn7JPTkaZbroHFzzk2qXe9k/g Date: Thu, 14 Feb 2019 08:25:28 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C019861@SHSMSX104.ccr.corp.intel.com> References: <20190213094633.8136-1-chasel.chiu@intel.com> <20190213094633.8136-4-chasel.chiu@intel.com> In-Reply-To: <20190213094633.8136-4-chasel.chiu@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2 3/3] UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Feb 2019 08:25:33 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Chiu, Chasel > Sent: Wednesday, February 13, 2019 5:47 PM > To: edk2-devel@lists.01.org > Cc: Dong, Eric ; Ni, Ray ; Laszlo > Ersek ; Chiu, Chasel > Subject: [PATCH v2 3/3] UefiCpuPkg/SecCore: Support > EFI_PEI_CORE_FV_LOCATION_PPI >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1524 >=20 > EFI_PEI_CORE_FV_LOCATION_PPI may be passed by platform when PeiCore > not in BFV so SecCore has to search PeiCore either from the FV location > provided by EFI_PEI_CORE_FV_LOCATION_PPI or from BFV. >=20 > Test: Verified on internal platform and booting successfully. >=20 > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Chasel Chiu > --- > UefiCpuPkg/SecCore/SecMain.c | 35 > +++++++++++++++++++++++++++++------ > UefiCpuPkg/SecCore/SecCore.inf | 3 ++- > UefiCpuPkg/SecCore/SecMain.h | 3 ++- > 3 files changed, 33 insertions(+), 8 deletions(-) >=20 > diff --git a/UefiCpuPkg/SecCore/SecMain.c > b/UefiCpuPkg/SecCore/SecMain.c index b24e190617..b99072599d 100644 > --- a/UefiCpuPkg/SecCore/SecMain.c > +++ b/UefiCpuPkg/SecCore/SecMain.c > @@ -1,7 +1,7 @@ > /** @file > C functions in SEC >=20 > - Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
> + Copyright (c) 2008 - 2019, Intel Corporation. All rights > + reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD > License > which accompanies this distribution. The full text of the license may= be > found at @@ -228,26 +228,49 @@ SecStartupPhase2( { > EFI_SEC_PEI_HAND_OFF *SecCoreData; > EFI_PEI_PPI_DESCRIPTOR *PpiList; > + EFI_PEI_PPI_DESCRIPTOR *PpiListTmp; 1. Maybe this local variable is not needed. We can use PpiList[Index]. > UINT32 Index; > EFI_PEI_PPI_DESCRIPTOR *AllSecPpiList; > EFI_PEI_CORE_ENTRY_POINT PeiCoreEntryPoint; >=20 > + PeiCoreEntryPoint =3D NULL; > SecCoreData =3D (EFI_SEC_PEI_HAND_OFF *) Context; > AllSecPpiList =3D (EFI_PEI_PPI_DESCRIPTOR *) SecCoreData- > >PeiTemporaryRamBase; > + > // > // Find Pei Core entry point. It will report SEC and Pei Core debug > information if remote debug > // is enabled. > // > - FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) > SecCoreData->BootFirmwareVolumeBase, &PeiCoreEntryPoint); > - if (PeiCoreEntryPoint =3D=3D NULL) > - { > - CpuDeadLoop (); > + PpiList =3D SecPlatformMain (SecCoreData); PpiListTmp =3D PpiList; f= or > + (;;) { 2. Similar comments as above. Maybe we can just use PpiList[Index] in the l= oop. By the way, original code logic checks PpiList against NULL. Do we still need to make sure to deference PpiList after checking against N= ULL? > + if (CompareGuid (PpiListTmp->Guid, &gEfiPeiCoreFvLocationPpiGuid) && > (((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiListTmp->Ppi)- > >PeiCoreFvLocation !=3D 0)) { > + FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) > ((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiListTmp->Ppi)->PeiCoreFvLocation, > &PeiCoreEntryPoint); > + if (PeiCoreEntryPoint !=3D NULL) { > + break; 3. Is it valid that PeiCore cannot be found in the PeiCoreFvLocation? If no, can we just dead-loop here when PeiCoreEntryPoint is NULL? > + } > + } > + if ((PpiListTmp->Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) =3D= =3D > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) { > + // > + // Continue until the end of the PPI List. > + // > + break; > + } > + PpiListTmp++; > + } > + // > + // If EFI_PEI_CORE_FV_LOCATION_PPI not found or no PeiCore found by > the pointer in provided PPI, try to locate PeiCore from BFV. > + // > + if (PeiCoreEntryPoint =3D=3D NULL) { > + FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) > SecCoreData->BootFirmwareVolumeBase, &PeiCoreEntryPoint); > + if (PeiCoreEntryPoint =3D=3D NULL) { > + CpuDeadLoop (); > + } > } >=20 > // > // Perform platform specific initialization before entering PeiCore. > // > - PpiList =3D SecPlatformMain (SecCoreData); > if (PpiList !=3D NULL) { > // > // Remove the terminal flag from the terminal PPI diff --git > a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf > index 442f663911..fc1485b5cb 100644 > --- a/UefiCpuPkg/SecCore/SecCore.inf > +++ b/UefiCpuPkg/SecCore/SecCore.inf > @@ -7,7 +7,7 @@ > # protected mode, setup flat memory model, enable temporary memory > and # call into SecStartup(). > # > -# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. > +# Copyright (c) 2006 - 2019, Intel Corporation. All rights > +reserved.
> # This program and the accompanying materials # are licensed and made > available under the terms and conditions of the BSD License # which > accompanies this distribution. The full text of the license may be found= at > @@ -73,6 +73,7 @@ > ## NOTIFY > ## SOMETIMES_CONSUMES > gPeiSecPerformancePpiGuid > + gEfiPeiCoreFvLocationPpiGuid >=20 > [Guids] > ## SOMETIMES_PRODUCES ## HOB > diff --git a/UefiCpuPkg/SecCore/SecMain.h > b/UefiCpuPkg/SecCore/SecMain.h index 83244af119..80045d34f4 100644 > --- a/UefiCpuPkg/SecCore/SecMain.h > +++ b/UefiCpuPkg/SecCore/SecMain.h > @@ -1,7 +1,7 @@ > /** @file > Master header file for SecCore. >=20 > - Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
> + Copyright (c) 2008 - 2019, Intel Corporation. All rights > + reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD > License > which accompanies this distribution. The full text of the license may= be > found at @@ -20,6 +20,7 @@ #include > #include #include > +#include >=20 > #include >=20 > -- > 2.13.3.windows.1