AMD General Hi Andrew, Peter,
> On Mon, 24 Aug 2026 at 14:31, Frager, Neal <[email protected]> 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
