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 <ray...@intel.com> > -----Original Message----- > From: Chiu, Chasel <chasel.c...@intel.com> > Sent: Tuesday, February 19, 2019 3:44 PM > To: edk2-devel@lists.01.org > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Laszlo > Ersek <ler...@redhat.com>; Chiu, Chasel <chasel.c...@intel.com> > Subject: [PATCH] UefiCpuPkg/SecCore: Wrong Debug Information for > SecCore > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1533 > > 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 FindAndReportEntryPoints > () and SecCore and PeiCore will be found in each FV and correct debug > information will be reported. > > Test: Booted with internal platform successfully. > > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Chasel Chiu <chasel.c...@intel.com> > --- > UefiCpuPkg/SecCore/FindPeiCore.c | 60 > ++++++++++++++++++++++++++++++------------------------------ > UefiCpuPkg/SecCore/SecMain.c | 10 ++++++++-- > UefiCpuPkg/SecCore/SecMain.h | 8 +++++--- > 3 files changed, 43 insertions(+), 35 deletions(-) > > 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 > > - Copyright (c) 2008 - 2011, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2008 - 2019, Intel Corporation. All rights > + reserved.<BR> > 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. > > - @param BootFirmwareVolumePtr Point to the boot firmware volume. > - @param SecCoreImageBase The base address of the SEC core image. > - @param PeiCoreImageBase The base address of the PEI core image. > + @param FirmwareVolumePtr Point to the firmware volume for finding > core image. > + @param FileType The FileType for searching, either > SecCore or > PeiCore. > + @param CoreImageBase The base address of the core image. > > **/ > 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; > > - *SecCoreImageBase = 0; > - *PeiCoreImageBase = 0; > + *CoreImageBase = 0; > > - CurrentAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) > BootFirmwareVolumePtr; > - EndOfFirmwareVolume = CurrentAddress + BootFirmwareVolumePtr- > >FvLength; > + CurrentAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) FirmwareVolumePtr; > + EndOfFirmwareVolume = CurrentAddress + FirmwareVolumePtr- > >FvLength; > > // > // Loop through the FFS files in the Boot Firmware Volume > // > - for (EndOfFile = CurrentAddress + BootFirmwareVolumePtr- > >HeaderLength; ; ) { > + for (EndOfFile = CurrentAddress + FirmwareVolumePtr->HeaderLength; ; > + ) { > > CurrentAddress = (EndOfFile + 7) & 0xfffffffffffffff8ULL; > if (CurrentAddress > EndOfFirmwareVolume) { @@ -75,10 +74,9 @@ > FindImageBase ( > } > > // > - // Look for SEC Core / PEI Core files > + // Look for particular Core file (either SEC Core or PEI Core) > // > - if (File->Type != EFI_FV_FILETYPE_SECURITY_CORE && > - File->Type != EFI_FV_FILETYPE_PEI_CORE) { > + if (File->Type != FileType) { > continue; > } > > @@ -115,17 +113,11 @@ FindImageBase ( > // Look for executable sections > // > if (Section->Type == EFI_SECTION_PE32 || Section->Type == > EFI_SECTION_TE) { > - if (File->Type == EFI_FV_FILETYPE_SECURITY_CORE) { > + if (File->Type == FileType) { > if (IS_SECTION2 (Section)) { > - *SecCoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > Section + sizeof (EFI_COMMON_SECTION_HEADER2)); > + *CoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > + Section + sizeof (EFI_COMMON_SECTION_HEADER2)); > } else { > - *SecCoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > Section + sizeof (EFI_COMMON_SECTION_HEADER)); > - } > - } else { > - if (IS_SECTION2 (Section)) { > - *PeiCoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > Section + sizeof (EFI_COMMON_SECTION_HEADER2)); > - } else { > - *PeiCoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > Section + sizeof (EFI_COMMON_SECTION_HEADER)); > + *CoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) > + Section + sizeof (EFI_COMMON_SECTION_HEADER)); > } > } > break; > @@ -133,9 +125,9 @@ FindImageBase ( > } > > // > - // Both SEC Core and PEI Core images found > + // Either SEC Core or PEI Core images found > // > - if (*SecCoreImageBase != 0 && *PeiCoreImageBase != 0) { > + if (*CoreImageBase != 0) { > return EFI_SUCCESS; > } > } > @@ -147,14 +139,16 @@ FindImageBase ( > It also find SEC and PEI Core file debug information. It will report them > if > remote debug is enabled. > > - @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. > > **/ > 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; > > // > - // Find SEC Core and PEI Core image base > + // Find SEC Core image base > // > - Status = FindImageBase (BootFirmwareVolumePtr, &SecCoreImageBase, > &PeiCoreImageBase); > + Status = FindImageBase (SecCoreFirmwareVolumePtr, > + EFI_FV_FILETYPE_SECURITY_CORE, &SecCoreImageBase); > ASSERT_EFI_ERROR (Status); > > ZeroMem ((VOID *) &ImageContext, sizeof > (PE_COFF_LOADER_IMAGE_CONTEXT)); @@ -178,6 +172,12 @@ > FindAndReportEntryPoints ( > PeCoffLoaderRelocateImageExtraAction (&ImageContext); > > // > + // Find PEI Core image base > + // > + Status = FindImageBase (PeiCoreFirmwareVolumePtr, > + EFI_FV_FILETYPE_PEI_CORE, &PeiCoreImageBase); ASSERT_EFI_ERROR > + (Status); > + > + // > // Report PEI Core debug information when remote debug is enabled > // > ImageContext.ImageAddress = 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) != > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST; > Index++) { > if (CompareGuid (PpiList[Index].Guid, &gEfiPeiCoreFvLocationPpiGuid) > && (((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)- > >PeiCoreFvLocation != 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 != 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 == 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 == 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 them > if > remote debug is enabled. > > - @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. > > **/ > 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 > ); > > -- > 2.13.3.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
