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.65; helo=mga03.intel.com; envelope-from=ray.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (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 E866A208EB3EC for ; Tue, 19 Feb 2019 01:23:17 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Feb 2019 01:23:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,387,1544515200"; d="scan'208";a="135413071" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga002.jf.intel.com with ESMTP; 19 Feb 2019 01:23:17 -0800 Received: from fmsmsx102.amr.corp.intel.com (10.18.124.200) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.408.0; Tue, 19 Feb 2019 01:23:16 -0800 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by FMSMSX102.amr.corp.intel.com (10.18.124.200) with Microsoft SMTP Server (TLS) id 14.3.408.0; Tue, 19 Feb 2019 01:23:16 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.102]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.36]) with mapi id 14.03.0415.000; Tue, 19 Feb 2019 17:23:14 +0800 From: "Ni, Ray" To: "Chiu, Chasel" , "edk2-devel@lists.01.org" CC: "Dong, Eric" , Laszlo Ersek Thread-Topic: [PATCH] UefiCpuPkg/SecCore: Wrong Debug Information for SecCore Thread-Index: AQHUyCb34s7Xgx3mUkGZXt2+FKTVZKXm2Vcw Date: Tue, 19 Feb 2019 09:23:14 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C02526D@SHSMSX104.ccr.corp.intel.com> References: <20190219074415.11044-1-chasel.chiu@intel.com> In-Reply-To: <20190219074415.11044-1-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] UefiCpuPkg/SecCore: Wrong Debug Information for SecCore 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: Tue, 19 Feb 2019 09:23:18 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Chasel, Please change the two FindAndReportEntryPoints() calls in SecStartupPhase2(= ) so that their line length doesn't exceed 120. With the modification, Reviewed-by: Ray Ni > -----Original Message----- > From: Chiu, Chasel > Sent: Tuesday, February 19, 2019 3:44 PM > To: edk2-devel@lists.01.org > Cc: Dong, Eric ; Ni, Ray ; Laszlo > Ersek ; Chiu, Chasel > Subject: [PATCH] UefiCpuPkg/SecCore: Wrong Debug Information for > SecCore >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1533 >=20 > When SecCore and PeiCore in different FV, current implementation still > assuming SecCore and PeiCore are in the same FV. > To fix this issue 2 FVs will be input parameters for FindAndReportEntryPo= ints > () and SecCore and PeiCore will be found in each FV and correct debug > information will be reported. >=20 > Test: Booted with internal platform 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/FindPeiCore.c | 60 > ++++++++++++++++++++++++++++++------------------------------ > UefiCpuPkg/SecCore/SecMain.c | 10 ++++++++-- > UefiCpuPkg/SecCore/SecMain.h | 8 +++++--- > 3 files changed, 43 insertions(+), 35 deletions(-) >=20 > diff --git a/UefiCpuPkg/SecCore/FindPeiCore.c > b/UefiCpuPkg/SecCore/FindPeiCore.c > index bb9c434d1e..6f2046ad95 100644 > --- a/UefiCpuPkg/SecCore/FindPeiCore.c > +++ b/UefiCpuPkg/SecCore/FindPeiCore.c > @@ -1,7 +1,7 @@ > /** @file > Locate the entry point for the PEI Core >=20 > - Copyright (c) 2008 - 2011, 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 @@ -19,17 +19,17 @@ > /** > Find core image base. >=20 > - @param BootFirmwareVolumePtr Point to the boot firmware volume. > - @param SecCoreImageBase The base address of the SEC core ima= ge. > - @param PeiCoreImageBase The base address of the PEI core ima= ge. > + @param FirmwareVolumePtr Point to the firmware volume for fin= ding > core image. > + @param FileType The FileType for searching, either S= ecCore or > PeiCore. > + @param CoreImageBase The base address of the core image. >=20 > **/ > EFI_STATUS > EFIAPI > FindImageBase ( > - IN EFI_FIRMWARE_VOLUME_HEADER *BootFirmwareVolumePtr, > - OUT EFI_PHYSICAL_ADDRESS *SecCoreImageBase, > - OUT EFI_PHYSICAL_ADDRESS *PeiCoreImageBase > + IN EFI_FIRMWARE_VOLUME_HEADER *FirmwareVolumePtr, > + IN EFI_FV_FILETYPE FileType, > + OUT EFI_PHYSICAL_ADDRESS *CoreImageBase > ) > { > EFI_PHYSICAL_ADDRESS CurrentAddress; > @@ -40,16 +40,15 @@ FindImageBase ( > EFI_COMMON_SECTION_HEADER *Section; > EFI_PHYSICAL_ADDRESS EndOfSection; >=20 > - *SecCoreImageBase =3D 0; > - *PeiCoreImageBase =3D 0; > + *CoreImageBase =3D 0; >=20 > - CurrentAddress =3D (EFI_PHYSICAL_ADDRESS)(UINTN) > BootFirmwareVolumePtr; > - EndOfFirmwareVolume =3D CurrentAddress + BootFirmwareVolumePtr- > >FvLength; > + CurrentAddress =3D (EFI_PHYSICAL_ADDRESS)(UINTN) FirmwareVolumePtr; > + EndOfFirmwareVolume =3D CurrentAddress + FirmwareVolumePtr- > >FvLength; >=20 > // > // Loop through the FFS files in the Boot Firmware Volume > // > - for (EndOfFile =3D CurrentAddress + BootFirmwareVolumePtr- > >HeaderLength; ; ) { > + for (EndOfFile =3D CurrentAddress + FirmwareVolumePtr->HeaderLength; ; > + ) { >=20 > CurrentAddress =3D (EndOfFile + 7) & 0xfffffffffffffff8ULL; > if (CurrentAddress > EndOfFirmwareVolume) { @@ -75,10 +74,9 @@ > FindImageBase ( > } >=20 > // > - // Look for SEC Core / PEI Core files > + // Look for particular Core file (either SEC Core or PEI Core) > // > - if (File->Type !=3D EFI_FV_FILETYPE_SECURITY_CORE && > - File->Type !=3D EFI_FV_FILETYPE_PEI_CORE) { > + if (File->Type !=3D FileType) { > continue; > } >=20 > @@ -115,17 +113,11 @@ FindImageBase ( > // Look for executable sections > // > if (Section->Type =3D=3D EFI_SECTION_PE32 || Section->Type =3D=3D > EFI_SECTION_TE) { > - if (File->Type =3D=3D EFI_FV_FILETYPE_SECURITY_CORE) { > + if (File->Type =3D=3D FileType) { > if (IS_SECTION2 (Section)) { > - *SecCoreImageBase =3D (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > Section + sizeof (EFI_COMMON_SECTION_HEADER2)); > + *CoreImageBase =3D (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > + Section + sizeof (EFI_COMMON_SECTION_HEADER2)); > } else { > - *SecCoreImageBase =3D (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > Section + sizeof (EFI_COMMON_SECTION_HEADER)); > - } > - } else { > - if (IS_SECTION2 (Section)) { > - *PeiCoreImageBase =3D (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > Section + sizeof (EFI_COMMON_SECTION_HEADER2)); > - } else { > - *PeiCoreImageBase =3D (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > Section + sizeof (EFI_COMMON_SECTION_HEADER)); > + *CoreImageBase =3D (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > + Section + sizeof (EFI_COMMON_SECTION_HEADER)); > } > } > break; > @@ -133,9 +125,9 @@ FindImageBase ( > } >=20 > // > - // Both SEC Core and PEI Core images found > + // Either SEC Core or PEI Core images found > // > - if (*SecCoreImageBase !=3D 0 && *PeiCoreImageBase !=3D 0) { > + if (*CoreImageBase !=3D 0) { > return EFI_SUCCESS; > } > } > @@ -147,14 +139,16 @@ FindImageBase ( > It also find SEC and PEI Core file debug information. It will report t= hem if > remote debug is enabled. >=20 > - @param BootFirmwareVolumePtr Point to the boot firmware volume. > + @param SecCoreFirmwareVolumePtr Point to the firmware volume for > finding SecCore. > + @param PeiCoreFirmwareVolumePtr Point to the firmware volume for > finding PeiCore. > @param PeiCoreEntryPoint The entry point of the PEI core. >=20 > **/ > VOID > EFIAPI > FindAndReportEntryPoints ( > - IN EFI_FIRMWARE_VOLUME_HEADER *BootFirmwareVolumePtr, > + IN EFI_FIRMWARE_VOLUME_HEADER *SecCoreFirmwareVolumePtr, > + IN EFI_FIRMWARE_VOLUME_HEADER *PeiCoreFirmwareVolumePtr, > OUT EFI_PEI_CORE_ENTRY_POINT *PeiCoreEntryPoint > ) > { > @@ -164,9 +158,9 @@ FindAndReportEntryPoints ( > PE_COFF_LOADER_IMAGE_CONTEXT ImageContext; >=20 > // > - // Find SEC Core and PEI Core image base > + // Find SEC Core image base > // > - Status =3D FindImageBase (BootFirmwareVolumePtr, &SecCoreImageBase, > &PeiCoreImageBase); > + Status =3D FindImageBase (SecCoreFirmwareVolumePtr, > + EFI_FV_FILETYPE_SECURITY_CORE, &SecCoreImageBase); > ASSERT_EFI_ERROR (Status); >=20 > ZeroMem ((VOID *) &ImageContext, sizeof > (PE_COFF_LOADER_IMAGE_CONTEXT)); @@ -178,6 +172,12 @@ > FindAndReportEntryPoints ( > PeCoffLoaderRelocateImageExtraAction (&ImageContext); >=20 > // > + // Find PEI Core image base > + // > + Status =3D FindImageBase (PeiCoreFirmwareVolumePtr, > + EFI_FV_FILETYPE_PEI_CORE, &PeiCoreImageBase); ASSERT_EFI_ERROR > + (Status); > + > + // > // Report PEI Core debug information when remote debug is enabled > // > ImageContext.ImageAddress =3D PeiCoreImageBase; diff --git > a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c index > 752156a98e..e4961fd7c6 100644 > --- a/UefiCpuPkg/SecCore/SecMain.c > +++ b/UefiCpuPkg/SecCore/SecMain.c > @@ -249,7 +249,10 @@ SecStartupPhase2( > (PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) != =3D > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST; > Index++) { > if (CompareGuid (PpiList[Index].Guid, &gEfiPeiCoreFvLocationPpiGui= d) > && (((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)- > >PeiCoreFvLocation !=3D 0)) { > - FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) > ((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)- > >PeiCoreFvLocation, &PeiCoreEntryPoint); > + // > + // In this case, SecCore is in BFV but PeiCore is in another FV = reported > by PPI. > + // > + FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) > + SecCoreData->BootFirmwareVolumeBase, > (EFI_FIRMWARE_VOLUME_HEADER *) > + ((EFI_PEI_CORE_FV_LOCATION_PPI *) > + PpiList[Index].Ppi)->PeiCoreFvLocation, &PeiCoreEntryPoint); > if (PeiCoreEntryPoint !=3D NULL) { > break; > } else { > @@ -265,7 +268,10 @@ SecStartupPhase2( > // If EFI_PEI_CORE_FV_LOCATION_PPI not found, try to locate PeiCore > from BFV. > // > if (PeiCoreEntryPoint =3D=3D NULL) { > - FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) > SecCoreData->BootFirmwareVolumeBase, &PeiCoreEntryPoint); > + // > + // Both SecCore and PeiCore are in BFV. > + // > + FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) > + SecCoreData->BootFirmwareVolumeBase, > (EFI_FIRMWARE_VOLUME_HEADER *) > + SecCoreData->BootFirmwareVolumeBase, &PeiCoreEntryPoint); > if (PeiCoreEntryPoint =3D=3D NULL) { > CpuDeadLoop (); > } > diff --git a/UefiCpuPkg/SecCore/SecMain.h > b/UefiCpuPkg/SecCore/SecMain.h index 80045d34f4..6f3b0c5d12 100644 > --- a/UefiCpuPkg/SecCore/SecMain.h > +++ b/UefiCpuPkg/SecCore/SecMain.h > @@ -89,14 +89,16 @@ SecStartup ( > It also find SEC and PEI Core file debug information. It will report t= hem if > remote debug is enabled. >=20 > - @param BootFirmwareVolumePtr Point to the boot firmware volume. > - @param PeiCoreEntryPoint Point to the PEI core entry point. > + @param SecCoreFirmwareVolumePtr Point to the firmware volume for > finding SecCore. > + @param PeiCoreFirmwareVolumePtr Point to the firmware volume for > finding PeiCore. > + @param PeiCoreEntryPoint The entry point of the PEI core. >=20 > **/ > VOID > EFIAPI > FindAndReportEntryPoints ( > - IN EFI_FIRMWARE_VOLUME_HEADER *BootFirmwareVolumePtr, > + IN EFI_FIRMWARE_VOLUME_HEADER *SecCoreFirmwareVolumePtr, > + IN EFI_FIRMWARE_VOLUME_HEADER *PeiCoreFirmwareVolumePtr, > OUT EFI_PEI_CORE_ENTRY_POINT *PeiCoreEntryPoint > ); >=20 > -- > 2.13.3.windows.1