All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Makarand Sonare <makarandsonare@google.com>
Cc: kvm@vger.kernel.org, pshier@google.com, jmattson@google.com
Subject: Re: [kvm PATCH 2/2] KVM: nVMX: Don't clobber preemption timer in the VMCS12 before L2 ran
Date: Tue, 21 Apr 2020 18:57:59 -0700	[thread overview]
Message-ID: <20200422015759.GE17836@linux.intel.com> (raw)
In-Reply-To: <20200417183452.115762-3-makarandsonare@google.com>

On Fri, Apr 17, 2020 at 11:34:52AM -0700, Makarand Sonare wrote:
> Don't clobber the VMX-preemption timer value in the VMCS12 during
> migration on the source while handling an L1 VMLAUNCH/VMRESUME but
> before L2 ran. In that case the VMCS12 preemption timer value
> should not be touched as it will be restarted on the target
> from its original value. This emulates migration occurring while L1
> awaits completion of its VMLAUNCH/VMRESUME instruction.
> 
> Signed-off-by: Makarand Sonare <makarandsonare@google.com>
> Signed-off-by: Peter Shier <pshier@google.com>

The SOB tags are reversed, i.e. Peter's should be first to show that he
wrote the patch and then transfered it to you for upstreaming.

> Change-Id: I376d151585d4f1449319f7512151f11bbf08c5bf
> ---
>  arch/x86/kvm/vmx/nested.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 5365d7e5921ea..66155e9114114 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -3897,11 +3897,13 @@ static void sync_vmcs02_to_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
>  		vmcs12->guest_activity_state = GUEST_ACTIVITY_ACTIVE;
>  
>  	if (nested_cpu_has_preemption_timer(vmcs12)) {
> -		vmx->nested.preemption_timer_remaining =
> -			vmx_get_preemption_timer_value(vcpu);
> -		if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_VMX_PREEMPTION_TIMER)
> -			vmcs12->vmx_preemption_timer_value =
> -				vmx->nested.preemption_timer_remaining;
> +		if (!vmx->nested.nested_run_pending) {
> +			vmx->nested.preemption_timer_remaining =
> +				vmx_get_preemption_timer_value(vcpu);
> +			if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_VMX_PREEMPTION_TIMER)
> +				vmcs12->vmx_preemption_timer_value =
> +					vmx->nested.preemption_timer_remaining;
> +			}

This indentation is messed up, the closing brace is for !nested_run_pending,
but it's aligned with (vm_exit_controls & ..._PREEMPTION_TIMER).

Even better than fixing the indentation would be to include !nested_run_pending
in the top-level if statement, which reduces the nesting level and produces
a much cleaner diff, e.g.

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 409a39af121f..7dd6440425ab 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3951,7 +3951,8 @@ static void sync_vmcs02_to_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
        else
                vmcs12->guest_activity_state = GUEST_ACTIVITY_ACTIVE;

-       if (nested_cpu_has_preemption_timer(vmcs12)) {
+       if (nested_cpu_has_preemption_timer(vmcs12) &&
+           !vmx->nested.nested_run_pending) {
                vmx->nested.preemption_timer_remaining =
                        vmx_get_preemption_timer_value(vcpu);
                if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_VMX_PREEMPTION_TIMER)

>  	}
>  
>  	/*
> -- 
> 2.26.1.301.g55bc3eb7cb9-goog
> 

  reply	other threads:[~2020-04-22  1:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-17 18:34 [kvm PATCH 0/2] *** Enable VMX preemption timer migration *** Makarand Sonare
2020-04-17 18:34 ` [kvm PATCH 1/2] KVM: nVMX - enable VMX preemption timer migration Makarand Sonare
2020-04-22  2:36   ` Sean Christopherson
2020-04-17 18:34 ` [kvm PATCH 2/2] KVM: nVMX: Don't clobber preemption timer in the VMCS12 before L2 ran Makarand Sonare
2020-04-22  1:57   ` Sean Christopherson [this message]
2020-04-22  2:02     ` Sean Christopherson
2020-04-22 17:05       ` Jim Mattson
2020-04-22 23:08         ` Sean Christopherson
2020-04-23 14:58         ` Paolo Bonzini
2020-04-23 21:11           ` Jim Mattson
2020-04-23 21:18             ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200422015759.GE17836@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=makarandsonare@google.com \
    --cc=pshier@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.