From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8EA1981DF4 for ; Fri, 28 Oct 2016 12:40:16 -0700 (PDT) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5B81C7F3E1; Fri, 28 Oct 2016 19:40:16 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-38.phx2.redhat.com [10.3.116.38]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9SJeEjF007480; Fri, 28 Oct 2016 15:40:15 -0400 To: "Kinney, Michael D" , "Duran, Leo" , "edk2-devel@ml01.01.org" References: Cc: "Fan, Jeff" , "Gao, Liming" From: Laszlo Ersek Message-ID: <2fd72990-687a-f728-a53f-d7a4ad23f7e8@redhat.com> Date: Fri, 28 Oct 2016 21:40:14 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Fri, 28 Oct 2016 19:40:16 +0000 (UTC) Subject: Re: LocalApicLib: Why two separate directories? X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Oct 2016 19:40:16 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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.duran@amd.com] >> Sent: Friday, October 28, 2016 12:04 PM >> To: edk2-devel@ml01.01.org >> Cc: Kinney, Michael D ; Fan, Jeff ; >> Gao, Liming ; 'Laszlo Ersek' >> 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. >> >> >> >> >