From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roy Franz Subject: Re: [PATCH V2 01/12] Create efi-shared.[ch], and move string functions Date: Fri, 8 Aug 2014 17:27:35 -0700 Message-ID: References: <1405989815-25236-1-git-send-email-roy.franz@linaro.org> <1405989815-25236-2-git-send-email-roy.franz@linaro.org> <53CFFF74020000780002540C@mail.emea.novell.com> <1406562064.17854.1.camel@kazak.uk.xensource.com> <53D68DD70200007800026C29@mail.emea.novell.com> <1406562976.17854.8.camel@kazak.uk.xensource.com> <53D68FBB0200007800026C45@mail.emea.novell.com> <1406563445.17854.14.camel@kazak.uk.xensource.com> <53D6920B0200007800026CB0@mail.emea.novell.com> <53E336280200007800029FCC@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53E336280200007800029FCC@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: keir , Ian Campbell , tim , xen-devel , Stefano Stabellini , linaro-uefi , Fu Wei List-Id: xen-devel@lists.xenproject.org On Wed, Aug 6, 2014 at 11:17 PM, Jan Beulich wrote: >>>> On 07.08.14 at 01:55, wrote: >> On Mon, Jul 28, 2014 at 9:10 AM, Jan Beulich wrote: >>>>>> On 28.07.14 at 18:04, wrote: >>>> Rather than arch .c files including common .c (or .inc) files how about >>>> making xen/arch/*/efi/Makefile link xen/commmon/efi/built-in.o into it's >>>> own built-in.o instead of having xen/common/Makefile do it like would >>>> normally happen? >>> >>> That's an option. But I agree the inclusion of .c in another .c isn't >>> really nice; I would therefore anyway favor a (set of) arch header >>> file(s) providing everything the common code can't do on its own. >> >> I think I have pretty good handle addressing all of the feedback with >> the exception of this build issue. Jan - I don't understand your above >> suggestion. > > The process would be > - move all files containing pieces you want to re-use from arch/x86/efi/ > to common/efi/, with no other changes than to make them build again > - possibly in a step by step manner, replace any code portions not > suitable for non-x86 by inline functions placed in said header > - add ARM variants of anything moved to the x86-specific helper > header (and possible empty x86 variants of stuff ARM needs that > x86 doesn't) > OK, I was missing the inline function bit of it. I took a quick look at more of what the x86 boot.c code does and I'm still not a fan of this. I think the differences of device tree versus not, e820 maps, etc will lead to either conditional code, or awkward helper functions. >>From a more practical view, this would be a major refactor of the x86 code, which I don't think I could complete in the XEN 4.5 timeline. >> Another way to handle this would be for the x86 EFI code to copy >> (or link) the efi-shared.c file locally for building, where it's use would >> be controlled by the PE/COFF toolchain autodetection. It seems that >> some kind of games will need to be played to have the autodetection >> cooperate with building shared code. > > Yes, that would be an option too, with the linking done in a prerequisite > build step. While I generally dislike this model, I do see its simplicity > being a possible advantage here. I'm not a huge fan of it either, but I think it's nicer that #including a C file, and it keeps all of the clever makefile bits in the x86/efi directory. It's the x86 EFI autoconfig that is complicating the build, so this keeps all the trickery together. The Linux self-decompressor uses this to get various bits of libfdt. I think the makefile copying the shared c file is better than what I have now, as it avoids the dead code in the non-efi x86 case (which was a consequence I had overlooked at the time.) I'm going to implement this as part of my next version of the patch series, which I should have out early next week. Roy > > Jan >