Re: [Xen-devel] [PATCH v2 1/2] x86/efi: move the logic to detect PE build support

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.07.18 at 16:07, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -163,10 +163,17 @@ EFI_LDFLAGS += --minor-image-version=$(XEN_SUBVERSION)
>  EFI_LDFLAGS += --major-os-version=2 --minor-os-version=0
>  EFI_LDFLAGS += --major-subsystem-version=2 --minor-subsystem-version=0
>  
> +# Check if the build system supports PE.
> +XEN_BUILD_PE := $(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c 
> efi/check.c -o efi/check.o 2>/dev/null && echo y)
> +export XEN_BUILD_PE := $(if $(XEN_BUILD_PE),$(shell $(LD) -mi386pep 
> --subsystem=10 -o efi/check.efi efi/check.o 2>/dev/null && echo y))

As said before I'm not overly happy with the movement here, but I guess
all other options are worse.

> +ifeq ($(XEN_BUILD_PE),y)
> +CFLAGS += -DXEN_BUILD_PE
> +endif

CFLAGS-$(XEN_BUILD_PE) += -DXEN_BUILD_PE

>  $(TARGET).efi: VIRT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A 
> VIRT_START$$,,p')
>  $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A 
> ALT_START$$,,p')
>  # Don't use $(wildcard ...) here - at least make 3.80 expands this too early!
> -$(TARGET).efi: guard = $(if $(shell echo efi/dis* | grep disabled),:)
> +$(TARGET).efi: guard = $(if $(XEN_BUILD_PE),,:)

With this new approach the check going wrong could actually be overridden
by passing XEN_BUILD_PE= to make. Since in the "disable" case it would be
more natural to pass XEN_BUILD_PE=n, I'd prefer if the above became

$(TARGET).efi: guard = $(if $(filter y,$(XEN_BUILD_PE)),,:)

With these adjustments
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
but I'd prefer for Daniel to confirm that this isn't going to make his life
more complicated.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel