kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Woodhouse <dwmw2@infradead.org>
To: kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	Joao Martins <joao.m.martins@oracle.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Metin Kaya <metikaya@amazon.co.uk>,
	Paul Durrant <pdurrant@amazon.co.uk>
Subject: [PATCH 1/2] KVM: x86/xen: PV oneshot timer fixes
Date: Wed,  9 Mar 2022 14:38:34 +0000	[thread overview]
Message-ID: <20220309143835.253911-2-dwmw2@infradead.org> (raw)
In-Reply-To: <20220309143835.253911-1-dwmw2@infradead.org>

From: David Woodhouse <dwmw@amazon.co.uk>

Fix the case where a restored timer is supposed to have triggered not
just in the past, but before this kernel even booted. The resulting
integer wrap caused the timer to be set a very long time into the future,
and thus effectively never trigger. Trigger timers immediately when
delta_ns <= 0 to avoid that situation.

Also switch to using HRTIMER_MODE_ABS_HARD, following the changes in the
local APIC timer in commits 2c0d278f3293f ("KVM: LAPIC: Mark hrtimer to
expire in hard interrupt context") and 4d151bf3b89e7 ("KVM: LAPIC: Make
lapic timer unpinned")... and the change I'm about to submit to fix the
local APIC timer not to gratuitously cancel and restart the hrtimer on
migration, since it isn't pinned any more anyway.

On serializing the timer state for migration, only set the expires_ns
if the timer is still active, otherwise return zero in that field.

Finally, fix the 'delta' in kvm_xen_hcall_set_timer_op() to explicitly
use 'int64_t' instead of 'long' to make the sanity check shift by 50
bits work correctly in the 32-bit build. That last one was

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/x86/kvm/irq.c |  1 -
 arch/x86/kvm/xen.c | 37 +++++++++++++++----------------------
 arch/x86/kvm/xen.h |  1 -
 3 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
index af2d26fc5458..f371f1292ca3 100644
--- a/arch/x86/kvm/irq.c
+++ b/arch/x86/kvm/irq.c
@@ -156,7 +156,6 @@ void __kvm_migrate_timers(struct kvm_vcpu *vcpu)
 {
 	__kvm_migrate_apic_timer(vcpu);
 	__kvm_migrate_pit_timer(vcpu);
-	__kvm_migrate_xen_timer(vcpu);
 	static_call_cond(kvm_x86_migrate_timers)(vcpu);
 }
 
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 8c85a71aa8ca..2131e21ce6c7 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -150,29 +150,19 @@ static enum hrtimer_restart xen_timer_callback(struct hrtimer *timer)
 	return HRTIMER_NORESTART;
 }
 
-void __kvm_migrate_xen_timer(struct kvm_vcpu *vcpu)
+static void kvm_xen_start_timer(struct kvm_vcpu *vcpu, u64 guest_abs, s64 delta_ns)
 {
-	struct hrtimer *timer;
-
-	if (!kvm_xen_timer_enabled(vcpu))
-		return;
-
-	timer = &vcpu->arch.xen.timer;
-	if (hrtimer_cancel(timer))
-		hrtimer_start_expires(timer, HRTIMER_MODE_ABS_PINNED);
-}
-
-static void kvm_xen_start_timer(struct kvm_vcpu *vcpu, u64 guest_abs, u64 delta_ns)
-{
-	ktime_t ktime_now;
-
 	atomic_set(&vcpu->arch.xen.timer_pending, 0);
 	vcpu->arch.xen.timer_expires = guest_abs;
 
-	ktime_now = ktime_get();
-	hrtimer_start(&vcpu->arch.xen.timer,
-		      ktime_add_ns(ktime_now, delta_ns),
-		      HRTIMER_MODE_ABS_PINNED);
+	if (delta_ns <= 0) {
+		xen_timer_callback(&vcpu->arch.xen.timer);
+	} else {
+		ktime_t ktime_now = ktime_get();
+		hrtimer_start(&vcpu->arch.xen.timer,
+			      ktime_add_ns(ktime_now, delta_ns),
+			      HRTIMER_MODE_ABS_HARD);
+	}
 }
 
 static void kvm_xen_stop_timer(struct kvm_vcpu *vcpu)
@@ -185,7 +175,7 @@ static void kvm_xen_stop_timer(struct kvm_vcpu *vcpu)
 static void kvm_xen_init_timer(struct kvm_vcpu *vcpu)
 {
 	hrtimer_init(&vcpu->arch.xen.timer, CLOCK_MONOTONIC,
-		     HRTIMER_MODE_ABS_PINNED);
+		     HRTIMER_MODE_ABS_HARD);
 	vcpu->arch.xen.timer.function = xen_timer_callback;
 }
 
@@ -839,7 +829,10 @@ int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 	case KVM_XEN_VCPU_ATTR_TYPE_TIMER:
 		data->u.timer.port = vcpu->arch.xen.timer_virq;
 		data->u.timer.priority = KVM_IRQ_ROUTING_XEN_EVTCHN_PRIO_2LEVEL;
-		data->u.timer.expires_ns = vcpu->arch.xen.timer_expires;
+		if (hrtimer_active(&vcpu->arch.xen.timer))
+			data->u.timer.expires_ns = vcpu->arch.xen.timer_expires;
+		else
+			data->u.timer.expires_ns = 0;
 		r = 0;
 		break;
 
@@ -1204,7 +1197,7 @@ static bool kvm_xen_hcall_set_timer_op(struct kvm_vcpu *vcpu, uint64_t timeout,
 
 	if (timeout) {
 		uint64_t guest_now = get_kvmclock_ns(vcpu->kvm);
-		long delta = timeout - guest_now;
+		int64_t delta = timeout - guest_now;
 
 		/* Xen has a 'Linux workaround' in do_set_timer_op() which
 		 * checks for negative absolute timeout values (caused by
diff --git a/arch/x86/kvm/xen.h b/arch/x86/kvm/xen.h
index ad0876a7c301..2bbbc1a3953e 100644
--- a/arch/x86/kvm/xen.h
+++ b/arch/x86/kvm/xen.h
@@ -75,7 +75,6 @@ static inline int kvm_xen_has_pending_timer(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-void __kvm_migrate_xen_timer(struct kvm_vcpu *vcpu);
 void kvm_xen_inject_timer_irqs(struct kvm_vcpu *vcpu);
 #else
 static inline int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data)
-- 
2.33.1


  reply	other threads:[~2022-03-09 14:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-09 14:38 [PATCH 0/2] KVM: Xen PV timer fixups David Woodhouse
2022-03-09 14:38 ` David Woodhouse [this message]
2022-03-09 15:39   ` [PATCH 1/2] KVM: x86/xen: PV oneshot timer fixes Paolo Bonzini
2022-03-09 16:11     ` David Woodhouse
2022-03-09 16:28       ` Paolo Bonzini
2022-03-09 16:37         ` David Woodhouse
2022-03-09 16:39           ` Paolo Bonzini
2022-03-09 16:41     ` [PATCH v2 " David Woodhouse
2022-03-13 14:30       ` Paolo Bonzini
2022-03-13 14:57         ` David Woodhouse
2022-03-09 14:38 ` [PATCH 2/2] KVM: x86/xen: Update self test for Xen PV timers David Woodhouse
2022-03-09 14:47 ` [PATCH 0/2] KVM: Xen PV timer fixups David Woodhouse

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=20220309143835.253911-2-dwmw2@infradead.org \
    --to=dwmw2@infradead.org \
    --cc=boris.ostrovsky@oracle.com \
    --cc=jmattson@google.com \
    --cc=joao.m.martins@oracle.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=metikaya@amazon.co.uk \
    --cc=pbonzini@redhat.com \
    --cc=pdurrant@amazon.co.uk \
    --cc=seanjc@google.com \
    --cc=vkuznets@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).