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
-- 
Anthony PERARD

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
> 

-- 
Julien Grall

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,

-- 
Anthony PERARD

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,

-- 
Julien Grall

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!

-- 
Anthony PERARD