2023-10-02 13:03:33

by Mingwei Zhang

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

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.

Move the code snippet processing KVM_REQ_NMI to the one after processing
KVM_REQ_PMI to make KVM vPMU more efficient. Note that changing the order
of the code snippets does make any functional change to KVM. KVM allows two
pending NMIs while x86 architecture allows only one pending. This is
because KVM may not be able to process NMI quickly. Therefore, if a PMI
generated NMI is coincident two other NMIs pending (or one other pending
NMI and KVM blocks NMI) in KVM, then it will be dropped and it is
architecturely ok, since x86 CPU does not guarantee the delivery of each
NMI.

Using SPEC2017 benchmark programs running along with Intel vtune in a VM
demonstrates that the following code change reduces 800~1500 canceled
VM-Enters per second.

Some glory details:

Probe the invocation to vmx_cancel_injection():

$ perf probe -a vmx_cancel_injection
$ perf stat -a -e probe:vmx_cancel_injection -I 10000 # per 10 seconds

Partial results when SPEC2017 with Intel vtune are running in the VM:

On kernel without the change:
10.010018010 14254 probe:vmx_cancel_injection
20.037646388 15207 probe:vmx_cancel_injection
30.078739816 15261 probe:vmx_cancel_injection
40.114033258 15085 probe:vmx_cancel_injection
50.149297460 15112 probe:vmx_cancel_injection
60.185103088 15104 probe:vmx_cancel_injection

On kernel with the change:
10.003595390 40 probe:vmx_cancel_injection
20.017855682 31 probe:vmx_cancel_injection
30.028355883 34 probe:vmx_cancel_injection
40.038686298 31 probe:vmx_cancel_injection
50.048795162 20 probe:vmx_cancel_injection
60.069057747 19 probe:vmx_cancel_injection

v1: https://lore.kernel.org/all/[email protected]/

Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Mingwei Zhang <[email protected]>
---
arch/x86/kvm/x86.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 42a4e8f5e89a..302b6f8ddfb1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10580,12 +10580,12 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
if (kvm_check_request(KVM_REQ_SMI, vcpu))
process_smi(vcpu);
#endif
- if (kvm_check_request(KVM_REQ_NMI, vcpu))
- process_nmi(vcpu);
if (kvm_check_request(KVM_REQ_PMU, vcpu))
kvm_pmu_handle_event(vcpu);
if (kvm_check_request(KVM_REQ_PMI, vcpu))
kvm_pmu_deliver_pmi(vcpu);
+ if (kvm_check_request(KVM_REQ_NMI, vcpu))
+ process_nmi(vcpu);
if (kvm_check_request(KVM_REQ_IOAPIC_EOI_EXIT, vcpu)) {
BUG_ON(vcpu->arch.pending_ioapic_eoi > 255);
if (test_bit(vcpu->arch.pending_ioapic_eoi,

base-commit: 73554b29bd70546c1a9efc9c160641ef1b849358
--
2.42.0.582.g8ccd20d70d-goog


2023-10-20 22:58:55

by Sean Christopherson

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

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.

[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

2023-10-21 02:37:28

by Mingwei Zhang

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

On Fri, Oct 20, 2023 at 3:58 PM Sean Christopherson <[email protected]> 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