Hi, > On 23 Aug 2022, at 15:41, Bertrand Marquis <[email protected]> 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. > >> >>> 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.
Only the 3 XEN_ACPI_ are needed and those are in fact only used by mk_dsdt.c. How about moving those to a xen-acpi.h header and include that one in xen.h ? Other solution as those are only used in mk_dsdt, I could just define them there … This is the commit which created the issue: https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=d6ac8e22c7c5525db1da79fd1d1f03ee6b557f0d Any other idea how to properly fix this ? Cheers Bertrand > >> >> 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
