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

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

[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 <aliguori@us.ibm.com>

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 <<EOF
+#include <libkvm.h>
+#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 0000000..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 (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_state = sc->read_state;
+	c->write_state = sc->write_state;
+	c->write_latch = sc->write_latch;
+	c->rw_mode = sc->rw_mode;
+	c->mode = sc->mode;
+	c->bcd = sc->bcd;
+	c->gate = sc->gate;
+	c->count_load_time = sc->count_load_time;
+    }
+
+    kvm_set_pit(kvm_context, &pit);
+
+    return 0;
+}
+
+PITState *kvm_pit_init(int base, qemu_irq irq)
+{
+    PITState *pit = &pit_state;
+
+    register_savevm(PIT_SAVEVM_NAME, base, PIT_SAVEVM_VERSION,
+		    kvm_pit_save, kvm_pit_load, pit);
+
+    qemu_register_reset(pit_reset, pit);
+    pit_reset(pit);
+
+    return pit;
+}
diff --git a/qemu/hw/i8254.c b/qemu/hw/i8254.c
index e215f8b..611c5f4 100644
--- a/qemu/hw/i8254.c
+++ b/qemu/hw/i8254.c
@@ -22,43 +22,12 @@
  * THE SOFTWARE.
  */
 #include "hw.h"
-#include "pc.h"
 #include "isa.h"
 #include "qemu-timer.h"
-
-#include "qemu-kvm.h"
+#include "i8254.h"
 
 //#define DEBUG_PIT
 
-#define RW_STATE_LSB 1
-#define RW_STATE_MSB 2
-#define RW_STATE_WORD0 3
-#define RW_STATE_WORD1 4
-
-typedef struct PITChannelState {
-    int count; /* can be 65536 */
-    uint16_t latched_count;
-    uint8_t count_latched;
-    uint8_t status_latched;
-    uint8_t status;
-    uint8_t read_state;
-    uint8_t write_state;
-    uint8_t write_latch;
-    uint8_t rw_mode;
-    uint8_t mode;
-    uint8_t bcd; /* not supported */
-    uint8_t gate; /* timer start */
-    int64_t count_load_time;
-    /* irq handling */
-    int64_t next_transition_time;
-    QEMUTimer *irq_timer;
-    qemu_irq irq;
-} PITChannelState;
-
-struct PITState {
-    PITChannelState channels[3];
-};
-
 static PITState pit_state;
 
 static void pit_irq_timer_update(PITChannelState *s, int64_t current_time);
@@ -414,78 +383,12 @@ static void pit_irq_timer(void *opaque)
     pit_irq_timer_update(s, s->next_transition_time);
 }
 
-#ifdef KVM_CAP_PIT
-
-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_state = sc->read_state;
-	c->write_state = sc->write_state;
-	c->write_latch = sc->write_latch;
-	c->rw_mode = sc->rw_mode;
-	c->mode = sc->mode;
-	c->bcd = sc->bcd;
-	c->gate = sc->gate;
-	c->count_load_time = sc->count_load_time;
-    }
-
-    kvm_set_pit(kvm_context, &pit);
-}
-
-#endif
-
-static void pit_save(QEMUFile *f, void *opaque)
+void pit_save(QEMUFile *f, void *opaque)
 {
     PITState *pit = opaque;
     PITChannelState *s;
     int i;
 
-#ifdef KVM_CAP_PIT
-    if (kvm_enabled() && qemu_kvm_pit_in_kernel()) {
-        kvm_kernel_pit_save_to_user(pit);
-    }
-#endif
-
     for(i = 0; i < 3; i++) {
         s = &pit->channels[i];
         qemu_put_be32(f, s->count);
@@ -508,7 +411,7 @@ static void pit_save(QEMUFile *f, void *opaque)
     }
 }
 
-static int pit_load(QEMUFile *f, void *opaque, int version_id)
+int pit_load(QEMUFile *f, void *opaque, int version_id)
 {
     PITState *pit = opaque;
     PITChannelState *s;
@@ -537,17 +440,10 @@ static int pit_load(QEMUFile *f, void *opaque, int version_id)
             qemu_get_timer(f, s->irq_timer);
         }
     }
-
-#ifdef KVM_CAP_PIT
-    if (kvm_enabled() && qemu_kvm_pit_in_kernel()) {
-        kvm_kernel_pit_load_from_user(pit);
-    }
-#endif
-
     return 0;
 }
 
-static void pit_reset(void *opaque)
+void pit_reset(void *opaque)
 {
     PITState *pit = opaque;
     PITChannelState *s;
@@ -566,14 +462,13 @@ PITState *pit_init(int base, qemu_irq irq)
     PITState *pit = &pit_state;
     PITChannelState *s;
 
-    if (!kvm_enabled() || !qemu_kvm_pit_in_kernel()) {
-	    s = &pit->channels[0];
-	    /* the timer 0 is connected to an IRQ */
-	    s->irq_timer = qemu_new_timer(vm_clock, pit_irq_timer, s);
-	    s->irq = irq;
-    }
+    s = &pit->channels[0];
+    /* the timer 0 is connected to an IRQ */
+    s->irq_timer = qemu_new_timer(vm_clock, pit_irq_timer, s);
+    s->irq = irq;
 
-    register_savevm("i8254", base, 1, pit_save, pit_load, pit);
+    register_savevm(PIT_SAVEVM_NAME, base, PIT_SAVEVM_VERSION,
+		    pit_save, pit_load, pit);
 
     qemu_register_reset(pit_reset, pit);
     register_ioport_write(base, 4, 1, pit_ioport_write, pit);
diff --git a/qemu/hw/i8254.h b/qemu/hw/i8254.h
new file mode 100644
index 0000000..5f44abd
--- /dev/null
+++ b/qemu/hw/i8254.h
@@ -0,0 +1,46 @@
+#ifndef _QEMU_I8254_H
+#define _QEMU_I8254_H
+
+#include "hw.h"
+#include "pc.h"
+#include "qemu-timer.h"
+
+#define RW_STATE_LSB 1
+#define RW_STATE_MSB 2
+#define RW_STATE_WORD0 3
+#define RW_STATE_WORD1 4
+
+typedef struct PITChannelState {
+    int count; /* can be 65536 */
+    uint16_t latched_count;
+    uint8_t count_latched;
+    uint8_t status_latched;
+    uint8_t status;
+    uint8_t read_state;
+    uint8_t write_state;
+    uint8_t write_latch;
+    uint8_t rw_mode;
+    uint8_t mode;
+    uint8_t bcd; /* not supported */
+    uint8_t gate; /* timer start */
+    int64_t count_load_time;
+    /* irq handling */
+    int64_t next_transition_time;
+    QEMUTimer *irq_timer;
+    qemu_irq irq;
+} PITChannelState;
+
+struct PITState {
+    PITChannelState channels[3];
+};
+
+#define PIT_SAVEVM_NAME		"i8254"
+#define PIT_SAVEVM_VERSION	1
+
+void pit_save(QEMUFile *f, void *opaque);
+
+int pit_load(QEMUFile *f, void *opaque, int version_id);
+
+void pit_reset(void *opaque);
+
+#endif
diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c
index 0d2e6c3..97b108a 100644
--- a/qemu/hw/pc.c
+++ b/qemu/hw/pc.c
@@ -983,7 +983,10 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
     if (pci_enabled) {
         ioapic = ioapic_init();
     }
-    pit = pit_init(0x40, i8259[0]);
+    if (kvm_enabled() && qemu_kvm_pit_in_kernel())
+	pit = kvm_pit_init(0x40, i8259[0]);
+    else
+	pit = pit_init(0x40, i8259[0]);
     pcspk_init(pit);
     if (pci_enabled) {
         pic_set_alt_irq_func(isa_pic, ioapic_set_irq, ioapic);
diff --git a/qemu/hw/pc.h b/qemu/hw/pc.h
index 453c641..fb6c07d 100644
--- a/qemu/hw/pc.h
+++ b/qemu/hw/pc.h
@@ -58,6 +58,10 @@ int pit_get_initial_count(PITState *pit, int channel);
 int pit_get_mode(PITState *pit, int channel);
 int pit_get_out(PITState *pit, int channel, int64_t current_time);
 
+/* i8254-kvm.c */
+
+PITState *kvm_pit_init(int base, qemu_irq irq);
+
 /* vmport.c */
 void vmport_init(CPUState *env);
 void vmport_register(unsigned char command, IOPortReadFunc *func, void *opaque);

-------------------------------------------------------------------------
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/

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

[Jerone Young]

Fixes issues with pit for PPC.

Acked-by: Jerone Young <jyoung5@us.ibm.com>

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 <aliguori@us.ibm.com>
> 
> 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 <<EOF
> +#include <libkvm.h>
> +#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 0000000..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 (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_state = sc->read_state;
> +	c->write_state = sc->write_state;
> +	c->write_latch = sc->write_latch;
> +	c->rw_mode = sc->rw_mode;
> +	c->mode = sc->mode;
> +	c->bcd = sc->bcd;
> +	c->gate = sc->gate;
> +	c->count_load_time = sc->count_load_time;
> +    }
> +
> +    kvm_set_pit(kvm_context, &pit);
> +
> +    return 0;
> +}
> +
> +PITState *kvm_pit_init(int base, qemu_irq irq)
> +{
> +    PITState *pit = &pit_state;
> +
> +    register_savevm(PIT_SAVEVM_NAME, base, PIT_SAVEVM_VERSION,
> +		    kvm_pit_save, kvm_pit_load, pit);
> +
> +    qemu_register_reset(pit_reset, pit);
> +    pit_reset(pit);
> +
> +    return pit;
> +}
> diff --git a/qemu/hw/i8254.c b/qemu/hw/i8254.c
> index e215f8b..611c5f4 100644
> --- a/qemu/hw/i8254.c
> +++ b/qemu/hw/i8254.c
> @@ -22,43 +22,12 @@
>   * THE SOFTWARE.
>   */
>  #include "hw.h"
> -#include "pc.h"
>  #include "isa.h"
>  #include "qemu-timer.h"
> -
> -#include "qemu-kvm.h"
> +#include "i8254.h"
> 
>  //#define DEBUG_PIT
> 
> -#define RW_STATE_LSB 1
> -#define RW_STATE_MSB 2
> -#define RW_STATE_WORD0 3
> -#define RW_STATE_WORD1 4
> -
> -typedef struct PITChannelState {
> -    int count; /* can be 65536 */
> -    uint16_t latched_count;
> -    uint8_t count_latched;
> -    uint8_t status_latched;
> -    uint8_t status;
> -    uint8_t read_state;
> -    uint8_t write_state;
> -    uint8_t write_latch;
> -    uint8_t rw_mode;
> -    uint8_t mode;
> -    uint8_t bcd; /* not supported */
> -    uint8_t gate; /* timer start */
> -    int64_t count_load_time;
> -    /* irq handling */
> -    int64_t next_transition_time;
> -    QEMUTimer *irq_timer;
> -    qemu_irq irq;
> -} PITChannelState;
> -
> -struct PITState {
> -    PITChannelState channels[3];
> -};
> -
>  static PITState pit_state;
> 
>  static void pit_irq_timer_update(PITChannelState *s, int64_t current_time);
> @@ -414,78 +383,12 @@ static void pit_irq_timer(void *opaque)
>      pit_irq_timer_update(s, s->next_transition_time);
>  }
> 
> -#ifdef KVM_CAP_PIT
> -
> -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_state = sc->read_state;
> -	c->write_state = sc->write_state;
> -	c->write_latch = sc->write_latch;
> -	c->rw_mode = sc->rw_mode;
> -	c->mode = sc->mode;
> -	c->bcd = sc->bcd;
> -	c->gate = sc->gate;
> -	c->count_load_time = sc->count_load_time;
> -    }
> -
> -    kvm_set_pit(kvm_context, &pit);
> -}
> -
> -#endif
> -
> -static void pit_save(QEMUFile *f, void *opaque)
> +void pit_save(QEMUFile *f, void *opaque)
>  {
>      PITState *pit = opaque;
>      PITChannelState *s;
>      int i;
> 
> -#ifdef KVM_CAP_PIT
> -    if (kvm_enabled() && qemu_kvm_pit_in_kernel()) {
> -        kvm_kernel_pit_save_to_user(pit);
> -    }
> -#endif
> -
>      for(i = 0; i < 3; i++) {
>          s = &pit->channels[i];
>          qemu_put_be32(f, s->count);
> @@ -508,7 +411,7 @@ static void pit_save(QEMUFile *f, void *opaque)
>      }
>  }
> 
> -static int pit_load(QEMUFile *f, void *opaque, int version_id)
> +int pit_load(QEMUFile *f, void *opaque, int version_id)
>  {
>      PITState *pit = opaque;
>      PITChannelState *s;
> @@ -537,17 +440,10 @@ static int pit_load(QEMUFile *f, void *opaque, int version_id)
>              qemu_get_timer(f, s->irq_timer);
>          }
>      }
> -
> -#ifdef KVM_CAP_PIT
> -    if (kvm_enabled() && qemu_kvm_pit_in_kernel()) {
> -        kvm_kernel_pit_load_from_user(pit);
> -    }
> -#endif
> -
>      return 0;
>  }
> 
> -static void pit_reset(void *opaque)
> +void pit_reset(void *opaque)
>  {
>      PITState *pit = opaque;
>      PITChannelState *s;
> @@ -566,14 +462,13 @@ PITState *pit_init(int base, qemu_irq irq)
>      PITState *pit = &pit_state;
>      PITChannelState *s;
> 
> -    if (!kvm_enabled() || !qemu_kvm_pit_in_kernel()) {
> -	    s = &pit->channels[0];
> -	    /* the timer 0 is connected to an IRQ */
> -	    s->irq_timer = qemu_new_timer(vm_clock, pit_irq_timer, s);
> -	    s->irq = irq;
> -    }
> +    s = &pit->channels[0];
> +    /* the timer 0 is connected to an IRQ */
> +    s->irq_timer = qemu_new_timer(vm_clock, pit_irq_timer, s);
> +    s->irq = irq;
> 
> -    register_savevm("i8254", base, 1, pit_save, pit_load, pit);
> +    register_savevm(PIT_SAVEVM_NAME, base, PIT_SAVEVM_VERSION,
> +		    pit_save, pit_load, pit);
> 
>      qemu_register_reset(pit_reset, pit);
>      register_ioport_write(base, 4, 1, pit_ioport_write, pit);
> diff --git a/qemu/hw/i8254.h b/qemu/hw/i8254.h
> new file mode 100644
> index 0000000..5f44abd
> --- /dev/null
> +++ b/qemu/hw/i8254.h
> @@ -0,0 +1,46 @@
> +#ifndef _QEMU_I8254_H
> +#define _QEMU_I8254_H
> +
> +#include "hw.h"
> +#include "pc.h"
> +#include "qemu-timer.h"
> +
> +#define RW_STATE_LSB 1
> +#define RW_STATE_MSB 2
> +#define RW_STATE_WORD0 3
> +#define RW_STATE_WORD1 4
> +
> +typedef struct PITChannelState {
> +    int count; /* can be 65536 */
> +    uint16_t latched_count;
> +    uint8_t count_latched;
> +    uint8_t status_latched;
> +    uint8_t status;
> +    uint8_t read_state;
> +    uint8_t write_state;
> +    uint8_t write_latch;
> +    uint8_t rw_mode;
> +    uint8_t mode;
> +    uint8_t bcd; /* not supported */
> +    uint8_t gate; /* timer start */
> +    int64_t count_load_time;
> +    /* irq handling */
> +    int64_t next_transition_time;
> +    QEMUTimer *irq_timer;
> +    qemu_irq irq;
> +} PITChannelState;
> +
> +struct PITState {
> +    PITChannelState channels[3];
> +};
> +
> +#define PIT_SAVEVM_NAME		"i8254"
> +#define PIT_SAVEVM_VERSION	1
> +
> +void pit_save(QEMUFile *f, void *opaque);
> +
> +int pit_load(QEMUFile *f, void *opaque, int version_id);
> +
> +void pit_reset(void *opaque);
> +
> +#endif
> diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c
> index 0d2e6c3..97b108a 100644
> --- a/qemu/hw/pc.c
> +++ b/qemu/hw/pc.c
> @@ -983,7 +983,10 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>      if (pci_enabled) {
>          ioapic = ioapic_init();
>      }
> -    pit = pit_init(0x40, i8259[0]);
> +    if (kvm_enabled() && qemu_kvm_pit_in_kernel())
> +	pit = kvm_pit_init(0x40, i8259[0]);
> +    else
> +	pit = pit_init(0x40, i8259[0]);
>      pcspk_init(pit);
>      if (pci_enabled) {
>          pic_set_alt_irq_func(isa_pic, ioapic_set_irq, ioapic);
> diff --git a/qemu/hw/pc.h b/qemu/hw/pc.h
> index 453c641..fb6c07d 100644
> --- a/qemu/hw/pc.h
> +++ b/qemu/hw/pc.h
> @@ -58,6 +58,10 @@ int pit_get_initial_count(PITState *pit, int channel);
>  int pit_get_mode(PITState *pit, int channel);
>  int pit_get_out(PITState *pit, int channel, int64_t current_time);
> 
> +/* i8254-kvm.c */
> +
> +PITState *kvm_pit_init(int base, qemu_irq irq);
> +
>  /* vmport.c */
>  void vmport_init(CPUState *env);
>  void vmport_register(unsigned char command, IOPortReadFunc *func, void *opaque);
> 
> -------------------------------------------------------------------------
> 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


-------------------------------------------------------------------------
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/

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

[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/vse0120000070mrt/direct/01/

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

[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

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

[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