On Wed, Sep 25, 2019 at 01:03:32PM +0200, Christophe de Dinechin wrote: > > > > On 23 Sep 2019, at 11:31, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote: > > > > Andrea Arcangeli <aarcange@xxxxxxxxxx <mailto:aarcange@xxxxxxxxxx>> writes: > > > >> It's enough to check the exit value and issue a direct call to avoid > >> the retpoline for all the common vmexit reasons. > >> > >> Signed-off-by: Andrea Arcangeli <aarcange@xxxxxxxxxx> > >> --- > >> arch/x86/kvm/vmx/vmx.c | 24 ++++++++++++++++++++++-- > >> 1 file changed, 22 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > >> index a6e597025011..9aa73e216df2 100644 > >> --- a/arch/x86/kvm/vmx/vmx.c > >> +++ b/arch/x86/kvm/vmx/vmx.c > >> @@ -5866,9 +5866,29 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu) > >> } > >> > >> if (exit_reason < kvm_vmx_max_exit_handlers > >> - && kvm_vmx_exit_handlers[exit_reason]) > >> + && kvm_vmx_exit_handlers[exit_reason]) { > >> +#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); > > > > I agree with the identified set of most common vmexits, however, this > > still looks a bit random. Would it be too much if we get rid of > > kvm_vmx_exit_handlers completely replacing this code with one switch()? > > Not sure, but if you do that, won’t the compiler generate a table and > bring you back to square one? Or is there a reason why the mitigation > is not needed for tables and indirect branches generated from switch > statements? When the kernel is built with retpolines the compiler is forbidden to use a table for any switch. I pointed out the relevant commit earlier in this thread. Instead the compiler will still try to bisect the exit_reason trying to make the cost more equal for all exit_reason and to reduce the number of checks, but we know the most likely exits so it should be better to prioritize the most frequent exit reasons. Thanks, Andrea
