RE: [PATCH v1 1/1] include/hw/xen/xen_native.h: downgrade include-order assertion to warning

["Frager, Neal" <neal.frager@xxxxxxx>]

AMD General

Hi Andrew, Peter,

> On Mon, 24 Aug 2026 at 14:31, Frager, Neal <neal.frager@xxxxxxx> wrote:
>> AMD General
>>
>> Hi Peter,
>>
>>> Hi Stewart,
>>>
>>>> The -I$(XEN_ROOT)/tools/include added to QEMU's extra-cflags causes
>>>> __XEN_INTERFACE_VERSION__ to be defined before xen_native.h is included,
>>>> triggering an include-order assertion. Downgrade to a warning since the
>>>> version is consistent in cross-compile.
>>>> Ref: https://github.com/qemu/qemu/commit/e2abfe5ec6
>>>> This is a buildroot issue, so I don't believe it's necessary to fix from 
>>>> the
>>>> qemu side.
>>> I am not sure I fully agree here. While this is a buildroot identified 
>>> issue,
>>> there could be other use cases for __XEN_INTERFACE_VERSION__ to be defined
>>> before xen_native.h is included.
>>> But what, though?
>>> And what we have found is that if
>>> __XEN_INTERFACE_VERSION__ to be defined before xen_native.h is included, it
>>> is not a hard error.  For buildroot, the qemu works just fine in spite of
>>> this.
>>
>>> I think that just means you got lucky. Either there is a hard requirement
>>> for one header to be included before the other (in which case it must
>>> be a #error, and whatever is causing the mis-ordering to happen must be
>>> fixed), or it's fine for the ordering to be either way (in which case it
>>> doesn't even need to be a #warning).
>> From my view, the order the header files are included does not matter, and
>> this should not be an error.  I agree with removing the warning as well, if
>> that is what we all agree on in the end.
> The rationale for the header ordering is in the comment in 
> include/hw/xen/xen.h:
>
> /*
>  * C files using Xen toolstack libraries will have included those headers
>  * already via xen_native.h, and having __XEM_TOOLS__ defined will have

> Lovely typo there.

> The define __XEN_TOOLS__ is woefully misnamed.  This is an error of
> Xen's, which I've not had time to fix yet.

> It should be named __XEN_UNSTABLE_APIS__, and thinking of it like this
> will make it's purpose a whole lot clearer.

>  * automatically set __XEN_INTERFACE_VERSION__ to the latest supported
>  * by the *system* Xen headers which were transitively included.
>  *
>  * C files which are part of the internal emulation, and which did not
>  * include xen_native.h, may need this defined so that the Xen headers
>  * imported to include/hw/xen/interface/ will expose the appropriate API
>  * version.
>  *
>  * This is why there's a rule that xen_native.h must be included first.
>  */
>
> ...basically, if something doesn't include xen_native.h before xen.h
> then __XEN_INTERFACE_VERSION__ can end up defined to the wrong thing.
> (Disclaimer: I'm not a Xen expert, I'm just applying Chesterton's Fence.)

> __XEN_INTERFACE_VERSION__ does alter structures.  It must be consistent
> across a codebase.

Stewart identified a better solution for this buildroot issue, so I am
withdrawing this patch.  This patch is not necessary.  Thank you.

Best regards,
Neal Frager
AMD