From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH v4] KVM: nVMX: Fully support of nested VMX preemption timer Date: Mon, 16 Sep 2013 12:50:41 +0200 Message-ID: <5236E281.9030907@redhat.com> References: <1378433091-18233-1-git-send-email-yzt356@gmail.com> <5233481F.8060606@redhat.com> <20130916074422.GG17294@redhat.com> <5236C824.8090700@redhat.com> <20130916090911.GL17294@redhat.com> <5236D434.8000605@redhat.com> <20130916104446.GN17294@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Arthur Chunqi Li , kvm@vger.kernel.org, jan.kiszka@web.de, yang.z.zhang@intel.com To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:53744 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752032Ab3IPKue (ORCPT ); Mon, 16 Sep 2013 06:50:34 -0400 In-Reply-To: <20130916104446.GN17294@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Il 16/09/2013 12:44, Gleb Natapov ha scritto: > On Mon, Sep 16, 2013 at 11:49:40AM +0200, Paolo Bonzini wrote: >> Il 16/09/2013 11:09, Gleb Natapov ha scritto: >>> On Mon, Sep 16, 2013 at 10:58:12AM +0200, Paolo Bonzini wrote: >>>>>>> vmx->__launched = vmx->loaded_vmcs->launched; >>>>>>> + if (is_guest_mode(vcpu) && !(vmx->nested.nested_vmx_exit)) >>>>>>> + nested_adjust_preemption_timer(vcpu); >>>>>> >>>>>> Please leave the assignment to __launched last, since it's already >>>>>> initializing the asm below. >>>>>> >>>>>> I don't like the is_guest_mode check here... Maybe it's >>>>>> micro-optimizing, but I wonder if we already do too many checks in >>>>>> vmx_vcpu_run... For example, is_guest_mode could be changed (I think) >>>>>> to a check for "vmx->loaded_vmcs == &vmx->vmcs1". >>>>>> >>>>> Why this will be more efficient that HF_GUEST_MASK check? >>>> >>>> Because we have already loaded vmx->loaded_vmcs, so it's one memory >>>> access less. >>>> >>> But we will have to load vmx->vmcs1 instead :) >> >> That's not a memory load, it's an add. >> > You assume that vmx->loaded_vmcs and vmx will be both in registers here, > isn't it too much to assume? Both of them are needed in this very close line: vmx->__launched = vmx->loaded_vmcs->launched; So it is not a big assumption---even on 32-bits, perhaps. By contrast, vcpu->hflags is very unlikely to be in a register. This is of course mostly pointless alone. But if we could shave 100 cycles off a lightweight vmexit, that could give a measurable improvement on nested virt. Paolo