Re: [XEN PATCH 07/15] build: move XEN_HAS_BUILD_ID out of Config.mk

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, May 25, 2023 at 01:56:53PM +0200, Jan Beulich wrote:
> On 23.05.2023 18:38, Anthony PERARD wrote:
> > Whether or not the linker can do build id is only used by the
> > hypervisor build, so move that there.
> > 
> > Rename $(build_id_linker) to $(XEN_LDFLAGS_BUILD_ID) as this is a
> > better name to be exported as to use the "XEN_*" namespace.
> > 
> > Also update XEN_TREEWIDE_CFLAGS so flags can be used for
> > arch/x86/boot/ CFLAGS_x86_32
> > 
> > Beside a reordering of the command line where CFLAGS is used, there
> > shouldn't be any other changes.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> with one nit:
> 
> > --- a/xen/scripts/Kbuild.include
> > +++ b/xen/scripts/Kbuild.include
> > @@ -91,6 +91,9 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) 
> > $(2)000 ] && echo $(3) || e
> >  
> >  clang-ifversion = $(shell [ $(CONFIG_CLANG_VERSION)0 $(1) $(2)000 ] && 
> > echo $(3) || echo $(4))
> >  
> > +ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \
> > +                                   grep -q build-id && echo n || echo y)
> 
> I realize you only move this line, but I think we want indentation here
> to improve at this occasion:
> 
> ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \
>                           grep -q build-id && echo n || echo y)
> 
> I'll be happy to adjust while committing. Which raises the question: Is
> there any dependency here on earlier patches in the series? It doesn't
> look so to me, but I may easily be overlooking something. (Of course
> first further arch maintainer acks would be needed.)

I don't see any dependency on earlier patch, so it looks fine to apply
earlier. Thanks.

> > --- a/xen/test/livepatch/Makefile
> > +++ b/xen/test/livepatch/Makefile
> > @@ -37,7 +37,7 @@ $(obj)/modinfo.o:
> >  
> >  #
> >  # This target is only accessible if CONFIG_LIVEPATCH is defined, which
> > -# depends on $(build_id_linker) being available. Hence we do not
> > +# depends on $(XEN_LDFLAGS_BUILD_ID) being available. Hence we do not
> >  # need any checks.
> 
> As an aside, I'm a little confused by "is only accessible" here. I don't
> see how CONFIG_LIVEPATCH controls reachability. At the very least the
> parent dir's Makefile doesn't use CONFIG_LIVEPATCH at all.

Yes. `make tests` works just fine without CONFIG_LIVEPATCH. Even when
that comment was introduce, it didn't seems like there were anything
preventing the target from been run.

The only thing is that `make tests` doesn't build the tests if xen-syms
was built without build_id, so the status of CONFIG_LIVEPATCH doesn't
seems to matter.

Thanks,

-- 
Anthony PERARD