From: Paolo Bonzini <pbonzini@redhat.com>
To: "Sean Christopherson" <sean.j.christopherson@intel.com>,
"Radim Krčmář" <rkrcmar@redhat.com>,
"Joerg Roedel" <joro@8bytes.org>
Cc: kvm@vger.kernel.org, Jim Mattson <jmattson@google.com>
Subject: Re: [PATCH 5/5] KVM: VMX: Handle NMIs, #MCs and async #PFs in common irqs-disabled fn
Date: Thu, 6 Jun 2019 15:20:49 +0200 [thread overview]
Message-ID: <746c7e2c-176f-d772-e37d-41bb9f524dd6@redhat.com> (raw)
In-Reply-To: <20190420055059.16816-6-sean.j.christopherson@intel.com>
On 20/04/19 07:50, Sean Christopherson wrote:
> Per commit 1b6269db3f833 ("KVM: VMX: Handle NMIs before enabling
> interrupts and preemption"), NMIs are handled directly in vmx_vcpu_run()
> to "make sure we handle NMI on the current cpu, and that we don't
> service maskable interrupts before non-maskable ones". The other
> exceptions handled by complete_atomic_exit(), e.g. async #PF and #MC,
> have similar requirements, and are located there to avoid extra VMREADs
> since VMX bins hardware exceptions and NMIs into a single exit reason.
>
> Clean up the code and eliminate the vaguely named complete_atomic_exit()
> by moving the interrupts-disabled exception and NMI handling into the
> existing handle_external_intrs() callback, and rename the callback to
> a more appropriate name.
>
> In addition to improving code readability, this also ensures the NMI
> handler is run with the host's debug registers loaded in the unlikely
> event that the user is debugging NMIs. Accuracy of the last_guest_tsc
> field is also improved when handling NMIs (and #MCs) as the handler
> will run after updating said field.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Very nice, just some changes I'd like to propose. "atomic" is Linux
lingo for "irqs disabled", so I'd like to rename the handler to
handle_exit_atomic so it has a correspondance with handle_exit.
Likewise we could have handle_exception_nmi_atomic and
handle_external_interrupt_atomic.
Putting everything together we get:
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 35e7937cc9ac..b7d5935c1637 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1117,7 +1117,7 @@ struct kvm_x86_ops {
int (*check_intercept)(struct kvm_vcpu *vcpu,
struct x86_instruction_info *info,
enum x86_intercept_stage stage);
- void (*handle_external_intr)(struct kvm_vcpu *vcpu);
+ void (*handle_exit_atomic)(struct kvm_vcpu *vcpu);
bool (*mpx_supported)(void);
bool (*xsaves_supported)(void);
bool (*umip_emulated)(void);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index acc09e9fc173..9c6458e60558 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -6172,7 +6172,7 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
return ret;
}
-static void svm_handle_external_intr(struct kvm_vcpu *vcpu)
+static void svm_handle_exit_atomic(struct kvm_vcpu *vcpu)
{
kvm_before_interrupt(vcpu);
local_irq_enable();
@@ -7268,7 +7268,7 @@ static bool svm_need_emulation_on_page_fault(struct kvm_vcpu *vcpu)
.set_tdp_cr3 = set_tdp_cr3,
.check_intercept = svm_check_intercept,
- .handle_external_intr = svm_handle_external_intr,
+ .handle_exit_atomic = svm_handle_exit_atomic,
.request_immediate_exit = __kvm_request_immediate_exit,
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 963c8c409223..dfaa770b9bb3 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4437,11 +4437,11 @@ static void kvm_machine_check(void)
static int handle_machine_check(struct kvm_vcpu *vcpu)
{
- /* already handled by vcpu_run */
+ /* handled by vmx_vcpu_run() */
return 1;
}
-static int handle_exception(struct kvm_vcpu *vcpu)
+static int handle_exception_nmi(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
struct kvm_run *kvm_run = vcpu->run;
@@ -4454,7 +4454,7 @@ static int handle_exception(struct kvm_vcpu *vcpu)
intr_info = vmx->exit_intr_info;
if (is_machine_check(intr_info) || is_nmi(intr_info))
- return 1; /* already handled by vmx_complete_atomic_exit */
+ return 1; /* handled by handle_exception_nmi_atomic() */
if (is_invalid_opcode(intr_info))
return handle_ud(vcpu);
@@ -5462,7 +5462,7 @@ static int handle_encls(struct kvm_vcpu *vcpu)
* to be done to userspace and return 0.
*/
static int (*kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
- [EXIT_REASON_EXCEPTION_NMI] = handle_exception,
+ [EXIT_REASON_EXCEPTION_NMI] = handle_exception_nmi,
[EXIT_REASON_EXTERNAL_INTERRUPT] = handle_external_interrupt,
[EXIT_REASON_TRIPLE_FAULT] = handle_triple_fault,
[EXIT_REASON_NMI_WINDOW] = handle_nmi_window,
@@ -6100,11 +6100,8 @@ static void vmx_apicv_post_state_restore(struct kvm_vcpu *vcpu)
memset(vmx->pi_desc.pir, 0, sizeof(vmx->pi_desc.pir));
}
-static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx)
+static void handle_exception_nmi_atomic(struct vcpu_vmx *vmx)
{
- if (vmx->exit_reason != EXIT_REASON_EXCEPTION_NMI)
- return;
-
vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
/* if exit due to PF check for async PF */
@@ -6123,7 +6120,7 @@ static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx)
}
}
-static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
+static void handle_external_interrupt_atomic(struct kvm_vcpu *vcpu)
{
unsigned int vector;
unsigned long entry;
@@ -6133,9 +6130,6 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
gate_desc *desc;
u32 intr_info;
- if (to_vmx(vcpu)->exit_reason != EXIT_REASON_EXTERNAL_INTERRUPT)
- return;
-
intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
if (WARN_ONCE(!is_external_intr(intr_info),
"KVM: unexpected VM-Exit interrupt info: 0x%x", intr_info))
@@ -6170,7 +6164,17 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
kvm_after_interrupt(vcpu);
}
-STACK_FRAME_NON_STANDARD(vmx_handle_external_intr);
+STACK_FRAME_NON_STANDARD(handle_external_interrupt_atomic);
+
+static void vmx_handle_exit_atomic(struct kvm_vcpu *vcpu)
+{
+ struct vcpu_vmx *vmx = to_vmx(vcpu);
+
+ if (vmx->exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
+ handle_external_interrupt_atomic(vcpu);
+ else if (vmx->exit_reason == EXIT_REASON_EXCEPTION_NMI)
+ handle_exception_nmi_atomic(vmx);
+}
static bool vmx_has_emulated_msr(int index)
{
@@ -6540,7 +6544,6 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
vmx->loaded_vmcs->launched = 1;
vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD);
- vmx_complete_atomic_exit(vmx);
vmx_recover_nmi_blocking(vmx);
vmx_complete_interrupts(vmx);
}
@@ -7694,7 +7697,7 @@ static __exit void hardware_unsetup(void)
.set_tdp_cr3 = vmx_set_cr3,
.check_intercept = vmx_check_intercept,
- .handle_external_intr = vmx_handle_external_intr,
+ .handle_exit_atomic = vmx_handle_exit_atomic,
.mpx_supported = vmx_mpx_supported,
.xsaves_supported = vmx_xsaves_supported,
.umip_emulated = vmx_umip_emulated,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6e2f53cd8ea8..88489af13e96 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7999,7 +7999,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
vcpu->mode = OUTSIDE_GUEST_MODE;
smp_wmb();
- kvm_x86_ops->handle_external_intr(vcpu);
+ kvm_x86_ops->handle_exit_atomic(vcpu);
++vcpu->stat.exits;
next prev parent reply other threads:[~2019-06-06 13:20 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-20 5:50 [PATCH 0/5] KVM: VMX: INTR, NMI and #MC cleanup Sean Christopherson
2019-04-20 5:50 ` [PATCH 1/5] KVM: VMX: Fix handling of #MC that occurs during VM-Entry Sean Christopherson
2019-06-06 12:57 ` Paolo Bonzini
2019-04-20 5:50 ` [PATCH 2/5] KVM: VMX: Read cached VM-Exit reason to detect external interrupt Sean Christopherson
2019-06-06 13:02 ` Paolo Bonzini
2019-06-06 14:09 ` Sean Christopherson
2019-04-20 5:50 ` [PATCH 3/5] KVM: VMX: Store the host kernel's IDT base in a global variable Sean Christopherson
2019-04-20 14:17 ` [RFC PATCH] KVM: VMX: host_idt_base can be static kbuild test robot
2019-04-20 5:50 ` [PATCH 4/5] KVM: x86: Move kvm_{before,after}_interrupt() calls to vendor code Sean Christopherson
2019-04-20 5:50 ` [PATCH 5/5] KVM: VMX: Handle NMIs, #MCs and async #PFs in common irqs-disabled fn Sean Christopherson
2019-06-06 13:20 ` Paolo Bonzini [this message]
2019-06-06 15:14 ` Sean Christopherson
2019-06-07 11:40 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=746c7e2c-176f-d772-e37d-41bb9f524dd6@redhat.com \
--to=pbonzini@redhat.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=rkrcmar@redhat.com \
--cc=sean.j.christopherson@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.