On Wed, Aug 6, 2014 at 11:17 PM, Jan Beulich JBeulich@suse.com wrote:
On 07.08.14 at 01:55, roy.franz@linaro.org wrote:
On Mon, Jul 28, 2014 at 9:10 AM, Jan Beulich JBeulich@suse.com wrote:
On 28.07.14 at 18:04, Ian.Campbell@citrix.com 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