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-boun...@lists.01.org] On Behalf Of
> Vladimir Olovyannikov
> Sent: Wednesday, December 27, 2017 3:22 AM
> To: Ard Biesheuvel <ard.biesheu...@linaro.org>; edk2-devel@lists.01.org
> Cc: leif.lindh...@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.ku...@nxp.com>
>     Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>     [ardb: updated commit log]
>     Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
>     Reviewed-by: Leif Lindholm <leif.lindh...@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-boun...@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Thursday, November 30, 2017 7:25 AM
> To: edk2-devel@lists.01.org
> Cc: leif.lindh...@linaro.org; Ard Biesheuvel
> Subject: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve
> primary FV in memory
> 
> From: Meenakshi Aggarwal <meenakshi.aggar...@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.ku...@nxp.com>
> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
> [ardb: updated commit log]
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@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

Reply via email to