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

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

[Laszlo Ersek]

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.

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").

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()).

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));
-- 
2.19.1.3.g30247aa5d201

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

[Philippe Mathieu-Daudé]

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));
> 

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

[gaoliming]

Laszlo:
  With this change, MemoryBaseAddress and MemoryLength can print to serial port after serial port constructor is called. 
  
  Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: Philippe Mathieu-Daudé <philmd@redhat.com>
> 发送时间: 2020年11月4日 2:13
> 收件人: Laszlo Ersek <lersek@redhat.com>; edk2-devel-groups-io
> <devel@edk2.groups.io>
> 抄送: Ard Biesheuvel <ard.biesheuvel@arm.com>; Dandan Bi
> <dandan.bi@intel.com>; Hao A Wu <hao.a.wu@intel.com>; Jeff Brasen
> <jbrasen@nvidia.com>; Jian J Wang <jian.j.wang@intel.com>; Leif Lindholm
> <leif@nuviainc.com>; Liming Gao <gaoliming@byosoft.com.cn>
> 主题: Re: [PATCH] MdeModulePkg/Core/Dxe: log memory base and length,
> after lib ctors again
> 
> 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));
> >

Re: [edk2-devel] 回复: [PATCH] MdeModulePkg/Core/Dxe: log memory base and length, after lib ctors again

[Laszlo Ersek]

On 11/05/20 03:07, gaoliming wrote:
> Laszlo:
>   With this change, MemoryBaseAddress and MemoryLength can print to serial port after serial port constructor is called. 
>   
>   Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thank you both for the reviews; merged as commit 1366cd58cd44, via
<https://github.com/tianocore/edk2/pull/1099>.

Laszlo

> 
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: Philippe Mathieu-Daudé <philmd@redhat.com>
>> 发送时间: 2020年11月4日 2:13
>> 收件人: Laszlo Ersek <lersek@redhat.com>; edk2-devel-groups-io
>> <devel@edk2.groups.io>
>> 抄送: Ard Biesheuvel <ard.biesheuvel@arm.com>; Dandan Bi
>> <dandan.bi@intel.com>; Hao A Wu <hao.a.wu@intel.com>; Jeff Brasen
>> <jbrasen@nvidia.com>; Jian J Wang <jian.j.wang@intel.com>; Leif Lindholm
>> <leif@nuviainc.com>; Liming Gao <gaoliming@byosoft.com.cn>
>> 主题: Re: [PATCH] MdeModulePkg/Core/Dxe: log memory base and length,
>> after lib ctors again
>>
>> 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));
>>>
> 
> 
> 
> 
> 
> 
> 
>