From: Wei Liu <wei.liu2@citrix.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Steve Capper <steve.capper@linaro.org>,
Wei Chen <wei.chen@arm.com>, Steve Capper <steve.capper@arm.com>,
Shannon Zhao <zhaoshenglong@huawei.com>,
Julien Grall <julien.grall@arm.com>,
Jan Beulich <JBeulich@suse.com>,
Xen-devel <xen-devel@lists.xenproject.org>,
Wei Liu <wei.liu2@citrix.com>,
Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH for-4.8 1/3] libacpi: fix arm64 build
Date: Tue, 18 Oct 2016 12:17:03 +0100 [thread overview]
Message-ID: <20161018111703.GJ23219@citrix.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1610171530120.16441@sstabellini-ThinkPad-X260>
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:
> > > > >
> > > > > 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@arm.com>
> > > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > > > ---
> > > > > Cc: Julien Grall <julien.grall@arm.com>
> > > > > Cc: Wei Chen <wei.chen@arm.com>
> > > > > Cc: Steve Capper <steve.capper@arm.com>
> > > > > Cc: Jan Beulich <JBeulich@suse.com>
> > > > > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > > > > Cc: Shannon Zhao <zhaoshenglong@huawei.com>
> > > > > Cc: Stefano Stebellini <sstabellini@kernel.org>
> > > > >
> > > > > 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.
>
Ah, thanks for digging into this.
> This is dangerous and wrong.
Yes, I agree. This should be fixed.
>
> ---
> 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>
>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Thanks for the patch. This is indeed how it should have been done in the
first place. Sorry for not paying enough attention to this while
reviewing.
> 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@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-10-18 11:17 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-14 17:02 [PATCH for-4.8 0/3] libacpi fixes Wei Liu
2016-10-14 17:02 ` [PATCH for-4.8 1/3] libacpi: fix arm64 build Wei Liu
2016-10-17 10:47 ` Wei Liu
2016-10-17 14:57 ` Steve Capper
2016-10-17 16:55 ` Wei Liu
2016-10-17 23:36 ` Stefano Stabellini
2016-10-18 7:52 ` Steve Capper
2016-10-18 11:21 ` Wei Liu
2016-10-18 11:43 ` Wei Liu
2016-10-18 11:17 ` Wei Liu [this message]
2016-10-14 17:02 ` [PATCH for-4.8 2/3] libacpi: require ACPI_BUILD_DIR to be set Wei Liu
2016-10-24 12:48 ` Jan Beulich
2016-10-14 17:02 ` [PATCH for-4.8 3/3] libacpi: add back the "G" in "GNU" in licence header Wei Liu
2016-10-14 17:19 ` [PATCH for-4.8 0/3] libacpi fixes Andrew Cooper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161018111703.GJ23219@citrix.com \
--to=wei.liu2@citrix.com \
--cc=JBeulich@suse.com \
--cc=boris.ostrovsky@oracle.com \
--cc=julien.grall@arm.com \
--cc=sstabellini@kernel.org \
--cc=steve.capper@arm.com \
--cc=steve.capper@linaro.org \
--cc=wei.chen@arm.com \
--cc=xen-devel@lists.xenproject.org \
--cc=zhaoshenglong@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).