| Message ID | 20210824105038.1257926-11-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Build system improvements, now with out-of-tree build! | expand |
Hi Anthony, Can you explain why this is moved? Cheers, On 24/08/2021 11:49, Anthony PERARD wrote: > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > xen/arch/arm/Makefile | 8 -------- > xen/arch/arm/arch.mk | 8 ++++++++ > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 0edd9dee6f49..3d0af8ebc93c 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -104,14 +104,6 @@ prelink.o: $(ALL_OBJS) $(ALL_LIBS) FORCE > $(call if_changed,ld) > endif > > -ifeq ($(CONFIG_ARM64_ERRATUM_843419),y) > - ifeq ($(call ld-option, --fix-cortex-a53-843419),n) > - $(warning ld does not support --fix-cortex-a53-843419; xen may be susceptible to erratum) > - else > - XEN_LDFLAGS += --fix-cortex-a53-843419 > - endif > -endif > - > targets += prelink.o > > $(TARGET)-syms: prelink.o xen.lds > diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk > index 11caec86ba14..6a2982059486 100644 > --- a/xen/arch/arm/arch.mk > +++ b/xen/arch/arm/arch.mk > @@ -17,3 +17,11 @@ $(call cc-option-add,CFLAGS-$(CONFIG_ARM_64),CC,-mno-outline-atomics) > ifneq ($(filter command line environment,$(origin CONFIG_EARLY_PRINTK)),) > $(error You must use 'make menuconfig' to enable/disable early printk now) > endif > + > +ifeq ($(CONFIG_ARM64_ERRATUM_843419),y) > + ifeq ($(call ld-option, --fix-cortex-a53-843419),n) > + $(warning ld does not support --fix-cortex-a53-843419; xen may be susceptible to erratum) > + else > + LDFLAGS += --fix-cortex-a53-843419 > + endif > +endif >
On Tue, Aug 24, 2021 at 01:50:33PM +0100, Julien Grall wrote: > Hi Anthony, > > Can you explain why this is moved? Well, it was in the wrong place, it is best to avoid making changes to XEN_CFLAGS or XEN_LDFLAGS in Makefile in subdirectories as those are exported variable and may not work as expected. They may or may not change *flags in subdirectories, or in the case of arch/*/Makefile, they might changes *flags in the other directories or not depending on whether one try to build "xen" or just "common/". But more importantly, with patch build: set ALL_OBJS to main Makefile; move prelink.o to main Makefile make isn't going to load arch/*/Makefile before building the rest of xen, and thus changes to XEN_LDFLAGS will only apply to arch/*/Makefile rather than for all objects. (XEN_LDFLAGS is used with all built_in.o and few other places like for libfdt-temp.o) If you want an explanation in the commit message, maybe this one will do: Changes to XEN_LDFLAGS may or may not apply to targets in for example "common/" depending on whether one runs `make` or `make common/`. But arch.mk is loaded before doing any build, so changes to LDFLAGS there mean that the value of XEN_LDFLAGS won't depends on the initial target. Thanks,
Hi Anthony, On 24/08/2021 16:00, Anthony PERARD wrote: > On Tue, Aug 24, 2021 at 01:50:33PM +0100, Julien Grall wrote: >> Hi Anthony, >> >> Can you explain why this is moved? > > Well, it was in the wrong place, it is best to avoid making changes to > XEN_CFLAGS or XEN_LDFLAGS in Makefile in subdirectories as those are > exported variable and may not work as expected. They may or may not > change *flags in subdirectories, or in the case of arch/*/Makefile, they > might changes *flags in the other directories or not depending on > whether one try to build "xen" or just "common/". > > But more importantly, with patch > build: set ALL_OBJS to main Makefile; move prelink.o to main Makefile > make isn't going to load arch/*/Makefile before building the rest of > xen, and thus changes to XEN_LDFLAGS will only apply to arch/*/Makefile > rather than for all objects. (XEN_LDFLAGS is used with all built_in.o > and few other places like for libfdt-temp.o) Thanks for the explanation. That makes sense. > > If you want an explanation in the commit message, maybe this one will > do: > Changes to XEN_LDFLAGS may or may not apply to targets in for > example "common/" depending on whether one runs `make` or `make > common/`. > > But arch.mk is loaded before doing any build, so changes to LDFLAGS > there mean that the value of XEN_LDFLAGS won't depends on the > initial target. An explanation in the commit message would be useful. This one you provided looks good. So: Acked-by: Julien Grall <jgrall@amazon.com> Can it be merged before the previous patch? If so, I can update the commit message and commit it. Cheers,
On Tue, Aug 24, 2021 at 04:23:22PM +0100, Julien Grall wrote: > Hi Anthony, > > On 24/08/2021 16:00, Anthony PERARD wrote: > > Changes to XEN_LDFLAGS may or may not apply to targets in for > > example "common/" depending on whether one runs `make` or `make > > common/`. > > > > But arch.mk is loaded before doing any build, so changes to LDFLAGS > > there mean that the value of XEN_LDFLAGS won't depends on the > > initial target. > > An explanation in the commit message would be useful. This one you provided > looks good. So: > > Acked-by: Julien Grall <jgrall@amazon.com> > > Can it be merged before the previous patch? If so, I can update the commit > message and commit it. Yes, that should be fine, thanks!
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 0edd9dee6f49..3d0af8ebc93c 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -104,14 +104,6 @@ prelink.o: $(ALL_OBJS) $(ALL_LIBS) FORCE $(call if_changed,ld) endif -ifeq ($(CONFIG_ARM64_ERRATUM_843419),y) - ifeq ($(call ld-option, --fix-cortex-a53-843419),n) - $(warning ld does not support --fix-cortex-a53-843419; xen may be susceptible to erratum) - else - XEN_LDFLAGS += --fix-cortex-a53-843419 - endif -endif - targets += prelink.o $(TARGET)-syms: prelink.o xen.lds diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk index 11caec86ba14..6a2982059486 100644 --- a/xen/arch/arm/arch.mk +++ b/xen/arch/arm/arch.mk @@ -17,3 +17,11 @@ $(call cc-option-add,CFLAGS-$(CONFIG_ARM_64),CC,-mno-outline-atomics) ifneq ($(filter command line environment,$(origin CONFIG_EARLY_PRINTK)),) $(error You must use 'make menuconfig' to enable/disable early printk now) endif + +ifeq ($(CONFIG_ARM64_ERRATUM_843419),y) + ifeq ($(call ld-option, --fix-cortex-a53-843419),n) + $(warning ld does not support --fix-cortex-a53-843419; xen may be susceptible to erratum) + else + LDFLAGS += --fix-cortex-a53-843419 + endif +endif
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- xen/arch/arm/Makefile | 8 -------- xen/arch/arm/arch.mk | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-)