> On Mar 18, 2016, at 7:29 AM, Gao, Liming <[email protected]> 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]] > Sent: Friday, March 18, 2016 2:06 AM > To: [email protected] > Cc: Ard Biesheuvel <[email protected]>; Laszlo Ersek > <[email protected]>; Gao, Liming <[email protected]>; edk2-devel > <[email protected]>; Zeng, Star <[email protected]> > 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] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
