All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: "Sean Christopherson" <sean.j.christopherson@intel.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org, Liran Alon <liran.alon@oracle.com>,
	Wanpeng Li <wanpengli@tencent.com>
Subject: Re: [PATCH v3 0/9] KVM: lapic: Fix a variety of timer adv issues
Date: Wed, 17 Apr 2019 15:06:53 +0200	[thread overview]
Message-ID: <e8668798-7af7-1096-4f4a-68aab71ef9d0@redhat.com> (raw)
In-Reply-To: <20190416203248.29429-1-sean.j.christopherson@intel.com>

On 16/04/19 22:32, Sean Christopherson wrote:
> The recent change to automatically calculate lapic_timer_advance_ns
> introduced a handful of gnarly bugs, and also exposed a latent bug by
> virtue of the advancement logic being enabled by default.  Inspection
> and debug revealed several other opportunities for minor improvements.
> 
> The primary issue is that the auto-tuning of lapic_timer_advance_ns is
> completely unbounded, e.g. there's nothing in the logic that prevents the
> advancement from creeping up to hundreds of milliseconds.  Adjusting the
> timers by large amounts creates major discrepancies in the guest, e.g. a
> timer that was configured to fire after multiple milliseconds may arrive
> before the guest executes a single instruction.  While technically correct
> from a time perspective, it breaks a reasonable assumption from the guest
> that it can execute some number of instructions between timer events.
> 
> The other major issue is that the advancement is global, while TSC scaling
> is done on a per-vCPU basic.  Adjusting the advancement at runtime
> exacerbates this as there is no protection against multiple vCPUs and/or
> multiple VMs concurrently modifying the advancement value, e.g. it can
> effectively become corrupted or never stabilize due to getting bounced all
> over tarnation.
> 
> As for the latent bug, when timer advancement was applied to the hv_timer,
> i.e. the VMX preemption timer, the logic to trigger wait_for_lapic_timer()
> was not updated.  As a result, a timer interrupt emulated via the hv_timer
> can easily arrive too early from a *time* perspective, as opposed to
> simply arriving early from a "number of instructions executed" perspective.

In a surprising twist of events, I've queued the last 5 patches.  For
the first four, I still plan to get them in 5.1 but I had some comments.
 They will have to wait for next week since I'll have a (short) Easter
vacation, but there should be plenty of time.

Paolo

      parent reply	other threads:[~2019-04-17 13:07 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-16 20:32 [PATCH v3 0/9] KVM: lapic: Fix a variety of timer adv issues Sean Christopherson
2019-04-16 20:32 ` [PATCH v3 1/9] KVM: lapic: Hard cap the auto-calculated timer advancement Sean Christopherson
2019-04-17 12:57   ` Paolo Bonzini
2019-04-17 14:34     ` Sean Christopherson
2019-04-17 15:00       ` Paolo Bonzini
2019-04-16 20:32 ` [PATCH v3 2/9] KVM: lapic: Convert guest TSC to host time domain when delaying Sean Christopherson
2019-04-17 12:56   ` Paolo Bonzini
2019-04-17 14:33     ` Sean Christopherson
2019-04-17 15:05       ` Paolo Bonzini
2019-04-16 20:32 ` [PATCH v3 3/9] KVM: lapic: Track lapic timer advance per vCPU Sean Christopherson
2019-04-16 20:32 ` [PATCH v3 4/9] KVM: lapic: Allow user to disable auto-tuning of timer advancement Sean Christopherson
2019-04-17 12:59   ` Paolo Bonzini
2019-04-17 15:22     ` Sean Christopherson
2019-04-16 20:32 ` [PATCH v3 5/9] KVM: lapic: Busy wait for timer to expire when using hv_timer Sean Christopherson
2019-04-16 20:32 ` [PATCH v3 6/9] KVM: lapic: Explicitly cancel the hv timer if it's pre-expired Sean Christopherson
2019-04-16 20:32 ` [PATCH v3 7/9] KVM: lapic: Refactor ->set_hv_timer to use an explicit expired param Sean Christopherson
2019-04-17 13:02   ` Paolo Bonzini
2019-04-16 20:32 ` [PATCH v3 8/9] KVM: lapic: Check for a pending timer intr prior to start_hv_timer() Sean Christopherson
2019-04-16 20:32 ` [PATCH v3 9/9] KVM: VMX: Skip delta_tsc shift-and-divide if the dividend is zero Sean Christopherson
2019-04-17 13:06 ` Paolo Bonzini [this message]

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=e8668798-7af7-1096-4f4a-68aab71ef9d0@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=liran.alon@oracle.com \
    --cc=rkrcmar@redhat.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=wanpengli@tencent.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.