[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86/efi: move the logic to detect PE build support
On Mon, Jul 16, 2018 at 03:52:49AM -0600, Jan Beulich wrote: > >>> On 16.07.18 at 11:25, <roger.pau@xxxxxxxxxx> wrote: > > On Mon, Jul 16, 2018 at 03:18:13AM -0600, Jan Beulich wrote: > >> >>> On 16.07.18 at 11:12, <roger.pau@xxxxxxxxxx> wrote: > >> > On Mon, Jul 16, 2018 at 02:55:16AM -0600, Jan Beulich wrote: > >> >> >>> On 16.07.18 at 10:26, <roger.pau@xxxxxxxxxx> wrote: > >> >> > On Mon, Jul 16, 2018 at 01:59:15AM -0600, Jan Beulich wrote: > >> >> >> >>> On 13.07.18 at 18:02, <roger.pau@xxxxxxxxxx> wrote: > >> >> >> > --- a/xen/arch/x86/Makefile > >> >> >> > +++ b/xen/arch/x86/Makefile > >> >> >> > @@ -168,6 +168,16 @@ $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) > >> >> >> > efi/relocs-dummy.o | sed -n 's, A ALT_ > >> >> >> > # Don't use $(wildcard ...) here - at least make 3.80 expands > >> >> >> > this too > >> >> > early! > >> >> >> > $(TARGET).efi: guard = $(if $(shell echo efi/dis* | grep > >> >> >> > disabled),:) > >> >> >> > > >> >> >> > +# Check if the build system supports PE. > >> >> >> > +efi := y$(shell rm -f efi/disabled) > >> >> >> > +efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) > >> >> >> > .%.d,$(CFLAGS)) > > -c > >> >> > efi/check.c -o efi/check.o 2>efi/disabled && echo y)) > >> >> >> > +efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o > >> >> >> > efi/check.efi > >> >> > efi/check.o 2>efi/disabled && echo y)) > >> >> >> > +efi := $(if $(efi),$(shell rm efi/disabled)y) > >> >> >> > +export BUILD_PE := $(efi) > >> >> >> > +ifeq ($(efi),y) > >> >> >> > +CFLAGS += -DBUILD_PE > >> >> >> > +endif > >> >> >> > >> >> >> For one I'm not really happy about this being moved here: I did > >> >> >> place it > >> >> >> in efi/Makefile for the simple reason of having as much as possible > >> >> >> of > >> >> >> the EFI specifics in that single file. > >> >> >> > >> >> >> Additionally I think it would be better if setting propagated > >> >> >> through the > >> >> >> environment had XEN_ prefixes. > >> >> >> > >> >> >> > --- a/xen/arch/x86/xen.lds.S > >> >> >> > +++ b/xen/arch/x86/xen.lds.S > >> >> >> > @@ -304,7 +304,9 @@ SECTIONS > >> >> >> > } :text > >> >> >> > #endif > >> >> >> > > >> >> >> > - efi = DEFINED(efi) ? efi : .; > >> >> >> > +#ifndef BUILD_PE > >> >> >> > + efi = .; > >> >> >> > +#endif > >> >> >> > >> >> >> And then I don't really understand how this is different from the > >> >> >> original #ifndef EFI that Daniel had problems with. > >> >> > > >> >> > As I understand it EFI only signals whether a PE binary will be > >> >> > created, > >> >> > >> >> But that is my point: BUILD_PE signals exactly that aiui. > >> > > >> > No, BUILD_PE signals whether the binary will use runtime.c instead of > >> > stub.c, and thus have the efi symbol defined. > >> > >> But in that case - why did you chose this particular name? > > > > Because that seems to be what the tests actually do. AFAICT it tests > > that the compiler supports the MS ABI and that the linker is able to > > generate PE binaries. > > > > I think this is slightly wrong, because for multiboot2 support you > > likely only need the compiler MS ABI support, but not the linker PE > > support? > > Indeed. But too strict checking is less of a problem than too lax one. > Otoh people are far more likely to have a suitable gcc but no suitably > configured binutils, so relaxing this would likely be worthwhile. Daniel? Yes, it is true and I think that it is worth relaxing the check. > > How about naming this BUILD_EFI_SERVICES or some such? > > Perhaps just BUILD_EFI, but yes. EFI, as used in xen.lds.S, would s/BUILD_EFI/XEN_BUILD_EFI/? > then perhaps better be renamed into BUILD_PE (or some such). XEN_BUILD_PE or XEN_LINK_PE? Daniel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |