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

Laszlo Ersek posted 1 patch 3 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/edk2 tags/patchew/20201103161557.30621-1-lersek@redhat.com
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 8 ++++++++
1 file changed, 8 insertions(+)
[edk2-devel] [PATCH] MdeModulePkg/Core/Dxe: log memory base and length, after lib ctors again
Posted by Laszlo Ersek 3 years, 5 months ago
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66932): https://edk2.groups.io/g/devel/message/66932
Mute This Topic: https://groups.io/mt/78009800/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] MdeModulePkg/Core/Dxe: log memory base and length, after lib ctors again
Posted by Philippe Mathieu-Daudé 3 years, 5 months ago
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));
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66937): https://edk2.groups.io/g/devel/message/66937
Mute This Topic: https://groups.io/mt/78009800/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


[edk2-devel] 回复: [PATCH] MdeModulePkg/Core/Dxe: log memory base and length, after lib ctors again
Posted by gaoliming 3 years, 5 months ago
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));
> >





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67019): https://edk2.groups.io/g/devel/message/67019
Mute This Topic: https://groups.io/mt/78043504/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] 回复: [PATCH] MdeModulePkg/Core/Dxe: log memory base and length, after lib ctors again
Posted by Laszlo Ersek 3 years, 5 months ago
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));
>>>
> 
> 
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67114): https://edk2.groups.io/g/devel/message/67114
Mute This Topic: https://groups.io/mt/78043504/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-