Re: [PATCH v2] KVM: x86: Service NMI requests after PMI requests in VM-Enter path

[Mingwei Zhang]

On Fri, Oct 20, 2023 at 3:58 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Mon, 02 Oct 2023 04:08:39 +0000, Mingwei Zhang wrote:
> > Service NMI requests after PMI requests in vcpu_enter_guest() so that KVM
> > does not need to cancel and redo the VM-Enter. Because APIC emulation
> > "injects" NMIs via KVM_REQ_NMI, handling PMI requests after NMI requests
> > means KVM won't detect the pending NMI request until the final check for
> > outstanding requests. Detecting requests at the final stage is costly as
> > KVM has already loaded guest state, potentially queued events for
> > injection, disabled IRQs, dropped SRCU, etc., most of which needs to be
> > unwound.
> >
> > [...]
>
> Applied to kvm-x86 pmu, thanks!
>
> I made a tweak to the code and massaged one part of the changelog.  For the
> code, I hoisted PMU/PMI above SMI too, mainly to keep SMI+NMI together, but
> also because *technically* the guest could configure LVTPC to send an SMI (LOL).
>
> Regarding the changelog, I replaced the justification about correctness with
> this:
>
>     Note that changing the order of request processing doesn't change the end
>     result, as KVM's final check for outstanding requests prevents entering
>     the guest until all requests are serviced.  I.e. KVM will ultimately
>     coalesce events (or not) regardless of the ordering.
>
> The architectural behavior of NMIs and KVM's unintuitive simultaneous NMI
> handling simply doesn't matter as far as this patch is concerned, especially
> when considering the SMI technicality.  E.g. the net effect would be the same
> even if KVM allowed only a single NMIs.
>
> Please holler if you disagree with either/both of the above changes.

That works for me. Initially, I thought you would touch the SMI code
(if so, I would push back). Nothing like that shows up in the change
so LGTM.

Thanks.
-Mingwei
>
> [1/1] KVM: x86: Service NMI requests after PMI requests in VM-Enter path
>       https://github.com/kvm-x86/linux/commit/4b09cc132a59
>
> --
> https://github.com/kvm-x86/linux/tree/next