On 10/28/16 21:28, Kinney, Michael D wrote: > Leo, > > Your observation is correct, but the reason not to make this change now is > the DSC file changes required that would break platform builds. As Laszlo > points out, it is possible to do this type of change and coordinate update > to all platforms in edk2/master. However, there are many other platforms > that use edk2 and a change like this would break them on next pull of > edk2/master. > > I believe the original X2 APIC implementation did not have as much common > code, so that was likely why it was added as a different library. > > I recommend we just leave them in their own directories for right now.
It would also be possible to extract the common code into a library *class* (internal to UefiCpuPkg), along with a single instance for that new lib class. This would leave the two current INFs undisturbed. However, as far as I understand, such arbitrary library classes could be frowned upon -- there's no other "guiding principle" to them than "well these functions happen to be shared by two other modules". Sometimes that's a good enough reason for introducing a new library class (with a single imlementation), sometimes it isn't. Thanks Laszlo >> -----Original Message----- >> From: Duran, Leo [mailto:leo.du...@amd.com] >> Sent: Friday, October 28, 2016 12:04 PM >> To: edk2-de...@ml01.01.org >> Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Fan, Jeff >> <jeff....@intel.com>; >> Gao, Liming <liming....@intel.com>; 'Laszlo Ersek' <ler...@redhat.com> >> Subject: LocalApicLib: Why two separate directories? >> >> All, >> Just a quick observation to request comments: >> >> Since a lot of the code in BaseXApicX2ApicLib.c and BaseXApicLib is the >> same, how about >> we merge the common code and build the libraries from the same directory? >> >> UefiCpuPkg/Library/LocalApilLib/ >> - LocalApicLib.c --> common code >> - BaseXApicLib.c --> legacy APIC code >> - BaseXApicX2ApicLib.c --> X2APIC code >> - BaseXApicLib.inf -> builds from LocalApicLib.c + BaseXApicLib.c >> - BaseXApicX2ApicLib.inf -> builds from LocalApicLib.c + BaseXApicX2ApicLib.c >> >> Of course, doing this would require modification to existing .DSC files, to >> point to >> the appropriate .INF under the merged LocalApicLib directory. >> Would that be too disruptive? >> >> Leo. >> >> >> >> > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
