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 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.