[XEN PATCH v8 04/47] build: set XEN_BUILD_EFI earlier

Anthony PERARD posted 47 patches 3 years ago
There is a newer version of this series
[XEN PATCH v8 04/47] build: set XEN_BUILD_EFI earlier
Posted by Anthony PERARD 3 years ago
We are going to need the variable XEN_BUILD_EFI earlier.

But a side effect of calculating the value of $(XEN_BUILD_EFI) is to
also to generate "efi/check.o" which is used for further checks.
Thus the whole chain that check for EFI support is moved to
"arch.mk".

Some other changes are made to avoid too much duplication:
    - $(efi-check-o): Used to avoid repeating "efi/check.*". We don't
      set it to the path to the source as it would be wrong as soon
      as we support out-of-tree build.
    - $(LD_PE_check_cmd): As it is called twice, with an updated
      $(EFI_LDFLAGS).

$(nr-fixups) is renamed to $(efi-nr-fixups) as the former might be
a bit too generic.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v8:
    - rename to efi-nr-fixups rather than efi-check-relocs
    - use := when assigning variable in makefile when recursive expansion
      isn't needed.
    - no more check of $(efi-y) value for "CFLAGS-$(XEN_BUILD_EFI) +=
      -DXEN_BUILD_EFI".
    
    v7:
    - Do the whole check for EFI support in arch.mk. So efi/check.o is
      produce there and used there, and produce efi/check.efi and use it there.
      Thus avoid the need to repeat the test done for XEN_BUILD_EFI.

 xen/arch/x86/Makefile | 38 ++------------------------------------
 xen/arch/x86/arch.mk  | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 8db4cb98edbb..d7d2adc1881e 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -121,44 +121,8 @@ $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
 		{ echo "No Multiboot2 header found" >&2; false; }
 	mv $(TMP) $(TARGET)
 
-ifneq ($(efi-y),)
-
-# Check if the compiler supports the MS ABI.
-export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 
-# Check if the linker supports PE.
-EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10
-XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(call ld-option,$(EFI_LDFLAGS) --image-base=0x100000000 -o efi/check.efi efi/check.o))
-# If the above failed, it may be merely because of the linker not dealing well
-# with debug info. Try again with stripping it.
-ifeq ($(CONFIG_DEBUG_INFO)-$(XEN_BUILD_PE),y-n)
-EFI_LDFLAGS += --strip-debug
-XEN_BUILD_PE := $(call ld-option,$(EFI_LDFLAGS) --image-base=0x100000000 -o efi/check.efi efi/check.o)
-endif
-
-ifeq ($(XEN_BUILD_PE),y)
-
-# Check if the linker produces fixups in PE by default
-nr-fixups := $(shell $(OBJDUMP) -p efi/check.efi | grep '^[[:blank:]]*reloc[[:blank:]]*[0-9][[:blank:]].*DIR64$$' | wc -l)
-ifeq ($(nr-fixups),2)
-MKRELOC := :
-relocs-dummy :=
-else
-MKRELOC := efi/mkreloc
-relocs-dummy := efi/relocs-dummy.o
-# If the linker produced fixups but not precisely two of them, we need to
-# disable it doing so.  But if it didn't produce any fixups, it also wouldn't
-# recognize the option.
-ifneq ($(nr-fixups),0)
-EFI_LDFLAGS += --disable-reloc-section
-endif
-endif
-
-endif # $(XEN_BUILD_PE)
-
-endif # $(efi-y)
-
 ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
 
 ifeq ($(CONFIG_LTO),y)
@@ -217,8 +181,10 @@ endif
 
 $(TARGET).efi: VIRT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A VIRT_START$$,,p')
 ifeq ($(MKRELOC),:)
+relocs-dummy :=
 $(TARGET).efi: ALT_BASE :=
 else
+relocs-dummy := efi/relocs-dummy.o
 $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A ALT_START$$,,p')
 endif
 
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index eea320e618b9..19c9cd206ed0 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -60,5 +60,47 @@ ifeq ($(CONFIG_UBSAN),y)
 $(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
 endif
 
+ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
+
+efi-check-o := arch/x86/efi/check.o
+
+# Check if the compiler supports the MS ABI.
+XEN_BUILD_EFI := $(call if-success,$(CC) $(CFLAGS) -c $(efi-check-o:.o=.c) -o $(efi-check-o),y)
+
+# Check if the linker supports PE.
+EFI_LDFLAGS := $(patsubst -m%,-mi386pep,$(LDFLAGS)) --subsystem=10
+LD_PE_check_cmd = $(call ld-option,$(EFI_LDFLAGS) --image-base=0x100000000 -o $(efi-check-o:.o=.efi) $(efi-check-o))
+XEN_BUILD_PE := $(LD_PE_check_cmd)
+
+# If the above failed, it may be merely because of the linker not dealing well
+# with debug info. Try again with stripping it.
+ifeq ($(CONFIG_DEBUG_INFO)-$(XEN_BUILD_PE),y-n)
+EFI_LDFLAGS += --strip-debug
+XEN_BUILD_PE := $(LD_PE_check_cmd)
+endif
+
+ifeq ($(XEN_BUILD_PE),y)
+
+# Check if the linker produces fixups in PE by default
+efi-nr-fixups := $(shell $(OBJDUMP) -p $(efi-check-o:.o=.efi) | grep '^[[:blank:]]*reloc[[:blank:]]*[0-9][[:blank:]].*DIR64$$' | wc -l)
+
+ifeq ($(efi-nr-fixups),2)
+MKRELOC := :
+else
+MKRELOC := efi/mkreloc
+# If the linker produced fixups but not precisely two of them, we need to
+# disable it doing so.  But if it didn't produce any fixups, it also wouldn't
+# recognize the option.
+ifneq ($(efi-nr-fixups),0)
+EFI_LDFLAGS += --disable-reloc-section
+endif
+endif
+
+endif # $(XEN_BUILD_PE)
+
+export XEN_BUILD_EFI XEN_BUILD_PE MKRELOC
+export EFI_LDFLAGS
+endif
+
 # Set up the assembler include path properly for older toolchains.
 CFLAGS += -Wa,-I$(BASEDIR)/include
-- 
Anthony PERARD


Re: [XEN PATCH v8 04/47] build: set XEN_BUILD_EFI earlier
Posted by Jan Beulich 2 years, 12 months ago
On 25.11.2021 14:39, Anthony PERARD wrote:
> We are going to need the variable XEN_BUILD_EFI earlier.
> 
> But a side effect of calculating the value of $(XEN_BUILD_EFI) is to
> also to generate "efi/check.o" which is used for further checks.
> Thus the whole chain that check for EFI support is moved to
> "arch.mk".
> 
> Some other changes are made to avoid too much duplication:
>     - $(efi-check-o): Used to avoid repeating "efi/check.*". We don't
>       set it to the path to the source as it would be wrong as soon
>       as we support out-of-tree build.
>     - $(LD_PE_check_cmd): As it is called twice, with an updated
>       $(EFI_LDFLAGS).
> 
> $(nr-fixups) is renamed to $(efi-nr-fixups) as the former might be
> a bit too generic.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Technically
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Nevertheless a suggestion and a remark:

> --- a/xen/arch/x86/arch.mk
> +++ b/xen/arch/x86/arch.mk
> @@ -60,5 +60,47 @@ ifeq ($(CONFIG_UBSAN),y)
>  $(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
>  endif
>  
> +ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
> +
> +efi-check-o := arch/x86/efi/check.o

How about making this

efi-check := arch/x86/efi/check

That way you wouldn't need to replace the extension in a number of places,
but simply append the respective one in every place using this.

> +# Check if the compiler supports the MS ABI.
> +XEN_BUILD_EFI := $(call if-success,$(CC) $(CFLAGS) -c $(efi-check-o:.o=.c) -o $(efi-check-o),y)
> +
> +# Check if the linker supports PE.
> +EFI_LDFLAGS := $(patsubst -m%,-mi386pep,$(LDFLAGS)) --subsystem=10
> +LD_PE_check_cmd = $(call ld-option,$(EFI_LDFLAGS) --image-base=0x100000000 -o $(efi-check-o:.o=.efi) $(efi-check-o))
> +XEN_BUILD_PE := $(LD_PE_check_cmd)
> +
> +# If the above failed, it may be merely because of the linker not dealing well
> +# with debug info. Try again with stripping it.
> +ifeq ($(CONFIG_DEBUG_INFO)-$(XEN_BUILD_PE),y-n)
> +EFI_LDFLAGS += --strip-debug
> +XEN_BUILD_PE := $(LD_PE_check_cmd)
> +endif
> +
> +ifeq ($(XEN_BUILD_PE),y)
> +
> +# Check if the linker produces fixups in PE by default
> +efi-nr-fixups := $(shell $(OBJDUMP) -p $(efi-check-o:.o=.efi) | grep '^[[:blank:]]*reloc[[:blank:]]*[0-9][[:blank:]].*DIR64$$' | wc -l)
> +
> +ifeq ($(efi-nr-fixups),2)
> +MKRELOC := :
> +else
> +MKRELOC := efi/mkreloc
> +# If the linker produced fixups but not precisely two of them, we need to
> +# disable it doing so.  But if it didn't produce any fixups, it also wouldn't
> +# recognize the option.
> +ifneq ($(efi-nr-fixups),0)
> +EFI_LDFLAGS += --disable-reloc-section
> +endif
> +endif
> +
> +endif # $(XEN_BUILD_PE)
> +
> +export XEN_BUILD_EFI XEN_BUILD_PE MKRELOC
> +export EFI_LDFLAGS
> +endif

Exporting MKRELOC in particular isn't very nice. I wonder whether there
wouldn't be a way to keep it local to xen/Makefile.

Jan