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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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:
> > > > 
> > > > 1. ACPI_BUILD_DIR was appended twice to dsdt_anycpu_arm.c.
> > > > 2. The inclusion of firmware/Rules.mk overrided XEN_TARGET_ARCH, which
> > > >    made CONFIG_ARM disappear.
> > > > 
> > > > Fix those by:
> > > > 
> > > > 1. Correctly generate full path for dsdt_anaycpu_arm.c.
> > > > 2. Include tools/Rules.mk instead, because libacpi/Makefile doesn't rely
> > > >    on settings in firmware/Rules.mk.
> > > > 
> > > > While at it, use CONFIG_ARM_64 instead of CONFIG_ARM as it is more
> > > > accurate.
> > > > 
> > > > Reported-by: Julien Grall <julien.grall@xxxxxxx>
> > > > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > ---
> > > > Cc: Julien Grall <julien.grall@xxxxxxx>
> > > > Cc: Wei Chen <wei.chen@xxxxxxx>
> > > > Cc: Steve Capper <steve.capper@xxxxxxx>
> > > > Cc: Jan Beulich <JBeulich@xxxxxxxx>
> > > > Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > > > Cc: Shannon Zhao <zhaoshenglong@xxxxxxxxxx>
> > > > Cc: Stefano Stebellini <sstabellini@xxxxxxxxxx>
> > > > 
> > > > Please check if CONFIG_ARM_64 is correct -- IIRC ACPI is only relevant
> > > > on arm64.
> > > > 
> > > > Would appreciate any build test report from ARM people.
> > > 
> > > I set up a chroot environment this morning and built arm64 Xen. It
> > > worked.
> > > 
> > > Since Jan and Julien are both away, I've taken the liberty of applying
> > > this patch with both my RM hat and tools maintainer hat on.
> > > 
> > > I have also applied patch #3 since it is rather trivial.
> > > 
> > > I will let Jan decide whether patch #2 is necessary.
> > > 
> > 
> > Thanks Wei,
> > I am trying to verify this patch, but I think I am running into another
> > issue with the libxl acpi support patches.
> > 
> > Essentially I'm getting namespace clashes with the following:
> >  * nonnull
> >  * noreturn
> >  * register_t
> > 
> > I think this is due to the following logic in the libxl/Makefile:
> > libxl_arm_acpi.o: libxl_arm_acpi.c
> >         $(CC) -c $(CFLAGS) -I../../xen/include/ -o $@ libxl_arm_acpi.c
> > 
> > When compiling libxl_arm_acpi.c, /usr/include/linux/types.h tries to pull
> > in xen/include/xen/types.h (instead of /usr/include/asm/types.h), which
> > then eventually pulls in xen/include/xen/compiler.h which redefines key
> > information.
> > 
> > Which rootfs are you chrooting into for the testing?
> > (I've experienced build issues on Debian Jessie and Ubuntu Xenial running
> > in a Docker container).
> > 
> 
> qemu-debootstrap, sid, arm64.
> 
> I saw similar errors. But they went away after I `git clean -fffddddxx`
> the tree.
> 
> The fact that they went away somehow and Julien succeeded in building on
> variant of this patch made me think it was due to some issues in my
> environment.
> 
> > Cheers,
> > -- 
> > Steve
> > 
> > I build Xen via:
> > $ git clean -f -d -x
> > $ ./configure --with-xenstored=xenstored 
> > --with-system-qemu=/usr/local/bin/qemu-system-aarch64
> > $ make
> > 
> > My builds finish like this:
> > 
> > gcc -c -O1 -fno-omit-frame-pointer  -DBUILD_ID -g -fno-strict-aliasing 
> > -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement 
> > -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O0 -g3 
> > -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF 
> > .libxl_arm_acpi.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE 
> > -fno-optimize-sibling-calls  -Werror -Wno-format-zero-length 
> > -Wmissing-declarations -Wno-declaration-after-statement -Wformat-nonliteral 
> > -I. -fPIC -pthread 
> > -I/home/steven/xen/tools/libxl/../../tools/libs/toollog/include 
> > -I/home/steven/xen/tools/libxl/../../tools/include 
> > -I/home/steven/xen/tools/libxl/../../tools/libs/evtchn/include 
> > -I/home/steven/xen/tools/libxl/../../tools/include 
> > -I/home/steven/xen/tools/libxl/../../tools/libxc/include 
> > -I/home/steven/xen/tools/libxl/../../tools/libs/toollog/include 
> > -I/home/steven/xen/tools/libxl/../../tools/include 
> > -I/home/steven/xen/tools/libxl/../../tools/libs/foreignmemory/include 
> > -I/home/steven/xen/tool
 s/libxl/../../tools/include -I/home/steven/xen/tools/libxl/../../tools/include 
-D__XEN_TOOLS__ -I/home/steven/xen/tools/libxl/../../tools/libxc/include 
-I/home/steven/xen/tools/libxl/../../tools/libs/evtchn/include 
-I/home/steven/xen/tools/libxl/../../tools/include 
-I/home/steven/xen/tools/libxl/../../tools/libs/foreignmemory/include 
-I/home/steven/xen/tools/libxl/../../tools/include 
-I/home/steven/xen/tools/libxl/../../tools/include 
-I/home/steven/xen/tools/libxl/../../tools/xenstore/include 
-I/home/steven/xen/tools/libxl/../../tools/include 
-I/home/steven/xen/tools/libxl/../../tools/blktap2/control 
-I/home/steven/xen/tools/libxl/../../tools/blktap2/include 
-I/home/steven/xen/tools/libxl/../../tools/include  -Wshadow -include 
/home/steven/xen/tools/libxl/../../tools/config.h -I../../xen/include/ -o 
libxl_arm_acpi.o libxl_arm_acpi.c
> > 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@xxxxxxxxxx>

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

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