From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Andrea Arcangeli <aarcange@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
Marcelo Tosatti <mtosatti@redhat.com>,
Peter Xu <peterx@redhat.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 15/17] KVM: retpolines: x86: eliminate retpoline from vmx.c exit handlers
Date: Mon, 23 Sep 2019 13:23:49 -0700 [thread overview]
Message-ID: <20190923202349.GL18195@linux.intel.com> (raw)
In-Reply-To: <20190923190514.GB19996@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3419 bytes --]
On Mon, Sep 23, 2019 at 03:05:14PM -0400, Andrea Arcangeli wrote:
> On Mon, Sep 23, 2019 at 11:57:57AM +0200, Paolo Bonzini wrote:
> > On 23/09/19 11:31, Vitaly Kuznetsov wrote:
> > > +#ifdef CONFIG_RETPOLINE
> > > + if (exit_reason == EXIT_REASON_MSR_WRITE)
> > > + return handle_wrmsr(vcpu);
> > > + else if (exit_reason == EXIT_REASON_PREEMPTION_TIMER)
> > > + return handle_preemption_timer(vcpu);
> > > + else if (exit_reason == EXIT_REASON_PENDING_INTERRUPT)
> > > + return handle_interrupt_window(vcpu);
> > > + else if (exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
> > > + return handle_external_interrupt(vcpu);
> > > + else if (exit_reason == EXIT_REASON_HLT)
> > > + return handle_halt(vcpu);
> > > + else if (exit_reason == EXIT_REASON_PAUSE_INSTRUCTION)
> > > + return handle_pause(vcpu);
> > > + else if (exit_reason == EXIT_REASON_MSR_READ)
> > > + return handle_rdmsr(vcpu);
> > > + else if (exit_reason == EXIT_REASON_CPUID)
> > > + return handle_cpuid(vcpu);
> > > + else if (exit_reason == EXIT_REASON_EPT_MISCONFIG)
> > > + return handle_ept_misconfig(vcpu);
> > > +#endif
> > > return kvm_vmx_exit_handlers[exit_reason](vcpu);
> >
> > Most of these, while frequent, are already part of slow paths.
> >
> > I would keep only EXIT_REASON_MSR_WRITE, EXIT_REASON_PREEMPTION_TIMER,
> > EXIT_REASON_EPT_MISCONFIG and add EXIT_REASON_IO_INSTRUCTION.
>
> Intuition doesn't work great when it comes to CPU speculative
> execution runtime. I can however run additional benchmarks to verify
> your theory that keeping around frequent retpolines will still perform
> ok.
>
> > If you make kvm_vmx_exit_handlers const, can the compiler substitute for
> > instance kvm_vmx_exit_handlers[EXIT_REASON_MSR_WRITE] with handle_wrmsr?
> > Just thinking out loud, not sure if it's an improvement code-wise.
>
> gcc gets right if you make it const, it calls kvm_emulate_wrmsr in
> fact. However I don't think const will fly
> with_vmx_hardware_setup()... in fact at runtime testing nested I just
> got:
>
> BUG: unable to handle page fault for address: ffffffffa00751e0
> #PF: supervisor write access in kernel mode
> #PF: error_code(0x0003) - permissions violation
> PGD 2424067 P4D 2424067 PUD 2425063 PMD 7cc09067 PTE 80000000741cb161
> Oops: 0003 [#1] SMP NOPTI
> CPU: 1 PID: 4458 Comm: insmod Not tainted 5.3.0+ #1
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS ?-20190711_202441-buildvm-armv7-10.arm.fedoraproject.or4
> RIP: 0010:nested_vmx_hardware_setup+0x29a/0x37a [kvm_intel]
> Code: 41 ff c5 66 89 2c 85 20 92 0b a0 66 44 89 34 85 22 92 0b a0 49 ff c7 e9 e6 fe ff ff 44 89 2d 28 24 fc ff 48
> RSP: 0018:ffffc90000257c18 EFLAGS: 00010246
> RAX: ffffffffa001e0b0 RBX: ffffffffa0075140 RCX: 0000000000000000
> RDX: ffff888078f60000 RSI: 0000000000002401 RDI: 0000000000000018
> RBP: 0000000000006c08 R08: 0000000000001000 R09: 000000000007ffdc
> R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000006c08
> R13: 0000000000000017 R14: 0000000000000268 R15: 0000000000000018
> FS: 00007f7fb7ef0b80(0000) GS:ffff88807da40000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffa00751e0 CR3: 0000000079620001 CR4: 0000000000160ee0
> Call Trace:
> hardware_setup+0x4df/0x5b2 [kvm_intel]
> kvm_arch_hardware_setup+0x2f/0x27b [kvm_intel]
> kvm_init+0x5d/0x26d [kvm_intel]
The attached patch should do the trick.
[-- Attachment #2: 0001-KVM-nVMX-Do-not-dynamically-set-VMX-instruction-exit.patch --]
[-- Type: text/x-diff, Size: 5070 bytes --]
From 4e0c2d73d796eae03aa289f77bef5f4a7acef655 Mon Sep 17 00:00:00 2001
From: Sean Christopherson <sean.j.christopherson@intel.com>
Date: Mon, 23 Sep 2019 13:19:43 -0700
Subject: [PATCH] KVM: nVMX: Do not dynamically set VMX instruction exit
handlers
Handle VMX instructions via a dedicated function and a switch statement
provided by the nVMX code instead of overwriting kvm_vmx_exit_handlers
when nested support is enabled. This will allow a future patch to make
kvm_vmx_exit_handlers a const, which in turn allows for better compiler
optimizations, e.g. direct calls instead of retpolined indirect calls.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
arch/x86/kvm/vmx/nested.c | 52 ++++++++++++++++++++++++++++-----------
arch/x86/kvm/vmx/nested.h | 3 ++-
arch/x86/kvm/vmx/vmx.c | 5 +++-
3 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 6ce83c602e7f..41c7fcf28ab6 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5072,6 +5072,43 @@ static int handle_vmfunc(struct kvm_vcpu *vcpu)
return 1;
}
+int nested_vmx_handle_vmx_instruction(struct kvm_vcpu *vcpu)
+{
+ switch (to_vmx(vcpu)->exit_reason) {
+ case EXIT_REASON_VMCLEAR:
+ return handle_vmclear(vcpu);
+ case EXIT_REASON_VMLAUNCH:
+ return handle_vmlaunch(vcpu);
+ case EXIT_REASON_VMPTRLD:
+ return handle_vmptrld(vcpu);
+ case EXIT_REASON_VMPTRST:
+ return handle_vmptrst(vcpu);
+ case EXIT_REASON_VMREAD:
+ return handle_vmread(vcpu);
+ case EXIT_REASON_VMRESUME:
+ return handle_vmresume(vcpu);
+ case EXIT_REASON_VMWRITE:
+ return handle_vmwrite(vcpu);
+ case EXIT_REASON_VMOFF:
+ return handle_vmoff(vcpu);
+ case EXIT_REASON_VMON:
+ return handle_vmon(vcpu);
+ case EXIT_REASON_INVEPT:
+ return handle_invept(vcpu);
+ case EXIT_REASON_INVVPID:
+ return handle_invvpid(vcpu);
+ case EXIT_REASON_VMFUNC:
+ return handle_vmfunc(vcpu);
+ }
+
+ WARN_ON_ONCE(1);
+ vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+ vcpu->run->internal.suberror =
+ KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON;
+ vcpu->run->internal.ndata = 1;
+ vcpu->run->internal.data[0] = to_vmx(vcpu)->exit_reason;
+ return 0;
+}
static bool nested_vmx_exit_handled_io(struct kvm_vcpu *vcpu,
struct vmcs12 *vmcs12)
@@ -5972,7 +6009,7 @@ void nested_vmx_hardware_unsetup(void)
}
}
-__init int nested_vmx_hardware_setup(int (*exit_handlers[])(struct kvm_vcpu *))
+__init int nested_vmx_hardware_setup(void)
{
int i;
@@ -5995,19 +6032,6 @@ __init int nested_vmx_hardware_setup(int (*exit_handlers[])(struct kvm_vcpu *))
init_vmcs_shadow_fields();
}
- exit_handlers[EXIT_REASON_VMCLEAR] = handle_vmclear,
- exit_handlers[EXIT_REASON_VMLAUNCH] = handle_vmlaunch,
- exit_handlers[EXIT_REASON_VMPTRLD] = handle_vmptrld,
- exit_handlers[EXIT_REASON_VMPTRST] = handle_vmptrst,
- exit_handlers[EXIT_REASON_VMREAD] = handle_vmread,
- exit_handlers[EXIT_REASON_VMRESUME] = handle_vmresume,
- exit_handlers[EXIT_REASON_VMWRITE] = handle_vmwrite,
- exit_handlers[EXIT_REASON_VMOFF] = handle_vmoff,
- exit_handlers[EXIT_REASON_VMON] = handle_vmon,
- exit_handlers[EXIT_REASON_INVEPT] = handle_invept,
- exit_handlers[EXIT_REASON_INVVPID] = handle_invvpid,
- exit_handlers[EXIT_REASON_VMFUNC] = handle_vmfunc,
-
kvm_x86_ops->check_nested_events = vmx_check_nested_events;
kvm_x86_ops->get_nested_state = vmx_get_nested_state;
kvm_x86_ops->set_nested_state = vmx_set_nested_state;
diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h
index 187d39bf0bf1..0da48c83cccf 100644
--- a/arch/x86/kvm/vmx/nested.h
+++ b/arch/x86/kvm/vmx/nested.h
@@ -10,9 +10,10 @@ void vmx_leave_nested(struct kvm_vcpu *vcpu);
void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps,
bool apicv);
void nested_vmx_hardware_unsetup(void);
-__init int nested_vmx_hardware_setup(int (*exit_handlers[])(struct kvm_vcpu *));
+__init int nested_vmx_hardware_setup(void);
void nested_vmx_vcpu_setup(void);
void nested_vmx_free_vcpu(struct kvm_vcpu *vcpu);
+int nested_vmx_handle_vmx_instruction(struct kvm_vcpu *vcpu);
int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry);
bool nested_vmx_exit_reflected(struct kvm_vcpu *vcpu, u32 exit_reason);
void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 73bf9a2e6fb6..229b3a5e0695 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5459,6 +5459,9 @@ static int handle_preemption_timer(struct kvm_vcpu *vcpu)
*/
static int handle_vmx_instruction(struct kvm_vcpu *vcpu)
{
+ if (nested)
+ return nested_vmx_handle_vmx_instruction(vcpu);
+
kvm_queue_exception(vcpu, UD_VECTOR);
return 1;
}
@@ -7631,7 +7634,7 @@ static __init int hardware_setup(void)
nested_vmx_setup_ctls_msrs(&vmcs_config.nested,
vmx_capability.ept, enable_apicv);
- r = nested_vmx_hardware_setup(kvm_vmx_exit_handlers);
+ r = nested_vmx_hardware_setup();
if (r)
return r;
}
--
2.22.0
next prev parent reply other threads:[~2019-09-23 20:23 UTC|newest]
Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-20 21:24 [PATCH 00/17] KVM monolithic v1 Andrea Arcangeli
2019-09-20 21:24 ` [PATCH 01/17] x86: spec_ctrl: fix SPEC_CTRL initialization after kexec Andrea Arcangeli
2019-09-23 10:22 ` Paolo Bonzini
2019-09-23 15:30 ` Sean Christopherson
2019-09-23 17:34 ` Andrea Arcangeli
2019-09-23 22:27 ` Sean Christopherson
2019-09-20 21:24 ` [PATCH 02/17] KVM: monolithic: x86: convert the kvm_x86_ops methods to external functions Andrea Arcangeli
2019-09-23 10:19 ` Paolo Bonzini
2019-09-23 16:13 ` Sean Christopherson
2019-09-23 16:51 ` Paolo Bonzini
2019-09-23 19:21 ` Andrea Arcangeli
2019-09-20 21:24 ` [PATCH 03/17] KVM: monolithic: x86: handle the request_immediate_exit variation Andrea Arcangeli
2019-09-23 22:35 ` Sean Christopherson
2019-09-23 23:06 ` Andrea Arcangeli
2019-09-23 23:45 ` Sean Christopherson
2019-09-24 0:24 ` Andrea Arcangeli
2019-09-20 21:24 ` [PATCH 04/17] KVM: monolithic: x86: convert the kvm_pmu_ops methods to external functions Andrea Arcangeli
2019-09-20 21:24 ` [PATCH 05/17] KVM: monolithic: x86: enable the kvm_x86_ops " Andrea Arcangeli
2019-09-20 21:24 ` [PATCH 06/17] KVM: monolithic: x86: enable the kvm_pmu_ops " Andrea Arcangeli
2019-09-20 21:24 ` [PATCH 07/17] KVM: monolithic: x86: adjust the section prefixes Andrea Arcangeli
2019-09-23 10:15 ` Paolo Bonzini
2019-09-25 12:13 ` Andrea Arcangeli
2019-09-25 12:32 ` Paolo Bonzini
2019-09-20 21:25 ` [PATCH 08/17] KVM: monolithic: adjust the section prefixes in the KVM common code Andrea Arcangeli
2019-09-20 21:25 ` [PATCH 09/17] KVM: monolithic: x86: remove kvm.ko Andrea Arcangeli
2019-09-20 21:25 ` [PATCH 10/17] KVM: monolithic: x86: use the external functions instead of kvm_x86_ops Andrea Arcangeli
2019-09-23 10:02 ` Paolo Bonzini
2019-09-20 21:25 ` [PATCH 11/17] KVM: monolithic: x86: remove exports Andrea Arcangeli
2019-09-20 21:25 ` [PATCH 12/17] KVM: monolithic: remove exports from KVM common code Andrea Arcangeli
2019-09-20 21:25 ` [PATCH 13/17] KVM: monolithic: x86: drop the kvm_pmu_ops structure Andrea Arcangeli
2019-09-23 10:21 ` Paolo Bonzini
2019-09-24 0:51 ` Andrea Arcangeli
2019-09-24 1:24 ` Paolo Bonzini
2019-09-20 21:25 ` [PATCH 14/17] KVM: monolithic: x86: inline more exit handlers in vmx.c Andrea Arcangeli
2019-09-23 10:19 ` Paolo Bonzini
2019-09-24 1:00 ` Andrea Arcangeli
2019-09-24 1:25 ` Paolo Bonzini
2019-09-24 1:55 ` Andrea Arcangeli
2019-09-24 2:56 ` Andrea Arcangeli
2019-09-25 7:52 ` Paolo Bonzini
2019-09-20 21:25 ` [PATCH 15/17] KVM: retpolines: x86: eliminate retpoline from vmx.c exit handlers Andrea Arcangeli
2019-09-23 9:31 ` Vitaly Kuznetsov
2019-09-23 9:57 ` Paolo Bonzini
2019-09-23 19:05 ` Andrea Arcangeli
2019-09-23 20:23 ` Sean Christopherson [this message]
2019-09-23 21:08 ` Andrea Arcangeli
2019-09-23 21:24 ` Sean Christopherson
2019-09-23 23:43 ` Andrea Arcangeli
2019-09-23 23:52 ` Sean Christopherson
2019-09-24 0:16 ` Paolo Bonzini
2019-09-24 0:35 ` Sean Christopherson
2019-09-24 0:37 ` Paolo Bonzini
2019-09-24 0:15 ` Paolo Bonzini
2019-09-24 0:38 ` Andrea Arcangeli
2019-09-24 0:46 ` Sean Christopherson
2019-09-24 21:46 ` Andrea Arcangeli
2019-09-25 7:50 ` Paolo Bonzini
2019-09-23 16:37 ` Sean Christopherson
2019-09-23 16:53 ` Paolo Bonzini
2019-09-23 17:42 ` Andrea Arcangeli
2019-09-23 18:15 ` Sean Christopherson
2019-09-23 19:12 ` Andrea Arcangeli
[not found] ` <E8FE7592-69C3-455E-8D80-A2D73BB2E14C@dinechin.org>
2019-09-25 20:51 ` Andrea Arcangeli
2019-09-23 16:28 ` Sean Christopherson
2019-09-20 21:25 ` [PATCH 16/17] KVM: retpolines: x86: eliminate retpoline from svm.c " Andrea Arcangeli
2019-09-23 10:01 ` Paolo Bonzini
2019-09-20 21:25 ` [PATCH 17/17] x86: retpolines: eliminate retpoline from msr event handlers Andrea Arcangeli
2019-09-23 15:39 ` [PATCH 00/17] KVM monolithic v1 Sean Christopherson
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=20190923202349.GL18195@linux.intel.com \
--to=sean.j.christopherson@intel.com \
--cc=aarcange@redhat.com \
--cc=dgilbert@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=vkuznets@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).