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: <0070888f-06b8-db53-3400-5ce8b35c9021@redhat.com>
Date:	Fri, 8 Jul 2016 19:41:48 +0200
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	yunhong jiang <yunhong.jiang@...ux.intel.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 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