Re: [Xen-devel] [PATCH v2 for-4.8] tools/libacpi: Fix compilation when cross building the tools

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 28/11/16 13:30, Jan Beulich wrote:
> > > > On 28.11.16 at 14:13, <julien.grall@xxxxxxx> wrote:
> > The tools (such as mk_dsdt) can be cross-built when it may not be
> > desirable to build them on the target.
> >
> > The commit c4ac1077 "libxl/arm: Generate static ACPI DSDT table"
> > introduced support of ARM64 in mk_dsdt but also break cross-building
> > tools because the ACPI tables are not correct.
> >
> > While mk_dsdt should generate ACPI table for the target architecture, it
> > currently generates the one for the host. This is because the source
> > code contains reference to the host architecture (__aarch64__,
> > __x86_64__, __i386__) when it should be the target architecture.
> >
> > Replace all __aarch64__, __x86_64__, __i386__ by the corresponding
> > CONFIG_*.
> >
> > Also expose the CONFIG_* to the source code as the currently only
> > exposed to the Makefile.
> >
> > Reported-by: Andrii Anisov <andrii.anisov@xxxxxxxxx>
> > Suggested-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> However, ...
>
> > --- a/tools/libacpi/mk_dsdt.c
> > +++ b/tools/libacpi/mk_dsdt.c
> > @@ -17,9 +17,9 @@
> >  #include <getopt.h>
> >  #include <stdlib.h>
> >  #include <stdbool.h>
> > -#if defined(__i386__) || defined(__x86_64__)
> > +#if defined(CONFIG_X86)
> >  #include <xen/hvm/hvm_info_table.h>
> > -#elif defined(__aarch64__)
> > +#elif defined(CONFIG_ARM_64)
> >  #include <xen/arch-arm.h>
> >  #endif
> .. for this and at least some of the others I wonder whether from an
> abstract pov these shouldn't be CONFIG_ARM. Agreed, it won't
> matter as long as there's no use of ACPI on ARM32, hence my R-b
> stands either way, but I'd like you to clarify whether it really should
> go in this way.
To answer the question, I am not aware of any plan to have support for 
ACPI on ARM32.
Regardless the answer, this would be a separate patch as CONFIG_ARM_64 
is the correct define to match __aarch64__. Let's not mix improvement 
and bug fix.
Regards,

--
Julien Grall

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