From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH 1/6] KVM: In kernel pit model
Date: Wed, 5 Mar 2008 10:15:29 +0100
Message-ID: <20080305091529.GA25357@elte.hu>
References: <200803041822.44757.sheng.yang@intel.com>
	<47CE438E.2090408@qumranet.com>
	<200803051504.40409.sheng.yang@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Cc: kvm-devel@lists.sourceforge.net, Avi Kivity <avi@qumranet.com>
To: "Yang, Sheng" <sheng.yang@intel.com>
Return-path: <kvm-devel-bounces@lists.sourceforge.net>
Content-Disposition: inline
In-Reply-To: <200803051504.40409.sheng.yang@intel.com>
List-Unsubscribe: <https://lists.sourceforge.net/lists/listinfo/kvm-devel>,
	<mailto:kvm-devel-request@lists.sourceforge.net?subject=unsubscribe>
List-Archive: <http://sourceforge.net/mailarchive/forum.php?forum_name=kvm-devel>
List-Post: <mailto:kvm-devel@lists.sourceforge.net>
List-Help: <mailto:kvm-devel-request@lists.sourceforge.net?subject=help>
List-Subscribe: <https://lists.sourceforge.net/lists/listinfo/kvm-devel>,
	<mailto:kvm-devel-request@lists.sourceforge.net?subject=subscribe>
Sender: kvm-devel-bounces@lists.sourceforge.net
Errors-To: kvm-devel-bounces@lists.sourceforge.net
List-Id: kvm.vger.kernel.org


* Yang, Sheng <sheng.yang@intel.com> wrote:

> +#if 1
> +#define pit_debug(fmt, arg...) printk(KERN_WARNING fmt, ##arg)
> +#else
> +#define pit_debug(fmt, arg...)
> +#endif

this should use pr_debug() instead i guess.

> +#ifndef CONFIG_X86_64
> +#define mod_64(x, y) ((x) - (y) * div64_64(x, y))
> +#else
> +#define mod_64(x, y) ((x) % (y))
> +#endif

> +/* Compute with 96 bit intermediate result: (a*b)/c */
> +static u64 muldiv64(u64 a, u32 b, u32 c)
> +{
> +	union {
> +		u64 ll;
> +		struct {
> +			u32 low, high;
> +		} l;
> +	} u, res;
> +	u64 rl, rh;
> +
> +	u.ll = a;
> +	rl = (u64)u.l.low * (u64)b;
> +	rh = (u64)u.l.high * (u64)b;
> +	rh += (rl >> 32);
> +	res.l.high = div64_64(rh, c);
> +	res.l.low = div64_64(((mod_64(rh, c) << 32) + (rl & 0xffffffff)), c);
> +	return res.ll;
> +}

eventually these should move into a generic file, for example 
lib/div64.c.

> +	ASSERT(mutex_is_locked(&kvm->arch.vpit->pit_state.lock));

could we please standardize on WARN_ON(!(x)) instead?

> +static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
> +{
> +	struct kvm_kpit_state *ps;
> +	int restart_timer = 0;
> +
> +	ps = container_of(data, struct kvm_kpit_state, pit_timer.timer);
> +
> +	restart_timer = __pit_timer_fn(ps);
> +
> +	if (restart_timer)
> +		return HRTIMER_RESTART;
> +	else
> +		return HRTIMER_NORESTART;
> +}

elegant use of hrtimers! :-)

> +	if (val == 0)
> +		val = 0x10000;

magic constant.

> +	val  &= 0xff;
> +	addr &= 3;

magic constants.

	Ingo

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/