| Message ID | 20171130152453.19205-6-ard.biesheuvel@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ArmPlatformPkg/PrePi: stop exposing internal code via HOBs | expand |
On 26 December 2017 at 21:52, Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> wrote: > Hi Ard, Meenakshi, > > I am having a problem I cannot explain the reason for, with this commit on > an ARM64 platform. > > ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory > > Now that PrePi no longer exposes its internal code via special HOBs, > we can remove the special handling of the primary FV, which needed to > be reserved so that DXE core could call into the PE/COFF and LZMA > libraries in the PrePi module. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Udit Kumar <udit.kumar@nxp.com> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > [ardb: updated commit log] > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > > If a Shell is built "as is" from the source tree, there are no issues. > However, if I slightly modify Shell.c like in the following patch: > > diff --git a/ShellPkg/Application/Shell/Shell.c > b/ShellPkg/Application/Shell/Shell.c > index 577e17311bea..bbbdde8ced96 100644 > --- a/ShellPkg/Application/Shell/Shell.c > +++ b/ShellPkg/Application/Shell/Shell.c > @@ -339,6 +339,11 @@ UefiMain ( > EFI_HANDLE ConInHandle; > EFI_SIMPLE_TEXT_INPUT_PROTOCOL *OldConIn; > SPLIT_LIST *Split; > + CHAR16 *DelayStr; > + CHAR16 *NoMapStr; > + UINTN DelayVarSize; > + UINTN NoMapVarSize; > + BOOLEAN SilentStart; > > if (PcdGet8(PcdShellSupportLevel) > 3) { > return (EFI_UNSUPPORTED); > @@ -360,6 +365,7 @@ UefiMain ( > ShellInfoObject.PageBreakEnabled = > PcdGetBool(PcdShellPageBreakDefault); > ShellInfoObject.ViewingSettings.InsertMode = > PcdGetBool(PcdShellInsertModeDefault); > ShellInfoObject.LogScreenCount = PcdGet8 > (PcdShellScreenLogCount ); > + SilentStart = FALSE; > > // > // verify we dont allow for spec violation > @@ -452,6 +458,21 @@ UefiMain ( > goto FreeResources; > } > > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay) { > + // Command line has priority over the variable > + Status = ShellFindEnvVarInList(L"startupdelay", &DelayStr, > &DelayVarSize, NULL); > + if (!EFI_ERROR (Status)) { > + ShellInfoObject.ShellInitSettings.Delay = ShellStrToUintn > (DelayStr); > + } > + } > + > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) { > + Status = ShellFindEnvVarInList(L"silentstart", &NoMapStr, > &NoMapVarSize, NULL); > + if (!EFI_ERROR (Status)) { > + SilentStart = (BOOLEAN)ShellStrToUintn (NoMapStr); > + } > + } > + > // > // If shell support level is >= 1 create the mappings and paths > // > @@ -492,7 +513,7 @@ UefiMain ( > // > // Display the version > // > - if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion) { > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion && > !SilentStart) { > ShellPrintHiiEx ( > 0, > gST->ConOut->Mode->CursorRow, > @@ -529,7 +550,7 @@ UefiMain ( > // > // Display the mapping > // > - if (PcdGet8(PcdShellSupportLevel) >= 2 && > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) { > + if (PcdGet8(PcdShellSupportLevel) >= 2 && > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap && !SilentStart) { > Status = RunCommand(L"map"); > ASSERT_EFI_ERROR(Status); > } > > Shell fails to load. > Here is an excerpt from the debug log: > > add-symbol-file > /uefi/Build/StingrayPkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/Shel > l/DEBUG/Shell.dll 0x88480000 > Loading driver at 0x0008847F000 EntryPoint=0x00088480000 Shell.efi > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 8D095118 > ProtectUefiImageCommon - 0x8D08ED40 > - 0x000000008847F000 - 0x0000000000152000 > SetUefiImageMemoryAttributes - 0x000000008847F000 - 0x0000000000001000 > (0x0000000000004008) > SetUefiImageMemoryAttributes - 0x0000000088480000 - 0x00000000000E6000 > (0x0000000000020008) > SetUefiImageMemoryAttributes - 0x0000000088566000 - 0x000000000006B000 > (0x0000000000004008) > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8D088920 > InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA 8C71AF98 > InstallProtocolInterface: 6302D008-7F9B-4F30-87AC-60C9FEF5DA4E 88566710 > --- Blank lines ----- > 3h > --- Blank lines ----- > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > ASSERT [DxeCore] > /uefi/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c(300) > : ((BOOLEAN)(0==1)) > > Here 387477C2-69C7-11D2-8E39-00A0C969723B GUID is > gEfiSimpleTextOutProtocolGuid. > > And there is no way to do source-level debug because FV image cannot be > found in memory at the given location. > As soon as I revert this commit > (8ae5fc182941cf9ff7a222eb0a484088a0db8e2e), everything gets back to > normal. > Could you please explain me what I am doing wrong? > Does this help? --- a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c +++ b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c @@ -24,7 +24,7 @@ PlatformPeim ( VOID ) { - BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize)); + //BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize)); return EFI_SUCCESS; } (I assume you are using PrePi, and have nothing except the PrePi module in the primary FV) _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
> -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Tuesday, December 26, 2017 3:07 PM > To: Vladimir Olovyannikov > Cc: edk2-devel@lists.01.org; Leif Lindholm > Subject: Re: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't > reserve primary FV in memory > > On 26 December 2017 at 21:52, Vladimir Olovyannikov > <vladimir.olovyannikov@broadcom.com> wrote: > > Hi Ard, Meenakshi, > > > > I am having a problem I cannot explain the reason for, with this > > commit on an ARM64 platform. > > > > ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory > > > > Now that PrePi no longer exposes its internal code via special HOBs, > > we can remove the special handling of the primary FV, which needed > > to > > be reserved so that DXE core could call into the PE/COFF and LZMA > > libraries in the PrePi module. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Udit Kumar <udit.kumar@nxp.com> > > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > > [ardb: updated commit log] > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > > > > If a Shell is built "as is" from the source tree, there are no issues. > > However, if I slightly modify Shell.c like in the following patch: > > > > diff --git a/ShellPkg/Application/Shell/Shell.c > > b/ShellPkg/Application/Shell/Shell.c > > index 577e17311bea..bbbdde8ced96 100644 > > --- a/ShellPkg/Application/Shell/Shell.c > > +++ b/ShellPkg/Application/Shell/Shell.c > > @@ -339,6 +339,11 @@ UefiMain ( > > EFI_HANDLE ConInHandle; > > EFI_SIMPLE_TEXT_INPUT_PROTOCOL *OldConIn; > > SPLIT_LIST *Split; > > + CHAR16 *DelayStr; > > + CHAR16 *NoMapStr; > > + UINTN DelayVarSize; > > + UINTN NoMapVarSize; > > + BOOLEAN SilentStart; > > > > if (PcdGet8(PcdShellSupportLevel) > 3) { > > return (EFI_UNSUPPORTED); > > @@ -360,6 +365,7 @@ UefiMain ( > > ShellInfoObject.PageBreakEnabled = > > PcdGetBool(PcdShellPageBreakDefault); > > ShellInfoObject.ViewingSettings.InsertMode = > > PcdGetBool(PcdShellInsertModeDefault); > > ShellInfoObject.LogScreenCount = PcdGet8 > > (PcdShellScreenLogCount ); > > + SilentStart = FALSE; > > > > // > > // verify we dont allow for spec violation @@ -452,6 +458,21 @@ > > UefiMain ( > > goto FreeResources; > > } > > > > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay) { > > + // Command line has priority over the variable > > + Status = ShellFindEnvVarInList(L"startupdelay", &DelayStr, > > &DelayVarSize, NULL); > > + if (!EFI_ERROR (Status)) { > > + ShellInfoObject.ShellInitSettings.Delay = ShellStrToUintn > > (DelayStr); > > + } > > + } > > + > > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) { > > + Status = ShellFindEnvVarInList(L"silentstart", &NoMapStr, > > &NoMapVarSize, NULL); > > + if (!EFI_ERROR (Status)) { > > + SilentStart = (BOOLEAN)ShellStrToUintn (NoMapStr); > > + } > > + } > > + > > // > > // If shell support level is >= 1 create the mappings and paths > > // > > @@ -492,7 +513,7 @@ UefiMain ( > > // > > // Display the version > > // > > - if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion) { > > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion && > > !SilentStart) { > > ShellPrintHiiEx ( > > 0, > > gST->ConOut->Mode->CursorRow, @@ -529,7 +550,7 @@ UefiMain > > ( > > // > > // Display the mapping > > // > > - if (PcdGet8(PcdShellSupportLevel) >= 2 && > > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) { > > + if (PcdGet8(PcdShellSupportLevel) >= 2 && > > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap && !SilentStart) > > { > > Status = RunCommand(L"map"); > > ASSERT_EFI_ERROR(Status); > > } > > > > Shell fails to load. > > Here is an excerpt from the debug log: > > > > add-symbol-file > > > /uefi/Build/StingrayPkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/ > > Shel > > l/DEBUG/Shell.dll 0x88480000 > > Loading driver at 0x0008847F000 EntryPoint=0x00088480000 Shell.efi > > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF > > 8D095118 ProtectUefiImageCommon - 0x8D08ED40 > > - 0x000000008847F000 - 0x0000000000152000 > > SetUefiImageMemoryAttributes - 0x000000008847F000 - > 0x0000000000001000 > > (0x0000000000004008) > > SetUefiImageMemoryAttributes - 0x0000000088480000 - > 0x00000000000E6000 > > (0x0000000000020008) > > SetUefiImageMemoryAttributes - 0x0000000088566000 - > 0x000000000006B000 > > (0x0000000000004008) > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8D088920 > > InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA > > 8C71AF98 > > InstallProtocolInterface: 6302D008-7F9B-4F30-87AC-60C9FEF5DA4E > > 88566710 > > --- Blank lines ----- > > 3h > > --- Blank lines ----- > > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > A3ABE6B398 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > A3ABE6B398 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > A3ABE6B398 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > A3ABE6B398 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1E18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1E18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1E18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1E18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1E18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1E18 > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > 8C0A1B18 ASSERT [DxeCore] > > > /uefi/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c( > > 300) > > : ((BOOLEAN)(0==1)) > > > > Here 387477C2-69C7-11D2-8E39-00A0C969723B GUID is > > gEfiSimpleTextOutProtocolGuid. > > > > And there is no way to do source-level debug because FV image cannot > > be found in memory at the given location. > > As soon as I revert this commit > > (8ae5fc182941cf9ff7a222eb0a484088a0db8e2e), everything gets back to > > normal. > > Could you please explain me what I am doing wrong? > > > > Does this help? > > --- a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c > +++ b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c > @@ -24,7 +24,7 @@ PlatformPeim ( > VOID > ) > { > - BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize)); > + //BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize)); > > return EFI_SUCCESS; > } > > (I assume you are using PrePi, and have nothing except the PrePi module in > the primary FV) Thanks for response Ard, No, this does not help. In ArmPlatformGetVirtualMemoryMap () the platform reads a Pcd describing SDRAM memory configuration (regions), For each described region it creates a memory resource HOB like this: if (PrepareMemoryResourceHob ( MyDDRInfo[Index].Address, MyDDRInfo[Index].Size, MyDDRInfo[Index].Size + 1, Reserve ? EFI_RESOURCE_MEMORY_RESERVED : EFI_RESOURCE_SYSTEM_MEMORY )) { UINTN SizeGB; TotalHighMemAdded += MyDDRInfo[Index].Size; SizeGB = MyDDRInfo[Index].Size >> 30; DEBUG(( EFI_D_INFO, "HighMemMap: Added DDR %a area (%d). Start address: 0x%llx, size %llu %a (0x%llx)\n", Reserve ? "reserved" : "highmem", Index + 1, MyDDRInfo[Index].Address, SizeGB ? SizeGB : MyDDRInfo[Index].Size >> 20, SizeGB ? "GiB" : "MiB", MyDDRInfo[Index].Size )); Index++; } } else { MyDDRInfo[Index].Address = 0; MyDDRInfo[Index].Size = 0; } Thus it reports described and filled in areas of memory like this: HighMemMap: Added DDR highmem area (1). Start address: 0x8F101000, size 1 GiB (0x70EFF000) HighMemMap: Added DDR highmem area (2). Start address: 0x880000000, size 14 GiB (0x380000000) HighMemMap: Added DDR highmem area (3). Start address: 0x9000000000, size 16 GiB (0x400000000) HighMemMap: Added DDR highmem area (4). Start address: 0xA000000000, size 16 GiB (0x400000000) HighMemMap: Added DDR reserved area (1). Start address: 0x8F000000, size 1 MiB (0x101000) IS this not the right way to describe memory HOBs? With your proposed change assertion happens very early, here: ASSERT_EFI_ERROR (Status = Not Found) ASSERT [ArmPlatformPrePiMPCore] /uefi/ArmPlatformPkg/PrePi/PrePi.c(157): !EFI_ERROR (Status) So it cannot DecompressFirstFv(). If I don't apply your suggested change and revert the commit I mentioned earlier, it works fine... Thank you, Vladimir _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
> -----Original Message----- > From: Vladimir Olovyannikov [mailto:vladimir.olovyannikov@broadcom.com] > Sent: Tuesday, December 26, 2017 5:59 PM > To: 'Ard Biesheuvel' > Cc: 'edk2-devel@lists.01.org'; 'Leif Lindholm' > Subject: RE: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't > reserve primary FV in memory > > > -----Original Message----- > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > Sent: Tuesday, December 26, 2017 3:07 PM > > To: Vladimir Olovyannikov > > Cc: edk2-devel@lists.01.org; Leif Lindholm > > Subject: Re: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't > > reserve primary FV in memory > > > > On 26 December 2017 at 21:52, Vladimir Olovyannikov > > <vladimir.olovyannikov@broadcom.com> wrote: > > > Hi Ard, Meenakshi, > > > > > > I am having a problem I cannot explain the reason for, with this > > > commit on an ARM64 platform. > > > > > > ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in > > > memory > > > > > > Now that PrePi no longer exposes its internal code via special > > > HOBs, > > > we can remove the special handling of the primary FV, which needed > > > to > > > be reserved so that DXE core could call into the PE/COFF and LZMA > > > libraries in the PrePi module. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Udit Kumar <udit.kumar@nxp.com> > > > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > > > [ardb: updated commit log] > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > > > > > > If a Shell is built "as is" from the source tree, there are no issues. > > > However, if I slightly modify Shell.c like in the following patch: > > > > > > diff --git a/ShellPkg/Application/Shell/Shell.c > > > b/ShellPkg/Application/Shell/Shell.c > > > index 577e17311bea..bbbdde8ced96 100644 > > > --- a/ShellPkg/Application/Shell/Shell.c > > > +++ b/ShellPkg/Application/Shell/Shell.c > > > @@ -339,6 +339,11 @@ UefiMain ( > > > EFI_HANDLE ConInHandle; > > > EFI_SIMPLE_TEXT_INPUT_PROTOCOL *OldConIn; > > > SPLIT_LIST *Split; > > > + CHAR16 *DelayStr; > > > + CHAR16 *NoMapStr; > > > + UINTN DelayVarSize; > > > + UINTN NoMapVarSize; > > > + BOOLEAN SilentStart; > > > > > > if (PcdGet8(PcdShellSupportLevel) > 3) { > > > return (EFI_UNSUPPORTED); > > > @@ -360,6 +365,7 @@ UefiMain ( > > > ShellInfoObject.PageBreakEnabled = > > > PcdGetBool(PcdShellPageBreakDefault); > > > ShellInfoObject.ViewingSettings.InsertMode = > > > PcdGetBool(PcdShellInsertModeDefault); > > > ShellInfoObject.LogScreenCount = PcdGet8 > > > (PcdShellScreenLogCount ); > > > + SilentStart = FALSE; > > > > > > // > > > // verify we dont allow for spec violation @@ -452,6 +458,21 @@ > > > UefiMain ( > > > goto FreeResources; > > > } > > > > > > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay) { > > > + // Command line has priority over the variable > > > + Status = ShellFindEnvVarInList(L"startupdelay", &DelayStr, > > > &DelayVarSize, NULL); > > > + if (!EFI_ERROR (Status)) { > > > + ShellInfoObject.ShellInitSettings.Delay = ShellStrToUintn > > > (DelayStr); > > > + } > > > + } > > > + > > > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) { > > > + Status = ShellFindEnvVarInList(L"silentstart", &NoMapStr, > > > &NoMapVarSize, NULL); > > > + if (!EFI_ERROR (Status)) { > > > + SilentStart = (BOOLEAN)ShellStrToUintn (NoMapStr); > > > + } > > > + } > > > + > > > // > > > // If shell support level is >= 1 create the mappings and paths > > > // > > > @@ -492,7 +513,7 @@ UefiMain ( > > > // > > > // Display the version > > > // > > > - if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion) { > > > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion > > > + && > > > !SilentStart) { > > > ShellPrintHiiEx ( > > > 0, > > > gST->ConOut->Mode->CursorRow, @@ -529,7 +550,7 @@ > > > UefiMain ( > > > // > > > // Display the mapping > > > // > > > - if (PcdGet8(PcdShellSupportLevel) >= 2 && > > > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) { > > > + if (PcdGet8(PcdShellSupportLevel) >= 2 && > > > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap && > > > !SilentStart) { > > > Status = RunCommand(L"map"); > > > ASSERT_EFI_ERROR(Status); > > > } > > > > > > Shell fails to load. > > > Here is an excerpt from the debug log: > > > > > > add-symbol-file > > > > > > /uefi/Build/StingrayPkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/ > > > Shel > > > l/DEBUG/Shell.dll 0x88480000 > > > Loading driver at 0x0008847F000 EntryPoint=0x00088480000 Shell.efi > > > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF > > > 8D095118 ProtectUefiImageCommon - 0x8D08ED40 > > > - 0x000000008847F000 - 0x0000000000152000 > > > SetUefiImageMemoryAttributes - 0x000000008847F000 - > > 0x0000000000001000 > > > (0x0000000000004008) > > > SetUefiImageMemoryAttributes - 0x0000000088480000 - > > 0x00000000000E6000 > > > (0x0000000000020008) > > > SetUefiImageMemoryAttributes - 0x0000000088566000 - > > 0x000000000006B000 > > > (0x0000000000004008) > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8D088920 > > > InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA > > > 8C71AF98 > > > InstallProtocolInterface: 6302D008-7F9B-4F30-87AC-60C9FEF5DA4E > > > 88566710 > > > --- Blank lines ----- > > > 3h > > > --- Blank lines ----- > > > > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > A3ABE6B398 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > A3ABE6B398 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > A3ABE6B398 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > A3ABE6B398 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1E18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1E18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1E18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1E18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1E18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1E18 > > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > > > 8C0A1B18 ASSERT [DxeCore] > > > > > > /uefi/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c( > > > 300) > > > : ((BOOLEAN)(0==1)) > > > > > > Here 387477C2-69C7-11D2-8E39-00A0C969723B GUID is > > > gEfiSimpleTextOutProtocolGuid. > > > > > > And there is no way to do source-level debug because FV image cannot > > > be found in memory at the given location. > > > As soon as I revert this commit > > > (8ae5fc182941cf9ff7a222eb0a484088a0db8e2e), everything gets back to > > > normal. > > > Could you please explain me what I am doing wrong? > > > > > > > Does this help? > > > > --- a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c > > +++ b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c > > @@ -24,7 +24,7 @@ PlatformPeim ( > > VOID > > ) > > { > > - BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize)); > > + //BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize)); > > > > return EFI_SUCCESS; > > } > > > > (I assume you are using PrePi, and have nothing except the PrePi > > module in the primary FV) > Thanks for response Ard, > No, this does not help. > In ArmPlatformGetVirtualMemoryMap () the platform reads a Pcd describing > SDRAM memory configuration (regions), For each described region it creates > a memory resource HOB like this: > > if (PrepareMemoryResourceHob ( > MyDDRInfo[Index].Address, > MyDDRInfo[Index].Size, > MyDDRInfo[Index].Size + 1, > Reserve ? EFI_RESOURCE_MEMORY_RESERVED : > EFI_RESOURCE_SYSTEM_MEMORY > )) { > UINTN SizeGB; > > TotalHighMemAdded += MyDDRInfo[Index].Size; > SizeGB = MyDDRInfo[Index].Size >> 30; > DEBUG(( > EFI_D_INFO, > "HighMemMap: Added DDR %a area (%d). Start address: 0x%llx, size > %llu %a (0x%llx)\n", > Reserve ? "reserved" : "highmem", > Index + 1, > MyDDRInfo[Index].Address, > SizeGB ? SizeGB : MyDDRInfo[Index].Size >> 20, > SizeGB ? "GiB" : "MiB", > MyDDRInfo[Index].Size > )); > Index++; > } > } else { > MyDDRInfo[Index].Address = 0; > MyDDRInfo[Index].Size = 0; > } > > Thus it reports described and filled in areas of memory like this: > HighMemMap: Added DDR highmem area (1). Start address: 0x8F101000, size > 1 GiB (0x70EFF000) > HighMemMap: Added DDR highmem area (2). Start address: 0x880000000, > size 14 GiB (0x380000000) > HighMemMap: Added DDR highmem area (3). Start address: 0x9000000000, > size 16 GiB (0x400000000) > HighMemMap: Added DDR highmem area (4). Start address: 0xA000000000, > size 16 GiB (0x400000000) > HighMemMap: Added DDR reserved area (1). Start address: 0x8F000000, size > 1 MiB (0x101000) > > IS this not the right way to describe memory HOBs? > > With your proposed change assertion happens very early, here: > > ASSERT_EFI_ERROR (Status = Not Found) > ASSERT [ArmPlatformPrePiMPCore] > /uefi/ArmPlatformPkg/PrePi/PrePi.c(157): !EFI_ERROR (Status) > > So it cannot DecompressFirstFv(). > If I don't apply your suggested change and revert the commit I mentioned > earlier, it works fine... > To clarify, PreparememoryResourceHob looks like this: BOOLEAN PrepareMemoryResourceHob ( IN EFI_PHYSICAL_ADDRESS Address, IN UINT64 MemSize, IN UINT64 MaxAllowedSize, UINTN MemType ) { BOOLEAN HobCreated; HobCreated = FALSE; if ((MemSize > 0) && (MemSize <= MaxAllowedSize)) { // Additional memory is available. Declare it. EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes; ResourceAttributes = SYSTEM_MEMORY_ATTRS; if (MemType != EFI_RESOURCE_SYSTEM_MEMORY) { ResourceAttributes = RESERVED_MEMORY_ATTRS; } BuildResourceDescriptorHob ( MemType, ResourceAttributes, Address, MemSize); HobCreated = TRUE; } return HobCreated; } And SYSTEM_MEMORY_ATTRS: #define RESERVED_MEMORY_ATTRS \ EFI_RESOURCE_ATTRIBUTE_PRESENT | \ EFI_RESOURCE_ATTRIBUTE_WRITE_PROTECTED | \ EFI_RESOURCE_ATTRIBUTE_WRITE_PROTECTABLE | \ EFI_RESOURCE_ATTRIBUTE_INITIALIZED | \ EFI_RESOURCE_ATTRIBUTE_READ_PROTECTED | \ EFI_RESOURCE_ATTRIBUTE_READ_PROTECTABLE | \ EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE | \ EFI_RESOURCE_ATTRIBUTE_TESTED > Thank you, > Vladimir _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hi Vladimir How re-allocation or say drivers are dispatched on your system. Could you check addresses, where FV is kept and where this is getting dispatched Thx Udit > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Vladimir Olovyannikov > Sent: Wednesday, December 27, 2017 3:22 AM > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org > Cc: leif.lindholm@linaro.org > Subject: Re: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't > reserve primary FV in memory > > Hi Ard, Meenakshi, > > I am having a problem I cannot explain the reason for, with this commit on > an ARM64 platform. > > ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory > > Now that PrePi no longer exposes its internal code via special HOBs, > we can remove the special handling of the primary FV, which needed to > be reserved so that DXE core could call into the PE/COFF and LZMA > libraries in the PrePi module. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Udit Kumar <udit.kumar@nxp.com> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > [ardb: updated commit log] > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > > If a Shell is built "as is" from the source tree, there are no issues. > However, if I slightly modify Shell.c like in the following patch: > > diff --git a/ShellPkg/Application/Shell/Shell.c > b/ShellPkg/Application/Shell/Shell.c > index 577e17311bea..bbbdde8ced96 100644 > --- a/ShellPkg/Application/Shell/Shell.c > +++ b/ShellPkg/Application/Shell/Shell.c > @@ -339,6 +339,11 @@ UefiMain ( > EFI_HANDLE ConInHandle; > EFI_SIMPLE_TEXT_INPUT_PROTOCOL *OldConIn; > SPLIT_LIST *Split; > + CHAR16 *DelayStr; > + CHAR16 *NoMapStr; > + UINTN DelayVarSize; > + UINTN NoMapVarSize; > + BOOLEAN SilentStart; > > if (PcdGet8(PcdShellSupportLevel) > 3) { > return (EFI_UNSUPPORTED); > @@ -360,6 +365,7 @@ UefiMain ( > ShellInfoObject.PageBreakEnabled = > PcdGetBool(PcdShellPageBreakDefault); > ShellInfoObject.ViewingSettings.InsertMode = > PcdGetBool(PcdShellInsertModeDefault); > ShellInfoObject.LogScreenCount = PcdGet8 > (PcdShellScreenLogCount ); > + SilentStart = FALSE; > > // > // verify we dont allow for spec violation @@ -452,6 +458,21 @@ UefiMain > ( > goto FreeResources; > } > > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay) { > + // Command line has priority over the variable > + Status = ShellFindEnvVarInList(L"startupdelay", &DelayStr, > &DelayVarSize, NULL); > + if (!EFI_ERROR (Status)) { > + ShellInfoObject.ShellInitSettings.Delay = ShellStrToUintn > (DelayStr); > + } > + } > + > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) { > + Status = ShellFindEnvVarInList(L"silentstart", &NoMapStr, > &NoMapVarSize, NULL); > + if (!EFI_ERROR (Status)) { > + SilentStart = (BOOLEAN)ShellStrToUintn (NoMapStr); > + } > + } > + > // > // If shell support level is >= 1 create the mappings and paths > // > @@ -492,7 +513,7 @@ UefiMain ( > // > // Display the version > // > - if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion) { > + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion && > !SilentStart) { > ShellPrintHiiEx ( > 0, > gST->ConOut->Mode->CursorRow, @@ -529,7 +550,7 @@ UefiMain ( > // > // Display the mapping > // > - if (PcdGet8(PcdShellSupportLevel) >= 2 && > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) { > + if (PcdGet8(PcdShellSupportLevel) >= 2 && > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap && !SilentStart) { > Status = RunCommand(L"map"); > ASSERT_EFI_ERROR(Status); > } > > Shell fails to load. > Here is an excerpt from the debug log: > > add-symbol-file > /uefi/Build/StingrayPkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/ > Shel > l/DEBUG/Shell.dll 0x88480000 > Loading driver at 0x0008847F000 EntryPoint=0x00088480000 Shell.efi > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 8D095118 > ProtectUefiImageCommon - 0x8D08ED40 > - 0x000000008847F000 - 0x0000000000152000 > SetUefiImageMemoryAttributes - 0x000000008847F000 - > 0x0000000000001000 > (0x0000000000004008) > SetUefiImageMemoryAttributes - 0x0000000088480000 - > 0x00000000000E6000 > (0x0000000000020008) > SetUefiImageMemoryAttributes - 0x0000000088566000 - > 0x000000000006B000 > (0x0000000000004008) > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8D088920 > InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA 8C71AF98 > InstallProtocolInterface: 6302D008-7F9B-4F30-87AC-60C9FEF5DA4E 88566710 > --- Blank lines ----- > 3h > --- Blank lines ----- > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > A3ABE6B398 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > A3ABE6B398 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > A3ABE6B398 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B > A3ABE6B398 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18 > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18 > ASSERT [DxeCore] > /uefi/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c(3 > 00) > : ((BOOLEAN)(0==1)) > > Here 387477C2-69C7-11D2-8E39-00A0C969723B GUID is > gEfiSimpleTextOutProtocolGuid. > > And there is no way to do source-level debug because FV image cannot be > found in memory at the given location. > As soon as I revert this commit > (8ae5fc182941cf9ff7a222eb0a484088a0db8e2e), everything gets back to > normal. > Could you please explain me what I am doing wrong? > > Thank you, > Vladimir > > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard > Biesheuvel > Sent: Thursday, November 30, 2017 7:25 AM > To: edk2-devel@lists.01.org > Cc: leif.lindholm@linaro.org; Ard Biesheuvel > Subject: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve > primary FV in memory > > From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > > Now that PrePi no longer exposes its internal code via special HOBs, we can > remove the special handling of the primary FV, which needed to be reserved > so that DXE core could call into the PE/COFF and LZMA libraries in the PrePi > module. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Udit Kumar <udit.kumar@nxp.com> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > [ardb: updated commit log] > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69 -------------------- > 1 file changed, 69 deletions(-) > > diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > index 2feb11f21d5d..d03214b5df66 100644 > --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > @@ -70,11 +70,7 @@ MemoryPeim ( > { > ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable; > EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes; > - UINT64 ResourceLength; > EFI_PEI_HOB_POINTERS NextHob; > - EFI_PHYSICAL_ADDRESS FdTop; > - EFI_PHYSICAL_ADDRESS SystemMemoryTop; > - EFI_PHYSICAL_ADDRESS ResourceTop; > BOOLEAN Found; > > // Get Virtual Memory Map from the Platform Library @@ -121,71 +117,6 > @@ MemoryPeim ( > ); > } > > - // > - // Reserved the memory space occupied by the firmware volume > - // > - > - SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 > (PcdSystemMemoryBase) > + (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize); > - FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + > (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize); > - > - // EDK2 does not have the concept of boot firmware copied into DRAM. To > avoid the DXE > - // core to overwrite this area we must mark the region with the attribute > non-present > - if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) > && (FdTop <= SystemMemoryTop)) { > - Found = FALSE; > - > - // Search for System Memory Hob that contains the firmware > - NextHob.Raw = GetHobList (); > - while ((NextHob.Raw = GetNextHob > (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, > NextHob.Raw)) != NULL) { > - if ((NextHob.ResourceDescriptor->ResourceType == > EFI_RESOURCE_SYSTEM_MEMORY) && > - (PcdGet64 (PcdFdBaseAddress) >= > NextHob.ResourceDescriptor->PhysicalStart) && > - (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + > NextHob.ResourceDescriptor->ResourceLength)) > - { > - ResourceAttributes = > NextHob.ResourceDescriptor->ResourceAttribute; > - ResourceLength = NextHob.ResourceDescriptor->ResourceLength; > - ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + > ResourceLength; > - > - if (PcdGet64 (PcdFdBaseAddress) == > NextHob.ResourceDescriptor->PhysicalStart) { > - if (SystemMemoryTop == FdTop) { > - NextHob.ResourceDescriptor->ResourceAttribute = > ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT; > - } else { > - // Create the System Memory HOB for the firmware with the > non-present attribute > - BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > - ResourceAttributes & > ~EFI_RESOURCE_ATTRIBUTE_PRESENT, > - PcdGet64 (PcdFdBaseAddress), > - PcdGet32 (PcdFdSize)); > - > - // Top of the FD is system memory available for UEFI > - NextHob.ResourceDescriptor->PhysicalStart += > PcdGet32(PcdFdSize); > - NextHob.ResourceDescriptor->ResourceLength -= > PcdGet32(PcdFdSize); > - } > - } else { > - // Create the System Memory HOB for the firmware with the > non-present attribute > - BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > - ResourceAttributes & > ~EFI_RESOURCE_ATTRIBUTE_PRESENT, > - PcdGet64 (PcdFdBaseAddress), > - PcdGet32 (PcdFdSize)); > - > - // Update the HOB > - NextHob.ResourceDescriptor->ResourceLength = PcdGet64 > (PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart; > - > - // If there is some memory available on the top of the FD then > create a HOB > - if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + > ResourceLength) { > - // Create the System Memory HOB for the remaining region (top > of the FD) > - BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > - ResourceAttributes, > - FdTop, > - ResourceTop - FdTop); > - } > - } > - Found = TRUE; > - break; > - } > - NextHob.Raw = GET_NEXT_HOB (NextHob); > - } > - > - ASSERT(Found); > - } > - > // Build Memory Allocation Hob > InitMmu (MemoryTable); > > -- > 2.11.0 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists. > 01.org%2Fmailman%2Flistinfo%2Fedk2- > devel&data=02%7C01%7Cudit.kumar%40nxp.com%7Ca75148c0714749bda72 > d08d54caaf73b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6364 > 99219600898902&sdata=nmhLohXfQGCJE3F%2FDan1YPZCcgbCZ6yyxcZsdZ71h > P8%3D&reserved=0 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists. > 01.org%2Fmailman%2Flistinfo%2Fedk2- > devel&data=02%7C01%7Cudit.kumar%40nxp.com%7Ca75148c0714749bda72 > d08d54caaf73b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6364 > 99219600898902&sdata=nmhLohXfQGCJE3F%2FDan1YPZCcgbCZ6yyxcZsdZ71h > P8%3D&reserved=0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c index 2feb11f21d5d..d03214b5df66 100644 --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c @@ -70,11 +70,7 @@ MemoryPeim ( { ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable; EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes; - UINT64 ResourceLength; EFI_PEI_HOB_POINTERS NextHob; - EFI_PHYSICAL_ADDRESS FdTop; - EFI_PHYSICAL_ADDRESS SystemMemoryTop; - EFI_PHYSICAL_ADDRESS ResourceTop; BOOLEAN Found; // Get Virtual Memory Map from the Platform Library @@ -121,71 +117,6 @@ MemoryPeim ( ); } - // - // Reserved the memory space occupied by the firmware volume - // - - SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize); - FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize); - - // EDK2 does not have the concept of boot firmware copied into DRAM. To avoid the DXE - // core to overwrite this area we must mark the region with the attribute non-present - if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) && (FdTop <= SystemMemoryTop)) { - Found = FALSE; - - // Search for System Memory Hob that contains the firmware - NextHob.Raw = GetHobList (); - while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) { - if ((NextHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) && - (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor->PhysicalStart) && - (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + NextHob.ResourceDescriptor->ResourceLength)) - { - ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute; - ResourceLength = NextHob.ResourceDescriptor->ResourceLength; - ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + ResourceLength; - - if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor->PhysicalStart) { - if (SystemMemoryTop == FdTop) { - NextHob.ResourceDescriptor->ResourceAttribute = ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT; - } else { - // Create the System Memory HOB for the firmware with the non-present attribute - BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, - ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT, - PcdGet64 (PcdFdBaseAddress), - PcdGet32 (PcdFdSize)); - - // Top of the FD is system memory available for UEFI - NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize); - NextHob.ResourceDescriptor->ResourceLength -= PcdGet32(PcdFdSize); - } - } else { - // Create the System Memory HOB for the firmware with the non-present attribute - BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, - ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT, - PcdGet64 (PcdFdBaseAddress), - PcdGet32 (PcdFdSize)); - - // Update the HOB - NextHob.ResourceDescriptor->ResourceLength = PcdGet64 (PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart; - - // If there is some memory available on the top of the FD then create a HOB - if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + ResourceLength) { - // Create the System Memory HOB for the remaining region (top of the FD) - BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, - ResourceAttributes, - FdTop, - ResourceTop - FdTop); - } - } - Found = TRUE; - break; - } - NextHob.Raw = GET_NEXT_HOB (NextHob); - } - - ASSERT(Found); - } - // Build Memory Allocation Hob InitMmu (MemoryTable);