From patchwork Wed Apr 6 10:33:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 65159 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp967073lbc; Wed, 6 Apr 2016 03:35:16 -0700 (PDT) X-Received: by 10.66.100.228 with SMTP id fb4mr68905043pab.84.1459938916352; Wed, 06 Apr 2016 03:35:16 -0700 (PDT) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id y26si3834885pfi.70.2016.04.06.03.35.16 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 06 Apr 2016 03:35:16 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) client-ip=2001:1868:205::9; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ankme-0007zJ-CT; Wed, 06 Apr 2016 10:34:00 +0000 Received: from mail-wm0-x22d.google.com ([2a00:1450:400c:c09::22d]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ankmZ-0007ok-Vw for linux-arm-kernel@lists.infradead.org; Wed, 06 Apr 2016 10:33:57 +0000 Received: by mail-wm0-x22d.google.com with SMTP id n3so56720587wmn.0 for ; Wed, 06 Apr 2016 03:33:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=LKKCrnqr4A7iZ3GajMYZph/+ufUWyVDYIqktan7OGoU=; b=cxbqN+7EKX9QrAaQha3/Rpm5qmn02rFSBXVBAPTJTQwfHpZF8BaDXNR332iD3ZiVCb Bx7RQyglXRD6WEK545TFUu3KBDV6fChe80TATIUmYfWaElASVowlmWsbicRtjOj/PTZA faXDFHew07j0RcaFl1W6vT/yvub/OheTxZXU8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=LKKCrnqr4A7iZ3GajMYZph/+ufUWyVDYIqktan7OGoU=; b=k01Ab1WlMI/aGY1G/2Tt8P784XT8o/X+jWZ3HABD0aoFOttJD7n6lPu1amfugnLr7u U3AxcRArMO9UdeaPQoPP4kbbxS3yPj1mgFF/6ER1dBNIjnO4WL4X+JQ2qNDUxOaVIwVS UYyXRavbUolmlox9uyDeY+0RZqoFNzpM1lgv9DoMNULYvEQBzG6hb/+pDutdBgIfHhJY zRsgdeoCCZgjTeNkXW92iBqn5NAH1g8SZga1hEUVaDEtIqwdf2LytUzw9SJdhkiNooL9 JoDQStQKmHWWofJocjuGsDwTeU0mji/QRjJAZbHgtklfW26riaXM5Zkn+zTj4a8qKqvU WTHA== X-Gm-Message-State: AD7BkJJwljp/qRtfEFwTGPUy+aW+TM4RBZnUUOd7DXYr4AXQa5HjoZM8D1uusoP6+KjZ6IvG X-Received: by 10.194.90.3 with SMTP id bs3mr8239637wjb.105.1459938814582; Wed, 06 Apr 2016 03:33:34 -0700 (PDT) Received: from localhost ([94.18.191.146]) by smtp.gmail.com with ESMTPSA id z127sm24183263wme.5.2016.04.06.03.33.33 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 06 Apr 2016 03:33:33 -0700 (PDT) Date: Wed, 6 Apr 2016 12:33:44 +0200 From: Christoffer Dall To: Marc Zyngier Subject: Re: [PATCH] KVM: arm/arm64: Handle forward time correction gracefully Message-ID: <20160406103344.GA17975@cbox> References: <1459931842-29465-1-git-send-email-marc.zyngier@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1459931842-29465-1-git-send-email-marc.zyngier@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160406_033356_331595_56B22610 X-CRM114-Status: GOOD ( 29.67 ) X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [2a00:1450:400c:c09:0:0:0:22d listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Alexander Graf , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org On Wed, Apr 06, 2016 at 09:37:22AM +0100, Marc Zyngier wrote: > On a host that runs NTP, corrections can have a direct impact on > the background timer that we program on the behalf of a vcpu. > > In particular, NTP performing a forward correction will result in > a timer expiring sooner than expected from a guest point of view. > Not a big deal, we kick the vcpu anyway. > > But on wake-up, the vcpu thread is going to perform a check to > find out whether or not it should block. And at that point, the > timer check is going to say "timer has not expired yet, go back > to sleep". This results in the timer event being lost forever. > > There are multiple ways to handle this. One would be record that > the timer has expired and let kvm_cpu_has_pending_timer return > true in that case, but that would be fairly invasive. Another is > to check for the "short sleep" condition in the hrtimer callback, > and restart the timer for the remaining time when the condition > is detected. > > This patch implements the latter, with a bit of refactoring in > order to avoid too much code duplication. > > Reported-by: Alexander Graf > Signed-off-by: Marc Zyngier > --- > virt/kvm/arm/arch_timer.c | 47 +++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 37 insertions(+), 10 deletions(-) > > diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c > index a9ad4fe..4d0e77a 100644 > --- a/virt/kvm/arm/arch_timer.c > +++ b/virt/kvm/arm/arch_timer.c > @@ -98,10 +98,46 @@ static void kvm_timer_inject_irq_work(struct work_struct *work) > kvm_vcpu_kick(vcpu); > } > > +static u64 kvm_timer_compute_delta(struct kvm_vcpu *vcpu) > +{ > + cycle_t cval, now; > + > + cval = vcpu->arch.timer_cpu.cntv_cval; > + now = kvm_phys_timer_read() - vcpu->kvm->arch.timer.cntvoff; > + > + if (now < cval) { > + u64 ns; > + > + ns = cyclecounter_cyc2ns(timecounter->cc, > + cval - now, > + timecounter->mask, > + &timecounter->frac); > + return ns; > + } > + > + return 0; > +} > + > static enum hrtimer_restart kvm_timer_expire(struct hrtimer *hrt) > { > struct arch_timer_cpu *timer; > + struct kvm_vcpu *vcpu; > + u64 ns; > + > timer = container_of(hrt, struct arch_timer_cpu, timer); > + vcpu = container_of(timer, struct kvm_vcpu, arch.timer_cpu); > + > + /* > + * Check that the timer has really expired from the guest's > + * PoV (NTP on the host may have forced it to expire > + * early). If we should have slept longer, restart it. > + */ > + ns = kvm_timer_compute_delta(vcpu); > + if (unlikely(ns)) { > + hrtimer_forward_now(hrt, ns_to_ktime(ns)); > + return HRTIMER_RESTART; > + } > + > queue_work(wqueue, &timer->expired); > return HRTIMER_NORESTART; > } > @@ -176,8 +212,6 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu) > void kvm_timer_schedule(struct kvm_vcpu *vcpu) > { > struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu; > - u64 ns; > - cycle_t cval, now; > > BUG_ON(timer_is_armed(timer)); > > @@ -197,14 +231,7 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu) > return; > > /* The timer has not yet expired, schedule a background timer */ > - cval = timer->cntv_cval; > - now = kvm_phys_timer_read() - vcpu->kvm->arch.timer.cntvoff; > - > - ns = cyclecounter_cyc2ns(timecounter->cc, > - cval - now, > - timecounter->mask, > - &timecounter->frac); > - timer_arm(timer, ns); > + timer_arm(timer, kvm_timer_compute_delta(vcpu)); > } > > void kvm_timer_unschedule(struct kvm_vcpu *vcpu) > -- > 2.1.4 > How do you guys feel about adding this to the patch for improved sleep at night (pun intended): Otherwise: Reviewed-by: Christoffer Dall And I can queue this with CC to stable, but I would like Alex's tested-by if possible. Thanks, -Christoffer _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c index a9ad4fe..230f720 100644 --- a/virt/kvm/arm/arch_timer.c +++ b/virt/kvm/arm/arch_timer.c @@ -91,6 +91,8 @@ static void kvm_timer_inject_irq_work(struct work_struct *work) vcpu = container_of(work, struct kvm_vcpu, arch.timer_cpu.expired); vcpu->arch.timer_cpu.armed = false; + BUG_ON(!kvm_timer_should_fire(vcpu)); + /* * If the vcpu is blocked we want to wake it up so that it will see * the timer has expired when entering the guest.