Thanks for comments!
On Wednesday 05 March 2008 17:15:29 Ingo Molnar wrote:
> * Yang, Sheng <[EMAIL PROTECTED]> 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.
Um... I followed example on ./virt/kvm/ioapic.c here. Though I think it's good
to substitute all self defined debug printk with pr_debug, why KVM have
little pr_xxx(the only ones are in x86.c)? Maybe for KVM is acting more like
a separate driver, and using printk is easier for separate debug? I really
don't know...
> > +#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.
That's my hope (of course with big endian support). But is there any user
outside KVM? I think it may not easy to get into the generic part.
> > + ASSERT(mutex_is_locked(&kvm->arch.vpit->pit_state.lock));
>
> could we please standardize on WARN_ON(!(x)) instead?
Sure. :)
> > +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.
I will update these constants. :) In fact, I have thought of these before, but
not insist...
Thanks!
Yang, Sheng
>
> 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/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
