linux-kernel - Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160708161819.20de34ad@jnakajim-build>
Date:	Fri, 8 Jul 2016 16:18:19 -0700
From:	yunhong jiang <yunhong.jiang@...ux.intel.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	Wanpeng Li <kernellwp@...il.com>,
	Radim Krčmář <rkrcmar@...hat.com>,
	Yunhong Jiang <yunhong.jiang@...el.com>,
	Jan Kiszka <jan.kiszka@...mens.com>,
	Haozhong Zhang <haozhong.zhang@...el.com>
Subject: Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled
 during L2 execution

On Fri, 8 Jul 2016 17:39:22 -0400 (EDT)
Paolo Bonzini <pbonzini@...hat.com> wrote:

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

Got it and thanks for clarification.

--jyh

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