On 28/02/2018 19:27, Maran Wilson wrote: > Sorry for the delay between this version and the last -- it was mostly > due to holidays and everyone being focused on security bug mitigation > issues. Here are the links to the previous email threads in case it is > helpful: > > V3: https://lkml.org/lkml/2017/12/12/1230 > V2: https://lkml.org/lkml/2017/12/7/1624 > V1: https://lkml.org/lkml/2017/11/28/1280 > > Changes from v3: > > * Implemented Juergen's suggestion for refactoring and moving the PVH > code so that CONFIG_XEN is no longer required for booting KVM guests > via the PVH entry point. > Functionally, nothing has changed from V3 really, but the patches > look completely different now because of all the code movement and > refactoring. Some of these patches can be combined, but I've left > them very small in some cases to make the refactoring and code > movement easier to review. > My approach for refactoring has been to create a PVH entry layer that > still has understanding and knowledge about Xen vs non-Xen guest types > so that it can make run time decisions to handle either case, as > opposed to going all the way and re-writing it to be a completely > hypervisor agnostic and architecturally pure layer that is separate > from guest type details. The latter seemed a bit overkill in this > situation. And I've handled the complexity of having to support > Qemu/KVM boot of kernels compiled with or without CONFIG_XEN via a > pair of xen specific __weak routines that can be overridden in kernels > that support Xen guests. Importantly, the __weak routines are for > xen specific code only (not generic "guest type" specific code) so > there is no clashing between xen version of the strong routine and, > say, a KVM version of the same routine. But I'm sure there are many > ways to skin this cat, so I'm open to alternate suggestions if there > is a compelling reason for not using __weak in this situation. As you say there are many ways to achieve this and I think your choice is fully reasonable (the other alternative that comes to mind is a "Xen detect" function that returns a struct of function pointers). Apart from the placement of the files, it looks great. Thanks! Paolo