| Message ID | 20220125110103.3527686-25-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: Build system improvements, now with out-of-tree build! | expand |
Hi, On 25/01/2022 11:00, Anthony PERARD wrote: > Rather than preparing the efi source file, we will make the symbolic > link as needed from the build location. It is not a request for this series. I have started to look a bit more how the EFI code work on Arm and I was wondering why we are using symbolic here? IOW, why wouldn't it be fine to build the "common" and "arch" specific bit (if any) separately and then assemble them? Cheers,
On 25.01.2022 12:00, Anthony PERARD wrote: > --- a/xen/arch/x86/Makefile > +++ b/xen/arch/x86/Makefile > @@ -77,8 +77,9 @@ obj-$(CONFIG_COMPAT) += x86_64/platform_hypercall.o > obj-y += sysctl.o > endif > > -# Allows "clean" to descend into boot/ > +# Allows "clean" to descend > subdir- += boot > +subdir- += efi No similar addition is needed for Arm? > --- /dev/null > +++ b/xen/common/efi/efi-common.mk > @@ -0,0 +1,15 @@ > +EFIOBJ-y := boot.init.o pe.init.o ebmalloc.o runtime.o > +EFIOBJ-$(CONFIG_COMPAT) += compat.o > + > +CFLAGS-y += -fshort-wchar > +CFLAGS-y += -iquote $(srctree)/common/efi > + > +# Part of the command line transforms $(obj) in to a relative reverted path. > +# e.g.: It transforms "dir/foo/bar" into successively > +# "dir foo bar", ".. .. ..", "../../.." > +$(obj)/%.c: $(srctree)/common/efi/%.c FORCE > + $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/common/efi/$(<F) $@ What is the "reverted" about in the comment? Also (nit) I think you want s/in to/into/. Jan
On Thu, Mar 03, 2022 at 11:37:08AM +0100, Jan Beulich wrote: > On 25.01.2022 12:00, Anthony PERARD wrote: > > --- a/xen/arch/x86/Makefile > > +++ b/xen/arch/x86/Makefile > > @@ -77,8 +77,9 @@ obj-$(CONFIG_COMPAT) += x86_64/platform_hypercall.o > > obj-y += sysctl.o > > endif > > > > -# Allows "clean" to descend into boot/ > > +# Allows "clean" to descend > > subdir- += boot > > +subdir- += efi > > No similar addition is needed for Arm? No, because Arm already have "obj-$(CONFIG_ARM_64) += efi/", which has the same effect on clean. Make clean doesn't use ${ALL_OBJS} to find out which directory to clean, so adding "subdir-" is needed at the moment. > > --- /dev/null > > +++ b/xen/common/efi/efi-common.mk > > @@ -0,0 +1,15 @@ > > +EFIOBJ-y := boot.init.o pe.init.o ebmalloc.o runtime.o > > +EFIOBJ-$(CONFIG_COMPAT) += compat.o > > + > > +CFLAGS-y += -fshort-wchar > > +CFLAGS-y += -iquote $(srctree)/common/efi > > + > > +# Part of the command line transforms $(obj) in to a relative reverted path. > > +# e.g.: It transforms "dir/foo/bar" into successively > > +# "dir foo bar", ".. .. ..", "../../.." > > +$(obj)/%.c: $(srctree)/common/efi/%.c FORCE > > + $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/common/efi/$(<F) $@ > > What is the "reverted" about in the comment? Also (nit) I think you want > s/in to/into/. I've tried to described in the single word that the result is a relative path that goes in the opposite direction to the original relative path. Instead of going down, it goes up the hierarchy of directories. Maybe "reversed" would be better? Do you have other suggestion? Thanks,
On 03.03.2022 16:41, Anthony PERARD wrote: > On Thu, Mar 03, 2022 at 11:37:08AM +0100, Jan Beulich wrote: >> On 25.01.2022 12:00, Anthony PERARD wrote: >>> --- a/xen/arch/x86/Makefile >>> +++ b/xen/arch/x86/Makefile >>> @@ -77,8 +77,9 @@ obj-$(CONFIG_COMPAT) += x86_64/platform_hypercall.o >>> obj-y += sysctl.o >>> endif >>> >>> -# Allows "clean" to descend into boot/ >>> +# Allows "clean" to descend >>> subdir- += boot >>> +subdir- += efi >> >> No similar addition is needed for Arm? > > No, because Arm already have "obj-$(CONFIG_ARM_64) += efi/", which has > the same effect on clean. > > Make clean doesn't use ${ALL_OBJS} to find out which directory to clean, so > adding "subdir-" is needed at the moment. Oh, I see. >>> --- /dev/null >>> +++ b/xen/common/efi/efi-common.mk >>> @@ -0,0 +1,15 @@ >>> +EFIOBJ-y := boot.init.o pe.init.o ebmalloc.o runtime.o >>> +EFIOBJ-$(CONFIG_COMPAT) += compat.o >>> + >>> +CFLAGS-y += -fshort-wchar >>> +CFLAGS-y += -iquote $(srctree)/common/efi >>> + >>> +# Part of the command line transforms $(obj) in to a relative reverted path. >>> +# e.g.: It transforms "dir/foo/bar" into successively >>> +# "dir foo bar", ".. .. ..", "../../.." >>> +$(obj)/%.c: $(srctree)/common/efi/%.c FORCE >>> + $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/common/efi/$(<F) $@ >> >> What is the "reverted" about in the comment? Also (nit) I think you want >> s/in to/into/. > > I've tried to described in the single word that the result is a relative > path that goes in the opposite direction to the original relative path. > Instead of going down, it goes up the hierarchy of directories. > Maybe "reversed" would be better? Do you have other suggestion? I'd simply omit the word. In case you're fine with that, I'd be happy to adjust while committing. Jan
On Thu, Mar 03, 2022 at 05:01:07PM +0100, Jan Beulich wrote: > On 03.03.2022 16:41, Anthony PERARD wrote: > > On Thu, Mar 03, 2022 at 11:37:08AM +0100, Jan Beulich wrote: > >> On 25.01.2022 12:00, Anthony PERARD wrote: > >>> +# Part of the command line transforms $(obj) in to a relative reverted path. > >>> +# e.g.: It transforms "dir/foo/bar" into successively > >>> +# "dir foo bar", ".. .. ..", "../../.." > >>> +$(obj)/%.c: $(srctree)/common/efi/%.c FORCE > >>> + $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/common/efi/$(<F) $@ > >> > >> What is the "reverted" about in the comment? Also (nit) I think you want > >> s/in to/into/. > > > > I've tried to described in the single word that the result is a relative > > path that goes in the opposite direction to the original relative path. > > Instead of going down, it goes up the hierarchy of directories. > > Maybe "reversed" would be better? Do you have other suggestion? > > I'd simply omit the word. In case you're fine with that, I'd be happy > to adjust while committing. I think that would sound kind of strange. $(obj) is already a relative path. It would probably be better to just drop the end of the sentence in that case. With the example showing what is happening, that would probably be enough. The sentence would then be: # Part of the command line transforms $(obj). # e.g.: It transforms "dir/foo/bar" into successively # "dir foo bar", ".. .. ..", "../../.."
On 03.03.2022 17:50, Anthony PERARD wrote: > On Thu, Mar 03, 2022 at 05:01:07PM +0100, Jan Beulich wrote: >> On 03.03.2022 16:41, Anthony PERARD wrote: >>> On Thu, Mar 03, 2022 at 11:37:08AM +0100, Jan Beulich wrote: >>>> On 25.01.2022 12:00, Anthony PERARD wrote: >>>>> +# Part of the command line transforms $(obj) in to a relative reverted path. >>>>> +# e.g.: It transforms "dir/foo/bar" into successively >>>>> +# "dir foo bar", ".. .. ..", "../../.." >>>>> +$(obj)/%.c: $(srctree)/common/efi/%.c FORCE >>>>> + $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/common/efi/$(<F) $@ >>>> >>>> What is the "reverted" about in the comment? Also (nit) I think you want >>>> s/in to/into/. >>> >>> I've tried to described in the single word that the result is a relative >>> path that goes in the opposite direction to the original relative path. >>> Instead of going down, it goes up the hierarchy of directories. >>> Maybe "reversed" would be better? Do you have other suggestion? >> >> I'd simply omit the word. In case you're fine with that, I'd be happy >> to adjust while committing. > > I think that would sound kind of strange. $(obj) is already a relative > path. It would probably be better to just drop the end of the sentence > in that case. With the example showing what is happening, that would > probably be enough. The sentence would then be: > > # Part of the command line transforms $(obj). > # e.g.: It transforms "dir/foo/bar" into successively > # "dir foo bar", ".. .. ..", "../../.." Fine with me. Then: Acked-by: Jan Beulich <jbeulich@suse.com> Jan
diff --git a/xen/Makefile b/xen/Makefile index 8baa260b93a7..443784dfce80 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -444,11 +444,6 @@ $(TARGET).gz: $(TARGET) $(TARGET): FORCE $(Q)$(MAKE) $(build)=tools $(Q)$(MAKE) $(build)=. include/xen/compile.h - [ -e arch/$(TARGET_ARCH)/efi ] && for f in $$(cd common/efi; echo *.[ch]); \ - do test -r arch/$(TARGET_ARCH)/efi/$$f || \ - ln -nsf ../../../common/efi/$$f arch/$(TARGET_ARCH)/efi/; \ - done; \ - true $(Q)$(MAKE) $(build)=include all $(Q)$(MAKE) $(build)=arch/$(TARGET_ARCH) include $(Q)$(MAKE) $(build)=. arch/$(TARGET_ARCH)/include/asm/asm-offsets.h diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile index 1b1ed06feddc..4313c390665f 100644 --- a/xen/arch/arm/efi/Makefile +++ b/xen/arch/arm/efi/Makefile @@ -1,4 +1,4 @@ -CFLAGS-y += -fshort-wchar +include $(srctree)/common/efi/efi-common.mk -obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o +obj-y += $(EFIOBJ-y) obj-$(CONFIG_ACPI) += efi-dom0.init.o diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index c94b4092d4c1..a847c989ff92 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -77,8 +77,9 @@ obj-$(CONFIG_COMPAT) += x86_64/platform_hypercall.o obj-y += sysctl.o endif -# Allows "clean" to descend into boot/ +# Allows "clean" to descend subdir- += boot +subdir- += efi extra-y += asm-macros.i extra-y += xen.lds diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile index e08b4d8e4808..034ec87895df 100644 --- a/xen/arch/x86/efi/Makefile +++ b/xen/arch/x86/efi/Makefile @@ -1,4 +1,4 @@ -CFLAGS-y += -fshort-wchar +include $(srctree)/common/efi/efi-common.mk quiet_cmd_objcopy_o_ihex = OBJCOPY $@ cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex -O binary $< $@ @@ -8,9 +8,6 @@ $(obj)/%.o: $(src)/%.ihex FORCE $(obj)/boot.init.o: $(obj)/buildid.o -EFIOBJ-y := boot.init.o pe.init.o ebmalloc.o runtime.o -EFIOBJ-$(CONFIG_COMPAT) += compat.o - $(call cc-option-add,cflags-stack-boundary,CC,-mpreferred-stack-boundary=4) $(addprefix $(obj)/,$(EFIOBJ-y)): CFLAGS_stack_boundary := $(cflags-stack-boundary) diff --git a/xen/common/efi/efi-common.mk b/xen/common/efi/efi-common.mk new file mode 100644 index 000000000000..ad3c6f2569c3 --- /dev/null +++ b/xen/common/efi/efi-common.mk @@ -0,0 +1,15 @@ +EFIOBJ-y := boot.init.o pe.init.o ebmalloc.o runtime.o +EFIOBJ-$(CONFIG_COMPAT) += compat.o + +CFLAGS-y += -fshort-wchar +CFLAGS-y += -iquote $(srctree)/common/efi + +# Part of the command line transforms $(obj) in to a relative reverted path. +# e.g.: It transforms "dir/foo/bar" into successively +# "dir foo bar", ".. .. ..", "../../.." +$(obj)/%.c: $(srctree)/common/efi/%.c FORCE + $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/common/efi/$(<F) $@ + +clean-files += $(patsubst %.o, %.c, $(EFIOBJ-y:.init.o=.o) $(EFIOBJ-)) + +.PRECIOUS: $(obj)/%.c
Rather than preparing the efi source file, we will make the symbolic link as needed from the build location. The `ln` command is run every time to allow to update the link in case the source tree change location. This patch also introduce "efi-common.mk" which allow to reuse the common make instructions without having to duplicate them into each arch. And now that we have a list of common source file, we can start to remove the links to the source files on clean. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v9: - rename efi_common.mk to efi-common.mk - generalize comment about cleaning "efi" and "boot" subdir in x86. - add a space after the other comma of $(patsubst ) - create a relative symlink instead of an absolute one - with the above, we don't need to use $(abs_srctree) anymore in the prerequisite of the link to the efi source file, use $(srctree). v8: - use symbolic link instead of making a copy of the source - introduce efi_common.mk - remove links to source file on clean - use -iquote for "efi.h" headers in common/efi xen/Makefile | 5 ----- xen/arch/arm/efi/Makefile | 4 ++-- xen/arch/x86/Makefile | 3 ++- xen/arch/x86/efi/Makefile | 5 +---- xen/common/efi/efi-common.mk | 15 +++++++++++++++ 5 files changed, 20 insertions(+), 12 deletions(-) create mode 100644 xen/common/efi/efi-common.mk