Re: [PATCH] MdeModulePkg/Core/Dxe: log memory base and length, after lib ctors again

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 11/3/20 5:15 PM, Laszlo Ersek wrote:
> CoreInitializeMemoryServices() logs "BaseAddress" and "Length" with
> DEBUG() before DxeMain() calls ProcessLibraryConstructorList()
> explicitly. (Library construction is not an automatic part of the DXE
> Core entry point.)
> 
> So those DEBUG()s in CoreInitializeMemoryServices() are issued against
> an un-constructed DebugLib, and also against a -- possibly underlying --
> un-constructed SerialPortLib.

Uh...

> 
> Some DebugLib instances can deal with this (see for example commit
> 91a5b1365075, "OvmfPkg/PlatformDebugLibIoPort: fix port detection for
> use in the DXE Core", 2018-08-06), while some others can't (see for
> example the DebugLib instance
> "MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf"
> coupled with the SerialPortLib instance
> "ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf").

Yes.

> 
> Addressing this issue in a SerialPortLib instance that underlies
> BaseDebugLibSerialPort seems wrong; either the DebugLib instance should
> cope directly with being called un-constructed (see again commit
> 91a5b1365075), or the DXE Core should log relevant information *at
> least* after library instances have been constructed. This patch
> implements the latter (only for the "BaseAddress" and "Length" values
> calculated by CoreInitializeMemoryServices()).

Sounds good.

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> 
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jeff Brasen <jbrasen@nvidia.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     Repo:   https://pagure.io/lersek/edk2.git
>     Branch: dxecore_report_mem
> 
>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> index 5ee4cd10b436..db21311f9352 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> @@ -297,8 +297,16 @@ DxeMain (
>    ProcessLibraryConstructorList (gDxeCoreImageHandle, gDxeCoreST);
>    PERF_CROSSMODULE_END   ("PEI");
>    PERF_CROSSMODULE_BEGIN ("DXE");
>  
> +  //
> +  // Log MemoryBaseAddress and MemoryLength again (from
> +  // CoreInitializeMemoryServices()), now that library constructors have
> +  // executed.
> +  //
> +  DEBUG ((DEBUG_INFO, "%a: MemoryBaseAddress=0x%Lx MemoryLength=0x%Lx\n",
> +    __FUNCTION__, MemoryBaseAddress, MemoryLength));
> +
>    //
>    // Report DXE Core image information to the PE/COFF Extra Action Library
>    //
>    ZeroMem (&ImageContext, sizeof (ImageContext));
>