[Xen-devel] [XEN PATCH for-4.13] xen: Fix race to build arch/x86/efi/relocs-dummy.o

[Xen-devel] [XEN PATCH for-4.13] xen: Fix race to build arch/x86/efi/relocs-dummy.o

[Anthony PERARD]

With $(TARGET).efi depending on efi/relocs-dummy.o, arch/x86/Makefile
will attempt to build that object. This result in the dependency file
been generated with relocs-dummy.o depending on efi/relocs-dummy.o.

Then, when arch/x86/efi/Makefile tries to build relocs-dummy.o, well
efi/relocs-dummy.S doesn't exist.

Have only one makefile responsible for building relocs-dummy.o.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/arch/x86/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 5e6b9d7028db..a6df19e901b3 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -219,8 +219,8 @@ $(TARGET).efi: prelink-efi.o $(note_file) efi.lds efi/relocs-dummy.o efi/mkreloc
 		| $(BASEDIR)/tools/symbols --xensyms --sysv --sort >$(@D)/$(@F).map; fi
 	rm -f $(@D)/.$(@F).[0-9]* $(@D)/..$(@F).[0-9]*
 
-efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o: $(BASEDIR)/arch/x86/efi/built_in.o
-efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o: ;
+efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: $(BASEDIR)/arch/x86/efi/built_in.o
+efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: ;
 
 asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h
 	$(CC) $(filter-out -Wa$(comma)% -flto,$(CFLAGS)) -S -o $@ $<
-- 
Anthony PERARD


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

Re: [Xen-devel] [XEN PATCH for-4.13] xen: Fix race to build arch/x86/efi/relocs-dummy.o

[Jan Beulich]

On 14.11.2019 19:05, Anthony PERARD wrote:
> With $(TARGET).efi depending on efi/relocs-dummy.o, arch/x86/Makefile
> will attempt to build that object. This result in the dependency file
> been generated with relocs-dummy.o depending on efi/relocs-dummy.o.

I cannot confirm this, what I see is

efi/relocs-dummy.o: efi/relocs-dummy.S \
 ...

Which isn't to say there's no issue here. IOW the actual adjustment
is ...

> Then, when arch/x86/efi/Makefile tries to build relocs-dummy.o, well
> efi/relocs-dummy.S doesn't exist.
> 
> Have only one makefile responsible for building relocs-dummy.o.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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

But I'd prefer if the description aspect could either be clarified
(different versions of make behaving differently?) or adjusted.

Jan

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

Re: [Xen-devel] [XEN PATCH for-4.13] xen: Fix race to build arch/x86/efi/relocs-dummy.o

[Anthony PERARD]

On Fri, Nov 15, 2019 at 11:06:27AM +0100, Jan Beulich wrote:
> On 14.11.2019 19:05, Anthony PERARD wrote:
> > With $(TARGET).efi depending on efi/relocs-dummy.o, arch/x86/Makefile
> > will attempt to build that object. This result in the dependency file
> > been generated with relocs-dummy.o depending on efi/relocs-dummy.o.
> 
> I cannot confirm this, what I see is
> 
> efi/relocs-dummy.o: efi/relocs-dummy.S \
>  ...


I've written the commit message base on few evidences, but I don't know
if the race comes from trying to build $(TARGET).efi. Here is what I
have:

# Building Xen with make -j8 after git clean
make[3]: *** No rule to make target 'efi/relocs-dummy.S', needed by 'relocs-dummy.o'.
$ head -1 arch/x86/efi/.relocs-dummy.o.d
relocs-dummy.o: efi/relocs-dummy.S \

arch/x86/.relocs-dummy.o.d doesn't exist.

looking back at the make output, relocs-dummy was built with:
gcc -D__ASSEMBLY__ -m64 -DBUILD_ID -fno-strict-aliasing -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include /local/home/sheep/work/xen/xen/include/xen/config.h '-D__OBJECT_FILE__="efi/relocs-dummy.o"' -Wa,--strip-local-absolute -g -MMD -MF efi/.relocs-dummy.o.d -DXEN_BUILD_EFI -I/local/home/sheep/work/xen/xen/include -I/local/home/sheep/work/xen/xen/include/asm-x86/mach-generic -I/local/home/sheep/work/xen/xen/include/asm-x86/mach-default -DXEN_IMG_OFFSET=0x200000 '-D__OBJECT_LABEL__=arch$x86$efi$relocs_dummy.o' -msoft-float -fno-stack-protector -fno-exceptions -Wnested-externs -DHAVE_AS_VMX -DHAVE_AS_SSE4_2 -DHAVE_AS_EPT -DHAVE_AS_RDRAND -DHAVE_AS_FSGSBASE -DHAVE_AS_XSAVEOPT -DHAVE_AS_RDSEED -DHAVE_AS_CLWB -U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM '-D__OBJECT_LABEL__=arch/x86/efi/relocs-dummy.o' -DHAVE_AS_INVPCID -DHAVE_AS_NEGATIVE_TRUE -DHAVE_AS_NOPS_DIRECTIVE -mno-red-zone -fpic -fno-asynchronous-unwind-tables -mno-sse -mskip-rax-setup -DGCC_HAS_VISIBILITY_ATTRIBUTE -mindirect-branch=thunk-extern -mindirect-branch-register -DCONFIG_INDIRECT_THUNK -fno-jump-tables -mpreferred-stack-boundary=3 -Wa,-I/local/home/sheep/work/xen/xen/include -DBUILD_ID_EFI -c efi/relocs-dummy.S -o efi/relocs-dummy.o

$ gcc --version
gcc (GCC) 9.2.0

I'm guessing that gcc behave differently between both our system?
On mine, `man gcc` seems to imply there's nothing wrong on my system.
If we want gcc to produce "efi/reloc-dummy.o: .." on my system, I think
we would need to add the -MT or -MQ option to the cflags.

Cheers,

-- 
Anthony PERARD

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

Re: [Xen-devel] [XEN PATCH for-4.13] xen: Fix race to build arch/x86/efi/relocs-dummy.o

[Jan Beulich]

On 15.11.2019 13:12, Anthony PERARD wrote:
> On Fri, Nov 15, 2019 at 11:06:27AM +0100, Jan Beulich wrote:
>> On 14.11.2019 19:05, Anthony PERARD wrote:
>>> With $(TARGET).efi depending on efi/relocs-dummy.o, arch/x86/Makefile
>>> will attempt to build that object. This result in the dependency file
>>> been generated with relocs-dummy.o depending on efi/relocs-dummy.o.
>>
>> I cannot confirm this, what I see is
>>
>> efi/relocs-dummy.o: efi/relocs-dummy.S \
>>  ...
> 
> 
> I've written the commit message base on few evidences, but I don't know
> if the race comes from trying to build $(TARGET).efi. Here is what I
> have:
> 
> # Building Xen with make -j8 after git clean
> make[3]: *** No rule to make target 'efi/relocs-dummy.S', needed by 'relocs-dummy.o'.
> $ head -1 arch/x86/efi/.relocs-dummy.o.d
> relocs-dummy.o: efi/relocs-dummy.S \
> 
> arch/x86/.relocs-dummy.o.d doesn't exist.

So I guess I'd like to include "may" then in that specific sentence of
the commit message, which would be easy enough to do while committing.

> looking back at the make output, relocs-dummy was built with:
> gcc -D__ASSEMBLY__ -m64 -DBUILD_ID -fno-strict-aliasing -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include /local/home/sheep/work/xen/xen/include/xen/config.h '-D__OBJECT_FILE__="efi/relocs-dummy.o"' -Wa,--strip-local-absolute -g -MMD -MF efi/.relocs-dummy.o.d -DXEN_BUILD_EFI -I/local/home/sheep/work/xen/xen/include -I/local/home/sheep/work/xen/xen/include/asm-x86/mach-generic -I/local/home/sheep/work/xen/xen/include/asm-x86/mach-default -DXEN_IMG_OFFSET=0x200000 '-D__OBJECT_LABEL__=arch$x86$efi$relocs_dummy.o' -msoft-float -fno-stack-protector -fno-exceptions -Wnested-externs -DHAVE_AS_VMX -DHAVE_AS_SSE4_2 -DHAVE_AS_EPT -DHAVE_AS_RDRAND -DHAVE_AS_FSGSBASE -DHAVE_AS_XSAVEOPT -DHAVE_AS_RDSEED -DHAVE_AS_CLWB -U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM '-D__OBJECT_LABEL__=arch/x86/efi/relocs-dummy.o' -DHAVE_AS_INVPCID -DHAVE_AS_NEGATIVE_TRUE -DHAVE_AS_NOPS_DIRECTIVE -mno-red-zone -fpic -fno-asynchronous-unwind-tables -mno-sse -mskip-rax-setup -DGCC_HAS_VISIBILITY_ATTRIBUTE -mindirect-branch=thunk-extern -mindirect-branch-register -DCONFIG_INDIRECT_THUNK -fno-jump-tables -mpreferred-stack-boundary=3 -Wa,-I/local/home/sheep/work/xen/xen/include -DBUILD_ID_EFI -c efi/relocs-dummy.S -o efi/relocs-dummy.o
> 
> $ gcc --version
> gcc (GCC) 9.2.0
> 
> I'm guessing that gcc behave differently between both our system?

Quite possible, albeit it's 9.2.0 here too. Different set of patches
on top of the upstream version, I guess.

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

Re: [Xen-devel] [XEN PATCH for-4.13] xen: Fix race to build arch/x86/efi/relocs-dummy.o

[Anthony PERARD]

On Fri, Nov 15, 2019 at 01:23:46PM +0100, Jan Beulich wrote:
> On 15.11.2019 13:12, Anthony PERARD wrote:
> > On Fri, Nov 15, 2019 at 11:06:27AM +0100, Jan Beulich wrote:
> >> On 14.11.2019 19:05, Anthony PERARD wrote:
> >>> With $(TARGET).efi depending on efi/relocs-dummy.o, arch/x86/Makefile
> >>> will attempt to build that object. This result in the dependency file
> >>> been generated with relocs-dummy.o depending on efi/relocs-dummy.o.
[..]
> So I guess I'd like to include "may" then in that specific sentence of
> the commit message, which would be easy enough to do while committing.

Ok, so that first paragraph can be rewritten:

With $(TARGET).efi depending on efi/relocs-dummy.o, arch/x86/Makefile
will attempt to build that object. This result in the dependency file
been generated that may have relocs-dummy.o depending on
efi/relocs-dummy.o.

Thanks,

-- 
Anthony PERARD

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

Re: [Xen-devel] [XEN PATCH for-4.13] xen: Fix race to build arch/x86/efi/relocs-dummy.o

[Jürgen Groß]

On 14.11.19 19:05, Anthony PERARD wrote:
> With $(TARGET).efi depending on efi/relocs-dummy.o, arch/x86/Makefile
> will attempt to build that object. This result in the dependency file
> been generated with relocs-dummy.o depending on efi/relocs-dummy.o.
> 
> Then, when arch/x86/efi/Makefile tries to build relocs-dummy.o, well
> efi/relocs-dummy.S doesn't exist.
> 
> Have only one makefile responsible for building relocs-dummy.o.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

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

Re: [Xen-devel] [XEN PATCH for-4.13] xen: Fix race to build arch/x86/efi/relocs-dummy.o

[Jan Beulich]

On 14.11.2019 19:05, Anthony PERARD wrote:
> With $(TARGET).efi depending on efi/relocs-dummy.o, arch/x86/Makefile
> will attempt to build that object. This result in the dependency file
> been generated with relocs-dummy.o depending on efi/relocs-dummy.o.
> 
> Then, when arch/x86/efi/Makefile tries to build relocs-dummy.o, well
> efi/relocs-dummy.S doesn't exist.
> 
> Have only one makefile responsible for building relocs-dummy.o.

On a system with too old a tool chain for the EFI build to get
enabled I now get about a dozen instances per build of

nm: 'efi/relocs-dummy.o': No such file

I don't suppose you did try out your change in such an oldish
environment? I assume the problem are these two lines:

$(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')

I'm not sure it is well defined when make would evaluate such
target specific variable assignments (i.e. I'm not sure this
doesn't point out an issue even on EFI capable tool chains).
Then again these not using := should cause them to get
evaluated only upon use, i.e. never. But that's clearly not
the case here; of course make is also the now pretty dated
3.81 one.

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

Re: [Xen-devel] [XEN PATCH for-4.13] xen: Fix race to build arch/x86/efi/relocs-dummy.o

[Jan Beulich]

On 19.11.2019 17:27, Jan Beulich wrote:
> On 14.11.2019 19:05, Anthony PERARD wrote:
>> With $(TARGET).efi depending on efi/relocs-dummy.o, arch/x86/Makefile
>> will attempt to build that object. This result in the dependency file
>> been generated with relocs-dummy.o depending on efi/relocs-dummy.o.
>>
>> Then, when arch/x86/efi/Makefile tries to build relocs-dummy.o, well
>> efi/relocs-dummy.S doesn't exist.
>>
>> Have only one makefile responsible for building relocs-dummy.o.
> 
> On a system with too old a tool chain for the EFI build to get
> enabled I now get about a dozen instances per build of
> 
> nm: 'efi/relocs-dummy.o': No such file
> 
> I don't suppose you did try out your change in such an oldish
> environment? I assume the problem are these two lines:
> 
> $(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')
> 
> I'm not sure it is well defined when make would evaluate such
> target specific variable assignments (i.e. I'm not sure this
> doesn't point out an issue even on EFI capable tool chains).
> Then again these not using := should cause them to get
> evaluated only upon use, i.e. never.

Ah, this was wrong - the $(guard) prefix causes them to get
evaluated even when xen.efi cannot be built. So I guess this is
just a cosmetic issue then, which would however still be nice
to see addressed.

Jan

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

Re: [Xen-devel] [XEN PATCH for-4.13] xen: Fix race to build arch/x86/efi/relocs-dummy.o

[Anthony PERARD]

On Tue, Nov 19, 2019 at 05:32:53PM +0100, Jan Beulich wrote:
> On 19.11.2019 17:27, Jan Beulich wrote:
> > On 14.11.2019 19:05, Anthony PERARD wrote:
> >> With $(TARGET).efi depending on efi/relocs-dummy.o, arch/x86/Makefile
> >> will attempt to build that object. This result in the dependency file
> >> been generated with relocs-dummy.o depending on efi/relocs-dummy.o.
> >>
> >> Then, when arch/x86/efi/Makefile tries to build relocs-dummy.o, well
> >> efi/relocs-dummy.S doesn't exist.
> >>
> >> Have only one makefile responsible for building relocs-dummy.o.
> > 
> > On a system with too old a tool chain for the EFI build to get
> > enabled I now get about a dozen instances per build of
> > 
> > nm: 'efi/relocs-dummy.o': No such file
> > 
> > I don't suppose you did try out your change in such an oldish
> > environment? I assume the problem are these two lines:
> > 
> > $(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')
> > 
> > I'm not sure it is well defined when make would evaluate such
> > target specific variable assignments (i.e. I'm not sure this
> > doesn't point out an issue even on EFI capable tool chains).
> > Then again these not using := should cause them to get
> > evaluated only upon use, i.e. never.
> 
> Ah, this was wrong - the $(guard) prefix causes them to get
> evaluated even when xen.efi cannot be built. So I guess this is
> just a cosmetic issue then, which would however still be nice
> to see addressed.

That $(guard) thing is weird, and can probably be replace now.

I'll try to remove that thing, and also avoid having $(TARGET).efi
depending on efi/relocs-dummy.o when it isn't going to be built (when
XEN_BUILD_EFI=n).

-- 
Anthony PERARD

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

[Xen-devel] [XEN PATCH] xen/arch/x86/Makefile: Remove $(guard) use from $(TARGET).efi target

[Anthony PERARD]

Following the patch 65d104984c04 ("x86: fix race to build
arch/x86/efi/relocs-dummy.o"), the error message
  nm: 'efi/relocs-dummy.o': No such file"
started to appear on system which can't build the .efi target. This is
because relocs-dummy.o isn't built anymore.
The error is printed by the evaluation of VIRT_BASE and ALT_BASE which
aren't use anyway.

But, we don't need that file as we don't want to build `$(TARGET).efi'
anyway.  On such system, $(guard) evaluate to the shell builtin ':',
which prevent any of the shell commands in `$(TARGET).efi' from been
executed.

Even if $(guard) is evaluated opon use, it depends on $(XEN_BUILD_PE)
which is evaluated at the assignment. So, we can replace $(guard) in
$(TARGET).efi by having two different rules depending on
$(XEN_BUILD_PE) instead.

The change with this patch is that none of the dependency of
$(TARGET).efi will be built if the linker doesn't support PE
and VIRT_BASE and ALT_BASE don't get evaluated anymore, so nm will not
complain about the missing relocs-dummy.o file anymore.

Since prelink-efi.o isn't built on system that can't build
$(TARGET).efi anymore, we can remove the $(guard) variable everywhere.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/arch/x86/Makefile | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index a6df19e901b3..a0b2f4ab1577 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -120,20 +120,20 @@ prelink_lto.o: $(ALL_OBJS)
 	$(LD_LTO) -r -o $@ $^
 
 prelink-efi_lto.o: $(ALL_OBJS) efi/runtime.o efi/compat.o
-	$(guard) $(LD_LTO) -r -o $@ $(filter-out %/efi/built_in.o,$^)
+	$(LD_LTO) -r -o $@ $(filter-out %/efi/built_in.o,$^)
 
 # Link it with all the binary objects
 prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
 	$(LD) $(LDFLAGS) -r -o $@ $^
 
 prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink-efi_lto.o efi/boot.init.o
-	$(guard) $(LD) $(LDFLAGS) -r -o $@ $^
+	$(LD) $(LDFLAGS) -r -o $@ $^
 else
 prelink.o: $(ALL_OBJS)
 	$(LD) $(LDFLAGS) -r -o $@ $^
 
 prelink-efi.o: $(ALL_OBJS) efi/boot.init.o efi/runtime.o efi/compat.o
-	$(guard) $(LD) $(LDFLAGS) -r -o $@ $(filter-out %/efi/built_in.o,$^)
+	$(LD) $(LDFLAGS) -r -o $@ $(filter-out %/efi/built_in.o,$^)
 endif
 
 $(TARGET)-syms: prelink.o xen.lds
@@ -178,8 +178,6 @@ CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 
 $(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 $(filter y,$(XEN_BUILD_PE)),,:)
 
 ifneq ($(build_id_linker),)
 ifeq ($(call ld-ver-build-id,$(LD) $(filter -m%,$(EFI_LDFLAGS))),y)
@@ -197,27 +195,31 @@ note_file :=
 endif
 note_file_option ?= $(note_file)
 
+ifeq ($(filter y,$(XEN_BUILD_PE)),y)
 $(TARGET).efi: prelink-efi.o $(note_file) efi.lds efi/relocs-dummy.o efi/mkreloc
 	$(foreach base, $(VIRT_BASE) $(ALT_BASE), \
-	          $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< efi/relocs-dummy.o \
+	          $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< efi/relocs-dummy.o \
 	                $(BASEDIR)/common/symbols-dummy.o $(note_file_option) -o $(@D)/.$(@F).$(base).0 &&) :
-	$(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).0) >$(@D)/.$(@F).0r.S
-	$(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).0 \
-		| $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0s.S
-	$(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o
+	efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).0) >$(@D)/.$(@F).0r.S
+	$(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).0 \
+		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0s.S
+	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o
 	$(foreach base, $(VIRT_BASE) $(ALT_BASE), \
-	          $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \
+	          $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \
 	                $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o $(note_file_option) -o $(@D)/.$(@F).$(base).1 &&) :
-	$(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).1) >$(@D)/.$(@F).1r.S
-	$(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \
-		| $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1s.S
-	$(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o
-	$(guard) $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \
+	efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).1) >$(@D)/.$(@F).1r.S
+	$(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \
+		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1s.S
+	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o
+	$(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \
 	                $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o $(note_file_option) -o $@
-	if $(guard) false; then rm -f $@; echo 'EFI support disabled'; \
-	else $(NM) -pa --format=sysv $(@D)/$(@F) \
-		| $(BASEDIR)/tools/symbols --xensyms --sysv --sort >$(@D)/$(@F).map; fi
+	$(NM) -pa --format=sysv $(@D)/$(@F) \
+		| $(BASEDIR)/tools/symbols --xensyms --sysv --sort >$(@D)/$(@F).map
 	rm -f $(@D)/.$(@F).[0-9]* $(@D)/..$(@F).[0-9]*
+else
+$(TARGET).efi: FORCE
+	rm -f $@; echo 'EFI support disabled'
+endif
 
 efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: $(BASEDIR)/arch/x86/efi/built_in.o
 efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: ;
-- 
Anthony PERARD


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

Re: [Xen-devel] [XEN PATCH] xen/arch/x86/Makefile: Remove $(guard) use from $(TARGET).efi target

[Jan Beulich]

On 19.11.2019 18:58, Anthony PERARD wrote:
> Following the patch 65d104984c04 ("x86: fix race to build
> arch/x86/efi/relocs-dummy.o"), the error message
>   nm: 'efi/relocs-dummy.o': No such file"
> started to appear on system which can't build the .efi target. This is
> because relocs-dummy.o isn't built anymore.
> The error is printed by the evaluation of VIRT_BASE and ALT_BASE which
> aren't use anyway.
> 
> But, we don't need that file as we don't want to build `$(TARGET).efi'
> anyway.  On such system, $(guard) evaluate to the shell builtin ':',
> which prevent any of the shell commands in `$(TARGET).efi' from been
> executed.
> 
> Even if $(guard) is evaluated opon use, it depends on $(XEN_BUILD_PE)
> which is evaluated at the assignment. So, we can replace $(guard) in
> $(TARGET).efi by having two different rules depending on
> $(XEN_BUILD_PE) instead.
> 
> The change with this patch is that none of the dependency of
> $(TARGET).efi will be built if the linker doesn't support PE
> and VIRT_BASE and ALT_BASE don't get evaluated anymore, so nm will not
> complain about the missing relocs-dummy.o file anymore.
> 
> Since prelink-efi.o isn't built on system that can't build
> $(TARGET).efi anymore, we can remove the $(guard) variable everywhere.

And indeed the need for it had disappeared with 18cd4997d2 ("x86/efi:
move the logic to detect PE build support").

> Reported-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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

Also Cc-ing Jürgen, as this addresses a (cosmetic) regression from
65d104984c ("x86: fix race to build arch/x86/efi/relocs-dummy.o").

> @@ -178,8 +178,6 @@ CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>  
>  $(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 $(filter y,$(XEN_BUILD_PE)),,:)

It is particularly telling that the comment here had been stale
as of that earlier change from Roger, as the (shell level)
wildcard use was the whole reason for needing $(guard) here.

Jan

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

Re: [Xen-devel] [XEN PATCH] xen/arch/x86/Makefile: Remove $(guard) use from $(TARGET).efi target

[Jürgen Groß]

On 20.11.19 08:50, Jan Beulich wrote:
> On 19.11.2019 18:58, Anthony PERARD wrote:
>> Following the patch 65d104984c04 ("x86: fix race to build
>> arch/x86/efi/relocs-dummy.o"), the error message
>>    nm: 'efi/relocs-dummy.o': No such file"
>> started to appear on system which can't build the .efi target. This is
>> because relocs-dummy.o isn't built anymore.
>> The error is printed by the evaluation of VIRT_BASE and ALT_BASE which
>> aren't use anyway.
>>
>> But, we don't need that file as we don't want to build `$(TARGET).efi'
>> anyway.  On such system, $(guard) evaluate to the shell builtin ':',
>> which prevent any of the shell commands in `$(TARGET).efi' from been
>> executed.
>>
>> Even if $(guard) is evaluated opon use, it depends on $(XEN_BUILD_PE)
>> which is evaluated at the assignment. So, we can replace $(guard) in
>> $(TARGET).efi by having two different rules depending on
>> $(XEN_BUILD_PE) instead.
>>
>> The change with this patch is that none of the dependency of
>> $(TARGET).efi will be built if the linker doesn't support PE
>> and VIRT_BASE and ALT_BASE don't get evaluated anymore, so nm will not
>> complain about the missing relocs-dummy.o file anymore.
>>
>> Since prelink-efi.o isn't built on system that can't build
>> $(TARGET).efi anymore, we can remove the $(guard) variable everywhere.
> 
> And indeed the need for it had disappeared with 18cd4997d2 ("x86/efi:
> move the logic to detect PE build support").
> 
>> Reported-by: Jan Beulich <jbeulich@suse.com>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Also Cc-ing Jürgen, as this addresses a (cosmetic) regression from
> 65d104984c ("x86: fix race to build arch/x86/efi/relocs-dummy.o").

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

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