On Mon, 2023-03-13 at 11:40 -0700, Sean Christopherson wrote: > On Mon, Mar 13, 2023, Huang, Kai wrote: > > On Fri, 2023-03-10 at 13:42 -0800, Sean Christopherson wrote: > > > Attempt to disable virtualization during an emergency reboot if and only > > > if there is a registered virt callback, i.e. iff a hypervisor (KVM) is > > > active. If there's no active hypervisor, then the CPU can't be operating > > > with VMX or SVM enabled (barring an egregious bug). > > > > IIUC, this patch is the final one that you want to achieve how the "disable > > virtualization" callback should work in the non-KVM core kernel (the rest > > patches are related to moving VMXOFF code to KVM as the core-kernel now just > > calls the callback, etc). � > > > > There are middle step patches (2-7) to eventually help to get to this point. > > But to be honest, personally, I am not sure whether those patches are necessary, > > i.e. to me they actually cost more time to review since I have to think whether > > such intermediate status is reasonable or not. I am wondering whether we can > > just merge those patches together as single one, so it's easy to see what is the > > final goal to achieve? > > I agree that the fine granularity makes it difficult to see the final form, but > from a bisection perspective I really, really want each change to be isolated as > much as possible. This code is extremely difficult, if not practically impossible, > to exhaustively test due to multiple points of entry from "this should never happen!" > types of flows. If any of these changes breaks someones deployment, I want to > make it as easy as possible for that someone to determine exactly what broke. Yeah sure. Yes in general I agree we should make bisection easy to pinpoint the exact code which breaks something, but I think over-splitting is also unnecessary especially when code change is small ;)
