Hi Jan, > On 24 Aug 2022, at 14:05, Jan Beulich <jbeul...@suse.com> wrote: > > On 24.08.2022 14:43, Bertrand Marquis wrote: >> >> >>> On 24 Aug 2022, at 08:37, Jan Beulich <jbeul...@suse.com> wrote: >>> >>> On 23.08.2022 17:56, Bertrand Marquis wrote: >>>>> On 23 Aug 2022, at 16:45, Jan Beulich <jbeul...@suse.com> wrote: >>>>> On 23.08.2022 17:09, Bertrand Marquis wrote: >>>>>> How about moving those to a xen-acpi.h header and include that one in >>>>>> xen.h ? >>>>> >>>>> In principle okay, if there wasn't the need for HVM_MAX_VCPUS. With a >>>>> suitable comment it may be okay to live there. I'd be curious what >>>>> others think. >>>> >>>> The problem with this already exists in the current status as this is >>>> defined in >>>> hvm_info_table.h which is never included from arch-x86/xen.h >>> >>> You're referring to it being necessary to explicitly include both headers. >>> That's not what I'm referring to, though: The tool imo shouldn't include >>> hvm_info_table.h, and hence the HVM_MAX_VCPUS would need to move as well. >> >> Any suggestion where ? > > Not really, no. That's why I said this is the one part where improvement > is more difficult. Otoh hvm_info_table.h is self-contained right now and > doesn't even produce potentially misleading struct layout for the one > struct it declares. So perhaps not too bad if left alone. > >> The more I dig, the more I find that everything is including xen.h and going >> round. >> Arch-x86_*.h headers are including arch-x86/xen.h including xen.h > > Indeed, all quite odd. > >>>> Including hvm_info_table.h from xen-acpi.h could create include path >>>> issues. >>> >>> Include path issues? Both are / would be public headers. But as said, I >>> don't think any new header introduced for the purpose at hand should >>> include _any_ other public header. >> >> For now I can create a arch-x86/xen-acpi.h and move there the XEN_ACPI_* >> definitions and include that one instead in mk_dsdt.h. >> The change will be small and should not have much impact. >> >> Modifying HVM_MAX_VCPUS is not per say needed and I think would not be >> enough to make the situation cleaner. >> >>> >>>> But as those are used nowhere apart from mk_dsdt, I would probably skip the >>>> include of xen-acpi.h from xen.h. >>> >>> Hmm, yes, that's reasonable I guess as far as XEN_ACPI_* go. Of course >>> HVM_MAX_VCPUS is a different matter. >>> >>>> Any chance that those XEN_ACPI_ are needed by some external tools that >>>> could get broken by this modification ? >>> >>> Requiring them to include another header is, I think, a tolerable form >>> of breakage, the more that such breakage isn't very likely anyway. >> >> Then if you are ok with it, I will just submit the xen-acpi.h creation patch >> and fix >> mk_dsdt compilation for x86 on arm. >> >> The rest would require more thinking and I do not think it should be done >> now. >> >> Can you confirm you agree ? > > Almost - I don't like xen-acpi.h as the name of the new header. Just > arch-x86/acpi.h would likely be too generic, so I'd like to suggest > arch-x86/hvm-acpi.h or arch-x86/guest-acpi.h. I have a slight > preference to the latter, because "hvm" also covers "pvh", yet PVH > Dom0 is dealt with entirely differently ACPI-wise. Plus "guest" > isn't misleading as to PV, because PV guests don't have ACPI anyway.
Guest-acpi.h it will be. Bertrand > > Jan
