> 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. > >> The tool needs at least: >> HVM_MAX_VCPUS >> XEN_ACPI_CPU_MAP >> XEN_ACPI_CPU_MAP_LEN >> XEN_ACPI_GPE0_CPUHP_BIT >> >> Which are defined in arch-x86/xen.h and hvm_info_table.h. >> >> I am not quite sure how to get those without the current include > > 1) Move those #define-s to a standalone header, which the ones > currently defining them would simply include. The tool would then > include _only_ this one header. Shouldn’t we try to unify a little bit what is done on arm and x86 here ? Not only for this tool but in general in the public headers I will try to reduce the problem a bit more to find what we would need to pull out in a standalone header. > > 2) Seddery on the headers, producing a local one to be used by the > tool. You mean autogenerating something ? This would just move the problem. Bertrand > > Jan
