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