On 2016/3/17 17:11, Laszlo Ersek wrote:
Adding Ard

On 03/17/16 08:28, Zeng, Star wrote:
On 2016/3/17 15:16, Andrew Fish wrote:

On Mar 17, 2016, at 12:10 AM, Gao, Liming <[email protected]> wrote:

Andrew:
   DxeDebugPrintErrorLevelLib has Constructor. DxeHobLib has
Constructor. If DebugLib also has Constructor, the circle of
Constructor() will happen and cause build break.
DxeDebugPrintErrorLevelLib instance increases the possibility of the
circle Constructor(). To resolve it, we can update
DxeDebugPrintErrorLevelLib or DxeHobLib library instance by moving
their constructor into every function API.


Liming,

Thanks, I came to the same conclusion. I'm making a local copy
DxeDebugPrintErrorLevelLib that does the HOB accesses in the driver
without the HobLib and that will solve our issue. I was just surprised
the edk2 library was not very useful in the real world. Not to mention
the build error message did not help find the real issue, the HOB lib.

I reproduced the issue locally. With the change below to integrate
HobLibConstructor() to GetHobList(), since all other interface of
DxeHobLib are just to call GetHobList(), the issue is gone. What is your
opinion?

This could make it possible for ArmVirtPkg to remove the
ArmVirtDxeHobLib library instance. That library instance was cloned
exactly in order to break this dependency cycle, see commit ad90df8ac0.

I think the proposal and the patch are good.

Thanks for the comments.
Got more information (interesting for me, but seems bad news) after running NT32 with the code change.

1. With the code change, the AutoGen.c of PcdDxe(The first executed DXE driver except DxeMain) has below list for ProcessLibraryConstructorList(), that makes NT32 run into exception.

VOID
EFIAPI
ProcessLibraryConstructorList (     // With the code change
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
Status = DxeDebugPrintErrorLevelLibConstructor (ImageHandle, SystemTable);
  ASSERT_EFI_ERROR (Status);

  Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
  ASSERT_EFI_ERROR (Status);

...

}

Why do the exception happen? Because
DxeDebugPrintErrorLevelLibConstructor() ->
GetDebugPrintErrorLevel() ->
GetFirstGuidHob() ->
GetHobList() ->
EfiGetSystemConfigurationTable() ->
gST->XXX (in UefiLib.c)

But gST is still NULL at this moment as UefiBootServicesTableLibConstructor() is not run yet. To fix it, I still need to refine the code in DxeDebugPrintErrorLevelLib.c to do not get HOB in Constructor.

2. Without the code change, the AutoGen.c of PcdDxe(The first executed DXE driver except DxeMain) has below list for ProcessLibraryConstructorList().

VOID
EFIAPI
ProcessLibraryConstructorList (     // Without the code change
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
  Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
  ASSERT_EFI_ERROR (Status);

...

Status = DxeDebugPrintErrorLevelLibConstructor (ImageHandle, SystemTable);
  ASSERT_EFI_ERROR (Status);
}

So according to this practice, a Library instance has Constructor makes the dependency relationship to be more accurate, but more possible to have circular issue. Oppositely without Constructor.

Thanks,
Star


Thanks
Laszlo



9281d5a1b5c201e58a6d1d64ea8d1fe1058dbfc3
  MdePkg/Library/DxeHobLib/DxeHobLib.inf |  1 -
  MdePkg/Library/DxeHobLib/HobLib.c      | 37
+++++++---------------------------
  2 files changed, 7 insertions(+), 31 deletions(-)

diff --git a/MdePkg/Library/DxeHobLib/DxeHobLib.inf
b/MdePkg/Library/DxeHobLib/DxeHobLib.inf
index 32cd5546ab37..cb6e9fa21684 100644
--- a/MdePkg/Library/DxeHobLib/DxeHobLib.inf
+++ b/MdePkg/Library/DxeHobLib/DxeHobLib.inf
@@ -24,7 +24,6 @@ [Defines]
    MODULE_TYPE                    = DXE_DRIVER
    VERSION_STRING                 = 1.0
    LIBRARY_CLASS                  = HobLib|DXE_DRIVER DXE_RUNTIME_DRIVER
DXE_SAL_DRIVER SMM_CORE DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
-  CONSTRUCTOR                    = HobLibConstructor

  #
  #  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
diff --git a/MdePkg/Library/DxeHobLib/HobLib.c
b/MdePkg/Library/DxeHobLib/HobLib.c
index f0861ffcb4e6..24c14a228adb 100644
--- a/MdePkg/Library/DxeHobLib/HobLib.c
+++ b/MdePkg/Library/DxeHobLib/HobLib.c
@@ -24,35 +24,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
EITHER EXPRESS OR IMPLIED.
  VOID  *mHobList = NULL;

  /**
-  The constructor function caches the pointer to HOB list.
-
-  The constructor function gets the start address of HOB list from
system configuration table.
-  It will ASSERT() if that operation fails and it will always return
EFI_SUCCESS.
-
-  @param  ImageHandle   The firmware allocated handle for the EFI image.
-  @param  SystemTable   A pointer to the EFI System Table.
-
-  @retval EFI_SUCCESS   The constructor successfully gets HobList.
-  @retval Other value   The constructor can't get HobList.
-
-**/
-EFI_STATUS
-EFIAPI
-HobLibConstructor (
-  IN EFI_HANDLE        ImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
-  )
-{
-  EFI_STATUS  Status;
-
-  Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &mHobList);
-  ASSERT_EFI_ERROR (Status);
-  ASSERT (mHobList != NULL);
-
-  return Status;
-}
-
-/**
    Returns the pointer to the HOB list.

    This function returns the pointer to first HOB in the list.
@@ -74,7 +45,13 @@ GetHobList (
    VOID
    )
  {
-  ASSERT (mHobList != NULL);
+  EFI_STATUS  Status;
+
+  if (mHobList == NULL) {
+    Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &mHobList);
+    ASSERT_EFI_ERROR (Status);
+    ASSERT (mHobList != NULL);
+  }
    return mHobList;
  }





Thanks,

Andrew Fish

Thanks
Liming
-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Thursday, March 17, 2016 10:50 AM
To: Zeng, Star
Cc: edk2-devel; Gao, Liming
Subject: Re: [edk2] Has any one else had issues trying to use
DxeDebugPrintErrorLevelLib?
DebugPrintErrorLevelLib|MdeModulePkg/Library/DxeDebugPrintErrorLevelL
ib/DxeDebugPrintErrorLevelLib.inf


On Mar 16, 2016, at 7:37 PM, Andrew Fish <[email protected]> wrote:


On Mar 16, 2016, at 7:18 PM, Zeng, Star <[email protected]> wrote:

On 2016/3/17 10:02, Andrew Fish wrote:

On Mar 16, 2016, at 6:59 PM, Zeng, Star <[email protected]>
wrote:

On 2016/3/17 7:12, Andrew Fish wrote:
I was trying to move over to DxeDebugPrintErrorLevelLib, but I
can't
get it to build

For example I made DXE use
DebugPrintErrorLevelLib|MdeModulePkg/Library/DxeDebugPrintErrorLevelL
ib/DxeDebugPrintErrorLevelLib.inf in the EmulatorPkg and I get this
error:

/Users/andrewfish/work/src/edk2/EmulatorPkg/EmulatorPkg.dsc(...):
error F002: Library
[/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiBootServicesTable
Lib/UefiBootServicesTableLib.inf] with constructors has a cycle
         consumed by
/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiLib/UefiLib.inf

The error message is not very helpful on root causing the
issue....

But a circular constructor issue does not seem that strange in the
DebugLib stack as everything uses that! If I go into
DxeDebugPrintErrorLevelLib.inf  and remove the HobLib I don't get the
Constructor error, but I do get a link error.

So it looks like the DxeDebugPrintErrorLevelLib depends on the Hob
lib that depends on the DebugLib that depends on the
DxeDebugPrintErrorLevelLib. How did this every work? The only place
I see it
used is the Nt32Pkg, and I don't have a system to test that on????

Just tried NT32 with VS2015, and it builds and works well.

Liming & Yonghong, any idea on this?


I was wondering if it had to do with the lower level libraries
having a
constructor? So maybe library implementation matters?

If this circular constructor issue could be reproduced with only one
module linked with DxeDebugPrintErrorLevelLib (I mean other modules to
link with BaseDebugPrintErrorLevelLib, only the module links to
DxeDebugPrintErrorLevelLib), then that may be helpful to scrub all
the library
instances the module links to find out what is the matter. :)


OK in the EmulatorPkg the DXE Core compiles, but
MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHan
dlerRuntimeDxe.inf will error out.


I think it is as simple as you can't have a library with a
constructor required by
the DebugLib (and child libraries) for a library that depends on the
DebugLib.
So the chain to get the DebugLib started depends on the DebugLib.

That kind of makes DxeDebugPrintErrorLevelLib hard to use I guess?

Thanks,

Andrew Fish

Library
-------------------------------------------------------------------------------------------

-----------------------------

/Users/andrewfish/work/src/edk2/MdePkg/Library/BasePcdLibNull/BasePc
dLibNull.inf
{PcdLib}
/Users/andrewfish/work/src/edk2/MdePkg/Library/BaseLib/BaseLib.inf
{BaseLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/BaseMemoryLib/BaseM
emoryLib.inf
{BaseMemoryLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/DxeCoreEntryPoint/Dxe
CoreEntryPoint.inf
{DxeCoreEntryPoint}

/Users/andrewfish/work/src/edk2/MdePkg/Library/DxeCoreHobLib/DxeCor
eHobLib.inf
{HobLib}

/Users/andrewfish/work/src/edk2/MdeModulePkg/Library/DxeDebugPrintE
rrorLevelLib/DxeDebugPrintErrorLevelLib.inf
{DebugPrintErrorLevelLib:  C =
DxeDebugPrintErrorLevelLibConstructor D =
DxeDebugPrintErrorLevelLibDestructor}

/Users/andrewfish/work/src/edk2/EmulatorPkg/Library/DxeEmuStdErrSerial
PortLib/DxeEmuStdErrSerialPortLib.inf
{SerialPortLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/BasePrintLib/BasePrintLi
b.inf
{PrintLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/BaseDebugLibSerialPort
/BaseDebugLibSerialPort.inf
{DebugLib:  C = BaseDebugLibSerialPortConstructor}

/Users/andrewfish/work/src/edk2/EmulatorPkg/Library/DxeEmuLib/DxeEm
uLib.inf
{EmuThunkLib:  C = DxeEmuLibConstructor}

/Users/andrewfish/work/src/edk2/MdeModulePkg/Library/DxeCoreMemor
yAllocationLib/DxeCoreMemoryAllocationLib.inf
{MemoryAllocationLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiBootServicesTableLi
b/UefiBootServicesTableLib.inf
{UefiBootServicesTableLib:  C = UefiBootServicesTableLibConstructor}

/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiRuntimeServicesTa
bleLib/UefiRuntimeServicesTableLib.inf
{UefiRuntimeServicesTableLib:  C =
UefiRuntimeServicesTableLibConstructor}

/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiDevicePathLib/Uefi
DevicePathLib.inf
{DevicePathLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/DxeExtractGuidedSectio
nLib/DxeExtractGuidedSectionLib.inf
{ExtractGuidedSectionLib:  C = DxeExtractGuidedSectionLibConstructor}

/Users/andrewfish/work/src/edk2/EmulatorPkg/Library/DxeEmuPeCoffExtr
aActionLib/DxeEmuPeCoffExtraActionLib.inf
{PeCoffExtraActionLib:  C = DxeEmuPeCoffLibExtraActionConstructor}
/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiLib/UefiLib.inf
{UefiLib:  C = UefiLibConstructor}

/Users/andrewfish/work/src/edk2/MdePkg/Library/BaseCacheMaintenance
Lib/BaseCacheMaintenanceLib.inf
{CacheMaintenanceLib}

/Users/andrewfish/work/src/edk2/IntelFrameworkModulePkg/Library/Base
UefiTianoCustomDecompressLib
/BaseUefiTianoCustomDecompressLib.inf
{UefiDecompressLib:  C = TianoDecompressLibConstructor}

/Users/andrewfish/work/src/edk2/MdePkg/Library/BasePerformanceLibNul
l/BasePerformanceLibNull.inf
{PerformanceLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/BasePeCoffLib/BasePeC
offLib.inf
{PeCoffLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/BasePeCoffGetEntryPoi
ntLib/BasePeCoffGetEntryPointLib.inf
{PeCoffGetEntryPointLib}

/Users/andrewfish/work/src/edk2/MdeModulePkg/Library/DxeReportStatu
sCodeLib/DxeReportStatusCodeLib.inf
{ReportStatusCodeLib}

/Users/andrewfish/work/src/edk2/EmulatorPkg/Library/DxeCoreTimerLib/D
xeCoreTimerLib.inf
{TimerLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/DxeServicesLib/DxeServ
icesLib.inf
{DxeServicesLib}

/Users/andrewfish/work/src/edk2/MdeModulePkg/Library/DebugAgentLib
Null/DebugAgentLibNull.inf
{DebugAgentLib}

/Users/andrewfish/work/src/edk2/MdeModulePkg/Library/CpuExceptionHa
ndlerLibNull/CpuExceptionHandlerLibNull.inf
{CpuExceptionHandlerLib}

/Users/andrewfish/work/src/edk2/MdeModulePkg/Library/DxeCrc32Guide
dSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
{NULL:  C = DxeCrc32GuidedSectionExtractLibConstructor}

/Users/andrewfish/work/src/edk2/IntelFrameworkModulePkg/Library/Lzma
CustomDecompressLib/LzmaCustomDecompressLib.inf
{NULL:  C = LzmaDecompressLibConstructor}
<------------------------------------------------------------------------------------------

---------------------------->


MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHan
dlerRuntimeDxe.inf

Library
-------------------------------------------------------------------------------------------

-----------------------------
/Users/andrewfish/work/src/edk2/MdePkg/Library/BaseLib/BaseLib.inf
{BaseLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/BaseMemoryLib/BaseM
emoryLib.inf
{BaseMemoryLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/DxePcdLib/DxePcdLib.in
f
{PcdLib:  Depex = gEfiPcdProtocolGuid }

/Users/andrewfish/work/src/edk2/MdePkg/Library/BaseDebugPrintErrorLe
velLib/BaseDebugPrintErrorLevelLib.inf
{DebugPrintErrorLevelLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/BasePrintLib/BasePrintLi
b.inf
{PrintLib}

/Users/andrewfish/work/src/edk2/EmulatorPkg/Library/DxeEmuStdErrSerial
PortLib/DxeEmuStdErrSerialPortLib.inf
{SerialPortLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/BaseDebugLibSerialPort
/BaseDebugLibSerialPort.inf
{DebugLib:  C = BaseDebugLibSerialPortConstructor}

/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiMemoryAllocationLi
b/UefiMemoryAllocationLib.inf
{MemoryAllocationLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiRuntimeServicesTa
bleLib/UefiRuntimeServicesTableLib.inf
{UefiRuntimeServicesTableLib:  C =
UefiRuntimeServicesTableLibConstructor}

/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiDevicePathLib/Uefi
DevicePathLib.inf
{DevicePathLib}

/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiBootServicesTableLi
b/UefiBootServicesTableLib.inf
{UefiBootServicesTableLib:  C = UefiBootServicesTableLibConstructor}
/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiLib/UefiLib.inf
{UefiLib:  C = UefiLibConstructor}

/Users/andrewfish/work/src/edk2/MdePkg/Library/DxeHobLib/DxeHobLib.i
nf
{HobLib:  C = HobLibConstructor}

/Users/andrewfish/work/src/edk2/EmulatorPkg/Library/DxeEmuLib/DxeEm
uLib.inf
{EmuThunkLib:  C = DxeEmuLibConstructor}

/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiRuntimeLib/UefiRu
ntimeLib.inf
{UefiRuntimeLib:  C = RuntimeDriverLibConstruct D =
RuntimeDriverLibDeconstruct}

/Users/andrewfish/work/src/edk2/MdePkg/Library/UefiDriverEntryPoint/U
efiDriverEntryPoint.inf
{UefiDriverEntryPoint}

/Users/andrewfish/work/src/edk2/MdeModulePkg/Library/DxeReportStatu
sCodeLib/DxeReportStatusCodeLib.inf
{ReportStatusCodeLib}
<------------------------------------------------------------------------------------------

---------------------------->

It looks like a HOB lib with the constructor is what triggers the
issue?

Thanks,

Andrew Fish

Thanks,
Star

Thanks,

Andrew Fish

Thanks,
Star

Any ideas?

Thanks,

Andrew Fish


_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel


_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to