Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1439367 > unrolled thread

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

Started byPaolo Bonzini <pbonzini@redhat.com>
First post2016-07-08 14:10 +0200
Last post2016-07-08 23:40 +0200
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution Paolo Bonzini <pbonzini@redhat.com> - 2016-07-08 14:10 +0200
    Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled  during L2 execution yunhong jiang <yunhong.jiang@linux.intel.com> - 2016-07-08 19:40 +0200
      Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled  during L2 execution Paolo Bonzini <pbonzini@redhat.com> - 2016-07-08 19:50 +0200
        Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled  during L2 execution Paolo Bonzini <pbonzini@redhat.com> - 2016-07-08 23:40 +0200
          Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled  during L2 execution yunhong jiang <yunhong.jiang@linux.intel.com> - 2016-07-09 01:30 +0200
        Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled  during L2 execution yunhong jiang <yunhong.jiang@linux.intel.com> - 2016-07-08 23:40 +0200

#1439367 — [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution

FromPaolo Bonzini <pbonzini@redhat.com>
Date2016-07-08 14:10 +0200
Subject[RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution
Message-ID<rSCLw-4Br-39@gated-at.bofh.it>
Because the vmcs12 preemption timer is emulated through a separate hrtimer,
we can keep on using the preemption timer in the vmcs02 to emulare L1's
TSC deadline timer.

However, the corresponding bit in the pin-based execution control field
must be kept consistent between vmcs01 and vmcs02.  On vmentry we copy
it into the vmcs02; on vmexit the preemption timer must be disabled in
the vmcs01 if a preemption timer vmexit happened while in guest mode.

The preemption timer value in the vmcs02 is set by vmx_vcpu_run, so it
need not be considered in prepare_vmcs02.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 0048be79c7b9..8cda4449a60e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9796,9 +9796,14 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	vmcs_write64(VMCS_LINK_POINTER, -1ull);
 
 	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;
+	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);
 
 	/* This is needed for same reason as it was needed in prepare_vmcs02 */
 	vmx->host_rsp = 0;
-- 
1.8.3.1

[toc] | [next] | [standalone]

#1439711 — Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution

Fromyunhong jiang <yunhong.jiang@linux.intel.com>
Date2016-07-08 19:40 +0200
SubjectRe: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution
Message-ID<rSHUS-7Ss-9@gated-at.bofh.it>
In reply to#1439367
On Fri,  8 Jul 2016 14:02:13 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Because the vmcs12 preemption timer is emulated through a separate
> hrtimer, we can keep on using the preemption timer in the vmcs02 to
> emulare L1's TSC deadline timer.
> 
> However, the corresponding bit in the pin-based execution control
> field must be kept consistent between vmcs01 and vmcs02.  On vmentry
> we copy it into the vmcs02; on vmexit the preemption timer must be
> disabled in the vmcs01 if a preemption timer vmexit happened while in
> guest mode.
> 
> The preemption timer value in the vmcs02 is set by vmx_vcpu_run, so it
> need not be considered in prepare_vmcs02.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/vmx.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 0048be79c7b9..8cda4449a60e 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -9796,9 +9796,14 @@ static void prepare_vmcs02(struct kvm_vcpu
> *vcpu, struct vmcs12 *vmcs12) vmcs_write64(VMCS_LINK_POINTER, -1ull);
>  
>  	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?

> +	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?  I'm not familiar with nested VMX, sorry if this is a naive
question.

Thanks
--jyh

[toc] | [prev] | [next] | [standalone]

#1439721 — Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution

FromPaolo Bonzini <pbonzini@redhat.com>
Date2016-07-08 19:50 +0200
SubjectRe: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution
Message-ID<rSI4x-7VV-1@gated-at.bofh.it>
In reply to#1439711

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

[toc] | [prev] | [next] | [standalone]

#1439804 — Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution

FromPaolo Bonzini <pbonzini@redhat.com>
Date2016-07-08 23:40 +0200
SubjectRe: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution
Message-ID<rSLF7-1Z3-15@gated-at.bofh.it>
In reply to#1439721
> > > > @@ -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

[toc] | [prev] | [next] | [standalone]

#1439846 — Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution

Fromyunhong jiang <yunhong.jiang@linux.intel.com>
Date2016-07-09 01:30 +0200
SubjectRe: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution
Message-ID<rSNnA-39n-1@gated-at.bofh.it>
In reply to#1439804
On Fri, 8 Jul 2016 17:39:22 -0400 (EDT)
Paolo Bonzini <pbonzini@redhat.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

[toc] | [prev] | [next] | [standalone]

#1439806 — Re: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution

Fromyunhong jiang <yunhong.jiang@linux.intel.com>
Date2016-07-08 23:40 +0200
SubjectRe: [RFT PATCH v5 3/3] KVM: nVMX: keep preemption timer enabled during L2 execution
Message-ID<rSLF7-1Z3-17@gated-at.bofh.it>
In reply to#1439721
On Fri, 8 Jul 2016 19:41:48 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

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

Thanks for the clarification.

> 
> > > +	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


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.

Thanks
--jyh

> 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web