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

[Steve Capper <steve.capper@linaro.org>]

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.

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

Cheers,
-- 
Steve

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