Hi Jan, > On 23 Aug 2022, at 17:15, Jan Beulich <[email protected]> wrote: > > On 23.08.2022 16:41, Bertrand Marquis wrote: >> >> >>> On 23 Aug 2022, at 15:31, Jan Beulich <[email protected]> wrote: >>> >>> On 23.08.2022 15:34, Bertrand Marquis wrote: >>>>> On 23 Aug 2022, at 13:33, Jan Beulich <[email protected]> wrote: >>>>> On 23.08.2022 12:24, Bertrand Marquis wrote: >>>>>> --- a/tools/libacpi/mk_dsdt.c >>>>>> +++ b/tools/libacpi/mk_dsdt.c >>>>>> @@ -18,6 +18,16 @@ >>>>>> #include <stdlib.h> >>>>>> #include <stdbool.h> >>>>>> #if defined(CONFIG_X86) >>>>>> +/* >>>>>> + * When building on non x86 host, arch-x86/xen.h will include xen.h >>>>>> which will >>>>>> + * try to include the arch xen.h (for example if built on arm, >>>>>> x86/xen.h will >>>>>> + * include xen.h which will include arch-arm.h). >>>>>> + * To prevent this effect, define x86 to have the proper sub arch >>>>>> included when >>>>>> + * the compiler does not define it. >>>>>> + */ >>>>>> +#if !(defined(__i386__) || defined(__x86_64__)) >>>>>> +#define __x86_64__ >>>>>> +#endif >>>>> >>>>> Besides being confusing this depends on the order of checks in xen.h. >>>>> >>>>>> #include <xen/arch-x86/xen.h> >>>>>> #include <xen/hvm/hvm_info_table.h> >>>>>> #elif defined(CONFIG_ARM_64) >>>>> >>>>> At the very least you will want to #undef the auxiliary define as soon >>>>> as practically possible. >>>> >>>> Ack >>>> >>>>> >>>>> But I think a different solution will want finding. Did you check what >>>>> the #include is needed for, really? I've glanced through the file >>>>> without being able to spot anything ... After all this is a build tool, >>>>> which generally can't correctly use many of the things declared in the >>>>> header. >>>> >>>> As stated in the comment after the commit message, this is not a good >>>> solution but an hack. >>>> >>>> Now I do not completely agree here, the tool is not really the problem >>>> but the headers are. >>> >>> Well - the issue is the tool depending on these headers. >> >> Yes but the tool itself cannot solve the issue, we need to have the values >> in properly accessible headers. >> >>> >>>> There is not such an issue on arm. >>> >>> Then why does the tool include xen/arch-arm.h for Arm64? >> >> Because this header defines the values required and as no such thing as >> include xen.h. >> The point is on arm, the arch-arm.h header does not depend on per cpu >> defines. > > At first I was surprised you get away there without including xen.h - > this may change at any time, as soon as you grow a dependency. > > But then the inclusion by arch-x86/xen.h looks suspicious, since xen.h > itself includes arch-x86/xen.h (first thing), so unless I'm missing > something arch-x86/xen.h can't really have a dependency on xen.h. So > maybe in the short term you could get away with removing that include > as a "fix"?
Just removing the include is ending up in errors (I tried that first). I will dig deeper to check if those are possible to solve but some files including arch-x86/xen.h should actually including xen.h instead and I think the amount of changes might get a bit bigger. I will give it a try. Bertrand > > Jan
