> On Mar 21, 2016, at 8:07 PM, Gao, Liming <[email protected]> wrote: > > Andrew: > Star gives the additional note that this change to remove the constructor > from the HobLib will cause NT32 boot failure, because it will cause the > inaccurate constructor dependency. To reduce impact, I prefer to try to > remove the constructor of DxeDebugPrintErrorLevelLib library instance. >
Liming, I also hit an issue in the EmulatorPkg. At this point I'm forced to make a custom copy of the library that inlines a copy of the HOB lib function to get it to work. I hit the same issue Star, and I fixed that and hit another issue with recursion related to the library being included in the DXE Core. It is looking like your original idea about how to solve are the best way to proceed. Thanks, Andrew Fish > Thanks > Liming > From: [email protected] [mailto:[email protected]] > Sent: Saturday, March 19, 2016 1:06 AM > To: Gao, Liming > Cc: [email protected]; edk2-devel; Laszlo Ersek; Zeng, Star; Ard > Biesheuvel > Subject: Re: [edk2] Has any one else had issues trying to use > DxeDebugPrintErrorLevelLib? > DebugPrintErrorLevelLib|MdeModulePkg/Library/DxeDebugPrintErrorLevelLib/DxeDebugPrintErrorLevelLib.inf > > >> On Mar 18, 2016, at 7:29 AM, Gao, Liming wrote: >> >> Andrew: >> First, I suggest to enhance BaseTools to report the detail information on >> the constructor circle. Next, we can add comments in >> DxeDebugPrintErrorLevelLib library instance to note it can't be used >> together with DxeHobLib and BaseDebugLibSerialPort instance until we have >> the complete solution to resolve it. >> > > Liming, > > I also noticed that removing the constructor from the HobLib fixes the issue. > We can update GetHobList to call EfiGetSystemConfigurationTable() if mHobList > is NULL. > > I agree enhancing the error message is a good idea. > > Thanks, > > Andrew Fish > >> Thanks >> Liming >> From: [email protected]<mailto:[email protected]> [mailto:[email protected]] >> Sent: Friday, March 18, 2016 2:06 AM >> To: [email protected]<mailto:[email protected]> >> Cc: Ard Biesheuvel ; Laszlo Ersek ; Gao, Liming ; edk2-devel ; Zeng, Star >> Subject: Re: [edk2] Has any one else had issues trying to use >> DxeDebugPrintErrorLevelLib? >> DebugPrintErrorLevelLib|MdeModulePkg/Library/DxeDebugPrintErrorLevelLib/DxeDebugPrintErrorLevelLib.inf >> >> >>> On Mar 17, 2016, at 5:15 AM, >>> [email protected]<mailto:[email protected]> wrote: >>> >>> Hi all, >>> >>> from my point of view a library should use a constructor to initialize OWN >>> resources only. That means that there is no need for external resources at >>> this point. The library can use PeiService/SystemTable to register for >>> events or protocols (when they need to react to them). Or they finalize >>> initialization on the first time a library function gets called. >>> I don't know how many EDK2 libraries would fulfill this requirement and if >>> it is worth to note that rule into specification (when you agree) but this >>> way one can get rid of dependency loops. >>> >> >> Peter, >> >> It might not be a bad idea to make this a recommendation, and implement it >> in the key packages (MdePkg, MdeModulePkg, etc.). >> >> I've been working on the edk2 since the beginning and this is the 1st time I >> every remember hitting this issue in a real world situation. I'm guessing >> since most libraries are small and have limited dependencies we don't hit >> this a lot. However given the error messages you get it is quite painful to >> debug. I basically had to do a binary search of disabling dependent >> libraries to figure this out. I think the DebugLib is a special case since >> it is used by almost every other library, and it has the possibility of >> being complex and depend on a lot of other libraries. >> >> Thanks, >> >> Andrew Fish >> >>> Best Regards, >>> Peter Kirmeier >>> >>> -----Original Message----- >>> From: edk2-devel [mailto:[email protected]] On Behalf Of Ard >>> Biesheuvel >>> Sent: Thursday, March 17, 2016 11:16 AM >>> To: Laszlo Ersek; Gao, Liming >>> Cc: edk2-devel; Andrew Fish; Zeng, Star >>> Subject: Re: [edk2] Has any one else had issues trying to use >>> DxeDebugPrintErrorLevelLib? >>> DebugPrintErrorLevelLib|MdeModulePkg/Library/DxeDebugPrintErrorLevelLib/DxeDebugPrintErrorLevelLib.inf >>> >>> On 17 March 2016 at 10: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 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. >>>> >>> >>> As I have mentioned in the past, there is a general issue with the >>> EDK2 BaseTools where a transitive dependency on a library that has a >>> constructor is not taken into account when checking for cycles. >>> >>> For example, >>> >>> lib A depends on lib B, which has a constructor lib B depends on lib C, >>> which has no constructor lib C depends on lib D, which has a constructor >>> lib D depends on lib A, which has no constructor >>> >>> In this case, the build will happily succeed, but which constructor >>> executes first (lib B or lib D) is undefined, and there is generally no >>> correct solution as either constructor could transitively depend on the >>> other library having been constructed already. >>> >>> So I think it would be a good step to fix the build tools to detect this >>> case, but I am afraid it will expose errors in many places in the code base. >>> >>> I am kind of glad someone else is hitting this right now, since the last >>> time I brought it up, nobody really cared afair. >>> >>> Thanks, >>> Ard. >>> _______________________________________________ >>> edk2-devel mailing list >>> [email protected]<mailto:[email protected]> >>> https://lists.01.org/mailman/listinfo/edk2-devel >> _______________________________________________ >> edk2-devel mailing list >> [email protected]<mailto:[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
