Re: [XEN PATCH v9 04/30] build: set ALL_OBJS in main Makefile; move prelink.o to main Makefile

[Julien Grall <julien@xxxxxxx>]

Hi,

On 17/02/2022 16:29, Julien Grall wrote:
> On 28/01/2022 11:41, Jan Beulich wrote:
> > On 28.01.2022 12:32, Anthony PERARD wrote:
> > > On Thu, Jan 27, 2022 at 04:50:32PM +0100, Jan Beulich wrote:
> > > > On 25.01.2022 12:00, Anthony PERARD wrote:
> > > > > --- a/xen/Makefile
> > > > > +++ b/xen/Makefile
> > > > > @@ -285,6 +285,16 @@ CFLAGS += -flto
> > > > >   LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
> > > > >   endif
> > > > > +# Note that link order matters!
> > > > Merely as a remark: I wonder how applicable that comment is anymore.
> > > > If anything I'd expect it to be relevant to $(TARGET_SUBARCH)/head.o
> > > > (Arm) and boot/built_in.o (x86), neither of which get named here.
> > > Indeed, the order here probably doesn't matter. I tried to build on x86
> > > with the list reversed (so still leaving boot/ first) and the build
> > > works. I didn't try to boot it.
> > It's quite unlikely for the order to matter at build time. Being able
> > to boot the result is the minimum. Even then you can't be sure you
> > merely avoided the problematic piece of code on the particular
> > hardware you did the test on. Perhaps the most fragile parts are
> > sections holding pointers which get processed in the order the linker
> > put them. E.g. unexpected interdependencies between initcalls.
> >
> > > Maybe it's time to retire the comment?
> > Probably, but Arm folks would want to confirm that's fine on their side
> > as well.
> Sorry I missed that comment. On Arm, the ordering is enforced in 
> xen/arm/Makefile:
> ALL_OBJS := $(TARGET_SUBARCH)/head.o $(ALL_OBJS)
>
> AFAIK, the ordering in xen/Makefile doesn't matter to us. So I think the 
> comment in xen/Makefile can be droppped.
> That said, I always found this approach too magic. I have a TODO (low 
> priority though) to enforce the ordering in the link script instead.
> Anyway, I will build Xen with your branch and test it to confirm there 
> are no changes.
Tested on Arm64. I confirm this can still boot :).

Cheers,

--
Julien Grall