From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [patch 14/18] time: export time information for KVM pvclock
Date: Tue, 13 Nov 2012 19:07:17 -0200
Message-ID: <20121113210717.GA27842@amt.cnet>
References: <20121024131340.742340256@redhat.com>
 <20121024131621.941019009@redhat.com>
 <509DA7BC.2050208@us.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, jeremy@goop.org, glommer@parallels.com,
	zamsden@gmail.com, gleb@redhat.com, avi@redhat.com,
	pbonzini@redhat.com
To: John Stultz <johnstul@us.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:27138 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S932157Ab2KMVWr (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 13 Nov 2012 16:22:47 -0500
Content-Disposition: inline
In-Reply-To: <509DA7BC.2050208@us.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Fri, Nov 09, 2012 at 05:02:52PM -0800, John Stultz wrote:
> On 10/24/2012 06:13 AM, Marcelo Tosatti wrote:
> >As suggested by John, export time data similarly to how its
> >done by vsyscall support. This allows KVM to retrieve necessary
> >information to implement vsyscall support in KVM guests.
> >
> >Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> Thanks Marcelo, I like this much better then what you were proposing
> privately earlier!
> 
> Fairly minor nit below.
> 
> >Index: vsyscall/kernel/time/timekeeping.c
> >===================================================================
> >--- vsyscall.orig/kernel/time/timekeeping.c
> >+++ vsyscall/kernel/time/timekeeping.c
> >@@ -21,6 +21,7 @@
> >  #include <linux/time.h>
> >  #include <linux/tick.h>
> >  #include <linux/stop_machine.h>
> >+#include <linux/pvclock_gtod.h>
> >
> >
> >  static struct timekeeper timekeeper;
> >@@ -180,6 +181,79 @@ static inline s64 timekeeping_get_ns_raw
> >  	return nsec + arch_gettimeoffset();
> >  }
> >
> >+static RAW_NOTIFIER_HEAD(pvclock_gtod_chain);
> >+
> >+/**
> >+ * pvclock_gtod_register_notifier - register a pvclock timedata update listener
> >+ *
> >+ * Must hold write on timekeeper.lock
> >+ */
> >+int pvclock_gtod_register_notifier(struct notifier_block *nb)
> >+{
> >+	struct timekeeper *tk = &timekeeper;
> >+	unsigned long flags;
> >+	int ret;
> >+
> >+	write_seqlock_irqsave(&tk->lock, flags);
> >+	ret = raw_notifier_chain_register(&pvclock_gtod_chain, nb);
> >+	write_sequnlock_irqrestore(&tk->lock, flags);
> >+
> >+	return ret;
> >+}
> >+EXPORT_SYMBOL_GPL(pvclock_gtod_register_notifier);
> >+
> >+/**
> >+ * pvclock_gtod_unregister_notifier - unregister a pvclock
> >+ * timedata update listener
> >+ *
> >+ * Must hold write on timekeeper.lock
> >+ */
> >+int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
> >+{
> >+	struct timekeeper *tk = &timekeeper;
> >+	unsigned long flags;
> >+	int ret;
> >+
> >+	write_seqlock_irqsave(&tk->lock, flags);
> >+	ret = raw_notifier_chain_unregister(&pvclock_gtod_chain, nb);
> >+	write_sequnlock_irqrestore(&tk->lock, flags);
> >+
> >+	return ret;
> >+}
> >+EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
> >+
> >+struct pvclock_gtod_data pvclock_gtod_data;
> >+EXPORT_SYMBOL_GPL(pvclock_gtod_data);
> >+
> >+static void update_pvclock_gtod(struct timekeeper *tk)
> >+{
> >+	struct pvclock_gtod_data *vdata = &pvclock_gtod_data;
> >+
> >+	write_seqcount_begin(&vdata->seq);
> >+
> >+	/* copy pvclock gtod data */
> >+	vdata->clock.vclock_mode	= tk->clock->archdata.vclock_mode;
> >+	vdata->clock.cycle_last		= tk->clock->cycle_last;
> >+	vdata->clock.mask		= tk->clock->mask;
> >+	vdata->clock.mult		= tk->mult;
> >+	vdata->clock.shift		= tk->shift;
> >+
> >+	vdata->monotonic_time_sec	= tk->xtime_sec
> >+					+ tk->wall_to_monotonic.tv_sec;
> >+	vdata->monotonic_time_snsec	= tk->xtime_nsec
> >+					+ (tk->wall_to_monotonic.tv_nsec
> >+						<< tk->shift);
> >+	while (vdata->monotonic_time_snsec >=
> >+					(((u64)NSEC_PER_SEC) << tk->shift)) {
> >+		vdata->monotonic_time_snsec -=
> >+					((u64)NSEC_PER_SEC) << tk->shift;
> >+		vdata->monotonic_time_sec++;
> >+	}
> >+
> >+	write_seqcount_end(&vdata->seq);
> >+	raw_notifier_call_chain(&pvclock_gtod_chain, 0, NULL);
> >+}
> >+
> 
> My only request is could the update_pvclock_gtod() be implemented
> similarly to the update_vsyscall, where the update function lives in
> the pvclock code (maybe using a weak symbol or something) so we
> don't have to have all these pvclock details in the timekeeping
> core?
> 
> thanks
> -john

In KVM code, yes, no problem.