From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled
 during L2 execution
Date: Fri, 8 Jul 2016 17:39:22 -0400 (EDT)
Message-ID: <450057441.5389401.1468013962741.JavaMail.zimbra@redhat.com>
References: <1467979333-19535-1-git-send-email-pbonzini@redhat.com> <1467979333-19535-4-git-send-email-pbonzini@redhat.com> <20160708102903.48b0421a@jnakajim-build> <0070888f-06b8-db53-3400-5ce8b35c9021@redhat.com> <20160708143011.4837cf86@jnakajim-build>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	Wanpeng Li <kernellwp@gmail.com>,
	Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>,
	Yunhong Jiang <yunhong.jiang@intel.com>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	Haozhong Zhang <haozhong.zhang@intel.com>
To: yunhong jiang <yunhong.jiang@linux.intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx3-phx2.redhat.com ([209.132.183.24]:60212 "EHLO
	mx3-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S932159AbcGHVja (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 8 Jul 2016 17:39:30 -0400
In-Reply-To: <20160708143011.4837cf86@jnakajim-build>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

> > > > @@ -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
> 
> Thanks for the explaination. I try to go through the code and still
> have one question. I'd describe below and hope get your input.
> 
> When the L2 guest running while the VMX Preemption timer triggered, the
> vcpu_enter_guest() will trigger vmx_handle_exit(), with the CPU vmcs as
> vmcs02. On the vmx_handle_exit(), the nested_vmx_exit_handled() return
> false as the 1st patch did, thus the vmcs is not switched. The
> kvm_lapic_expired_hv_timer() will be called with vmcs02, instead of
> vmcs01. Is it something we wanted? I assume we should use vmcs01 there
> since we will clear the preemption timer VMCS bit there.

Actually we want both.  For whatever reason, the interrupt might not
cause a vmexit---for example if the L0 PPR is masking the LVTT vector.
In this case, we need to cancel the preemption timer in the vmcs02
(done by kvm_lapic_expired_hv_timer) and keep running L2.  On the next
vmexit, nested_vmx_vmexit will load the vmcs01 and clear the preemption
timer bit.

Of course this is only theory until Wanpeng confirms that my patch works
for him. :)

Paolo