Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-07 Thread Yang, Sheng
On Thursday 06 March 2008 17:41:03 Yang, Sheng wrote:
 On Thursday 06 March 2008 16:43:18 Yang, Sheng wrote:
  On Thursday 06 March 2008 16:06:51 Avi Kivity wrote:
   Yang, Sheng wrote:
Here is the updated patch. I kept 0xff because I think it's OK for
understand easily. :)
  
   Any news on the regression with older Linux guests?  That's the only
   thing keeping my from applying the patchset.
 
  Not much. PIT interrupts injection is all right, 1000 per second. Just
  found two clock source in guest got problem: PM timer and TSC. Seems both
  due to compensate for lost ticks. Get rid of something like jiffies_64
  += lost -1 get the time ok.
 
  And I think it's a exist bug. As you see, in most condition, userspace
  pit + in kernel irqchip resulted in time flow slowly, due to the lost of
  interrupts. But RHEL4 runs even faster than host...
 
  I will do more investigate.

 Get some clues.

 It seems like for the kernel which is active to inject lost interrupt, when
 some PIT interrupts were pending, the TSC/other clocksource got the wrong
 impression that some PIT interrupt lost, then using itself's counter to
 adjust the jiffies. So the problem occurs.

 Xen adjust TSC to fit this mode. It pull TSC backward to get the correct
 value when injecting one PIT interrupt. But this would causing trouble on
 some Windows. Then, it got the time mode concept...

Found more complex for KVM. Xen pulled pm timer down to kernel part, and used 
the guest TSC as source. So only adjust TSC is OK for it. But we are still 
using pm timer in QEmu, which using host time as source. So even we pull back 
TSC, the problem still exists, for 2.6.9 prefer to pm timer by default.

-- 
Thanks
Yang, Sheng

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-07 Thread Avi Kivity
Yang, Sheng wrote:
 Found more complex for KVM. Xen pulled pm timer down to kernel part, and used 
 the guest TSC as source. So only adjust TSC is OK for it. But we are still 
 using pm timer in QEmu, which using host time as source. So even we pull back 
 TSC, the problem still exists, for 2.6.9 prefer to pm timer by default

Interesting.  I guess we should pull the pm timer into the kernel as 
well.  Timing is too tricky for userspace.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-07 Thread Yang, Sheng
On Friday 07 March 2008 16:53:40 Avi Kivity wrote:
 Yang, Sheng wrote:
  Found more complex for KVM. Xen pulled pm timer down to kernel part, and
  used the guest TSC as source. So only adjust TSC is OK for it. But we are
  still using pm timer in QEmu, which using host time as source. So even we
  pull back TSC, the problem still exists, for 2.6.9 prefer to pm timer by
  default

 Interesting.  I guess we should pull the pm timer into the kernel as
 well.  Timing is too tricky for userspace.

... Should we suggest using clock=pit on pae 2.6.9 at first?

-- 
Thanks
Yang, Sheng

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-07 Thread Avi Kivity
Yang, Sheng wrote:
 On Friday 07 March 2008 16:53:40 Avi Kivity wrote:
   
 Yang, Sheng wrote:
 
 Found more complex for KVM. Xen pulled pm timer down to kernel part, and
 used the guest TSC as source. So only adjust TSC is OK for it. But we are
 still using pm timer in QEmu, which using host time as source. So even we
 pull back TSC, the problem still exists, for 2.6.9 prefer to pm timer by
 default
   
 Interesting.  I guess we should pull the pm timer into the kernel as
 well.  Timing is too tricky for userspace.
 

 ... Should we suggest using clock=pit on pae 2.6.9 at first?

   

While it is hardly a lovely solution (things should work out of the box) 
it is reasonable as a temporary measure.


Can you repost your patchset?  If you're quick I can apply it today, 
otherwise it will have to wait until next week.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


[kvm-devel] [PATCH 1/6] KVM: In kernel PIT model

2008-03-07 Thread Yang, Sheng
From 19bc8000b0ed1c2021ddb509a3d923e1cd8d53ec Mon Sep 17 00:00:00 2001
From: Sheng Yang [EMAIL PROTECTED]
Date: Mon, 28 Jan 2008 05:10:22 +0800
Subject: [PATCH] KVM: In kernel PIT model

The patch moved PIT from userspace to kernel, and increase the timer accuracy 
greatly.

Signed-off-by: Sheng Yang [EMAIL PROTECTED]
---
 arch/x86/kvm/Makefile  |3 +-
 arch/x86/kvm/i8254.c   |  585 

 arch/x86/kvm/i8254.h   |   60 +
 arch/x86/kvm/irq.c |3 +
 arch/x86/kvm/x86.c |9 +
 include/asm-x86/kvm_host.h |1 +
 include/linux/kvm.h|2 +
 7 files changed, 662 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/kvm/i8254.c
 create mode 100644 arch/x86/kvm/i8254.h

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index ffdd0b3..4d0c22e 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -6,7 +6,8 @@ common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o 
ioapic.o)

 EXTRA_CFLAGS += -Ivirt/kvm -Iarch/x86/kvm

-kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o
+kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o \
+   i8254.o
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
new file mode 100644
index 000..1031901
--- /dev/null
+++ b/arch/x86/kvm/i8254.c
@@ -0,0 +1,585 @@
+/*
+ * 8253/8254 interval timer emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2006 Intel Corporation
+ * Copyright (c) 2007 Keir Fraser, XenSource Inc
+ * Copyright (c) 2008 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a 
copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the 
rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Authors:
+ *   Sheng Yang [EMAIL PROTECTED]
+ *   Based on QEMU and Xen.
+ */
+
+#include linux/kvm_host.h
+
+#include irq.h
+#include i8254.h
+
+#ifndef CONFIG_X86_64
+#define mod_64(x, y) ((x) - (y) * div64_64(x, y))
+#else
+#define mod_64(x, y) ((x) % (y))
+#endif
+
+#define RW_STATE_LSB 1
+#define RW_STATE_MSB 2
+#define RW_STATE_WORD0 3
+#define RW_STATE_WORD1 4
+
+/* 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  0x)), c);
+   return res.ll;
+}
+
+static void pit_set_gate(struct kvm *kvm, int channel, u32 val)
+{
+   struct kvm_kpit_channel_state *c =
+   kvm-arch.vpit-pit_state.channels[channel];
+
+   WARN_ON(!mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   switch (c-mode) {
+   default:
+   case 0:
+   case 4:
+   /* XXX: just disable/enable counting */
+   break;
+   case 1:
+   case 2:
+   case 3:
+   case 5:
+   /* Restart counting on rising edge. */
+   if (c-gate  val)
+   c-count_load_time = ktime_get();
+   break;
+   }
+
+   c-gate = val;
+}
+
+int pit_get_gate(struct kvm *kvm, int channel)
+{
+   WARN_ON(!mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   return kvm-arch.vpit-pit_state.channels[channel].gate;
+}
+
+static int pit_get_count(struct kvm *kvm, int channel)
+{
+   struct kvm_kpit_channel_state *c =
+   kvm-arch.vpit-pit_state.channels[channel];
+   s64 d, t;
+   int counter;
+
+   WARN_ON(!mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   t = ktime_to_ns(ktime_sub(ktime_get(), c-count_load_time));
+   d = muldiv64(t, KVM_PIT_FREQ, NSEC_PER_SEC);
+
+   switch (c-mode) {
+   case 0:
+   case 1:
+   case 4:
+ 

Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-06 Thread Yang, Sheng
On Wednesday 05 March 2008 19:35:40 Yang, Sheng wrote:
 On Wednesday 05 March 2008 17:15:29 Ingo Molnar wrote:
  * Yang, Sheng [EMAIL PROTECTED] wrote:
   + val  = 0xff;
   + addr = 3;
 
  magic constants.

 I will update these constants. :) In fact, I have thought of these before,
 but not insist...

Here is the updated patch. I kept 0xff because I think it's OK for understand 
easily. :)

Thanks.

Yang, Sheng

---
 arch/x86/kvm/Makefile  |3 +-
 arch/x86/kvm/i8254.c   |  583 

 arch/x86/kvm/i8254.h   |   60 +
 arch/x86/kvm/irq.c |3 +
 arch/x86/kvm/x86.c |9 +
 include/asm-x86/kvm_host.h |1 +
 include/linux/kvm.h|2 +
 7 files changed, 660 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/kvm/i8254.c
 create mode 100644 arch/x86/kvm/i8254.h

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index ffdd0b3..4d0c22e 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -6,7 +6,8 @@ common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o 
ioapic.o)

 EXTRA_CFLAGS += -Ivirt/kvm -Iarch/x86/kvm

-kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o
+kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o \
+   i8254.o
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
new file mode 100644
index 000..3a7e9da
--- /dev/null
+++ b/arch/x86/kvm/i8254.c
@@ -0,0 +1,583 @@
+/*
+ * 8253/8254 interval timer emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2006 Intel Corporation
+ * Copyright (c) 2007 Keir Fraser, XenSource Inc
+ * Copyright (c) 2008 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a 
copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the 
rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Authors:
+ *   Sheng Yang [EMAIL PROTECTED]
+ *   Based on QEMU and Xen.
+ */
+
+#include linux/kvm_host.h
+
+#include irq.h
+#include i8254.h
+
+#ifndef CONFIG_X86_64
+#define mod_64(x, y) ((x) - (y) * div64_64(x, y))
+#else
+#define mod_64(x, y) ((x) % (y))
+#endif
+
+#define RW_STATE_LSB 1
+#define RW_STATE_MSB 2
+#define RW_STATE_WORD0 3
+#define RW_STATE_WORD1 4
+
+/* 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  0x)), c);
+   return res.ll;
+}
+
+static void pit_set_gate(struct kvm *kvm, int channel, u32 val)
+{
+   struct kvm_kpit_channel_state *c =
+   kvm-arch.vpit-pit_state.channels[channel];
+
+   WARN_ON(!mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   switch (c-mode) {
+   default:
+   case 0:
+   case 4:
+   /* XXX: just disable/enable counting */
+   break;
+   case 1:
+   case 2:
+   case 3:
+   case 5:
+   /* Restart counting on rising edge. */
+   if (c-gate  val)
+   c-count_load_time = ktime_get();
+   break;
+   }
+
+   c-gate = val;
+}
+
+int pit_get_gate(struct kvm *kvm, int channel)
+{
+   WARN_ON(!mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   return kvm-arch.vpit-pit_state.channels[channel].gate;
+}
+
+static int pit_get_count(struct kvm *kvm, int channel)
+{
+   struct kvm_kpit_channel_state *c =
+   kvm-arch.vpit-pit_state.channels[channel];
+   s64 d, t;
+   int counter;
+
+   WARN_ON(!mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   t = ktime_to_ns(ktime_sub(ktime_get(), c-count_load_time));
+   d = muldiv64(t, 

Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-06 Thread Avi Kivity
Yang, Sheng wrote:
 Here is the updated patch. I kept 0xff because I think it's OK for understand 
 easily. :)

   

Any news on the regression with older Linux guests?  That's the only 
thing keeping my from applying the patchset.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.


-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-06 Thread Yang, Sheng
On Thursday 06 March 2008 16:06:51 Avi Kivity wrote:
 Yang, Sheng wrote:
  Here is the updated patch. I kept 0xff because I think it's OK for
  understand easily. :)

 Any news on the regression with older Linux guests?  That's the only
 thing keeping my from applying the patchset.

Not much. PIT interrupts injection is all right, 1000 per second. Just found 
two clock source in guest got problem: PM timer and TSC. Seems both due 
to compensate for lost ticks. Get rid of something like jiffies_64 += 
lost -1 get the time ok. 

And I think it's a exist bug. As you see, in most condition, userspace pit + 
in kernel irqchip resulted in time flow slowly, due to the lost of 
interrupts. But RHEL4 runs even faster than host... 

I will do more investigate.

-- 
Thanks
Yang, Sheng

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-06 Thread Ingo Molnar

* Yang, Sheng [EMAIL PROTECTED] wrote:

 + /* Though spec said the state of 8254 is undefined after power-up,
 +  * seems some tricky OS like Windows XP depends on IRQ0 interrupt
 +  * when booting up.
 +  * So here setting initialize rate for it, and not a specific number */

another silly style nit, the canonical comment style is:

 + /*
 +  * Though spec said the state of 8254 is undefined after power-up,
 +  * seems some tricky OS like Windows XP depends on IRQ0 interrupt
 +  * when booting up.
 +  * So here setting initialize rate for it, and not a specific number
 +  */

we are standardizing on that in other areas of arch/x86 and KVM uses 
that too.

but your patch looks good otherwise :)

/me goes back into lurker mode

Ingo

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-06 Thread Yang, Sheng
On Thursday 06 March 2008 17:14:34 Ingo Molnar wrote:
 * Yang, Sheng [EMAIL PROTECTED] wrote:
  +   /* Though spec said the state of 8254 is undefined after power-up,
  +* seems some tricky OS like Windows XP depends on IRQ0 interrupt
  +* when booting up.
  +* So here setting initialize rate for it, and not a specific number */

 another silly style nit, the canonical comment style is:
  +   /*
  +* Though spec said the state of 8254 is undefined after power-up,
  +* seems some tricky OS like Windows XP depends on IRQ0 interrupt
  +* when booting up.
  +* So here setting initialize rate for it, and not a specific number
  +*/

 we are standardizing on that in other areas of arch/x86 and KVM uses
 that too.

... Seems I still can't fully depend on script/checkpatch.pl to correct my 
coding style error...

 but your patch looks good otherwise :)

 /me goes back into lurker mode

   Ingo

Thanks for review! :)

Yang, Sheng

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-06 Thread Yang, Sheng
On Thursday 06 March 2008 16:43:18 Yang, Sheng wrote:
 On Thursday 06 March 2008 16:06:51 Avi Kivity wrote:
  Yang, Sheng wrote:
   Here is the updated patch. I kept 0xff because I think it's OK for
   understand easily. :)
 
  Any news on the regression with older Linux guests?  That's the only
  thing keeping my from applying the patchset.

 Not much. PIT interrupts injection is all right, 1000 per second. Just
 found two clock source in guest got problem: PM timer and TSC. Seems both
 due to compensate for lost ticks. Get rid of something like jiffies_64 +=
 lost -1 get the time ok.

 And I think it's a exist bug. As you see, in most condition, userspace pit
 + in kernel irqchip resulted in time flow slowly, due to the lost of
 interrupts. But RHEL4 runs even faster than host...

 I will do more investigate.

Get some clues.

It seems like for the kernel which is active to inject lost interrupt, when 
some PIT interrupts were pending, the TSC/other clocksource got the wrong 
impression that some PIT interrupt lost, then using itself's counter to 
adjust the jiffies. So the problem occurs.  

Xen adjust TSC to fit this mode. It pull TSC backward to get the correct value 
when injecting one PIT interrupt. But this would causing trouble on some 
Windows. Then, it got the time mode concept...

-- 
Thanks
Yang, Sheng

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-05 Thread Ingo Molnar

* 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.

 +#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  0x)), 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 = 0x1;

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/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-05 Thread Yang, Sheng
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  0x)), 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 = 0x1;

 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/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-05 Thread Avi Kivity
Yang, Sheng wrote:
 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...

   

It's mostly due to lack of knowledge about pr_debug(); it wasn't 
intentional.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.


-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-05 Thread Ingo Molnar

* Yang, Sheng [EMAIL PROTECTED] 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...

it's just a small detail - it's really not a big issue and my suggestion 
should be functionally equivalent. What i meant is that with pr_debug() 
you can remove the pit_debug() define altogether (and change all 
pit_debug's to pr_debug). In that case all you need to do is to get the 
printks is to stick this to the head of the source code file (to before 
the include files):

#define DEBUG

and the pr_debug() calls turn from a NOP into a printk(KERN_DEBUG,...).

Ingo

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


[kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-04 Thread Yang, Sheng
From f389ff3bc38b1684f15d8073c6d9abd2f2c9 Mon Sep 17 00:00:00 2001
From: Sheng Yang [EMAIL PROTECTED]
Date: Mon, 28 Jan 2008 05:10:22 +0800
Subject: [PATCH] KVM: In kernel pit model

The patch moved PIT from userspace to kernel, and increase the timer accuracy 
greatly.

Signed-off-by: Sheng Yang [EMAIL PROTECTED]
---
 arch/x86/kvm/Makefile  |3 +-
 arch/x86/kvm/i8254.c   |  583 

 arch/x86/kvm/i8254.h   |   59 +
 arch/x86/kvm/irq.c |3 +
 arch/x86/kvm/x86.c |9 +
 include/asm-x86/kvm_host.h |1 +
 include/linux/kvm.h|2 +
 7 files changed, 659 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/kvm/i8254.c
 create mode 100644 arch/x86/kvm/i8254.h

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index ffdd0b3..4d0c22e 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -6,7 +6,8 @@ common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o 
ioapic.o)

 EXTRA_CFLAGS += -Ivirt/kvm -Iarch/x86/kvm

-kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o
+kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o \
+   i8254.o
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
new file mode 100644
index 000..6bbb254
--- /dev/null
+++ b/arch/x86/kvm/i8254.c
@@ -0,0 +1,583 @@
+/*
+ * 8253/8254 interval timer emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2006 Intel Corporation
+ * Copyright (c) 2007 Keir Fraser, XenSource Inc
+ * Copyright (c) 2008 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a 
copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the 
rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Authors:
+ *   Sheng Yang [EMAIL PROTECTED]
+ *   Based on QEMU and Xen.
+ */
+
+#include linux/kvm_host.h
+
+#include irq.h
+#include i8254.h
+
+#if 0
+#define pit_debug(fmt, arg...) printk(KERN_WARNING fmt, ##arg)
+#else
+#define pit_debug(fmt, arg...)
+#endif
+
+#ifndef CONFIG_X86_64
+#define mod_64(x, y) ((x) - (y) * div64_64(x, y))
+#else
+#define mod_64(x, y) ((x) % (y))
+#endif
+
+#define RW_STATE_LSB 1
+#define RW_STATE_MSB 2
+#define RW_STATE_WORD0 3
+#define RW_STATE_WORD1 4
+
+/* 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  0x)), c);
+   return res.ll;
+}
+
+static void pit_set_gate(struct kvm *kvm, int channel, u32 val)
+{
+   struct kvm_kpit_channel_state *c =
+   kvm-arch.vpit-pit_state.channels[channel];
+
+   ASSERT(mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   switch (c-mode) {
+   default:
+   case 0:
+   case 4:
+   /* XXX: just disable/enable counting */
+   break;
+   case 1:
+   case 2:
+   case 3:
+   case 5:
+   /* Restart counting on rising edge. */
+   if (c-gate  val)
+   c-count_load_time = ktime_get();
+   break;
+   }
+
+   c-gate = val;
+}
+
+int pit_get_gate(struct kvm *kvm, int channel)
+{
+   ASSERT(mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   return kvm-arch.vpit-pit_state.channels[channel].gate;
+}
+
+static int pit_get_count(struct kvm *kvm, int channel)
+{
+   struct kvm_kpit_channel_state *c =
+   kvm-arch.vpit-pit_state.channels[channel];
+   s64 d, t;
+   int counter;
+
+   ASSERT(mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   t = ktime_to_ns(ktime_sub(ktime_get(), c-count_load_time));
+   d = 

Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-04 Thread Avi Kivity
Yang, Sheng wrote:
 +
 +static int pit_get_out(struct kvm *kvm, int channel)
 +{
 + struct kvm_kpit_channel_state *c =
 + kvm-arch.vpit-pit_state.channels[channel];
 + s64 d, t;
 + int out;
 +
 + ASSERT(mutex_is_locked(kvm-arch.vpit-pit_state.lock));
 +
 + t = ktime_to_ns(ktime_sub(ktime_get(), c-count_load_time));
 + d = muldiv64(t, PIT_FREQ, 1e9);
   

NSECS_PER_SEC to avoid people jumping on you saying you can't use 
floating point in the kernel (yes, the compiler converts it at 
compile-time, but they'll still say it).

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.


-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/6] KVM: In kernel pit model

2008-03-04 Thread Yang, Sheng
On Wednesday 05 March 2008 14:54:06 Avi Kivity wrote:
 Yang, Sheng wrote:
  +
  +static int pit_get_out(struct kvm *kvm, int channel)
  +{
  +   struct kvm_kpit_channel_state *c =
  +   kvm-arch.vpit-pit_state.channels[channel];
  +   s64 d, t;
  +   int out;
  +
  +   ASSERT(mutex_is_locked(kvm-arch.vpit-pit_state.lock));
  +
  +   t = ktime_to_ns(ktime_sub(ktime_get(), c-count_load_time));
  +   d = muldiv64(t, PIT_FREQ, 1e9);

 NSECS_PER_SEC to avoid people jumping on you saying you can't use
 floating point in the kernel (yes, the compiler converts it at
 compile-time, but they'll still say it).

Sorry... I remembered I've modified it, seems something got wrong...

Here is the updated patch:

---
 arch/x86/kvm/Makefile  |3 +-
 arch/x86/kvm/i8254.c   |  583 

 arch/x86/kvm/i8254.h   |   59 +
 arch/x86/kvm/irq.c |3 +
 arch/x86/kvm/x86.c |9 +
 include/asm-x86/kvm_host.h |1 +
 include/linux/kvm.h|2 +
 7 files changed, 659 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/kvm/i8254.c
 create mode 100644 arch/x86/kvm/i8254.h

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index ffdd0b3..4d0c22e 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -6,7 +6,8 @@ common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o 
ioapic.o)

 EXTRA_CFLAGS += -Ivirt/kvm -Iarch/x86/kvm

-kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o
+kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o \
+   i8254.o
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
new file mode 100644
index 000..84fb3d9
--- /dev/null
+++ b/arch/x86/kvm/i8254.c
@@ -0,0 +1,583 @@
+/*
+ * 8253/8254 interval timer emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2006 Intel Corporation
+ * Copyright (c) 2007 Keir Fraser, XenSource Inc
+ * Copyright (c) 2008 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a 
copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the 
rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Authors:
+ *   Sheng Yang [EMAIL PROTECTED]
+ *   Based on QEMU and Xen.
+ */
+
+#include linux/kvm_host.h
+
+#include irq.h
+#include i8254.h
+
+#if 1
+#define pit_debug(fmt, arg...) printk(KERN_WARNING fmt, ##arg)
+#else
+#define pit_debug(fmt, arg...)
+#endif
+
+#ifndef CONFIG_X86_64
+#define mod_64(x, y) ((x) - (y) * div64_64(x, y))
+#else
+#define mod_64(x, y) ((x) % (y))
+#endif
+
+#define RW_STATE_LSB 1
+#define RW_STATE_MSB 2
+#define RW_STATE_WORD0 3
+#define RW_STATE_WORD1 4
+
+/* 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  0x)), c);
+   return res.ll;
+}
+
+static void pit_set_gate(struct kvm *kvm, int channel, u32 val)
+{
+   struct kvm_kpit_channel_state *c =
+   kvm-arch.vpit-pit_state.channels[channel];
+
+   ASSERT(mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   switch (c-mode) {
+   default:
+   case 0:
+   case 4:
+   /* XXX: just disable/enable counting */
+   break;
+   case 1:
+   case 2:
+   case 3:
+   case 5:
+   /* Restart counting on rising edge. */
+   if (c-gate  val)
+   c-count_load_time = ktime_get();
+   break;
+   }
+
+   c-gate = val;
+}
+
+int pit_get_gate(struct kvm *kvm, int channel)
+{
+   ASSERT(mutex_is_locked(kvm-arch.vpit-pit_state.lock));
+
+   return