[kvm-devel] [PATCH][QEMU] Use a separate device for in-kernel PIT

2008-03-12 Thread Anthony Liguori 
Part of the feedback we received from Fabrice about the KVM patches for QEMU
is that we should create a separate device for the in-kernel APIC to avoid
having lots of if (kvm_enabled()) within the APIC code that were difficult to
understand why there were needed.

This patch separates the in-kernel PIT into a separate device.  It also
introduces some configure logic to only compile in support for the in-kernel
PIT if it's available.

The result of this is that we now only need a single if (kvm_enabled()) to
determine which device to use.  Besides making it more upstream friendly, I
think this makes the code much easier to understand.

Signed-off-by: Anthony Liguori <[EMAIL PROTECTED]>

diff --git a/qemu/Makefile.target b/qemu/Makefile.target
index 67a433e..2579688 100644
--- a/qemu/Makefile.target
+++ b/qemu/Makefile.target
@@ -585,6 +585,9 @@ OBJS+= ide.o pckbd.o ps2.o vga.o $(SOUND_HW) dma.o
 OBJS+= fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
 OBJS+= cirrus_vga.o apic.o parallel.o acpi.o piix_pci.o
 OBJS+= usb-uhci.o vmmouse.o vmport.o vmware_vga.o extboot.o
+ifeq ($(USE_KVM_PIT), 1)
+OBJS+= i8254-kvm.o
+endif
 CPPFLAGS += -DHAS_AUDIO -DHAS_AUDIO_CHOICE
 endif
 ifeq ($(TARGET_BASE_ARCH), ia64)
diff --git a/qemu/configure b/qemu/configure
index bbedddc..22f44c8 100755
--- a/qemu/configure
+++ b/qemu/configure
@@ -100,6 +100,7 @@ bsd="no"
 linux="no"
 kqemu="no"
 kvm="no"
+kvm_cap_pit="no"
 profiler="no"
 kernel_path=""
 cocoa="no"
@@ -612,6 +613,22 @@ int main(void) {
 EOF
 
 ##
+# KVM probe
+
+if test "$kvm" = "yes" ; then
+cat > $TMPC <
+#ifndef KVM_CAP_PIT
+#error "kvm no pit capability"
+#endif
+int main(void) { return 0; }
+EOF
+if $cc $ARCH_CFLAGS -o $TMPE ${OS_CFLAGS} $TMPC 2> /dev/null ; then
+   kvm_cap_pit="yes"
+fi
+fi
+
+##
 # SDL probe
 
 sdl_too_old=no
@@ -1136,6 +1153,9 @@ configure_kvm() {
 echo "#define USE_KVM 1" >> $config_h
 echo "USE_KVM=1" >> $config_mak
 echo "CONFIG_KVM_KERNEL_INC=$kernel_path/include" >> $config_mak
+if test $kvm_cap_pit = "yes" ; then
+   echo "USE_KVM_PIT=1" >> $config_mak
+fi
 disable_cpu_emulation
   fi
 }
diff --git a/qemu/hw/i8254-kvm.c b/qemu/hw/i8254-kvm.c
new file mode 100644
index 000..f0669cb
--- /dev/null
+++ b/qemu/hw/i8254-kvm.c
@@ -0,0 +1,178 @@
+/*
+ * QEMU 8253/8254 interval timer emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * 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.
+ */
+#include "hw.h"
+#include "pc.h"
+#include "isa.h"
+#include "i8254.h"
+
+#include "qemu-kvm.h"
+
+static PITState pit_state;
+
+static void kvm_kernel_pit_save_to_user(PITState *s)
+{
+struct kvm_pit_state pit;
+struct kvm_pit_channel_state *c;
+struct PITChannelState *sc;
+int i;
+
+kvm_get_pit(kvm_context, &pit);
+
+for (i = 0; i < 3; i++) {
+   c = &pit.channels[i];
+   sc = &s->channels[i];
+   sc->count = c->count;
+   sc->latched_count = c->latched_count;
+   sc->count_latched = c->count_latched;
+   sc->status_latched = c->status_latched;
+   sc->status = c->status;
+   sc->read_state = c->read_state;
+   sc->write_state = c->write_state;
+   sc->write_latch = c->write_latch;
+   sc->rw_mode = c->rw_mode;
+   sc->mode = c->mode;
+   sc->bcd = c->bcd;
+   sc->gate = c->gate;
+   sc->count_load_time = c->count_load_time;
+}
+}
+
+static void kvm_kernel_pit_load_from_user(PITState *s)
+{
+struct kvm_pit_state pit;
+struct kvm_pit_channel_state *c;
+struct PITChannelState *sc;
+int i;
+
+for (i = 0; i < 3; i++) {
+   c = &pit.channels[i];
+   sc = &s->channels[i];
+   c->count = sc->count;
+   c->latched_count = sc->latched_count;
+   c->count_latched = sc->count_latched;
+   c->status_latched = sc->status_latched;
+   c->status = sc->status;
+   c->read_sta

Re: [kvm-devel] [PATCH][QEMU] Use a separate device for in-kernel PIT

2008-03-12 Thread Dor Laor 

On Wed, 2008-03-12 at 12:38 -0500, Anthony Liguori wrote:
> Part of the feedback we received from Fabrice about the KVM patches
> for QEMU
> is that we should create a separate device for the in-kernel APIC to
> avoid
> having lots of if (kvm_enabled()) within the APIC code that were
> difficult to
> understand why there were needed.
> 
> This patch separates the in-kernel PIT into a separate device.  It
> also
> introduces some configure logic to only compile in support for the
> in-kernel
> PIT if it's available.
> 
> The result of this is that we now only need a single if
> (kvm_enabled()) to
> determine which device to use.  Besides making it more upstream
> friendly, I
> think this makes the code much easier to understand.
> 

Seems like a good idea.

[snip]
..



> +static void pit_reset(void *opaque)
> +{
> +PITState *pit = opaque;
> +PITChannelState *s;
> +int i;
> +
> +for(i = 0;i < 3; i++) {
> +s = &pit->channels[i];
> +s->mode = 3;
> +s->gate = (i != 2);
> +pit_load_count(s, 0);
> +}
> +}
> +

Seems like pit_reset won't change the in-kernel pit state.
Actually this should handled as a part of more general reset ioctl to
all of kvm's in-kernel devices.

Cheers,
Dor


-
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][QEMU] Use a separate device for in-kernel PIT

2008-03-12 Thread Anthony Liguori 
Dor Laor wrote:
> On Wed, 2008-03-12 at 12:38 -0500, Anthony Liguori wrote:
>   
>> Part of the feedback we received from Fabrice about the KVM patches
>> for QEMU
>> is that we should create a separate device for the in-kernel APIC to
>> avoid
>> having lots of if (kvm_enabled()) within the APIC code that were
>> difficult to
>> understand why there were needed.
>>
>> This patch separates the in-kernel PIT into a separate device.  It
>> also
>> introduces some configure logic to only compile in support for the
>> in-kernel
>> PIT if it's available.
>>
>> The result of this is that we now only need a single if
>> (kvm_enabled()) to
>> determine which device to use.  Besides making it more upstream
>> friendly, I
>> think this makes the code much easier to understand.
>>
>> 
>
> Seems like a good idea.
>
> [snip]
> ..
>
>
>
>   
>> +static void pit_reset(void *opaque)
>> +{
>> +PITState *pit = opaque;
>> +PITChannelState *s;
>> +int i;
>> +
>> +for(i = 0;i < 3; i++) {
>> +s = &pit->channels[i];
>> +s->mode = 3;
>> +s->gate = (i != 2);
>> +pit_load_count(s, 0);
>> +}
>> +}
>> +
>> 
>
> Seems like pit_reset won't change the in-kernel pit state.
>   

Yeah, that seemed suspicious to me too.  I didn't want to change the 
existing behavior though for fear of introducing regressions.  Perhaps 
Sheng can comment on whether it's necessary to even have a reset handler 
in userspace?

Regards,

Anthony Liguori

> Actually this should handled as a part of more general reset ioctl to
> all of kvm's in-kernel devices.
>
> Cheers,
> Dor
>
>   


-
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][QEMU] Use a separate device for in-kernel PIT

2008-03-12 Thread Yang, Sheng 
On Thursday 13 March 2008 06:13:48 Anthony Liguori wrote:
> Dor Laor wrote:
> > On Wed, 2008-03-12 at 12:38 -0500, Anthony Liguori wrote:
> >> Part of the feedback we received from Fabrice about the KVM patches
> >> for QEMU
> >> is that we should create a separate device for the in-kernel APIC to
> >> avoid
> >> having lots of if (kvm_enabled()) within the APIC code that were
> >> difficult to
> >> understand why there were needed.
> >>
> >> This patch separates the in-kernel PIT into a separate device.  It
> >> also
> >> introduces some configure logic to only compile in support for the
> >> in-kernel
> >> PIT if it's available.
> >>
> >> The result of this is that we now only need a single if
> >> (kvm_enabled()) to
> >> determine which device to use.  Besides making it more upstream
> >> friendly, I
> >> think this makes the code much easier to understand.
> >
> > Seems like a good idea.
> >
> > [snip]
> > ..
> >
> >> +static void pit_reset(void *opaque)
> >> +{
> >> +PITState *pit = opaque;
> >> +PITChannelState *s;
> >> +int i;
> >> +
> >> +for(i = 0;i < 3; i++) {
> >> +s = &pit->channels[i];
> >> +s->mode = 3;
> >> +s->gate = (i != 2);
> >> +pit_load_count(s, 0);
> >> +}
> >> +}
> >> +
> >
> > Seems like pit_reset won't change the in-kernel pit state.
>
> Yeah, that seemed suspicious to me too.  I didn't want to change the
> existing behavior though for fear of introducing regressions.  Perhaps
> Sheng can comment on whether it's necessary to even have a reset handler
> in userspace?

IMO the reset support is needed even it would introduce regression (e would 
finally solve the regression if exists, won't we? :) ).

And we got two choices in userspace: one ioctl to reset all kvm devices, or 
one ioctl for each device. For we are separating in kernel device into 
separate devices, seems the later is more proper. But would it bring other 
troubles like inconsistent state for smp? 

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][QEMU] Use a separate device for in-kernel PIT

2008-03-16 Thread Avi Kivity 
Anthony Liguori wrote:
> Part of the feedback we received from Fabrice about the KVM patches for QEMU
> is that we should create a separate device for the in-kernel APIC to avoid
> having lots of if (kvm_enabled()) within the APIC code that were difficult to
> understand why there were needed.
>
> This patch separates the in-kernel PIT into a separate device.  It also
> introduces some configure logic to only compile in support for the in-kernel
> PIT if it's available.
>
> The result of this is that we now only need a single if (kvm_enabled()) to
> determine which device to use.  Besides making it more upstream friendly, I
> think this makes the code much easier to understand.
>
>   

It introduces a new issue, the save/restore format is effectively forked 
and will have to be maintained in parallel if there are any changes.

Perhaps keep in the same file, but as two separate devices that can 
share the save/restore code?

-- 
error compiling committee.c: too many arguments to function


-
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][QEMU] Use a separate device for in-kernel PIT

2008-03-16 Thread Avi Kivity 
Yang, Sheng wrote:
> And we got two choices in userspace: one ioctl to reset all kvm devices, or 
> one ioctl for each device. For we are separating in kernel device into 
> separate devices, seems the later is more proper. But would it bring other 
> troubles like inconsistent state for smp? 
>
>   

I agree that a separate ioctl would introduce smp problems, so I think 
one ioctl that resets all devices and all vcpus is the way to go.

-- 
error compiling committee.c: too many arguments to function


-
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][QEMU] Use a separate device for in-kernel PIT

2008-03-21 Thread Hollis Blanchard 
On Wed, 2008-03-12 at 12:38 -0500, Anthony Liguori wrote:
> Part of the feedback we received from Fabrice about the KVM patches
> for QEMU
> is that we should create a separate device for the in-kernel APIC to
> avoid
> having lots of if (kvm_enabled()) within the APIC code that were
> difficult to
> understand why there were needed.
> 
> This patch separates the in-kernel PIT into a separate device.  It
> also
> introduces some configure logic to only compile in support for the
> in-kernel
> PIT if it's available.
> 
> The result of this is that we now only need a single if
> (kvm_enabled()) to
> determine which device to use.  Besides making it more upstream
> friendly, I
> think this makes the code much easier to understand.
> 
> Signed-off-by: Anthony Liguori <[EMAIL PROTECTED]>

This patch solves annoying qemu build breakage hitting PowerPC around
struct kvm_pit_state, so that's another vote in favor...

-- 
Hollis Blanchard
IBM Linux Technology Center


-
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][QEMU] Use a separate device for in-kernel PIT

2008-03-21 Thread Anthony Liguori 

Hollis Blanchard wrote:

This patch solves annoying qemu build breakage hitting PowerPC around
struct kvm_pit_state, so that's another vote in favor...
  


I have an updated version of the patch but it's breaking the build b/c 
something fouled up right now with configure.  libkvm pulls in 
linux/kvm.h which wants to pull in linux/compiler.h.  We don't ship a 
linux/compiler.h though so it's pulling from /usr/include/linux which on 
my system doesn't have a compiler.h.


The lack of this header is causing the configure test to fail.  I've 
attached the patch here for you to use and I'll send it out again once I 
figure out the fix for this linux/compiler.h.


Regards,

Anthony Liguori




qemu:cleanup_pit.patch
Description: application/mbox
-
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][QEMU] Use a separate device for in-kernel PIT

2008-03-23 Thread Avi Kivity 
Anthony Liguori wrote:
> Hollis Blanchard wrote:
>> This patch solves annoying qemu build breakage hitting PowerPC around
>> struct kvm_pit_state, so that's another vote in favor...
>>   
>
> I have an updated version of the patch but it's breaking the build b/c 
> something fouled up right now with configure.  libkvm pulls in 
> linux/kvm.h which wants to pull in linux/compiler.h.  We don't ship a 
> linux/compiler.h though so it's pulling from /usr/include/linux which 
> on my system doesn't have a compiler.h.
>
> The lack of this header is causing the configure test to fail.  I've 
> attached the patch here for you to use and I'll send it out again once 
> I figure out the fix for this linux/compiler.h.
>

The patch suffers from the same problem as the apic split; the 
save/restore code is needlessly duplicated.

-- 
error compiling committee.c: too many arguments to function


-
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][QEMU] Use a separate device for in-kernel PIT

2008-03-23 Thread Anthony Liguori 
Avi Kivity wrote:
> Anthony Liguori wrote:
>   
>> Hollis Blanchard wrote:
>> 
>>> This patch solves annoying qemu build breakage hitting PowerPC around
>>> struct kvm_pit_state, so that's another vote in favor...
>>>   
>>>   
>> I have an updated version of the patch but it's breaking the build b/c 
>> something fouled up right now with configure.  libkvm pulls in 
>> linux/kvm.h which wants to pull in linux/compiler.h.  We don't ship a 
>> linux/compiler.h though so it's pulling from /usr/include/linux which 
>> on my system doesn't have a compiler.h.
>>
>> The lack of this header is causing the configure test to fail.  I've 
>> attached the patch here for you to use and I'll send it out again once 
>> I figure out the fix for this linux/compiler.h.
>>
>> 
>
> The patch suffers from the same problem as the apic split; the 
> save/restore code is needlessly duplicated.
>   

The updated patch addresses this problem.  I have to fix the 
linux/compiler.h issue first though before it can be applied or it will 
break the build.

Regards,

Anthony Liguori



-
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][QEMU] Use a separate device for in-kernel PIT (v2)

2008-03-24 Thread Anthony Liguori 
Part of the feedback we received from Fabrice about the KVM patches for QEMU
is that we should create a separate device for the in-kernel APIC to avoid
having lots of if (kvm_enabled()) within the APIC code that were difficult to
understand why there were needed.

This patch separates the in-kernel PIT into a separate device.  It also
introduces some configure logic to only compile in support for the in-kernel
PIT if it's available.

The result of this is that we now only need a single if (kvm_enabled()) to
determine which device to use.  Besides making it more upstream friendly, I
think this makes the code much easier to understand.

Since v1=>v2, we make sure to use common code for save/restore between
in-kernel pit and in-qemu pit.

This patch also fixes the build for PPC.

Signed-off-by: Anthony Liguori <[EMAIL PROTECTED]>

diff --git a/qemu/Makefile.target b/qemu/Makefile.target
index e29bbeb..a947147 100644
--- a/qemu/Makefile.target
+++ b/qemu/Makefile.target
@@ -587,6 +587,9 @@ OBJS+= ide.o pckbd.o ps2.o vga.o $(SOUND_HW) dma.o
 OBJS+= fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
 OBJS+= cirrus_vga.o apic.o parallel.o acpi.o piix_pci.o
 OBJS+= usb-uhci.o vmmouse.o vmport.o vmware_vga.o extboot.o
+ifeq ($(USE_KVM_PIT), 1)
+OBJS+= i8254-kvm.o
+endif
 CPPFLAGS += -DHAS_AUDIO -DHAS_AUDIO_CHOICE
 endif
 ifeq ($(TARGET_BASE_ARCH), ia64)
diff --git a/qemu/configure b/qemu/configure
index bbedddc..bfbbae9 100755
--- a/qemu/configure
+++ b/qemu/configure
@@ -100,6 +100,7 @@ bsd="no"
 linux="no"
 kqemu="no"
 kvm="no"
+kvm_cap_pit="no"
 profiler="no"
 kernel_path=""
 cocoa="no"
@@ -612,6 +613,22 @@ int main(void) {
 EOF
 
 ##
+# KVM probe
+
+if test "$kvm" = "yes" ; then
+cat > $TMPC <
+#ifndef KVM_CAP_PIT
+#error "kvm no pit capability"
+#endif
+int main(void) { return 0; }
+EOF
+if $cc $ARCH_CFLAGS $CFLAGS -I"$kernel_path"/include -o $TMPE ${OS_CFLAGS} 
$TMPC 2> /dev/null ; then
+   kvm_cap_pit="yes"
+fi
+fi
+
+##
 # SDL probe
 
 sdl_too_old=no
@@ -1136,6 +1153,9 @@ configure_kvm() {
 echo "#define USE_KVM 1" >> $config_h
 echo "USE_KVM=1" >> $config_mak
 echo "CONFIG_KVM_KERNEL_INC=$kernel_path/include" >> $config_mak
+if test $kvm_cap_pit = "yes" ; then
+   echo "USE_KVM_PIT=1" >> $config_mak
+fi
 disable_cpu_emulation
   fi
 }
diff --git a/qemu/hw/i8254-kvm.c b/qemu/hw/i8254-kvm.c
new file mode 100644
index 000..b40af4a
--- /dev/null
+++ b/qemu/hw/i8254-kvm.c
@@ -0,0 +1,108 @@
+/*
+ * QEMU 8253/8254 interval timer emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * 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.
+ */
+#include "hw.h"
+#include "pc.h"
+#include "isa.h"
+#include "i8254.h"
+
+#include "qemu-kvm.h"
+
+static PITState pit_state;
+
+static void kvm_pit_save(QEMUFile *f, void *opaque)
+{
+PITState *s = opaque;
+struct kvm_pit_state pit;
+struct kvm_pit_channel_state *c;
+struct PITChannelState *sc;
+int i;
+
+kvm_get_pit(kvm_context, &pit);
+
+for (i = 0; i < 3; i++) {
+   c = &pit.channels[i];
+   sc = &s->channels[i];
+   sc->count = c->count;
+   sc->latched_count = c->latched_count;
+   sc->count_latched = c->count_latched;
+   sc->status_latched = c->status_latched;
+   sc->status = c->status;
+   sc->read_state = c->read_state;
+   sc->write_state = c->write_state;
+   sc->write_latch = c->write_latch;
+   sc->rw_mode = c->rw_mode;
+   sc->mode = c->mode;
+   sc->bcd = c->bcd;
+   sc->gate = c->gate;
+   sc->count_load_time = c->count_load_time;
+}
+
+pit_save(f, s);
+}
+
+static int kvm_pit_load(QEMUFile *f, void *opaque, int version_id)
+{
+PITState *s = opaque;
+struct kvm_pit_state pit;
+struct kvm_pit_channel_state *c;
+struct PITChannelState *sc;
+int i;
+
+pit_load(f, s, version_id);
+
+for (

Re: [kvm-devel] [PATCH][QEMU] Use a separate device for in-kernel PIT (v2)

2008-03-24 Thread Jerone Young 
Fixes issues with pit for PPC.

Acked-by: Jerone Young <[EMAIL PROTECTED]>

On Mon, 2008-03-24 at 13:54 -0500, Anthony Liguori wrote:
> Part of the feedback we received from Fabrice about the KVM patches for QEMU
> is that we should create a separate device for the in-kernel APIC to avoid
> having lots of if (kvm_enabled()) within the APIC code that were difficult to
> understand why there were needed.
> 
> This patch separates the in-kernel PIT into a separate device.  It also
> introduces some configure logic to only compile in support for the in-kernel
> PIT if it's available.
> 
> The result of this is that we now only need a single if (kvm_enabled()) to
> determine which device to use.  Besides making it more upstream friendly, I
> think this makes the code much easier to understand.
> 
> Since v1=>v2, we make sure to use common code for save/restore between
> in-kernel pit and in-qemu pit.
> 
> This patch also fixes the build for PPC.
> 
> Signed-off-by: Anthony Liguori <[EMAIL PROTECTED]>
> 
> diff --git a/qemu/Makefile.target b/qemu/Makefile.target
> index e29bbeb..a947147 100644
> --- a/qemu/Makefile.target
> +++ b/qemu/Makefile.target
> @@ -587,6 +587,9 @@ OBJS+= ide.o pckbd.o ps2.o vga.o $(SOUND_HW) dma.o
>  OBJS+= fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
>  OBJS+= cirrus_vga.o apic.o parallel.o acpi.o piix_pci.o
>  OBJS+= usb-uhci.o vmmouse.o vmport.o vmware_vga.o extboot.o
> +ifeq ($(USE_KVM_PIT), 1)
> +OBJS+= i8254-kvm.o
> +endif
>  CPPFLAGS += -DHAS_AUDIO -DHAS_AUDIO_CHOICE
>  endif
>  ifeq ($(TARGET_BASE_ARCH), ia64)
> diff --git a/qemu/configure b/qemu/configure
> index bbedddc..bfbbae9 100755
> --- a/qemu/configure
> +++ b/qemu/configure
> @@ -100,6 +100,7 @@ bsd="no"
>  linux="no"
>  kqemu="no"
>  kvm="no"
> +kvm_cap_pit="no"
>  profiler="no"
>  kernel_path=""
>  cocoa="no"
> @@ -612,6 +613,22 @@ int main(void) {
>  EOF
> 
>  ##
> +# KVM probe
> +
> +if test "$kvm" = "yes" ; then
> +cat > $TMPC < +#include 
> +#ifndef KVM_CAP_PIT
> +#error "kvm no pit capability"
> +#endif
> +int main(void) { return 0; }
> +EOF
> +if $cc $ARCH_CFLAGS $CFLAGS -I"$kernel_path"/include -o $TMPE 
> ${OS_CFLAGS} $TMPC 2> /dev/null ; then
> + kvm_cap_pit="yes"
> +fi
> +fi
> +
> +##
>  # SDL probe
> 
>  sdl_too_old=no
> @@ -1136,6 +1153,9 @@ configure_kvm() {
>  echo "#define USE_KVM 1" >> $config_h
>  echo "USE_KVM=1" >> $config_mak
>  echo "CONFIG_KVM_KERNEL_INC=$kernel_path/include" >> $config_mak
> +if test $kvm_cap_pit = "yes" ; then
> + echo "USE_KVM_PIT=1" >> $config_mak
> +fi
>  disable_cpu_emulation
>fi
>  }
> diff --git a/qemu/hw/i8254-kvm.c b/qemu/hw/i8254-kvm.c
> new file mode 100644
> index 000..b40af4a
> --- /dev/null
> +++ b/qemu/hw/i8254-kvm.c
> @@ -0,0 +1,108 @@
> +/*
> + * QEMU 8253/8254 interval timer emulation
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + *
> + * 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.
> + */
> +#include "hw.h"
> +#include "pc.h"
> +#include "isa.h"
> +#include "i8254.h"
> +
> +#include "qemu-kvm.h"
> +
> +static PITState pit_state;
> +
> +static void kvm_pit_save(QEMUFile *f, void *opaque)
> +{
> +PITState *s = opaque;
> +struct kvm_pit_state pit;
> +struct kvm_pit_channel_state *c;
> +struct PITChannelState *sc;
> +int i;
> +
> +kvm_get_pit(kvm_context, &pit);
> +
> +for (i = 0; i < 3; i++) {
> + c = &pit.channels[i];
> + sc = &s->channels[i];
> + sc->count = c->count;
> + sc->latched_count = c->latched_count;
> + sc->count_latched = c->count_latched;
> + sc->status_latched = c->status_latched;
> + sc->status = c->status;
> + sc->read_state = c->read_state;
> + sc->write_state = c->write_state;
> + sc->write_latch = c->write_latch;
> + sc->rw_mode = c->rw_mode;
> + sc->mode = c-

Re: [kvm-devel] [PATCH][QEMU] Use a separate device for in-kernel PIT (v2)

2008-03-25 Thread Avi Kivity 
Anthony Liguori wrote:
> Part of the feedback we received from Fabrice about the KVM patches for QEMU
> is that we should create a separate device for the in-kernel APIC to avoid
> having lots of if (kvm_enabled()) within the APIC code that were difficult to
> understand why there were needed.
>
> This patch separates the in-kernel PIT into a separate device.  It also
> introduces some configure logic to only compile in support for the in-kernel
> PIT if it's available.
>
> The result of this is that we now only need a single if (kvm_enabled()) to
> determine which device to use.  Besides making it more upstream friendly, I
> think this makes the code much easier to understand.
>
> Since v1=>v2, we make sure to use common code for save/restore between
> in-kernel pit and in-qemu pit.
>
>   

Please separate the code movement and changes into separate patches.

-- 
error compiling committee.c: too many arguments to function


-
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][QEMU] Use a separate device for in-kernel PIT (v2)

2008-03-25 Thread Anthony Liguori 
Avi Kivity wrote:
>
> Please separate the code movement and changes into separate patches.

I'm not sure there's a great way to do this that preserves bisectability 
and results in meaningful history.  I could leave the #ifdef's in 
i8254-kvm.c and then have a second patch that removes them.  That 
doesn't seem to be terribly valuable though from a history perspective 
as it still requires a bit of code change to keep things working.  Were 
you thinking of something else?

Regards,

Anthony Liguori



-
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [kvm-devel] [PATCH][QEMU] Use a separate device for in-kernel PIT (v2)

2008-03-26 Thread Avi Kivity 
Anthony Liguori wrote:
> Avi Kivity wrote:
>>
>> Please separate the code movement and changes into separate patches.
>
> I'm not sure there's a great way to do this that preserves 
> bisectability and results in meaningful history.  I could leave the 
> #ifdef's in i8254-kvm.c and then have a second patch that removes 
> them.  That doesn't seem to be terribly valuable though from a history 
> perspective as it still requires a bit of code change to keep things 
> working.  Were you thinking of something else?
>

Nothing exotic.  The patch moves some structure declarations to header 
files, that can be split off so we have one patch that paves the way 
(doing nothing but code movement) and the other actually introduces the 
new device, only adding new code (and possibly exporting functions).

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


-
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel