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=chasel.chiu@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 2BF772194EB70 for ; Thu, 14 Feb 2019 02:31:06 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Feb 2019 02:31:05 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,368,1544515200"; d="scan'208";a="133528125" Received: from pgsmsx113.gar.corp.intel.com ([10.108.55.202]) by FMSMGA003.fm.intel.com with ESMTP; 14 Feb 2019 02:31:04 -0800 Received: from pgsmsx111.gar.corp.intel.com ([169.254.2.192]) by pgsmsx113.gar.corp.intel.com ([169.254.6.236]) with mapi id 14.03.0415.000; Thu, 14 Feb 2019 18:31:03 +0800 From: "Chiu, Chasel" To: "Ni, Ray" , "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: AQHUw4EXy4K1Gw7UwE2VE8tpwWZUaaXecJAAgACozNA= Date: Thu, 14 Feb 2019 10:31:03 +0000 Message-ID: <3C3EFB470A303B4AB093197B6777CCEC502649BD@PGSMSX111.gar.corp.intel.com> References: <20190213094633.8136-1-chasel.chiu@intel.com> <20190213094633.8136-4-chasel.chiu@intel.com> <734D49CCEBEEF84792F5B80ED585239D5C019861@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C019861@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNWI5MGQ0NmYtOTBkMC00MjQ5LWFlYzMtM2I5MDg2ZjkzNGQ5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiY1RnSVZMeFVjXC9MckIrUFMzTisyYTlhY0ZcLytuNEpaK0oxZHZyNzduM2dnXC81WGpXcGVmbXc1YjY4Q3FyVzl3bSJ9 x-ctpclassification: CTP_NT x-originating-ip: [172.30.20.205] 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 10:31:07 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Ni, Ray > Sent: Thursday, February 14, 2019 4:25 PM > To: Chiu, Chasel ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Laszlo Ersek > Subject: RE: [PATCH v2 3/3] UefiCpuPkg/SecCore: Support > EFI_PEI_CORE_FV_LOCATION_PPI >=20 >=20 >=20 > > -----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 > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1524 > > > > 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. > > > > Test: Verified on internal platform and booting successfully. > > > > 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(-) > > > > 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 > > > > - 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; >=20 >=20 > 1. Maybe this local variable is not needed. We can use PpiList[Index]. Agree! I will correct this. >=20 > > UINT32 Index; > > EFI_PEI_PPI_DESCRIPTOR *AllSecPpiList; > > EFI_PEI_CORE_ENTRY_POINT PeiCoreEntryPoint; > > > > + 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; > > + for > > + (;;) { >=20 > 2. Similar comments as above. Maybe we can just use PpiList[Index] in the= loop. > By the way, original code logic checks PpiList against NULL. > Do we still need to make sure to deference PpiList after checking against= NULL? Good catch! I will update. >=20 > > + 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; >=20 > 3. Is it valid that PeiCore cannot be found in the PeiCoreFvLocation? > If no, can we just dead-loop here when PeiCoreEntryPoint is NULL? Yes. I will add dead-loop. >=20 > > + } > > + } > > + 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 (); > > + } > > } > > > > // > > // 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 > > > > [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. > > > > - 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 > > > > #include > > > > -- > > 2.13.3.windows.1