From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>,
kvm@vger.kernel.org, "Liran Alon" <liran.alon@oracle.com>,
"Wanpeng Li" <wanpengli@tencent.com>
Subject: Re: [PATCH v3 2/9] KVM: lapic: Convert guest TSC to host time domain when delaying
Date: Wed, 17 Apr 2019 07:33:43 -0700 [thread overview]
Message-ID: <20190417143342.GA8567@linux.intel.com> (raw)
In-Reply-To: <e18b929f-8ee8-3fdd-d766-0ab0d55a4994@redhat.com>
On Wed, Apr 17, 2019 at 02:56:53PM +0200, Paolo Bonzini wrote:
> On 16/04/19 22:32, Sean Christopherson wrote:
> > + /* ndelay uses delay_tsc whenever the hardware has TSC, thus always. */
> > + if (guest_tsc < tsc_deadline) {
> > + ns = (tsc_deadline - guest_tsc) * 1000000ULL;
> > + do_div(ns, vcpu->arch.virtual_tsc_khz);
> > + ndelay(min_t(u32, ns, lapic_timer_advance_ns));
> > + }
> >
>
> This min_t would allow the timer to expire in advance of the indicated
> deadline. If this happens, wouldn't it be a bug elsewhere?
Maybe?
In this exact variation of the code, no, since userspace can modify
lapic_timer_advance_ns while the vCPU is running. Obviously that case
goes away when lapic_timer_advance_ns is no longer directly consumed by
vCPUS.
Originally, the check was added because lockup occurred if the guest's
TSC was sent backwards[1]. The discussion surrounding the bugfix patch[2]
is probably more interesting/amusing/relevant. We're basically rehashing
that discussion. :-)
Anyways, capping at a KVM controlled value is prudent since not doing
so can hang the host if something does go wrong. As for the __delay()
vs ndelay() discussion, I'll try to dig more into how/why I saw the timer
fire with an incorrect TSC, i.e. why ndelay() "works" but __delay() does
not.
https://bugzilla.redhat.com/show_bug.cgi?id=1337667
https://patchwork.kernel.org/patch/9130687/
next prev parent reply other threads:[~2019-04-17 14:33 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 [this message]
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 ` [PATCH v3 0/9] KVM: lapic: Fix a variety of timer adv issues 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=20190417143342.GA8567@linux.intel.com \
--to=sean.j.christopherson@intel.com \
--cc=kvm@vger.kernel.org \
--cc=liran.alon@oracle.com \
--cc=pbonzini@redhat.com \
--cc=rkrcmar@redhat.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.