From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932832AbcGHRmB (ORCPT ); Fri, 8 Jul 2016 13:42:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49614 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932652AbcGHRlx (ORCPT ); Fri, 8 Jul 2016 13:41:53 -0400 Subject: Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution To: yunhong jiang References: <1467979333-19535-1-git-send-email-pbonzini@redhat.com> <1467979333-19535-4-git-send-email-pbonzini@redhat.com> <20160708102903.48b0421a@jnakajim-build> Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Wanpeng Li , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Yunhong Jiang , Jan Kiszka , Haozhong Zhang From: Paolo Bonzini Message-ID: <0070888f-06b8-db53-3400-5ce8b35c9021@redhat.com> Date: Fri, 8 Jul 2016 19:41:48 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <20160708102903.48b0421a@jnakajim-build> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 08 Jul 2016 17:41:52 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/07/2016 19:29, yunhong jiang wrote: > > > > exec_control = vmcs12->pin_based_vm_exec_control; > > - exec_control |= vmcs_config.pin_based_exec_ctrl; > > + > > + /* Preemption timer setting is only taken from vmcs01. */ > > exec_control &= ~PIN_BASED_VMX_PREEMPTION_TIMER; > > Do we still keep this clear here with followed changes? Yes. If L1 wants to use the preemption timer the bit will be set in vmcs12->pin_based_vm_exec_control In this case, however, KVM uses an hrtimer to emulate L1's preemption timer, so we must not copy the bit into the vmcs02 (i.e. the VMCS that L0 uses to run L2). Thus the preemption timer control of the vmcs02 must come exclusively from vmx->hv_deadline_tsc. > > + exec_control |= vmcs_config.pin_based_exec_ctrl; > > + if (vmx->hv_deadline_tsc == -1) > > + exec_control &= ~PIN_BASED_VMX_PREEMPTION_TIMER; > > > > + /* Posted interrupts setting is only taken from vmcs12. */ > > if (nested_cpu_has_posted_intr(vmcs12)) { > > /* > > * Note that we use L0's vector here and in > > @@ -10727,8 +10732,14 @@ static void nested_vmx_vmexit(struct > > kvm_vcpu *vcpu, u32 exit_reason, > > load_vmcs12_host_state(vcpu, vmcs12); > > > > - /* Update TSC_OFFSET if TSC was changed while L2 ran */ > > + /* Update any VMCS fields that might have changed while L2 > > ran */ vmcs_write64(TSC_OFFSET, vmx->nested.vmcs01_tsc_offset); > > + if (vmx->hv_deadline_tsc == -1) > > + vmcs_clear_bits(PIN_BASED_VM_EXEC_CONTROL, > > + PIN_BASED_VMX_PREEMPTION_TIMER); > > + else > > + vmcs_set_bits(PIN_BASED_VM_EXEC_CONTROL, > > + PIN_BASED_VMX_PREEMPTION_TIMER); > > Why do we need change the vmcs01 here? Per my understanding, the vmcs01 is not > changed when the L2 guest is running thus the PIN_BASED_VM_EXEC_CONTROL should > not be changed? This is the point where we are updating the vmcs01 after exiting. If vmx->hv_deadline_tsc has changed (for example because of a preemption timer vmexit, or because L2 did a HLT and L1 is not intercepting HLT) we need to update the preemption timer control to synchronize it with vmx->hv_deadline_tsc. > I'm not familiar with nested VMX, sorry if this is a naive question. It's not naive, don't worry! :) Paolo