Re: [PATCH for-4.8 1/3] libacpi: fix arm64 build

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Oct 18, 2016 at 08:52:24AM +0100, Steve Capper wrote:
> On Mon, Oct 17, 2016 at 04:36:18PM -0700, Stefano Stabellini wrote:
> > On Mon, 17 Oct 2016, Wei Liu wrote:
> > > On Mon, Oct 17, 2016 at 03:57:06PM +0100, Steve Capper wrote:
> > > > On Mon, Oct 17, 2016 at 11:47:00AM +0100, Wei Liu wrote:
> > > > > On Fri, Oct 14, 2016 at 06:02:30PM +0100, Wei Liu wrote:
> > > > > > The arm64 build for libacpi was broken due to two reasons:
> > > > > > 
> 
> [ ... ]
> 
> > > > In file included from /usr/include/linux/types.h:4:0,
> > > >                  from /usr/include/aarch64-linux-gnu/asm/sigcontext.h:19,
> > > >                  from /usr/include/aarch64-linux-gnu/bits/sigcontext.h:27,
> > > >                  from /usr/include/signal.h:332,
> > > >                  from libxl_internal.h:30,
> > > >                  from libxl_arm.h:17,
> > > >                  from libxl_arm_acpi.c:19:
> > > > ../../xen/include/asm/types.h:54:13: error: conflicting types for 'register_t'
> > > >  typedef u64 register_t;
> > > >              ^
> > > 
> > > Now that I think about this, we indeed had similar error in the past.
> > > 
> > > But I'm curious why I succeeded.
> > 
> > I confirm that your patch (344da4f3ad6c4f76ef4efd530f4b1cc6901d6ff9)
> > fixes the dsdt_anycpu_arm.c build issue.
> > 
> > This error is due to the libxl build picking up
> > "../../xen/include/asm/types.h" for
> > 
> > #include <asm/types.h>
> > 
> > which is wrong (it should be /usr/include/asm/types.h).
> > I did some digging and it depends on the build order:
> > 
> > * build tools/ before xen/  -->  works
> > * build xen/ before tools/  -->  does not work
> > 
> > The reason for this is that building Xen creates the xen/include/arm
> > link, which causes ../../xen/include/asm/types.h from being chosen
> > first, because of -I../../xen/include/ in libxl/Makefile.
> > 
> > This is dangerous and wrong.
> > 
> > ---
> > ARM64: fix libxl build, do not include ../../xen/include
> > 
> > Do not include ../../xen/include/ to build libxl_arm_acpi.c: header
> > files clashing against default headers under /usr/include are present in
> > that directory.
> > 
> > Link only $(XEN_ROOT)/xen/include/acpi under tools/include instead.
> > 
> > Build tested on ARM64 and x86_64.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > 
> > diff --git a/tools/include/Makefile b/tools/include/Makefile
> > index dec8b3d..d95d837 100644
> > --- a/tools/include/Makefile
> > +++ b/tools/include/Makefile
> > @@ -20,6 +20,7 @@ xen/.dir:
> >  	ln -sf ../xen-sys/$(XEN_OS) xen/sys
> >  	ln -sf $(addprefix $(XEN_ROOT)/xen/include/xen/,libelf.h elfstructs.h) xen/libelf/
> >  	ln -s ../xen-foreign xen/foreign
> > +	ln -s $(XEN_ROOT)/xen/include/acpi acpi
> >  	touch $@
> >  
> >  # Not xen/xsm as that clashes with link to
> > diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> > index c4e4117..dac19ac 100644
> > --- a/tools/libxl/Makefile
> > +++ b/tools/libxl/Makefile
> > @@ -96,7 +96,7 @@ dsdt_anycpu_arm.c:
> >  	$(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR)
> >  
> >  libxl_arm_acpi.o: libxl_arm_acpi.c
> > -	$(CC) -c $(CFLAGS) -I../../xen/include/ -o $@ libxl_arm_acpi.c
> > +	$(CC) -c $(CFLAGS) -o $@ libxl_arm_acpi.c
> >  else
> >  LIBXL_OBJS-$(CONFIG_ARM) += libxl_arm_no_acpi.o
> >  endif
> 
> Thanks Stefano,
> It hadn't occurred to me to add the acpi symlink :-).
> 
> I can confirm that this fix works for me.
> 

Right. I will turn this into a Tested-by tag. Thanks for testing.

> ( I think we just delete the libxl_arm_acpi.o build rule and rely on
> implicit make? )
> 

Yes, that can be done. Patch is welcome. :-)

Wei.

> Cheers,
> -- 
> Steve

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel