Messages in this thread | | | Date | Fri, 25 Mar 2022 22:25:47 +0100 | From | "Maciej S. Szmigiero" <> | Subject | Re: [PATCH 00/21] KVM: x86: Event/exception fixes and cleanups |
| |
On 24.03.2022 22:31, Sean Christopherson wrote: > On Sun, Mar 13, 2022, Maxim Levitsky wrote: >> On Fri, 2022-03-11 at 03:27 +0000, Sean Christopherson wrote: >>> The main goal of this series is to fix KVM's longstanding bug of not >>> honoring L1's exception intercepts wants when handling an exception that >>> occurs during delivery of a different exception. E.g. if L0 and L1 are >>> using shadow paging, and L2 hits a #PF, and then hits another #PF while >>> vectoring the first #PF due to _L1_ not having a shadow page for the IDT, >>> KVM needs to check L1's intercepts before morphing the #PF => #PF => #DF >>> so that the #PF is routed to L1, not injected into L2 as a #DF. >>> >>> nVMX has hacked around the bug for years by overriding the #PF injector >>> for shadow paging to go straight to VM-Exit, and nSVM has started doing >>> the same. The hacks mostly work, but they're incomplete, confusing, and >>> lead to other hacky code, e.g. bailing from the emulator because #PF >>> injection forced a VM-Exit and suddenly KVM is back in L1. >>> >>> Everything leading up to that are related fixes and cleanups I encountered >>> along the way; some through code inspection, some through tests (I truly >>> thought this series was finished 10 commits and 3 days ago...). >>> >>> Nothing in here is all that urgent; all bugs tagged for stable have been >>> around for multiple releases (years in most cases). >>> >> I am just curious. Are you aware that I worked on this few months ago? > > Ah, so that's why I had a feeling of deja vu when factoring out kvm_queued_exception. > I completely forgot about it :-/ In my defense, that was nearly a year ago[1][2], though > I suppose one could argue 11 == "a few" :-) > > [1] https://lore.kernel.org/all/20210225154135.405125-1-mlevitsk@redhat.com > [2] https://lore.kernel.org/all/20210401143817.1030695-3-mlevitsk@redhat.com > >> I am sure that you even reviewed some of my code back then. > > Yep, now that I've found the threads I remember discussing the mechanics. > >> If so, could you have had at least mentioned this and/or pinged me to continue >> working on this instead of re-implementing it? > > I'm invoking Hanlon's razor[*]; I certainly didn't intended to stomp over your > work, I simply forgot. > > As for the technical aspects, looking back at your series, I strongly considered > taking the same approach of splitting pending vs. injected (again, without any > recollection of your work). I ultimately opted to go with the "immediated morph > to pending VM-Exit" approach as it allows KVM to do the right thing in almost every > case without requiring new ABI, and even if KVM screws up, e.g. queues multiple > pending exceptions. It also neatly handles one-off things like async #PF in L2. > > However, I hadn't considered your approach, which addresses the ABI conundrum by > processing pending=>injected immediately after handling the VM-Exit. I can't think > of any reason that wouldn't work, but I really don't like splitting the event > priority logic, nor do I like having two event injection sites (getting rid of the > extra calls to kvm_check_nested_events() is still on my wish list). If we could go > back in time, I would likely vote for properly tracking injected vs. pending, but > since we're mostly stuck with KVM's ABI, I prefer the "immediately morph to pending > VM-Exit" hack over the "immediately morph to 'injected' exception" hack.
So, what's the plan here: is your patch set Sean considered to supersede Maxim's earlier proposed changes or will you post an updated patch set incorporating at least some of them?
I am asking because I have a series that touches the same general area of KVM [1] and would preferably have it based on the final form of the event injection code to avoid unforeseen negative interactions between these changes.
Thanks, Maciej
[1]: https://lore.kernel.org/kvm/d04e096a-b12e-91e2-204e-b3643a62d705@maciej.szmigiero.name/
| |