From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6A2BC10F11 for ; Mon, 22 Apr 2019 21:13:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AD6FF20B1F for ; Mon, 22 Apr 2019 21:13:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729526AbfDVVNw (ORCPT ); Mon, 22 Apr 2019 17:13:52 -0400 Received: from mga14.intel.com ([192.55.52.115]:38982 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728796AbfDVVNw (ORCPT ); Mon, 22 Apr 2019 17:13:52 -0400 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Apr 2019 14:13:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,383,1549958400"; d="scan'208";a="166919543" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.181]) by fmsmga001.fm.intel.com with ESMTP; 22 Apr 2019 14:13:51 -0700 Date: Mon, 22 Apr 2019 14:13:51 -0700 From: Sean Christopherson To: Liran Alon Cc: pbonzini@redhat.com, rkrcmar@redhat.com, kvm@vger.kernel.org, Joao Martins Subject: Re: [PATCH] KVM: x86: Consider LAPIC TSC-Deadline timer expired if deadline too short Message-ID: <20190422211351.GJ1236@linux.intel.com> References: <20190416173634.31501-1-liran.alon@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190416173634.31501-1-liran.alon@oracle.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Tue, Apr 16, 2019 at 08:36:34PM +0300, Liran Alon wrote: > If guest sets MSR_IA32_TSCDEADLINE to value such that in host > time-domain it's shorter than lapic_timer_advance_ns, we can > reach a case that we call hrtimer_start() with expiration time set at > the past. > > Because lapic_timer.timer is init with HRTIMER_MODE_ABS_PINNED, it > is not allowed to run in softirq and therefore will never expire. > > To avoid such a scenario, verify that deadline expiration time is set on > host time-domain further than (now + lapic_timer_advance_ns). > > A future patch can also consider adding a min_timer_deadline_ns module parameter, > similar to min_timer_period_us to avoid races that amount of ns it takes > to run logic could still call hrtimer_start() with expiration timer set > at the past. > > Reviewed-by: Joao Martins > Signed-off-by: Liran Alon > --- > arch/x86/kvm/lapic.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 9f089e2e09d0..fbd37e07e90d 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -1538,9 +1538,12 @@ static void start_sw_tscdeadline(struct kvm_lapic *apic) > > now = ktime_get(); > guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()); > - if (likely(tscdeadline > guest_tsc)) { > - ns = (tscdeadline - guest_tsc) * 1000000ULL; > - do_div(ns, this_tsc_khz); > + > + ns = (tscdeadline - guest_tsc) * 1000000ULL; > + do_div(ns, this_tsc_khz); This will do the division even if 'tscdeadline <= guest_tsc'. I don't see any reason to use ktime_sub_ns() since neither 'ns' or 'lapic_timer_advance_ns' is a ktime_t, e.g.: if (likely(tscdeadline > guest_tsc)) { ns = (tscdeadline - guest_tsc) * 1000000ULL; do_div(ns, this_tsc_khz); if (likely(ns > lapic_timer_advance_ns)) ns -= lapic_timer_advance_ns; else ns = 0; } else { ns = 0; } if (ns) { now = ktime_get(); expire = ktime_add_ns(now, ns); hrtimer_start(&apic->lapic_timer.timer, expire, HRTIMER_MODE_ABS_PINNED); } else apic_timer_expired(apic); > + > + if (likely(tscdeadline > guest_tsc) && > + likely(ns > lapic_timer_advance_ns)) { > expire = ktime_add_ns(now, ns); > expire = ktime_sub_ns(expire, lapic_timer_advance_ns); > hrtimer_start(&apic->lapic_timer.timer, > -- > 2.20.1 >